Merge pull request #29256 from peppy/import-as-update-date-maintain

Fix date added sometimes getting reset when updating a beatmap
ppy · Aug 2, 2024 · 3457721 · 3457721
2 parents 0fde22b + dc73856
commit 3457721
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 3 deletions.
diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs
@@ -260,7 +260,7 @@ public void TestUpdatedImportContainsNothing()
         }
 
         [Test]
-        public void TestNoChanges()
+        public void TestNoChangesAfterDelete()
         {
             RunTestWithRealmAsync(async (realm, storage) =>
             {
@@ -272,21 +272,63 @@ public void TestNoChanges()
 
                 var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal));
 
+                importBeforeUpdate!.PerformWrite(s => s.DeletePending = true);
+
+                var dateBefore = importBeforeUpdate.Value.DateAdded;
+
                 Assert.That(importBeforeUpdate, Is.Not.Null);
                 Debug.Assert(importBeforeUpdate != null);
 
                 var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginalSecond), importBeforeUpdate.Value);
 
+                realm.Run(r => r.Refresh());
+
                 Assert.That(importAfterUpdate, Is.Not.Null);
                 Debug.Assert(importAfterUpdate != null);
 
+                checkCount<BeatmapSetInfo>(realm, 1);
+                checkCount<BeatmapInfo>(realm, count_beatmaps);
+                checkCount<BeatmapMetadata>(realm, count_beatmaps);
+
+                Assert.That(importBeforeUpdate.Value.Beatmaps.First().OnlineID, Is.GreaterThan(-1));
+                Assert.That(importBeforeUpdate.Value.DateAdded, Is.EqualTo(dateBefore));
+                Assert.That(importAfterUpdate.Value.DateAdded, Is.EqualTo(dateBefore));
+                Assert.That(importBeforeUpdate.ID, Is.EqualTo(importAfterUpdate.ID));
+            });
+        }
+
+        [Test]
+        public void TestNoChanges()
+        {
+            RunTestWithRealmAsync(async (realm, storage) =>
+            {
+                var importer = new BeatmapImporter(storage, realm);
+                using var rulesets = new RealmRulesetStore(realm, storage);
+
+                using var __ = getBeatmapArchive(out string pathOriginal);
+                using var _ = getBeatmapArchive(out string pathOriginalSecond);
+
+                var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal));
+
+                var dateBefore = importBeforeUpdate!.Value.DateAdded;
+
+                Assert.That(importBeforeUpdate, Is.Not.Null);
+                Debug.Assert(importBeforeUpdate != null);
+
+                var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginalSecond), importBeforeUpdate.Value);
+
                 realm.Run(r => r.Refresh());
 
+                Assert.That(importAfterUpdate, Is.Not.Null);
+                Debug.Assert(importAfterUpdate != null);
+
                 checkCount<BeatmapSetInfo>(realm, 1);
                 checkCount<BeatmapInfo>(realm, count_beatmaps);
                 checkCount<BeatmapMetadata>(realm, count_beatmaps);
 
                 Assert.That(importBeforeUpdate.Value.Beatmaps.First().OnlineID, Is.GreaterThan(-1));
+                Assert.That(importBeforeUpdate.Value.DateAdded, Is.EqualTo(dateBefore));
+                Assert.That(importAfterUpdate.Value.DateAdded, Is.EqualTo(dateBefore));
                 Assert.That(importBeforeUpdate.ID, Is.EqualTo(importAfterUpdate.ID));
             });
         }

diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs
@@ -43,6 +43,8 @@ public BeatmapImporter(Storage storage, RealmAccess realm)
 
         public override async Task<Live<BeatmapSetInfo>?> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original)
         {
+            var originalDateAdded = original.DateAdded;
+
             Guid originalId = original.ID;
 
             var imported = await Import(notification, new[] { importTask }).ConfigureAwait(false);
@@ -57,8 +59,11 @@ public BeatmapImporter(Storage storage, RealmAccess realm)
             // If there were no changes, ensure we don't accidentally nuke ourselves.
             if (first.ID == originalId)
             {
-                first.PerformRead(s =>
+                first.PerformWrite(s =>
                 {
+                    // Transfer local values which should be persisted across a beatmap update.
+                    s.DateAdded = originalDateAdded;
+
                     // Re-run processing even in this case. We might have outdated metadata.
                     ProcessBeatmap?.Invoke(s, MetadataLookupScope.OnlineFirst);
                 });
@@ -79,7 +84,7 @@ public BeatmapImporter(Storage storage, RealmAccess realm)
                 original.DeletePending = true;
 
                 // Transfer local values which should be persisted across a beatmap update.
-                updated.DateAdded = original.DateAdded;
+                updated.DateAdded = originalDateAdded;
 
                 transferCollectionReferences(realm, original, updated);
 
@@ -278,6 +283,9 @@ protected override bool CanReuseExisting(BeatmapSetInfo existing, BeatmapSetInfo
 
         protected override void UndeleteForReuse(BeatmapSetInfo existing)
         {
+            if (!existing.DeletePending)
+                return;
+
             base.UndeleteForReuse(existing);
             existing.DateAdded = DateTimeOffset.UtcNow;
         }