diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index 2575411e7c..8619355c91 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -15,13 +15,12 @@ */ import { NotFoundError } from '@backstage/backend-common'; +import type { Entity } from '@backstage/catalog-model'; import { - EntityName, generateUpdatedEntity, getEntityName, LOCATION_ANNOTATION, } from '@backstage/catalog-model'; -import type { Entity } from '@backstage/catalog-model'; import type { Database, DbEntityResponse, EntityFilters } from '../database'; import type { EntitiesCatalog } from './types'; @@ -35,20 +34,6 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { return items.map(i => i.entity); } - async entityByUid(uid: string): Promise { - const response = await this.database.transaction(tx => - this.database.entityByUid(tx, uid), - ); - return response?.entity; - } - - async entityByName(name: EntityName): Promise { - const response = await this.database.transaction(tx => - this.database.entityByName(tx, name), - ); - return response?.entity; - } - async addOrUpdateEntity( entity: Entity, locationId?: string, @@ -100,12 +85,9 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { const location = entityResponse.entity.metadata.annotations?.[LOCATION_ANNOTATION]; const colocatedEntities = location - ? await this.database.entities(tx, [ - { - key: `metadata.annotations.${LOCATION_ANNOTATION}`, - values: [location], - }, - ]) + ? await this.database.entities(tx, { + [`metadata.annotations.${LOCATION_ANNOTATION}`]: location, + }) : [entityResponse]; for (const dbResponse of colocatedEntities) { await this.database.removeEntityByUid( diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 5d84cbcebc..810d51a6c1 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Entity, EntityName, Location } from '@backstage/catalog-model'; +import { Entity, Location } from '@backstage/catalog-model'; import type { EntityFilters } from '../database'; // @@ -23,8 +23,6 @@ import type { EntityFilters } from '../database'; export type EntitiesCatalog = { entities(filters?: EntityFilters): Promise; - entityByUid(uid: string): Promise; - entityByName(name: EntityName): Promise; addOrUpdateEntity(entity: Entity, locationId?: string): Promise; addEntities(entities: Entity[], locationId?: string): 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 840af74825..f3ca212df0 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.test.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.test.ts @@ -347,7 +347,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([ @@ -389,10 +389,7 @@ describe('CommonDatabase', () => { await expect( db.transaction(async tx => - db.entities(tx, [ - { key: 'kind', values: ['k2'] }, - { key: 'spec.c', values: ['some'] }, - ]), + db.entities(tx, { kind: 'k2', 'spec.c': 'some' }), ), ).resolves.toEqual([ { @@ -427,10 +424,7 @@ describe('CommonDatabase', () => { }); const rows = await db.transaction(async tx => - db.entities(tx, [ - { key: 'apiVersion', values: ['a'] }, - { key: 'spec.c', values: [null, 'some'] }, - ]), + db.entities(tx, { apiVersion: 'a', 'spec.c': [null, 'some'] }), ); expect(rows.length).toEqual(3); @@ -477,10 +471,7 @@ describe('CommonDatabase', () => { }); const rows = await db.transaction(async tx => - db.entities(tx, [ - { key: 'ApiVersioN', values: ['A'] }, - { key: 'spEc.C', values: [null, 'some'] }, - ]), + db.entities(tx, { ApiVersioN: 'A', 'spEc.C': [null, '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 49b36e0933..dca5072b30 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.ts @@ -219,15 +219,16 @@ export class CommonDatabase implements Database { let entitiesQuery = tx('entities'); - for (const filter of filters || []) { - const key = filter.key.toLowerCase().replace(/[*]/g, '%'); - const keyOp = filter.key.includes('*') ? 'like' : '='; + for (const [matchKey, matchVal] of Object.entries(filters ?? {})) { + 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 filter.values) { + for (const value of values) { if (!value) { matchNulls = true; } else if (value.includes('*')) { diff --git a/plugins/catalog-backend/src/database/search.test.ts b/plugins/catalog-backend/src/database/search.test.ts index d108820ca9..c6793cbdf4 100644 --- a/plugins/catalog-backend/src/database/search.test.ts +++ b/plugins/catalog-backend/src/database/search.test.ts @@ -24,8 +24,11 @@ describe('search', () => { const output = traverse(input); expect(output).toEqual([ { key: 'a', value: 'b' }, + { key: 'a.b', value: true }, { key: 'a', value: 'c' }, + { key: 'a.c', value: true }, { key: 'a', value: 'd' }, + { key: 'a.d', value: true }, ]); }); diff --git a/plugins/catalog-backend/src/database/search.ts b/plugins/catalog-backend/src/database/search.ts index 863b86ee9a..9e58d2468b 100644 --- a/plugins/catalog-backend/src/database/search.ts +++ b/plugins/catalog-backend/src/database/search.ts @@ -35,7 +35,7 @@ const MAX_VALUE_LENGTH = 200; type Kv = { key: string; - value: any; + value: unknown; }; // Helper for traversing through a nested structure and outputting a list of @@ -58,15 +58,17 @@ type Kv = { // "a", 1 // "b.c", null // "b.e": "f" +// "b.e.f": true // "b.e": "g" +// "b.e.g": true // "h.i": 1 // "h.j": "k" // "h.i": 2 // "h.j": "l" -export function traverse(root: any): Kv[] { +export function traverse(root: unknown): Kv[] { const output: Kv[] = []; - function visit(path: string, current: any) { + function visit(path: string, current: unknown) { if (SPECIAL_KEYS.includes(path)) { return; } @@ -89,14 +91,29 @@ export function traverse(root: any): Kv[] { // array if (Array.isArray(current)) { for (const item of current) { - // keep the same path as currently + // NOTE(freben): The reason that these are output in two different ways, + // is to support use cases where you want to express that MORE than one + // tag is present in a list. Since the EntityFilters structure is a + // record, you can't have several entries of the same key. Therefore + // you will have to match on + // + // { "a.b": ["true"], "a.c": ["true"] } + // + // rather than + // + // { "a": ["b", "c"] } + // + // because the latter means EITHER b or c has to be present. visit(path, item); + if (typeof item === 'string') { + output.push({ key: `${path}.${item}`, value: true }); + } } return; } // object - for (const [key, value] of Object.entries(current)) { + for (const [key, value] of Object.entries(current!)) { visit(path ? `${path}.${key}` : key, value); } } @@ -113,12 +130,12 @@ export function mapToRows( ): DbEntitiesSearchRow[] { const result: DbEntitiesSearchRow[] = []; - for (let { key, value } of input) { - key = key.toLowerCase(); - if (value === undefined || value === null) { + for (const { key: rawKey, value: rawValue } of input) { + const key = rawKey.toLowerCase(); + if (rawValue === undefined || rawValue === null) { result.push({ entity_id: entityId, key, value: null }); } else { - value = String(value).toLowerCase(); + const value = String(rawValue).toLowerCase(); if (value.length <= MAX_VALUE_LENGTH) { result.push({ entity_id: entityId, key, value }); } diff --git a/plugins/catalog-backend/src/database/types.ts b/plugins/catalog-backend/src/database/types.ts index 20cc123c0a..14e8d5be64 100644 --- a/plugins/catalog-backend/src/database/types.ts +++ b/plugins/catalog-backend/src/database/types.ts @@ -67,11 +67,27 @@ export type DatabaseLocationUpdateLogEvent = { message?: string; }; -export type EntityFilter = { - key: string; - values: (string | null)[]; -}; -export type EntityFilters = EntityFilter[]; +/** + * 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. + */ +export type EntityFilter = null | string | (null | string)[]; + +/** + * A set of filter matchers used for filtering 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. + */ +export type EntityFilters = Record; /** * An abstraction on top of the underlying database, wrapping the basic CRUD diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts index 15d9168732..909139d678 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts @@ -36,8 +36,6 @@ describe('HigherOrderOperations', () => { beforeAll(() => { entitiesCatalog = { entities: jest.fn(), - entityByUid: jest.fn(), - entityByName: jest.fn(), addOrUpdateEntity: jest.fn(), addEntities: jest.fn(), removeEntityByUid: jest.fn(), @@ -206,14 +204,11 @@ describe('HigherOrderOperations', () => { target: 'thing', }); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenNthCalledWith( - 1, - expect.arrayContaining([ - { key: 'kind', values: ['Component'] }, - { key: 'metadata.namespace', values: [ENTITY_DEFAULT_NAMESPACE] }, - { key: 'metadata.name', values: ['c1'] }, - ]), - ); + expect(entitiesCatalog.entities).toHaveBeenNthCalledWith(1, { + kind: 'Component', + 'metadata.namespace': ENTITY_DEFAULT_NAMESPACE, + 'metadata.name': ['c1'], + }); expect(entitiesCatalog.addEntities).toHaveBeenCalledTimes(1); expect(entitiesCatalog.addEntities).toHaveBeenNthCalledWith( 1, diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts index 737166436c..fa73c58348 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts @@ -277,12 +277,11 @@ export class HigherOrderOperations implements HigherOrderOperation { }> { const markTimestamp = process.hrtime(); - const names = newEntities.map(e => e.metadata.name); - const oldEntities = await this.entitiesCatalog.entities([ - { key: 'kind', values: [kind] }, - { key: 'metadata.namespace', values: [namespace] }, - { key: 'metadata.name', values: names }, - ]); + const oldEntities = await this.entitiesCatalog.entities({ + kind: kind, + 'metadata.namespace': namespace, + 'metadata.name': newEntities.map(e => e.metadata.name), + }); const oldEntitiesByName = new Map( oldEntities.map(e => [e.metadata.name, e]), diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index ecf4b0e614..f7ab4878f8 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -32,8 +32,6 @@ describe('createRouter', () => { beforeAll(async () => { entitiesCatalog = { entities: jest.fn(), - entityByUid: jest.fn(), - entityByName: jest.fn(), addOrUpdateEntity: jest.fn(), addEntities: jest.fn(), removeEntityByUid: jest.fn(), @@ -83,11 +81,11 @@ describe('createRouter', () => { expect(response.status).toEqual(200); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith([ - { key: 'a', values: ['1', null, '3'] }, - { key: 'b', values: ['4'] }, - { key: 'c', values: [null] }, - ]); + expect(entitiesCatalog.entities).toHaveBeenCalledWith({ + a: ['1', null, '3'], + b: ['4'], + c: [null], + }); }); }); @@ -100,23 +98,27 @@ describe('createRouter', () => { name: 'c', }, }; - entitiesCatalog.entityByUid.mockResolvedValue(entity); + entitiesCatalog.entities.mockResolvedValue([entity]); const response = await request(app).get('/entities/by-uid/zzz'); - expect(entitiesCatalog.entityByUid).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entityByUid).toHaveBeenCalledWith('zzz'); + expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); + expect(entitiesCatalog.entities).toHaveBeenCalledWith({ + 'metadata.uid': 'zzz', + }); expect(response.status).toEqual(200); expect(response.body).toEqual(expect.objectContaining(entity)); }); it('responds with a 404 for missing entities', async () => { - entitiesCatalog.entityByUid.mockResolvedValue(undefined); + entitiesCatalog.entities.mockResolvedValue([]); const response = await request(app).get('/entities/by-uid/zzz'); - expect(entitiesCatalog.entityByUid).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entityByUid).toHaveBeenCalledWith('zzz'); + expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); + expect(entitiesCatalog.entities).toHaveBeenCalledWith({ + 'metadata.uid': 'zzz', + }); expect(response.status).toEqual(404); expect(response.text).toMatch(/uid/); }); @@ -132,30 +134,30 @@ describe('createRouter', () => { namespace: 'ns', }, }; - entitiesCatalog.entityByName.mockResolvedValue(entity); + entitiesCatalog.entities.mockResolvedValue([entity]); const response = await request(app).get('/entities/by-name/k/ns/n'); - expect(entitiesCatalog.entityByName).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entityByName).toHaveBeenCalledWith({ + expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); + expect(entitiesCatalog.entities).toHaveBeenCalledWith({ kind: 'k', - namespace: 'ns', - name: 'n', + 'metadata.namespace': 'ns', + 'metadata.name': 'n', }); expect(response.status).toEqual(200); expect(response.body).toEqual(expect.objectContaining(entity)); }); it('responds with a 404 for missing entities', async () => { - entitiesCatalog.entityByName.mockResolvedValue(undefined); + entitiesCatalog.entities.mockResolvedValue([]); const response = await request(app).get('/entities/by-name/b/d/c'); - expect(entitiesCatalog.entityByName).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entityByName).toHaveBeenCalledWith({ + expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); + expect(entitiesCatalog.entities).toHaveBeenCalledWith({ kind: 'b', - namespace: 'd', - name: 'c', + 'metadata.namespace': 'd', + 'metadata.name': 'c', }); expect(response.status).toEqual(404); expect(response.text).toMatch(/name/); diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index 8da589c721..a262e9b5f4 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -54,11 +54,13 @@ export async function createRouter( }) .get('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; - const entity = await entitiesCatalog.entityByUid(uid); - if (!entity) { + const entities = await entitiesCatalog.entities({ + 'metadata.uid': uid, + }); + if (!entities.length) { res.status(404).send(`No entity with uid ${uid}`); } - res.status(200).send(entity); + res.status(200).send(entities[0]); }) .delete('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; @@ -67,19 +69,19 @@ export async function createRouter( }) .get('/entities/by-name/:kind/:namespace/:name', async (req, res) => { const { kind, namespace, name } = req.params; - const entity = await entitiesCatalog.entityByName({ - kind, - namespace, - name, + const entities = await entitiesCatalog.entities({ + kind: kind, + 'metadata.namespace': namespace, + 'metadata.name': name, }); - if (!entity) { + if (!entities.length) { res .status(404) .send( `No entity with kind ${kind} namespace ${namespace} name ${name}`, ); } - res.status(200).send(entity); + res.status(200).send(entities[0]); }); } @@ -121,7 +123,7 @@ export async function createRouter( function translateQueryToEntityFilters( request: express.Request, ): EntityFilters { - const filters: EntityFilters = []; + const filters: Record = {}; for (const [key, valueOrValues] of Object.entries(request.query)) { const values = Array.isArray(valueOrValues) @@ -132,10 +134,8 @@ function translateQueryToEntityFilters( throw new InputError('Complex query parameters are not supported'); } - filters.push({ - key, - values: values.map(v => v || null) as string[], - }); + const matchers = key in filters ? filters[key] : (filters[key] = []); + matchers.push(...(values.map(v => v || null) as (string | null)[])); } return filters;