Merge pull request #18359 from backstage/freben/catalog-flake

run `deleteWithEagerPruningOfChildren` test without transactions

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

@@ -81,7 +81,7 @@ export class DefaultProviderDatabase implements ProviderDatabase {
 
     if (toRemove.length) {
       const removedCount = await deleteWithEagerPruningOfChildren({
-        tx,
+        knex: tx,
         entityRefs: toRemove,
         sourceKey: options.sourceKey,
       });

plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.test.ts

@@ -34,26 +34,6 @@ describe('deleteWithEagerPruningOfChildren', () => {
     return knex;
   }
 
-  async function run(
-    knex: Knex,
-    options: { entityRefs: string[]; sourceKey: string },
-  ): Promise<number> {
-    let result: number;
-    await knex.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 deleteWithEagerPruningOfChildren({ tx, ...options });
-      },
-      {
-        // If we explicitly trigger a rollback, don't fail.
-        doNotRejectOnRollback: true,
-      },
-    );
-    return result!;
-  }
-
   async function insertReference(
     knex: Knex,
     ...refs: DbRefreshStateReferencesRow[]
@@ -110,7 +90,11 @@ describe('deleteWithEagerPruningOfChildren', () => {
         { source_key: 'P1', target_entity_ref: 'E4' },
         { source_key: 'P2', target_entity_ref: 'E5' },
       );
-      await run(knex, { sourceKey: 'P1', entityRefs: ['E1', 'E3'] });
+      await deleteWithEagerPruningOfChildren({
+        knex,
+        sourceKey: 'P1',
+        entityRefs: ['E1', 'E3'],
+      });
       await expect(remainingEntities(knex)).resolves.toEqual(['E4', 'E5']);
     },
   );
@@ -139,7 +123,11 @@ describe('deleteWithEagerPruningOfChildren', () => {
         { source_key: 'P1', target_entity_ref: 'E1' },
         { source_key: 'P1', target_entity_ref: 'E2' },
       );
-      await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] });
+      await deleteWithEagerPruningOfChildren({
+        knex,
+        sourceKey: 'P1',
+        entityRefs: ['E1'],
+      });
       await expect(remainingEntities(knex)).resolves.toEqual(['E2']);
     },
   );
@@ -167,7 +155,11 @@ describe('deleteWithEagerPruningOfChildren', () => {
         { source_key: 'P2', target_entity_ref: 'E3' },
         { source_entity_ref: 'E3', target_entity_ref: 'E2' },
       );
-      await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] });
+      await deleteWithEagerPruningOfChildren({
+        knex,
+        sourceKey: 'P1',
+        entityRefs: ['E1'],
+      });
       await expect(remainingEntities(knex)).resolves.toEqual(['E2', 'E3']);
     },
   );
@@ -195,7 +187,11 @@ describe('deleteWithEagerPruningOfChildren', () => {
         { source_key: 'P1', target_entity_ref: 'E3' },
         { source_entity_ref: 'E3', target_entity_ref: 'E2' },
       );
-      await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] });
+      await deleteWithEagerPruningOfChildren({
+        knex,
+        sourceKey: 'P1',
+        entityRefs: ['E1'],
+      });
       await expect(remainingEntities(knex)).resolves.toEqual(['E2', 'E3']);
     },
   );
@@ -228,14 +224,22 @@ describe('deleteWithEagerPruningOfChildren', () => {
         { source_entity_ref: 'E3', target_entity_ref: 'E5' },
         { source_entity_ref: 'E5', target_entity_ref: 'E6' },
       );
-      await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] });
+      await deleteWithEagerPruningOfChildren({
+        knex,
+        sourceKey: 'P1',
+        entityRefs: ['E1'],
+      });
       await expect(remainingEntities(knex)).resolves.toEqual([
         'E3',
         'E4',
         'E5',
         'E6',
       ]);
-      await run(knex, { sourceKey: 'P1', entityRefs: ['E3'] });
+      await deleteWithEagerPruningOfChildren({
+        knex,
+        sourceKey: 'P1',
+        entityRefs: ['E3'],
+      });
       await expect(remainingEntities(knex)).resolves.toEqual([]);
     },
   );
@@ -263,7 +267,11 @@ describe('deleteWithEagerPruningOfChildren', () => {
         { source_key: 'P1', target_entity_ref: 'E3' },
         { source_key: 'P2', target_entity_ref: 'E4' },
       );
-      await run(knex, { sourceKey: 'P1', entityRefs: ['E2', 'E3', 'E4'] });
+      await deleteWithEagerPruningOfChildren({
+        knex,
+        sourceKey: 'P1',
+        entityRefs: ['E2', 'E3', 'E4'],
+      });
       await expect(remainingEntities(knex)).resolves.toEqual([
         'E1',
         'E2',

plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.ts

@@ -26,17 +26,17 @@ import { DbRefreshStateReferencesRow } from '../../tables';
  * the removal of their parents.
  */
 export async function deleteWithEagerPruningOfChildren(options: {
-  tx: Knex.Transaction;
+  knex: Knex | Knex.Transaction;
   entityRefs: string[];
   sourceKey: string;
 }): Promise<number> {
-  const { tx, entityRefs, sourceKey } = options;
+  const { knex, entityRefs, sourceKey } = options;
 
   // Split up the operation by (large) chunks, so that we do not hit database
   // limits for the number of permitted bindings on a precompiled statement
   let removedCount = 0;
   for (const refs of lodash.chunk(entityRefs, 1000)) {
-    removedCount += await tx
+    removedCount += await knex
       .delete()
       .from('refresh_state')
       .whereIn('entity_ref', orphans =>
@@ -186,7 +186,7 @@ export async function deleteWithEagerPruningOfChildren(options: {
     // Delete the references that originate only from this entity provider. Note
     // that there may be more than one entity provider making a "claim" for a
     // given root entity, if they emit with the same location key.
-    await tx<DbRefreshStateReferencesRow>('refresh_state_references')
+    await knex<DbRefreshStateReferencesRow>('refresh_state_references')
       .where('source_key', '=', sourceKey)
       .whereIn('target_entity_ref', refs)
       .delete();