Skip to content

Commit 716bba0

Browse files
authored
[release/7.0] Stop skipping shadow skip navigation slots when creating the shadow values array (#30911)
* Stop skipping shadow skip navigation slots when creating the shadow values array (#30902) * Add quirk
1 parent 1c77de6 commit 716bba0

File tree

8 files changed

+121
-11
lines changed

8 files changed

+121
-11
lines changed

src/EFCore/ChangeTracking/Internal/ShadowValuesFactoryFactory.cs

+4-2
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,17 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
1313
/// </summary>
1414
public class ShadowValuesFactoryFactory : SnapshotFactoryFactory<ValueBuffer>
1515
{
16+
private static readonly bool UseOldBehavior30764
17+
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue30764", out var enabled) && enabled;
18+
1619
/// <summary>
1720
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
1821
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
1922
/// any release. You should only use it directly in your code with extreme caution and knowing that
2023
/// doing so can result in application failures when updating to a new Entity Framework Core release.
2124
/// </summary>
2225
protected override int GetPropertyIndex(IPropertyBase propertyBase)
23-
// Navigations are not included in the supplied value buffer
24-
=> (propertyBase as IProperty)?.GetShadowIndex() ?? -1;
26+
=> UseOldBehavior30764 ? ((propertyBase as IProperty)?.GetShadowIndex() ?? -1) : propertyBase.GetShadowIndex();
2527

2628
/// <summary>
2729
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to

src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ protected virtual Expression CreateConstructorExpression(
4343
var count = GetPropertyCount(entityType);
4444

4545
var types = new Type[count];
46-
var propertyBases = new IPropertyBase[count];
46+
var propertyBases = new IPropertyBase?[count];
4747

4848
foreach (var propertyBase in entityType.GetPropertiesAndNavigations())
4949
{
@@ -95,7 +95,7 @@ protected virtual Expression CreateSnapshotExpression(
9595
Type? entityType,
9696
ParameterExpression parameter,
9797
Type[] types,
98-
IList<IPropertyBase> propertyBases)
98+
IList<IPropertyBase?> propertyBases)
9999
{
100100
var count = types.Length;
101101

src/EFCore/ChangeTracking/Internal/TemporaryValuesFactoryFactory.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ protected override Expression CreateSnapshotExpression(
2323
[DynamicallyAccessedMembers(IEntityType.DynamicallyAccessedMemberTypes)] Type? entityType,
2424
ParameterExpression parameter,
2525
Type[] types,
26-
IList<IPropertyBase> propertyBases)
26+
IList<IPropertyBase?> propertyBases)
2727
{
2828
var constructorExpression = Expression.Convert(
2929
Expression.New(

src/EFCore/Infrastructure/ExpressionExtensions.cs

+10-5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ namespace Microsoft.EntityFrameworkCore.Infrastructure;
2222
/// </remarks>
2323
public static class ExpressionExtensions
2424
{
25+
private static readonly bool UseOldBehavior30764
26+
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue30764", out var enabled) && enabled;
27+
2528
/// <summary>
2629
/// Creates a printable string representation of the given expression.
2730
/// </summary>
@@ -288,11 +291,13 @@ public static Expression CreateValueBufferReadValueExpression(
288291
Type type,
289292
int index,
290293
IPropertyBase? property)
291-
=> Expression.Call(
292-
MakeValueBufferTryReadValueMethod(type),
293-
valueBuffer,
294-
Expression.Constant(index),
295-
Expression.Constant(property, typeof(IPropertyBase)));
294+
=> (property is INavigationBase && !UseOldBehavior30764)
295+
? Expression.Constant(null, typeof(object))
296+
: Expression.Call(
297+
MakeValueBufferTryReadValueMethod(type),
298+
valueBuffer,
299+
Expression.Constant(index),
300+
Expression.Constant(property, typeof(IPropertyBase)));
296301

297302
/// <summary>
298303
/// <para>

src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs

+12-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ namespace Microsoft.EntityFrameworkCore.Query;
3232
/// </remarks>
3333
public abstract class ShapedQueryCompilingExpressionVisitor : ExpressionVisitor
3434
{
35+
private static readonly bool UseOldBehavior30764
36+
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue30764", out var enabled) && enabled;
37+
3538
private static readonly PropertyInfo CancellationTokenMemberInfo
3639
= typeof(QueryContext).GetTypeInfo().GetProperty(nameof(QueryContext.CancellationToken))!;
3740

@@ -586,7 +589,15 @@ private BlockExpression CreateFullMaterializeExpression(
586589
{
587590
var valueBufferExpression = Expression.Call(
588591
materializationContextVariable, MaterializationContext.GetValueBufferMethod);
589-
var shadowProperties = concreteEntityType.GetProperties().Where(p => p.IsShadowProperty());
592+
593+
var shadowProperties = UseOldBehavior30764
594+
? (IEnumerable<IPropertyBase>)concreteEntityType.GetProperties()
595+
.Where(p => p.IsShadowProperty())
596+
: concreteEntityType.GetProperties()
597+
.Concat<IPropertyBase>(concreteEntityType.GetNavigations())
598+
.Concat(concreteEntityType.GetSkipNavigations())
599+
.Where(n => n.IsShadowProperty())
600+
.OrderBy(e => e.GetShadowIndex());
590601

591602
blockExpressions.Add(
592603
Expression.Assign(

test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs

+73
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,17 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
549549
modelBuilder.Entity<Bayaz>();
550550
modelBuilder.Entity<SecondLaw>();
551551
modelBuilder.Entity<ThirdLaw>();
552+
553+
modelBuilder.Entity<Beetroot2>().HasData(
554+
new { Id = 1, Key = "root-1", Name = "Root One" });
555+
556+
modelBuilder.Entity<Lettuce2>().HasData(
557+
new { Id = 4, Key = "root-1/leaf-1", Name = "Leaf One-One", RootId = 1 });
558+
559+
modelBuilder.Entity<Radish2>()
560+
.HasMany(entity => entity.Entities)
561+
.WithMany()
562+
.UsingEntity<RootStructure>();
552563
}
553564

554565
protected virtual object CreateFullGraph()
@@ -3984,6 +3995,68 @@ public virtual SecondLaw SecondLaw
39843995
}
39853996
}
39863997

3998+
protected abstract class Parsnip2 : NotifyingEntity
3999+
{
4000+
private int _id;
4001+
4002+
public int Id
4003+
{
4004+
get => _id;
4005+
set => SetWithNotify(value, ref _id);
4006+
}
4007+
}
4008+
4009+
protected class Lettuce2 : Parsnip2
4010+
{
4011+
private Beetroot2 _root;
4012+
4013+
public Beetroot2 Root
4014+
{
4015+
get => _root;
4016+
set => SetWithNotify(value, ref _root);
4017+
}
4018+
}
4019+
4020+
protected class RootStructure : NotifyingEntity
4021+
{
4022+
private Guid _radish2Id;
4023+
private int _parsnip2Id;
4024+
4025+
public Guid Radish2Id
4026+
{
4027+
get => _radish2Id;
4028+
set => SetWithNotify(value, ref _radish2Id);
4029+
}
4030+
4031+
public int Parsnip2Id
4032+
{
4033+
get => _parsnip2Id;
4034+
set => SetWithNotify(value, ref _parsnip2Id);
4035+
}
4036+
}
4037+
4038+
protected class Radish2 : NotifyingEntity
4039+
{
4040+
private Guid _id;
4041+
private ICollection<Parsnip2> _entities = new ObservableHashSet<Parsnip2>();
4042+
4043+
public Guid Id
4044+
{
4045+
get => _id;
4046+
set => SetWithNotify(value, ref _id);
4047+
}
4048+
4049+
public ICollection<Parsnip2> Entities
4050+
{
4051+
get => _entities;
4052+
set => SetWithNotify(value, ref _entities);
4053+
}
4054+
}
4055+
4056+
protected class Beetroot2 : Parsnip2
4057+
{
4058+
}
4059+
39874060
protected class NotifyingEntity : INotifyPropertyChanging, INotifyPropertyChanged
39884061
{
39894062
protected void SetWithNotify<T>(T value, ref T field, [CallerMemberName] string propertyName = "")

test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs

+15
Original file line numberDiff line numberDiff line change
@@ -1932,4 +1932,19 @@ private static SecondLaw AddSecondLevel(bool thirdLevel1, bool thirdLevel2)
19321932

19331933
return secondLevel;
19341934
}
1935+
1936+
[ConditionalTheory] // Issue #30764
1937+
[InlineData(false)]
1938+
[InlineData(true)]
1939+
public virtual Task Shadow_skip_navigation_in_base_class_is_handled(bool async)
1940+
=> ExecuteWithStrategyInTransactionAsync(
1941+
async context =>
1942+
{
1943+
var entities = async
1944+
? await context.Set<Lettuce2>().ToListAsync()
1945+
: context.Set<Lettuce2>().ToList();
1946+
1947+
Assert.Equal(1, entities.Count);
1948+
Assert.Equal(nameof(Lettuce2), context.Entry(entities[0]).Property<string>("Discriminator").CurrentValue);
1949+
});
19351950
}

test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerOwnedTest.cs

+4
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ public override Task Update_root_by_collection_replacement_of_deleted_third_leve
3838
public override Task Sever_relationship_that_will_later_be_deleted(bool async)
3939
=> Task.CompletedTask;
4040

41+
// No owned types
42+
public override Task Shadow_skip_navigation_in_base_class_is_handled(bool async)
43+
=> Task.CompletedTask;
44+
4145
// Owned dependents are always loaded
4246
public override void Required_one_to_one_are_cascade_deleted_in_store(
4347
CascadeTiming? cascadeDeleteTiming,

0 commit comments

Comments
 (0)