From 840efd7ac2469ca128b798da94e0e78cbbceeffc Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Sun, 17 May 2020 12:20:42 +0300 Subject: [PATCH 1/9] Test case --- src/NHibernate.Test/Async/Linq/ODataTests.cs | 11 +++++++++++ src/NHibernate.Test/Linq/ODataTests.cs | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/NHibernate.Test/Async/Linq/ODataTests.cs b/src/NHibernate.Test/Async/Linq/ODataTests.cs index 44135195c13..c57e4fec768 100644 --- a/src/NHibernate.Test/Async/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Async/Linq/ODataTests.cs @@ -87,6 +87,17 @@ public async Task BasePropertyFilterAsync(string queryString, int expectedRows) Assert.That(results, Has.Count.EqualTo(expectedRows)); } + [TestCase("$filter=CustomerId le 'ANATR'",2 )] + [TestCase("$filter=startswith(CustomerId, 'ANATR')", 1)] + [TestCase("$filter=endswith(CustomerId, 'ANATR')", 1)] + [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1)] + public async Task StringFilterAsync(string queryString, int expectedCount) + { + Assert.That( + await (ApplyFilter(session.Query(), queryString).Cast().ToListAsync()), + Has.Count.EqualTo(expectedCount)); + } + private IQueryable ApplyFilter(IQueryable query, string queryString) { var context = new ODataQueryContext(CreatEdmModel(), typeof(T), null) { }; diff --git a/src/NHibernate.Test/Linq/ODataTests.cs b/src/NHibernate.Test/Linq/ODataTests.cs index 572392110c4..ccae995f5e6 100644 --- a/src/NHibernate.Test/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Linq/ODataTests.cs @@ -75,6 +75,17 @@ public void BasePropertyFilter(string queryString, int expectedRows) Assert.That(results, Has.Count.EqualTo(expectedRows)); } + [TestCase("$filter=CustomerId le 'ANATR'",2 )] + [TestCase("$filter=startswith(CustomerId, 'ANATR')", 1)] + [TestCase("$filter=endswith(CustomerId, 'ANATR')", 1)] + [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1)] + public void StringFilter(string queryString, int expectedCount) + { + Assert.That( + ApplyFilter(session.Query(), queryString).Cast().ToList(), + Has.Count.EqualTo(expectedCount)); + } + private IQueryable ApplyFilter(IQueryable query, string queryString) { var context = new ODataQueryContext(CreatEdmModel(), typeof(T), null) { }; From 4a610aa707a5e82dd217ce4b5768e782a19986a4 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Sun, 17 May 2020 12:20:52 +0300 Subject: [PATCH 2/9] Fix --- src/NHibernate/Linq/Functions/CompareGenerator.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/NHibernate/Linq/Functions/CompareGenerator.cs b/src/NHibernate/Linq/Functions/CompareGenerator.cs index a4590cd06e8..0d382d71b2f 100644 --- a/src/NHibernate/Linq/Functions/CompareGenerator.cs +++ b/src/NHibernate/Linq/Functions/CompareGenerator.cs @@ -15,6 +15,7 @@ internal class CompareGenerator : BaseHqlGeneratorForMethod, IRuntimeMethodHqlGe private static readonly HashSet ActingMethods = new HashSet { ReflectHelper.FastGetMethod(string.Compare, default(string), default(string)), + ReflectHelper.FastGetMethod(string.Compare, default(string), default(string), default(StringComparison)), ReflectHelper.GetMethodDefinition(s => s.CompareTo(s)), ReflectHelper.GetMethodDefinition(x => x.CompareTo(x)), From e4b26350939256b2dcb86d9998118ea67368a422 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Sun, 17 May 2020 12:47:19 +0300 Subject: [PATCH 3/9] Fix locate function for SQLite --- src/NHibernate/Dialect/SQLiteDialect.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/NHibernate/Dialect/SQLiteDialect.cs b/src/NHibernate/Dialect/SQLiteDialect.cs index fa71342185f..26a2e6b37a5 100644 --- a/src/NHibernate/Dialect/SQLiteDialect.cs +++ b/src/NHibernate/Dialect/SQLiteDialect.cs @@ -105,6 +105,7 @@ protected virtual void RegisterFunctions() RegisterFunction("current_date", new SQLFunctionTemplate(NHibernateUtil.LocalDate, "datetime(date(current_timestamp, 'localtime'))")); RegisterFunction("substring", new StandardSQLFunction("substr", NHibernateUtil.String)); + RegisterFunction("locate", new StandardSQLFunction("instr", NHibernateUtil.Int32)); RegisterFunction("left", new SQLFunctionTemplate(NHibernateUtil.String, "substr(?1,1,?2)")); RegisterFunction("trim", new AnsiTrimEmulationFunction()); RegisterFunction("replace", new StandardSafeSQLFunction("replace", NHibernateUtil.String, 3)); From 66a356243bd111809fb19a5ce9f9cc70f896caa1 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Tue, 19 May 2020 11:10:38 +0300 Subject: [PATCH 4/9] Support with warn StringComparison in StartsWith, EndsWith, IndexOf, Replace --- src/NHibernate.Test/Async/Linq/ODataTests.cs | 1 + src/NHibernate.Test/Async/Linq/WhereTests.cs | 31 ++++++++++++ src/NHibernate.Test/Linq/ODataTests.cs | 1 + src/NHibernate.Test/Linq/WhereTests.cs | 30 ++++++++++++ .../Functions/BaseHqlGeneratorForMethod.cs | 27 +++++++++- .../Linq/Functions/CompareGenerator.cs | 7 ++- .../Linq/Functions/StringGenerator.cs | 49 ++++++++++++++----- 7 files changed, 133 insertions(+), 13 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/ODataTests.cs b/src/NHibernate.Test/Async/Linq/ODataTests.cs index c57e4fec768..f672a1ff388 100644 --- a/src/NHibernate.Test/Async/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Async/Linq/ODataTests.cs @@ -87,6 +87,7 @@ public async Task BasePropertyFilterAsync(string queryString, int expectedRows) Assert.That(results, Has.Count.EqualTo(expectedRows)); } + //GH-2362 [TestCase("$filter=CustomerId le 'ANATR'",2 )] [TestCase("$filter=startswith(CustomerId, 'ANATR')", 1)] [TestCase("$filter=endswith(CustomerId, 'ANATR')", 1)] diff --git a/src/NHibernate.Test/Async/Linq/WhereTests.cs b/src/NHibernate.Test/Async/Linq/WhereTests.cs index 44615d96277..7f9f3406190 100644 --- a/src/NHibernate.Test/Async/Linq/WhereTests.cs +++ b/src/NHibernate.Test/Async/Linq/WhereTests.cs @@ -14,14 +14,17 @@ using System.Collections.ObjectModel; using System.Linq; using System.Linq.Expressions; +using log4net.Core; using NHibernate.Engine.Query; using NHibernate.Linq; using NHibernate.DomainModel.Northwind.Entities; +using NHibernate.Linq.Functions; using NUnit.Framework; namespace NHibernate.Test.Linq { using System.Threading.Tasks; + using System.Threading; [TestFixture] public class WhereTestsAsync : LinqTestCase { @@ -430,6 +433,34 @@ public async Task UsersWithStringContainsAndNotNullNameHQLAsync() Assert.That(users.Count, Is.EqualTo(1)); } + [Test()] + public void StringComparisonParamEmitsWarningAsync() + { + Assert.Multiple( + async () => + { + await (AssertStringComparisonWarningAsync(x => string.Compare(x.CustomerId, "ANATR", StringComparison.Ordinal) <= 0, 2)); + await (AssertStringComparisonWarningAsync(x => x.CustomerId.StartsWith("ANATR", StringComparison.Ordinal), 1)); + await (AssertStringComparisonWarningAsync(x => x.CustomerId.EndsWith("ANATR", StringComparison.Ordinal), 1)); + await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1)); + await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1)); +#if NETCOREAPP2_0 + await (AssertStringComparisonWarningAsync(x => x.CustomerId.Replace("AN", "XX", StringComparison.Ordinal) == "XXATR", 1)); +#endif + }); + } + + private async Task AssertStringComparisonWarningAsync(Expression> whereParam, int expected, CancellationToken cancellationToken = default(CancellationToken)) + { + using (var log = new LogSpy(typeof(BaseHqlGeneratorForMethod))) + { + var customers = await (session.Query().Where(whereParam).ToListAsync(cancellationToken)); + + Assert.That(customers, Has.Count.EqualTo(expected), whereParam.ToString); + Assert.That(log.GetWholeLog(), Does.Contain($"parameter of type '{nameof(StringComparison)}' is ignored"), whereParam.ToString); + } + } + [Test] public async Task UsersWithArrayContainsAsync() { diff --git a/src/NHibernate.Test/Linq/ODataTests.cs b/src/NHibernate.Test/Linq/ODataTests.cs index ccae995f5e6..8a7e99b3532 100644 --- a/src/NHibernate.Test/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Linq/ODataTests.cs @@ -75,6 +75,7 @@ public void BasePropertyFilter(string queryString, int expectedRows) Assert.That(results, Has.Count.EqualTo(expectedRows)); } + //GH-2362 [TestCase("$filter=CustomerId le 'ANATR'",2 )] [TestCase("$filter=startswith(CustomerId, 'ANATR')", 1)] [TestCase("$filter=endswith(CustomerId, 'ANATR')", 1)] diff --git a/src/NHibernate.Test/Linq/WhereTests.cs b/src/NHibernate.Test/Linq/WhereTests.cs index b331d2604ef..c819b4c8504 100644 --- a/src/NHibernate.Test/Linq/WhereTests.cs +++ b/src/NHibernate.Test/Linq/WhereTests.cs @@ -4,9 +4,11 @@ using System.Collections.ObjectModel; using System.Linq; using System.Linq.Expressions; +using log4net.Core; using NHibernate.Engine.Query; using NHibernate.Linq; using NHibernate.DomainModel.Northwind.Entities; +using NHibernate.Linq.Functions; using NUnit.Framework; namespace NHibernate.Test.Linq @@ -419,6 +421,34 @@ public void UsersWithStringContainsAndNotNullNameHQL() Assert.That(users.Count, Is.EqualTo(1)); } + [Test()] + public void StringComparisonParamEmitsWarning() + { + Assert.Multiple( + () => + { + AssertStringComparisonWarning(x => string.Compare(x.CustomerId, "ANATR", StringComparison.Ordinal) <= 0, 2); + AssertStringComparisonWarning(x => x.CustomerId.StartsWith("ANATR", StringComparison.Ordinal), 1); + AssertStringComparisonWarning(x => x.CustomerId.EndsWith("ANATR", StringComparison.Ordinal), 1); + AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1); + AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1); +#if NETCOREAPP2_0 + AssertStringComparisonWarning(x => x.CustomerId.Replace("AN", "XX", StringComparison.Ordinal) == "XXATR", 1); +#endif + }); + } + + private void AssertStringComparisonWarning(Expression> whereParam, int expected) + { + using (var log = new LogSpy(typeof(BaseHqlGeneratorForMethod))) + { + var customers = session.Query().Where(whereParam).ToList(); + + Assert.That(customers, Has.Count.EqualTo(expected), whereParam.ToString); + Assert.That(log.GetWholeLog(), Does.Contain($"parameter of type '{nameof(StringComparison)}' is ignored"), whereParam.ToString); + } + } + [Test] public void UsersWithArrayContains() { diff --git a/src/NHibernate/Linq/Functions/BaseHqlGeneratorForMethod.cs b/src/NHibernate/Linq/Functions/BaseHqlGeneratorForMethod.cs index 53955d136a6..1239aa41950 100644 --- a/src/NHibernate/Linq/Functions/BaseHqlGeneratorForMethod.cs +++ b/src/NHibernate/Linq/Functions/BaseHqlGeneratorForMethod.cs @@ -1,5 +1,7 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Linq; using System.Linq.Expressions; using System.Reflection; using NHibernate.Hql.Ast; @@ -9,10 +11,33 @@ namespace NHibernate.Linq.Functions { public abstract class BaseHqlGeneratorForMethod : IHqlGeneratorForMethod, IHqlGeneratorForMethodExtended { + protected static readonly INHibernateLogger Log = NHibernateLogger.For(typeof(BaseHqlGeneratorForMethod)); + public IEnumerable SupportedMethods { get; protected set; } public abstract HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, ReadOnlyCollection arguments, HqlTreeBuilder treeBuilder, IHqlExpressionVisitor visitor); public virtual bool AllowsNullableReturnType(MethodInfo method) => true; + + private protected static void LogIgnoredParameter(MethodInfo method, string paramType) + { + if(Log.IsWarnEnabled()) + Log.Warn("Method parameter of type '{0}' is ignored when converting to hql the following method: {1}", paramType, method); + } + + private protected static void LogIgnoredStringComparisonParameter(MethodInfo actualMethod, MethodInfo methodWithStringComparison) + { + if(actualMethod == methodWithStringComparison) + LogIgnoredParameter(actualMethod, nameof(StringComparison)); + } + + private protected bool LogIgnoredStringComparisonParameter(MethodInfo actualMethod, params MethodInfo[] methodsWithStringComparison) + { + if (!methodsWithStringComparison.Contains(actualMethod)) + return false; + + LogIgnoredParameter(actualMethod, nameof(StringComparison)); + return true; + } } } diff --git a/src/NHibernate/Linq/Functions/CompareGenerator.cs b/src/NHibernate/Linq/Functions/CompareGenerator.cs index 0d382d71b2f..9fa966525e6 100644 --- a/src/NHibernate/Linq/Functions/CompareGenerator.cs +++ b/src/NHibernate/Linq/Functions/CompareGenerator.cs @@ -12,10 +12,12 @@ namespace NHibernate.Linq.Functions { internal class CompareGenerator : BaseHqlGeneratorForMethod, IRuntimeMethodHqlGenerator { + private static readonly MethodInfo MethodWithComparer = ReflectHelper.FastGetMethod(string.Compare, default(string), default(string), default(StringComparison)); + private static readonly HashSet ActingMethods = new HashSet { ReflectHelper.FastGetMethod(string.Compare, default(string), default(string)), - ReflectHelper.FastGetMethod(string.Compare, default(string), default(string), default(StringComparison)), + MethodWithComparer, ReflectHelper.GetMethodDefinition(s => s.CompareTo(s)), ReflectHelper.GetMethodDefinition(x => x.CompareTo(x)), @@ -44,7 +46,10 @@ internal class CompareGenerator : BaseHqlGeneratorForMethod, IRuntimeMethodHqlGe internal static bool IsCompareMethod(MethodInfo methodInfo) { if (ActingMethods.Contains(methodInfo)) + { + LogIgnoredStringComparisonParameter(methodInfo, MethodWithComparer); return true; + } // This is .Net 4 only, and in the System.Data.Services assembly, which we don't depend directly on. return methodInfo != null && methodInfo.Name == "Compare" && diff --git a/src/NHibernate/Linq/Functions/StringGenerator.cs b/src/NHibernate/Linq/Functions/StringGenerator.cs index 31edf4a42d1..53473d07fda 100644 --- a/src/NHibernate/Linq/Functions/StringGenerator.cs +++ b/src/NHibernate/Linq/Functions/StringGenerator.cs @@ -76,15 +76,18 @@ public override HqlTreeNode BuildHql(MemberInfo member, Expression expression, H public class StartsWithGenerator : BaseHqlGeneratorForMethod { + private static readonly MethodInfo MethodWithComparer = ReflectHelper.GetMethodDefinition(x => x.StartsWith(null, default(StringComparison))); + public StartsWithGenerator() { - SupportedMethods = new[] { ReflectHelper.GetMethodDefinition(x => x.StartsWith(null)) }; + SupportedMethods = new[] {ReflectHelper.GetMethodDefinition(x => x.StartsWith(null)), MethodWithComparer}; } public override bool AllowsNullableReturnType(MethodInfo method) => false; public override HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, ReadOnlyCollection arguments, HqlTreeBuilder treeBuilder, IHqlExpressionVisitor visitor) { + LogIgnoredStringComparisonParameter(method, MethodWithComparer); return treeBuilder.Like( visitor.Visit(targetObject).AsExpression(), treeBuilder.Concat( @@ -95,15 +98,18 @@ public override HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, public class EndsWithGenerator : BaseHqlGeneratorForMethod { + private static readonly MethodInfo MethodWithComparer = ReflectHelper.GetMethodDefinition(x => x.EndsWith(null, default(StringComparison))); + public EndsWithGenerator() { - SupportedMethods = new[] { ReflectHelper.GetMethodDefinition(x => x.EndsWith(null)) }; + SupportedMethods = new[] {ReflectHelper.GetMethodDefinition(x => x.EndsWith(null)), MethodWithComparer,}; } public override bool AllowsNullableReturnType(MethodInfo method) => false; public override HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, ReadOnlyCollection arguments, HqlTreeBuilder treeBuilder, IHqlExpressionVisitor visitor) { + LogIgnoredStringComparisonParameter(method, MethodWithComparer); return treeBuilder.Like( visitor.Visit(targetObject).AsExpression(), treeBuilder.Concat( @@ -210,6 +216,9 @@ public override HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, public class IndexOfGenerator : BaseHqlGeneratorForMethod { + private static readonly MethodInfo MethodWithComparer1 = ReflectHelper.GetMethodDefinition(x => x.IndexOf(string.Empty, default(StringComparison))); + private static readonly MethodInfo MethodWithComparer2 = ReflectHelper.GetMethodDefinition(x => x.IndexOf(string.Empty, 0, default(StringComparison))); + public IndexOfGenerator() { SupportedMethods = new[] @@ -217,11 +226,18 @@ public IndexOfGenerator() ReflectHelper.GetMethodDefinition(s => s.IndexOf(' ')), ReflectHelper.GetMethodDefinition(s => s.IndexOf(" ")), ReflectHelper.GetMethodDefinition(s => s.IndexOf(' ', 0)), - ReflectHelper.GetMethodDefinition(s => s.IndexOf(" ", 0)) + ReflectHelper.GetMethodDefinition(s => s.IndexOf(" ", 0)), + MethodWithComparer1, + MethodWithComparer2, }; } public override HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, ReadOnlyCollection arguments, HqlTreeBuilder treeBuilder, IHqlExpressionVisitor visitor) { + if (LogIgnoredStringComparisonParameter(method, MethodWithComparer1, MethodWithComparer2)) + { + arguments = arguments.Where(a => a.Type != typeof(StringComparison)).ToList().AsReadOnly(); + } + HqlMethodCall locate; if (arguments.Count == 1) { @@ -244,21 +260,32 @@ public override HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, public class ReplaceGenerator : BaseHqlGeneratorForMethod { +#if NETCOREAPP2_0 + private static readonly MethodInfo MethodWithComparer = ReflectHelper.GetMethodDefinition(x => x.Replace(string.Empty, string.Empty, default(StringComparison))); +#endif + public ReplaceGenerator() { SupportedMethods = new[] - { - ReflectHelper.GetMethodDefinition(s => s.Replace(' ', ' ')), - ReflectHelper.GetMethodDefinition(s => s.Replace("", "")) - }; + { + ReflectHelper.GetMethodDefinition(s => s.Replace(' ', ' ')), + ReflectHelper.GetMethodDefinition(s => s.Replace("", "")), +#if NETCOREAPP2_0 + MethodWithComparer, +#endif + }; } public override HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, ReadOnlyCollection arguments, HqlTreeBuilder treeBuilder, IHqlExpressionVisitor visitor) { - return treeBuilder.MethodCall("replace", - visitor.Visit(targetObject).AsExpression(), - visitor.Visit(arguments[0]).AsExpression(), - visitor.Visit(arguments[1]).AsExpression()); +#if NETCOREAPP2_0 + LogIgnoredStringComparisonParameter(method, MethodWithComparer); +#endif + return treeBuilder.MethodCall( + "replace", + visitor.Visit(targetObject).AsExpression(), + visitor.Visit(arguments[0]).AsExpression(), + visitor.Visit(arguments[1]).AsExpression()); } } From ccb0bfa35568e3afae288586acc77351486f0d43 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Thu, 21 May 2020 11:56:21 +0300 Subject: [PATCH 5/9] Revert "Fix locate function for SQLite" 'instr' supported since SQLite 3.7.15. So maybe better be placed in separate class or in separate PR --- src/NHibernate/Dialect/SQLiteDialect.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/NHibernate/Dialect/SQLiteDialect.cs b/src/NHibernate/Dialect/SQLiteDialect.cs index 26a2e6b37a5..fa71342185f 100644 --- a/src/NHibernate/Dialect/SQLiteDialect.cs +++ b/src/NHibernate/Dialect/SQLiteDialect.cs @@ -105,7 +105,6 @@ protected virtual void RegisterFunctions() RegisterFunction("current_date", new SQLFunctionTemplate(NHibernateUtil.LocalDate, "datetime(date(current_timestamp, 'localtime'))")); RegisterFunction("substring", new StandardSQLFunction("substr", NHibernateUtil.String)); - RegisterFunction("locate", new StandardSQLFunction("instr", NHibernateUtil.Int32)); RegisterFunction("left", new SQLFunctionTemplate(NHibernateUtil.String, "substr(?1,1,?2)")); RegisterFunction("trim", new AnsiTrimEmulationFunction()); RegisterFunction("replace", new StandardSafeSQLFunction("replace", NHibernateUtil.String, 3)); From 08ba31c6093cf57b3c1858129c55ab5fb3488ed2 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Thu, 21 May 2020 12:06:02 +0300 Subject: [PATCH 6/9] Skip locate on SQLite --- src/NHibernate.Test/Async/Linq/ODataTests.cs | 7 +++++-- src/NHibernate.Test/Async/Linq/WhereTests.cs | 7 +++++-- src/NHibernate.Test/Linq/ODataTests.cs | 7 +++++-- src/NHibernate.Test/Linq/WhereTests.cs | 7 +++++-- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/ODataTests.cs b/src/NHibernate.Test/Async/Linq/ODataTests.cs index f672a1ff388..92066481aeb 100644 --- a/src/NHibernate.Test/Async/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Async/Linq/ODataTests.cs @@ -91,9 +91,12 @@ public async Task BasePropertyFilterAsync(string queryString, int expectedRows) [TestCase("$filter=CustomerId le 'ANATR'",2 )] [TestCase("$filter=startswith(CustomerId, 'ANATR')", 1)] [TestCase("$filter=endswith(CustomerId, 'ANATR')", 1)] - [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1)] - public async Task StringFilterAsync(string queryString, int expectedCount) + [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1, true)] + public async Task StringFilterAsync(string queryString, int expectedCount, bool locateFunction = false) { + if(locateFunction && !TestDialect.SupportsLocate) + Assert.Ignore("Locate function is not supported."); + Assert.That( await (ApplyFilter(session.Query(), queryString).Cast().ToListAsync()), Has.Count.EqualTo(expectedCount)); diff --git a/src/NHibernate.Test/Async/Linq/WhereTests.cs b/src/NHibernate.Test/Async/Linq/WhereTests.cs index 7f9f3406190..dc09825ca91 100644 --- a/src/NHibernate.Test/Async/Linq/WhereTests.cs +++ b/src/NHibernate.Test/Async/Linq/WhereTests.cs @@ -442,8 +442,11 @@ public void StringComparisonParamEmitsWarningAsync() await (AssertStringComparisonWarningAsync(x => string.Compare(x.CustomerId, "ANATR", StringComparison.Ordinal) <= 0, 2)); await (AssertStringComparisonWarningAsync(x => x.CustomerId.StartsWith("ANATR", StringComparison.Ordinal), 1)); await (AssertStringComparisonWarningAsync(x => x.CustomerId.EndsWith("ANATR", StringComparison.Ordinal), 1)); - await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1)); - await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1)); + if (TestDialect.SupportsLocate) + { + await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1)); + await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1)); + } #if NETCOREAPP2_0 await (AssertStringComparisonWarningAsync(x => x.CustomerId.Replace("AN", "XX", StringComparison.Ordinal) == "XXATR", 1)); #endif diff --git a/src/NHibernate.Test/Linq/ODataTests.cs b/src/NHibernate.Test/Linq/ODataTests.cs index 8a7e99b3532..2cef3e9aeb3 100644 --- a/src/NHibernate.Test/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Linq/ODataTests.cs @@ -79,9 +79,12 @@ public void BasePropertyFilter(string queryString, int expectedRows) [TestCase("$filter=CustomerId le 'ANATR'",2 )] [TestCase("$filter=startswith(CustomerId, 'ANATR')", 1)] [TestCase("$filter=endswith(CustomerId, 'ANATR')", 1)] - [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1)] - public void StringFilter(string queryString, int expectedCount) + [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1, true)] + public void StringFilter(string queryString, int expectedCount, bool locateFunction = false) { + if(locateFunction && !TestDialect.SupportsLocate) + Assert.Ignore("Locate function is not supported."); + Assert.That( ApplyFilter(session.Query(), queryString).Cast().ToList(), Has.Count.EqualTo(expectedCount)); diff --git a/src/NHibernate.Test/Linq/WhereTests.cs b/src/NHibernate.Test/Linq/WhereTests.cs index c819b4c8504..7f76b31fc55 100644 --- a/src/NHibernate.Test/Linq/WhereTests.cs +++ b/src/NHibernate.Test/Linq/WhereTests.cs @@ -430,8 +430,11 @@ public void StringComparisonParamEmitsWarning() AssertStringComparisonWarning(x => string.Compare(x.CustomerId, "ANATR", StringComparison.Ordinal) <= 0, 2); AssertStringComparisonWarning(x => x.CustomerId.StartsWith("ANATR", StringComparison.Ordinal), 1); AssertStringComparisonWarning(x => x.CustomerId.EndsWith("ANATR", StringComparison.Ordinal), 1); - AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1); - AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1); + if (TestDialect.SupportsLocate) + { + AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1); + AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1); + } #if NETCOREAPP2_0 AssertStringComparisonWarning(x => x.CustomerId.Replace("AN", "XX", StringComparison.Ordinal) == "XXATR", 1); #endif From 64be7b642a2cd996e880ff8394a5fc7d875657f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Sun, 24 May 2020 19:32:58 +0200 Subject: [PATCH 7/9] Fix some whitespace glitches --- src/NHibernate.Test/Async/Linq/ODataTests.cs | 2 +- src/NHibernate.Test/Linq/ODataTests.cs | 2 +- src/NHibernate/Linq/Functions/BaseHqlGeneratorForMethod.cs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/ODataTests.cs b/src/NHibernate.Test/Async/Linq/ODataTests.cs index 92066481aeb..24f550d30b7 100644 --- a/src/NHibernate.Test/Async/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Async/Linq/ODataTests.cs @@ -94,7 +94,7 @@ public async Task BasePropertyFilterAsync(string queryString, int expectedRows) [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1, true)] public async Task StringFilterAsync(string queryString, int expectedCount, bool locateFunction = false) { - if(locateFunction && !TestDialect.SupportsLocate) + if (locateFunction && !TestDialect.SupportsLocate) Assert.Ignore("Locate function is not supported."); Assert.That( diff --git a/src/NHibernate.Test/Linq/ODataTests.cs b/src/NHibernate.Test/Linq/ODataTests.cs index 2cef3e9aeb3..78630cb8ba1 100644 --- a/src/NHibernate.Test/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Linq/ODataTests.cs @@ -82,7 +82,7 @@ public void BasePropertyFilter(string queryString, int expectedRows) [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1, true)] public void StringFilter(string queryString, int expectedCount, bool locateFunction = false) { - if(locateFunction && !TestDialect.SupportsLocate) + if (locateFunction && !TestDialect.SupportsLocate) Assert.Ignore("Locate function is not supported."); Assert.That( diff --git a/src/NHibernate/Linq/Functions/BaseHqlGeneratorForMethod.cs b/src/NHibernate/Linq/Functions/BaseHqlGeneratorForMethod.cs index 1239aa41950..bf97cc54cb7 100644 --- a/src/NHibernate/Linq/Functions/BaseHqlGeneratorForMethod.cs +++ b/src/NHibernate/Linq/Functions/BaseHqlGeneratorForMethod.cs @@ -21,13 +21,13 @@ public abstract class BaseHqlGeneratorForMethod : IHqlGeneratorForMethod, IHqlGe private protected static void LogIgnoredParameter(MethodInfo method, string paramType) { - if(Log.IsWarnEnabled()) + if (Log.IsWarnEnabled()) Log.Warn("Method parameter of type '{0}' is ignored when converting to hql the following method: {1}", paramType, method); } private protected static void LogIgnoredStringComparisonParameter(MethodInfo actualMethod, MethodInfo methodWithStringComparison) { - if(actualMethod == methodWithStringComparison) + if (actualMethod == methodWithStringComparison) LogIgnoredParameter(actualMethod, nameof(StringComparison)); } From aa706077b17c2723df43563ef0dfe14ed28a5b17 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Sun, 24 May 2020 20:41:36 +0300 Subject: [PATCH 8/9] Avoid ToList for arguments --- src/NHibernate/Linq/Functions/StringGenerator.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/NHibernate/Linq/Functions/StringGenerator.cs b/src/NHibernate/Linq/Functions/StringGenerator.cs index 53473d07fda..5e358f22194 100644 --- a/src/NHibernate/Linq/Functions/StringGenerator.cs +++ b/src/NHibernate/Linq/Functions/StringGenerator.cs @@ -233,13 +233,15 @@ public IndexOfGenerator() } public override HqlTreeNode BuildHql(MethodInfo method, Expression targetObject, ReadOnlyCollection arguments, HqlTreeBuilder treeBuilder, IHqlExpressionVisitor visitor) { + var argsCount = arguments.Count; if (LogIgnoredStringComparisonParameter(method, MethodWithComparer1, MethodWithComparer2)) { - arguments = arguments.Where(a => a.Type != typeof(StringComparison)).ToList().AsReadOnly(); + //StringComparison is last argument, just ignore it + argsCount--; } HqlMethodCall locate; - if (arguments.Count == 1) + if (argsCount == 1) { locate = treeBuilder.MethodCall("locate", visitor.Visit(arguments[0]).AsExpression(), From a2421a47cd2cd273a7d3c03ac294b3422a112d54 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 25 May 2020 10:19:46 +0300 Subject: [PATCH 9/9] Remove SupportsLocate checks --- src/NHibernate.Test/Async/Linq/ODataTests.cs | 9 +++------ src/NHibernate.Test/Async/Linq/WhereTests.cs | 11 ++++------- src/NHibernate.Test/Linq/ODataTests.cs | 9 +++------ src/NHibernate.Test/Linq/WhereTests.cs | 11 ++++------- 4 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/ODataTests.cs b/src/NHibernate.Test/Async/Linq/ODataTests.cs index 24f550d30b7..ec0cd901f11 100644 --- a/src/NHibernate.Test/Async/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Async/Linq/ODataTests.cs @@ -88,15 +88,12 @@ public async Task BasePropertyFilterAsync(string queryString, int expectedRows) } //GH-2362 - [TestCase("$filter=CustomerId le 'ANATR'",2 )] + [TestCase("$filter=CustomerId le 'ANATR'", 2)] [TestCase("$filter=startswith(CustomerId, 'ANATR')", 1)] [TestCase("$filter=endswith(CustomerId, 'ANATR')", 1)] - [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1, true)] - public async Task StringFilterAsync(string queryString, int expectedCount, bool locateFunction = false) + [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1)] + public async Task StringFilterAsync(string queryString, int expectedCount) { - if (locateFunction && !TestDialect.SupportsLocate) - Assert.Ignore("Locate function is not supported."); - Assert.That( await (ApplyFilter(session.Query(), queryString).Cast().ToListAsync()), Has.Count.EqualTo(expectedCount)); diff --git a/src/NHibernate.Test/Async/Linq/WhereTests.cs b/src/NHibernate.Test/Async/Linq/WhereTests.cs index dc09825ca91..3afa7cda451 100644 --- a/src/NHibernate.Test/Async/Linq/WhereTests.cs +++ b/src/NHibernate.Test/Async/Linq/WhereTests.cs @@ -433,7 +433,7 @@ public async Task UsersWithStringContainsAndNotNullNameHQLAsync() Assert.That(users.Count, Is.EqualTo(1)); } - [Test()] + [Test] public void StringComparisonParamEmitsWarningAsync() { Assert.Multiple( @@ -442,18 +442,15 @@ public void StringComparisonParamEmitsWarningAsync() await (AssertStringComparisonWarningAsync(x => string.Compare(x.CustomerId, "ANATR", StringComparison.Ordinal) <= 0, 2)); await (AssertStringComparisonWarningAsync(x => x.CustomerId.StartsWith("ANATR", StringComparison.Ordinal), 1)); await (AssertStringComparisonWarningAsync(x => x.CustomerId.EndsWith("ANATR", StringComparison.Ordinal), 1)); - if (TestDialect.SupportsLocate) - { - await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1)); - await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1)); - } + await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1)); + await (AssertStringComparisonWarningAsync(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1)); #if NETCOREAPP2_0 await (AssertStringComparisonWarningAsync(x => x.CustomerId.Replace("AN", "XX", StringComparison.Ordinal) == "XXATR", 1)); #endif }); } - private async Task AssertStringComparisonWarningAsync(Expression> whereParam, int expected, CancellationToken cancellationToken = default(CancellationToken)) + private async Task AssertStringComparisonWarningAsync(Expression> whereParam, int expected, CancellationToken cancellationToken = default(CancellationToken)) { using (var log = new LogSpy(typeof(BaseHqlGeneratorForMethod))) { diff --git a/src/NHibernate.Test/Linq/ODataTests.cs b/src/NHibernate.Test/Linq/ODataTests.cs index 78630cb8ba1..bdb1ee7c592 100644 --- a/src/NHibernate.Test/Linq/ODataTests.cs +++ b/src/NHibernate.Test/Linq/ODataTests.cs @@ -76,15 +76,12 @@ public void BasePropertyFilter(string queryString, int expectedRows) } //GH-2362 - [TestCase("$filter=CustomerId le 'ANATR'",2 )] + [TestCase("$filter=CustomerId le 'ANATR'", 2)] [TestCase("$filter=startswith(CustomerId, 'ANATR')", 1)] [TestCase("$filter=endswith(CustomerId, 'ANATR')", 1)] - [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1, true)] - public void StringFilter(string queryString, int expectedCount, bool locateFunction = false) + [TestCase("$filter=indexof(CustomerId, 'ANATR') eq 0", 1)] + public void StringFilter(string queryString, int expectedCount) { - if (locateFunction && !TestDialect.SupportsLocate) - Assert.Ignore("Locate function is not supported."); - Assert.That( ApplyFilter(session.Query(), queryString).Cast().ToList(), Has.Count.EqualTo(expectedCount)); diff --git a/src/NHibernate.Test/Linq/WhereTests.cs b/src/NHibernate.Test/Linq/WhereTests.cs index 7f76b31fc55..f991df12030 100644 --- a/src/NHibernate.Test/Linq/WhereTests.cs +++ b/src/NHibernate.Test/Linq/WhereTests.cs @@ -421,7 +421,7 @@ public void UsersWithStringContainsAndNotNullNameHQL() Assert.That(users.Count, Is.EqualTo(1)); } - [Test()] + [Test] public void StringComparisonParamEmitsWarning() { Assert.Multiple( @@ -430,18 +430,15 @@ public void StringComparisonParamEmitsWarning() AssertStringComparisonWarning(x => string.Compare(x.CustomerId, "ANATR", StringComparison.Ordinal) <= 0, 2); AssertStringComparisonWarning(x => x.CustomerId.StartsWith("ANATR", StringComparison.Ordinal), 1); AssertStringComparisonWarning(x => x.CustomerId.EndsWith("ANATR", StringComparison.Ordinal), 1); - if (TestDialect.SupportsLocate) - { - AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1); - AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1); - } + AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", StringComparison.Ordinal) == 0, 1); + AssertStringComparisonWarning(x => x.CustomerId.IndexOf("ANATR", 0, StringComparison.Ordinal) == 0, 1); #if NETCOREAPP2_0 AssertStringComparisonWarning(x => x.CustomerId.Replace("AN", "XX", StringComparison.Ordinal) == "XXATR", 1); #endif }); } - private void AssertStringComparisonWarning(Expression> whereParam, int expected) + private void AssertStringComparisonWarning(Expression> whereParam, int expected) { using (var log = new LogSpy(typeof(BaseHqlGeneratorForMethod))) {