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

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

Merged
merged 2 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -20,8 +20,7 @@ public class ShadowValuesFactoryFactory : SnapshotFactoryFactory<ValueBuffer>
/// 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;
=> propertyBase.GetShadowIndex();

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,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 @@ -93,7 +93,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
12 changes: 7 additions & 5 deletions src/EFCore/Infrastructure/ExpressionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,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
? Expression.Constant(null, typeof(object))
: Expression.Call(
MakeValueBufferTryReadValueMethod(type),
valueBuffer,
Expression.Constant(index),
Expression.Constant(property, typeof(IPropertyBase)));

/// <summary>
/// <para>
Expand Down
6 changes: 5 additions & 1 deletion src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,11 @@ private BlockExpression CreateFullMaterializeExpression(
{
var valueBufferExpression = Expression.Call(
materializationContextVariable, MaterializationContext.GetValueBufferMethod);
var shadowProperties = concreteEntityType.GetProperties().Where(p => p.IsShadowProperty());

var shadowProperties = concreteEntityType.GetProperties()
.Concat<IPropertyBase>(concreteEntityType.GetSkipNavigations())
ajcvickers marked this conversation as resolved.
Show resolved Hide resolved
.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 @@ -555,6 +555,17 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.HasOne(x => x.Parent).WithMany(x => x.Children).OnDelete(DeleteBehavior.Restrict);
b.HasAlternateKey(x => new { x.Id, x.ParentId });
});

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 @@ -4073,6 +4084,68 @@ public virtual NaiveParent Parent
}
}

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 @@ -1983,4 +1983,19 @@ public virtual Task Alternate_key_over_foreign_key_doesnt_bypass_delete_behavior
: context.SaveChanges();
})).Message);
});

[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 @@ -42,6 +42,10 @@ public override Task Sever_relationship_that_will_later_be_deleted(bool async)
public override Task Alternate_key_over_foreign_key_doesnt_bypass_delete_behavior(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