diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts index f3db4c8dc6..00898b33a6 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts @@ -17,6 +17,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import type { Entity } from '@backstage/catalog-model'; import { Database, DatabaseManager, Transaction } from '../database'; +import { EntityFilters } from '../service/EntityFilters'; import { DatabaseEntitiesCatalog } from './DatabaseEntitiesCatalog'; import { EntityUpsertRequest } from './types'; @@ -73,13 +74,12 @@ describe('DatabaseEntitiesCatalog', () => { ]); expect(db.entities).toHaveBeenCalledTimes(1); - expect(db.entities).toHaveBeenCalledWith(expect.anything(), [ - { - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': ['c'], - }, - ]); + expect(db.entities).toHaveBeenCalledWith( + expect.anything(), + EntityFilters.ofFilterString( + 'kind=b,metadata.namespace=d,metadata.name=c', + ), + ); expect(db.setRelations).toHaveBeenCalledTimes(1); expect(db.setRelations).toHaveBeenCalledWith(expect.anything(), 'u', []); expect(db.addEntities).toHaveBeenCalledTimes(1); @@ -107,13 +107,12 @@ describe('DatabaseEntitiesCatalog', () => { ); expect(db.entities).toHaveBeenCalledTimes(1); - expect(db.entities).toHaveBeenCalledWith(expect.anything(), [ - { - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': ['c'], - }, - ]); + expect(db.entities).toHaveBeenCalledWith( + expect.anything(), + EntityFilters.ofFilterString( + 'kind=b,metadata.namespace=d,metadata.name=c', + ), + ); expect(db.setRelations).toHaveBeenCalledTimes(1); expect(db.setRelations).toHaveBeenCalledWith(expect.anything(), 'u', []); expect(db.addEntities).toHaveBeenCalledTimes(1); @@ -204,13 +203,12 @@ describe('DatabaseEntitiesCatalog', () => { ]); expect(db.entities).toHaveBeenCalledTimes(1); - expect(db.entities).toHaveBeenCalledWith(expect.anything(), [ - { - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': ['c'], - }, - ]); + expect(db.entities).toHaveBeenCalledWith( + expect.anything(), + EntityFilters.ofFilterString( + 'kind=b,metadata.namespace=d,metadata.name=c', + ), + ); expect(db.entityByName).not.toHaveBeenCalled(); expect(db.entityByUid).toHaveBeenCalledTimes(1); expect(db.entityByUid).toHaveBeenCalledWith(transaction, 'u'); @@ -280,13 +278,12 @@ describe('DatabaseEntitiesCatalog', () => { ]); expect(db.entities).toHaveBeenCalledTimes(1); - expect(db.entities).toHaveBeenCalledWith(expect.anything(), [ - { - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': ['c'], - }, - ]); + expect(db.entities).toHaveBeenCalledWith( + expect.anything(), + EntityFilters.ofFilterString( + 'kind=b,metadata.namespace=d,metadata.name=c', + ), + ); expect(db.entityByName).toHaveBeenCalledTimes(1); expect(db.entityByName).toHaveBeenCalledWith(transaction, { kind: 'b', @@ -342,13 +339,12 @@ describe('DatabaseEntitiesCatalog', () => { ]); expect(db.entities).toHaveBeenCalledTimes(1); - expect(db.entities).toHaveBeenCalledWith(expect.anything(), [ - { - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': ['c'], - }, - ]); + expect(db.entities).toHaveBeenCalledWith( + expect.anything(), + EntityFilters.ofFilterString( + 'kind=b,metadata.namespace=d,metadata.name=c', + ), + ); expect(db.entityByName).not.toHaveBeenCalled(); expect(db.entityByUid).not.toHaveBeenCalled(); expect(db.updateEntity).not.toHaveBeenCalled(); diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index c97ee023e6..5c55a30a55 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -16,13 +16,13 @@ import { ConflictError, NotFoundError } from '@backstage/backend-common'; import { + Entity, entityHasChanges, + EntityRelationSpec, generateUpdatedEntity, getEntityName, LOCATION_ANNOTATION, serializeEntityRef, - Entity, - EntityRelationSpec, } from '@backstage/catalog-model'; import { chunk, groupBy } from 'lodash'; import limiterFactory from 'p-limit'; @@ -30,9 +30,10 @@ import { Logger } from 'winston'; import type { Database, DbEntityResponse, - EntityFilters, + EntityFilter, Transaction, } from '../database'; +import { EntityFilters } from '../service/EntityFilters'; import { durationText } from '../util/timing'; import type { EntitiesCatalog, @@ -65,9 +66,9 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { private readonly logger: Logger, ) {} - async entities(filters?: EntityFilters[]): Promise { + async entities(filter?: EntityFilter): Promise { const items = await this.database.transaction(tx => - this.database.entities(tx, filters), + this.database.entities(tx, filter), ); return items.map(i => i.entity); } @@ -113,9 +114,12 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { const location = entityResponse.entity.metadata.annotations?.[LOCATION_ANNOTATION]; const colocatedEntities = location - ? await this.database.entities(tx, [ - { [`metadata.annotations.${LOCATION_ANNOTATION}`]: location }, - ]) + ? await this.database.entities( + tx, + EntityFilters.ofMatchers({ + [`metadata.annotations.${LOCATION_ANNOTATION}`]: location, + }), + ) : [entityResponse]; for (const dbResponse of colocatedEntities) { await this.database.removeEntityByUid( @@ -219,9 +223,12 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { } if (options?.outputEntities) { - const writtenEntities = await this.database.entities(tx, [ - { 'metadata.uid': modifiedEntityIds.map(e => e.entityId) }, - ]); + const writtenEntities = await this.database.entities( + tx, + EntityFilters.ofMatchers({ + 'metadata.uid': modifiedEntityIds.map(e => e.entityId), + }), + ); modifiedEntityIds = writtenEntities.map(e => ({ entityId: e.entity.metadata.uid!, @@ -241,7 +248,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { // If this is only a dry run, cancel the database transaction even if it was successful. await tx.rollback(); - this.logger.debug(`Perfomed successful dry run of adding entities`); + this.logger.debug(`Performed successful dry run of adding entities`); } return entityUpserts; @@ -273,13 +280,14 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { const markTimestamp = process.hrtime(); const names = requests.map(({ entity }) => entity.metadata.name); - const oldEntities = await this.database.entities(tx, [ - { + const oldEntities = await this.database.entities( + tx, + EntityFilters.ofMatchers({ kind: kind, 'metadata.namespace': namespace, 'metadata.name': names, - }, - ]); + }), + ); const oldEntitiesByName = new Map( oldEntities.map(e => [e.entity.metadata.name, e.entity]), diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index edc6ed2772..52bb6e95ab 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -15,7 +15,7 @@ */ import { Entity, EntityRelationSpec, Location } from '@backstage/catalog-model'; -import type { EntityFilters } from '../database'; +import type { EntityFilter } from '../database'; // // Entities @@ -32,7 +32,7 @@ export type EntityUpsertResponse = { }; export type EntitiesCatalog = { - entities(filters?: EntityFilters[]): Promise; + entities(filter?: EntityFilter): Promise; removeEntityByUid(uid: string): Promise; /** diff --git a/plugins/catalog-backend/src/database/CommonDatabase.test.ts b/plugins/catalog-backend/src/database/CommonDatabase.test.ts index b2a238b111..ffffd85f04 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.test.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.test.ts @@ -16,6 +16,7 @@ import { ConflictError } from '@backstage/backend-common'; import { Entity, Location, parseEntityRef } from '@backstage/catalog-model'; +import { EntityFilters } from '../service/EntityFilters'; import { DatabaseManager } from './DatabaseManager'; import type { DbEntityRequest, @@ -348,7 +349,7 @@ describe('CommonDatabase', () => { await db.transaction(async tx => { await db.addEntities(tx, [{ entity: e1 }, { entity: e2 }]); }); - const result = await db.transaction(async tx => db.entities(tx, [])); + const result = await db.transaction(async tx => db.entities(tx)); expect(result.length).toEqual(2); expect(result).toEqual( expect.arrayContaining([ @@ -390,7 +391,7 @@ describe('CommonDatabase', () => { await expect( db.transaction(async tx => - db.entities(tx, [{ kind: 'k2', 'spec.c': 'some' }]), + db.entities(tx, EntityFilters.ofFilterString('kind=k2,spec.c=some')), ), ).resolves.toEqual([ { @@ -400,56 +401,14 @@ describe('CommonDatabase', () => { ]); }); - it('can get all specific entities for matching filters with nulls (both missing and literal null value)', async () => { + it('can get all specific entities for matching filters case insensitively', async () => { const entities: Entity[] = [ - { apiVersion: 'a', kind: 'k1', metadata: { name: 'n' } }, { - apiVersion: 'a', - kind: 'k2', - metadata: { name: 'n' }, - spec: { c: 'some' }, + apiVersion: 'A', + kind: 'K1', + metadata: { name: 'N' }, + spec: { c: 'SOME' }, }, - { - apiVersion: 'a', - kind: 'k3', - metadata: { name: 'n' }, - spec: { c: null }, - }, - ]; - - await db.transaction(async tx => { - await db.addEntities( - tx, - entities.map(entity => ({ entity })), - ); - }); - - const rows = await db.transaction(async tx => - db.entities(tx, [{ apiVersion: 'a', 'spec.c': [null, 'some'] }]), - ); - - expect(rows.length).toEqual(3); - expect(rows).toEqual( - expect.arrayContaining([ - { - locationId: undefined, - entity: expect.objectContaining({ kind: 'k1' }), - }, - { - locationId: undefined, - entity: expect.objectContaining({ kind: 'k2' }), - }, - { - locationId: undefined, - entity: expect.objectContaining({ kind: 'k3' }), - }, - ]), - ); - }); - - it('can get all specific entities for matching filters case insensitively)', async () => { - const entities: Entity[] = [ - { apiVersion: 'A', kind: 'K1', metadata: { name: 'N' } }, { apiVersion: 'a', kind: 'k2', @@ -460,7 +419,7 @@ describe('CommonDatabase', () => { apiVersion: 'a', kind: 'k3', metadata: { name: 'n' }, - spec: { c: null }, + spec: { c: 'somE' }, }, ]; @@ -472,7 +431,10 @@ describe('CommonDatabase', () => { }); const rows = await db.transaction(async tx => - db.entities(tx, [{ ApiVersioN: 'A', 'spEc.C': [null, 'some'] }]), + db.entities( + tx, + EntityFilters.ofFilterString('ApiVersioN=A,spEc.C=some'), + ), ); expect(rows.length).toEqual(3); diff --git a/plugins/catalog-backend/src/database/CommonDatabase.ts b/plugins/catalog-backend/src/database/CommonDatabase.ts index bd6dc29abc..f86ab9e598 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.ts @@ -45,7 +45,7 @@ import { DbEntityResponse, DbLocationsRow, DbLocationsRowWithStatus, - EntityFilters, + EntityFilter, Transaction, } from './types'; @@ -233,71 +233,36 @@ export class CommonDatabase implements Database { async entities( txOpaque: Transaction, - filters?: EntityFilters[], + filter?: EntityFilter, ): Promise { const tx = txOpaque as Knex.Transaction; let entitiesQuery = tx('entities'); - for (const singleFilter of filters ?? []) { + for (const singleFilter of filter?.anyOf ?? []) { entitiesQuery = entitiesQuery.orWhere(function singleFilterFn() { - for (const [matchKey, matchVal] of Object.entries(singleFilter)) { - const key = matchKey.toLowerCase().replace(/[*]/g, '%'); - const keyOp = key.includes('%') ? 'like' : '='; - const values = Array.isArray(matchVal) ? matchVal : [matchVal]; - - let matchNulls = false; - const matchIn: string[] = []; - const matchLike: string[] = []; - - for (const value of values) { - if (!value) { - matchNulls = true; - } else if (value.includes('*')) { - matchLike.push(value.toLowerCase().replace(/[*]/g, '%')); - } else { - matchIn.push(value.toLowerCase()); - } - } - + for (const { key, matchValueIn } 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 = tx('entities_search') .select('entity_id') .where(function keyFilter() { - this.andWhere('key', keyOp, key); - this.andWhere(function valueFilter() { - if (matchIn.length === 1) { - this.orWhere({ value: matchIn[0] }); - } else if (matchIn.length > 1) { - this.orWhereIn('value', matchIn); + this.andWhere({ key: key.toLowerCase() }); + if (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()), + ); } - if (matchLike.length) { - for (const x of matchLike) { - this.orWhere('value', 'like', tx.raw('?', [x])); - } - } - if (matchNulls) { - // Match explicit nulls, and then handle absence separately - // below - this.orWhereNull('value'); - } - }); + } }); - // Handle absence as nulls as well - this.andWhere(function match() { - this.whereIn('id', matchQuery); - if (matchNulls) { - this.orWhereNotIn( - 'id', - tx('entities_search') - .select('entity_id') - .where('key', keyOp, key), - ); - } - }); + this.andWhere('id', 'in', matchQuery); } }); } diff --git a/plugins/catalog-backend/src/database/index.ts b/plugins/catalog-backend/src/database/index.ts index 0d84106815..c2cc07788a 100644 --- a/plugins/catalog-backend/src/database/index.ts +++ b/plugins/catalog-backend/src/database/index.ts @@ -20,7 +20,7 @@ export type { Database, DbEntityRequest, DbEntityResponse, + EntitiesSearchFilter, EntityFilter, - EntityFilters, Transaction, } from './types'; diff --git a/plugins/catalog-backend/src/database/types.ts b/plugins/catalog-backend/src/database/types.ts index 62b2d55b88..1307b063f2 100644 --- a/plugins/catalog-backend/src/database/types.ts +++ b/plugins/catalog-backend/src/database/types.ts @@ -17,8 +17,8 @@ import type { Entity, EntityName, - Location, EntityRelationSpec, + Location, } from '@backstage/catalog-model'; export type DbEntitiesRow = { @@ -80,26 +80,35 @@ export type DatabaseLocationUpdateLogEvent = { }; /** - * Filter matcher for a single entity field. - * - * Can be either null or a string, or an array of those. Null and the empty - * string are treated equally, and match both a present field with a null or - * empty value, as well as an absent field. - * - * A filter may contain asterisks (*) that are treated as wildcards for zero - * or more arbitrary characters. + * Matches rows in the entities_search table. */ -export type EntityFilter = null | string | (null | string)[]; +export type EntitiesSearchFilter = { + /** + * The key to match on. + * + * Matches are always case insensitive. + */ + key: string; + + /** + * Match on plain equality of values. + * + * If undefined, this factor is not taken into account. Otherwise, match on + * values that are equal to any of the given array items. Matches are always + * case insensitive. + */ + matchValueIn?: string[]; +}; /** - * A set of filter matchers used for filtering entities. + * A filter expression for entities. * - * The keys are full dot-separated paths into the structure of an entity, for - * example "metadata.name". You can also address any item in an array the same - * way, e.g. "a.b.c": "x" works if b is an array of objects that have a c field - * and any of those have the value x. + * Any (at least one) of the outer sets must match, within which all of the + * individual filters must match. */ -export type EntityFilters = Record; +export type EntityFilter = { + anyOf: { allOf: EntitiesSearchFilter[] }[]; +}; /** * An abstraction for transactions of the underlying database technology. @@ -160,10 +169,7 @@ export type Database = { matchingGeneration?: number, ): Promise; - entities( - tx: Transaction, - filters?: EntityFilters[], - ): Promise; + entities(tx: Transaction, filter?: EntityFilter): Promise; entityByName( tx: Transaction, diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts index 902e26e0e3..f352e8d3fd 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts @@ -18,7 +18,7 @@ import { Location, LocationSpec } from '@backstage/catalog-model'; import { v4 as uuidv4 } from 'uuid'; import { Logger } from 'winston'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; -import { durationText } from '../util/timing'; +import { durationText } from '../util'; import { AddLocationResult, HigherOrderOperation, diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts index 74c02e3f84..2a63b20619 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts +++ b/plugins/catalog-backend/src/ingestion/processors/ldap/config.ts @@ -17,7 +17,7 @@ import { Config, JsonValue } from '@backstage/config'; import { SearchOptions } from 'ldapjs'; import mergeWith from 'lodash/mergeWith'; -import { RecursivePartial } from './util'; +import { RecursivePartial } from '../../../util'; /** * The configuration parameters for a single LDAP provider. diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/read.test.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/read.test.ts index e7a7d1c567..e644bab22a 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/read.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/ldap/read.test.ts @@ -17,6 +17,7 @@ import { GroupEntity, UserEntity } from '@backstage/catalog-model'; import { SearchEntry } from 'ldapjs'; import merge from 'lodash/merge'; +import { RecursivePartial } from '../../../util'; import { LdapClient } from './client'; import { GroupConfig, UserConfig } from './config'; import { @@ -25,7 +26,6 @@ import { LDAP_UUID_ANNOTATION, } from './constants'; import { readLdapGroups, readLdapUsers, resolveRelations } from './read'; -import { RecursivePartial } from './util'; function user(data: RecursivePartial): UserEntity { return merge( diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/util.test.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/util.test.ts index 2def6ad960..63c7253411 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/util.test.ts +++ b/plugins/catalog-backend/src/ingestion/processors/ldap/util.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { errorString, RecursivePartial } from './util'; +import { errorString } from './util'; describe('errorString', () => { it('formats', () => { @@ -22,17 +22,3 @@ describe('errorString', () => { expect(errorString(e)).toEqual('1 n: m'); }); }); - -describe('RecursivePartial', () => { - it('is recursive', () => { - type X = { - required: { - required: string; - }; - }; - const x: RecursivePartial = { - required: {}, - }; - expect(x).toEqual({ required: {} }); - }); -}); diff --git a/plugins/catalog-backend/src/ingestion/processors/ldap/util.ts b/plugins/catalog-backend/src/ingestion/processors/ldap/util.ts index 29159dc219..f5751b1e9f 100644 --- a/plugins/catalog-backend/src/ingestion/processors/ldap/util.ts +++ b/plugins/catalog-backend/src/ingestion/processors/ldap/util.ts @@ -24,14 +24,3 @@ import { Error as LDAPError } from 'ldapjs'; export function errorString(error: LDAPError) { return `${error.code} ${error.name}: ${error.message}`; } - -/** - * Makes all keys of an entire hierarchy optional. - */ -export type RecursivePartial = { - [P in keyof T]?: T[P] extends (infer U)[] - ? RecursivePartial[] - : T[P] extends object - ? RecursivePartial - : T[P]; -}; diff --git a/plugins/catalog-backend/src/service/EntityFilters.test.ts b/plugins/catalog-backend/src/service/EntityFilters.test.ts new file mode 100644 index 0000000000..bfc20c625a --- /dev/null +++ b/plugins/catalog-backend/src/service/EntityFilters.test.ts @@ -0,0 +1,113 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { EntityFilters } from './EntityFilters'; + +describe('EntityFilters', () => { + describe('ofQuery', () => { + it('translates empty query to empty list', () => { + const result = EntityFilters.ofQuery({}); + expect(result).toEqual(undefined); + }); + + it('supports single-string format', () => { + const result = EntityFilters.ofQuery({ filter: 'a=1' })!; + expect(result).toEqual({ + anyOf: [{ allOf: [{ key: 'a', matchValueIn: ['1'] }] }], + }); + }); + + it('supports array-of-strings format', () => { + const result = EntityFilters.ofQuery({ filter: ['a=1', 'b=2'] }); + expect(result).toEqual({ + anyOf: [ + { allOf: [{ key: 'a', matchValueIn: ['1'] }] }, + { allOf: [{ key: 'b', matchValueIn: ['2'] }] }, + ], + }); + }); + + it('throws for non-strings', () => { + expect(() => EntityFilters.ofQuery({ filter: [3] })).toThrow(/string/); + }); + }); + + describe('ofFilterString', () => { + it('runs the happy path', () => { + const result = EntityFilters.ofFilterString('a=1,b=2'); + expect(result).toEqual({ + anyOf: [ + { + allOf: [ + { key: 'a', matchValueIn: ['1'] }, + { key: 'b', matchValueIn: ['2'] }, + ], + }, + ], + }); + }); + + it('ignores empty', () => { + const result = EntityFilters.ofFilterString('a=1,,b=2,'); + expect(result).toEqual({ + anyOf: [ + { + allOf: [ + { key: 'a', matchValueIn: ['1'] }, + { key: 'b', matchValueIn: ['2'] }, + ], + }, + ], + }); + }); + + it('trims', () => { + const result = EntityFilters.ofFilterString(' a = 1 ,, b=2 ,'); + expect(result).toEqual({ + anyOf: [ + { + allOf: [ + { key: 'a', matchValueIn: ['1'] }, + { key: 'b', matchValueIn: ['2'] }, + ], + }, + ], + }); + }); + + it('merges multiple of the same key', () => { + const result = EntityFilters.ofFilterString('a=1,a=2,b=3'); + expect(result).toEqual({ + anyOf: [ + { + allOf: [ + { key: 'a', matchValueIn: ['1', '2'] }, + { key: 'b', matchValueIn: ['3'] }, + ], + }, + ], + }); + }); + + it('throws on missing equal sign', () => { + expect(() => EntityFilters.ofFilterString('a,b=2')).toThrow(); + }); + + it('throws on misplaced equal sign', () => { + expect(() => EntityFilters.ofFilterString('a=,b=2')).toThrow(); + }); + }); +}); diff --git a/plugins/catalog-backend/src/service/EntityFilters.ts b/plugins/catalog-backend/src/service/EntityFilters.ts new file mode 100644 index 0000000000..5a93407424 --- /dev/null +++ b/plugins/catalog-backend/src/service/EntityFilters.ts @@ -0,0 +1,120 @@ +/* + * Copyright 2020 Spotify AB + * + * 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/backend-common'; +import { EntitiesSearchFilter, EntityFilter } from '../database'; + +/** + * A builder that assists in creating entity filter instances. + */ +export class EntityFilters { + // Builds a filter from the value of a filter=a=1,b=2 type filter string + static ofFilterString(filterString: string): EntityFilter | undefined { + const builder = new EntityFilters(); + builder.addFilterString(filterString); + return builder.build(); + } + + // Builds a filter from an entire query params structure + static ofQuery(query: Record): EntityFilter | undefined { + const filterStrings = [query.filter || []].flat(); + + if (filterStrings.some(f => typeof f !== 'string')) { + throw new InputError( + 'Only string type filter query parameters are supported', + ); + } + + const builder = new EntityFilters(); + for (const filterString of filterStrings) { + builder.addFilterString(filterString); + } + + return builder.build(); + } + + // Builds a filter from a { key: value } matcher object + static ofMatchers( + matchers: Record, + ): EntityFilter | undefined { + const builder = new EntityFilters(); + builder.addMatchers(matchers); + return builder.build(); + } + + private filters: EntitiesSearchFilter[][]; + + constructor() { + this.filters = []; + } + + addFilterString(filterString: string): EntityFilters { + const filtersByKey: Record = {}; + + const addFilter = (key: string, value: string) => { + const f = + key in filtersByKey + ? filtersByKey[key] + : (filtersByKey[key] = { key, matchValueIn: [] }); + f.matchValueIn!.push(value); + }; + + const statements = filterString + .split(',') + .map(s => s.trim()) + .filter(Boolean); + + for (const statement of statements) { + const equalsIndex = statement.indexOf('='); + if (equalsIndex < 1) { + throw new InputError('Malformed filter query'); + } else { + const key = statement.substr(0, equalsIndex).trim(); + const value = statement.substr(equalsIndex + 1).trim(); + if (!key || !value) { + throw new InputError('Malformed filter query'); + } + addFilter(key, value); + } + } + + this.filters.push(Object.values(filtersByKey)); + + return this; + } + + addMatchers(matchers: Record): EntityFilters { + const filters: EntitiesSearchFilter[] = []; + + for (const [key, value] of Object.entries(matchers)) { + filters.push({ key, matchValueIn: [value].flat() }); + } + + if (filters.length) { + this.filters.push(filters); + } + + return this; + } + + build(): EntityFilter | undefined { + if (!this.filters.length) { + return undefined; + } + + return { anyOf: this.filters.map(f => ({ allOf: f })) }; + } +} diff --git a/plugins/catalog-backend/src/service/filterQuery.test.ts b/plugins/catalog-backend/src/service/filterQuery.test.ts index bfb58568b5..a4b3d9c671 100644 --- a/plugins/catalog-backend/src/service/filterQuery.test.ts +++ b/plugins/catalog-backend/src/service/filterQuery.test.ts @@ -15,66 +15,7 @@ */ import { Entity } from '@backstage/catalog-model'; -import { - translateFilterQueryEntryToEntityFilters, - translateQueryToEntityFilters, - translateQueryToFieldMapper, -} from './filterQuery'; - -describe('translateQueryToEntityFilters', () => { - it('translates empty query to empty list', () => { - const result = translateQueryToEntityFilters({}); - expect(result).toEqual([]); - }); - - it('supports single-string format', () => { - const result = translateQueryToEntityFilters({ filter: 'a=1' }); - expect(result).toEqual([{ a: ['1'] }]); - }); - - it('supports array-of-strings format', () => { - const result = translateQueryToEntityFilters({ filter: ['a=1', 'b=2'] }); - expect(result).toEqual([{ a: ['1'] }, { b: ['2'] }]); - }); - - it('throws for non-strings', () => { - expect(() => translateQueryToEntityFilters({ filter: [3] })).toThrow( - /string/, - ); - }); -}); - -describe('translateFilterQueryEntryToEntityFilters', () => { - it('runs the happy path', () => { - const result = translateFilterQueryEntryToEntityFilters('a=1,b=2'); - expect(result).toEqual({ a: ['1'], b: ['2'] }); - }); - - it('ignores empty', () => { - const result = translateFilterQueryEntryToEntityFilters('a=1,,b=2,'); - expect(result).toEqual({ a: ['1'], b: ['2'] }); - }); - - it('trims', () => { - const result = translateFilterQueryEntryToEntityFilters(' a = 1 ,, b=2 ,'); - expect(result).toEqual({ a: ['1'], b: ['2'] }); - }); - - it('merges multiple of the same key', () => { - const result = translateFilterQueryEntryToEntityFilters('a=1,a=2,b=3'); - expect(result).toEqual({ a: ['1', '2'], b: ['3'] }); - }); - - it('treats missing equal signs as presence', () => { - const result = translateFilterQueryEntryToEntityFilters('a,b=2'); - expect(result).toEqual({ a: ['*'], b: ['2'] }); - }); - - it('treats empty value as null/absence', () => { - const result = translateFilterQueryEntryToEntityFilters('a=,b=2'); - expect(result).toEqual({ a: [null], b: ['2'] }); - }); -}); +import { translateQueryToFieldMapper } from './filterQuery'; describe('translateQueryToFieldMapper', () => { const entity: Entity = { diff --git a/plugins/catalog-backend/src/service/filterQuery.ts b/plugins/catalog-backend/src/service/filterQuery.ts index 4ab471ffcd..e9b6ef6062 100644 --- a/plugins/catalog-backend/src/service/filterQuery.ts +++ b/plugins/catalog-backend/src/service/filterQuery.ts @@ -17,62 +17,7 @@ import { InputError } from '@backstage/backend-common'; import { Entity } from '@backstage/catalog-model'; import lodash from 'lodash'; -import { EntityFilters } from '../database'; -import { RecursivePartial } from '../ingestion/processors/ldap/util'; - -export function translateQueryToEntityFilters( - query: Record, -): EntityFilters[] { - if (!query.filter) { - return []; - } - - const filterStrings = [query.filter].flat(); - - if (filterStrings.some(s => typeof s !== 'string')) { - throw new InputError( - 'Only string type filter query parameters are supported', - ); - } - - return filterStrings - .filter(Boolean) - .map(translateFilterQueryEntryToEntityFilters); -} - -// Parses the value of a filter=a=1,b=2 type query param -export function translateFilterQueryEntryToEntityFilters( - filterString: string, -): EntityFilters { - const filters: Record = {}; - - const addFilter = (key: string, value: string | null) => { - const matchers = key in filters ? filters[key] : (filters[key] = []); - matchers.push(value || null); - }; - - const statements = filterString - .split(',') - .map(s => s.trim()) - .filter(Boolean); - - for (const statement of statements) { - const equalsIndex = statement.indexOf('='); - if (equalsIndex < 0) { - // Check presence, any value - addFilter(statement, '*'); - } else { - const key = statement.substr(0, equalsIndex).trim(); - const value = statement.substr(equalsIndex + 1).trim(); - if (!key) { - throw new InputError('Malformed filter query'); - } - addFilter(key, value); - } - } - - return filters; -} +import { RecursivePartial } from '../util'; type FieldMapper = (entity: Entity) => Entity; diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index 1cf64945ee..cef40a9aac 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -21,6 +21,7 @@ import request from 'supertest'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; import { LocationResponse } from '../catalog/types'; import { HigherOrderOperation } from '../ingestion/types'; +import { EntityFilters } from './EntityFilters'; import { createRouter } from './router'; describe('createRouter', () => { @@ -78,18 +79,22 @@ describe('createRouter', () => { it('parses single and multiple request parameters and passes them down', async () => { entitiesCatalog.entities.mockResolvedValueOnce([]); const response = await request(app).get( - '/entities?filter=a=1,a=,a=3,b=4&filter=c=', + '/entities?filter=a=1,a=2,b=3&filter=c=4', ); expect(response.status).toEqual(200); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith([ - { - a: ['1', null, '3'], - b: ['4'], - }, - { c: [null] }, - ]); + expect(entitiesCatalog.entities).toHaveBeenCalledWith({ + anyOf: [ + { + allOf: [ + { key: 'a', matchValueIn: ['1', '2'] }, + { key: 'b', matchValueIn: ['3'] }, + ], + }, + { allOf: [{ key: 'c', matchValueIn: ['4'] }] }, + ], + }); }); }); @@ -107,9 +112,9 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-uid/zzz'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith([ - { 'metadata.uid': 'zzz' }, - ]); + expect(entitiesCatalog.entities).toHaveBeenCalledWith( + EntityFilters.ofMatchers({ 'metadata.uid': 'zzz' }), + ); expect(response.status).toEqual(200); expect(response.body).toEqual(expect.objectContaining(entity)); }); @@ -120,9 +125,9 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-uid/zzz'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith([ - { 'metadata.uid': 'zzz' }, - ]); + expect(entitiesCatalog.entities).toHaveBeenCalledWith( + EntityFilters.ofMatchers({ 'metadata.uid': 'zzz' }), + ); expect(response.status).toEqual(404); expect(response.text).toMatch(/uid/); }); @@ -143,13 +148,13 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-name/k/ns/n'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith([ - { + expect(entitiesCatalog.entities).toHaveBeenCalledWith( + EntityFilters.ofMatchers({ kind: 'k', 'metadata.namespace': 'ns', 'metadata.name': 'n', - }, - ]); + }), + ); expect(response.status).toEqual(200); expect(response.body).toEqual(expect.objectContaining(entity)); }); @@ -160,13 +165,13 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-name/b/d/c'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith([ - { + expect(entitiesCatalog.entities).toHaveBeenCalledWith( + EntityFilters.ofMatchers({ kind: 'b', 'metadata.namespace': 'd', 'metadata.name': 'c', - }, - ]); + }), + ); expect(response.status).toEqual(404); expect(response.text).toMatch(/name/); }); @@ -209,9 +214,9 @@ describe('createRouter', () => { { entity, relations: [] }, ]); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith([ - { 'metadata.uid': 'u' }, - ]); + expect(entitiesCatalog.entities).toHaveBeenCalledWith( + EntityFilters.ofMatchers({ 'metadata.uid': 'u' }), + ); expect(response.status).toEqual(200); expect(response.body).toEqual(entity); }); diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index 96b343a184..ebad544bef 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -23,10 +23,8 @@ import { Logger } from 'winston'; import yn from 'yn'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; import { HigherOrderOperation } from '../ingestion/types'; -import { - translateQueryToEntityFilters, - translateQueryToFieldMapper, -} from './filterQuery'; +import { EntityFilters } from './EntityFilters'; +import { translateQueryToFieldMapper } from './filterQuery'; import { requireRequestBody, validateRequestBody } from './util'; export interface RouterOptions { @@ -47,9 +45,9 @@ export async function createRouter( if (entitiesCatalog) { router .get('/entities', async (req, res) => { - const filters = translateQueryToEntityFilters(req.query); + const filter = EntityFilters.ofQuery(req.query); const fieldMapper = translateQueryToFieldMapper(req.query); - const entities = await entitiesCatalog.entities(filters); + const entities = await entitiesCatalog.entities(filter); res.status(200).send(entities.map(fieldMapper)); }) .post('/entities', async (req, res) => { @@ -57,18 +55,16 @@ export async function createRouter( const [result] = await entitiesCatalog.batchAddOrUpdateEntities([ { entity: body as Entity, relations: [] }, ]); - const [entity] = await entitiesCatalog.entities([ - { 'metadata.uid': result.entityId }, - ]); + const [entity] = await entitiesCatalog.entities( + EntityFilters.ofMatchers({ 'metadata.uid': result.entityId }), + ); res.status(200).send(entity); }) .get('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; - const entities = await entitiesCatalog.entities([ - { - 'metadata.uid': uid, - }, - ]); + const entities = await entitiesCatalog.entities( + EntityFilters.ofMatchers({ 'metadata.uid': uid }), + ); if (!entities.length) { res.status(404).send(`No entity with uid ${uid}`); } @@ -81,13 +77,13 @@ export async function createRouter( }) .get('/entities/by-name/:kind/:namespace/:name', async (req, res) => { const { kind, namespace, name } = req.params; - const entities = await entitiesCatalog.entities([ - { + const entities = await entitiesCatalog.entities( + EntityFilters.ofMatchers({ kind: kind, 'metadata.namespace': namespace, 'metadata.name': name, - }, - ]); + }), + ); if (!entities.length) { res .status(404) diff --git a/plugins/catalog-backend/src/util/RecursivePartial.test.ts b/plugins/catalog-backend/src/util/RecursivePartial.test.ts new file mode 100644 index 0000000000..7f16226188 --- /dev/null +++ b/plugins/catalog-backend/src/util/RecursivePartial.test.ts @@ -0,0 +1,31 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { RecursivePartial } from './RecursivePartial'; + +describe('RecursivePartial', () => { + it('is recursive', () => { + type X = { + required: { + required: string; + }; + }; + const x: RecursivePartial = { + required: {}, + }; + expect(x).toEqual({ required: {} }); + }); +}); diff --git a/plugins/catalog-backend/src/util/RecursivePartial.ts b/plugins/catalog-backend/src/util/RecursivePartial.ts new file mode 100644 index 0000000000..0fa468b4c4 --- /dev/null +++ b/plugins/catalog-backend/src/util/RecursivePartial.ts @@ -0,0 +1,26 @@ +/* + * Copyright 2020 Spotify AB + * + * 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. + */ + +/** + * Makes all keys of an entire hierarchy optional. + */ +export type RecursivePartial = { + [P in keyof T]?: T[P] extends (infer U)[] + ? RecursivePartial[] + : T[P] extends object + ? RecursivePartial + : T[P]; +}; diff --git a/plugins/catalog-backend/src/util/index.ts b/plugins/catalog-backend/src/util/index.ts index 6f0505d8c5..8364819f99 100644 --- a/plugins/catalog-backend/src/util/index.ts +++ b/plugins/catalog-backend/src/util/index.ts @@ -15,3 +15,5 @@ */ export * from './runPeriodically'; +export * from './RecursivePartial'; +export * from './timing';