From 6bee3ce985a5de59dd87d6d230e997bafb40822c Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Fri, 15 May 2020 14:25:07 +0200 Subject: [PATCH 1/4] feat: deduplicate locations on insert --- plugins/catalog-backend/package.json | 1 + .../src/catalog/DatabaseCatalog.test.ts | 16 +++++++++++++ .../src/catalog/DatabaseCatalog.ts | 24 +++++++++++++++---- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index 629f4c6d18..477cf5e54f 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -21,6 +21,7 @@ "helmet": "^3.22.0", "knex": "^0.21.1", "morgan": "^1.10.0", + "sqlite3": "^4.2.0", "uuid": "^8.0.0", "winston": "^3.2.1", "yaml": "^1.9.2", diff --git a/plugins/catalog-backend/src/catalog/DatabaseCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseCatalog.test.ts index 98a5e9acd4..e244831bfd 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseCatalog.test.ts @@ -60,4 +60,20 @@ describe('DatabaseCatalog', () => { /Found no location/, ); }); + + it('instead of adding second location with the same target, returns existing one', async () => { + // Prepare + const catalog = new DatabaseCatalog(database, logger); + const input = { type: 'a', target: 'b' }; + const output1 = await catalog.addLocation(input); + + // Try to insert the same location + const output2 = await catalog.addLocation(input); + const locations = await catalog.locations(); + + // Output is the same + expect(output2).toEqual(output1); + // Locations contain only one record + expect(locations).toEqual([output1]); + }); }); diff --git a/plugins/catalog-backend/src/catalog/DatabaseCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseCatalog.ts index 87ff5a503f..acf5e02272 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseCatalog.ts @@ -53,7 +53,7 @@ export class DatabaseCatalog implements Catalog { name: descriptor.metadata.name, }; - await this.database.transaction(async (tx) => { + await this.database.transaction(async tx => { // TODO(freben): Currently, several locations can compete for the same component const count = await tx('components') .where({ name: component.name }) @@ -74,7 +74,9 @@ export class DatabaseCatalog implements Catalog { // eslint-disable-next-line @typescript-eslint/no-unused-vars async component(name: string): Promise { - const items = await this.database('components').where({ name }).select(); + const items = await this.database('components') + .where({ name }) + .select(); if (!items.length) { throw new NotFoundError(`Found no component with name ${name}`); } @@ -82,6 +84,10 @@ export class DatabaseCatalog implements Catalog { } async addLocation(location: AddLocationRequest): Promise { + const existingLocation = await this.locationByTarget(location.target); + if (existingLocation) { + return existingLocation; + } const id = uuidv4(); const { type, target } = location; await this.database('locations').insert({ id, type, target }); @@ -89,7 +95,9 @@ export class DatabaseCatalog implements Catalog { } async removeLocation(id: string): Promise { - const result = await this.database('locations').where({ id }).del(); + const result = await this.database('locations') + .where({ id }) + .del(); if (!result) { throw new NotFoundError(`Found no location with ID ${id}`); @@ -97,13 +105,21 @@ export class DatabaseCatalog implements Catalog { } async location(id: string): Promise { - const items = await this.database('locations').where({ id }).select(); + const items = await this.database('locations') + .where({ id }) + .select(); if (!items.length) { throw new NotFoundError(`Found no location with ID ${id}`); } return items[0]; } + private async locationByTarget( + target: string, + ): Promise { + return (await this.locations()).find(l => l.target === target); + } + async locations(): Promise { return await this.database('locations').select(); } From 4ca8e3370e079592b8cd8506c735b5b96a2ab805 Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Mon, 18 May 2020 13:10:10 +0200 Subject: [PATCH 2/4] fix: moved into transaction for atomicity --- .../catalog-backend/src/database/Database.ts | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index fba8728fcd..9b37980c04 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -60,19 +60,31 @@ export class Database { } async addLocation(location: AddDatabaseLocation): Promise { - const existingLocation = await this.locationByTarget(location.target); - if (existingLocation) { - return existingLocation; - } + return await this.database.transaction(async tx => { + const existingLocation = await tx('locations') + .where({ + target: location.target, + }) + .select(); - const id = uuidv4(); - const { type, target } = location; - await this.database('locations').insert({ - id, - type, - target, + if (existingLocation?.[0]) { + return existingLocation[0]; + } + + const id = uuidv4(); + const { type, target } = location; + await tx('locations').insert({ + id, + type, + target, + }); + + return ( + await tx('locations') + .where({ id }) + .select() + )?.[0]; }); - return await this.location(id); } async removeLocation(id: string): Promise { From 6084eeeb4343b05d638cddca269358c1c0461329 Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Mon, 18 May 2020 17:52:51 +0200 Subject: [PATCH 3/4] fix: leftovers --- plugins/catalog-backend/src/database/Database.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index 9b37980c04..04755e8f4d 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -28,7 +28,7 @@ export class Database { constructor(private readonly database: Knex) {} async addOrUpdateComponent(component: AddDatabaseComponent): Promise { - await this.database.transaction(async tx => { + await this.database.transaction(async (tx) => { // TODO(freben): Currently, several locations can compete for the same component // TODO(freben): If locationId is unset in the input, it won't be overwritten - should we instead replace with null? const count = await tx('components') @@ -60,7 +60,7 @@ export class Database { } async addLocation(location: AddDatabaseLocation): Promise { - return await this.database.transaction(async tx => { + return await this.database.transaction(async (tx) => { const existingLocation = await tx('locations') .where({ target: location.target, @@ -80,9 +80,7 @@ export class Database { }); return ( - await tx('locations') - .where({ id }) - .select() + await tx('locations').where({ id }).select() )?.[0]; }); } @@ -107,12 +105,6 @@ export class Database { return items[0]; } - private async locationByTarget( - target: string, - ): Promise { - return (await this.locations()).find(l => l.target === target); - } - async locations(): Promise { return await this.database('locations').select(); } From b9bd2fd52363ef92c49990ef0818c9f1f3e0ea4f Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Tue, 19 May 2020 11:03:01 +0200 Subject: [PATCH 4/4] fix: throw if error --- plugins/catalog-backend/src/database/Database.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index 04755e8f4d..39e075efb8 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -81,7 +81,7 @@ export class Database { return ( await tx('locations').where({ id }).select() - )?.[0]; + )![0]; }); }