Ability to properly backroll migration jobs (#1062)

Toastbrot236 · web-flow · commit b19ac2340021 · 2026-04-01T11:55:33.000+02:00
Makes the `WorkerManager` automatically remove job states from DB if the
manager doesn't have a matching job class, as long as the state belongs
to a `Refresh`-class job. This way, if an instance admin wants to
backroll their instance to a Refresh version which doesn't have a
certain migration job (which was previously already executed due to them
running the newer version first), and then they want to update their
instance again, the new job will migrate all entities again, which means
that it'll also catch all entities uploaded or updated after the
rollback, but before the update.

This does mean that such jobs could try to migrate already migrated
entities, however the jobs themselves should have to ensure that no
issues will happen because of that, e.g. by simply skipping entities
which don't need a migration (TODO until next release).

This also adds a few workarounds to weird test-exclusive issues I've
found when writing unit tests for this, where EF would reasonlessly try
to also insert the user and their stats when trying to insert a
different entity (`GameLevel` or `Event` mostly) into the database,
while having their user reference set.
diff --git a/Refresh.Database/GameDatabaseContext.Levels.cs b/Refresh.Database/GameDatabaseContext.Levels.cs
@@ -52,7 +52,7 @@ public GameLevel AddLevel(ISerializedPublishLevel createInfo, TokenGame game, Ga
             EnforceMinMaxPlayers = createInfo.EnforceMinMaxPlayers,
             SameScreenGame = createInfo.SameScreenGame,
             BackgroundGuid = createInfo.BackgroundGuid,
-            Publisher = publisher,
+            PublisherUserId = publisher.UserId,
             GameVersion = game,
             PublishDate = timestamp,
             UpdateDate = timestamp,
@@ -63,22 +63,19 @@ public GameLevel AddLevel(ISerializedPublishLevel createInfo, TokenGame game, Ga
 
         this.SaveChanges();
 
-        this.CreateRevisionForLevel(level, level.Publisher);
-        this.GameLevelStatistics.Add(level.Statistics = new GameLevelStatistics
-        {
-            LevelId = level.LevelId,
-        });
-
-        this.SaveChanges();
-
-        if (level.Publisher != null)
+        this.WriteEnsuringStatistics(publisher, () =>
         {
-            this.WriteEnsuringStatistics(level.Publisher, () =>
+            this.GameLevelStatistics.Add(level.Statistics = new GameLevelStatistics
             {
-                level.Publisher.Statistics!.LevelCount++;
+                LevelId = level.LevelId,
             });
-        }
 
+            this.CreateRevisionForLevel(level, level.Publisher);
+            publisher.Statistics!.LevelCount++;
+        });
+
+        level.Publisher = publisher;
+        this.Entry(level.Publisher).State = EntityState.Unchanged;
         return level;
     }
 
diff --git a/Refresh.Database/GameDatabaseContext.Workers.cs b/Refresh.Database/GameDatabaseContext.Workers.cs
@@ -50,6 +50,13 @@ public bool MarkWorkerContacted(int id)
         return JsonConvert.DeserializeObject(state.State, type);
     }
 
+    public IQueryable<string> GetAllJobIds(WorkerClass? workerClass)
+    {
+        IQueryable<PersistentJobState> states = this.JobStates;
+        if (workerClass != null) states = states.Where(s => s.Class == workerClass);
+        return states.Select(s => s.JobId);
+    }
+
     public void UpdateOrCreateJobState(string jobId, object state, WorkerClass workerClass)
     {
         PersistentJobState? jobState = this.JobStates.FirstOrDefault(s => s.JobId == jobId && s.Class == workerClass);
@@ -58,6 +65,7 @@ public void UpdateOrCreateJobState(string jobId, object state, WorkerClass worke
             jobState = new PersistentJobState
             {
                 JobId = jobId,
+                Class = workerClass,
             };
 
             this.JobStates.Add(jobState);
@@ -66,4 +74,10 @@ public void UpdateOrCreateJobState(string jobId, object state, WorkerClass worke
         jobState.State = JsonConvert.SerializeObject(state, Formatting.None);
         this.SaveChanges();
     }
+
+    public void RemoveJobState(string jobId, bool save = true)
+    {
+        this.JobStates.RemoveRange(s => s.JobId == jobId);
+        if (save) this.SaveChanges();
+    }
 }
diff --git a/Refresh.Workers/WorkerManager.cs b/Refresh.Workers/WorkerManager.cs
@@ -42,7 +42,7 @@ public void AddJob(WorkerJob worker)
         this._jobs.Add(worker);
     }
 
-    private void RunWorkCycle()
+    public void RunWorkCycle()
     {
         WorkContext context = new()
         {
@@ -99,12 +99,31 @@ private void RunWorkCycle()
         }
     }
 
+    public void RemoveUnusedJobStates()
+    {
+        GameDatabaseContext database = this._databaseProvider.GetContext();
+        string[] jobsFromWorkerManager = this._jobs.Select(j => j.GetType().Name).ToArray();
+        string[] jobsFromDatabase = database.GetAllJobIds(WorkerClass.Refresh).ToArray();
+        
+        foreach (string jobId in jobsFromDatabase)
+        {
+            if (!jobsFromWorkerManager.Any(j => j == jobId))
+            {
+                this._logger.LogInfo(RefreshContext.Worker, $"Removing job state for {jobId} because it doesn't exist in RefreshWorkerManager (likely from a newer/different Refresh build).");
+                database.RemoveJobState(jobId, false);
+            }
+        }
+        database.SaveChanges();
+    }
+
     public void Start()
     {
         this._logger.LogDebug(RefreshContext.Startup, "Starting the worker thread");
         this._threadShouldRun = true;
         Thread thread = new(() =>
         {
+            this.RemoveUnusedJobStates();
+
             while (this._threadShouldRun)
             {
                 try
diff --git a/RefreshTests.GameServer/TestContext.cs b/RefreshTests.GameServer/TestContext.cs
@@ -17,6 +17,7 @@
 using Refresh.Database.Models.Playlists;
 using Refresh.Interfaces.APIv3.Endpoints.DataTypes.Request;
 using Refresh.Database.Models.Photos;
+using Microsoft.EntityFrameworkCore;
 
 namespace RefreshTests.GameServer;
 
@@ -111,6 +112,7 @@ public GameUser CreateUser(string? username = null, GameUserRole role = GameUser
         GameUser user = this.Database.CreateUser(username, $"{username}@{username}.local");
         if (role != GameUserRole.User) this.Database.SetUserRole(user, role);
         if (verifyEmail) this.Database.VerifyUserEmail(user);
+        this.Database.Entry(user).State = EntityState.Unchanged;
 
         return user;
     }
@@ -237,11 +239,16 @@ public WorkContext GetWorkContext()
         {
             Database = this.Database,
             Logger = this.Server.Value.Logger,
-            DataStore = (IDataStore)this.GetService<StorageService>()
-                .AddParameterToEndpoint(null!, new BunkumParameterInfo(typeof(IDataStore), ""), null!)!,
+            DataStore = this.GetDataStore(),
         };
     }
 
+    public IDataStore GetDataStore()
+    {
+        return (IDataStore)this.GetService<StorageService>()
+            .AddParameterToEndpoint(null!, new BunkumParameterInfo(typeof(IDataStore), ""), null!)!;
+    }
+
     public void Dispose()
     {
         this.Database.Dispose();
diff --git a/RefreshTests.GameServer/Tests/Levels/UploadTests.cs b/RefreshTests.GameServer/Tests/Levels/UploadTests.cs
@@ -24,6 +24,33 @@ public void CanCreateLevelDirectly()
         Assert.That(context.Database.GetLevelById(1), Is.Not.Null);
     }
 
+    [Test]
+    public void CanCreateMultipleLevelsWhileRefreshing()
+    {
+        using TestContext context = this.GetServer(false);
+        GameUser user = context.CreateUser();
+
+        context.Database.Refresh();
+        GameLevel level1 = context.CreateLevel(user);
+        context.Database.Refresh();
+        GameLevel level2 = context.CreateLevel(user);
+        context.Database.Refresh();
+        GameLevel level3 = context.CreateLevel(user);
+        context.Database.Refresh();
+    }
+
+    [Test]
+    public void LevelReferencesAreSetAfterCreation()
+    {
+        using TestContext context = this.GetServer(false);
+        GameUser user = context.CreateUser();
+
+        context.Database.Refresh();
+        GameLevel level = context.CreateLevel(user);
+        Assert.That(level.Publisher, Is.Not.Null);
+        Assert.That(level.Statistics, Is.Not.Null);
+    }
+
     [Test]
     public void CanUpdateLevel()
     {
diff --git a/RefreshTests.GameServer/Tests/Workers/JobStateTests.cs b/RefreshTests.GameServer/Tests/Workers/JobStateTests.cs
@@ -0,0 +1,180 @@
+using Bunkum.Core.Storage;
+using Refresh.Database.Models.Levels;
+using Refresh.Database.Models.Users;
+using Refresh.Database.Models.Workers;
+using Refresh.Workers;
+using Refresh.Workers.State;
+
+namespace RefreshTests.GameServer.Tests.Workers;
+
+public class JobStateTests : GameServerTest
+{
+    [Test]
+    public void RemovesJobStateIfJobDoesntExist()
+    {
+        using TestContext context = this.GetServer();
+        IDataStore dataStore = context.GetDataStore();
+        WorkerManager manager = new(Logger, dataStore, context.DatabaseProvider);
+
+        context.Database.UpdateOrCreateJobState(typeof(TestMigrationJob).Name, new MigrationJobState(), WorkerClass.Refresh);
+        Assert.That(context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Refresh), Is.Not.Null);
+
+        manager.RemoveUnusedJobStates();
+        manager.RunWorkCycle();
+        context.Database.Refresh();
+
+        object? stateObject = context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Refresh);
+        Assert.That(stateObject, Is.Null);
+    }
+
+    [Test]
+    [TestCase(true)]
+    [TestCase(false)]
+    public void DoesNotRemoveJobStateIfJobExists(bool uploadLevel)
+    {
+        using TestContext context = this.GetServer();
+        IDataStore dataStore = context.GetDataStore();
+        WorkerManager manager = new(Logger, dataStore, context.DatabaseProvider);
+        TestMigrationJob job = new();
+        manager.AddJob(job);
+
+        context.Database.UpdateOrCreateJobState(typeof(TestMigrationJob).Name, new MigrationJobState()
+        {
+            Total = uploadLevel ? 1 : 0, // Since we're creating the state manually, instead of having WorkerManager do it
+        }, WorkerClass.Refresh);
+        Assert.That(context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Refresh), Is.Not.Null);
+
+        if (uploadLevel)
+        {
+            context.CreateLevel(context.CreateUser());
+            Assert.That(context.Database.GetTotalLevelCount(), Is.EqualTo(1));
+            
+        }
+        manager.RemoveUnusedJobStates();
+        manager.RunWorkCycle();
+        context.Database.Refresh();
+
+        object? stateObject = context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Refresh);
+        Assert.That(stateObject, Is.Not.Null);
+
+        MigrationJobState jobState = (MigrationJobState)stateObject!;
+        Assert.That(jobState.Processed, Is.EqualTo(uploadLevel ? 1 : 0));
+        Assert.That(jobState.Total, Is.EqualTo(uploadLevel ? 1 : 0));
+        Assert.That(jobState.Complete, Is.True);
+    }
+
+    [Test]
+    public void DoesNotRemoveJobStateIfNotRefreshClass()
+    {
+        using TestContext context = this.GetServer();
+        IDataStore dataStore = context.GetDataStore();
+        WorkerManager manager = new(Logger, dataStore, context.DatabaseProvider);
+        
+        context.Database.UpdateOrCreateJobState(typeof(TestMigrationJob).Name, new MigrationJobState(), WorkerClass.Craftworld);
+        Assert.That(context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Craftworld), Is.Not.Null);
+
+        manager.RemoveUnusedJobStates();
+        manager.RunWorkCycle();
+        context.Database.Refresh();
+
+        object? stateObject = context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Craftworld);
+        Assert.That(stateObject, Is.Not.Null);
+
+        MigrationJobState jobState = (MigrationJobState)stateObject!;
+        Assert.That(jobState.Complete, Is.True);
+    }
+
+    [Test]
+    public void ReExecutesMigrationJobAfterRollbackAndReupdate()
+    {
+        using TestContext context = this.GetServer();
+        IDataStore dataStore = context.GetDataStore();
+        WorkerManager manager = new(Logger, dataStore, context.DatabaseProvider);
+        TestMigrationJob job = new();
+        manager.AddJob(job);
+
+        GameUser user = context.CreateUser();
+        GameLevel firstLevel = context.CreateLevel(user);
+        
+        // migrate first level
+        manager.RemoveUnusedJobStates();
+        manager.RunWorkCycle();
+        context.Database.Refresh();
+
+        object? stateObject = context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Refresh);
+        Assert.That(stateObject, Is.Not.Null);
+
+        MigrationJobState jobState = (MigrationJobState)stateObject!;
+        Assert.That(jobState.Complete, Is.True);
+        Assert.That(jobState.Processed, Is.EqualTo(1));
+
+        GameLevel? firstLevelMigrated = context.Database.GetLevelById(firstLevel.LevelId);
+        Assert.That(firstLevelMigrated, Is.Not.Null);
+        Assert.That(firstLevelMigrated!.Title, Does.EndWith(" test"));
+
+        // We can ignore the fact that this level's title won't end on " test", since when we add a real migration job for a certain entity,
+        // we also adjust that entity's creation/update methods in order to apply whatever change we want to new entities aswell.
+        // We don't do it here, and it doesn't matter in this test.
+        context.Database.Refresh();
+        GameLevel secondLevel = context.CreateLevel(user);
+        // Should skip job because it's still "complete", so the new level won't be migrated.
+        manager.RunWorkCycle();
+        context.Database.Refresh();
+
+        GameLevel? secondLevelFromDb = context.Database.GetLevelById(secondLevel.LevelId);
+        Assert.That(secondLevelFromDb, Is.Not.Null);
+        Assert.That(secondLevelFromDb!.Title, Does.Not.EndWith(" test"));
+
+        stateObject = context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Refresh);
+        Assert.That(stateObject, Is.Not.Null);
+
+        jobState = (MigrationJobState)stateObject!;
+        Assert.That(jobState.Complete, Is.True);
+        Assert.That(jobState.Processed, Is.EqualTo(1));
+
+        // Simulate a roll-back, meaning the job wouldn't be in the WorkerManager anymore, so no migrations will happen, and the job state would be
+        // auto-removed by WorkerManager.Start() in real cases.
+        context.Database.Refresh();
+        manager = new(Logger, dataStore, context.DatabaseProvider);
+        GameLevel thirdLevel = context.CreateLevel(user);
+
+        manager.RemoveUnusedJobStates();
+        manager.RunWorkCycle();
+        context.Database.Refresh();
+
+        // No new levels were migrated, and the job state was removed
+        stateObject = context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Refresh);
+        Assert.That(stateObject, Is.Null);
+
+        GameLevel? secondLevelMigrated = context.Database.GetLevelById(secondLevel.LevelId);
+        Assert.That(secondLevelMigrated, Is.Not.Null);
+        Assert.That(secondLevelMigrated!.Title, Does.Not.EndWith(" test"));
+
+        GameLevel? thirdLevelMigrated = context.Database.GetLevelById(thirdLevel.LevelId);
+        Assert.That(thirdLevelMigrated, Is.Not.Null);
+        Assert.That(thirdLevelMigrated!.Title, Does.Not.EndWith(" test"));
+
+        // Now simulate a re-update, where the job is in the WorkerManager again
+        manager = new(Logger, dataStore, context.DatabaseProvider);
+        job = new();
+        manager.AddJob(job);
+        manager.RemoveUnusedJobStates();
+        manager.RunWorkCycle();
+        context.Database.Refresh();
+
+        stateObject = context.Database.GetJobState(typeof(TestMigrationJob).Name, typeof(MigrationJobState), WorkerClass.Refresh);
+        Assert.That(stateObject, Is.Not.Null);
+
+        jobState = (MigrationJobState)stateObject!;
+        Assert.That(jobState.Complete, Is.True);
+        Assert.That(jobState.Processed, Is.EqualTo(3));
+
+        secondLevelMigrated = context.Database.GetLevelById(secondLevel.LevelId);
+        Assert.That(secondLevelMigrated, Is.Not.Null);
+        Assert.That(secondLevelMigrated!.Title, Does.EndWith(" test"));
+
+        thirdLevelMigrated = context.Database.GetLevelById(thirdLevel.LevelId);
+        Assert.That(thirdLevelMigrated, Is.Not.Null);
+        Assert.That(thirdLevelMigrated!.Title, Does.EndWith(" test"));
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -50,6 +50,13 @@ public bool MarkWorkerContacted(int id)`
`50`	`50`	`return JsonConvert.DeserializeObject(state.State, type);`
`51`	`51`	`}`
`52`	`52`
	`53`	`+ public IQueryable<string> GetAllJobIds(WorkerClass? workerClass)`
	`54`	`+ {`
	`55`	`+ IQueryable<PersistentJobState> states = this.JobStates;`
	`56`	`+ if (workerClass != null) states = states.Where(s => s.Class == workerClass);`
	`57`	`+ return states.Select(s => s.JobId);`
	`58`	`+ }`
	`59`	`+`
`53`	`60`	`public void UpdateOrCreateJobState(string jobId, object state, WorkerClass workerClass)`
`54`	`61`	`{`
`55`	`62`	`PersistentJobState? jobState = this.JobStates.FirstOrDefault(s => s.JobId == jobId && s.Class == workerClass);`
`@@ -58,6 +65,7 @@ public void UpdateOrCreateJobState(string jobId, object state, WorkerClass worke`
`58`	`65`	`jobState = new PersistentJobState`
`59`	`66`	`{`
`60`	`67`	`JobId = jobId,`
	`68`	`+ Class = workerClass,`
`61`	`69`	`};`
`62`	`70`
`63`	`71`	`this.JobStates.Add(jobState);`
`@@ -66,4 +74,10 @@ public void UpdateOrCreateJobState(string jobId, object state, WorkerClass worke`
`66`	`74`	`jobState.State = JsonConvert.SerializeObject(state, Formatting.None);`
`67`	`75`	`this.SaveChanges();`
`68`	`76`	`}`
	`77`	`+`
	`78`	`+ public void RemoveJobState(string jobId, bool save = true)`
	`79`	`+ {`
	`80`	`+ this.JobStates.RemoveRange(s => s.JobId == jobId);`
	`81`	`+ if (save) this.SaveChanges();`
	`82`	`+ }`
`69`	`83`	`}`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ public void AddJob(WorkerJob worker)`
`42`	`42`	`this._jobs.Add(worker);`
`43`	`43`	`}`
`44`	`44`
`45`		`- private void RunWorkCycle()`
	`45`	`+ public void RunWorkCycle()`
`46`	`46`	`{`
`47`	`47`	`WorkContext context = new()`
`48`	`48`	`{`
`@@ -99,12 +99,31 @@ private void RunWorkCycle()`
`99`	`99`	`}`
`100`	`100`	`}`
`101`	`101`
	`102`	`+ public void RemoveUnusedJobStates()`
	`103`	`+ {`
	`104`	`+ GameDatabaseContext database = this._databaseProvider.GetContext();`
	`105`	`+ string[] jobsFromWorkerManager = this._jobs.Select(j => j.GetType().Name).ToArray();`
	`106`	`+ string[] jobsFromDatabase = database.GetAllJobIds(WorkerClass.Refresh).ToArray();`
	`107`	`+`
	`108`	`+ foreach (string jobId in jobsFromDatabase)`
	`109`	`+ {`
	`110`	`+ if (!jobsFromWorkerManager.Any(j => j == jobId))`
	`111`	`+ {`
	`112`	`+ this._logger.LogInfo(RefreshContext.Worker, $"Removing job state for {jobId} because it doesn't exist in RefreshWorkerManager (likely from a newer/different Refresh build).");`
	`113`	`+ database.RemoveJobState(jobId, false);`
	`114`	`+ }`
	`115`	`+ }`
	`116`	`+ database.SaveChanges();`
	`117`	`+ }`
	`118`	`+`
`102`	`119`	`public void Start()`
`103`	`120`	`{`
`104`	`121`	`this._logger.LogDebug(RefreshContext.Startup, "Starting the worker thread");`
`105`	`122`	`this._threadShouldRun = true;`
`106`	`123`	`Thread thread = new(() =>`
`107`	`124`	`{`
	`125`	`+ this.RemoveUnusedJobStates();`
	`126`	`+`
`108`	`127`	`while (this._threadShouldRun)`
`109`	`128`	`{`
`110`	`129`	`try`