implement ordering in the catalog backend and client

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

.changeset/fifty-zoos-nail.md

@@ -0,0 +1,5 @@
+---
+'@backstage/catalog-client': minor
+---
+
+Implemented support for the `order` directive on `getEntities`

.changeset/silver-chicken-travel.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend': minor
+---
+
+Implemented server side ordering in the entities endpoint

packages/catalog-client/api-report.md

@@ -161,6 +161,17 @@ export type EntityFilterQuery =
   | Record<string, string | symbol | (string | symbol)[]>[]
   | Record<string, string | symbol | (string | symbol)[]>;
 
+// @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

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', () => {

packages/catalog-client/src/CatalogClient.ts

@@ -99,7 +99,14 @@ export class CatalogClient implements CatalogApi {
     request?: GetEntitiesRequest,
     options?: CatalogRequestOptions,
   ): Promise<GetEntitiesResponse> {
-    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}`);
     }

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.
    */

packages/catalog-client/src/types/index.ts

@@ -22,6 +22,7 @@ export type {
   CatalogRequestOptions,
   EntityFieldsQuery,
   EntityFilterQuery,
+  EntityOrderQuery,
   GetEntitiesByRefsRequest,
   GetEntitiesByRefsResponse,
   GetEntitiesRequest,

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;
 };

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<string[]> {
+          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,
     );
   });
 });

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<DbSearchRow>('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<DbFinalEntitiesRow>('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 <order>" 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) {

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')),
         });

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',
+    );
+  });
+});

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<string, unknown>,
+): 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 };
+  });
+}