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

[release/7.0] Stop skipping shadow skip navigation slots when creating the shadow values array #30911

Merged
merged 2 commits into from
Jun 7, 2023
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 @@ -13,15 +13,17 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
/// </summary>
public class ShadowValuesFactoryFactory : SnapshotFactoryFactory<ValueBuffer>
{
private static readonly bool UseOldBehavior30764
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue30764", out var enabled) && enabled;

/// <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>
protected override int GetPropertyIndex(IPropertyBase propertyBase)
// Navigations are not included in the supplied value buffer
=> (propertyBase as IProperty)?.GetShadowIndex() ?? -1;
=> UseOldBehavior30764 ? ((propertyBase as IProperty)?.GetShadowIndex() ?? -1) : propertyBase.GetShadowIndex();

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ protected virtual Expression CreateConstructorExpression(
var count = GetPropertyCount(entityType);

var types = new Type[count];
var propertyBases = new IPropertyBase[count];
var propertyBases = new IPropertyBase?[count];

foreach (var propertyBase in entityType.GetPropertiesAndNavigations())
{
Expand Down Expand Up @@ -95,7 +95,7 @@ protected virtual Expression CreateSnapshotExpression(
Type? entityType,
ParameterExpression parameter,
Type[] types,
IList<IPropertyBase> propertyBases)
IList<IPropertyBase?> propertyBases)
{
var count = types.Length;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ protected override Expression CreateSnapshotExpression(
[DynamicallyAccessedMembers(IEntityType.DynamicallyAccessedMemberTypes)] Type? entityType,
ParameterExpression parameter,
Type[] types,
IList<IPropertyBase> propertyBases)
IList<IPropertyBase?> propertyBases)
{
var constructorExpression = Expression.Convert(
Expression.New(
Expand Down
15 changes: 10 additions & 5 deletions src/EFCore/Infrastructure/ExpressionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ namespace Microsoft.EntityFrameworkCore.Infrastructure;
/// </remarks>
public static class ExpressionExtensions
{
private static readonly bool UseOldBehavior30764
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue30764", out var enabled) && enabled;

/// <summary>
/// Creates a printable string representation of the given expression.
/// </summary>
Expand Down Expand Up @@ -288,11 +291,13 @@ public static Expression CreateValueBufferReadValueExpression(
Type type,
int index,
IPropertyBase? property)
=> Expression.Call(
MakeValueBufferTryReadValueMethod(type),
valueBuffer,
Expression.Constant(index),
Expression.Constant(property, typeof(IPropertyBase)));
=> (property is INavigationBase && !UseOldBehavior30764)
? Expression.Constant(null, typeof(object))
: Expression.Call(
MakeValueBufferTryReadValueMethod(type),
valueBuffer,
Expression.Constant(index),
Expression.Constant(property, typeof(IPropertyBase)));

/// <summary>
/// <para>
Expand Down
13 changes: 12 additions & 1 deletion src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ namespace Microsoft.EntityFrameworkCore.Query;
/// </remarks>
public abstract class ShapedQueryCompilingExpressionVisitor : ExpressionVisitor
{
private static readonly bool UseOldBehavior30764
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue30764", out var enabled) && enabled;

private static readonly PropertyInfo CancellationTokenMemberInfo
= typeof(QueryContext).GetTypeInfo().GetProperty(nameof(QueryContext.CancellationToken))!;

Expand Down Expand Up @@ -586,7 +589,15 @@ private BlockExpression CreateFullMaterializeExpression(
{
var valueBufferExpression = Expression.Call(
materializationContextVariable, MaterializationContext.GetValueBufferMethod);
var shadowProperties = concreteEntityType.GetProperties().Where(p => p.IsShadowProperty());

var shadowProperties = UseOldBehavior30764
? (IEnumerable<IPropertyBase>)concreteEntityType.GetProperties()
.Where(p => p.IsShadowProperty())
: concreteEntityType.GetProperties()
.Concat<IPropertyBase>(concreteEntityType.GetNavigations())
.Concat(concreteEntityType.GetSkipNavigations())
.Where(n => n.IsShadowProperty())
.OrderBy(e => e.GetShadowIndex());

blockExpressions.Add(
Expression.Assign(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,17 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
modelBuilder.Entity<Bayaz>();
modelBuilder.Entity<SecondLaw>();
modelBuilder.Entity<ThirdLaw>();

modelBuilder.Entity<Beetroot2>().HasData(
new { Id = 1, Key = "root-1", Name = "Root One" });

modelBuilder.Entity<Lettuce2>().HasData(
new { Id = 4, Key = "root-1/leaf-1", Name = "Leaf One-One", RootId = 1 });

modelBuilder.Entity<Radish2>()
.HasMany(entity => entity.Entities)
.WithMany()
.UsingEntity<RootStructure>();
}

protected virtual object CreateFullGraph()
Expand Down Expand Up @@ -3984,6 +3995,68 @@ public virtual SecondLaw SecondLaw
}
}

protected abstract class Parsnip2 : NotifyingEntity
{
private int _id;

public int Id
{
get => _id;
set => SetWithNotify(value, ref _id);
}
}

protected class Lettuce2 : Parsnip2
{
private Beetroot2 _root;

public Beetroot2 Root
{
get => _root;
set => SetWithNotify(value, ref _root);
}
}

protected class RootStructure : NotifyingEntity
{
private Guid _radish2Id;
private int _parsnip2Id;

public Guid Radish2Id
{
get => _radish2Id;
set => SetWithNotify(value, ref _radish2Id);
}

public int Parsnip2Id
{
get => _parsnip2Id;
set => SetWithNotify(value, ref _parsnip2Id);
}
}

protected class Radish2 : NotifyingEntity
{
private Guid _id;
private ICollection<Parsnip2> _entities = new ObservableHashSet<Parsnip2>();

public Guid Id
{
get => _id;
set => SetWithNotify(value, ref _id);
}

public ICollection<Parsnip2> Entities
{
get => _entities;
set => SetWithNotify(value, ref _entities);
}
}

protected class Beetroot2 : Parsnip2
{
}

protected class NotifyingEntity : INotifyPropertyChanging, INotifyPropertyChanged
{
protected void SetWithNotify<T>(T value, ref T field, [CallerMemberName] string propertyName = "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1932,4 +1932,19 @@ private static SecondLaw AddSecondLevel(bool thirdLevel1, bool thirdLevel2)

return secondLevel;
}

[ConditionalTheory] // Issue #30764
[InlineData(false)]
[InlineData(true)]
public virtual Task Shadow_skip_navigation_in_base_class_is_handled(bool async)
=> ExecuteWithStrategyInTransactionAsync(
async context =>
{
var entities = async
? await context.Set<Lettuce2>().ToListAsync()
: context.Set<Lettuce2>().ToList();

Assert.Equal(1, entities.Count);
Assert.Equal(nameof(Lettuce2), context.Entry(entities[0]).Property<string>("Discriminator").CurrentValue);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ public override Task Update_root_by_collection_replacement_of_deleted_third_leve
public override Task Sever_relationship_that_will_later_be_deleted(bool async)
=> Task.CompletedTask;

// No owned types
public override Task Shadow_skip_navigation_in_base_class_is_handled(bool async)
=> Task.CompletedTask;

// Owned dependents are always loaded
public override void Required_one_to_one_are_cascade_deleted_in_store(
CascadeTiming? cascadeDeleteTiming,
Expand Down