From c8edded6d84d7d3be55f0eac5e27b05d66c3b2c2 Mon Sep 17 00:00:00 2001 From: Stef Heyenrath Date: Tue, 14 Feb 2023 16:55:40 +0100 Subject: [PATCH] Fixed ExpressionParser when WrappedValue-string is used for equals-operator for Enum (#672) * Fixed ExpressionParser when WrappedValue-string is used for equals-operator to compare Enum * . * . * Partial fixed ExpressionParser when WrappedValue-string is used for in-operator and left operand is enum (#668) * Fix * . --------- Co-authored-by: neilbgr --- .../Parser/ConstantExpressionWrapper.cs | 14 +- .../Parser/ExpressionHelper.cs | 38 +++- .../Parser/ExpressionParser.cs | 45 +++- .../Parser/IConstantExpressionWrapper.cs | 4 +- .../Parser/IExpressionHelper.cs | 6 +- .../Parser/WrappedValue.cs | 74 +++++- .../Parser/WrappedValueTests.cs | 88 ++++++++ ...ts.UseParameterizedNamesInDynamicQuery .cs | 213 +++++++++++++++++- 8 files changed, 463 insertions(+), 19 deletions(-) create mode 100644 test/System.Linq.Dynamic.Core.Tests/Parser/WrappedValueTests.cs diff --git a/src/System.Linq.Dynamic.Core/Parser/ConstantExpressionWrapper.cs b/src/System.Linq.Dynamic.Core/Parser/ConstantExpressionWrapper.cs index 39e7a429..18c6e4ca 100644 --- a/src/System.Linq.Dynamic.Core/Parser/ConstantExpressionWrapper.cs +++ b/src/System.Linq.Dynamic.Core/Parser/ConstantExpressionWrapper.cs @@ -227,7 +227,7 @@ public void Wrap(ref Expression expression) } } - public bool TryUnwrap(MemberExpression? expression, [NotNullWhen(true)] out TValue? value) + public bool TryUnwrapAsValue(MemberExpression? expression, [NotNullWhen(true)] out TValue? value) { if (expression?.Expression is ConstantExpression { Value: WrappedValue wrapper }) { @@ -239,6 +239,18 @@ public bool TryUnwrap(MemberExpression? expression, [NotNullWhen(true)] return false; } + public bool TryUnwrapAsConstantExpression(MemberExpression? expression, [NotNullWhen(true)] out ConstantExpression? value) + { + if (TryUnwrapAsValue(expression, out var wrappedValue)) + { + value = Expression.Constant(wrappedValue); + return true; + } + + value = default; + return false; + } + private static MemberExpression Wrap(TValue value) { var wrapper = new WrappedValue(value); diff --git a/src/System.Linq.Dynamic.Core/Parser/ExpressionHelper.cs b/src/System.Linq.Dynamic.Core/Parser/ExpressionHelper.cs index 75dd8752..28d08acc 100644 --- a/src/System.Linq.Dynamic.Core/Parser/ExpressionHelper.cs +++ b/src/System.Linq.Dynamic.Core/Parser/ExpressionHelper.cs @@ -26,9 +26,43 @@ public void WrapConstantExpression(ref Expression argument) } } - public bool TryUnwrapConstantExpression(Expression? expression, [NotNullWhen(true)] out TValue? value) + public bool TryUnwrapAsValue(Expression? expression, [NotNullWhen(true)] out TValue? value) { - if (_parsingConfig.UseParameterizedNamesInDynamicQuery && _constantExpressionWrapper.TryUnwrap(expression as MemberExpression, out value)) + if (_parsingConfig.UseParameterizedNamesInDynamicQuery && _constantExpressionWrapper.TryUnwrapAsValue(expression as MemberExpression, out value)) + { + return true; + } + + value = default; + return false; + } + + public bool TryUnwrapAsConstantExpression(Expression? expression, [NotNullWhen(true)] out ConstantExpression? value) + { + if (_parsingConfig.UseParameterizedNamesInDynamicQuery && _constantExpressionWrapper.TryUnwrapAsConstantExpression(expression as MemberExpression, out value)) + { + return true; + } + + value = default; + return false; + } + + public bool TryUnwrapAsConstantExpression(Expression? expression, [NotNullWhen(true)] out ConstantExpression? value) + { + if (!_parsingConfig.UseParameterizedNamesInDynamicQuery || expression is not MemberExpression memberExpression) + { + value = default; + return false; + } + + if + ( + _constantExpressionWrapper.TryUnwrapAsConstantExpression(memberExpression, out value) || + _constantExpressionWrapper.TryUnwrapAsConstantExpression(memberExpression, out value) || + _constantExpressionWrapper.TryUnwrapAsConstantExpression(memberExpression, out value) || + _constantExpressionWrapper.TryUnwrapAsConstantExpression(memberExpression, out value) + ) { return true; } diff --git a/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs b/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs index 22089efe..e09049d2 100644 --- a/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs +++ b/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs @@ -313,9 +313,16 @@ private Expression ParseIn() Expression right = ParseUnary(); // if the identifier is an Enum, try to convert the right-side also to an Enum. - if (left.Type.GetTypeInfo().IsEnum && right is ConstantExpression constantExpression) + if (left.Type.GetTypeInfo().IsEnum) { - right = ParseEnumToConstantExpression(op.Pos, left.Type, constantExpression); + if (right is ConstantExpression constantExprRight) + { + right = ParseEnumToConstantExpression(op.Pos, left.Type, constantExprRight); + } + else if (_expressionHelper.TryUnwrapAsConstantExpression(right, out var unwrappedConstantExprRight)) + { + right = ParseEnumToConstantExpression(op.Pos, left.Type, unwrappedConstantExprRight); + } } // else, check for direct type match @@ -476,13 +483,27 @@ private Expression ParseComparisonOperator() { left = e; } - else if (TypeHelper.IsEnumType(left.Type) && (constantExpr = right as ConstantExpression) != null) + else if (TypeHelper.IsEnumType(left.Type)) { - right = ParseEnumToConstantExpression(op.Pos, left.Type, constantExpr); + if (right is ConstantExpression constantExprRight) + { + right = ParseEnumToConstantExpression(op.Pos, left.Type, constantExprRight); + } + else if (_expressionHelper.TryUnwrapAsConstantExpression(right, out var unwrappedConstantExprRight)) + { + right = ParseEnumToConstantExpression(op.Pos, left.Type, unwrappedConstantExprRight); + } } - else if (TypeHelper.IsEnumType(right.Type) && (constantExpr = left as ConstantExpression) != null) + else if (TypeHelper.IsEnumType(right.Type)) { - left = ParseEnumToConstantExpression(op.Pos, right.Type, constantExpr); + if (left is ConstantExpression constantExprLeft) + { + left = ParseEnumToConstantExpression(op.Pos, right.Type, constantExprLeft); + } + else if (_expressionHelper.TryUnwrapAsConstantExpression(left, out var unwrappedConstantExprLeft)) + { + left = ParseEnumToConstantExpression(op.Pos, right.Type, unwrappedConstantExprLeft); + } } else { @@ -498,11 +519,11 @@ private Expression ParseComparisonOperator() { left = Expression.Constant(typeConverter.ConvertFromInvariantString(stringValueL), right.Type); } - else if (_expressionHelper.TryUnwrapConstantExpression(right, out var unwrappedStringValueR) && (typeConverter = _typeConverterFactory.GetConverter(left.Type)) != null && typeConverter.CanConvertFrom(right.Type)) + else if (_expressionHelper.TryUnwrapAsValue(right, out var unwrappedStringValueR) && (typeConverter = _typeConverterFactory.GetConverter(left.Type)) != null && typeConverter.CanConvertFrom(right.Type)) { right = Expression.Constant(typeConverter.ConvertFromInvariantString(unwrappedStringValueR), left.Type); } - else if (_expressionHelper.TryUnwrapConstantExpression(left, out var unwrappedStringValueL) && (typeConverter = _typeConverterFactory.GetConverter(right.Type)) != null && typeConverter.CanConvertFrom(left.Type)) + else if (_expressionHelper.TryUnwrapAsValue(left, out var unwrappedStringValueL) && (typeConverter = _typeConverterFactory.GetConverter(right.Type)) != null && typeConverter.CanConvertFrom(left.Type)) { left = Expression.Constant(typeConverter.ConvertFromInvariantString(unwrappedStringValueL), right.Type); } @@ -581,7 +602,7 @@ private Expression ParseComparisonOperator() return left; } - private bool HasImplicitConversion(Type baseType, Type targetType) + private static bool HasImplicitConversion(Type baseType, Type targetType) { var baseTypeHasConversion = baseType.GetMethods(BindingFlags.Public | BindingFlags.Static) .Where(mi => mi.Name == "op_Implicit" && mi.ReturnType == targetType) @@ -597,12 +618,12 @@ private bool HasImplicitConversion(Type baseType, Type targetType) .Any(mi => mi.GetParameters().FirstOrDefault()?.ParameterType == baseType); } - private ConstantExpression ParseEnumToConstantExpression(int pos, Type leftType, ConstantExpression constantExpr) + private static ConstantExpression ParseEnumToConstantExpression(int pos, Type leftType, ConstantExpression constantExpr) { return Expression.Constant(ParseConstantExpressionToEnum(pos, leftType, constantExpr), leftType); } - private object ParseConstantExpressionToEnum(int pos, Type leftType, ConstantExpression constantExpr) + private static object ParseConstantExpressionToEnum(int pos, Type leftType, ConstantExpression constantExpr) { try { @@ -618,7 +639,7 @@ private object ParseConstantExpressionToEnum(int pos, Type leftType, ConstantExp try { - return Enum.ToObject(TypeHelper.GetNonNullableType(leftType), constantExpr.Value); + return Enum.ToObject(TypeHelper.GetNonNullableType(leftType), constantExpr.Value!); } catch { diff --git a/src/System.Linq.Dynamic.Core/Parser/IConstantExpressionWrapper.cs b/src/System.Linq.Dynamic.Core/Parser/IConstantExpressionWrapper.cs index e2cb4904..93a604c5 100644 --- a/src/System.Linq.Dynamic.Core/Parser/IConstantExpressionWrapper.cs +++ b/src/System.Linq.Dynamic.Core/Parser/IConstantExpressionWrapper.cs @@ -7,5 +7,7 @@ internal interface IConstantExpressionWrapper { void Wrap(ref Expression expression); - bool TryUnwrap(MemberExpression? expression, [NotNullWhen(true)] out TValue? value); + bool TryUnwrapAsValue(MemberExpression? expression, [NotNullWhen(true)] out TValue? value); + + bool TryUnwrapAsConstantExpression(MemberExpression? expression, [NotNullWhen(true)] out ConstantExpression? value); } \ No newline at end of file diff --git a/src/System.Linq.Dynamic.Core/Parser/IExpressionHelper.cs b/src/System.Linq.Dynamic.Core/Parser/IExpressionHelper.cs index b276a712..f78fac30 100644 --- a/src/System.Linq.Dynamic.Core/Parser/IExpressionHelper.cs +++ b/src/System.Linq.Dynamic.Core/Parser/IExpressionHelper.cs @@ -35,7 +35,11 @@ internal interface IExpressionHelper void WrapConstantExpression(ref Expression argument); - bool TryUnwrapConstantExpression(Expression? expression, [NotNullWhen(true)] out TValue? value); + bool TryUnwrapAsValue(Expression? expression, [NotNullWhen(true)] out TValue? value); + + bool TryUnwrapAsConstantExpression(Expression? expression, [NotNullWhen(true)] out ConstantExpression? value); + + bool TryUnwrapAsConstantExpression(Expression? expression, [NotNullWhen(true)] out ConstantExpression? value); bool MemberExpressionIsDynamic(Expression expression); diff --git a/src/System.Linq.Dynamic.Core/Parser/WrappedValue.cs b/src/System.Linq.Dynamic.Core/Parser/WrappedValue.cs index 3121f4af..7f107177 100644 --- a/src/System.Linq.Dynamic.Core/Parser/WrappedValue.cs +++ b/src/System.Linq.Dynamic.Core/Parser/WrappedValue.cs @@ -1,4 +1,6 @@ -namespace System.Linq.Dynamic.Core.Parser; +using System.Collections.Generic; + +namespace System.Linq.Dynamic.Core.Parser; internal class WrappedValue { @@ -8,4 +10,74 @@ public WrappedValue(TValue value) { Value = value; } + + public static bool operator ==(WrappedValue? left, WrappedValue? right) + { + if (ReferenceEquals(left, right)) + { + return true; + } + + if (ReferenceEquals(left, null) || ReferenceEquals(right, null)) + { + return false; + } + + return EqualityComparer.Default.Equals(left.Value, right.Value); + } + + public static bool operator !=(WrappedValue? left, WrappedValue? right) + { + return !(left == right); + } + + public static bool operator ==(WrappedValue? left, TValue? right) + { + if (ReferenceEquals(left, null)) + { + return false; + } + + return EqualityComparer.Default.Equals(left.Value, right); + } + + public static bool operator !=(WrappedValue? left, TValue? right) + { + return !(left == right); + } + + public static bool operator ==(TValue? left, WrappedValue? right) + { + if (ReferenceEquals(right, null)) + { + return false; + } + + return EqualityComparer.Default.Equals(left, right.Value); + } + + public static bool operator !=(TValue? left, WrappedValue? right) + { + return !(left == right); + } + + public override bool Equals(object? obj) + { + if (ReferenceEquals(this, obj)) + { + return true; + } + + if (obj is not WrappedValue other) + { + return false; + } + + return EqualityComparer.Default.Equals(Value, other.Value); + } + + public override int GetHashCode() + { + return Value?.GetHashCode() ?? 0; + } } \ No newline at end of file diff --git a/test/System.Linq.Dynamic.Core.Tests/Parser/WrappedValueTests.cs b/test/System.Linq.Dynamic.Core.Tests/Parser/WrappedValueTests.cs new file mode 100644 index 00000000..c069cdce --- /dev/null +++ b/test/System.Linq.Dynamic.Core.Tests/Parser/WrappedValueTests.cs @@ -0,0 +1,88 @@ +using System.Linq.Dynamic.Core.Parser; +using FluentAssertions; +using Xunit; + +namespace System.Linq.Dynamic.Core.Tests.Parser; + +public class WrappedValueTests +{ + [Fact] + public void WrappedValue_OfTypeString_OperatorEquals_String() + { + // Arrange + var wrapped = new WrappedValue("str"); + + // Act + var result1A = wrapped == "str"; + var result1B = "str" == wrapped; + var result2A = wrapped == "x"; + var result2B = "x" == wrapped; + + // Assert + result1A.Should().BeTrue(); + result1B.Should().BeTrue(); + result2A.Should().BeFalse(); + result2B.Should().BeFalse(); + } + + [Fact] + public void WrappedValue_OfTypeString_OperatorNotEquals_String() + { + // Arrange + var wrapped = new WrappedValue("str"); + + // Act + var result1A = wrapped != "str"; + var result1B = "str" != wrapped; + var result2A = wrapped == "x"; + var result2B = "x" == wrapped; + + // Assert + result1A.Should().BeFalse(); + result1B.Should().BeFalse(); + result2A.Should().BeFalse(); + result2B.Should().BeFalse(); + } + + [Fact] + public void WrappedValue_OfTypeString_OperatorEquals_WrappedValue_OfTypeString() + { + // Arrange + var wrapped = new WrappedValue("str"); + var testEqual = new WrappedValue("str"); + var testNotEqual = new WrappedValue("x"); + + // Act + var result1A = wrapped == testEqual; + var result1B = testEqual == wrapped; + var result2A = wrapped == testNotEqual; + var result2B = testNotEqual == wrapped; + + // Assert + result1A.Should().BeTrue(); + result1B.Should().BeTrue(); + result2A.Should().BeFalse(); + result2B.Should().BeFalse(); + } + + [Fact] + public void WrappedValue_OfTypeString_OperatorNotEquals_WrappedValue_OfTypeString() + { + // Arrange + var wrapped = new WrappedValue("str"); + var testEqual = new WrappedValue("str"); + var testNotEqual = new WrappedValue("x"); + + // Act + var result1A = wrapped != testEqual; + var result1B = testEqual != wrapped; + var result2A = wrapped != testNotEqual; + var result2B = testNotEqual != wrapped; + + // Assert + result1A.Should().BeFalse(); + result1B.Should().BeFalse(); + result2A.Should().BeTrue(); + result2B.Should().BeTrue(); + } +} \ No newline at end of file diff --git a/test/System.Linq.Dynamic.Core.Tests/QueryableTests.UseParameterizedNamesInDynamicQuery .cs b/test/System.Linq.Dynamic.Core.Tests/QueryableTests.UseParameterizedNamesInDynamicQuery .cs index 698e63fe..c28ba2d2 100644 --- a/test/System.Linq.Dynamic.Core.Tests/QueryableTests.UseParameterizedNamesInDynamicQuery .cs +++ b/test/System.Linq.Dynamic.Core.Tests/QueryableTests.UseParameterizedNamesInDynamicQuery .cs @@ -10,7 +10,7 @@ public partial class QueryableTests /// Issue #645 /// [Fact] - public void When_UseParameterizedNamesInDynamicQuery_IsTrue_WrappedStringValue_Should_Be_Unwrapped() + public void When_UseParameterizedNamesInDynamicQuery_IsTrue_WrappedStringValueForDateTime_Should_Be_Unwrapped() { // Arrange var list = new List @@ -64,6 +64,209 @@ public void When_UseParameterizedNamesInDynamicQuery_IsTrue_WrappedStringValue_S // Assert 2B result2B.Should().HaveCount(1); } + + /// + /// Issue #668 + /// + [Fact] + public void When_UseParameterizedNamesInDynamicQuery_IsTrue_WrappedStringValueEnum_Should_Be_Unwrapped() + { + // Arrange + var list = new List + { + new() + { + Name = "Duffy", + GenderType = Gender.Female + }, + new() + { + Name = "Garry", + GenderType = Gender.Male + } + }; + + var config = new ParsingConfig + { + UseParameterizedNamesInDynamicQuery = true + }; + + // Act + var result = list.AsQueryable().Where(config, "GenderType = \"Female\"").ToArray(); + + // Assert + result.Should().HaveCount(1); + } + + [Fact] + public void When_UseParameterizedNamesInDynamicQuery_IsTrue_WrappedStringValueEnumAsParameter_Should_Be_Unwrapped() + { + // Arrange + var list = new List + { + new() + { + Name = "Duffy", + GenderType = Gender.Female + }, + new() + { + Name = "Garry", + GenderType = Gender.Male + } + }; + + var config = new ParsingConfig + { + UseParameterizedNamesInDynamicQuery = true + }; + + // Act + var result = list.AsQueryable().Where(config, "GenderType = @0", "Female").ToArray(); + + // Assert + result.Should().HaveCount(1); + } + + [Fact] + public void When_UseParameterizedNamesInDynamicQuery_IsTrue_WrappedStringValueEnumArray_Should_Be_Unwrapped() + { + // Arrange + var list = new List + { + new() + { + Name = "Duffy", + GenderType = Gender.Female + }, + new() + { + Name = "Garry", + GenderType = Gender.Male + }, + new() + { + Name = "Garry", + GenderType = Gender.Other + }, + new() + { + Name = "Garry", + GenderType = Gender.Male + } + }; + + var config = new ParsingConfig + { + UseParameterizedNamesInDynamicQuery = true + }; + + // Act + var result = list.AsQueryable().Where(config, "GenderType in (\"Female\", \"Other\")").ToArray(); + + // Assert + result.Should().HaveCount(2); + } + + [Fact] + public void When_UseParameterizedNamesInDynamicQuery_IsTrue_WrappedIntegerValueEnumArray_Should_Be_Unwrapped() + { + // Arrange + var list = new List + { + new() + { + Name = "Duffy", + GenderType = Gender.Female + }, + new() + { + Name = "Garry", + GenderType = Gender.Male + }, + new() + { + Name = "Test A", + GenderType = Gender.Other + }, + new() + { + Name = "Test B", + GenderType = Gender.Male + } + }; + + var config = new ParsingConfig + { + UseParameterizedNamesInDynamicQuery = true + }; + + // Act + var result = list.AsQueryable().Where(config, "GenderType in (0, 2)").ToArray(); + + // Assert + result.Should().HaveCount(3); + } + + [Fact] + public void When_UseParameterizedNamesInDynamicQuery_IsTrue_WrappedIntegerValueEnum_Should_Be_Unwrapped() + { + // Arrange + var list = new List + { + new() + { + Name = "Duffy", + GenderType = Gender.Female + }, + new() + { + Name = "Garry", + GenderType = Gender.Male + } + }; + + var config = new ParsingConfig + { + UseParameterizedNamesInDynamicQuery = true + }; + + // Act + var result = list.AsQueryable().Where(config, "GenderType = 1").ToArray(); + + // Assert + result.Should().HaveCount(1); + } + + [Fact] + public void When_UseParameterizedNamesInDynamicQuery_IsTrue_WrappedIntegerStringValueEnumAsParameter_Should_Be_Unwrapped() + { + // Arrange + var list = new List + { + new() + { + Name = "Duffy", + GenderType = Gender.Female + }, + new() + { + Name = "Garry", + GenderType = Gender.Male + } + }; + + var config = new ParsingConfig + { + UseParameterizedNamesInDynamicQuery = true + }; + + // Act + var result = list.AsQueryable().Where(config, "GenderType = @0", 1).ToArray(); + + // Assert + result.Should().HaveCount(1); + } } public class Customer @@ -75,6 +278,7 @@ public class Customer public string Phone { get; set; } public Location Location { get; set; } public DateTimeOffset? LastContact { get; set; } + public Gender GenderType { get; set; } } public class Location @@ -82,4 +286,11 @@ public class Location public int LocationID { get; set; } public string Name { get; set; } public DateTimeOffset UpdateAt { get; set; } +} + +public enum Gender +{ + Male = 0, + Female = 1, + Other = 2 } \ No newline at end of file