From cd41862e3bcfd579b40eea94c84cb00c8d1d62ff Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 17 Oct 2017 15:00:27 +0900 Subject: [PATCH] Add back transaction support for beatmap importing --- osu.Game/Beatmaps/BeatmapManager.cs | 56 +++++++++++-------- osu.Game/Beatmaps/BeatmapStore.cs | 4 +- osu.Game/Database/DatabaseBackedStore.cs | 9 ++- osu.Game/Database/DatabaseContextFactory.cs | 19 +++++++ osu.Game/IO/FileStore.cs | 2 +- osu.Game/Input/KeyBindingStore.cs | 5 +- osu.Game/OsuGameBase.cs | 18 +++--- osu.Game/Rulesets/RulesetStore.cs | 4 +- osu.Game/Rulesets/Scoring/ScoreStore.cs | 3 +- .../Tests/Visual/TestCasePlaySongSelect.cs | 11 ++-- osu.Game/osu.Game.csproj | 1 + 11 files changed, 84 insertions(+), 48 deletions(-) create mode 100644 osu.Game/Database/DatabaseContextFactory.cs diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 7d174e6a28..1423e9138f 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -57,6 +57,18 @@ namespace osu.Game.Beatmaps private readonly Storage storage; + private BeatmapStore createBeatmapStore(Func context) + { + var store = new BeatmapStore(context); + store.BeatmapSetAdded += s => BeatmapSetAdded?.Invoke(s); + store.BeatmapSetRemoved += s => BeatmapSetRemoved?.Invoke(s); + store.BeatmapHidden += b => BeatmapHidden?.Invoke(b); + store.BeatmapRestored += b => BeatmapRestored?.Invoke(b); + return store; + } + + private readonly Func createContext; + private readonly FileStore files; private readonly RulesetStore rulesets; @@ -80,16 +92,13 @@ namespace osu.Game.Beatmaps /// public Func GetStableStorage { private get; set; } - public BeatmapManager(Storage storage, FileStore files, OsuDbContext context, RulesetStore rulesets, APIAccess api, IIpcHost importHost = null) + public BeatmapManager(Storage storage, Func context, RulesetStore rulesets, APIAccess api, IIpcHost importHost = null) { - beatmaps = new BeatmapStore(context); - beatmaps.BeatmapSetAdded += s => BeatmapSetAdded?.Invoke(s); - beatmaps.BeatmapSetRemoved += s => BeatmapSetRemoved?.Invoke(s); - beatmaps.BeatmapHidden += b => BeatmapHidden?.Invoke(b); - beatmaps.BeatmapRestored += b => BeatmapRestored?.Invoke(b); + createContext = context; + beatmaps = createBeatmapStore(context); + files = new FileStore(context, storage); this.storage = storage; - this.files = files; this.rulesets = rulesets; this.api = api; @@ -160,13 +169,24 @@ namespace osu.Game.Beatmaps /// The beatmap to be imported. public BeatmapSetInfo Import(ArchiveReader archiveReader) { - BeatmapSetInfo set; - // let's only allow one concurrent import at a time for now. lock (importLock) - Import(set = importToStorage(archiveReader)); + { + var context = createContext(); - return set; + using (var transaction = context.Database.BeginTransaction()) + { + // create local stores so we can isolate and thread safely, and share a context/transaction. + var filesForImport = new FileStore(() => context, storage); + var beatmapsForImport = createBeatmapStore(() => context); + + BeatmapSetInfo set = importToStorage(filesForImport, archiveReader); + beatmapsForImport.Add(set); + context.SaveChanges(); + transaction.Commit(); + return set; + } + } } /// @@ -178,8 +198,7 @@ namespace osu.Game.Beatmaps // If we have an ID then we already exist in the database. if (beatmapSetInfo.ID != 0) return; - lock (beatmaps) - beatmaps.Add(beatmapSetInfo); + createBeatmapStore(createContext).Add(beatmapSetInfo); } /// @@ -322,15 +341,6 @@ namespace osu.Game.Beatmaps return working; } - /// - /// Reset the manager to an empty state. - /// - public void Reset() - { - lock (beatmaps) - beatmaps.Reset(); - } - /// /// Perform a lookup query on available s. /// @@ -400,7 +410,7 @@ namespace osu.Game.Beatmaps /// /// The beatmap archive to be read. /// The imported beatmap, or an existing instance if it is already present. - private BeatmapSetInfo importToStorage(ArchiveReader reader) + private BeatmapSetInfo importToStorage(FileStore files, ArchiveReader reader) { // let's make sure there are actually .osu files to import. string mapName = reader.Filenames.FirstOrDefault(f => f.EndsWith(".osu")); diff --git a/osu.Game/Beatmaps/BeatmapStore.cs b/osu.Game/Beatmaps/BeatmapStore.cs index 134059fd07..f3d3caeb0f 100644 --- a/osu.Game/Beatmaps/BeatmapStore.cs +++ b/osu.Game/Beatmaps/BeatmapStore.cs @@ -20,8 +20,8 @@ namespace osu.Game.Beatmaps public event Action BeatmapHidden; public event Action BeatmapRestored; - public BeatmapStore(OsuDbContext context) - : base(context) + public BeatmapStore(Func factory) + : base(factory) { } diff --git a/osu.Game/Database/DatabaseBackedStore.cs b/osu.Game/Database/DatabaseBackedStore.cs index cba334d58a..9d3d020250 100644 --- a/osu.Game/Database/DatabaseBackedStore.cs +++ b/osu.Game/Database/DatabaseBackedStore.cs @@ -10,12 +10,15 @@ namespace osu.Game.Database public abstract class DatabaseBackedStore { protected readonly Storage Storage; - protected readonly OsuDbContext Context; - protected DatabaseBackedStore(OsuDbContext context, Storage storage = null) + private readonly Func contextSource; + + protected OsuDbContext Context => contextSource(); + + protected DatabaseBackedStore(Func contextSource, Storage storage = null) { Storage = storage; - Context = context; + this.contextSource = contextSource; try { diff --git a/osu.Game/Database/DatabaseContextFactory.cs b/osu.Game/Database/DatabaseContextFactory.cs new file mode 100644 index 0000000000..3fc5141880 --- /dev/null +++ b/osu.Game/Database/DatabaseContextFactory.cs @@ -0,0 +1,19 @@ +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu-framework/master/LICENCE + +using osu.Framework.Platform; + +namespace osu.Game.Database +{ + public class DatabaseContextFactory + { + private readonly GameHost host; + + public DatabaseContextFactory(GameHost host) + { + this.host = host; + } + + public OsuDbContext GetContext() => new OsuDbContext(host.Storage.GetDatabaseConnectionString(@"client")); + } +} diff --git a/osu.Game/IO/FileStore.cs b/osu.Game/IO/FileStore.cs index ed295c76b2..d715ccd0a7 100644 --- a/osu.Game/IO/FileStore.cs +++ b/osu.Game/IO/FileStore.cs @@ -22,7 +22,7 @@ namespace osu.Game.IO public readonly ResourceStore Store; - public FileStore(OsuDbContext context, Storage storage) : base(context, storage) + public FileStore(Func contextSource, Storage storage) : base(contextSource, storage) { Store = new NamespacedResourceStore(new StorageBackedResourceStore(storage), prefix); } diff --git a/osu.Game/Input/KeyBindingStore.cs b/osu.Game/Input/KeyBindingStore.cs index 2d0aabdd7f..5c41179418 100644 --- a/osu.Game/Input/KeyBindingStore.cs +++ b/osu.Game/Input/KeyBindingStore.cs @@ -1,6 +1,7 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE +using System; using System.Collections.Generic; using System.Linq; using Microsoft.EntityFrameworkCore; @@ -14,8 +15,8 @@ namespace osu.Game.Input { public class KeyBindingStore : DatabaseBackedStore { - public KeyBindingStore(OsuDbContext context, RulesetStore rulesets, Storage storage = null) - : base(context, storage) + public KeyBindingStore(Func contextSource, RulesetStore rulesets, Storage storage = null) + : base(contextSource, storage) { foreach (var info in rulesets.AvailableRulesets) { diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index d1e684499c..5ecc7279da 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -81,16 +81,18 @@ namespace osu.Game protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(base.CreateLocalDependencies(parent)); - private OsuDbContext createDbContext() => new OsuDbContext(Host.Storage.GetDatabaseConnectionString(@"client")); + private DatabaseContextFactory contextFactory; [BackgroundDependencyLoader] private void load() { + dependencies.Cache(contextFactory = new DatabaseContextFactory(Host)); + dependencies.Cache(this); dependencies.Cache(LocalConfig); - using (var dbContext = createDbContext()) - dbContext.Database.Migrate(); + using (var context = contextFactory.GetContext()) + context.Database.Migrate(); dependencies.Cache(API = new APIAccess { @@ -98,11 +100,11 @@ namespace osu.Game Token = LocalConfig.Get(OsuSetting.Token) }); - dependencies.Cache(RulesetStore = new RulesetStore(createDbContext())); - dependencies.Cache(FileStore = new FileStore(createDbContext(), Host.Storage)); - dependencies.Cache(BeatmapManager = new BeatmapManager(Host.Storage, FileStore, createDbContext(), RulesetStore, API, Host)); - dependencies.Cache(ScoreStore = new ScoreStore(Host.Storage, createDbContext(), Host, BeatmapManager, RulesetStore)); - dependencies.Cache(KeyBindingStore = new KeyBindingStore(createDbContext(), RulesetStore)); + dependencies.Cache(RulesetStore = new RulesetStore(contextFactory.GetContext)); + dependencies.Cache(FileStore = new FileStore(contextFactory.GetContext, Host.Storage)); + dependencies.Cache(BeatmapManager = new BeatmapManager(Host.Storage, contextFactory.GetContext, RulesetStore, API, Host)); + dependencies.Cache(ScoreStore = new ScoreStore(Host.Storage, contextFactory.GetContext, Host, BeatmapManager, RulesetStore)); + dependencies.Cache(KeyBindingStore = new KeyBindingStore(contextFactory.GetContext, RulesetStore)); dependencies.Cache(new OsuColour()); //this completely overrides the framework default. will need to change once we make a proper FontStore. diff --git a/osu.Game/Rulesets/RulesetStore.cs b/osu.Game/Rulesets/RulesetStore.cs index 82252b76fa..bd3c22fc42 100644 --- a/osu.Game/Rulesets/RulesetStore.cs +++ b/osu.Game/Rulesets/RulesetStore.cs @@ -26,8 +26,8 @@ namespace osu.Game.Rulesets loadRulesetFromFile(file); } - public RulesetStore(OsuDbContext context) - : base(context) + public RulesetStore(Func factory) + : base(factory) { } diff --git a/osu.Game/Rulesets/Scoring/ScoreStore.cs b/osu.Game/Rulesets/Scoring/ScoreStore.cs index 02dd5c40ac..67a8e5372e 100644 --- a/osu.Game/Rulesets/Scoring/ScoreStore.cs +++ b/osu.Game/Rulesets/Scoring/ScoreStore.cs @@ -1,6 +1,7 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE +using System; using System.Collections.Generic; using System.IO; using osu.Framework.Platform; @@ -25,7 +26,7 @@ namespace osu.Game.Rulesets.Scoring // ReSharper disable once NotAccessedField.Local (we should keep a reference to this so it is not finalised) private ScoreIPCChannel ipc; - public ScoreStore(Storage storage, OsuDbContext context, IIpcHost importHost = null, BeatmapManager beatmaps = null, RulesetStore rulesets = null) : base(context) + public ScoreStore(Storage storage, Func factory, IIpcHost importHost = null, BeatmapManager beatmaps = null, RulesetStore rulesets = null) : base(factory) { this.storage = storage; this.beatmaps = beatmaps; diff --git a/osu.Game/Tests/Visual/TestCasePlaySongSelect.cs b/osu.Game/Tests/Visual/TestCasePlaySongSelect.cs index a8a00e9a0d..9ca6aa2fb5 100644 --- a/osu.Game/Tests/Visual/TestCasePlaySongSelect.cs +++ b/osu.Game/Tests/Visual/TestCasePlaySongSelect.cs @@ -1,13 +1,13 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE +using System; using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; using osu.Framework.MathUtils; using osu.Game.Beatmaps; using osu.Game.Database; -using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Filter; @@ -25,8 +25,6 @@ namespace osu.Game.Tests.Visual private DependencyContainer dependencies; - private FileStore files; - protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(parent); [BackgroundDependencyLoader] @@ -40,9 +38,10 @@ namespace osu.Game.Tests.Visual var dbConnectionString = storage.GetDatabaseConnectionString(@"client"); - dependencies.Cache(rulesets = new RulesetStore(new OsuDbContext(dbConnectionString))); - dependencies.Cache(files = new FileStore(new OsuDbContext(dbConnectionString), storage)); - dependencies.Cache(manager = new BeatmapManager(storage, files, new OsuDbContext(dbConnectionString), rulesets, null)); + Func contextFactory = () => new OsuDbContext(dbConnectionString); + + dependencies.Cache(rulesets = new RulesetStore(contextFactory)); + dependencies.Cache(manager = new BeatmapManager(storage, contextFactory, rulesets, null)); for (int i = 0; i < 100; i += 10) manager.Import(createTestBeatmapSet(i)); diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 731e065a48..d3ac2d6189 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -285,6 +285,7 @@ + 20171014052545_Init.cs