Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.0.2] Stop marking the inverse navigation as loaded when loading a many-to-many navigation #23589

Merged
merged 3 commits into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,10 @@ private void AddInclude(
Expression.Constant(navigation),
Expression.Constant(inverseNavigation, typeof(INavigation)),
Expression.Constant(fixup),
Expression.Constant(initialize, typeof(Action<>).MakeGenericType(includingClrType))));
Expression.Constant(initialize, typeof(Action<>).MakeGenericType(includingClrType)),
#pragma warning disable EF1001 // Internal EF Core API usage.
Expression.Constant(includeExpression.SetLoaded)));
#pragma warning restore EF1001 // Internal EF Core API usage.
}

private static readonly MethodInfo _includeReferenceMethodInfo
Expand All @@ -409,7 +412,8 @@ private static void IncludeReference<TIncludingEntity, TIncludedEntity>(
INavigation navigation,
INavigation inverseNavigation,
Action<TIncludingEntity, TIncludedEntity> fixup,
Action<TIncludingEntity> _)
Action<TIncludingEntity> _,
bool __)
{
if (entity == null
|| !navigation.DeclaringEntityType.IsAssignableFrom(entityType))
Expand Down Expand Up @@ -454,7 +458,8 @@ private static void IncludeCollection<TIncludingEntity, TIncludedEntity>(
INavigation navigation,
INavigation inverseNavigation,
Action<TIncludingEntity, TIncludedEntity> fixup,
Action<TIncludingEntity> initialize)
Action<TIncludingEntity> initialize,
bool setLoaded)
{
if (entity == null
|| !navigation.DeclaringEntityType.IsAssignableFrom(entityType))
Expand Down Expand Up @@ -485,9 +490,13 @@ private static void IncludeCollection<TIncludingEntity, TIncludedEntity>(
}
else
{
if (setLoaded)
{
#pragma warning disable EF1001 // Internal EF Core API usage.
entry.SetIsLoaded(navigation);
entry.SetIsLoaded(navigation);
#pragma warning restore EF1001 // Internal EF Core API usage.
}

if (relatedEntities != null)
{
using var enumerator = relatedEntities.GetEnumerator();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ private static void IncludeCollection<TEntity, TIncludingEntity, TIncludedEntity
INavigationBase navigation,
INavigationBase inverseNavigation,
Action<TIncludingEntity, TIncludedEntity> fixup,
bool trackingQuery)
bool trackingQuery,
bool setLoaded)
where TIncludingEntity : class, TEntity
where TEntity : class
where TIncludedEntity : class
Expand All @@ -97,13 +98,16 @@ private static void IncludeCollection<TEntity, TIncludingEntity, TIncludedEntity
var collectionAccessor = navigation.GetCollectionAccessor();
collectionAccessor.GetOrCreate(includingEntity, forMaterialization: true);

if (trackingQuery)
{
queryContext.SetNavigationIsLoaded(entity, navigation);
}
else
if (setLoaded)
{
navigation.SetIsLoadedWhenNoTracking(entity);
if (trackingQuery)
{
queryContext.SetNavigationIsLoaded(entity, navigation);
}
else
{
navigation.SetIsLoadedWhenNoTracking(entity);
}
}

foreach (var valueBuffer in innerValueBuffers)
Expand Down Expand Up @@ -178,7 +182,10 @@ protected override Expression VisitExtension(Expression extensionExpression)
Expression.Constant(
GenerateFixup(
includingClrType, relatedEntityClrType, includeExpression.Navigation, inverseNavigation).Compile()),
Expression.Constant(_tracking));
Expression.Constant(_tracking),
#pragma warning disable EF1001 // Internal EF Core API usage.
Expression.Constant(includeExpression.SetLoaded));
#pragma warning restore EF1001 // Internal EF Core API usage.
}

return Expression.Call(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,10 @@ protected override Expression VisitExtension(Expression extensionExpression)
Expression.Constant(outerIdentifierLambda.Compile()),
Expression.Constant(navigation),
Expression.Constant(navigation.GetCollectionAccessor()),
Expression.Constant(_isTracking)));
Expression.Constant(_isTracking),
#pragma warning disable EF1001 // Internal EF Core API usage.
Expression.Constant(includeExpression.SetLoaded)));
#pragma warning restore EF1001 // Internal EF Core API usage.

var relatedEntityType = innerShaper.ReturnType;
var inverseNavigation = navigation.Inverse;
Expand Down Expand Up @@ -629,7 +632,10 @@ protected override Expression VisitExtension(Expression extensionExpression)
Expression.Constant(parentIdentifierLambda.Compile()),
Expression.Constant(navigation),
Expression.Constant(navigation.GetCollectionAccessor()),
Expression.Constant(_isTracking)));
Expression.Constant(_isTracking),
#pragma warning disable EF1001 // Internal EF Core API usage.
Expression.Constant(includeExpression.SetLoaded)));
#pragma warning restore EF1001 // Internal EF Core API usage.

var relatedEntityType = innerShaper.ReturnType;
var inverseNavigation = navigation.Inverse;
Expand Down Expand Up @@ -1207,20 +1213,24 @@ private static void InitializeIncludeCollection<TParent, TNavigationEntity>(
Func<QueryContext, DbDataReader, object[]> outerIdentifier,
INavigationBase navigation,
IClrCollectionAccessor clrCollectionAccessor,
bool trackingQuery)
bool trackingQuery,
bool setLoaded)
where TParent : class
where TNavigationEntity : class, TParent
{
object collection = null;
if (entity is TNavigationEntity)
{
if (trackingQuery)
{
queryContext.SetNavigationIsLoaded(entity, navigation);
}
else
if (setLoaded)
{
navigation.SetIsLoadedWhenNoTracking(entity);
if (trackingQuery)
{
queryContext.SetNavigationIsLoaded(entity, navigation);
}
else
{
navigation.SetIsLoadedWhenNoTracking(entity);
}
}

collection = clrCollectionAccessor.GetOrCreate(entity, forMaterialization: true);
Expand Down Expand Up @@ -1361,20 +1371,24 @@ private static void InitializeSplitIncludeCollection<TParent, TNavigationEntity>
Func<QueryContext, DbDataReader, object[]> parentIdentifier,
INavigationBase navigation,
IClrCollectionAccessor clrCollectionAccessor,
bool trackingQuery)
bool trackingQuery,
bool setLoaded)
where TParent : class
where TNavigationEntity : class, TParent
{
object collection = null;
if (entity is TNavigationEntity)
{
if (trackingQuery)
{
queryContext.SetNavigationIsLoaded(entity, navigation);
}
else
if (setLoaded)
{
navigation.SetIsLoadedWhenNoTracking(entity);
if (trackingQuery)
{
queryContext.SetNavigationIsLoaded(entity, navigation);
}
else
{
navigation.SetIsLoadedWhenNoTracking(entity);
}
}

collection = clrCollectionAccessor.GetOrCreate(entity, forMaterialization: true);
Expand Down
25 changes: 25 additions & 0 deletions src/EFCore/Extensions/EntityFrameworkQueryableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2376,6 +2376,15 @@ internal static readonly MethodInfo IncludeMethodInfo
&& mi.GetParameters().Any(
pi => pi.Name == "navigationPropertyPath" && pi.ParameterType != typeof(string)));

internal static readonly MethodInfo NotQuiteIncludeMethodInfo
= typeof(EntityFrameworkQueryableExtensions)
.GetTypeInfo().GetDeclaredMethods(nameof(NotQuiteInclude))
.Single(
mi =>
mi.GetGenericArguments().Count() == 2
&& mi.GetParameters().Any(
pi => pi.Name == "navigationPropertyPath" && pi.ParameterType != typeof(string)));

/// <summary>
/// Specifies related entities to include in the query results. The navigation property to be included is specified starting with the
/// type of entity being queried (<typeparamref name="TEntity" />). If you wish to include additional types based on the navigation
Expand Down Expand Up @@ -2443,6 +2452,22 @@ source.Provider is EntityQueryProvider
: source);
}

// A version of Include that doesn't set the navigation as loaded
internal static IIncludableQueryable<TEntity, TProperty> NotQuiteInclude<TEntity, TProperty>(
[NotNull] this IQueryable<TEntity> source,
[NotNull] Expression<Func<TEntity, TProperty>> navigationPropertyPath)
where TEntity : class
{
return new IncludableQueryable<TEntity, TProperty>(
source.Provider is EntityQueryProvider
? source.Provider.CreateQuery<TEntity>(
Expression.Call(
instance: null,
method: NotQuiteIncludeMethodInfo.MakeGenericMethod(typeof(TEntity), typeof(TProperty)),
arguments: new[] { source.Expression, Expression.Quote(navigationPropertyPath) }))
: source);
}

internal static readonly MethodInfo ThenIncludeAfterEnumerableMethodInfo
= typeof(EntityFrameworkQueryableExtensions)
.GetTypeInfo().GetDeclaredMethods(nameof(ThenInclude))
Expand Down
18 changes: 13 additions & 5 deletions src/EFCore/Internal/ManyToManyLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,18 +147,26 @@ private IQueryable<TEntity> Query(
// .AsTracking()
// .Where(e => e.Id == left.Id)
// .SelectMany(e => e.TwoSkip)
// .Include(e => e.OneSkip.Where(e => e.Id == left.Id));
// .NotQuiteInclude(e => e.OneSkip.Where(e => e.Id == left.Id));

var queryRoot = _skipNavigation.DeclaringEntityType.HasSharedClrType
? context.Set<TSourceEntity>(_skipNavigation.DeclaringEntityType.Name)
: context.Set<TSourceEntity>();

return queryRoot
var queryable = queryRoot
.AsTracking()
.Where(BuildWhereLambda(loadProperties, new ValueBuffer(keyValues)))
.SelectMany(BuildSelectManyLambda(_skipNavigation))
.Include(BuildIncludeLambda(_skipNavigation.Inverse, loadProperties, new ValueBuffer(keyValues)))
.AsQueryable();
.SelectMany(BuildSelectManyLambda(_skipNavigation));

var useOldBehavior = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23475", out var enabled) && enabled;

return useOldBehavior
? queryable
.Include(BuildIncludeLambda(_skipNavigation.Inverse, loadProperties, new ValueBuffer(keyValues)))
.AsQueryable()
: queryable
.NotQuiteInclude(BuildIncludeLambda(_skipNavigation.Inverse, loadProperties, new ValueBuffer(keyValues)))
.AsQueryable();
}

private static Expression<Func<TEntity, IEnumerable<TSourceEntity>>> BuildIncludeLambda(
Expand Down
34 changes: 32 additions & 2 deletions src/EFCore/Query/IncludeExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Utilities;

Expand All @@ -21,7 +22,8 @@ namespace Microsoft.EntityFrameworkCore.Query
public class IncludeExpression : Expression, IPrintableExpression
{
/// <summary>
/// Creates a new instance of the <see cref="IncludeExpression" /> class.
/// Creates a new instance of the <see cref="IncludeExpression" /> class. The navigation will be set
/// as loaded after completing the Include.
/// </summary>
/// <param name="entityExpression"> An expression to get entity which is performing include. </param>
/// <param name="navigationExpression"> An expression to get included navigation element. </param>
Expand All @@ -30,6 +32,22 @@ public IncludeExpression(
[NotNull] Expression entityExpression,
[NotNull] Expression navigationExpression,
[NotNull] INavigationBase navigation)
: this(entityExpression, navigationExpression, navigation, setLoaded: true)
{
}

/// <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
/// 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>
[EntityFrameworkInternal]
public IncludeExpression(
[NotNull] Expression entityExpression,
[NotNull] Expression navigationExpression,
[NotNull] INavigationBase navigation,
bool setLoaded)
{
Check.NotNull(entityExpression, nameof(entityExpression));
Check.NotNull(navigationExpression, nameof(navigationExpression));
Expand All @@ -39,6 +57,9 @@ public IncludeExpression(
NavigationExpression = navigationExpression;
Navigation = navigation;
Type = EntityExpression.Type;

var useOldBehavior = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23475", out var enabled) && enabled;
SetLoaded = useOldBehavior || setLoaded;
}

/// <summary>
Expand All @@ -56,6 +77,15 @@ public IncludeExpression(
/// </summary>
public virtual INavigationBase Navigation { get; }

/// <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
/// 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>
[EntityFrameworkInternal]
public virtual bool SetLoaded { get; }
ajcvickers marked this conversation as resolved.
Show resolved Hide resolved

/// <inheritdoc />
public sealed override ExpressionType NodeType
=> ExpressionType.Extension;
Expand Down Expand Up @@ -87,7 +117,7 @@ public virtual IncludeExpression Update([NotNull] Expression entityExpression, [
Check.NotNull(navigationExpression, nameof(navigationExpression));

return entityExpression != EntityExpression || navigationExpression != NavigationExpression
? new IncludeExpression(entityExpression, navigationExpression, Navigation)
? new IncludeExpression(entityExpression, navigationExpression, Navigation, SetLoaded)
: this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,9 @@ private Expression ExpandIncludesHelper(Expression root, EntityReference entityR
}
}

result = new IncludeExpression(result, included, navigationBase);
#pragma warning disable EF1001 // Internal EF Core API usage.
result = new IncludeExpression(result, included, navigationBase, kvp.Value.SetLoaded);
#pragma warning restore EF1001 // Internal EF Core API usage.
}

return result;
Expand Down
Loading