Merge pull request #2761 from peppy/populate-missing-online-ids

Lookup online IDs on import when missing from .osu files

osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs

@@ -86,7 +86,7 @@ namespace osu.Game.Tests.Beatmaps.Formats
                 Assert.AreEqual(string.Empty, metadata.Source);
                 Assert.AreEqual("MBC7 Unisphere 地球ヤバイEP Chikyu Yabai", metadata.Tags);
                 Assert.AreEqual(557821, beatmapInfo.OnlineBeatmapID);
-                Assert.AreEqual(241526, metadata.OnlineBeatmapSetID);
+                Assert.AreEqual(241526, beatmapInfo.BeatmapSet.OnlineBeatmapSetID);
             }
         }
 

osu.Game.Tests/Beatmaps/Formats/OsuJsonDecoderTest.cs

@@ -28,7 +28,7 @@ namespace osu.Game.Tests.Beatmaps.Formats
         {
             var beatmap = decodeAsJson(normal);
             var meta = beatmap.BeatmapInfo.Metadata;
-            Assert.AreEqual(241526, meta.OnlineBeatmapSetID);
+            Assert.AreEqual(241526, beatmap.BeatmapInfo.BeatmapSet.OnlineBeatmapSetID);
             Assert.AreEqual("Soleily", meta.Artist);
             Assert.AreEqual("Soleily", meta.ArtistUnicode);
             Assert.AreEqual("03. Renatus - Soleily 192kbps.mp3", meta.AudioFile);

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

@@ -48,11 +48,14 @@ namespace osu.Game.Tests.Beatmaps.IO
             {
                 var reader = new ZipArchiveReader(osz);
 
-                BeatmapMetadata meta;
-                using (var stream = new StreamReader(reader.GetStream("Soleily - Renatus (Deif) [Platter].osu")))
-                    meta = Decoder.GetDecoder<Beatmap>(stream).Decode(stream).Metadata;
+                Beatmap beatmap;
 
-                Assert.AreEqual(241526, meta.OnlineBeatmapSetID);
+                using (var stream = new StreamReader(reader.GetStream("Soleily - Renatus (Deif) [Platter].osu")))
+                    beatmap = Decoder.GetDecoder<Beatmap>(stream).Decode(stream);
+
+                var meta = beatmap.Metadata;
+
+                Assert.AreEqual(241526, beatmap.BeatmapInfo.BeatmapSet.OnlineBeatmapSetID);
                 Assert.AreEqual("Soleily", meta.Artist);
                 Assert.AreEqual("Soleily", meta.ArtistUnicode);
                 Assert.AreEqual("03. Renatus - Soleily 192kbps.mp3", meta.AudioFile);

osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs

@@ -449,7 +449,6 @@ namespace osu.Game.Tests.Visual
                 Hash = new MemoryStream(Encoding.UTF8.GetBytes(Guid.NewGuid().ToString())).ComputeMD5Hash(),
                 Metadata = new BeatmapMetadata
                 {
-                    OnlineBeatmapSetID = id,
                     // Create random metadata, then we can check if sorting works based on these
                     Artist = $"peppy{id.ToString().PadLeft(6, '0')}",
                     Title = $"test set #{id}!",
@@ -503,7 +502,6 @@ namespace osu.Game.Tests.Visual
                 Hash = new MemoryStream(Encoding.UTF8.GetBytes(Guid.NewGuid().ToString())).ComputeMD5Hash(),
                 Metadata = new BeatmapMetadata
                 {
-                    OnlineBeatmapSetID = id,
                     // Create random metadata, then we can check if sorting works based on these
                     Artist = $"peppy{id.ToString().PadLeft(6, '0')}",
                     Title = $"test set #{id}!",

osu.Game.Tests/Visual/TestCaseBeatmapScoresContainer.cs

@@ -52,7 +52,7 @@ namespace osu.Game.Tests.Visual
             AddStep("remove scores", () => scoresContainer.Scores = null);
             AddStep("resize to big", () => container.ResizeWidthTo(1, 300));
             AddStep("resize to normal", () => container.ResizeWidthTo(0.8f, 300));
-            AddStep("online scores", () => scoresContainer.Beatmap = new BeatmapInfo { OnlineBeatmapSetID = 1, OnlineBeatmapID = 75, Ruleset = new OsuRuleset().RulesetInfo });
+            AddStep("online scores", () => scoresContainer.Beatmap = new BeatmapInfo { OnlineBeatmapID = 75, Ruleset = new OsuRuleset().RulesetInfo });
 
 
             scores = new[]

osu.Game.Tests/Visual/TestCasePlaySongSelect.cs

@@ -122,7 +122,6 @@ namespace osu.Game.Tests.Visual
                 Hash = new MemoryStream(Encoding.UTF8.GetBytes(Guid.NewGuid().ToString())).ComputeMD5Hash(),
                 Metadata = new BeatmapMetadata
                 {
-                    OnlineBeatmapSetID = 1234 + i,
                     // Create random metadata, then we can check if sorting works based on these
                     Artist = "MONACA " + RNG.Next(0, 9),
                     Title = "Black Song " + RNG.Next(0, 9),

osu.Game/Beatmaps/BeatmapInfo.cs

@@ -23,7 +23,6 @@ namespace osu.Game.Beatmaps
         public int BeatmapVersion;
 
         private int? onlineBeatmapID;
-        private int? onlineBeatmapSetID;
 
         [JsonProperty("id")]
         public int? OnlineBeatmapID
@@ -32,19 +31,10 @@ namespace osu.Game.Beatmaps
             set { onlineBeatmapID = value > 0 ? value : null; }
         }
 
-        [JsonProperty("beatmapset_id")]
-        [NotMapped]
-        public int? OnlineBeatmapSetID
-        {
-            get { return onlineBeatmapSetID; }
-            set { onlineBeatmapSetID = value > 0 ? value : null; }
-        }
-
         [JsonIgnore]
         public int BeatmapSetInfoID { get; set; }
 
         [Required]
-        [JsonIgnore]
         public BeatmapSetInfo BeatmapSet { get; set; }
 
         public BeatmapMetadata Metadata { get; set; }
@@ -141,8 +131,8 @@ namespace osu.Game.Beatmaps
                                                       (Metadata ?? BeatmapSet.Metadata).AudioFile == (other.Metadata ?? other.BeatmapSet.Metadata).AudioFile;
 
         public bool BackgroundEquals(BeatmapInfo other) => other != null && BeatmapSet != null && other.BeatmapSet != null &&
-                                                      BeatmapSet.Hash == other.BeatmapSet.Hash &&
-                                                      (Metadata ?? BeatmapSet.Metadata).BackgroundFile == (other.Metadata ?? other.BeatmapSet.Metadata).BackgroundFile;
+                                                           BeatmapSet.Hash == other.BeatmapSet.Hash &&
+                                                           (Metadata ?? BeatmapSet.Metadata).BackgroundFile == (other.Metadata ?? other.BeatmapSet.Metadata).BackgroundFile;
 
         /// <summary>
         /// Returns a shallow-clone of this <see cref="BeatmapInfo"/>.

osu.Game/Beatmaps/BeatmapManager.cs

@@ -81,12 +81,31 @@ namespace osu.Game.Beatmaps
 
         protected override void Populate(BeatmapSetInfo model, ArchiveReader archive)
         {
-            model.Beatmaps = createBeatmapDifficulties(model, archive);
+            model.Beatmaps = createBeatmapDifficulties(archive);
 
-            // remove metadata from difficulties where it matches the set
             foreach (BeatmapInfo b in model.Beatmaps)
+            {
+                // remove metadata from difficulties where it matches the set
                 if (model.Metadata.Equals(b.Metadata))
                     b.Metadata = null;
+
+                // by setting the model here, we can update the noline set id below.
+                b.BeatmapSet = model;
+
+                fetchAndPopulateOnlineIDs(b);
+            }
+
+            // check if a set already exists with the same online id, delete if it does.
+            if (model.OnlineBeatmapSetID != null)
+            {
+                var existingOnlineId = beatmaps.ConsumableItems.FirstOrDefault(b => b.OnlineBeatmapSetID == model.OnlineBeatmapSetID);
+                if (existingOnlineId != null)
+                {
+                    Delete(existingOnlineId);
+                    beatmaps.PurgeDeletable(s => s.ID == existingOnlineId.ID);
+                    Logger.Log($"Found existing beatmap set with same OnlineBeatmapSetID ({model.OnlineBeatmapSetID}). It has been purged.", LoggingTarget.Database);
+                }
+            }
         }
 
         protected override BeatmapSetInfo CheckForExisting(BeatmapSetInfo model)
@@ -99,18 +118,6 @@ namespace osu.Game.Beatmaps
                 return existingHashMatch;
             }
 
-            // check if a set already exists with the same online id
-            if (model.OnlineBeatmapSetID != null)
-            {
-                var existingOnlineId = beatmaps.ConsumableItems.FirstOrDefault(b => b.OnlineBeatmapSetID == model.OnlineBeatmapSetID);
-                if (existingOnlineId != null)
-                {
-                    Delete(existingOnlineId);
-                    beatmaps.PurgeDeletable(s => s.ID == existingOnlineId.ID);
-                    Logger.Log($"Found existing beatmap set with same OnlineBeatmapSetID ({model.OnlineBeatmapSetID}). It has been purged.", LoggingTarget.Database);
-                }
-            }
-
             return null;
         }
 
@@ -306,29 +313,29 @@ namespace osu.Game.Beatmaps
             return hashable.ComputeSHA2Hash();
         }
 
-       protected override BeatmapSetInfo CreateModel(ArchiveReader reader)
+        protected override BeatmapSetInfo CreateModel(ArchiveReader reader)
         {
             // let's make sure there are actually .osu files to import.
             string mapName = reader.Filenames.FirstOrDefault(f => f.EndsWith(".osu"));
             if (string.IsNullOrEmpty(mapName)) throw new InvalidOperationException("No beatmap files found in this beatmap archive.");
 
-            BeatmapMetadata metadata;
+            Beatmap beatmap;
             using (var stream = new StreamReader(reader.GetStream(mapName)))
-                metadata = Decoder.GetDecoder<Beatmap>(stream).Decode(stream).Metadata;
+                beatmap = Decoder.GetDecoder<Beatmap>(stream).Decode(stream);
 
             return new BeatmapSetInfo
             {
-                OnlineBeatmapSetID = metadata.OnlineBeatmapSetID,
+                OnlineBeatmapSetID = beatmap.BeatmapInfo.BeatmapSet?.OnlineBeatmapSetID,
                 Beatmaps = new List<BeatmapInfo>(),
                 Hash = computeBeatmapSetHash(reader),
-                Metadata = metadata
+                Metadata = beatmap.Metadata
             };
         }
 
         /// <summary>
         /// Create all required <see cref="BeatmapInfo"/>s for the provided archive.
         /// </summary>
-        private List<BeatmapInfo> createBeatmapDifficulties(BeatmapSetInfo model, ArchiveReader reader)
+        private List<BeatmapInfo> createBeatmapDifficulties(ArchiveReader reader)
         {
             var beatmapInfos = new List<BeatmapInfo>();
 
@@ -348,10 +355,6 @@ namespace osu.Game.Beatmaps
                     beatmap.BeatmapInfo.Hash = ms.ComputeSHA2Hash();
                     beatmap.BeatmapInfo.MD5Hash = ms.ComputeMD5Hash();
 
-                    // ensure we have the same online set ID as the set itself.
-                    beatmap.BeatmapInfo.OnlineBeatmapSetID = model.OnlineBeatmapSetID;
-                    beatmap.BeatmapInfo.Metadata.OnlineBeatmapSetID = model.OnlineBeatmapSetID;
-
                     // check that no existing beatmap exists that is imported with the same online beatmap ID. if so, give it precedence.
                     if (beatmap.BeatmapInfo.OnlineBeatmapID.HasValue && QueryBeatmap(b => b.OnlineBeatmapID.Value == beatmap.BeatmapInfo.OnlineBeatmapID.Value) != null)
                         beatmap.BeatmapInfo.OnlineBeatmapID = null;
@@ -376,6 +379,40 @@ namespace osu.Game.Beatmaps
             return beatmapInfos;
         }
 
+        /// <summary>
+        /// Query the API to populate mising OnlineBeatmapID / OnlineBeatmapSetID properties.
+        /// </summary>
+        /// <param name="beatmap">The beatmap to populate.</param>
+        /// <param name="force">Whether to re-query if the provided beatmap already has populated values.</param>
+        /// <returns>True if population was successful.</returns>
+        private bool fetchAndPopulateOnlineIDs(BeatmapInfo beatmap, bool force = false)
+        {
+            if (!force && beatmap.OnlineBeatmapID != null && beatmap.BeatmapSet.OnlineBeatmapSetID != null)
+                return true;
+
+            Logger.Log("Attempting online lookup for IDs...", LoggingTarget.Database);
+
+            try
+            {
+                var req = new GetBeatmapRequest(beatmap);
+
+                req.Perform(api);
+
+                var res = req.Result;
+
+                Logger.Log($"Successfully mapped to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}.", LoggingTarget.Database);
+
+                beatmap.BeatmapSet.OnlineBeatmapSetID = res.OnlineBeatmapSetID;
+                beatmap.OnlineBeatmapID = res.OnlineBeatmapID;
+                return true;
+            }
+            catch (Exception e)
+            {
+                Logger.Log($"Failed ({e})", LoggingTarget.Database);
+                return false;
+            }
+        }
+
         /// <summary>
         /// A dummy WorkingBeatmap for the purpose of retrieving a beatmap for star difficulty calculation.
         /// </summary>

osu.Game/Beatmaps/BeatmapMetadata.cs

@@ -17,16 +17,6 @@ namespace osu.Game.Beatmaps
         [JsonIgnore]
         public int ID { get; set; }
 
-        private int? onlineBeatmapSetID;
-
-        [NotMapped]
-        [JsonProperty(@"id")]
-        public int? OnlineBeatmapSetID
-        {
-            get { return onlineBeatmapSetID; }
-            set { onlineBeatmapSetID = value > 0 ? value : null; }
-        }
-
         public string Title { get; set; }
         public string TitleUnicode { get; set; }
         public string Artist { get; set; }
@@ -82,8 +72,7 @@ namespace osu.Game.Beatmaps
             if (other == null)
                 return false;
 
-            return onlineBeatmapSetID == other.onlineBeatmapSetID
-                && Title == other.Title
+            return Title == other.Title
                 && TitleUnicode == other.TitleUnicode
                 && Artist == other.Artist
                 && ArtistUnicode == other.ArtistUnicode

osu.Game/Beatmaps/BeatmapSetInfo.cs

@@ -22,18 +22,18 @@ namespace osu.Game.Beatmaps
         [NotMapped]
         public BeatmapSetOnlineInfo OnlineInfo { get; set; }
 
-        public double MaxStarDifficulty => Beatmaps.Max(b => b.StarDifficulty);
+        public double MaxStarDifficulty => Beatmaps?.Max(b => b.StarDifficulty) ?? 0;
 
         [NotMapped]
         public bool DeletePending { get; set; }
 
         public string Hash { get; set; }
 
-        public string StoryboardFile => Files.FirstOrDefault(f => f.Filename.EndsWith(".osb"))?.Filename;
+        public string StoryboardFile => Files?.FirstOrDefault(f => f.Filename.EndsWith(".osb"))?.Filename;
 
         public List<BeatmapSetFileInfo> Files { get; set; }
 
-        public override string ToString() => Metadata.ToString();
+        public override string ToString() => Metadata?.ToString() ?? base.ToString();
 
         public bool Protected { get; set; }
     }

osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs

@@ -34,7 +34,8 @@ namespace osu.Game.Beatmaps.Formats
 
         private readonly int offset;
 
-        public LegacyBeatmapDecoder(int version = LATEST_VERSION) : base(version)
+        public LegacyBeatmapDecoder(int version = LATEST_VERSION)
+            : base(version)
         {
             // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off)
             offset = FormatVersion < 5 ? 24 : 0;
@@ -135,6 +136,7 @@ namespace osu.Game.Beatmaps.Formats
                             parser = new Rulesets.Objects.Legacy.Mania.ConvertHitObjectParser();
                             break;
                     }
+
                     break;
                 case @"LetterboxInBreaks":
                     beatmap.BeatmapInfo.LetterboxInBreaks = int.Parse(pair.Value) == 1;
@@ -207,8 +209,7 @@ namespace osu.Game.Beatmaps.Formats
                     beatmap.BeatmapInfo.OnlineBeatmapID = int.Parse(pair.Value);
                     break;
                 case @"BeatmapSetID":
-                    beatmap.BeatmapInfo.OnlineBeatmapSetID = int.Parse(pair.Value);
-                    metadata.OnlineBeatmapSetID = int.Parse(pair.Value);
+                    beatmap.BeatmapInfo.BeatmapSet = new BeatmapSetInfo { OnlineBeatmapSetID = int.Parse(pair.Value) };
                     break;
             }
         }

osu.Game/Online/API/APIRequest.cs

@@ -14,16 +14,19 @@ namespace osu.Game.Online.API
     {
         protected override WebRequest CreateWebRequest() => new JsonWebRequest<T>(Uri);
 
+        public T Result => ((JsonWebRequest<T>)WebRequest).ResponseObject;
+
         protected APIRequest()
         {
             base.Success += onSuccess;
         }
 
-        private void onSuccess()
-        {
-            Success?.Invoke(((JsonWebRequest<T>)WebRequest).ResponseObject);
-        }
+        private void onSuccess() => Success?.Invoke(Result);
 
+        /// <summary>
+        /// Invoked on successful completion of an API request.
+        /// This will be scheduled to the API's internal scheduler (run on update thread automatically).
+        /// </summary>
         public new event APISuccessHandler<T> Success;
     }
 
@@ -52,7 +55,16 @@ namespace osu.Game.Online.API
         protected APIAccess API;
         protected WebRequest WebRequest;
 
+        /// <summary>
+        /// Invoked on successful completion of an API request.
+        /// This will be scheduled to the API's internal scheduler (run on update thread automatically).
+        /// </summary>
         public event APISuccessHandler Success;
+
+        /// <summary>
+        /// Invoked on failure to complete an API request.
+        /// This will be scheduled to the API's internal scheduler (run on update thread automatically).
+        /// </summary>
         public event APIFailureHandler Failure;
 
         private bool cancelled;

osu.Game/Online/API/Requests/GetBeatmapDetailsRequest.cs

@@ -10,13 +10,11 @@ namespace osu.Game.Online.API.Requests
     {
         private readonly BeatmapInfo beatmap;
 
-        private string lookupString => beatmap.OnlineBeatmapID > 0 ? beatmap.OnlineBeatmapID.ToString() : $@"lookup?checksum={beatmap.Hash}&filename={System.Uri.EscapeUriString(beatmap.Path)}";
-
         public GetBeatmapDetailsRequest(BeatmapInfo beatmap)
         {
             this.beatmap = beatmap;
         }
 
-        protected override string Target => $@"beatmaps/{lookupString}";
+        protected override string Target => $@"beatmaps/{beatmap.OnlineBeatmapID}";
     }
 }

osu.Game/Online/API/Requests/GetBeatmapRequest.cs

@@ -0,0 +1,22 @@
+// Copyright (c) 2007-2018 ppy Pty Ltd <contact@ppy.sh>.
+// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
+
+using osu.Game.Beatmaps;
+using osu.Game.Online.API.Requests.Responses;
+
+namespace osu.Game.Online.API.Requests
+{
+    public class GetBeatmapRequest : APIRequest<APIBeatmap>
+    {
+        private readonly BeatmapInfo beatmap;
+
+        private string lookupString => beatmap.OnlineBeatmapID > 0 ? beatmap.OnlineBeatmapID.ToString() : $@"lookup?checksum={beatmap.MD5Hash}&filename={System.Uri.EscapeUriString(beatmap.Path)}";
+
+        public GetBeatmapRequest(BeatmapInfo beatmap)
+        {
+            this.beatmap = beatmap;
+        }
+
+        protected override string Target => $@"beatmaps/{lookupString}";
+    }
+}

osu.Game/Online/API/Requests/Responses/APIBeatmap.cs

@@ -10,7 +10,10 @@ namespace osu.Game.Online.API.Requests.Responses
     public class APIBeatmap : BeatmapMetadata
     {
         [JsonProperty(@"id")]
-        private int onlineBeatmapID { get; set; }
+        public int OnlineBeatmapID { get; set; }
+
+        [JsonProperty(@"beatmapset_id")]
+        public int OnlineBeatmapSetID { get; set; }
 
         [JsonProperty(@"playcount")]
         private int playCount { get; set; }
@@ -55,7 +58,11 @@ namespace osu.Game.Online.API.Requests.Responses
                 Metadata = this,
                 Ruleset = rulesets.GetRuleset(ruleset),
                 StarDifficulty = starDifficulty,
-                OnlineBeatmapID = onlineBeatmapID,
+                OnlineBeatmapID = OnlineBeatmapID,
+                BeatmapSet = new BeatmapSetInfo
+                {
+                    OnlineBeatmapSetID = OnlineBeatmapSetID,
+                },
                 Version = version,
                 BaseDifficulty = new BeatmapDifficulty
                 {

osu.Game/Online/API/Requests/Responses/APIBeatmapSet.cs

@@ -15,6 +15,15 @@ namespace osu.Game.Online.API.Requests.Responses
         [JsonProperty(@"covers")]
         private BeatmapSetOnlineCovers covers { get; set; }
 
+        private int? onlineBeatmapSetID;
+
+        [JsonProperty(@"id")]
+        public int? OnlineBeatmapSetID
+        {
+            get { return onlineBeatmapSetID; }
+            set { onlineBeatmapSetID = value > 0 ? value : null; }
+        }
+
         [JsonProperty(@"preview_url")]
         private string preview { get; set; }
 

osu.Game/Online/API/Requests/Responses/APIUserMostPlayedBeatmap.cs

@@ -25,7 +25,6 @@ namespace osu.Game.Online.API.Requests.Responses
         {
             BeatmapSetInfo setInfo = beatmapSet.ToBeatmapSet(rulesets);
             beatmap.BeatmapSet = setInfo;
-            beatmap.OnlineBeatmapSetID = setInfo.OnlineBeatmapSetID;
             beatmap.Metadata = setInfo.Metadata;
             return beatmap;
         }

osu.Game/Overlays/Profile/Sections/BeatmapMetadataContainer.cs

@@ -32,7 +32,7 @@ namespace osu.Game.Overlays.Profile.Sections
         {
             Action = () =>
             {
-                if (beatmap.OnlineBeatmapSetID.HasValue) beatmapSetOverlay?.FetchAndShowBeatmapSet(beatmap.OnlineBeatmapSetID.Value);
+                if (beatmap.BeatmapSet?.OnlineBeatmapSetID != null) beatmapSetOverlay?.FetchAndShowBeatmapSet(beatmap.BeatmapSet.OnlineBeatmapSetID.Value);
             };
 
             Child = new FillFlowContainer