From 90c313e2ad4d67d827f5617feacba4b1a693fb12 Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sun, 29 Aug 2021 19:19:55 +0100 Subject: [PATCH 01/16] add methods to get a user from their username --- osu.Game/Online/API/Requests/GetUserRequest.cs | 16 +++++++++++++--- osu.Game/OsuGame.cs | 9 +++++++-- osu.Game/Overlays/UserProfileOverlay.cs | 4 +++- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/osu.Game/Online/API/Requests/GetUserRequest.cs b/osu.Game/Online/API/Requests/GetUserRequest.cs index 42aad6f9eb..48041cd40c 100644 --- a/osu.Game/Online/API/Requests/GetUserRequest.cs +++ b/osu.Game/Online/API/Requests/GetUserRequest.cs @@ -8,15 +8,25 @@ namespace osu.Game.Online.API.Requests { public class GetUserRequest : APIRequest { - private readonly long? userId; + private readonly string userIdentifier; public readonly RulesetInfo Ruleset; + public GetUserRequest() + { + } + public GetUserRequest(long? userId = null, RulesetInfo ruleset = null) { - this.userId = userId; + this.userIdentifier = userId.ToString(); Ruleset = ruleset; } - protected override string Target => userId.HasValue ? $@"users/{userId}/{Ruleset?.ShortName}" : $@"me/{Ruleset?.ShortName}"; + public GetUserRequest(string username = null, RulesetInfo ruleset = null) + { + this.userIdentifier = username; + Ruleset = ruleset; + } + + protected override string Target => (userIdentifier != null) ? $@"users/{userIdentifier}/{Ruleset?.ShortName}" : $@"me/{Ruleset?.ShortName}"; } } diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 4d952c39c6..26fa1d5a4c 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -329,8 +329,7 @@ namespace osu.Game break; case LinkAction.OpenUserProfile: - if (int.TryParse(link.Argument, out int userId)) - ShowUser(userId); + ShowUser(link.Argument); break; case LinkAction.OpenWiki: @@ -378,6 +377,12 @@ namespace osu.Game /// The user to display. public void ShowUser(int userId) => waitForReady(() => userProfile, _ => userProfile.ShowUser(userId)); + /// + /// Show a user's profile as an overlay. + /// + /// The user to display. + public void ShowUser(string username) => waitForReady(() => userProfile, _ => userProfile.ShowUser(username)); + /// /// Show a beatmap's set as an overlay, displaying the given beatmap. /// diff --git a/osu.Game/Overlays/UserProfileOverlay.cs b/osu.Game/Overlays/UserProfileOverlay.cs index 299a14b250..6e74acc96a 100644 --- a/osu.Game/Overlays/UserProfileOverlay.cs +++ b/osu.Game/Overlays/UserProfileOverlay.cs @@ -40,6 +40,8 @@ namespace osu.Game.Overlays public void ShowUser(int userId) => ShowUser(new User { Id = userId }); + public void ShowUser(string username) => ShowUser(new User { Username = username }); + public void ShowUser(User user, bool fetchOnline = true) { if (user == User.SYSTEM_USER) @@ -116,7 +118,7 @@ namespace osu.Game.Overlays if (fetchOnline) { - userReq = new GetUserRequest(user.Id); + userReq = user.Username != null ? new GetUserRequest(user.Username) : new GetUserRequest(user.Id); userReq.Success += userLoadComplete; API.Queue(userReq); } From 1aeae2b8c8537927bc9c93553abddf3c1c935b24 Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Mon, 30 Aug 2021 10:11:41 +0100 Subject: [PATCH 02/16] reverse ternary operator --- osu.Game/Overlays/UserProfileOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/UserProfileOverlay.cs b/osu.Game/Overlays/UserProfileOverlay.cs index 6e74acc96a..b0327987f2 100644 --- a/osu.Game/Overlays/UserProfileOverlay.cs +++ b/osu.Game/Overlays/UserProfileOverlay.cs @@ -118,7 +118,7 @@ namespace osu.Game.Overlays if (fetchOnline) { - userReq = user.Username != null ? new GetUserRequest(user.Username) : new GetUserRequest(user.Id); + userReq = user.Id > 1 ? new GetUserRequest(user.Id) : new GetUserRequest(user.Username); userReq.Success += userLoadComplete; API.Queue(userReq); } From c789163d01e367f867e8f4a754516c6e895ade24 Mon Sep 17 00:00:00 2001 From: rednir Date: Mon, 30 Aug 2021 13:22:12 +0100 Subject: [PATCH 03/16] use user ID overload when its supposed to be used Co-authored-by: Salman Ahmed --- osu.Game/OsuGame.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 26fa1d5a4c..1e6e1e0ead 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -329,7 +329,11 @@ namespace osu.Game break; case LinkAction.OpenUserProfile: - ShowUser(link.Argument); + if (int.TryParse(link.Argument, out var userId)) + ShowUser(userId); + else + ShowUser(link.Argument); + break; case LinkAction.OpenWiki: From 8104b15874c5b358c22f7d3f8d6fb9ac42b09b94 Mon Sep 17 00:00:00 2001 From: rednir Date: Mon, 30 Aug 2021 13:23:33 +0100 Subject: [PATCH 04/16] remove braces Co-authored-by: Salman Ahmed --- osu.Game/Online/API/Requests/GetUserRequest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/API/Requests/GetUserRequest.cs b/osu.Game/Online/API/Requests/GetUserRequest.cs index 48041cd40c..0c8a4a3578 100644 --- a/osu.Game/Online/API/Requests/GetUserRequest.cs +++ b/osu.Game/Online/API/Requests/GetUserRequest.cs @@ -27,6 +27,6 @@ namespace osu.Game.Online.API.Requests Ruleset = ruleset; } - protected override string Target => (userIdentifier != null) ? $@"users/{userIdentifier}/{Ruleset?.ShortName}" : $@"me/{Ruleset?.ShortName}"; + protected override string Target => userIdentifier != null ? $@"users/{userIdentifier}/{Ruleset?.ShortName}" : $@"me/{Ruleset?.ShortName}"; } } From 79f71e5181eb5410090769117175a03f094a265e Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Tue, 31 Aug 2021 13:56:44 +0100 Subject: [PATCH 05/16] get user id when importing scores --- osu.Game/Scoring/ScoreManager.cs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 83bcac01ac..4cfcc00bb8 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -23,6 +23,7 @@ using osu.Game.Rulesets; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Scoring; using osu.Game.Scoring.Legacy; +using osu.Game.Users; namespace osu.Game.Scoring { @@ -43,6 +44,8 @@ namespace osu.Game.Scoring [CanBeNull] private readonly OsuConfigManager configManager; + private IAPIProvider api { get; set; } + public ScoreManager(RulesetStore rulesets, Func beatmaps, Storage storage, IAPIProvider api, IDatabaseContextFactory contextFactory, IIpcHost importHost = null, Func difficulties = null, OsuConfigManager configManager = null) : base(storage, contextFactory, api, new ScoreStore(contextFactory, storage), importHost) @@ -51,6 +54,7 @@ namespace osu.Game.Scoring this.beatmaps = beatmaps; this.difficulties = difficulties; this.configManager = configManager; + this.api = api; } protected override ScoreInfo CreateModel(ArchiveReader archive) @@ -72,8 +76,31 @@ namespace osu.Game.Scoring } } + private Dictionary previouslyLookedUpUsernames = new Dictionary(); + protected override Task Populate(ScoreInfo model, ArchiveReader archive, CancellationToken cancellationToken = default) - => Task.CompletedTask; + { + // These scores only provide the user's username but we need the user's ID too. + if (model.UserID <= 1 && model.UserString != null) + { + if (previouslyLookedUpUsernames.TryGetValue(model.UserString, out User user)) + { + model.UserID = user.Id; + return Task.CompletedTask; + } + + var request = new GetUserRequest(model.UserString); + request.Success += user => + { + model.UserID = user.Id; + previouslyLookedUpUsernames.TryAdd(model.UserString, user); + }; + + api.Queue(request); + } + + return Task.CompletedTask; + } protected override void ExportModelTo(ScoreInfo model, Stream outputStream) { From 0a87b461d70ad61e423675d45f04e253bae73259 Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Tue, 31 Aug 2021 14:11:37 +0100 Subject: [PATCH 06/16] fix code quality issues --- osu.Game/Scoring/ScoreManager.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 4cfcc00bb8..d5d33283b3 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -76,7 +76,7 @@ namespace osu.Game.Scoring } } - private Dictionary previouslyLookedUpUsernames = new Dictionary(); + private readonly Dictionary previouslyLookedUpUsernames = new Dictionary(); protected override Task Populate(ScoreInfo model, ArchiveReader archive, CancellationToken cancellationToken = default) { @@ -90,10 +90,10 @@ namespace osu.Game.Scoring } var request = new GetUserRequest(model.UserString); - request.Success += user => + request.Success += u => { - model.UserID = user.Id; - previouslyLookedUpUsernames.TryAdd(model.UserString, user); + model.UserID = u.Id; + previouslyLookedUpUsernames.TryAdd(model.UserString, u); }; api.Queue(request); From 9288ca1191dfbbcf0099ff749890580cc050e27f Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Tue, 31 Aug 2021 14:34:45 +0100 Subject: [PATCH 07/16] handle api is null --- osu.Game/Scoring/ScoreManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index d5d33283b3..3c99dd6637 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -96,7 +96,7 @@ namespace osu.Game.Scoring previouslyLookedUpUsernames.TryAdd(model.UserString, u); }; - api.Queue(request); + api?.Queue(request); } return Task.CompletedTask; From 4c006333e0298f0998a1a0044df978c521e52dec Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sat, 4 Sep 2021 19:42:14 +0100 Subject: [PATCH 08/16] add /chat command --- osu.Game/Online/Chat/ChannelManager.cs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 1937019ef6..fb8c90a80c 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -256,8 +256,21 @@ namespace osu.Game.Online.Chat JoinChannel(channel); break; + case "chat": + if (string.IsNullOrWhiteSpace(content)) + { + target.AddNewMessages(new ErrorMessage("Usage: /chat [user]")); + break; + } + + var request = new GetUserRequest(content); + request.Success += u => OpenPrivateChannel(u); + request.Failure += _ => target.AddNewMessages(new ErrorMessage("User not found.")); + api.Queue(request); + break; + case "help": - target.AddNewMessages(new InfoMessage("Supported commands: /help, /me [action], /join [channel], /np")); + target.AddNewMessages(new InfoMessage("Supported commands: /help, /me [action], /join [channel], /chat [user], /np")); break; default: From ea3be927d7f5c8071bacded41a021340b6a6c5e2 Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sat, 4 Sep 2021 20:02:10 +0100 Subject: [PATCH 09/16] convert to method group --- osu.Game/Online/Chat/ChannelManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index fb8c90a80c..f58f1ff40c 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -264,7 +264,7 @@ namespace osu.Game.Online.Chat } var request = new GetUserRequest(content); - request.Success += u => OpenPrivateChannel(u); + request.Success += OpenPrivateChannel; request.Failure += _ => target.AddNewMessages(new ErrorMessage("User not found.")); api.Queue(request); break; From 9aa1564e0de3482c29e9b07927090df4bf0ebd75 Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sun, 5 Sep 2021 10:19:04 +0100 Subject: [PATCH 10/16] revert ChannelManager changes --- osu.Game/Online/Chat/ChannelManager.cs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index f58f1ff40c..bf411d59f6 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -256,21 +256,8 @@ namespace osu.Game.Online.Chat JoinChannel(channel); break; - case "chat": - if (string.IsNullOrWhiteSpace(content)) - { - target.AddNewMessages(new ErrorMessage("Usage: /chat [user]")); - break; - } - - var request = new GetUserRequest(content); - request.Success += OpenPrivateChannel; - request.Failure += _ => target.AddNewMessages(new ErrorMessage("User not found.")); - api.Queue(request); - break; - case "help": - target.AddNewMessages(new InfoMessage("Supported commands: /help, /me [action], /join [channel], /chat [user], /np")); + target.AddNewMessages(new InfoMessage("Supported commands: /help, /me [action], /join [channel], /np")); break; default: @@ -614,4 +601,4 @@ namespace osu.Game.Online.Chat : channel.Id == Id; } } -} +} \ No newline at end of file From e409f2dc6d28257b822ffa06230377c80402dae0 Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sun, 5 Sep 2021 10:42:38 +0100 Subject: [PATCH 11/16] add xmldoc --- osu.Game/Online/API/Requests/GetUserRequest.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/osu.Game/Online/API/Requests/GetUserRequest.cs b/osu.Game/Online/API/Requests/GetUserRequest.cs index 0c8a4a3578..9470c77b79 100644 --- a/osu.Game/Online/API/Requests/GetUserRequest.cs +++ b/osu.Game/Online/API/Requests/GetUserRequest.cs @@ -11,16 +11,30 @@ namespace osu.Game.Online.API.Requests private readonly string userIdentifier; public readonly RulesetInfo Ruleset; + + /// + /// Gets the currently logged-in user. + /// public GetUserRequest() { } + /// + /// Gets a user from their ID. + /// + /// The user to get. + /// The ruleset to get the user's info for. public GetUserRequest(long? userId = null, RulesetInfo ruleset = null) { this.userIdentifier = userId.ToString(); Ruleset = ruleset; } + /// + /// Gets a user from their username. + /// + /// The user to get. + /// The ruleset to get the user's info for. public GetUserRequest(string username = null, RulesetInfo ruleset = null) { this.userIdentifier = username; From e5f886a3158e25181b5c047b9e596f88a72332fb Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sun, 5 Sep 2021 10:45:38 +0100 Subject: [PATCH 12/16] revert unnecessary change --- osu.Game/OsuGame.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 1e6e1e0ead..a83357f4f5 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -329,7 +329,7 @@ namespace osu.Game break; case LinkAction.OpenUserProfile: - if (int.TryParse(link.Argument, out var userId)) + if (int.TryParse(link.Argument, out int userId)) ShowUser(userId); else ShowUser(link.Argument); From f7369e0d682cc27e0e210e1135b1959abc4e1058 Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sun, 5 Sep 2021 14:47:46 +0100 Subject: [PATCH 13/16] create UserIdLookupCache to get user ID when importing scores --- osu.Game/OsuGameBase.cs | 2 +- osu.Game/Scoring/ScoreManager.cs | 35 ++++---- .../Scoring/ScoreManager_UserIdLookupCache.cs | 85 +++++++++++++++++++ 3 files changed, 102 insertions(+), 20 deletions(-) create mode 100644 osu.Game/Scoring/ScoreManager_UserIdLookupCache.cs diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index f2d575550a..484cc23161 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -263,7 +263,7 @@ namespace osu.Game dependencies.Cache(fileStore = new FileStore(contextFactory, Storage)); // ordering is important here to ensure foreign keys rules are not broken in ModelStore.Cleanup() - dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, contextFactory, Host, () => difficultyCache, LocalConfig)); + dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, contextFactory, Host, () => difficultyCache, LocalConfig, true)); dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, contextFactory, RulesetStore, API, Audio, Resources, Host, defaultBeatmap, true)); // this should likely be moved to ArchiveModelManager when another case appears where it is necessary diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 3c99dd6637..fcf214268a 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -10,6 +10,7 @@ using System.Threading; using System.Threading.Tasks; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore; +using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Logging; using osu.Framework.Platform; @@ -27,7 +28,7 @@ using osu.Game.Users; namespace osu.Game.Scoring { - public class ScoreManager : DownloadableArchiveModelManager + public partial class ScoreManager : DownloadableArchiveModelManager { public override IEnumerable HandledExtensions => new[] { ".osr" }; @@ -44,10 +45,13 @@ namespace osu.Game.Scoring [CanBeNull] private readonly OsuConfigManager configManager; + [CanBeNull] + private readonly UserIdLookupCache userIdLookupCache; + private IAPIProvider api { get; set; } public ScoreManager(RulesetStore rulesets, Func beatmaps, Storage storage, IAPIProvider api, IDatabaseContextFactory contextFactory, IIpcHost importHost = null, - Func difficulties = null, OsuConfigManager configManager = null) + Func difficulties = null, OsuConfigManager configManager = null, bool performOnlineLookups = false) : base(storage, contextFactory, api, new ScoreStore(contextFactory, storage), importHost) { this.rulesets = rulesets; @@ -55,6 +59,9 @@ namespace osu.Game.Scoring this.difficulties = difficulties; this.configManager = configManager; this.api = api; + + if (performOnlineLookups) + userIdLookupCache = new UserIdLookupCache(api); } protected override ScoreInfo CreateModel(ArchiveReader archive) @@ -76,30 +83,20 @@ namespace osu.Game.Scoring } } - private readonly Dictionary previouslyLookedUpUsernames = new Dictionary(); - - protected override Task Populate(ScoreInfo model, ArchiveReader archive, CancellationToken cancellationToken = default) + protected override async Task Populate(ScoreInfo model, ArchiveReader archive, CancellationToken cancellationToken = default) { // These scores only provide the user's username but we need the user's ID too. - if (model.UserID <= 1 && model.UserString != null) + if (model.UserID <= 1 && model.UserString != null && userIdLookupCache != null) { - if (previouslyLookedUpUsernames.TryGetValue(model.UserString, out User user)) + try { - model.UserID = user.Id; - return Task.CompletedTask; + model.UserID = await userIdLookupCache.GetUserIdAsync(model.UserString, cancellationToken).ConfigureAwait(false); } - - var request = new GetUserRequest(model.UserString); - request.Success += u => + catch (Exception e) { - model.UserID = u.Id; - previouslyLookedUpUsernames.TryAdd(model.UserString, u); - }; - - api?.Queue(request); + LogForModel(model, $"Online retrieval failed for {model.User} ({e.Message})", e); + } } - - return Task.CompletedTask; } protected override void ExportModelTo(ScoreInfo model, Stream outputStream) diff --git a/osu.Game/Scoring/ScoreManager_UserIdLookupCache.cs b/osu.Game/Scoring/ScoreManager_UserIdLookupCache.cs new file mode 100644 index 0000000000..dc7e244f14 --- /dev/null +++ b/osu.Game/Scoring/ScoreManager_UserIdLookupCache.cs @@ -0,0 +1,85 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using osu.Game.Online.API; +using osu.Game.Online.API.Requests; +using osu.Game.Database; + +namespace osu.Game.Scoring +{ + public partial class ScoreManager + { + private class UserIdLookupCache : MemoryCachingComponent + { + private readonly IAPIProvider api; + + public UserIdLookupCache(IAPIProvider api) + { + this.api = api; + } + + /// + /// Perform an API lookup on the specified username, returning the associated ID. + /// + /// The username to lookup. + /// An optional cancellation token. + /// The user ID, or 1 if the user does not exist or the request could not be satisfied. + public Task GetUserIdAsync(string username, CancellationToken token = default) => GetAsync(username, token); + + protected override async Task ComputeValueAsync(string lookup, CancellationToken token = default) + => await queryUserId(lookup).ConfigureAwait(false); + + private readonly Queue<(string username, TaskCompletionSource)> pendingUserTasks = new Queue<(string, TaskCompletionSource)>(); + private Task pendingRequestTask; + private readonly object taskAssignmentLock = new object(); + + private Task queryUserId(string username) + { + lock (taskAssignmentLock) + { + var tcs = new TaskCompletionSource(); + + // Add to the queue. + pendingUserTasks.Enqueue((username, tcs)); + + // Create a request task if there's not already one. + if (pendingRequestTask == null) + createNewTask(); + + return tcs.Task; + } + } + + private void performLookup() + { + (string username, TaskCompletionSource task) next; + + lock (taskAssignmentLock) + { + next = pendingUserTasks.Dequeue(); + } + + var request = new GetUserRequest(next.username); + + // rather than queueing, we maintain our own single-threaded request stream. + // todo: we probably want retry logic here. + api.Perform(request); + + // Create a new request task if there's still more users to query. + lock (taskAssignmentLock) + { + pendingRequestTask = null; + if (pendingUserTasks.Count > 0) + createNewTask(); + } + + next.task.SetResult(request.Result?.Id ?? 1); + } + + private void createNewTask() => pendingRequestTask = Task.Run(performLookup); + } + } +} From 7f9b80e3e5da3c9e362fe028f90d5bfdb5a9e2dc Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sun, 5 Sep 2021 15:11:41 +0100 Subject: [PATCH 14/16] add tests for ShowUser() username overload --- osu.Game.Tests/Visual/Online/TestSceneUserProfileOverlay.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneUserProfileOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneUserProfileOverlay.cs index 03d079261d..70271b0b08 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneUserProfileOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneUserProfileOverlay.cs @@ -104,6 +104,9 @@ namespace osu.Game.Tests.Visual.Online CoverUrl = @"https://osu.ppy.sh/images/headers/profile-covers/c4.jpg" }, api.IsLoggedIn)); + AddStep("Show ppy from username", () => profile.ShowUser(@"peppy")); + AddStep("Show flyte from username", () => profile.ShowUser(@"flyte")); + AddStep("Hide", profile.Hide); AddStep("Show without reload", profile.Show); } From e78dc1bb4ce6c8d06792bb0c3e2b7042eb33d44f Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sun, 5 Sep 2021 15:27:28 +0100 Subject: [PATCH 15/16] more code quality :/ --- osu.Game/Online/API/Requests/GetUserRequest.cs | 1 - osu.Game/Scoring/ScoreManager.cs | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/osu.Game/Online/API/Requests/GetUserRequest.cs b/osu.Game/Online/API/Requests/GetUserRequest.cs index 9470c77b79..281926c096 100644 --- a/osu.Game/Online/API/Requests/GetUserRequest.cs +++ b/osu.Game/Online/API/Requests/GetUserRequest.cs @@ -11,7 +11,6 @@ namespace osu.Game.Online.API.Requests private readonly string userIdentifier; public readonly RulesetInfo Ruleset; - /// /// Gets the currently logged-in user. /// diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index fcf214268a..ccc5579ee5 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -10,7 +10,6 @@ using System.Threading; using System.Threading.Tasks; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore; -using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Logging; using osu.Framework.Platform; @@ -24,7 +23,6 @@ using osu.Game.Rulesets; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Scoring; using osu.Game.Scoring.Legacy; -using osu.Game.Users; namespace osu.Game.Scoring { @@ -48,7 +46,7 @@ namespace osu.Game.Scoring [CanBeNull] private readonly UserIdLookupCache userIdLookupCache; - private IAPIProvider api { get; set; } + private readonly IAPIProvider api; public ScoreManager(RulesetStore rulesets, Func beatmaps, Storage storage, IAPIProvider api, IDatabaseContextFactory contextFactory, IIpcHost importHost = null, Func difficulties = null, OsuConfigManager configManager = null, bool performOnlineLookups = false) From b1a995e0bb9d64a663c098518f24790ca0e46a47 Mon Sep 17 00:00:00 2001 From: Davran Dilshat Date: Sun, 5 Sep 2021 15:49:48 +0100 Subject: [PATCH 16/16] revert changes --- osu.Game/Online/Chat/ChannelManager.cs | 2 +- osu.Game/OsuGameBase.cs | 2 +- osu.Game/Scoring/ScoreManager.cs | 30 +------ .../Scoring/ScoreManager_UserIdLookupCache.cs | 85 ------------------- 4 files changed, 6 insertions(+), 113 deletions(-) delete mode 100644 osu.Game/Scoring/ScoreManager_UserIdLookupCache.cs diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index bf411d59f6..1937019ef6 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -601,4 +601,4 @@ namespace osu.Game.Online.Chat : channel.Id == Id; } } -} \ No newline at end of file +} diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 484cc23161..f2d575550a 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -263,7 +263,7 @@ namespace osu.Game dependencies.Cache(fileStore = new FileStore(contextFactory, Storage)); // ordering is important here to ensure foreign keys rules are not broken in ModelStore.Cleanup() - dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, contextFactory, Host, () => difficultyCache, LocalConfig, true)); + dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, contextFactory, Host, () => difficultyCache, LocalConfig)); dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, contextFactory, RulesetStore, API, Audio, Resources, Host, defaultBeatmap, true)); // this should likely be moved to ArchiveModelManager when another case appears where it is necessary diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index ccc5579ee5..83bcac01ac 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -26,7 +26,7 @@ using osu.Game.Scoring.Legacy; namespace osu.Game.Scoring { - public partial class ScoreManager : DownloadableArchiveModelManager + public class ScoreManager : DownloadableArchiveModelManager { public override IEnumerable HandledExtensions => new[] { ".osr" }; @@ -43,23 +43,14 @@ namespace osu.Game.Scoring [CanBeNull] private readonly OsuConfigManager configManager; - [CanBeNull] - private readonly UserIdLookupCache userIdLookupCache; - - private readonly IAPIProvider api; - public ScoreManager(RulesetStore rulesets, Func beatmaps, Storage storage, IAPIProvider api, IDatabaseContextFactory contextFactory, IIpcHost importHost = null, - Func difficulties = null, OsuConfigManager configManager = null, bool performOnlineLookups = false) + Func difficulties = null, OsuConfigManager configManager = null) : base(storage, contextFactory, api, new ScoreStore(contextFactory, storage), importHost) { this.rulesets = rulesets; this.beatmaps = beatmaps; this.difficulties = difficulties; this.configManager = configManager; - this.api = api; - - if (performOnlineLookups) - userIdLookupCache = new UserIdLookupCache(api); } protected override ScoreInfo CreateModel(ArchiveReader archive) @@ -81,21 +72,8 @@ namespace osu.Game.Scoring } } - protected override async Task Populate(ScoreInfo model, ArchiveReader archive, CancellationToken cancellationToken = default) - { - // These scores only provide the user's username but we need the user's ID too. - if (model.UserID <= 1 && model.UserString != null && userIdLookupCache != null) - { - try - { - model.UserID = await userIdLookupCache.GetUserIdAsync(model.UserString, cancellationToken).ConfigureAwait(false); - } - catch (Exception e) - { - LogForModel(model, $"Online retrieval failed for {model.User} ({e.Message})", e); - } - } - } + protected override Task Populate(ScoreInfo model, ArchiveReader archive, CancellationToken cancellationToken = default) + => Task.CompletedTask; protected override void ExportModelTo(ScoreInfo model, Stream outputStream) { diff --git a/osu.Game/Scoring/ScoreManager_UserIdLookupCache.cs b/osu.Game/Scoring/ScoreManager_UserIdLookupCache.cs deleted file mode 100644 index dc7e244f14..0000000000 --- a/osu.Game/Scoring/ScoreManager_UserIdLookupCache.cs +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; -using osu.Game.Online.API; -using osu.Game.Online.API.Requests; -using osu.Game.Database; - -namespace osu.Game.Scoring -{ - public partial class ScoreManager - { - private class UserIdLookupCache : MemoryCachingComponent - { - private readonly IAPIProvider api; - - public UserIdLookupCache(IAPIProvider api) - { - this.api = api; - } - - /// - /// Perform an API lookup on the specified username, returning the associated ID. - /// - /// The username to lookup. - /// An optional cancellation token. - /// The user ID, or 1 if the user does not exist or the request could not be satisfied. - public Task GetUserIdAsync(string username, CancellationToken token = default) => GetAsync(username, token); - - protected override async Task ComputeValueAsync(string lookup, CancellationToken token = default) - => await queryUserId(lookup).ConfigureAwait(false); - - private readonly Queue<(string username, TaskCompletionSource)> pendingUserTasks = new Queue<(string, TaskCompletionSource)>(); - private Task pendingRequestTask; - private readonly object taskAssignmentLock = new object(); - - private Task queryUserId(string username) - { - lock (taskAssignmentLock) - { - var tcs = new TaskCompletionSource(); - - // Add to the queue. - pendingUserTasks.Enqueue((username, tcs)); - - // Create a request task if there's not already one. - if (pendingRequestTask == null) - createNewTask(); - - return tcs.Task; - } - } - - private void performLookup() - { - (string username, TaskCompletionSource task) next; - - lock (taskAssignmentLock) - { - next = pendingUserTasks.Dequeue(); - } - - var request = new GetUserRequest(next.username); - - // rather than queueing, we maintain our own single-threaded request stream. - // todo: we probably want retry logic here. - api.Perform(request); - - // Create a new request task if there's still more users to query. - lock (taskAssignmentLock) - { - pendingRequestTask = null; - if (pendingUserTasks.Count > 0) - createNewTask(); - } - - next.task.SetResult(request.Result?.Id ?? 1); - } - - private void createNewTask() => pendingRequestTask = Task.Run(performLookup); - } - } -}