From 487b84b4a0079dafc90fa5f61078c95006d25ccb Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Mon, 16 Aug 2021 09:31:23 +0200 Subject: [PATCH] Implement `offset` and `limit` based paging in pg search engine Signed-off-by: Oliver Sand --- .../src/PgSearchEngine/PgSearchEngine.test.ts | 53 +++++++++-- .../src/PgSearchEngine/PgSearchEngine.ts | 15 ++- .../database/DatabaseDocumentStore.test.ts | 94 ++++++++++++++++++- .../src/database/DatabaseDocumentStore.ts | 44 ++++++--- .../src/database/types.ts | 3 + 5 files changed, 185 insertions(+), 24 deletions(-) diff --git a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts index 7bedee6dea..0940e4d550 100644 --- a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts +++ b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts @@ -27,6 +27,7 @@ describe('PgSearchEngine', () => { transaction: jest.fn(), insertDocuments: jest.fn(), query: jest.fn(), + count: jest.fn(), completeInsert: jest.fn(), prepareInsert: jest.fn(), }; @@ -44,24 +45,55 @@ describe('PgSearchEngine', () => { await searchEngine.query({ term: 'testTerm', filters: {}, - pageCursor: '', + offset: 25, + limit: 50, }); expect(translatorSpy).toHaveBeenCalledWith({ term: 'testTerm', filters: {}, - pageCursor: '', + offset: 25, + limit: 50, + }); + }); + + it('should pass offset and limit', async () => { + const actualTranslatedQuery = searchEngine.translator({ + term: 'Hello', + offset: 25, + limit: 50, + }) as PgSearchQuery; + + expect(actualTranslatedQuery).toMatchObject({ + pgTerm: '("Hello" | "Hello":*)', + offset: 25, + limit: 50, + }); + }); + + it('should have maximum limit of 100', async () => { + const actualTranslatedQuery = searchEngine.translator({ + term: 'Hello', + offset: 25, + limit: 1000, + }) as PgSearchQuery; + + expect(actualTranslatedQuery).toMatchObject({ + pgTerm: '("Hello" | "Hello":*)', + offset: 25, + limit: 100, }); }); it('should return translated query term', async () => { const actualTranslatedQuery = searchEngine.translator({ term: 'Hello World', - pageCursor: '', }) as PgSearchQuery; expect(actualTranslatedQuery).toMatchObject({ pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', + offset: 0, + limit: 25, }); }); @@ -81,13 +113,14 @@ describe('PgSearchEngine', () => { term: 'testTerm', filters: { kind: 'testKind' }, types: ['my-filter'], - pageCursor: '', }) as PgSearchQuery; expect(actualTranslatedQuery).toMatchObject({ pgTerm: '("testTerm" | "testTerm":*)', fields: { kind: 'testKind' }, types: ['my-filter'], + offset: 0, + limit: 25, }); }); }); @@ -138,6 +171,7 @@ describe('PgSearchEngine', () => { describe('query', () => { it('should perform query', async () => { database.transaction.mockImplementation(fn => fn(tx)); + database.count.mockResolvedValue(1337); database.query.mockResolvedValue([ { document: { @@ -151,7 +185,6 @@ describe('PgSearchEngine', () => { const results = await searchEngine.query({ term: 'Hello World', - pageCursor: '', }); expect(results).toEqual({ @@ -165,10 +198,18 @@ describe('PgSearchEngine', () => { type: 'my-type', }, ], + totalCount: 1337, }); - expect(database.transaction).toHaveBeenCalledTimes(1); + expect(database.transaction).toHaveBeenCalledTimes(2); expect(database.query).toHaveBeenCalledWith(tx, { pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', + offset: 0, + limit: 25, + }); + expect(database.count).toHaveBeenCalledWith(tx, { + pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', + offset: 0, + limit: 25, }); }); }); diff --git a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts index 74a1e09010..d5cf498cc1 100644 --- a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts +++ b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts @@ -56,6 +56,8 @@ export class PgSearchEngine implements SearchEngine { .join('&'), fields: query.filters as Record, types: query.types, + offset: query.offset ?? 0, + limit: Math.min(query.limit ?? 25, 100), }; } @@ -79,14 +81,19 @@ export class PgSearchEngine implements SearchEngine { async query(query: SearchQuery): Promise { const pgQuery = this.translator(query); - const rows = await this.databaseStore.transaction(async tx => - this.databaseStore.query(tx, pgQuery), - ); + const [rows, totalCount] = await Promise.all([ + this.databaseStore.transaction(async tx => + this.databaseStore.query(tx, pgQuery), + ), + this.databaseStore.transaction(async tx => + this.databaseStore.count(tx, pgQuery), + ), + ]); const results = rows.map(({ type, document }) => ({ type, document, })); - return { results }; + return { results, totalCount }; } } diff --git a/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.test.ts b/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.test.ts index b860faca6a..a456d0caf5 100644 --- a/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.test.ts +++ b/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.test.ts @@ -192,6 +192,83 @@ describe('DatabaseDocumentStore', () => { 60_000, ); + it.each(databases.eachSupportedId())( + 'should return requested range, %p', + async databaseId => { + const { store } = await createStore(databaseId); + + await store.transaction(async tx => { + await store.prepareInsert(tx); + await store.insertDocuments(tx, 'test', [ + { + title: 'Lorem Ipsum', + text: 'Hello World', + location: 'LOCATION-1', + }, + { + title: 'Hello World', + text: 'Around the world', + location: 'LOCATION-1', + }, + { + title: 'Another one', + text: 'From the next page', + location: 'LOCATION-1', + }, + ]); + await store.completeInsert(tx, 'test'); + }); + + const rows = await store.transaction(tx => + store.query(tx, { pgTerm: 'Hello & World', offset: 1, limit: 1 }), + ); + + expect(rows).toEqual([ + { + document: { + location: 'LOCATION-1', + text: 'Hello World', + title: 'Lorem Ipsum', + }, + rank: expect.any(Number), + type: 'test', + }, + ]); + }, + 60_000, + ); + + it.each(databases.eachSupportedId())( + 'count by term, %p', + async databaseId => { + const { store } = await createStore(databaseId); + + await store.transaction(async tx => { + await store.prepareInsert(tx); + await store.insertDocuments(tx, 'test', [ + { + title: 'Lorem Ipsum', + text: 'Hello World', + location: 'LOCATION-1', + }, + { + title: 'Hello World', + text: 'Around the world', + location: 'LOCATION-1', + }, + ]); + await store.completeInsert(tx, 'test'); + }); + + const totalCount = await store.transaction(tx => + store.count(tx, { pgTerm: 'Hello & World', offset: 0, limit: 25 }), + ); + + expect(totalCount).toEqual(2); + }, + 60_000, + ); + it.each(databases.eachSupportedId())( 'query by term, %p', async databaseId => { @@ -215,7 +292,7 @@ describe('DatabaseDocumentStore', () => { }); const rows = await store.transaction(tx => - store.query(tx, { pgTerm: 'Hello & World' }), + store.query(tx, { pgTerm: 'Hello & World', offset: 0, limit: 25 }), ); expect(rows).toEqual([ @@ -271,7 +348,12 @@ describe('DatabaseDocumentStore', () => { }); const rows = await store.transaction(tx => - store.query(tx, { pgTerm: 'Hello & World', types: ['my-type'] }), + store.query(tx, { + pgTerm: 'Hello & World', + types: ['my-type'], + offset: 0, + limit: 25, + }), ); expect(rows).toEqual([ @@ -322,6 +404,8 @@ describe('DatabaseDocumentStore', () => { store.query(tx, { pgTerm: 'Hello & World', fields: { myField: 'this' }, + offset: 0, + limit: 25, }), ); @@ -374,6 +458,8 @@ describe('DatabaseDocumentStore', () => { store.query(tx, { pgTerm: 'Hello & World', fields: { myField: ['this', 'that'] }, + offset: 0, + limit: 25, }), ); @@ -433,6 +519,8 @@ describe('DatabaseDocumentStore', () => { store.query(tx, { pgTerm: 'Hello & World', fields: { myField: 'this', otherField: 'another' }, + offset: 0, + limit: 25, }), ); @@ -480,6 +568,8 @@ describe('DatabaseDocumentStore', () => { const rows = await store.transaction(tx => store.query(tx, { fields: { myField: 'this' }, + offset: 0, + limit: 25, }), ); diff --git a/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.ts b/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.ts index 77f15746a6..80398ec707 100644 --- a/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.ts +++ b/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.ts @@ -128,7 +128,7 @@ export class DatabaseDocumentStore implements DatabaseStore { async query( tx: Knex.Transaction, - { types, pgTerm, fields }: PgSearchQuery, + searchQuery: PgSearchQuery, ): Promise { // Builds a query like: // SELECT ts_rank_cd(body, query) AS rank, type, document @@ -136,6 +136,36 @@ export class DatabaseDocumentStore implements DatabaseStore { // WHERE query @@ body AND (document @> '{"kind": "API"}') // ORDER BY rank DESC // LIMIT 10; + const query = this.buildQuery(tx, searchQuery); + + query.select('type', 'document'); + + const { pgTerm, limit, offset } = searchQuery; + + if (pgTerm) { + query + .select(tx.raw('ts_rank_cd(body, query) AS "rank"')) + .orderBy('rank', 'desc'); + } else { + query.select(tx.raw('1 as rank')); + } + + return await query.offset(offset).limit(limit); + } + + async count( + tx: Knex.Transaction, + searchQuery: PgSearchQuery, + ): Promise { + const query = this.buildQuery(tx, searchQuery); + const [row] = await query.count(); + return Number(row.count); + } + + private buildQuery( + tx: Knex.Transaction, + { types, pgTerm, fields }: PgSearchQuery, + ) { const query = tx('documents'); if (pgTerm) { @@ -164,16 +194,6 @@ export class DatabaseDocumentStore implements DatabaseStore { }); } - query.select('type', 'document'); - - if (pgTerm) { - query - .select(tx.raw('ts_rank_cd(body, query) AS "rank"')) - .orderBy('rank', 'desc'); - } else { - query.select(tx.raw('1 as rank')); - } - - return await query.limit(100); + return query; } } diff --git a/plugins/search-backend-module-pg/src/database/types.ts b/plugins/search-backend-module-pg/src/database/types.ts index 8c98628caf..f8425c41b6 100644 --- a/plugins/search-backend-module-pg/src/database/types.ts +++ b/plugins/search-backend-module-pg/src/database/types.ts @@ -20,6 +20,8 @@ export interface PgSearchQuery { fields?: Record; types?: string[]; pgTerm?: string; + offset: number; + limit: number; } export interface DatabaseStore { @@ -35,6 +37,7 @@ export interface DatabaseStore { tx: Knex.Transaction, pgQuery: PgSearchQuery, ): Promise; + count(tx: Knex.Transaction, pgQuery: PgSearchQuery): Promise; } export interface RawDocumentRow {