Skip to content

Commit

Permalink
Allow ignored key values for fixup before saving using TPT (#32439)
Browse files Browse the repository at this point in the history
  • Loading branch information
ajcvickers authored Nov 30, 2023
1 parent caaf394 commit 17fa62f
Show file tree
Hide file tree
Showing 10 changed files with 276 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ static bool IsMapped(IReadOnlyForeignKey foreignKey, StoreObjectIdentifier store

/// <summary>
/// <para>
/// Finds the first <see cref="IConventionForeignKey" /> that is mapped to the same constraint in a shared table-like object.
/// Finds the first <see cref="IForeignKey" /> that is mapped to the same constraint in a shared table-like object.
/// </para>
/// <para>
/// This method is typically used by database providers (and other extensions). It is generally
Expand Down
12 changes: 6 additions & 6 deletions src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -673,8 +673,8 @@ private void AddForeignKeyEdges()
{
if (!CanCreateDependency(foreignKey, command, principal: true)
|| !IsModified(foreignKey.PrincipalKey.Properties, entry)
|| command.Table != null
&& !HasTempKey(entry, foreignKey.PrincipalKey))
|| (command.Table != null
&& !IsStoreGenerated(entry, foreignKey.PrincipalKey)))
{
continue;
}
Expand Down Expand Up @@ -843,7 +843,7 @@ private void AddForeignKeyEdges()
}
}

private static bool HasTempKey(IUpdateEntry entry, IKey key)
private static bool IsStoreGenerated(IUpdateEntry entry, IKey key)
{
var keyProperties = key.Properties;

Expand All @@ -853,7 +853,7 @@ private static bool HasTempKey(IUpdateEntry entry, IKey key)
{
var keyProperty = keyProperties[i];

if (entry.HasTemporaryValue(keyProperty))
if (entry.IsStoreGenerated(keyProperty))
{
return true;
}
Expand Down Expand Up @@ -1052,7 +1052,7 @@ private void AddMatchingPredecessorEdge<T>(
for (var j = 0; j < predecessor.Entries.Count; j++)
{
var entry = predecessor.Entries[j];
if (HasTempKey(entry, foreignKey.PrincipalKey))
if (IsStoreGenerated(entry, foreignKey.PrincipalKey))
{
requiresBatchingBoundary = true;
goto AfterLoop;
Expand Down Expand Up @@ -1096,7 +1096,7 @@ private void AddMatchingPredecessorEdge<T>(
foreach (var key in foreignKey.PrincipalUniqueConstraint.MappedKeys)
{
if (key.DeclaringEntityType.IsAssignableFrom(entry.EntityType)
&& HasTempKey(entry, key))
&& IsStoreGenerated(entry, key))
{
requiresBatchingBoundary = true;
goto AfterLoop;
Expand Down
4 changes: 3 additions & 1 deletion src/EFCore.Relational/Update/ModificationCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,9 @@ void HandleColumn(IColumnMappingBase columnMapping)
{
if (adding)
{
writeValue = property.GetBeforeSaveBehavior() == PropertySaveBehavior.Save;
writeValue = property.GetBeforeSaveBehavior() == PropertySaveBehavior.Save
|| entry.HasStoreGeneratedValue(property);

columnPropagator?.TryPropagate(columnMapping, entry);
}
else if (storedProcedureParameter is not { ForOriginalValue: true }
Expand Down
12 changes: 11 additions & 1 deletion src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1813,14 +1813,24 @@ public void DiscardStoreGeneratedValues()
public bool IsStoreGenerated(IProperty property)
=> (property.ValueGenerated.ForAdd()
&& EntityState == EntityState.Added
&& (property.GetBeforeSaveBehavior() == PropertySaveBehavior.Ignore
&& ((property.GetBeforeSaveBehavior() == PropertySaveBehavior.Ignore
&& GetValueType(property) != CurrentValueType.StoreGenerated)
|| HasTemporaryValue(property)
|| !HasExplicitValue(property)))
|| (property.ValueGenerated.ForUpdate()
&& (EntityState is EntityState.Modified or EntityState.Deleted)
&& (property.GetAfterSaveBehavior() == PropertySaveBehavior.Ignore
|| !IsModified(property)));

/// <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>
public bool HasStoreGeneratedValue(IProperty property)
=> GetValueType(property) == CurrentValueType.StoreGenerated;

/// <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
Expand Down
7 changes: 7 additions & 0 deletions src/EFCore/Update/IUpdateEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ public interface IUpdateEntry
/// <returns><see langword="true" /> if the property has a temporary value, otherwise <see langword="false" />.</returns>
bool HasTemporaryValue(IProperty property);

/// <summary>
/// Gets a value indicating if the specified property has a store-generated value that has not yet been saved to the entity.
/// </summary>
/// <param name="property">The property to be checked.</param>
/// <returns><see langword="true" /> if the property has a store-gen value, otherwise <see langword="false" />.</returns>
bool HasStoreGeneratedValue(IProperty property);

/// <summary>
/// Gets a value indicating if the specified property should have a value generated by the database.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,9 @@ public TProperty GetOriginalValue<TProperty>(IProperty property)
public bool HasTemporaryValue(IProperty property)
=> throw new NotImplementedException();

public bool HasStoreGeneratedValue(IProperty property)
=> throw new NotImplementedException();

public bool IsModified(IProperty property)
=> throw new NotImplementedException();

Expand Down
240 changes: 240 additions & 0 deletions test/EFCore.Specification.Tests/Update/UpdatesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,238 @@ public Task SaveChanges_false_processes_all_tracked_entities_without_calling_Acc
});
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public Task Ignore_before_save_property_is_still_generated(bool async)
=> ExecuteWithStrategyInTransactionAsync(
async context =>
{
var entities = new List<object>
{
new CupCake
{
Id = -1102,
Name = "B2",
CakeName = "C2",
CupCakeName = "CC2"
}
};

if (async)
{
await context.AddRangeAsync(entities);
await context.SaveChangesAsync();
}
else
{
context.AddRange(entities);
context.SaveChanges();
}
},
async context =>
{
var query = context.Set<Baked>().Include(e => e.Tin).Include(e => e.Ingredients).Include(e => ((Muffin)e).Top);
var bakedGoods = async ? await query.ToListAsync() : query.ToList();

Assert.Equal(1, bakedGoods.Count);
Assert.Equal("B2", bakedGoods[0].Name);
Assert.Equal("C2", ((Cake)bakedGoods[0]).CakeName);
Assert.Equal("CC2", ((CupCake)bakedGoods[0]).CupCakeName);
});

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public Task Ignore_before_save_property_is_still_generated_graph(bool async)
=> ExecuteWithStrategyInTransactionAsync(
async context =>
{
var entities = new List<object>
{
new Baked { Id = -100, Name = "B0" },
new Tin
{
Id = -1000,
BakedId = -100,
TinName = "B0"
},
new Ingredient
{
Id = -2000,
BakedId = -100,
IngredientName = "B00"
},
new Ingredient
{
Id = -2001,
BakedId = -100,
IngredientName = "B01"
},
new Cake
{
Id = -101,
Name = "B1",
CakeName = "C1"
},
new Tin
{
Id = -1001,
BakedId = -101,
TinName = "B1"
},
new Ingredient
{
Id = -2010,
BakedId = -101,
IngredientName = "B10"
},
new Ingredient
{
Id = -2011,
BakedId = -101,
IngredientName = "B11"
},
new CupCake
{
Id = -102,
Name = "B2",
CakeName = "C2",
CupCakeName = "CC2"
},
new Tin
{
Id = -1002,
BakedId = -102,
TinName = "B2"
},
new Ingredient
{
Id = -2020,
BakedId = -102,
IngredientName = "B20"
},
new Ingredient
{
Id = -2021,
BakedId = -102,
IngredientName = "B21"
},
new Muffin
{
Id = -103,
Name = "B3",
MuffinName = "M1"
},
new Tin
{
Id = -1003,
BakedId = -103,
TinName = "B3"
},
new Ingredient
{
Id = -2030,
BakedId = -103,
IngredientName = "B30"
},
new Ingredient
{
Id = -2031,
BakedId = -103,
IngredientName = "B31"
},
new Top
{
Id = -3003,
MuffinId = -103,
TopName = "M1"
}
};

if (async)
{
await context.AddRangeAsync(entities);
await context.SaveChangesAsync();
}
else
{
context.AddRange(entities);
context.SaveChanges();
}
},
async context =>
{
var query = context.Set<Baked>().Include(e => e.Tin).Include(e => e.Ingredients).Include(e => ((Muffin)e).Top);
var bakedGoods = async ? await query.ToListAsync() : query.ToList();

Assert.Equal(4, bakedGoods.Count);
AssertFixup("B0");
AssertFixup("B1");
AssertFixup("B2");
AssertFixup("B3");

var muffin = bakedGoods.OfType<Muffin>().Single();
Assert.Equal("M1", muffin.MuffinName);
Assert.Equal("M1", muffin.Top.TopName);

void AssertFixup(string bakedName)
{
var b0 = bakedGoods.Single(e => e.Name == bakedName);
Assert.Equal(bakedName, b0.Tin!.TinName);
Assert.Equal(2, b0.Ingredients.Count);
Assert.Contains(bakedName + "0", b0.Ingredients.Select(e => e.IngredientName));
Assert.Contains(bakedName + "1", b0.Ingredients.Select(e => e.IngredientName));
}
});

protected class Baked
{
public long Id { get; set; }
public required string Name { get; set; }
public Tin? Tin { get; set; }
public List<Ingredient> Ingredients { get; } = new();
}

protected class Cake : Baked
{
public required string CakeName { get; set; }
}

protected class CupCake : Cake
{
public required string CupCakeName { get; set; }
}

protected class Muffin : Baked
{
public required string MuffinName { get; set; }
public Top Top { get; set; } = null!;

}

protected class Tin
{
public long Id { get; set; }
public long? BakedId { get; set; }
public string? TinName { get; set; }
public Baked? Baked { get; set; }
}

protected class Ingredient
{
public long Id { get; set; }
public long BakedId { get; set; }
public string? IngredientName { get; set; }
public Baked Baked { get; set; } = null!;
}

protected class Top
{
public long Id { get; set; }
public long MuffinId { get; set; }
public string? TopName { get; set; }
public Muffin Muffin { get; set; } = null!;
}

protected abstract string UpdateConcurrencyMessage { get; }

protected abstract string UpdateConcurrencyTokenMessage { get; }
Expand Down Expand Up @@ -699,6 +931,14 @@ protected override string StoreName

protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
{
modelBuilder.Entity<Baked>().Property(e => e.Id).Metadata.SetBeforeSaveBehavior(PropertySaveBehavior.Ignore);
modelBuilder.Entity<Cake>();
modelBuilder.Entity<CupCake>();
modelBuilder.Entity<Muffin>();
modelBuilder.Entity<Tin>().Property(e => e.Id).Metadata.SetBeforeSaveBehavior(PropertySaveBehavior.Ignore);
modelBuilder.Entity<Ingredient>().Property(e => e.Id).Metadata.SetBeforeSaveBehavior(PropertySaveBehavior.Ignore);
modelBuilder.Entity<Top>().Property(e => e.Id).Metadata.SetBeforeSaveBehavior(PropertySaveBehavior.Ignore);

modelBuilder.Entity<Product>().HasMany(e => e.ProductCategories).WithOne()
.HasForeignKey(e => e.ProductId);
modelBuilder.Entity<ProductWithBytes>().HasMany(e => e.ProductCategories).WithOne()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
modelBuilder.Entity<Category>().UseTpcMappingStrategy();
modelBuilder.Entity<GiftObscurer>().UseTpcMappingStrategy();
modelBuilder.Entity<LiftObscurer>().UseTpcMappingStrategy();
modelBuilder.Entity<Baked>().UseTpcMappingStrategy();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
modelBuilder.Entity<Category>().UseTptMappingStrategy();
modelBuilder.Entity<GiftObscurer>().UseTptMappingStrategy();
modelBuilder.Entity<LiftObscurer>().UseTptMappingStrategy();
modelBuilder.Entity<Baked>().UseTptMappingStrategy();
}
}
}
3 changes: 3 additions & 0 deletions test/EFCore.Tests/ExceptionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ public bool IsModified(IProperty property)
public bool HasTemporaryValue(IProperty property)
=> throw new NotImplementedException();

public bool HasStoreGeneratedValue(IProperty property)
=> throw new NotImplementedException();

public bool IsStoreGenerated(IProperty property)
=> throw new NotImplementedException();

Expand Down

0 comments on commit 17fa62f

Please sign in to comment.