From bbcc149e2e7cc01045374b7b25be85983658aa0e Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Tue, 25 Jan 2022 13:42:41 +0900
Subject: [PATCH] During import if files are found to be missing, ensure they
 are restored

This is one step closer to sanity in terms of physical files. As per the
comment I have left in place, we really should be checking file sizes or
hashes, but to keep things simple and fast I've opted to just cover the
"missing file" scenario for now.

Ran into this when testing against a foreign `client.realm` by:
- Noticing a beatmap doesn't load
- Deleting said beatmap
- Downloading via beatmap overlay
- Beatmap is restored but still doesn't work

Note that I've kept the logic where this will undelete an existing
import rather than create one from fresh, as I think that is beneficial
to the user (ie. it will still keep any linked scores on restore).
---
 .../Database/BeatmapImporterTests.cs          | 32 +++++++++++++++++++
 osu.Game/Stores/RealmArchiveModelImporter.cs  | 10 ++++--
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs
index 227314cffd..abab8ae3c6 100644
--- a/osu.Game.Tests/Database/BeatmapImporterTests.cs
+++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs
@@ -600,6 +600,38 @@ namespace osu.Game.Tests.Database
             });
         }
 
+        [Test]
+        public void TestImportThenReimportAfterMissingFiles()
+        {
+            RunTestWithRealmAsync(async (realmFactory, storage) =>
+            {
+                using var importer = new BeatmapModelManager(realmFactory, storage);
+                using var store = new RulesetStore(realmFactory, storage);
+
+                var imported = await LoadOszIntoStore(importer, realmFactory.Context);
+
+                deleteBeatmapSet(imported, realmFactory.Context);
+
+                Assert.IsTrue(imported.DeletePending);
+
+                // intentionally nuke all files
+                storage.DeleteDirectory("files");
+
+                Assert.That(imported.Files.All(f => !storage.GetStorageForDirectory("files").Exists(f.File.GetStoragePath())));
+
+                var importedSecondTime = await LoadOszIntoStore(importer, realmFactory.Context);
+
+                // check the newly "imported" beatmap is actually just the restored previous import. since it matches hash.
+                Assert.IsTrue(imported.ID == importedSecondTime.ID);
+                Assert.IsTrue(imported.Beatmaps.First().ID == importedSecondTime.Beatmaps.First().ID);
+                Assert.IsFalse(imported.DeletePending);
+                Assert.IsFalse(importedSecondTime.DeletePending);
+
+                // check that the files now exist, even though they were deleted above.
+                Assert.That(importedSecondTime.Files.All(f => storage.GetStorageForDirectory("files").Exists(f.File.GetStoragePath())));
+            });
+        }
+
         [Test]
         public void TestImportThenDeleteThenImportNonOptimisedPath()
         {
diff --git a/osu.Game/Stores/RealmArchiveModelImporter.cs b/osu.Game/Stores/RealmArchiveModelImporter.cs
index 3d8e9f2703..154135ea71 100644
--- a/osu.Game/Stores/RealmArchiveModelImporter.cs
+++ b/osu.Game/Stores/RealmArchiveModelImporter.cs
@@ -342,7 +342,8 @@ namespace osu.Game.Stores
                         // note that this should really be checking filesizes on disk (of existing files) for some degree of sanity.
                         // or alternatively doing a faster hash check. either of these require database changes and reprocessing of existing files.
                         if (CanSkipImport(existing, item) &&
-                            getFilenames(existing.Files).SequenceEqual(getShortenedFilenames(archive).Select(p => p.shortened).OrderBy(f => f)))
+                            getFilenames(existing.Files).SequenceEqual(getShortenedFilenames(archive).Select(p => p.shortened).OrderBy(f => f)) &&
+                            checkAllFilesExist(existing))
                         {
                             LogForModel(item, @$"Found existing (optimised) {HumanisedModelName} for {item} (ID {existing.ID}) – skipping import.");
 
@@ -459,7 +460,6 @@ namespace osu.Game.Stores
             if (!(prefix.EndsWith('/') || prefix.EndsWith('\\')))
                 prefix = string.Empty;
 
-            // import files to manager
             foreach (string file in reader.Filenames)
                 yield return (file, file.Substring(prefix.Length).ToStandardisedPath());
         }
@@ -519,7 +519,11 @@ namespace osu.Game.Stores
             // for the best or worst, we copy and import files of a new import before checking whether
             // it is a duplicate. so to check if anything has changed, we can just compare all File IDs.
             getIDs(existing.Files).SequenceEqual(getIDs(import.Files)) &&
-            getFilenames(existing.Files).SequenceEqual(getFilenames(import.Files));
+            getFilenames(existing.Files).SequenceEqual(getFilenames(import.Files)) &&
+            checkAllFilesExist(existing);
+
+        private bool checkAllFilesExist(TModel model) =>
+            model.Files.All(f => Files.Storage.Exists(f.File.GetStoragePath()));
 
         /// <summary>
         /// Whether this specified path should be removed after successful import.