Better test, still not working
This commit is contained in:
parent
84aae1489b
commit
b3b15bdfe6
|
@ -3,6 +3,7 @@
|
|||
const assert = require('assert');
|
||||
|
||||
const urlJoin = require('url-join');
|
||||
const { DIRECTION } = require('matrix-public-archive-shared/lib/reference-values');
|
||||
const { fetchEndpointAsJson } = require('../fetch-endpoint');
|
||||
const { traceFunction } = require('../../tracing/trace-utilities');
|
||||
|
||||
|
@ -43,18 +44,34 @@ async function requestPublicRooms(
|
|||
return publicRoomsRes;
|
||||
}
|
||||
|
||||
// eslint-disable-next-line complexity
|
||||
// eslint-disable-next-line complexity, max-statements
|
||||
async function fetchAccessibleRooms(
|
||||
accessToken,
|
||||
{ server, searchTerm, paginationToken, limit, abortSignal } = {}
|
||||
{
|
||||
server,
|
||||
searchTerm,
|
||||
// Direction is baked into the pagination token but we're unable to decipher it from
|
||||
// the opaque token, we also have to pass it in explicitly.
|
||||
paginationToken,
|
||||
direction = DIRECTION.forward,
|
||||
limit,
|
||||
abortSignal,
|
||||
} = {}
|
||||
) {
|
||||
assert(accessToken);
|
||||
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);
|
||||
|
||||
let accessibleRooms = [];
|
||||
let nextPaginationToken = paginationToken;
|
||||
let prevPaginationToken;
|
||||
|
||||
let lastPaginationToken;
|
||||
let firstResponse;
|
||||
let lastResponse;
|
||||
|
||||
let loopToken = paginationToken;
|
||||
let lastLoopToken;
|
||||
let continuationIndex;
|
||||
let currentRequestCount = 0;
|
||||
while (
|
||||
|
@ -66,31 +83,28 @@ async function fetchAccessibleRooms(
|
|||
// Always do the first request
|
||||
(currentRequestCount === 0 ||
|
||||
// If we have a next token, we can do another request
|
||||
(currentRequestCount > 0 && nextPaginationToken))
|
||||
(currentRequestCount > 0 && loopToken))
|
||||
) {
|
||||
const publicRoomsRes = await requestPublicRooms(accessToken, {
|
||||
server,
|
||||
searchTerm,
|
||||
paginationToken: nextPaginationToken,
|
||||
// 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.
|
||||
limit: Math.ceil(2.5 * limit),
|
||||
paginationToken: loopToken,
|
||||
limit: bulkPaginationLimit,
|
||||
abortSignal,
|
||||
});
|
||||
lastPaginationToken = nextPaginationToken;
|
||||
lastLoopToken = loopToken;
|
||||
lastResponse = publicRoomsRes;
|
||||
|
||||
// We keep track prev_batch token from the first request only as we may be
|
||||
// paginating over many pages but to the client, it just appears like one they can
|
||||
// go back to.
|
||||
if (currentRequestCount === 0) {
|
||||
prevPaginationToken = publicRoomsRes.prev_batch;
|
||||
firstResponse = publicRoomsRes;
|
||||
}
|
||||
|
||||
// Keep track of this as we go. For the final pagination token, we return to the
|
||||
// client, we might need to back-track later to get the perfect continuation point
|
||||
// if we find more than the limit of rooms we want to fetch. Alternatively, we could
|
||||
// just not worry about and show more than the limit of rooms.
|
||||
nextPaginationToken = publicRoomsRes.next_batch;
|
||||
loopToken =
|
||||
direction === DIRECTION.forward ? publicRoomsRes.next_batch : publicRoomsRes.prev_batch;
|
||||
|
||||
// We only want to see world_readable rooms in the archive
|
||||
let index = 0;
|
||||
|
@ -114,30 +128,64 @@ async function fetchAccessibleRooms(
|
|||
currentRequestCount += 1;
|
||||
}
|
||||
|
||||
// Back-track to get the perfect continuation point
|
||||
//
|
||||
// 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.
|
||||
//
|
||||
// Alternatively, we could just not worry about and show more than the limit of rooms
|
||||
let nextPaginationToken;
|
||||
let prevPaginationToken;
|
||||
if (continuationIndex) {
|
||||
// let continuationLimit;
|
||||
// if (direction === DIRECTION.forward) {
|
||||
// continuationLimit = continuationIndex + 1;
|
||||
// } else if (direction === DIRECTION.backward) {
|
||||
// continuationLimit = bulkPaginationLimit - continuationIndex;
|
||||
// } else {
|
||||
// throw new Error(`Invalid direction: ${direction}`);
|
||||
// }
|
||||
|
||||
const publicRoomsRes = await requestPublicRooms(accessToken, {
|
||||
server,
|
||||
searchTerm,
|
||||
// Start from the beginning of the last request
|
||||
paginationToken: lastPaginationToken,
|
||||
// Then go out only as far as the continuation index (the point when we filled the limit)
|
||||
// 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)
|
||||
limit: continuationIndex + 1,
|
||||
abortSignal,
|
||||
});
|
||||
|
||||
nextPaginationToken = publicRoomsRes.next_batch;
|
||||
// console.log('firstResponse', firstResponse.prev_batch, firstResponse.next_batch);
|
||||
// console.log('back-track publicRoomsRes', publicRoomsRes.prev_batch, publicRoomsRes.next_batch);
|
||||
|
||||
if (direction === DIRECTION.forward) {
|
||||
prevPaginationToken = firstResponse.prev_batch;
|
||||
nextPaginationToken = publicRoomsRes.next_batch;
|
||||
} else if (direction === DIRECTION.backward) {
|
||||
prevPaginationToken = publicRoomsRes.prev_batch;
|
||||
nextPaginationToken = firstResponse.next_batch;
|
||||
} else {
|
||||
throw new Error(`Invalid direction: ${direction}`);
|
||||
}
|
||||
} else {
|
||||
if (direction === DIRECTION.forward) {
|
||||
prevPaginationToken = firstResponse.prev_batch;
|
||||
nextPaginationToken = lastResponse.next_batch;
|
||||
} else if (direction === DIRECTION.backward) {
|
||||
prevPaginationToken = lastResponse.prev_batch;
|
||||
nextPaginationToken = firstResponse.next_batch;
|
||||
} else {
|
||||
throw new Error(`Invalid direction: ${direction}`);
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
rooms: accessibleRooms,
|
||||
nextPaginationToken,
|
||||
prevPaginationToken,
|
||||
nextPaginationToken,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
@ -6,6 +6,7 @@ const urlJoin = require('url-join');
|
|||
const express = require('express');
|
||||
const asyncHandler = require('../lib/express-async-handler');
|
||||
|
||||
const { DIRECTION } = require('matrix-public-archive-shared/lib/reference-values');
|
||||
const RouteTimeoutAbortError = require('../lib/errors/route-timeout-abort-error');
|
||||
const UserClosedConnectionAbortError = require('../lib/errors/user-closed-connection-abort-error');
|
||||
const identifyRoute = require('../middleware/identify-route-middleware');
|
||||
|
@ -33,9 +34,19 @@ router.get(
|
|||
'/',
|
||||
identifyRoute('app-room-directory-index'),
|
||||
asyncHandler(async function (req, res) {
|
||||
const paginationToken = req.query.page;
|
||||
const searchTerm = req.query.search;
|
||||
const homeserver = req.query.homeserver;
|
||||
const paginationToken = req.query.page;
|
||||
const direction = req.query.dir;
|
||||
|
||||
// You must provide both `paginationToken` and `direction` if either is defined
|
||||
if (paginationToken || direction) {
|
||||
assert(
|
||||
[DIRECTION.forward, DIRECTION.backward].includes(direction),
|
||||
'?dir query parameter must be [f|b]'
|
||||
);
|
||||
assert(paginationToken, '?page query parameter must be defined if ?dir is defined');
|
||||
}
|
||||
|
||||
// It would be good to grab more rooms than we display in case we need
|
||||
// to filter any out but then the pagination tokens with the homeserver
|
||||
|
@ -54,6 +65,7 @@ router.get(
|
|||
server: homeserver,
|
||||
searchTerm,
|
||||
paginationToken,
|
||||
direction,
|
||||
limit,
|
||||
abortSignal: req.abortSignal,
|
||||
}
|
||||
|
|
|
@ -3,7 +3,10 @@
|
|||
const urlJoin = require('url-join');
|
||||
|
||||
const assert = require('matrix-public-archive-shared/lib/assert');
|
||||
const { TIME_PRECISION_VALUES } = require('matrix-public-archive-shared/lib/reference-values');
|
||||
const {
|
||||
DIRECTION,
|
||||
TIME_PRECISION_VALUES,
|
||||
} = require('matrix-public-archive-shared/lib/reference-values');
|
||||
|
||||
function qsToUrlPiece(qs) {
|
||||
if (qs.toString()) {
|
||||
|
@ -25,7 +28,16 @@ class URLCreator {
|
|||
return `https://matrix.to/#/${roomIdOrAlias}`;
|
||||
}
|
||||
|
||||
roomDirectoryUrl({ searchTerm, homeserver, paginationToken } = {}) {
|
||||
roomDirectoryUrl({ searchTerm, homeserver, paginationToken, direction } = {}) {
|
||||
// You must provide both `paginationToken` and `direction` if either is defined
|
||||
if (paginationToken || direction) {
|
||||
assert(
|
||||
[DIRECTION.forward, DIRECTION.backward].includes(direction),
|
||||
'direction must be [f|b]'
|
||||
);
|
||||
assert(paginationToken);
|
||||
}
|
||||
|
||||
let qs = new URLSearchParams();
|
||||
if (searchTerm) {
|
||||
qs.append('search', searchTerm);
|
||||
|
@ -36,6 +48,9 @@ class URLCreator {
|
|||
if (paginationToken) {
|
||||
qs.append('page', paginationToken);
|
||||
}
|
||||
if (direction) {
|
||||
qs.append('dir', direction);
|
||||
}
|
||||
|
||||
return `${this._basePath}${qsToUrlPiece(qs)}`;
|
||||
}
|
||||
|
|
|
@ -9,6 +9,7 @@ const ModalViewModel = require('matrix-public-archive-shared/viewmodels/ModalVie
|
|||
const HomeserverSelectionModalContentViewModel = require('matrix-public-archive-shared/viewmodels/HomeserverSelectionModalContentViewModel');
|
||||
const RoomCardViewModel = require('matrix-public-archive-shared/viewmodels/RoomCardViewModel');
|
||||
const checkTextForNsfw = require('matrix-public-archive-shared/lib/check-text-for-nsfw');
|
||||
const { DIRECTION } = require('../lib/reference-values');
|
||||
|
||||
const DEFAULT_SERVER_LIST = ['matrix.org', 'gitter.im'];
|
||||
|
||||
|
@ -304,6 +305,7 @@ class RoomDirectoryViewModel extends ViewModel {
|
|||
homeserver: this.homeserverSelection,
|
||||
searchTerm: this.searchTerm,
|
||||
paginationToken: this._nextPaginationToken,
|
||||
direction: DIRECTION.forward,
|
||||
});
|
||||
}
|
||||
|
||||
|
@ -316,6 +318,7 @@ class RoomDirectoryViewModel extends ViewModel {
|
|||
homeserver: this.homeserverSelection,
|
||||
searchTerm: this.searchTerm,
|
||||
paginationToken: this._prevPaginationToken,
|
||||
direction: DIRECTION.backward,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
@ -2745,7 +2745,7 @@ describe('matrix-public-archive', () => {
|
|||
// 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.
|
||||
await Promise.all(
|
||||
const createdRoomsIds = await Promise.all(
|
||||
roomsConfigurationsToCreate.map((roomCreateOptions) =>
|
||||
createTestRoom(client, roomCreateOptions)
|
||||
)
|
||||
|
@ -2786,6 +2786,7 @@ describe('matrix-public-archive', () => {
|
|||
const nextPaginationLink = nextLinkElement.getAttribute('href');
|
||||
|
||||
return {
|
||||
archiveUrl,
|
||||
roomsIdsOnPage,
|
||||
previousPaginationLink,
|
||||
nextPaginationLink,
|
||||
|
@ -2817,12 +2818,56 @@ describe('matrix-public-archive', () => {
|
|||
const firstPage = await checkRoomsOnPage(archiveUrl);
|
||||
const secondPage = await checkRoomsOnPage(firstPage.nextPaginationLink);
|
||||
const thirdPage = await checkRoomsOnPage(secondPage.nextPaginationLink);
|
||||
const backtrackSecondPage = await checkRoomsOnPage(thirdPage.previousPaginationLink);
|
||||
const backtrackFirstPage = await checkRoomsOnPage(
|
||||
backtrackSecondPage.previousPaginationLink
|
||||
);
|
||||
|
||||
// TODO: This does not work (pagination tokens are not stable)
|
||||
assert.strictEqual(firstPage.previousPaginationLink, null);
|
||||
assert.strictEqual(firstPage.nextPaginationLink, secondPage.previousPaginationLink);
|
||||
assert.strictEqual(secondPage.nextPaginationLink, thirdPage.previousPaginationLink);
|
||||
assert.strictEqual(thirdPage.nextPaginationLink, null);
|
||||
// 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]
|
||||
.map(roomIdToRoomName)
|
||||
.sort(),
|
||||
visibleRoomConfigurations.map((roomConfig) => roomConfig.name).sort(),
|
||||
'Make sure we saw all visible rooms paginating through the directory'
|
||||
);
|
||||
|
||||
// Ensure that we see the same rooms in the same order going backward that we saw going forward
|
||||
archiveUrl = backtrackSecondPage.archiveUrl;
|
||||
assert.deepStrictEqual(
|
||||
backtrackSecondPage.roomsIdsOnPage.map(roomIdToRoomName),
|
||||
secondPage.roomsIdsOnPage.map(roomIdToRoomName),
|
||||
'From the third page, going backward to second page should show the same rooms that we saw on the second page when going forward'
|
||||
);
|
||||
archiveUrl = backtrackFirstPage.archiveUrl;
|
||||
assert.deepStrictEqual(
|
||||
backtrackFirstPage.roomsIdsOnPage.map(roomIdToRoomName),
|
||||
firstPage.roomsIdsOnPage.map(roomIdToRoomName),
|
||||
'From the second page, going backward to first page should show the same rooms that we saw on first page when going forward'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
Loading…
Reference in New Issue