diff --git a/server/postgres/userRepository.js b/server/postgres/userRepository.js index c8e79d3..d9baa62 100644 --- a/server/postgres/userRepository.js +++ b/server/postgres/userRepository.js @@ -1,6 +1,6 @@ const { pool } = require('./index'); const { stableId } = require('./dualWrite'); -const { normalizeUsername } = require('../username'); +const { normalizeStoredUsername } = require('../username'); const numericId = (value) => { const id = Number(value); @@ -40,13 +40,16 @@ const toUser = (row) => { const userParams = (user, fallbackName = 'Unknown User') => { const id = numericId(user.id); if (!id) throw new Error('A valid WCA user ID is required'); - const normalized = normalizeUsername(user.username); + const normalized = normalizeStoredUsername(user.username); + const username = normalized.username === undefined ? null : normalized.username; + const usernameNormalized = normalized.usernameNormalized === undefined + ? null : normalized.usernameNormalized; return [ stableId('user', id), id, user.name || fallbackName, - normalized.username || null, - normalized.usernameNormalized || null, + username, + usernameNormalized, user.wcaId || null, { showWCAID: !!user.showWCAID, diff --git a/server/postgres/userRepository.test.js b/server/postgres/userRepository.test.js new file mode 100644 index 0000000..6ccc77f --- /dev/null +++ b/server/postgres/userRepository.test.js @@ -0,0 +1,42 @@ +/** @jest-environment node */ + +jest.mock('./index', () => ({ + pool: { + query: jest.fn(), + }, +})); +jest.mock('./dualWrite', () => ({ + stableId: jest.fn(() => 'stable-user-id'), +})); + +const { pool } = require('./index'); +const { updateUser } = require('./userRepository'); + +describe('PostgreSQL user repository', () => { + it('keeps a legacy non-ASCII username during a profile update', async () => { + pool.query.mockResolvedValue({ + rows: [{ + id: 'stable-user-id', + wca_user_id: 2022, + name: 'Alessandro van Burken', + username: 'Αλέξανδρος', + username_normalized: null, + preferences: {}, + avatar: {}, + }], + }); + + await expect(updateUser({ + id: 2022, + name: 'Alessandro van Burken', + username: 'Αλέξανδρος', + })).resolves.toMatchObject({ + username: 'Αλέξανδρος', + usernameNormalized: undefined, + }); + + const params = pool.query.mock.calls[0][1]; + expect(params[3]).toBe('Αλέξανδρος'); + expect(params[4]).toBeNull(); + }); +}); diff --git a/server/username.js b/server/username.js index 8316c7e..e5d092d 100644 --- a/server/username.js +++ b/server/username.js @@ -51,6 +51,19 @@ const normalizeUsername = (value, { allowEmpty = true } = {}) => { }; }; +const normalizeStoredUsername = (value) => { + try { + return normalizeUsername(value); + } catch (err) { + if (err.code !== 'INVALID_USERNAME' || typeof value !== 'string') { + throw err; + } + + // Keep legacy values readable while their owner chooses a valid username. + return { username: value, usernameNormalized: undefined }; + } +}; + const usernameConflict = () => new UsernameError( 'USERNAME_TAKEN', 'Username is already in use', @@ -117,6 +130,7 @@ module.exports = { findUserByUsername, isUsernameDuplicateKeyError, normalizeUsername, + normalizeStoredUsername, searchUsersByUsernamePrefix, updateUsername, }; diff --git a/server/username.test.js b/server/username.test.js index 9dcf95b..81b8ad1 100644 --- a/server/username.test.js +++ b/server/username.test.js @@ -1,10 +1,10 @@ /** @jest-environment node */ -/* eslint-env jest */ const { canonicalizeUsername, findUserByUsername, normalizeUsername, + normalizeStoredUsername, searchUsersByUsernamePrefix, updateUsername, } = require('./username'); @@ -29,6 +29,13 @@ describe('username normalization and lookup', () => { }); }); + it('preserves legacy non-ASCII usernames during unrelated writes', () => { + expect(normalizeStoredUsername('Αλέξανδρος')).toEqual({ + username: 'Αλέξανδρος', + usernameNormalized: undefined, + }); + }); + it.each([ 'cuber.name', 'cuber+name',