From d4864107814e40231a74a1b192dba622d1afe737 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 00:48:32 +0000 Subject: [PATCH 1/2] Initial plan From 35eaddc87c7a8e179de1c7c1a0b71a9332669db3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:10:27 +0000 Subject: [PATCH 2/2] Fix regression in CommandBatchPreparer.AddUniqueValueEdges ordering unique-index updates Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Update/Internal/CommandBatchPreparer.cs | 15 ++-- .../Update/NonSharedModelUpdatesTestBase.cs | 71 +++++++++++++++++++ .../Update/CommandBatchPreparerTest.cs | 47 ++++++++++++ 3 files changed, 124 insertions(+), 9 deletions(-) diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 08f184837c9..c1f9efba043 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -1264,15 +1264,12 @@ private void AddUniqueValueEdges() var rowIndexValueFactory = ((TableIndex)index).GetRowIndexValueFactory(); var (value, hasNullValue) = rowIndexValueFactory.CreateEquatableIndexValue(command); - if (command.EntityState == EntityState.Modified) - { - var (originalValue, _) = rowIndexValueFactory.CreateEquatableIndexValue(command, fromOriginalValues: true); - if (Equals(originalValue, value)) - { - continue; - } - } - + // Note: unlike the predecessor (first) pass above, we don't skip creating an edge here when the + // command's own index value appears unchanged. A predecessor is only ever registered in + // indexPredecessorsMap for a value it is actually releasing (see the check above), so if this + // command's value matches one, it genuinely needs to wait for that release, regardless of whether + // its own current/original value comparison (which can be unreliable, e.g. for attached entities + // whose original value was snapshotted after being modified) suggests no change occurred. if (value != null) { AddMatchingPredecessorEdge( diff --git a/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs index 053f9e2c8c4..66cc126de82 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs @@ -119,6 +119,77 @@ public class Blog public string? Name { get; set; } } + [Theory, MemberData(nameof(IsAsyncData))] // Issue #38917 + public virtual async Task Reassigning_dependent_with_unique_FK_to_attached_entity_orders_updates_correctly(bool async) + { + var contextFactory = await InitializeNonSharedTest( + onModelCreating: mb => mb.Entity() + .HasOne(g => g.Color) + .WithOne(c => c.Group) + .HasForeignKey("GroupId"), + seed: async context => + { + var group0 = new WorkItemGroup38917 { Name = "G0" }; + var group1 = new WorkItemGroup38917 { Name = "G1" }; + var color0 = new RgbColor38917 { Id = "0x9a79e4", DisplayName = "red" }; + var color1 = new RgbColor38917 { Id = "0xb4e6b9", DisplayName = "turquoise" }; + + group0.Color = color0; + group1.Color = color1; + context.AddRange(group0, group1); + + await context.SaveChangesAsync(); + }); + + await ExecuteWithStrategyInTransactionAsync( + contextFactory, + async context => + { + var g1 = await context.Set() + .Include(g => g.Color) + .SingleAsync(g => g.Name == "G1"); + + var c0 = new RgbColor38917 { Id = "0x9a79e4" }; + context.Attach(c0); + g1.Color = c0; + + if (async) + { + await context.SaveChangesAsync(); + } + else + { + context.SaveChanges(); + } + }, + async context => + { + var g1 = await context.Set() + .Include(g => g.Color) + .SingleAsync(g => g.Name == "G1"); + Assert.Equal("0x9a79e4", g1.Color!.Id); + + var g0 = await context.Set() + .Include(g => g.Color) + .SingleAsync(g => g.Name == "G0"); + Assert.Null(g0.Color); + }); + } + + public class WorkItemGroup38917 + { + public Guid Id { get; set; } = Guid.NewGuid(); + public string Name { get; set; } = ""; + public RgbColor38917? Color { get; set; } + } + + public class RgbColor38917 + { + public string Id { get; set; } = ""; + public string? DisplayName { get; set; } + public WorkItemGroup38917? Group { get; set; } + } + [Theory, MemberData(nameof(IsAsyncData))] public virtual async Task Update_entity_with_not_loaded_property_excludes_column_from_SQL(bool async) { diff --git a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs index 401cbfd3746..d940261184c 100644 --- a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs +++ b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs @@ -599,6 +599,40 @@ public void BatchCommands_skips_unique_index_edges_for_unchanged_store_generated Assert.Equal(2, batch.ModificationCommands.Count); } + [Fact] // Issue #38917 + public void BatchCommands_orders_unique_index_release_before_reacquisition_with_unreliable_original_value() + { + var model = CreateModelWithUniqueIndexOnly(); + var configuration = CreateContextServices(model); + var stateManager = configuration.GetRequiredService(); + + // Releases "Test" by changing UniqueValue away from it. Uses a higher Id than the acquiring entry below so + // that the default (primary-key-based) command ordering alone would produce the wrong (unsafe) order, + // and only the dependency edge asserted by this test can produce the correct order. + var releasingEntry = stateManager.GetOrCreateEntry( + new FakeEntity { Id = 2, UniqueValue = "Other" }); + releasingEntry.SetEntityState(EntityState.Modified); + releasingEntry.SetOriginalValue(releasingEntry.EntityType.FindProperty(nameof(FakeEntity.UniqueValue))!, "Test"); + + // Simulates an attached entity whose original value snapshot ends up matching its (newly assigned) current + // value; it still needs to wait for another command to release "Test" before it can claim it. + var acquiringEntry = stateManager.GetOrCreateEntry( + new FakeEntity { Id = 1, UniqueValue = "Test" }); + acquiringEntry.SetEntityState(EntityState.Modified); + acquiringEntry.SetOriginalValue(acquiringEntry.EntityType.FindProperty(nameof(FakeEntity.UniqueValue))!, "Test"); + + var modelData = new UpdateAdapter(stateManager); + + var batches = CreateBatches([acquiringEntry, releasingEntry], modelData); + var batch = Assert.Single(batches); + + // The command releasing "Test" must be ordered before the command acquiring it, otherwise we'd get a + // unique constraint violation. + Assert.Equal( + [releasingEntry, acquiringEntry], + batch.ModificationCommands.Select(c => c.Entries.Single())); + } + [Fact] public void BatchCommands_creates_valid_batch_for_shared_table_added_entities() { @@ -1063,6 +1097,19 @@ private static IModel CreateSimpleFKModel() return modelBuilder.Model.FinalizeModel(); } + private static IModel CreateModelWithUniqueIndexOnly() + { + var modelBuilder = FakeRelationalTestHelpers.Instance.CreateConventionBuilder(); + + modelBuilder.Entity(b => + { + b.Ignore(c => c.RelatedId); + b.HasIndex(c => c.UniqueValue).IsUnique(); + }); + + return modelBuilder.Model.FinalizeModel(); + } + private static IModel CreateFKOneToManyModelWithGeneratedIds() { var modelBuilder = FakeRelationalTestHelpers.Instance.CreateConventionBuilder();