Update based on code review feedback and use of EF.Property for loadi…

…ng with filtered include.

src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs

@@ -918,10 +918,10 @@ private void WritePropertyValue(
  /// </summary>
  public object GetOrCreateCollection(INavigationBase navigationBase, bool forMaterialization)
  => navigationBase.IsShadowProperty()
- ? GetOrCreateCollectionTyped(navigationBase)
+ ? GetOrCreateShadowCollection(navigationBase)
  : navigationBase.GetCollectionAccessor()!.GetOrCreate(Entity, forMaterialization);
 
- private object GetOrCreateCollectionTyped(INavigationBase navigation)
+ private object GetOrCreateShadowCollection(INavigationBase navigation)
  {
  var collection = _shadowValues[navigation.GetShadowIndex()]; 
  if (collection == null)
@@ -943,7 +943,7 @@ public bool CollectionContains(INavigationBase navigationBase, object value)
  {
  var collectionAccessor = navigationBase.GetCollectionAccessor()!;
  return navigationBase.IsShadowProperty()
- ? collectionAccessor.ContainsStandalone(GetOrCreateCollectionTyped(navigationBase), value)
+ ? collectionAccessor.ContainsStandalone(GetOrCreateShadowCollection(navigationBase), value)
  : collectionAccessor.Contains(Entity, value);
  }
 
@@ -964,7 +964,7 @@ public bool AddToCollection(
  return collectionAccessor.Add(Entity, value, forMaterialization);
  }
 
- var collection = GetOrCreateCollectionTyped(navigationBase);
+ var collection = GetOrCreateShadowCollection(navigationBase);
  if (!collectionAccessor.ContainsStandalone(collection, value))
  {
  collectionAccessor.AddStandalone(collection, value);
@@ -984,7 +984,7 @@ public bool RemoveFromCollection(INavigationBase navigationBase, object value)
  {
  var collectionAccessor = navigationBase.GetCollectionAccessor()!;
  return navigationBase.IsShadowProperty()
- ? collectionAccessor.RemoveStandalone(GetOrCreateCollectionTyped(navigationBase), value)
+ ? collectionAccessor.RemoveStandalone(GetOrCreateShadowCollection(navigationBase), value)
  : collectionAccessor.Remove(Entity, value);
  }
 

src/EFCore/Internal/ManyToManyLoader.cs

@@ -3,7 +3,6 @@
 
 using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
-using ValueBuffer = Microsoft.EntityFrameworkCore.Storage.ValueBuffer;
 
 namespace Microsoft.EntityFrameworkCore.Internal;
 
@@ -13,10 +12,9 @@ namespace Microsoft.EntityFrameworkCore.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.
 /// </summary>
-public class ManyToManyLoader<TEntity, TSourceEntity, TJoinEntity> : ICollectionLoader<TEntity>
+public class ManyToManyLoader<TEntity, TSourceEntity> : ICollectionLoader<TEntity>
  where TEntity : class
  where TSourceEntity : class
- where TJoinEntity : class
 {
  private readonly ISkipNavigation _skipNavigation;
 
@@ -44,7 +42,7 @@ public virtual void Load(InternalEntityEntry entry)
  // Short-circuit for any null key values for perf and because of #6129
  if (keyValues != null)
  {
- Query(entry.Context, keyValues).Result.Load();
+ Query(entry.Context, keyValues).Load();
  }
 
  entry.SetIsLoaded(_skipNavigation);
@@ -63,8 +61,7 @@ public virtual async Task LoadAsync(InternalEntityEntry entry, CancellationToken
  // Short-circuit for any null key values for perf and because of #6129
  if (keyValues != null)
  {
- await (await Query(entry.Context, keyValues, true, cancellationToken).ConfigureAwait(false))
- .LoadAsync(cancellationToken).ConfigureAwait(false);
+ await Query(entry.Context, keyValues).LoadAsync(cancellationToken).ConfigureAwait(false);
  }
 
  entry.SetIsLoaded(_skipNavigation);
@@ -92,7 +89,7 @@ public virtual IQueryable<TEntity> Query(InternalEntityEntry entry)
  return queryRoot.Where(e => false);
  }
 
- return Query(context, keyValues).Result;
+ return Query(context, keyValues);
  }
 
  private object[]? PrepareForLoad(InternalEntityEntry entry)
@@ -101,24 +98,19 @@ public virtual IQueryable<TEntity> Query(InternalEntityEntry entry)
  {
  throw new InvalidOperationException(CoreStrings.CannotLoadDetached(_skipNavigation.Name, entry.EntityType.DisplayName()));
  }
-
  var properties = _skipNavigation.ForeignKey.PrincipalKey.Properties;
  var values = new object[properties.Count];
-
  for (var i = 0; i < values.Length; i++)
  {
  var value = entry[properties[i]];
  if (value == null)
  {
  return null;
  }
-
  values[i] = value;
  }
-
  return values;
  }
-
  /// <summary>
  /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
  /// the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -128,9 +120,12 @@ public virtual IQueryable<TEntity> Query(InternalEntityEntry entry)
  IQueryable ICollectionLoader.Query(InternalEntityEntry entry)
  => Query(entry);
 
- private async Task<IQueryable<TEntity>> Query(
- DbContext context, object[] keyValues, bool async = false, CancellationToken cancellationToken = default)
+ private IQueryable<TEntity> Query(
+ DbContext context,
+ object[] keyValues)
  {
+ var loadProperties = _skipNavigation.ForeignKey.PrincipalKey.Properties;
+
  // Example of query being built:
  //
  // IQueryable<EntityTwo> loaded
@@ -140,38 +135,16 @@ private async Task<IQueryable<TEntity>> Query(
  // .SelectMany(e => e.TwoSkip)
  // .NotQuiteInclude(e => e.OneSkip.Where(e => e.Id == left.Id));
 
- var loadProperties = _skipNavigation.ForeignKey.PrincipalKey.Properties;
  var queryRoot = _skipNavigation.DeclaringEntityType.HasSharedClrType
  ? context.Set<TSourceEntity>(_skipNavigation.DeclaringEntityType.Name)
  : context.Set<TSourceEntity>();
 
- var queryable = queryRoot
+ return queryRoot
  .AsTracking()
- .Where(BuildWhereLambda<TSourceEntity>(loadProperties, new ValueBuffer(keyValues)))
- .SelectMany(BuildSelectManyLambda(_skipNavigation));
-
- if (_skipNavigation.Inverse.IsShadowProperty())
- {
- // TODOU: This is a hack, until #27493 is implemented.
- var joinEntitiesQuery = (_skipNavigation.JoinEntityType.HasSharedClrType
- ? context.Set<TJoinEntity>(_skipNavigation.JoinEntityType.Name)
- : context.Set<TJoinEntity>())
- .AsTracking()
- .Where(BuildWhereLambda<TJoinEntity>(_skipNavigation.ForeignKey.Properties, new ValueBuffer(keyValues)));
-
- if (async)
- {
- await joinEntitiesQuery.LoadAsync(cancellationToken).ConfigureAwait(false);
- }
- else
- {
- joinEntitiesQuery.Load();
- }
-
- return queryable;
- }
-
- return queryable.NotQuiteInclude(BuildIncludeLambda(_skipNavigation.Inverse, loadProperties, new ValueBuffer(keyValues)));
+ .Where(BuildWhereLambda(loadProperties, new ValueBuffer(keyValues)))
+ .SelectMany(BuildSelectManyLambda(_skipNavigation))
+ .NotQuiteInclude(BuildIncludeLambda(_skipNavigation.Inverse, loadProperties, new ValueBuffer(keyValues)))
+ .AsQueryable();
  }
 
  private static Expression<Func<TEntity, IEnumerable<TSourceEntity>>> BuildIncludeLambda(
@@ -181,32 +154,31 @@ private static Expression<Func<TEntity, IEnumerable<TSourceEntity>>> BuildInclud
  {
  var whereParameter = Expression.Parameter(typeof(TSourceEntity), "e");
  var entityParameter = Expression.Parameter(typeof(TEntity), "e");
-
  return Expression.Lambda<Func<TEntity, IEnumerable<TSourceEntity>>>(
  Expression.Call(
  EnumerableMethods.Where.MakeGenericMethod(typeof(TSourceEntity)),
- Expression.MakeMemberAccess(
+ Expression.Call(
+ EF.PropertyMethod.MakeGenericMethod(skipNavigation.ClrType),
  entityParameter,
- skipNavigation.GetIdentifyingMemberInfo()!),
+ Expression.Constant(skipNavigation.Name, typeof(string))),
  Expression.Lambda<Func<TSourceEntity, bool>>(
  ExpressionExtensions.BuildPredicate(keyProperties, keyValues, whereParameter),
  whereParameter)), entityParameter);
  }
 
- private static Expression<Func<T, bool>> BuildWhereLambda<T>(
+ private static Expression<Func<TSourceEntity, bool>> BuildWhereLambda(
  IReadOnlyList<IProperty> keyProperties,
  ValueBuffer keyValues)
  {
- var entityParameter = Expression.Parameter(typeof(T), "e");
+ var entityParameter = Expression.Parameter(typeof(TSourceEntity), "e");
 
- return Expression.Lambda<Func<T, bool>>(
+ return Expression.Lambda<Func<TSourceEntity, bool>>(
  ExpressionExtensions.BuildPredicate(keyProperties, keyValues, entityParameter), entityParameter);
  }
 
  private static Expression<Func<TSourceEntity, IEnumerable<TEntity>>> BuildSelectManyLambda(INavigationBase navigation)
  {
  var entityParameter = Expression.Parameter(typeof(TSourceEntity), "e");
-
  return Expression.Lambda<Func<TSourceEntity, IEnumerable<TEntity>>>(
  Expression.MakeMemberAccess(
  entityParameter,

src/EFCore/Internal/ManyToManyLoaderFactory.cs

@@ -25,14 +25,12 @@ private static readonly MethodInfo GenericCreate
  public virtual ICollectionLoader Create(ISkipNavigation skipNavigation)
  => (ICollectionLoader)GenericCreate.MakeGenericMethod(
  skipNavigation.TargetEntityType.ClrType,
- skipNavigation.DeclaringEntityType.ClrType,
- skipNavigation.JoinEntityType.ClrType)
+ skipNavigation.DeclaringEntityType.ClrType)
  .Invoke(null, new object[] { skipNavigation })!;
 
  [UsedImplicitly]
- private static ICollectionLoader CreateManyToMany<TEntity, TTargetEntity, TJoinEntity>(ISkipNavigation skipNavigation)
+ private static ICollectionLoader CreateManyToMany<TEntity, TTargetEntity>(ISkipNavigation skipNavigation)
  where TEntity : class
  where TTargetEntity : class
- where TJoinEntity : class
- => new ManyToManyLoader<TEntity, TTargetEntity, TJoinEntity>(skipNavigation);
+ => new ManyToManyLoader<TEntity, TTargetEntity>(skipNavigation);
 }

src/EFCore/Metadata/Builders/CollectionNavigationBuilder.cs

@@ -214,7 +214,7 @@ private InternalForeignKeyBuilder WithOneBuilder(MemberIdentity reference)
  /// The name of the collection navigation property on the other end of this relationship.
  /// </param>
  /// <returns>An object to further configure the relationship.</returns>
- public virtual CollectionCollectionBuilder WithMany(string navigationName)
+ public virtual CollectionCollectionBuilder WithMany(string? navigationName = null)
  {
  var leftName = Builder?.Metadata.PrincipalToDependent?.Name;
  var collectionCollectionBuilder =

src/EFCore/Metadata/IClrCollectionAccessor.cs

@@ -45,23 +45,23 @@ public interface IClrCollectionAccessor
  bool Remove(object entity, object value);
 
  /// <summary>
- /// Adds a value to the collection, unless it is already contained in the collection.
+ /// Adds a value to the passed collection, unless it is already contained in the collection.
  /// </summary>
  /// <param name="collection">The collection.</param>
  /// <param name="value">The value to add.</param>
  /// <returns><see langword="true" /> if a value was added; <see langword="false" /> if it was already in the collection.</returns>
  bool AddStandalone(object collection, object value);
 
  /// <summary>
- /// Checks whether the value is contained in the collection.
+ /// Checks whether the value is contained in the passed collection.
  /// </summary>
  /// <param name="collection">The collection.</param>
  /// <param name="value">The value to check.</param>
  /// <returns><see langword="true" /> if the value is contained in the collection; <see langword="false" /> otherwise.</returns>
  bool ContainsStandalone(object collection, object value);
 
  /// <summary>
- /// Removes a value from the collection.
+ /// Removes a value from the passed collection.
  /// </summary>
  /// <param name="collection">The collection.</param>
  /// <param name="value">The value to check.</param>

src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs

@@ -4253,15 +4253,12 @@ private static bool Contains(IReadOnlyForeignKey? inheritedFk, IReadOnlyForeignK
  }
  else
  {
- if (navigationName == null)
+ var generatedNavigationName = targetEntityType.ShortName();
+ navigationName = generatedNavigationName;
+ var uniquifier = 0;
+ while (Metadata.FindMembersInHierarchy(navigationName).Any())
  {
- var generatedNavigationName = targetEntityType.ShortName();
- navigationName = generatedNavigationName;
- var uniquifier = 0;
- while(Metadata.FindMembersInHierarchy(navigationName).Any())
- {
- navigationName = generatedNavigationName + (++uniquifier);
- }
+ navigationName = generatedNavigationName + (++uniquifier);
  }
 
  builder = Metadata.AddSkipNavigation(

src/EFCore/Properties/CoreStrings.resx

@@ -1370,9 +1370,6 @@
  <data name="SetOperationWithDifferentIncludesInOperands" xml:space="preserve">
  <value>Unable to translate set operation since both operands have different 'Include' operations. Consider having same 'Include' applied on both sides.</value>
  </data>
- <data name="ShadowManyToManyNavigation" xml:space="preserve">
- <value>Unable to set up a many-to-many relationship between '{leftEntityType}.{leftNavigation}' and '{rightEntityType}.{rightNavigation}' because one or both of the navigations don't have a corresponding CLR property. Consider adding a corresponding private property to the entity CLR type.</value>
- </data>
  <data name="SharedTypeDerivedType" xml:space="preserve">
  <value>The shared-type entity type '{entityType}' cannot have a base type.</value>
  </data>