From a1014614be49d192708b78a7dcb5b1ced975ce09 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Mon, 22 Jul 2024 14:04:27 +0300 Subject: [PATCH 01/13] Refactor InBinder to convert left operand to enum type if necessary --- src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs index 953576c020..5c4e1760ae 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs @@ -58,6 +58,13 @@ internal QueryNode BindInOperator(InToken inToken, BindingState state) inToken.Right, new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetUntyped())), state.Model); } + // If the left operand is either an integral or a string type and the right operand is a collection of enums, + // Calls the MetadataBindingUtils.ConvertToTypeIfNeeded() method to convert the left operand to the same enum type as the right operand. + if ((right is CollectionPropertyAccessNode && right.ItemType.IsEnum()) && (left.TypeReference.IsString() || left.TypeReference.IsIntegral())) + { + left = MetadataBindingUtils.ConvertToTypeIfNeeded(left, right.ItemType); + } + return new InNode(left, right); } From cdc0cb1dd0c4a92608062d9d1c01bd70da1f631e Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Mon, 22 Jul 2024 14:28:25 +0300 Subject: [PATCH 02/13] Add tests for filter with In Operator with enums member names and member integral values --- .../FilterAndOrderByFunctionalTests.cs | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index bf0e28666f..774127bee4 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -2041,6 +2041,91 @@ public void FilterWithInOperationWithEnums() Assert.Equal("FavoriteColors", Assert.IsType(inNode.Right).Property.Name); } + [Fact] + public void FilterWithInOperationWithEnumsMemberName_WithoutQualifiedNamespace() + { + // Arrange + string filterQuery = "'SolidYellow' in FavoriteColors"; + + string expectedLiteralText = "'SolidYellow'"; + string expectedfullyQualifiedName = "Fully.Qualified.Namespace.ColorPattern'SolidYellow'"; + string expectedCollectionPropertyName = "FavoriteColors"; + + // Act + FilterClause filter = ParseFilter(filterQuery, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); + + // Assert + InNode inNode = Assert.IsType(filter.Expression); + ConstantNode left = Assert.IsType(inNode.Left); + ODataEnumValue enumValue = Assert.IsType(left.Value); + + Assert.Equal(expectedLiteralText, left.LiteralText); + Assert.Equal(expectedfullyQualifiedName, (enumValue.TypeName + left.LiteralText)); + Assert.Equal(expectedCollectionPropertyName, Assert.IsType(inNode.Right).Property.Name); + } + + [Fact] + public void FilterWithInOperationWithEnumsMemberIntegralValue_WithSingleQuotes() + { + // Arrange + string filterQuery = "'12' in FavoriteColors"; + + string expectedLiteralText = "'SolidYellow'"; + string expectedfullyQualifiedName = "Fully.Qualified.Namespace.ColorPattern'SolidYellow'"; + string expectedCollectionPropertyName = "FavoriteColors"; + + // Act + FilterClause filter = ParseFilter(filterQuery, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); + + // Assert + InNode inNode = Assert.IsType(filter.Expression); + ConstantNode left = Assert.IsType(inNode.Left); + ODataEnumValue enumValue = Assert.IsType(left.Value); + + Assert.Equal(expectedLiteralText, left.LiteralText); + Assert.Equal(expectedfullyQualifiedName, (enumValue.TypeName + left.LiteralText)); + Assert.Equal(expectedCollectionPropertyName, Assert.IsType(inNode.Right).Property.Name); + } + + [Fact] + public void FilterWithInOperationWithEnumsMemberIntegralValue_WithoutSingleQuotes() + { + // Arrange + string filterQuery = "12 in FavoriteColors"; + + string expectedLiteralText = "'SolidYellow'"; + string expectedfullyQualifiedName = "Fully.Qualified.Namespace.ColorPattern'SolidYellow'"; + string expectedCollectionPropertyName = "FavoriteColors"; + + // Act + FilterClause filter = ParseFilter(filterQuery, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); + + // Assert + InNode inNode = Assert.IsType(filter.Expression); + ConstantNode left = Assert.IsType(inNode.Left); + ODataEnumValue enumValue = Assert.IsType(left.Value); + + Assert.Equal(expectedLiteralText, left.LiteralText); + Assert.Equal(expectedfullyQualifiedName, (enumValue.TypeName + left.LiteralText)); + Assert.Equal(expectedCollectionPropertyName, Assert.IsType(inNode.Right).Property.Name); + } + + [Theory] + [InlineData(53)] + [InlineData("53")] + [InlineData("'53'")] + public void FilterWithInOperationWithEnumsInValidMemberIntegralValue_ThrowsIsNotValidEnumConstantException(object integralValue) + { + // Arrange + string filterQuery = $"{integralValue} in FavoriteColors"; + + // Act + Action action = () => ParseFilter(filterQuery, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); + + // Assert + action.Throws(ODataErrorStrings.Binder_IsNotValidEnumConstant("53")); + } + [Fact] public void FilterWithInOperationWithAny() { From 2eb3a56a337e1a87a4a58b33f44c7e99c56a2133 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Tue, 23 Jul 2024 18:18:28 +0300 Subject: [PATCH 03/13] Add more functional tests for parseFilter with 'In' Operation with Enums --- .../UriParser/EnumFilterFunctionalTests.cs | 143 +++++++++++++++++- 1 file changed, 138 insertions(+), 5 deletions(-) diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs index d8a39ba1f5..3c60c1531a 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs @@ -10,6 +10,7 @@ using Microsoft.OData.UriParser; using Microsoft.OData.Edm; using Xunit; +using System.Linq; namespace Microsoft.OData.Tests.ScenarioTests.UriParser { @@ -41,8 +42,8 @@ public EnumFilterFunctionalTests() // set up the edm model etc this.userModel = new EdmModel(); - var enumType = new EdmEnumType("NS", "Color", EdmPrimitiveTypeKind.Int32, false); - var red = new EdmEnumMember(enumType, "Red", new EdmEnumMemberValue(1)); + EdmEnumType enumType = new EdmEnumType("NS", "Color", EdmPrimitiveTypeKind.Int32, false); + EdmEnumMember red = new EdmEnumMember(enumType, "Red", new EdmEnumMemberValue(1)); enumType.AddMember(red); enumType.AddMember("Green", new EdmEnumMemberValue(2)); enumType.AddMember("Blue", new EdmEnumMemberValue(3)); @@ -53,11 +54,11 @@ public EnumFilterFunctionalTests() // add enum property this.entityType = new EdmEntityType("NS", "MyEntityType", isAbstract: false, isOpen: true, baseType: null); - var enumTypeReference = new EdmEnumTypeReference(enumType, true); + EdmEnumTypeReference enumTypeReference = new EdmEnumTypeReference(enumType, true); this.entityType.AddProperty(new EdmStructuralProperty(this.entityType, "Color", enumTypeReference)); // enum with flags - var enumFlagsType = new EdmEnumType("NS", "ColorFlags", EdmPrimitiveTypeKind.Int64, true); + EdmEnumType enumFlagsType = new EdmEnumType("NS", "ColorFlags", EdmPrimitiveTypeKind.Int64, true); enumFlagsType.AddMember("Red", new EdmEnumMemberValue(1L)); enumFlagsType.AddMember("Green", new EdmEnumMemberValue(2L)); enumFlagsType.AddMember("Blue", new EdmEnumMemberValue(4L)); @@ -67,9 +68,13 @@ public EnumFilterFunctionalTests() this.userModel.AddElement(enumFlagsType); // add enum with flags - var enumFlagsTypeReference = new EdmEnumTypeReference(enumFlagsType, true); + EdmEnumTypeReference enumFlagsTypeReference = new EdmEnumTypeReference(enumFlagsType, true); this.entityType.AddProperty(new EdmStructuralProperty(this.entityType, "ColorFlags", enumFlagsTypeReference)); + // add colors collection + EdmCollectionTypeReference colorCollectionTypeRef = new EdmCollectionTypeReference(new EdmCollectionType(new EdmEnumTypeReference(enumType, true))); + this.entityType.AddProperty(new EdmStructuralProperty(this.entityType, "Colors", colorCollectionTypeRef)); + this.userModel.AddElement(this.entityType); var defaultContainer = new EdmEntityContainer("NS", "DefaultContainer"); @@ -515,6 +520,127 @@ public void ParseFilterEnumTypesWrongCast3() parse.Throws(Strings.MetadataBinder_CastOrIsOfFunctionWithoutATypeArgument); } + [Fact] + public void ParseFilterWithInOperatorWithEnumsFullyQualifiedMemberName() + { + // Arrange + string enumValue = "White"; + string filterQuery = $"NS.Color'{enumValue}' in Colors"; // "NS.Color'White' in Colors" + string expectedLiteral = "NS.Color'White'"; + + IEdmEnumType colorType = this.GetColorType(this.userModel); + IEdmEnumMember enumMember = colorType.Members.Single(m => m.Name == enumValue); + + // Act + FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); + SingleValueNode expression = filterQueryNode.Expression; + + // Assert + Assert.Equal("NS.Color'White' in Colors", filterQuery); + InNode inNode = expression.ShouldBeInNode(); + ConstantNode leftNode = Assert.IsType(inNode.Left); + leftNode.ShouldBeEnumNode(colorType, enumMember.Value.Value); + + Assert.True(leftNode.TypeReference.IsEnum()); + Assert.Equal(enumMember.Value.Value.ToString(), leftNode.Value.ToString()); + Assert.Equal(expectedLiteral, leftNode.LiteralText); + + CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + Assert.Equal(colorType, rightNode.ItemType.Definition); + } + + [Fact] + public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutFullyQualifiedName() + { + // Arrange + string enumValue = "Green"; + string filterQuery = $"'{enumValue}' in Colors"; // "'Green' in Colors" + string expectedLiteral = "'Green'"; + + IEdmEnumType colorType = this.GetColorType(this.userModel); + + // Act + FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); + SingleValueNode expression = filterQueryNode.Expression; + + // Assert + Assert.Equal("'Green' in Colors", filterQuery); + InNode inNode = expression.ShouldBeInNode(); + ConstantNode leftNode = Assert.IsType(inNode.Left); + leftNode.ShouldBeEnumNode(colorType, enumValue); + + Assert.True(leftNode.TypeReference.IsEnum()); + Assert.Equal(enumValue, leftNode.Value.ToString()); + Assert.Equal(expectedLiteral, leftNode.LiteralText); + + CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + Assert.Equal(colorType, rightNode.ItemType.Definition); + } + + [Fact] + public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithSingleQuotes() + { + // Arrange + int enumValue = 1; + string filterQuery = $"'{enumValue}' in Colors"; // "'1' in Colors" + string expectedLiteral = "'Red'"; + + IEdmEnumType colorType = this.GetColorType(this.userModel); + bool success = colorType.TryParse(enumValue, out IEdmEnumMember enumMember); + + // Act + FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); + SingleValueNode expression = filterQueryNode.Expression; + + // Assert + Assert.Equal("'1' in Colors", filterQuery); + Assert.True(success); + InNode inNode = expression.ShouldBeInNode(); + ConstantNode leftNode = Assert.IsType(inNode.Left); + leftNode.ShouldBeEnumNode(colorType, enumMember.Name); + + Assert.True(leftNode.TypeReference.IsEnum()); + Assert.Equal(enumMember.Name, leftNode.Value.ToString()); + Assert.Equal(expectedLiteral, leftNode.LiteralText); + + CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + Assert.Equal(colorType, rightNode.ItemType.Definition); + } + + [Fact] + public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithoutSingleQuotes() + { + // Arrange + int enumValue = 3; + string filterQuery = $"{enumValue} in Colors"; // "3 in Colors" + string expectedLiteral = "'Blue'"; + + IEdmEnumType colorType = this.GetColorType(this.userModel); + bool success = colorType.TryParse(enumValue, out IEdmEnumMember enumMember); + + // Act + FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); + SingleValueNode expression = filterQueryNode.Expression; + + // Assert + Assert.Equal("3 in Colors", filterQuery); + Assert.True(success); + InNode inNode = expression.ShouldBeInNode(); + ConstantNode leftNode = Assert.IsType(inNode.Left); + leftNode.ShouldBeEnumNode(colorType, enumMember.Name); + + Assert.True(leftNode.TypeReference.IsEnum()); + Assert.Equal(enumMember.Name, leftNode.Value.ToString()); + Assert.Equal(expectedLiteral, leftNode.LiteralText); + + CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + Assert.Equal(colorType, rightNode.ItemType.Definition); + } + private IEdmStructuralProperty GetColorProp(IEdmModel model) { return (IEdmStructuralProperty)((IEdmStructuredType)model @@ -522,6 +648,13 @@ private IEdmStructuralProperty GetColorProp(IEdmModel model) .FindProperty("Color"); } + private IEdmStructuralProperty GetColorsProperty(IEdmModel model) + { + return (IEdmStructuralProperty)((IEdmStructuredType)model + .FindType("NS.MyEntityType")) + .FindProperty("Colors"); + } + private IEdmEnumType GetColorType(IEdmModel model) { return (IEdmEnumType)model.FindType("NS.Color"); From 1ad2413072f92fdb56bcb239ec7d8117eb06a25c Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Thu, 25 Jul 2024 12:34:20 +0300 Subject: [PATCH 04/13] Add a failed tests when Float are used as enum integral value with In Operator --- .../UriParser/EnumFilterFunctionalTests.cs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs index 3c60c1531a..4506282181 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs @@ -641,6 +641,56 @@ public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithoutSingleQu Assert.Equal(colorType, rightNode.ItemType.Definition); } + [Fact] + public void ParseFilterWithInOperatorWithEnumsMemberDoubleIntegralValue() + { + // Arrange + long enumValue = 3; + string filterQuery = $"{enumValue} in Colors"; // "3 in Colors" + string expectedLiteral = "'Blue'"; + + IEdmEnumType colorType = this.GetColorType(this.userModel); + bool success = colorType.TryParse(enumValue, out IEdmEnumMember enumMember); + + // Act + FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); + SingleValueNode expression = filterQueryNode.Expression; + + // Assert + Assert.Equal("3 in Colors", filterQuery); + Assert.True(success); + InNode inNode = expression.ShouldBeInNode(); + ConstantNode leftNode = Assert.IsType(inNode.Left); + leftNode.ShouldBeEnumNode(colorType, enumMember.Name); + + Assert.True(leftNode.TypeReference.IsEnum()); + Assert.Equal(enumMember.Name, leftNode.Value.ToString()); + Assert.Equal(expectedLiteral, leftNode.LiteralText); + + CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + Assert.Equal(colorType, rightNode.ItemType.Definition); + } + + [Theory] + [InlineData("3.0")] + [InlineData("3.1")] + [InlineData("4.0")] + [InlineData("5.0")] + public void ParseFilterWithInOperatorWithEnumsMemberFloatIntegralValue(string floatString) + { + // Arrange + string filterQuery = $"{floatString} in Colors"; + + IEdmEnumType colorType = this.GetColorType(this.userModel); + + // Act + Action test = () => ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); + + // Assert + test.Throws(Strings.Nodes_InNode_CollectionItemTypeMustBeSameAsSingleItemType("NS.Color", "Edm.Single")); // Float are of Type Edm.Single + } + private IEdmStructuralProperty GetColorProp(IEdmModel model) { return (IEdmStructuralProperty)((IEdmStructuredType)model From 8f35954a31eca03904984cd172ade4b231b01eef Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Thu, 25 Jul 2024 15:56:28 +0300 Subject: [PATCH 05/13] Add parse filter test with 'In' operator with enums where left operand is Invalid integral values --- .../UriParser/EnumFilterFunctionalTests.cs | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs index 4506282181..d16176c61d 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs @@ -42,8 +42,8 @@ public EnumFilterFunctionalTests() // set up the edm model etc this.userModel = new EdmModel(); - EdmEnumType enumType = new EdmEnumType("NS", "Color", EdmPrimitiveTypeKind.Int32, false); - EdmEnumMember red = new EdmEnumMember(enumType, "Red", new EdmEnumMemberValue(1)); + var enumType = new EdmEnumType("NS", "Color", EdmPrimitiveTypeKind.Int32, false); + var red = new EdmEnumMember(enumType, "Red", new EdmEnumMemberValue(1)); enumType.AddMember(red); enumType.AddMember("Green", new EdmEnumMemberValue(2)); enumType.AddMember("Blue", new EdmEnumMemberValue(3)); @@ -54,11 +54,11 @@ public EnumFilterFunctionalTests() // add enum property this.entityType = new EdmEntityType("NS", "MyEntityType", isAbstract: false, isOpen: true, baseType: null); - EdmEnumTypeReference enumTypeReference = new EdmEnumTypeReference(enumType, true); + var enumTypeReference = new EdmEnumTypeReference(enumType, true); this.entityType.AddProperty(new EdmStructuralProperty(this.entityType, "Color", enumTypeReference)); // enum with flags - EdmEnumType enumFlagsType = new EdmEnumType("NS", "ColorFlags", EdmPrimitiveTypeKind.Int64, true); + var enumFlagsType = new EdmEnumType("NS", "ColorFlags", EdmPrimitiveTypeKind.Int64, true); enumFlagsType.AddMember("Red", new EdmEnumMemberValue(1L)); enumFlagsType.AddMember("Green", new EdmEnumMemberValue(2L)); enumFlagsType.AddMember("Blue", new EdmEnumMemberValue(4L)); @@ -68,7 +68,7 @@ public EnumFilterFunctionalTests() this.userModel.AddElement(enumFlagsType); // add enum with flags - EdmEnumTypeReference enumFlagsTypeReference = new EdmEnumTypeReference(enumFlagsType, true); + var enumFlagsTypeReference = new EdmEnumTypeReference(enumFlagsType, true); this.entityType.AddProperty(new EdmStructuralProperty(this.entityType, "ColorFlags", enumFlagsTypeReference)); // add colors collection @@ -682,8 +682,6 @@ public void ParseFilterWithInOperatorWithEnumsMemberFloatIntegralValue(string fl // Arrange string filterQuery = $"{floatString} in Colors"; - IEdmEnumType colorType = this.GetColorType(this.userModel); - // Act Action test = () => ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); @@ -691,6 +689,22 @@ public void ParseFilterWithInOperatorWithEnumsMemberFloatIntegralValue(string fl test.Throws(Strings.Nodes_InNode_CollectionItemTypeMustBeSameAsSingleItemType("NS.Color", "Edm.Single")); // Float are of Type Edm.Single } + [Theory] + [InlineData(-20)] + [InlineData("-20")] + [InlineData("'-20'")] + public void ParseFilterWithInOperatorWithEnumsMemberInvalidIntegralValue(object integralValue) + { + // Arrange + string filterQuery = $"{integralValue} in Colors"; + + // Act + Action action = () => ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); + + // Assert + action.Throws(Strings.Binder_IsNotValidEnumConstant("-20")); + } + private IEdmStructuralProperty GetColorProp(IEdmModel model) { return (IEdmStructuralProperty)((IEdmStructuredType)model From 2f7bda73ac8df0b1dd1a530fef89c29b1840cf4d Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Mon, 29 Jul 2024 14:35:33 +0300 Subject: [PATCH 06/13] Adding tests for errors thrown when using string literal that doesn't exists as enum member name --- .../UriParser/EnumFilterFunctionalTests.cs | 40 +++++++++++++++++-- .../FilterAndOrderByFunctionalTests.cs | 19 ++++++++- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs index d16176c61d..61e05474f8 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs @@ -5,12 +5,12 @@ //--------------------------------------------------------------------- using System; +using System.Linq; using System.Collections.Generic; using Microsoft.OData.Tests.UriParser; using Microsoft.OData.UriParser; using Microsoft.OData.Edm; using Xunit; -using System.Linq; namespace Microsoft.OData.Tests.ScenarioTests.UriParser { @@ -677,7 +677,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberDoubleIntegralValue() [InlineData("3.1")] [InlineData("4.0")] [InlineData("5.0")] - public void ParseFilterWithInOperatorWithEnumsMemberFloatIntegralValue(string floatString) + public void ParseFilterWithInOperatorWithEnumsMemberFloatIntegralValue_ThrowCollectionItemTypeMustBeSameAsSingleItemTypeException(string floatString) { // Arrange string filterQuery = $"{floatString} in Colors"; @@ -693,7 +693,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberFloatIntegralValue(string fl [InlineData(-20)] [InlineData("-20")] [InlineData("'-20'")] - public void ParseFilterWithInOperatorWithEnumsMemberInvalidIntegralValue(object integralValue) + public void ParseFilterWithInOperatorWithEnumsInvalidIntegralValues_ThrowsIsNotValidEnumConstantException(object integralValue) { // Arrange string filterQuery = $"{integralValue} in Colors"; @@ -705,6 +705,40 @@ public void ParseFilterWithInOperatorWithEnumsMemberInvalidIntegralValue(object action.Throws(Strings.Binder_IsNotValidEnumConstant("-20")); } + [Fact] + public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutSingleQuotes_ThrowsNullReferenceException() + { + // Arrange + string filterQuery = "Red in Colors"; + + string erroMessage = "Object reference not set to an instance of an object."; + + // Act + Action action = () => ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); + + // Assert + action.Throws(erroMessage); + } + + [Theory] + [InlineData("'Yellow'")] + [InlineData("'Teal'")] + [InlineData("NS.Color'Yellow'")] + [InlineData("NS.Color'Teal'")] + public void ParseFilterWithInOperatorWithEnumsInvalidMemberNames_ThrowsIsNotValidEnumConstantException(string memberName) + { + // Arrange + string filterQuery = $"{memberName} in Colors"; + + string expectedExceptionParameter = memberName.StartsWith("'") ? memberName.Trim('\'') : memberName; // Trim('\'') method removes any single quotes from the start and end of the string + + // Act + Action action = () => ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); + + // Assert + action.Throws(Strings.Binder_IsNotValidEnumConstant(expectedExceptionParameter)); + } + private IEdmStructuralProperty GetColorProp(IEdmModel model) { return (IEdmStructuralProperty)((IEdmStructuredType)model diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index 774127bee4..b5acd8c67f 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -2114,7 +2114,7 @@ public void FilterWithInOperationWithEnumsMemberIntegralValue_WithoutSingleQuote [InlineData(53)] [InlineData("53")] [InlineData("'53'")] - public void FilterWithInOperationWithEnumsInValidMemberIntegralValue_ThrowsIsNotValidEnumConstantException(object integralValue) + public void FilterWithInOperationWithEnumsInvalidMemberIntegralValue_ThrowsIsNotValidEnumConstantException(object integralValue) { // Arrange string filterQuery = $"{integralValue} in FavoriteColors"; @@ -2126,6 +2126,23 @@ public void FilterWithInOperationWithEnumsInValidMemberIntegralValue_ThrowsIsNot action.Throws(ODataErrorStrings.Binder_IsNotValidEnumConstant("53")); } + [Theory] + [InlineData("'Teal'")] + [InlineData("NS.Color'Teal'")] + public void FilterWithInOperationWithEnumsInvalidMemberNames_ThrowsIsNotValidEnumConstantException(string memberName) + { + // Arrange + string filterQuery = $"{memberName} in FavoriteColors"; + + string expectedExceptionParameter = memberName.StartsWith("'") ? memberName.Trim('\'') : memberName; // Trim('\'') method removes any single quotes from the start and end of the string + + // Act + Action action = () => ParseFilter(filterQuery, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); + + // Assert + action.Throws(ODataErrorStrings.Binder_IsNotValidEnumConstant(expectedExceptionParameter)); + } + [Fact] public void FilterWithInOperationWithAny() { From a2d1935975dee839563c51b93eed89a8df1858a0 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Tue, 30 Jul 2024 17:35:45 +0300 Subject: [PATCH 07/13] Refactor to reuse methods and move logic that check if convert left node is required to private method --- .../UriParser/Binders/InBinder.cs | 17 ++- .../UriParser/EnumFilterFunctionalTests.cs | 103 ++++++++---------- 2 files changed, 61 insertions(+), 59 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs index 5c4e1760ae..8c524c0f82 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs @@ -58,9 +58,7 @@ internal QueryNode BindInOperator(InToken inToken, BindingState state) inToken.Right, new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetUntyped())), state.Model); } - // If the left operand is either an integral or a string type and the right operand is a collection of enums, - // Calls the MetadataBindingUtils.ConvertToTypeIfNeeded() method to convert the left operand to the same enum type as the right operand. - if ((right is CollectionPropertyAccessNode && right.ItemType.IsEnum()) && (left.TypeReference.IsString() || left.TypeReference.IsIntegral())) + if (ShouldConvertLeftOperand(left, right)) { left = MetadataBindingUtils.ConvertToTypeIfNeeded(left, right.ItemType); } @@ -68,6 +66,19 @@ internal QueryNode BindInOperator(InToken inToken, BindingState state) return new InNode(left, right); } + /// + /// Checks if the left operand is either an integral or a string type and the right operand is a collection of enums. + /// + /// The left operand. + /// The right operand. + /// True if the condition is met, otherwise false. + private static bool ShouldConvertLeftOperand(SingleValueNode leftNode, CollectionNode rightNode) + { + // If the left operand is either an integral or a string type and the right operand is a collection of enums, + // Calls the MetadataBindingUtils.ConvertToTypeIfNeeded() method to convert the left operand to the same enum type as the right operand. + return (rightNode is CollectionPropertyAccessNode && rightNode.ItemType.IsEnum()) && (leftNode.TypeReference.IsString() || leftNode.TypeReference.IsIntegral()); + } + /// /// Retrieve SingleValueNode bound with given query token. /// diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs index 61e05474f8..69fb24fdd9 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs @@ -99,11 +99,11 @@ public void ParseFilterWithEnum() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorType(this.userModel), (int)Color.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.Color"), (int)Color.Green); } [Fact] @@ -121,11 +121,11 @@ public void ParseFilterWithEnumInt() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorType(this.userModel), (int)Color.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.Color"), (int)Color.Green); } [Fact] @@ -143,11 +143,11 @@ public void ParseFilterWithHasOperatorEnumMemberName() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorFlagsType(this.userModel), (int)ColorFlags.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.ColorFlags"), (int)ColorFlags.Green); } [Fact] @@ -165,11 +165,11 @@ public void ParseFilterWithHasOperatorEnumUnderlyingValue() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorFlagsType(this.userModel), (int)ColorFlags.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.ColorFlags"), (int)ColorFlags.Green); } [Fact] @@ -187,11 +187,11 @@ public void ParseFilterWithHasOperatorEnumLiteralValueAsLeftOperand() binaryNode .Left - .ShouldBeEnumNode(this.GetColorFlagsType(this.userModel), (int)(ColorFlags.Green | ColorFlags.Red)); + .ShouldBeEnumNode(this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); binaryNode .Right - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); } [Fact] @@ -209,11 +209,11 @@ public void ParseFilterWithHasOperatorNonFlagsEnum() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorType(this.userModel), (int)Color.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.Color"), (int)Color.Green); } [Fact] @@ -231,12 +231,12 @@ public void ParseFilterWithEnumNormalConbinedValues() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -255,12 +255,12 @@ public void ParseFilterWithEnumCombinedValuesOrderReversed() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -279,12 +279,12 @@ public void ParseFilterWithEnumValuesCompatibleWithString() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), "Red"); } @@ -303,12 +303,12 @@ public void ParseFilterWithEnumDefinedConbinedValues() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -327,12 +327,12 @@ public void ParseFilterWithEnumCombinedUnderlyingValues() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -351,12 +351,12 @@ public void ParseFilterWithEnumNegativeMember() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorType(this.userModel), + this.GetIEdmType("NS.Color"), (int)(Color.White)); var constantNode = Assert.IsType(binaryNode.Right); @@ -379,12 +379,12 @@ public void ParseFilterWithEnumUndefinedMember() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorType(this.userModel), + this.GetIEdmType("NS.Color"), -132534290); var constantNode = Assert.IsType(binaryNode.Right); @@ -403,7 +403,7 @@ public void ParseFilterWithNullEnumValue() { var filterQueryNode = ParseFilter("Color eq null", this.userModel, this.entityType, this.entitySet); var binaryNode = filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.Equal); - binaryNode.Left.ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); + binaryNode.Left.ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); var convertNode = Assert.IsType(binaryNode.Right); convertNode.Source.ShouldBeConstantQueryNode((object)null); @@ -528,7 +528,7 @@ public void ParseFilterWithInOperatorWithEnumsFullyQualifiedMemberName() string filterQuery = $"NS.Color'{enumValue}' in Colors"; // "NS.Color'White' in Colors" string expectedLiteral = "NS.Color'White'"; - IEdmEnumType colorType = this.GetColorType(this.userModel); + IEdmEnumType colorType = this.GetIEdmType("NS.Color"); IEdmEnumMember enumMember = colorType.Members.Single(m => m.Name == enumValue); // Act @@ -546,7 +546,7 @@ public void ParseFilterWithInOperatorWithEnumsFullyQualifiedMemberName() Assert.Equal(expectedLiteral, leftNode.LiteralText); CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); - rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); Assert.Equal(colorType, rightNode.ItemType.Definition); } @@ -558,7 +558,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutFullyQualifiedNam string filterQuery = $"'{enumValue}' in Colors"; // "'Green' in Colors" string expectedLiteral = "'Green'"; - IEdmEnumType colorType = this.GetColorType(this.userModel); + IEdmEnumType colorType = this.GetIEdmType("NS.Color"); // Act FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); @@ -575,7 +575,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutFullyQualifiedNam Assert.Equal(expectedLiteral, leftNode.LiteralText); CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); - rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); Assert.Equal(colorType, rightNode.ItemType.Definition); } @@ -587,7 +587,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithSingleQuote string filterQuery = $"'{enumValue}' in Colors"; // "'1' in Colors" string expectedLiteral = "'Red'"; - IEdmEnumType colorType = this.GetColorType(this.userModel); + IEdmEnumType colorType = this.GetIEdmType("NS.Color"); bool success = colorType.TryParse(enumValue, out IEdmEnumMember enumMember); // Act @@ -606,7 +606,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithSingleQuote Assert.Equal(expectedLiteral, leftNode.LiteralText); CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); - rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); Assert.Equal(colorType, rightNode.ItemType.Definition); } @@ -618,7 +618,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithoutSingleQu string filterQuery = $"{enumValue} in Colors"; // "3 in Colors" string expectedLiteral = "'Blue'"; - IEdmEnumType colorType = this.GetColorType(this.userModel); + IEdmEnumType colorType = this.GetIEdmType("NS.Color"); bool success = colorType.TryParse(enumValue, out IEdmEnumMember enumMember); // Act @@ -637,7 +637,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithoutSingleQu Assert.Equal(expectedLiteral, leftNode.LiteralText); CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); - rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); Assert.Equal(colorType, rightNode.ItemType.Definition); } @@ -649,7 +649,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberDoubleIntegralValue() string filterQuery = $"{enumValue} in Colors"; // "3 in Colors" string expectedLiteral = "'Blue'"; - IEdmEnumType colorType = this.GetColorType(this.userModel); + IEdmEnumType colorType = this.GetIEdmType("NS.Color"); bool success = colorType.TryParse(enumValue, out IEdmEnumMember enumMember); // Act @@ -668,7 +668,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberDoubleIntegralValue() Assert.Equal(expectedLiteral, leftNode.LiteralText); CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); - rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetColorsProperty(this.userModel)); + rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); Assert.Equal(colorType, rightNode.ItemType.Definition); } @@ -739,35 +739,26 @@ public void ParseFilterWithInOperatorWithEnumsInvalidMemberNames_ThrowsIsNotVali action.Throws(Strings.Binder_IsNotValidEnumConstant(expectedExceptionParameter)); } - private IEdmStructuralProperty GetColorProp(IEdmModel model) + private T GetIEdmType(string typeName) where T : IEdmType { - return (IEdmStructuralProperty)((IEdmStructuredType)model - .FindType("NS.MyEntityType")) - .FindProperty("Color"); + return (T)this.userModel.FindType(typeName); } - private IEdmStructuralProperty GetColorsProperty(IEdmModel model) + private T GetIEdmType(IEdmModel model, string typeName) where T : IEdmType { - return (IEdmStructuralProperty)((IEdmStructuredType)model - .FindType("NS.MyEntityType")) - .FindProperty("Colors"); + return (T)model.FindType(typeName); } - private IEdmEnumType GetColorType(IEdmModel model) + private IEdmStructuralProperty GetIEdmProperty(string propertyName) { - return (IEdmEnumType)model.FindType("NS.Color"); + return (IEdmStructuralProperty)this.GetIEdmType("NS.MyEntityType") + .FindProperty(propertyName); } - private IEdmStructuralProperty GetColorFlagsProp(IEdmModel model) + private IEdmStructuralProperty GetIEdmProperty(string entityName, string propertyName) { - return (IEdmStructuralProperty)((IEdmStructuredType)model - .FindType("NS.MyEntityType")) - .FindProperty("ColorFlags"); - } - - private IEdmEnumType GetColorFlagsType(IEdmModel model) - { - return (IEdmEnumType)model.FindType("NS.ColorFlags"); + return (IEdmStructuralProperty)this.GetIEdmType(entityName) + .FindProperty(propertyName); } private FilterClause ParseFilter(string text, IEdmModel edmModel, IEdmEntityType edmEntityType, IEdmEntitySet edmEntitySet) From 299516fe0da374c7e02a58203d663845c8f83395 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Wed, 14 Aug 2024 11:54:12 +0300 Subject: [PATCH 08/13] Remove unnecessary ShouldConvertLeftOperand() method and put the logic inside BindInOperator() --- .../UriParser/Binders/InBinder.cs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs index 8c524c0f82..5c4e1760ae 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs @@ -58,7 +58,9 @@ internal QueryNode BindInOperator(InToken inToken, BindingState state) inToken.Right, new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetUntyped())), state.Model); } - if (ShouldConvertLeftOperand(left, right)) + // If the left operand is either an integral or a string type and the right operand is a collection of enums, + // Calls the MetadataBindingUtils.ConvertToTypeIfNeeded() method to convert the left operand to the same enum type as the right operand. + if ((right is CollectionPropertyAccessNode && right.ItemType.IsEnum()) && (left.TypeReference.IsString() || left.TypeReference.IsIntegral())) { left = MetadataBindingUtils.ConvertToTypeIfNeeded(left, right.ItemType); } @@ -66,19 +68,6 @@ internal QueryNode BindInOperator(InToken inToken, BindingState state) return new InNode(left, right); } - /// - /// Checks if the left operand is either an integral or a string type and the right operand is a collection of enums. - /// - /// The left operand. - /// The right operand. - /// True if the condition is met, otherwise false. - private static bool ShouldConvertLeftOperand(SingleValueNode leftNode, CollectionNode rightNode) - { - // If the left operand is either an integral or a string type and the right operand is a collection of enums, - // Calls the MetadataBindingUtils.ConvertToTypeIfNeeded() method to convert the left operand to the same enum type as the right operand. - return (rightNode is CollectionPropertyAccessNode && rightNode.ItemType.IsEnum()) && (leftNode.TypeReference.IsString() || leftNode.TypeReference.IsIntegral()); - } - /// /// Retrieve SingleValueNode bound with given query token. /// From 5f653545c3f01ce29a75a2b0a803a31282d74ce4 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Wed, 14 Aug 2024 12:14:26 +0300 Subject: [PATCH 09/13] Make the new tests to only use generic methods introduced --- .../UriParser/EnumFilterFunctionalTests.cs | 82 ++++++++++++------- 1 file changed, 53 insertions(+), 29 deletions(-) diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs index 69fb24fdd9..f08132d91a 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs @@ -99,11 +99,11 @@ public void ParseFilterWithEnum() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); + .ShouldBeSingleValuePropertyAccessQueryNode(GetColorProp(this.userModel)); binaryNode .Right - .ShouldBeEnumNode(this.GetIEdmType("NS.Color"), (int)Color.Green); + .ShouldBeEnumNode(this.GetColorType(this.userModel), (int)Color.Green); } [Fact] @@ -121,11 +121,11 @@ public void ParseFilterWithEnumInt() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); + .ShouldBeSingleValuePropertyAccessQueryNode(GetColorProp(this.userModel)); binaryNode .Right - .ShouldBeEnumNode(this.GetIEdmType("NS.Color"), (int)Color.Green); + .ShouldBeEnumNode(this.GetColorType(this.userModel), (int)Color.Green); } [Fact] @@ -143,11 +143,11 @@ public void ParseFilterWithHasOperatorEnumMemberName() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); binaryNode .Right - .ShouldBeEnumNode(this.GetIEdmType("NS.ColorFlags"), (int)ColorFlags.Green); + .ShouldBeEnumNode(this.GetColorFlagsType(this.userModel), (int)ColorFlags.Green); } [Fact] @@ -165,11 +165,11 @@ public void ParseFilterWithHasOperatorEnumUnderlyingValue() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); binaryNode .Right - .ShouldBeEnumNode(this.GetIEdmType("NS.ColorFlags"), (int)ColorFlags.Green); + .ShouldBeEnumNode(this.GetColorFlagsType(this.userModel), (int)ColorFlags.Green); } [Fact] @@ -187,11 +187,11 @@ public void ParseFilterWithHasOperatorEnumLiteralValueAsLeftOperand() binaryNode .Left - .ShouldBeEnumNode(this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); + .ShouldBeEnumNode(this.GetColorFlagsType(this.userModel), (int)(ColorFlags.Green | ColorFlags.Red)); binaryNode .Right - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); } [Fact] @@ -209,11 +209,11 @@ public void ParseFilterWithHasOperatorNonFlagsEnum() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); binaryNode .Right - .ShouldBeEnumNode(this.GetIEdmType("NS.Color"), (int)Color.Green); + .ShouldBeEnumNode(this.GetColorType(this.userModel), (int)Color.Green); } [Fact] @@ -231,12 +231,12 @@ public void ParseFilterWithEnumNormalConbinedValues() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); binaryNode .Right .ShouldBeEnumNode( - this.GetIEdmType("NS.ColorFlags"), + this.GetColorFlagsType(this.userModel), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -255,18 +255,18 @@ public void ParseFilterWithEnumCombinedValuesOrderReversed() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); binaryNode .Right .ShouldBeEnumNode( - this.GetIEdmType("NS.ColorFlags"), + this.GetColorFlagsType(this.userModel), (int)(ColorFlags.Green | ColorFlags.Red)); } [Fact] public void ParseFilterWithEnumValuesCompatibleWithString() - { + { var filterQueryNode = ParseFilter( "ColorFlags has 'Red'", this.userModel, @@ -279,12 +279,12 @@ public void ParseFilterWithEnumValuesCompatibleWithString() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); - + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + binaryNode .Right .ShouldBeEnumNode( - this.GetIEdmType("NS.ColorFlags"), + this.GetColorFlagsType(this.userModel), "Red"); } @@ -303,12 +303,12 @@ public void ParseFilterWithEnumDefinedConbinedValues() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); binaryNode .Right .ShouldBeEnumNode( - this.GetIEdmType("NS.ColorFlags"), + this.GetColorFlagsType(this.userModel), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -327,12 +327,12 @@ public void ParseFilterWithEnumCombinedUnderlyingValues() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); binaryNode .Right .ShouldBeEnumNode( - this.GetIEdmType("NS.ColorFlags"), + this.GetColorFlagsType(this.userModel), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -351,12 +351,12 @@ public void ParseFilterWithEnumNegativeMember() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); binaryNode .Right .ShouldBeEnumNode( - this.GetIEdmType("NS.Color"), + this.GetColorType(this.userModel), (int)(Color.White)); var constantNode = Assert.IsType(binaryNode.Right); @@ -379,12 +379,12 @@ public void ParseFilterWithEnumUndefinedMember() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); binaryNode .Right .ShouldBeEnumNode( - this.GetIEdmType("NS.Color"), + this.GetColorType(this.userModel), -132534290); var constantNode = Assert.IsType(binaryNode.Right); @@ -403,7 +403,7 @@ public void ParseFilterWithNullEnumValue() { var filterQueryNode = ParseFilter("Color eq null", this.userModel, this.entityType, this.entitySet); var binaryNode = filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.Equal); - binaryNode.Left.ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); + binaryNode.Left.ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); var convertNode = Assert.IsType(binaryNode.Right); convertNode.Source.ShouldBeConstantQueryNode((object)null); @@ -739,6 +739,30 @@ public void ParseFilterWithInOperatorWithEnumsInvalidMemberNames_ThrowsIsNotVali action.Throws(Strings.Binder_IsNotValidEnumConstant(expectedExceptionParameter)); } + private IEdmStructuralProperty GetColorProp(IEdmModel model) + { + return (IEdmStructuralProperty)((IEdmStructuredType)model + .FindType("NS.MyEntityType")) + .FindProperty("Color"); + } + + private IEdmEnumType GetColorType(IEdmModel model) + { + return (IEdmEnumType)model.FindType("NS.Color"); + } + + private IEdmStructuralProperty GetColorFlagsProp(IEdmModel model) + { + return (IEdmStructuralProperty)((IEdmStructuredType)model + .FindType("NS.MyEntityType")) + .FindProperty("ColorFlags"); + } + + private IEdmEnumType GetColorFlagsType(IEdmModel model) + { + return (IEdmEnumType)model.FindType("NS.ColorFlags"); + } + private T GetIEdmType(string typeName) where T : IEdmType { return (T)this.userModel.FindType(typeName); From b524e5b66639cace604eb578966bf2c9443d64e5 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Wed, 14 Aug 2024 17:29:46 +0300 Subject: [PATCH 10/13] Resolve comments --- .../UriParser/Binders/InBinder.cs | 2 +- .../UriParser/EnumFilterFunctionalTests.cs | 58 ++++--------------- 2 files changed, 11 insertions(+), 49 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs index 5c4e1760ae..d80dc638d9 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs @@ -60,7 +60,7 @@ internal QueryNode BindInOperator(InToken inToken, BindingState state) // If the left operand is either an integral or a string type and the right operand is a collection of enums, // Calls the MetadataBindingUtils.ConvertToTypeIfNeeded() method to convert the left operand to the same enum type as the right operand. - if ((right is CollectionPropertyAccessNode && right.ItemType.IsEnum()) && (left.TypeReference.IsString() || left.TypeReference.IsIntegral())) + if ((right is CollectionPropertyAccessNode && right.ItemType.IsEnum()) && (left.TypeReference != null && (left.TypeReference.IsString() || left.TypeReference.IsIntegral()))) { left = MetadataBindingUtils.ConvertToTypeIfNeeded(left, right.ItemType); } diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs index 69fb24fdd9..fe41ddeba4 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs @@ -53,7 +53,7 @@ public EnumFilterFunctionalTests() this.userModel.AddElement(enumType); // add enum property - this.entityType = new EdmEntityType("NS", "MyEntityType", isAbstract: false, isOpen: true, baseType: null); + this.entityType = new EdmEntityType("NS", "MyEntityType", isAbstract: false, isOpen: false, baseType: null); var enumTypeReference = new EdmEnumTypeReference(enumType, true); this.entityType.AddProperty(new EdmStructuralProperty(this.entityType, "Color", enumTypeReference)); @@ -72,8 +72,8 @@ public EnumFilterFunctionalTests() this.entityType.AddProperty(new EdmStructuralProperty(this.entityType, "ColorFlags", enumFlagsTypeReference)); // add colors collection - EdmCollectionTypeReference colorCollectionTypeRef = new EdmCollectionTypeReference(new EdmCollectionType(new EdmEnumTypeReference(enumType, true))); - this.entityType.AddProperty(new EdmStructuralProperty(this.entityType, "Colors", colorCollectionTypeRef)); + var colorTypeReference = new EdmEnumTypeReference(enumType, false); + this.entityType.AddStructuralProperty("Colors", new EdmCollectionTypeReference(new EdmCollectionType(colorTypeReference))); this.userModel.AddElement(this.entityType); @@ -524,19 +524,17 @@ public void ParseFilterEnumTypesWrongCast3() public void ParseFilterWithInOperatorWithEnumsFullyQualifiedMemberName() { // Arrange - string enumValue = "White"; - string filterQuery = $"NS.Color'{enumValue}' in Colors"; // "NS.Color'White' in Colors" + string filterQuery = "NS.Color'White' in Colors"; string expectedLiteral = "NS.Color'White'"; IEdmEnumType colorType = this.GetIEdmType("NS.Color"); - IEdmEnumMember enumMember = colorType.Members.Single(m => m.Name == enumValue); + IEdmEnumMember enumMember = colorType.Members.Single(m => m.Name == "White"); // Act FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); SingleValueNode expression = filterQueryNode.Expression; // Assert - Assert.Equal("NS.Color'White' in Colors", filterQuery); InNode inNode = expression.ShouldBeInNode(); ConstantNode leftNode = Assert.IsType(inNode.Left); leftNode.ShouldBeEnumNode(colorType, enumMember.Value.Value); @@ -555,7 +553,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutFullyQualifiedNam { // Arrange string enumValue = "Green"; - string filterQuery = $"'{enumValue}' in Colors"; // "'Green' in Colors" + string filterQuery = "'Green' in Colors"; // "'Green' in Colors" string expectedLiteral = "'Green'"; IEdmEnumType colorType = this.GetIEdmType("NS.Color"); @@ -565,7 +563,6 @@ public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutFullyQualifiedNam SingleValueNode expression = filterQueryNode.Expression; // Assert - Assert.Equal("'Green' in Colors", filterQuery); InNode inNode = expression.ShouldBeInNode(); ConstantNode leftNode = Assert.IsType(inNode.Left); leftNode.ShouldBeEnumNode(colorType, enumValue); @@ -584,7 +581,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithSingleQuote { // Arrange int enumValue = 1; - string filterQuery = $"'{enumValue}' in Colors"; // "'1' in Colors" + string filterQuery = "'1' in Colors"; string expectedLiteral = "'Red'"; IEdmEnumType colorType = this.GetIEdmType("NS.Color"); @@ -595,7 +592,6 @@ public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithSingleQuote SingleValueNode expression = filterQueryNode.Expression; // Assert - Assert.Equal("'1' in Colors", filterQuery); Assert.True(success); InNode inNode = expression.ShouldBeInNode(); ConstantNode leftNode = Assert.IsType(inNode.Left); @@ -615,7 +611,7 @@ public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithoutSingleQu { // Arrange int enumValue = 3; - string filterQuery = $"{enumValue} in Colors"; // "3 in Colors" + string filterQuery = "3 in Colors"; string expectedLiteral = "'Blue'"; IEdmEnumType colorType = this.GetIEdmType("NS.Color"); @@ -626,38 +622,6 @@ public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithoutSingleQu SingleValueNode expression = filterQueryNode.Expression; // Assert - Assert.Equal("3 in Colors", filterQuery); - Assert.True(success); - InNode inNode = expression.ShouldBeInNode(); - ConstantNode leftNode = Assert.IsType(inNode.Left); - leftNode.ShouldBeEnumNode(colorType, enumMember.Name); - - Assert.True(leftNode.TypeReference.IsEnum()); - Assert.Equal(enumMember.Name, leftNode.Value.ToString()); - Assert.Equal(expectedLiteral, leftNode.LiteralText); - - CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); - rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); - Assert.Equal(colorType, rightNode.ItemType.Definition); - } - - [Fact] - public void ParseFilterWithInOperatorWithEnumsMemberDoubleIntegralValue() - { - // Arrange - long enumValue = 3; - string filterQuery = $"{enumValue} in Colors"; // "3 in Colors" - string expectedLiteral = "'Blue'"; - - IEdmEnumType colorType = this.GetIEdmType("NS.Color"); - bool success = colorType.TryParse(enumValue, out IEdmEnumMember enumMember); - - // Act - FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); - SingleValueNode expression = filterQueryNode.Expression; - - // Assert - Assert.Equal("3 in Colors", filterQuery); Assert.True(success); InNode inNode = expression.ShouldBeInNode(); ConstantNode leftNode = Assert.IsType(inNode.Left); @@ -706,18 +670,16 @@ public void ParseFilterWithInOperatorWithEnumsInvalidIntegralValues_ThrowsIsNotV } [Fact] - public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutSingleQuotes_ThrowsNullReferenceException() + public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutSingleQuotes_ThrowsPropertyNotDeclaredException() { // Arrange string filterQuery = "Red in Colors"; - string erroMessage = "Object reference not set to an instance of an object."; - // Act Action action = () => ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); // Assert - action.Throws(erroMessage); + action.Throws(Strings.MetadataBinder_PropertyNotDeclared(this.entityType.FullName, "Red")); } [Theory] From 8f0698ba21273258358ff89f7869fd48d100f8ba Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Wed, 14 Aug 2024 18:16:54 +0300 Subject: [PATCH 11/13] Reuse GetIEdmProperty() and GetIEdmType() methods in the other existing tests --- .../UriParser/EnumFilterFunctionalTests.cs | 78 +++++++------------ 1 file changed, 27 insertions(+), 51 deletions(-) diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs index b43822ff21..45f8b20bb4 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs @@ -99,11 +99,11 @@ public void ParseFilterWithEnum() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorType(this.userModel), (int)Color.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.Color"), (int)Color.Green); } [Fact] @@ -121,11 +121,11 @@ public void ParseFilterWithEnumInt() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorType(this.userModel), (int)Color.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.Color"), (int)Color.Green); } [Fact] @@ -143,11 +143,11 @@ public void ParseFilterWithHasOperatorEnumMemberName() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorFlagsType(this.userModel), (int)ColorFlags.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.ColorFlags"), (int)ColorFlags.Green); } [Fact] @@ -165,11 +165,11 @@ public void ParseFilterWithHasOperatorEnumUnderlyingValue() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorFlagsType(this.userModel), (int)ColorFlags.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.ColorFlags"), (int)ColorFlags.Green); } [Fact] @@ -187,11 +187,11 @@ public void ParseFilterWithHasOperatorEnumLiteralValueAsLeftOperand() binaryNode .Left - .ShouldBeEnumNode(this.GetColorFlagsType(this.userModel), (int)(ColorFlags.Green | ColorFlags.Red)); + .ShouldBeEnumNode(this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); binaryNode .Right - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); } [Fact] @@ -209,11 +209,11 @@ public void ParseFilterWithHasOperatorNonFlagsEnum() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right - .ShouldBeEnumNode(this.GetColorType(this.userModel), (int)Color.Green); + .ShouldBeEnumNode(this.GetIEdmType("NS.Color"), (int)Color.Green); } [Fact] @@ -231,12 +231,12 @@ public void ParseFilterWithEnumNormalConbinedValues() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -255,12 +255,12 @@ public void ParseFilterWithEnumCombinedValuesOrderReversed() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -279,12 +279,12 @@ public void ParseFilterWithEnumValuesCompatibleWithString() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), "Red"); } @@ -303,12 +303,12 @@ public void ParseFilterWithEnumDefinedConbinedValues() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -327,12 +327,12 @@ public void ParseFilterWithEnumCombinedUnderlyingValues() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorFlagsProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("ColorFlags")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorFlagsType(this.userModel), + this.GetIEdmType("NS.ColorFlags"), (int)(ColorFlags.Green | ColorFlags.Red)); } @@ -351,12 +351,12 @@ public void ParseFilterWithEnumNegativeMember() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorType(this.userModel), + this.GetIEdmType("NS.Color"), (int)(Color.White)); var constantNode = Assert.IsType(binaryNode.Right); @@ -379,12 +379,12 @@ public void ParseFilterWithEnumUndefinedMember() binaryNode .Left - .ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); + .ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); binaryNode .Right .ShouldBeEnumNode( - this.GetColorType(this.userModel), + this.GetIEdmType("NS.Color"), -132534290); var constantNode = Assert.IsType(binaryNode.Right); @@ -403,7 +403,7 @@ public void ParseFilterWithNullEnumValue() { var filterQueryNode = ParseFilter("Color eq null", this.userModel, this.entityType, this.entitySet); var binaryNode = filterQueryNode.Expression.ShouldBeBinaryOperatorNode(BinaryOperatorKind.Equal); - binaryNode.Left.ShouldBeSingleValuePropertyAccessQueryNode(this.GetColorProp(this.userModel)); + binaryNode.Left.ShouldBeSingleValuePropertyAccessQueryNode(this.GetIEdmProperty("Color")); var convertNode = Assert.IsType(binaryNode.Right); convertNode.Source.ShouldBeConstantQueryNode((object)null); @@ -701,30 +701,6 @@ public void ParseFilterWithInOperatorWithEnumsInvalidMemberNames_ThrowsIsNotVali action.Throws(Strings.Binder_IsNotValidEnumConstant(expectedExceptionParameter)); } - private IEdmStructuralProperty GetColorProp(IEdmModel model) - { - return (IEdmStructuralProperty)((IEdmStructuredType)model - .FindType("NS.MyEntityType")) - .FindProperty("Color"); - } - - private IEdmEnumType GetColorType(IEdmModel model) - { - return (IEdmEnumType)model.FindType("NS.Color"); - } - - private IEdmStructuralProperty GetColorFlagsProp(IEdmModel model) - { - return (IEdmStructuralProperty)((IEdmStructuredType)model - .FindType("NS.MyEntityType")) - .FindProperty("ColorFlags"); - } - - private IEdmEnumType GetColorFlagsType(IEdmModel model) - { - return (IEdmEnumType)model.FindType("NS.ColorFlags"); - } - private T GetIEdmType(string typeName) where T : IEdmType { return (T)this.userModel.FindType(typeName); From 07655ee14f571b2d9013a515dcd73a05c2222c74 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Thu, 15 Aug 2024 11:44:22 +0300 Subject: [PATCH 12/13] Merge related tests using [Theory] --- .../UriParser/EnumFilterFunctionalTests.cs | 129 ++++-------------- .../FilterAndOrderByFunctionalTests.cs | 56 +------- 2 files changed, 32 insertions(+), 153 deletions(-) diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs index 45f8b20bb4..64b9bb37b9 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/EnumFilterFunctionalTests.cs @@ -520,49 +520,35 @@ public void ParseFilterEnumTypesWrongCast3() parse.Throws(Strings.MetadataBinder_CastOrIsOfFunctionWithoutATypeArgument); } - [Fact] - public void ParseFilterWithInOperatorWithEnumsFullyQualifiedMemberName() - { - // Arrange - string filterQuery = "NS.Color'White' in Colors"; - string expectedLiteral = "NS.Color'White'"; - - IEdmEnumType colorType = this.GetIEdmType("NS.Color"); - IEdmEnumMember enumMember = colorType.Members.Single(m => m.Name == "White"); - - // Act - FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); - SingleValueNode expression = filterQueryNode.Expression; - - // Assert - InNode inNode = expression.ShouldBeInNode(); - ConstantNode leftNode = Assert.IsType(inNode.Left); - leftNode.ShouldBeEnumNode(colorType, enumMember.Value.Value); - - Assert.True(leftNode.TypeReference.IsEnum()); - Assert.Equal(enumMember.Value.Value.ToString(), leftNode.Value.ToString()); - Assert.Equal(expectedLiteral, leftNode.LiteralText); - - CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); - rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); - Assert.Equal(colorType, rightNode.ItemType.Definition); - } - - [Fact] - public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutFullyQualifiedName() + [Theory] + [InlineData("NS.Color'Green'")] + [InlineData("'Green'")] + [InlineData("'2'")] + [InlineData("2")] + public void ParseFilterWithInOperatorWithEnums(string filterOptionValue) { // Arrange + string filterQuery = $"{filterOptionValue} in Colors"; string enumValue = "Green"; - string filterQuery = "'Green' in Colors"; // "'Green' in Colors" - string expectedLiteral = "'Green'"; + string colorTypeName = "NS.Color"; - IEdmEnumType colorType = this.GetIEdmType("NS.Color"); + IEdmEnumType colorType = this.GetIEdmType(colorTypeName); + IEdmEnumMember enumMember = colorType.Members.First(m => m.Name == enumValue); + + string expectedLiteral = "'Green'"; + if (filterOptionValue.StartsWith(colorTypeName)) // if the filterOptionValue is already fully qualified, then the expectedLiteral should be the same as filterOptionValue + { + expectedLiteral = "NS.Color'Green'"; + enumValue = enumMember.Value.Value.ToString(); // "2" The backing type, can be "3" or "White" or "Black,Yellow,Cyan" + } // Act FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); SingleValueNode expression = filterQueryNode.Expression; // Assert + Assert.Equal(2, enumMember.Value.Value); + Assert.Equal("Green", enumMember.Name); InNode inNode = expression.ShouldBeInNode(); ConstantNode leftNode = Assert.IsType(inNode.Left); leftNode.ShouldBeEnumNode(colorType, enumValue); @@ -574,77 +560,14 @@ public void ParseFilterWithInOperatorWithEnumsMemberNameWithoutFullyQualifiedNam CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); Assert.Equal(colorType, rightNode.ItemType.Definition); - } - [Fact] - public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithSingleQuotes() - { - // Arrange - int enumValue = 1; - string filterQuery = "'1' in Colors"; - string expectedLiteral = "'Red'"; - - IEdmEnumType colorType = this.GetIEdmType("NS.Color"); - bool success = colorType.TryParse(enumValue, out IEdmEnumMember enumMember); - - // Act - FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); - SingleValueNode expression = filterQueryNode.Expression; - - // Assert - Assert.True(success); - InNode inNode = expression.ShouldBeInNode(); - ConstantNode leftNode = Assert.IsType(inNode.Left); - leftNode.ShouldBeEnumNode(colorType, enumMember.Name); - - Assert.True(leftNode.TypeReference.IsEnum()); - Assert.Equal(enumMember.Name, leftNode.Value.ToString()); - Assert.Equal(expectedLiteral, leftNode.LiteralText); - - CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); - rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); - Assert.Equal(colorType, rightNode.ItemType.Definition); } [Fact] - public void ParseFilterWithInOperatorWithEnumsMemberIntegralValueWithoutSingleQuotes() - { - // Arrange - int enumValue = 3; - string filterQuery = "3 in Colors"; - string expectedLiteral = "'Blue'"; - - IEdmEnumType colorType = this.GetIEdmType("NS.Color"); - bool success = colorType.TryParse(enumValue, out IEdmEnumMember enumMember); - - // Act - FilterClause filterQueryNode = ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); - SingleValueNode expression = filterQueryNode.Expression; - - // Assert - Assert.True(success); - InNode inNode = expression.ShouldBeInNode(); - ConstantNode leftNode = Assert.IsType(inNode.Left); - leftNode.ShouldBeEnumNode(colorType, enumMember.Name); - - Assert.True(leftNode.TypeReference.IsEnum()); - Assert.Equal(enumMember.Name, leftNode.Value.ToString()); - Assert.Equal(expectedLiteral, leftNode.LiteralText); - - CollectionPropertyAccessNode rightNode = Assert.IsType(inNode.Right); - rightNode.ShouldBeCollectionPropertyAccessQueryNode(this.GetIEdmProperty("Colors")); - Assert.Equal(colorType, rightNode.ItemType.Definition); - } - - [Theory] - [InlineData("3.0")] - [InlineData("3.1")] - [InlineData("4.0")] - [InlineData("5.0")] - public void ParseFilterWithInOperatorWithEnumsMemberFloatIntegralValue_ThrowCollectionItemTypeMustBeSameAsSingleItemTypeException(string floatString) + public void ParseFilterWithInOperatorWithEnumsMemberFloatIntegralValue_ThrowCollectionItemTypeMustBeSameAsSingleItemTypeException() { // Arrange - string filterQuery = $"{floatString} in Colors"; + string filterQuery = $"3.0 in Colors"; // Act Action test = () => ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); @@ -654,10 +577,10 @@ public void ParseFilterWithInOperatorWithEnumsMemberFloatIntegralValue_ThrowColl } [Theory] - [InlineData(-20)] - [InlineData("-20")] - [InlineData("'-20'")] - public void ParseFilterWithInOperatorWithEnumsInvalidIntegralValues_ThrowsIsNotValidEnumConstantException(object integralValue) + [InlineData("-20", "-20")] + [InlineData("'-20'", "-20")] + [InlineData("'3.0'", "3.0")] + public void ParseFilterWithInOperatorWithEnumsInvalidIntegralValues_ThrowsIsNotValidEnumConstantException(string integralValue, string errorMessageParam) { // Arrange string filterQuery = $"{integralValue} in Colors"; @@ -666,7 +589,7 @@ public void ParseFilterWithInOperatorWithEnumsInvalidIntegralValues_ThrowsIsNotV Action action = () => ParseFilter(filterQuery, this.userModel, this.entityType, this.entitySet); // Assert - action.Throws(Strings.Binder_IsNotValidEnumConstant("-20")); + action.Throws(Strings.Binder_IsNotValidEnumConstant(errorMessageParam)); } [Fact] diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index b5acd8c67f..7b9bc13ea3 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -2041,57 +2041,14 @@ public void FilterWithInOperationWithEnums() Assert.Equal("FavoriteColors", Assert.IsType(inNode.Right).Property.Name); } - [Fact] - public void FilterWithInOperationWithEnumsMemberName_WithoutQualifiedNamespace() - { - // Arrange - string filterQuery = "'SolidYellow' in FavoriteColors"; - - string expectedLiteralText = "'SolidYellow'"; - string expectedfullyQualifiedName = "Fully.Qualified.Namespace.ColorPattern'SolidYellow'"; - string expectedCollectionPropertyName = "FavoriteColors"; - - // Act - FilterClause filter = ParseFilter(filterQuery, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); - - // Assert - InNode inNode = Assert.IsType(filter.Expression); - ConstantNode left = Assert.IsType(inNode.Left); - ODataEnumValue enumValue = Assert.IsType(left.Value); - - Assert.Equal(expectedLiteralText, left.LiteralText); - Assert.Equal(expectedfullyQualifiedName, (enumValue.TypeName + left.LiteralText)); - Assert.Equal(expectedCollectionPropertyName, Assert.IsType(inNode.Right).Property.Name); - } - - [Fact] - public void FilterWithInOperationWithEnumsMemberIntegralValue_WithSingleQuotes() - { - // Arrange - string filterQuery = "'12' in FavoriteColors"; - - string expectedLiteralText = "'SolidYellow'"; - string expectedfullyQualifiedName = "Fully.Qualified.Namespace.ColorPattern'SolidYellow'"; - string expectedCollectionPropertyName = "FavoriteColors"; - - // Act - FilterClause filter = ParseFilter(filterQuery, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); - - // Assert - InNode inNode = Assert.IsType(filter.Expression); - ConstantNode left = Assert.IsType(inNode.Left); - ODataEnumValue enumValue = Assert.IsType(left.Value); - - Assert.Equal(expectedLiteralText, left.LiteralText); - Assert.Equal(expectedfullyQualifiedName, (enumValue.TypeName + left.LiteralText)); - Assert.Equal(expectedCollectionPropertyName, Assert.IsType(inNode.Right).Property.Name); - } - - [Fact] - public void FilterWithInOperationWithEnumsMemberIntegralValue_WithoutSingleQuotes() + [Theory] + [InlineData("'SolidYellow'")] + [InlineData("'12'")] + [InlineData("12")] + public void FilterWithInOperationWithEnumsMemberIntegralValue(string filterOptionValue) { // Arrange - string filterQuery = "12 in FavoriteColors"; + string filterQuery = $"{filterOptionValue} in FavoriteColors"; string expectedLiteralText = "'SolidYellow'"; string expectedfullyQualifiedName = "Fully.Qualified.Namespace.ColorPattern'SolidYellow'"; @@ -2111,7 +2068,6 @@ public void FilterWithInOperationWithEnumsMemberIntegralValue_WithoutSingleQuote } [Theory] - [InlineData(53)] [InlineData("53")] [InlineData("'53'")] public void FilterWithInOperationWithEnumsInvalidMemberIntegralValue_ThrowsIsNotValidEnumConstantException(object integralValue) From 3f4f00a3a36d40eda60df6f81d3ef1c54631f604 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Thu, 15 Aug 2024 11:56:35 +0300 Subject: [PATCH 13/13] use string instead of object --- .../ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index 7b9bc13ea3..16883797a9 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -2070,7 +2070,7 @@ public void FilterWithInOperationWithEnumsMemberIntegralValue(string filterOptio [Theory] [InlineData("53")] [InlineData("'53'")] - public void FilterWithInOperationWithEnumsInvalidMemberIntegralValue_ThrowsIsNotValidEnumConstantException(object integralValue) + public void FilterWithInOperationWithEnumsInvalidMemberIntegralValue_ThrowsIsNotValidEnumConstantException(string integralValue) { // Arrange string filterQuery = $"{integralValue} in FavoriteColors";