diff --git a/.changeset/search-happy-owls-sneeze.md b/.changeset/search-happy-owls-sneeze.md new file mode 100644 index 0000000000..1f3cfd5dd1 --- /dev/null +++ b/.changeset/search-happy-owls-sneeze.md @@ -0,0 +1,13 @@ +--- +'@backstage/search-common': minor +'@backstage/plugin-search': patch +'@backstage/plugin-search-backend': patch +'@backstage/plugin-search-backend-module-elasticsearch': patch +'@backstage/plugin-search-backend-module-pg': minor +'@backstage/plugin-search-backend-node': patch +--- + +Implement optional `pageCursor` based paging in search. + +To use paging in your app, add a `` to your +`SearchPage.tsx`. diff --git a/packages/app/cypress/integration/components/search/SearchPage.js b/packages/app/cypress/integration/components/search/SearchPage.js index 4db7acf4fd..6a18d16198 100644 --- a/packages/app/cypress/integration/components/search/SearchPage.js +++ b/packages/app/cypress/integration/components/search/SearchPage.js @@ -34,7 +34,7 @@ describe('SearchPage', () => { cy.visit('/search-next', { onBeforeLoad(win) { cy.stub(win, 'fetch') - .withArgs(`${API_ENDPOINT}?term=&pageCursor=`) + .withArgs(`${API_ENDPOINT}?term=`) .resolves({ ok: true, json: () => ({ results }), @@ -56,7 +56,7 @@ describe('SearchPage', () => { onBeforeLoad(win) { cy.stub(win, 'fetch') .withArgs( - `${API_ENDPOINT}?term=&filters%5Bkind%5D=Component&filters%5Blifecycle%5D%5B0%5D=experimental&pageCursor=`, + `${API_ENDPOINT}?term=&filters%5Bkind%5D=Component&filters%5Blifecycle%5D%5B0%5D=experimental`, ) .resolves({ ok: true, @@ -102,7 +102,7 @@ describe('SearchPage', () => { cy.visit('/search-next?query=backstage', { onBeforeLoad(win) { cy.stub(win, 'fetch') - .withArgs(`${API_ENDPOINT}?term=backstage&pageCursor=`) + .withArgs(`${API_ENDPOINT}?term=backstage`) .resolves({ ok: true, json: () => ({ results: [] }), diff --git a/packages/app/src/components/search/SearchPage.tsx b/packages/app/src/components/search/SearchPage.tsx index 9b0ae7eecf..54f59d1a6e 100644 --- a/packages/app/src/components/search/SearchPage.tsx +++ b/packages/app/src/components/search/SearchPage.tsx @@ -14,20 +14,19 @@ * limitations under the License. */ -import React, { useState } from 'react'; -import { makeStyles, Theme, Grid, List, Paper } from '@material-ui/core'; -import Pagination from '@material-ui/lab/Pagination'; +import { Content, Header, Lifecycle, Page } from '@backstage/core-components'; import { CatalogResultListItem } from '@backstage/plugin-catalog'; import { + DefaultResultListItem, SearchBar, SearchFilter, SearchResult, + SearchResultPager, SearchType, - DefaultResultListItem, } from '@backstage/plugin-search'; -import { Content, Header, Lifecycle, Page } from '@backstage/core-components'; import { DocsResultListItem } from '@backstage/plugin-techdocs'; -import { SearchResultSet } from '@backstage/search-common'; +import { Grid, List, makeStyles, Paper, Theme } from '@material-ui/core'; +import React from 'react'; const useStyles = makeStyles((theme: Theme) => ({ bar: { @@ -43,59 +42,6 @@ const useStyles = makeStyles((theme: Theme) => ({ }, })); -// TODO: Move this into the search plugin once pagination is natively supported. -// See: https://github.com/backstage/backstage/issues/6062 -const SearchResultList = ({ results }: SearchResultSet) => { - const pageSize = 10; - const [page, setPage] = useState(1); - const changePage = (_: any, pageIndex: number) => { - setPage(pageIndex); - }; - const pageAmount = Math.ceil((results.length || 0) / pageSize); - return ( - <> - - {results - .slice(pageSize * (page - 1), pageSize * page) - .map(({ type, document }) => { - switch (type) { - case 'software-catalog': - return ( - - ); - case 'techdocs': - return ( - - ); - default: - return ( - - ); - } - })} - - {pageAmount > 1 && ( - - )} - - ); -}; - const SearchPage = () => { const classes = useStyles(); return ( @@ -129,8 +75,37 @@ const SearchPage = () => { - {({ results }) => } + {({ results }) => ( + + {results.map(({ type, document }) => { + switch (type) { + case 'software-catalog': + return ( + + ); + case 'techdocs': + return ( + + ); + default: + return ( + + ); + } + })} + + )} + diff --git a/packages/search-common/api-report.md b/packages/search-common/api-report.md index 055f2d152f..f00beb6830 100644 --- a/packages/search-common/api-report.md +++ b/packages/search-common/api-report.md @@ -53,7 +53,7 @@ export interface SearchQuery { // (undocumented) filters?: JsonObject; // (undocumented) - pageCursor: string; + pageCursor?: string; // (undocumented) term: string; // (undocumented) @@ -74,6 +74,10 @@ export interface SearchResult { // // @public (undocumented) export interface SearchResultSet { + // (undocumented) + nextPageCursor?: string; + // (undocumented) + previousPageCursor?: string; // (undocumented) results: SearchResult[]; } diff --git a/packages/search-common/src/types.ts b/packages/search-common/src/types.ts index 2a4447efac..063e0cb96c 100644 --- a/packages/search-common/src/types.ts +++ b/packages/search-common/src/types.ts @@ -19,7 +19,7 @@ export interface SearchQuery { term: string; filters?: JsonObject; types?: string[]; - pageCursor: string; + pageCursor?: string; } export interface SearchResult { @@ -29,6 +29,8 @@ export interface SearchResult { export interface SearchResultSet { results: SearchResult[]; + 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 eac76a30b9..1644ce65ea 100644 --- a/plugins/search-backend-module-elasticsearch/api-report.md +++ b/plugins/search-backend-module-elasticsearch/api-report.md @@ -45,6 +45,7 @@ export class ElasticSearchSearchEngine implements SearchEngine { term, filters, types, + 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 b4bba9de9c..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() { @@ -87,13 +89,11 @@ describe('ElasticSearchSearchEngine', () => { await testSearchEngine.query({ term: 'testTerm', filters: {}, - pageCursor: '', }); expect(translatorSpy).toHaveBeenCalledWith({ term: 'testTerm', filters: {}, - pageCursor: '', }); }); @@ -104,7 +104,6 @@ describe('ElasticSearchSearchEngine', () => { types: ['indexName'], term: 'testTerm', filters: { kind: 'testKind' }, - pageCursor: '', }) as ConcreteElasticSearchQuery; expect(actualTranslatedQuery).toMatchObject({ @@ -132,7 +131,43 @@ describe('ElasticSearchSearchEngine', () => { }, }, }, - size: 100, + from: 0, + size: 25, + }); + }); + + it('should pass page cursor', async () => { + const translatorUnderTest = inspectableSearchEngine.getTranslator(); + + const actualTranslatedQuery = translatorUnderTest({ + types: ['indexName'], + term: 'testTerm', + pageCursor: 'MQ==', + }) 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: 25, }); }); @@ -143,7 +178,6 @@ describe('ElasticSearchSearchEngine', () => { types: ['indexName'], term: 'testTerm', filters: { kind: 'testKind', namespace: 'testNameSpace' }, - pageCursor: '', }) as ConcreteElasticSearchQuery; expect(actualTranslatedQuery).toMatchObject({ @@ -178,7 +212,8 @@ describe('ElasticSearchSearchEngine', () => { ], }, }, - size: 100, + from: 0, + size: 25, }); }); @@ -189,7 +224,6 @@ describe('ElasticSearchSearchEngine', () => { types: ['indexName'], term: 'testTerm', filters: { kind: ['testKind', 'kastTeint'] }, - pageCursor: '', }) as ConcreteElasticSearchQuery; expect(actualTranslatedQuery).toMatchObject({ @@ -228,7 +262,8 @@ describe('ElasticSearchSearchEngine', () => { }, }, }, - size: 100, + from: 0, + size: 25, }); }); @@ -239,7 +274,6 @@ describe('ElasticSearchSearchEngine', () => { types: ['indexName'], term: 'testTerm', filters: { kind: { a: 'b' } }, - pageCursor: '', }) as ConcreteElasticSearchQuery; expect(actualTranslatedQuery).toThrow(); }); @@ -315,11 +349,106 @@ describe('ElasticSearchSearchEngine', () => { const mockedSearchResult = await testSearchEngine.query({ term: 'testTerm', filters: {}, - pageCursor: '', }); // Should return 0 results as nothing is indexed here - expect(mockedSearchResult).toMatchObject({ results: [] }); + 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 () => { @@ -327,7 +456,6 @@ describe('ElasticSearchSearchEngine', () => { await testSearchEngine.query({ term: 'testTerm', filters: {}, - pageCursor: '', }); expect(elasticSearchQuerySpy).toHaveBeenCalled(); @@ -346,7 +474,8 @@ describe('ElasticSearchSearchEngine', () => { filter: [], }, }, - size: 100, + from: 0, + size: 25, }, index: '*__search', }); @@ -359,7 +488,6 @@ describe('ElasticSearchSearchEngine', () => { await testSearchEngine.query({ term: '', filters: {}, - pageCursor: '', }); expect(elasticSearchQuerySpy).toHaveBeenCalled(); @@ -373,7 +501,8 @@ describe('ElasticSearchSearchEngine', () => { filter: [], }, }, - size: 100, + from: 0, + size: 25, }, index: '*__search', }); @@ -386,7 +515,6 @@ describe('ElasticSearchSearchEngine', () => { await testSearchEngine.query({ term: '', filters: {}, - pageCursor: '', types: ['test-type'], }); @@ -401,7 +529,8 @@ describe('ElasticSearchSearchEngine', () => { filter: [], }, }, - size: 100, + from: 0, + size: 25, }, index: ['test-type__search'], }); @@ -430,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 d1e831b2ca..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,6 +141,7 @@ export class ElasticSearchSearchEngine implements SearchEngine { term, filters = {}, types, + pageCursor, }: SearchQuery): ConcreteElasticSearchQuery { const filter = Object.entries(filters) .filter(([_, value]) => Boolean(value)) @@ -167,16 +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])) - // TODO: Replace size limit with page cursor after pagination approach decided - // See: https://github.com/backstage/backstage/issues/6062 - .size(100) + .from(page * pageSize) + .size(pageSize) .toJSON(), documentTypes: types, + pageSize, }; } @@ -248,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('*'); @@ -257,11 +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, })), + nextPageCursor, + previousPageCursor, }; } catch (e) { this.logger.error( @@ -289,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 23a1f3ecbb..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) @@ -32,7 +40,7 @@ export class DatabaseDocumentStore implements DatabaseStore { // (undocumented) query( tx: Knex.Transaction, - { types, pgTerm, fields }: PgSearchQuery, + { types, pgTerm, fields, offset, limit }: PgSearchQuery, ): Promise; // (undocumented) static supported(knex: Knex): Promise; @@ -79,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) @@ -93,6 +103,10 @@ export interface PgSearchQuery { // (undocumented) fields?: Record; // (undocumented) + limit: number; + // (undocumented) + offset: number; + // (undocumented) pgTerm?: string; // (undocumented) types?: string[]; 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..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; @@ -44,24 +49,42 @@ describe('PgSearchEngine', () => { await searchEngine.query({ term: 'testTerm', filters: {}, - pageCursor: '', }); expect(translatorSpy).toHaveBeenCalledWith({ term: 'testTerm', filters: {}, - pageCursor: '', + }); + }); + + it('should pass page cursor', async () => { + const actualTranslatedQuery = searchEngine.translator({ + term: 'Hello', + pageCursor: 'MQ==', + }); + + expect(actualTranslatedQuery).toMatchObject({ + pgQuery: { + pgTerm: '("Hello" | "Hello":*)', + offset: 25, + limit: 26, + }, + pageSize: 25, }); }); 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":*)', + pgQuery: { + pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', + offset: 0, + limit: 26, + }, + pageSize: 25, }); }); @@ -69,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, }); }); @@ -81,13 +107,17 @@ 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'], + pgQuery: { + pgTerm: '("testTerm" | "testTerm":*)', + fields: { kind: 'testKind' }, + types: ['my-filter'], + offset: 0, + limit: 26, + }, + pageSize: 25, }); }); }); @@ -151,7 +181,6 @@ describe('PgSearchEngine', () => { const results = await searchEngine.query({ term: 'Hello World', - pageCursor: '', }); expect(results).toEqual({ @@ -165,11 +194,113 @@ describe('PgSearchEngine', () => { type: 'my-type', }, ], + nextPageCursor: undefined, }); expect(database.transaction).toHaveBeenCalledTimes(1); expect(database.query).toHaveBeenCalledWith(tx, { pgTerm: '("Hello" | "Hello":*)&("World" | "World":*)', + offset: 0, + limit: 26, + }); + }); + + 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: 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 74a1e09010..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,20 +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, + 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; } @@ -77,16 +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 = await this.databaseStore.transaction(async tx => this.databaseStore.query(tx, pgQuery), ); - const results = rows.map(({ type, document }) => ({ + + // 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 }; + 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 b860faca6a..71c012ec00 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,52 @@ 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())( 'query by term, %p', async databaseId => { @@ -215,7 +261,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 +317,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 +373,8 @@ describe('DatabaseDocumentStore', () => { store.query(tx, { pgTerm: 'Hello & World', fields: { myField: 'this' }, + offset: 0, + limit: 25, }), ); @@ -374,6 +427,8 @@ describe('DatabaseDocumentStore', () => { store.query(tx, { pgTerm: 'Hello & World', fields: { myField: ['this', 'that'] }, + offset: 0, + limit: 25, }), ); @@ -433,6 +488,8 @@ describe('DatabaseDocumentStore', () => { store.query(tx, { pgTerm: 'Hello & World', fields: { myField: 'this', otherField: 'another' }, + offset: 0, + limit: 25, }), ); @@ -480,6 +537,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..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, - { types, pgTerm, fields }: PgSearchQuery, + { types, pgTerm, fields, offset, limit }: PgSearchQuery, ): Promise { // Builds a query like: // SELECT ts_rank_cd(body, query) AS rank, type, document @@ -174,6 +174,6 @@ export class DatabaseDocumentStore implements DatabaseStore { query.select(tx.raw('1 as rank')); } - return await query.limit(100); + 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 8c98628caf..0c0596160e 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 { diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts b/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts index 3233c3adae..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,14 +53,14 @@ describe('LunrSearchEngine', () => { await testLunrSearchEngine.query({ term: 'testTerm', filters: {}, - pageCursor: '', + pageCursor: 'MQ==', }); // Then: the translator is invoked with expected args. expect(translatorSpy).toHaveBeenCalledWith({ term: 'testTerm', filters: {}, - pageCursor: '', + pageCursor: 'MQ==', }); }); @@ -68,12 +73,53 @@ describe('LunrSearchEngine', () => { const actualTranslatedQuery = translatorUnderTest({ term: 'testTerm', filters: {}, - pageCursor: '', }) as ConcreteLunrQuery; expect(actualTranslatedQuery).toMatchObject({ documentTypes: undefined, lunrQueryBuilder: expect.any(Function), + pageSize: 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 default offset and limit', async () => { + const inspectableSearchEngine = new LunrSearchEngineForTranslatorTests({ + logger: getVoidLogger(), + }); + const translatorUnderTest = inspectableSearchEngine.getTranslator(); + + const actualTranslatedQuery = translatorUnderTest({ + term: 'testTerm', + }) as ConcreteLunrQuery; + + expect(actualTranslatedQuery).toMatchObject({ + documentTypes: undefined, + lunrQueryBuilder: expect.any(Function), + pageSize: 25, }); const query: jest.Mocked = { @@ -110,7 +156,6 @@ describe('LunrSearchEngine', () => { const actualTranslatedQuery = translatorUnderTest({ term: 'testTerm', filters: { kind: 'testKind' }, - pageCursor: '', }) as ConcreteLunrQuery; expect(actualTranslatedQuery).toMatchObject({ @@ -156,7 +201,6 @@ describe('LunrSearchEngine', () => { const actualTranslatedQuery = translatorUnderTest({ term: 'testTerm', filters: { kind: 'testKind', namespace: 'testNameSpace' }, - pageCursor: '', }) as ConcreteLunrQuery; expect(actualTranslatedQuery).toMatchObject({ @@ -206,7 +250,6 @@ describe('LunrSearchEngine', () => { const actualTranslatedQuery = translatorUnderTest({ term: 'testTerm', filters: { kind: 'testKind' }, - pageCursor: '', }) as ConcreteLunrQuery; expect(actualTranslatedQuery).toMatchObject({ @@ -235,18 +278,19 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'testTerm', filters: {}, - pageCursor: '', }); expect(querySpy).toHaveBeenCalled(); expect(querySpy).toHaveBeenCalledWith({ term: 'testTerm', filters: {}, - pageCursor: '', }); // Should return 0 results as nothing is indexed here - expect(mockedSearchResult).toMatchObject({ results: [] }); + expect(mockedSearchResult).toMatchObject({ + results: [], + nextPageCursor: undefined, + }); }); it('should perform search query and return 0 results on no match', async () => { @@ -265,11 +309,13 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'unknown', filters: {}, - pageCursor: '', }); // Should return 0 results as we are mocking the indexing of 1 document but with no match on the fields - expect(mockedSearchResult).toMatchObject({ results: [] }); + expect(mockedSearchResult).toMatchObject({ + results: [], + nextPageCursor: undefined, + }); }); it('should perform search query and return all results on empty term', async () => { @@ -288,7 +334,6 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: '', filters: {}, - pageCursor: '', }); expect(mockedSearchResult).toMatchObject({ @@ -302,6 +347,7 @@ describe('LunrSearchEngine', () => { type: 'test-index', }, ], + nextPageCursor: undefined, }); }); @@ -321,7 +367,6 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'testTitle', filters: {}, - pageCursor: '', }); expect(mockedSearchResult).toMatchObject({ @@ -334,6 +379,7 @@ describe('LunrSearchEngine', () => { }, }, ], + nextPageCursor: undefined, }); }); @@ -353,7 +399,6 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'testTitle', filters: {}, - pageCursor: '', }); expect(mockedSearchResult).toMatchObject({ @@ -366,6 +411,7 @@ describe('LunrSearchEngine', () => { }, }, ], + nextPageCursor: undefined, }); }); @@ -385,7 +431,6 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'testTitel', // Intentional typo filters: {}, - pageCursor: '', }); // Should return 1 result as we are mocking the indexing of 1 document with match on the title field @@ -399,6 +444,7 @@ describe('LunrSearchEngine', () => { }, }, ], + nextPageCursor: undefined, }); }); @@ -418,7 +464,6 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'World', filters: {}, - pageCursor: '', }); // Should return 1 result as we are mocking the indexing of 1 document with match on the title field @@ -432,6 +477,7 @@ describe('LunrSearchEngine', () => { }, }, ], + nextPageCursor: undefined, }); }); @@ -451,7 +497,6 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'Search', filters: {}, - pageCursor: '', }); // Should return 1 result as we are mocking the indexing of 1 document with match on the title field @@ -465,6 +510,7 @@ describe('LunrSearchEngine', () => { }, }, ], + nextPageCursor: undefined, }); }); @@ -489,7 +535,6 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'testTitle', filters: { location: 'test/location2' }, - pageCursor: '', }); // Should return 1 of 2 results as we are @@ -505,6 +550,7 @@ describe('LunrSearchEngine', () => { }, }, ], + nextPageCursor: undefined, }); }); @@ -532,7 +578,6 @@ describe('LunrSearchEngine', () => { // Perform search query scoped to "test-index-2" with a filter on the field "extraField" const mockedSearchResult = await testLunrSearchEngine.query({ term: 'testTitle', - pageCursor: '', filters: { extraField: 'testExtraField' }, }); @@ -547,6 +592,7 @@ describe('LunrSearchEngine', () => { }, }, ], + nextPageCursor: undefined, }); }); @@ -571,7 +617,6 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'testTitle', filters: { location: 'test:location2' }, - pageCursor: '', }); // Should return 1 of 2 results as we are @@ -587,6 +632,7 @@ describe('LunrSearchEngine', () => { }, }, ], + nextPageCursor: undefined, }); }); @@ -625,7 +671,6 @@ describe('LunrSearchEngine', () => { const mockedSearchResult = await testLunrSearchEngine.query({ term: 'testTitle', types: ['test-index-2'], - pageCursor: '', }); expect(mockedSearchResult).toMatchObject({ @@ -645,8 +690,75 @@ describe('LunrSearchEngine', () => { }, }, ], + 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', () => { @@ -669,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 0cb4fd8336..f22762cb9f 100644 --- a/plugins/search-backend-node/src/engines/LunrSearchEngine.ts +++ b/plugins/search-backend-node/src/engines/LunrSearchEngine.ts @@ -27,6 +27,7 @@ import { Logger } from 'winston'; export type ConcreteLunrQuery = { lunrQueryBuilder: lunr.Index.QueryBuilder; documentTypes?: string[]; + pageSize: number; }; type LunrResultEnvelope = { @@ -51,6 +52,8 @@ export class LunrSearchEngine implements SearchEngine { filters, types, }: SearchQuery): ConcreteLunrQuery => { + const pageSize = 25; + return { lunrQueryBuilder: q => { const termToken = lunr.tokenizer(term); @@ -107,6 +110,7 @@ export class LunrSearchEngine implements SearchEngine { } }, documentTypes: types, + pageSize, }; }; @@ -141,7 +145,7 @@ export class LunrSearchEngine implements SearchEngine { } async query(query: SearchQuery): Promise { - const { lunrQueryBuilder, documentTypes } = this.translator( + const { lunrQueryBuilder, documentTypes, pageSize } = this.translator( query, ) as ConcreteLunrQuery; @@ -177,13 +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.map(d => { + results: results.slice(offset, offset + pageSize).map(d => { return { type: d.type, document: this.docStore[d.result.ref] }; }), + 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 3a0a43522e..6df39ca1eb 100644 --- a/plugins/search-backend/src/service/router.ts +++ b/plugins/search-backend/src/service/router.ts @@ -36,11 +36,13 @@ export async function createRouter({ req: express.Request, res: express.Response, ) => { - const { term, filters = {}, pageCursor = '' } = req.query; + const { term, filters = {}, types, pageCursor } = req.query; logger.info( `Search request received: term="${term}", filters=${JSON.stringify( filters, - )}, ${pageCursor}`, + )}, types=${types ? types.join(',') : ''}, pageCursor=${ + pageCursor ?? '' + }`, ); try { diff --git a/plugins/search/api-report.md b/plugins/search/api-report.md index e3075a448d..03c24609e2 100644 --- a/plugins/search/api-report.md +++ b/plugins/search/api-report.md @@ -143,6 +143,11 @@ export const SearchResult: ({ children: (results: { results: SearchResult_2[] }) => JSX.Element; }) => JSX.Element; +// Warning: (ae-missing-release-tag) "SearchResultPager" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export const SearchResultPager: () => JSX.Element; + // Warning: (ae-forgotten-export) The symbol "SearchTypeProps" needs to be exported by the entry point index.d.ts // Warning: (ae-missing-release-tag) "SearchType" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // @@ -167,7 +172,7 @@ export const useSearch: () => SearchContextValue; // Warnings were encountered during analysis: // -// src/components/SearchContext/SearchContext.d.ts:19: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 cc59ca1a77..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: [], - pageCursor: '', }; const baseUrl = 'https://base-url.com/'; @@ -53,7 +52,7 @@ describe('apis', () => { it('Fetch is called with expected URL (including stringified Q params)', async () => { await client.query(query); expect(getBaseUrl).toHaveBeenLastCalledWith('search/query'); - expect(fetch).toHaveBeenLastCalledWith(`${baseUrl}?term=&pageCursor=`, { + expect(fetch).toHaveBeenLastCalledWith(`${baseUrl}?term=`, { headers: {}, }); }); @@ -65,7 +64,7 @@ describe('apis', () => { }); await authedClient.query(query); expect(getBaseUrl).toHaveBeenLastCalledWith('search/query'); - expect(fetch).toHaveBeenLastCalledWith(`${baseUrl}?term=&pageCursor=`, { + expect(fetch).toHaveBeenLastCalledWith(`${baseUrl}?term=`, { headers: { Authorization: `Bearer ${token}` }, }); }); diff --git a/plugins/search/src/components/SearchBar/SearchBar.test.tsx b/plugins/search/src/components/SearchBar/SearchBar.test.tsx index b99b375252..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: '', - pageCursor: '', filters: {}, types: ['*'], }; diff --git a/plugins/search/src/components/SearchContext/SearchContext.test.tsx b/plugins/search/src/components/SearchContext/SearchContext.test.tsx index 4de2a8f0a6..6513dc21bb 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: '', - pageCursor: '', filters: {}, types: ['*'], }; @@ -93,22 +90,26 @@ describe('SearchContext', () => { initialProps: { initialState: { ...initialState, - pageCursor: '1', + pageCursor: 'SOMEPAGE', }, }, }); await waitForNextUpdate(); - expect(result.current.pageCursor).toBe('1'); + expect(result.current.pageCursor).toEqual('SOMEPAGE'); act(() => { result.current.setTerm('first term'); }); + act(() => { + result.current.setPageCursor('OTHERPAGE'); + }); + await waitForNextUpdate(); - expect(result.current.pageCursor).toBe('1'); + expect(result.current.pageCursor).toEqual('OTHERPAGE'); act(() => { result.current.setTerm('second term'); @@ -116,7 +117,7 @@ describe('SearchContext', () => { await waitForNextUpdate(); - expect(result.current.pageCursor).toBe(''); + expect(result.current.pageCursor).toEqual(undefined); }); describe('Performs search (and sets results)', () => { @@ -139,7 +140,8 @@ describe('SearchContext', () => { await waitForNextUpdate(); expect(query).toHaveBeenLastCalledWith({ - ...initialState, + filters: {}, + types: ['*'], term, }); }); @@ -163,12 +165,13 @@ describe('SearchContext', () => { await waitForNextUpdate(); expect(query).toHaveBeenLastCalledWith({ - ...initialState, filters, + types: ['*'], + term: '', }); }); - it('When pageCursor is set', async () => { + it('When page is set', async () => { const { result, waitForNextUpdate } = renderHook(() => useSearch(), { wrapper, initialProps: { @@ -178,17 +181,17 @@ describe('SearchContext', () => { await waitForNextUpdate(); - const pageCursor = 'pageCursor'; - act(() => { - result.current.setPageCursor(pageCursor); + result.current.setPageCursor('SOMEPAGE'); }); await waitForNextUpdate(); expect(query).toHaveBeenLastCalledWith({ - ...initialState, - pageCursor, + filters: {}, + types: ['*'], + pageCursor: 'SOMEPAGE', + term: '', }); }); @@ -211,8 +214,73 @@ describe('SearchContext', () => { await waitForNextUpdate(); expect(query).toHaveBeenLastCalledWith({ - ...initialState, types, + filters: {}, + 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 45a74f5355..16d1371964 100644 --- a/plugins/search/src/components/SearchContext/SearchContext.tsx +++ b/plugins/search/src/components/SearchContext/SearchContext.tsx @@ -14,19 +14,20 @@ * limitations under the License. */ -import React, { - PropsWithChildren, - createContext, - useContext, - useState, - useEffect, -} from 'react'; -import { useAsync, usePrevious } from 'react-use'; -import { SearchResultSet } from '@backstage/search-common'; -import { searchApiRef } from '../../apis'; -import { AsyncState } from 'react-use/lib/useAsync'; import { JsonObject } from '@backstage/config'; import { useApi } from '@backstage/core-plugin-api'; +import { SearchResultSet } from '@backstage/search-common'; +import React, { + createContext, + PropsWithChildren, + useCallback, + useContext, + useEffect, + useState, +} from 'react'; +import { useAsync, usePrevious } from 'react-use'; +import { AsyncState } from 'react-use/lib/useAsync'; +import { searchApiRef } from '../../apis'; type SearchContextValue = { result: AsyncState; @@ -36,13 +37,21 @@ type SearchContextValue = { setTypes: React.Dispatch>; filters: JsonObject; setFilters: React.Dispatch>; - pageCursor: string; - setPageCursor: React.Dispatch>; + pageCursor?: string; + setPageCursor: React.Dispatch>; + fetchNextPage?: React.DispatchWithoutAction; + fetchPreviousPage?: React.DispatchWithoutAction; }; type SettableSearchContext = Omit< SearchContextValue, - 'result' | 'setTerm' | 'setTypes' | 'setFilters' | 'setPageCursor' + | 'result' + | 'setTerm' + | 'setTypes' + | 'setFilters' + | 'setPageCursor' + | 'fetchNextPage' + | 'fetchPreviousPage' >; export const SearchContext = createContext( @@ -52,14 +61,16 @@ export const SearchContext = createContext( export const SearchContextProvider = ({ initialState = { term: '', - pageCursor: '', + pageCursor: undefined, filters: {}, types: [], }, children, }: PropsWithChildren<{ initialState?: SettableSearchContext }>) => { const searchApi = useApi(searchApiRef); - const [pageCursor, setPageCursor] = useState(initialState.pageCursor); + const [pageCursor, setPageCursor] = useState( + initialState.pageCursor, + ); const [filters, setFilters] = useState(initialState.filters); const [term, setTerm] = useState(initialState.term); const [types, setTypes] = useState(initialState.types); @@ -70,18 +81,29 @@ export const SearchContextProvider = ({ searchApi.query({ term, filters, - pageCursor, + pageCursor: pageCursor, types, }), [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); + }, [result.value?.nextPageCursor]); + const fetchPreviousPage = useCallback(() => { + setPageCursor(result.value?.previousPageCursor); + }, [result.value?.previousPageCursor]); + useEffect(() => { // Any time a term is reset, we want to start from page 0. if (term && prevTerm && term !== prevTerm) { - setPageCursor(''); + setPageCursor(undefined); } - }, [term, prevTerm]); + }, [term, prevTerm, initialState.pageCursor]); const value: SearchContextValue = { result, @@ -93,6 +115,8 @@ export const SearchContextProvider = ({ setTypes, pageCursor, setPageCursor, + fetchNextPage: hasNextPage ? fetchNextPage : undefined, + fetchPreviousPage: hasPreviousPage ? fetchPreviousPage : undefined, }; return ; diff --git a/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx b/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx index e8c2f048b9..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: [], - pageCursor: '', }; const name = 'field'; diff --git a/plugins/search/src/components/SearchPage/SearchPage.test.tsx b/plugins/search/src/components/SearchPage/SearchPage.test.tsx index c387d57b60..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,9 +73,9 @@ describe('SearchPage', () => { const expectedTerm = 'justin bieber'; const expectedTypes = ['software-catalog']; const expectedFilters = { [expectedFilterField]: expectedFilterValue }; - const expectedPageCursor = 'page2-or-something'; + const expectedPageCursor = 'SOMEPAGE'; - // e.g. ?query=petstore&pageCursor=1&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}&pageCursor=${expectedPageCursor}`, }); @@ -108,7 +107,7 @@ describe('SearchPage', () => { (useSearch as jest.Mock).mockReturnValueOnce({ term: 'bieber', types: ['software-catalog'], - pageCursor: 'page2-or-something', + pageCursor: 'SOMEPAGE', filters: { anyKey: 'anyValue' }, setTerm: setTermMock, setTypes: setTypesMock, @@ -116,7 +115,7 @@ describe('SearchPage', () => { setPageCursor: setPageCursorMock, }); const expectedLocation = encodeURI( - '?query=bieber&types[]=software-catalog&pageCursor=page2-or-something&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 bb8f393d67..43a9cdb022 100644 --- a/plugins/search/src/components/SearchResult/SearchResult.test.tsx +++ b/plugins/search/src/components/SearchResult/SearchResult.test.tsx @@ -93,16 +93,33 @@ describe('SearchResult', () => { it('Calls children with results set to result.value', async () => { (useSearch as jest.Mock).mockReturnValueOnce({ - result: { loading: false, error: '', value: { results: [] } }, + result: { + loading: false, + error: '', + value: { + totalCount: 1, + results: [ + { + type: 'some-type', + document: { + title: 'some-title', + text: 'some-text', + location: 'some-location', + }, + }, + ], + }, + }, }); - await renderInTestApp( + const { getByText } = await renderInTestApp( {({ results }) => { - expect(results).toEqual([]); - return <>; + return <>Results {results.length}; }} , ); + + expect(getByText('Results 1')).toBeInTheDocument(); }); }); diff --git a/plugins/search/src/components/SearchResult/SearchResult.tsx b/plugins/search/src/components/SearchResult/SearchResult.tsx index 26a2484a86..34b6840f21 100644 --- a/plugins/search/src/components/SearchResult/SearchResult.tsx +++ b/plugins/search/src/components/SearchResult/SearchResult.tsx @@ -27,7 +27,7 @@ type Props = { children: (results: { results: SearchResult[] }) => JSX.Element; }; -const SearchResultComponent = ({ children }: Props) => { +export const SearchResultComponent = ({ children }: Props) => { const { result: { loading, error, value }, } = useSearch(); @@ -48,7 +48,7 @@ const SearchResultComponent = ({ children }: Props) => { return ; } - return children({ results: value.results }); + return <>{children({ results: value.results })}; }; export { SearchResultComponent as SearchResult }; 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..f1850772bb --- /dev/null +++ b/plugins/search/src/components/SearchResultPager/SearchResultPager.tsx @@ -0,0 +1,63 @@ +/* + * 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: 'space-between', + gap: theme.spacing(2), + margin: theme.spacing(4), + }, +})); + +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 b8bf59d13e..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: [], - pageCursor: '', }; 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'; diff --git a/plugins/search/src/index.ts b/plugins/search/src/index.ts index f09ba739bd..d178329ab4 100644 --- a/plugins/search/src/index.ts +++ b/plugins/search/src/index.ts @@ -15,25 +15,26 @@ */ export { searchApiRef } from './apis'; -export { - searchPlugin, - searchPlugin as plugin, - SearchPage, - SearchPageNext, - SearchBarNext, - SearchResult, - DefaultResultListItem, -} from './plugin'; export { Filters, FiltersButton, SearchBar, SearchContextProvider, - useSearch, - SearchPage as Router, SearchFilter, - SearchType, SearchFilterNext, + SearchPage as Router, + SearchResultPager, + SearchType, SidebarSearch, + useSearch, } from './components'; export type { FiltersState } from './components'; +export { + DefaultResultListItem, + SearchBarNext, + SearchPage, + SearchPageNext, + searchPlugin as plugin, + searchPlugin, + SearchResult, +} from './plugin';