diff --git a/packages/backend/src/plugins/catalog.ts b/packages/backend/src/plugins/catalog.ts index 8d8ac5b47b..c472623c3d 100644 --- a/packages/backend/src/plugins/catalog.ts +++ b/packages/backend/src/plugins/catalog.ts @@ -31,7 +31,7 @@ export default async function createPlugin({ }: PluginEnvironment) { const locationReader = new LocationReaders(logger); - const db = await DatabaseManager.createDatabase(database, logger); + const db = await DatabaseManager.createDatabase(database, { logger }); const entitiesCatalog = new DatabaseEntitiesCatalog(db); const locationsCatalog = new DatabaseLocationsCatalog(db); const higherOrderOperation = new HigherOrderOperations( diff --git a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.test.ts index 907bdea2d5..8c7897fa2c 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.test.ts @@ -14,29 +14,14 @@ * limitations under the License. */ -import { getVoidLogger } from '@backstage/backend-common'; -import Knex from 'knex'; -import path from 'path'; -import { CommonDatabase } from '../database'; +import { DatabaseManager } from '../database'; import { DatabaseLocationsCatalog } from './DatabaseLocationsCatalog'; describe('DatabaseLocationsCatalog', () => { let catalog: DatabaseLocationsCatalog; beforeEach(async () => { - const knex = Knex({ - client: 'sqlite3', - connection: ':memory:', - useNullAsDefault: true, - }); - knex.client.pool.on('createSuccess', (_eventId: any, resource: any) => { - resource.run('PRAGMA foreign_keys = ON', () => {}); - }); - await knex.migrate.latest({ - directory: path.resolve(__dirname, '../database/migrations'), - loadExtensions: ['.ts'], - }); - const db = new CommonDatabase(knex, getVoidLogger()); + const db = await DatabaseManager.createTestDatabase(); catalog = new DatabaseLocationsCatalog(db); }); diff --git a/plugins/catalog-backend/src/database/CommonDatabase.test.ts b/plugins/catalog-backend/src/database/CommonDatabase.test.ts index 1802ea5e7d..169880e3c9 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.test.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.test.ts @@ -14,16 +14,10 @@ * limitations under the License. */ -import { - ConflictError, - getVoidLogger, - NotFoundError, -} from '@backstage/backend-common'; +import { ConflictError, NotFoundError } from '@backstage/backend-common'; import type { Entity, Location } from '@backstage/catalog-model'; -import Knex from 'knex'; -import path from 'path'; -import { CommonDatabase } from './CommonDatabase'; -import { DatabaseLocationUpdateLogStatus } from './types'; +import { DatabaseManager } from './DatabaseManager'; +import { Database, DatabaseLocationUpdateLogStatus } from './types'; import type { DbEntityRequest, DbEntityResponse, @@ -31,22 +25,12 @@ import type { } from './types'; describe('CommonDatabase', () => { - let knex: Knex; + let db: Database; let entityRequest: DbEntityRequest; let entityResponse: DbEntityResponse; beforeEach(async () => { - knex = Knex({ - client: 'sqlite3', - connection: ':memory:', - useNullAsDefault: true, - }); - - await knex.raw('PRAGMA foreign_keys = ON'); - await knex.migrate.latest({ - directory: path.resolve(__dirname, 'migrations'), - loadExtensions: ['.ts'], - }); + db = await DatabaseManager.createTestDatabase(); entityRequest = { entity: { @@ -84,7 +68,6 @@ describe('CommonDatabase', () => { }); it('manages locations', async () => { - const db = new CommonDatabase(knex, getVoidLogger()); const input: Location = { id: 'dd12620d-0436-422f-93bd-929aa0788123', type: 'a', @@ -115,55 +98,76 @@ describe('CommonDatabase', () => { describe('addEntity', () => { it('happy path: adds entity to empty database', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); - const added = await catalog.transaction(tx => - catalog.addEntity(tx, entityRequest), - ); + const added = await db.transaction(tx => db.addEntity(tx, entityRequest)); expect(added).toStrictEqual(entityResponse); expect(added.entity.metadata.generation).toBe(1); }); it('rejects adding the same-named entity twice', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); - await catalog.transaction(tx => catalog.addEntity(tx, entityRequest)); + await db.transaction(tx => db.addEntity(tx, entityRequest)); await expect( - catalog.transaction(tx => catalog.addEntity(tx, entityRequest)), + db.transaction(tx => db.addEntity(tx, entityRequest)), + ).rejects.toThrow(ConflictError); + }); + + it('rejects adding the almost-same-kind entity twice', async () => { + entityRequest.entity.kind = 'some-kind'; + await db.transaction(tx => db.addEntity(tx, entityRequest)); + entityRequest.entity.kind = 'SomeKind'; + await expect( + db.transaction(tx => db.addEntity(tx, entityRequest)), + ).rejects.toThrow(ConflictError); + }); + + it('rejects adding the almost-same-named entity twice', async () => { + entityRequest.entity.metadata.name = 'some-name'; + await db.transaction(tx => db.addEntity(tx, entityRequest)); + entityRequest.entity.metadata.name = 'SomeName'; + await expect( + db.transaction(tx => db.addEntity(tx, entityRequest)), + ).rejects.toThrow(ConflictError); + }); + + it('rejects adding the almost-same-namespace entity twice', async () => { + entityRequest.entity.metadata.namespace = undefined; + await db.transaction(tx => db.addEntity(tx, entityRequest)); + entityRequest.entity.metadata.namespace = ''; + await expect( + db.transaction(tx => db.addEntity(tx, entityRequest)), ).rejects.toThrow(ConflictError); }); it('accepts adding the same-named entity twice if on different namespaces', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); entityRequest.entity.metadata.namespace = 'namespace1'; - await catalog.transaction(tx => catalog.addEntity(tx, entityRequest)); + await db.transaction(tx => db.addEntity(tx, entityRequest)); entityRequest.entity.metadata.namespace = 'namespace2'; await expect( - catalog.transaction(tx => catalog.addEntity(tx, entityRequest)), + db.transaction(tx => db.addEntity(tx, entityRequest)), ).resolves.toBeDefined(); }); }); describe('locationHistory', () => { it('outputs the history correctly', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); const location: Location = { id: 'dd12620d-0436-422f-93bd-929aa0788123', type: 'a', target: 'b', }; - await catalog.addLocation(location); + await db.addLocation(location); - await catalog.addLocationUpdateLogEvent( + await db.addLocationUpdateLogEvent( 'dd12620d-0436-422f-93bd-929aa0788123', DatabaseLocationUpdateLogStatus.SUCCESS, ); - await catalog.addLocationUpdateLogEvent( + await db.addLocationUpdateLogEvent( 'dd12620d-0436-422f-93bd-929aa0788123', DatabaseLocationUpdateLogStatus.FAIL, undefined, 'Something went wrong', ); - const result = await catalog.locationHistory( + const result = await db.locationHistory( 'dd12620d-0436-422f-93bd-929aa0788123', ); expect(result).toEqual([ @@ -189,12 +193,9 @@ describe('CommonDatabase', () => { describe('updateEntity', () => { it('can read and no-op-update an entity', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); - const added = await catalog.transaction(tx => - catalog.addEntity(tx, entityRequest), - ); - const updated = await catalog.transaction(tx => - catalog.updateEntity(tx, { entity: added.entity }), + const added = await db.transaction(tx => db.addEntity(tx, entityRequest)); + const updated = await db.transaction(tx => + db.updateEntity(tx, { entity: added.entity }), ); expect(updated.entity.apiVersion).toEqual(added.entity.apiVersion); expect(updated.entity.kind).toEqual(added.entity.kind); @@ -211,77 +212,55 @@ describe('CommonDatabase', () => { }); it('can update name if uid matches', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); - const added = await catalog.transaction(tx => - catalog.addEntity(tx, entityRequest), - ); + const added = await db.transaction(tx => db.addEntity(tx, entityRequest)); added.entity.metadata.name! = 'new!'; - const updated = await catalog.transaction(tx => - catalog.updateEntity(tx, { entity: added.entity }), + const updated = await db.transaction(tx => + db.updateEntity(tx, { entity: added.entity }), ); expect(updated.entity.metadata.name).toEqual('new!'); }); it('can update fields if kind, name, and namespace match', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); - const added = await catalog.transaction(tx => - catalog.addEntity(tx, entityRequest), - ); + const added = await db.transaction(tx => db.addEntity(tx, entityRequest)); added.entity.apiVersion = 'something.new'; delete added.entity.metadata.uid; delete added.entity.metadata.generation; - const updated = await catalog.transaction(tx => - catalog.updateEntity(tx, { entity: added.entity }), + const updated = await db.transaction(tx => + db.updateEntity(tx, { entity: added.entity }), ); expect(updated.entity.apiVersion).toEqual('something.new'); }); it('rejects if kind, name, but not namespace match', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); - const added = await catalog.transaction(tx => - catalog.addEntity(tx, entityRequest), - ); + const added = await db.transaction(tx => db.addEntity(tx, entityRequest)); added.entity.apiVersion = 'something.new'; delete added.entity.metadata.uid; delete added.entity.metadata.generation; added.entity.metadata.namespace = 'something.wrong'; await expect( - catalog.transaction(tx => - catalog.updateEntity(tx, { entity: added.entity }), - ), + db.transaction(tx => db.updateEntity(tx, { entity: added.entity })), ).rejects.toThrow(NotFoundError); }); it('fails to update an entity if etag does not match', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); - const added = await catalog.transaction(tx => - catalog.addEntity(tx, entityRequest), - ); + const added = await db.transaction(tx => db.addEntity(tx, entityRequest)); added.entity.metadata.etag = 'garbage'; await expect( - catalog.transaction(tx => - catalog.updateEntity(tx, { entity: added.entity }), - ), + db.transaction(tx => db.updateEntity(tx, { entity: added.entity })), ).rejects.toThrow(ConflictError); }); it('fails to update an entity if generation does not match', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); - const added = await catalog.transaction(tx => - catalog.addEntity(tx, entityRequest), - ); + const added = await db.transaction(tx => db.addEntity(tx, entityRequest)); added.entity.metadata.generation! += 100; await expect( - catalog.transaction(tx => - catalog.updateEntity(tx, { entity: added.entity }), - ), + db.transaction(tx => db.updateEntity(tx, { entity: added.entity })), ).rejects.toThrow(ConflictError); }); }); describe('entities', () => { it('can get all entities with empty filters list', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); const e1: Entity = { apiVersion: 'a', kind: 'k1', @@ -293,13 +272,11 @@ describe('CommonDatabase', () => { metadata: { name: 'n' }, spec: { c: null }, }; - await catalog.transaction(async tx => { - await catalog.addEntity(tx, { entity: e1 }); - await catalog.addEntity(tx, { entity: e2 }); + await db.transaction(async tx => { + await db.addEntity(tx, { entity: e1 }); + await db.addEntity(tx, { entity: e2 }); }); - const result = await catalog.transaction(async tx => - catalog.entities(tx, []), - ); + const result = await db.transaction(async tx => db.entities(tx, [])); expect(result.length).toEqual(2); expect(result).toEqual( expect.arrayContaining([ @@ -316,7 +293,6 @@ describe('CommonDatabase', () => { }); it('can get all specific entities for matching filters (naive case)', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); const entities: Entity[] = [ { apiVersion: 'a', kind: 'k1', metadata: { name: 'n' } }, { @@ -333,15 +309,15 @@ describe('CommonDatabase', () => { }, ]; - await catalog.transaction(async tx => { + await db.transaction(async tx => { for (const entity of entities) { - await catalog.addEntity(tx, { entity }); + await db.addEntity(tx, { entity }); } }); await expect( - catalog.transaction(async tx => - catalog.entities(tx, [ + db.transaction(async tx => + db.entities(tx, [ { key: 'kind', values: ['k2'] }, { key: 'spec.c', values: ['some'] }, ]), @@ -355,7 +331,6 @@ describe('CommonDatabase', () => { }); it('can get all specific entities for matching filters with nulls (both missing and literal null value)', async () => { - const catalog = new CommonDatabase(knex, getVoidLogger()); const entities: Entity[] = [ { apiVersion: 'a', kind: 'k1', metadata: { name: 'n' } }, { @@ -372,14 +347,14 @@ describe('CommonDatabase', () => { }, ]; - await catalog.transaction(async tx => { + await db.transaction(async tx => { for (const entity of entities) { - await catalog.addEntity(tx, { entity }); + await db.addEntity(tx, { entity }); } }); - const rows = await catalog.transaction(async tx => - catalog.entities(tx, [ + const rows = await db.transaction(async tx => + db.entities(tx, [ { key: 'apiVersion', values: ['a'] }, { key: 'spec.c', values: [null, 'some'] }, ]), diff --git a/plugins/catalog-backend/src/database/CommonDatabase.ts b/plugins/catalog-backend/src/database/CommonDatabase.ts index 2a1b33bbbe..67feb82fac 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.ts @@ -43,7 +43,6 @@ function getStrippedMetadata(metadata: EntityMeta): EntityMeta { delete output.uid; delete output.etag; delete output.generation; - return output; } @@ -62,6 +61,7 @@ function serializeSpec(spec: Entity['spec']): DbEntitiesRow['spec'] { function toEntityRow( locationId: string | undefined, entity: Entity, + normalize: (value: string) => string, ): DbEntitiesRow { return { id: entity.metadata.uid!, @@ -70,8 +70,11 @@ function toEntityRow( generation: entity.metadata.generation!, api_version: entity.apiVersion, kind: entity.kind, - name: entity.metadata.name || null, + name: entity.metadata.name, namespace: entity.metadata.namespace || null, + kind_normalized: normalize(entity.kind), + name_normalized: normalize(entity.metadata.name), + namespace_normalized: normalize(entity.metadata.namespace || ''), metadata: serializeMetadata(entity.metadata), spec: serializeSpec(entity.spec), }; @@ -124,6 +127,7 @@ function generateEtag(): string { export class CommonDatabase implements Database { constructor( private readonly database: Knex, + private readonly normalize: (value: string) => string, private readonly logger: Logger, ) {} @@ -166,7 +170,7 @@ export class CommonDatabase implements Database { generation: 1, }; - const newRow = toEntityRow(request.locationId, newEntity); + const newRow = toEntityRow(request.locationId, newEntity, this.normalize); await tx('entities').insert(newRow); await this.updateEntitiesSearch(tx, newRow.id, newEntity); @@ -257,7 +261,7 @@ export class CommonDatabase implements Database { // Store the updated entity; select on the old etag to ensure that we do // not lose to another writer - const newRow = toEntityRow(request.locationId, newEntity); + const newRow = toEntityRow(request.locationId, newEntity, this.normalize); const updatedRows = await tx('entities') .where({ id: oldRow.id, etag: oldRow.etag }) .update(newRow); diff --git a/plugins/catalog-backend/src/database/DatabaseManager.ts b/plugins/catalog-backend/src/database/DatabaseManager.ts index f0fbe92600..8710aede4c 100644 --- a/plugins/catalog-backend/src/database/DatabaseManager.ts +++ b/plugins/catalog-backend/src/database/DatabaseManager.ts @@ -14,26 +14,39 @@ * limitations under the License. */ +import { getVoidLogger } from '@backstage/backend-common'; +import { makeValidator } from '@backstage/catalog-model'; import Knex from 'knex'; import path from 'path'; import { Logger } from 'winston'; import { CommonDatabase } from './CommonDatabase'; import { Database } from './types'; +export type CreateDatabaseOptions = { + logger: Logger; + fieldNormalizer: (value: string) => string; +}; + +const defaultOptions: CreateDatabaseOptions = { + logger: getVoidLogger(), + fieldNormalizer: makeValidator().normalizeEntityName, +}; + export class DatabaseManager { public static async createDatabase( knex: Knex, - logger: Logger, + options: Partial = {}, ): Promise { await knex.migrate.latest({ directory: path.resolve(__dirname, 'migrations'), loadExtensions: ['.js'], }); - return new CommonDatabase(knex, logger); + const { logger, fieldNormalizer } = { ...defaultOptions, ...options }; + return new CommonDatabase(knex, fieldNormalizer, logger); } public static async createInMemoryDatabase( - logger: Logger, + options: Partial = {}, ): Promise { const knex = Knex({ client: 'sqlite3', @@ -43,6 +56,23 @@ export class DatabaseManager { knex.client.pool.on('createSuccess', (_eventId: any, resource: any) => { resource.run('PRAGMA foreign_keys = ON', () => {}); }); - return DatabaseManager.createDatabase(knex, logger); + return DatabaseManager.createDatabase(knex, options); + } + + public static async createTestDatabase(): Promise { + const knex = Knex({ + client: 'sqlite3', + connection: ':memory:', + useNullAsDefault: true, + }); + knex.client.pool.on('createSuccess', (_eventId: any, resource: any) => { + resource.run('PRAGMA foreign_keys = ON', () => {}); + }); + await knex.migrate.latest({ + directory: path.resolve(__dirname, 'migrations'), + loadExtensions: ['.ts'], + }); + const { logger, fieldNormalizer } = defaultOptions; + return new CommonDatabase(knex, fieldNormalizer, logger); } } diff --git a/plugins/catalog-backend/src/database/migrations/20200612102317_unique_normalized.ts b/plugins/catalog-backend/src/database/migrations/20200612102317_unique_normalized.ts new file mode 100644 index 0000000000..38ed7f6047 --- /dev/null +++ b/plugins/catalog-backend/src/database/migrations/20200612102317_unique_normalized.ts @@ -0,0 +1,51 @@ +/* + * 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 * as Knex from 'knex'; + +export async function up(knex: Knex): Promise { + return knex.schema.alterTable('entities', table => { + // these are added as nullable because sqlite does not support alter column + table + .string('kind_normalized') + .nullable() + .comment('The kind field of the entity, normalized'); + table + .string('name_normalized') + .nullable() + .comment('The metadata.name field of the entity, normalized'); + table + .string('namespace_normalized') + .nullable() + .comment('The metadata.namespace field of the entity, normalized'); + table.dropUnique([], 'entities_unique_name'); + table.unique( + ['kind_normalized', 'name_normalized', 'namespace_normalized'], + 'entities_unique_name_normalized', + ); + }); +} + +export async function down(knex: Knex): Promise { + return knex.schema.alterTable('entities', table => { + table.dropUnique([], 'entities_unique_name_normalized'); + table.unique(['kind', 'name', 'namespace'], 'entities_unique_name'); + table.dropColumns( + 'kind_normalized', + 'name_normalized', + 'namespace_normalized', + ); + }); +} diff --git a/plugins/catalog-backend/src/database/types.ts b/plugins/catalog-backend/src/database/types.ts index fc81e124ba..1cf949c4c2 100644 --- a/plugins/catalog-backend/src/database/types.ts +++ b/plugins/catalog-backend/src/database/types.ts @@ -23,6 +23,9 @@ export type DbEntitiesRow = { kind: string; name: string | null; namespace: string | null; + kind_normalized: string; + name_normalized: string; + namespace_normalized: string; etag: string; generation: number; metadata: string; diff --git a/plugins/catalog-backend/src/service/standaloneServer.ts b/plugins/catalog-backend/src/service/standaloneServer.ts index f19dfaf282..6a9755c0c4 100644 --- a/plugins/catalog-backend/src/service/standaloneServer.ts +++ b/plugins/catalog-backend/src/service/standaloneServer.ts @@ -36,7 +36,7 @@ export async function startStandaloneServer( const logger = options.logger.child({ service: 'catalog-backend' }); logger.debug('Creating application...'); - const db = await DatabaseManager.createInMemoryDatabase(logger); + const db = await DatabaseManager.createInMemoryDatabase({ logger }); const entitiesCatalog = new DatabaseEntitiesCatalog(db); const locationsCatalog = new DatabaseLocationsCatalog(db); const locationReader = new LocationReaders();