Fix pin and photo subject migration jobs (#1063)

This does a few fixes/adjustments for existing migration jobs:
- Deleting entities during a migration job will now cause the job state
to be adjusted, fixing an issue where
`CorrectWebsitePinProgressPlatformMigration` would never be marked as
complete (and possibly also unintentionally skip pins) if there were
more than 1000 pins to migrate (the job was also renamed for consistency
+ to allow it to fully retry on instances where it has already been
executed), and to allow similar jobs in the future to also run properly
- `MoveSubjectsOutOfGamePhotosMigration` will not try to migrate photos
again if they already have associated `GamePhotoSubject`s, avoiding
duplicate key insertions
- Some migration job-exclusive code was moved from `Refresh.Database` to
the jobs' `Migrate()` method, to not keep methods only used by one
specific job there

Author: Toastbrot236

Refresh.Database/GameDatabaseContext.Photos.cs

@@ -119,67 +119,20 @@ public GamePhoto UploadPhoto(IPhotoUpload photo, IEnumerable<IPhotoUploadSubject
         return newPhoto;
     }
 
-    /// <remarks>
-    /// Migration only!!
-    /// </remarks>
-    public void MigratePhotoSubjects(GamePhoto photo, bool saveChanges)
+    public GamePhotoSubject AddSubjectForPhoto(GamePhoto photo, int playerId, string displayName, GameUser? user, List<float> bounds, bool save = true)
     {
-        List<GamePhotoSubject> subjects = [];
-
-#pragma warning disable CS0618 // obsoletion
-        
-        // If DisplayName is not null, there is a subject in that spot
-        if (photo.Subject1DisplayName != null)
-        {
-            subjects.Add(new()
-            {
-                Photo = photo,
-                PlayerId = 1,
-                DisplayName = photo.Subject1DisplayName,
-                User = photo.Subject1User,
-                Bounds = photo.Subject1Bounds,
-            });
-        }
-
-        if (photo.Subject2DisplayName != null)
-        {
-            subjects.Add(new()
-            {
-                Photo = photo,
-                PlayerId = 2,
-                DisplayName = photo.Subject2DisplayName,
-                User = photo.Subject2User,
-                Bounds = photo.Subject2Bounds,
-            });
-        }
-
-        if (photo.Subject3DisplayName != null)
-        {
-            subjects.Add(new()
-            {
-                Photo = photo,
-                PlayerId = 3,
-                DisplayName = photo.Subject3DisplayName,
-                User = photo.Subject3User,
-                Bounds = photo.Subject3Bounds,
-            });
-        }
-
-        if (photo.Subject4DisplayName != null)
+        GamePhotoSubject subject = new()
         {
-            subjects.Add(new()
-            {
-                Photo = photo,
-                PlayerId = 4,
-                DisplayName = photo.Subject4DisplayName,
-                User = photo.Subject4User,
-                Bounds = photo.Subject4Bounds,
-            });
-        }
-#pragma warning restore CS0618
+            Photo = photo,
+            PlayerId = playerId,
+            DisplayName = displayName,
+            User = user,
+            Bounds = bounds,
+        };
 
-        this.GamePhotoSubjects.AddRange(subjects);
-        if (saveChanges) this.SaveChanges();
+        this.GamePhotoSubjects.Add(subject);
+        if (save) this.SaveChanges();
+        return subject;
     }
 
     public void RemovePhoto(GamePhoto photo)
@@ -227,6 +180,9 @@ public IQueryable<GamePhotoSubject> GetSubjectsInPhoto(GamePhoto photo)
             .Where(s => s.PhotoId == photo.PhotoId)
             .OrderBy(s => s.PlayerId);
 
+    public int GetTotalSubjectsInPhoto(GamePhoto photo)
+        => this.GamePhotoSubjects.Count(s => s.PhotoId == photo.PhotoId);
+    
     public IQueryable<GameUser> GetUsersInPhoto(GamePhoto photo)
         => this.GetSubjectsInPhoto(photo)
             .Where(s => s.User != null)

Refresh.Database/GameDatabaseContext.Pins.cs

@@ -180,20 +180,19 @@ private IEnumerable<ProfilePinRelation> GetProfilePinsByUser(GameUser user, Toke
 
     public DatabaseList<ProfilePinRelation> GetProfilePinsByUser(GameUser user, TokenGame game, TokenPlatform platform, int skip, int count)
         => new(this.GetProfilePinsByUser(user, game, platform), skip, count);
-
-    #region Migration Methods
 
     public void AddPinProgress(PinProgressRelation relation, bool save)
     {
         this.PinProgressRelations.Add(relation);
         if (save) this.SaveChanges();
     }
 
-    public void RemoveAllPinProgressesByIdAndUser(long pinId, ObjectId userId, bool save)
+    public void RemovePinProgress(PinProgressRelation relation, bool save)
     {
-        this.PinProgressRelations.RemoveRange(p => p.PinId == pinId && p.PublisherId == userId);
+        this.PinProgressRelations.Remove(relation);
         if (save) this.SaveChanges();
     }
 
-    #endregion
+    public int GetTotalPinProgresses()
+        => this.PinProgressRelations.Count();
 }

Refresh.Database/GameDatabaseContext.Relations.cs

@@ -477,20 +477,6 @@ public GameReview UpdateReview(ISubmitReviewRequest updateInfo, GameReview revie
         this.SaveChanges();
         return review;
     }
-
-    public void MigrateReviewLabels(IEnumerable<GameReview> reviews)
-    {
-        foreach (GameReview review in reviews)
-        {
-#pragma warning disable CS0618 // LabelsString is obsolete          
-            if (string.IsNullOrWhiteSpace(review.LabelsString)) continue;
-
-            review.Labels = LabelExtensions.FromLbpCommaList(review.LabelsString).ToList();
-#pragma warning restore CS0618
-        }
-
-        this.SaveChanges();
-    }
 
     public void DeleteReviewsPostedByUser(GameUser user)
     {

Refresh.Interfaces.Workers/Migrations/BackfillLevelAttributesMigration.cs

@@ -12,13 +12,14 @@ protected override IQueryable<GameLevel> SortAndFilter(IQueryable<GameLevel> que
             .OrderBy(l => l.LevelId);
     }
 
-    protected override void Migrate(WorkContext context, GameLevel[] batch)
+    protected override int Migrate(WorkContext context, GameLevel[] batch)
     {
         foreach (GameLevel level in batch)
         {
             context.Database.ApplyLevelMetadataFromAttributes(level);
         }
 
         context.Database.SaveChanges();
+        return batch.Length;
     }
 }

Refresh.Interfaces.Workers/Migrations/BackfillModdedPlanetFlagsMigration.cs

@@ -16,13 +16,14 @@ protected override IQueryable<GameUser> SortAndFilter(IQueryable<GameUser> query
             || u.BetaPlanetsHash != "0");
     }
 
-    protected override void Migrate(WorkContext context, GameUser[] batch)
+    protected override int Migrate(WorkContext context, GameUser[] batch)
     {
         foreach (GameUser user in batch)
         {
             context.Database.UpdatePlanetModdedStatus(user);
         }
 
         context.Database.SaveChanges();
+        return batch.Length;
     }
 }

Refresh.Interfaces.Workers/Migrations/BackfillReviewLabelsMigration.cs

@@ -1,13 +1,25 @@
-using Refresh.Database.Models.Comments; 
+using Refresh.Database.Models.Comments;
+using Refresh.Database.Models.Levels;
 using Refresh.Workers;
 
 namespace Refresh.Interfaces.Workers.Migrations;
 
 public class BackfillReviewLabelsMigration : MigrationJob<GameReview>
 {
-    protected override void Migrate(WorkContext context, GameReview[] batch)
+    protected override int Migrate(WorkContext context, GameReview[] batch)
     {
-        context.Database.MigrateReviewLabels(batch);
+        foreach (GameReview review in batch)
+        {
+#pragma warning disable CS0618 // LabelsString is obsolete          
+            if (string.IsNullOrWhiteSpace(review.LabelsString)) continue;
+
+            context.Database.Update(review);
+            review.Labels = LabelExtensions.FromLbpCommaList(review.LabelsString).ToList();
+#pragma warning restore CS0618
+        }
+
+        context.Database.SaveChanges();
+        return batch.Length;
     }
 
     protected override IQueryable<GameReview> SortAndFilter(IQueryable<GameReview> query)

Refresh.Interfaces.Workers/Migrations/BackfillRevisionMigration.cs

@@ -5,12 +5,13 @@ namespace Refresh.Interfaces.Workers.Migrations;
 
 public class BackfillRevisionMigration : MigrationJob<GameLevel>
 {
-    protected override void Migrate(WorkContext context, GameLevel[] batch)
+    protected override int Migrate(WorkContext context, GameLevel[] batch)
     {
         foreach (GameLevel level in batch)
         {
             context.Database.CreateRevisionForLevel(level, null);
         }
+        return batch.Length;
     }
 
     protected override IQueryable<GameLevel> SortAndFilter(IQueryable<GameLevel> query)

Refresh.Interfaces.Workers/Migrations/CalculateScoreRanksMigration.cs

@@ -12,12 +12,13 @@ protected override IQueryable<GameLevel> SortAndFilter(IQueryable<GameLevel> que
         return query.OrderBy(l => l.LevelId);
     }
 
-    protected override void Migrate(WorkContext context, GameLevel[] batch)
+    protected override int Migrate(WorkContext context, GameLevel[] batch)
     {
         foreach (GameLevel level in batch)
         {
             context.Database.RecalculateScoreStatistics(level);
         }
         context.Database.SaveChanges();
+        return batch.Length;
     }
 }

Refresh.Interfaces.Workers/Migrations/ClampPlayerLimitsMigration.cs

@@ -13,7 +13,7 @@ protected override IQueryable<GameLevel> SortAndFilter(IQueryable<GameLevel> que
         return query.OrderBy(l => l.LevelId);
     }
 
-    protected override void Migrate(WorkContext context, GameLevel[] batch)
+    protected override int Migrate(WorkContext context, GameLevel[] batch)
     {
         foreach (GameLevel level in batch)
         {
@@ -26,5 +26,7 @@ protected override void Migrate(WorkContext context, GameLevel[] batch)
             level.MinPlayers = Math.Clamp(level.MinPlayers, 1, 4);
             level.MaxPlayers = Math.Clamp(level.MaxPlayers, 1, 4);
         }
+
+        return batch.Length;
     }
 }

…ons/CorrectWebsitePinProgressPlatform.cs …ctWebsitePinProgressPlatformMigration.csRefresh.Interfaces.Workers/Migrations/CorrectWebsitePinProgressPlatform.cs renamed to Refresh.Interfaces.Workers/Migrations/CorrectWebsitePinProgressPlatformMigration.cs Refresh.Interfaces.Workers/Migrations/CorrectWebsitePinProgressPlatform.cs renamed to Refresh.Interfaces.Workers/Migrations/CorrectWebsitePinProgressPlatformMigration.cs

@@ -7,9 +7,9 @@
 
 namespace Refresh.Interfaces.Workers.Migrations;
 
-public class CorrectWebsitePinProgressPlatform : MigrationJob<PinProgressRelation>
+public class CorrectWebsitePinProgressPlatformMigration : MigrationJob<PinProgressRelation>
 {
-    private List<long> websitePinIds =
+    private readonly List<long> WebsitePinIds =
     [
         (long)ServerPins.HeartPlayerOnWebsite,
         (long)ServerPins.QueueLevelOnWebsite,
@@ -19,13 +19,15 @@ public class CorrectWebsitePinProgressPlatform : MigrationJob<PinProgressRelatio
     protected override IQueryable<PinProgressRelation> SortAndFilter(IQueryable<PinProgressRelation> query)
     {
         return query
-            .Where(p => this.websitePinIds.Contains(p.PinId))
+            .Where(p => this.WebsitePinIds.Contains(p.PinId))
             .OrderBy(p => p.PinId);
     }
 
-    protected override void Migrate(WorkContext context, PinProgressRelation[] batch)
+    protected override int Migrate(WorkContext context, PinProgressRelation[] batch)
     {
-        foreach (long pinId in this.websitePinIds)
+        int pinsLeft = batch.Length;
+
+        foreach (long pinId in this.WebsitePinIds)
         {
             IEnumerable<IGrouping<ObjectId, PinProgressRelation>> pinsByUser = batch
                 .Where(r => r.PinId == pinId)
@@ -38,12 +40,16 @@ protected override void Migrate(WorkContext context, PinProgressRelation[] batch
 
                 // Find best one by the current user
                 PinProgressRelation relationToMigrate = group.MaxBy(r => r.Progress)!;
+                List<PinProgressRelation> relationsToRemove = group.ToList();
 
-                // Remove all already existing progresses to remove duplicates
-                context.Database.RemoveAllPinProgressesByIdAndUser(pinId, relationToMigrate.PublisherId, false);
+                foreach (PinProgressRelation relation in group)
+                {
+                    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
-                context.Database.AddPinProgress(new()
+                PinProgressRelation newRelation = new()
                 {
                     PinId = relationToMigrate.PinId,
                     Progress = relationToMigrate.Progress,
@@ -52,10 +58,14 @@ protected override void Migrate(WorkContext context, PinProgressRelation[] batch
                     LastUpdated = relationToMigrate.LastUpdated,
                     IsBeta = false, // doesn't matter here
                     Platform = TokenPlatform.Website,
-                }, false);
+                };
+
+                context.Database.AddPinProgress(newRelation, false);
+                pinsLeft++;
             }
         }
 
         context.Database.SaveChanges();
+        return pinsLeft;
     }
 }