EW-1059 fix slow Query by account searchByUsername (#5339)

hpi-schul-cloud · Nov 14, 2024 · 78fccf6 · 78fccf6
1 parent 990ad4e
commit 78fccf6
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 55 deletions.
diff --git a/apps/server/src/modules/account/repo/micro-orm/account.repo.integration.spec.ts b/apps/server/src/modules/account/repo/micro-orm/account.repo.integration.spec.ts
@@ -343,30 +343,6 @@ describe('account repo', () => {
 			});
 		});
 
-		describe('When searching by username', () => {
-			const setup = async () => {
-				const originalUsername = '[email protected]';
-				const partialLowerCaseUsername = '[email protected]';
-				const lowercaseUsername = '[email protected]';
-				const account = accountFactory.build({ username: originalUsername });
-				await em.persistAndFlush([account]);
-				em.clear();
-				return { originalUsername, partialLowerCaseUsername, lowercaseUsername, account };
-			};
-
-			it('should find account by user name, ignoring case', async () => {
-				const { originalUsername, partialLowerCaseUsername, lowercaseUsername } = await setup();
-
-				let [accounts] = await repo.searchByUsernameExactMatch(partialLowerCaseUsername);
-				expect(accounts).toHaveLength(1);
-				expect(accounts[0]).toEqual(expect.objectContaining({ username: originalUsername }));
-
-				[accounts] = await repo.searchByUsernameExactMatch(lowercaseUsername);
-				expect(accounts).toHaveLength(1);
-				expect(accounts[0]).toEqual(expect.objectContaining({ username: originalUsername }));
-			});
-		});
-
 		describe('When using wildcard', () => {
 			const setup = async () => {
 				const originalUsername = '[email protected]';

diff --git a/apps/server/src/modules/account/repo/micro-orm/account.repo.ts b/apps/server/src/modules/account/repo/micro-orm/account.repo.ts
@@ -108,12 +108,47 @@ export class AccountRepo {
 		await this.em.flush();
 	}
 
-	public async searchByUsernameExactMatch(username: string, skip = 0, limit = 1): Promise<Counted<Account[]>> {
-		return this.searchByUsername(username, skip, limit, true);
+	public async searchByUsernameExactMatch(username: string, offset = 0, limit = 1): Promise<Counted<Account[]>> {
+		const [entities, count] = await this.em.findAndCount(
+			this.entityName,
+			{
+				username,
+			},
+			{
+				offset,
+				limit,
+				orderBy: { username: 1 },
+			}
+		);
+		const accounts = AccountEntityToDoMapper.mapEntitiesToDos(entities);
+
+		return [accounts, count];
 	}
 
-	public async searchByUsernamePartialMatch(username: string, skip = 0, limit = 10): Promise<Counted<Account[]>> {
-		return this.searchByUsername(username, skip, limit, false);
+	/**
+	 * @deprecated we want to discourage the usage of this function, because it uses a regular expression
+	 *             for partial matching and this can have some serious performance implications. Use it with caution.
+	 */
+	public async searchByUsernamePartialMatch(username: string, offset = 0, limit = 10): Promise<Counted<Account[]>> {
+		const escapedUsername = username.replace(/[^(\p{L}\p{N})]/gu, '\\$&');
+		const [entities, count] = await this.em.findAndCount(
+			this.entityName,
+			{
+				// NOTE: The default behavior of the MongoDB driver allows
+				// to pass regular expressions directly into the where clause
+				// without the need of using the $re operator, this will NOT
+				// work with SQL drivers.
+				username: new RegExp(escapedUsername, 'i'),
+			},
+			{
+				offset,
+				limit,
+				orderBy: { username: 1 },
+			}
+		);
+		const accounts = AccountEntityToDoMapper.mapEntitiesToDos(entities);
+
+		return [accounts, count];
 	}
 
 	public async deleteById(accountId: EntityId | ObjectId): Promise<void> {
@@ -154,31 +189,4 @@ export class AccountRepo {
 
 		return result;
 	}
-
-	private async searchByUsername(
-		username: string,
-		offset: number,
-		limit: number,
-		exactMatch: boolean
-	): Promise<Counted<Account[]>> {
-		const escapedUsername = username.replace(/[^(\p{L}\p{N})]/gu, '\\$&');
-		const searchUsername = exactMatch ? `^${escapedUsername}$` : escapedUsername;
-		const [entities, count] = await this.em.findAndCount(
-			this.entityName,
-			{
-				// NOTE: The default behavior of the MongoDB driver allows
-				// to pass regular expressions directly into the where clause
-				// without the need of using the $re operator, this will NOT
-				// work with SQL drivers
-				username: new RegExp(searchUsername, 'i'),
-			},
-			{
-				offset,
-				limit,
-				orderBy: { username: 1 },
-			}
-		);
-		const accounts = AccountEntityToDoMapper.mapEntitiesToDos(entities);
-		return [accounts, count];
-	}
 }