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 all 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
88 changes: 83 additions & 5 deletions src/Microsoft.OData.Core/UriParser/Binders/FunctionCallBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,38 @@ internal QueryNode BindFunctionCall(FunctionCallToken functionCallToken)
}

// If there isn't, bind as Uri function
// Initialize a stack to keep track of previous query tokens
Stack<QueryToken> previousQueryTokens = new Stack<QueryToken>();

// Bind all arguments
List<QueryNode> argumentNodes = new List<QueryNode>(functionCallToken.Arguments.Select(ar => this.bindMethod(ar)));
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
string functionCallTokenName = IsUnboundFunction(functionCallToken.Name);
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
if ((ExpressionConstants.UnboundFunctionIsOf == functionCallTokenName || ExpressionConstants.UnboundFunctionCast == functionCallTokenName) && argument.ValueToken is DottedIdentifierToken dottedIdentifier)
{
// Pop the previous query token if available
QueryToken previousArgument = previousQueryTokens.Count > 0 ? previousQueryTokens.Pop() : null;

// Find the type of the dotted identifier by resolving it against the model. This also ensure case-insensitive resolution.
IEdmSchemaType dottedIdentifierType = UriEdmHelpers.FindTypeFromModel(state.Model, dottedIdentifier.Identifier, this.Resolver);

// If the dotted identifier is not a primitive type, set the next token to the previous argument
if (dottedIdentifierType is not IEdmPrimitiveType && previousArgument != null)
{
// Set the next token of the dotted identifier to the previous argument
dottedIdentifier.NextToken = previousArgument;
}

// isof and cast can have 1 or 2 arguments, so we need to keep track of the previous argument
previousQueryTokens.Push(argument);

return this.TryBindDottedIdentifierForIsOfOrCastFunctionCall(dottedIdentifier, dottedIdentifierType);
}

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

return BindAsUriFunction(functionCallToken, argumentNodes);
}

Expand Down Expand Up @@ -426,6 +456,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 Expand Up @@ -720,12 +794,16 @@ private static IEdmTypeReference ValidateIsOfOrCast(BindingState state, bool isC
ODataErrorStrings.MetadataBinder_CastOrIsOfExpressionWithWrongNumberOfOperands(args.Count));
}

ConstantNode typeArgument = args.Last() as ConstantNode;
QueryNode queryNode = args.Last();

IEdmTypeReference returnType = null;
if (typeArgument != null)
if (queryNode is SingleResourceCastNode singleResourceCastNode)
{
returnType = singleResourceCastNode.TypeReference;
}
else if (queryNode is ConstantNode constantNode)
{
returnType = TryGetTypeReference(state.Model, typeArgument.Value as string, state.Configuration.Resolver);
returnType = TryGetTypeReference(state.Model, constantNode.Value as string, state.Configuration.Resolver);
}

if (returnType == null)
Expand Down Expand Up @@ -758,7 +836,7 @@ private static IEdmTypeReference ValidateIsOfOrCast(BindingState state, bool isC
{
// throw if cast enum to not-string :
if ((args[0].GetEdmTypeReference() is IEdmEnumTypeReference)
&& !string.Equals(typeArgument.Value as string, Microsoft.OData.Metadata.EdmConstants.EdmStringTypeName, StringComparison.Ordinal))
&& !string.Equals(returnType.FullName(), Microsoft.OData.Metadata.EdmConstants.EdmStringTypeName, StringComparison.Ordinal))
{
throw new ODataException(ODataErrorStrings.CastBinder_EnumOnlyCastToOrFromString);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,20 +409,26 @@ 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'")]
[InlineData("cast(NS.Color'Green', edm.string) eq 'blue'")]
[InlineData("cast(NS.Color'Green', EDM.STRING) eq 'blue'")]
public void ParseFilterCastMethodWithEdmPrimitiveTypes(string filterQuery)
{
var filter = ParseFilter("cast(NS.Color'Green', 'Edm.String') eq 'blue'", this.userModel, this.entityType, this.entitySet);
var filter = ParseFilter(filterQuery, true, 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 ParseFilterCastMethodWithOrWithoutSingleQuotesOnType(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 +505,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 Expand Up @@ -650,5 +660,11 @@ private FilterClause ParseFilter(string text, IEdmModel edmModel, IEdmEntityType
{
return new ODataQueryOptionParser(edmModel, entityType, edmEntitySet, new Dictionary<string, string>() { { "$filter", text } }).ParseFilter();
}

private FilterClause ParseFilter(string text, bool caseInsensitive, IEdmModel edmModel, IEdmEntityType edmEntityType, IEdmEntitySet edmEntitySet)
{
return new ODataQueryOptionParser(edmModel, entityType, edmEntitySet, new Dictionary<string, string>() { { "$filter", text } })
{ Resolver = new ODataUriResolver() { EnableCaseInsensitive = caseInsensitive } }.ParseFilter();
}
}
}
Loading
Loading