Properly fix website pin migration

Refresh.Database/GameDatabaseContext.Pins.cs

@@ -169,6 +169,9 @@ private IEnumerable<PinProgressRelation> GetPinProgressesByUser(GameUser user, b
     public DatabaseList<PinProgressRelation> GetPinProgressesByUser(GameUser user, bool isBeta, TokenPlatform platform, int skip, int count)
         => new(this.GetPinProgressesByUser(user, isBeta, platform), skip, count);
 
+    public IQueryable<PinProgressRelation> GetAllPinProgressesByUserAndId(GameUser user, long pinId)
+        => this.PinProgressRelations.Where(p => p.PublisherId == user.UserId && p.PinId == pinId);
+
     public PinProgressRelation? GetUserPinProgress(long pinId, GameUser user, bool isBeta, TokenPlatform platform)
         => this.PinProgressRelations.FirstOrDefault(p => p.PinId == pinId && p.PublisherId == user.UserId 
             && (p.IsBeta == isBeta && p.Platform == platform || p.Platform == TokenPlatform.Website));

Refresh.Interfaces.Workers/Migrations/CorrectWebsitePinProgressPlatformMigration.cs

@@ -1,13 +1,13 @@
-using MongoDB.Bson;
-using Refresh.Common.Time;
-using Refresh.Database.Models.Authentication;
+using Refresh.Database.Models.Authentication;
 using Refresh.Database.Models.Pins;
 using Refresh.Database.Models.Relations;
+using Refresh.Database.Models.Users;
 using Refresh.Workers;
 
 namespace Refresh.Interfaces.Workers.Migrations;
 
-public class CorrectWebsitePinProgressPlatformMigration : MigrationJob<PinProgressRelation>
+// Easier to do this by user than by pin relation, to avoid various pagination issues
+public class CorrectWebsitePinProgressPlatformMigration : MigrationJob<GameUser>
 {
     private readonly List<long> WebsitePinIds =
     [
@@ -16,36 +16,25 @@ public class CorrectWebsitePinProgressPlatformMigration : MigrationJob<PinProgre
         (long)ServerPins.SignIntoWebsite,
     ];
 
-    protected override IQueryable<PinProgressRelation> SortAndFilter(IQueryable<PinProgressRelation> query)
+    protected override IQueryable<GameUser> SortAndFilter(IQueryable<GameUser> query)
     {
-        return query
-            .Where(p => this.WebsitePinIds.Contains(p.PinId))
-            .OrderBy(p => p.PinId);
+        return query.OrderBy(u => u.UserId);
     }
 
-    protected override int Migrate(WorkContext context, PinProgressRelation[] batch)
+    protected override int Migrate(WorkContext context, GameUser[] batch)
     {
-        int pinsLeft = batch.Length;
-
-        foreach (long pinId in this.WebsitePinIds)
+        foreach (GameUser user in batch)
         {
-            IEnumerable<IGrouping<ObjectId, PinProgressRelation>> pinsByUser = batch
-                .Where(r => r.PinId == pinId)
-                .GroupBy(r => r.PublisherId);
-
-            foreach (IEnumerable<PinProgressRelation> group in pinsByUser)
+            foreach (long pinId in this.WebsitePinIds)
             {
-                // Should never happen, but just incase
-                if (!group.Any()) continue;
+                List<PinProgressRelation> pinsByUserAndId = context.Database.GetAllPinProgressesByUserAndId(user, pinId).ToList();
+                if (pinsByUserAndId.Count <= 0) continue; // no need to deduplicate if there is nothing (do still migrate if there's only 1 pin, to overwrite its platform)
 
-                // Find best one by the current user
-                PinProgressRelation relationToMigrate = group.MaxBy(r => r.Progress)!;
-                List<PinProgressRelation> relationsToRemove = group.ToList();
+                PinProgressRelation relationToMigrate = pinsByUserAndId.MaxBy(r => r.Progress)!;
 
-                foreach (PinProgressRelation relation in group)
+                foreach (PinProgressRelation relation in pinsByUserAndId)
                 {
                     context.Database.RemovePinProgress(relation, false);
-                    pinsLeft--;
                 }
 
                 // Now take the best progress we've just got and add it as a website pin, preserving other old metadata
@@ -61,11 +50,10 @@ protected override int Migrate(WorkContext context, PinProgressRelation[] batch)
                 };
 
                 context.Database.AddPinProgress(newRelation, false);
-                pinsLeft++;
             }
         }
 
         context.Database.SaveChanges();
-        return pinsLeft;
+        return batch.Length;
     }
 }

Refresh.Workers/MigrationJob.cs

@@ -56,7 +56,8 @@ public override void ExecuteJob(WorkContext context)
     /// <returns>
     /// The number of entities in the batch, minus the number of entities removed from DB during this migration.
     /// E.g. if 1000 items are in the given batch, and 5 got removed from DB during migration, 
-    /// the returned number will be 1000 - 5 = 995
+    /// the returned number will be 1000 - 5 = 995.
+    /// This allows you to write migration jobs, which need to delete entities from the given batch, without breaking pagination done by the worker.
     /// </returns>
     protected abstract int Migrate(WorkContext context, TEntity[] batch);
 }

RefreshTests.GameServer/Tests/Workers/PinMigrationTests.cs

@@ -1,5 +1,6 @@
 using Refresh.Database.Models.Authentication;
 using Refresh.Database.Models.Pins;
+using Refresh.Database.Models.Relations;
 using Refresh.Database.Models.Users;
 using Refresh.Database.Models.Workers;
 using Refresh.Interfaces.Workers.Migrations;
@@ -46,7 +47,7 @@ public void WebsitePinMigrationDoesNotBreakPagination()
         CorrectWebsitePinProgressPlatformMigration job = new();
         context.Database.UpdateOrCreateJobState(typeof(CorrectWebsitePinProgressPlatformMigration).Name, new MigrationJobState()
         {
-            Total = 100 // Since we already have to manually create the state
+            Total = 50,
         }, WorkerClass.Refresh);
         context.Database.Refresh();
 
@@ -68,6 +69,121 @@ public void WebsitePinMigrationDoesNotBreakPagination()
         Assert.That(jobState.Total, Is.EqualTo(50));
         Assert.That(jobState.Complete, Is.True);
 
-        Assert.That(context.Database.GetTotalPinProgresses(), Is.EqualTo(50));
+        Assert.That(context.Database.GetTotalPinProgresses(), Is.EqualTo(50)); // Pins were deduplicated
+    }
+
+    [Test]
+    [TestCase(ServerPins.SignIntoWebsite)]
+    [TestCase(ServerPins.HeartPlayerOnWebsite)]
+    [TestCase(ServerPins.QueueLevelOnWebsite)]
+    public void WebsitePinMigrationProperlyOverwritesPlatform(long pinId)
+    {
+        using TestContext context = this.GetServer();
+
+        GameUser user = context.CreateUser();
+        context.Database.AddPinProgress(new()
+        {
+            PinId = pinId,
+            Progress = 20,
+            Publisher = user,
+            FirstPublished = new(),
+            LastUpdated = new(),
+            IsBeta = false,
+            Platform = TokenPlatform.RPCS3,
+        }, true);
+
+        context.Database.AddPinProgress(new()
+        {
+            PinId = pinId,
+            Progress = 30,
+            Publisher = user,
+            FirstPublished = new(),
+            LastUpdated = new(),
+            IsBeta = true,
+            Platform = TokenPlatform.RPCS3,
+        }, true);
+
+        Assert.That(context.Database.GetTotalPinProgresses(), Is.EqualTo(2));
+
+        // Prepare migration
+        CorrectWebsitePinProgressPlatformMigration job = new();
+        context.Database.UpdateOrCreateJobState(typeof(CorrectWebsitePinProgressPlatformMigration).Name, new MigrationJobState()
+        {
+            Total = 1
+        }, WorkerClass.Refresh);
+        context.Database.Refresh();
+
+        object? stateObject = context.Database.GetJobState(typeof(CorrectWebsitePinProgressPlatformMigration).Name, typeof(MigrationJobState), WorkerClass.Refresh);
+        Assert.That(stateObject, Is.Not.Null);
+        job.JobState = stateObject!;
+
+        // Migrate
+        job.ExecuteJob(context.GetWorkContext());
+        context.Database.Refresh();
+
+        List<PinProgressRelation> relations = context.Database.GetAllPinProgressesByUserAndId(user, pinId).ToList();
+        Assert.That(relations.Count, Is.EqualTo(1));
+        Assert.That(relations[0].Platform, Is.EqualTo(TokenPlatform.Website));
+        Assert.That(relations[0].Progress, Is.EqualTo(30));
+    }
+
+    [Test]
+    [TestCase(ServerPins.SignIntoWebsite, ServerPins.CherryShooterLbp3ChallengeMedal)]
+    [TestCase(ServerPins.HeartPlayerOnWebsite, ServerPins.TopFourthOfXCommunityLevelsWithOver50Scores)]
+    [TestCase(ServerPins.QueueLevelOnWebsite, 420)]
+    public void WebsitePinMigrationOnlyMigratesWebsitePins(long websitePinId, long otherPinId)
+    {
+        using TestContext context = this.GetServer();
+
+        GameUser user = context.CreateUser();
+        context.Database.AddPinProgress(new()
+        {
+            PinId = websitePinId,
+            Progress = 20,
+            Publisher = user,
+            FirstPublished = new(),
+            LastUpdated = new(),
+            IsBeta = false,
+            Platform = TokenPlatform.RPCS3,
+        }, true);
+
+        context.Database.AddPinProgress(new()
+        {
+            PinId = otherPinId,
+            Progress = 30,
+            Publisher = user,
+            FirstPublished = new(),
+            LastUpdated = new(),
+            IsBeta = true,
+            Platform = TokenPlatform.RPCS3,
+        }, true);
+
+        Assert.That(context.Database.GetTotalPinProgresses(), Is.EqualTo(2));
+
+        // Prepare migration
+        CorrectWebsitePinProgressPlatformMigration job = new();
+        context.Database.UpdateOrCreateJobState(typeof(CorrectWebsitePinProgressPlatformMigration).Name, new MigrationJobState()
+        {
+            Total = 1
+        }, WorkerClass.Refresh);
+        context.Database.Refresh();
+
+        object? stateObject = context.Database.GetJobState(typeof(CorrectWebsitePinProgressPlatformMigration).Name, typeof(MigrationJobState), WorkerClass.Refresh);
+        Assert.That(stateObject, Is.Not.Null);
+        job.JobState = stateObject!;
+
+        // Migrate
+        job.ExecuteJob(context.GetWorkContext());
+        context.Database.Refresh();
+
+        List<PinProgressRelation> websitePins = context.Database.GetAllPinProgressesByUserAndId(user, websitePinId).ToList();
+        Assert.That(websitePins.Count, Is.EqualTo(1));
+        Assert.That(websitePins[0].Platform, Is.EqualTo(TokenPlatform.Website));
+        Assert.That(websitePins[0].Progress, Is.EqualTo(20));
+
+        List<PinProgressRelation> otherPins = context.Database.GetAllPinProgressesByUserAndId(user, otherPinId).ToList();
+        Assert.That(otherPins.Count, Is.EqualTo(1));
+        Assert.That(otherPins[0].Platform, Is.EqualTo(TokenPlatform.RPCS3));
+        Assert.That(otherPins[0].Progress, Is.EqualTo(30));
     }
 }