Merge pull request #2836 from spotify/freben/simpler

feat(catalog-backend): some more simplification
This commit is contained in:
Fredrik Adelöw
2020-10-15 08:33:21 +02:00
committed by GitHub
11 changed files with 113 additions and 109 deletions
@@ -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<Entity | undefined> {
const response = await this.database.transaction(tx =>
this.database.entityByUid(tx, uid),
);
return response?.entity;
}
async entityByName(name: EntityName): Promise<Entity | undefined> {
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(
+1 -3
View File
@@ -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<Entity[]>;
entityByUid(uid: string): Promise<Entity | undefined>;
entityByName(name: EntityName): Promise<Entity | undefined>;
addOrUpdateEntity(entity: Entity, locationId?: string): Promise<Entity>;
addEntities(entities: Entity[], locationId?: string): Promise<void>;
removeEntityByUid(uid: string): Promise<void>;
@@ -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);
@@ -219,15 +219,16 @@ export class CommonDatabase implements Database {
let entitiesQuery = tx<DbEntitiesRow>('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('*')) {
@@ -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 },
]);
});
+26 -9
View File
@@ -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 });
}
+21 -5
View File
@@ -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<string, EntityFilter>;
/**
* An abstraction on top of the underlying database, wrapping the basic CRUD
@@ -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,
@@ -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]),
@@ -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/);
+14 -14
View File
@@ -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<string, (string | null)[]> = {};
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;