From 948052cbb4b692bb9a6ad444cc8f19431e763a12 Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Wed, 21 Oct 2020 09:34:43 +0200 Subject: [PATCH 1/6] feat: add ability to dry run adding a new location The location is now added in a transaction and afterwards rolled back. This allows users to dry run this operation to see if there entity has issues. This is probably done my automated tools. --- .changeset/angry-crabs-relate.md | 9 ++ .../features/software-catalog/installation.md | 1 + .../catalog/DatabaseEntitiesCatalog.test.ts | 46 +++++++-- .../src/catalog/DatabaseEntitiesCatalog.ts | 42 +++++--- .../src/catalog/DatabaseLocationsCatalog.ts | 18 +++- plugins/catalog-backend/src/catalog/types.ts | 14 ++- .../src/database/CommonDatabase.test.ts | 5 +- .../src/database/CommonDatabase.ts | 60 +++++++---- plugins/catalog-backend/src/database/index.ts | 1 + plugins/catalog-backend/src/database/types.ts | 33 +++++-- .../ingestion/HigherOrderOperations.test.ts | 70 ++++++++++++- .../src/ingestion/HigherOrderOperations.ts | 72 ++++++++------ .../catalog-backend/src/ingestion/types.ts | 5 +- .../src/service/CatalogBuilder.ts | 1 + .../src/service/router.test.ts | 99 +++++++++++++------ plugins/catalog-backend/src/service/router.ts | 40 ++++---- 16 files changed, 378 insertions(+), 138 deletions(-) create mode 100644 .changeset/angry-crabs-relate.md diff --git a/.changeset/angry-crabs-relate.md b/.changeset/angry-crabs-relate.md new file mode 100644 index 0000000000..66cdb29be5 --- /dev/null +++ b/.changeset/angry-crabs-relate.md @@ -0,0 +1,9 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +Add ability to dry run adding a new location ot the catalog API. + +The location is now added in a transaction and afterwards rolled back. +This allows users to dry run this operation to see if there entity has issues. +This is probably done by automated tools in the CI/CD pipeline. diff --git a/docs/features/software-catalog/installation.md b/docs/features/software-catalog/installation.md index 26990033aa..1e037461b4 100644 --- a/docs/features/software-catalog/installation.md +++ b/docs/features/software-catalog/installation.md @@ -122,6 +122,7 @@ export default async function createPlugin({ entitiesCatalog, locationsCatalog, locationReader, + db, logger, ); diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts index 6407000b6f..7e751bab92 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts @@ -16,12 +16,13 @@ import { getVoidLogger } from '@backstage/backend-common'; import type { Entity } from '@backstage/catalog-model'; -import { Database, DatabaseManager } from '../database'; +import { Database, DatabaseManager, Transaction } from '../database'; import { DatabaseEntitiesCatalog } from './DatabaseEntitiesCatalog'; import { EntityUpsertRequest } from './types'; describe('DatabaseEntitiesCatalog', () => { let db: jest.Mocked; + let transaction: jest.Mocked; beforeAll(() => { db = { @@ -40,11 +41,14 @@ describe('DatabaseEntitiesCatalog', () => { locationHistory: jest.fn(), addLocationUpdateLogEvent: jest.fn(), }; + transaction = { + rollback: jest.fn(), + }; }); beforeEach(() => { jest.resetAllMocks(); - db.transaction.mockImplementation(async f => f('tx')); + db.transaction.mockImplementation(async f => f(transaction)); }); describe('batchAddOrUpdateEntities', () => { @@ -82,6 +86,36 @@ describe('DatabaseEntitiesCatalog', () => { expect(result).toEqual([{ entityId: 'u' }]); }); + it('adds using existing transaction', async () => { + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + namespace: 'd', + }, + }; + + const existingTransaction: jest.Mocked = { + rollback: jest.fn(), + }; + + db.entities.mockResolvedValue([]); + db.addEntities.mockResolvedValue([{ entity }]); + + const catalog = new DatabaseEntitiesCatalog(db, getVoidLogger()); + const result = await catalog.addOrUpdateEntity(entity, { + tx: existingTransaction, + }); + + expect(db.addEntities).toHaveBeenCalledTimes(1); + expect(db.addEntities).toHaveBeenCalledWith( + existingTransaction, + expect.anything(), + ); + expect(result).toBe(entity); + }); + it('updates when given uid', async () => { const entity: Entity = { apiVersion: 'a', @@ -131,10 +165,10 @@ describe('DatabaseEntitiesCatalog', () => { ]); expect(db.entityByName).not.toHaveBeenCalled(); expect(db.entityByUid).toHaveBeenCalledTimes(1); - expect(db.entityByUid).toHaveBeenCalledWith(expect.anything(), 'u'); + expect(db.entityByUid).toHaveBeenCalledWith(transaction, 'u'); expect(db.updateEntity).toHaveBeenCalledTimes(1); expect(db.updateEntity).toHaveBeenCalledWith( - expect.anything(), + transaction, { entity: { apiVersion: 'a', @@ -206,14 +240,14 @@ describe('DatabaseEntitiesCatalog', () => { }, ]); expect(db.entityByName).toHaveBeenCalledTimes(1); - expect(db.entityByName).toHaveBeenCalledWith(expect.anything(), { + expect(db.entityByName).toHaveBeenCalledWith(transaction, { kind: 'b', namespace: 'd', name: 'c', }); expect(db.updateEntity).toHaveBeenCalledTimes(1); expect(db.updateEntity).toHaveBeenCalledWith( - expect.anything(), + transaction, { entity: { apiVersion: 'a', diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index 947736dc06..28a972e8e3 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -27,7 +27,12 @@ import { import { chunk, groupBy } from 'lodash'; import limiterFactory from 'p-limit'; import { Logger } from 'winston'; -import type { Database, DbEntityResponse, EntityFilters } from '../database'; +import type { + Database, + DbEntityResponse, + EntityFilters, + Transaction, +} from '../database'; import { durationText } from '../util/timing'; import type { EntitiesCatalog, @@ -60,7 +65,14 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { private readonly logger: Logger, ) {} - async entities(filters?: EntityFilters[]): Promise { + async entities(options?: { + filters?: EntityFilters[]; + tx?: Transaction; + }): Promise { + const filters = options?.filters; + + // TODO: Support with and without transaction! + const items = await this.database.transaction(tx => this.database.entities(tx, filters), ); @@ -135,8 +147,10 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { */ async batchAddOrUpdateEntities( requests: EntityUpsertRequest[], - locationId?: string, + options?: { locationId?: string; tx?: Transaction }, ): Promise { + // TODO: How to work with the transaction here? + // Group the entities by unique kind+namespace combinations const entitiesByKindAndNamespace = groupBy(requests, ({ entity }) => { const name = getEntityName(entity); @@ -163,7 +177,11 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { ); // Retry the batch write a few times to deal with contention - const context = { kind, namespace, locationId }; + const context = { + kind, + namespace, + locationId: options?.locationId, + }; for (let attempt = 1; attempt <= BATCH_ATTEMPTS; ++attempt) { try { const { toAdd, toUpdate, toIgnore } = await this.analyzeBatch( @@ -234,13 +252,15 @@ 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({ + filters: [ + { + kind: kind, + 'metadata.namespace': namespace, + 'metadata.name': names, + }, + ], + }); const oldEntitiesByName = new Map( oldEntities.map(e => [e.metadata.name, e]), diff --git a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts index f515829b91..b78b9933b9 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts @@ -15,7 +15,7 @@ */ import { Location } from '@backstage/catalog-model'; -import type { Database } from '../database'; +import type { Database, Transaction } from '../database'; import { DatabaseLocationUpdateLogEvent, DatabaseLocationUpdateLogStatus, @@ -25,9 +25,19 @@ import { LocationResponse, LocationsCatalog } from './types'; export class DatabaseLocationsCatalog implements LocationsCatalog { constructor(private readonly database: Database) {} - async addLocation(location: Location): Promise { - const added = await this.database.addLocation(location); - return added; + async addLocation( + location: Location, + options?: { tx?: Transaction }, + ): Promise { + const transaction = options?.tx; + + if (transaction) { + return await this.database.addLocation(transaction, location); + } + + return await this.database.transaction( + async tx => await this.database.addLocation(tx, location), + ); } async removeLocation(id: string): Promise { diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 539a0e531f..1ac06618b5 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -15,7 +15,7 @@ */ import { Entity, EntityRelationSpec, Location } from '@backstage/catalog-model'; -import type { EntityFilters } from '../database'; +import type { EntityFilters, Transaction } from '../database'; // // Entities @@ -31,7 +31,10 @@ export type EntityUpsertResponse = { }; export type EntitiesCatalog = { - entities(filters?: EntityFilters[]): Promise; + entities(options?: { + filters?: EntityFilters[]; + tx?: Transaction; + }): Promise; removeEntityByUid(uid: string): Promise; /** @@ -42,7 +45,7 @@ export type EntitiesCatalog = { */ batchAddOrUpdateEntities( entities: EntityUpsertRequest[], - locationId?: string, + options?: { locationId?: string; tx?: Transaction }, ): Promise; }; @@ -70,7 +73,10 @@ export type LocationResponse = { }; export type LocationsCatalog = { - addLocation(location: Location): Promise; + addLocation( + location: Location, + options?: { tx?: Transaction }, + ): Promise; removeLocation(id: string): Promise; locations(): Promise; location(id: string): Promise; diff --git a/plugins/catalog-backend/src/database/CommonDatabase.test.ts b/plugins/catalog-backend/src/database/CommonDatabase.test.ts index ac11f1dd1a..b2a238b111 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.test.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.test.ts @@ -91,7 +91,7 @@ describe('CommonDatabase', () => { timestamp: null, }; - await db.addLocation(input); + await db.transaction(async tx => await db.addLocation(tx, input)); const locations = await db.locations(); expect(locations).toEqual( @@ -238,7 +238,8 @@ describe('CommonDatabase', () => { type: 'a', target: 'b', }; - await db.addLocation(location); + + await db.transaction(async tx => await db.addLocation(tx, location)); await db.addLocationUpdateLogEvent( 'dd12620d-0436-422f-93bd-929aa0788123', diff --git a/plugins/catalog-backend/src/database/CommonDatabase.ts b/plugins/catalog-backend/src/database/CommonDatabase.ts index 72e6bf0a5f..bd6dc29abc 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.ts @@ -46,6 +46,7 @@ import { DbLocationsRow, DbLocationsRowWithStatus, EntityFilters, + Transaction, } from './types'; // The number of items that are sent per batch to the database layer, when @@ -63,9 +64,23 @@ export class CommonDatabase implements Database { private readonly logger: Logger, ) {} - async transaction(fn: (tx: unknown) => Promise): Promise { + async transaction(fn: (tx: Transaction) => Promise): Promise { try { - return await this.database.transaction(fn); + let result: T | undefined = undefined; + + await this.database.transaction( + async tx => { + // We can't return here, as knex swallows the return type in case the transaction is rolled back: + // https://github.com/knex/knex/blob/e37aeaa31c8ef9c1b07d2e4d3ec6607e557d800d/lib/transaction.js#L136 + result = await fn(tx); + }, + { + // If we explicitly trigger a rollback, don't fail. + doNotRejectOnRollback: true, + }, + ); + + return result!; } catch (e) { this.logger.debug(`Error during transaction, ${e}`); @@ -81,7 +96,7 @@ export class CommonDatabase implements Database { } async addEntity( - txOpaque: unknown, + txOpaque: Transaction, request: DbEntityRequest, ): Promise { const tx = txOpaque as Knex.Transaction; @@ -110,7 +125,7 @@ export class CommonDatabase implements Database { } async addEntities( - txOpaque: unknown, + txOpaque: Transaction, request: DbEntityRequest[], ): Promise { const tx = txOpaque as Knex.Transaction; @@ -158,7 +173,7 @@ export class CommonDatabase implements Database { } async updateEntity( - txOpaque: unknown, + txOpaque: Transaction, request: DbEntityRequest, matchingEtag?: string, matchingGeneration?: number, @@ -217,7 +232,7 @@ export class CommonDatabase implements Database { } async entities( - txOpaque: unknown, + txOpaque: Transaction, filters?: EntityFilters[], ): Promise { const tx = txOpaque as Knex.Transaction; @@ -295,7 +310,7 @@ export class CommonDatabase implements Database { } async entityByName( - txOpaque: unknown, + txOpaque: Transaction, name: EntityName, ): Promise { const tx = txOpaque as Knex.Transaction; @@ -314,7 +329,7 @@ export class CommonDatabase implements Database { } async entityByUid( - txOpaque: unknown, + txOpaque: Transaction, uid: string, ): Promise { const tx = txOpaque as Knex.Transaction; @@ -330,7 +345,7 @@ export class CommonDatabase implements Database { return this.toEntityResponse(tx, rows[0]); } - async removeEntityByUid(txOpaque: unknown, uid: string): Promise { + async removeEntityByUid(txOpaque: Transaction, uid: string): Promise { const tx = txOpaque as Knex.Transaction; const result = await tx('entities').where({ id: uid }).del(); @@ -341,7 +356,7 @@ export class CommonDatabase implements Database { } async setRelations( - txOpaque: unknown, + txOpaque: Transaction, originatingEntityId: string, relations: EntityRelationSpec[], ): Promise { @@ -368,19 +383,22 @@ export class CommonDatabase implements Database { await tx.batchInsert('entities_relations', relationsRows, BATCH_SIZE); } - async addLocation(location: Location): Promise { - return await this.database.transaction(async tx => { - const row: DbLocationsRow = { - id: location.id, - type: location.type, - target: location.target, - }; - await tx('locations').insert(row); - return row; - }); + async addLocation( + txOpaque: Transaction, + location: Location, + ): Promise { + const tx = txOpaque as Knex.Transaction; + + const row: DbLocationsRow = { + id: location.id, + type: location.type, + target: location.target, + }; + await tx('locations').insert(row); + return row; } - async removeLocation(txOpaque: unknown, id: string): Promise { + async removeLocation(txOpaque: Transaction, id: string): Promise { const tx = txOpaque as Knex.Transaction; await tx('entities') diff --git a/plugins/catalog-backend/src/database/index.ts b/plugins/catalog-backend/src/database/index.ts index 565a41cfb2..0d84106815 100644 --- a/plugins/catalog-backend/src/database/index.ts +++ b/plugins/catalog-backend/src/database/index.ts @@ -22,4 +22,5 @@ export type { DbEntityResponse, EntityFilter, EntityFilters, + Transaction, } from './types'; diff --git a/plugins/catalog-backend/src/database/types.ts b/plugins/catalog-backend/src/database/types.ts index b8a2a6231a..62b2d55b88 100644 --- a/plugins/catalog-backend/src/database/types.ts +++ b/plugins/catalog-backend/src/database/types.ts @@ -101,6 +101,13 @@ export type EntityFilter = null | string | (null | string)[]; */ export type EntityFilters = Record; +/** + * An abstraction for transactions of the underlying database technology. + */ +export type Transaction = { + rollback(): Promise; +}; + /** * An abstraction on top of the underlying database, wrapping the basic CRUD * needs. @@ -114,7 +121,7 @@ export type Database = { * * @param fn The callback that implements the transaction */ - transaction(fn: (tx: unknown) => Promise): Promise; + transaction(fn: (tx: Transaction) => Promise): Promise; /** * Adds a set of new entities to the catalog. @@ -123,7 +130,7 @@ export type Database = { * @param request The entities being added */ addEntities( - tx: unknown, + tx: Transaction, request: DbEntityRequest[], ): Promise; @@ -147,22 +154,28 @@ export type Database = { * @returns The updated entity */ updateEntity( - tx: unknown, + tx: Transaction, request: DbEntityRequest, matchingEtag?: string, matchingGeneration?: number, ): Promise; - entities(tx: unknown, filters?: EntityFilters[]): Promise; + entities( + tx: Transaction, + filters?: EntityFilters[], + ): Promise; entityByName( - tx: unknown, + tx: Transaction, name: EntityName, ): Promise; - entityByUid(tx: unknown, uid: string): Promise; + entityByUid( + tx: Transaction, + uid: string, + ): Promise; - removeEntityByUid(tx: unknown, uid: string): Promise; + removeEntityByUid(tx: Transaction, uid: string): Promise; /** * Remove current relations for the entity and replace them with the new relations array @@ -171,14 +184,14 @@ export type Database = { * @param relations the relationships to be set */ setRelations( - tx: unknown, + tx: Transaction, entityUid: string, relations: EntityRelationSpec[], ): Promise; - addLocation(location: Location): Promise; + addLocation(tx: Transaction, location: Location): Promise; - removeLocation(tx: unknown, id: string): Promise; + removeLocation(tx: Transaction, id: string): Promise; location(id: string): Promise; diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts index 933433b47e..174428799d 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts @@ -18,7 +18,11 @@ import { getVoidLogger } from '@backstage/backend-common'; import { Entity, Location, LocationSpec } from '@backstage/catalog-model'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; import { LocationUpdateStatus } from '../catalog/types'; -import { DatabaseLocationUpdateLogStatus } from '../database/types'; +import { + Database, + DatabaseLocationUpdateLogStatus, + Transaction, +} from '../database/types'; import { HigherOrderOperations } from './HigherOrderOperations'; import { LocationReader } from './types'; @@ -26,6 +30,8 @@ describe('HigherOrderOperations', () => { let entitiesCatalog: jest.Mocked; let locationsCatalog: jest.Mocked; let locationReader: jest.Mocked; + let transaction: jest.Mocked; + let database: jest.Mocked; let higherOrderOperation: HigherOrderOperations; beforeAll(() => { @@ -46,10 +52,29 @@ describe('HigherOrderOperations', () => { locationReader = { read: jest.fn(), }; + transaction = { + rollback: jest.fn(), + }; + database = { + transaction: jest.fn(), + addEntities: jest.fn(), + updateEntity: jest.fn(), + entities: jest.fn(), + entityByName: jest.fn(), + entityByUid: jest.fn(), + removeEntityByUid: jest.fn(), + addLocation: jest.fn(), + removeLocation: jest.fn(), + location: jest.fn(), + locations: jest.fn(), + locationHistory: jest.fn(), + addLocationUpdateLogEvent: jest.fn(), + }; higherOrderOperation = new HigherOrderOperations( entitiesCatalog, locationsCatalog, locationReader, + database, getVoidLogger(), ); }); @@ -64,6 +89,7 @@ describe('HigherOrderOperations', () => { type: 'a', target: 'b', }; + database.transaction.mockImplementation(x => x(transaction)); locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x)); locationsCatalog.locations.mockResolvedValue([]); locationReader.read.mockResolvedValue({ @@ -90,7 +116,9 @@ describe('HigherOrderOperations', () => { id: expect.anything(), ...spec, }), + { tx: transaction }, ); + expect(transaction.rollback).not.toBeCalled(); }); it('reuses the location if a match already existed', async () => { @@ -103,6 +131,7 @@ describe('HigherOrderOperations', () => { ...spec, }; + database.transaction.mockImplementation(x => x(transaction)); locationsCatalog.locations.mockResolvedValue([ { currentStatus: { timestamp: '', status: '', message: '' }, @@ -123,6 +152,7 @@ describe('HigherOrderOperations', () => { expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' }); expect(entitiesCatalog.batchAddOrUpdateEntities).not.toBeCalled(); expect(locationsCatalog.addLocation).not.toBeCalled(); + expect(transaction.rollback).not.toBeCalled(); }); it('rejects the whole operation if any entity could not be read', async () => { @@ -137,6 +167,7 @@ describe('HigherOrderOperations', () => { metadata: { name: 'n' }, }; + database.transaction.mockImplementation(x => x(transaction)); locationsCatalog.locations.mockResolvedValue([]); locationReader.read.mockResolvedValue({ entities: [{ entity, location, relations: [] }], @@ -149,6 +180,43 @@ describe('HigherOrderOperations', () => { expect(locationsCatalog.locations).toBeCalledTimes(1); expect(entitiesCatalog.batchAddOrUpdateEntities).not.toBeCalled(); expect(locationsCatalog.addLocation).not.toBeCalled(); + expect(transaction.rollback).not.toBeCalled(); + }); + + it('rollback everything after a dry run', async () => { + const spec = { + type: 'a', + target: 'b', + }; + database.transaction.mockImplementation(x => x(transaction)); + locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x)); + locationsCatalog.locations.mockResolvedValue([]); + locationReader.read.mockResolvedValue({ entities: [], errors: [] }); + + const result = await higherOrderOperation.addLocation(spec, { + dryRun: true, + }); + + expect(result.location).toEqual( + expect.objectContaining({ + id: expect.anything(), + ...spec, + }), + ); + expect(result.entities).toEqual([]); + expect(locationsCatalog.locations).toBeCalledTimes(1); + expect(locationReader.read).toBeCalledTimes(1); + expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' }); + expect(entitiesCatalog.addOrUpdateEntity).not.toBeCalled(); + expect(locationsCatalog.addLocation).toBeCalledTimes(1); + expect(locationsCatalog.addLocation).toBeCalledWith( + expect.objectContaining({ + id: expect.anything(), + ...spec, + }), + { tx: transaction }, + ); + expect(transaction.rollback).toBeCalled(); }); }); diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts index 0e62815be5..2ca79c60b4 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts @@ -17,6 +17,7 @@ import { Location, LocationSpec } from '@backstage/catalog-model'; import { v4 as uuidv4 } from 'uuid'; import { Logger } from 'winston'; +import { Database } from '../database'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; import { durationText } from '../util/timing'; import { @@ -33,22 +34,13 @@ import { * database more directly. */ export class HigherOrderOperations implements HigherOrderOperation { - private readonly entitiesCatalog: EntitiesCatalog; - private readonly locationsCatalog: LocationsCatalog; - private readonly locationReader: LocationReader; - private readonly logger: Logger; - constructor( - entitiesCatalog: EntitiesCatalog, - locationsCatalog: LocationsCatalog, - locationReader: LocationReader, - logger: Logger, - ) { - this.entitiesCatalog = entitiesCatalog; - this.locationsCatalog = locationsCatalog; - this.locationReader = locationReader; - this.logger = logger; - } + private readonly entitiesCatalog: EntitiesCatalog, + private readonly locationsCatalog: LocationsCatalog, + private readonly locationReader: LocationReader, + private readonly database: Database, + private readonly logger: Logger, + ) {} /** * Adds a single location to the catalog. @@ -62,7 +54,12 @@ export class HigherOrderOperations implements HigherOrderOperation { * * @param spec The location to add */ - async addLocation(spec: LocationSpec): Promise { + async addLocation( + spec: LocationSpec, + options?: { dryRun?: boolean }, + ): Promise { + const dryRun = options?.dryRun || false; + // Attempt to find a previous location matching the spec const previousLocations = await this.locationsCatalog.locations(); const previousLocation = previousLocations.find( @@ -88,21 +85,36 @@ export class HigherOrderOperations implements HigherOrderOperation { // in the entities list. But we aren't sure what to do about those yet. // Write - if (!previousLocation) { - await this.locationsCatalog.addLocation(location); - } - if (readerOutput.entities.length === 0) { - return { location, entities: [] }; - } + const entities = await this.database.transaction(async tx => { + if (!previousLocation) { + await this.locationsCatalog.addLocation(location, { tx }); + } + if (readerOutput.entities.length === 0) { + return { location, entities: [] }; + } - const writtenEntities = await this.entitiesCatalog.batchAddOrUpdateEntities( - readerOutput.entities, - location.id, - ); + const writtenEntities = await this.entitiesCatalog.batchAddOrUpdateEntities( + readerOutput.entities, + { + locationId: location.id, + tx, + }, + ); - const entities = await this.entitiesCatalog.entities([ - { 'metadata.uid': writtenEntities.map(e => e.entityId) }, - ]); + const outputEntities = await this.entitiesCatalog.entities( + [{ 'metadata.uid': writtenEntities.map(e => e.entityId) }], + { tx }, + ); + + if (dryRun) { + // If this is only a dry run, cancel the database transaction even if it was successful. + await tx.rollback(); + + this.logger.debug(`Perfomed successful dry run of adding a location`); + } + + return outputEntities; + }); return { location, entities }; } @@ -168,7 +180,7 @@ export class HigherOrderOperations implements HigherOrderOperation { try { await this.entitiesCatalog.batchAddOrUpdateEntities( readerOutput.entities, - location.id, + { locationId: location.id }, ); } catch (e) { for (const entity of readerOutput.entities) { diff --git a/plugins/catalog-backend/src/ingestion/types.ts b/plugins/catalog-backend/src/ingestion/types.ts index c586e2d438..a83b4adecc 100644 --- a/plugins/catalog-backend/src/ingestion/types.ts +++ b/plugins/catalog-backend/src/ingestion/types.ts @@ -26,7 +26,10 @@ import type { // export type HigherOrderOperation = { - addLocation(spec: LocationSpec): Promise; + addLocation( + spec: LocationSpec, + options?: { dryRun?: boolean }, + ): Promise; refreshAllLocations(): Promise; }; diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index b041dac5ab..f2a4ac7468 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -227,6 +227,7 @@ export class CatalogBuilder { entitiesCatalog, locationsCatalog, locationReader, + db, logger, ); diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index a6b22eaad9..3a43ac96ad 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -23,6 +23,8 @@ import { LocationResponse } from '../catalog/types'; import { HigherOrderOperation } from '../ingestion/types'; import { createRouter } from './router'; +// TODO: ... + describe('createRouter', () => { let entitiesCatalog: jest.Mocked; let locationsCatalog: jest.Mocked; @@ -83,13 +85,15 @@ describe('createRouter', () => { 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({ + filters: [ + { + a: ['1', null, '3'], + b: ['4'], + }, + { c: [null] }, + ], + }); }); }); @@ -107,9 +111,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({ + filters: [{ 'metadata.uid': 'zzz' }], + }); expect(response.status).toEqual(200); expect(response.body).toEqual(expect.objectContaining(entity)); }); @@ -120,9 +124,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({ + filters: [{ 'metadata.uid': 'zzz' }], + }); expect(response.status).toEqual(404); expect(response.text).toMatch(/uid/); }); @@ -143,13 +147,15 @@ 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({ + filters: [ + { + kind: 'k', + 'metadata.namespace': 'ns', + 'metadata.name': 'n', + }, + ], + }); expect(response.status).toEqual(200); expect(response.body).toEqual(expect.objectContaining(entity)); }); @@ -160,13 +166,15 @@ 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({ + filters: [ + { + kind: 'b', + 'metadata.namespace': 'd', + 'metadata.name': 'c', + }, + ], + }); expect(response.status).toEqual(404); expect(response.text).toMatch(/name/); }); @@ -209,9 +217,9 @@ describe('createRouter', () => { { entity, relations: [] }, ]); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith([ - { 'metadata.uid': 'u' }, - ]); + expect(entitiesCatalog.entities).toHaveBeenCalledWith({ + filters: [{ 'metadata.uid': 'u' }], + }); expect(response.status).toEqual(200); expect(response.body).toEqual(entity); }); @@ -285,7 +293,36 @@ describe('createRouter', () => { const response = await request(app).post('/locations').send(spec); expect(higherOrderOperation.addLocation).toHaveBeenCalledTimes(1); - expect(higherOrderOperation.addLocation).toHaveBeenCalledWith(spec); + expect(higherOrderOperation.addLocation).toHaveBeenCalledWith(spec, { + dryRun: false, + }); + expect(response.status).toEqual(201); + expect(response.body).toEqual( + expect.objectContaining({ + location: { id: 'a', ...spec }, + }), + ); + }); + + it('supports dry run', async () => { + const spec: LocationSpec = { + type: 'b', + target: 'c', + }; + + higherOrderOperation.addLocation.mockResolvedValue({ + location: { id: 'a', ...spec }, + entities: [], + }); + + const response = await request(app) + .post('/locations?dryRun=true') + .send(spec); + + expect(higherOrderOperation.addLocation).toHaveBeenCalledTimes(1); + expect(higherOrderOperation.addLocation).toHaveBeenCalledWith(spec, { + dryRun: true, + }); expect(response.status).toEqual(201); expect(response.body).toEqual( expect.objectContaining({ diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index 785e44f68c..e297bf16c6 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -20,6 +20,7 @@ import { locationSpecSchema } from '@backstage/catalog-model'; import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; +import yn from 'yn'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; import { HigherOrderOperation } from '../ingestion/types'; import { @@ -48,7 +49,7 @@ export async function createRouter( .get('/entities', async (req, res) => { const filters = translateQueryToEntityFilters(req.query); const fieldMapper = translateQueryToFieldMapper(req.query); - const entities = await entitiesCatalog.entities(filters); + const entities = await entitiesCatalog.entities({ filters }); res.status(200).send(entities.map(fieldMapper)); }) .post('/entities', async (req, res) => { @@ -56,18 +57,20 @@ 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({ + filters: [{ '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({ + filters: [ + { + 'metadata.uid': uid, + }, + ], + }); if (!entities.length) { res.status(404).send(`No entity with uid ${uid}`); } @@ -80,13 +83,15 @@ 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({ + filters: [ + { + kind: kind, + 'metadata.namespace': namespace, + 'metadata.name': name, + }, + ], + }); if (!entities.length) { res .status(404) @@ -101,7 +106,8 @@ export async function createRouter( if (higherOrderOperation) { router.post('/locations', async (req, res) => { const input = await validateRequestBody(req, locationSpecSchema); - const output = await higherOrderOperation.addLocation(input); + const dryRun = yn(req.query.dryRun, { default: false }); + const output = await higherOrderOperation.addLocation(input, { dryRun }); res.status(201).send(output); }); } From 2ebcfac8d4487313c9d4bd90225395965b5945b8 Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Wed, 21 Oct 2020 09:37:25 +0200 Subject: [PATCH 2/6] feat: add a validate button to the register-component page This allows the user to validate his location before adding it. --- .changeset/angry-crabs-relate.md | 2 +- .changeset/nervous-drinks-notice.md | 8 ++ .../catalog/DatabaseEntitiesCatalog.test.ts | 30 +++-- .../src/catalog/DatabaseEntitiesCatalog.ts | 126 +++++++++++------- plugins/catalog-backend/src/catalog/types.ts | 5 +- .../ingestion/HigherOrderOperations.test.ts | 99 ++++++++++++-- .../src/ingestion/HigherOrderOperations.ts | 15 +-- .../src/service/router.test.ts | 2 - plugins/catalog/src/api/CatalogClient.ts | 5 +- plugins/catalog/src/api/types.ts | 1 + .../RegisterComponentForm.test.tsx | 10 +- .../RegisterComponentForm.tsx | 48 +++++-- .../RegisterComponentPage.tsx | 12 +- .../RegisterComponentResultDialog.tsx | 115 +++++++++------- 14 files changed, 327 insertions(+), 151 deletions(-) create mode 100644 .changeset/nervous-drinks-notice.md diff --git a/.changeset/angry-crabs-relate.md b/.changeset/angry-crabs-relate.md index 66cdb29be5..678093c1ac 100644 --- a/.changeset/angry-crabs-relate.md +++ b/.changeset/angry-crabs-relate.md @@ -2,7 +2,7 @@ '@backstage/plugin-catalog-backend': minor --- -Add ability to dry run adding a new location ot the catalog API. +Add ability to dry run adding a new location to the catalog API. The location is now added in a transaction and afterwards rolled back. This allows users to dry run this operation to see if there entity has issues. diff --git a/.changeset/nervous-drinks-notice.md b/.changeset/nervous-drinks-notice.md new file mode 100644 index 0000000000..7c6af21f64 --- /dev/null +++ b/.changeset/nervous-drinks-notice.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-catalog': minor +'@backstage/plugin-register-component': minor +--- + +Add a validate button to the register-component page + +This allows the user to validate his location before adding it. diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts index 7e751bab92..90b11b179d 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts @@ -95,25 +95,33 @@ describe('DatabaseEntitiesCatalog', () => { namespace: 'd', }, }; - const existingTransaction: jest.Mocked = { rollback: jest.fn(), }; db.entities.mockResolvedValue([]); - db.addEntities.mockResolvedValue([{ entity }]); + db.addEntities.mockResolvedValue([ + { entity: { ...entity, metadata: { ...entity.metadata, uid: 'u' } } }, + ]); const catalog = new DatabaseEntitiesCatalog(db, getVoidLogger()); - const result = await catalog.addOrUpdateEntity(entity, { - tx: existingTransaction, - }); - - expect(db.addEntities).toHaveBeenCalledTimes(1); - expect(db.addEntities).toHaveBeenCalledWith( - existingTransaction, - expect.anything(), + const result = await catalog.batchAddOrUpdateEntities( + [{ entity, relations: [] }], + { tx: existingTransaction }, ); - expect(result).toBe(entity); + + expect(db.entities).toHaveBeenCalledTimes(1); + 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); + expect(result).toEqual([{ entityId: 'u' }]); }); it('updates when given uid', async () => { diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index 28a972e8e3..53fb50677b 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -70,46 +70,44 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { tx?: Transaction; }): Promise { const filters = options?.filters; - - // TODO: Support with and without transaction! - - const items = await this.database.transaction(tx => - this.database.entities(tx, filters), - ); + const items = options?.tx + ? await this.database.entities(options?.tx, filters) + : await this.database.transaction(tx => + this.database.entities(tx, filters), + ); return items.map(i => i.entity); } private async addOrUpdateEntity( entity: Entity, + tx: Transaction, locationId?: string, ): Promise { - return await this.database.transaction(async tx => { - // Find a matching (by uid, or by compound name, depending on the given - // entity) existing entity, to know whether to update or add - const existing = entity.metadata.uid - ? await this.database.entityByUid(tx, entity.metadata.uid) - : await this.database.entityByName(tx, getEntityName(entity)); + // Find a matching (by uid, or by compound name, depending on the given + // entity) existing entity, to know whether to update or add + const existing = entity.metadata.uid + ? await this.database.entityByUid(tx, entity.metadata.uid) + : await this.database.entityByName(tx, getEntityName(entity)); - // If it's an update, run the algorithm for annotation merging, updating - // etag/generation, etc. - let response: DbEntityResponse; - if (existing) { - const updated = generateUpdatedEntity(existing.entity, entity); - response = await this.database.updateEntity( - tx, - { locationId, entity: updated }, - existing.entity.metadata.etag, - existing.entity.metadata.generation, - ); - } else { - const added = await this.database.addEntities(tx, [ - { locationId, entity }, - ]); - response = added[0]; - } + // If it's an update, run the algorithm for annotation merging, updating + // etag/generation, etc. + let response: DbEntityResponse; + if (existing) { + const updated = generateUpdatedEntity(existing.entity, entity); + response = await this.database.updateEntity( + tx, + { locationId, entity: updated }, + existing.entity.metadata.etag, + existing.entity.metadata.generation, + ); + } else { + const added = await this.database.addEntities(tx, [ + { locationId, entity }, + ]); + response = added[0]; + } - return response.entity; - }); + return response.entity; } async removeEntityByUid(uid: string): Promise { @@ -143,14 +141,41 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { * Writes a number of entities efficiently to storage. * * @param entities Some entities - * @param locationId The location that they all belong to + * @param options.locationId The location that they all belong to + * @param options.tx A database transaction to execute the queries in */ async batchAddOrUpdateEntities( requests: EntityUpsertRequest[], - options?: { locationId?: string; tx?: Transaction }, + options?: { + locationId?: string; + tx?: Transaction; + }, ): Promise { - // TODO: How to work with the transaction here? + const locationId = options?.locationId; + if (options?.tx) { + return this.batchAddOrUpdateEntitiesInTransaction( + requests, + options.tx, + locationId, + ); + } + + return await this.database.transaction( + async tx => + await this.batchAddOrUpdateEntitiesInTransaction( + requests, + tx, + locationId, + ), + ); + } + + private async batchAddOrUpdateEntitiesInTransaction( + requests: EntityUpsertRequest[], + tx: Transaction, + locationId?: string, + ): Promise { // Group the entities by unique kind+namespace combinations const entitiesByKindAndNamespace = groupBy(requests, ({ entity }) => { const name = getEntityName(entity); @@ -180,29 +205,30 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { const context = { kind, namespace, - locationId: options?.locationId, + locationId, }; for (let attempt = 1; attempt <= BATCH_ATTEMPTS; ++attempt) { try { const { toAdd, toUpdate, toIgnore } = await this.analyzeBatch( batch, context, + tx, ); if (toAdd.length) { modifiedEntityIds.push( - ...(await this.batchAdd(toAdd, context)), + ...(await this.batchAdd(toAdd, context, tx)), ); } if (toUpdate.length) { modifiedEntityIds.push( - ...(await this.batchUpdate(toUpdate, context)), + ...(await this.batchUpdate(toUpdate, context, tx)), ); } // TODO(Rugvip): We currently always update relations, but we // likely want to figure out a way to avoid that for (const { entity, relations } of toIgnore) { const entityId = entity.metadata.uid!; - await this.setRelations(entityId, relations); + await this.setRelations(entityId, relations, tx); modifiedEntityIds.push({ entityId }); } @@ -231,10 +257,9 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { private async setRelations( originatingEntityId: string, relations: EntityRelationSpec[], + tx: Transaction, ): Promise { - return await this.database.transaction(tx => - this.database.setRelations(tx, originatingEntityId, relations), - ); + await this.database.setRelations(tx, originatingEntityId, relations); } // Given a batch of entities that were just read from a location, take them @@ -244,6 +269,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { private async analyzeBatch( requests: EntityUpsertRequest[], { kind, namespace }: BatchContext, + tx: Transaction, ): Promise<{ toAdd: EntityUpsertRequest[]; toUpdate: EntityUpsertRequest[]; @@ -260,6 +286,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { 'metadata.name': names, }, ], + tx, }); const oldEntitiesByName = new Map( @@ -299,15 +326,13 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { private async batchAdd( requests: EntityUpsertRequest[], { locationId }: BatchContext, + tx: Transaction, ): Promise { const markTimestamp = process.hrtime(); - const res = await this.database.transaction( - async tx => - await this.database.addEntities( - tx, - requests.map(({ entity }) => ({ locationId, entity })), - ), + const res = await this.database.addEntities( + tx, + requests.map(({ entity }) => ({ locationId, entity })), ); const entityIds = res.map(({ entity }) => ({ @@ -315,7 +340,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { })); for (const [index, { entityId }] of entityIds.entries()) { - await this.setRelations(entityId, requests[index].relations); + await this.setRelations(entityId, requests[index].relations, tx); } this.logger.debug( @@ -330,15 +355,16 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { private async batchUpdate( requests: EntityUpsertRequest[], { locationId }: BatchContext, + tx: Transaction, ): Promise { const markTimestamp = process.hrtime(); const responseIds: EntityUpsertResponse[] = []; // TODO(freben): Still not batched for (const entity of requests) { - const res = await this.addOrUpdateEntity(entity.entity, locationId); + const res = await this.addOrUpdateEntity(entity.entity, tx, locationId); const entityId = res.metadata.uid!; responseIds.push({ entityId }); - await this.setRelations(entityId, entity.relations); + await this.setRelations(entityId, entity.relations, tx); } this.logger.debug( diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 1ac06618b5..6f2977619a 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -45,7 +45,10 @@ export type EntitiesCatalog = { */ batchAddOrUpdateEntities( entities: EntityUpsertRequest[], - options?: { locationId?: string; tx?: Transaction }, + options?: { + tx?: Transaction; + locationId?: string; + }, ): Promise; }; diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts index 174428799d..da722e56bc 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts @@ -63,6 +63,7 @@ describe('HigherOrderOperations', () => { entityByName: jest.fn(), entityByUid: jest.fn(), removeEntityByUid: jest.fn(), + setRelations: jest.fn(), addLocation: jest.fn(), removeLocation: jest.fn(), location: jest.fn(), @@ -81,6 +82,7 @@ describe('HigherOrderOperations', () => { beforeEach(() => { jest.resetAllMocks(); + database.transaction.mockImplementation(async f => f(transaction)); }); describe('addLocation', () => { @@ -89,7 +91,6 @@ describe('HigherOrderOperations', () => { type: 'a', target: 'b', }; - database.transaction.mockImplementation(x => x(transaction)); locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x)); locationsCatalog.locations.mockResolvedValue([]); locationReader.read.mockResolvedValue({ @@ -121,6 +122,62 @@ describe('HigherOrderOperations', () => { expect(transaction.rollback).not.toBeCalled(); }); + it('insert the location and its entities', async () => { + const spec = { + type: 'a', + target: 'b', + }; + const location: LocationSpec = { type: '', target: '' }; + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { name: 'n' }, + }; + locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x)); + locationsCatalog.locations.mockResolvedValue([]); + locationsCatalog.locations.mockResolvedValue([]); + entitiesCatalog.batchAddOrUpdateEntities.mockResolvedValue([ + { + entityId: 'id', + }, + ]); + entitiesCatalog.entities.mockResolvedValue([entity]); + locationReader.read.mockResolvedValue({ + entities: [ + { + location, + entity, + relations: [], + }, + ], + errors: [], + }); + + const result = await higherOrderOperation.addLocation(spec); + + expect(result.location).toEqual( + expect.objectContaining({ + id: expect.anything(), + ...spec, + }), + ); + expect(result.entities).toEqual([entity]); + expect(locationsCatalog.locations).toBeCalledTimes(1); + expect(locationsCatalog.addLocation).toBeCalledTimes(1); + expect(locationsCatalog.addLocation).toBeCalledWith( + expect.objectContaining({ + id: expect.anything(), + ...spec, + }), + { tx: transaction }, + ); + expect(locationReader.read).toBeCalledTimes(1); + expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' }); + expect(entitiesCatalog.entities).toBeCalledTimes(1); + expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledTimes(1); + expect(transaction.rollback).not.toBeCalled(); + }); + it('reuses the location if a match already existed', async () => { const spec = { type: 'a', @@ -131,7 +188,6 @@ describe('HigherOrderOperations', () => { ...spec, }; - database.transaction.mockImplementation(x => x(transaction)); locationsCatalog.locations.mockResolvedValue([ { currentStatus: { timestamp: '', status: '', message: '' }, @@ -167,7 +223,6 @@ describe('HigherOrderOperations', () => { metadata: { name: 'n' }, }; - database.transaction.mockImplementation(x => x(transaction)); locationsCatalog.locations.mockResolvedValue([]); locationReader.read.mockResolvedValue({ entities: [{ entity, location, relations: [] }], @@ -188,10 +243,31 @@ describe('HigherOrderOperations', () => { type: 'a', target: 'b', }; - database.transaction.mockImplementation(x => x(transaction)); + const location: LocationSpec = { type: '', target: '' }; + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { name: 'n' }, + }; locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x)); locationsCatalog.locations.mockResolvedValue([]); - locationReader.read.mockResolvedValue({ entities: [], errors: [] }); + locationsCatalog.locations.mockResolvedValue([]); + entitiesCatalog.batchAddOrUpdateEntities.mockResolvedValue([ + { + entityId: 'id', + }, + ]); + entitiesCatalog.entities.mockResolvedValue([entity]); + locationReader.read.mockResolvedValue({ + entities: [ + { + location, + entity, + relations: [], + }, + ], + errors: [], + }); const result = await higherOrderOperation.addLocation(spec, { dryRun: true, @@ -203,11 +279,8 @@ describe('HigherOrderOperations', () => { ...spec, }), ); - expect(result.entities).toEqual([]); + expect(result.entities).toEqual([entity]); expect(locationsCatalog.locations).toBeCalledTimes(1); - expect(locationReader.read).toBeCalledTimes(1); - expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' }); - expect(entitiesCatalog.addOrUpdateEntity).not.toBeCalled(); expect(locationsCatalog.addLocation).toBeCalledTimes(1); expect(locationsCatalog.addLocation).toBeCalledWith( expect.objectContaining({ @@ -216,6 +289,10 @@ describe('HigherOrderOperations', () => { }), { tx: transaction }, ); + expect(locationReader.read).toBeCalledTimes(1); + expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' }); + expect(entitiesCatalog.entities).toBeCalledTimes(1); + expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledTimes(1); expect(transaction.rollback).toBeCalled(); }); }); @@ -281,7 +358,9 @@ describe('HigherOrderOperations', () => { relations: [], }), ], - '123', + { + locationId: '123', + }, ); }); diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts index 2ca79c60b4..ac21fc6155 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts @@ -90,21 +90,18 @@ export class HigherOrderOperations implements HigherOrderOperation { await this.locationsCatalog.addLocation(location, { tx }); } if (readerOutput.entities.length === 0) { - return { location, entities: [] }; + return []; } const writtenEntities = await this.entitiesCatalog.batchAddOrUpdateEntities( readerOutput.entities, - { - locationId: location.id, - tx, - }, + { locationId: location.id, tx }, ); - const outputEntities = await this.entitiesCatalog.entities( - [{ 'metadata.uid': writtenEntities.map(e => e.entityId) }], - { tx }, - ); + const outputEntities = await this.entitiesCatalog.entities({ + filters: [{ 'metadata.uid': writtenEntities.map(e => e.entityId) }], + tx, + }); if (dryRun) { // If this is only a dry run, cancel the database transaction even if it was successful. diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index 3a43ac96ad..95dc8e82d1 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -23,8 +23,6 @@ import { LocationResponse } from '../catalog/types'; import { HigherOrderOperation } from '../ingestion/types'; import { createRouter } from './router'; -// TODO: ... - describe('createRouter', () => { let entitiesCatalog: jest.Mocked; let locationsCatalog: jest.Mocked; diff --git a/plugins/catalog/src/api/CatalogClient.ts b/plugins/catalog/src/api/CatalogClient.ts index 40622da60a..85fd44cf01 100644 --- a/plugins/catalog/src/api/CatalogClient.ts +++ b/plugins/catalog/src/api/CatalogClient.ts @@ -95,9 +95,12 @@ export class CatalogClient implements CatalogApi { async addLocation({ type = 'url', target, + dryRun, }: AddLocationRequest): Promise { const response = await fetch( - `${await this.discoveryApi.getBaseUrl('catalog')}/locations`, + `${await this.discoveryApi.getBaseUrl('catalog')}/locations${ + dryRun ? '?dryRun=true' : '' + }`, { headers: { 'Content-Type': 'application/json', diff --git a/plugins/catalog/src/api/types.ts b/plugins/catalog/src/api/types.ts index 324a8d43fd..06030e6ac5 100644 --- a/plugins/catalog/src/api/types.ts +++ b/plugins/catalog/src/api/types.ts @@ -43,6 +43,7 @@ export interface CatalogApi { export type AddLocationRequest = { type?: string; target: string; + dryRun?: boolean; }; export type AddLocationResponse = { diff --git a/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.test.tsx b/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.test.tsx index 875b1f8660..fae5d649b9 100644 --- a/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.test.tsx +++ b/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.test.tsx @@ -20,17 +20,18 @@ import React from 'react'; import { RegisterComponentForm } from './RegisterComponentForm'; describe('RegisterComponentForm', () => { - it('should initially render a disabled button', async () => { + it('should initially render disabled buttons', async () => { render(); expect( await screen.findByText(/Enter the full path to the catalog-info.yaml/), ).toBeInTheDocument(); - expect(screen.getByText('Submit').closest('button')).toBeDisabled(); + expect(screen.getByText('Validate').closest('button')).toBeDisabled(); + expect(screen.getByText('Register').closest('button')).toBeDisabled(); }); - it('should enable a submit button when the target url is set', async () => { + it('should enable the submit buttons when the target url is set', async () => { render(); await act(async () => { @@ -40,7 +41,8 @@ describe('RegisterComponentForm', () => { ); }); - expect(screen.getByText('Submit').closest('button')).not.toBeDisabled(); + expect(screen.getByText('Validate').closest('button')).not.toBeDisabled(); + expect(screen.getByText('Register').closest('button')).not.toBeDisabled(); }); it('should show spinner while submitting', async () => { diff --git a/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.tsx b/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.tsx index 56dd50ea67..de183869f7 100644 --- a/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.tsx +++ b/plugins/register-component/src/components/RegisterComponentForm/RegisterComponentForm.tsx @@ -33,8 +33,11 @@ const useStyles = makeStyles(theme => ({ display: 'flex', flexFlow: 'column nowrap', }, - submit: { - marginTop: theme.spacing(1), + buttonSpacing: { + marginLeft: theme.spacing(1), + }, + buttons: { + marginTop: theme.spacing(2), }, select: { minWidth: 120, @@ -54,12 +57,21 @@ export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => { const hasErrors = !!errors.entityLocation; const dirty = formState?.isDirty; + const onSubmitValidate = handleSubmit(data => { + data.mode = 'validate'; + onSubmit(data); + }); + + const onSubmitRegister = handleSubmit(data => { + data.mode = 'register'; + onSubmit(data); + }); + return submitting ? ( ) : (
@@ -87,15 +99,27 @@ export const RegisterComponentForm = ({ onSubmit, submitting }: Props) => { )} - +
+ + +
); }; diff --git a/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.tsx b/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.tsx index c7f94e2060..d0b56598b2 100644 --- a/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.tsx +++ b/plugins/register-component/src/components/RegisterComponentPage/RegisterComponentPage.tsx @@ -75,27 +75,30 @@ export const RegisterComponentPage = ({ location: Location; } | null; error: null | Error; + dryRun: boolean; }>({ data: null, error: null, + dryRun: false, }); const handleSubmit = async (formData: Record) => { setFormState(FormStates.Submitting); - const { entityLocation: target } = formData; + const { entityLocation: target, mode } = formData; + const dryRun = mode === 'validate'; try { - const data = await catalogApi.addLocation({ target }); + const data = await catalogApi.addLocation({ target, dryRun }); if (!isMounted()) return; - setResult({ error: null, data }); + setResult({ error: null, data, dryRun }); setFormState(FormStates.Success); } catch (e) { errorApi.post(e); if (!isMounted()) return; - setResult({ error: e, data: null }); + setResult({ error: e, data: null, dryRun }); setFormState(FormStates.Idle); } }; @@ -124,6 +127,7 @@ export const RegisterComponentPage = ({ {formState === FormStates.Success && ( setFormState(FormStates.Idle)} classes={{ paper: classes.dialogPaper }} catalogRouteRef={catalogRouteRef} diff --git a/plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.tsx b/plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.tsx index 9631865ffb..2b57050661 100644 --- a/plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.tsx +++ b/plugins/register-component/src/components/RegisterComponentResultDialog/RegisterComponentResultDialog.tsx @@ -29,7 +29,7 @@ import { List, ListItem, } from '@material-ui/core'; -import React from 'react'; +import React, { useState } from 'react'; import { generatePath, resolvePath } from 'react-router'; import { Link as RouterLink } from 'react-router-dom'; @@ -37,6 +37,7 @@ type Props = { onClose: () => void; classes?: Record; entities: Entity[]; + dryRun?: boolean; catalogRouteRef: RouteRef; }; @@ -64,49 +65,71 @@ export const RegisterComponentResultDialog = ({ onClose, classes, entities, + dryRun, catalogRouteRef, -}: Props) => ( - - Registration Result - - - The following entities have been successfully created: - - - {entities.map((entity: any, index: number) => { - const entityPath = getEntityCatalogPath({ entity, catalogRouteRef }); - return ( - - - - {entityPath} - - ), - }} - /> - - {index < entities.length - 1 && } - - ); - })} - - - - - - -); +}: Props) => { + const [open, setOpen] = useState(true); + const handleClose = () => { + setOpen(false); + }; + + return ( + + + {dryRun ? 'Validation Result' : 'Registration Result'} + + + + {dryRun + ? 'The following entities would be created:' + : 'The following entities have been successfully created:'} + + + {entities.map((entity: any, index: number) => { + const entityPath = getEntityCatalogPath({ + entity, + catalogRouteRef, + }); + return ( + + + + {entityPath} + + ), + }} + /> + + {index < entities.length - 1 && } + + ); + })} + + + + {dryRun && ( + + )} + {!dryRun && ( + + )} + + + ); +}; From a6da3d1c06b33dcdf8b3588868646caaf9bbaeda Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Thu, 29 Oct 2020 11:10:11 +0100 Subject: [PATCH 3/6] fix: upgrade knex to 0.21.6 This reduces a overly noisy warning during tests: https://github.com/knex/knex/pull/3936 --- packages/backend-common/package.json | 2 +- packages/backend/package.json | 2 +- .../packages/backend/package.json.hbs | 2 +- plugins/auth-backend/package.json | 2 +- plugins/catalog-backend/package.json | 2 +- plugins/techdocs-backend/package.json | 2 +- yarn.lock | 22 +++++++++---------- 7 files changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index dea2f16bae..076e644afd 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -43,7 +43,7 @@ "express-promise-router": "^3.0.3", "git-url-parse": "^11.4.0", "helmet": "^4.0.0", - "knex": "^0.21.1", + "knex": "^0.21.6", "lodash": "^4.17.15", "logform": "^2.1.1", "minimist": "^1.2.5", diff --git a/packages/backend/package.json b/packages/backend/package.json index 9f8e66bc61..0d066d1139 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -38,7 +38,7 @@ "example-app": "^0.1.1-alpha.26", "express": "^4.17.1", "express-promise-router": "^3.0.3", - "knex": "^0.21.1", + "knex": "^0.21.6", "pg": "^8.3.0", "pg-connection-string": "^2.3.0", "sqlite3": "^5.0.0", diff --git a/packages/create-app/templates/default-app/packages/backend/package.json.hbs b/packages/create-app/templates/default-app/packages/backend/package.json.hbs index dde04868b9..bf887dc014 100644 --- a/packages/create-app/templates/default-app/packages/backend/package.json.hbs +++ b/packages/create-app/templates/default-app/packages/backend/package.json.hbs @@ -31,7 +31,7 @@ "dockerode": "^3.2.0", "express": "^4.17.1", "express-promise-router": "^3.0.3", - "knex": "^0.21.1", + "knex": "^0.21.6", {{#if dbTypePG}} "pg": "^8.3.0", {{/if}} diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 6489c8547d..caa20e361b 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -35,7 +35,7 @@ "helmet": "^4.0.0", "jose": "^1.27.1", "jwt-decode": "2.2.0", - "knex": "^0.21.1", + "knex": "^0.21.6", "moment": "^2.26.0", "morgan": "^1.10.0", "passport": "^0.4.1", diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index cd8e363901..d15779bf4e 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -32,7 +32,7 @@ "express-promise-router": "^3.0.3", "fs-extra": "^9.0.0", "git-url-parse": "^11.4.0", - "knex": "^0.21.1", + "knex": "^0.21.6", "ldapjs": "^2.2.0", "lodash": "^4.17.15", "morgan": "^1.10.0", diff --git a/plugins/techdocs-backend/package.json b/plugins/techdocs-backend/package.json index 5bdc7ba282..50eb0253cc 100644 --- a/plugins/techdocs-backend/package.json +++ b/plugins/techdocs-backend/package.json @@ -32,7 +32,7 @@ "express-promise-router": "^3.0.3", "fs-extra": "^9.0.1", "git-url-parse": "^11.4.0", - "knex": "^0.21.1", + "knex": "^0.21.6", "nodegit": "^0.27.0", "winston": "^3.2.1" }, diff --git a/yarn.lock b/yarn.lock index b2b87be5e0..70b224a600 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13469,7 +13469,7 @@ inflight@^1.0.4: once "^1.3.0" wrappy "1" -inherits@2, inherits@2.0.4, inherits@^2.0.0, inherits@^2.0.1, inherits@^2.0.3, inherits@^2.0.4, inherits@~2.0.0, inherits@~2.0.1, inherits@~2.0.3, inherits@~2.0.4: +inherits@2, inherits@2.0.4, inherits@^2.0.0, inherits@^2.0.1, inherits@^2.0.3, inherits@^2.0.4, inherits@~2.0.0, inherits@~2.0.1, inherits@~2.0.3: version "2.0.4" resolved "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz#0fa2c64f932917c3433a0ded55363aae37416b7c" integrity sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ== @@ -15194,23 +15194,21 @@ kleur@^3.0.3: resolved "https://registry.npmjs.org/kleur/-/kleur-3.0.3.tgz#a79c9ecc86ee1ce3fa6206d1216c501f147fc07e" integrity sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w== -knex@^0.21.1: - version "0.21.5" - resolved "https://registry.npmjs.org/knex/-/knex-0.21.5.tgz#c4be1958488f348aed3510aa4b7115639ee1bd01" - integrity sha512-cQj7F2D/fu03eTr6ZzYCYKdB9w7fPYlvTiU/f2OeXay52Pq5PwD+NAkcf40WDnppt/4/4KukROwlMOaE7WArcA== +knex@^0.21.6: + version "0.21.8" + resolved "https://registry.npmjs.org/knex/-/knex-0.21.8.tgz#e5c07af61ee6aa006d3468e10e3a69351deb0c26" + integrity sha512-ziUu4vAlIGA8j2l0S4xcD1d3XdpJA4HYGhwHEhgAgefGCmB1OLSjUGCs/ebkJal42fSvAkyZaB0tcOtTXKgS5g== dependencies: colorette "1.2.1" commander "^5.1.0" debug "4.1.1" esm "^3.2.25" getopts "2.2.5" - inherits "~2.0.4" interpret "^2.2.0" liftoff "3.1.0" lodash "^4.17.20" - mkdirp "^1.0.4" pg-connection-string "2.3.0" - tarn "^3.0.0" + tarn "^3.0.1" tildify "2.0.0" uuid "^7.0.3" v8flags "^3.2.0" @@ -22264,10 +22262,10 @@ tar@^6.0.1, tar@^6.0.2: mkdirp "^1.0.3" yallist "^4.0.0" -tarn@^3.0.0: - version "3.0.0" - resolved "https://registry.npmjs.org/tarn/-/tarn-3.0.0.tgz#a4082405216c0cce182b8b4cb2639c52c1e870d4" - integrity sha512-PKUnlDFODZueoA8owLehl8vLcgtA8u4dRuVbZc92tspDYZixjJL6TqYOmryf/PfP/EBX+2rgNcrj96NO+RPkdQ== +tarn@^3.0.1: + version "3.0.1" + resolved "https://registry.npmjs.org/tarn/-/tarn-3.0.1.tgz#ebac2c6dbc6977d34d4526e0a7814200386a8aec" + integrity sha512-6usSlV9KyHsspvwu2duKH+FMUhqJnAh6J5J/4MITl8s94iSUQTLkJggdiewKv4RyARQccnigV48Z+khiuVZDJw== tdigest@^0.1.1: version "0.1.1" From 4d66ae31c266b9b1de6b1523306a57c14a58f046 Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Thu, 29 Oct 2020 14:08:16 +0100 Subject: [PATCH 4/6] feat: upgrade ts-jest to 26.3.0 This adds support for typescript 4.0 and resolves a warning during testing. --- packages/cli/package.json | 2 +- yarn.lock | 110 ++++++++++++++++++++++++++++++-------- 2 files changed, 90 insertions(+), 22 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 668ec5e000..00bf463018 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -96,7 +96,7 @@ "sucrase": "^3.16.0", "tar": "^6.0.1", "terser-webpack-plugin": "^1.4.3", - "ts-jest": "^26.0.0", + "ts-jest": "^26.3.0", "ts-loader": "^7.0.4", "typescript": "^4.0.3", "url-loader": "^4.1.0", diff --git a/yarn.lock b/yarn.lock index 70b224a600..52f797f241 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2315,6 +2315,11 @@ slash "^3.0.0" strip-ansi "^6.0.0" +"@jest/create-cache-key-function@^26.5.0": + version "26.5.0" + resolved "https://registry.npmjs.org/@jest/create-cache-key-function/-/create-cache-key-function-26.5.0.tgz#1d07947adc51ea17766d9f0ccf5a8d6ea94c47dc" + integrity sha512-DJ+pEBUIqarrbv1W/C39f9YH0rJ4wsXZ/VC6JafJPlHW2HOucKceeaqTOQj9MEDQZjySxMLkOq5mfXZXNZcmWw== + "@jest/environment@^26.5.2": version "26.5.2" resolved "https://registry.npmjs.org/@jest/environment/-/environment-26.5.2.tgz#eba3cfc698f6e03739628f699c28e8a07f5e65fe" @@ -2461,6 +2466,17 @@ "@types/yargs" "^15.0.0" chalk "^4.0.0" +"@jest/types@^26.6.1": + version "26.6.1" + resolved "https://registry.npmjs.org/@jest/types/-/types-26.6.1.tgz#2638890e8031c0bc8b4681e0357ed986e2f866c5" + integrity sha512-ywHavIKNpAVrStiRY5wiyehvcktpijpItvGiK72RAn5ctqmzvPk8OvKnvHeBqa1XdQr959CTWAJMqxI8BTibyg== + dependencies: + "@types/istanbul-lib-coverage" "^2.0.0" + "@types/istanbul-reports" "^3.0.0" + "@types/node" "*" + "@types/yargs" "^15.0.0" + chalk "^4.0.0" + "@jsdevtools/ono@^7.1.3": version "7.1.3" resolved "https://registry.npmjs.org/@jsdevtools/ono/-/ono-7.1.3.tgz#9df03bbd7c696a5c58885c34aa06da41c8543796" @@ -5242,6 +5258,14 @@ jest-diff "^25.2.1" pretty-format "^25.2.1" +"@types/jest@26.x": + version "26.0.15" + resolved "https://registry.npmjs.org/@types/jest/-/jest-26.0.15.tgz#12e02c0372ad0548e07b9f4e19132b834cb1effe" + integrity sha512-s2VMReFXRg9XXxV+CW9e5Nz8fH2K1aEhwgjUqPPbQd7g95T0laAcvLv032EhFHIa5GHsZ8W7iJEQVaJq6k3Gog== + dependencies: + jest-diff "^26.0.0" + pretty-format "^26.0.0" + "@types/jquery@^3.3.34": version "3.5.1" resolved "https://registry.npmjs.org/@types/jquery/-/jquery-3.5.1.tgz#cebb057acf5071c40e439f30e840c57a30d406c3" @@ -14350,6 +14374,16 @@ jest-diff@^25.2.1: jest-get-type "^25.2.6" pretty-format "^25.5.0" +jest-diff@^26.0.0: + version "26.6.1" + resolved "https://registry.npmjs.org/jest-diff/-/jest-diff-26.6.1.tgz#38aa194979f454619bb39bdee299fb64ede5300c" + integrity sha512-BBNy/zin2m4kG5In126O8chOBxLLS/XMTuuM2+YhgyHk87ewPzKTuTJcqj3lOWOi03NNgrl+DkMeV/exdvG9gg== + dependencies: + chalk "^4.0.0" + diff-sequences "^26.5.0" + jest-get-type "^26.3.0" + pretty-format "^26.6.1" + jest-diff@^26.5.2: version "26.5.2" resolved "https://registry.npmjs.org/jest-diff/-/jest-diff-26.5.2.tgz#8e26cb32dc598e8b8a1b9deff55316f8313c8053" @@ -14627,6 +14661,18 @@ jest-snapshot@^26.5.3: pretty-format "^26.5.2" semver "^7.3.2" +jest-util@^26.1.0: + version "26.6.1" + resolved "https://registry.npmjs.org/jest-util/-/jest-util-26.6.1.tgz#4cc0d09ec57f28d12d053887eec5dc976a352e9b" + integrity sha512-xCLZUqVoqhquyPLuDXmH7ogceGctbW8SMyQVjD9o+1+NPWI7t0vO08udcFLVPLgKWcvc+zotaUv/RuaR6l8HIA== + dependencies: + "@jest/types" "^26.6.1" + "@types/node" "*" + chalk "^4.0.0" + graceful-fs "^4.2.4" + is-ci "^2.0.0" + micromatch "^4.0.2" + jest-util@^26.5.2: version "26.5.2" resolved "https://registry.npmjs.org/jest-util/-/jest-util-26.5.2.tgz#8403f75677902cc52a1b2140f568e91f8ed4f4d7" @@ -16198,14 +16244,6 @@ microevent.ts@~0.1.1: resolved "https://registry.npmjs.org/microevent.ts/-/microevent.ts-0.1.1.tgz#70b09b83f43df5172d0205a63025bce0f7357fa0" integrity sha512-jo1OfR4TaEwd5HOrt5+tAZ9mqT4jmpNAusXtyfNzqVm9uiSYFZlKM1wYL4oU7azZW/PxQW53wM0S6OR1JHNa2g== -micromatch@4.x, micromatch@^4.0.0, micromatch@^4.0.2: - version "4.0.2" - resolved "https://registry.npmjs.org/micromatch/-/micromatch-4.0.2.tgz#4fcb0999bf9fbc2fcbdd212f6d629b9a56c39259" - integrity sha512-y7FpHSbMUMoyPbYUSzO6PaZ6FyRnQOpHuKwbo1G+Knck95XVU4QAiKdGEnj5wwoS7PlOgthX/09u5iFJ+aYf5Q== - dependencies: - braces "^3.0.1" - picomatch "^2.0.5" - micromatch@^3.0.4, micromatch@^3.1.10, micromatch@^3.1.4: version "3.1.10" resolved "https://registry.npmjs.org/micromatch/-/micromatch-3.1.10.tgz#70859bc95c9840952f359a068a3fc49f9ecfac23" @@ -16225,6 +16263,14 @@ micromatch@^3.0.4, micromatch@^3.1.10, micromatch@^3.1.4: snapdragon "^0.8.1" to-regex "^3.0.2" +micromatch@^4.0.0, micromatch@^4.0.2: + version "4.0.2" + resolved "https://registry.npmjs.org/micromatch/-/micromatch-4.0.2.tgz#4fcb0999bf9fbc2fcbdd212f6d629b9a56c39259" + integrity sha512-y7FpHSbMUMoyPbYUSzO6PaZ6FyRnQOpHuKwbo1G+Knck95XVU4QAiKdGEnj5wwoS7PlOgthX/09u5iFJ+aYf5Q== + dependencies: + braces "^3.0.1" + picomatch "^2.0.5" + miller-rabin@^4.0.0: version "4.0.1" resolved "https://registry.npmjs.org/miller-rabin/-/miller-rabin-4.0.1.tgz#f080351c865b0dc562a8462966daa53543c78a4d" @@ -18811,6 +18857,16 @@ pretty-format@^25.2.1, pretty-format@^25.5.0: ansi-styles "^4.0.0" react-is "^16.12.0" +pretty-format@^26.0.0, pretty-format@^26.6.1: + version "26.6.1" + resolved "https://registry.npmjs.org/pretty-format/-/pretty-format-26.6.1.tgz#af9a2f63493a856acddeeb11ba6bcf61989660a8" + integrity sha512-MeqqsP5PYcRBbGMvwzsyBdmAJ4EFX7pWFyl7x4+dMVg5pE0ZDdBIvEH2ergvIO+Gvwv1wh64YuOY9y5LuyY/GA== + dependencies: + "@jest/types" "^26.6.1" + ansi-regex "^5.0.0" + ansi-styles "^4.0.0" + react-is "^17.0.1" + pretty-format@^26.4.2: version "26.4.2" resolved "https://registry.npmjs.org/pretty-format/-/pretty-format-26.4.2.tgz#d081d032b398e801e2012af2df1214ef75a81237" @@ -19496,6 +19552,11 @@ react-is@^16.12.0, react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.0, react-i resolved "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4" integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ== +react-is@^17.0.1: + version "17.0.1" + resolved "https://registry.npmjs.org/react-is/-/react-is-17.0.1.tgz#5b3531bd76a645a4c9fb6e693ed36419e3301339" + integrity sha512-NAnt2iGDXohE5LI7uBnLnqvLQMtzhkiAOLXTmv+qnF9Ky7xAPcX8Up/xWIhxvLVGJvuLiNc4xQLtuqDRzb4fSA== + react-lazylog@^4.5.2, react-lazylog@^4.5.3: version "4.5.3" resolved "https://registry.npmjs.org/react-lazylog/-/react-lazylog-4.5.3.tgz#289e24995b5599e75943556ac63f5e2c04d0001e" @@ -22739,21 +22800,23 @@ ts-invariant@^0.4.0: dependencies: tslib "^1.9.3" -ts-jest@^26.0.0: - version "26.1.1" - resolved "https://registry.npmjs.org/ts-jest/-/ts-jest-26.1.1.tgz#b98569b8a4d4025d966b3d40c81986dd1c510f8d" - integrity sha512-Lk/357quLg5jJFyBQLnSbhycnB3FPe+e9i7ahxokyXxAYoB0q1pPmqxxRPYr4smJic1Rjcf7MXDBhZWgxlli0A== +ts-jest@^26.3.0: + version "26.4.3" + resolved "https://registry.npmjs.org/ts-jest/-/ts-jest-26.4.3.tgz#d153a616033e7ec8544b97ddbe2638cbe38d53db" + integrity sha512-pFDkOKFGY+nL9v5pkhm+BIFpoAuno96ff7GMnIYr/3L6slFOS365SI0fGEVYx2RKGji5M2elxhWjDMPVcOCdSw== dependencies: + "@jest/create-cache-key-function" "^26.5.0" + "@types/jest" "26.x" bs-logger "0.x" buffer-from "1.x" fast-json-stable-stringify "2.x" + jest-util "^26.1.0" json5 "2.x" lodash.memoize "4.x" make-error "1.x" - micromatch "4.x" mkdirp "1.x" semver "7.x" - yargs-parser "18.x" + yargs-parser "20.x" ts-loader@^7.0.4: version "7.0.4" @@ -24214,13 +24277,10 @@ yaml@*, yaml@^1.10.0, yaml@^1.7.2, yaml@^1.9.2: resolved "https://registry.npmjs.org/yaml/-/yaml-1.10.0.tgz#3b593add944876077d4d683fee01081bd9fff31e" integrity sha512-yr2icI4glYaNG+KWONODapy2/jDdMSDnrONSjblABjD9B4Z5LgiircSt8m8sRZFNi08kG9Sm0uSHtEmP3zaEGg== -yargs-parser@18.x, yargs-parser@^18.1.2: - version "18.1.3" - resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-18.1.3.tgz#be68c4975c6b2abf469236b0c870362fab09a7b0" - integrity sha512-o50j0JeToy/4K6OZcaQmW6lyXXKhq7csREXcDwk2omFPJEwUNOVtJKvmDr9EI1fAJZUyZcRF7kxGBWmRXudrCQ== - dependencies: - camelcase "^5.0.0" - decamelize "^1.2.0" +yargs-parser@20.x: + version "20.2.3" + resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.3.tgz#92419ba867b858c868acf8bae9bf74af0dd0ce26" + integrity sha512-emOFRT9WVHw03QSvN5qor9QQT9+sw5vwxfYweivSMHTcAXPefwVae2FjO7JJjj8hCE4CzPOPeFM83VwT29HCww== yargs-parser@^10.0.0: version "10.1.0" @@ -24245,6 +24305,14 @@ yargs-parser@^15.0.1: camelcase "^5.0.0" decamelize "^1.2.0" +yargs-parser@^18.1.2: + version "18.1.3" + resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-18.1.3.tgz#be68c4975c6b2abf469236b0c870362fab09a7b0" + integrity sha512-o50j0JeToy/4K6OZcaQmW6lyXXKhq7csREXcDwk2omFPJEwUNOVtJKvmDr9EI1fAJZUyZcRF7kxGBWmRXudrCQ== + dependencies: + camelcase "^5.0.0" + decamelize "^1.2.0" + yargs-parser@^20.0.0: version "20.2.1" resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.1.tgz#28f3773c546cdd8a69ddae68116b48a5da328e77" From 16aefb099ed2892eba3919e798c65c7102e6da00 Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Thu, 29 Oct 2020 20:58:03 +0100 Subject: [PATCH 5/6] Revert "feat: upgrade ts-jest to 26.3.0" This reverts commit 4d66ae31c266b9b1de6b1523306a57c14a58f046. --- packages/cli/package.json | 2 +- yarn.lock | 110 ++++++++------------------------------ 2 files changed, 22 insertions(+), 90 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 00bf463018..668ec5e000 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -96,7 +96,7 @@ "sucrase": "^3.16.0", "tar": "^6.0.1", "terser-webpack-plugin": "^1.4.3", - "ts-jest": "^26.3.0", + "ts-jest": "^26.0.0", "ts-loader": "^7.0.4", "typescript": "^4.0.3", "url-loader": "^4.1.0", diff --git a/yarn.lock b/yarn.lock index 52f797f241..70b224a600 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2315,11 +2315,6 @@ slash "^3.0.0" strip-ansi "^6.0.0" -"@jest/create-cache-key-function@^26.5.0": - version "26.5.0" - resolved "https://registry.npmjs.org/@jest/create-cache-key-function/-/create-cache-key-function-26.5.0.tgz#1d07947adc51ea17766d9f0ccf5a8d6ea94c47dc" - integrity sha512-DJ+pEBUIqarrbv1W/C39f9YH0rJ4wsXZ/VC6JafJPlHW2HOucKceeaqTOQj9MEDQZjySxMLkOq5mfXZXNZcmWw== - "@jest/environment@^26.5.2": version "26.5.2" resolved "https://registry.npmjs.org/@jest/environment/-/environment-26.5.2.tgz#eba3cfc698f6e03739628f699c28e8a07f5e65fe" @@ -2466,17 +2461,6 @@ "@types/yargs" "^15.0.0" chalk "^4.0.0" -"@jest/types@^26.6.1": - version "26.6.1" - resolved "https://registry.npmjs.org/@jest/types/-/types-26.6.1.tgz#2638890e8031c0bc8b4681e0357ed986e2f866c5" - integrity sha512-ywHavIKNpAVrStiRY5wiyehvcktpijpItvGiK72RAn5ctqmzvPk8OvKnvHeBqa1XdQr959CTWAJMqxI8BTibyg== - dependencies: - "@types/istanbul-lib-coverage" "^2.0.0" - "@types/istanbul-reports" "^3.0.0" - "@types/node" "*" - "@types/yargs" "^15.0.0" - chalk "^4.0.0" - "@jsdevtools/ono@^7.1.3": version "7.1.3" resolved "https://registry.npmjs.org/@jsdevtools/ono/-/ono-7.1.3.tgz#9df03bbd7c696a5c58885c34aa06da41c8543796" @@ -5258,14 +5242,6 @@ jest-diff "^25.2.1" pretty-format "^25.2.1" -"@types/jest@26.x": - version "26.0.15" - resolved "https://registry.npmjs.org/@types/jest/-/jest-26.0.15.tgz#12e02c0372ad0548e07b9f4e19132b834cb1effe" - integrity sha512-s2VMReFXRg9XXxV+CW9e5Nz8fH2K1aEhwgjUqPPbQd7g95T0laAcvLv032EhFHIa5GHsZ8W7iJEQVaJq6k3Gog== - dependencies: - jest-diff "^26.0.0" - pretty-format "^26.0.0" - "@types/jquery@^3.3.34": version "3.5.1" resolved "https://registry.npmjs.org/@types/jquery/-/jquery-3.5.1.tgz#cebb057acf5071c40e439f30e840c57a30d406c3" @@ -14374,16 +14350,6 @@ jest-diff@^25.2.1: jest-get-type "^25.2.6" pretty-format "^25.5.0" -jest-diff@^26.0.0: - version "26.6.1" - resolved "https://registry.npmjs.org/jest-diff/-/jest-diff-26.6.1.tgz#38aa194979f454619bb39bdee299fb64ede5300c" - integrity sha512-BBNy/zin2m4kG5In126O8chOBxLLS/XMTuuM2+YhgyHk87ewPzKTuTJcqj3lOWOi03NNgrl+DkMeV/exdvG9gg== - dependencies: - chalk "^4.0.0" - diff-sequences "^26.5.0" - jest-get-type "^26.3.0" - pretty-format "^26.6.1" - jest-diff@^26.5.2: version "26.5.2" resolved "https://registry.npmjs.org/jest-diff/-/jest-diff-26.5.2.tgz#8e26cb32dc598e8b8a1b9deff55316f8313c8053" @@ -14661,18 +14627,6 @@ jest-snapshot@^26.5.3: pretty-format "^26.5.2" semver "^7.3.2" -jest-util@^26.1.0: - version "26.6.1" - resolved "https://registry.npmjs.org/jest-util/-/jest-util-26.6.1.tgz#4cc0d09ec57f28d12d053887eec5dc976a352e9b" - integrity sha512-xCLZUqVoqhquyPLuDXmH7ogceGctbW8SMyQVjD9o+1+NPWI7t0vO08udcFLVPLgKWcvc+zotaUv/RuaR6l8HIA== - dependencies: - "@jest/types" "^26.6.1" - "@types/node" "*" - chalk "^4.0.0" - graceful-fs "^4.2.4" - is-ci "^2.0.0" - micromatch "^4.0.2" - jest-util@^26.5.2: version "26.5.2" resolved "https://registry.npmjs.org/jest-util/-/jest-util-26.5.2.tgz#8403f75677902cc52a1b2140f568e91f8ed4f4d7" @@ -16244,6 +16198,14 @@ microevent.ts@~0.1.1: resolved "https://registry.npmjs.org/microevent.ts/-/microevent.ts-0.1.1.tgz#70b09b83f43df5172d0205a63025bce0f7357fa0" integrity sha512-jo1OfR4TaEwd5HOrt5+tAZ9mqT4jmpNAusXtyfNzqVm9uiSYFZlKM1wYL4oU7azZW/PxQW53wM0S6OR1JHNa2g== +micromatch@4.x, micromatch@^4.0.0, micromatch@^4.0.2: + version "4.0.2" + resolved "https://registry.npmjs.org/micromatch/-/micromatch-4.0.2.tgz#4fcb0999bf9fbc2fcbdd212f6d629b9a56c39259" + integrity sha512-y7FpHSbMUMoyPbYUSzO6PaZ6FyRnQOpHuKwbo1G+Knck95XVU4QAiKdGEnj5wwoS7PlOgthX/09u5iFJ+aYf5Q== + dependencies: + braces "^3.0.1" + picomatch "^2.0.5" + micromatch@^3.0.4, micromatch@^3.1.10, micromatch@^3.1.4: version "3.1.10" resolved "https://registry.npmjs.org/micromatch/-/micromatch-3.1.10.tgz#70859bc95c9840952f359a068a3fc49f9ecfac23" @@ -16263,14 +16225,6 @@ micromatch@^3.0.4, micromatch@^3.1.10, micromatch@^3.1.4: snapdragon "^0.8.1" to-regex "^3.0.2" -micromatch@^4.0.0, micromatch@^4.0.2: - version "4.0.2" - resolved "https://registry.npmjs.org/micromatch/-/micromatch-4.0.2.tgz#4fcb0999bf9fbc2fcbdd212f6d629b9a56c39259" - integrity sha512-y7FpHSbMUMoyPbYUSzO6PaZ6FyRnQOpHuKwbo1G+Knck95XVU4QAiKdGEnj5wwoS7PlOgthX/09u5iFJ+aYf5Q== - dependencies: - braces "^3.0.1" - picomatch "^2.0.5" - miller-rabin@^4.0.0: version "4.0.1" resolved "https://registry.npmjs.org/miller-rabin/-/miller-rabin-4.0.1.tgz#f080351c865b0dc562a8462966daa53543c78a4d" @@ -18857,16 +18811,6 @@ pretty-format@^25.2.1, pretty-format@^25.5.0: ansi-styles "^4.0.0" react-is "^16.12.0" -pretty-format@^26.0.0, pretty-format@^26.6.1: - version "26.6.1" - resolved "https://registry.npmjs.org/pretty-format/-/pretty-format-26.6.1.tgz#af9a2f63493a856acddeeb11ba6bcf61989660a8" - integrity sha512-MeqqsP5PYcRBbGMvwzsyBdmAJ4EFX7pWFyl7x4+dMVg5pE0ZDdBIvEH2ergvIO+Gvwv1wh64YuOY9y5LuyY/GA== - dependencies: - "@jest/types" "^26.6.1" - ansi-regex "^5.0.0" - ansi-styles "^4.0.0" - react-is "^17.0.1" - pretty-format@^26.4.2: version "26.4.2" resolved "https://registry.npmjs.org/pretty-format/-/pretty-format-26.4.2.tgz#d081d032b398e801e2012af2df1214ef75a81237" @@ -19552,11 +19496,6 @@ react-is@^16.12.0, react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.0, react-i resolved "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4" integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ== -react-is@^17.0.1: - version "17.0.1" - resolved "https://registry.npmjs.org/react-is/-/react-is-17.0.1.tgz#5b3531bd76a645a4c9fb6e693ed36419e3301339" - integrity sha512-NAnt2iGDXohE5LI7uBnLnqvLQMtzhkiAOLXTmv+qnF9Ky7xAPcX8Up/xWIhxvLVGJvuLiNc4xQLtuqDRzb4fSA== - react-lazylog@^4.5.2, react-lazylog@^4.5.3: version "4.5.3" resolved "https://registry.npmjs.org/react-lazylog/-/react-lazylog-4.5.3.tgz#289e24995b5599e75943556ac63f5e2c04d0001e" @@ -22800,23 +22739,21 @@ ts-invariant@^0.4.0: dependencies: tslib "^1.9.3" -ts-jest@^26.3.0: - version "26.4.3" - resolved "https://registry.npmjs.org/ts-jest/-/ts-jest-26.4.3.tgz#d153a616033e7ec8544b97ddbe2638cbe38d53db" - integrity sha512-pFDkOKFGY+nL9v5pkhm+BIFpoAuno96ff7GMnIYr/3L6slFOS365SI0fGEVYx2RKGji5M2elxhWjDMPVcOCdSw== +ts-jest@^26.0.0: + version "26.1.1" + resolved "https://registry.npmjs.org/ts-jest/-/ts-jest-26.1.1.tgz#b98569b8a4d4025d966b3d40c81986dd1c510f8d" + integrity sha512-Lk/357quLg5jJFyBQLnSbhycnB3FPe+e9i7ahxokyXxAYoB0q1pPmqxxRPYr4smJic1Rjcf7MXDBhZWgxlli0A== dependencies: - "@jest/create-cache-key-function" "^26.5.0" - "@types/jest" "26.x" bs-logger "0.x" buffer-from "1.x" fast-json-stable-stringify "2.x" - jest-util "^26.1.0" json5 "2.x" lodash.memoize "4.x" make-error "1.x" + micromatch "4.x" mkdirp "1.x" semver "7.x" - yargs-parser "20.x" + yargs-parser "18.x" ts-loader@^7.0.4: version "7.0.4" @@ -24277,10 +24214,13 @@ yaml@*, yaml@^1.10.0, yaml@^1.7.2, yaml@^1.9.2: resolved "https://registry.npmjs.org/yaml/-/yaml-1.10.0.tgz#3b593add944876077d4d683fee01081bd9fff31e" integrity sha512-yr2icI4glYaNG+KWONODapy2/jDdMSDnrONSjblABjD9B4Z5LgiircSt8m8sRZFNi08kG9Sm0uSHtEmP3zaEGg== -yargs-parser@20.x: - version "20.2.3" - resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.3.tgz#92419ba867b858c868acf8bae9bf74af0dd0ce26" - integrity sha512-emOFRT9WVHw03QSvN5qor9QQT9+sw5vwxfYweivSMHTcAXPefwVae2FjO7JJjj8hCE4CzPOPeFM83VwT29HCww== +yargs-parser@18.x, yargs-parser@^18.1.2: + version "18.1.3" + resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-18.1.3.tgz#be68c4975c6b2abf469236b0c870362fab09a7b0" + integrity sha512-o50j0JeToy/4K6OZcaQmW6lyXXKhq7csREXcDwk2omFPJEwUNOVtJKvmDr9EI1fAJZUyZcRF7kxGBWmRXudrCQ== + dependencies: + camelcase "^5.0.0" + decamelize "^1.2.0" yargs-parser@^10.0.0: version "10.1.0" @@ -24305,14 +24245,6 @@ yargs-parser@^15.0.1: camelcase "^5.0.0" decamelize "^1.2.0" -yargs-parser@^18.1.2: - version "18.1.3" - resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-18.1.3.tgz#be68c4975c6b2abf469236b0c870362fab09a7b0" - integrity sha512-o50j0JeToy/4K6OZcaQmW6lyXXKhq7csREXcDwk2omFPJEwUNOVtJKvmDr9EI1fAJZUyZcRF7kxGBWmRXudrCQ== - dependencies: - camelcase "^5.0.0" - decamelize "^1.2.0" - yargs-parser@^20.0.0: version "20.2.1" resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.1.tgz#28f3773c546cdd8a69ddae68116b48a5da328e77" From 530909696954f357750c8d1d20dfcae570872c3d Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Fri, 30 Oct 2020 10:52:10 +0100 Subject: [PATCH 6/6] refactor: don't dryRun addLocation This allows to keep the HigherOderOperations free from db Operations --- .../catalog/DatabaseEntitiesCatalog.test.ts | 52 ++++- .../src/catalog/DatabaseEntitiesCatalog.ts | 208 +++++++++--------- .../src/catalog/DatabaseLocationsCatalog.ts | 13 +- plugins/catalog-backend/src/catalog/types.ts | 16 +- .../ingestion/HigherOrderOperations.test.ts | 67 ++---- .../src/ingestion/HigherOrderOperations.ts | 44 ++-- .../src/service/CatalogBuilder.ts | 1 - .../src/service/router.test.ts | 66 +++--- plugins/catalog-backend/src/service/router.ts | 36 ++- 9 files changed, 236 insertions(+), 267 deletions(-) diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts index 90b11b179d..f3db4c8dc6 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts @@ -86,7 +86,7 @@ describe('DatabaseEntitiesCatalog', () => { expect(result).toEqual([{ entityId: 'u' }]); }); - it('adds using existing transaction', async () => { + it('dry run of add operation', async () => { const entity: Entity = { apiVersion: 'a', kind: 'b', @@ -95,10 +95,6 @@ describe('DatabaseEntitiesCatalog', () => { namespace: 'd', }, }; - const existingTransaction: jest.Mocked = { - rollback: jest.fn(), - }; - db.entities.mockResolvedValue([]); db.addEntities.mockResolvedValue([ { entity: { ...entity, metadata: { ...entity.metadata, uid: 'u' } } }, @@ -107,7 +103,7 @@ describe('DatabaseEntitiesCatalog', () => { const catalog = new DatabaseEntitiesCatalog(db, getVoidLogger()); const result = await catalog.batchAddOrUpdateEntities( [{ entity, relations: [] }], - { tx: existingTransaction }, + { dryRun: true }, ); expect(db.entities).toHaveBeenCalledTimes(1); @@ -121,9 +117,53 @@ describe('DatabaseEntitiesCatalog', () => { expect(db.setRelations).toHaveBeenCalledTimes(1); expect(db.setRelations).toHaveBeenCalledWith(expect.anything(), 'u', []); expect(db.addEntities).toHaveBeenCalledTimes(1); + expect(transaction.rollback).toBeCalledTimes(1); expect(result).toEqual([{ entityId: 'u' }]); }); + it('output modified entities', async () => { + const entity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + namespace: 'd', + }, + }; + const dbEntity: Entity = { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + namespace: 'd', + uid: 'u', + }, + }; + db.entities.mockResolvedValue([ + { + entity: dbEntity, + }, + ]); + db.addEntities.mockResolvedValue([ + { entity: { ...entity, metadata: { ...entity.metadata, uid: 'u' } } }, + ]); + + const catalog = new DatabaseEntitiesCatalog(db, getVoidLogger()); + const result = await catalog.batchAddOrUpdateEntities( + [{ entity, relations: [] }], + { outputEntities: true }, + ); + + expect(db.entities).toHaveBeenCalledTimes(2); + expect(db.setRelations).toHaveBeenCalledTimes(1); + expect(result).toEqual([ + { + entityId: 'u', + entity: dbEntity, + }, + ]); + }); + it('updates when given uid', async () => { const entity: Entity = { apiVersion: 'a', diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index 53fb50677b..c97ee023e6 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -65,16 +65,10 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { private readonly logger: Logger, ) {} - async entities(options?: { - filters?: EntityFilters[]; - tx?: Transaction; - }): Promise { - const filters = options?.filters; - const items = options?.tx - ? await this.database.entities(options?.tx, filters) - : await this.database.transaction(tx => - this.database.entities(tx, filters), - ); + async entities(filters?: EntityFilters[]): Promise { + const items = await this.database.transaction(tx => + this.database.entities(tx, filters), + ); return items.map(i => i.entity); } @@ -148,109 +142,110 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { requests: EntityUpsertRequest[], options?: { locationId?: string; - tx?: Transaction; + dryRun?: boolean; + outputEntities?: boolean; }, ): Promise { const locationId = options?.locationId; - if (options?.tx) { - return this.batchAddOrUpdateEntitiesInTransaction( - requests, - options.tx, - locationId, - ); - } + return await this.database.transaction(async tx => { + // Group the entities by unique kind+namespace combinations + const entitiesByKindAndNamespace = groupBy(requests, ({ entity }) => { + const name = getEntityName(entity); + return `${name.kind}:${name.namespace}`.toLowerCase(); + }); - return await this.database.transaction( - async tx => - await this.batchAddOrUpdateEntitiesInTransaction( - requests, - tx, - locationId, - ), - ); - } + const limiter = limiterFactory(BATCH_CONCURRENCY); + const tasks: Promise[] = []; - private async batchAddOrUpdateEntitiesInTransaction( - requests: EntityUpsertRequest[], - tx: Transaction, - locationId?: string, - ): Promise { - // Group the entities by unique kind+namespace combinations - const entitiesByKindAndNamespace = groupBy(requests, ({ entity }) => { - const name = getEntityName(entity); - return `${name.kind}:${name.namespace}`.toLowerCase(); - }); + for (const groupRequests of Object.values(entitiesByKindAndNamespace)) { + const { kind, namespace } = getEntityName(groupRequests[0].entity); - const limiter = limiterFactory(BATCH_CONCURRENCY); - const tasks: Promise[] = []; + // Go through the new entities in reasonable chunk sizes (sometimes, + // sources produce tens of thousands of entities, and those are too large + // batch sizes to reasonably send to the database) + for (const batch of chunk(groupRequests, BATCH_SIZE)) { + tasks.push( + limiter(async () => { + const first = serializeEntityRef(batch[0].entity); + const last = serializeEntityRef(batch[batch.length - 1].entity); + let modifiedEntityIds: EntityUpsertResponse[] = []; - for (const groupRequests of Object.values(entitiesByKindAndNamespace)) { - const { kind, namespace } = getEntityName(groupRequests[0].entity); + this.logger.debug( + `Considering batch ${first}-${last} (${batch.length} entries)`, + ); - // Go through the new entities in reasonable chunk sizes (sometimes, - // sources produce tens of thousands of entities, and those are too large - // batch sizes to reasonably send to the database) - for (const batch of chunk(groupRequests, BATCH_SIZE)) { - tasks.push( - limiter(async () => { - const first = serializeEntityRef(batch[0].entity); - const last = serializeEntityRef(batch[batch.length - 1].entity); - const modifiedEntityIds: EntityUpsertResponse[] = []; - this.logger.debug( - `Considering batch ${first}-${last} (${batch.length} entries)`, - ); - - // Retry the batch write a few times to deal with contention - const context = { - kind, - namespace, - locationId, - }; - for (let attempt = 1; attempt <= BATCH_ATTEMPTS; ++attempt) { - try { - const { toAdd, toUpdate, toIgnore } = await this.analyzeBatch( - batch, - context, - tx, - ); - if (toAdd.length) { - modifiedEntityIds.push( - ...(await this.batchAdd(toAdd, context, tx)), + // Retry the batch write a few times to deal with contention + const context = { + kind, + namespace, + locationId, + }; + for (let attempt = 1; attempt <= BATCH_ATTEMPTS; ++attempt) { + try { + const { toAdd, toUpdate, toIgnore } = await this.analyzeBatch( + batch, + context, + tx, ); - } - if (toUpdate.length) { - modifiedEntityIds.push( - ...(await this.batchUpdate(toUpdate, context, tx)), - ); - } - // TODO(Rugvip): We currently always update relations, but we - // likely want to figure out a way to avoid that - for (const { entity, relations } of toIgnore) { - const entityId = entity.metadata.uid!; - await this.setRelations(entityId, relations, tx); - modifiedEntityIds.push({ entityId }); - } + if (toAdd.length) { + modifiedEntityIds.push( + ...(await this.batchAdd(toAdd, context, tx)), + ); + } + if (toUpdate.length) { + modifiedEntityIds.push( + ...(await this.batchUpdate(toUpdate, context, tx)), + ); + } + // TODO(Rugvip): We currently always update relations, but we + // likely want to figure out a way to avoid that + for (const { entity, relations } of toIgnore) { + const entityId = entity.metadata.uid!; + await this.setRelations(entityId, relations, tx); + modifiedEntityIds.push({ entityId }); + } - break; - } catch (e) { - if (e instanceof ConflictError && attempt < BATCH_ATTEMPTS) { - this.logger.warn( - `Failed to write batch at attempt ${attempt}/${BATCH_ATTEMPTS}, ${e}`, - ); - } else { - throw e; + break; + } catch (e) { + if (e instanceof ConflictError && attempt < BATCH_ATTEMPTS) { + this.logger.warn( + `Failed to write batch at attempt ${attempt}/${BATCH_ATTEMPTS}, ${e}`, + ); + } else { + throw e; + } } } - } - return modifiedEntityIds; - }), - ); + if (options?.outputEntities) { + const writtenEntities = await this.database.entities(tx, [ + { 'metadata.uid': modifiedEntityIds.map(e => e.entityId) }, + ]); + + modifiedEntityIds = writtenEntities.map(e => ({ + entityId: e.entity.metadata.uid!, + entity: e.entity, + })); + } + + return modifiedEntityIds; + }), + ); + } } - } - return (await Promise.all(tasks)).flat(); + const entityUpserts = (await Promise.all(tasks)).flat(); + + if (options?.dryRun) { + // If this is only a dry run, cancel the database transaction even if it was successful. + await tx.rollback(); + + this.logger.debug(`Perfomed successful dry run of adding entities`); + } + + return entityUpserts; + }); } // Set the relations originating from an entity using the DB layer @@ -278,19 +273,16 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { const markTimestamp = process.hrtime(); const names = requests.map(({ entity }) => entity.metadata.name); - const oldEntities = await this.entities({ - filters: [ - { - kind: kind, - 'metadata.namespace': namespace, - 'metadata.name': names, - }, - ], - tx, - }); + const oldEntities = await this.database.entities(tx, [ + { + kind: kind, + 'metadata.namespace': namespace, + 'metadata.name': names, + }, + ]); const oldEntitiesByName = new Map( - oldEntities.map(e => [e.metadata.name, e]), + oldEntities.map(e => [e.entity.metadata.name, e.entity]), ); const toAdd: EntityUpsertRequest[] = []; diff --git a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts index b78b9933b9..9ce53d2850 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts @@ -15,7 +15,7 @@ */ import { Location } from '@backstage/catalog-model'; -import type { Database, Transaction } from '../database'; +import type { Database } from '../database'; import { DatabaseLocationUpdateLogEvent, DatabaseLocationUpdateLogStatus, @@ -25,16 +25,7 @@ import { LocationResponse, LocationsCatalog } from './types'; export class DatabaseLocationsCatalog implements LocationsCatalog { constructor(private readonly database: Database) {} - async addLocation( - location: Location, - options?: { tx?: Transaction }, - ): Promise { - const transaction = options?.tx; - - if (transaction) { - return await this.database.addLocation(transaction, location); - } - + async addLocation(location: Location): Promise { return await this.database.transaction( async tx => await this.database.addLocation(tx, location), ); diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 6f2977619a..edc6ed2772 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -15,7 +15,7 @@ */ import { Entity, EntityRelationSpec, Location } from '@backstage/catalog-model'; -import type { EntityFilters, Transaction } from '../database'; +import type { EntityFilters } from '../database'; // // Entities @@ -28,13 +28,11 @@ export type EntityUpsertRequest = { export type EntityUpsertResponse = { entityId: string; + entity?: Entity; }; export type EntitiesCatalog = { - entities(options?: { - filters?: EntityFilters[]; - tx?: Transaction; - }): Promise; + entities(filters?: EntityFilters[]): Promise; removeEntityByUid(uid: string): Promise; /** @@ -46,8 +44,9 @@ export type EntitiesCatalog = { batchAddOrUpdateEntities( entities: EntityUpsertRequest[], options?: { - tx?: Transaction; locationId?: string; + dryRun?: boolean; + outputEntities?: boolean; }, ): Promise; }; @@ -76,10 +75,7 @@ export type LocationResponse = { }; export type LocationsCatalog = { - addLocation( - location: Location, - options?: { tx?: Transaction }, - ): Promise; + addLocation(location: Location): Promise; removeLocation(id: string): Promise; locations(): Promise; location(id: string): Promise; diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts index da722e56bc..40b48ca4ab 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.test.ts @@ -18,11 +18,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { Entity, Location, LocationSpec } from '@backstage/catalog-model'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; import { LocationUpdateStatus } from '../catalog/types'; -import { - Database, - DatabaseLocationUpdateLogStatus, - Transaction, -} from '../database/types'; +import { DatabaseLocationUpdateLogStatus } from '../database/types'; import { HigherOrderOperations } from './HigherOrderOperations'; import { LocationReader } from './types'; @@ -30,8 +26,6 @@ describe('HigherOrderOperations', () => { let entitiesCatalog: jest.Mocked; let locationsCatalog: jest.Mocked; let locationReader: jest.Mocked; - let transaction: jest.Mocked; - let database: jest.Mocked; let higherOrderOperation: HigherOrderOperations; beforeAll(() => { @@ -52,37 +46,16 @@ describe('HigherOrderOperations', () => { locationReader = { read: jest.fn(), }; - transaction = { - rollback: jest.fn(), - }; - database = { - transaction: jest.fn(), - addEntities: jest.fn(), - updateEntity: jest.fn(), - entities: jest.fn(), - entityByName: jest.fn(), - entityByUid: jest.fn(), - removeEntityByUid: jest.fn(), - setRelations: jest.fn(), - addLocation: jest.fn(), - removeLocation: jest.fn(), - location: jest.fn(), - locations: jest.fn(), - locationHistory: jest.fn(), - addLocationUpdateLogEvent: jest.fn(), - }; higherOrderOperation = new HigherOrderOperations( entitiesCatalog, locationsCatalog, locationReader, - database, getVoidLogger(), ); }); beforeEach(() => { jest.resetAllMocks(); - database.transaction.mockImplementation(async f => f(transaction)); }); describe('addLocation', () => { @@ -117,9 +90,7 @@ describe('HigherOrderOperations', () => { id: expect.anything(), ...spec, }), - { tx: transaction }, ); - expect(transaction.rollback).not.toBeCalled(); }); it('insert the location and its entities', async () => { @@ -139,9 +110,10 @@ describe('HigherOrderOperations', () => { entitiesCatalog.batchAddOrUpdateEntities.mockResolvedValue([ { entityId: 'id', + entity, }, ]); - entitiesCatalog.entities.mockResolvedValue([entity]); + locationReader.read.mockResolvedValue({ entities: [ { @@ -169,13 +141,18 @@ describe('HigherOrderOperations', () => { id: expect.anything(), ...spec, }), - { tx: transaction }, ); expect(locationReader.read).toBeCalledTimes(1); expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' }); - expect(entitiesCatalog.entities).toBeCalledTimes(1); expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledTimes(1); - expect(transaction.rollback).not.toBeCalled(); + expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + locationId: expect.anything(), + dryRun: false, + outputEntities: true, + }), + ); }); it('reuses the location if a match already existed', async () => { @@ -208,7 +185,6 @@ describe('HigherOrderOperations', () => { expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' }); expect(entitiesCatalog.batchAddOrUpdateEntities).not.toBeCalled(); expect(locationsCatalog.addLocation).not.toBeCalled(); - expect(transaction.rollback).not.toBeCalled(); }); it('rejects the whole operation if any entity could not be read', async () => { @@ -235,7 +211,6 @@ describe('HigherOrderOperations', () => { expect(locationsCatalog.locations).toBeCalledTimes(1); expect(entitiesCatalog.batchAddOrUpdateEntities).not.toBeCalled(); expect(locationsCatalog.addLocation).not.toBeCalled(); - expect(transaction.rollback).not.toBeCalled(); }); it('rollback everything after a dry run', async () => { @@ -249,15 +224,14 @@ describe('HigherOrderOperations', () => { kind: 'b', metadata: { name: 'n' }, }; - locationsCatalog.addLocation.mockImplementation(x => Promise.resolve(x)); locationsCatalog.locations.mockResolvedValue([]); locationsCatalog.locations.mockResolvedValue([]); entitiesCatalog.batchAddOrUpdateEntities.mockResolvedValue([ { entityId: 'id', + entity, }, ]); - entitiesCatalog.entities.mockResolvedValue([entity]); locationReader.read.mockResolvedValue({ entities: [ { @@ -281,19 +255,16 @@ describe('HigherOrderOperations', () => { ); expect(result.entities).toEqual([entity]); expect(locationsCatalog.locations).toBeCalledTimes(1); - expect(locationsCatalog.addLocation).toBeCalledTimes(1); - expect(locationsCatalog.addLocation).toBeCalledWith( - expect.objectContaining({ - id: expect.anything(), - ...spec, - }), - { tx: transaction }, - ); expect(locationReader.read).toBeCalledTimes(1); expect(locationReader.read).toBeCalledWith({ type: 'a', target: 'b' }); - expect(entitiesCatalog.entities).toBeCalledTimes(1); expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledTimes(1); - expect(transaction.rollback).toBeCalled(); + expect(entitiesCatalog.batchAddOrUpdateEntities).toBeCalledWith( + expect.anything(), + expect.objectContaining({ + dryRun: true, + outputEntities: true, + }), + ); }); }); diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts index ac21fc6155..902e26e0e3 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts @@ -17,7 +17,6 @@ import { Location, LocationSpec } from '@backstage/catalog-model'; import { v4 as uuidv4 } from 'uuid'; import { Logger } from 'winston'; -import { Database } from '../database'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; import { durationText } from '../util/timing'; import { @@ -38,7 +37,6 @@ export class HigherOrderOperations implements HigherOrderOperation { private readonly entitiesCatalog: EntitiesCatalog, private readonly locationsCatalog: LocationsCatalog, private readonly locationReader: LocationReader, - private readonly database: Database, private readonly logger: Logger, ) {} @@ -85,33 +83,25 @@ export class HigherOrderOperations implements HigherOrderOperation { // in the entities list. But we aren't sure what to do about those yet. // Write - const entities = await this.database.transaction(async tx => { - if (!previousLocation) { - await this.locationsCatalog.addLocation(location, { tx }); - } - if (readerOutput.entities.length === 0) { - return []; - } + if (!previousLocation && !dryRun) { + // TODO: We do not include location operations in the dryRun. We might perform + // this operation as a seperate dry run. + await this.locationsCatalog.addLocation(location); + } + if (readerOutput.entities.length === 0) { + return { location, entities: [] }; + } - const writtenEntities = await this.entitiesCatalog.batchAddOrUpdateEntities( - readerOutput.entities, - { locationId: location.id, tx }, - ); + const writtenEntities = await this.entitiesCatalog.batchAddOrUpdateEntities( + readerOutput.entities, + { + locationId: dryRun ? undefined : location.id, + dryRun, + outputEntities: true, + }, + ); - const outputEntities = await this.entitiesCatalog.entities({ - filters: [{ 'metadata.uid': writtenEntities.map(e => e.entityId) }], - tx, - }); - - if (dryRun) { - // If this is only a dry run, cancel the database transaction even if it was successful. - await tx.rollback(); - - this.logger.debug(`Perfomed successful dry run of adding a location`); - } - - return outputEntities; - }); + const entities = writtenEntities.map(e => e.entity!); return { location, entities }; } diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index f2a4ac7468..b041dac5ab 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -227,7 +227,6 @@ export class CatalogBuilder { entitiesCatalog, locationsCatalog, locationReader, - db, logger, ); diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index 95dc8e82d1..1cf64945ee 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -83,15 +83,13 @@ describe('createRouter', () => { expect(response.status).toEqual(200); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - filters: [ - { - a: ['1', null, '3'], - b: ['4'], - }, - { c: [null] }, - ], - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { + a: ['1', null, '3'], + b: ['4'], + }, + { c: [null] }, + ]); }); }); @@ -109,9 +107,9 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-uid/zzz'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - filters: [{ 'metadata.uid': 'zzz' }], - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { 'metadata.uid': 'zzz' }, + ]); expect(response.status).toEqual(200); expect(response.body).toEqual(expect.objectContaining(entity)); }); @@ -122,9 +120,9 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-uid/zzz'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - filters: [{ 'metadata.uid': 'zzz' }], - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { 'metadata.uid': 'zzz' }, + ]); expect(response.status).toEqual(404); expect(response.text).toMatch(/uid/); }); @@ -145,15 +143,13 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-name/k/ns/n'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - filters: [ - { - 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)); }); @@ -164,15 +160,13 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-name/b/d/c'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - filters: [ - { - 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/); }); @@ -215,9 +209,9 @@ describe('createRouter', () => { { entity, relations: [] }, ]); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - filters: [{ 'metadata.uid': 'u' }], - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { 'metadata.uid': 'u' }, + ]); expect(response.status).toEqual(200); expect(response.body).toEqual(entity); }); diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index e297bf16c6..96b343a184 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -49,7 +49,7 @@ export async function createRouter( .get('/entities', async (req, res) => { const filters = translateQueryToEntityFilters(req.query); const fieldMapper = translateQueryToFieldMapper(req.query); - const entities = await entitiesCatalog.entities({ filters }); + const entities = await entitiesCatalog.entities(filters); res.status(200).send(entities.map(fieldMapper)); }) .post('/entities', async (req, res) => { @@ -57,20 +57,18 @@ export async function createRouter( const [result] = await entitiesCatalog.batchAddOrUpdateEntities([ { entity: body as Entity, relations: [] }, ]); - const [entity] = await entitiesCatalog.entities({ - filters: [{ '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({ - filters: [ - { - 'metadata.uid': uid, - }, - ], - }); + const entities = await entitiesCatalog.entities([ + { + 'metadata.uid': uid, + }, + ]); if (!entities.length) { res.status(404).send(`No entity with uid ${uid}`); } @@ -83,15 +81,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({ - filters: [ - { - 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)