feat(api): add support for ULIDs to every applicable endpoint
Implements https://github.com/RetroAchievements/api-docs/pull/76.
Generally, every endpoint in the web API that returns a username now also returns a ULID alongside the field to represent that user.
Additionally, anything that lets a consumer query by u for username now also allows i for ULID. If an endpoint already is using an i param for something else, d is used instead for ULID.
The following endpoints are affected:
- API_GetAchievementOfTheWeek
- API_GetAchievementUnlocks
- API_GetAchievementsEarnedBetween
- API_GetAchievementsEarnedOnDay
- API_GetActiveClaims
- API_GetClaims
- API_GetComments
- API_GetGameExtended
- API_GetGameInfoAndUserProgress
- API_GetGameLeaderboards
- API_GetGameRankAndScore
- API_GetLeaderboardEntries
- API_GetRecentGameAwards
- API_GetTicketData
- API_GetTopTenUsers
- API_GetUserAwards
- API_GetUserClaims
- API_GetUserCompletedGames
- API_GetUserCompletionProgress
- API_GetUserGameLeaderboards
- API_GetUserGameRankAndScore
- API_GetUserPoints
- API_GetUserProgress
- API_GetUserRecentAchievements
- API_GetUserRecentlyPlayedGames
- API_GetUserSetRequests
- API_GetUserSummary
- API_GetUserWantToPlayList
- API_GetUsersFollowingMe
- API_GetUsersIFollow
Additionally, anything that lets a consumer query by u for username now also allows i for ULID. If an endpoint already is using an i param for something else, d is used instead for ULID.
Why not just use ulid as the parameter name (or some other letter that's not used by any script - I think v, w, and x are available)?
@Jamiras
Why not just use ulid as the parameter name
I like that more than the current approach in the branch. Alternatively, we could also just fold everything into existing u params, ie:
* API_GetAchievementsEarnedBetween - returns achievements earned by a user between two timestamps
* u : username or user ULID
$input = Validator::validate(Arr::wrap(request()->query()), [
'u' => ['required', 'string', 'min:2', 'max:26'],
]);
// Try ULID first if length matches, otherwise treat as username.
$user = strlen($input['u']) === 26
? User::whereUlid($input['u'])->first()
: User::whereName($input['u'])->first();
If you're not opposed to this approach, I'll make this change to work with the single u query param. I want to verify if this is agreeable first, as whatever approach we take will touch a lot of endpoint code.
My only concern with overloading the parameter is knowing which value to query. The DB field is 32 chars, but the new username validator only allows 20. The longest in the DB is also 20. As long as that holds true, we can safely assume anything that's 26 characters is a ULID.
I'm wondering if it would be reasonable to turn this into a helper function, since it'll be used by a lot of scripts:
$input = Validator::validate(Arr::wrap(request()->query()), [
'u' => ['required', 'string', 'min:2', 'max:26'],
]);
// Try ULID first if length matches, otherwise treat as username.
$user = strlen($input['u']) === 26
? User::whereUlid($input['u'])->first()
: User::whereName($input['u'])->first();
We can probably do something like this:
/**
* @return Builder<User>
*/
public static function whereUlidOrName(?string $identifier): Builder
{
if ($identifier === null) {
return static::query()->whereRaw('1 = 0');
}
return static::query()
->where(function ($query) use ($identifier) {
$ulidLength = 26;
if (mb_strlen($identifier) === $ulidLength) {
$query->whereUlid($identifier);
} else {
$query->where('display_name', $identifier)
->orWhere('User', $identifier);
}
});
}
If this is agreeable, I'll update all the endpoints (and their validators).
If this is agreeable, I'll update all the endpoints (and their validators).
I was kind of hoping to roll the validator in there too to avoid duplicating the definition of a user parameter 20-some times.
Something like:
function lookupApiTargetUser(): ?User
{
$input = Validator::validate(Arr::wrap(request()->query()), [
'u' => ['required', 'string', 'min:2', 'max:26'],
]);
// Try ULID first if length matches, otherwise treat as username.
return strlen($input['u']) === 26
? User::whereUlid($input['u'])->first()
: User::whereName($input['u'])->first();
}
But I don't know where it would be reasonable to put it. It's not like we have an "API_helpers.php". And I don't know if it's acceptable to call Validator::validate multiple times (as most of the endpoints have other parameters to validate).
It doesn't feel right putting a whereUlidOrName on the User model, when it's an API-specific helper.
Also, if we do decide having a whereUlidOrName is desirable, I believe it can be simplified (based on the original non-helper implementation):
public static function whereUlidOrName(?string $identifier): Builder
{
$ulidLength = 26;
if (mb_strlen($identifier) === $ulidLength) {
return $this->whereUlid($identifier);
} else {
return $this->whereName($identifier);
}
}
Maybe the approach we could take is:
- Create a custom validation rule we can stick on any param (similar to
CtypeAlnum()), ie:
'u' => ['required', new UserIdentifier()],
- Create an action class in the
Httpnamespace for lookups:
$user = (new FindUserByIdentifier())->execute($input['u']);
With this pattern, we're mostly decoupled from both the model and the web API domain.
Seems reasonable.
I've gone through each endpoint in latest and applied the pattern discussed above:
'u' => ['required', new UserIdentifier()],
$user = (new FindUserByIdentifier())->execute($input['u']);
Once this PR is approved, before merging I will update the accompanying docs PR.
Docs PR (https://github.com/RetroAchievements/api-docs/pull/76) has been updated with the latest changes. Will be merged on deploy.