feat(catalog-backend): move filters into their own query key (#3066)

This commit is contained in:
Fredrik Adelöw
2020-10-26 10:10:18 +01:00
committed by GitHub
parent 31ba976ea8
commit 002860e7ac
13 changed files with 324 additions and 157 deletions
+12
View File
@@ -0,0 +1,12 @@
---
'@backstage/plugin-catalog-backend': minor
---
Filters passed to the `/entities` endpoint of the catalog has changed format.
The old way was to pass things on the form `?a=b&c=d`; the new way is to pass
things on the form `?filter=a=b,c=d`. See discussion in
[#2910](https://github.com/spotify/backstage/issues/2910) for details.
The comma separated items within a single filter have an AND between them. If
multiple such filters are passed, they have an OR between those item groups.
@@ -14,13 +14,13 @@
* limitations under the License.
*/
import fetch from 'cross-fetch';
import { UserEntity } from '@backstage/catalog-model';
import {
ConflictError,
NotFoundError,
PluginEndpointDiscovery,
} from '@backstage/backend-common';
import { UserEntity } from '@backstage/catalog-model';
import fetch from 'cross-fetch';
type UserQuery = {
annotations: Record<string, string>;
@@ -42,15 +42,17 @@ export class CatalogIdentityClient {
* Throws a NotFoundError or ConflictError if 0 or multiple users are found.
*/
async findUser(query: UserQuery): Promise<UserEntity> {
const params = new URLSearchParams();
params.append('kind', 'User');
const conditions = ['kind=user'];
for (const [key, value] of Object.entries(query.annotations)) {
params.append(`metadata.annotations.${key}`, value);
const uk = encodeURIComponent(key);
const uv = encodeURIComponent(value);
conditions.push(`metadata.annotations.${uk}=${uv}`);
}
const baseUrl = await this.discovery.getBaseUrl('catalog');
const response = await fetch(`${baseUrl}/entities?${params}`);
const response = await fetch(
`${baseUrl}/entities?filter=${conditions.join(',')}`,
);
if (!response.ok) {
const text = await response.text();
@@ -69,11 +69,13 @@ 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(), [
{
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);
@@ -120,11 +122,13 @@ 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(), [
{
kind: 'b',
'metadata.namespace': 'd',
'metadata.name': ['c'],
},
]);
expect(db.entityByName).not.toHaveBeenCalled();
expect(db.entityByUid).toHaveBeenCalledTimes(1);
expect(db.entityByUid).toHaveBeenCalledWith(expect.anything(), 'u');
@@ -194,11 +198,13 @@ 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(), [
{
kind: 'b',
'metadata.namespace': 'd',
'metadata.name': ['c'],
},
]);
expect(db.entityByName).toHaveBeenCalledTimes(1);
expect(db.entityByName).toHaveBeenCalledWith(expect.anything(), {
kind: 'b',
@@ -254,11 +260,13 @@ 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(), [
{
kind: 'b',
'metadata.namespace': 'd',
'metadata.name': ['c'],
},
]);
expect(db.entityByName).not.toHaveBeenCalled();
expect(db.entityByUid).not.toHaveBeenCalled();
expect(db.updateEntity).not.toHaveBeenCalled();
@@ -60,7 +60,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
private readonly logger: Logger,
) {}
async entities(filters?: EntityFilters): Promise<Entity[]> {
async entities(filters?: EntityFilters[]): Promise<Entity[]> {
const items = await this.database.transaction(tx =>
this.database.entities(tx, filters),
);
@@ -109,9 +109,9 @@ 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, [
{ [`metadata.annotations.${LOCATION_ANNOTATION}`]: location },
])
: [entityResponse];
for (const dbResponse of colocatedEntities) {
await this.database.removeEntityByUid(
@@ -234,11 +234,13 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog {
const markTimestamp = process.hrtime();
const names = requests.map(({ entity }) => entity.metadata.name);
const oldEntities = await this.entities({
kind: kind,
'metadata.namespace': namespace,
'metadata.name': names,
});
const oldEntities = await this.entities([
{
kind: kind,
'metadata.namespace': namespace,
'metadata.name': names,
},
]);
const oldEntitiesByName = new Map(
oldEntities.map(e => [e.metadata.name, e]),
+2 -2
View File
@@ -14,7 +14,7 @@
* limitations under the License.
*/
import { Entity, Location, EntityRelationSpec } from '@backstage/catalog-model';
import { Entity, EntityRelationSpec, Location } from '@backstage/catalog-model';
import type { EntityFilters } from '../database';
//
@@ -31,7 +31,7 @@ export type EntityUpsertResponse = {
};
export type EntitiesCatalog = {
entities(filters?: EntityFilters): Promise<Entity[]>;
entities(filters?: EntityFilters[]): Promise<Entity[]>;
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,7 +389,7 @@ describe('CommonDatabase', () => {
await expect(
db.transaction(async tx =>
db.entities(tx, { kind: 'k2', 'spec.c': 'some' }),
db.entities(tx, [{ kind: 'k2', 'spec.c': 'some' }]),
),
).resolves.toEqual([
{
@@ -424,7 +424,7 @@ describe('CommonDatabase', () => {
});
const rows = await db.transaction(async tx =>
db.entities(tx, { apiVersion: 'a', 'spec.c': [null, 'some'] }),
db.entities(tx, [{ apiVersion: 'a', 'spec.c': [null, 'some'] }]),
);
expect(rows.length).toEqual(3);
@@ -471,7 +471,7 @@ describe('CommonDatabase', () => {
});
const rows = await db.transaction(async tx =>
db.entities(tx, { ApiVersioN: 'A', 'spEc.C': [null, 'some'] }),
db.entities(tx, [{ ApiVersioN: 'A', 'spEc.C': [null, 'some'] }]),
);
expect(rows.length).toEqual(3);
@@ -22,12 +22,12 @@ import {
import {
Entity,
EntityName,
EntityRelationSpec,
ENTITY_DEFAULT_NAMESPACE,
ENTITY_META_GENERATED_FIELDS,
generateEntityEtag,
generateEntityUid,
Location,
EntityRelationSpec,
parseEntityName,
} from '@backstage/catalog-model';
import Knex from 'knex';
@@ -218,66 +218,71 @@ export class CommonDatabase implements Database {
async entities(
txOpaque: unknown,
filters?: EntityFilters,
filters?: EntityFilters[],
): Promise<DbEntityResponse[]> {
const tx = txOpaque as Knex.Transaction<any, any>;
let entitiesQuery = tx<DbEntitiesRow>('entities');
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];
for (const singleFilter of filters ?? []) {
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[] = [];
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());
}
}
// 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<DbEntitiesSearchRow>('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);
}
if (matchLike.length) {
for (const x of matchLike) {
this.orWhere('value', 'like', tx.raw('?', [x]));
}
for (const value of values) {
if (!value) {
matchNulls = true;
} else if (value.includes('*')) {
matchLike.push(value.toLowerCase().replace(/[*]/g, '%'));
} else {
matchIn.push(value.toLowerCase());
}
}
// 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<DbEntitiesSearchRow>('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);
}
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) {
// Match explicit nulls, and then handle absence separately below
this.orWhereNull('value');
this.orWhereNotIn(
'id',
tx<DbEntitiesSearchRow>('entities_search')
.select('entity_id')
.where('key', keyOp, key),
);
}
});
});
// Handle absence as nulls as well
entitiesQuery = entitiesQuery.andWhere(function match() {
this.whereIn('id', matchQuery);
if (matchNulls) {
this.orWhereNotIn(
'id',
tx<DbEntitiesSearchRow>('entities_search')
.select('entity_id')
.where('key', keyOp, key),
);
}
});
}
@@ -153,7 +153,7 @@ export type Database = {
matchingGeneration?: number,
): Promise<DbEntityResponse>;
entities(tx: unknown, filters?: EntityFilters): Promise<DbEntityResponse[]>;
entities(tx: unknown, filters?: EntityFilters[]): Promise<DbEntityResponse[]>;
entityByName(
tx: unknown,
@@ -100,9 +100,9 @@ export class HigherOrderOperations implements HigherOrderOperation {
location.id,
);
const entities = await this.entitiesCatalog.entities({
'metadata.uid': writtenEntities.map(e => e.entityId),
});
const entities = await this.entitiesCatalog.entities([
{ 'metadata.uid': writtenEntities.map(e => e.entityId) },
]);
return { location, entities };
}
@@ -0,0 +1,75 @@
/*
* 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 {
translateQueryToEntityFilters,
translateFilterQueryEntryToEntityFilters,
} 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'] });
});
});
@@ -0,0 +1,72 @@
/*
* 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 { EntityFilters } from '../database';
export function translateQueryToEntityFilters(
query: Record<string, any>,
): 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<string, (string | null)[]> = {};
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;
}
@@ -76,15 +76,19 @@ describe('createRouter', () => {
});
it('parses single and multiple request parameters and passes them down', async () => {
const response = await request(app).get('/entities?a=1&a=&a=3&b=4&c=');
const response = await request(app).get(
'/entities?filter=a=1,a=,a=3,b=4&filter=c=',
);
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([
{
a: ['1', null, '3'],
b: ['4'],
},
{ c: [null] },
]);
});
});
@@ -102,9 +106,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([
{ 'metadata.uid': 'zzz' },
]);
expect(response.status).toEqual(200);
expect(response.body).toEqual(expect.objectContaining(entity));
});
@@ -115,9 +119,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([
{ 'metadata.uid': 'zzz' },
]);
expect(response.status).toEqual(404);
expect(response.text).toMatch(/uid/);
});
@@ -138,11 +142,13 @@ describe('createRouter', () => {
const response = await request(app).get('/entities/by-name/k/ns/n');
expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1);
expect(entitiesCatalog.entities).toHaveBeenCalledWith({
kind: 'k',
'metadata.namespace': 'ns',
'metadata.name': 'n',
});
expect(entitiesCatalog.entities).toHaveBeenCalledWith([
{
kind: 'k',
'metadata.namespace': 'ns',
'metadata.name': 'n',
},
]);
expect(response.status).toEqual(200);
expect(response.body).toEqual(expect.objectContaining(entity));
});
@@ -153,11 +159,13 @@ describe('createRouter', () => {
const response = await request(app).get('/entities/by-name/b/d/c');
expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1);
expect(entitiesCatalog.entities).toHaveBeenCalledWith({
kind: 'b',
'metadata.namespace': 'd',
'metadata.name': 'c',
});
expect(entitiesCatalog.entities).toHaveBeenCalledWith([
{
kind: 'b',
'metadata.namespace': 'd',
'metadata.name': 'c',
},
]);
expect(response.status).toEqual(404);
expect(response.text).toMatch(/name/);
});
@@ -200,9 +208,9 @@ describe('createRouter', () => {
{ entity, relations: [] },
]);
expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1);
expect(entitiesCatalog.entities).toHaveBeenCalledWith({
'metadata.uid': 'u',
});
expect(entitiesCatalog.entities).toHaveBeenCalledWith([
{ 'metadata.uid': 'u' },
]);
expect(response.status).toEqual(200);
expect(response.body).toEqual(entity);
});
+19 -36
View File
@@ -14,15 +14,15 @@
* limitations under the License.
*/
import { errorHandler, InputError } from '@backstage/backend-common';
import { locationSpecSchema } from '@backstage/catalog-model';
import { errorHandler } from '@backstage/backend-common';
import type { Entity } from '@backstage/catalog-model';
import { locationSpecSchema } from '@backstage/catalog-model';
import express from 'express';
import Router from 'express-promise-router';
import { Logger } from 'winston';
import { EntitiesCatalog, LocationsCatalog } from '../catalog';
import { EntityFilters } from '../database';
import { HigherOrderOperation } from '../ingestion/types';
import { translateQueryToEntityFilters } from './filterQuery';
import { requireRequestBody, validateRequestBody } from './util';
export interface RouterOptions {
@@ -43,7 +43,7 @@ export async function createRouter(
if (entitiesCatalog) {
router
.get('/entities', async (req, res) => {
const filters = translateQueryToEntityFilters(req);
const filters = translateQueryToEntityFilters(req.query);
const entities = await entitiesCatalog.entities(filters);
res.status(200).send(entities);
})
@@ -52,16 +52,18 @@ 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([
{ '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([
{
'metadata.uid': uid,
},
]);
if (!entities.length) {
res.status(404).send(`No entity with uid ${uid}`);
}
@@ -74,11 +76,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({
kind: kind,
'metadata.namespace': namespace,
'metadata.name': name,
});
const entities = await entitiesCatalog.entities([
{
kind: kind,
'metadata.namespace': namespace,
'metadata.name': name,
},
]);
if (!entities.length) {
res
.status(404)
@@ -124,24 +128,3 @@ export async function createRouter(
router.use(errorHandler());
return router;
}
function translateQueryToEntityFilters(
request: express.Request,
): EntityFilters {
const filters: Record<string, (string | null)[]> = {};
for (const [key, valueOrValues] of Object.entries(request.query)) {
const values = Array.isArray(valueOrValues)
? valueOrValues
: [valueOrValues];
if (values.some(v => typeof v !== 'string')) {
throw new InputError('Complex query parameters are not supported');
}
const matchers = key in filters ? filters[key] : (filters[key] = []);
matchers.push(...(values.map(v => v || null) as (string | null)[]));
}
return filters;
}