From 2e14d8730c397f86cf5ad75cc6b7bd24ef73ffd7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 19:31:46 +0900 Subject: [PATCH] Move implementation of updating a beatmap to `BeatmapImporter` --- ...eneOnlinePlayBeatmapAvailabilityTracker.cs | 2 +- osu.Game/Beatmaps/BeatmapImporter.cs | 56 ++++++++++++++++ osu.Game/Beatmaps/BeatmapManager.cs | 3 + osu.Game/Beatmaps/BeatmapModelDownloader.cs | 64 ++++--------------- .../Drawables/BundledBeatmapDownloader.cs | 3 +- osu.Game/Database/ModelDownloader.cs | 20 ++++-- 6 files changed, 89 insertions(+), 59 deletions(-) diff --git a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs index 536322805b..0a980efbae 100644 --- a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs @@ -237,7 +237,7 @@ namespace osu.Game.Tests.Online internal class TestBeatmapModelDownloader : BeatmapModelDownloader { - public TestBeatmapModelDownloader(IModelImporter importer, IAPIProvider apiProvider) + public TestBeatmapModelDownloader(BeatmapManager importer, IAPIProvider apiProvider) : base(importer, apiProvider) { } diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 2fdaeb9eed..3cfeb0df67 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -3,9 +3,11 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Threading; +using System.Threading.Tasks; using osu.Framework.Extensions; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Logging; @@ -16,6 +18,7 @@ using osu.Game.Database; using osu.Game.Extensions; using osu.Game.IO; using osu.Game.IO.Archives; +using osu.Game.Overlays.Notifications; using osu.Game.Rulesets; using Realms; @@ -38,6 +41,59 @@ namespace osu.Game.Beatmaps { } + public async Task>> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) + { + var imported = await Import(notification, importTask); + + Debug.Assert(imported.Count() == 1); + + imported.First().PerformWrite(updated => + { + Logger.Log($"Beatmap \"{updated}\"update completed successfully", LoggingTarget.Database); + + original = updated.Realm.Find(original.ID); + + // Generally the import process will do this for us if the OnlineIDs match, + // but that isn't a guarantee (ie. if the .osu file doesn't have OnlineIDs populated). + original.DeletePending = true; + + // Transfer local values which should be persisted across a beatmap update. + updated.DateAdded = original.DateAdded; + + foreach (var beatmap in original.Beatmaps.ToArray()) + { + var updatedBeatmap = updated.Beatmaps.FirstOrDefault(b => b.Hash == beatmap.Hash); + + if (updatedBeatmap != null) + { + // If the updated beatmap matches an existing one, transfer any user data across.. + if (beatmap.Scores.Any()) + { + Logger.Log($"Transferring {beatmap.Scores.Count()} scores for unchanged difficulty \"{beatmap}\"", LoggingTarget.Database); + + foreach (var score in beatmap.Scores) + score.BeatmapInfo = updatedBeatmap; + } + + // ..then nuke the old beatmap completely. + // this is done instead of a soft deletion to avoid a user potentially creating weird + // interactions, like restoring the outdated beatmap then updating a second time + // (causing user data to be wiped). + original.Beatmaps.Remove(beatmap); + } + else + { + // If the beatmap differs in the original, leave it in a soft-deleted state but reset online info. + // This caters to the case where a user has made modifications they potentially want to restore, + // but after restoring we want to ensure it can't be used to trigger an update of the beatmap. + beatmap.ResetOnlineInfo(); + } + } + }); + + return imported; + } + protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == ".osz"; protected override void Populate(BeatmapSetInfo beatmapSet, ArchiveReader? archive, Realm realm, CancellationToken cancellationToken = default) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 62edd6e8da..1b31d29c2b 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -408,6 +408,9 @@ namespace osu.Game.Beatmaps Realm.Run(r => Undelete(r.All().Where(s => s.DeletePending).ToList())); } + public Task>> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) => + beatmapImporter.ImportAsUpdate(notification, importTask, original); + #region Implementation of ICanAcceptFiles public Task Import(params string[] paths) => beatmapImporter.Import(paths); diff --git a/osu.Game/Beatmaps/BeatmapModelDownloader.cs b/osu.Game/Beatmaps/BeatmapModelDownloader.cs index a566aee49e..e70168f1b3 100644 --- a/osu.Game/Beatmaps/BeatmapModelDownloader.cs +++ b/osu.Game/Beatmaps/BeatmapModelDownloader.cs @@ -1,77 +1,39 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Linq; -using osu.Framework.Logging; +using System.Collections.Generic; +using System.Threading.Tasks; using osu.Game.Database; using osu.Game.Online.API; using osu.Game.Online.API.Requests; +using osu.Game.Overlays.Notifications; namespace osu.Game.Beatmaps { public class BeatmapModelDownloader : ModelDownloader { + private readonly BeatmapManager beatmapManager; + protected override ArchiveDownloadRequest CreateDownloadRequest(IBeatmapSetInfo set, bool minimiseDownloadSize) => new DownloadBeatmapSetRequest(set, minimiseDownloadSize); public override ArchiveDownloadRequest? GetExistingDownload(IBeatmapSetInfo model) => CurrentDownloads.Find(r => r.Model.OnlineID == model.OnlineID); - public BeatmapModelDownloader(IModelImporter beatmapImporter, IAPIProvider api) + public BeatmapModelDownloader(BeatmapManager beatmapImporter, IAPIProvider api) : base(beatmapImporter, api) { + beatmapManager = beatmapImporter; } - public bool Update(BeatmapSetInfo model) + protected override Task>> Import(ProgressNotification notification, string filename, BeatmapSetInfo? originalModel) { - return Download(model, false, onSuccess); + if (originalModel != null) + return beatmapManager.ImportAsUpdate(notification, new ImportTask(filename), originalModel); - void onSuccess(Live imported) - { - imported.PerformWrite(updated => - { - Logger.Log($"Beatmap \"{updated}\"update completed successfully", LoggingTarget.Database); - - var original = updated.Realm.Find(model.ID); - - // Generally the import process will do this for us if the OnlineIDs match, - // but that isn't a guarantee (ie. if the .osu file doesn't have OnlineIDs populated). - original.DeletePending = true; - - // Transfer local values which should be persisted across a beatmap update. - updated.DateAdded = original.DateAdded; - - foreach (var beatmap in original.Beatmaps.ToArray()) - { - var updatedBeatmap = updated.Beatmaps.FirstOrDefault(b => b.Hash == beatmap.Hash); - - if (updatedBeatmap != null) - { - // If the updated beatmap matches an existing one, transfer any user data across.. - if (beatmap.Scores.Any()) - { - Logger.Log($"Transferring {beatmap.Scores.Count()} scores for unchanged difficulty \"{beatmap}\"", LoggingTarget.Database); - - foreach (var score in beatmap.Scores) - score.BeatmapInfo = updatedBeatmap; - } - - // ..then nuke the old beatmap completely. - // this is done instead of a soft deletion to avoid a user potentially creating weird - // interactions, like restoring the outdated beatmap then updating a second time - // (causing user data to be wiped). - original.Beatmaps.Remove(beatmap); - } - else - { - // If the beatmap differs in the original, leave it in a soft-deleted state but reset online info. - // This caters to the case where a user has made modifications they potentially want to restore, - // but after restoring we want to ensure it can't be used to trigger an update of the beatmap. - beatmap.ResetOnlineInfo(); - } - } - }); - } + return base.Import(notification, filename, null); } + + public bool Update(BeatmapSetInfo model) => Download(model, false, model); } } diff --git a/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs b/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs index 80af4108c7..bcbe7084d5 100644 --- a/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs +++ b/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs @@ -11,7 +11,6 @@ using System.Text.RegularExpressions; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Game.Database; using osu.Game.Online; using osu.Game.Online.API; using osu.Game.Online.API.Requests; @@ -109,7 +108,7 @@ namespace osu.Game.Beatmaps.Drawables private class BundledBeatmapModelDownloader : BeatmapModelDownloader { - public BundledBeatmapModelDownloader(IModelImporter beatmapImporter, IAPIProvider api) + public BundledBeatmapModelDownloader(BeatmapManager beatmapImporter, IAPIProvider api) : base(beatmapImporter, api) { } diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index 8fa1c12d88..5fd124cafc 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -44,7 +44,7 @@ namespace osu.Game.Database public bool Download(T model, bool minimiseDownloadSize = false) => Download(model, minimiseDownloadSize, null); - protected bool Download(T model, bool minimiseDownloadSize, Action>? onSuccess) + protected bool Download(T model, bool minimiseDownloadSize, TModel? originalModel) { if (!canDownload(model)) return false; @@ -66,12 +66,10 @@ namespace osu.Game.Database Task.Factory.StartNew(async () => { // This gets scheduled back to the update thread, but we want the import to run in the background. - var imported = await importer.Import(notification, new ImportTask(filename)).ConfigureAwait(false); + var imported = await Import(notification, filename, originalModel).ConfigureAwait(false); // for now a failed import will be marked as a failed download for simplicity. - if (imported.Any()) - onSuccess?.Invoke(imported.Single()); - else + if (!imported.Any()) DownloadFailed?.Invoke(request); CurrentDownloads.Remove(request); @@ -107,6 +105,18 @@ namespace osu.Game.Database } } + /// + /// Run the post-download import for the model. + /// + /// The notification to update. + /// The path of the temporary downloaded file. + /// An optional model for update scenarios, to be used as a reference. + /// The imported model. + protected virtual Task>> Import(ProgressNotification notification, string filename, TModel? originalModel) + { + return importer.Import(notification, new ImportTask(filename)); + } + public abstract ArchiveDownloadRequest? GetExistingDownload(T model); private bool canDownload(T model) => GetExistingDownload(model) == null && api != null;