Some clean-up

server/lib/matrix-utils/fetch-accessible-rooms.js

@@ -11,6 +11,7 @@ const config = require('../config');
 const matrixServerUrl = config.get('matrixServerUrl');
 assert(matrixServerUrl);
 
+// The number of requests we should make to try to fill the limit before bailing out
 const NUM_MAX_REQUESTS = 10;
 
 async function requestPublicRooms(
@@ -62,8 +63,10 @@ async function fetchAccessibleRooms(
   assert([DIRECTION.forward, DIRECTION.backward].includes(direction), 'direction must be [f|b]');
 
   // Based off of the matrix.org room directory, only 42% of rooms are world_readable,
-  // which means our best bet to fill up the results to the limit is to request 2.5 times as many.
-  const bulkPaginationLimit = Math.ceil(2.5 * limit);
+  // which means our best bet to fill up the results to the limit is to request at least
+  // 2.4 times as many. I've doubled and rounded it up to 5 times as many so we can have
+  // less round-trips.
+  const bulkPaginationLimit = Math.ceil(5 * limit);
 
   let accessibleRooms = [];
 
@@ -135,14 +138,14 @@ async function fetchAccessibleRooms(
     currentRequestCount += 1;
   }
 
-  // Back-track to get the perfect continuation point
+  // Back-track to get the perfect continuation point and show exactly the limit of
+  // rooms in the grid.
   //
   // Alternatively, we could just not worry about and show more than the limit of rooms
   //
   // XXX: Since the room directory order is not stable, this is slightly flawed as the
-  // results could have shifted slightly from when we made the last request to get the
-  // `continuationIndex` to now when we're trying to get the actual token to continue
-  // seamlessly.
+  // results could have shifted slightly from when we made the last request to now but
+  // we assume it's good enough.
   let nextPaginationToken;
   let prevPaginationToken;
   if (continuationIndex) {
@@ -151,7 +154,8 @@ async function fetchAccessibleRooms(
       searchTerm,
       // Start from the last request
       paginationToken: lastLoopToken,
-      // Then only go out as far out as the continuation index (the point when we filled the limit)
+      // Then only go out as far out as the continuation index (the point when we filled
+      // the limit)
       limit: continuationIndex + 1,
       abortSignal,
     });

test/e2e-tests.js

@@ -2744,13 +2744,31 @@ describe('matrix-public-archive', () => {
         // Doing all of these create room requests in parallel is about 2x faster than
         // doing them serially and the room directory doesn't return the rooms in any
         // particular order so it doesn't make the test any more clear doing them
-        // serially.
+        // serially anyway.
         const createdRoomsIds = await Promise.all(
           roomsConfigurationsToCreate.map((roomCreateOptions) =>
             createTestRoom(client, roomCreateOptions)
           )
         );
 
+        function roomIdToRoomName(expectedRoomId) {
+          const roomIndex = createdRoomsIds.findIndex((roomId) => {
+            return roomId === expectedRoomId;
+          });
+          assert(
+            roomIndex > 0,
+            `Expected to find expectedRoomId=${expectedRoomId} in the list of created rooms createdRoomsIds=${createdRoomsIds}`
+          );
+
+          const roomConfig = roomsConfigurationsToCreate[roomIndex];
+          assert(
+            roomConfig,
+            `Expected to find room config for roomIndex=${roomIndex} in the list of roomsConfigurationsToCreate (length ${roomsConfigurationsToCreate.length})}`
+          );
+
+          return roomConfig.name;
+        }
+
         async function checkRoomsOnPage(archiveUrl) {
           const { data: roomDirectoryWithSearchPageHtml } = await fetchEndpointAsText(archiveUrl);
           const dom = parseHTML(roomDirectoryWithSearchPageHtml);
@@ -2814,7 +2832,7 @@ describe('matrix-public-archive', () => {
           searchTerm: visibleRoomConfigurations[visibleRoomConfigurations.length - 1].name,
         });
 
-        // Visit a sequence of pages using the pagination links
+        // Visit a sequence of pages using the pagination links: 1 -> 2 -> 3 -> 2 -> 1
         const firstPage = await checkRoomsOnPage(archiveUrl);
         const secondPage = await checkRoomsOnPage(firstPage.nextPaginationLink);
         const thirdPage = await checkRoomsOnPage(secondPage.nextPaginationLink);
@@ -2823,29 +2841,6 @@ describe('matrix-public-archive', () => {
           backtrackSecondPage.previousPaginationLink
         );
 
-        // assert.strictEqual(firstPage.previousPaginationLink, null);
-        // assert.strictEqual(firstPage.nextPaginationLink, secondPage.previousPaginationLink);
-        // assert.strictEqual(secondPage.nextPaginationLink, thirdPage.previousPaginationLink);
-        // assert.strictEqual(thirdPage.nextPaginationLink, null);
-
-        function roomIdToRoomName(expectedRoomId) {
-          const roomIndex = createdRoomsIds.findIndex((roomId) => {
-            return roomId === expectedRoomId;
-          });
-          assert(
-            roomIndex > 0,
-            `Expected to find expectedRoomId=${expectedRoomId} in the list of created rooms createdRoomsIds=${createdRoomsIds}`
-          );
-
-          const roomConfig = roomsConfigurationsToCreate[roomIndex];
-          assert(
-            roomConfig,
-            `Expected to find room config for roomIndex=${roomIndex} in the list of roomsConfigurationsToCreate (length ${roomsConfigurationsToCreate.length})}`
-          );
-
-          return roomConfig.name;
-        }
-
         // Ensure that we saw all of the visible rooms paginating through the directory
         assert.deepStrictEqual(
           [...firstPage.roomsIdsOnPage, ...secondPage.roomsIdsOnPage, ...thirdPage.roomsIdsOnPage]