Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 'isof' and 'cast' unquoted type params issue #3117

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1f406c8
For isof and cast, unable to cast type parameters should not throw ex…
WanjohiSammy Nov 12, 2024
981b428
Rename to TryBindDottedIdentifierForIsOfOrCastFunctionCall
WanjohiSammy Nov 12, 2024
a460cbf
Added tests
WanjohiSammy Nov 12, 2024
bab5e32
Remove unnecessary declarations
WanjohiSammy Nov 12, 2024
df8bfe6
Refactor
WanjohiSammy Nov 12, 2024
4d38f5f
Use 'ToList()' instead of 'new List(..)'
WanjohiSammy Nov 13, 2024
8483730
Adding more tests
WanjohiSammy Nov 15, 2024
26eb9ad
Fix the issue with case insensitive by not set the NextToken for prim…
WanjohiSammy Nov 17, 2024
9a76fac
Rename tests correctly
WanjohiSammy Nov 18, 2024
fb4b6b1
Added a few E2E tests
WanjohiSammy Nov 18, 2024
013a49e
Refactor test
WanjohiSammy Nov 18, 2024
e7cd279
Direct check for functioncall cast and isof
WanjohiSammy Nov 18, 2024
bdf86ef
Added more E2E tests
WanjohiSammy Nov 19, 2024
aa5963b
For isof and cast, unable to cast type parameters should not throw ex…
WanjohiSammy Nov 12, 2024
f895c8c
Rename to TryBindDottedIdentifierForIsOfOrCastFunctionCall
WanjohiSammy Nov 12, 2024
6fc5b4b
Added tests
WanjohiSammy Nov 12, 2024
1298f29
Remove unnecessary declarations
WanjohiSammy Nov 12, 2024
b9456c6
Refactor
WanjohiSammy Nov 12, 2024
ac72823
Use 'ToList()' instead of 'new List(..)'
WanjohiSammy Nov 13, 2024
949e08b
Adding more tests
WanjohiSammy Nov 15, 2024
a05ec58
Fix the issue with case insensitive by not set the NextToken for prim…
WanjohiSammy Nov 17, 2024
2513f89
Rename tests correctly
WanjohiSammy Nov 18, 2024
11a4fd0
Added a few E2E tests
WanjohiSammy Nov 18, 2024
8ee5c3f
Refactor test
WanjohiSammy Nov 18, 2024
a46b41f
Direct check for functioncall cast and isof
WanjohiSammy Nov 18, 2024
f2c4d60
Added more E2E tests
WanjohiSammy Nov 19, 2024
44f9896
Add support for unquoted type parameters in isof and cast function call
WanjohiSammy Nov 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 77 additions & 1 deletion src/Microsoft.OData.Core/UriParser/Binders/FunctionCallBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,39 @@ internal QueryNode BindFunctionCall(FunctionCallToken functionCallToken)

// If there isn't, bind as Uri function
// Bind all arguments
List<QueryNode> argumentNodes = new List<QueryNode>(functionCallToken.Arguments.Select(ar => this.bindMethod(ar)));
// Initialize a stack to keep track of previous query tokens
Stack<QueryToken> previousQueryTokens = new Stack<QueryToken>();
List<QueryNode> argumentNodes = functionCallToken.Arguments.Select(argument =>
{
// If the function is IsOf or Cast and the argument is a dotted identifier, we need to bind it differently
if (UnboundFunctionNames.Contains(functionCallToken.Name))
{
if(argument.ValueToken is DottedIdentifierToken dottedIdentifier)
{
// Pop the previous query token if available
QueryToken previousArgument = previousQueryTokens.Count > 0 ? previousQueryTokens.Pop() : null;

IEdmSchemaType dottedIdentifierType = UriEdmHelpers.FindTypeFromModel(state.Model, dottedIdentifier.Identifier, this.Resolver);

// If cast or isof has 2 arguments, then the first argument is the next token of the second argument
// If the parent is not a primitive type, set the NextToken of the current dotted identifier to the first argument
if (previousArgument != null && dottedIdentifierType is not IEdmPrimitiveType)
{
// Push the current argument onto the stack to keep track of the previous argument
dottedIdentifier.NextToken = previousArgument;
}

return this.TryBindDottedIdentifierForIsOfOrCastFunctionCall(dottedIdentifier, dottedIdentifierType);
}

// first we need to set the parent of the next argument as the current argument.
// isof and cast can have 1 or 2 arguments, so we need to keep track of the previous argument
previousQueryTokens.Push(argument);
}

return this.bindMethod(argument);
}).ToList();

return BindAsUriFunction(functionCallToken, argumentNodes);
}

Expand Down Expand Up @@ -426,6 +458,50 @@ private bool TryBindIdentifier(string identifier, IEnumerable<FunctionParameterT
return true;
}

/// <summary>
/// Binds a <see cref="DottedIdentifierToken"/> for the 'isof' and 'cast' function calls.
/// </summary>
/// <param name="dottedIdentifierToken">The dotted identifier token to bind.</param>
/// <param name="childType">The child type to bind to.</param>
/// <returns>A <see cref="QueryNode"/> representing the bound single resource node.</returns>
/// <exception cref="ODataException">Thrown when the token cannot be bound as a single resource node.</exception>
private QueryNode TryBindDottedIdentifierForIsOfOrCastFunctionCall(DottedIdentifierToken dottedIdentifierToken, IEdmSchemaType childType)
{
QueryNode parent = null;
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
IEdmType parentType = null;

if (state.ImplicitRangeVariable != null)
{
if (dottedIdentifierToken.NextToken == null)
{
parent = NodeFactory.CreateRangeVariableReferenceNode(state.ImplicitRangeVariable);
parentType = state.ImplicitRangeVariable.TypeReference.Definition;
}
else
{
parent = this.bindMethod(dottedIdentifierToken.NextToken);
parentType = parent.GetEdmType();
}
}

if (childType is not IEdmStructuredType childStructuredType)
{
return this.bindMethod(dottedIdentifierToken);
}

if (parentType != null)
{
this.state.ParsedSegments.Add(new TypeSegment(childType, parentType, null));
}

if (parent is CollectionResourceNode parentAsCollection)
{
return new CollectionResourceCastNode(parentAsCollection, childStructuredType);
}

return new SingleResourceCastNode(parent as SingleResourceNode, childStructuredType);
}

/// <summary>
/// Bind path segment's operation or operationImport's parameters.
/// </summary>
Expand Down
56 changes: 1 addition & 55 deletions src/Microsoft.OData.Core/UriParser/Parsers/FunctionCallParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ internal sealed class FunctionCallParser : IFunctionCallParser
/// </summary>
private readonly bool restoreStateIfFail;

/// <summary>
/// If the function call is cast or isof, set to true.
/// </summary>
private bool isFunctionCallNameCastOrIsOf = false;

/// <summary>
/// Create a new FunctionCallParser.
/// </summary>
Expand Down Expand Up @@ -107,9 +102,6 @@ public bool TryParseIdentifierAsFunction(QueryToken parent, out QueryToken resul
this.Lexer.NextToken();
}

// This is used to set the parent of the next argument to the current argument for cast and isof functions.
isFunctionCallNameCastOrIsOf = functionName.SequenceEqual(ExpressionConstants.UnboundFunctionCast.AsSpan()) || functionName.SequenceEqual(ExpressionConstants.UnboundFunctionIsOf.AsSpan());

FunctionParameterToken[] arguments = this.ParseArgumentListOrEntityKeyList(() => lexer.RestorePosition(position));
if (arguments != null)
{
Expand Down Expand Up @@ -188,32 +180,16 @@ public FunctionParameterToken[] ParseArguments()
/// <returns>A list of FunctionParameterTokens representing each argument</returns>
private List<FunctionParameterToken> ReadArgumentsAsPositionalValues()
{
// Store the parent expression of the current argument.
Stack<QueryToken> expressionParents = new Stack<QueryToken>();

List<FunctionParameterToken> argList = new List<FunctionParameterToken>();
while (true)
{
// If we have a parent expression, we need to set the parent of the next argument to the current argument.
QueryToken parentExpression = expressionParents.Count > 0 ? expressionParents.Pop() : null;
QueryToken parameterToken = this.parser.ParseExpression();

// If the function call is cast or isof, set the parent of the next argument to the current argument.
if (parentExpression != null && isFunctionCallNameCastOrIsOf)
{
parameterToken = SetParentForCurrentParameterToken(parentExpression, parameterToken);
}

argList.Add(new FunctionParameterToken(null, parameterToken));
argList.Add(new FunctionParameterToken(null, this.parser.ParseExpression()));
if (this.Lexer.CurrentToken.Kind != ExpressionTokenKind.Comma)
{
break;
}

// In case of comma, we need to parse the next argument
// but first we need to set the parent of the next argument to the current argument.
expressionParents.Push(parameterToken);

this.Lexer.NextToken();
}

Expand All @@ -239,35 +215,5 @@ private bool TryReadArgumentsAsNamedValues(out ICollection<FunctionParameterToke

return false;
}

/// <summary>
/// Set the parent of the next argument to the current argument.
/// For example, in the following query:
/// cast(Location, NS.HomeAddress) where Location is the parentExpression and NS.HomeAddress is the parameterToken.
/// isof(Location, NS.HomeAddress) where Location is the parentExpression and NS.HomeAddress is the parameterToken.
/// </summary>
/// <param name="parentExpression">The previous parameter token.</param>
/// <param name="parameterToken">The current parameter token.</param>
/// <returns>The updated parameter token.</returns>
private static QueryToken SetParentForCurrentParameterToken(QueryToken parentExpression, QueryToken parameterToken)
{
// If the parameter is a dotted identifier, we need to set the parent of the next argument to the current argument.
// But if it is primitive literal, no need to set the parent.
if (parameterToken is DottedIdentifierToken dottedIdentifierToken && dottedIdentifierToken.NextToken == null)
{
// Check if the dottedIdentifier is a primitive type
EdmPrimitiveTypeKind primitiveTypeKind = EdmCoreModel.Instance.GetPrimitiveTypeKind(dottedIdentifierToken.Identifier);

// If the dottedIdentifier is not a primitive type, set the parent of the next argument to the current argument.
// cast(1, Edm.Int32) -> Edm.Int32 is a dottedIdentifierToken but it is a primitive type
if (primitiveTypeKind == EdmPrimitiveTypeKind.None)
{
dottedIdentifierToken.NextToken = parentExpression;
parameterToken = dottedIdentifierToken;
}
}

return parameterToken;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -409,20 +409,24 @@ public void ParseFilterWithNullEnumValue()
convertNode.Source.ShouldBeConstantQueryNode((object)null);
}

[Fact]
public void ParseFilterCastMethod1()
[Theory]
[InlineData("cast(NS.Color'Green', 'Edm.String') eq 'blue'")]
[InlineData("cast(NS.Color'Green', Edm.String) eq 'blue'")]
public void ParseFilterCastMethod1(string filterQuery)
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
{
var filter = ParseFilter("cast(NS.Color'Green', 'Edm.String') eq 'blue'", this.userModel, this.entityType, this.entitySet);
var filter = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet);
var bon = filter.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.Equal);
var convertNode = Assert.IsType<ConvertNode>(bon.Left);
var functionCallNode = Assert.IsType<SingleValueFunctionCallNode>(convertNode.Source);
Assert.Equal("cast", functionCallNode.Name); // ConvertNode is because cast() result's nullable=false.
}

[Fact]
public void ParseFilterCastMethod2()
[Theory]
[InlineData("cast('Green', 'NS.Color') eq NS.Color'Green'")]
[InlineData("cast('Green', NS.Color) eq NS.Color'Green'")]
public void ParseFilterCastMethod2(string filterQuery)
{
var filter = ParseFilter("cast('Green', 'NS.Color') eq NS.Color'Green'", this.userModel, this.entityType, this.entitySet);
var filter = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet);
var bon = filter.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.Equal);
var functionCallNode = Assert.IsType<SingleValueFunctionCallNode>(bon.Left);
Assert.Equal("cast", functionCallNode.Name);
Expand Down Expand Up @@ -499,17 +503,21 @@ public void ParseFilterEnumMemberUndefined4()
parse.Throws<ODataException>(Strings.Binder_IsNotValidEnumConstant("NS.ColorFlags'Red,2'"));
}

[Fact]
public void ParseFilterEnumTypesWrongCast1()
[Theory]
[InlineData("cast(NS.ColorFlags'Green', 'Edm.Int64') eq 2")]
[InlineData("cast(NS.ColorFlags'Green', Edm.Int64) eq 2")]
public void ParseFilterEnumTypesWrongCast1(string filter)
{
Action parse = () => ParseFilter("cast(NS.ColorFlags'Green', 'Edm.Int64') eq 2", this.userModel, this.entityType, this.entitySet);
Action parse = () => ParseFilter(filter, this.userModel, this.entityType, this.entitySet);
parse.Throws<ODataException>(Strings.CastBinder_EnumOnlyCastToOrFromString);
}

[Fact]
public void ParseFilterEnumTypesWrongCast2()
[Theory]
[InlineData("cast(321, 'NS.ColorFlags') eq 2")]
[InlineData("cast(321, NS.ColorFlags) eq 2")]
public void ParseFilterEnumTypesWrongCast2(string filter)
{
Action parse = () => ParseFilter("cast(321, 'NS.ColorFlags') eq 2", this.userModel, this.entityType, this.entitySet);
Action parse = () => ParseFilter(filter, this.userModel, this.entityType, this.entitySet);
parse.Throws<ODataException>(Strings.CastBinder_EnumOnlyCastToOrFromString);
}

Expand Down
Loading