diff --git a/dspace-api/src/test/java/org/dspace/AbstractIntegrationTestWithDatabase.java b/dspace-api/src/test/java/org/dspace/AbstractIntegrationTestWithDatabase.java index d5bb6ab8a4c8..ec85f5bbaf16 100644 --- a/dspace-api/src/test/java/org/dspace/AbstractIntegrationTestWithDatabase.java +++ b/dspace-api/src/test/java/org/dspace/AbstractIntegrationTestWithDatabase.java @@ -166,30 +166,71 @@ public void setUp() throws Exception { */ @After public void destroy() throws Exception { - // Cleanup our global context object + // Contain the blast radius of teardown failures: the shared static state (Solr cores, authority + // cache, configuration, builder cache) must be reset regardless of whether builder cleanup threw. + // Otherwise a single flake in one test's teardown poisons every subsequent test in the class. + Exception primaryFailure = null; + try { AbstractBuilder.cleanupObjects(); - parentCommunity = null; + } catch (Exception e) { + primaryFailure = new RuntimeException("Error cleaning up builder objects", e); + } + parentCommunity = null; + try { cleanupContext(); + } catch (Exception e) { + if (primaryFailure == null) { + primaryFailure = new RuntimeException("Error cleaning up context object", e); + } else { + primaryFailure.addSuppressed(e); + } + } + try { + resetSharedState(); + } catch (Exception e) { + if (primaryFailure == null) { + primaryFailure = e; + } else { + primaryFailure.addSuppressed(e); + } + } + if (primaryFailure != null) { + throw primaryFailure; + } + } + + /** + * Reset all shared static state between tests: Solr cores, authority cache, configuration + * service, and the builder cache. Called from {@link #destroy()} inside a finally block so this always + * runs, even if earlier teardown steps failed. + * + * @throws Exception if reloading configuration or resetting the builder cache fails + */ + private void resetSharedState() throws Exception { + ServiceManager serviceManager = DSpaceServicesFactory.getInstance().getServiceManager(); - ServiceManager serviceManager = DSpaceServicesFactory.getInstance().getServiceManager(); - // Clear the search core. - MockSolrSearchCore searchService = serviceManager - .getServiceByName(null, MockSolrSearchCore.class); - searchService.reset(); - // Clear the statistics core. - serviceManager - .getServiceByName(SolrStatisticsCore.class.getName(), MockSolrStatisticsCore.class) - .reset(); + // Clear the search core. + MockSolrSearchCore searchService = serviceManager + .getServiceByName(null, MockSolrSearchCore.class); + searchService.reset(); - MockSolrLoggerServiceImpl statisticsService = serviceManager - .getServiceByName("solrLoggerService", MockSolrLoggerServiceImpl.class); - statisticsService.reset(); + // Clear the statistics core. + serviceManager + .getServiceByName(SolrStatisticsCore.class.getName(), MockSolrStatisticsCore.class) + .reset(); - MockAuthoritySolrServiceImpl authorityService = serviceManager - .getServiceByName(AuthoritySearchService.class.getName(), MockAuthoritySolrServiceImpl.class); - authorityService.reset(); + // Reset the statistics logger service + MockSolrLoggerServiceImpl loggerService = serviceManager + .getServiceByName("solrLoggerService", MockSolrLoggerServiceImpl.class); + loggerService.reset(); + // Clear the authority core. + MockAuthoritySolrServiceImpl authorityService = serviceManager + .getServiceByName(AuthoritySearchService.class.getName(), MockAuthoritySolrServiceImpl.class); + authorityService.reset(); + + try { // Reload our ConfigurationService (to reset configs to defaults again) DSpaceServicesFactory.getInstance().getConfigurationService().reloadConfig(); diff --git a/dspace-api/src/test/java/org/dspace/builder/util/AbstractBuilderCleanupUtil.java b/dspace-api/src/test/java/org/dspace/builder/util/AbstractBuilderCleanupUtil.java index 18f1f82c290a..d41b9768f3e1 100644 --- a/dspace-api/src/test/java/org/dspace/builder/util/AbstractBuilderCleanupUtil.java +++ b/dspace-api/src/test/java/org/dspace/builder/util/AbstractBuilderCleanupUtil.java @@ -12,6 +12,8 @@ import java.util.List; import java.util.Map; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.dspace.builder.AbstractBuilder; import org.dspace.builder.BitstreamBuilder; import org.dspace.builder.BitstreamFormatBuilder; @@ -48,6 +50,8 @@ */ public class AbstractBuilderCleanupUtil { + private static final Logger log = LogManager.getLogger(AbstractBuilderCleanupUtil.class); + private final LinkedHashMap> map = new LinkedHashMap<>(); @@ -104,15 +108,34 @@ public void addToMap(AbstractBuilder abstractBuilder) { /** * This method takes care of iterating over all the AbstractBuilders in the predefined order and calls * the cleanup method to delete the objects from the database. - * @throws Exception If something goes wrong + *

+ * Each builder is cleaned up inside its own try/catch so that a single failure (e.g. the intermittent + * Hibernate {@link java.util.ConcurrentModificationException} in resource registry cleanup) does not + * abort cleanup of the remaining builders. The first failure is rethrown at the end with any subsequent + * failures attached as suppressed exceptions, so nothing is silently swallowed. + *

+ * @throws Exception If one or more builders fail to clean up */ public void cleanupBuilders() throws Exception { + Exception firstFailure = null; for (Map.Entry> entry : map.entrySet()) { List list = entry.getValue(); for (AbstractBuilder abstractBuilder : list) { - abstractBuilder.cleanup(); + try { + abstractBuilder.cleanup(); + } catch (Exception e) { + log.error("Error cleaning up builder {}", abstractBuilder.getClass().getName(), e); + if (firstFailure == null) { + firstFailure = e; + } else { + firstFailure.addSuppressed(e); + } + } } } + if (firstFailure != null) { + throw firstFailure; + } } /** diff --git a/dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java b/dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java index c5a73ed539a7..ae9f33270999 100644 --- a/dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java +++ b/dspace-api/src/test/java/org/dspace/content/packager/ITDSpaceAIP.java @@ -310,7 +310,6 @@ public void init() { @After @Override public void destroy() { - context.abort(); super.destroy(); }