Add tests and fix scenario where all matching are contained by duplicate candidate

osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs

@@ -207,6 +207,41 @@ namespace osu.Game.Tests.Beatmaps.IO
             }
         }
 
+        [TestCase(true)]
+        [TestCase(false)]
+        public void TestImportThenDeleteThenImportWithOnlineIDMismatch(bool set)
+        {
+            //unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here.
+            using (HeadlessGameHost host = new CleanRunHeadlessGameHost($"TestImportThenDeleteThenImport-{set}"))
+            {
+                try
+                {
+                    var osu = loadOsu(host);
+
+                    var imported = LoadOszIntoOsu(osu);
+
+                    if (set)
+                        imported.OnlineBeatmapSetID = 1234;
+                    else
+                        imported.Beatmaps.First().OnlineBeatmapID = 1234;
+
+                    osu.Dependencies.Get<BeatmapManager>().Update(imported);
+
+                    deleteBeatmapSet(imported, osu);
+
+                    var importedSecondTime = LoadOszIntoOsu(osu);
+
+                    // check the newly "imported" beatmap has been reimported due to mismatch (even though hashes matched)
+                    Assert.IsTrue(imported.ID != importedSecondTime.ID);
+                    Assert.IsTrue(imported.Beatmaps.First().ID != importedSecondTime.Beatmaps.First().ID);
+                }
+                finally
+                {
+                    host.Exit();
+                }
+            }
+        }
+
         [Test]
         [NonParallelizable]
         [Ignore("Binding IPC on Appveyor isn't working (port in use). Need to figure out why")]

osu.Game/Beatmaps/BeatmapManager.cs

@@ -102,7 +102,7 @@ namespace osu.Game.Beatmaps
                 b.BeatmapSet = beatmapSet;
             }
 
-            validateOnlineIds(beatmapSet.Beatmaps);
+            validateOnlineIds(beatmapSet);
 
             foreach (BeatmapInfo b in beatmapSet.Beatmaps)
                 fetchAndPopulateOnlineValues(b, beatmapSet.Beatmaps);
@@ -123,14 +123,30 @@ namespace osu.Game.Beatmaps
             }
         }
 
-        private void validateOnlineIds(List<BeatmapInfo> beatmaps)
+        private void validateOnlineIds(BeatmapSetInfo beatmapSet)
         {
-            var beatmapIds = beatmaps.Where(b => b.OnlineBeatmapID.HasValue).Select(b => b.OnlineBeatmapID).ToList();
+            var beatmapIds = beatmapSet.Beatmaps.Where(b => b.OnlineBeatmapID.HasValue).Select(b => b.OnlineBeatmapID).ToList();
 
-            // ensure all IDs are unique in this set and none match existing IDs in the local beatmap store.
+            // ensure all IDs are unique
-            if (beatmapIds.GroupBy(b => b).Any(g => g.Count() > 1) || QueryBeatmaps(b => beatmapIds.Contains(b.OnlineBeatmapID)).Any())
+            if (beatmapIds.GroupBy(b => b).Any(g => g.Count() > 1))
-                // remove all online IDs if any problems were found.
+            {
-                beatmaps.ForEach(b => b.OnlineBeatmapID = null);
+                resetIds();
+                return;
+            }
+
+            // find any existing beatmaps in the database that have matching online ids
+            var existingBeatmaps = QueryBeatmaps(b => beatmapIds.Contains(b.OnlineBeatmapID)).ToList();
+
+            if (existingBeatmaps.Count > 0)
+            {
+                // reset the import ids (to force a re-fetch) *unless* they match the candidate CheckForExisting set.
+                // we can ignore the case where the new ids are contained by the CheckForExisting set as it will either be used (import skipped) or deleted.
+                var existing = CheckForExisting(beatmapSet);
+                if (existing == null || existingBeatmaps.Any(b => !existing.Beatmaps.Contains(b)))
+                    resetIds();
+            }
+
+            void resetIds() => beatmapSet.Beatmaps.ForEach(b => b.OnlineBeatmapID = null);
         }
 
         /// <summary>
@@ -262,8 +278,11 @@ namespace osu.Game.Beatmaps
             if (!base.CanUndelete(existing, import))
                 return false;
 
+            var existingIds = existing.Beatmaps.Select(b => b.OnlineBeatmapID).OrderBy(i => i);
+            var importIds = import.Beatmaps.Select(b => b.OnlineBeatmapID).OrderBy(i => i);
+
             // force re-import if we are not in a sane state.
-            return existing.OnlineBeatmapSetID == import.OnlineBeatmapSetID;
+            return existing.OnlineBeatmapSetID == import.OnlineBeatmapSetID && existingIds.SequenceEqual(importIds);
         }
 
         /// <summary>