diff --git a/.changeset/search-happy-owls-sneeze.md b/.changeset/search-happy-owls-sneeze.md index 7c2959d020..4740c767d2 100644 --- a/.changeset/search-happy-owls-sneeze.md +++ b/.changeset/search-happy-owls-sneeze.md @@ -7,4 +7,4 @@ '@backstage/plugin-search-backend-node': patch --- -Implement `offset` and `limit` based paging in search. +Implement optional `pageCursor` based paging in search. diff --git a/packages/search-common/api-report.md b/packages/search-common/api-report.md index b4fcac22ca..f00beb6830 100644 --- a/packages/search-common/api-report.md +++ b/packages/search-common/api-report.md @@ -53,9 +53,7 @@ export interface SearchQuery { // (undocumented) filters?: JsonObject; // (undocumented) - limit?: number; - // (undocumented) - offset?: number; + pageCursor?: string; // (undocumented) term: string; // (undocumented) @@ -77,9 +75,11 @@ export interface SearchResult { // @public (undocumented) export interface SearchResultSet { // (undocumented) - results: SearchResult[]; + nextPageCursor?: string; // (undocumented) - totalCount: number; + previousPageCursor?: string; + // (undocumented) + results: SearchResult[]; } // (No @packageDocumentation comment for this package) diff --git a/packages/search-common/src/types.ts b/packages/search-common/src/types.ts index 0bd12fbd42..063e0cb96c 100644 --- a/packages/search-common/src/types.ts +++ b/packages/search-common/src/types.ts @@ -19,8 +19,7 @@ export interface SearchQuery { term: string; filters?: JsonObject; types?: string[]; - offset?: number; - limit?: number; + pageCursor?: string; } export interface SearchResult { @@ -30,7 +29,8 @@ export interface SearchResult { export interface SearchResultSet { results: SearchResult[]; - totalCount: number; + nextPageCursor?: string; + previousPageCursor?: string; } /** diff --git a/plugins/search-backend-module-elasticsearch/api-report.md b/plugins/search-backend-module-elasticsearch/api-report.md index e538201bc0..1644ce65ea 100644 --- a/plugins/search-backend-module-elasticsearch/api-report.md +++ b/plugins/search-backend-module-elasticsearch/api-report.md @@ -45,8 +45,7 @@ export class ElasticSearchSearchEngine implements SearchEngine { term, filters, types, - offset, - limit, + pageCursor, }: SearchQuery): ConcreteElasticSearchQuery; } diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts index 18ee1314ce..36fb2e56c6 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts +++ b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts @@ -16,12 +16,14 @@ import { getVoidLogger } from '@backstage/backend-common'; import { SearchEngine } from '@backstage/search-common'; -import { - ConcreteElasticSearchQuery, - ElasticSearchSearchEngine, -} from './ElasticSearchSearchEngine'; import { Client } from '@elastic/elasticsearch'; import Mock from '@elastic/elasticsearch-mock'; +import { + ConcreteElasticSearchQuery, + decodePageCursor, + ElasticSearchSearchEngine, + encodePageCursor, +} from './ElasticSearchSearchEngine'; class ElasticSearchSearchEngineForTranslatorTests extends ElasticSearchSearchEngine { getTranslator() { @@ -134,14 +136,13 @@ describe('ElasticSearchSearchEngine', () => { }); }); - it('should pass offset and limit', async () => { + it('should pass page cursor', async () => { const translatorUnderTest = inspectableSearchEngine.getTranslator(); const actualTranslatedQuery = translatorUnderTest({ types: ['indexName'], term: 'testTerm', - offset: 25, - limit: 50, + pageCursor: 'MQ==', }) as ConcreteElasticSearchQuery; expect(actualTranslatedQuery).toMatchObject({ @@ -166,43 +167,7 @@ describe('ElasticSearchSearchEngine', () => { }, }, from: 25, - size: 50, - }); - }); - - it('should have maximum limit of 100', async () => { - const translatorUnderTest = inspectableSearchEngine.getTranslator(); - - const actualTranslatedQuery = translatorUnderTest({ - types: ['indexName'], - term: 'testTerm', - offset: 25, - limit: 500, - }) as ConcreteElasticSearchQuery; - - expect(actualTranslatedQuery).toMatchObject({ - documentTypes: ['indexName'], - elasticSearchQuery: expect.any(Object), - }); - - const queryBody = actualTranslatedQuery.elasticSearchQuery; - - expect(queryBody).toEqual({ - query: { - bool: { - filter: [], - must: { - multi_match: { - query: 'testTerm', - fields: ['*'], - fuzziness: 'auto', - minimum_should_match: 1, - }, - }, - }, - }, - from: 25, - size: 100, + size: 25, }); }); @@ -387,7 +352,103 @@ describe('ElasticSearchSearchEngine', () => { }); // Should return 0 results as nothing is indexed here - expect(mockedSearchResult).toMatchObject({ results: [], totalCount: 0 }); + expect(mockedSearchResult).toMatchObject({ + results: [], + nextPageCursor: undefined, + }); + }); + + it('should perform search query with more results than one page', async () => { + mock.clear({ + method: 'POST', + path: '/*__search/_search', + }); + mock.add( + { + method: 'POST', + path: '/*__search/_search', + }, + () => { + return { + hits: { + total: { value: 30, relation: 'eq' }, + hits: Array(25) + .fill(null) + .map((_, i) => ({ + _index: 'mytype-index__', + _source: { + value: `${i}`, + }, + })), + }, + }; + }, + ); + + const mockedSearchResult = await testSearchEngine.query({ + term: 'testTerm', + filters: {}, + }); + + expect(mockedSearchResult).toMatchObject({ + results: expect.arrayContaining( + Array(25) + .fill(null) + .map((_, i) => ({ + type: 'mytype', + document: { value: `${i}` }, + })), + ), + nextPageCursor: 'MQ==', + }); + }); + + it('should perform search query for second page', async () => { + mock.clear({ + method: 'POST', + path: '/*__search/_search', + }); + mock.add( + { + method: 'POST', + path: '/*__search/_search', + }, + () => { + return { + hits: { + total: { value: 30, relation: 'eq' }, + hits: Array(30) + .fill(null) + .map((_, i) => ({ + _index: 'mytype-index__', + _source: { + value: `${i}`, + }, + })) + .slice(25), + }, + }; + }, + ); + + const mockedSearchResult = await testSearchEngine.query({ + term: 'testTerm', + filters: {}, + pageCursor: 'MQ==', + }); + + expect(mockedSearchResult).toMatchObject({ + results: expect.arrayContaining( + Array(30) + .fill(null) + .map((_, i) => ({ + type: 'mytype', + document: { value: `${i}` }, + })) + .slice(25), + ), + previousPageCursor: 'MA==', + }); }); it('should handle index/search type filtering correctly', async () => { @@ -498,3 +559,19 @@ describe('ElasticSearchSearchEngine', () => { }); }); }); + +describe('decodePageCursor', () => { + test('should decode page', () => { + expect(decodePageCursor('MQ==')).toEqual({ page: 1 }); + }); + + test('should fallback to first page if empty', () => { + expect(decodePageCursor()).toEqual({ page: 0 }); + }); +}); + +describe('encodePageCursor', () => { + test('should encode page', () => { + expect(encodePageCursor({ page: 1 })).toEqual('MQ=='); + }); +}); diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts index c1c959778b..a12daef282 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts +++ b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts @@ -15,24 +15,25 @@ */ import { - IndexableDocument, - SearchQuery, - SearchResultSet, - SearchEngine, -} from '@backstage/search-common'; -import { Logger } from 'winston'; -import esb from 'elastic-builder'; -import { Client } from '@elastic/elasticsearch'; + awsGetCredentials, + createAWSConnection, +} from '@acuris/aws-es-connection'; import { Config } from '@backstage/config'; import { - createAWSConnection, - awsGetCredentials, -} from '@acuris/aws-es-connection'; + IndexableDocument, + SearchEngine, + SearchQuery, + SearchResultSet, +} from '@backstage/search-common'; +import { Client } from '@elastic/elasticsearch'; +import esb from 'elastic-builder'; import { isEmpty, isNaN as nan, isNumber } from 'lodash'; +import { Logger } from 'winston'; export type ConcreteElasticSearchQuery = { documentTypes?: string[]; elasticSearchQuery: Object; + pageSize: number; }; type ElasticSearchQueryTranslator = ( @@ -140,8 +141,7 @@ export class ElasticSearchSearchEngine implements SearchEngine { term, filters = {}, types, - offset, - limit, + pageCursor, }: SearchQuery): ConcreteElasticSearchQuery { const filter = Object.entries(filters) .filter(([_, value]) => Boolean(value)) @@ -169,15 +169,18 @@ export class ElasticSearchSearchEngine implements SearchEngine { .multiMatchQuery(['*'], term) .fuzziness('auto') .minimumShouldMatch(1); + const pageSize = 25; + const { page } = decodePageCursor(pageCursor); return { elasticSearchQuery: esb .requestBodySearch() .query(esb.boolQuery().filter(filter).must([query])) - .from(offset ?? 0) - .size(Math.min(limit ?? 25, 100)) + .from(page * pageSize) + .size(pageSize) .toJSON(), documentTypes: types, + pageSize, }; } @@ -249,7 +252,8 @@ export class ElasticSearchSearchEngine implements SearchEngine { } async query(query: SearchQuery): Promise { - const { elasticSearchQuery, documentTypes } = this.translator(query); + const { elasticSearchQuery, documentTypes, pageSize } = + this.translator(query); const queryIndices = documentTypes ? documentTypes.map(it => this.constructSearchAlias(it)) : this.constructSearchAlias('*'); @@ -258,12 +262,23 @@ export class ElasticSearchSearchEngine implements SearchEngine { index: queryIndices, body: elasticSearchQuery, }); + const { page } = decodePageCursor(query.pageCursor); + const hasNextPage = result.body.hits.total.value > page * pageSize; + const hasPreviousPage = page > 0; + const nextPageCursor = hasNextPage + ? encodePageCursor({ page: page + 1 }) + : undefined; + const previousPageCursor = hasPreviousPage + ? encodePageCursor({ page: page - 1 }) + : undefined; + return { results: result.body.hits.hits.map((d: ElasticSearchResult) => ({ type: this.getTypeFromIndex(d._index), document: d._source, })), - totalCount: result.body.hits.total.value, + nextPageCursor, + previousPageCursor, }; } catch (e) { this.logger.error( @@ -291,3 +306,17 @@ export class ElasticSearchSearchEngine implements SearchEngine { return `${this.indexPrefix}${type}${postFix}`; } } + +export function decodePageCursor(pageCursor?: string): { page: number } { + if (!pageCursor) { + return { page: 0 }; + } + + return { + page: Number(Buffer.from(pageCursor, 'base64').toString('utf-8')), + }; +} + +export function encodePageCursor({ page }: { page: number }): string { + return Buffer.from(`${page}`, 'utf-8').toString('base64'); +} diff --git a/plugins/search-backend-module-pg/api-report.md b/plugins/search-backend-module-pg/api-report.md index 4879e64d06..6a56daa8d7 100644 --- a/plugins/search-backend-module-pg/api-report.md +++ b/plugins/search-backend-module-pg/api-report.md @@ -10,6 +10,14 @@ import { SearchEngine } from '@backstage/plugin-search-backend-node'; import { SearchQuery } from '@backstage/search-common'; import { SearchResultSet } from '@backstage/search-common'; +// Warning: (ae-missing-release-tag) "ConcretePgSearchQuery" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export type ConcretePgSearchQuery = { + pgQuery: PgSearchQuery; + pageSize: number; +}; + // Warning: (ae-missing-release-tag) "DatabaseDocumentStore" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) @@ -18,8 +26,6 @@ export class DatabaseDocumentStore implements DatabaseStore { // (undocumented) completeInsert(tx: Knex.Transaction, type: string): Promise; // (undocumented) - count(tx: Knex.Transaction, searchQuery: PgSearchQuery): Promise; - // (undocumented) static create(knex: Knex): Promise; // (undocumented) insertDocuments( @@ -34,7 +40,7 @@ export class DatabaseDocumentStore implements DatabaseStore { // (undocumented) query( tx: Knex.Transaction, - searchQuery: PgSearchQuery, + { types, pgTerm, fields, offset, limit }: PgSearchQuery, ): Promise; // (undocumented) static supported(knex: Knex): Promise; @@ -49,8 +55,6 @@ export interface DatabaseStore { // (undocumented) completeInsert(tx: Knex.Transaction, type: string): Promise; // (undocumented) - count(tx: Knex.Transaction, pgQuery: PgSearchQuery): Promise; - // (undocumented) insertDocuments( tx: Knex.Transaction, type: string, @@ -83,11 +87,13 @@ export class PgSearchEngine implements SearchEngine { // (undocumented) query(query: SearchQuery): Promise; // (undocumented) - setTranslator(translator: (query: SearchQuery) => PgSearchQuery): void; + setTranslator( + translator: (query: SearchQuery) => ConcretePgSearchQuery, + ): void; // (undocumented) static supported(database: PluginDatabaseManager): Promise; // (undocumented) - translator(query: SearchQuery): PgSearchQuery; + translator(query: SearchQuery): ConcretePgSearchQuery; } // Warning: (ae-missing-release-tag) "PgSearchQuery" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) 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 0940e4d550..56a3a6e643 100644 --- a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts +++ b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.test.ts @@ -14,8 +14,13 @@ * limitations under the License. */ import { range } from 'lodash'; -import { DatabaseStore, PgSearchQuery } from '../database'; -import { PgSearchEngine } from './PgSearchEngine'; +import { DatabaseStore } from '../database'; +import { + ConcretePgSearchQuery, + decodePageCursor, + encodePageCursor, + PgSearchEngine, +} from './PgSearchEngine'; describe('PgSearchEngine', () => { const tx: any = {} as any; @@ -27,7 +32,6 @@ describe('PgSearchEngine', () => { transaction: jest.fn(), insertDocuments: jest.fn(), query: jest.fn(), - count: jest.fn(), completeInsert: jest.fn(), prepareInsert: jest.fn(), }; @@ -45,55 +49,42 @@ describe('PgSearchEngine', () => { await searchEngine.query({ term: 'testTerm', filters: {}, - offset: 25, - limit: 50, }); expect(translatorSpy).toHaveBeenCalledWith({ term: 'testTerm', filters: {}, - offset: 25, - limit: 50, }); }); - it('should pass offset and limit', async () => { + it('should pass page cursor', async () => { const actualTranslatedQuery = searchEngine.translator({ term: 'Hello', - offset: 25, - limit: 50, - }) as PgSearchQuery; - - expect(actualTranslatedQuery).toMatchObject({ - pgTerm: '("Hello" | "Hello":*)', - offset: 25, - limit: 50, + pageCursor: 'MQ==', }); - }); - - 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, + pgQuery: { + pgTerm: '("Hello" | "Hello":*)', + offset: 25, + limit: 26, + }, + pageSize: 25, }); }); it('should return translated query term', async () => { const actualTranslatedQuery = searchEngine.translator({ term: 'Hello World', - }) as PgSearchQuery; + }); expect(actualTranslatedQuery).toMatchObject({ - pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', - offset: 0, - limit: 25, + pgQuery: { + pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', + offset: 0, + limit: 26, + }, + pageSize: 25, }); }); @@ -101,10 +92,13 @@ describe('PgSearchEngine', () => { const actualTranslatedQuery = searchEngine.translator({ term: 'H&e|l!l*o W\0o(r)l:d', pageCursor: '', - }) as PgSearchQuery; + }) as ConcretePgSearchQuery; expect(actualTranslatedQuery).toMatchObject({ - pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', + pgQuery: { + pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', + }, + pageSize: 25, }); }); @@ -113,14 +107,17 @@ describe('PgSearchEngine', () => { term: 'testTerm', filters: { kind: 'testKind' }, types: ['my-filter'], - }) as PgSearchQuery; + }); expect(actualTranslatedQuery).toMatchObject({ - pgTerm: '("testTerm" | "testTerm":*)', - fields: { kind: 'testKind' }, - types: ['my-filter'], - offset: 0, - limit: 25, + pgQuery: { + pgTerm: '("testTerm" | "testTerm":*)', + fields: { kind: 'testKind' }, + types: ['my-filter'], + offset: 0, + limit: 26, + }, + pageSize: 25, }); }); }); @@ -171,7 +168,6 @@ describe('PgSearchEngine', () => { describe('query', () => { it('should perform query', async () => { database.transaction.mockImplementation(fn => fn(tx)); - database.count.mockResolvedValue(1337); database.query.mockResolvedValue([ { document: { @@ -198,19 +194,113 @@ describe('PgSearchEngine', () => { type: 'my-type', }, ], - totalCount: 1337, + nextPageCursor: undefined, }); - expect(database.transaction).toHaveBeenCalledTimes(2); + expect(database.transaction).toHaveBeenCalledTimes(1); expect(database.query).toHaveBeenCalledWith(tx, { pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', offset: 0, - limit: 25, + limit: 26, }); - expect(database.count).toHaveBeenCalledWith(tx, { + }); + + it('should include next page cursor if results exceed page size', async () => { + database.transaction.mockImplementation(fn => fn(tx)); + database.query.mockResolvedValue( + Array(30) + .fill(0) + .map((_, i) => ({ + document: { + title: 'Hello World', + text: 'Lorem Ipsum', + location: `location-${i}`, + }, + type: 'my-type', + })), + ); + + const results = await searchEngine.query({ + term: 'Hello World', + }); + + expect(results).toEqual({ + results: Array(25) + .fill(0) + .map((_, i) => ({ + document: { + title: 'Hello World', + text: 'Lorem Ipsum', + location: `location-${i}`, + }, + type: 'my-type', + })), + nextPageCursor: 'MQ==', + }); + expect(database.transaction).toHaveBeenCalledTimes(1); + expect(database.query).toHaveBeenCalledWith(tx, { pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', offset: 0, - limit: 25, + limit: 26, + }); + }); + + it('should include previous page cursor if on another page', async () => { + database.transaction.mockImplementation(fn => fn(tx)); + database.query.mockResolvedValue( + Array(30) + .fill(0) + .map((_, i) => ({ + document: { + title: 'Hello World', + text: 'Lorem Ipsum', + location: `location-${i}`, + }, + type: 'my-type', + })) + .slice(25), + ); + + const results = await searchEngine.query({ + term: 'Hello World', + pageCursor: 'MQ==', + }); + + expect(results).toEqual({ + results: Array(30) + .fill(0) + .map((_, i) => ({ + document: { + title: 'Hello World', + text: 'Lorem Ipsum', + location: `location-${i}`, + }, + type: 'my-type', + })) + .slice(25), + previousPageCursor: 'MA==', + }); + expect(database.transaction).toHaveBeenCalledTimes(1); + expect(database.query).toHaveBeenCalledWith(tx, { + pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', + offset: 25, + limit: 26, }); }); }); }); + +describe('decodePageCursor', () => { + test('should decode page', () => { + expect(decodePageCursor('MQ==')).toEqual({ page: 1 }); + }); + + test('should fallback to first page if empty', () => { + expect(decodePageCursor()).toEqual({ page: 0 }); + }); +}); + +describe('encodePageCursor', () => { + test('should encode page', () => { + expect(encodePageCursor({ page: 1 })).toEqual('MQ=='); + }); +}); diff --git a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts index d5cf498cc1..a032dd783e 100644 --- a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts +++ b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts @@ -27,7 +27,10 @@ import { PgSearchQuery, } from '../database'; -// TODO: Support paging using page cursor (return cursor and parse cursor) +export type ConcretePgSearchQuery = { + pgQuery: PgSearchQuery; + pageSize: number; +}; export class PgSearchEngine implements SearchEngine { constructor(private readonly databaseStore: DatabaseStore) {} @@ -46,22 +49,33 @@ export class PgSearchEngine implements SearchEngine { return await DatabaseDocumentStore.supported(await database.getClient()); } - translator(query: SearchQuery): PgSearchQuery { + translator(query: SearchQuery): ConcretePgSearchQuery { + const pageSize = 25; + const { page } = decodePageCursor(query.pageCursor); + const offset = page * pageSize; + // We request more result to know whether there is another page + const limit = pageSize + 1; + return { - pgTerm: query.term - .split(/\s/) - .map(p => p.replace(/[\0()|&:*!]/g, '').trim()) - .filter(p => p !== '') - .map(p => `(${JSON.stringify(p)} | ${JSON.stringify(p)}:*)`) - .join('&'), - fields: query.filters as Record, - types: query.types, - offset: query.offset ?? 0, - limit: Math.min(query.limit ?? 25, 100), + pgQuery: { + pgTerm: query.term + .split(/\s/) + .map(p => p.replace(/[\0()|&:*!]/g, '').trim()) + .filter(p => p !== '') + .map(p => `(${JSON.stringify(p)} | ${JSON.stringify(p)}:*)`) + .join('&'), + fields: query.filters as Record, + types: query.types, + offset, + limit, + }, + pageSize, }; } - setTranslator(translator: (query: SearchQuery) => PgSearchQuery): void { + setTranslator( + translator: (query: SearchQuery) => ConcretePgSearchQuery, + ): void { this.translator = translator; } @@ -79,21 +93,44 @@ export class PgSearchEngine implements SearchEngine { } async query(query: SearchQuery): Promise { - const pgQuery = this.translator(query); + const { pgQuery, pageSize } = this.translator(query); - 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 }) => ({ + const rows = await this.databaseStore.transaction(async tx => + this.databaseStore.query(tx, pgQuery), + ); + + // We requested one result more than the page size to know whether there is + // another page. + const { page } = decodePageCursor(query.pageCursor); + const hasNextPage = rows.length > pageSize; + const hasPreviousPage = page > 0; + const pageRows = rows.slice(0, pageSize); + const nextPageCursor = hasNextPage + ? encodePageCursor({ page: page + 1 }) + : undefined; + const previousPageCursor = hasPreviousPage + ? encodePageCursor({ page: page - 1 }) + : undefined; + + const results = pageRows.map(({ type, document }) => ({ type, document, })); - return { results, totalCount }; + return { results, nextPageCursor, previousPageCursor }; } } + +export function decodePageCursor(pageCursor?: string): { page: number } { + if (!pageCursor) { + return { page: 0 }; + } + + return { + page: Number(Buffer.from(pageCursor, 'base64').toString('utf-8')), + }; +} + +export function encodePageCursor({ page }: { page: number }): string { + return Buffer.from(`${page}`, 'utf-8').toString('base64'); +} diff --git a/plugins/search-backend-module-pg/src/PgSearchEngine/index.ts b/plugins/search-backend-module-pg/src/PgSearchEngine/index.ts index b28e03ee5c..7994998baf 100644 --- a/plugins/search-backend-module-pg/src/PgSearchEngine/index.ts +++ b/plugins/search-backend-module-pg/src/PgSearchEngine/index.ts @@ -14,3 +14,4 @@ * limitations under the License. */ export { PgSearchEngine } from './PgSearchEngine'; +export type { ConcretePgSearchQuery } from './PgSearchEngine'; 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 a456d0caf5..71c012ec00 100644 --- a/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.test.ts +++ b/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.test.ts @@ -238,37 +238,6 @@ describe('DatabaseDocumentStore', () => { 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 => { diff --git a/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.ts b/plugins/search-backend-module-pg/src/database/DatabaseDocumentStore.ts index 80398ec707..8c180ea019 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, - searchQuery: PgSearchQuery, + { types, pgTerm, fields, offset, limit }: PgSearchQuery, ): Promise { // Builds a query like: // SELECT ts_rank_cd(body, query) AS rank, type, document @@ -136,36 +136,6 @@ 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) { @@ -194,6 +164,16 @@ export class DatabaseDocumentStore implements DatabaseStore { }); } - return query; + 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.offset(offset).limit(limit); } } diff --git a/plugins/search-backend-module-pg/src/database/types.ts b/plugins/search-backend-module-pg/src/database/types.ts index f8425c41b6..0c0596160e 100644 --- a/plugins/search-backend-module-pg/src/database/types.ts +++ b/plugins/search-backend-module-pg/src/database/types.ts @@ -37,7 +37,6 @@ export interface DatabaseStore { tx: Knex.Transaction, pgQuery: PgSearchQuery, ): Promise; - count(tx: Knex.Transaction, pgQuery: PgSearchQuery): Promise; } export interface RawDocumentRow { diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts b/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts index 71c2b95a29..a2cbb4ecc3 100644 --- a/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts +++ b/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts @@ -17,7 +17,12 @@ import { getVoidLogger } from '@backstage/backend-common'; import lunr from 'lunr'; import { SearchEngine } from '@backstage/search-common'; -import { ConcreteLunrQuery, LunrSearchEngine } from './LunrSearchEngine'; +import { + ConcreteLunrQuery, + LunrSearchEngine, + decodePageCursor, + encodePageCursor, +} from './LunrSearchEngine'; /** * Just used to test the default translator shipped with LunrSearchEngine. @@ -48,12 +53,14 @@ describe('LunrSearchEngine', () => { await testLunrSearchEngine.query({ term: 'testTerm', filters: {}, + pageCursor: 'MQ==', }); // Then: the translator is invoked with expected args. expect(translatorSpy).toHaveBeenCalledWith({ term: 'testTerm', filters: {}, + pageCursor: 'MQ==', }); }); @@ -66,15 +73,12 @@ describe('LunrSearchEngine', () => { const actualTranslatedQuery = translatorUnderTest({ term: 'testTerm', filters: {}, - offset: 0, - limit: 50, }) as ConcreteLunrQuery; expect(actualTranslatedQuery).toMatchObject({ documentTypes: undefined, lunrQueryBuilder: expect.any(Function), - offset: 0, - limit: 50, + pageSize: 25, }); const query: jest.Mocked = { @@ -115,52 +119,7 @@ describe('LunrSearchEngine', () => { expect(actualTranslatedQuery).toMatchObject({ documentTypes: undefined, lunrQueryBuilder: expect.any(Function), - offset: 0, - limit: 25, - }); - - const query: jest.Mocked = { - allFields: [], - clauses: [], - term: jest.fn(), - clause: jest.fn(), - }; - - actualTranslatedQuery.lunrQueryBuilder.bind(query)(query); - - expect(query.term).toBeCalledWith(lunr.tokenizer('testTerm'), { - boost: 100, - usePipeline: true, - }); - expect(query.term).toBeCalledWith(lunr.tokenizer('testTerm'), { - boost: 10, - usePipeline: false, - wildcard: lunr.Query.wildcard.TRAILING, - }); - expect(query.term).toBeCalledWith(lunr.tokenizer('testTerm'), { - boost: 1, - usePipeline: false, - editDistance: 2, - }); - }); - - it('should have maximum limit', async () => { - const inspectableSearchEngine = new LunrSearchEngineForTranslatorTests({ - logger: getVoidLogger(), - }); - const translatorUnderTest = inspectableSearchEngine.getTranslator(); - - const actualTranslatedQuery = translatorUnderTest({ - term: 'testTerm', - offset: 0, - limit: 1000, - }) as ConcreteLunrQuery; - - expect(actualTranslatedQuery).toMatchObject({ - documentTypes: undefined, - lunrQueryBuilder: expect.any(Function), - offset: 0, - limit: 100, + pageSize: 25, }); const query: jest.Mocked = { @@ -328,7 +287,10 @@ describe('LunrSearchEngine', () => { }); // Should return 0 results as nothing is indexed here - expect(mockedSearchResult).toMatchObject({ results: [], totalCount: 0 }); + expect(mockedSearchResult).toMatchObject({ + results: [], + nextPageCursor: undefined, + }); }); it('should perform search query and return 0 results on no match', async () => { @@ -350,7 +312,10 @@ describe('LunrSearchEngine', () => { }); // Should return 0 results as we are mocking the indexing of 1 document but with no match on the fields - expect(mockedSearchResult).toMatchObject({ results: [], totalCount: 0 }); + expect(mockedSearchResult).toMatchObject({ + results: [], + nextPageCursor: undefined, + }); }); it('should perform search query and return all results on empty term', async () => { @@ -382,7 +347,7 @@ describe('LunrSearchEngine', () => { type: 'test-index', }, ], - totalCount: 1, + nextPageCursor: undefined, }); }); @@ -414,7 +379,7 @@ describe('LunrSearchEngine', () => { }, }, ], - totalCount: 1, + nextPageCursor: undefined, }); }); @@ -446,7 +411,7 @@ describe('LunrSearchEngine', () => { }, }, ], - totalCount: 1, + nextPageCursor: undefined, }); }); @@ -479,7 +444,7 @@ describe('LunrSearchEngine', () => { }, }, ], - totalCount: 1, + nextPageCursor: undefined, }); }); @@ -512,7 +477,7 @@ describe('LunrSearchEngine', () => { }, }, ], - totalCount: 1, + nextPageCursor: undefined, }); }); @@ -545,7 +510,7 @@ describe('LunrSearchEngine', () => { }, }, ], - totalCount: 1, + nextPageCursor: undefined, }); }); @@ -585,7 +550,7 @@ describe('LunrSearchEngine', () => { }, }, ], - totalCount: 1, + nextPageCursor: undefined, }); }); @@ -627,7 +592,7 @@ describe('LunrSearchEngine', () => { }, }, ], - totalCount: 1, + nextPageCursor: undefined, }); }); @@ -667,7 +632,7 @@ describe('LunrSearchEngine', () => { }, }, ], - totalCount: 1, + nextPageCursor: undefined, }); }); @@ -725,9 +690,75 @@ describe('LunrSearchEngine', () => { }, }, ], - totalCount: 2, + nextPageCursor: undefined, }); }); + + it('should return next page cursor if results exceed page size', async () => { + const mockDocuments = Array(30) + .fill(0) + .map((_, i) => ({ + title: 'testTitle', + text: 'testText', + location: `test/location/${i}`, + })); + + await testLunrSearchEngine.index('test-index', mockDocuments); + + const mockedSearchResult = await testLunrSearchEngine.query({ + term: 'testTitle', + types: ['test-index'], + }); + + expect(mockedSearchResult).toMatchObject({ + results: Array(25) + .fill(0) + .map((_, i) => ({ + document: { + title: 'testTitle', + text: 'testText', + location: `test/location/${i}`, + }, + type: 'test-index', + })), + nextPageCursor: 'MQ==', + previousPageCursor: undefined, + }); + }); + }); + + it('should return previous page cursor if on another page', async () => { + const mockDocuments = Array(30) + .fill(0) + .map((_, i) => ({ + title: 'testTitle', + text: 'testText', + location: `test/location/${i}`, + })); + + await testLunrSearchEngine.index('test-index', mockDocuments); + + const mockedSearchResult = await testLunrSearchEngine.query({ + term: 'testTitle', + types: ['test-index'], + pageCursor: 'MQ==', + }); + + expect(mockedSearchResult).toMatchObject({ + results: Array(30) + .fill(0) + .map((_, i) => ({ + document: { + title: 'testTitle', + text: 'testText', + location: `test/location/${i}`, + }, + type: 'test-index', + })) + .slice(25), + nextPageCursor: undefined, + previousPageCursor: 'MA==', + }); }); describe('index', () => { @@ -750,3 +781,19 @@ describe('LunrSearchEngine', () => { }); }); }); + +describe('decodePageCursor', () => { + test('should decode page', () => { + expect(decodePageCursor('MQ==')).toEqual({ page: 1 }); + }); + + test('should fallback to first page if empty', () => { + expect(decodePageCursor()).toEqual({ page: 0 }); + }); +}); + +describe('encodePageCursor', () => { + test('should encode page', () => { + expect(encodePageCursor({ page: 1 })).toEqual('MQ=='); + }); +}); diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngine.ts b/plugins/search-backend-node/src/engines/LunrSearchEngine.ts index 75e5fdc625..f22762cb9f 100644 --- a/plugins/search-backend-node/src/engines/LunrSearchEngine.ts +++ b/plugins/search-backend-node/src/engines/LunrSearchEngine.ts @@ -27,8 +27,7 @@ import { Logger } from 'winston'; export type ConcreteLunrQuery = { lunrQueryBuilder: lunr.Index.QueryBuilder; documentTypes?: string[]; - offset: number; - limit: number; + pageSize: number; }; type LunrResultEnvelope = { @@ -52,9 +51,9 @@ export class LunrSearchEngine implements SearchEngine { term, filters, types, - offset, - limit, }: SearchQuery): ConcreteLunrQuery => { + const pageSize = 25; + return { lunrQueryBuilder: q => { const termToken = lunr.tokenizer(term); @@ -111,8 +110,7 @@ export class LunrSearchEngine implements SearchEngine { } }, documentTypes: types, - offset: offset ?? 0, - limit: Math.min(limit ?? 25, 100), + pageSize, }; }; @@ -147,7 +145,7 @@ export class LunrSearchEngine implements SearchEngine { } async query(query: SearchQuery): Promise { - const { lunrQueryBuilder, documentTypes, offset, limit } = this.translator( + const { lunrQueryBuilder, documentTypes, pageSize } = this.translator( query, ) as ConcreteLunrQuery; @@ -183,14 +181,41 @@ export class LunrSearchEngine implements SearchEngine { return doc2.result.score - doc1.result.score; }); + // Perform paging + const { page } = decodePageCursor(query.pageCursor); + const offset = page * pageSize; + const hasPreviousPage = page > 0; + const hasNextPage = results.length > offset + pageSize; + const nextPageCursor = hasNextPage + ? encodePageCursor({ page: page + 1 }) + : undefined; + const previousPageCursor = hasPreviousPage + ? encodePageCursor({ page: page - 1 }) + : undefined; + // Translate results into SearchResultSet const realResultSet: SearchResultSet = { - results: results.slice(offset, offset + limit).map(d => { + results: results.slice(offset, offset + pageSize).map(d => { return { type: d.type, document: this.docStore[d.result.ref] }; }), - totalCount: results.length, + nextPageCursor, + previousPageCursor, }; return realResultSet; } } + +export function decodePageCursor(pageCursor?: string): { page: number } { + if (!pageCursor) { + return { page: 0 }; + } + + return { + page: Number(Buffer.from(pageCursor, 'base64').toString('utf-8')), + }; +} + +export function encodePageCursor({ page }: { page: number }): string { + return Buffer.from(`${page}`, 'utf-8').toString('base64'); +} diff --git a/plugins/search-backend/src/service/router.ts b/plugins/search-backend/src/service/router.ts index 5a87fb1734..6df39ca1eb 100644 --- a/plugins/search-backend/src/service/router.ts +++ b/plugins/search-backend/src/service/router.ts @@ -36,21 +36,17 @@ export async function createRouter({ req: express.Request, res: express.Response, ) => { - const { term, filters = {}, types, offset, limit } = req.query; + const { term, filters = {}, types, pageCursor } = req.query; logger.info( `Search request received: term="${term}", filters=${JSON.stringify( filters, - )}, offset=${offset ?? ''}, limit=${limit ?? ''}`, + )}, types=${types ? types.join(',') : ''}, pageCursor=${ + pageCursor ?? '' + }`, ); try { - const results = await engine?.query({ - term, - types, - filters, - offset: offset ? Number(offset) : undefined, - limit: limit ? Number(limit) : undefined, - }); + const results = await engine?.query(req.query); res.send(results); } catch (err) { throw new Error( diff --git a/plugins/search/api-report.md b/plugins/search/api-report.md index 616c6f2cae..2cdac79cad 100644 --- a/plugins/search/api-report.md +++ b/plugins/search/api-report.md @@ -139,10 +139,8 @@ export { searchPlugin }; // @public (undocumented) export const SearchResult: ({ children, - initialPageSize, }: { children: (results: { results: SearchResult_2[] }) => JSX.Element; - initialPageSize?: number | undefined; }) => JSX.Element; // Warning: (ae-forgotten-export) The symbol "SearchTypeProps" needs to be exported by the entry point index.d.ts @@ -169,7 +167,7 @@ export const useSearch: () => SearchContextValue; // Warnings were encountered during analysis: // -// src/components/SearchContext/SearchContext.d.ts:23:5 - (ae-forgotten-export) The symbol "SettableSearchContext" needs to be exported by the entry point index.d.ts +// src/components/SearchContext/SearchContext.d.ts:21:5 - (ae-forgotten-export) The symbol "SettableSearchContext" needs to be exported by the entry point index.d.ts // src/components/SearchFilter/SearchFilter.d.ts:13:5 - (ae-forgotten-export) The symbol "Props" needs to be exported by the entry point index.d.ts // src/components/SearchFilter/SearchFilter.d.ts:14:5 - (ae-forgotten-export) The symbol "Component" needs to be exported by the entry point index.d.ts diff --git a/plugins/search/src/apis.test.ts b/plugins/search/src/apis.test.ts index 38f34bb91b..91395d1f0b 100644 --- a/plugins/search/src/apis.test.ts +++ b/plugins/search/src/apis.test.ts @@ -21,7 +21,6 @@ describe('apis', () => { term: '', filters: {}, types: [], - page: {}, }; const baseUrl = 'https://base-url.com/'; diff --git a/plugins/search/src/components/SearchBar/SearchBar.test.tsx b/plugins/search/src/components/SearchBar/SearchBar.test.tsx index acfeb2cb8b..188ffb279b 100644 --- a/plugins/search/src/components/SearchBar/SearchBar.test.tsx +++ b/plugins/search/src/components/SearchBar/SearchBar.test.tsx @@ -30,7 +30,6 @@ jest.mock('@backstage/core-plugin-api', () => ({ describe('SearchBar', () => { const initialState = { term: '', - page: {}, filters: {}, types: ['*'], }; diff --git a/plugins/search/src/components/SearchContext/SearchContext.test.tsx b/plugins/search/src/components/SearchContext/SearchContext.test.tsx index c1618be5ec..d4153e8e4b 100644 --- a/plugins/search/src/components/SearchContext/SearchContext.test.tsx +++ b/plugins/search/src/components/SearchContext/SearchContext.test.tsx @@ -14,13 +14,11 @@ * limitations under the License. */ -import React from 'react'; -import { render, screen, waitFor } from '@testing-library/react'; -import { renderHook, act } from '@testing-library/react-hooks'; - -import { useSearch, SearchContextProvider } from './SearchContext'; - import { useApi } from '@backstage/core-plugin-api'; +import { render, screen, waitFor } from '@testing-library/react'; +import { act, renderHook } from '@testing-library/react-hooks'; +import React from 'react'; +import { SearchContextProvider, useSearch } from './SearchContext'; jest.mock('@backstage/core-plugin-api', () => ({ ...jest.requireActual('@backstage/core-plugin-api'), @@ -38,7 +36,6 @@ describe('SearchContext', () => { const initialState = { term: '', - page: {}, filters: {}, types: ['*'], }; @@ -46,6 +43,7 @@ describe('SearchContext', () => { beforeEach(() => { query.mockResolvedValue({}); (useApi as jest.Mock).mockReturnValue({ query: query }); + window.scrollTo = jest.fn(); }); afterAll(() => { @@ -93,26 +91,26 @@ describe('SearchContext', () => { initialProps: { initialState: { ...initialState, - page: { offset: 0, limit: 25 }, + pageCursor: 'SOMEPAGE', }, }, }); await waitForNextUpdate(); - expect(result.current.page).toEqual({ offset: 0, limit: 25 }); + expect(result.current.pageCursor).toEqual('SOMEPAGE'); act(() => { result.current.setTerm('first term'); }); act(() => { - result.current.setPage({ offset: 75, limit: 25 }); + result.current.setPageCursor('OTHERPAGE'); }); await waitForNextUpdate(); - expect(result.current.page).toEqual({ offset: 75, limit: 25 }); + expect(result.current.pageCursor).toEqual('OTHERPAGE'); act(() => { result.current.setTerm('second term'); @@ -120,7 +118,7 @@ describe('SearchContext', () => { await waitForNextUpdate(); - expect(result.current.page).toEqual({ offset: 0, limit: 25 }); + expect(result.current.pageCursor).toEqual(undefined); }); describe('Performs search (and sets results)', () => { @@ -145,8 +143,6 @@ describe('SearchContext', () => { expect(query).toHaveBeenLastCalledWith({ filters: {}, types: ['*'], - limit: undefined, - offset: undefined, term, }); }); @@ -172,8 +168,6 @@ describe('SearchContext', () => { expect(query).toHaveBeenLastCalledWith({ filters, types: ['*'], - limit: undefined, - offset: undefined, term: '', }); }); @@ -188,10 +182,8 @@ describe('SearchContext', () => { await waitForNextUpdate(); - const page = { offset: 25, limit: 50 }; - act(() => { - result.current.setPage(page); + result.current.setPageCursor('SOMEPAGE'); }); await waitForNextUpdate(); @@ -199,8 +191,7 @@ describe('SearchContext', () => { expect(query).toHaveBeenLastCalledWith({ filters: {}, types: ['*'], - limit: 50, - offset: 25, + pageCursor: 'SOMEPAGE', term: '', }); }); @@ -226,10 +217,72 @@ describe('SearchContext', () => { expect(query).toHaveBeenLastCalledWith({ types, filters: {}, - limit: undefined, - offset: undefined, term: '', }); }); + + it('provides function for fetch the next page', async () => { + query.mockResolvedValue({ + results: [], + nextPageCursor: 'NEXT', + }); + + const { result, waitForNextUpdate } = renderHook(() => useSearch(), { + wrapper, + initialProps: { + initialState, + }, + }); + + await waitForNextUpdate(); + + expect(result.current.fetchNextPage).toBeDefined(); + expect(result.current.fetchPreviousPage).toBeUndefined(); + + act(() => { + result.current.fetchNextPage!(); + }); + + await waitForNextUpdate(); + + expect(query).toHaveBeenLastCalledWith({ + types: ['*'], + filters: {}, + term: '', + pageCursor: 'NEXT', + }); + }); + + it('provides function for fetch the previous page', async () => { + query.mockResolvedValue({ + results: [], + previousPageCursor: 'PREVIOUS', + }); + + const { result, waitForNextUpdate } = renderHook(() => useSearch(), { + wrapper, + initialProps: { + initialState, + }, + }); + + await waitForNextUpdate(); + + expect(result.current.fetchNextPage).toBeUndefined(); + expect(result.current.fetchPreviousPage).toBeDefined(); + + act(() => { + result.current.fetchPreviousPage!(); + }); + + await waitForNextUpdate(); + + expect(query).toHaveBeenLastCalledWith({ + types: ['*'], + filters: {}, + term: '', + pageCursor: 'PREVIOUS', + }); + }); }); }); diff --git a/plugins/search/src/components/SearchContext/SearchContext.tsx b/plugins/search/src/components/SearchContext/SearchContext.tsx index 018496876b..0badc7fd20 100644 --- a/plugins/search/src/components/SearchContext/SearchContext.tsx +++ b/plugins/search/src/components/SearchContext/SearchContext.tsx @@ -20,6 +20,7 @@ import { SearchResultSet } from '@backstage/search-common'; import React, { createContext, PropsWithChildren, + useCallback, useContext, useEffect, useState, @@ -28,8 +29,6 @@ import { useAsync, usePrevious } from 'react-use'; import { AsyncState } from 'react-use/lib/useAsync'; import { searchApiRef } from '../../apis'; -type Page = { limit?: number; offset?: number }; - type SearchContextValue = { result: AsyncState; term: string; @@ -38,13 +37,21 @@ type SearchContextValue = { setTypes: React.Dispatch>; filters: JsonObject; setFilters: React.Dispatch>; - page: Page; - setPage: React.Dispatch>; + pageCursor?: string; + setPageCursor: React.Dispatch>; + fetchNextPage?: React.DispatchWithoutAction; + fetchPreviousPage?: React.DispatchWithoutAction; }; type SettableSearchContext = Omit< SearchContextValue, - 'result' | 'setTerm' | 'setTypes' | 'setFilters' | 'setPage' + | 'result' + | 'setTerm' + | 'setTypes' + | 'setFilters' + | 'setPageCursor' + | 'fetchNextPage' + | 'fetchPreviousPage' >; export const SearchContext = createContext( @@ -54,14 +61,16 @@ export const SearchContext = createContext( export const SearchContextProvider = ({ initialState = { term: '', - page: {}, + pageCursor: undefined, filters: {}, types: [], }, children, }: PropsWithChildren<{ initialState?: SettableSearchContext }>) => { const searchApi = useApi(searchApiRef); - const [page, setPage] = useState(initialState.page); + const [pageCursor, setPageCursor] = useState( + initialState.pageCursor, + ); const [filters, setFilters] = useState(initialState.filters); const [term, setTerm] = useState(initialState.term); const [types, setTypes] = useState(initialState.types); @@ -72,19 +81,31 @@ export const SearchContextProvider = ({ searchApi.query({ term, filters, - offset: page?.offset, - limit: page?.limit, + pageCursor: pageCursor, types, }), - [term, filters, types, page], + [term, filters, types, pageCursor], ); + const hasNextPage = + !result.loading && !result.error && result.value?.nextPageCursor; + const hasPreviousPage = + !result.loading && !result.error && result.value?.previousPageCursor; + const fetchNextPage = useCallback(() => { + setPageCursor(result.value?.nextPageCursor); + resetScrollPosition(); + }, [result.value?.nextPageCursor]); + const fetchPreviousPage = useCallback(() => { + setPageCursor(result.value?.previousPageCursor); + resetScrollPosition(); + }, [result.value?.previousPageCursor]); + useEffect(() => { // Any time a term is reset, we want to start from page 0. if (term && prevTerm && term !== prevTerm) { - setPage(initialState.page); + setPageCursor(undefined); } - }, [term, prevTerm, initialState.page]); + }, [term, prevTerm, initialState.pageCursor]); const value: SearchContextValue = { result, @@ -94,8 +115,10 @@ export const SearchContextProvider = ({ setTerm, types, setTypes, - page, - setPage, + pageCursor, + setPageCursor, + fetchNextPage: hasNextPage ? fetchNextPage : undefined, + fetchPreviousPage: hasPreviousPage ? fetchPreviousPage : undefined, }; return ; @@ -108,3 +131,7 @@ export const useSearch = () => { } return context; }; + +function resetScrollPosition() { + window.scrollTo({ top: 0, left: 0, behavior: 'smooth' }); +} diff --git a/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx b/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx index b5dd56bfb6..8f716caf4b 100644 --- a/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx +++ b/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx @@ -32,7 +32,6 @@ describe('SearchFilter', () => { term: '', filters: {}, types: [], - page: {}, }; const name = 'field'; diff --git a/plugins/search/src/components/SearchPage/SearchPage.test.tsx b/plugins/search/src/components/SearchPage/SearchPage.test.tsx index 91568cddbe..5ff61a3dbe 100644 --- a/plugins/search/src/components/SearchPage/SearchPage.test.tsx +++ b/plugins/search/src/components/SearchPage/SearchPage.test.tsx @@ -14,10 +14,9 @@ * limitations under the License. */ -import React from 'react'; import { renderInTestApp } from '@backstage/test-utils'; +import React from 'react'; import { useLocation, useOutlet } from 'react-router'; - import { useSearch } from '../SearchContext'; import { SearchPage } from './'; @@ -74,25 +73,21 @@ describe('SearchPage', () => { const expectedTerm = 'justin bieber'; const expectedTypes = ['software-catalog']; const expectedFilters = { [expectedFilterField]: expectedFilterValue }; - const expectedOffset = 25; - const expectedLimit = 50; + const expectedPageCursor = 'SOMEPAGE'; - // e.g. ?query=petstore&offset=25&limit=50&filters[lifecycle][]=experimental&filters[kind]=Component + // e.g. ?query=petstore&pageCursor=SOMEPAGE&filters[lifecycle][]=experimental&filters[kind]=Component (useLocation as jest.Mock).mockReturnValueOnce({ - search: `?query=${expectedTerm}&types[]=${expectedTypes[0]}&filters[${expectedFilterField}]=${expectedFilterValue}&offset=${expectedOffset}&limit=${expectedLimit}`, + search: `?query=${expectedTerm}&types[]=${expectedTypes[0]}&filters[${expectedFilterField}]=${expectedFilterValue}&pageCursor=${expectedPageCursor}`, }); // When we render the page... await renderInTestApp(); - // Then search context should be initialized with these values... - const calls = (SearchContextProvider as jest.Mock).mock.calls[0]; - const actualInitialState = calls[0].initialState; - expect(actualInitialState.term).toEqual(expectedTerm); - expect(actualInitialState.types).toEqual(expectedTypes); - expect(actualInitialState.page.limit).toEqual(expectedLimit); - expect(actualInitialState.page.offset).toEqual(expectedOffset); - expect(actualInitialState.filters).toStrictEqual(expectedFilters); + // Then search context should be set with these values... + expect(setTermMock).toHaveBeenCalledWith(expectedTerm); + expect(setTypesMock).toHaveBeenCalledWith(expectedTypes); + expect(setPageCursorMock).toHaveBeenCalledWith(expectedPageCursor); + expect(setFiltersMock).toHaveBeenCalledWith(expectedFilters); }); it('renders provided router element', async () => { @@ -112,7 +107,7 @@ describe('SearchPage', () => { (useSearch as jest.Mock).mockReturnValueOnce({ term: 'bieber', types: ['software-catalog'], - page: { offset: 25, limit: 50 }, + pageCursor: 'SOMEPAGE', filters: { anyKey: 'anyValue' }, setTerm: setTermMock, setTypes: setTypesMock, @@ -120,7 +115,7 @@ describe('SearchPage', () => { setPageCursor: setPageCursorMock, }); const expectedLocation = encodeURI( - '?query=bieber&types[]=software-catalog&offset=25&limit=50&filters[anyKey]=anyValue', + '?query=bieber&types[]=software-catalog&pageCursor=SOMEPAGE&filters[anyKey]=anyValue', ); await renderInTestApp(); diff --git a/plugins/search/src/components/SearchResult/SearchResult.test.tsx b/plugins/search/src/components/SearchResult/SearchResult.test.tsx index d28bcc9136..43a9cdb022 100644 --- a/plugins/search/src/components/SearchResult/SearchResult.test.tsx +++ b/plugins/search/src/components/SearchResult/SearchResult.test.tsx @@ -24,7 +24,6 @@ jest.mock('../SearchContext', () => ({ ...jest.requireActual('../SearchContext'), useSearch: jest.fn().mockReturnValue({ result: {}, - page: {}, }), })); @@ -111,7 +110,6 @@ describe('SearchResult', () => { ], }, }, - page: {}, }); const { getByText } = await renderInTestApp( @@ -124,43 +122,4 @@ describe('SearchResult', () => { expect(getByText('Results 1')).toBeInTheDocument(); }); - - it('Starts on initial page if no offset is set', async () => { - (useSearch as jest.Mock).mockReturnValueOnce({ - page: {}, - result: { - loading: false, - error: '', - value: { results: [{}], totalCount: 100 }, - }, - }); - - const { getByLabelText } = await renderInTestApp( - {({}) => <>}, - ); - - expect(getByLabelText('page 1')).toHaveAttribute('aria-current', 'true'); - expect(getByLabelText('Go to page 2')).toBeInTheDocument(); - expect(getByLabelText('Go to page 3')).toBeInTheDocument(); - expect(getByLabelText('Go to page 4')).toBeInTheDocument(); - }); - - it('Shows the right page', async () => { - (useSearch as jest.Mock).mockReturnValueOnce({ - page: { offset: 25 }, - result: { - loading: false, - error: '', - value: { results: [{}], totalCount: 63 }, - }, - }); - - const { getByLabelText } = await renderInTestApp( - {({}) => <>}, - ); - - expect(getByLabelText('Go to page 1')).toBeInTheDocument(); - expect(getByLabelText('page 2')).toHaveAttribute('aria-current', 'true'); - expect(getByLabelText('Go to page 3')).toBeInTheDocument(); - }); }); diff --git a/plugins/search/src/components/SearchResult/SearchResult.tsx b/plugins/search/src/components/SearchResult/SearchResult.tsx index 5ac2c86a79..ff01e96e74 100644 --- a/plugins/search/src/components/SearchResult/SearchResult.tsx +++ b/plugins/search/src/components/SearchResult/SearchResult.tsx @@ -20,20 +20,17 @@ import { ResponseErrorPanel, } from '@backstage/core-components'; import { SearchResult } from '@backstage/search-common'; -import { Pagination } from '@material-ui/lab'; import React from 'react'; import { useSearch } from '../SearchContext'; +import { SearchResultPager } from '../SearchResultPager'; type Props = { children: (results: { results: SearchResult[] }) => JSX.Element; - initialPageSize?: number; }; -const SearchResultComponent = ({ children, initialPageSize = 25 }: Props) => { +export const SearchResultComponent = ({ children }: Props) => { const { result: { loading, error, value }, - page, - setPage, } = useSearch(); if (loading) { @@ -52,23 +49,10 @@ const SearchResultComponent = ({ children, initialPageSize = 25 }: Props) => { return ; } - const pageSize = page.limit ?? initialPageSize; - const totalPages = Math.ceil(value.totalCount / pageSize); - const currentPage = page.offset ? Math.floor(page.offset / pageSize) + 1 : 1; - - const handlePageChange = (_: React.ChangeEvent, pageNum: number) => { - setPage({ offset: (pageNum - 1) * pageSize, limit: pageSize }); - window.scrollTo({ top: 0, left: 0, behavior: 'smooth' }); - }; - return ( <> {children({ results: value.results })} - + ); }; diff --git a/plugins/search/src/components/SearchResultPager/SearchResultPager.test.tsx b/plugins/search/src/components/SearchResultPager/SearchResultPager.test.tsx new file mode 100644 index 0000000000..ab46d77938 --- /dev/null +++ b/plugins/search/src/components/SearchResultPager/SearchResultPager.test.tsx @@ -0,0 +1,59 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { renderInTestApp } from '@backstage/test-utils'; +import { waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; +import { useSearch } from '../SearchContext'; +import { SearchResultPager } from './SearchResultPager'; + +jest.mock('../SearchContext', () => ({ + ...jest.requireActual('../SearchContext'), + useSearch: jest.fn().mockReturnValue({ + result: {}, + }), +})); + +describe('SearchResultPager', () => { + it('renders pager buttons', async () => { + const fetchNextPage = jest.fn(); + const fetchPreviousPage = jest.fn(); + (useSearch as jest.Mock).mockReturnValueOnce({ + result: { loading: false, value: [] }, + fetchNextPage, + fetchPreviousPage, + }); + + const { getByLabelText } = await renderInTestApp(); + + await waitFor(() => { + expect(getByLabelText('previous page')).toBeInTheDocument(); + + userEvent.click(getByLabelText('previous page')); + }); + + expect(fetchPreviousPage).toBeCalled(); + + await waitFor(() => { + expect(getByLabelText('next page')).toBeInTheDocument(); + + userEvent.click(getByLabelText('next page')); + }); + + expect(fetchNextPage).toBeCalled(); + }); +}); diff --git a/plugins/search/src/components/SearchResultPager/SearchResultPager.tsx b/plugins/search/src/components/SearchResultPager/SearchResultPager.tsx new file mode 100644 index 0000000000..97fa0048e3 --- /dev/null +++ b/plugins/search/src/components/SearchResultPager/SearchResultPager.tsx @@ -0,0 +1,64 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Button, makeStyles } from '@material-ui/core'; +import ArrowBackIosIcon from '@material-ui/icons/ArrowBackIos'; +import ArrowForwardIosIcon from '@material-ui/icons/ArrowForwardIos'; +import React from 'react'; +import { useSearch } from '../SearchContext'; + +const useStyles = makeStyles(theme => ({ + root: { + display: 'flex', + justifyContent: 'center', + gap: theme.spacing(2), + marginTop: theme.spacing(1), + marginBottom: theme.spacing(1), + }, +})); + +export const SearchResultPager = () => { + const { fetchNextPage, fetchPreviousPage } = useSearch(); + const classes = useStyles(); + + if (!fetchNextPage && !fetchPreviousPage) { + return <>; + } + + return ( + + ); +}; diff --git a/plugins/search/src/components/SearchResultPager/index.ts b/plugins/search/src/components/SearchResultPager/index.ts new file mode 100644 index 0000000000..39b72ec71f --- /dev/null +++ b/plugins/search/src/components/SearchResultPager/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { SearchResultPager } from './SearchResultPager'; diff --git a/plugins/search/src/components/SearchType/SearchType.test.tsx b/plugins/search/src/components/SearchType/SearchType.test.tsx index f8171b0779..d3792bb9f4 100644 --- a/plugins/search/src/components/SearchType/SearchType.test.tsx +++ b/plugins/search/src/components/SearchType/SearchType.test.tsx @@ -31,7 +31,6 @@ describe('SearchType', () => { term: '', filters: {}, types: [], - page: {}, }; const name = 'field'; diff --git a/plugins/search/src/components/index.tsx b/plugins/search/src/components/index.tsx index 889eb36334..8b6493ae9b 100644 --- a/plugins/search/src/components/index.tsx +++ b/plugins/search/src/components/index.tsx @@ -14,12 +14,13 @@ * limitations under the License. */ +export * from './DefaultResultListItem'; export * from './Filters'; -export * from './SearchFilter'; -export * from './SearchType'; export * from './SearchBar'; +export * from './SearchContext'; +export * from './SearchFilter'; export * from './SearchPage'; export * from './SearchResult'; -export * from './DefaultResultListItem'; +export * from './SearchResultPager'; +export * from './SearchType'; export * from './SidebarSearch'; -export * from './SearchContext';