fix(API_GetUserSummary): handle username case sensitivity and use canonical username

[G3mha]

Description

This PR addresses two related issues in the GetUserSummary API endpoint:

1. Makes username lookups case-insensitive
2. Uses canonical username in the response

Changes

• Modified User::scopeByDisplayName to use case-insensitive comparison with LOWER()
• Updated API response to use canonical username in all fields
• Added test cases for username case sensitivity handling

Test Coverage

Added new test method testUsernameCaseSensitivityHandling() that verifies:

• Lowercase username requests (e.g., 'nicoplaysthings')
• Uppercase username requests (e.g., 'NICOPLAYSTHINGS')
• Mixed case username requests (e.g., 'NicoPLAYSthings')

Fixes #2769

[wescopeland]

Hi @G3mha! Thanks for opening a PR into RAWeb!

Username lookups are already case-insensitive. Please try hitting prod with your favorite API testing tool:

GET https://retroachievements.org/API/API_GetUserSummary.php?y=YOUR_WEB_API_KEY&u=NICOPLAYSTHINGS

{
	"User": "NicoPlaysThings",
	"MemberSince": "2021-02-06 23:36:29",
	"LastActivity": {
		"ID": 0,
		"timestamp": null,
		"lastupdate": null,
		"activitytype": null,
		"User": "NICOPLAYSTHINGS",
		"data": null,
		"data2": null
	},
	"RichPresenceMsg": "Mario is exploring Overworld ⭐[65\/70]",
	"LastGameID": 17700,
	"ContribCount": 0,
	"ContribYield": 0,
	"TotalPoints": 16631,
	"TotalSoftcorePoints": 0,
	"TotalTruePoints": 25619,
	"Permissions": 1,
	"Untracked": 0,
	"ID": 165298,
	"UserWallActive": 1,
	"Motto": "",
	"Rank": 3477,
	"RecentlyPlayedCount": 0,
	"RecentlyPlayed": [],
	"UserPic": "\/UserPic\/NICOPLAYSTHINGS.png",
	"TotalRanked": 78279,
	"Awarded": {},
	"RecentAchievements": {},
	"Status": "Offline"
}

Notice in this response from prod:

"User": "NicoPlaysThings",

So it appears to me the case-corrected value is already available - it just needs to be used instead of the query param where appropriate.

[G3mha]

@wescopeland Thank you for the clarification, however I'm unable to find the canonicalUser if in API the code executes:

// Get canonical username from db
$canonicalUser = User::byDisplayName($user)->value('User');
if (!$canonicalUser) {
    return response()->json([
        'ID' => null,
        'User' => $user,
    ], 404);
}

If the method searches up for a canonical username using a non-canonical username to do so in the DB.

public function scopeByDisplayName(Builder $query, string $username): Builder
    {
        return $query->where('User', $username);
    }

When I do so hitting the test with username=NICOPLAYSTHINGS, I get an error on the lookup:

php artisan test tests/Feature/Api/V1/UserSummaryTest.php --filter testUsernameCaseConsistency
array:2 [
  "ID" => null
  "User" => "NICOPLAYSTHINGS"
] // tests/Feature/Api/V1/UserSummaryTest.php:501

I agree with you that in production the API returns the canonicalUsername, not case-sensitive. However, by querying the DB, it is indeed case-sensitive.

Could you please help me up understand how to get a better picture of the problem?

[wescopeland]

Hi @G3mha!

I agree with you that in production the API returns the canonicalUsername, not case-sensitive. However, by querying the DB, it is indeed case-sensitive.

Try running:

SHOW TABLE STATUS WHERE Name = 'UserAccounts';

This will output a bunch of metadata about the table, including the collation name (which determines case-sensitivity of queries). You should see a Collation value suffixed with "_ci". "ci" stands for case-insensitive.

SELECT * FROM UserAccounts WHERE User = "WCopeland"; -- 1 row returned
SELECT * FROM UserAccounts WHERE User = "wcopeland"; -- 1 row returned
SELECT * FROM UserAccounts WHERE User = "WCOPELAND"; -- 1 row returned

If queries to this table were case-sensitive, then ?u= NICOPLAYSTHINGS on the API endpoint wouldn't work.

In API_GetUserSummary, there's a call to getUserInfo(), which is making the query:

$user = User::firstWhere('User', $username);

If you place dd($user->User) immediately after this line and run the code, you will see the case-corrected username output regardless of the input case of the $username variable.

API_GetUserSummary is a very high-traffic endpoint. Unfortunately, we probably cannot afford to add another DB query in it 😞

However, I believe you should be able to solve this with values already available in the endpoint code 🙂

The problem is LastActivity.User and UserPic in the response are incorrect. Focus on why the casings for those values are incorrect and how to get the case-corrected value, and I think you will be on the right track 🙂

[G3mha]

That is perfect @wescopeland, however the test for it is not passing.

It appears to me that MySQL is non case sensitive, but SQLite (on the tests) is case-sensitive. Could you point out to me how to change the testing from SQLite to MySQL? Or make my SQL non case sensitive?

I've inserted two dumps on API_GetUserSummary.php, in this section (so you can better understand the output):

dump($user);
$retVal = getUserPageInfo($user, $recentGamesPlayed, $recentAchievementsEarned);
dump($retVal);

And the output of the test was this:

php artisan test tests/Feature/Api/V1/UserSummaryTest.php --filter testUsernameCaseConsistency
"NICOPLAYSTHINGS" // public/API/API_GetUserSummary.php:106
[] // public/API/API_GetUserSummary.php:108

   FAIL  Tests\Feature\Api\V1\UserSummaryTest
  ⨯ username case consistency                                                                                                          0.51s  
  ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────  
   FAILED  Tests\Feature\Api\V1\UserSummaryTest > username case consistency                                                                   
  Expected response status code [200] but received 404.
Failed asserting that 404 is identical to 200.

  at tests/Feature/Api/V1/UserSummaryTest.php:498
    494▕ 
    495▕         foreach ($variations as $input) {
    496▕             $response = $this->get($this->apiUrl('GetUserSummary', ['u' => $input]));
    497▕ 
  ➜ 498▕             $response->assertOk()
    499▕                     ->assertJsonPath('User', 'NicoPlaysThings')
    500▕                     ->assertJsonPath('LastActivity.User', 'NicoPlaysThings')
    501▕                     ->assertJsonPath('UserPic', '/UserPic/NicoPlaysThings.png');
    502▕         }


  Tests:    1 failed (1 assertions)
  Duration: 0.53s

[wescopeland]

Regrettably, it appears SQLite will not let us get test coverage of this fix due to the collation differences. At the moment we're not equipped to run tests in MySQL/MariaDB.

I don't like to do this, but for now let's omit writing a new test case for this change. I'll verify manually.

Given that, we'll still need to remove this from the endpoint code:

$canonicalUser = User::byDisplayName($user)->value('User');

[G3mha]

@wescopeland I understand it. Certainly, not the best practice, but as Jim Gordon in the Dark Knight would say "I don't get [...] points for being an idealist, I have to do the best I can with what I have."

I just kept on this PR the essential changes, therefore, just the API_GetUserSummary.php. Could you review it for me?

[wescopeland]

Thanks!