From 4ae6884a32e4b2c4eb14ab9bd892151d279a9e40 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 28 Jan 2025 12:11:48 +0100 Subject: [PATCH] catalog-backend: include parent refs for deferred entities in cache key Signed-off-by: Patrik Oldsberg --- .changeset/hip-hotels-type.md | 5 ++ .../src/database/DefaultProcessingDatabase.ts | 2 +- plugins/catalog-backend/src/database/types.ts | 2 +- .../DefaultCatalogProcessingEngine.ts | 7 +- .../src/tests/integration.test.ts | 90 +++++++++++++++++++ 5 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 .changeset/hip-hotels-type.md diff --git a/.changeset/hip-hotels-type.md b/.changeset/hip-hotels-type.md new file mode 100644 index 0000000000..6c2d1dddb9 --- /dev/null +++ b/.changeset/hip-hotels-type.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Fixed an bug in the entity processing caching that would prevent entities that were emitted during processing to be restored after being overridden. diff --git a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts index bb5c38de18..0aa31dd36d 100644 --- a/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts +++ b/plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts @@ -272,7 +272,7 @@ export class DefaultProcessingDatabase implements ProcessingDatabase { const rows = await tx( 'refresh_state_references', ) - .where({ target_entity_ref: options.entityRef }) + .whereIn('target_entity_ref', options.entityRefs) .select(); const entityRefs = rows.map(r => r.source_entity_ref!).filter(Boolean); diff --git a/plugins/catalog-backend/src/database/types.ts b/plugins/catalog-backend/src/database/types.ts index ad2be8e06c..82a1ac3f6b 100644 --- a/plugins/catalog-backend/src/database/types.ts +++ b/plugins/catalog-backend/src/database/types.ts @@ -101,7 +101,7 @@ export type ListAncestorsResult = { }; export type ListParentsOptions = { - entityRef: string; + entityRefs: string[]; }; export type ListParentsResult = { diff --git a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts index 59843ac430..a3ef926b47 100644 --- a/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts +++ b/plugins/catalog-backend/src/processing/DefaultCatalogProcessingEngine.ts @@ -215,7 +215,12 @@ export class DefaultCatalogProcessingEngine { const { entityRefs: parents } = await this.processingDatabase.transaction(tx => this.processingDatabase.listParents(tx, { - entityRef, + entityRefs: [ + entityRef, + ...result.deferredEntities.map(e => + stringifyEntityRef(e.entity), + ), + ], }), ); diff --git a/plugins/catalog-backend/src/tests/integration.test.ts b/plugins/catalog-backend/src/tests/integration.test.ts index 92d1b1c143..aa6d241d23 100644 --- a/plugins/catalog-backend/src/tests/integration.test.ts +++ b/plugins/catalog-backend/src/tests/integration.test.ts @@ -1223,4 +1223,94 @@ describe('Catalog Backend Integration', () => { 'component:default/b': withOutputFields(entityBOverride), }); }); + + it('should resolve handle location key conflicts across processed and provided entities', async () => { + const baseEntity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + annotations: { + 'backstage.io/managed-by-location': 'url:.', + 'backstage.io/managed-by-origin-location': 'url:.', + }, + }, + }; + const entityA = merge({ metadata: { name: 'a' } }, baseEntity); + const entityB = merge({ metadata: { name: 'b' } }, baseEntity); + const entityBOverride = merge({ metadata: { override: true } }, entityB); + + const processEntity = jest.fn( + async ( + entity: Entity, + _location: LocationSpec, + _emit: CatalogProcessorEmit, + ) => entity, + ); + const harness = await TestHarness.create({ processEntity }); + + processEntity.mockImplementation(async (entity, location, emit) => { + if (entity.metadata.name === entityA.metadata.name) { + emit( + processingResult.entity(location, entityB, { + locationKey: 'my-key', + }), + ); + } + return entity; + }); + + // Start with just A, which outputs B via processing + await harness.setInputEntities([entityA]); + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/a': withOutputFields(entityA), + 'component:default/b': withOutputFields(entityB), + }); + + // Now apply A and B', but since B already has a different location key, B' should not be used + await harness.setInputEntities([entityA, entityBOverride]); + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/a': withOutputFields(entityA), + 'component:default/b': withOutputFields(entityB), + }); + + // Stop emitting B from A + processEntity.mockImplementation(async e => e); + + // Apply A and B', but since B still has a location key set it should remain, but now orphaned + await harness.setInputEntities([entityA, entityBOverride]); + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/a': withOutputFields(entityA), + 'component:default/b': withOutputFields( + merge(entityB, { + metadata: { annotations: { 'backstage.io/orphan': 'true' } }, + }), + ), + }); + + // Apply A and B' but this time with the matching location key, causing B' to override B + await harness.setInputEntities([ + entityA, + { ...entityBOverride, locationKey: 'my-key' }, + ]); + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/a': withOutputFields(entityA), + 'component:default/b': withOutputFields(entityBOverride), + }); + + // Finally, remove B', leaving just A + await harness.setInputEntities([entityA]); + await expect(harness.process()).resolves.toEqual({}); + + await expect(harness.getOutputEntities()).resolves.toEqual({ + 'component:default/a': withOutputFields(entityA), + }); + }); });