Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
Copilot marked this conversation as resolved.
Comment on lines +204 to +206
*
* @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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -48,6 +50,8 @@
*/
public class AbstractBuilderCleanupUtil {

private static final Logger log = LogManager.getLogger(AbstractBuilderCleanupUtil.class);

private final LinkedHashMap<String, List<AbstractBuilder>> map
= new LinkedHashMap<>();

Expand Down Expand Up @@ -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
* <p>
* 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.
* </p>
* @throws Exception If one or more builders fail to clean up
*/
public void cleanupBuilders() throws Exception {
Exception firstFailure = null;
for (Map.Entry<String, List<AbstractBuilder>> entry : map.entrySet()) {
List<AbstractBuilder> 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;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ public void init() {
@After
@Override
public void destroy() {
context.abort();
super.destroy();
}

Expand Down
Loading