Merge branch 'master' into realm-integration/live-queryable-fix

osu.Game.Tests/Database/BeatmapImporterTests.cs

@@ -12,6 +12,7 @@ using NUnit.Framework;
 using osu.Framework.Extensions;
 using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Logging;
+using osu.Framework.Platform;
 using osu.Game.Beatmaps;
 using osu.Game.Database;
 using osu.Game.Extensions;
@@ -474,7 +475,7 @@ namespace osu.Game.Tests.Database
         }
 
         [Test]
-        public void TestImportThenDeleteThenImport()
+        public void TestImportThenDeleteThenImportOptimisedPath()
         {
             RunTestWithRealmAsync(async (realmFactory, storage) =>
             {
@@ -485,11 +486,39 @@ namespace osu.Game.Tests.Database
 
                 deleteBeatmapSet(imported, realmFactory.Context);
 
+                Assert.IsTrue(imported.DeletePending);
+
                 var importedSecondTime = await LoadOszIntoStore(importer, realmFactory.Context);
 
                 // check the newly "imported" beatmap is actually just the restored previous import. since it matches hash.
                 Assert.IsTrue(imported.ID == importedSecondTime.ID);
                 Assert.IsTrue(imported.Beatmaps.First().ID == importedSecondTime.Beatmaps.First().ID);
+                Assert.IsFalse(imported.DeletePending);
+                Assert.IsFalse(importedSecondTime.DeletePending);
+            });
+        }
+
+        [Test]
+        public void TestImportThenDeleteThenImportNonOptimisedPath()
+        {
+            RunTestWithRealmAsync(async (realmFactory, storage) =>
+            {
+                using var importer = new NonOptimisedBeatmapImporter(realmFactory, storage);
+                using var store = new RealmRulesetStore(realmFactory, storage);
+
+                var imported = await LoadOszIntoStore(importer, realmFactory.Context);
+
+                deleteBeatmapSet(imported, realmFactory.Context);
+
+                Assert.IsTrue(imported.DeletePending);
+
+                var importedSecondTime = await LoadOszIntoStore(importer, realmFactory.Context);
+
+                // check the newly "imported" beatmap is actually just the restored previous import. since it matches hash.
+                Assert.IsTrue(imported.ID == importedSecondTime.ID);
+                Assert.IsTrue(imported.Beatmaps.First().ID == importedSecondTime.Beatmaps.First().ID);
+                Assert.IsFalse(imported.DeletePending);
+                Assert.IsFalse(importedSecondTime.DeletePending);
             });
         }
 
@@ -867,5 +896,15 @@ namespace osu.Game.Tests.Database
 
             Assert.Fail(failureMessage);
         }
+
+        public class NonOptimisedBeatmapImporter : BeatmapImporter
+        {
+            public NonOptimisedBeatmapImporter(RealmContextFactory realmFactory, Storage storage)
+                : base(realmFactory, storage)
+            {
+            }
+
+            protected override bool HasCustomHashFunction => true;
+        }
     }
 }

osu.Game.Tests/Database/RealmLiveTests.cs

@@ -29,6 +29,22 @@ namespace osu.Game.Tests.Database
             });
         }
 
+        [Test]
+        public void TestAccessAfterAttach()
+        {
+            RunTestWithRealm((realmFactory, _) =>
+            {
+                var beatmap = new RealmBeatmap(CreateRuleset(), new RealmBeatmapDifficulty(), new RealmBeatmapMetadata());
+
+                var liveBeatmap = beatmap.ToLive();
+
+                using (var context = realmFactory.CreateContext())
+                    context.Write(r => r.Add(beatmap));
+
+                Assert.IsFalse(liveBeatmap.PerformRead(l => l.Hidden));
+            });
+        }
+
         [Test]
         public void TestAccessNonManaged()
         {
@@ -51,7 +67,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealm((realmFactory, _) =>
             {
-                RealmLive<RealmBeatmap>? liveBeatmap = null;
+                ILive<RealmBeatmap>? liveBeatmap = null;
                 Task.Factory.StartNew(() =>
                 {
                     using (var threadContext = realmFactory.CreateContext())
@@ -80,7 +96,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealm((realmFactory, _) =>
             {
-                RealmLive<RealmBeatmap>? liveBeatmap = null;
+                ILive<RealmBeatmap>? liveBeatmap = null;
                 Task.Factory.StartNew(() =>
                 {
                     using (var threadContext = realmFactory.CreateContext())
@@ -121,7 +137,8 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealm((realmFactory, _) =>
             {
-                RealmLive<RealmBeatmap>? liveBeatmap = null;
+                ILive<RealmBeatmap>? liveBeatmap = null;
+
                 Task.Factory.StartNew(() =>
                 {
                     using (var threadContext = realmFactory.CreateContext())
@@ -159,7 +176,7 @@ namespace osu.Game.Tests.Database
         {
             RunTestWithRealm((realmFactory, _) =>
             {
-                RealmLive<RealmBeatmap>? liveBeatmap = null;
+                ILive<RealmBeatmap>? liveBeatmap = null;
                 Task.Factory.StartNew(() =>
                 {
                     using (var threadContext = realmFactory.CreateContext())
@@ -192,7 +209,7 @@ namespace osu.Game.Tests.Database
                 using (var updateThreadContext = realmFactory.CreateContext())
                 {
                     updateThreadContext.All<RealmBeatmap>().SubscribeForNotifications(gotChange);
-                    RealmLive<RealmBeatmap>? liveBeatmap = null;
+                    ILive<RealmBeatmap>? liveBeatmap = null;
 
                     Task.Factory.StartNew(() =>
                     {

osu.Game/Beatmaps/BeatmapManager.cs

@@ -288,9 +288,9 @@ namespace osu.Game.Beatmaps
 
         #region Implementation of IModelFileManager<in BeatmapSetInfo,in BeatmapSetFileInfo>
 
-        public void ReplaceFile(BeatmapSetInfo model, BeatmapSetFileInfo file, Stream contents, string filename = null)
+        public void ReplaceFile(BeatmapSetInfo model, BeatmapSetFileInfo file, Stream contents)
         {
-            beatmapModelManager.ReplaceFile(model, file, contents, filename);
+            beatmapModelManager.ReplaceFile(model, file, contents);
         }
 
         public void DeleteFile(BeatmapSetInfo model, BeatmapSetFileInfo file)

osu.Game/Database/ArchiveModelManager.cs

@@ -453,13 +453,12 @@ namespace osu.Game.Database
         /// <param name="model">The item to operate on.</param>
         /// <param name="file">The existing file to be replaced.</param>
         /// <param name="contents">The new file contents.</param>
-        /// <param name="filename">An optional filename for the new file. Will use the previous filename if not specified.</param>
+        public void ReplaceFile(TModel model, TFileModel file, Stream contents)
-        public void ReplaceFile(TModel model, TFileModel file, Stream contents, string filename = null)
         {
             using (ContextFactory.GetForWrite())
             {
                 DeleteFile(model, file);
-                AddFile(model, contents, filename ?? file.Filename);
+                AddFile(model, contents, file.Filename);
             }
         }
 

osu.Game/Database/IModelFileManager.cs

@@ -15,8 +15,7 @@ namespace osu.Game.Database
         /// <param name="model">The item to operate on.</param>
         /// <param name="file">The existing file to be replaced.</param>
         /// <param name="contents">The new file contents.</param>
-        /// <param name="filename">An optional filename for the new file. Will use the previous filename if not specified.</param>
+        void ReplaceFile(TModel model, TFileModel file, Stream contents);
-        void ReplaceFile(TModel model, TFileModel file, Stream contents, string filename = null);
 
         /// <summary>
         /// Delete an existing file.

osu.Game/Database/RealmObjectExtensions.cs

@@ -49,13 +49,13 @@ namespace osu.Game.Database
             return mapper.Map<T>(item);
         }
 
-        public static List<RealmLive<T>> ToLive<T>(this IEnumerable<T> realmList)
+        public static List<ILive<T>> ToLive<T>(this IEnumerable<T> realmList)
             where T : RealmObject, IHasGuidPrimaryKey
         {
-            return realmList.Select(l => new RealmLive<T>(l)).ToList();
+            return realmList.Select(l => new RealmLive<T>(l)).Cast<ILive<T>>().ToList();
         }
 
-        public static RealmLive<T> ToLive<T>(this T realmObject)
+        public static ILive<T> ToLive<T>(this T realmObject)
             where T : RealmObject, IHasGuidPrimaryKey
         {
             return new RealmLive<T>(realmObject);

osu.Game/Models/RealmNamedFileUsage.cs

@@ -16,6 +16,7 @@ namespace osu.Game.Models
     {
         public RealmFile File { get; set; } = null!;
 
+        // [Indexed] cannot be used on `EmbeddedObject`s as it only applies to top-level queries. May need to reconsider this if performance becomes a concern.
         public string Filename { get; set; } = null!;
 
         public RealmNamedFileUsage(RealmFile file, string filename)

osu.Game/Screens/Edit/Setup/ResourcesSection.cs

@@ -78,8 +78,8 @@ namespace osu.Game.Screens.Edit.Setup
             using (var stream = info.OpenRead())
             {
                 if (oldFile != null)
-                    beatmaps.ReplaceFile(set, oldFile, stream, info.Name);
+                    beatmaps.DeleteFile(set, oldFile);
-                else
+
                 beatmaps.AddFile(set, stream, info.Name);
             }
 
@@ -105,8 +105,7 @@ namespace osu.Game.Screens.Edit.Setup
             using (var stream = info.OpenRead())
             {
                 if (oldFile != null)
-                    beatmaps.ReplaceFile(set, oldFile, stream, info.Name);
+                    beatmaps.DeleteFile(set, oldFile);
-                else
                 beatmaps.AddFile(set, stream, info.Name);
             }
 

osu.Game/Skinning/SkinManager.cs

@@ -171,7 +171,7 @@ namespace osu.Game.Skinning
                     var oldFile = skin.SkinInfo.Files.FirstOrDefault(f => f.Filename == filename);
 
                     if (oldFile != null)
-                        skinModelManager.ReplaceFile(skin.SkinInfo, oldFile, streamContent, oldFile.Filename);
+                        skinModelManager.ReplaceFile(skin.SkinInfo, oldFile, streamContent);
                     else
                         skinModelManager.AddFile(skin.SkinInfo, streamContent, filename);
                 }

osu.Game/Stores/RealmArchiveModelImporter.cs

@@ -294,12 +294,8 @@ namespace osu.Game.Stores
         /// <remarks>
         ///  In the case of no matching files, a hash will be generated from the passed archive's <see cref="ArchiveReader.Name"/>.
         /// </remarks>
-        protected virtual string ComputeHash(TModel item, ArchiveReader? reader = null)
+        protected virtual string ComputeHash(TModel item)
         {
-            if (reader != null)
-                // fast hashing for cases where the item's files may not be populated.
-                return computeHashFast(reader);
-
             // for now, concatenate all hashable files in the set to create a unique hash.
             MemoryStream hashable = new MemoryStream();
 
@@ -374,7 +370,7 @@ namespace osu.Game.Stores
                             // TODO: look into rollback of file additions (or delayed commit).
                             item.Files.AddRange(createFileInfos(archive, Files, realm));
 
-                        item.Hash = ComputeHash(item, archive);
+                        item.Hash = ComputeHash(item);
 
                         // TODO: we may want to run this outside of the transaction.
                         await Populate(item, archive, realm, cancellationToken).ConfigureAwait(false);
@@ -387,7 +383,9 @@ namespace osu.Game.Stores
                             if (CanReuseExisting(existing, item))
                             {
                                 LogForModel(item, @$"Found existing {HumanisedModelName} for {item} (ID {existing.ID}) – skipping import.");
+
                                 existing.DeletePending = false;
+                                transaction.Commit();
 
                                 return existing.ToLive();
                             }