From 9ccf617b82e9ff6f95429d9c3bd7b995070aa75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 28 May 2020 23:01:35 +0200 Subject: [PATCH 1/3] Add routes for add/delete entity to the catalog --- packages/backend/src/plugins/catalog.ts | 2 +- .../catalog/DatabaseEntitiesCatalog.test.ts | 110 ++++++++++++++++++ .../src/catalog/DatabaseEntitiesCatalog.ts | 75 +++++++++--- .../catalog/DatabaseLocationsCatalog.test.ts | 17 +-- .../src/catalog/DatabaseLocationsCatalog.ts | 5 +- .../src/catalog/StaticEntitiesCatalog.ts | 23 ++-- plugins/catalog-backend/src/catalog/index.ts | 14 ++- plugins/catalog-backend/src/catalog/types.ts | 9 +- ...atabase.test.ts => CommonDatabase.test.ts} | 52 ++++----- .../{Database.ts => CommonDatabase.ts} | 75 +++++------- .../src/database/DatabaseManager.test.ts | 10 +- .../src/database/DatabaseManager.ts | 79 +++++++++---- plugins/catalog-backend/src/database/index.ts | 12 +- .../src/database/search.test.ts | 4 +- .../catalog-backend/src/database/search.ts | 4 +- plugins/catalog-backend/src/database/types.ts | 81 ++++++++++++- .../src/ingestion/IngestionModels.ts | 4 +- .../src/service/router.test.ts | 101 +++++++++++++++- plugins/catalog-backend/src/service/router.ts | 15 ++- plugins/catalog-backend/src/service/util.ts | 22 +++- 20 files changed, 545 insertions(+), 169 deletions(-) create mode 100644 plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts rename plugins/catalog-backend/src/database/{Database.test.ts => CommonDatabase.test.ts} (90%) rename plugins/catalog-backend/src/database/{Database.ts => CommonDatabase.ts} (88%) diff --git a/packages/backend/src/plugins/catalog.ts b/packages/backend/src/plugins/catalog.ts index 7e843cc80b..9f5315fa08 100644 --- a/packages/backend/src/plugins/catalog.ts +++ b/packages/backend/src/plugins/catalog.ts @@ -41,7 +41,7 @@ export default async function ({ logger, database }: PluginEnvironment) { 10000, ); - const entitiesCatalog = new DatabaseEntitiesCatalog(db); + const entitiesCatalog = new DatabaseEntitiesCatalog(db, policy); const locationsCatalog = new DatabaseLocationsCatalog(db, ingestion); return await createRouter({ entitiesCatalog, locationsCatalog, logger }); diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts new file mode 100644 index 0000000000..d897584c95 --- /dev/null +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts @@ -0,0 +1,110 @@ +/* + * 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 type { Entity, EntityPolicy } from '@backstage/catalog-model'; +import type { Database } from '../database'; +import { DatabaseEntitiesCatalog } from './DatabaseEntitiesCatalog'; + +describe('DatabaseEntitiesCatalog', () => { + let db: Database; + let policy: EntityPolicy; + + beforeEach(() => { + // Since the database has a large API surface, we just leave it empty and + // let the tests insert whatever methods they need to call + db = ({ + transaction: jest.fn(async f => f('mock_tx')), + } as unknown) as Database; + policy = { enforce: jest.fn(async x => x) }; + }); + + describe('addOrUpdateEntity', () => { + it('adds when no given uid and no matching by name', async () => { + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + namespace: 'd', + }, + }; + + db.entities = jest.fn().mockResolvedValue([]); + db.addEntity = jest.fn().mockResolvedValue({ entity }); + + const catalog = new DatabaseEntitiesCatalog(db, policy); + const result = await catalog.addOrUpdateEntity(entity); + + expect(policy.enforce).toBeCalledWith(entity); + expect(db.entities).toHaveBeenCalledTimes(1); + expect(db.addEntity).toHaveBeenCalledTimes(1); + expect(result).toBe(entity); + }); + + it('updates when given uid', async () => { + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { + uid: 'uuuu', + name: 'c', + namespace: 'd', + }, + }; + + db.entities = jest.fn().mockResolvedValue([]); + db.updateEntity = jest.fn().mockResolvedValue({ entity }); + + const catalog = new DatabaseEntitiesCatalog(db, policy); + const result = await catalog.addOrUpdateEntity(entity); + + expect(policy.enforce).toBeCalledWith(entity); + expect(db.entities).toHaveBeenCalledTimes(0); + expect(db.updateEntity).toHaveBeenCalledTimes(1); + expect(result).toBe(entity); + }); + + it('update when no given uid and matching by name', async () => { + const added: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + namespace: 'd', + }, + }; + const existing: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + namespace: 'd', + }, + }; + + db.entities = jest.fn().mockResolvedValue([{ entity: existing }]); + db.updateEntity = jest.fn().mockResolvedValue({ entity: added }); + + const catalog = new DatabaseEntitiesCatalog(db, policy); + const result = await catalog.addOrUpdateEntity(added); + + expect(policy.enforce).toBeCalledWith(added); + expect(db.entities).toHaveBeenCalledTimes(1); + expect(db.updateEntity).toHaveBeenCalledTimes(1); + expect(result).toEqual(existing); + }); + }); +}); diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index 972410a639..d13ac9a1ba 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -14,12 +14,15 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; -import { Database } from '../database'; -import { EntitiesCatalog, EntityFilters } from './types'; +import type { Entity, EntityPolicy } from '@backstage/catalog-model'; +import type { Database, DbEntityResponse, EntityFilters } from '../database'; +import type { EntitiesCatalog } from './types'; export class DatabaseEntitiesCatalog implements EntitiesCatalog { - constructor(private readonly database: Database) {} + constructor( + private readonly database: Database, + private readonly policy: EntityPolicy, + ) {} async entities(filters?: EntityFilters): Promise { const items = await this.database.transaction(tx => @@ -41,19 +44,59 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { name: string, namespace: string | undefined, ): Promise { - const matches = await this.database.transaction(tx => - this.database.entities(tx, [ - { key: 'kind', values: [kind] }, - { key: 'name', values: [name] }, - { - key: 'namespace', - values: - !namespace || namespace === 'default' - ? [null, 'default'] - : [namespace], - }, - ]), + return await this.database.transaction(tx => + this.entityByNameInternal(tx, kind, name, namespace), ); + } + + async addOrUpdateEntity(entity: Entity): Promise { + await this.policy.enforce(entity); + return await this.database.transaction(async tx => { + let response: DbEntityResponse; + + if (entity.metadata.uid) { + response = await this.database.updateEntity(tx, { entity }); + } else { + const existing = await this.entityByNameInternal( + tx, + entity.kind, + entity.metadata.name, + entity.metadata.namespace, + ); + if (existing) { + response = await this.database.updateEntity(tx, { entity }); + } else { + response = await this.database.addEntity(tx, { entity }); + } + } + + return response.entity; + }); + } + + async removeEntityByUid(uid: string): Promise { + return await this.database.transaction(async tx => { + await this.database.removeEntity(tx, uid); + }); + } + + private async entityByNameInternal( + tx: unknown, + kind: string, + name: string, + namespace: string | undefined, + ): Promise { + const matches = await this.database.entities(tx, [ + { key: 'kind', values: [kind] }, + { key: 'name', values: [name] }, + { + key: 'namespace', + values: + !namespace || namespace === 'default' + ? [null, 'default'] + : [namespace], + }, + ]); return matches.length ? matches[0].entity : undefined; } diff --git a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.test.ts index 56a3b3828f..443b3bf6b0 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.test.ts @@ -14,11 +14,12 @@ * limitations under the License. */ import { getVoidLogger } from '@backstage/backend-common'; -import { Entity } from '@backstage/catalog-model'; -import knex from 'knex'; +import type { Entity } from '@backstage/catalog-model'; +import Knex from 'knex'; import path from 'path'; -import { Database } from '../database'; -import { IngestionModel } from '../ingestion/types'; +import { CommonDatabase } from '../database'; +import type { Database } from '../database'; +import type { IngestionModel } from '../ingestion/types'; import { DatabaseLocationsCatalog } from './DatabaseLocationsCatalog'; class MockIngestionModel implements IngestionModel { @@ -36,12 +37,12 @@ class MockIngestionModel implements IngestionModel { } describe('DatabaseLocationsCatalog', () => { - const database = knex({ + const knex = Knex({ client: 'sqlite3', connection: ':memory:', useNullAsDefault: true, }); - database.client.pool.on('createSuccess', (_eventId: any, resource: any) => { + knex.client.pool.on('createSuccess', (_eventId: any, resource: any) => { resource.run('PRAGMA foreign_keys = ON', () => {}); }); let db: Database; @@ -49,11 +50,11 @@ describe('DatabaseLocationsCatalog', () => { let ingestionModel: IngestionModel; beforeEach(async () => { - await database.migrate.latest({ + await knex.migrate.latest({ directory: path.resolve(__dirname, '../database/migrations'), loadExtensions: ['.ts'], }); - db = new Database(database, getVoidLogger()); + db = new CommonDatabase(knex, getVoidLogger()); ingestionModel = new MockIngestionModel(); catalog = new DatabaseLocationsCatalog(db, ingestionModel); }); diff --git a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts index b5841c8c2c..ae910e0b9b 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts @@ -14,13 +14,14 @@ * limitations under the License. */ -import { Database, DatabaseLocationUpdateLogEvent } from '../database'; +import type { Database } from '../database'; +import { DatabaseLocationUpdateLogEvent } from '../database/types'; import { IngestionModel } from '../ingestion/types'; import { AddLocation, Location, - LocationsCatalog, LocationResponse, + LocationsCatalog, } from './types'; export class DatabaseLocationsCatalog implements LocationsCatalog { diff --git a/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts index 64ac5f57d5..22bbd2e1a3 100644 --- a/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts @@ -14,10 +14,9 @@ * limitations under the License. */ -import { NotFoundError } from '@backstage/backend-common'; -import { Entity } from '@backstage/catalog-model'; +import type { Entity } from '@backstage/catalog-model'; import lodash from 'lodash'; -import { EntitiesCatalog } from './types'; +import type { EntitiesCatalog } from './types'; export class StaticEntitiesCatalog implements EntitiesCatalog { private _entities: Entity[]; @@ -32,10 +31,7 @@ export class StaticEntitiesCatalog implements EntitiesCatalog { async entityByUid(uid: string): Promise { const item = this._entities.find(e => uid === e.metadata.uid); - if (!item) { - throw new NotFoundError('Entity cannot be found'); - } - return lodash.cloneDeep(item); + return item ? lodash.cloneDeep(item) : undefined; } async entityByName( @@ -49,9 +45,14 @@ export class StaticEntitiesCatalog implements EntitiesCatalog { name === e.metadata.name && namespace === e.metadata.namespace, ); - if (!item) { - throw new NotFoundError('Entity cannot be found'); - } - return lodash.cloneDeep(item); + return item ? lodash.cloneDeep(item) : undefined; + } + + async addOrUpdateEntity(): Promise { + throw new Error('Not supported'); + } + + async removeEntityByUid(): Promise { + throw new Error('Not supported'); } } diff --git a/plugins/catalog-backend/src/catalog/index.ts b/plugins/catalog-backend/src/catalog/index.ts index 58ae531944..6768268f34 100644 --- a/plugins/catalog-backend/src/catalog/index.ts +++ b/plugins/catalog-backend/src/catalog/index.ts @@ -14,7 +14,13 @@ * limitations under the License. */ -export * from './DatabaseEntitiesCatalog'; -export * from './DatabaseLocationsCatalog'; -export * from './StaticEntitiesCatalog'; -export * from './types'; +export { DatabaseEntitiesCatalog } from './DatabaseEntitiesCatalog'; +export { DatabaseLocationsCatalog } from './DatabaseLocationsCatalog'; +export { StaticEntitiesCatalog } from './StaticEntitiesCatalog'; +export { addLocationSchema } from './types'; +export type { + AddLocation, + EntitiesCatalog, + Location, + LocationsCatalog, +} from './types'; diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 5f55de251f..61fba33ebf 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -16,17 +16,12 @@ import { Entity } from '@backstage/catalog-model'; import * as yup from 'yup'; +import type { EntityFilters } from '../database'; // // Entities // -export type EntityFilter = { - key: string; - values: (string | null)[]; -}; -export type EntityFilters = EntityFilter[]; - export type EntitiesCatalog = { entities(filters?: EntityFilters): Promise; entityByUid(uid: string): Promise; @@ -35,6 +30,8 @@ export type EntitiesCatalog = { namespace: string | undefined, name: string, ): Promise; + addOrUpdateEntity(entity: Entity): Promise; + removeEntityByUid(uid: string): Promise; }; // diff --git a/plugins/catalog-backend/src/database/Database.test.ts b/plugins/catalog-backend/src/database/CommonDatabase.test.ts similarity index 90% rename from plugins/catalog-backend/src/database/Database.test.ts rename to plugins/catalog-backend/src/database/CommonDatabase.test.ts index 6f15440fff..3b783a7691 100644 --- a/plugins/catalog-backend/src/database/Database.test.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.test.ts @@ -19,33 +19,33 @@ import { getVoidLogger, NotFoundError, } from '@backstage/backend-common'; +import type { Entity } from '@backstage/catalog-model'; import Knex from 'knex'; import path from 'path'; -import { +import { CommonDatabase } from './CommonDatabase'; +import { DatabaseLocationUpdateLogStatus } from './types'; +import type { + AddDatabaseLocation, DbEntityRequest, DbEntityResponse, - Database, - AddDatabaseLocation, DbLocationsRow, DbLocationsRowWithStatus, - DatabaseLocationUpdateLogStatus, -} from '.'; -import { Entity } from '@backstage/catalog-model'; +} from './types'; -describe('Database', () => { - let database: Knex; +describe('CommonDatabase', () => { + let knex: Knex; let entityRequest: DbEntityRequest; let entityResponse: DbEntityResponse; beforeEach(async () => { - database = Knex({ + knex = Knex({ client: 'sqlite3', connection: ':memory:', useNullAsDefault: true, }); - await database.raw('PRAGMA foreign_keys = ON'); - await database.migrate.latest({ + await knex.raw('PRAGMA foreign_keys = ON'); + await knex.migrate.latest({ directory: path.resolve(__dirname, 'migrations'), loadExtensions: ['.ts'], }); @@ -86,7 +86,7 @@ describe('Database', () => { }); it('manages locations', async () => { - const db = new Database(database, getVoidLogger()); + const db = new CommonDatabase(knex, getVoidLogger()); const input: AddDatabaseLocation = { type: 'a', target: 'b' }; const output: DbLocationsRowWithStatus = { id: expect.anything(), @@ -114,7 +114,7 @@ describe('Database', () => { it('instead of adding second location with the same target, returns existing one', async () => { // Prepare - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const input: AddDatabaseLocation = { type: 'a', target: 'b' }; const output1: DbLocationsRow = await catalog.addLocation(input); @@ -130,7 +130,7 @@ describe('Database', () => { describe('addEntity', () => { it('happy path: adds entity to empty database', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); @@ -139,7 +139,7 @@ describe('Database', () => { }); it('rejects adding the same-named entity twice', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); await catalog.transaction(tx => catalog.addEntity(tx, entityRequest)); await expect( catalog.transaction(tx => catalog.addEntity(tx, entityRequest)), @@ -147,7 +147,7 @@ describe('Database', () => { }); it('accepts adding the same-named entity twice if on different namespaces', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); entityRequest.entity.metadata.namespace = 'namespace1'; await catalog.transaction(tx => catalog.addEntity(tx, entityRequest)); entityRequest.entity.metadata.namespace = 'namespace2'; @@ -159,7 +159,7 @@ describe('Database', () => { describe('locationHistory', () => { it('outputs the history correctly', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const location: AddDatabaseLocation = { type: 'a', target: 'b' }; const { id: locationId } = await catalog.addLocation(location); @@ -198,7 +198,7 @@ describe('Database', () => { describe('updateEntity', () => { it('can read and no-op-update an entity', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); @@ -220,7 +220,7 @@ describe('Database', () => { }); it('can update name if uid matches', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); @@ -232,7 +232,7 @@ describe('Database', () => { }); it('can update fields if kind, name, and namespace match', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); @@ -246,7 +246,7 @@ describe('Database', () => { }); it('rejects if kind, name, but not namespace match', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); @@ -262,7 +262,7 @@ describe('Database', () => { }); it('fails to update an entity if etag does not match', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); @@ -275,7 +275,7 @@ describe('Database', () => { }); it('fails to update an entity if generation does not match', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const added = await catalog.transaction(tx => catalog.addEntity(tx, entityRequest), ); @@ -290,7 +290,7 @@ describe('Database', () => { describe('entities', () => { it('can get all entities with empty filters list', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const e1: Entity = { apiVersion: 'a', kind: 'k1', @@ -325,7 +325,7 @@ describe('Database', () => { }); it('can get all specific entities for matching filters (naive case)', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const entities: Entity[] = [ { apiVersion: 'a', kind: 'k1', metadata: { name: 'n' } }, { @@ -364,7 +364,7 @@ describe('Database', () => { }); it('can get all specific entities for matching filters with nulls (both missing and literal null value)', async () => { - const catalog = new Database(database, getVoidLogger()); + const catalog = new CommonDatabase(knex, getVoidLogger()); const entities: Entity[] = [ { apiVersion: 'a', kind: 'k1', metadata: { name: 'n' } }, { diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/CommonDatabase.ts similarity index 88% rename from plugins/catalog-backend/src/database/Database.ts rename to plugins/catalog-backend/src/database/CommonDatabase.ts index ea823a3da7..2dac08b9bc 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.ts @@ -19,15 +19,15 @@ import { InputError, NotFoundError, } from '@backstage/backend-common'; -import { Entity, EntityMeta } from '@backstage/catalog-model'; +import type { Entity, EntityMeta } from '@backstage/catalog-model'; import Knex from 'knex'; import lodash from 'lodash'; import { v4 as uuidv4 } from 'uuid'; -import { Logger } from 'winston'; -import { EntityFilters } from '../catalog'; +import type { Logger } from 'winston'; import { buildEntitySearch } from './search'; -import { +import type { AddDatabaseLocation, + Database, DatabaseLocationUpdateLogEvent, DatabaseLocationUpdateLogStatus, DbEntitiesRow, @@ -36,6 +36,7 @@ import { DbEntityResponse, DbLocationsRow, DbLocationsRowWithStatus, + EntityFilters, } from './types'; function getStrippedMetadata(metadata: EntityMeta): EntityMeta { @@ -121,27 +122,13 @@ function generateEtag(): string { return Buffer.from(uuidv4(), 'utf8').toString('base64').replace(/[^\w]/g, ''); } -/** - * An abstraction on top of the underlying database, wrapping the basic CRUD - * needs. - */ -export class Database { +export class CommonDatabase implements Database { constructor( private readonly database: Knex, private readonly logger: Logger, ) {} - /** - * Runs a transaction. - * - * The callback is expected to make calls back into this class. When it - * completes, the transaction is closed. - * - * @param fn The callback that implements the transaction - */ - async transaction( - fn: (tx: Knex.Transaction) => Promise, - ): Promise { + async transaction(fn: (tx: unknown) => Promise): Promise { try { return await this.database.transaction(fn); } catch (e) { @@ -158,17 +145,12 @@ export class Database { } } - /** - * Adds a new entity to the catalog. - * - * @param tx An ongoing transaction - * @param request The entity being added - * @returns The added entity, with uid, etag and generation set - */ async addEntity( - tx: Knex.Transaction, + txOpaque: unknown, request: DbEntityRequest, ): Promise { + const tx = txOpaque as Knex.Transaction; + if (request.entity.metadata.uid !== undefined) { throw new InputError('May not specify uid for new entities'); } else if (request.entity.metadata.etag !== undefined) { @@ -198,25 +180,12 @@ export class Database { return { locationId: request.locationId, entity: newEntity }; } - /** - * Updates an existing entity in the catalog. - * - * The given entity must contain enough information to identify an already - * stored entity in the catalog - either by uid, or by kind + namespace + - * name. If no matching entity is found, the operation fails. - * - * If etag or generation are given, they are taken into account. Attempts to - * update a matching entity, but where the etag and/or generation are not - * equal to the passed values, will fail. - * - * @param tx An ongoing transaction - * @param request The entity being updated - * @returns The updated entity - */ async updateEntity( - tx: Knex.Transaction, + txOpaque: unknown, request: DbEntityRequest, ): Promise { + const tx = txOpaque as Knex.Transaction; + const { kind } = request.entity; const { uid, @@ -310,9 +279,11 @@ export class Database { } async entities( - tx: Knex.Transaction, + txOpaque: unknown, filters?: EntityFilters, ): Promise { + const tx = txOpaque as Knex.Transaction; + let builder = tx('entities'); for (const [index, filter] of (filters ?? []).entries()) { builder = builder @@ -337,11 +308,13 @@ export class Database { } async entity( - tx: Knex.Transaction, + txOpaque: unknown, kind: string, name: string, namespace?: string, ): Promise { + const tx = txOpaque as Knex.Transaction; + const rows = await tx('entities') .where({ kind, name, namespace: namespace || null }) .select(); @@ -353,6 +326,16 @@ export class Database { return toEntityResponse(rows[0]); } + async removeEntity(txOpaque: unknown, uid: string): Promise { + const tx = txOpaque as Knex.Transaction; + + const result = await tx('entities').where({ id: uid }).del(); + + if (!result) { + throw new NotFoundError(`Found no entity with ID ${uid}`); + } + } + async addLocation(location: AddDatabaseLocation): Promise { return await this.database.transaction(async tx => { const existingLocation = await tx('locations') diff --git a/plugins/catalog-backend/src/database/DatabaseManager.test.ts b/plugins/catalog-backend/src/database/DatabaseManager.test.ts index 6e18eaf419..0c28524f8d 100644 --- a/plugins/catalog-backend/src/database/DatabaseManager.test.ts +++ b/plugins/catalog-backend/src/database/DatabaseManager.test.ts @@ -15,16 +15,16 @@ */ import { getVoidLogger } from '@backstage/backend-common'; +import type { Entity, EntityPolicy } from '@backstage/catalog-model'; import Knex from 'knex'; -import { Database } from './Database'; +import type { IngestionModel } from '../ingestion/types'; import { DatabaseManager } from './DatabaseManager'; -import { - DatabaseLocationUpdateLogStatus, +import { DatabaseLocationUpdateLogStatus } from './types'; +import type { + Database, DbLocationsRow, DbLocationsRowWithStatus, } from './types'; -import { EntityPolicy, Entity } from '@backstage/catalog-model'; -import { IngestionModel } from '..'; describe('DatabaseManager', () => { describe('refreshLocations', () => { diff --git a/plugins/catalog-backend/src/database/DatabaseManager.ts b/plugins/catalog-backend/src/database/DatabaseManager.ts index 05b6eaef8d..4268dc4146 100644 --- a/plugins/catalog-backend/src/database/DatabaseManager.ts +++ b/plugins/catalog-backend/src/database/DatabaseManager.ts @@ -14,25 +14,26 @@ * limitations under the License. */ -import { Entity, EntityPolicy } from '@backstage/catalog-model'; +import type { Entity, EntityPolicy } from '@backstage/catalog-model'; import Knex from 'knex'; import lodash from 'lodash'; import path from 'path'; import { Logger } from 'winston'; -import { IngestionModel } from '../ingestion/types'; -import { Database } from './Database'; -import { DatabaseLocationUpdateLogStatus, DbEntityRequest } from './types'; +import type { IngestionModel } from '../ingestion/types'; +import { CommonDatabase } from './CommonDatabase'; +import { DatabaseLocationUpdateLogStatus } from './types'; +import type { Database, DbEntityRequest } from './types'; export class DatabaseManager { public static async createDatabase( - database: Knex, + knex: Knex, logger: Logger, ): Promise { - await database.migrate.latest({ + await knex.migrate.latest({ directory: path.resolve(__dirname, 'migrations'), loadExtensions: ['.js'], }); - return new Database(database, logger); + return new CommonDatabase(knex, logger); } private static async logUpdateSuccess( @@ -158,22 +159,58 @@ export class DatabaseManager { }); } - private static entitiesAreEqual(first: Entity, second: Entity) { - const firstClone = lodash.cloneDeep(first); - const secondClone = lodash.cloneDeep(second); + private static entitiesAreEqual(previous: Entity, next: Entity) { + if ( + previous.apiVersion !== next.apiVersion || + previous.kind !== next.kind || + !lodash.isEqual(previous.spec, next.spec) // Accept that {} !== undefined + ) { + return false; + } + + // Since the next annotations get merged into the previous, extract only + // the overlapping keys and check if their values match. + if (next.metadata.annotations) { + if (!previous.metadata.annotations) { + return false; + } + if ( + !lodash.isEqual( + next.metadata.annotations, + lodash.pick( + previous.metadata.annotations, + Object.keys(next.metadata.annotations), + ), + ) + ) { + return false; + } + } + + const e1 = lodash.cloneDeep(previous); + const e2 = lodash.cloneDeep(next); + + if (!e1.metadata.labels) { + e1.metadata.labels = {}; + } + if (!e2.metadata.labels) { + e2.metadata.labels = {}; + } // Remove generated fields - if (firstClone.metadata) { - delete firstClone.metadata.uid; - delete firstClone.metadata.etag; - delete firstClone.metadata.generation; - } - if (secondClone.metadata) { - delete secondClone.metadata.uid; - delete secondClone.metadata.etag; - delete secondClone.metadata.generation; - } + delete e1.metadata.uid; + delete e1.metadata.etag; + delete e1.metadata.generation; + delete e2.metadata.uid; + delete e2.metadata.etag; + delete e2.metadata.generation; - return lodash.isEqual(firstClone, secondClone); + // Remove already compared things + delete e1.metadata.annotations; + delete e1.spec; + delete e2.metadata.annotations; + delete e2.spec; + + return lodash.isEqual(e1, e2); } } diff --git a/plugins/catalog-backend/src/database/index.ts b/plugins/catalog-backend/src/database/index.ts index 616808fb67..565a41cfb2 100644 --- a/plugins/catalog-backend/src/database/index.ts +++ b/plugins/catalog-backend/src/database/index.ts @@ -14,6 +14,12 @@ * limitations under the License. */ -export * from './Database'; -export * from './DatabaseManager'; -export * from './types'; +export { CommonDatabase } from './CommonDatabase'; +export { DatabaseManager } from './DatabaseManager'; +export type { + Database, + DbEntityRequest, + DbEntityResponse, + EntityFilter, + EntityFilters, +} from './types'; diff --git a/plugins/catalog-backend/src/database/search.test.ts b/plugins/catalog-backend/src/database/search.test.ts index 8f011fb250..38a2d40e74 100644 --- a/plugins/catalog-backend/src/database/search.test.ts +++ b/plugins/catalog-backend/src/database/search.test.ts @@ -14,9 +14,9 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; +import type { Entity } from '@backstage/catalog-model'; import { buildEntitySearch, visitEntityPart } from './search'; -import { DbEntitiesSearchRow } from './types'; +import type { DbEntitiesSearchRow } from './types'; describe('search', () => { describe('visitEntityPart', () => { diff --git a/plugins/catalog-backend/src/database/search.ts b/plugins/catalog-backend/src/database/search.ts index 87fc59185d..c14acb6661 100644 --- a/plugins/catalog-backend/src/database/search.ts +++ b/plugins/catalog-backend/src/database/search.ts @@ -14,8 +14,8 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; -import { DbEntitiesSearchRow } from './types'; +import type { Entity } from '@backstage/catalog-model'; +import type { DbEntitiesSearchRow } from './types'; // Search entries that start with these prefixes, also get a shorthand without // that prefix diff --git a/plugins/catalog-backend/src/database/types.ts b/plugins/catalog-backend/src/database/types.ts index 0b9485242f..f69a7353c7 100644 --- a/plugins/catalog-backend/src/database/types.ts +++ b/plugins/catalog-backend/src/database/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; +import type { Entity } from '@backstage/catalog-model'; import * as yup from 'yup'; export type DbEntitiesRow = { @@ -83,3 +83,82 @@ export type DatabaseLocationUpdateLogEvent = { created_at?: string; message?: string; }; + +export type EntityFilter = { + key: string; + values: (string | null)[]; +}; +export type EntityFilters = EntityFilter[]; + +/** + * An abstraction on top of the underlying database, wrapping the basic CRUD + * needs. + */ +export type Database = { + /** + * Runs a transaction. + * + * The callback is expected to make calls back into this class. When it + * completes, the transaction is closed. + * + * @param fn The callback that implements the transaction + */ + transaction(fn: (tx: unknown) => Promise): Promise; + + /** + * Adds a new entity to the catalog. + * + * @param tx An ongoing transaction + * @param request The entity being added + * @returns The added entity, with uid, etag and generation set + */ + addEntity(tx: unknown, request: DbEntityRequest): Promise; + + /** + * Updates an existing entity in the catalog. + * + * The given entity must contain enough information to identify an already + * stored entity in the catalog - either by uid, or by kind + namespace + + * name. If no matching entity is found, the operation fails. + * + * If etag or generation are given, they are taken into account. Attempts to + * update a matching entity, but where the etag and/or generation are not + * equal to the passed values, will fail. + * + * @param tx An ongoing transaction + * @param request The entity being updated + * @returns The updated entity + */ + updateEntity( + tx: unknown, + request: DbEntityRequest, + ): Promise; + + entities(tx: unknown, filters?: EntityFilters): Promise; + + entity( + tx: unknown, + kind: string, + name: string, + namespace?: string, + ): Promise; + + removeEntity(tx: unknown, uid: string): Promise; + + addLocation(location: AddDatabaseLocation): Promise; + + removeLocation(id: string): Promise; + + location(id: string): Promise; + + locations(): Promise; + + locationHistory(id: string): Promise; + + addLocationUpdateLogEvent( + locationId: string, + status: DatabaseLocationUpdateLogStatus, + entityName?: string, + message?: string, + ): Promise; +}; diff --git a/plugins/catalog-backend/src/ingestion/IngestionModels.ts b/plugins/catalog-backend/src/ingestion/IngestionModels.ts index def6f1fa5c..8febdecd18 100644 --- a/plugins/catalog-backend/src/ingestion/IngestionModels.ts +++ b/plugins/catalog-backend/src/ingestion/IngestionModels.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import { EntityPolicy, EntityPolicies } from '@backstage/catalog-model'; +import { EntityPolicies, EntityPolicy } from '@backstage/catalog-model'; +import { DescriptorParsers } from './descriptor'; import { DescriptorParser, ReaderOutput } from './descriptor/parsers/types'; import { LocationReader, LocationReaders } from './source'; import { IngestionModel } from './types'; -import { DescriptorParsers } from './descriptor'; export class IngestionModels implements IngestionModel { private readonly reader: LocationReader; diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index 8c7946816c..4092ed1395 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -14,8 +14,8 @@ * limitations under the License. */ -import { getVoidLogger } from '@backstage/backend-common'; -import { Entity } from '@backstage/catalog-model'; +import { getVoidLogger, NotFoundError } from '@backstage/backend-common'; +import type { Entity } from '@backstage/catalog-model'; import express from 'express'; import request from 'supertest'; import { EntitiesCatalog, Location, LocationsCatalog } from '../catalog'; @@ -25,6 +25,9 @@ class MockEntitiesCatalog implements EntitiesCatalog { entities = jest.fn(); entityByUid = jest.fn(); entityByName = jest.fn(); + addEntity = jest.fn(); + addOrUpdateEntity = jest.fn(); + removeEntityByUid = jest.fn(); } class MockLocationsCatalog implements LocationsCatalog { @@ -36,7 +39,7 @@ class MockLocationsCatalog implements LocationsCatalog { } describe('createRouter', () => { - describe('entities', () => { + describe('GET /entities', () => { it('happy path: lists entities', async () => { const entities: Entity[] = [ { apiVersion: 'a', kind: 'b', metadata: { name: 'n' } }, @@ -77,7 +80,7 @@ describe('createRouter', () => { }); }); - describe('entityByUid', () => { + describe('GET /entities/by-uid/:uid', () => { it('can fetch entity by uid', async () => { const entity: Entity = { apiVersion: 'a', @@ -118,7 +121,7 @@ describe('createRouter', () => { }); }); - describe('entityByName', () => { + describe('GET /entities/by-name/:kind/:namespace/:name', () => { it('can fetch entity by name', async () => { const entity: Entity = { apiVersion: 'a', @@ -160,7 +163,91 @@ describe('createRouter', () => { }); }); - describe('locations', () => { + describe('POST /entities', () => { + it('requires a body', async () => { + const catalog = new MockEntitiesCatalog(); + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app) + .post('/entities') + .set('Content-Type', 'application/json') + .send(); + + expect(response.status).toEqual(400); + expect(response.text).toMatch(/body/); + expect(catalog.addOrUpdateEntity).not.toHaveBeenCalled(); + }); + + it('passes the body down', async () => { + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + namespace: 'd', + }, + }; + + const catalog = new MockEntitiesCatalog(); + catalog.addOrUpdateEntity.mockResolvedValue(entity); + + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app) + .post('/entities') + .send(entity) + .set('Content-Type', 'application/json'); + + expect(response.status).toEqual(200); + expect(response.body).toEqual(entity); + expect(catalog.addOrUpdateEntity).toHaveBeenCalledTimes(1); + expect(catalog.addOrUpdateEntity).toHaveBeenNthCalledWith(1, entity); + }); + }); + + describe('DELETE /entities/by-uid/:uid', () => { + it('can remove', async () => { + const catalog = new MockEntitiesCatalog(); + catalog.removeEntityByUid.mockResolvedValue(undefined); + + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).delete('/entities/by-uid/apa'); + + expect(response.status).toEqual(204); + expect(catalog.removeEntityByUid).toHaveBeenCalledTimes(1); + }); + + it('responds with a 404 for missing entities', async () => { + const catalog = new MockEntitiesCatalog(); + catalog.removeEntityByUid.mockRejectedValue(new NotFoundError('nope')); + + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).delete('/entities/by-uid/apa'); + + expect(response.status).toEqual(404); + expect(catalog.removeEntityByUid).toHaveBeenCalledTimes(1); + }); + }); + + describe('GET /locations', () => { it('happy path: lists locations', async () => { const locations: Location[] = [{ id: 'a', type: 'b', target: 'c' }]; @@ -178,7 +265,9 @@ describe('createRouter', () => { expect(response.status).toEqual(200); expect(response.body).toEqual(locations); }); + }); + describe('POST /locations', () => { it('rejects malformed locations', async () => { const location = ({ id: 'a', diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index 32bba44984..f5adbd84ca 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -15,16 +15,17 @@ */ import { errorHandler, InputError } from '@backstage/backend-common'; +import { Entity } from '@backstage/catalog-model'; import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; import { addLocationSchema, EntitiesCatalog, - EntityFilters, LocationsCatalog, } from '../catalog'; -import { validateRequestBody } from './util'; +import { EntityFilters } from '../database'; +import { requireRequestBody, validateRequestBody } from './util'; export interface RouterOptions { entitiesCatalog?: EntitiesCatalog; @@ -47,6 +48,11 @@ export async function createRouter( const entities = await entitiesCatalog.entities(filters); res.status(200).send(entities); }) + .post('/entities', async (req, res) => { + const body = await requireRequestBody(req); + const result = await entitiesCatalog.addOrUpdateEntity(body as Entity); + res.status(200).send(result); + }) .get('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; const entity = await entitiesCatalog.entityByUid(uid); @@ -55,6 +61,11 @@ export async function createRouter( } res.status(200).send(entity); }) + .delete('/entities/by-uid/:uid', async (req, res) => { + const { uid } = req.params; + await entitiesCatalog.removeEntityByUid(uid); + res.status(204).send(); + }) .get('/entities/by-name/:kind/:namespace/:name', async (req, res) => { const { kind, namespace, name } = req.params; const entity = await entitiesCatalog.entityByName( diff --git a/plugins/catalog-backend/src/service/util.ts b/plugins/catalog-backend/src/service/util.ts index 4c37154c48..39692030e3 100644 --- a/plugins/catalog-backend/src/service/util.ts +++ b/plugins/catalog-backend/src/service/util.ts @@ -16,12 +16,10 @@ import { InputError } from '@backstage/backend-common'; import { Request } from 'express'; +import lodash from 'lodash'; import yup from 'yup'; -export async function validateRequestBody( - req: Request, - schema: yup.Schema, -): Promise { +export async function requireRequestBody(req: Request): Promise { const contentType = req.header('content-type'); if (!contentType) { throw new InputError('Content-Type missing'); @@ -32,13 +30,27 @@ export async function validateRequestBody( const body = req.body; if (!body) { throw new InputError('Missing request body'); + } else if (!lodash.isPlainObject(body)) { + throw new InputError('Expected body to be a JSON object'); + } else if (Object.keys(body).length === 0) { + // Because of how express.json() translates the empty body to {} + throw new InputError('Empty request body'); } + return body; +} + +export async function validateRequestBody( + req: Request, + schema: yup.Schema, +): Promise { + const body = await requireRequestBody(req); + try { await schema.validate(body, { strict: true }); } catch (e) { throw new InputError(`Malformed request: ${e}`); } - return body as T; + return (body as unknown) as T; } From 0f41ac4bf7667854f4bd0c2b45b1e71e329b80d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 1 Jun 2020 09:01:32 +0200 Subject: [PATCH 2/3] Add the MockedMemberFunctions helper --- packages/backend-common/src/index.ts | 1 + .../src/testing/MockedMemberFunctions.ts | 35 +++++++++++++++++++ packages/backend-common/src/testing/index.ts | 17 +++++++++ .../catalog/DatabaseEntitiesCatalog.test.ts | 35 ++++++++++++------- 4 files changed, 76 insertions(+), 12 deletions(-) create mode 100644 packages/backend-common/src/testing/MockedMemberFunctions.ts create mode 100644 packages/backend-common/src/testing/index.ts diff --git a/packages/backend-common/src/index.ts b/packages/backend-common/src/index.ts index b2c38ab506..4bc60f557f 100644 --- a/packages/backend-common/src/index.ts +++ b/packages/backend-common/src/index.ts @@ -17,3 +17,4 @@ export * from './errors'; export * from './logging'; export * from './middleware'; +export * from './testing'; diff --git a/packages/backend-common/src/testing/MockedMemberFunctions.ts b/packages/backend-common/src/testing/MockedMemberFunctions.ts new file mode 100644 index 0000000000..9b35908bff --- /dev/null +++ b/packages/backend-common/src/testing/MockedMemberFunctions.ts @@ -0,0 +1,35 @@ +/* + * 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. + */ + +/** + * For any type T, generate a new type that is identical but also has the + * jest.fn signature on all member functions. + * + * When writing tests against a type, you sometimes end up in a situation where + * you need to write expect(x.y as jest.Mock).toHaveBeenCalled... because the + * x.y member was considered to be the actual function type. You could also + * change your test to instead create a "raw" object { y: jest.fn() } but then + * you lose type safety when doing x.y.mockReturnValue(...). So you start + * trying to do { y: jest.fn() as X['y'] } as X or similar trickery. + * + * This type lets you say const x: MockedMemberFunctions = { y: jest.fn() } + * and keep all the type safety at every step. + */ +export type MockedMemberFunctions = { + [K in keyof T]: T[K] extends (...args: infer A) => infer B + ? T[K] & jest.Mock + : T[K]; +}; diff --git a/packages/backend-common/src/testing/index.ts b/packages/backend-common/src/testing/index.ts new file mode 100644 index 0000000000..c12297465b --- /dev/null +++ b/packages/backend-common/src/testing/index.ts @@ -0,0 +1,17 @@ +/* + * 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. + */ + +export type { MockedMemberFunctions } from './MockedMemberFunctions'; diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts index d897584c95..c5101dff7a 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts @@ -14,20 +14,31 @@ * limitations under the License. */ +import type { MockedMemberFunctions } from '@backstage/backend-common'; import type { Entity, EntityPolicy } from '@backstage/catalog-model'; import type { Database } from '../database'; import { DatabaseEntitiesCatalog } from './DatabaseEntitiesCatalog'; describe('DatabaseEntitiesCatalog', () => { - let db: Database; + let db: MockedMemberFunctions; let policy: EntityPolicy; beforeEach(() => { - // Since the database has a large API surface, we just leave it empty and - // let the tests insert whatever methods they need to call - db = ({ - transaction: jest.fn(async f => f('mock_tx')), - } as unknown) as Database; + db = { + transaction: jest.fn(), + addEntity: jest.fn(), + updateEntity: jest.fn(), + entities: jest.fn(), + entity: jest.fn(), + removeEntity: jest.fn(), + addLocation: jest.fn(), + removeLocation: jest.fn(), + location: jest.fn(), + locations: jest.fn(), + locationHistory: jest.fn(), + addLocationUpdateLogEvent: jest.fn(), + }; + db.transaction.mockImplementation(async f => f('tx')); policy = { enforce: jest.fn(async x => x) }; }); @@ -42,8 +53,8 @@ describe('DatabaseEntitiesCatalog', () => { }, }; - db.entities = jest.fn().mockResolvedValue([]); - db.addEntity = jest.fn().mockResolvedValue({ entity }); + db.entities.mockResolvedValue([]); + db.addEntity.mockResolvedValue({ entity }); const catalog = new DatabaseEntitiesCatalog(db, policy); const result = await catalog.addOrUpdateEntity(entity); @@ -65,8 +76,8 @@ describe('DatabaseEntitiesCatalog', () => { }, }; - db.entities = jest.fn().mockResolvedValue([]); - db.updateEntity = jest.fn().mockResolvedValue({ entity }); + db.entities.mockResolvedValue([]); + db.updateEntity.mockResolvedValue({ entity }); const catalog = new DatabaseEntitiesCatalog(db, policy); const result = await catalog.addOrUpdateEntity(entity); @@ -95,8 +106,8 @@ describe('DatabaseEntitiesCatalog', () => { }, }; - db.entities = jest.fn().mockResolvedValue([{ entity: existing }]); - db.updateEntity = jest.fn().mockResolvedValue({ entity: added }); + db.entities.mockResolvedValue([{ entity: existing }]); + db.updateEntity.mockResolvedValue({ entity: added }); const catalog = new DatabaseEntitiesCatalog(db, policy); const result = await catalog.addOrUpdateEntity(added); From a90e6ab44ff3470d5bcbd7fe1ada2bfe7f21fafc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 1 Jun 2020 14:47:09 +0200 Subject: [PATCH 3/3] Update plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts Co-authored-by: Patrik Oldsberg --- packages/backend-common/src/index.ts | 1 - .../src/testing/MockedMemberFunctions.ts | 35 ------------------- packages/backend-common/src/testing/index.ts | 17 --------- .../catalog/DatabaseEntitiesCatalog.test.ts | 3 +- 4 files changed, 1 insertion(+), 55 deletions(-) delete mode 100644 packages/backend-common/src/testing/MockedMemberFunctions.ts delete mode 100644 packages/backend-common/src/testing/index.ts diff --git a/packages/backend-common/src/index.ts b/packages/backend-common/src/index.ts index 4bc60f557f..b2c38ab506 100644 --- a/packages/backend-common/src/index.ts +++ b/packages/backend-common/src/index.ts @@ -17,4 +17,3 @@ export * from './errors'; export * from './logging'; export * from './middleware'; -export * from './testing'; diff --git a/packages/backend-common/src/testing/MockedMemberFunctions.ts b/packages/backend-common/src/testing/MockedMemberFunctions.ts deleted file mode 100644 index 9b35908bff..0000000000 --- a/packages/backend-common/src/testing/MockedMemberFunctions.ts +++ /dev/null @@ -1,35 +0,0 @@ -/* - * 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. - */ - -/** - * For any type T, generate a new type that is identical but also has the - * jest.fn signature on all member functions. - * - * When writing tests against a type, you sometimes end up in a situation where - * you need to write expect(x.y as jest.Mock).toHaveBeenCalled... because the - * x.y member was considered to be the actual function type. You could also - * change your test to instead create a "raw" object { y: jest.fn() } but then - * you lose type safety when doing x.y.mockReturnValue(...). So you start - * trying to do { y: jest.fn() as X['y'] } as X or similar trickery. - * - * This type lets you say const x: MockedMemberFunctions = { y: jest.fn() } - * and keep all the type safety at every step. - */ -export type MockedMemberFunctions = { - [K in keyof T]: T[K] extends (...args: infer A) => infer B - ? T[K] & jest.Mock - : T[K]; -}; diff --git a/packages/backend-common/src/testing/index.ts b/packages/backend-common/src/testing/index.ts deleted file mode 100644 index c12297465b..0000000000 --- a/packages/backend-common/src/testing/index.ts +++ /dev/null @@ -1,17 +0,0 @@ -/* - * 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. - */ - -export type { MockedMemberFunctions } from './MockedMemberFunctions'; diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts index c5101dff7a..1de5038112 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts @@ -14,13 +14,12 @@ * limitations under the License. */ -import type { MockedMemberFunctions } from '@backstage/backend-common'; import type { Entity, EntityPolicy } from '@backstage/catalog-model'; import type { Database } from '../database'; import { DatabaseEntitiesCatalog } from './DatabaseEntitiesCatalog'; describe('DatabaseEntitiesCatalog', () => { - let db: MockedMemberFunctions; + let db: jest.Mocked; let policy: EntityPolicy; beforeEach(() => {