From 5ed3cd205f068d572caddc0ae01aa7ab8a580a6a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 3 Jun 2020 22:35:01 +0900 Subject: [PATCH] Simplify reuse check using FileInfo IDs --- .../Beatmaps/IO/ImportBeatmapTest.cs | 9 +++++---- osu.Game/Database/ArchiveModelManager.cs | 20 +++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index 12f06059f7..9b34eece5f 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -154,9 +154,9 @@ namespace osu.Game.Tests.Beatmaps.IO } [Test] - public async Task TestImportThenImportWithNewerTimestamp() + public async Task TestImportThenImportWithChangedFile() { - using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestImportThenImportWithNewerTimestamp))) + using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestImportThenImportWithChangedFile))) { try { @@ -174,8 +174,9 @@ namespace osu.Game.Tests.Beatmaps.IO using (var zip = ZipArchive.Open(temp)) zip.WriteToDirectory(extractedFolder); - // change timestamp - new FileInfo(Directory.GetFiles(extractedFolder).First()).LastWriteTime = DateTime.Now; + // arbitrary write to non-hashed file + using (var sw = new FileInfo(Directory.GetFiles(extractedFolder, "*.mp3").First()).AppendText()) + sw.WriteLine("text"); using (var zip = ZipArchive.Create()) { diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 5ca9423de2..0fe8dd1268 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -667,10 +667,16 @@ namespace osu.Game.Database /// The newly imported model. /// Whether the existing model should be restored and used. Returning false will delete the existing and force a re-import. protected virtual bool CanReuseExisting(TModel existing, TModel import) => - getFilenames(existing.Files).SequenceEqual(getFilenames(import.Files)) && - // poor-man's (cheap) equality comparison, avoiding hashing unnecessarily. - // can switch to full hash checks on a per-case basis (or for all) if we decide this is not a performance issue. - getTimestamps(existing.Files).SequenceEqual(getTimestamps(import.Files)); + // 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 FileInfo IDs. + getIDs(existing.Files).SequenceEqual(getIDs(import.Files)) && + getFilenames(existing.Files).SequenceEqual(getFilenames(import.Files)); + + private IEnumerable getIDs(List files) + { + foreach (var f in files.OrderBy(f => f.Filename)) + yield return f.FileInfo.ID; + } private IEnumerable getFilenames(List files) { @@ -678,12 +684,6 @@ namespace osu.Game.Database yield return f.Filename; } - private IEnumerable getTimestamps(List files) - { - foreach (var f in files.OrderBy(f => f.Filename)) - yield return File.GetLastWriteTimeUtc(Files.Storage.GetFullPath(f.FileInfo.StoragePath)).ToFileTime(); - } - private DbSet queryModel() => ContextFactory.Get().Set(); protected virtual string HumanisedModelName => $"{typeof(TModel).Name.Replace("Info", "").ToLower()}";