From 90076b57312e58307a0fb3f33dfe1c15e195da0e Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Fri, 20 Nov 2020 14:15:21 -0800 Subject: [PATCH] Query: Don't terminate translation if indexer method does not bind to indexer property Resolves #23410 --- ...lationalSqlTranslatingExpressionVisitor.cs | 8 +- .../Query/QueryBugsTest.cs | 166 +++++++++++++++++- 2 files changed, 172 insertions(+), 2 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs index 9d1b2c652ca..3c655cac61a 100644 --- a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs @@ -465,7 +465,13 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp // EF Indexer property if (methodCallExpression.TryGetIndexerArguments(_model, out source, out propertyName)) { - return TryBindMember(Visit(source), MemberIdentity.Create(propertyName)); + var result = TryBindMember(Visit(source), MemberIdentity.Create(propertyName)); + var useOldBehavior = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23410", out var enabled) && enabled; + if (result != null + || useOldBehavior) + { + return result; + } } // GroupBy Aggregate case diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs index 56f76a7b1f1..2f0825f2a3b 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs @@ -17,13 +17,16 @@ using Microsoft.EntityFrameworkCore.Diagnostics.Internal; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Query.Internal; +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Storage.ValueConversion; using Microsoft.EntityFrameworkCore.TestUtilities; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; using NetTopologySuite.Geometries; +using Newtonsoft.Json.Linq; using Xunit; using Xunit.Abstractions; @@ -9308,7 +9311,7 @@ public virtual void Can_query_point_with_buffered_data_reader() @"SELECT TOP(1) [l].[Id], [l].[Name], [l].[Address_County], [l].[Address_Line1], [l].[Address_Line2], [l].[Address_Point], [l].[Address_Postcode], [l].[Address_Town] FROM [Locations] AS [l] WHERE [l].[Name] = N'My Location'" }); - } + } [Owned] private class Address23282 @@ -9379,6 +9382,167 @@ public MyContext23282(DbContextOptions options) #endregion + #region Issue23410 + + // TODO: Remove when JSON is first class. See issue#4021 + + [ConditionalFact] + public virtual void Method_call_translators_are_invoked_for_indexer_if_not_indexer_property() + { + var (options, testSqlLoggerFactory) = CreateOptions23410(); + using var context = new MyContext23410(options); + + var testUser = context.Blogs.FirstOrDefault(x => x.JObject["Author"].Value() == "Maumar"); + + Assert.NotNull(testUser); + + testSqlLoggerFactory.AssertBaseline( + new[] { + @"SELECT TOP(1) [b].[Id], [b].[JObject], [b].[Name] +FROM [Blogs] AS [b] +WHERE JSON_VALUE([b].[JObject], '$.Author') = N'Maumar'" }); + } + + private class Blog23410 + { + public int Id { get; set; } + + public string Name { get; set; } + public JObject JObject { get; set; } + } + + private class MyContext23410 : DbContext + { + public DbSet Blogs { get; set; } + + public MyContext23410(DbContextOptions options) + : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity().Property(e => e.JObject).HasConversion( + e => e.ToString(), + e => JObject.Parse(e)); + } + } + + private class JsonMethodCallTranslatorPlugin : IMethodCallTranslatorPlugin + { + public JsonMethodCallTranslatorPlugin(ISqlExpressionFactory sqlExpressionFactory) + { + Translators = new IMethodCallTranslator[] + { + new JsonIndexerMethodTranslator(sqlExpressionFactory), + new JsonValueMethodTranslator(sqlExpressionFactory) + }; + } + + public IEnumerable Translators { get; } + } + + private class JsonValueMethodTranslator : IMethodCallTranslator + { + private readonly ISqlExpressionFactory _sqlExpressionFactory; + + public JsonValueMethodTranslator(ISqlExpressionFactory sqlExpressionFactory) + { + _sqlExpressionFactory = sqlExpressionFactory; + } + + public SqlExpression Translate( + SqlExpression instance, + MethodInfo method, + IReadOnlyList arguments, + IDiagnosticsLogger logger) + { + if (method.IsGenericMethod + && method.DeclaringType == typeof(Newtonsoft.Json.Linq.Extensions) + && method.Name == "Value" + && arguments.Count == 1 + && arguments[0] is SqlFunctionExpression sqlFunctionExpression) + { + return _sqlExpressionFactory.Function( + sqlFunctionExpression.Name, + sqlFunctionExpression.Arguments, + sqlFunctionExpression.IsNullable, + sqlFunctionExpression.ArgumentsPropagateNullability, + method.ReturnType); + } + + return null; + } + } + + private class JsonIndexerMethodTranslator : IMethodCallTranslator + { + private readonly MethodInfo _indexerMethod = typeof(JObject).GetRuntimeMethod("get_Item", new[] { typeof(string) }); + + private readonly ISqlExpressionFactory _sqlExpressionFactory; + + public JsonIndexerMethodTranslator(ISqlExpressionFactory sqlExpressionFactory) + { + _sqlExpressionFactory = sqlExpressionFactory; + } + + public SqlExpression Translate( + SqlExpression instance, + MethodInfo method, + IReadOnlyList arguments, + IDiagnosticsLogger logger) + { + if (Equals(_indexerMethod, method)) + { + return _sqlExpressionFactory.Function( + "JSON_VALUE", + new[] { + instance, + _sqlExpressionFactory.Fragment($"'$.{((SqlConstantExpression)arguments[0]).Value}'") + }, + nullable: true, + argumentsPropagateNullability: new[] { true, false }, + _indexerMethod.ReturnType); + } + + return null; + } + } + + private (DbContextOptions, TestSqlLoggerFactory) CreateOptions23410() + { + var testStore = SqlServerTestStore.CreateInitialized("QueryBugsTest"); + var testSqlLoggerFactory = new TestSqlLoggerFactory(); + var serviceCollection = new ServiceCollection() + .AddSingleton(testSqlLoggerFactory) + .AddEntityFrameworkSqlServer(); + serviceCollection.TryAddEnumerable(new ServiceDescriptor( + typeof(IMethodCallTranslatorPlugin), typeof(JsonMethodCallTranslatorPlugin), ServiceLifetime.Singleton)); + var serviceProvider = serviceCollection.BuildServiceProvider(); + + var optionsBuilder = Fixture.AddOptions(testStore.AddProviderOptions(new DbContextOptionsBuilder())) + .EnableDetailedErrors() + .UseInternalServiceProvider(serviceProvider) + .EnableServiceProviderCaching(false); + + var context = new MyContext23410(optionsBuilder.Options); + if (context.Database.EnsureCreatedResiliently()) + { + context.Blogs.Add(new Blog23410 + { + Name = "My Location", + JObject = JObject.Parse(@"{ ""Author"": ""Maumar"" }") + }); + context.SaveChanges(); + } + + testSqlLoggerFactory.Clear(); + + return (optionsBuilder.Options, testSqlLoggerFactory); + } + + #endregion + private DbContextOptions _options; private SqlServerTestStore CreateTestStore(