From abfb2c00bcaaf071b264f82e5205350ec7a5edf4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Mon, 6 May 2024 13:24:34 +0200
Subject: [PATCH] Look up users by ID if available when importing scores

---
 osu.Game/Scoring/ScoreImporter.cs | 71 ++++++++++++++++++++++++++-----
 1 file changed, 61 insertions(+), 10 deletions(-)

diff --git a/osu.Game/Scoring/ScoreImporter.cs b/osu.Game/Scoring/ScoreImporter.cs
index 4ae8e51f6d..69c53af16f 100644
--- a/osu.Game/Scoring/ScoreImporter.cs
+++ b/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,24 +135,34 @@ private void populateUserDetails(ScoreInfo model)
             if (model.RealmUser.OnlineID == APIUser.SYSTEM_USER_ID)
                 return;
 
-            if (model.OnlineID < 0 && model.LegacyOnlineID <= 0)
-                return;
-
-            string username = model.RealmUser.Username;
-
-            if (usernameLookupCache.TryGetValue(username, out var existing))
+            if (model.RealmUser.OnlineID > 1)
             {
-                model.User = existing;
+                model.User = lookupUserById(model.RealmUser.OnlineID) ?? model.User;
                 return;
             }
 
-            var userRequest = new GetUserRequest(username);
+            if (model.OnlineID < 0 && model.LegacyOnlineID <= 0)
+                return;
+
+            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)
             {
-                usernameLookupCache.TryAdd(username, new APIUser
+                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
@@ -154,8 +172,41 @@ private void populateUserDetails(ScoreInfo model)
                     CountryCode = user.CountryCode,
                 });
 
-                model.User = user;
+                return cachedUser;
             }
+
+            return null;
+        }
+
+        private APIUser? lookupUserByName(string username)
+        {
+            if (usernameLookupCache.TryGetValue(username, out var existing))
+            {
+                return existing;
+            }
+
+            var userRequest = new GetUserRequest(username);
+
+            api.Perform(userRequest);
+
+            if (userRequest.Response is APIUser user)
+            {
+                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
+                    // in realm, this should be undone.
+                    Id = user.Id,
+                    Username = user.Username,
+                    CountryCode = user.CountryCode,
+                });
+
+                return cachedUser;
+            }
+
+            return null;
         }
     }
 }