From d0cfba7e8b58b2be96eed481e0f3b6f175a052cd Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Mon, 8 Apr 2024 11:35:38 -0400 Subject: [PATCH 1/2] include realm version meta in search results --- packages/host/tests/unit/query-test.ts | 46 ++++++++++++++++++++++- packages/runtime-common/indexer/client.ts | 33 ++++++++++++---- packages/runtime-common/query.ts | 2 +- 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/packages/host/tests/unit/query-test.ts b/packages/host/tests/unit/query-test.ts index e4782e18ef..2218ad6733 100644 --- a/packages/host/tests/unit/query-test.ts +++ b/packages/host/tests/unit/query-test.ts @@ -173,7 +173,11 @@ module('Unit | query', function (hooks) { let { mango, vangogh, paper } = testCards; await setupIndex(client, [mango, vangogh, paper]); - let { cards, meta } = await client.search({}, loader); + let { cards, meta } = await client.search( + new URL(testRealmURL), + {}, + loader, + ); assert.strictEqual(meta.page.total, 3, 'the total results meta is correct'); assert.deepEqual( cards, @@ -194,7 +198,7 @@ module('Unit | query', function (hooks) { { card: paper, data: { is_deleted: true } }, ]); - let { meta } = await client.search({}, loader); + let { meta } = await client.search(new URL(testRealmURL), {}, loader); assert.strictEqual(meta.page.total, 2, 'the total results meta is correct'); }); @@ -203,6 +207,7 @@ module('Unit | query', function (hooks) { await setupIndex(client, [mango, vangogh, paper]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { type: { module: `${testRealmURL}person`, name: 'Person' }, @@ -230,6 +235,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { eq: { name: 'Mango' }, @@ -285,6 +291,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -326,6 +333,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -369,6 +377,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -406,6 +415,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { @@ -457,6 +467,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { @@ -505,6 +516,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -552,6 +564,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -599,6 +612,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -640,6 +654,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -675,6 +690,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -730,6 +746,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -780,6 +797,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -809,6 +827,7 @@ module('Unit | query', function (hooks) { try { await client.search( + new URL(testRealmURL), { filter: { on: { @@ -837,6 +856,7 @@ module('Unit | query', function (hooks) { }; try { await client.search( + new URL(testRealmURL), { filter: { on: cardRef, @@ -863,6 +883,7 @@ module('Unit | query', function (hooks) { try { await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -907,6 +928,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -950,6 +972,7 @@ module('Unit | query', function (hooks) { { let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -968,6 +991,7 @@ module('Unit | query', function (hooks) { } { let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -1014,6 +1038,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -1057,6 +1082,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -1097,6 +1123,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -1128,6 +1155,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { on: { module: `${testRealmURL}person`, name: 'Person' }, @@ -1171,6 +1199,7 @@ module('Unit | query', function (hooks) { ); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { eq: { name: 'Mango' }, @@ -1182,6 +1211,11 @@ module('Unit | query', function (hooks) { ); assert.strictEqual(meta.page.total, 2, 'the total results meta is correct'); + assert.strictEqual( + meta.page.realmVersion, + 2, + 'the realm version queried is correct', + ); assert.deepEqual( getIds(cards), [mango.id, vangogh.id], @@ -1215,6 +1249,7 @@ module('Unit | query', function (hooks) { ); let { cards, meta } = await client.search( + new URL(testRealmURL), { filter: { eq: { name: 'Mango' }, @@ -1225,6 +1260,11 @@ module('Unit | query', function (hooks) { ); assert.strictEqual(meta.page.total, 1, 'the total results meta is correct'); + assert.strictEqual( + meta.page.realmVersion, + 1, + 'the realm version queried is correct', + ); assert.deepEqual(getIds(cards), [mango.id], 'results are correct'); }); @@ -1258,6 +1298,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { sort: [ { @@ -1307,6 +1348,7 @@ module('Unit | query', function (hooks) { ]); let { cards, meta } = await client.search( + new URL(testRealmURL), { sort: [ { diff --git a/packages/runtime-common/indexer/client.ts b/packages/runtime-common/indexer/client.ts index 5e9f8bbd96..97e1c0ac25 100644 --- a/packages/runtime-common/indexer/client.ts +++ b/packages/runtime-common/indexer/client.ts @@ -146,7 +146,7 @@ export class IndexerDBClient { WHERE i.card_url =`, param(`${!url.href.endsWith('.json') ? url.href + '.json' : url.href}`), 'AND', - ...realmVersionExpression(!!opts?.useWorkInProgressIndex), + ...realmVersionExpression(opts), ] as Expression)) as unknown as IndexedCardsTable[]; let maybeResult: IndexedCardsTable | undefined = result[0]; if (!maybeResult) { @@ -181,7 +181,7 @@ export class IndexerDBClient { deps_each.value =`, param(cardId), 'AND', - ...realmVersionExpression(true), + ...realmVersionExpression({ useWorkInProgressIndex: true }), 'ORDER BY i.card_url', ] as Expression)) as Pick[]; return rows.map((r) => r.card_url); @@ -192,17 +192,29 @@ export class IndexerDBClient { // which could have conflicting loaders. It is up to the caller to provide the // loader that we should be using. async search( - { filter, sort }: Query, + realmURL: URL, + { filter, sort, page }: Query, loader: Loader, opts?: QueryOptions, // TODO this should be returning a CardCollectionDocument--handle that in // subsequent PR where we start storing card documents in "pristine_doc" ): Promise<{ cards: LooseCardResource[]; meta: QueryResultsMeta }> { + let [{ current_version }] = (await this.query([ + 'SELECT current_version FROM realm_versions WHERE realm_url =', + param(realmURL.href), + ])) as Pick[]; + if (current_version == null) { + throw new Error(`No current version found for realm ${realmURL.href}`); + } + let version = opts?.useWorkInProgressIndex + ? current_version + 1 + : current_version; let conditions: CardExpression[] = [ [ ...every([ + ['i.realm_url = ', param(realmURL.href)], ['is_deleted = FALSE OR is_deleted IS NULL'], - realmVersionExpression(!!opts?.useWorkInProgressIndex), + realmVersionExpression({ withMaxVersion: version }), ]), ], ]; @@ -238,7 +250,9 @@ export class IndexerDBClient { let cards = results .map((r) => r.pristine_doc) .filter(Boolean) as LooseCardResource[]; - let meta = { page: { total: totalResults[0].total } }; + let meta: QueryResultsMeta = { + page: { total: totalResults[0].total, realmVersion: version }, + }; return { cards, meta }; } @@ -959,14 +973,19 @@ async function loadFieldOrCard( } } -function realmVersionExpression(useWorkInProgressIndex: boolean) { +function realmVersionExpression(opts?: { + useWorkInProgressIndex?: boolean; + withMaxVersion?: number; +}) { return [ 'realm_version =', ...addExplicitParens([ 'SELECT MAX(i2.realm_version)', 'FROM indexed_cards i2', 'WHERE i2.card_url = i.card_url', - ...(!useWorkInProgressIndex + ...(opts?.withMaxVersion + ? ['AND i2.realm_version <=', param(opts?.withMaxVersion)] + : !opts?.useWorkInProgressIndex ? // if we are not using the work in progress index then we limit the max // version permitted to the current version for the realm ['AND i2.realm_version <= r.current_version'] diff --git a/packages/runtime-common/query.ts b/packages/runtime-common/query.ts index 34fb06b6a4..f6f8cac59b 100644 --- a/packages/runtime-common/query.ts +++ b/packages/runtime-common/query.ts @@ -7,7 +7,7 @@ import { type CodeRef, isCodeRef } from './index'; export interface Query { filter?: Filter; sort?: Sort; - page?: { size?: number | string; cursor?: string }; // Support for this is not yet implmented + page?: { size?: number | string; realmVersion?: number }; } export type CardURL = string; From deb33dcd8c32484bfa69a85b31f2947ab2c2121e Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Mon, 8 Apr 2024 14:08:36 -0400 Subject: [PATCH 2/2] Add paging support for search --- packages/host/tests/helpers/indexer.ts | 2 +- packages/host/tests/unit/indexer-test.ts | 8 +- packages/host/tests/unit/query-test.ts | 175 +++++++++++++++++++++- packages/runtime-common/indexer/client.ts | 67 ++++----- packages/runtime-common/query.ts | 6 +- 5 files changed, 213 insertions(+), 45 deletions(-) diff --git a/packages/host/tests/helpers/indexer.ts b/packages/host/tests/helpers/indexer.ts index 11830d7aa2..d45d78097a 100644 --- a/packages/host/tests/helpers/indexer.ts +++ b/packages/host/tests/helpers/indexer.ts @@ -73,7 +73,7 @@ export async function serializeCard(card: CardDef): Promise { return api.serializeCard(card).data as CardResource; } -type TestIndexRow = +export type TestIndexRow = | (Pick & Partial>) | CardDef diff --git a/packages/host/tests/unit/indexer-test.ts b/packages/host/tests/unit/indexer-test.ts index 65b2cce30c..d679e12bdd 100644 --- a/packages/host/tests/unit/indexer-test.ts +++ b/packages/host/tests/unit/indexer-test.ts @@ -461,11 +461,11 @@ module('Unit | indexer', function (hooks) { ); assert.strictEqual( versions.length, - 1, + 2, 'correct number of versions exist for the entry after finishing the batch', ); - let [finalVersion] = versions; + let [_, finalVersion] = versions; assert.deepEqual( finalVersion, { @@ -558,11 +558,11 @@ module('Unit | indexer', function (hooks) { ); assert.strictEqual( versions.length, - 1, + 2, 'correct number of versions exist for the entry after finishing the batch', ); - let [finalVersion] = versions; + let [_, finalVersion] = versions; assert.deepEqual( finalVersion, { realm_version: 2, is_deleted: true }, diff --git a/packages/host/tests/unit/query-test.ts b/packages/host/tests/unit/query-test.ts index 2218ad6733..bd5548993d 100644 --- a/packages/host/tests/unit/query-test.ts +++ b/packages/host/tests/unit/query-test.ts @@ -17,7 +17,13 @@ import { shimExternals } from '@cardstack/host/lib/externals'; import { CardDef } from 'https://cardstack.com/base/card-api'; -import { testRealmURL, setupIndex, serializeCard, p } from '../helpers'; +import { + testRealmURL, + setupIndex, + serializeCard, + p, + type TestIndexRow, +} from '../helpers'; let cardApi: typeof import('https://cardstack.com/base/card-api'); let string: typeof import('https://cardstack.com/base/string'); @@ -1368,4 +1374,171 @@ module('Unit | query', function (hooks) { 'results are correct', ); }); + + test('can get paginated results that are stable during index mutations', async function (assert) { + let { mango } = testCards; + let Card = mango.constructor as typeof CardDef; + let testData: TestIndexRow[] = []; + for (let i = 0; i < 10; i++) { + testData.push({ + card: new Card({ id: `${testRealmURL}mango${i}` }), + data: { search_doc: { name: `Mango-${i}` } }, + }); + } + + await setupIndex(client, testData); + + // page 1 + let { cards, meta } = await client.search( + new URL(testRealmURL), + { + page: { number: 0, size: 3 }, + sort: [ + { + on: { module: `${testRealmURL}person`, name: 'Person' }, + by: 'name', + direction: 'desc', + }, + ], + filter: { + on: { module: `${testRealmURL}person`, name: 'Person' }, + contains: { name: 'Mango' }, + }, + }, + loader, + ); + + let { + page: { total, realmVersion }, + } = meta; + assert.strictEqual(total, 10, 'the total results meta is correct'); + assert.strictEqual(realmVersion, 1, 'the query realm version is correct'); + assert.deepEqual(getIds(cards), [ + `${testRealmURL}mango9`, + `${testRealmURL}mango8`, + `${testRealmURL}mango7`, + ]); + + { + // page 2 + let { cards, meta } = await client.search( + new URL(testRealmURL), + { + // providing the realm version received from the 1st page's meta keeps + // the result set stable while we page over it + page: { number: 1, size: 3, realmVersion }, + sort: [ + { + on: { module: `${testRealmURL}person`, name: 'Person' }, + by: 'name', + direction: 'desc', + }, + ], + filter: { + on: { module: `${testRealmURL}person`, name: 'Person' }, + contains: { name: 'Mango' }, + }, + }, + loader, + ); + assert.strictEqual( + meta.page.total, + 10, + 'the total results meta is correct', + ); + assert.strictEqual( + meta.page.realmVersion, + 1, + 'the query realm version is correct', + ); + assert.deepEqual(getIds(cards), [ + `${testRealmURL}mango6`, + `${testRealmURL}mango5`, + `${testRealmURL}mango4`, + ]); + } + + // mutate the index + let batch = await client.createBatch(new URL(testRealmURL)); + await batch.deleteEntry(new URL(`${testRealmURL}mango3.json`)); + await batch.done(); + + { + // page 3 + let { cards, meta } = await client.search( + new URL(testRealmURL), + { + // providing the realm version received from the 1st page's meta keeps + // the result set stable while we page over it + page: { number: 2, size: 3, realmVersion }, + sort: [ + { + on: { module: `${testRealmURL}person`, name: 'Person' }, + by: 'name', + direction: 'desc', + }, + ], + filter: { + on: { module: `${testRealmURL}person`, name: 'Person' }, + contains: { name: 'Mango' }, + }, + }, + loader, + ); + assert.strictEqual( + meta.page.total, + 10, + 'the total results meta is correct', + ); + assert.strictEqual( + meta.page.realmVersion, + 1, + 'the query realm version is correct', + ); + assert.deepEqual(getIds(cards), [ + `${testRealmURL}mango3`, // this is actually removed in the current index + `${testRealmURL}mango2`, + `${testRealmURL}mango1`, + ]); + } + + // assert that a new search against the current index no longer contains the + // removed card + { + let { cards, meta } = await client.search( + new URL(testRealmURL), + { + sort: [ + { + on: { module: `${testRealmURL}person`, name: 'Person' }, + by: 'name', + direction: 'desc', + }, + ], + filter: { + on: { module: `${testRealmURL}person`, name: 'Person' }, + contains: { name: 'Mango' }, + }, + }, + loader, + ); + + let { + page: { total, realmVersion }, + } = meta; + assert.strictEqual(total, 9, 'the total results meta is correct'); + assert.strictEqual(realmVersion, 2, 'the query realm version is correct'); + assert.deepEqual(getIds(cards), [ + `${testRealmURL}mango9`, + `${testRealmURL}mango8`, + `${testRealmURL}mango7`, + `${testRealmURL}mango6`, + `${testRealmURL}mango5`, + `${testRealmURL}mango4`, + `${testRealmURL}mango2`, + `${testRealmURL}mango1`, + `${testRealmURL}mango0`, + ]); + } + }); }); diff --git a/packages/runtime-common/indexer/client.ts b/packages/runtime-common/indexer/client.ts index 97e1c0ac25..969055415e 100644 --- a/packages/runtime-common/indexer/client.ts +++ b/packages/runtime-common/indexer/client.ts @@ -83,9 +83,7 @@ interface QueryResultsMeta { // consistent paginated results... page: { total: number; - realmVersion?: number; - startIndex?: number; - pageSize?: number; + realmVersion: number; }; } @@ -166,11 +164,12 @@ export class IndexerDBClient { } async cardsThatReference(cardId: string): Promise { - // TODO we really need a cursor based solution to iterate through - // this--pervious implementations ran into a bug that necessitated a cursor - // for large invalidations. But beware, cursor support for SQLite in worker - // mode is very limited. This will likely require some custom work... - + // TODO we really need a solution to iterate through large invalidation + // result sets for this--pervious implementations ran into a bug that + // necessitated a cursor for large invalidations. But beware, there is no + // cursor support for SQLite in worker mode. Instead, implement paging for + // this query. we can probably do something similar to how we are paging the + // search() method using realm_version for stability between pages. let rows = (await this.query([ `SELECT card_url FROM @@ -199,24 +198,25 @@ export class IndexerDBClient { // TODO this should be returning a CardCollectionDocument--handle that in // subsequent PR where we start storing card documents in "pristine_doc" ): Promise<{ cards: LooseCardResource[]; meta: QueryResultsMeta }> { - let [{ current_version }] = (await this.query([ - 'SELECT current_version FROM realm_versions WHERE realm_url =', - param(realmURL.href), - ])) as Pick[]; - if (current_version == null) { - throw new Error(`No current version found for realm ${realmURL.href}`); + let version: number; + if (page?.realmVersion) { + version = page.realmVersion; + } else { + let [{ current_version }] = (await this.query([ + 'SELECT current_version FROM realm_versions WHERE realm_url =', + param(realmURL.href), + ])) as Pick[]; + if (current_version == null) { + throw new Error(`No current version found for realm ${realmURL.href}`); + } + version = opts?.useWorkInProgressIndex + ? current_version + 1 + : current_version; } - let version = opts?.useWorkInProgressIndex - ? current_version + 1 - : current_version; let conditions: CardExpression[] = [ - [ - ...every([ - ['i.realm_url = ', param(realmURL.href)], - ['is_deleted = FALSE OR is_deleted IS NULL'], - realmVersionExpression({ withMaxVersion: version }), - ]), - ], + ['i.realm_url = ', param(realmURL.href)], + ['is_deleted = FALSE OR is_deleted IS NULL'], + realmVersionExpression({ withMaxVersion: version }), ]; if (filter) { conditions.push(this.filterCondition(filter, baseCardRef)); @@ -225,16 +225,17 @@ export class IndexerDBClient { let everyCondition = every(conditions); let query = [ 'SELECT card_url, pristine_doc', - `FROM indexed_cards as i ${placeholder}`, + `FROM indexed_cards AS i ${placeholder}`, `INNER JOIN realm_versions r ON i.realm_url = r.realm_url`, 'WHERE', ...everyCondition, 'GROUP BY card_url', ...this.orderExpression(sort), + ...(page ? [`LIMIT ${page.size} OFFSET ${page.number * page.size}`] : []), ]; let queryCount = [ - 'SELECT count(DISTINCT card_url) as total', - `FROM indexed_cards as i ${placeholder}`, + 'SELECT count(DISTINCT card_url) AS total', + `FROM indexed_cards AS i ${placeholder}`, `INNER JOIN realm_versions r ON i.realm_url = r.realm_url`, 'WHERE', ...everyCondition, @@ -823,23 +824,13 @@ export class Batch { ...addExplicitParens(separatedByCommas(valueExpressions)), ] as Expression); - // prune obsolete index entries + // prune obsolete generation index entries if (this.isNewGeneration) { await this.client.query([ `DELETE FROM indexed_cards`, 'WHERE realm_version <', param(this.realmVersion), ]); - } else { - await this.client.query([ - `DELETE FROM indexed_cards`, - `WHERE card_url IN`, - ...addExplicitParens( - separatedByCommas([...this.touched].map((i) => [param(i)])), - ), - 'AND realm_version <', - param(this.realmVersion), - ] as Expression); } } diff --git a/packages/runtime-common/query.ts b/packages/runtime-common/query.ts index f6f8cac59b..5f294fb85d 100644 --- a/packages/runtime-common/query.ts +++ b/packages/runtime-common/query.ts @@ -7,7 +7,11 @@ import { type CodeRef, isCodeRef } from './index'; export interface Query { filter?: Filter; sort?: Sort; - page?: { size?: number | string; realmVersion?: number }; + page?: { + number: number; // page.number is 0-based + size: number; + realmVersion?: number; + }; } export type CardURL = string;