Skip to content

Commit

Permalink
Always store original values for index properties
Browse files Browse the repository at this point in the history 
Since these are used by the update pipeline.

Fixes #18859
  • Loading branch information
@ajcvickers
ajcvickers committed Dec 31, 2019
1 parent 3149140 commit 0701c65
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 13 deletions.
3 changes: 2 additions & 1 deletion src/EFCore/Metadata/Internal/PropertyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ public static bool RequiresOriginalValue([NotNull] this IProperty property)
=> property.DeclaringEntityType.GetChangeTrackingStrategy() != ChangeTrackingStrategy.ChangingAndChangedNotifications
|| property.IsConcurrencyToken
|| property.IsKey()
|| property.IsForeignKey();
|| property.IsForeignKey()
|| property.IsIndex();

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
33 changes: 33 additions & 0 deletions test/EFCore.Specification.Tests/GraphUpdatesFixtureBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,14 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con

modelBuilder.Entity<Poost>();
modelBuilder.Entity<Bloog>();

modelBuilder.Entity<Produce>(
b =>
{
b.HasKey(e => e.ProduceId);
b.HasIndex(e => e.BarCode).IsUnique();
b.Property(e => e.ProduceId).ValueGeneratedOnAdd();
});
}

protected virtual object CreateFullGraph()
Expand Down Expand Up @@ -2706,6 +2714,31 @@ public ICollection<TaskChoice> Choices
}
}

protected class Produce : NotifyingEntity
{
private Guid _produceId;
private string _name;
private int _barCode;

public Guid ProduceId
{
get => _produceId;
set => SetWithNotify(value, ref _produceId);
}

public string Name
{
get => _name;
set => SetWithNotify(value, ref _name);
}

public int BarCode
{
get => _barCode;
set => SetWithNotify(value, ref _barCode);
}
}

protected class Bloog : NotifyingEntity
{
private int _id;
Expand Down
28 changes: 28 additions & 0 deletions test/EFCore.Specification.Tests/GraphUpdatesTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,34 @@ public virtual void Save_removed_optional_many_to_one_dependents(
});
}

[ConditionalFact]
public virtual void Notification_entities_can_have_indexes()
{
ExecuteWithStrategyInTransaction(
context =>
{
var produce = new Produce { Name = "Apple", BarCode = 77 };
context.Add(produce);
Assert.Equal(EntityState.Added, context.Entry(produce).State);
context.SaveChanges();
Assert.Equal(EntityState.Unchanged, context.Entry(produce).State);
Assert.NotEqual(Guid.Empty, context.Entry(produce).Property(e => e.ProduceId).OriginalValue);
Assert.Equal(77, context.Entry(produce).Property(e => e.BarCode).OriginalValue);
context.Remove(produce);
Assert.Equal(EntityState.Deleted, context.Entry(produce).State);
Assert.NotEqual(Guid.Empty, context.Entry(produce).Property(e => e.ProduceId).OriginalValue);
Assert.Equal(77, context.Entry(produce).Property(e => e.BarCode).OriginalValue);
context.SaveChanges();
Assert.Equal(EntityState.Detached, context.Entry(produce).State);
});
}

[ConditionalFact]
public virtual void Resetting_a_deleted_reference_fixes_up_again()
{
Expand Down
35 changes: 23 additions & 12 deletions test/EFCore.Tests/Metadata/Internal/EntityTypeTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2580,10 +2580,11 @@ public void All_properties_have_original_value_indexes_when_using_snapshot_chang

Assert.Equal(0, entityType.FindProperty("Id").GetOriginalValueIndex());
Assert.Equal(1, entityType.FindProperty("AnotherEntityId").GetOriginalValueIndex());
Assert.Equal(2, entityType.FindProperty("Name").GetOriginalValueIndex());
Assert.Equal(3, entityType.FindProperty("Token").GetOriginalValueIndex());
Assert.Equal(2, entityType.FindProperty("Index").GetOriginalValueIndex());
Assert.Equal(3, entityType.FindProperty("Name").GetOriginalValueIndex());
Assert.Equal(4, entityType.FindProperty("Token").GetOriginalValueIndex());

Assert.Equal(4, entityType.OriginalValueCount());
Assert.Equal(5, entityType.OriginalValueCount());
}

[ConditionalFact]
Expand All @@ -2595,6 +2596,7 @@ public void All_relationship_properties_have_relationship_indexes_when_using_sna

Assert.Equal(0, entityType.FindProperty("Id").GetRelationshipIndex());
Assert.Equal(1, entityType.FindProperty("AnotherEntityId").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Index").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Name").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Token").GetRelationshipIndex());
Assert.Equal(2, entityType.FindNavigation("CollectionNav").GetRelationshipIndex());
Expand All @@ -2612,10 +2614,11 @@ public void All_properties_have_original_value_indexes_when_using_changed_only_t

Assert.Equal(0, entityType.FindProperty("Id").GetOriginalValueIndex());
Assert.Equal(1, entityType.FindProperty("AnotherEntityId").GetOriginalValueIndex());
Assert.Equal(2, entityType.FindProperty("Name").GetOriginalValueIndex());
Assert.Equal(3, entityType.FindProperty("Token").GetOriginalValueIndex());
Assert.Equal(2, entityType.FindProperty("Index").GetOriginalValueIndex());
Assert.Equal(3, entityType.FindProperty("Name").GetOriginalValueIndex());
Assert.Equal(4, entityType.FindProperty("Token").GetOriginalValueIndex());

Assert.Equal(4, entityType.OriginalValueCount());
Assert.Equal(5, entityType.OriginalValueCount());
}

[ConditionalFact]
Expand All @@ -2627,6 +2630,7 @@ public void Collections_dont_have_relationship_indexes_when_using_changed_only_c

Assert.Equal(0, entityType.FindProperty("Id").GetRelationshipIndex());
Assert.Equal(1, entityType.FindProperty("AnotherEntityId").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Index").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Name").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Token").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindNavigation("CollectionNav").GetRelationshipIndex());
Expand All @@ -2636,7 +2640,7 @@ public void Collections_dont_have_relationship_indexes_when_using_changed_only_c
}

[ConditionalFact]
public void Only_concurrency_and_key_properties_have_original_value_indexes_when_using_full_notifications()
public void Only_concurrency_index_and_key_properties_have_original_value_indexes_when_using_full_notifications()
{
var entityType = BuildFullNotificationEntityModel().FindEntityType(typeof(FullNotificationEntity));
entityType.SetChangeTrackingStrategy(ChangeTrackingStrategy.ChangingAndChangedNotifications);
Expand All @@ -2645,9 +2649,10 @@ public void Only_concurrency_and_key_properties_have_original_value_indexes_when
Assert.Equal(0, entityType.FindProperty("Id").GetOriginalValueIndex());
Assert.Equal(1, entityType.FindProperty("AnotherEntityId").GetOriginalValueIndex());
Assert.Equal(-1, entityType.FindProperty("Name").GetOriginalValueIndex());
Assert.Equal(2, entityType.FindProperty("Token").GetOriginalValueIndex());
Assert.Equal(2, entityType.FindProperty("Index").GetOriginalValueIndex());
Assert.Equal(3, entityType.FindProperty("Token").GetOriginalValueIndex());

Assert.Equal(3, entityType.OriginalValueCount());
Assert.Equal(4, entityType.OriginalValueCount());
}

[ConditionalFact]
Expand All @@ -2659,6 +2664,7 @@ public void Collections_dont_have_relationship_indexes_when_using_full_notificat

Assert.Equal(0, entityType.FindProperty("Id").GetRelationshipIndex());
Assert.Equal(1, entityType.FindProperty("AnotherEntityId").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Index").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Name").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Token").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindNavigation("CollectionNav").GetRelationshipIndex());
Expand All @@ -2676,10 +2682,11 @@ public void All_properties_have_original_value_indexes_when_full_notifications_w

Assert.Equal(0, entityType.FindProperty("Id").GetOriginalValueIndex());
Assert.Equal(1, entityType.FindProperty("AnotherEntityId").GetOriginalValueIndex());
Assert.Equal(2, entityType.FindProperty("Name").GetOriginalValueIndex());
Assert.Equal(3, entityType.FindProperty("Token").GetOriginalValueIndex());
Assert.Equal(2, entityType.FindProperty("Index").GetOriginalValueIndex());
Assert.Equal(3, entityType.FindProperty("Name").GetOriginalValueIndex());
Assert.Equal(4, entityType.FindProperty("Token").GetOriginalValueIndex());

Assert.Equal(4, entityType.OriginalValueCount());
Assert.Equal(5, entityType.OriginalValueCount());
}

[ConditionalFact]
Expand All @@ -2691,6 +2698,7 @@ public void Collections_dont_have_relationship_indexes_when_full_notifications_w

Assert.Equal(0, entityType.FindProperty("Id").GetRelationshipIndex());
Assert.Equal(1, entityType.FindProperty("AnotherEntityId").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Index").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Name").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindProperty("Token").GetRelationshipIndex());
Assert.Equal(-1, entityType.FindNavigation("CollectionNav").GetRelationshipIndex());
Expand Down Expand Up @@ -2869,6 +2877,8 @@ private static IMutableModel BuildFullNotificationEntityModel()
.WithOne();
b.Property(e => e.Token).IsConcurrencyToken();
b.HasIndex(e => e.Index);
});

return (Model)builder.Model;
Expand All @@ -2880,6 +2890,7 @@ private class FullNotificationEntity : INotifyPropertyChanging, INotifyPropertyC
public int Id { get; set; }
public string Name { get; set; }
public int Token { get; set; }
public int Index { get; set; }

public AnotherEntity ReferenceNav { get; set; }
public int AnotherEntityId { get; set; }
Expand Down

0 comments on commit 0701c65

Please sign in to comment.