fix: a drop during card migration no longer duplicates the card (MS-27523)

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;
+
+/// <summary>
+/// Tests that a drop arriving while cards are still migrating between decks leaves each card in one place.
+/// </summary>
+public partial class PDCardDeckTests
+{
+	private const string AnimationModulePath = "./_content/PanoramicData.Blazor/PDAnimation.razor.js";
+
+	private IRenderedComponent<PDCardDeckGroup<Card>> RenderAnimatedGroup(Board board)
+	{
+		var group = Render<PDCardDeckGroup<Card>>(parameters => parameters
+			.Add(p => p.DataProvider, new ListDataProviderService<Card>())
+			.Add(p => p.Transformation, (_, source, destination, cards) => board.Transform(source, destination, cards))
+			.AddChildContent<PDCardDeck<Card>>(deck => deck
+				.Add(p => p.Id, "left")
+				.Add(p => p.IsAnimated, true)
+				.Add(p => p.DataFunction, board.DataFor("left")))
+			.AddChildContent<PDCardDeck<Card>>(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;
+	}
+
+	/// <summary>
+	/// Verifies that the drop reaching every deck at once, as each deck listens for it on the document, transforms once.
+	/// </summary>
+	[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"));
+	}
+
+	/// <summary>
+	/// Verifies that a drop arriving while the cards are still leaving the source deck does not put them in the destination twice.
+	/// </summary>
+	[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");
+	}
+}

PanoramicData.Blazor.Test/Components/PDCardDeckTests.Group.cs

@@ -12,7 +12,7 @@ namespace PanoramicData.Blazor.Test.Components;
 /// </summary>
 public partial class PDCardDeckTests
 {
-	private sealed class Board
+	private sealed class Board(TimeSpan transformDuration = default, bool movesTheDroppedCards = false)
 	{
 		public Dictionary<string, List<Card>> Decks { get; } = new()
 		{
@@ -25,12 +25,30 @@ private sealed class Board
 		public Func<Task<DataResponse<Card>>> DataFor(string deckId)
 			=> () => Task.FromResult(new DataResponse<Card>([.. Decks[deckId]], Decks[deckId].Count));
 
-		public Task Transform(PDCardDeck<Card> source, PDCardDeck<Card> destination, List<Card> cards)
+		private readonly TimeSpan _transformDuration = transformDuration;
+
+		/// <summary>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.</summary>
+		private readonly bool _movesTheDroppedCards = movesTheDroppedCards;
+
+		public async Task Transform(PDCardDeck<Card> source, PDCardDeck<Card> destination, List<Card> 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);
+			}
+
 		}
 	}
 

PanoramicData.Blazor/PDCardDeckGroup.razor.cs

@@ -9,6 +9,12 @@ namespace PanoramicData.Blazor;
 /// <typeparam name="TCard">The type of card data managed by the decks in this group. Must implement <see cref="ICard"/>.</typeparam>
 public partial class PDCardDeckGroup<TCard> : IDisposable where TCard : ICard
 {
+	/// <summary>
+	/// 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.
+	/// </summary>
+	private readonly SemaphoreSlim _dragLock = new(1, 1);
+
 	/// <summary>
 	/// Holds a list of destination deck IDs that are part of the current card migration process (this is for UI only).
 	/// </summary>
@@ -166,6 +172,19 @@ public void RegisterDestination(PDCardDeck<TCard> deck)
 	/// </summary>
 	/// <returns></returns>
 	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();
 	}
 
 	/// <summary>
@@ -300,6 +330,19 @@ internal void SetSourceDeck(PDCardDeck<TCard> 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);
 	}
 }