From d9fd05a5afe96a1b70b833a555bd10c06509407a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 19 Oct 2017 23:19:30 +0900 Subject: [PATCH 01/10] Hidden cannot be null --- osu.Game/Database/OsuDbContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/OsuDbContext.cs b/osu.Game/Database/OsuDbContext.cs index 93d23cc1bc..4e663159ff 100644 --- a/osu.Game/Database/OsuDbContext.cs +++ b/osu.Game/Database/OsuDbContext.cs @@ -219,7 +219,7 @@ namespace osu.Game.Database Database.ExecuteSqlCommand("DROP TABLE RulesetInfo_Old"); Database.ExecuteSqlCommand( - "INSERT INTO BeatmapInfo SELECT ID, AudioLeadIn, BaseDifficultyID, BeatDivisor, BeatmapSetInfoID, Countdown, DistanceSpacing, GridSize, Hash, Hidden, LetterboxInBreaks, MD5Hash, NULLIF(BeatmapMetadataID, 0), OnlineBeatmapID, Path, RulesetID, SpecialStyle, StackLeniency, StarDifficulty, StoredBookmarks, TimelineZoom, Version, WidescreenStoryboard FROM BeatmapInfo_Old"); + "INSERT INTO BeatmapInfo SELECT ID, AudioLeadIn, BaseDifficultyID, BeatDivisor, BeatmapSetInfoID, Countdown, DistanceSpacing, GridSize, Hash, IFNULL(Hidden, 0), LetterboxInBreaks, MD5Hash, NULLIF(BeatmapMetadataID, 0), OnlineBeatmapID, Path, RulesetID, SpecialStyle, StackLeniency, StarDifficulty, StoredBookmarks, TimelineZoom, Version, WidescreenStoryboard FROM BeatmapInfo_Old"); Database.ExecuteSqlCommand("DROP TABLE BeatmapInfo_Old"); } catch From 9b1ed5b3aaca927ef41ec637a07405a47e60b115 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 19 Oct 2017 23:33:27 +0900 Subject: [PATCH 02/10] Keep trying until delete succeeds Turns out it can fail if file handles are still open. --- osu.Game/OsuGameBase.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index e7ad77d099..4a80a0fa06 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -97,8 +97,18 @@ namespace osu.Game } catch (MigrationFailedException) { - using (var context = contextFactory.GetContext()) - context.Database.EnsureDeleted(); + while (true) + { + try + { + using (var context = contextFactory.GetContext()) + { + context.Database.EnsureDeleted(); + break; + } + } + catch { } + } using (var context = contextFactory.GetContext()) context.Migrate(); } From efaf98c5cf235d4c439e8ab147f1640b48533963 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2017 00:48:27 +0900 Subject: [PATCH 03/10] Allow recovery from a very broken database --- osu.Game/Database/OsuDbContext.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/OsuDbContext.cs b/osu.Game/Database/OsuDbContext.cs index 4e663159ff..1e3d76789d 100644 --- a/osu.Game/Database/OsuDbContext.cs +++ b/osu.Game/Database/OsuDbContext.cs @@ -160,7 +160,14 @@ namespace osu.Game.Database public void Migrate() { migrateFromSqliteNet(); - Database.Migrate(); + try + { + Database.Migrate(); + } + catch + { + throw new MigrationFailedException(); + } } private void migrateFromSqliteNet() From 0e1328a30eb0ef949abec7f90a8422073607d36b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2017 08:01:38 +0900 Subject: [PATCH 04/10] Add maximum try count before bailing --- osu.Game/OsuGameBase.cs | 55 ++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 4a80a0fa06..de9ed70ac9 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -90,28 +90,7 @@ namespace osu.Game dependencies.Cache(this); dependencies.Cache(LocalConfig); - try - { - using (var context = contextFactory.GetContext()) - context.Migrate(); - } - catch (MigrationFailedException) - { - while (true) - { - try - { - using (var context = contextFactory.GetContext()) - { - context.Database.EnsureDeleted(); - break; - } - } - catch { } - } - using (var context = contextFactory.GetContext()) - context.Migrate(); - } + runMigrations(); dependencies.Cache(API = new APIAccess { @@ -183,6 +162,38 @@ namespace osu.Game FileStore.Cleanup(); } + private void runMigrations() + { + try + { + using (var context = contextFactory.GetContext()) + context.Migrate(); + } + catch (MigrationFailedException) + { + // 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. + int retries = 20; + while (retries-- > 0) + { + try + { + using (var context = contextFactory.GetContext()) + { + context.Database.EnsureDeleted(); + break; + } + } + catch + { + } + } + + using (var context = contextFactory.GetContext()) + context.Migrate(); + } + } + private WorkingBeatmap lastBeatmap; public void APIStateChanged(APIAccess api, APIState state) From ca780784361ccc695c05a8c53ab505f043cdc51d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2017 08:01:45 +0900 Subject: [PATCH 05/10] Add more logging output --- osu.Game/Database/OsuDbContext.cs | 18 +++++++++++++----- osu.Game/OsuGameBase.cs | 7 ++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/osu.Game/Database/OsuDbContext.cs b/osu.Game/Database/OsuDbContext.cs index 1e3d76789d..bfa3c0a81b 100644 --- a/osu.Game/Database/OsuDbContext.cs +++ b/osu.Game/Database/OsuDbContext.cs @@ -160,13 +160,14 @@ namespace osu.Game.Database public void Migrate() { migrateFromSqliteNet(); + try { Database.Migrate(); } - catch + catch (Exception e) { - throw new MigrationFailedException(); + throw new MigrationFailedException(e); } } @@ -186,6 +187,8 @@ namespace osu.Game.Database try { + Logger.Log("Performing migration from sqlite-net to EF...", LoggingTarget.Database, Framework.Logging.LogLevel.Important); + // we are good to perform messy migration of data!. Database.ExecuteSqlCommand("ALTER TABLE BeatmapDifficulty RENAME TO BeatmapDifficulty_Old"); Database.ExecuteSqlCommand("ALTER TABLE BeatmapMetadata RENAME TO BeatmapMetadata_Old"); @@ -228,11 +231,12 @@ namespace osu.Game.Database Database.ExecuteSqlCommand( "INSERT INTO BeatmapInfo SELECT ID, AudioLeadIn, BaseDifficultyID, BeatDivisor, BeatmapSetInfoID, Countdown, DistanceSpacing, GridSize, Hash, IFNULL(Hidden, 0), LetterboxInBreaks, MD5Hash, NULLIF(BeatmapMetadataID, 0), OnlineBeatmapID, Path, RulesetID, SpecialStyle, StackLeniency, StarDifficulty, StoredBookmarks, TimelineZoom, Version, WidescreenStoryboard FROM BeatmapInfo_Old"); Database.ExecuteSqlCommand("DROP TABLE BeatmapInfo_Old"); + + Logger.Log("Migration complete!", LoggingTarget.Database, Framework.Logging.LogLevel.Important); } - catch + catch (Exception e) { - // if anything went wrong during migration just nuke the database. - throw new MigrationFailedException(); + throw new MigrationFailedException(e); } } catch (MigrationFailedException e) @@ -248,5 +252,9 @@ namespace osu.Game.Database public class MigrationFailedException : Exception { + public MigrationFailedException(Exception exception) + : base("sqlite-net migration failed", exception) + { + } } } diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index de9ed70ac9..0d575c52d0 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -17,6 +17,7 @@ using osu.Game.Graphics; using osu.Game.Graphics.Cursor; using osu.Game.Online.API; using osu.Framework.Graphics.Performance; +using osu.Framework.Logging; using osu.Game.Database; using osu.Game.Input; using osu.Game.Input.Bindings; @@ -169,8 +170,11 @@ namespace osu.Game using (var context = contextFactory.GetContext()) context.Migrate(); } - catch (MigrationFailedException) + catch (MigrationFailedException e) { + Logger.Log(e.ToString(), LoggingTarget.Database, LogLevel.Error); + Logger.Log("Migration failed! We'll be starting with a fresh database.", LoggingTarget.Database, LogLevel.Error); + // 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. int retries = 20; @@ -181,6 +185,7 @@ namespace osu.Game using (var context = contextFactory.GetContext()) { context.Database.EnsureDeleted(); + Logger.Log("Database purged successfully.", LoggingTarget.Database, LogLevel.Important); break; } } From d32059a7bac9da500ab120408e5ca1875302e8bd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2017 09:25:54 +0900 Subject: [PATCH 06/10] Ignore include-ignore warnings for now --- osu.Game/Database/OsuDbContext.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Game/Database/OsuDbContext.cs b/osu.Game/Database/OsuDbContext.cs index bfa3c0a81b..20d36f3ebe 100644 --- a/osu.Game/Database/OsuDbContext.cs +++ b/osu.Game/Database/OsuDbContext.cs @@ -3,6 +3,7 @@ using System; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.Logging; using osu.Framework.Logging; using osu.Game.Beatmaps; @@ -63,8 +64,12 @@ namespace osu.Game.Database protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) { base.OnConfiguring(optionsBuilder); - optionsBuilder.UseSqlite(connectionString); - optionsBuilder.UseLoggerFactory(logger.Value); + optionsBuilder + // this is required for the time being due to the way we are querying in places like BeatmapStore. + // if we ever move to having consumers file their own .Includes, or get eager loading support, this could be re-enabled. + .ConfigureWarnings(warnings => warnings.Ignore(CoreEventId.IncludeIgnoredWarning)) + .UseSqlite(connectionString) + .UseLoggerFactory(logger.Value); } protected override void OnModelCreating(ModelBuilder modelBuilder) From b805174143f8313d247fd2f4d28d9f75f6c1b536 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2017 14:33:35 +0900 Subject: [PATCH 07/10] Output the inner exception to the log when possible --- osu.Game/OsuGameBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 0d575c52d0..520627ad4a 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -172,7 +172,7 @@ namespace osu.Game } catch (MigrationFailedException e) { - Logger.Log(e.ToString(), LoggingTarget.Database, LogLevel.Error); + Logger.Log((e.InnerException ?? e).ToString(), LoggingTarget.Database, LogLevel.Error); Logger.Log("Migration failed! We'll be starting with a fresh database.", LoggingTarget.Database, LogLevel.Error); // if we failed, let's delete the database and start fresh. From 47213d2498e81440f6e2d24bd15d6c37933a4eee Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 21 Oct 2017 00:15:02 +0900 Subject: [PATCH 08/10] Rely on storage.DeleteDatabase for guaranteed database deletion Relies on https://github.com/ppy/osu-framework/pull/1100 being merged for most effectiveness. --- osu.Game/Database/DatabaseContextFactory.cs | 10 +++++++++- osu.Game/OsuGameBase.cs | 18 ++---------------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/osu.Game/Database/DatabaseContextFactory.cs b/osu.Game/Database/DatabaseContextFactory.cs index 359188b4e2..6154016083 100644 --- a/osu.Game/Database/DatabaseContextFactory.cs +++ b/osu.Game/Database/DatabaseContextFactory.cs @@ -9,11 +9,19 @@ namespace osu.Game.Database { private readonly GameHost host; + private const string database_name = @"client"; + public DatabaseContextFactory(GameHost host) { this.host = host; } - public OsuDbContext GetContext() => new OsuDbContext(host.Storage.GetDatabaseConnectionString(@"client")); + public OsuDbContext GetContext() => new OsuDbContext(host.Storage.GetDatabaseConnectionString(database_name)); + + public void ResetDatabase() + { + // todo: we probably want to make sure there are no active contexts before performing this operation. + host.Storage.DeleteDatabase(database_name); + } } } diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 520627ad4a..50639e3427 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -177,22 +177,8 @@ 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. - int retries = 20; - while (retries-- > 0) - { - try - { - using (var context = contextFactory.GetContext()) - { - context.Database.EnsureDeleted(); - Logger.Log("Database purged successfully.", LoggingTarget.Database, LogLevel.Important); - break; - } - } - catch - { - } - } + contextFactory.ResetDatabase(); + Logger.Log("Database purged successfully.", LoggingTarget.Database, LogLevel.Important); using (var context = contextFactory.GetContext()) context.Migrate(); From a9657d2142d3537ed0d38f315fa11c6852b91603 Mon Sep 17 00:00:00 2001 From: Shane Woolcock Date: Sun, 22 Oct 2017 00:55:32 +1030 Subject: [PATCH 09/10] Change beatmap import to use OpenTK's FileDrop event --- osu.Desktop/OsuGameDesktop.cs | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/osu.Desktop/OsuGameDesktop.cs b/osu.Desktop/OsuGameDesktop.cs index f4fb10a496..1e4bf3119d 100644 --- a/osu.Desktop/OsuGameDesktop.cs +++ b/osu.Desktop/OsuGameDesktop.cs @@ -7,13 +7,13 @@ using System.IO; using System.Linq; using System.Reflection; using System.Threading.Tasks; -using System.Windows.Forms; using Microsoft.Win32; using osu.Desktop.Overlays; using osu.Framework.Graphics.Containers; using osu.Framework.Platform; using osu.Game; using osu.Game.Screens.Menu; +using OpenTK.Input; namespace osu.Desktop { @@ -105,16 +105,13 @@ namespace osu.Desktop desktopWindow.Icon = new Icon(Assembly.GetExecutingAssembly().GetManifestResourceStream(GetType(), "lazer.ico")); desktopWindow.Title = Name; - desktopWindow.DragEnter += dragEnter; - desktopWindow.DragDrop += dragDrop; + desktopWindow.FileDrop += fileDrop; } } - private void dragDrop(DragEventArgs e) + private void fileDrop(object sender, FileDropEventArgs e) { - // this method will only be executed if e.Effect in dragEnter gets set to something other that None. - var dropData = (object[])e.Data.GetData(DataFormats.FileDrop); - var filePaths = dropData.Select(f => f.ToString()).ToArray(); + var filePaths = new [] { e.FileName }; if (filePaths.All(f => Path.GetExtension(f) == @".osz")) Task.Run(() => BeatmapManager.Import(filePaths)); @@ -127,16 +124,5 @@ namespace osu.Desktop } private static readonly string[] allowed_extensions = { @".osz", @".osr" }; - - private void dragEnter(DragEventArgs e) - { - // dragDrop will only be executed if e.Effect gets set to something other that None in this method. - bool isFile = e.Data.GetDataPresent(DataFormats.FileDrop); - if (isFile) - { - var paths = ((object[])e.Data.GetData(DataFormats.FileDrop)).Select(f => f.ToString()).ToArray(); - e.Effect = allowed_extensions.Any(ext => paths.All(p => p.EndsWith(ext))) ? DragDropEffects.Copy : DragDropEffects.None; - } - } } } From 30307de498c56632b8881914f0f8ac18183f66f2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 22 Oct 2017 11:58:40 +0900 Subject: [PATCH 10/10] Update framework --- osu-framework | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu-framework b/osu-framework index dbcfa5c244..383a8da7bc 160000 --- a/osu-framework +++ b/osu-framework @@ -1 +1 @@ -Subproject commit dbcfa5c244555e7901dac7d94eab53b3b04d17e6 +Subproject commit 383a8da7bc45af498288b4b72c72a048a0996e74