Merge pull request #15146 from backstage/mob/catafix

catalog-backend: fix sticky refresh state references

.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.

plugins/catalog-backend/src/database/DefaultProcessingDatabase.test.ts

@@ -462,12 +462,17 @@ describe('DefaultProcessingDatabase', () => {
               knexTx<DbRefreshStateReferencesRow>(
                 '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();
           }

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<string>();
-    const conflictingStateReferences = new Array<string>();
 
     // 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<DbRefreshStateReferencesRow>('refresh_state_references')
-      .whereNotIn('target_entity_ref', conflictingStateReferences)
       .andWhere({ source_entity_ref: options.sourceEntityRef })
       .delete();
     await tx.batchInsert(

plugins/catalog-backend/src/database/DefaultProviderDatabase.test.ts

@@ -610,6 +610,23 @@ describe('DefaultProviderDatabase', () => {
             }),
           ]),
         );
+        let references = await knex<DbRefreshStateReferencesRow>(
+          '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<DbRefreshStateReferencesRow>(
+          '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,
     );

plugins/catalog-backend/src/database/DefaultProviderDatabase.ts

@@ -161,6 +161,11 @@ export class DefaultProviderDatabase implements ProviderDatabase {
             });
           }
 
+          await tx<DbRefreshStateReferencesRow>('refresh_state_references')
+            .where('target_entity_ref', entityRef)
+            .andWhere({ source_key: options.sourceKey })
+            .delete();
+
           if (ok) {
             await tx<DbRefreshStateReferencesRow>(
               'refresh_state_references',