From 988bc9fcefd96a8dee311a57f2f3832569e1cb06 Mon Sep 17 00:00:00 2001 From: David Bond Date: Wed, 14 Oct 2026 16:53:34 +0100 Subject: [PATCH 1/2] fix: a drop during card migration no longer duplicates the card (MS-27523) PDCardDeckGroup migrated cards between decks on drag-enter, awaiting the browser part way through (cancelling animations), and ran the drop's transformation and refresh without waiting for it. A drop landing in that gap refreshed the destination from data that already held the card, then the migration resumed and inserted it again, so the deck held the same card twice and the render threw 'More than one sibling ... has the same key value', terminating the Blazor Server circuit. Every deck also listens for the drop on the document, so the transformation ran once per deck. Migration, drop and end of drag now take one lock, so a drop waits for the migration and later drops find the drag already over. Reproduced in a real browser on the demo kanban (circuit killed on the first drop) and verified fixed over 3 x 40 drags. Co-Authored-By: Claude Sonnet 5.5 --- .../PDCardDeckTests.ConcurrentDrop.cs | 70 +++++++++++++++++++ .../Components/PDCardDeckTests.Group.cs | 26 +++++-- PanoramicData.Blazor/PDCardDeckGroup.razor.cs | 50 ++++++++++++- 3 files changed, 139 insertions(+), 7 deletions(-) create mode 100644 PanoramicData.Blazor.Test/Components/PDCardDeckTests.ConcurrentDrop.cs diff --git a/PanoramicData.Blazor.Test/Components/PDCardDeckTests.ConcurrentDrop.cs b/PanoramicData.Blazor.Test/Components/PDCardDeckTests.ConcurrentDrop.cs new file mode 100644 index 000000000..ff4504f7a --- /dev/null +++ b/PanoramicData.Blazor.Test/Components/PDCardDeckTests.ConcurrentDrop.cs @@ -0,0 +1,70 @@ +using AwesomeAssertions; +using Bunit; +using Microsoft.AspNetCore.Components.Web; +using PanoramicData.Blazor.Services; + +namespace PanoramicData.Blazor.Test.Components; + +/// +/// Tests that a drop arriving while cards are still migrating between decks leaves each card in one place. +/// +public partial class PDCardDeckTests +{ + private const string AnimationModulePath = "./_content/PanoramicData.Blazor/PDAnimation.razor.js"; + + private IRenderedComponent> RenderAnimatedGroup(Board board) + { + var group = Render>(parameters => parameters + .Add(p => p.DataProvider, new ListDataProviderService()) + .Add(p => p.Transformation, (_, source, destination, cards) => board.Transform(source, destination, cards)) + .AddChildContent>(deck => deck + .Add(p => p.Id, "left") + .Add(p => p.IsAnimated, true) + .Add(p => p.DataFunction, board.DataFor("left"))) + .AddChildContent>(deck => deck + .Add(p => p.Id, "right") + .Add(p => p.IsAnimated, true) + .Add(p => p.DataFunction, board.DataFor("right")))); + group.WaitForAssertion(() => group.FindAll("div.card").Should().HaveCount(3)); + return group; + } + + /// + /// Verifies that the drop reaching every deck at once, as each deck listens for it on the document, transforms once. + /// + [Fact] + public async Task Group_DropReachingEveryDeck_TransformsOnce() + { + var board = new Board { TransformDuration = TimeSpan.FromMilliseconds(100) }; + var group = RenderGroup(board); + await group.InvokeAsync(() => group.FindAll("#left div.card")[0].DragStartAsync(new DragEventArgs())); + await group.InvokeAsync(() => group.Find("#right").DragEnterAsync(new DragEventArgs())); + + await group.InvokeAsync(() => Task.WhenAll( + Deck(group, "left").Instance.InitiateTransformAsync(), + Deck(group, "right").Instance.InitiateTransformAsync())); + + board.Transforms.Should().ContainSingle(); + group.WaitForAssertion(() => Names(group, "right").Should().Equal("A", "C")); + } + + /// + /// Verifies that a drop arriving while the cards are still leaving the source deck does not put them in the destination twice. + /// + [Fact] + public async Task Group_DropDuringTheMigration_PutsTheCardInTheDestinationOnce() + { + var board = new Board { MovesTheDroppedCards = true }; + var cancelAnimation = JSInterop.SetupModule(AnimationModulePath).SetupVoid("cancelAnimation", _ => true); + var group = RenderAnimatedGroup(board); + await group.InvokeAsync(() => group.FindAll("#left div.card")[0].DragStartAsync(new DragEventArgs())); + + var entering = group.InvokeAsync(() => group.Find("#right").DragEnterAsync(new DragEventArgs())); + var dropping = group.InvokeAsync(Deck(group, "right").Instance.InitiateTransformAsync); + cancelAnimation.SetVoidResult(); + await Task.WhenAll(entering, dropping); + + group.WaitForAssertion(() => Names(group, "right").Should().Equal("A", "C")); + Names(group, "left").Should().Equal("B"); + } +} diff --git a/PanoramicData.Blazor.Test/Components/PDCardDeckTests.Group.cs b/PanoramicData.Blazor.Test/Components/PDCardDeckTests.Group.cs index 6c0610090..ec93f3b8f 100644 --- a/PanoramicData.Blazor.Test/Components/PDCardDeckTests.Group.cs +++ b/PanoramicData.Blazor.Test/Components/PDCardDeckTests.Group.cs @@ -25,12 +25,30 @@ private sealed class Board public Func>> DataFor(string deckId) => () => Task.FromResult(new DataResponse([.. Decks[deckId]], Decks[deckId].Count)); - public Task Transform(PDCardDeck source, PDCardDeck destination, List cards) + public TimeSpan TransformDuration { get; init; } + + /// Moves the dropped cards in the data store, as a board backed by a real data source does, rather than copying what the decks show. + public bool MovesTheDroppedCards { get; init; } + + public async Task Transform(PDCardDeck source, PDCardDeck destination, List cards) { Transforms.Add((source.Id, destination.Id, [.. cards.Select(c => c.Name)])); - Decks[source.Id] = [.. source.Cards]; - Decks[destination.Id] = [.. destination.Cards]; - return Task.CompletedTask; + if (MovesTheDroppedCards) + { + Decks[source.Id].RemoveAll(cards.Contains); + Decks[destination.Id].InsertRange(0, cards); + } + else + { + Decks[source.Id] = [.. source.Cards]; + Decks[destination.Id] = [.. destination.Cards]; + } + + if (TransformDuration > TimeSpan.Zero) + { + await Task.Delay(TransformDuration); + } + } } diff --git a/PanoramicData.Blazor/PDCardDeckGroup.razor.cs b/PanoramicData.Blazor/PDCardDeckGroup.razor.cs index f40eed077..b8554ae8f 100644 --- a/PanoramicData.Blazor/PDCardDeckGroup.razor.cs +++ b/PanoramicData.Blazor/PDCardDeckGroup.razor.cs @@ -9,6 +9,12 @@ namespace PanoramicData.Blazor; /// The type of card data managed by the decks in this group. Must implement . public partial class PDCardDeckGroup : IDisposable where TCard : ICard { + /// + /// Serialises card migration, drop and end of drag: each awaits the browser part way through, and a drop + /// that arrives in that gap would otherwise run against a half-migrated group. + /// + private readonly SemaphoreSlim _dragLock = new(1, 1); + /// /// Holds a list of destination deck IDs that are part of the current card migration process (this is for UI only). /// @@ -166,6 +172,19 @@ public void RegisterDestination(PDCardDeck deck) /// /// internal async Task UpdateGraphicalCardLocationsAsync() + { + await _dragLock.WaitAsync(); + try + { + await MigrateCardsAsync(); + } + finally + { + _ = _dragLock.Release(); + } + } + + private async Task MigrateCardsAsync() { // Must have two destinations to perform a migration. if (_destinations.Count < 2) @@ -208,9 +227,20 @@ internal async Task UpdateGraphicalCardLocationsAsync() internal async Task InitiateTransformAsync() { - await Task.CompletedTask; + await _dragLock.WaitAsync(); + try + { + await TransformCardsAsync(); + } + finally + { + _ = _dragLock.Release(); + } + } - // Refresh if a transform has occurred + private async Task TransformCardsAsync() + { + // Every deck listens for the drop on the document, so one drop arrives once per deck; later ones find the drag over. if (Transformation is null || _destinations.Count == 0) { return; @@ -235,7 +265,7 @@ internal async Task InitiateTransformAsync() await Transformation(_dataProviderService, _sourceDeck, destination, movingCards); // Refresh the source deck and destination deck - await EndDragOperationAsync(); + await RefreshDecksAsync(); } /// @@ -300,6 +330,19 @@ internal void SetSourceDeck(PDCardDeck source) } internal async Task EndDragOperationAsync() + { + await _dragLock.WaitAsync(); + try + { + await RefreshDecksAsync(); + } + finally + { + _ = _dragLock.Release(); + } + } + + private async Task RefreshDecksAsync() { var destination = _decks.FirstOrDefault(d => _destinations.Count > 0 && d.Id == _destinations.Last()); @@ -323,6 +366,7 @@ internal async Task EndDragOperationAsync() public void Dispose() { LoadingIcon?.Dispose(); + _dragLock.Dispose(); GC.SuppressFinalize(this); } } \ No newline at end of file From dea0e15cf4229dd7c370698bd044d63e1f351fee Mon Sep 17 00:00:00 2001 From: David Bond Date: Wed, 14 Oct 2026 17:46:24 +0100 Subject: [PATCH 2/2] Codacy: pass the test board's options through its constructor (S3459) Co-Authored-By: Claude Sonnet 5.5 --- .../Components/PDCardDeckTests.ConcurrentDrop.cs | 4 ++-- .../Components/PDCardDeckTests.Group.cs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/PanoramicData.Blazor.Test/Components/PDCardDeckTests.ConcurrentDrop.cs b/PanoramicData.Blazor.Test/Components/PDCardDeckTests.ConcurrentDrop.cs index ff4504f7a..0a49bab29 100644 --- a/PanoramicData.Blazor.Test/Components/PDCardDeckTests.ConcurrentDrop.cs +++ b/PanoramicData.Blazor.Test/Components/PDCardDeckTests.ConcurrentDrop.cs @@ -35,7 +35,7 @@ private IRenderedComponent> RenderAnimatedGroup(Board boar [Fact] public async Task Group_DropReachingEveryDeck_TransformsOnce() { - var board = new Board { TransformDuration = TimeSpan.FromMilliseconds(100) }; + var board = new Board(transformDuration: TimeSpan.FromMilliseconds(100)); var group = RenderGroup(board); await group.InvokeAsync(() => group.FindAll("#left div.card")[0].DragStartAsync(new DragEventArgs())); await group.InvokeAsync(() => group.Find("#right").DragEnterAsync(new DragEventArgs())); @@ -54,7 +54,7 @@ await group.InvokeAsync(() => Task.WhenAll( [Fact] public async Task Group_DropDuringTheMigration_PutsTheCardInTheDestinationOnce() { - var board = new Board { MovesTheDroppedCards = true }; + var board = new Board(movesTheDroppedCards: true); var cancelAnimation = JSInterop.SetupModule(AnimationModulePath).SetupVoid("cancelAnimation", _ => true); var group = RenderAnimatedGroup(board); await group.InvokeAsync(() => group.FindAll("#left div.card")[0].DragStartAsync(new DragEventArgs())); diff --git a/PanoramicData.Blazor.Test/Components/PDCardDeckTests.Group.cs b/PanoramicData.Blazor.Test/Components/PDCardDeckTests.Group.cs index ec93f3b8f..ba34de33b 100644 --- a/PanoramicData.Blazor.Test/Components/PDCardDeckTests.Group.cs +++ b/PanoramicData.Blazor.Test/Components/PDCardDeckTests.Group.cs @@ -12,7 +12,7 @@ namespace PanoramicData.Blazor.Test.Components; /// public partial class PDCardDeckTests { - private sealed class Board + private sealed class Board(TimeSpan transformDuration = default, bool movesTheDroppedCards = false) { public Dictionary> Decks { get; } = new() { @@ -25,15 +25,15 @@ private sealed class Board public Func>> DataFor(string deckId) => () => Task.FromResult(new DataResponse([.. Decks[deckId]], Decks[deckId].Count)); - public TimeSpan TransformDuration { get; init; } + private readonly TimeSpan _transformDuration = transformDuration; /// Moves the dropped cards in the data store, as a board backed by a real data source does, rather than copying what the decks show. - public bool MovesTheDroppedCards { get; init; } + private readonly bool _movesTheDroppedCards = movesTheDroppedCards; public async Task Transform(PDCardDeck source, PDCardDeck destination, List cards) { Transforms.Add((source.Id, destination.Id, [.. cards.Select(c => c.Name)])); - if (MovesTheDroppedCards) + if (_movesTheDroppedCards) { Decks[source.Id].RemoveAll(cards.Contains); Decks[destination.Id].InsertRange(0, cards); @@ -44,9 +44,9 @@ public async Task Transform(PDCardDeck source, PDCardDeck destinatio Decks[destination.Id] = [.. destination.Cards]; } - if (TransformDuration > TimeSpan.Zero) + if (_transformDuration > TimeSpan.Zero) { - await Task.Delay(TransformDuration); + await Task.Delay(_transformDuration); } }