From 47b5dcb0a71f7bc04130956b8d6c6361e8b43237 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 20 Aug 2025 21:44:57 +0200 Subject: [PATCH] ClrCollectionAccessor cleanup for complex JSON collections --- ...ocessingExpressionVisitor.ClientMethods.cs | 11 +++- .../Internal/ClrCollectionAccessor.cs | 35 +++++------ .../Internal/ClrCollectionAccessorFactory.cs | 59 ++++++++++--------- 3 files changed, 58 insertions(+), 47 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ClientMethods.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ClientMethods.cs index e70acb55192..b6715a2c9a7 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ClientMethods.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ClientMethods.cs @@ -5,6 +5,7 @@ using System.Runtime.CompilerServices; using System.Text.Json; using Microsoft.EntityFrameworkCore.Internal; +using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Query.Internal; using Microsoft.EntityFrameworkCore.Storage.Json; @@ -1076,8 +1077,12 @@ static async Task InitializeReaderAsync( throw new InvalidOperationException(CoreStrings.JsonReaderInvalidTokenType(tokenType.ToString())); } - var collectionAccessor = relationship.GetCollectionAccessor(); - var result = (TResult)collectionAccessor!.Create(); + // var collectionAccessor = relationship.GetCollectionAccessor(); +#pragma warning disable EF1001 // Internal EF Core API usage. + var collectionAccessor = ((IRuntimePropertyBase)relationship).GetIndexedCollectionAccessor(); +#pragma warning restore EF1001 // Internal EF Core API usage. + + var result = (TResult)collectionAccessor!.Create(0); object[]? newKeyPropertyValues = null; @@ -1098,7 +1103,7 @@ static async Task InitializeReaderAsync( { manager.CaptureState(); var entity = innerShaper(queryContext, newKeyPropertyValues, jsonReaderData); - collectionAccessor.AddStandalone(result, entity); + // collectionAccessor.AddStandalone(result, entity); manager = new Utf8JsonReaderManager(manager.Data, queryContext.QueryLogger); if (manager.CurrentReader.TokenType != JsonTokenType.EndObject) diff --git a/src/EFCore/Metadata/Internal/ClrCollectionAccessor.cs b/src/EFCore/Metadata/Internal/ClrCollectionAccessor.cs index 40ca6a48b29..3aeae04a8b0 100644 --- a/src/EFCore/Metadata/Internal/ClrCollectionAccessor.cs +++ b/src/EFCore/Metadata/Internal/ClrCollectionAccessor.cs @@ -11,16 +11,17 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Internal; /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// -public class ClrCollectionAccessor : IClrCollectionAccessor +public class ClrCollectionAccessor : IClrCollectionAccessor + where TEntity : class where TCollection : class, IEnumerable where TElement : class { private readonly string _propertyName; private readonly bool _shadow; - private readonly Func? _getCollection; - private readonly Action? _setCollection; - private readonly Action? _setCollectionForMaterialization; - private readonly Func, TCollection>? _createAndSetCollection; + private readonly Func? _getCollection; + private readonly Action? _setCollection; + private readonly Action? _setCollectionForMaterialization; + private readonly Func, TCollection>? _createAndSetCollection; private readonly Func? _createCollection; /// @@ -41,10 +42,10 @@ public virtual Type CollectionType public ClrCollectionAccessor( string propertyName, bool shadow, - Func? getCollection, - Action? setCollection, - Action? setCollectionForMaterialization, - Func, TCollection>? createAndSetCollection, + Func? getCollection, + Action? setCollection, + Action? setCollectionForMaterialization, + Func, TCollection>? createAndSetCollection, Func? createCollection) { _propertyName = propertyName; @@ -95,7 +96,7 @@ public virtual object Create() { throw new InvalidOperationException( CoreStrings.NavigationCannotCreateType( - _propertyName, typeof(TStructural).ShortDisplayName(), typeof(TCollection).ShortDisplayName())); + _propertyName, typeof(TEntity).ShortDisplayName(), typeof(TCollection).ShortDisplayName())); } return _createCollection(); @@ -121,17 +122,17 @@ private ICollection GetOrCreateCollection(object instance, bool forMat if (setCollection == null) { - throw new InvalidOperationException(CoreStrings.NavigationNoSetter(_propertyName, typeof(TStructural).ShortDisplayName())); + throw new InvalidOperationException(CoreStrings.NavigationNoSetter(_propertyName, typeof(TEntity).ShortDisplayName())); } if (_createAndSetCollection == null) { throw new InvalidOperationException( CoreStrings.NavigationCannotCreateType( - _propertyName, typeof(TStructural).ShortDisplayName(), typeof(TCollection).ShortDisplayName())); + _propertyName, typeof(TEntity).ShortDisplayName(), typeof(TCollection).ShortDisplayName())); } - collection = (ICollection)_createAndSetCollection((TStructural)instance, setCollection); + collection = (ICollection)_createAndSetCollection((TEntity)instance, setCollection); } return collection; @@ -146,7 +147,7 @@ private ICollection GetOrCreateCollection(object instance, bool forMat return (ICollection?)_createCollection?.Invoke(); } - var enumerable = _getCollection!((TStructural)instance); + var enumerable = _getCollection!((TEntity)instance); var collection = enumerable as ICollection; if (enumerable != null @@ -155,7 +156,7 @@ private ICollection GetOrCreateCollection(object instance, bool forMat throw new InvalidOperationException( CoreStrings.NavigationBadType( _propertyName, - typeof(TStructural).ShortDisplayName(), + typeof(TEntity).ShortDisplayName(), enumerable.GetType().ShortDisplayName(), typeof(TElement).ShortDisplayName())); } @@ -170,7 +171,7 @@ private ICollection GetOrCreateCollection(object instance, bool forMat /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual bool Contains(object entity, object value) - => Contains(GetCollection((TStructural)entity), value); + => Contains(GetCollection((TEntity)entity), value); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -188,7 +189,7 @@ public virtual bool ContainsStandalone(object collection, object value) /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual bool Remove(object entity, object value) - => RemoveStandalone(GetCollection((TStructural)entity), value); + => RemoveStandalone(GetCollection((TEntity)entity), value); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs b/src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs index d162e49a949..5a56de2633c 100644 --- a/src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs +++ b/src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs @@ -109,11 +109,12 @@ private ClrCollectionAccessorFactory() } [UsedImplicitly] - private static IClrCollectionAccessor CreateGeneric(IPropertyBase relationship) + private static IClrCollectionAccessor CreateGeneric(IPropertyBase relationship) + where TEntity : class where TCollection : class, IEnumerable where TElement : class { - CreateExpressions( + CreateExpressions( relationship, out var getCollection, out var setCollection, @@ -121,7 +122,7 @@ private static IClrCollectionAccessor CreateGeneric( + return new ClrCollectionAccessor( relationship.Name, relationship.IsShadowProperty(), getCollection?.Compile(), @@ -179,13 +180,14 @@ private static readonly MethodInfo GenericCreateExpressions = typeof(ClrCollectionAccessorFactory).GetMethod(nameof(CreateExpressions), BindingFlags.Static | BindingFlags.NonPublic)!; [UsedImplicitly] - private static void CreateExpressions( + private static void CreateExpressions( IPropertyBase relationship, - out Expression>? getCollection, - out Expression>? setCollection, - out Expression>? setCollectionForMaterialization, - out Expression, TCollection>>? createAndSetCollection, + out Expression>? getCollection, + out Expression>? setCollection, + out Expression>? setCollectionForMaterialization, + out Expression, TCollection>>? createAndSetCollection, out Expression>? createCollection) + where TEntity : class where TCollection : class, IEnumerable where TElement : class { @@ -195,7 +197,7 @@ private static void CreateExpressions( createAndSetCollection = null; createCollection = null; - var entityParameter = Expression.Parameter(typeof(TStructural), "entity"); + var entityParameter = Expression.Parameter(typeof(TEntity), "entity"); var valueParameter = Expression.Parameter(typeof(TCollection), "collection"); if (!relationship.IsShadowProperty()) @@ -209,7 +211,7 @@ private static void CreateExpressions( memberAccessForRead = Expression.Convert(memberAccessForRead, typeof(TCollection)); } - getCollection = Expression.Lambda>( + getCollection = Expression.Lambda>( memberAccessForRead, entityParameter); @@ -225,7 +227,7 @@ private static void CreateExpressions( } var concreteType = CollectionTypeFactory.Instance.TryFindTypeToInstantiate( - typeof(TStructural), + typeof(TEntity), typeof(TCollection), relationship.DeclaringType.Model[CoreAnnotationNames.FullChangeTrackingNotificationsRequired] != null); if (concreteType != null) @@ -234,18 +236,18 @@ private static void CreateExpressions( if (setCollection != null || setCollectionForMaterialization != null) { - var setterParameter = Expression.Parameter(typeof(Action), "setter"); + var setterParameter = Expression.Parameter(typeof(Action), "setter"); var createAndSetCollectionMethod = isHashSet ? CreateAndSetHashSetMethod - .MakeGenericMethod(typeof(TStructural), typeof(TCollection), typeof(TElement)) + .MakeGenericMethod(typeof(TEntity), typeof(TCollection), typeof(TElement)) : IsObservableHashSet(concreteType) ? CreateAndSetObservableHashSetMethod - .MakeGenericMethod(typeof(TStructural), typeof(TCollection), typeof(TElement)) + .MakeGenericMethod(typeof(TEntity), typeof(TCollection), typeof(TElement)) : CreateAndSetMethod - .MakeGenericMethod(typeof(TStructural), typeof(TCollection), concreteType); + .MakeGenericMethod(typeof(TEntity), typeof(TCollection), concreteType); - createAndSetCollection = Expression.Lambda, TCollection>>( + createAndSetCollection = Expression.Lambda, TCollection>>( Expression.Call(createAndSetCollectionMethod, entityParameter, setterParameter), entityParameter, setterParameter); @@ -258,11 +260,11 @@ private static void CreateExpressions( : Expression.Lambda>(Expression.New(concreteType)); } - static Expression> CreateSetterDelegate( + static Expression> CreateSetterDelegate( ParameterExpression parameterExpression, MemberInfo memberInfo, ParameterExpression valueParameter1) - => Expression.Lambda>( + => Expression.Lambda>( Expression.MakeMemberAccess( parameterExpression, memberInfo).Assign( @@ -296,9 +298,10 @@ private static bool IsObservableHashSet(Type type) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public static TCollection CreateAndSet( - TStructural entity, - Action setterDelegate) + public static TCollection CreateAndSet( + TEntity entity, + Action setterDelegate) + where TEntity : class where TCollection : class where TConcreteCollection : TCollection, new() { @@ -313,9 +316,10 @@ public static TCollection CreateAndSet - public static TCollection CreateAndSetHashSet( - TStructural entity, - Action setterDelegate) + public static TCollection CreateAndSetHashSet( + TEntity entity, + Action setterDelegate) + where TEntity : class where TCollection : class where TElement : class { @@ -330,9 +334,10 @@ public static TCollection CreateAndSetHashSet - public static TCollection CreateAndSetObservableHashSet( - TStructural entity, - Action setterDelegate) + public static TCollection CreateAndSetObservableHashSet( + TEntity entity, + Action setterDelegate) + where TEntity : class where TCollection : class where TElement : class {