Skip to content

Commit

Permalink
Don't consider a generated value stable just because it was explicitl…
Browse files Browse the repository at this point in the history
…y set

Fixes #32084
  • Loading branch information
ajcvickers committed Dec 2, 2023
1 parent 32c6133 commit 297be87
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 5 deletions.
12 changes: 10 additions & 2 deletions src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,23 @@ public virtual bool Generate(InternalEntityEntry entry, bool includePrimaryKey =
//TODO: Handle complex properties
foreach (var property in entry.EntityType.GetValueGeneratingProperties())
{
var valueGenerator = GetValueGenerator(property);
if (valueGenerator.GeneratesStableValues)
{
hasStableValues = true;
}
else
{
hasNonStableValues = true;
}

if (entry.HasExplicitValue(property)
|| (!includePrimaryKey
&& property.IsPrimaryKey()))
{
continue;
}

var valueGenerator = GetValueGenerator(property);

var generatedValue = valueGenerator.Next(entityEntry);
var temporary = valueGenerator.GeneratesTemporaryValues;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ private void Compare_returns_0_only_for_entries_that_have_same_key_values_generi

var keyProperty = entityType.AddProperty("Id", typeof(T));
keyProperty.IsNullable = false;
keyProperty.ValueGenerated = ValueGenerated.Never;
entityType.SetPrimaryKey(keyProperty);

var model = modelBuilder.FinalizeModel();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,12 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.OwnsMany(e => e.OptionalChildren).OwnsMany(e => e.Children);
b.OwnsMany(e => e.RequiredChildren).OwnsMany(e => e.Children);
});

modelBuilder.Entity<ParentEntity32084>().HasOne(x => x.Child)
.WithOne()
.HasForeignKey<ChildBaseEntity32084>(x => x.ParentId);

modelBuilder.Entity<ChildEntity32084>();
}

protected virtual object CreateFullGraph()
Expand Down Expand Up @@ -4341,6 +4347,53 @@ protected class Beetroot2 : Parsnip2
{
}

protected class ParentEntity32084 : NotifyingEntity
{
private Guid _id;
private ChildBaseEntity32084 _child;

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

public ChildBaseEntity32084 Child
{
get => _child;
set => SetWithNotify(value, ref _child);
}
}

protected abstract class ChildBaseEntity32084 : NotifyingEntity
{
private Guid _id;
private Guid _parentId;

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

public Guid ParentId
{
get => _parentId;
set => SetWithNotify(value, ref _parentId);
}
}

protected class ChildEntity32084 : ChildBaseEntity32084
{
private string _childValue;

public string ChildValue
{
get => _childValue;
set => SetWithNotify(value, ref _childValue);
}
}

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 @@ -2122,4 +2122,45 @@ public virtual Task Shadow_skip_navigation_in_base_class_is_handled(bool async)
Assert.Equal(1, entities.Count);
Assert.Equal(nameof(Lettuce2), context.Entry(entities[0]).Property<string>("Discriminator").CurrentValue);
});

[ConditionalTheory] // Issue #32084
[InlineData(false)]
[InlineData(true)]
public virtual Task Mark_explicitly_set_dependent_appropriately_with_any_inheritance_and_stable_generator(bool async)
{
var parentId = Guid.NewGuid();
var childId = Guid.NewGuid();

return ExecuteWithStrategyInTransactionAsync(
async context =>
{
context.Add(new ParentEntity32084 { Id = parentId });
_ = async ? await context.SaveChangesAsync() : context.SaveChanges();
},
async context =>
{
var parent = async
? await context.FindAsync<ParentEntity32084>(parentId)
: context.Find<ParentEntity32084>(parentId);

var child = new ChildEntity32084
{
Id = childId,
ParentId = parent!.Id,
ChildValue = "test value"
};

parent.Child = child;

context.ChangeTracker.DetectChanges();

Assert.Equal(2, context.ChangeTracker.Entries().Count());
Assert.Equal(EntityState.Modified, context.Entry(child).State);
Assert.Equal(EntityState.Unchanged, context.Entry(parent).State);

await Assert.ThrowsAsync<DbUpdateConcurrencyException>(
async () => _ = async ? await context.SaveChangesAsync() : context.SaveChanges());

});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ public GraphUpdatesSqlServerOwnedTest(SqlServerFixture fixture)
public override Task Update_root_by_collection_replacement_of_inserted_first_level(bool async)
=> Task.CompletedTask;

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

// No owned types
public override Task Update_root_by_collection_replacement_of_deleted_first_level(bool async)
=> Task.CompletedTask;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
modelBuilder.Entity<Poost>().ToTable(nameof(Poost));
modelBuilder.Entity<Bloog>().ToTable(nameof(Bloog));
modelBuilder.Entity<Produce>().ToTable(nameof(Produce));
modelBuilder.Entity<ParentEntity32084>().ToTable(nameof(ParentEntity32084));
modelBuilder.Entity<ChildBaseEntity32084>().ToTable(nameof(ChildBaseEntity32084));
modelBuilder.Entity<ChildEntity32084>().ToTable(nameof(ChildEntity32084));
}
}
}
6 changes: 3 additions & 3 deletions test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2299,17 +2299,17 @@ protected internal override void OnModelCreating(ModelBuilder modelBuilder)
modelBuilder
.Entity<Cat>()
.Property(e => e.Id)
.HasValueGenerator<InMemoryIntegerValueGenerator<int>>();
.HasValueGenerator((_, __) => new ResettableValueGenerator());

modelBuilder
.Entity<Hat>()
.Property(e => e.Id)
.HasValueGenerator<InMemoryIntegerValueGenerator<int>>();
.HasValueGenerator((_, __) => new ResettableValueGenerator());

modelBuilder.Entity<Mat>(
b =>
{
b.Property(e => e.Id).HasValueGenerator<InMemoryIntegerValueGenerator<int>>();
b.Property(e => e.Id).HasValueGenerator((_, __) => new ResettableValueGenerator());
b.HasMany(e => e.Cats)
.WithMany(e => e.Mats)
.UsingEntity<CatMat>(
Expand Down

0 comments on commit 297be87

Please sign in to comment.