diff --git a/.changeset/fifty-zoos-nail.md b/.changeset/fifty-zoos-nail.md new file mode 100644 index 0000000000..7260a0b542 --- /dev/null +++ b/.changeset/fifty-zoos-nail.md @@ -0,0 +1,5 @@ +--- +'@backstage/catalog-client': minor +--- + +Implemented support for the `order` directive on `getEntities` diff --git a/.changeset/silver-chicken-travel.md b/.changeset/silver-chicken-travel.md new file mode 100644 index 0000000000..e5dea6381d --- /dev/null +++ b/.changeset/silver-chicken-travel.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +Implemented server side ordering in the entities endpoint diff --git a/packages/catalog-client/api-report.md b/packages/catalog-client/api-report.md index 2f76be79e2..a81314308b 100644 --- a/packages/catalog-client/api-report.md +++ b/packages/catalog-client/api-report.md @@ -161,6 +161,17 @@ export type EntityFilterQuery = | Record[] | Record; +// @public +export type EntityOrderQuery = + | { + field: string; + order: 'asc' | 'desc'; + } + | Array<{ + field: string; + order: 'asc' | 'desc'; + }>; + // @public export interface GetEntitiesByRefsRequest { entityRefs: string[]; @@ -179,6 +190,7 @@ export interface GetEntitiesRequest { filter?: EntityFilterQuery; limit?: number; offset?: number; + order?: EntityOrderQuery; } // @public diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 1206d557c1..f87b12353d 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -193,6 +193,29 @@ describe('CatalogClient', () => { expect(response.items).toEqual([]); }); + + it('handles ordering properly', async () => { + expect.assertions(2); + + server.use( + rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { + expect(req.url.search).toBe('?order=kind&order=-metadata.name'); + return res(ctx.json([])); + }), + ); + + const response = await client.getEntities( + { + order: [ + { field: 'kind', order: 'asc' }, + { field: 'metadata.name', order: 'desc' }, + ], + }, + { token }, + ); + + expect(response.items).toEqual([]); + }); }); describe('getEntitiesByRefs', () => { diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index d15f382ddd..0a5954d4bd 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -99,7 +99,14 @@ export class CatalogClient implements CatalogApi { request?: GetEntitiesRequest, options?: CatalogRequestOptions, ): Promise { - const { filter = [], fields = [], offset, limit, after } = request ?? {}; + const { + filter = [], + fields = [], + order, + offset, + limit, + after, + } = request ?? {}; const params: string[] = []; // filter param can occur multiple times, for example @@ -129,6 +136,20 @@ export class CatalogClient implements CatalogApi { params.push(`fields=${fields.map(encodeURIComponent).join(',')}`); } + if (order) { + for (const directive of [order].flat()) { + if (directive) { + // We could choose to always put in the + prefix, but it's not + // required (ascending sort is the default) and it always gets URL + // encoded to %2B which looks a bit less pretty - so we don't + const str = `${directive.order === 'desc' ? '-' : ''}${ + directive.field + }`; + params.push(`order=${encodeURIComponent(str)}`); + } + } + } + if (offset !== undefined) { params.push(`offset=${offset}`); } diff --git a/packages/catalog-client/src/types/api.ts b/packages/catalog-client/src/types/api.ts index 771adaaeee..d662a9d106 100644 --- a/packages/catalog-client/src/types/api.ts +++ b/packages/catalog-client/src/types/api.ts @@ -98,6 +98,48 @@ export type EntityFilterQuery = */ export type EntityFieldsQuery = string[]; +/** + * Dot-separated field based ordering directives, controlling the sort order of + * the output entities. + * + * @remarks + * + * Each field is a dot-separated path into an entity's keys. The order is either + * ascending (`asc`, lexicographical order) or descending (`desc`, reverse + * lexicographical order). The ordering is case insensitive. + * + * If more than one order directive is given, later directives have lower + * precedence (they are applied only when directives of higher precedence have + * equal values). + * + * Example: + * + * ``` + * [ + * { field: 'kind', order: 'asc' }, + * { field: 'metadata.name', order: 'desc' }, + * ] + * ``` + * + * This will order the output first by kind ascending, and then within each kind + * (if there's more than one of a given kind) by their name descending. + * + * When given a field that does NOT exist on all entities in the result set, + * those entities that do not have the field will always be sorted last in that + * particular order step, no matter what the desired order was. + * + * @public + */ +export type EntityOrderQuery = + | { + field: string; + order: 'asc' | 'desc'; + } + | Array<{ + field: string; + order: 'asc' | 'desc'; + }>; + /** * The request type for {@link CatalogClient.getEntities}. * @@ -113,6 +155,10 @@ export interface GetEntitiesRequest { * declarations. */ fields?: EntityFieldsQuery; + /** + *If given, order the result set by those directives. + */ + order?: EntityOrderQuery; /** * If given, skips over the first N items in the result set. */ diff --git a/packages/catalog-client/src/types/index.ts b/packages/catalog-client/src/types/index.ts index 5ac058e349..81b985eb42 100644 --- a/packages/catalog-client/src/types/index.ts +++ b/packages/catalog-client/src/types/index.ts @@ -22,6 +22,7 @@ export type { CatalogRequestOptions, EntityFieldsQuery, EntityFilterQuery, + EntityOrderQuery, GetEntitiesByRefsRequest, GetEntitiesByRefsResponse, GetEntitiesRequest, diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 84673a3900..faf7451acd 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -38,6 +38,14 @@ export type EntityPagination = { after?: string; }; +/** + * A sorting rule for entities. + */ +export type EntityOrder = { + field: string; + order: 'asc' | 'desc'; +}; + /** * Matches rows in the search table. * @public @@ -71,6 +79,7 @@ export type PageInfo = export type EntitiesRequest = { filter?: EntityFilter; fields?: (entity: Entity) => Entity; + order?: EntityOrder[]; pagination?: EntityPagination; authorizationToken?: string; }; diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts index b84596c78c..6735709662 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts @@ -28,6 +28,7 @@ import { import { Stitcher } from '../stitching/Stitcher'; import { buildEntitySearch } from '../stitching/buildEntitySearch'; import { DefaultEntitiesCatalog } from './DefaultEntitiesCatalog'; +import { EntitiesRequest } from '../catalog/types'; describe('DefaultEntitiesCatalog', () => { const databases = TestDatabases.create({ @@ -254,7 +255,7 @@ describe('DefaultEntitiesCatalog', () => { describe('entities', () => { it.each(databases.eachSupportedId())( - 'should return correct entity for simple filter', + 'should return correct entity for simple filter, %p', async databaseId => { const { knex } = await createDatabase(databaseId); const entity1: Entity = { @@ -284,10 +285,11 @@ describe('DefaultEntitiesCatalog', () => { expect(entities.length).toBe(1); expect(entities[0]).toEqual(entity2); }, + 60_000, ); it.each(databases.eachSupportedId())( - 'should return correct entity for negation filter', + 'should return correct entity for negation filter, %p', async databaseId => { const { knex } = await createDatabase(databaseId); const entity1: Entity = { @@ -319,10 +321,11 @@ describe('DefaultEntitiesCatalog', () => { expect(entities.length).toBe(1); expect(entities[0]).toEqual(entity1); }, + 60_000, ); it.each(databases.eachSupportedId())( - 'should return correct entities for nested filter', + 'should return correct entities for nested filter, %p', async databaseId => { const { knex } = await createDatabase(databaseId); const entity1: Entity = { @@ -388,10 +391,11 @@ describe('DefaultEntitiesCatalog', () => { expect(entities).toContainEqual(entity2); expect(entities).toContainEqual(entity4); }, + 60_000, ); it.each(databases.eachSupportedId())( - 'should return correct entities for complex negation filter', + 'should return correct entities for complex negation filter, %p', async databaseId => { const { knex } = await createDatabase(databaseId); const entity1: Entity = { @@ -429,10 +433,11 @@ describe('DefaultEntitiesCatalog', () => { expect(entities.length).toBe(1); expect(entities).toContainEqual(entity1); }, + 60_000, ); it.each(databases.eachSupportedId())( - 'should return no matches for an empty values array', + 'should return no matches for an empty values array, %p', // NOTE: An empty values array is not a sensible input in a realistic scenario. async databaseId => { const { knex } = await createDatabase(databaseId); @@ -461,6 +466,7 @@ describe('DefaultEntitiesCatalog', () => { expect(entities.length).toBe(0); }, + 60_000, ); it.each(databases.eachSupportedId())( @@ -517,12 +523,105 @@ describe('DefaultEntitiesCatalog', () => { }, ]); }, + 60_000, + ); + + it.each(databases.eachSupportedId())( + 'can order and combine with filtering, %p', + async databaseId => { + const { knex } = await createDatabase(databaseId); + + const entity1: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n1' }, + spec: { a: 'foo' }, + }; + const entity2: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n2' }, + spec: { a: 'bar' }, + }; + const entity3: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n3' }, + spec: { a: 'bar', b: 'lonely' }, + }; + const entity4: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'n4' }, + spec: { a: 'baz', b: 'only' }, + }; + await addEntityToSearch(knex, entity1); + await addEntityToSearch(knex, entity2); + await addEntityToSearch(knex, entity3); + await addEntityToSearch(knex, entity4); + + const catalog = new DefaultEntitiesCatalog(knex, stitcher); + + function f(request: EntitiesRequest): Promise { + return catalog + .entities(request) + .then(response => response.entities.map(e => e.metadata.name)); + } + + await expect( + f({ order: [{ field: 'metadata.name', order: 'asc' }] }), + ).resolves.toEqual(['n1', 'n2', 'n3', 'n4']); + + await expect( + f({ order: [{ field: 'metadata.name', order: 'desc' }] }), + ).resolves.toEqual(['n4', 'n3', 'n2', 'n1']); + + await expect( + f({ + order: [ + { field: 'spec.a', order: 'asc' }, + { field: 'metadata.name', order: 'desc' }, + ], + }), + ).resolves.toEqual(['n3', 'n2', 'n4', 'n1']); + + await expect( + f({ + filter: { not: { key: 'spec.b', values: ['lonely'] } }, + order: [ + { field: 'spec.a', order: 'asc' }, + { field: 'metadata.name', order: 'desc' }, + ], + }), + ).resolves.toEqual(['n2', 'n4', 'n1']); + + // only n3 and n4 has spec.b, nulls (no match) always goes last no matter the order + await expect( + f({ + order: [ + { field: 'spec.b', order: 'asc' }, + { field: 'metadata.name', order: 'asc' }, + ], + }), + ).resolves.toEqual(['n3', 'n4', 'n1', 'n2']); + + // only n3 and n4 has spec.b, nulls (no match) always goes last no matter the order + await expect( + f({ + order: [ + { field: 'spec.b', order: 'desc' }, + { field: 'metadata.name', order: 'asc' }, + ], + }), + ).resolves.toEqual(['n4', 'n3', 'n1', 'n2']); + }, + 60_000, ); }); describe('entitiesBatch', () => { it.each(databases.eachSupportedId())( - 'queries for entities by ref, including duplicates, and gracefully returns null for missing entities', + 'queries for entities by ref, including duplicates, and gracefully returns null for missing entities, %p', async databaseId => { const { knex } = await createDatabase(databaseId); @@ -571,12 +670,13 @@ describe('DefaultEntitiesCatalog', () => { 'k:default/two', ]); }, + 60_000, ); }); describe('removeEntityByUid', () => { it.each(databases.eachSupportedId())( - 'also clears parent hashes', + 'also clears parent hashes, %p', async databaseId => { const { knex } = await createDatabase(databaseId); @@ -659,12 +759,13 @@ describe('DefaultEntitiesCatalog', () => { new Set(['k:default/unrelated1', 'k:default/unrelated2']), ); }, + 60_000, ); }); describe('facets', () => { it.each(databases.eachSupportedId())( - 'can filter and collect properly', + 'can filter and collect properly, %p', async databaseId => { const { knex } = await createDatabase(databaseId); @@ -697,10 +798,11 @@ describe('DefaultEntitiesCatalog', () => { }, }); }, + 60_000, ); it.each(databases.eachSupportedId())( - 'can match on annotations and labels with dots in them', + 'can match on annotations and labels with dots in them, %p', async databaseId => { const { knex } = await createDatabase(databaseId); @@ -743,10 +845,11 @@ describe('DefaultEntitiesCatalog', () => { }, }); }, + 60_000, ); it.each(databases.eachSupportedId())( - 'can match on strings in arrays', + 'can match on strings in arrays, %p', async databaseId => { const { knex } = await createDatabase(databaseId); @@ -784,6 +887,7 @@ describe('DefaultEntitiesCatalog', () => { }, }); }, + 60_000, ); }); }); diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index 14f22f862b..d598881371 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -97,7 +97,7 @@ function addCondition( // make a lot of sense. However, it had abysmal performance on sqlite // when datasets grew large, so we're using IN instead. const matchQuery = db('search') - .select(entityIdField) + .select('search.entity_id') .where({ key: filter.key.toLowerCase() }) .andWhere(function keyFilter() { if (filter.values) { @@ -178,14 +178,48 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { let entitiesQuery = db('final_entities').select('final_entities.*'); + + request?.order?.forEach(({ field }, index) => { + const alias = `order_${index}`; + entitiesQuery = entitiesQuery.leftOuterJoin( + { [alias]: 'search' }, + function search(inner) { + inner + .on(`${alias}.entity_id`, 'final_entities.entity_id') + .andOn(`${alias}.key`, db.raw('?', [field])); + }, + ); + }); + + entitiesQuery = entitiesQuery.whereNotNull('final_entities.final_entity'); + if (request?.filter) { - entitiesQuery = parseFilter(request.filter, entitiesQuery, db); + entitiesQuery = parseFilter( + request.filter, + entitiesQuery, + db, + false, + 'final_entities.entity_id', + ); } - // TODO: move final_entities to use entity_ref - entitiesQuery = entitiesQuery - .whereNotNull('final_entities.final_entity') - .orderBy('entity_id', 'asc'); + request?.order?.forEach(({ order }, index) => { + if (db.client.config.client === 'pg') { + // pg correctly orders by the column value and handling nulls in one go + entitiesQuery = entitiesQuery.orderBy([ + { column: `order_${index}.value`, order, nulls: 'last' }, + ]); + } else { + // sqlite and mysql translate the above statement ONLY into "order by (value is null) asc" + // no matter what the order is, for some reason, so we have to manually add back the statement + // that translates to "order by value " while avoiding to give an order + entitiesQuery = entitiesQuery.orderBy([ + { column: `order_${index}.value`, order: undefined, nulls: 'last' }, + { column: `order_${index}.value`, order }, + ]); + } + }); + entitiesQuery = entitiesQuery.orderBy('final_entities.entity_id', 'asc'); // stable sort const { limit, offset } = parsePagination(request?.pagination); if (limit !== undefined) { diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 4680dbb899..6a31fcd0b6 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -41,6 +41,7 @@ import { parseEntityTransformParams, } from './request'; import { parseEntityFacetParams } from './request/parseEntityFacetParams'; +import { parseEntityOrderParams } from './request/parseEntityOrderParams'; import { LocationService, RefreshOptions, RefreshService } from './types'; import { disallowReadonlyMode, @@ -113,6 +114,7 @@ export async function createRouter( const { entities, pageInfo } = await entitiesCatalog.entities({ filter: parseEntityFilterParams(req.query), fields: parseEntityTransformParams(req.query), + order: parseEntityOrderParams(req.query), pagination: parseEntityPaginationParams(req.query), authorizationToken: getBearerToken(req.header('authorization')), }); diff --git a/plugins/catalog-backend/src/service/request/parseEntityOrderParams.test.ts b/plugins/catalog-backend/src/service/request/parseEntityOrderParams.test.ts new file mode 100644 index 0000000000..77cf73f57b --- /dev/null +++ b/plugins/catalog-backend/src/service/request/parseEntityOrderParams.test.ts @@ -0,0 +1,43 @@ +/* + * 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 { parseEntityOrderParams } from './parseEntityOrderParams'; + +describe('parseEntityOrderParams', () => { + it('handles missing parameter', () => { + expect(parseEntityOrderParams({})).toBeUndefined(); + }); + + it('handles parameters with various orders', () => { + expect(parseEntityOrderParams({ order: ['a', '+b', '-c'] })).toEqual([ + { field: 'a', order: 'asc' }, + { field: 'b', order: 'asc' }, + { field: 'c', order: 'desc' }, + ]); + }); + + it('rejects missing order or key', () => { + expect(() => parseEntityOrderParams({ order: [''] })).toThrow( + 'Invalid order parameter "", no field given', + ); + expect(() => parseEntityOrderParams({ order: ['+'] })).toThrow( + 'Invalid order parameter "+", no field given', + ); + expect(() => parseEntityOrderParams({ order: ['-'] })).toThrow( + 'Invalid order parameter "-", no field given', + ); + }); +}); diff --git a/plugins/catalog-backend/src/service/request/parseEntityOrderParams.ts b/plugins/catalog-backend/src/service/request/parseEntityOrderParams.ts new file mode 100644 index 0000000000..d10119fd47 --- /dev/null +++ b/plugins/catalog-backend/src/service/request/parseEntityOrderParams.ts @@ -0,0 +1,48 @@ +/* + * 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 { InputError } from '@backstage/errors'; +import { EntityOrder } from '../../catalog/types'; +import { parseStringsParam } from './common'; + +export function parseEntityOrderParams( + params: Record, +): EntityOrder[] | undefined { + return parseStringsParam(params.order, 'order')?.map(item => { + let order: 'asc' | 'desc'; + let field: string; + switch (item[0]) { + case '+': + order = 'asc'; + field = item.slice(1).trim(); + break; + case '-': + order = 'desc'; + field = item.slice(1).trim(); + break; + default: + order = 'asc'; + field = item.trim(); + break; + } + + if (!field) { + throw new InputError(`Invalid order parameter "${item}", no field given`); + } + + return { field, order }; + }); +}