From 754afb9c0bbd5d049352c455ec1122de69a9a9fa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 6 May 2020 18:31:17 +0900 Subject: [PATCH 01/13] Expose ContextFactory to allow for connection flushing --- osu.Game/OsuGameBase.cs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index d9f9e2de42..d0c06df8ea 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -121,7 +121,7 @@ namespace osu.Game protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); - private DatabaseContextFactory contextFactory; + protected DatabaseContextFactory ContextFactory; protected override UserInputManager CreateUserInputManager() => new OsuUserInputManager(); @@ -130,7 +130,7 @@ namespace osu.Game { Resources.AddStore(new DllResourceStore(OsuResources.ResourceAssembly)); - dependencies.Cache(contextFactory = new DatabaseContextFactory(Storage)); + dependencies.Cache(ContextFactory = new DatabaseContextFactory(Storage)); dependencies.CacheAs(Storage); @@ -161,7 +161,7 @@ namespace osu.Game runMigrations(); - dependencies.Cache(SkinManager = new SkinManager(Storage, contextFactory, Host, Audio, new NamespacedResourceStore(Resources, "Skins/Legacy"))); + dependencies.Cache(SkinManager = new SkinManager(Storage, ContextFactory, Host, Audio, new NamespacedResourceStore(Resources, "Skins/Legacy"))); dependencies.CacheAs(SkinManager); if (API == null) API = new APIAccess(LocalConfig); @@ -170,12 +170,12 @@ namespace osu.Game var defaultBeatmap = new DummyWorkingBeatmap(Audio, Textures); - dependencies.Cache(RulesetStore = new RulesetStore(contextFactory, Storage)); - dependencies.Cache(FileStore = new FileStore(contextFactory, Storage)); + dependencies.Cache(RulesetStore = new RulesetStore(ContextFactory, Storage)); + dependencies.Cache(FileStore = new FileStore(ContextFactory, Storage)); // ordering is important here to ensure foreign keys rules are not broken in ModelStore.Cleanup() - dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, contextFactory, Host)); - dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, contextFactory, RulesetStore, API, Audio, Host, defaultBeatmap)); + dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, ContextFactory, Host)); + dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, ContextFactory, RulesetStore, API, Audio, Host, defaultBeatmap)); // this should likely be moved to ArchiveModelManager when another case appers where it is necessary // to have inter-dependent model managers. this could be obtained with an IHasForeign interface to @@ -189,8 +189,8 @@ namespace osu.Game BeatmapManager.ItemRemoved += i => ScoreManager.Delete(getBeatmapScores(i), true); BeatmapManager.ItemAdded += i => ScoreManager.Undelete(getBeatmapScores(i), true); - dependencies.Cache(KeyBindingStore = new KeyBindingStore(contextFactory, RulesetStore)); - dependencies.Cache(SettingsStore = new SettingsStore(contextFactory)); + dependencies.Cache(KeyBindingStore = new KeyBindingStore(ContextFactory, RulesetStore)); + dependencies.Cache(SettingsStore = new SettingsStore(ContextFactory)); dependencies.Cache(RulesetConfigCache = new RulesetConfigCache(SettingsStore)); dependencies.Cache(new SessionStatics()); dependencies.Cache(new OsuColour()); @@ -279,7 +279,7 @@ namespace osu.Game { try { - using (var db = contextFactory.GetForWrite(false)) + using (var db = ContextFactory.GetForWrite(false)) db.Context.Migrate(); } catch (Exception e) @@ -288,12 +288,12 @@ namespace osu.Game // if we failed, let's delete the database and start fresh. // todo: we probably want a better (non-destructive) migrations/recovery process at a later point than this. - contextFactory.ResetDatabase(); + ContextFactory.ResetDatabase(); Logger.Log("Database purged successfully.", LoggingTarget.Database); // only run once more, then hard bail. - using (var db = contextFactory.GetForWrite(false)) + using (var db = ContextFactory.GetForWrite(false)) db.Context.Migrate(); } } From 49a03f1c06ca6824fc8e131ddd1e8f845c4ea1f2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 6 May 2020 18:31:36 +0900 Subject: [PATCH 02/13] Add basic blocking migration, move not copy --- osu.Game/IO/OsuStorage.cs | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index ee42c491d1..f6cac2f4f1 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; +using System.IO; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Configuration; @@ -9,10 +11,15 @@ namespace osu.Game.IO { public class OsuStorage : WrappedStorage { + private readonly GameHost host; + private readonly StorageConfigManager storageConfig; + public OsuStorage(GameHost host) : base(host.Storage, string.Empty) { - var storageConfig = new StorageConfigManager(host.Storage); + this.host = host; + + storageConfig = new StorageConfigManager(host.Storage); var customStoragePath = storageConfig.Get(StorageConfig.FullPath); @@ -22,5 +29,34 @@ namespace osu.Game.IO Logger.Storage = UnderlyingStorage.GetStorageForDirectory("logs"); } } + + public void Migrate(string newLocation) + { + string oldLocation = GetFullPath("."); + + // ensure the new location has no files present, else hard abort + if (Directory.Exists(newLocation)) + { + if (Directory.GetFiles(newLocation).Length > 0) + throw new InvalidOperationException("Migration destination already has files present"); + + Directory.Delete(newLocation, true); + } + + Directory.Move(oldLocation, newLocation); + + Directory.CreateDirectory(newLocation); + // temporary + Directory.CreateDirectory(oldLocation); + + // move back exceptions for now + Directory.Move(Path.Combine(newLocation, "cache"), Path.Combine(oldLocation, "cache")); + File.Move(Path.Combine(newLocation, "framework.ini"), Path.Combine(oldLocation, "framework.ini")); + + ChangeTargetStorage(host.GetStorage(newLocation)); + + storageConfig.Set(StorageConfig.FullPath, newLocation); + storageConfig.Save(); + } } } From 7a2020fd4561e461ef7700a5178d7b05a8c23b6b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 7 May 2020 19:00:59 +0900 Subject: [PATCH 03/13] User copy operation instead of move --- osu.Game/IO/OsuStorage.cs | 63 ++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index f6cac2f4f1..955aae7b68 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Linq; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Configuration; @@ -14,6 +15,14 @@ namespace osu.Game.IO private readonly GameHost host; private readonly StorageConfigManager storageConfig; + internal static readonly string[] IGNORE_DIRECTORIES = { "cache" }; + + internal static readonly string[] IGNORE_FILES = + { + "framework.ini", + "storage.ini" + }; + public OsuStorage(GameHost host) : base(host.Storage, string.Empty) { @@ -43,20 +52,58 @@ namespace osu.Game.IO Directory.Delete(newLocation, true); } - Directory.Move(oldLocation, newLocation); + var source = new DirectoryInfo(oldLocation); + var destination = new DirectoryInfo(newLocation); - Directory.CreateDirectory(newLocation); - // temporary - Directory.CreateDirectory(oldLocation); - - // move back exceptions for now - Directory.Move(Path.Combine(newLocation, "cache"), Path.Combine(oldLocation, "cache")); - File.Move(Path.Combine(newLocation, "framework.ini"), Path.Combine(oldLocation, "framework.ini")); + copyRecursive(source, destination); ChangeTargetStorage(host.GetStorage(newLocation)); storageConfig.Set(StorageConfig.FullPath, newLocation); storageConfig.Save(); + + deleteRecursive(source); + } + + private static void deleteRecursive(DirectoryInfo target, bool topLevelExcludes = true) + { + foreach (System.IO.FileInfo fi in target.GetFiles()) + { + if (IGNORE_FILES.Contains(fi.Name)) + continue; + + fi.Delete(); + } + + foreach (DirectoryInfo dir in target.GetDirectories()) + { + if (IGNORE_DIRECTORIES.Contains(dir.Name)) + continue; + + dir.Delete(true); + } + } + + private static void copyRecursive(DirectoryInfo source, DirectoryInfo destination, bool topLevelExcludes = true) + { + // based off example code https://docs.microsoft.com/en-us/dotnet/api/system.io.directoryinfo + Directory.CreateDirectory(destination.FullName); + + foreach (System.IO.FileInfo fi in source.GetFiles()) + { + if (IGNORE_FILES.Contains(fi.Name)) + continue; + + fi.CopyTo(Path.Combine(destination.FullName, fi.Name), true); + } + + foreach (DirectoryInfo dir in source.GetDirectories()) + { + if (IGNORE_DIRECTORIES.Contains(dir.Name)) + continue; + + copyRecursive(dir, destination.CreateSubdirectory(dir.Name), false); + } } } } From 5e65bda8d8ba49b5213fcf163861bdee4ebddb6c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 7 May 2020 19:01:19 +0900 Subject: [PATCH 04/13] Add test coverage --- .../NonVisual/CustomDataDirectoryTest.cs | 60 ++++++++++++++++++- 1 file changed, 58 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs index d741bc5de1..7f08fad5be 100644 --- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs +++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs @@ -7,14 +7,23 @@ using System.Threading; using System.Threading.Tasks; using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Configuration; using osu.Framework.Platform; using osu.Game.Configuration; +using osu.Game.IO; namespace osu.Game.Tests.NonVisual { [TestFixture] public class CustomDataDirectoryTest { + [SetUp] + public void SetUp() + { + if (Directory.Exists(customPath)) + Directory.Delete(customPath, true); + } + [Test] public void TestDefaultDirectory() { @@ -36,6 +45,8 @@ namespace osu.Game.Tests.NonVisual } } + private string customPath => Path.Combine(Environment.CurrentDirectory, "custom-path"); + [Test] public void TestCustomDirectory() { @@ -49,7 +60,7 @@ namespace osu.Game.Tests.NonVisual storage.DeleteDirectory(string.Empty); using (var storageConfig = new StorageConfigManager(storage)) - storageConfig.Set(StorageConfig.FullPath, Path.Combine(Environment.CurrentDirectory, "custom-path")); + storageConfig.Set(StorageConfig.FullPath, customPath); try { @@ -58,7 +69,52 @@ namespace osu.Game.Tests.NonVisual // switch to DI'd storage storage = osu.Dependencies.Get(); - Assert.That(storage.GetFullPath("."), Is.EqualTo(Path.Combine(Environment.CurrentDirectory, "custom-path"))); + Assert.That(storage.GetFullPath("."), Is.EqualTo(customPath)); + } + finally + { + host.Exit(); + } + } + } + + [Test] + public void TestMigration() + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestMigration))) + { + try + { + var osu = loadOsu(host); + var storage = osu.Dependencies.Get(); + + // ensure we perform a save + host.Dependencies.Get().Save(); + + // ensure we "use" cache + host.Storage.GetStorageForDirectory("cache"); + + string defaultStorageLocation = Path.Combine(Environment.CurrentDirectory, "headless", nameof(TestMigration)); + + Assert.That(storage.GetFullPath("."), Is.EqualTo(defaultStorageLocation)); + + (storage as OsuStorage)?.Migrate(customPath); + + Assert.That(storage.GetFullPath("."), Is.EqualTo(customPath)); + + foreach (var file in OsuStorage.IGNORE_FILES) + { + Assert.That(host.Storage.Exists(file), Is.True); + Assert.That(storage.Exists(file), Is.False); + } + + foreach (var dir in OsuStorage.IGNORE_DIRECTORIES) + { + Assert.That(host.Storage.ExistsDirectory(dir), Is.True); + Assert.That(storage.ExistsDirectory(dir), Is.False); + } + + Assert.That(new StreamReader(host.Storage.GetStream("storage.ini")).ReadToEnd().Contains($"FullPath = {customPath}")); } finally { From bbebd26efb46b2091a53a966c08faca896b20e15 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 9 May 2020 20:13:31 +0900 Subject: [PATCH 05/13] Use DirectoryInfo in more places --- osu.Game/IO/OsuStorage.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index 955aae7b68..e178cb0a02 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -41,20 +41,18 @@ namespace osu.Game.IO public void Migrate(string newLocation) { - string oldLocation = GetFullPath("."); + var source = new DirectoryInfo(GetFullPath(".")); + var destination = new DirectoryInfo(newLocation); // ensure the new location has no files present, else hard abort - if (Directory.Exists(newLocation)) + if (destination.Exists) { - if (Directory.GetFiles(newLocation).Length > 0) + if (destination.GetFiles().Length > 0) throw new InvalidOperationException("Migration destination already has files present"); - Directory.Delete(newLocation, true); + deleteRecursive(destination); } - var source = new DirectoryInfo(oldLocation); - var destination = new DirectoryInfo(newLocation); - copyRecursive(source, destination); ChangeTargetStorage(host.GetStorage(newLocation)); From 5dda94187e54d3980507872f69947330a5bfc62d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 9 May 2020 20:13:37 +0900 Subject: [PATCH 06/13] Add more edge case testing --- .../NonVisual/CustomDataDirectoryTest.cs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs index 7f08fad5be..8688ecd078 100644 --- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs +++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs @@ -123,6 +123,56 @@ namespace osu.Game.Tests.NonVisual } } + [Test] + public void TestMigrationBetweenTwoTargets() + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestMigrationBetweenTwoTargets))) + { + try + { + var osu = loadOsu(host); + var storage = osu.Dependencies.Get(); + + string customPath2 = $"{customPath}-2"; + + const string database_filename = "client.db"; + + Assert.DoesNotThrow(() => (storage as OsuStorage)?.Migrate(customPath)); + Assert.That(File.Exists(Path.Combine(customPath, database_filename))); + + Assert.DoesNotThrow(() => (storage as OsuStorage)?.Migrate(customPath2)); + Assert.That(File.Exists(Path.Combine(customPath2, database_filename))); + + Assert.DoesNotThrow(() => (storage as OsuStorage)?.Migrate(customPath)); + Assert.That(File.Exists(Path.Combine(customPath, database_filename))); + } + finally + { + host.Exit(); + } + } + } + + [Test] + public void TestMigrationToSameTargetFails() + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestMigrationToSameTargetFails))) + { + try + { + var osu = loadOsu(host); + var storage = osu.Dependencies.Get(); + + Assert.DoesNotThrow(() => (storage as OsuStorage)?.Migrate(customPath)); + Assert.Throws(() => (storage as OsuStorage)?.Migrate(customPath)); + } + finally + { + host.Exit(); + } + } + } + private OsuGameBase loadOsu(GameHost host) { var osu = new OsuGameBase(); From 6c350db0973c74f0294cc975c2c2b3aa26bf17a1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 May 2020 21:37:07 +0900 Subject: [PATCH 07/13] Add connection flushing support --- .../NonVisual/CustomDataDirectoryTest.cs | 14 ++++++-------- osu.Game/Database/DatabaseContextFactory.cs | 8 ++++++++ osu.Game/OsuGameBase.cs | 8 ++++++++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs index 4bce5056ce..ed83ff358c 100644 --- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs +++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs @@ -137,7 +137,7 @@ namespace osu.Game.Tests.NonVisual Assert.That(storage.GetFullPath("."), Is.EqualTo(defaultStorageLocation)); - (storage as OsuStorage)?.Migrate(customPath); + osu.Migrate(customPath); Assert.That(storage.GetFullPath("."), Is.EqualTo(customPath)); @@ -170,19 +170,18 @@ namespace osu.Game.Tests.NonVisual try { var osu = loadOsu(host); - var storage = osu.Dependencies.Get(); string customPath2 = $"{customPath}-2"; const string database_filename = "client.db"; - Assert.DoesNotThrow(() => (storage as OsuStorage)?.Migrate(customPath)); + Assert.DoesNotThrow(() => osu.Migrate(customPath)); Assert.That(File.Exists(Path.Combine(customPath, database_filename))); - Assert.DoesNotThrow(() => (storage as OsuStorage)?.Migrate(customPath2)); + Assert.DoesNotThrow(() => osu.Migrate(customPath2)); Assert.That(File.Exists(Path.Combine(customPath2, database_filename))); - Assert.DoesNotThrow(() => (storage as OsuStorage)?.Migrate(customPath)); + Assert.DoesNotThrow(() => osu.Migrate(customPath)); Assert.That(File.Exists(Path.Combine(customPath, database_filename))); } finally @@ -200,10 +199,9 @@ namespace osu.Game.Tests.NonVisual try { var osu = loadOsu(host); - var storage = osu.Dependencies.Get(); - Assert.DoesNotThrow(() => (storage as OsuStorage)?.Migrate(customPath)); - Assert.Throws(() => (storage as OsuStorage)?.Migrate(customPath)); + Assert.DoesNotThrow(() => osu.Migrate(customPath)); + Assert.Throws(() => osu.Migrate(customPath)); } finally { diff --git a/osu.Game/Database/DatabaseContextFactory.cs b/osu.Game/Database/DatabaseContextFactory.cs index 1ed5fb3268..1cceb59b11 100644 --- a/osu.Game/Database/DatabaseContextFactory.cs +++ b/osu.Game/Database/DatabaseContextFactory.cs @@ -160,5 +160,13 @@ namespace osu.Game.Database } } } + + public void FlushConnections() + { + foreach (var context in threadContexts.Values) + context.Dispose(); + + recycleThreadContexts(); + } } } diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 8fbde67afe..6282f5cb8b 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -328,6 +328,8 @@ namespace osu.Game { base.Dispose(isDisposing); RulesetStore?.Dispose(); + + ContextFactory.FlushConnections(); } private class OsuUserInputManager : UserInputManager @@ -355,5 +357,11 @@ namespace osu.Game public override bool ChangeFocusOnClick => false; } } + + public void Migrate(string path) + { + ContextFactory.FlushConnections(); + (Storage as OsuStorage)?.Migrate(path); + } } } From a11be07bb122b3e1c186cfc418d31a48a1f8869d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 May 2020 21:38:27 +0900 Subject: [PATCH 08/13] Move log storage location after migration complete --- osu.Game/IO/OsuStorage.cs | 9 ++++++--- osu.Game/IO/WrappedStorage.cs | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index e178cb0a02..7e1c676324 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -33,10 +33,13 @@ namespace osu.Game.IO var customStoragePath = storageConfig.Get(StorageConfig.FullPath); if (!string.IsNullOrEmpty(customStoragePath)) - { ChangeTargetStorage(host.GetStorage(customStoragePath)); - Logger.Storage = UnderlyingStorage.GetStorageForDirectory("logs"); - } + } + + protected override void ChangeTargetStorage(Storage newStorage) + { + base.ChangeTargetStorage(newStorage); + Logger.Storage = UnderlyingStorage.GetStorageForDirectory("logs"); } public void Migrate(string newLocation) diff --git a/osu.Game/IO/WrappedStorage.cs b/osu.Game/IO/WrappedStorage.cs index cc59e2cc28..646faba9eb 100644 --- a/osu.Game/IO/WrappedStorage.cs +++ b/osu.Game/IO/WrappedStorage.cs @@ -27,7 +27,7 @@ namespace osu.Game.IO protected virtual string MutatePath(string path) => !string.IsNullOrEmpty(subPath) ? Path.Combine(subPath, path) : path; - protected void ChangeTargetStorage(Storage newStorage) + protected virtual void ChangeTargetStorage(Storage newStorage) { UnderlyingStorage = newStorage; } From 984f27c107ed817ee677a4642e995c594b8805db Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 May 2020 21:38:41 +0900 Subject: [PATCH 09/13] Add simple retry logic on file copy failure (may be in use) --- osu.Game/IO/OsuStorage.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index 7e1c676324..7c0b90e208 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -4,6 +4,7 @@ using System; using System.IO; using System.Linq; +using System.Threading; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Configuration; @@ -95,7 +96,20 @@ namespace osu.Game.IO if (IGNORE_FILES.Contains(fi.Name)) continue; - fi.CopyTo(Path.Combine(destination.FullName, fi.Name), true); + int tries = 5; + + while (tries-- > 0) + { + try + { + fi.CopyTo(Path.Combine(destination.FullName, fi.Name), true); + break; + } + catch (Exception) + { + Thread.Sleep(50); + } + } } foreach (DirectoryInfo dir in source.GetDirectories()) From e4a23b3e7d7eb065578f48c2d53e7981e37c6a59 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 12 May 2020 12:39:04 +0900 Subject: [PATCH 10/13] Fix ignored excluding more than top level --- osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs | 10 ++++++++++ osu.Game/IO/OsuStorage.cs | 8 ++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs index ed83ff358c..ef2b20de64 100644 --- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs +++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs @@ -133,6 +133,9 @@ namespace osu.Game.Tests.NonVisual // ensure we "use" cache host.Storage.GetStorageForDirectory("cache"); + // for testing nested files are not ignored (only top level) + host.Storage.GetStorageForDirectory("test-nested").GetStorageForDirectory("cache"); + string defaultStorageLocation = Path.Combine(Environment.CurrentDirectory, "headless", nameof(TestMigration)); Assert.That(storage.GetFullPath("."), Is.EqualTo(defaultStorageLocation)); @@ -141,6 +144,13 @@ namespace osu.Game.Tests.NonVisual Assert.That(storage.GetFullPath("."), Is.EqualTo(customPath)); + // ensure cache was not moved + Assert.That(host.Storage.ExistsDirectory("cache")); + + // ensure nested cache was moved + Assert.That(!host.Storage.ExistsDirectory(Path.Combine("test-nested", "cache"))); + Assert.That(storage.ExistsDirectory(Path.Combine("test-nested", "cache"))); + foreach (var file in OsuStorage.IGNORE_FILES) { Assert.That(host.Storage.Exists(file), Is.True); diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index 7c0b90e208..6dc25e871c 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -71,7 +71,7 @@ namespace osu.Game.IO { foreach (System.IO.FileInfo fi in target.GetFiles()) { - if (IGNORE_FILES.Contains(fi.Name)) + if (topLevelExcludes && IGNORE_FILES.Contains(fi.Name)) continue; fi.Delete(); @@ -79,7 +79,7 @@ namespace osu.Game.IO foreach (DirectoryInfo dir in target.GetDirectories()) { - if (IGNORE_DIRECTORIES.Contains(dir.Name)) + if (topLevelExcludes && IGNORE_DIRECTORIES.Contains(dir.Name)) continue; dir.Delete(true); @@ -93,7 +93,7 @@ namespace osu.Game.IO foreach (System.IO.FileInfo fi in source.GetFiles()) { - if (IGNORE_FILES.Contains(fi.Name)) + if (topLevelExcludes && IGNORE_FILES.Contains(fi.Name)) continue; int tries = 5; @@ -114,7 +114,7 @@ namespace osu.Game.IO foreach (DirectoryInfo dir in source.GetDirectories()) { - if (IGNORE_DIRECTORIES.Contains(dir.Name)) + if (topLevelExcludes && IGNORE_DIRECTORIES.Contains(dir.Name)) continue; copyRecursive(dir, destination.CreateSubdirectory(dir.Name), false); From 75a40578e85690ae62bf089ae77ad06519b4ef08 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 12 May 2020 12:39:52 +0900 Subject: [PATCH 11/13] Revert ContextFactory to private --- osu.Game/OsuGameBase.cs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 6282f5cb8b..11a3834c71 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -121,7 +121,7 @@ namespace osu.Game protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); - protected DatabaseContextFactory ContextFactory; + private DatabaseContextFactory contextFactory; protected override UserInputManager CreateUserInputManager() => new OsuUserInputManager(); @@ -130,7 +130,7 @@ namespace osu.Game { Resources.AddStore(new DllResourceStore(OsuResources.ResourceAssembly)); - dependencies.Cache(ContextFactory = new DatabaseContextFactory(Storage)); + dependencies.Cache(contextFactory = new DatabaseContextFactory(Storage)); dependencies.CacheAs(Storage); @@ -161,7 +161,7 @@ namespace osu.Game runMigrations(); - dependencies.Cache(SkinManager = new SkinManager(Storage, ContextFactory, Host, Audio, new NamespacedResourceStore(Resources, "Skins/Legacy"))); + dependencies.Cache(SkinManager = new SkinManager(Storage, contextFactory, Host, Audio, new NamespacedResourceStore(Resources, "Skins/Legacy"))); dependencies.CacheAs(SkinManager); if (API == null) API = new APIAccess(LocalConfig); @@ -170,12 +170,12 @@ namespace osu.Game var defaultBeatmap = new DummyWorkingBeatmap(Audio, Textures); - dependencies.Cache(RulesetStore = new RulesetStore(ContextFactory, Storage)); - dependencies.Cache(FileStore = new FileStore(ContextFactory, Storage)); + dependencies.Cache(RulesetStore = new RulesetStore(contextFactory, Storage)); + dependencies.Cache(FileStore = new FileStore(contextFactory, Storage)); // ordering is important here to ensure foreign keys rules are not broken in ModelStore.Cleanup() - dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, ContextFactory, Host)); - dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, ContextFactory, RulesetStore, API, Audio, Host, defaultBeatmap)); + dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, contextFactory, Host)); + dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, contextFactory, RulesetStore, API, Audio, Host, defaultBeatmap)); // this should likely be moved to ArchiveModelManager when another case appers where it is necessary // to have inter-dependent model managers. this could be obtained with an IHasForeign interface to @@ -189,8 +189,8 @@ namespace osu.Game BeatmapManager.ItemRemoved += i => ScoreManager.Delete(getBeatmapScores(i), true); BeatmapManager.ItemAdded += i => ScoreManager.Undelete(getBeatmapScores(i), true); - dependencies.Cache(KeyBindingStore = new KeyBindingStore(ContextFactory, RulesetStore)); - dependencies.Cache(SettingsStore = new SettingsStore(ContextFactory)); + dependencies.Cache(KeyBindingStore = new KeyBindingStore(contextFactory, RulesetStore)); + dependencies.Cache(SettingsStore = new SettingsStore(contextFactory)); dependencies.Cache(RulesetConfigCache = new RulesetConfigCache(SettingsStore)); dependencies.Cache(new SessionStatics()); dependencies.Cache(new OsuColour()); @@ -279,7 +279,7 @@ namespace osu.Game { try { - using (var db = ContextFactory.GetForWrite(false)) + using (var db = contextFactory.GetForWrite(false)) db.Context.Migrate(); } catch (Exception e) @@ -288,12 +288,12 @@ namespace osu.Game // if we failed, let's delete the database and start fresh. // todo: we probably want a better (non-destructive) migrations/recovery process at a later point than this. - ContextFactory.ResetDatabase(); + contextFactory.ResetDatabase(); Logger.Log("Database purged successfully.", LoggingTarget.Database); // only run once more, then hard bail. - using (var db = ContextFactory.GetForWrite(false)) + using (var db = contextFactory.GetForWrite(false)) db.Context.Migrate(); } } @@ -329,7 +329,7 @@ namespace osu.Game base.Dispose(isDisposing); RulesetStore?.Dispose(); - ContextFactory.FlushConnections(); + contextFactory.FlushConnections(); } private class OsuUserInputManager : UserInputManager @@ -360,7 +360,7 @@ namespace osu.Game public void Migrate(string path) { - ContextFactory.FlushConnections(); + contextFactory.FlushConnections(); (Storage as OsuStorage)?.Migrate(path); } } From 49e616b7e561ab940a2fa10f294af179f2b39e91 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 May 2020 20:19:14 +0900 Subject: [PATCH 12/13] Also check for directory presence before migrating --- osu.Game/IO/OsuStorage.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index 6dc25e871c..b060add03b 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -51,7 +51,7 @@ namespace osu.Game.IO // ensure the new location has no files present, else hard abort if (destination.Exists) { - if (destination.GetFiles().Length > 0) + if (destination.GetFiles().Length > 0 || destination.GetDirectories().Length > 0) throw new InvalidOperationException("Migration destination already has files present"); deleteRecursive(destination); From ad1d050fb437673f35380f8751c7a3f7cc68c7e9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 May 2020 20:29:15 +0900 Subject: [PATCH 13/13] Throw exception on copy timeout --- osu.Game/IO/OsuStorage.cs | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index b060add03b..71b01ce479 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -96,20 +96,7 @@ namespace osu.Game.IO if (topLevelExcludes && IGNORE_FILES.Contains(fi.Name)) continue; - int tries = 5; - - while (tries-- > 0) - { - try - { - fi.CopyTo(Path.Combine(destination.FullName, fi.Name), true); - break; - } - catch (Exception) - { - Thread.Sleep(50); - } - } + attemptCopy(fi, Path.Combine(destination.FullName, fi.Name)); } foreach (DirectoryInfo dir in source.GetDirectories()) @@ -120,5 +107,26 @@ namespace osu.Game.IO copyRecursive(dir, destination.CreateSubdirectory(dir.Name), false); } } + + private static void attemptCopy(System.IO.FileInfo fileInfo, string destination) + { + int tries = 5; + + while (true) + { + try + { + fileInfo.CopyTo(destination, true); + return; + } + catch (Exception) + { + if (tries-- == 0) + throw; + } + + Thread.Sleep(50); + } + } } }