more efficient slug generation

helpers/generateSlug.ts

@@ -12,7 +12,23 @@ async function getCollectionBySlug(gameId: GameId, slug: string, options?: Query
   return await CollectionModel.findOne({ slug: slug, gameId: gameId }, {}, options);
 }
 
-const MAX_SLUGS_WITH_SAME_NAME = process.env.NODE_ENV === 'test' ? 4 : 20;
+// NB: with makeId(4) and 4 custom slug attempts, we have a 1 in 4.8e28 ((62^4)^4) chance of a not generating a slug
+const SLUG_ID_LENGTH = 4;
+const MAX_SLUGS_WITH_SAME_NAME = 5;
+
+export class SlugUtil {
+  static makeId(length: number) {
+    let result = '';
+    const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
+    const charactersLength = characters.length;
+
+    for (let i = 0; i < length; i++) {
+      result = result + characters.charAt(Math.floor(Math.random() * charactersLength));
+    }
+
+    return result;
+  }
+}
 
 function slugify(str: string) {
   const slug = str
@@ -33,9 +49,9 @@ export async function generateCollectionSlug(
 ) {
   const og_slug = slugify(userName) + '/' + slugify(collectionName);
   let slug = og_slug;
-  let i = 2;
+  let attempts = 0;
 
-  while (i < MAX_SLUGS_WITH_SAME_NAME) {
+  while (attempts < MAX_SLUGS_WITH_SAME_NAME) {
     const collection = await getCollectionBySlug(gameId, slug, options);
 
     if (!collection) {
@@ -46,8 +62,8 @@ export async function generateCollectionSlug(
       return slug;
     }
 
-    slug = og_slug + '-' + i;
-    i++;
+    slug = og_slug + '-' + SlugUtil.makeId(SLUG_ID_LENGTH);
+    attempts++;
   }
 
   throw new Error('Couldn\'t generate a unique collection slug');
@@ -62,9 +78,9 @@ export async function generateLevelSlug(
 ) {
   const og_slug = slugify(userName) + '/' + slugify(levelName);
   let slug = og_slug;
-  let i = 2;
+  let attempts = 0;
 
-  while (i < 20) {
+  while (attempts < MAX_SLUGS_WITH_SAME_NAME) {
     const level = await getLevelBySlug(gameId, slug, options);
 
     if (!level) {
@@ -75,8 +91,8 @@ export async function generateLevelSlug(
       return slug;
     }
 
-    slug = og_slug + '-' + i;
-    i++;
+    slug = og_slug + '-' + SlugUtil.makeId(SLUG_ID_LENGTH);
+    attempts++;
   }
 
   throw new Error('Couldn\'t generate a unique level slug');

pages/api/match/index.ts

@@ -2,6 +2,7 @@ import { AchievementCategory } from '@root/constants/achievements/achievementInf
 import DiscordChannel from '@root/constants/discordChannel';
 import { GameId } from '@root/constants/GameId';
 import queueDiscordWebhook from '@root/helpers/discordWebhook';
+import { SlugUtil } from '@root/helpers/generateSlug';
 import { getGameFromId } from '@root/helpers/getGameIdFromReq';
 import { abortMatch } from '@root/helpers/match/abortMatch';
 import { LEVEL_DEFAULT_PROJECTION } from '@root/models/constants/projections';
@@ -23,19 +24,6 @@ import { computeMatchScoreTable, enrichMultiplayerMatch, generateMatchLog } from
 import { USER_DEFAULT_PROJECTION } from '../../../models/schemas/userSchema';
 import { queueRefreshAchievements } from '../internal-jobs/worker';
 
-function makeId(length: number) {
-  let result = '';
-  const characters =
-    'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
-  const charactersLength = characters.length;
-
-  for (let i = 0; i < length; i++) {
-    result = result + characters.charAt(Math.floor(Math.random() * charactersLength));
-  }
-
-  return result;
-}
-
 export async function checkForFinishedMatches() {
   const matches = await MultiplayerMatchModel.find(
     {
@@ -334,7 +322,7 @@ async function createMatch(req: NextApiRequestWithAuth) {
         throw new Error(errorMessage);
       }
 
-      const matchId = makeId(11);
+      const matchId = SlugUtil.makeId(11);
       const matchUrl = `${req.headers.origin}/match/${matchId}`;
 
       const matchCreate = await MultiplayerMatchModel.create([{

tests/pages/api/collection/collection.test.ts

@@ -396,17 +396,15 @@ describe('pages/api/collection/index.ts', () => {
       },
     });
   });
-  test('Create 3 collections with same name in DB, so that we can test to make sure the server will not crash. The 19th should crash however.', async () => {
-    for (let i = 0; i < 2; i++) {
+  test('Create 20 collections with same name in DB, we should never crash because it is so unlikely', async () => {
+    const slugs = new Set<string>();
+
+    for (let i = 1; i <= 20; i++) {
       // expect no exceptions
-      const promise = initCollection(TestId.USER, 'Sample');
+      const collection = await initCollection(TestId.USER, 'Sample');
 
-      await expect(promise).resolves.toBeDefined();
+      expect(slugs.has(collection.slug)).toBe(false);
+      slugs.add(collection.slug);
     }
-
-    // Now create one more, it should throw exception
-    const promise = initCollection(TestId.USER, 'Sample');
-
-    await expect(promise).rejects.toThrow('Couldn\'t generate a unique collection slug');
-  }, 30000);
+  });
 });

tests/pages/api/level/level.byslug.test.ts

@@ -1,6 +1,7 @@
 import { DEFAULT_GAME_ID } from '@root/constants/GameId';
 import NotificationType from '@root/constants/notificationType';
 import { NextApiRequestWrapper } from '@root/helpers/apiWrapper';
+import { SlugUtil } from '@root/helpers/generateSlug';
 import { enableFetchMocks } from 'jest-fetch-mock';
 import { testApiHandler } from 'next-test-api-route-handler';
 import TestId from '../../../../constants/testId';
@@ -444,7 +445,8 @@ describe('Testing slugs for levels', () => {
     });
   });
 
-  test('Updating level_id_2 name to test-level-2 should become test-level-2-2 in the db', async () => {
+  test('Updating level_id_2 name to test-level-2 should create a slug id', async () => {
+    jest.spyOn(SlugUtil, 'makeId').mockReturnValueOnce('test');
     await testApiHandler({
       pagesHandler: async (_, res) => {
         const req: NextApiRequestWithAuth = {
@@ -479,9 +481,10 @@ describe('Testing slugs for levels', () => {
 
     // testing through querying DB since this level is a draft
     expect(level).toBeDefined();
-    expect(level?.slug).toBe('newuser/test-level-2-2');
+    expect(level?.slug).toBe('newuser/test-level-2-test');
   });
-  test('Updating level_id_2 name to test-level-2 again should REMAIN test-level-2-2 in the db', async () => {
+  test('Updating level_id_2 name to test-level-2 again should update the slug id', async () => {
+    jest.spyOn(SlugUtil, 'makeId').mockReturnValueOnce('a1b2');
     await testApiHandler({
       pagesHandler: async (_, res) => {
         const req: NextApiRequestWithAuth = {
@@ -516,19 +519,17 @@ describe('Testing slugs for levels', () => {
 
     // testing through querying DB since this level is a draft
     expect(level).toBeDefined();
-    expect(level?.slug).toBe('newuser/test-level-2-2');
+    expect(level?.slug).toBe('newuser/test-level-2-a1b2');
   });
-  test('Create 18 levels with same name in DB, so that we can test to make sure the server will not crash. The 19th should crash however.', async () => {
-    for (let i = 1; i <= 18; i++) {
+  test('Create 20 levels with same name in DB, we should never crash because it is so unlikely', async () => {
+    const slugs = new Set<string>();
+
+    for (let i = 1; i <= 20; i++) {
       // expect no exceptions
-      const promise = initLevel(DEFAULT_GAME_ID, TestId.USER, `Sample${'!'.repeat(i)}`);
+      const level = await initLevel(DEFAULT_GAME_ID, TestId.USER, `Sample${'!'.repeat(i)}`);
 
-      await expect(promise).resolves.toBeDefined();
+      expect(slugs.has(level.slug)).toBe(false);
+      slugs.add(level.slug);
     }
-
-    // Now create one more, it should throw exception
-    const promise = initLevel(DEFAULT_GAME_ID, TestId.USER, 'Sample');
-
-    await expect(promise).rejects.toThrow('Couldn\'t generate a unique level slug');
-  }, 30000);
+  });
 });