Save a retried workout only once (#18)

* Save a retried workout only once

With EF's retries, an insert whose commit confirmation was lost is repeated. Each workout now has an id created by the app with a unique index, so the repeated insert is recognised as already saved.

* Give test workouts their own client id

* Fix the SonarCloud findings in the new tests

Author: fdaniel02

src/WorkoutTracker/Components/Pages/WorkoutPage.razor

@@ -160,18 +160,16 @@
         saving = true;
         state.Stop(UtcNow);
         var typeName = type!.Name;
-        using (var activity = Telemetry.ActivitySource.StartActivity("SaveWorkout"))
+        await using (var db = await DbFactory.CreateDbContextAsync())
         {
-            activity?.SetTag("workouttracker.workout_type", typeName);
-            activity?.SetTag("workouttracker.reps", state.Reps);
-            await using var db = await DbFactory.CreateDbContextAsync();
-            db.Workouts.Add(new Workout
+            var workout = new Workout
             {
+                ClientId = state.Id,
                 WorkoutTypeId = Id,
                 StartedAtUtc = state.StartedUtc,
                 FinishedAtUtc = UtcNow
-            });
-            await db.SaveChangesAsync();
+            };
+            await db.SaveWorkoutAsync(workout, typeName, state.Reps);
         }
         Metrics.WorkoutSaved(typeName);
         if (saved) await Store.ClearAsync();

src/WorkoutTracker/Components/WorkoutState.cs

@@ -7,6 +7,8 @@ namespace WorkoutTracker.Components;
 // stays correct after a reload, including the time the phone was locked.
 public class WorkoutState
 {
+    // A workout saved before ids existed has none in its JSON, so it gets a new one here when it's loaded.
+    public Guid Id { get; set; } = Guid.NewGuid();
     public int TypeId { get; set; }
     public DateTime StartedUtc { get; set; }
     public int Reps { get; set; }

src/WorkoutTracker/Data/AppDbContext.cs

@@ -19,6 +19,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
             e.HasOne(x => x.WorkoutType).WithMany(t => t.Workouts)
                 .HasForeignKey(x => x.WorkoutTypeId).OnDelete(DeleteBehavior.Cascade);
             e.HasIndex(x => new { x.WorkoutTypeId, x.StartedAtUtc });
+            e.HasIndex(x => x.ClientId).IsUnique();
         });
     }
 }

src/WorkoutTracker/Data/Models.cs

@@ -10,6 +10,9 @@ public class WorkoutType
 public class Workout
 {
     public int Id { get; set; }
+    // Created by the app when the workout starts, so an insert that EF repeats after a lost commit confirmation
+    // hits the unique index instead of storing the workout twice.
+    public Guid ClientId { get; set; }
     public int WorkoutTypeId { get; set; }
     public WorkoutType? WorkoutType { get; set; }
     public DateTime StartedAtUtc { get; set; }

src/WorkoutTracker/Data/WorkoutCommands.cs

@@ -0,0 +1,33 @@
+using Microsoft.EntityFrameworkCore;
+
+namespace WorkoutTracker.Data;
+
+public static class WorkoutCommands
+{
+    // Returns true if the workout was already stored: EF repeated the insert after its commit confirmation was lost.
+    public static async Task<bool> SaveWorkoutAsync(this AppDbContext db, Workout workout, string typeName, int reps,
+        CancellationToken cancellationToken = default)
+    {
+        using var activity = Telemetry.ActivitySource.StartActivity("SaveWorkout");
+        activity?.SetTag("workouttracker.workout_type", typeName);
+        activity?.SetTag("workouttracker.reps", reps);
+        var alreadySaved = await AddAsync(db, workout, cancellationToken);
+        activity?.SetTag("workouttracker.already_saved", alreadySaved);
+        return alreadySaved;
+    }
+
+    static async Task<bool> AddAsync(AppDbContext db, Workout workout, CancellationToken cancellationToken)
+    {
+        db.Workouts.Add(workout);
+        try
+        {
+            await db.SaveChangesAsync(cancellationToken);
+            return false;
+        }
+        // ClientId is the only unique index on Workouts.
+        catch (DbUpdateException ex) when (SqlErrors.IsUniqueViolation(ex))
+        {
+            return true;
+        }
+    }
+}

src/WorkoutTracker/Migrations/20261021113437_AddWorkoutClientId.Designer.cs

@@ -0,0 +1,42 @@
+using System;
+using Microsoft.EntityFrameworkCore.Migrations;
+
+#nullable disable
+
+namespace WorkoutTracker.Migrations
+{
+    /// <inheritdoc />
+    public partial class AddWorkoutClientId : Migration
+    {
+        /// <inheritdoc />
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.AddColumn<Guid>(
+                name: "ClientId",
+                table: "Workouts",
+                type: "uniqueidentifier",
+                nullable: false,
+                // Not the generated empty Guid: existing rows need distinct values, or the unique index below fails.
+                // Only here, not in the model: SQLite (bUnit tests, EnsureCreated) has no NEWID().
+                defaultValueSql: "NEWID()");
+
+            migrationBuilder.CreateIndex(
+                name: "IX_Workouts_ClientId",
+                table: "Workouts",
+                column: "ClientId",
+                unique: true);
+        }
+
+        /// <inheritdoc />
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropIndex(
+                name: "IX_Workouts_ClientId",
+                table: "Workouts");
+
+            migrationBuilder.DropColumn(
+                name: "ClientId",
+                table: "Workouts");
+        }
+    }
+}

src/WorkoutTracker/Migrations/20261021113437_AddWorkoutClientId.cs

@@ -17,7 +17,7 @@ protected override void BuildModel(ModelBuilder modelBuilder)
         {
 #pragma warning disable 612, 618
             modelBuilder
-                .HasAnnotation("ProductVersion", "10.0.12")
+                .HasAnnotation("ProductVersion", "10.0.13")
                 .HasAnnotation("Relational:MaxIdentifierLength", 128);
 
             SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder);
@@ -30,6 +30,9 @@ protected override void BuildModel(ModelBuilder modelBuilder)
 
                     SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<int>("Id"));
 
+                    b.Property<Guid>("ClientId")
+                        .HasColumnType("uniqueidentifier");
+
                     b.Property<DateTime>("FinishedAtUtc")
                         .HasColumnType("datetime2");
 
@@ -41,6 +44,9 @@ protected override void BuildModel(ModelBuilder modelBuilder)
 
                     b.HasKey("Id");
 
+                    b.HasIndex("ClientId")
+                        .IsUnique();
+
                     b.HasIndex("WorkoutTypeId", "StartedAtUtc");
 
                     b.ToTable("Workouts");

src/WorkoutTracker/Migrations/AppDbContextModelSnapshot.cs

@@ -63,7 +63,8 @@ public async Task Loading_the_page_records_a_span_with_the_type_id()
     [Fact]
     public async Task Finish_records_a_span_and_counts_the_workout()
     {
-        using var spans = new ActivityCollector(a => a.OperationName == "SaveWorkout");
+        using var trace = new TestTrace();
+        using var spans = new ActivityCollector(a => a.OperationName == "SaveWorkout" && a.TraceId == trace.Id);
         using var saved = new MetricCollector<long>(Services.GetRequiredService<IMeterFactory>(), Telemetry.Name, "workouttracker.workouts.saved");
         var (page, _) = await StartWorkoutAsync();
         await Button(page, "+1").ClickAsync();
@@ -232,6 +233,38 @@ public async Task Finish_saves_when_the_page_was_opened_and_when_it_was_finished
         Assert.Equal(openedAt.AddMinutes(12), workout.FinishedAtUtc);
     }
 
+    [Fact]
+    public async Task Finish_saves_the_workout_with_its_id()
+    {
+        var (page, _) = await StartWorkoutAsync();
+        var saved = JsonSerializer.Deserialize<WorkoutState>(
+            JSInterop.Invocations["workoutState.save"][^1].Arguments[0]!.ToString()!)!;
+        saveGate.Release();
+
+        await Button(page, "Finish").ClickAsync();
+
+        await using var db = Database.CreateDbContext();
+        var workout = await db.Workouts.SingleAsync(Xunit.TestContext.Current.CancellationToken);
+        Assert.NotEqual(Guid.Empty, saved.Id);
+        Assert.Equal(saved.Id, workout.ClientId);
+    }
+
+    [Fact]
+    public async Task Restored_workout_keeps_its_id()
+    {
+        var type = await Database.AddWorkoutTypeAsync("Push-ups");
+        var id = Guid.NewGuid();
+        SetUpSavedWorkout(new WorkoutState { Id = id, TypeId = type.Id, StartedUtc = Time.GetUtcNow().UtcDateTime, Started = true });
+        var page = Render<WorkoutPage>(p => p.Add(x => x.Id, type.Id));
+        page.WaitForElement("button.wide");
+        saveGate.Release();
+
+        await Button(page, "Finish").ClickAsync();
+
+        await using var db = Database.CreateDbContext();
+        Assert.Equal(id, (await db.Workouts.SingleAsync(Xunit.TestContext.Current.CancellationToken)).ClientId);
+    }
+
     // The page redraws the timer on a real 250 ms tick, so a moved clock shows up within a few ticks.
     static readonly TimeSpan TickTimeout = TimeSpan.FromSeconds(5);
 

tests/WorkoutTracker.Tests/Components/WorkoutPageTests.cs

@@ -1,6 +1,7 @@
 using Microsoft.Data.SqlClient;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Migrations;
 using Microsoft.Extensions.Logging.Testing;
 using WorkoutTracker.Data;
 
@@ -26,6 +27,26 @@ public async Task Applies_all_migrations_to_an_empty_database()
         Assert.Empty(logger.Collector.GetSnapshot());
     }
 
+    [Fact]
+    public async Task Existing_workouts_get_distinct_client_ids()
+    {
+        var factory = Factory(sqlServer.ConnectionString(SqlServerFixture.NewDatabaseName()));
+        await using var db = await factory.CreateDbContextAsync(TestContext.Current.CancellationToken);
+        var migrator = db.GetService<IMigrator>();
+        await migrator.MigrateAsync("20260920173150_InitialCreate", TestContext.Current.CancellationToken);
+        await db.Database.ExecuteSqlRawAsync("""
+            INSERT INTO WorkoutTypes (Name) VALUES ('Push ups');
+            INSERT INTO Workouts (WorkoutTypeId, StartedAtUtc, FinishedAtUtc)
+            VALUES (1, '2026-10-01T07:00:00', '2026-10-01T07:10:00'), (1, '2026-10-02T07:00:00', '2026-10-02T07:10:00');
+            """, TestContext.Current.CancellationToken);
+
+        await migrator.MigrateAsync(cancellationToken: TestContext.Current.CancellationToken);
+
+        var clientIds = await db.Workouts.Select(w => w.ClientId).ToListAsync(TestContext.Current.CancellationToken);
+        Assert.Equal(2, clientIds.Distinct().Count());
+        Assert.DoesNotContain(Guid.Empty, clientIds);
+    }
+
     [Fact]
     public async Task Wrong_password_fails_without_retrying()
     {