From 24ff18621cdf08f1a0d806f0f08198c6e0fe3cb1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Dec 2022 17:27:56 +0100 Subject: [PATCH 1/3] catalog-backend: reference cleanup fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Signed-off-by: Patrik Oldsberg --- .../database/DefaultProcessingDatabase.test.ts | 17 +++++++++++------ .../src/database/DefaultProcessingDatabase.ts | 3 --- .../src/database/DefaultProviderDatabase.ts | 5 +++++ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts index 4fbb38b4fc..540c93ef13 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts +++ b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts @@ -462,12 +462,17 @@ describe('DefaultProcessingDatabase', () => { knexTx( 'refresh_state_references', ).select(), - ).resolves.toEqual([ - expect.objectContaining({ - source_entity_ref: 'location:default/fakelocation', - target_entity_ref: 'component:default/1', - }), - ]); + ).resolves.toEqual( + step.expectConflict + ? [] + : [ + // eslint-disable-next-line jest/no-conditional-expect + expect.objectContaining({ + source_entity_ref: 'location:default/fakelocation', + target_entity_ref: 'component:default/1', + }), + ], + ); expect(mockLogger.error).not.toHaveBeenCalled(); } diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts index 1127deed7e..761f5bfd72 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts +++ b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts @@ -314,7 +314,6 @@ export class DefaultProcessingDatabase implements ProcessingDatabase { // Keeps track of the entities that we end up inserting to update refresh_state_references afterwards const stateReferences = new Array(); - const conflictingStateReferences = new Array(); // Upsert all of the unprocessed entities into the refresh_state table, by // their entity ref. @@ -357,13 +356,11 @@ export class DefaultProcessingDatabase implements ProcessingDatabase { this.options.logger.warn( `Detected conflicting entityRef ${entityRef} already referenced by ${conflictingKey} and now also ${locationKey}`, ); - conflictingStateReferences.push(entityRef); } } // Replace all references for the originating entity or source and then create new ones await tx('refresh_state_references') - .whereNotIn('target_entity_ref', conflictingStateReferences) .andWhere({ source_entity_ref: options.sourceEntityRef }) .delete(); await tx.batchInsert( diff --git a/plugins/catalog-backend/src/database/DefaultProviderDatabase.ts b/plugins/catalog-backend/src/database/DefaultProviderDatabase.ts index f1073fd984..a0d376327b 100644 --- a/plugins/catalog-backend/src/database/DefaultProviderDatabase.ts +++ b/plugins/catalog-backend/src/database/DefaultProviderDatabase.ts @@ -161,6 +161,11 @@ export class DefaultProviderDatabase implements ProviderDatabase { }); } + await tx('refresh_state_references') + .where('target_entity_ref', entityRef) + .andWhere({ source_key: options.sourceKey }) + .delete(); + if (ok) { await tx( 'refresh_state_references', From 22e51086eb761e9b4354d452ba6084d1c8b49a17 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Dec 2022 17:31:18 +0100 Subject: [PATCH 2/3] catalog-backend: add failing test from #15111 Signed-off-by: Patrik Oldsberg --- .../database/DefaultProviderDatabase.test.ts | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/plugins/catalog-backend/src/database/DefaultProviderDatabase.test.ts b/plugins/catalog-backend/src/database/DefaultProviderDatabase.test.ts index 43f2209b60..140925b6fd 100644 --- a/plugins/catalog-backend/src/database/DefaultProviderDatabase.test.ts +++ b/plugins/catalog-backend/src/database/DefaultProviderDatabase.test.ts @@ -610,6 +610,23 @@ describe('DefaultProviderDatabase', () => { }), ]), ); + let references = await knex( + 'refresh_state_references', + ).select(); + expect(references).toEqual([ + { + id: 1, + source_key: 'lols', + source_entity_ref: null, + target_entity_ref: 'component:default/a', + }, + { + id: 2, + source_key: 'lols', + source_entity_ref: null, + target_entity_ref: 'component:default/b', + }, + ]); await db.transaction(async tx => { await db.replaceUnprocessedEntities(tx, { @@ -653,6 +670,23 @@ describe('DefaultProviderDatabase', () => { }), ]), ); + references = await knex( + 'refresh_state_references', + ).select(); + expect(references).toEqual([ + { + id: 2, + source_key: 'lols', + source_entity_ref: null, + target_entity_ref: 'component:default/b', + }, + { + id: 3, + source_key: 'lols', + source_entity_ref: null, + target_entity_ref: 'component:default/a', + }, + ]); }, 60_000, ); From d136793ff0abe67b4d43d16ccfa1b14cfaf72f69 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Dec 2022 17:32:25 +0100 Subject: [PATCH 3/3] changesets: added changeset for catalog reference fix Signed-off-by: Patrik Oldsberg --- .changeset/tasty-impalas-mix.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tasty-impalas-mix.md diff --git a/.changeset/tasty-impalas-mix.md b/.changeset/tasty-impalas-mix.md new file mode 100644 index 0000000000..c830c45c56 --- /dev/null +++ b/.changeset/tasty-impalas-mix.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Fixed an issue where internal references in the catalog would stick around for longer than expected, causing entities to not be deleted or orphaned as expected.