From a9e65b1b5a6908a149ee4e7887a89a69de9a2296 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Sat, 10 Sep 2022 14:36:13 +0100 Subject: [PATCH] Generated stable values to not imply that an entity is Added Fixes #26448 --- .../Internal/EntityGraphAttacher.cs | 3 +- .../Internal/IValueGenerationManager.cs | 12 +---- .../Internal/InternalEntityEntry.cs | 42 ++++++++++++----- .../Internal/ValueGenerationManager.cs | 47 ++++++++++++------- .../ChangeTracking/ChangeTrackerTest.cs | 38 +++++++++++++++ 5 files changed, 102 insertions(+), 40 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs index d5b5fb306bd..ba10d87d7d9 100644 --- a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs +++ b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs @@ -127,7 +127,8 @@ private bool PaintAction( ? (isGenerated ? storeGenTargetState : targetState) : EntityState.Added, // Key can only be not-set if it is store-generated acceptChanges: true, - forceStateWhenUnknownKey: force ? targetState : null); + forceStateWhenUnknownKey: force ? targetState : null, + fallbackState: targetState); } return true; diff --git a/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs b/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs index 7310b1f33df..deacc268997 100644 --- a/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs +++ b/src/EFCore/ChangeTracking/Internal/IValueGenerationManager.cs @@ -23,7 +23,7 @@ public interface IValueGenerationManager /// 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. /// - void Generate(InternalEntityEntry entry, bool includePrimaryKey = true); + bool Generate(InternalEntityEntry entry, bool includePrimaryKey = true); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -39,16 +39,8 @@ public interface IValueGenerationManager /// 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. /// - Task GenerateAsync( + Task GenerateAsync( InternalEntityEntry entry, bool includePrimaryKey = true, CancellationToken cancellationToken = default); - - /// - /// 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. - /// - bool MayGetTemporaryValue(IProperty property, IEntityType entityType); } diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs index ae03f45f7f6..8fee8e0d56c 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs @@ -146,19 +146,28 @@ public void SetEntityState( EntityState entityState, bool acceptChanges = false, bool modifyProperties = true, - EntityState? forceStateWhenUnknownKey = null) + EntityState? forceStateWhenUnknownKey = null, + EntityState? fallbackState = null) { var oldState = _stateData.EntityState; - var adding = PrepareForAdd(entityState); - - entityState = PropagateToUnknownKey(oldState, entityState, adding, forceStateWhenUnknownKey); + bool adding; + Setup(); - if (adding || oldState is EntityState.Detached) + if ((adding || oldState is EntityState.Detached) + && StateManager.ValueGenerationManager.Generate(this, includePrimaryKey: adding) + && fallbackState.HasValue) { - StateManager.ValueGenerationManager.Generate(this, includePrimaryKey: adding); + entityState = fallbackState.Value; + Setup(); } SetEntityState(oldState, entityState, acceptChanges, modifyProperties); + + void Setup() + { + adding = PrepareForAdd(entityState); + entityState = PropagateToUnknownKey(oldState, entityState, adding, forceStateWhenUnknownKey); + } } /// @@ -172,20 +181,29 @@ public async Task SetEntityStateAsync( bool acceptChanges = false, bool modifyProperties = true, EntityState? forceStateWhenUnknownKey = null, + EntityState? fallbackState = null, CancellationToken cancellationToken = default) { var oldState = _stateData.EntityState; - var adding = PrepareForAdd(entityState); + bool adding; + Setup(); - entityState = PropagateToUnknownKey(oldState, entityState, adding, forceStateWhenUnknownKey); - - if (adding || oldState is EntityState.Detached) + if ((adding || oldState is EntityState.Detached) + && await StateManager.ValueGenerationManager + .GenerateAsync(this, includePrimaryKey: adding, cancellationToken).ConfigureAwait(false) + && fallbackState.HasValue) { - await StateManager.ValueGenerationManager.GenerateAsync(this, includePrimaryKey: adding, cancellationToken) - .ConfigureAwait(false); + entityState = fallbackState.Value; + Setup(); } SetEntityState(oldState, entityState, acceptChanges, modifyProperties); + + void Setup() + { + adding = PrepareForAdd(entityState); + entityState = PropagateToUnknownKey(oldState, entityState, adding, forceStateWhenUnknownKey); + } } private EntityState PropagateToUnknownKey( diff --git a/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs b/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs index ac0a5d43f00..b7684119a74 100644 --- a/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs +++ b/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs @@ -1,8 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using Microsoft.EntityFrameworkCore.Metadata.Internal; - namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal; /// @@ -65,9 +63,12 @@ public ValueGenerationManager( /// 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. /// - public virtual void Generate(InternalEntityEntry entry, bool includePrimaryKey = true) + public virtual bool Generate(InternalEntityEntry entry, bool includePrimaryKey = true) { var entityEntry = new EntityEntry(entry); + var hasStableValues = false; + var hasNonStableValues = false; + foreach (var property in entry.EntityType.GetValueGeneratingProperties()) { if (!entry.HasDefaultValue(property) @@ -82,12 +83,23 @@ public virtual void Generate(InternalEntityEntry entry, bool includePrimaryKey = var generatedValue = valueGenerator.Next(entityEntry); var temporary = valueGenerator.GeneratesTemporaryValues; + if (valueGenerator.GeneratesStableValues) + { + hasStableValues = true; + } + else + { + hasNonStableValues = true; + } + Log(entry, property, generatedValue, temporary); SetGeneratedValue(entry, property, generatedValue, temporary); MarkKeyUnknown(entry, includePrimaryKey, property, valueGenerator); } + + return hasStableValues && !hasNonStableValues; } private void Log(InternalEntityEntry entry, IProperty property, object? generatedValue, bool temporary) @@ -108,13 +120,14 @@ private void Log(InternalEntityEntry entry, IProperty property, object? generate /// 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. /// - public virtual async Task GenerateAsync( + public virtual async Task GenerateAsync( InternalEntityEntry entry, bool includePrimaryKey = true, CancellationToken cancellationToken = default) { var entityEntry = new EntityEntry(entry); - + var hasStableValues = false; + var hasNonStableValues = false; foreach (var property in entry.EntityType.GetValueGeneratingProperties()) { if (!entry.HasDefaultValue(property) @@ -125,10 +138,18 @@ public virtual async Task GenerateAsync( } var valueGenerator = GetValueGenerator(property); - var generatedValue = await valueGenerator.NextAsync(entityEntry, cancellationToken) - .ConfigureAwait(false); + var generatedValue = await valueGenerator.NextAsync(entityEntry, cancellationToken).ConfigureAwait(false); var temporary = valueGenerator.GeneratesTemporaryValues; + if (valueGenerator.GeneratesStableValues) + { + hasStableValues = true; + } + else + { + hasNonStableValues = true; + } + Log(entry, property, generatedValue, temporary); SetGeneratedValue( @@ -139,21 +160,13 @@ public virtual async Task GenerateAsync( MarkKeyUnknown(entry, includePrimaryKey, property, valueGenerator); } + + return hasStableValues && !hasNonStableValues; } private ValueGenerator GetValueGenerator(IProperty property) => _valueGeneratorSelector.Select(property, property.DeclaringEntityType); - /// - /// 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. - /// - public virtual bool MayGetTemporaryValue(IProperty property, IEntityType entityType) - => property.RequiresValueGenerator() - && _valueGeneratorSelector.Select(property, entityType).GeneratesTemporaryValues; - private static void SetGeneratedValue(InternalEntityEntry entry, IProperty property, object? generatedValue, bool isTemporary) { if (generatedValue != null) diff --git a/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs b/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs index 19329e44a34..39dca2a350f 100644 --- a/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs +++ b/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs @@ -3549,6 +3549,23 @@ public void Clearing_change_tracker_resets_local_view_count() Assert.Equal(originalCount, context.Cats.Local.Count); } + [ConditionalFact] // Issue #26448 + public void Stable_generated_values_do_not_force_Added_state() + { + using var context = new EarlyLearningCenter(); + + Assert.Equal(EntityState.Added, context.Add(new Stable()).State); + + context.ChangeTracker.Clear(); + Assert.Equal(EntityState.Modified, context.Update(new Stable()).State); + + context.ChangeTracker.Clear(); + Assert.Equal(EntityState.Unchanged, context.Attach(new Stable()).State); + + context.ChangeTracker.Clear(); + Assert.Equal(EntityState.Deleted, context.Remove(new Stable()).State); + } + private static void AssertValuesSaved(int id, int someInt, string? someString) { using var context = new TheShadows(); @@ -3749,6 +3766,23 @@ public class Buggy public int Id { get; set; } } + private class Stable + { + public Guid Id { get; set; } + } + + private class TenantIdGenerator : ValueGenerator + { + public override Guid Next(EntityEntry entry) + => Guid.Parse("98D06A82-C691-4988-EA39-08D98E2C8D8F"); + + public override bool GeneratesTemporaryValues + => false; + + public override bool GeneratesStableValues + => true; + } + private class EarlyLearningCenter : DbContext { private readonly IInterceptor[] _interceptors; @@ -3856,6 +3890,10 @@ protected internal override void OnModelCreating(ModelBuilder modelBuilder) .HasForeignKey("BobbyId") .OnDelete(DeleteBehavior.Cascade); }); + + modelBuilder.Entity() + .Property(e => e.Id) + .HasValueGenerator(); } private class DummyValueGenerator : ValueGenerator