From 5b35637307e072872bd51af8415f375fa21508d0 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 9 Sep 2022 11:17:21 +0200 Subject: [PATCH] catalog: add support for multiple sortfields Signed-off-by: Vincenzo Scamporlino --- .../catalog-client/src/CatalogClient.test.ts | 11 ++-- packages/catalog-client/src/CatalogClient.ts | 18 ++---- packages/catalog-client/src/types/api.ts | 8 ++- plugins/catalog-backend/src/catalog/types.ts | 26 +++++---- .../service/DefaultEntitiesCatalog.test.ts | 8 +-- .../src/service/DefaultEntitiesCatalog.ts | 47 ++++++++------- .../src/service/createRouter.test.ts | 6 +- .../parseEntitySortFieldParams.test.ts | 57 +++++++++++++++++++ .../request/parseEntitySortFieldParams.ts | 41 +++++++++++++ .../parsePaginatedEntitiesParams.test.ts | 14 ++--- .../request/parsePaginatedEntitiesParams.ts | 23 +------- 11 files changed, 173 insertions(+), 86 deletions(-) create mode 100644 plugins/catalog-backend/src/service/request/parseEntitySortFieldParams.test.ts create mode 100644 plugins/catalog-backend/src/service/request/parseEntitySortFieldParams.ts diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 8a449c730e..4e5065e089 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -362,11 +362,13 @@ describe('CatalogClient', () => { fields: ['a', 'b'], limit: 100, query: 'query', - sortField: 'metadata.name', - sortFieldOrder: 'asc', + sortFields: [ + { field: 'metadata.name', order: 'asc' }, + { field: 'metadata.uid', order: 'desc' }, + ], }); expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( - '?limit=100&sortField=metadata.name&sortFieldOrder=asc&fields=a,b&query=query', + '?limit=100&sortField=metadata.name,asc&sortField=metadata.uid,desc&fields=a,b&query=query', ); }); @@ -383,8 +385,7 @@ describe('CatalogClient', () => { fields: ['a', 'b'], limit: 100, query: 'query', - sortField: 'metadata.name', - sortFieldOrder: 'asc', + sortFields: [{ field: 'metadata.name', order: 'asc' }], cursor: 'cursor', }); expect(mockedEndpoint.mock.calls[0][0].url.search).toBe( diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 5846430a78..18b614fd2e 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -217,24 +217,16 @@ export class CatalogClient implements CatalogApi { const params: string[] = []; if (isPaginatedEntitiesInitialRequest(request)) { - const { - fields = [], - filter, - limit, - sortField, - sortFieldOrder, - query, - } = request ?? {}; + const { fields = [], filter, limit, sortFields, query } = request ?? {}; params.push(...this.getParams(filter)); if (limit !== undefined) { params.push(`limit=${limit}`); } - if (sortField !== undefined) { - params.push(`sortField=${sortField}`); - } - if (sortFieldOrder !== undefined) { - params.push(`sortFieldOrder=${sortFieldOrder}`); + if (sortFields !== undefined) { + sortFields.forEach(({ field, order }) => + params.push(`sortField=${field},${order}`), + ); } if (fields.length) { params.push(`fields=${fields.map(encodeURIComponent).join(',')}`); diff --git a/packages/catalog-client/src/types/api.ts b/packages/catalog-client/src/types/api.ts index b273eb9e13..d006a6877c 100644 --- a/packages/catalog-client/src/types/api.ts +++ b/packages/catalog-client/src/types/api.ts @@ -366,6 +366,11 @@ export type EntitiesFilter = | Record | undefined; +/** + * Used for sorting the entities. + */ +export type EntitySortField = { field: string; order: 'asc' | 'desc' }; + /** * The response type for {@link CatalogClient.addLocation}. * @@ -411,9 +416,8 @@ export type GetPaginatedEntitiesInitialRequest = fields?: string[]; limit?: number; filter?: EntitiesFilter; - sortField?: string; + sortFields?: EntitySortField[]; query?: string; - sortFieldOrder?: 'asc' | 'desc'; } | undefined; diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 9f5ad642f2..966e09b821 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -46,6 +46,12 @@ export type EntityOrder = { order: 'asc' | 'desc'; }; +// TODO(vinzscam): remove this type in favor of EntityOrder +export type EntitySortField = { + field: string; + order?: 'asc' | 'desc' | undefined; +}; + /** * Matches rows in the search table. * @public @@ -234,9 +240,8 @@ export interface PaginatedEntitiesInitialRequest { fields?: (entity: Entity) => Entity; limit?: number; filter?: EntityFilter; - sortField?: string; + sortFields?: EntitySortField[]; query?: string; - sortFieldOrder?: 'asc' | 'desc' | undefined; } /** @@ -283,20 +288,19 @@ export interface PaginatedEntitiesResponse { */ export type Cursor = { /** - * the id of the field used for sorting the data. - * For example, metadata.name + * An array of fields used for sorting the data. + * For example, [ { field: 'metadata.name', order: 'asc' } ] */ - sortField: string; + sortFields: EntitySortField[]; /** - * The value of the last item returned - * by the request. This is used for performing - * cursor based pagination. + * The value of the cursor of the last item returned. + * This is used for performing pagination. */ - sortFieldId: string; + sortFieldIds: string[]; + /** - * In which order the data should be paginated. + * A filter to apply on the full list of entities. */ - sortFieldOrder: 'asc' | 'desc'; filter?: EntityFilter; /** * true if the cursor is a previous cursor. diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts index 8f043a594b..af4bacadaa 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts @@ -729,7 +729,7 @@ describe('DefaultEntitiesCatalog', () => { const request1: PaginatedEntitiesInitialRequest = { filter, limit, - sortField: 'metadata.name', + sortFields: [{ field: 'metadata.name' }], }; const response1 = await catalog.paginatedEntities(request1); expect(response1.entities).toEqual([entityFrom('A'), entityFrom('B')]); @@ -881,8 +881,7 @@ describe('DefaultEntitiesCatalog', () => { const request1: PaginatedEntitiesInitialRequest = { filter, limit, - sortField: 'metadata.name', - sortFieldOrder: 'desc', + sortFields: [{ field: 'metadata.name', order: 'desc' }], }; const response1 = await catalog.paginatedEntities(request1); expect(response1.entities).toEqual([entityFrom('G'), entityFrom('F')]); @@ -1032,7 +1031,8 @@ describe('DefaultEntitiesCatalog', () => { const request: PaginatedEntitiesInitialRequest = { filter, limit: 100, - sortField: 'metadata.name', + + sortFields: [{ field: 'metadata.name' }], query: 'cAt ', }; const response = await catalog.paginatedEntities(request); diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index 9190c76dcc..022313db7c 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -35,6 +35,7 @@ import { EntityFacetsResponse, EntityFilter, EntityPagination, + EntitySortField, PaginatedEntitiesRequest, PaginatedEntitiesResponse, } from '../catalog/types'; @@ -54,8 +55,10 @@ import { isPaginatedEntitiesInitialRequest, } from './util'; -const defaultSortField = 'metadata.name'; -const defaultSortFieldOrder = 'asc'; +const defaultSortField: EntitySortField = { + field: 'metadata.name', + order: 'asc', +}; function parsePagination(input?: EntityPagination): { limit?: number; @@ -328,19 +331,26 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { const db = this.database; const limit = request?.limit ?? 20; - const cursor: Omit & { sortFieldId?: string } = { + const cursor: Omit & { sortFieldIds?: string[] } = { firstFieldId: '', - sortField: defaultSortField, - sortFieldOrder: defaultSortFieldOrder, + sortFields: [defaultSortField], isPrevious: false, ...parseCursorFromRequest(request), }; const isFetchingBackwards = cursor.isPrevious; + // TODO(vinzscam): at the moment only a single sortField is supported + const sortField: EntitySortField = { + ...defaultSortField, + ...cursor.sortFields[0], + }; + + const sortFieldId = cursor.sortFieldIds?.[0]; + const dbQuery = db('search') .join('final_entities', 'search.entity_id', 'final_entities.entity_id') - .where('key', cursor.sortField); + .where('key', sortField.field); if (cursor.filter) { parseFilter(cursor.filter, dbQuery, db, false, 'search.entity_id'); @@ -356,21 +366,19 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { const countQuery = dbQuery.clone(); - const isOrderingDescending = cursor.sortFieldOrder === 'desc'; - if (cursor.sortFieldId) { + const isOrderingDescending = sortField.order === 'desc'; + if (sortFieldId) { dbQuery.andWhere( 'value', isFetchingBackwards !== isOrderingDescending ? '<' : '>', - cursor.sortFieldId, + sortFieldId, ); } dbQuery .orderBy( 'value', - isFetchingBackwards - ? invertOrder(cursor.sortFieldOrder) - : cursor.sortFieldOrder, + isFetchingBackwards ? invertOrder(sortField.order) : sortField.order, ) // fetch an extra item to check if there are more items. .limit(isFetchingBackwards ? limit : limit + 1); @@ -408,7 +416,7 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { const nextCursor = hasMoreResults ? encodeCursor({ ...cursor, - sortFieldId: rows[rows.length - 1].value, + sortFieldIds: [rows[rows.length - 1].value], firstFieldId, isPrevious: false, totalItems, @@ -421,7 +429,7 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { rows[0].value !== cursor.firstFieldId ? encodeCursor({ ...cursor, - sortFieldId: rows[0].value, + sortFieldIds: [rows[0].value], firstFieldId: cursor.firstFieldId, isPrevious: true, totalItems, @@ -625,13 +633,8 @@ function parseCursorFromRequest( request?: PaginatedEntitiesRequest, ): Partial { if (isPaginatedEntitiesInitialRequest(request)) { - const { - filter, - sortField = defaultSortField, - sortFieldOrder = defaultSortFieldOrder, - query, - } = request; - return { filter, sortField, sortFieldOrder, query }; + const { filter, sortFields = [defaultSortField], query } = request; + return { filter, sortFields, query }; } if (isPaginatedEntitiesCursorRequest(request)) { try { @@ -646,6 +649,6 @@ function parseCursorFromRequest( return {}; } -function invertOrder(order: Cursor['sortFieldOrder']) { +function invertOrder(order: EntitySortField['order']) { return order === 'asc' ? 'desc' : 'asc'; } diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 6e2d52c63e..c3b96fbfd8 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -163,7 +163,7 @@ describe('createRouter readonly disabled', () => { totalItems: 0, }); const response = await request(app).get( - '/v2beta1/entities?filter=a=1,a=2,b=3&filter=c=4', + '/v2beta1/entities?filter=a=1,a=2,b=3&filter=c=4&sortField=metadata.name,asc&sortField=metadata.uid,desc', ); expect(response.status).toEqual(200); @@ -180,6 +180,10 @@ describe('createRouter readonly disabled', () => { { allOf: [{ key: 'c', values: ['4'] }] }, ], }, + sortFields: [ + { field: 'metadata.name', order: 'asc' }, + { field: 'metadata.uid', order: 'desc' }, + ], }); }); diff --git a/plugins/catalog-backend/src/service/request/parseEntitySortFieldParams.test.ts b/plugins/catalog-backend/src/service/request/parseEntitySortFieldParams.test.ts new file mode 100644 index 0000000000..35f7c2cac0 --- /dev/null +++ b/plugins/catalog-backend/src/service/request/parseEntitySortFieldParams.test.ts @@ -0,0 +1,57 @@ +/* + * Copyright 2022 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 { parseEntitySortFieldParams } from './parseEntitySortFieldParams'; + +describe('parseEntitySortFieldParams', () => { + it('supports no sort fields', () => { + const result = parseEntitySortFieldParams({}); + expect(result).toEqual(undefined); + }); + + it('supports single sortField', () => { + const result = parseEntitySortFieldParams({ + sortField: ['metadata.name,desc'], + })!; + expect(result).toEqual([{ field: 'metadata.name', order: 'desc' }]); + }); + + it('supports single sortField without order', () => { + const result = parseEntitySortFieldParams({ + sortField: ['metadata.name'], + })!; + expect(result).toEqual([{ field: 'metadata.name' }]); + }); + + it('supports multiple sort fields', () => { + const result = parseEntitySortFieldParams({ + sortField: ['metadata.name,desc', 'metadata.uid,asc'], + }); + + expect(result).toEqual([ + { field: 'metadata.name', order: 'desc' }, + { field: 'metadata.uid', order: 'asc' }, + ]); + }); + + it('throws if sortField order is not valid', () => { + expect(() => + parseEntitySortFieldParams({ + sortField: ['metadata.name,desc', 'metadata.uid,invalid'], + }), + ).toThrow(/Invalid sort field order/); + }); +}); diff --git a/plugins/catalog-backend/src/service/request/parseEntitySortFieldParams.ts b/plugins/catalog-backend/src/service/request/parseEntitySortFieldParams.ts new file mode 100644 index 0000000000..d16295aa30 --- /dev/null +++ b/plugins/catalog-backend/src/service/request/parseEntitySortFieldParams.ts @@ -0,0 +1,41 @@ +/* + * Copyright 2022 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 { EntitySortField } from '../../catalog/types'; +import { parseStringsParam } from './common'; + +export function parseEntitySortFieldParams( + params: Record, +): EntitySortField[] | undefined { + const sortFieldStrings = parseStringsParam(params.sortField, 'sortField'); + if (!sortFieldStrings) { + return undefined; + } + + return sortFieldStrings.map(sortFieldString => { + const [field, order] = sortFieldString.split(','); + + if (order !== undefined && !isOrder(order)) { + throw new InputError('Invalid sort field order, must be asc or desc'); + } + return { field, order }; + }); +} + +export function isOrder(order: string): order is 'asc' | 'desc' { + return ['asc', 'desc'].includes(order); +} diff --git a/plugins/catalog-backend/src/service/request/parsePaginatedEntitiesParams.test.ts b/plugins/catalog-backend/src/service/request/parsePaginatedEntitiesParams.test.ts index 838b931888..53d44c8ca5 100644 --- a/plugins/catalog-backend/src/service/request/parsePaginatedEntitiesParams.test.ts +++ b/plugins/catalog-backend/src/service/request/parsePaginatedEntitiesParams.test.ts @@ -28,8 +28,7 @@ describe('parsePaginatedEntitiesParams', () => { fields: ['kind'], limit: '3', filter: ['a=1', 'b=2'], - sortField: 'sortField', - sortFieldOrder: 'desc', + sortField: ['metadata.name,desc'], query: 'query', }; const parsedObj = parsePaginatedEntitiesParams( @@ -37,8 +36,9 @@ describe('parsePaginatedEntitiesParams', () => { ) as PaginatedEntitiesInitialRequest; expect(parsedObj.limit).toBe(3); expect(parsedObj.fields).toBeDefined(); - expect(parsedObj.sortField).toBe('sortField'); - expect(parsedObj.sortFieldOrder).toBe('desc'); + expect(parsedObj.sortFields).toEqual([ + { field: 'metadata.name', order: 'desc' }, + ]); expect(parsedObj.filter).toBeDefined(); expect(parsedObj.query).toBe('query'); expect(parsedObj).not.toHaveProperty('authorizationToken'); @@ -50,8 +50,7 @@ describe('parsePaginatedEntitiesParams', () => { ) as PaginatedEntitiesInitialRequest; expect(parsedObj.limit).toBeUndefined(); expect(parsedObj.fields).toBeUndefined(); - expect(parsedObj.sortField).toBeUndefined(); - expect(parsedObj.sortFieldOrder).toBeUndefined(); + expect(parsedObj.sortFields).toBeUndefined(); expect(parsedObj.filter).toBeUndefined(); expect(parsedObj.query).toBeUndefined(); expect(parsedObj).not.toHaveProperty('authorizationToken'); @@ -63,8 +62,7 @@ describe('parsePaginatedEntitiesParams', () => { limit: 'asd', }, { filter: 3 }, - { sortField: [] }, - { sortFieldOrder: 'something' }, + { sortField: ['metadata.uid,diagonal'] }, { fields: [4] }, { query: [] }, ])('should throw if some parameter is not valid %p', params => { diff --git a/plugins/catalog-backend/src/service/request/parsePaginatedEntitiesParams.ts b/plugins/catalog-backend/src/service/request/parsePaginatedEntitiesParams.ts index 84d519ec0b..f5730d3297 100644 --- a/plugins/catalog-backend/src/service/request/parsePaginatedEntitiesParams.ts +++ b/plugins/catalog-backend/src/service/request/parsePaginatedEntitiesParams.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { InputError } from '@backstage/errors'; import { PaginatedEntitiesCursorRequest, PaginatedEntitiesInitialRequest, @@ -22,6 +21,7 @@ import { } from '../../catalog/types'; import { parseIntegerParam, parseStringParam } from './common'; import { parseEntityFilterParams } from './parseEntityFilterParams'; +import { parseEntitySortFieldParams } from './parseEntitySortFieldParams'; import { parseEntityTransformParams } from './parseEntityTransformParams'; export function parsePaginatedEntitiesParams( @@ -42,33 +42,16 @@ export function parsePaginatedEntitiesParams( const filter = parseEntityFilterParams(params); const query = parseStringParam(params.query, 'query'); - const sortField = parseStringParam(params.sortField, 'sortField'); - const sortFieldOrder = parseSortFieldOrder( - params.sortFieldOrder, - 'sortFieldOrder', - ); + const sortFields = parseEntitySortFieldParams(params); const response: Omit = { fields, filter, limit, - sortField, - sortFieldOrder, + sortFields, query, }; return response; } - -function parseSortFieldOrder(sortFieldOrder: unknown, ctx: string) { - const isSortFieldOrder = - sortFieldOrder === undefined || - sortFieldOrder === 'asc' || - sortFieldOrder === 'desc'; - - if (isSortFieldOrder) { - return sortFieldOrder; - } - throw new InputError(`Invalid ${ctx}, not asc or desc`); -}