Skip to content

Commit

Permalink
Merge pull request ppy#30146 from bdach/lookup-users-endpoint
Browse files Browse the repository at this point in the history
Fix currently online display hitting rate limits
  • Loading branch information
peppy authored Oct 22, 2024
2 parents 7b3376c + 187fa5e commit c15490e
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,9 @@ public void TestStartMatchWhileSpectating()

AddUntilStep("wait for room join", () => RoomJoined);

AddStep("join other user (ready)", () =>
{
MultiplayerClient.AddUser(new APIUser { Id = PLAYER_1_ID });
MultiplayerClient.ChangeUserState(PLAYER_1_ID, MultiplayerUserState.Ready);
});
AddStep("join other user", void () => MultiplayerClient.AddUser(new APIUser { Id = PLAYER_1_ID }));
AddUntilStep("wait for user populated", () => MultiplayerClient.ClientRoom!.Users.Single(u => u.UserID == PLAYER_1_ID).User, () => Is.Not.Null);
AddStep("other user ready", () => MultiplayerClient.ChangeUserState(PLAYER_1_ID, MultiplayerUserState.Ready));

ClickButtonWhenEnabled<MultiplayerSpectateButton>();

Expand Down
6 changes: 3 additions & 3 deletions osu.Game/Database/UserLookupCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace osu.Game.Database
{
public partial class UserLookupCache : OnlineLookupCache<int, APIUser, GetUsersRequest>
public partial class UserLookupCache : OnlineLookupCache<int, APIUser, LookupUsersRequest>
{
/// <summary>
/// Perform an API lookup on the specified user, populating a <see cref="APIUser"/> model.
Expand All @@ -28,8 +28,8 @@ public partial class UserLookupCache : OnlineLookupCache<int, APIUser, GetUsersR
/// <returns>The populated users. May include null results for failed retrievals.</returns>
public Task<APIUser?[]> GetUsersAsync(int[] userIds, CancellationToken token = default) => LookupAsync(userIds, token);

protected override GetUsersRequest CreateRequest(IEnumerable<int> ids) => new GetUsersRequest(ids.ToArray());
protected override LookupUsersRequest CreateRequest(IEnumerable<int> ids) => new LookupUsersRequest(ids.ToArray());

protected override IEnumerable<APIUser>? RetrieveResults(GetUsersRequest request) => request.Response?.Users;
protected override IEnumerable<APIUser>? RetrieveResults(LookupUsersRequest request) => request.Response?.Users;
}
}
6 changes: 6 additions & 0 deletions osu.Game/Online/API/Requests/GetUsersRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@
// See the LICENCE file in the repository root for full licence text.

using System;
using osu.Game.Online.API.Requests.Responses;

namespace osu.Game.Online.API.Requests
{
/// <summary>
/// Looks up users with the given <see cref="UserIds"/>.
/// In comparison to <see cref="LookupUsersRequest"/>, the response here contains <see cref="APIUser.RulesetsStatistics"/>,
/// but in exchange is subject to more stringent rate limiting.
/// </summary>
public class GetUsersRequest : APIRequest<GetUsersResponse>
{
public readonly int[] UserIds;
Expand Down
30 changes: 30 additions & 0 deletions osu.Game/Online/API/Requests/LookupUsersRequest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using osu.Game.Online.API.Requests.Responses;

namespace osu.Game.Online.API.Requests
{
/// <summary>
/// Looks up users with the given <see cref="UserIds"/>.
/// In comparison to <see cref="GetUsersRequest"/>, the response here does not contain <see cref="APIUser.RulesetsStatistics"/>,
/// but in exchange is subject to less stringent rate limiting, making it suitable for mass user listings.
/// </summary>
public class LookupUsersRequest : APIRequest<GetUsersResponse>
{
public readonly int[] UserIds;

private const int max_ids_per_request = 50;

public LookupUsersRequest(int[] userIds)
{
if (userIds.Length > max_ids_per_request)
throw new ArgumentException($"{nameof(LookupUsersRequest)} calls only support up to {max_ids_per_request} IDs at once");

UserIds = userIds;
}

protected override string Target => @"users/lookup/?ids[]=" + string.Join(@"&ids[]=", UserIds);
}
}
2 changes: 1 addition & 1 deletion osu.Game/Online/API/Requests/Responses/APIUser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ private APIRankHistory rankHistory
public APIUserHistoryCount[] ReplaysWatchedCounts;

/// <summary>
/// All user statistics per ruleset's short name (in the case of a <see cref="GetUsersRequest"/> response).
/// All user statistics per ruleset's short name (in the case of a <see cref="GetUsersRequest"/> or <see cref="GetMeRequest"/> response).
/// Otherwise empty. Can be altered for testing purposes.
/// </summary>
// todo: this should likely be moved to a separate UserCompact class at some point.
Expand Down
27 changes: 22 additions & 5 deletions osu.Game/Online/Multiplayer/MultiplayerClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using osu.Game.Database;
using osu.Game.Localisation;
using osu.Game.Online.API;
using osu.Game.Online.API.Requests;
using osu.Game.Online.API.Requests.Responses;
using osu.Game.Online.Multiplayer.Countdown;
using osu.Game.Online.Rooms;
Expand Down Expand Up @@ -188,7 +189,7 @@ await joinOrLeaveTaskChain.Add(async () =>

// Populate users.
Debug.Assert(joinedRoom.Users != null);
await Task.WhenAll(joinedRoom.Users.Select(PopulateUser)).ConfigureAwait(false);
await PopulateUsers(joinedRoom.Users).ConfigureAwait(false);

// Update the stored room (must be done on update thread for thread-safety).
await runOnUpdateThreadAsync(() =>
Expand Down Expand Up @@ -416,7 +417,7 @@ Task IMultiplayerClient.RoomStateChanged(MultiplayerRoomState state)

async Task IMultiplayerClient.UserJoined(MultiplayerRoomUser user)
{
await PopulateUser(user).ConfigureAwait(false);
await PopulateUsers([user]).ConfigureAwait(false);

Scheduler.Add(() =>
{
Expand Down Expand Up @@ -803,10 +804,26 @@ public Task PlaylistItemChanged(MultiplayerPlaylistItem item)
}

/// <summary>
/// Populates the <see cref="APIUser"/> for a given <see cref="MultiplayerRoomUser"/>.
/// Populates the <see cref="APIUser"/> for a given collection of <see cref="MultiplayerRoomUser"/>s.
/// </summary>
/// <param name="multiplayerUser">The <see cref="MultiplayerRoomUser"/> to populate.</param>
protected async Task PopulateUser(MultiplayerRoomUser multiplayerUser) => multiplayerUser.User ??= await userLookupCache.GetUserAsync(multiplayerUser.UserID).ConfigureAwait(false);
/// <param name="multiplayerUsers">The <see cref="MultiplayerRoomUser"/>s to populate.</param>
protected async Task PopulateUsers(IEnumerable<MultiplayerRoomUser> multiplayerUsers)
{
var request = new GetUsersRequest(multiplayerUsers.Select(u => u.UserID).Distinct().ToArray());

await API.PerformAsync(request).ConfigureAwait(false);

if (request.Response == null)
return;

Dictionary<int, APIUser> users = request.Response.Users.ToDictionary(user => user.Id);

foreach (var multiplayerUser in multiplayerUsers)
{
if (users.TryGetValue(multiplayerUser.UserID, out var user))
multiplayerUser.User = user;
}
}

/// <summary>
/// Updates the local room settings with the given <see cref="MultiplayerRoomSettings"/>.
Expand Down
16 changes: 16 additions & 0 deletions osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,22 @@ public bool HandleRequest(APIRequest request, APIUser localUser, BeatmapManager
getBeatmapSetRequest.TriggerSuccess(OsuTestScene.CreateAPIBeatmapSet(baseBeatmap));
return true;
}

case GetUsersRequest getUsersRequest:
{
getUsersRequest.TriggerSuccess(new GetUsersResponse
{
Users = getUsersRequest.UserIds.Select(id => id == TestUserLookupCache.UNRESOLVED_USER_ID
? null
: new APIUser
{
Id = id,
Username = $"User {id}"
})
.Where(u => u != null).ToList(),
});
return true;
}
}

List<APIBeatmap> createResponseBeatmaps(params int[] beatmapIds)
Expand Down

0 comments on commit c15490e

Please sign in to comment.