1
0
mirror of https://github.com/ppy/osu.git synced 2024-09-22 03:27:24 +08:00

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).
This commit is contained in:
Dean Herbert 2022-01-25 13:42:41 +09:00
parent 7d63999eef
commit bbcc149e2e
2 changed files with 39 additions and 3 deletions

View File

@ -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()
{

View File

@ -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.