diff --git a/.changeset/empty-pens-invent.md b/.changeset/empty-pens-invent.md new file mode 100644 index 0000000000..d6937afc84 --- /dev/null +++ b/.changeset/empty-pens-invent.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-search-backend': minor +--- + +**BREAKING**: The `authorization` property is no longer returned on search results when queried. Note: this will only result in a breaking change if you have custom code in your frontend that relies on the `authorization.resourceRef` property on documents. diff --git a/.changeset/happy-mugs-camp.md b/.changeset/happy-mugs-camp.md new file mode 100644 index 0000000000..22d13d80d1 --- /dev/null +++ b/.changeset/happy-mugs-camp.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-search-common': patch +--- + +- Introduce `SearchDocument` type. This type contains the subset of `IndexableDocument` properties relevant to the frontend, and is intended to be used for documents returned to the frontend from the search API. +- `SearchResultSet` is now a wrapper for documents of type `SearchDocument`, and is intended to be used in the frontend. This isn't a breaking change, since `IndexableDocument`s are valid `SearchDocument`s, so the old and new types are compatible. +- Introduce `IndexableResultSet` type, which wraps `IndexableDocument` instances in the same way as `SearchResultSet`. diff --git a/.changeset/light-drinks-rule.md b/.changeset/light-drinks-rule.md new file mode 100644 index 0000000000..2b99f5988c --- /dev/null +++ b/.changeset/light-drinks-rule.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-search-backend': patch +'@backstage/plugin-search-backend-node': patch +'@backstage/plugin-search-backend-module-elasticsearch': patch +'@backstage/plugin-search-backend-module-pg': patch +--- + +Use new `IndexableResultSet` type as return type of query method in `SearchEngine` implementation. diff --git a/.changeset/ninety-fishes-vanish.md b/.changeset/ninety-fishes-vanish.md new file mode 100644 index 0000000000..b0d3ac370f --- /dev/null +++ b/.changeset/ninety-fishes-vanish.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-search': patch +--- + +Switch to `SearchDocument` type in `DefaultResultListItem` props diff --git a/plugins/search-backend-module-elasticsearch/api-report.md b/plugins/search-backend-module-elasticsearch/api-report.md index 265fdda686..10231f3a83 100644 --- a/plugins/search-backend-module-elasticsearch/api-report.md +++ b/plugins/search-backend-module-elasticsearch/api-report.md @@ -10,10 +10,10 @@ import { Client } from '@elastic/elasticsearch'; import { Config } from '@backstage/config'; import type { ConnectionOptions } from 'tls'; import { IndexableDocument } from '@backstage/plugin-search-common'; +import { IndexableResultSet } from '@backstage/plugin-search-common'; import { Logger } from 'winston'; import { SearchEngine } from '@backstage/plugin-search-common'; import { SearchQuery } from '@backstage/plugin-search-common'; -import { SearchResultSet } from '@backstage/plugin-search-common'; // Warning: (ae-missing-release-tag) "ElasticSearchClientOptions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "@backstage/plugin-search-backend-module-elasticsearch" does not have an export "ElasticSearchEngine" @@ -119,7 +119,7 @@ export class ElasticSearchSearchEngine implements SearchEngine { getIndexer(type: string): Promise; newClient(create: (options: ElasticSearchClientOptions) => T): T; // (undocumented) - query(query: SearchQuery): Promise; + query(query: SearchQuery): Promise; // Warning: (ae-forgotten-export) The symbol "ElasticSearchQueryTranslator" needs to be exported by the entry point index.d.ts // // (undocumented) diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts index f2ed5ed2b0..aec225a64b 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts +++ b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts @@ -21,9 +21,9 @@ import { import { Config } from '@backstage/config'; import { IndexableDocument, + IndexableResultSet, SearchEngine, SearchQuery, - SearchResultSet, } from '@backstage/plugin-search-common'; import { Client } from '@elastic/elasticsearch'; import esb from 'elastic-builder'; @@ -192,7 +192,7 @@ export class ElasticSearchSearchEngine implements SearchEngine { return indexer; } - async query(query: SearchQuery): Promise { + async query(query: SearchQuery): Promise { const { elasticSearchQuery, documentTypes, pageSize } = this.translator(query); const queryIndices = documentTypes diff --git a/plugins/search-backend-module-pg/api-report.md b/plugins/search-backend-module-pg/api-report.md index 39847a4935..71ceac996e 100644 --- a/plugins/search-backend-module-pg/api-report.md +++ b/plugins/search-backend-module-pg/api-report.md @@ -5,11 +5,11 @@ ```ts import { BatchSearchEngineIndexer } from '@backstage/plugin-search-backend-node'; import { IndexableDocument } from '@backstage/plugin-search-common'; +import { IndexableResultSet } from '@backstage/plugin-search-common'; import { Knex } from 'knex'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { SearchEngine } from '@backstage/plugin-search-backend-node'; import { SearchQuery } from '@backstage/plugin-search-common'; -import { SearchResultSet } from '@backstage/plugin-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) // @@ -88,7 +88,7 @@ export class PgSearchEngine implements SearchEngine { // (undocumented) getIndexer(type: string): Promise; // (undocumented) - query(query: SearchQuery): Promise; + query(query: SearchQuery): Promise; // (undocumented) setTranslator( translator: (query: SearchQuery) => ConcretePgSearchQuery, diff --git a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts index 8f75112837..ea363d8ea1 100644 --- a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts +++ b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngine.ts @@ -15,7 +15,10 @@ */ import { PluginDatabaseManager } from '@backstage/backend-common'; import { SearchEngine } from '@backstage/plugin-search-backend-node'; -import { SearchQuery, SearchResultSet } from '@backstage/plugin-search-common'; +import { + SearchQuery, + IndexableResultSet, +} from '@backstage/plugin-search-common'; import { PgSearchEngineIndexer } from './PgSearchEngineIndexer'; import { DatabaseDocumentStore, @@ -81,7 +84,7 @@ export class PgSearchEngine implements SearchEngine { }); } - async query(query: SearchQuery): Promise { + async query(query: SearchQuery): Promise { const { pgQuery, pageSize } = this.translator(query); const rows = await this.databaseStore.transaction(async tx => diff --git a/plugins/search-backend-node/api-report.md b/plugins/search-backend-node/api-report.md index e4359eafac..ebc49df280 100644 --- a/plugins/search-backend-node/api-report.md +++ b/plugins/search-backend-node/api-report.md @@ -9,13 +9,13 @@ import { DocumentCollatorFactory } from '@backstage/plugin-search-common'; import { DocumentDecoratorFactory } from '@backstage/plugin-search-common'; import { DocumentTypeInfo } from '@backstage/plugin-search-common'; import { IndexableDocument } from '@backstage/plugin-search-common'; +import { IndexableResultSet } from '@backstage/plugin-search-common'; import { Logger } from 'winston'; import { default as lunr_2 } from 'lunr'; import { QueryTranslator } from '@backstage/plugin-search-common'; import { Readable } from 'stream'; import { SearchEngine } from '@backstage/plugin-search-common'; import { SearchQuery } from '@backstage/plugin-search-common'; -import { SearchResultSet } from '@backstage/plugin-search-common'; import { Transform } from 'stream'; import { Writable } from 'stream'; @@ -87,7 +87,7 @@ export class LunrSearchEngine implements SearchEngine { // (undocumented) protected lunrIndices: Record; // (undocumented) - query(query: SearchQuery): Promise; + query(query: SearchQuery): Promise; // (undocumented) setTranslator(translator: LunrQueryTranslator): void; // (undocumented) diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngine.ts b/plugins/search-backend-node/src/engines/LunrSearchEngine.ts index b1d695ad1d..09e8729ba8 100644 --- a/plugins/search-backend-node/src/engines/LunrSearchEngine.ts +++ b/plugins/search-backend-node/src/engines/LunrSearchEngine.ts @@ -16,8 +16,8 @@ import { IndexableDocument, + IndexableResultSet, SearchQuery, - SearchResultSet, QueryTranslator, SearchEngine, } from '@backstage/plugin-search-common'; @@ -147,7 +147,7 @@ export class LunrSearchEngine implements SearchEngine { return indexer; } - async query(query: SearchQuery): Promise { + async query(query: SearchQuery): Promise { const { lunrQueryBuilder, documentTypes, pageSize } = this.translator( query, ) as ConcreteLunrQuery; @@ -196,8 +196,8 @@ export class LunrSearchEngine implements SearchEngine { ? encodePageCursor({ page: page - 1 }) : undefined; - // Translate results into SearchResultSet - const realResultSet: SearchResultSet = { + // Translate results into IndexableResultSet + const realResultSet: IndexableResultSet = { results: results.slice(offset, offset + pageSize).map(d => { return { type: d.type, document: this.docStore[d.result.ref] }; }), diff --git a/plugins/search-backend/src/service/AuthorizedSearchEngine.ts b/plugins/search-backend/src/service/AuthorizedSearchEngine.ts index 2e65e18a84..2a5fc471f5 100644 --- a/plugins/search-backend/src/service/AuthorizedSearchEngine.ts +++ b/plugins/search-backend/src/service/AuthorizedSearchEngine.ts @@ -25,12 +25,12 @@ import { } from '@backstage/plugin-permission-common'; import { DocumentTypeInfo, + IndexableResult, + IndexableResultSet, QueryRequestOptions, QueryTranslator, SearchEngine, SearchQuery, - SearchResult, - SearchResultSet, } from '@backstage/plugin-search-common'; import { Config } from '@backstage/config'; import { InputError } from '@backstage/errors'; @@ -85,7 +85,7 @@ export class AuthorizedSearchEngine implements SearchEngine { async query( query: SearchQuery, options: QueryRequestOptions, - ): Promise { + ): Promise { const queryStartTime = Date.now(); const authorizer = new DataLoader( @@ -144,7 +144,7 @@ export class AuthorizedSearchEngine implements SearchEngine { const { page } = decodePageCursor(query.pageCursor); const targetResults = (page + 1) * this.pageSize; - let filteredResults: SearchResult[] = []; + let filteredResults: IndexableResult[] = []; let nextPageCursor: string | undefined; let latencyBudgetExhausted = false; @@ -183,7 +183,7 @@ export class AuthorizedSearchEngine implements SearchEngine { } private async filterResults( - results: SearchResult[], + results: IndexableResult[], typeDecisions: Record, authorizer: DataLoader, ) { diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index bdf46a240b..2efab91256 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/plugins/search-backend/src/service/router.test.ts @@ -19,7 +19,6 @@ import { ConfigReader } from '@backstage/config'; import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; import { IndexBuilder, - LunrSearchEngine, SearchEngine, } from '@backstage/plugin-search-backend-node'; import express from 'express'; @@ -39,8 +38,19 @@ describe('createRouter', () => { beforeAll(async () => { const logger = getVoidLogger(); - const searchEngine = new LunrSearchEngine({ logger }); - const indexBuilder = new IndexBuilder({ logger, searchEngine }); + mockSearchEngine = { + getIndexer: jest.fn(), + setTranslator: jest.fn(), + query: jest.fn().mockResolvedValue({ + results: [], + nextPageCursor: '', + previousPageCursor: '', + }), + }; + const indexBuilder = new IndexBuilder({ + logger, + searchEngine: mockSearchEngine, + }); const router = await createRouter({ engine: indexBuilder.getSearchEngine(), @@ -56,7 +66,7 @@ describe('createRouter', () => { }); beforeEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); }); describe('GET /query', () => { @@ -101,6 +111,42 @@ describe('createRouter', () => { }); }); + it('removes backend-only properties from search documents', async () => { + mockSearchEngine.query.mockResolvedValue({ + results: [ + { + type: 'software-catalog', + document: { + text: 'foo', + title: 'bar baz', + location: '/catalog/default/component/example', + authorization: { + resourceRef: 'component:default/example', + }, + }, + }, + ], + nextPageCursor: '', + previousPageCursor: '', + }); + + const response = await request(app).get('/query'); + + expect(response.status).toEqual(200); + expect(response.body).toMatchObject({ + results: [ + { + type: 'software-catalog', + document: { + text: 'foo', + title: 'bar baz', + location: '/catalog/default/component/example', + }, + }, + ], + }); + }); + describe('search result filtering', () => { beforeAll(async () => { const logger = getVoidLogger(); diff --git a/plugins/search-backend/src/service/router.ts b/plugins/search-backend/src/service/router.ts index 92e4e5526f..ff91465cf4 100644 --- a/plugins/search-backend/src/service/router.ts +++ b/plugins/search-backend/src/service/router.ts @@ -26,6 +26,7 @@ import { getBearerTokenFromAuthorizationHeader } from '@backstage/plugin-auth-no import { PermissionAuthorizer } from '@backstage/plugin-permission-common'; import { DocumentTypeInfo, + IndexableResultSet, SearchResultSet, } from '@backstage/plugin-search-common'; import { SearchEngine } from '@backstage/plugin-search-backend-node'; @@ -89,6 +90,17 @@ export async function createRouter( }), }); + const toSearchResults = (resultSet: IndexableResultSet): SearchResultSet => ({ + ...resultSet, + results: resultSet.results.map(result => ({ + ...result, + document: { + ...result.document, + authorization: undefined, + }, + })), + }); + const router = Router(); router.get( '/query', @@ -116,7 +128,7 @@ export async function createRouter( try { const resultSet = await engine?.query(query, { token }); - res.send(filterResultSet(resultSet)); + res.send(filterResultSet(toSearchResults(resultSet))); } catch (err) { throw new Error( `There was a problem performing the search query. ${err}`, diff --git a/plugins/search-common/api-report.md b/plugins/search-common/api-report.md index 8ab25d5e42..bd5a56c5cc 100644 --- a/plugins/search-common/api-report.md +++ b/plugins/search-common/api-report.md @@ -30,14 +30,17 @@ export type DocumentTypeInfo = { }; // @beta -export interface IndexableDocument { +export type IndexableDocument = SearchDocument & { authorization?: { resourceRef: string; }; - location: string; - text: string; - title: string; -} +}; + +// @beta (undocumented) +export type IndexableResult = Result; + +// @beta (undocumented) +export type IndexableResultSet = ResultSet; // @beta export type QueryRequestOptions = { @@ -47,13 +50,38 @@ export type QueryRequestOptions = { // @beta export type QueryTranslator = (query: SearchQuery) => unknown; +// @beta (undocumented) +export interface Result { + // (undocumented) + document: TDocument; + // (undocumented) + type: string; +} + +// @beta (undocumented) +export interface ResultSet { + // (undocumented) + nextPageCursor?: string; + // (undocumented) + previousPageCursor?: string; + // (undocumented) + results: Result[]; +} + +// @beta +export interface SearchDocument { + location: string; + text: string; + title: string; +} + // @beta export interface SearchEngine { getIndexer(type: string): Promise; query( query: SearchQuery, options?: QueryRequestOptions, - ): Promise; + ): Promise; setTranslator(translator: QueryTranslator): void; } @@ -70,20 +98,8 @@ export interface SearchQuery { } // @beta (undocumented) -export interface SearchResult { - // (undocumented) - document: IndexableDocument; - // (undocumented) - type: string; -} +export type SearchResult = Result; // @beta (undocumented) -export interface SearchResultSet { - // (undocumented) - nextPageCursor?: string; - // (undocumented) - previousPageCursor?: string; - // (undocumented) - results: SearchResult[]; -} +export type SearchResultSet = ResultSet; ``` diff --git a/plugins/search-common/src/types.ts b/plugins/search-common/src/types.ts index 51ce45617b..4eedabeb71 100644 --- a/plugins/search-common/src/types.ts +++ b/plugins/search-common/src/types.ts @@ -31,26 +31,45 @@ export interface SearchQuery { /** * @beta */ -export interface SearchResult { +export interface Result { type: string; - document: IndexableDocument; + document: TDocument; } /** * @beta */ -export interface SearchResultSet { - results: SearchResult[]; +export interface ResultSet { + results: Result[]; nextPageCursor?: string; previousPageCursor?: string; } /** - * Base properties that all indexed documents must include, as well as some - * common properties that documents are encouraged to use where appropriate. * @beta */ -export interface IndexableDocument { +export type SearchResult = Result; + +/** + * @beta + */ +export type SearchResultSet = ResultSet; + +/** + * @beta + */ +export type IndexableResult = Result; + +/** + * @beta + */ +export type IndexableResultSet = ResultSet; + +/** + * Base properties that all search documents must include. + * @beta + */ +export interface SearchDocument { /** * The primary name of the document (e.g. name, title, identifier, etc). */ @@ -66,7 +85,16 @@ export interface IndexableDocument { * is clicked). */ location: string; +} +/** + * Properties related to indexing of documents. This type is only useful for + * backends working directly with documents being inserted or retrieved from + * search indexes. When dealing with documents in the frontend, use + * {@link SearchDocument}. + * @beta + */ +export type IndexableDocument = SearchDocument & { /** * Optional authorization information to be used when determining whether this * search result should be visible to a given user. @@ -77,7 +105,7 @@ export interface IndexableDocument { */ resourceRef: string; }; -} +}; /** * Information about a specific document type. Intended to be used in the @@ -178,5 +206,5 @@ export interface SearchEngine { query( query: SearchQuery, options?: QueryRequestOptions, - ): Promise; + ): Promise; } diff --git a/plugins/search/api-report.md b/plugins/search/api-report.md index 0a277dab85..1bdd918802 100644 --- a/plugins/search/api-report.md +++ b/plugins/search/api-report.md @@ -9,13 +9,13 @@ import { ApiRef } from '@backstage/core-plugin-api'; import { AsyncState } from 'react-use/lib/useAsync'; import { BackstagePlugin } from '@backstage/core-plugin-api'; import { IconComponent } from '@backstage/core-plugin-api'; -import { IndexableDocument } from '@backstage/plugin-search-common'; import { InputBaseProps } from '@material-ui/core'; import { JsonObject } from '@backstage/types'; import { default as React_2 } from 'react'; import { ReactElement } from 'react'; import { ReactNode } from 'react'; import { RouteRef } from '@backstage/core-plugin-api'; +import { SearchDocument } from '@backstage/plugin-search-common'; import { SearchQuery } from '@backstage/plugin-search-common'; import { SearchResult as SearchResult_2 } from '@backstage/plugin-search-common'; import { SearchResultSet } from '@backstage/plugin-search-common'; @@ -31,7 +31,7 @@ export const DefaultResultListItem: ({ }: { icon?: ReactNode; secondaryAction?: ReactNode; - result: IndexableDocument; + result: SearchDocument; lineClamp?: number | undefined; }) => JSX.Element; diff --git a/plugins/search/src/components/DefaultResultListItem/DefaultResultListItem.tsx b/plugins/search/src/components/DefaultResultListItem/DefaultResultListItem.tsx index d9fbfe315e..46045f91ee 100644 --- a/plugins/search/src/components/DefaultResultListItem/DefaultResultListItem.tsx +++ b/plugins/search/src/components/DefaultResultListItem/DefaultResultListItem.tsx @@ -15,7 +15,7 @@ */ import React, { ReactNode } from 'react'; -import { IndexableDocument } from '@backstage/plugin-search-common'; +import { SearchDocument } from '@backstage/plugin-search-common'; import { ListItem, ListItemIcon, @@ -29,7 +29,7 @@ import TextTruncate from 'react-text-truncate'; type Props = { icon?: ReactNode; secondaryAction?: ReactNode; - result: IndexableDocument; + result: SearchDocument; lineClamp?: number; };