From 86bef79ad1eaa499c0cd9afe443d4cd1029f22f4 Mon Sep 17 00:00:00 2001 From: Joon Park Date: Fri, 29 Oct 2021 15:31:46 +0100 Subject: [PATCH] Allow nested EntityFilters This makes the format of EntityFilters more flexible, and paves the way for the permissions system, which requires composing multiple _collections_ of filters. Signed-off-by: Joon Park --- .changeset/slimy-days-leave.md | 5 + plugins/catalog-backend/api-report.md | 2 +- plugins/catalog-backend/src/catalog/types.ts | 2 +- .../src/legacy/database/CommonDatabase.ts | 24 ++- .../src/service/NextEntitiesCatalog.test.ts | 155 ++++++++++++++++++ .../src/service/NextEntitiesCatalog.ts | 97 +++++++---- 6 files changed, 242 insertions(+), 43 deletions(-) create mode 100644 .changeset/slimy-days-leave.md diff --git a/.changeset/slimy-days-leave.md b/.changeset/slimy-days-leave.md new file mode 100644 index 0000000000..250bf32962 --- /dev/null +++ b/.changeset/slimy-days-leave.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Allow nested EntityFilters diff --git a/plugins/catalog-backend/api-report.md b/plugins/catalog-backend/api-report.md index 6d21884c12..e83fa68cad 100644 --- a/plugins/catalog-backend/api-report.md +++ b/plugins/catalog-backend/api-report.md @@ -872,7 +872,7 @@ export type EntityAncestryResponse = { // @public export type EntityFilter = { anyOf: { - allOf: EntitiesSearchFilter[]; + allOf: (EntitiesSearchFilter | EntityFilter)[]; }[]; }; diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 0bc0ec4311..b0b06c563c 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -23,7 +23,7 @@ import { Entity, EntityRelationSpec } from '@backstage/catalog-model'; * individual filters must match. */ export type EntityFilter = { - anyOf: { allOf: EntitiesSearchFilter[] }[]; + anyOf: { allOf: (EntitiesSearchFilter | EntityFilter)[] }[]; }; /** diff --git a/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts b/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts index 5cc31b5be5..81b0612989 100644 --- a/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts +++ b/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts @@ -46,7 +46,11 @@ import { DbPageInfo, Transaction, } from './types'; -import { EntityPagination } from '../../catalog/types'; +import { + EntityPagination, + EntityFilter, + EntitiesSearchFilter, +} from '../../catalog/types'; // The number of items that are sent per batch to the database layer, when // doing .batchInsert calls to knex. This needs to be low enough to not cause @@ -219,11 +223,13 @@ export class CommonDatabase implements Database { for (const singleFilter of request?.filter?.anyOf ?? []) { entitiesQuery = entitiesQuery.orWhere(function singleFilterFn() { - for (const { - key, - matchValueIn, - matchValueExists, - } of singleFilter.allOf) { + for (const filter of singleFilter.allOf) { + if (isEntityFilter(filter)) { + throw new Error( + 'Nested filters are not supported in the legacy CommonDatabase', + ); + } + const { key, matchValueIn, matchValueExists } = filter; // NOTE(freben): This used to be a set of OUTER JOIN, which may seem to // make a lot of sense. However, it had abysmal performance on sqlite // when datasets grew large, so we're using IN instead. @@ -606,3 +612,9 @@ function deduplicateRelations( r => `${r.source_full_name}:${r.target_full_name}:${r.type}`, ); } + +function isEntityFilter( + filter: EntitiesSearchFilter | EntityFilter, +): filter is EntityFilter { + return filter.hasOwnProperty('anyOf'); +} diff --git a/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts index 082aa47df5..0c805e1b17 100644 --- a/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts @@ -23,6 +23,7 @@ import { DbFinalEntitiesRow, DbRefreshStateReferencesRow, DbRefreshStateRow, + DbSearchRow, } from '../database/tables'; import { NextEntitiesCatalog } from './NextEntitiesCatalog'; @@ -73,6 +74,52 @@ describe('NextEntitiesCatalog', () => { } } + async function addEntityToSearch(knex: Knex, entity: Entity) { + const id = uuid(); + const entityRef = stringifyEntityRef(entity); + const entityJson = JSON.stringify(entity); + + await knex('refresh_state').insert({ + entity_id: id, + entity_ref: entityRef, + unprocessed_entity: entityJson, + errors: '[]', + next_update_at: '2031-01-01 23:00:00', + last_discovery_at: '2021-04-01 13:37:00', + }); + + await knex('final_entities').insert({ + entity_id: id, + final_entity: entityJson, + hash: 'h', + stitch_ticket: '', + }); + + await insertSearchRow(knex, id, null, entity); + } + + async function insertSearchRow( + knex: Knex, + id: string, + previousKey: string | null, + previousValue: Object, + ) { + return Promise.all( + Object.entries(previousValue).map(async ([key, value]) => { + const currentKey = `${previousKey ? `${previousKey}.` : ``}${key}`; + if (typeof value === 'object') { + await insertSearchRow(knex, id, currentKey, value); + } else { + await knex('search').insert({ + entity_id: id, + key: currentKey, + value: value, + }); + } + }), + ); + } + describe('entityAncestry', () => { it.each(databases.eachSupportedId())( 'should return the ancestry with one parent, %p', @@ -209,4 +256,112 @@ describe('NextEntitiesCatalog', () => { 60_000, ); }); + + describe('entities', () => { + it.each(databases.eachSupportedId())( + 'should return correct entity for simple filter', + async databaseId => { + const { knex } = await createDatabase(databaseId); + const entity1: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'one' }, + spec: {}, + }; + const entity2: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'two' }, + spec: { + test: 'test value', + }, + }; + await addEntityToSearch(knex, entity1); + await addEntityToSearch(knex, entity2); + const catalog = new NextEntitiesCatalog(knex); + + const testFilter = { + key: 'spec.test', + matchValueExists: true, + }; + const request = { + filter: { anyOf: [{ allOf: [testFilter] }] }, + }; + const { entities } = await catalog.entities(request); + + expect(entities.length).toBe(1); + expect(entities[0]).toEqual(entity2); + }, + ); + + it.each(databases.eachSupportedId())( + 'should return correct entity for nested filter', + async databaseId => { + const { knex } = await createDatabase(databaseId); + const entity1: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'one', org: 'a', desc: 'description' }, + spec: {}, + }; + const entity2: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'two', org: 'b', desc: 'description' }, + spec: {}, + }; + const entity3: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'three', org: 'b', color: 'red' }, + spec: {}, + }; + const entity4: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'four', org: 'b', color: 'blue' }, + spec: {}, + }; + await addEntityToSearch(knex, entity1); + await addEntityToSearch(knex, entity2); + await addEntityToSearch(knex, entity3); + await addEntityToSearch(knex, entity4); + const catalog = new NextEntitiesCatalog(knex); + + const testFilter1 = { + key: 'metadata.org', + matchValueExists: true, + matchValueIn: ['b'], + }; + const testFilter2 = { + key: 'metadata.desc', + matchValueExists: true, + }; + const testFilter3 = { + key: 'metadata.color', + matchValueExists: true, + matchValueIn: ['blue'], + }; + const request = { + filter: { + anyOf: [ + { + allOf: [ + testFilter1, + { + anyOf: [{ allOf: [testFilter2] }, { allOf: [testFilter3] }], + }, + ], + }, + ], + }, + }; + const { entities } = await catalog.entities(request); + + expect(entities.length).toBe(2); + expect(entities).toContainEqual(entity2); + expect(entities).toContainEqual(entity4); + }, + ); + }); }); diff --git a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts index 0891be045d..52d6ed1bdd 100644 --- a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts @@ -23,6 +23,8 @@ import { EntitiesResponse, EntityAncestryResponse, EntityPagination, + EntityFilter, + EntitiesSearchFilter, } from '../catalog/types'; import { DbFinalEntitiesRow, @@ -73,6 +75,64 @@ function stringifyPagination(input: { limit: number; offset: number }) { return base64; } +function addCondition( + queryBuilder: Knex.QueryBuilder, + db: Knex, + { key, matchValueIn, matchValueExists }: EntitiesSearchFilter, +) { + // NOTE(freben): This used to be a set of OUTER JOIN, which may seem to + // 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('entity_id') + .where(function keyFilter() { + this.andWhere({ key: key.toLowerCase() }); + if (matchValueExists !== false && matchValueIn) { + if (matchValueIn.length === 1) { + this.andWhere({ value: matchValueIn[0].toLowerCase() }); + } else if (matchValueIn.length > 1) { + this.andWhere( + 'value', + 'in', + matchValueIn.map(v => v.toLowerCase()), + ); + } + } + }); + // Explicitly evaluate matchValueExists as a boolean since it may be undefined + queryBuilder.andWhere( + 'entity_id', + matchValueExists === false ? 'not in' : 'in', + matchQuery, + ); +} + +function isEntityFilter( + filter: EntitiesSearchFilter | EntityFilter, +): filter is EntityFilter { + return filter.hasOwnProperty('anyOf'); +} + +function parseFilter( + filters: EntityFilter, + query: Knex.QueryBuilder, + db: Knex, +): Knex.QueryBuilder { + let cumulativeQuery = query; + for (const singleFilter of filters?.anyOf ?? []) { + cumulativeQuery = cumulativeQuery.orWhere(function singleFilterFn() { + for (const filter of singleFilter.allOf) { + if (isEntityFilter(filter)) { + this.andWhere(subQuery => parseFilter(filter, subQuery, db)); + } else { + addCondition(this, db, filter); + } + } + }); + } + return cumulativeQuery; +} + export class NextEntitiesCatalog implements EntitiesCatalog { constructor(private readonly database: Knex) {} @@ -80,41 +140,8 @@ export class NextEntitiesCatalog implements EntitiesCatalog { const db = this.database; let entitiesQuery = db('final_entities'); - - for (const singleFilter of request?.filter?.anyOf ?? []) { - entitiesQuery = entitiesQuery.orWhere(function singleFilterFn() { - for (const { - key, - matchValueIn, - matchValueExists, - } of singleFilter.allOf) { - // NOTE(freben): This used to be a set of OUTER JOIN, which may seem to - // 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('entity_id') - .where(function keyFilter() { - this.andWhere({ key: key.toLowerCase() }); - if (matchValueExists !== false && matchValueIn) { - if (matchValueIn.length === 1) { - this.andWhere({ value: matchValueIn[0].toLowerCase() }); - } else if (matchValueIn.length > 1) { - this.andWhere( - 'value', - 'in', - matchValueIn.map(v => v.toLowerCase()), - ); - } - } - }); - // Explicitly evaluate matchValueExists as a boolean since it may be undefined - this.andWhere( - 'entity_id', - matchValueExists === false ? 'not in' : 'in', - matchQuery, - ); - } - }); + if (request?.filter) { + entitiesQuery = parseFilter(request.filter, entitiesQuery, db); } // TODO: move final_entities to use entity_ref