Merge pull request #9672 from jrafanie/improve-database-cleaner-seeded-deletion-thread-safety

Make database cleaner active record seeded deletion start/clean more threadsafe

Author: Fryguy

cypress/e2e/ui/Settings/Application-Settings/tenant.cy.js

@@ -202,7 +202,6 @@ function resetParentTenantForm() {
   );
 }
 
-// TODO: Aside from test that validates deletion, replace with a more reliable cleanup mechanism when ready
 function deleteAccordionItems(accordionsToDelete) {
   cy.get(`#${ACCESS_CONTROL_ACCORDION_ITEM_ID} li.list-group-item`).each(
     (item) => {
@@ -261,28 +260,6 @@ function editQuotasTable(quotaName = ALLOCATED_STORAGE_QUOTA, quotaValue) {
   });
 }
 
-function rollbackQuotas() {
-  cy.toolbar(CONFIG_TOOLBAR_BUTTON, MANAGE_QUOTAS_CONFIG_OPTION);
-  let quotaDisabled = false;
-  cy.get('#rbac_details .miq-data-table table tbody tr').each((row) => {
-    const quotaValueInputWrapper = row.find('div.bx--text-input-wrapper');
-    const isEnabled = !quotaValueInputWrapper.hasClass(
-      'bx--text-input-wrapper--readonly'
-    );
-    if (isEnabled) {
-      quotaDisabled = true;
-      cy.wrap(row).find('span.bx--toggle__switch').click();
-    }
-  });
-  // Save the form only if any quotas are reverted to disabled.
-  cy.then(() => {
-    if (quotaDisabled) {
-      // Saving the form
-      saveFormWithOptionalFlashCheck({ assertFlashMessage: false });
-    }
-  });
-}
-
 describe('Automate Tenant form operations: Settings > Application Settings > Access Control > Tenants', () => {
   beforeEach(() => {
     cy.login();
@@ -301,6 +278,10 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         cy.toolbar(CONFIG_TOOLBAR_BUTTON, EDIT_TENANT_CONFIG_OPTION);
       });
 
+      afterEach(() => {
+        confirmUiNavigation(() => resetParentTenantForm());
+      });
+
       it('Validate Edit tenant form elements', () => {
         validateFormElements();
       });
@@ -325,27 +306,27 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         );
         saveFormWithOptionalFlashCheck({ apiMethod: 'PUT' });
       });
+    });
 
+    describe('Validate Add Project to parent tenant', () => {
       afterEach(() => {
-        confirmUiNavigation(() => resetParentTenantForm());
+        cy.appDbState('restore');
       });
-    });
 
-    describe('Validate Add Project to parent tenant', () => {
       it('Validate Add Project function', () => {
         addProjectToTenant();
       });
-
-      afterEach(() => {
-        confirmUiNavigation(() => deleteAccordionItems([PROJECT_NAME_VALUE]));
-      });
     });
 
     describe('Validate Manage Quotas in parent tenant', () => {
       beforeEach(() => {
         cy.toolbar(CONFIG_TOOLBAR_BUTTON, MANAGE_QUOTAS_CONFIG_OPTION);
       });
 
+      afterEach(() => {
+        cy.appDbState('restore');
+      });
+
       it('Validate Reset & Cancel buttons in Manage Quotas form', () => {
         cy.getFormFooterButtonByTypeWithText({
           buttonText: RESET_BUTTON_TEXT,
@@ -360,10 +341,6 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         editQuotasTable(ALLOCATED_STORAGE_QUOTA, '10');
         saveFormWithOptionalFlashCheck();
       });
-
-      afterEach(() => {
-        confirmUiNavigation(() => rollbackQuotas());
-      });
     });
   });
 
@@ -373,6 +350,10 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         cy.toolbar(CONFIG_TOOLBAR_BUTTON, ADD_CHILD_TENANT_CONFIG_OPTION);
       });
 
+      afterEach(() => {
+        cy.appDbState('restore');
+      });
+
       it('Validate Add child tenant form elements', () => {
         validateFormElements(false);
       });
@@ -415,12 +396,6 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         );
         cancelFormWithOptionalFlashCheck(false);
       });
-
-      afterEach(() => {
-        confirmUiNavigation(() =>
-          deleteAccordionItems([INITIAL_CHILD_TENANT_NAME])
-        );
-      });
     });
 
     describe('Validate Edit child tenant', () => {
@@ -429,6 +404,10 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         cy.toolbar(CONFIG_TOOLBAR_BUTTON, EDIT_TENANT_CONFIG_OPTION);
       });
 
+      afterEach(() => {
+        cy.appDbState('restore');
+      });
+
       it('Validate Edit child tenant form elements', () => {
         validateFormElements();
         cancelFormWithOptionalFlashCheck();
@@ -454,20 +433,17 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         );
         saveFormWithOptionalFlashCheck({ apiMethod: 'PUT' });
       });
-
-      afterEach(() => {
-        deleteAccordionItems([
-          INITIAL_CHILD_TENANT_NAME,
-          EDITED_TENANT_NAME_VALUE,
-        ]);
-      });
     });
 
     describe('Validate Add Project to child tenant', () => {
       beforeEach(() => {
         createAndSelectChildTenant();
       });
 
+      afterEach(() => {
+        cy.appDbState('restore');
+      });
+
       it('Validate Add Project function', () => {
         addProjectToTenant();
       });
@@ -477,13 +453,6 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         deleteAccordionItems([INITIAL_CHILD_TENANT_NAME]);
         cy.expect_flash(flashClassMap.error, FLASH_MESSAGE_CANT_DELETE);
       });
-
-      afterEach(() => {
-        confirmUiNavigation(() => {
-          deleteAccordionItems([PROJECT_NAME_VALUE]);
-          deleteAccordionItems([INITIAL_CHILD_TENANT_NAME]);
-        });
-      });
     });
 
     describe('Validate Manage Quotas in child tenant', () => {
@@ -492,6 +461,10 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         cy.toolbar(CONFIG_TOOLBAR_BUTTON, MANAGE_QUOTAS_CONFIG_OPTION);
       });
 
+      afterEach(() => {
+        cy.appDbState('restore');
+      });
+
       it('Validate Reset & Cancel buttons in Manage Quotas form', () => {
         cy.getFormFooterButtonByTypeWithText({
           buttonText: RESET_BUTTON_TEXT,
@@ -506,12 +479,6 @@ describe('Automate Tenant form operations: Settings > Application Settings > Acc
         editQuotasTable(ALLOCATED_VM_QUOTA, '10');
         saveFormWithOptionalFlashCheck();
       });
-
-      afterEach(() => {
-        confirmUiNavigation(() => {
-          deleteAccordionItems([INITIAL_CHILD_TENANT_NAME]);
-        });
-      });
     });
   });
 });

lib/extensions/database_cleaner-activerecord-seeded_deletion.rb

@@ -1,12 +1,18 @@
 require 'database_cleaner-active_record'
+require 'thread'
 
 module DatabaseCleaner
   module ActiveRecord
     # SeededDeletion is a strategy that deletes all records from tables except those that existed before it was instantiated
     # This is useful for tests that need the seeded data to be present.
     class SeededDeletion < Deletion
+      # Class level mutex for thread safety around start/clean actions.
+      @mutex = Mutex.new
+
       def clean
-        connection.transaction(:requires_new => true) do
+        # Use a transaction isolation to prevent other threads
+        # from modifying the tables during deletion
+        connection.transaction(:requires_new => true, :isolation => :read_committed) do
           super
         end
       end
@@ -17,12 +23,14 @@ def start
       end
 
       def self.table_max_id_cache
-        @table_max_id_cache ||= {}
+        # wrap the cache initialization in a mutex to ensure thread safety
+        @mutex.synchronize { @table_max_id_cache ||= {} }
       end
 
-      # Memoize the maximum ID for each class table with non-zero number of rows
       def self.table_max_id_cache=(table_id_hash)
-        @table_max_id_cache ||= table_id_hash
+        @mutex.synchronize do
+          @table_max_id_cache = table_id_hash
+        end
       end
 
       delegate :table_max_id_cache, to: :class