Skip to content

Commit

Permalink
[release/7.0] Stop skipping shadow skip navigation slots when creatin…
Browse files Browse the repository at this point in the history
…g the shadow values array (#30911)

* Stop skipping shadow skip navigation slots when creating the shadow values array (#30902)

* Add quirk
  • Loading branch information
ajcvickers authored Jun 7, 2023
1 parent 1c77de6 commit 716bba0
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 11 deletions.
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

0 comments on commit 716bba0

Please sign in to comment.