From 8452e315f478f3470f9204214986b739c284b21c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Oct 2017 18:28:28 +0900 Subject: [PATCH 1/6] Allow "refreshing" instances via DatabaseBackedStore --- osu.Game/Database/DatabaseBackedStore.cs | 5 +++++ osu.Game/Database/IHasPrimaryKey.cs | 10 ++++++++++ osu.Game/osu.Game.csproj | 1 + 3 files changed, 16 insertions(+) create mode 100644 osu.Game/Database/IHasPrimaryKey.cs diff --git a/osu.Game/Database/DatabaseBackedStore.cs b/osu.Game/Database/DatabaseBackedStore.cs index 04aa8f91ae..040b23a82e 100644 --- a/osu.Game/Database/DatabaseBackedStore.cs +++ b/osu.Game/Database/DatabaseBackedStore.cs @@ -18,6 +18,11 @@ public abstract class DatabaseBackedStore private readonly ThreadLocal queryContext; + /// + /// Refresh an instance potentially from a diffrent thread with a local context-tracked instance. + /// + protected void Refresh(ref T obj) where T : class, IHasPrimaryKey => obj = GetContext().Find(obj.ID); + /// /// Retrieve a shared context for performing lookups (or write operations on the update thread, for now). /// diff --git a/osu.Game/Database/IHasPrimaryKey.cs b/osu.Game/Database/IHasPrimaryKey.cs new file mode 100644 index 0000000000..9af9852b03 --- /dev/null +++ b/osu.Game/Database/IHasPrimaryKey.cs @@ -0,0 +1,10 @@ +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +namespace osu.Game.Database +{ + public interface IHasPrimaryKey + { + int ID { get; set; } + } +} diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 8f519d8915..dc6cf7e300 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -282,6 +282,7 @@ + 20171019041408_InitialCreate.cs From 5001e9f2640477d9cc91aa79396e90a68c07b412 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Oct 2017 22:07:32 +0900 Subject: [PATCH 2/6] Fix Hide/Restore/Delete etc. --- osu.Game/Beatmaps/BeatmapInfo.cs | 3 ++- osu.Game/Beatmaps/BeatmapSetInfo.cs | 3 ++- osu.Game/Beatmaps/BeatmapStore.cs | 16 ++++++++-------- osu.Game/Database/DatabaseBackedStore.cs | 18 ++++++++++++++++-- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- 5 files changed, 29 insertions(+), 13 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapInfo.cs b/osu.Game/Beatmaps/BeatmapInfo.cs index 5347953c85..022d64db03 100644 --- a/osu.Game/Beatmaps/BeatmapInfo.cs +++ b/osu.Game/Beatmaps/BeatmapInfo.cs @@ -6,12 +6,13 @@ using System.ComponentModel.DataAnnotations.Schema; using System.Linq; using Newtonsoft.Json; +using osu.Game.Database; using osu.Game.IO.Serialization; using osu.Game.Rulesets; namespace osu.Game.Beatmaps { - public class BeatmapInfo : IEquatable, IJsonSerializable + public class BeatmapInfo : IEquatable, IJsonSerializable, IHasPrimaryKey { [DatabaseGenerated(DatabaseGeneratedOption.Identity)] public int ID { get; set; } diff --git a/osu.Game/Beatmaps/BeatmapSetInfo.cs b/osu.Game/Beatmaps/BeatmapSetInfo.cs index 2dfc4d0fe0..c870c31a8b 100644 --- a/osu.Game/Beatmaps/BeatmapSetInfo.cs +++ b/osu.Game/Beatmaps/BeatmapSetInfo.cs @@ -4,10 +4,11 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations.Schema; using System.Linq; +using osu.Game.Database; namespace osu.Game.Beatmaps { - public class BeatmapSetInfo + public class BeatmapSetInfo : IHasPrimaryKey { [DatabaseGenerated(DatabaseGeneratedOption.Identity)] public int ID { get; set; } diff --git a/osu.Game/Beatmaps/BeatmapStore.cs b/osu.Game/Beatmaps/BeatmapStore.cs index 2e6efee0aa..31de11cce6 100644 --- a/osu.Game/Beatmaps/BeatmapStore.cs +++ b/osu.Game/Beatmaps/BeatmapStore.cs @@ -48,10 +48,10 @@ public bool Delete(BeatmapSetInfo beatmapSet) { var context = GetContext(); - if (beatmapSet.DeletePending) return false; + Refresh(ref beatmapSet, BeatmapSets); + if (beatmapSet.DeletePending) return false; beatmapSet.DeletePending = true; - context.Update(beatmapSet); context.SaveChanges(); BeatmapSetRemoved?.Invoke(beatmapSet); @@ -67,10 +67,10 @@ public bool Undelete(BeatmapSetInfo beatmapSet) { var context = GetContext(); - if (!beatmapSet.DeletePending) return false; + Refresh(ref beatmapSet, BeatmapSets); + if (!beatmapSet.DeletePending) return false; beatmapSet.DeletePending = false; - context.Update(beatmapSet); context.SaveChanges(); BeatmapSetAdded?.Invoke(beatmapSet); @@ -86,10 +86,10 @@ public bool Hide(BeatmapInfo beatmap) { var context = GetContext(); - if (beatmap.Hidden) return false; + Refresh(ref beatmap, Beatmaps); + if (beatmap.Hidden) return false; beatmap.Hidden = true; - context.Update(beatmap); context.SaveChanges(); BeatmapHidden?.Invoke(beatmap); @@ -105,10 +105,10 @@ public bool Restore(BeatmapInfo beatmap) { var context = GetContext(); - if (!beatmap.Hidden) return false; + Refresh(ref beatmap, Beatmaps); + if (!beatmap.Hidden) return false; beatmap.Hidden = false; - context.Update(beatmap); context.SaveChanges(); BeatmapRestored?.Invoke(beatmap); diff --git a/osu.Game/Database/DatabaseBackedStore.cs b/osu.Game/Database/DatabaseBackedStore.cs index 040b23a82e..7ca20ac28e 100644 --- a/osu.Game/Database/DatabaseBackedStore.cs +++ b/osu.Game/Database/DatabaseBackedStore.cs @@ -2,7 +2,10 @@ // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE using System; +using System.Collections.Generic; +using System.Linq; using System.Threading; +using Microsoft.EntityFrameworkCore; using osu.Framework.Platform; namespace osu.Game.Database @@ -19,9 +22,20 @@ public abstract class DatabaseBackedStore private readonly ThreadLocal queryContext; /// - /// Refresh an instance potentially from a diffrent thread with a local context-tracked instance. + /// Refresh an instance potentially from a different thread with a local context-tracked instance. /// - protected void Refresh(ref T obj) where T : class, IHasPrimaryKey => obj = GetContext().Find(obj.ID); + /// + /// + /// + protected virtual void Refresh(ref T obj, IEnumerable lookupSource = null) where T : class, IHasPrimaryKey + { + var context = GetContext(); + + if (context.Entry(obj).State != EntityState.Detached) return; + + var id = obj.ID; + obj = lookupSource?.FirstOrDefault(t => t.ID == id) ?? context.Find(id); + } /// /// Retrieve a shared context for performing lookups (or write operations on the update thread, for now). diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 582eeebd48..0cfe07628a 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -119,7 +119,7 @@ public void RemoveBeatmap(BeatmapSetInfo beatmapSet) internal void UpdateBeatmap(BeatmapInfo beatmap) { // todo: this method should not run more than once for the same BeatmapSetInfo. - var set = manager.Refresh(beatmap.BeatmapSet); + var set = manager.QueryBeatmapSet(s => s.ID == beatmap.BeatmapSetInfoID); // todo: this method should be smarter as to not recreate panels that haven't changed, etc. var group = groups.Find(b => b.BeatmapSet.ID == set.ID); From a5fb70022901054d28863554d29e8f8abffc1790 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Oct 2017 18:59:15 +0900 Subject: [PATCH 3/6] Fix KeyBinding updates --- osu.Game/Input/Bindings/DatabasedKeyBinding.cs | 3 ++- osu.Game/Input/KeyBindingStore.cs | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/osu.Game/Input/Bindings/DatabasedKeyBinding.cs b/osu.Game/Input/Bindings/DatabasedKeyBinding.cs index c0cecf361d..7e9e25aeff 100644 --- a/osu.Game/Input/Bindings/DatabasedKeyBinding.cs +++ b/osu.Game/Input/Bindings/DatabasedKeyBinding.cs @@ -3,11 +3,12 @@ using System.ComponentModel.DataAnnotations.Schema; using osu.Framework.Input.Bindings; +using osu.Game.Database; namespace osu.Game.Input.Bindings { [Table("KeyBinding")] - public class DatabasedKeyBinding : KeyBinding + public class DatabasedKeyBinding : KeyBinding, IHasPrimaryKey { [DatabaseGenerated(DatabaseGeneratedOption.Identity)] public int ID { get; set; } diff --git a/osu.Game/Input/KeyBindingStore.cs b/osu.Game/Input/KeyBindingStore.cs index 53309fc72d..e21597130a 100644 --- a/osu.Game/Input/KeyBindingStore.cs +++ b/osu.Game/Input/KeyBindingStore.cs @@ -60,7 +60,7 @@ private void insertDefaults(IEnumerable defaults, int? rulesetId = n } /// - /// Retrieve s for a specified ruleset/variant content. + /// Retrieve s for a specified ruleset/variant content. /// /// The ruleset's internal ID. /// An optional variant. @@ -70,8 +70,13 @@ public IEnumerable Query(int? rulesetId = null, int? variant = null) public void Update(KeyBinding keyBinding) { + var dbKeyBinding = (DatabasedKeyBinding)keyBinding; + var context = GetContext(); - context.Update(keyBinding); + + Refresh(ref dbKeyBinding); + + context.Update(dbKeyBinding); context.SaveChanges(); KeyBindingChanged?.Invoke(); From c1d133977eb49ab0ec169b4e85b5b42e11fb56ab Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Oct 2017 22:17:17 +0900 Subject: [PATCH 4/6] FirstOrDefault -> SingleOrDefault --- osu.Game/Database/DatabaseBackedStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/DatabaseBackedStore.cs b/osu.Game/Database/DatabaseBackedStore.cs index 7ca20ac28e..2944b9eeca 100644 --- a/osu.Game/Database/DatabaseBackedStore.cs +++ b/osu.Game/Database/DatabaseBackedStore.cs @@ -34,7 +34,7 @@ protected virtual void Refresh(ref T obj, IEnumerable lookupSource = null) if (context.Entry(obj).State != EntityState.Detached) return; var id = obj.ID; - obj = lookupSource?.FirstOrDefault(t => t.ID == id) ?? context.Find(id); + obj = lookupSource?.SingleOrDefault(t => t.ID == id) ?? context.Find(id); } /// From 6f7ba55f8010e37c197ec0db2f86756041c78d55 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Oct 2017 22:19:47 +0900 Subject: [PATCH 5/6] Fill out xmldoc --- osu.Game/Database/DatabaseBackedStore.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/DatabaseBackedStore.cs b/osu.Game/Database/DatabaseBackedStore.cs index 2944b9eeca..bc1b7132eb 100644 --- a/osu.Game/Database/DatabaseBackedStore.cs +++ b/osu.Game/Database/DatabaseBackedStore.cs @@ -24,9 +24,9 @@ public abstract class DatabaseBackedStore /// /// Refresh an instance potentially from a different thread with a local context-tracked instance. /// - /// - /// - /// + /// The object to use as a reference when negotiating a local instance. + /// An optional lookup source which will be used to query and populate a freshly retrieved replacement. If not provided, the refreshed object will still be returned but will not have any includes. + /// A valid EF-stored type. protected virtual void Refresh(ref T obj, IEnumerable lookupSource = null) where T : class, IHasPrimaryKey { var context = GetContext(); From 4ef80ee6c4c5c416ba15bc97f76bccf9240c18a8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 25 Oct 2017 23:21:47 +0900 Subject: [PATCH 6/6] Fix potential incorrect update in KeyBindingStore --- osu.Game/Input/KeyBindingStore.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Input/KeyBindingStore.cs b/osu.Game/Input/KeyBindingStore.cs index e21597130a..3407705909 100644 --- a/osu.Game/Input/KeyBindingStore.cs +++ b/osu.Game/Input/KeyBindingStore.cs @@ -76,7 +76,8 @@ public void Update(KeyBinding keyBinding) Refresh(ref dbKeyBinding); - context.Update(dbKeyBinding); + dbKeyBinding.KeyCombination = keyBinding.KeyCombination; + context.SaveChanges(); KeyBindingChanged?.Invoke();