From 8370ca976557116af528330389f7fe18583d0896 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 15:46:29 +0900 Subject: [PATCH] Add `ImportAsUpdate` method to `IModelImporter` to avoid otehr changes --- .../Database/BeatmapImporterUpdateTests.cs | 2 +- ...ceneOnlinePlayBeatmapAvailabilityTracker.cs | 2 +- osu.Game/Beatmaps/BeatmapImporter.cs | 10 ++++++---- osu.Game/Beatmaps/BeatmapManager.cs | 2 +- osu.Game/Beatmaps/BeatmapModelDownloader.cs | 18 +----------------- osu.Game/Database/IModelImporter.cs | 10 ++++++++++ osu.Game/Database/ModelDownloader.cs | 16 +++------------- osu.Game/Database/RealmArchiveModelImporter.cs | 2 ++ osu.Game/Scoring/ScoreManager.cs | 2 ++ .../Select/Carousel/UpdateBeatmapSetButton.cs | 2 +- osu.Game/Skinning/SkinManager.cs | 2 ++ 11 files changed, 30 insertions(+), 38 deletions(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs index 713a73d64e..cf4245ae83 100644 --- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs @@ -60,7 +60,7 @@ namespace osu.Game.Tests.Database Assert.That(realm.Realm.All().First(s => !s.DeletePending).Beatmaps, Has.Count.EqualTo(11)); // Second import matches first but contains one extra .osu file. - var secondImport = (await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), firstImport.Value)).FirstOrDefault(); + var secondImport = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), firstImport.Value); Assert.That(secondImport, Is.Not.Null); Debug.Assert(secondImport != null); diff --git a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs index 0a980efbae..536322805b 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(BeatmapManager importer, IAPIProvider apiProvider) + public TestBeatmapModelDownloader(IModelImporter importer, IAPIProvider apiProvider) : base(importer, apiProvider) { } diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 2a6121b541..1e90e14e68 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -41,16 +41,18 @@ namespace osu.Game.Beatmaps { } - public async Task>> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) + public override async Task?> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) { var imported = await Import(notification, importTask); if (!imported.Any()) - return imported; + return null; Debug.Assert(imported.Count() == 1); - imported.First().PerformWrite(updated => + var first = imported.First(); + + first.PerformWrite(updated => { var realm = updated.Realm; @@ -103,7 +105,7 @@ namespace osu.Game.Beatmaps realm.Remove(original); }); - return imported; + return first; } protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == ".osz"; diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 1b31d29c2b..debe4c6829 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -408,7 +408,7 @@ namespace osu.Game.Beatmaps Realm.Run(r => Undelete(r.All().Where(s => s.DeletePending).ToList())); } - public Task>> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) => + public Task?> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) => beatmapImporter.ImportAsUpdate(notification, importTask, original); #region Implementation of ICanAcceptFiles diff --git a/osu.Game/Beatmaps/BeatmapModelDownloader.cs b/osu.Game/Beatmaps/BeatmapModelDownloader.cs index e70168f1b3..4295def5c3 100644 --- a/osu.Game/Beatmaps/BeatmapModelDownloader.cs +++ b/osu.Game/Beatmaps/BeatmapModelDownloader.cs @@ -1,39 +1,23 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -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(BeatmapManager beatmapImporter, IAPIProvider api) + public BeatmapModelDownloader(IModelImporter beatmapImporter, IAPIProvider api) : base(beatmapImporter, api) { - beatmapManager = beatmapImporter; } - - protected override Task>> Import(ProgressNotification notification, string filename, BeatmapSetInfo? originalModel) - { - if (originalModel != null) - return beatmapManager.ImportAsUpdate(notification, new ImportTask(filename), originalModel); - - return base.Import(notification, filename, null); - } - - public bool Update(BeatmapSetInfo model) => Download(model, false, model); } } diff --git a/osu.Game/Database/IModelImporter.cs b/osu.Game/Database/IModelImporter.cs index ebb8be39ef..4085f122d0 100644 --- a/osu.Game/Database/IModelImporter.cs +++ b/osu.Game/Database/IModelImporter.cs @@ -23,6 +23,16 @@ namespace osu.Game.Database /// The imported models. Task>> Import(ProgressNotification notification, params ImportTask[] tasks); + /// + /// Process a single import as an update for an existing model. + /// This will still run a full import, but perform any post-processing required to make it feel like an update to the user. + /// + /// The notification to update. + /// The import task. + /// The original model which is being updated. + /// The imported model. + Task?> ImportAsUpdate(ProgressNotification notification, ImportTask task, TModel original); + /// /// A user displayable name for the model type associated with this manager. /// diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index 5fd124cafc..7ce0fa46b7 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -44,6 +44,8 @@ namespace osu.Game.Database public bool Download(T model, bool minimiseDownloadSize = false) => Download(model, minimiseDownloadSize, null); + public void DownloadAsUpdate(TModel originalModel) => Download(originalModel, false, originalModel); + protected bool Download(T model, bool minimiseDownloadSize, TModel? originalModel) { if (!canDownload(model)) return false; @@ -66,7 +68,7 @@ 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 Import(notification, filename, originalModel).ConfigureAwait(false); + var imported = await importer.Import(notification, new ImportTask(filename)).ConfigureAwait(false); // for now a failed import will be marked as a failed download for simplicity. if (!imported.Any()) @@ -105,18 +107,6 @@ 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; diff --git a/osu.Game/Database/RealmArchiveModelImporter.cs b/osu.Game/Database/RealmArchiveModelImporter.cs index aa7fac07a8..a0cf98b978 100644 --- a/osu.Game/Database/RealmArchiveModelImporter.cs +++ b/osu.Game/Database/RealmArchiveModelImporter.cs @@ -174,6 +174,8 @@ namespace osu.Game.Database return imported; } + public virtual Task?> ImportAsUpdate(ProgressNotification notification, ImportTask task, TModel original) => throw new NotImplementedException(); + /// /// Import one from the filesystem and delete the file on success. /// Note that this bypasses the UI flow and should only be used for special cases or testing. diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 7cfc55580b..7367a1ef77 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -268,6 +268,8 @@ namespace osu.Game.Scoring public Task>> Import(ProgressNotification notification, params ImportTask[] tasks) => scoreImporter.Import(notification, tasks); + public Task> ImportAsUpdate(ProgressNotification notification, ImportTask task, ScoreInfo original) => scoreImporter.ImportAsUpdate(notification, task, original); + public Live Import(ScoreInfo item, ArchiveReader archive = null, bool batchImport = false, CancellationToken cancellationToken = default) => scoreImporter.ImportModel(item, archive, batchImport, cancellationToken); diff --git a/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs b/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs index 3746c1975c..73e7d23df0 100644 --- a/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs +++ b/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs @@ -90,7 +90,7 @@ namespace osu.Game.Screens.Select.Carousel Action = () => { - beatmapDownloader.Update(beatmapSetInfo); + beatmapDownloader.DownloadAsUpdate(beatmapSetInfo); attachExistingDownload(); }; } diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index dc0197e613..dd35f83434 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -272,6 +272,8 @@ namespace osu.Game.Skinning public Task>> Import(ProgressNotification notification, params ImportTask[] tasks) => skinImporter.Import(notification, tasks); + public Task> ImportAsUpdate(ProgressNotification notification, ImportTask task, SkinInfo original) => skinImporter.ImportAsUpdate(notification, task, original); + public Task> Import(ImportTask task, bool batchImport = false, CancellationToken cancellationToken = default) => skinImporter.Import(task, batchImport, cancellationToken); #endregion