Merge pull request ppy#28119 from bdach/user-id-in-replays

Persist user ID to replay data

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

@@ -14,6 +14,7 @@
 using osu.Game.Beatmaps.Formats;
 using osu.Game.Beatmaps.Legacy;
 using osu.Game.IO.Legacy;
+using osu.Game.Online.API.Requests.Responses;
 using osu.Game.Replays;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Catch;
@@ -31,6 +32,7 @@
 using osu.Game.Scoring;
 using osu.Game.Scoring.Legacy;
 using osu.Game.Tests.Resources;
+using osu.Game.Users;
 
 namespace osu.Game.Tests.Beatmaps.Formats
 {
@@ -224,6 +226,12 @@ public void TestSoloScoreData()
                 new OsuModDoubleTime { SpeedChange = { Value = 1.1 } }
             };
             scoreInfo.OnlineID = 123123;
+            scoreInfo.User = new APIUser
+            {
+                Username = "spaceman_atlas",
+                Id = 3035836,
+                CountryCode = CountryCode.PL
+            };
             scoreInfo.ClientVersion = "2023.1221.0";
 
             var beatmap = new TestBeatmap(ruleset);
@@ -248,6 +256,7 @@ public void TestSoloScoreData()
                 Assert.That(decodedAfterEncode.ScoreInfo.MaximumStatistics, Is.EqualTo(scoreInfo.MaximumStatistics));
                 Assert.That(decodedAfterEncode.ScoreInfo.Mods, Is.EqualTo(scoreInfo.Mods));
                 Assert.That(decodedAfterEncode.ScoreInfo.ClientVersion, Is.EqualTo("2023.1221.0"));
+                Assert.That(decodedAfterEncode.ScoreInfo.RealmUser.OnlineID, Is.EqualTo(3035836));
             });
         }
 

osu.Game.Tests/Scores/IO/ImportScoreTest.cs

@@ -287,7 +287,7 @@ public void TestOnlineScoreIsAvailableLocally()
         }
 
         [Test]
-        public void TestUserLookedUpForOnlineScore()
+        public void TestUserLookedUpByUsernameForOnlineScoreIfUserIDMissing()
         {
             using (HeadlessGameHost host = new CleanRunHeadlessGameHost())
             {
@@ -301,6 +301,9 @@ public void TestUserLookedUpForOnlineScore()
                         switch (req)
                         {
                             case GetUserRequest userRequest:
+                                if (userRequest.Lookup != "Test user")
+                                    return false;
+
                                 userRequest.TriggerSuccess(new APIUser
                                 {
                                     Username = "Test user",
@@ -350,7 +353,7 @@ public void TestUserLookedUpForOnlineScore()
         }
 
         [Test]
-        public void TestUserLookedUpForLegacyOnlineScore()
+        public void TestUserLookedUpByUsernameForLegacyOnlineScore()
         {
             using (HeadlessGameHost host = new CleanRunHeadlessGameHost())
             {
@@ -364,6 +367,9 @@ public void TestUserLookedUpForLegacyOnlineScore()
                         switch (req)
                         {
                             case GetUserRequest userRequest:
+                                if (userRequest.Lookup != "Test user")
+                                    return false;
+
                                 userRequest.TriggerSuccess(new APIUser
                                 {
                                     Username = "Test user",
@@ -413,7 +419,7 @@ public void TestUserLookedUpForLegacyOnlineScore()
         }
 
         [Test]
-        public void TestUserNotLookedUpForOfflineScore()
+        public void TestUserNotLookedUpForOfflineScoreIfUserIDMissing()
         {
             using (HeadlessGameHost host = new CleanRunHeadlessGameHost())
             {
@@ -427,6 +433,9 @@ public void TestUserNotLookedUpForOfflineScore()
                         switch (req)
                         {
                             case GetUserRequest userRequest:
+                                if (userRequest.Lookup != "Test user")
+                                    return false;
+
                                 userRequest.TriggerSuccess(new APIUser
                                 {
                                     Username = "Test user",
@@ -476,6 +485,73 @@ public void TestUserNotLookedUpForOfflineScore()
             }
         }
 
+        [Test]
+        public void TestUserLookedUpByOnlineIDIfPresent([Values] bool isOnlineScore)
+        {
+            using (HeadlessGameHost host = new CleanRunHeadlessGameHost())
+            {
+                try
+                {
+                    var osu = LoadOsuIntoHost(host, true);
+
+                    var api = (DummyAPIAccess)osu.API;
+                    api.HandleRequest = req =>
+                    {
+                        switch (req)
+                        {
+                            case GetUserRequest userRequest:
+                                if (userRequest.Lookup != "5555")
+                                    return false;
+
+                                userRequest.TriggerSuccess(new APIUser
+                                {
+                                    Username = "Some other guy",
+                                    CountryCode = CountryCode.DE,
+                                    Id = 5555
+                                });
+                                return true;
+
+                            default:
+                                return false;
+                        }
+                    };
+
+                    var beatmap = BeatmapImportHelper.LoadOszIntoOsu(osu, TestResources.GetQuickTestBeatmapForImport()).GetResultSafely();
+
+                    var toImport = new ScoreInfo
+                    {
+                        Rank = ScoreRank.B,
+                        TotalScore = 987654,
+                        Accuracy = 0.8,
+                        MaxCombo = 500,
+                        Combo = 250,
+                        User = new APIUser { Id = 5555 },
+                        Date = DateTimeOffset.Now,
+                        Ruleset = new OsuRuleset().RulesetInfo,
+                        BeatmapInfo = beatmap.Beatmaps.First()
+                    };
+                    if (isOnlineScore)
+                        toImport.OnlineID = 12345;
+
+                    var imported = LoadScoreIntoOsu(osu, toImport);
+
+                    Assert.AreEqual(toImport.Rank, imported.Rank);
+                    Assert.AreEqual(toImport.TotalScore, imported.TotalScore);
+                    Assert.AreEqual(toImport.Accuracy, imported.Accuracy);
+                    Assert.AreEqual(toImport.MaxCombo, imported.MaxCombo);
+                    Assert.AreEqual(toImport.Date, imported.Date);
+                    Assert.AreEqual(toImport.OnlineID, imported.OnlineID);
+                    Assert.AreEqual("Some other guy", imported.RealmUser.Username);
+                    Assert.AreEqual(5555, imported.RealmUser.OnlineID);
+                    Assert.AreEqual(CountryCode.DE, imported.RealmUser.CountryCode);
+                }
+                finally
+                {
+                    host.Exit();
+                }
+            }
+        }
+
         public static ScoreInfo LoadScoreIntoOsu(OsuGameBase osu, ScoreInfo score, ArchiveReader archive = null)
         {
             // clone to avoid attaching the input score to realm.

osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs

@@ -43,6 +43,9 @@ public class LegacyReplaySoloScoreInfo
         [JsonConverter(typeof(StringEnumConverter))]
         public ScoreRank? Rank;
 
+        [JsonProperty("user_id")]
+        public int UserID = -1;
+
         public static LegacyReplaySoloScoreInfo FromScore(ScoreInfo score) => new LegacyReplaySoloScoreInfo
         {
             OnlineID = score.OnlineID,
@@ -51,6 +54,7 @@ public class LegacyReplaySoloScoreInfo
             MaximumStatistics = score.MaximumStatistics.Where(kvp => kvp.Value != 0).ToDictionary(),
             ClientVersion = score.ClientVersion,
             Rank = score.Rank,
+            UserID = score.User.OnlineID,
         };
     }
 }

osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs

@@ -131,6 +131,8 @@ public Score Parse(Stream stream)
                         score.ScoreInfo.Mods = readScore.Mods.Select(m => m.ToMod(currentRuleset)).ToArray();
                         score.ScoreInfo.ClientVersion = readScore.ClientVersion;
                         decodedRank = readScore.Rank;
+                        if (readScore.UserID > 1)
+                            score.ScoreInfo.RealmUser.OnlineID = readScore.UserID;
                     });
                 }
             }

osu.Game/Scoring/ScoreImporter.cs

@@ -103,6 +103,14 @@ protected override void Populate(ScoreInfo model, ArchiveReader? archive, Realm
         }
 
         // Very naive local caching to improve performance of large score imports (where the username is usually the same for most or all scores).
+
+        // TODO: `UserLookupCache` cannot currently be used here because of async foibles.
+        // It only supports lookups by user ID (username would require web changes), and even then the ID lookups cannot be used.
+        // That is because that component provides an async interface, and async functions cannot be consumed safely here due to the rigid structure of `RealmArchiveModelImporter`.
+        // The importer has two paths, one async and one sync; the async path runs the sync path in a task.
+        // This means that sometimes `PostImport()` is called from a sync context, and sometimes from an async one, whilst itself being a sync method.
+        // That in turn makes `.GetResultSafely()` not callable inside `PostImport()`, as it will throw when called from an async context,
+        private readonly Dictionary<int, APIUser> idLookupCache = new Dictionary<int, APIUser>();
         private readonly Dictionary<string, APIUser> usernameLookupCache = new Dictionary<string, APIUser>();
 
         protected override void PostImport(ScoreInfo model, Realm realm, ImportParameters parameters)
@@ -127,15 +135,54 @@ private void populateUserDetails(ScoreInfo model)
             if (model.RealmUser.OnlineID == APIUser.SYSTEM_USER_ID)
                 return;
 
+            if (model.RealmUser.OnlineID > 1)
+            {
+                model.User = lookupUserById(model.RealmUser.OnlineID) ?? model.User;
+                return;
+            }
+
             if (model.OnlineID < 0 && model.LegacyOnlineID <= 0)
                 return;
 
-            string username = model.RealmUser.Username;
+            model.User = lookupUserByName(model.RealmUser.Username) ?? model.User;
+        }
 
+        private APIUser? lookupUserById(int id)
+        {
+            if (idLookupCache.TryGetValue(id, out var existing))
+            {
+                return existing;
+            }
+
+            var userRequest = new GetUserRequest(id);
+
+            api.Perform(userRequest);
+
+            if (userRequest.Response is APIUser user)
+            {
+                APIUser cachedUser;
+
+                idLookupCache.TryAdd(id, cachedUser = new APIUser
+                {
+                    // Because this is a permanent cache, let's only store the pieces we're interested in,
+                    // rather than the full API response. If we start to store more than these three fields
+                    // in realm, this should be undone.
+                    Id = user.Id,
+                    Username = user.Username,
+                    CountryCode = user.CountryCode,
+                });
+
+                return cachedUser;
+            }
+
+            return null;
+        }
+
+        private APIUser? lookupUserByName(string username)
+        {
             if (usernameLookupCache.TryGetValue(username, out var existing))
             {
-                model.User = existing;
-                return;
+                return existing;
             }
 
             var userRequest = new GetUserRequest(username);
@@ -144,7 +191,9 @@ private void populateUserDetails(ScoreInfo model)
 
             if (userRequest.Response is APIUser user)
             {
-                usernameLookupCache.TryAdd(username, new APIUser
+                APIUser cachedUser;
+
+                usernameLookupCache.TryAdd(username, cachedUser = new APIUser
                 {
                     // Because this is a permanent cache, let's only store the pieces we're interested in,
                     // rather than the full API response. If we start to store more than these three fields
@@ -154,8 +203,10 @@ private void populateUserDetails(ScoreInfo model)
                     CountryCode = user.CountryCode,
                 });
 
-                model.User = user;
+                return cachedUser;
             }
+
+            return null;
         }
     }
 }