Properly fix website pin migration (#1066)

Toastbrot236 · web-flow · commit 4060ec45340d · 2026-04-04T11:33:06.000+02:00
Found another pagination issue with this migration right after merging
the previous PR, so I've (partially) reworked it to iterate users
instead of pin relations.

This does make the newly introduced feature to alter the processed and
total counts in the `JobState` unused for now, but I have not removed it
because it might eventually become necessary, if a different job ever
needs to delete entities.
diff --git a/Refresh.Database/GameDatabaseContext.Pins.cs b/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));
diff --git a/Refresh.Interfaces.Workers/Migrations/CorrectWebsitePinProgressPlatformMigration.cs b/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;
     }
 }
diff --git a/Refresh.Workers/MigrationJob.cs b/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);
 }
diff --git a/RefreshTests.GameServer/Tests/Workers/PinMigrationTests.cs b/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));
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -1,13 +1,13 @@`
`1`		`-using MongoDB.Bson;`
`2`		`-using Refresh.Common.Time;`
`3`		`-using Refresh.Database.Models.Authentication;`
	`1`	`+using Refresh.Database.Models.Authentication;`
`4`	`2`	`using Refresh.Database.Models.Pins;`
`5`	`3`	`using Refresh.Database.Models.Relations;`
	`4`	`+using Refresh.Database.Models.Users;`
`6`	`5`	`using Refresh.Workers;`
`7`	`6`
`8`	`7`	`namespace Refresh.Interfaces.Workers.Migrations;`
`9`	`8`
`10`		`-public class CorrectWebsitePinProgressPlatformMigration : MigrationJob<PinProgressRelation>`
	`9`	`+// Easier to do this by user than by pin relation, to avoid various pagination issues`
	`10`	`+public class CorrectWebsitePinProgressPlatformMigration : MigrationJob<GameUser>`
`11`	`11`	`{`
`12`	`12`	`private readonly List<long> WebsitePinIds =`
`13`	`13`	`[`
`@@ -16,36 +16,25 @@ public class CorrectWebsitePinProgressPlatformMigration : MigrationJob<PinProgre`
`16`	`16`	`(long)ServerPins.SignIntoWebsite,`
`17`	`17`	`];`
`18`	`18`
`19`		`- protected override IQueryable<PinProgressRelation> SortAndFilter(IQueryable<PinProgressRelation> query)`
	`19`	`+ protected override IQueryable<GameUser> SortAndFilter(IQueryable<GameUser> query)`
`20`	`20`	`{`
`21`		`- return query`
`22`		`- .Where(p => this.WebsitePinIds.Contains(p.PinId))`
`23`		`- .OrderBy(p => p.PinId);`
	`21`	`+ return query.OrderBy(u => u.UserId);`
`24`	`22`	`}`
`25`	`23`
`26`		`- protected override int Migrate(WorkContext context, PinProgressRelation[] batch)`
	`24`	`+ protected override int Migrate(WorkContext context, GameUser[] batch)`
`27`	`25`	`{`
`28`		`- int pinsLeft = batch.Length;`
`29`		`-`
`30`		`- foreach (long pinId in this.WebsitePinIds)`
	`26`	`+ foreach (GameUser user in batch)`
`31`	`27`	`{`
`32`		`- IEnumerable<IGrouping<ObjectId, PinProgressRelation>> pinsByUser = batch`
`33`		`- .Where(r => r.PinId == pinId)`
`34`		`- .GroupBy(r => r.PublisherId);`
`35`		`-`
`36`		`- foreach (IEnumerable<PinProgressRelation> group in pinsByUser)`
	`28`	`+ foreach (long pinId in this.WebsitePinIds)`
`37`	`29`	`{`
`38`		`- // Should never happen, but just incase`
`39`		`- if (!group.Any()) continue;`
	`30`	`+ List<PinProgressRelation> pinsByUserAndId = context.Database.GetAllPinProgressesByUserAndId(user, pinId).ToList();`
	`31`	`+ 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)`
`40`	`32`
`41`		`- // Find best one by the current user`
`42`		`- PinProgressRelation relationToMigrate = group.MaxBy(r => r.Progress)!;`
`43`		`- List<PinProgressRelation> relationsToRemove = group.ToList();`
	`33`	`+ PinProgressRelation relationToMigrate = pinsByUserAndId.MaxBy(r => r.Progress)!;`
`44`	`34`
`45`		`- foreach (PinProgressRelation relation in group)`
	`35`	`+ foreach (PinProgressRelation relation in pinsByUserAndId)`
`46`	`36`	`{`
`47`	`37`	`context.Database.RemovePinProgress(relation, false);`
`48`		`- pinsLeft--;`
`49`	`38`	`}`
`50`	`39`
`51`	`40`	`// 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)`
`61`	`50`	`};`
`62`	`51`
`63`	`52`	`context.Database.AddPinProgress(newRelation, false);`
`64`		`- pinsLeft++;`
`65`	`53`	`}`
`66`	`54`	`}`
`67`	`55`
`68`	`56`	`context.Database.SaveChanges();`
`69`		`- return pinsLeft;`
	`57`	`+ return batch.Length;`
`70`	`58`	`}`
`71`	`59`	`}`