Fix to #30244 - Json column fails to materialize only when default in…

…terceptor used (#30403)

* Fix to #30244 - Json column fails to materialize only when default interceptor used

Problem was that when we build materializer expression, ValueBufferTryReadValue call that extracts values of keys is typed with key's CLR type. This fails for json as we visit the materializer expression, because calls to ValueBufferTryReadValue are replaced with array accesses to object[] that we store key values in. We should be using ValueBufferTryReadValue<object> when trying to get value for keys, like we do everywhere else, so that after the visitation expression types are correct.

Fixes #30244

* Move testing to the interceptor tests, which are setup for singleton options

src/EFCore/Query/Internal/EntityMaterializerSource.cs

@@ -372,8 +372,14 @@ BlockExpression CreateAccessorDictionaryExpression()
                 Expression CreateAccessorReadExpression()
                     => property is IServiceProperty serviceProperty
                         ? serviceProperty.ParameterBinding.BindToParameter(bindingInfo)
-                        : valueBufferExpression
-                            .CreateValueBufferReadValueExpression(
+                        : (property as IProperty)?.IsPrimaryKey() == true
+                            ? Expression.Convert(
+                                valueBufferExpression.CreateValueBufferReadValueExpression(
+                                    typeof(object),
+                                    property.GetIndex(),
+                                    property),
+                                property.ClrType)
+                            : valueBufferExpression.CreateValueBufferReadValueExpression(
                                 property.ClrType,
                                 property.GetIndex(),
                                 property);

test/EFCore.Cosmos.FunctionalTests/MaterializationInterceptionCosmosTest.cs

@@ -3,14 +3,32 @@
 
 namespace Microsoft.EntityFrameworkCore.Cosmos;
 
-public class MaterializationInterceptionCosmosTest : MaterializationInterceptionTestBase,
+public class MaterializationInterceptionCosmosTest : MaterializationInterceptionTestBase<MaterializationInterceptionCosmosTest.CosmosLibraryContext>,
     IClassFixture<MaterializationInterceptionCosmosTest.MaterializationInterceptionCosmosFixture>
 {
     public MaterializationInterceptionCosmosTest(MaterializationInterceptionCosmosFixture fixture)
         : base(fixture)
     {
     }
 
+    public class CosmosLibraryContext : LibraryContext
+    {
+        public CosmosLibraryContext(DbContextOptions options)
+            : base(options)
+        {
+        }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            base.OnModelCreating(modelBuilder);
+
+            modelBuilder.Entity<TestEntity30244>();
+        }
+    }
+
+    public override LibraryContext CreateContext(IEnumerable<ISingletonInterceptor> interceptors, bool inject)
+        => new CosmosLibraryContext(Fixture.CreateOptions(interceptors, inject));
+
     public class MaterializationInterceptionCosmosFixture : SingletonInterceptorsFixtureBase
     {
         protected override string StoreName

test/EFCore.InMemory.FunctionalTests/MaterializationInterceptionInMemoryTest.cs

@@ -5,14 +5,32 @@
 
 namespace Microsoft.EntityFrameworkCore;
 
-public class MaterializationInterceptionInMemoryTest : MaterializationInterceptionTestBase,
+public class MaterializationInterceptionInMemoryTest : MaterializationInterceptionTestBase<MaterializationInterceptionInMemoryTest.InMemoryLibraryContext>,
     IClassFixture<MaterializationInterceptionInMemoryTest.MaterializationInterceptionInMemoryFixture>
 {
     public MaterializationInterceptionInMemoryTest(MaterializationInterceptionInMemoryFixture fixture)
         : base(fixture)
     {
     }
 
+    public class InMemoryLibraryContext : LibraryContext
+    {
+        public InMemoryLibraryContext(DbContextOptions options)
+            : base(options)
+        {
+        }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            base.OnModelCreating(modelBuilder);
+
+            modelBuilder.Entity<TestEntity30244>().OwnsMany(e => e.Settings);
+        }
+    }
+
+    public override LibraryContext CreateContext(IEnumerable<ISingletonInterceptor> interceptors, bool inject)
+        => new InMemoryLibraryContext(Fixture.CreateOptions(interceptors, inject));
+
     public class MaterializationInterceptionInMemoryFixture : SingletonInterceptorsFixtureBase
     {
         protected override string StoreName

test/EFCore.Relational.Specification.Tests/Query/JsonQueryAdHocTestBase.cs

@@ -322,7 +322,7 @@ public virtual async Task Predicate_based_on_element_of_json_array_of_primitives
             else
             {
                 Assert.Throws<InvalidOperationException>(() => query.ToList());
-            }    
+            }
         }
     }
 

test/EFCore.Specification.Tests/MaterializationInterceptionTestBase.cs

@@ -5,7 +5,8 @@
 
 namespace Microsoft.EntityFrameworkCore;
 
-public abstract class MaterializationInterceptionTestBase : SingletonInterceptorsTestBase
+public abstract class MaterializationInterceptionTestBase<TContext> : SingletonInterceptorsTestBase<TContext>
+    where TContext : DbContext
 {
     protected MaterializationInterceptionTestBase(SingletonInterceptorsFixtureBase fixture)
         : base(fixture)
@@ -169,6 +170,97 @@ public virtual void Intercept_query_materialization_for_empty_constructor(bool i
         }
     }
 
+    private static int _id;
+
+    [ConditionalTheory] // Issue #30244
+    [InlineData(false)]
+    [InlineData(true)]
+    public virtual async Task Intercept_query_materialization_with_owned_types(bool async)
+    {
+        var creatingInstanceCounts = new Dictionary<Type, int>();
+        var createdInstanceCounts = new Dictionary<Type, int>();
+        var initializingInstanceCounts = new Dictionary<Type, int>();
+        var initializedInstanceCounts = new Dictionary<Type, int>();
+        LibraryContext? context = null;
+
+        var interceptors = new[]
+        {
+            new ValidatingMaterializationInterceptor(
+                (data, instance, method) =>
+                {
+                    Assert.Same(context, data.Context);
+                    Assert.Equal(QueryTrackingBehavior.TrackAll, data.QueryTrackingBehavior);
+
+                    int count;
+                    var clrType = data.EntityType.ClrType;
+                    switch (method)
+                    {
+                        case nameof(IMaterializationInterceptor.CreatingInstance):
+                            count = creatingInstanceCounts.GetOrAddNew(clrType);
+                            creatingInstanceCounts[clrType] = count + 1;
+                            Assert.Null(instance);
+                            break;
+                        case nameof(IMaterializationInterceptor.CreatedInstance):
+                            count = createdInstanceCounts.GetOrAddNew(clrType);
+                            createdInstanceCounts[clrType] = count + 1;
+                            Assert.Same(clrType, instance!.GetType());
+                            break;
+                        case nameof(IMaterializationInterceptor.InitializingInstance):
+                            count = initializingInstanceCounts.GetOrAddNew(clrType);
+                            initializingInstanceCounts[clrType] = count + 1;
+                            Assert.Same(clrType, instance!.GetType());
+                            break;
+                        case nameof(IMaterializationInterceptor.InitializedInstance):
+                            count = initializedInstanceCounts.GetOrAddNew(clrType);
+                            initializedInstanceCounts[clrType] = count + 1;
+                            Assert.Same(clrType, instance!.GetType());
+                            break;
+                    }
+                })
+        };
+
+        using (context = CreateContext(interceptors, inject: true))
+        {
+            context.Add(
+                new TestEntity30244
+                {
+                    Id = _id++,
+                    Name = "TestIssue",
+                    Settings = { new("Value1", "1"), new("Value2", "9") }
+                });
+
+            _ = async
+                ? await context.SaveChangesAsync()
+                : context.SaveChanges();
+
+            context.ChangeTracker.Clear();
+
+            var entity = async
+                ? await context.Set<TestEntity30244>().OrderBy(e => e.Id).FirstOrDefaultAsync()
+                : context.Set<TestEntity30244>().OrderBy(e => e.Id).FirstOrDefault();
+
+            Assert.NotNull(entity);
+            Assert.Contains(("Value1", "1"), entity.Settings.Select(e => (e.Key, e.Value)));
+            Assert.Contains(("Value2", "9"), entity.Settings.Select(e => (e.Key, e.Value)));
+
+            Assert.Equal(2, creatingInstanceCounts.Count);
+            Assert.Equal(1, creatingInstanceCounts[typeof(TestEntity30244)]);
+            Assert.Equal(2, creatingInstanceCounts[typeof(KeyValueSetting30244)]);
+
+            Assert.Equal(2, createdInstanceCounts.Count);
+            Assert.Equal(1, createdInstanceCounts[typeof(TestEntity30244)]);
+            Assert.Equal(2, createdInstanceCounts[typeof(KeyValueSetting30244)]);
+
+            Assert.Equal(2, initializingInstanceCounts.Count);
+            Assert.Equal(1, initializingInstanceCounts[typeof(TestEntity30244)]);
+            Assert.Equal(2, initializingInstanceCounts[typeof(KeyValueSetting30244)]);
+
+            Assert.Equal(2, initializedInstanceCounts.Count);
+            Assert.Equal(1, initializedInstanceCounts[typeof(TestEntity30244)]);
+            Assert.Equal(2, initializedInstanceCounts[typeof(KeyValueSetting30244)]);
+        }
+    }
+
     [ConditionalTheory]
     [InlineData(false)]
     [InlineData(true)]

test/EFCore.Specification.Tests/SingletonInterceptorsTestBase.cs

@@ -7,7 +7,8 @@
 
 namespace Microsoft.EntityFrameworkCore;
 
-public abstract class SingletonInterceptorsTestBase
+public abstract class SingletonInterceptorsTestBase<TContext>
+    where TContext : DbContext
 {
     protected SingletonInterceptorsTestBase(SingletonInterceptorsFixtureBase fixture)
     {
@@ -46,9 +47,31 @@ public Pamphlet(Guid id, string? title)
         public string? Title { get; set; }
     }
 
-    public class LibraryContext : PoolableDbContext
+    public class TestEntity30244
     {
-        public LibraryContext(DbContextOptions options)
+        [DatabaseGenerated((DatabaseGeneratedOption.None))]
+        public int Id { get; set; }
+
+        public string? Name { get; set; }
+        public List<KeyValueSetting30244> Settings { get; } = new();
+    }
+
+    public class KeyValueSetting30244
+    {
+        public KeyValueSetting30244(string key, string value)
+        {
+            Key = key;
+            Value = value;
+        }
+
+        public string Key { get; set; }
+        public string Value { get; set; }
+    }
+
+
+    public abstract class LibraryContext : PoolableDbContext
+    {
+        protected LibraryContext(DbContextOptions options)
             : base(options)
         {
         }
@@ -69,10 +92,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
         }
     }
 
-    public LibraryContext CreateContext(IEnumerable<ISingletonInterceptor> interceptors, bool inject)
-        => new(Fixture.CreateOptions(interceptors, inject));
+    public abstract LibraryContext CreateContext(IEnumerable<ISingletonInterceptor> interceptors, bool inject);
 
-    public abstract class SingletonInterceptorsFixtureBase : SharedStoreFixtureBase<LibraryContext>
+    public abstract class SingletonInterceptorsFixtureBase : SharedStoreFixtureBase<TContext>
     {
         public virtual DbContextOptions CreateOptions(IEnumerable<ISingletonInterceptor> interceptors, bool inject)
         {

test/EFCore.SqlServer.FunctionalTests/MaterializationInterceptionSqlServerTest.cs

@@ -7,14 +7,32 @@
 
 namespace Microsoft.EntityFrameworkCore;
 
-public class MaterializationInterceptionSqlServerTest : MaterializationInterceptionTestBase,
+public class MaterializationInterceptionSqlServerTest : MaterializationInterceptionTestBase<MaterializationInterceptionSqlServerTest.SqlServerLibraryContext>,
     IClassFixture<MaterializationInterceptionSqlServerTest.MaterializationInterceptionSqlServerFixture>
 {
     public MaterializationInterceptionSqlServerTest(MaterializationInterceptionSqlServerFixture fixture)
         : base(fixture)
     {
     }
 
+    public class SqlServerLibraryContext : LibraryContext
+    {
+        public SqlServerLibraryContext(DbContextOptions options)
+            : base(options)
+        {
+        }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            base.OnModelCreating(modelBuilder);
+
+            modelBuilder.Entity<TestEntity30244>().OwnsMany(e => e.Settings, b => b.ToJson());
+        }
+    }
+
+    public override LibraryContext CreateContext(IEnumerable<ISingletonInterceptor> interceptors, bool inject)
+        => new SqlServerLibraryContext(Fixture.CreateOptions(interceptors, inject));
+
     public class MaterializationInterceptionSqlServerFixture : SingletonInterceptorsFixtureBase
     {
         protected override string StoreName

test/EFCore.Sqlite.FunctionalTests/MaterializationInterceptionSqliteTest.cs

@@ -5,14 +5,32 @@
 
 namespace Microsoft.EntityFrameworkCore;
 
-public class MaterializationInterceptionSqliteTest : MaterializationInterceptionTestBase,
+public class MaterializationInterceptionSqliteTest : MaterializationInterceptionTestBase<MaterializationInterceptionSqliteTest.SqliteLibraryContext>,
     IClassFixture<MaterializationInterceptionSqliteTest.MaterializationInterceptionSqliteFixture>
 {
     public MaterializationInterceptionSqliteTest(MaterializationInterceptionSqliteFixture fixture)
         : base(fixture)
     {
     }
 
+    public class SqliteLibraryContext : LibraryContext
+    {
+        public SqliteLibraryContext(DbContextOptions options)
+            : base(options)
+        {
+        }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            base.OnModelCreating(modelBuilder);
+
+            modelBuilder.Entity<TestEntity30244>().OwnsMany(e => e.Settings, b => b.ToJson());
+        }
+    }
+
+    public override LibraryContext CreateContext(IEnumerable<ISingletonInterceptor> interceptors, bool inject)
+        => new SqliteLibraryContext(Fixture.CreateOptions(interceptors, inject));
+
     public class MaterializationInterceptionSqliteFixture : SingletonInterceptorsFixtureBase
     {
         protected override string StoreName
@@ -25,5 +43,8 @@ protected override IServiceCollection InjectInterceptors(
             IServiceCollection serviceCollection,
             IEnumerable<ISingletonInterceptor> injectedInterceptors)
             => base.InjectInterceptors(serviceCollection.AddEntityFrameworkSqlite(), injectedInterceptors);
+
+        public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+            => base.AddOptions(builder.ConfigureWarnings(e => e.Ignore(SqliteEventId.CompositeKeyWithValueGeneration)));
     }
 }