From 404497fa101504da2488331e6160346fda145caf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 31 Jul 2017 21:48:03 +0900 Subject: [PATCH 1/6] Allow a single beatmap to reference the same file multiple times This fixes incorrect reference counts causing database desync. --- osu.Game/Beatmaps/BeatmapManager.cs | 16 +++++++---- osu.Game/Beatmaps/BeatmapSetFileInfo.cs | 14 +++++++-- osu.Game/Beatmaps/BeatmapSetInfo.cs | 5 ++-- osu.Game/Database/DatabaseBackedStore.cs | 1 - osu.Game/IO/FileInfo.cs | 2 -- osu.Game/IO/FileStore.cs | 36 +++++++++++++++--------- 6 files changed, 46 insertions(+), 28 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 99117afe35..15225fe821 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -19,7 +19,6 @@ using osu.Game.IO; using osu.Game.IPC; using osu.Game.Rulesets; using SQLite.Net; -using FileInfo = osu.Game.IO.FileInfo; namespace osu.Game.Beatmaps { @@ -135,7 +134,7 @@ namespace osu.Game.Beatmaps if (!beatmaps.Delete(beatmapSet)) return; if (!beatmapSet.Protected) - files.Dereference(beatmapSet.Files); + files.Dereference(beatmapSet.Files.Select(f => f.FileInfo)); } /// @@ -147,7 +146,8 @@ namespace osu.Game.Beatmaps { if (!beatmaps.Undelete(beatmapSet)) return; - files.Reference(beatmapSet.Files); + if (!beatmapSet.Protected) + files.Reference(beatmapSet.Files.Select(f => f.FileInfo)); } /// @@ -265,12 +265,16 @@ namespace osu.Game.Beatmaps return beatmapSet; } - List fileInfos = new List(); + List fileInfos = new List(); // import files to manager foreach (string file in reader.Filenames) using (Stream s = reader.GetStream(file)) - fileInfos.Add(files.Add(s, file)); + fileInfos.Add(new BeatmapSetFileInfo + { + Filename = file, + FileInfo = files.Add(s) + }); BeatmapMetadata metadata; @@ -366,7 +370,7 @@ namespace osu.Game.Beatmaps catch { return null; } } - private string getPathForFile(string filename) => BeatmapSetInfo.Files.First(f => f.Filename == filename).StoragePath; + private string getPathForFile(string filename) => BeatmapSetInfo.Files.First(f => f.Filename == filename).FileInfo.StoragePath; protected override Texture GetBackground() { diff --git a/osu.Game/Beatmaps/BeatmapSetFileInfo.cs b/osu.Game/Beatmaps/BeatmapSetFileInfo.cs index d18b1e833b..a05362b32d 100644 --- a/osu.Game/Beatmaps/BeatmapSetFileInfo.cs +++ b/osu.Game/Beatmaps/BeatmapSetFileInfo.cs @@ -2,16 +2,26 @@ // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE using osu.Game.IO; +using SQLite.Net.Attributes; using SQLiteNetExtensions.Attributes; namespace osu.Game.Beatmaps { public class BeatmapSetFileInfo { - [ForeignKey(typeof(BeatmapSetInfo))] + [PrimaryKey, AutoIncrement] + public int ID { get; set; } + + [ForeignKey(typeof(BeatmapSetInfo)), NotNull] public int BeatmapSetInfoID { get; set; } - [ForeignKey(typeof(FileInfo))] + [ForeignKey(typeof(FileInfo)), NotNull] public int FileInfoID { get; set; } + + [OneToOne(CascadeOperations = CascadeOperation.CascadeRead)] + public FileInfo FileInfo { get; set; } + + [NotNull] + public string Filename { get; set; } } } \ No newline at end of file diff --git a/osu.Game/Beatmaps/BeatmapSetInfo.cs b/osu.Game/Beatmaps/BeatmapSetInfo.cs index a99ba94e9a..f47affcab8 100644 --- a/osu.Game/Beatmaps/BeatmapSetInfo.cs +++ b/osu.Game/Beatmaps/BeatmapSetInfo.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Linq; -using osu.Game.IO; using SQLite.Net.Attributes; using SQLiteNetExtensions.Attributes; @@ -37,8 +36,8 @@ namespace osu.Game.Beatmaps public string StoryboardFile => Files.FirstOrDefault(f => f.Filename.EndsWith(".osb"))?.Filename; - [ManyToMany(typeof(BeatmapSetFileInfo), CascadeOperations = CascadeOperation.CascadeRead)] - public List Files { get; set; } + [OneToMany(CascadeOperations = CascadeOperation.All)] + public List Files { get; set; } public bool Protected { get; set; } } diff --git a/osu.Game/Database/DatabaseBackedStore.cs b/osu.Game/Database/DatabaseBackedStore.cs index bb61fc1870..5214372a8b 100644 --- a/osu.Game/Database/DatabaseBackedStore.cs +++ b/osu.Game/Database/DatabaseBackedStore.cs @@ -101,7 +101,6 @@ namespace osu.Game.Database public void Populate(T item, bool recursive = true) { checkType(item.GetType()); - Connection.GetChildren(item, recursive); } diff --git a/osu.Game/IO/FileInfo.cs b/osu.Game/IO/FileInfo.cs index 6f4c4b26e8..367fd68f7b 100644 --- a/osu.Game/IO/FileInfo.cs +++ b/osu.Game/IO/FileInfo.cs @@ -11,8 +11,6 @@ namespace osu.Game.IO [PrimaryKey, AutoIncrement] public int ID { get; set; } - public string Filename { get; set; } - [Indexed(Unique = true)] public string Hash { get; set; } diff --git a/osu.Game/IO/FileStore.cs b/osu.Game/IO/FileStore.cs index e8cabafe17..2132037559 100644 --- a/osu.Game/IO/FileStore.cs +++ b/osu.Game/IO/FileStore.cs @@ -42,15 +42,11 @@ namespace osu.Game.IO deletePending(); } - public FileInfo Add(Stream data, string filename = null) + public FileInfo Add(Stream data) { string hash = data.ComputeSHA2Hash(); - var info = new FileInfo - { - Filename = filename, - Hash = hash, - }; + var info = new FileInfo { Hash = hash }; var existing = Connection.Table().FirstOrDefault(f => f.Hash == info.Hash); @@ -79,20 +75,32 @@ namespace osu.Game.IO public void Reference(IEnumerable files) { - foreach (var f in files) + Connection.RunInTransaction(() => { - f.ReferenceCount++; - Connection.Update(f); - } + var incrementedFiles = files.GroupBy(f => f.ID).Select(f => + { + var accurateRefCount = Connection.Get(f.First().ID); + accurateRefCount.ReferenceCount += f.Count(); + return accurateRefCount; + }); + + Connection.UpdateAll(incrementedFiles); + }); } public void Dereference(IEnumerable files) { - foreach (var f in files) + Connection.RunInTransaction(() => { - f.ReferenceCount--; - Connection.Update(f); - } + var incrementedFiles = files.GroupBy(f => f.ID).Select(f => + { + var accurateRefCount = Connection.Get(f.First().ID); + accurateRefCount.ReferenceCount -= f.Count(); + return accurateRefCount; + }); + + Connection.UpdateAll(incrementedFiles); + }); } private void deletePending() From c060d32765d486f49e7ac9b1d02b71c3579809ad Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Aug 2017 10:08:05 +0900 Subject: [PATCH 2/6] Separate out startup tasks to ensure they run after migrations --- osu.Game/Beatmaps/BeatmapStore.cs | 12 ++++++++---- osu.Game/Database/DatabaseBackedStore.cs | 20 ++++++++++++++++++-- osu.Game/IO/FileStore.cs | 4 ++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapStore.cs b/osu.Game/Beatmaps/BeatmapStore.cs index 102900ae81..d1c33d1151 100644 --- a/osu.Game/Beatmaps/BeatmapStore.cs +++ b/osu.Game/Beatmaps/BeatmapStore.cs @@ -52,7 +52,11 @@ namespace osu.Game.Beatmaps Connection.CreateTable(); Connection.CreateTable(); Connection.CreateTable(); + } + protected override void StartupTasks() + { + base.StartupTasks(); cleanupPendingDeletions(); } @@ -60,12 +64,12 @@ namespace osu.Game.Beatmaps /// Perform migrations between two store versions. /// /// The current store version. This will be zero on a fresh database initialisation. - /// The target version which we are migrating to (equal to the current ). - protected override void PerformMigration(int currentVersion, int newVersion) + /// The target version which we are migrating to (equal to the current ). + protected override void PerformMigration(int currentVersion, int targetVersion) { - base.PerformMigration(currentVersion, newVersion); + base.PerformMigration(currentVersion, targetVersion); - while (currentVersion++ < newVersion) + while (currentVersion++ < targetVersion) { switch (currentVersion) { diff --git a/osu.Game/Database/DatabaseBackedStore.cs b/osu.Game/Database/DatabaseBackedStore.cs index 5214372a8b..ae3409bbcf 100644 --- a/osu.Game/Database/DatabaseBackedStore.cs +++ b/osu.Game/Database/DatabaseBackedStore.cs @@ -51,14 +51,30 @@ namespace osu.Game.Database PerformMigration(reportedVersion.Version, reportedVersion.Version = StoreVersion); Connection.InsertOrReplace(reportedVersion); + + StartupTasks(); } - protected virtual void PerformMigration(int currentVersion, int newVersion) + /// + /// Called when the database version of this store doesn't match the local version. + /// Any manual migration operations should be performed in this. + /// + /// The current store version. This will be zero on a fresh database initialisation. + /// The target version which we are migrating to (equal to the current ). + protected virtual void PerformMigration(int currentVersion, int targetVersion) { } /// - /// Prepare this database for use. + /// Perform any common startup tasks. Runs after and . + /// + protected virtual void StartupTasks() + { + + } + + /// + /// Prepare this database for use. Tables should be created here. /// protected abstract void Prepare(bool reset = false); diff --git a/osu.Game/IO/FileStore.cs b/osu.Game/IO/FileStore.cs index 2132037559..e35382ea8c 100644 --- a/osu.Game/IO/FileStore.cs +++ b/osu.Game/IO/FileStore.cs @@ -38,7 +38,11 @@ namespace osu.Game.IO Connection.DropTable(); Connection.CreateTable(); + } + protected override void StartupTasks() + { + base.StartupTasks(); deletePending(); } From c73e139954e42c03d627b22ef6924ecead8140df Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Aug 2017 10:16:48 +0900 Subject: [PATCH 3/6] Add "migration" Also simplify initial migration for BeatmapStore by just nuking everything. --- osu.Game/Beatmaps/BeatmapStore.cs | 13 ++++------- osu.Game/IO/FileStore.cs | 36 +++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapStore.cs b/osu.Game/Beatmaps/BeatmapStore.cs index d1c33d1151..97cfdcf31b 100644 --- a/osu.Game/Beatmaps/BeatmapStore.cs +++ b/osu.Game/Beatmaps/BeatmapStore.cs @@ -21,7 +21,7 @@ namespace osu.Game.Beatmaps /// The current version of this store. Used for migrations (see ). /// The initial version is 1. /// - protected override int StoreVersion => 1; + protected override int StoreVersion => 2; public BeatmapStore(SQLiteConnection connection) : base(connection) @@ -74,14 +74,9 @@ namespace osu.Game.Beatmaps switch (currentVersion) { case 1: - // initialising from a version before we had versioning (or a fresh install). - - // force adding of Protected column (not automatically migrated). - Connection.MigrateTable(); - - // remove all existing beatmaps. - foreach (var b in Connection.GetAllWithChildren(null, true)) - Connection.Delete(b, true); + case 2: + // cannot migrate; breaking underlying changes. + Reset(); break; } } diff --git a/osu.Game/IO/FileStore.cs b/osu.Game/IO/FileStore.cs index e35382ea8c..94c9697922 100644 --- a/osu.Game/IO/FileStore.cs +++ b/osu.Game/IO/FileStore.cs @@ -23,6 +23,8 @@ namespace osu.Game.IO public readonly ResourceStore Store; + protected override int StoreVersion => 2; + public FileStore(SQLiteConnection connection, Storage storage) : base(connection, storage) { Store = new NamespacedResourceStore(new StorageBackedResourceStore(storage), prefix); @@ -35,7 +37,19 @@ namespace osu.Game.IO protected override void Prepare(bool reset = false) { if (reset) + { + try + { + foreach (var f in Query()) + Storage.Delete(Path.Combine(prefix, f.Hash)); + } + catch + { + // we don't want to ever crash as a result of a reset operation. + } + Connection.DropTable(); + } Connection.CreateTable(); } @@ -46,6 +60,28 @@ namespace osu.Game.IO deletePending(); } + /// + /// Perform migrations between two store versions. + /// + /// The current store version. This will be zero on a fresh database initialisation. + /// The target version which we are migrating to (equal to the current ). + protected override void PerformMigration(int currentVersion, int targetVersion) + { + base.PerformMigration(currentVersion, targetVersion); + + while (currentVersion++ < targetVersion) + { + switch (currentVersion) + { + case 1: + case 2: + // cannot migrate; breaking underlying changes. + Reset(); + break; + } + } + } + public FileInfo Add(Stream data) { string hash = data.ComputeSHA2Hash(); From 821f65c381ac2bcb9a41589df3b781285ca3e2fd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Aug 2017 10:20:28 +0900 Subject: [PATCH 4/6] Actually delete files --- osu.Game/IO/FileStore.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/IO/FileStore.cs b/osu.Game/IO/FileStore.cs index 94c9697922..191c97e272 100644 --- a/osu.Game/IO/FileStore.cs +++ b/osu.Game/IO/FileStore.cs @@ -41,7 +41,7 @@ namespace osu.Game.IO try { foreach (var f in Query()) - Storage.Delete(Path.Combine(prefix, f.Hash)); + Storage.Delete(Path.Combine(prefix, f.StoragePath)); } catch { @@ -150,7 +150,7 @@ namespace osu.Game.IO try { Connection.Delete(f); - Storage.Delete(Path.Combine(prefix, f.Hash)); + Storage.Delete(Path.Combine(prefix, f.StoragePath)); } catch (Exception e) { From 9d630e446eec6205e773064a2d2d17a1d5059535 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Aug 2017 10:57:46 +0900 Subject: [PATCH 5/6] Use new storage methods to reset FileStore Guarantees that backing files are cleaned up correctly. Also handles lingering "beatmaps" directory from older builds. --- osu.Game/IO/FileStore.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/osu.Game/IO/FileStore.cs b/osu.Game/IO/FileStore.cs index 191c97e272..e55af2e23a 100644 --- a/osu.Game/IO/FileStore.cs +++ b/osu.Game/IO/FileStore.cs @@ -38,15 +38,12 @@ namespace osu.Game.IO { if (reset) { - try - { - foreach (var f in Query()) - Storage.Delete(Path.Combine(prefix, f.StoragePath)); - } - catch - { - // we don't want to ever crash as a result of a reset operation. - } + // in earlier versions we stored beatmaps as solid archives, but not any more. + if (Storage.ExistsDirectory("beatmaps")) + Storage.DeleteDirectory("beatmaps"); + + if (Storage.ExistsDirectory(prefix)) + Storage.DeleteDirectory(prefix); Connection.DropTable(); } From 9db4caafba5eca07b59a75d803be17e91cdfb16b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Aug 2017 11:05:33 +0900 Subject: [PATCH 6/6] Update framework --- osu-framework | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu-framework b/osu-framework index 5a9ca94fc3..2204764944 160000 --- a/osu-framework +++ b/osu-framework @@ -1 +1 @@ -Subproject commit 5a9ca94fc31bc796b45572eb3d0b27b46556c586 +Subproject commit 2204764944ff693e857649253547054cc91764a0