diff --git a/dspace-api/src/test/java/org/dspace/curate/ItemHandleCheckerIT.java b/dspace-api/src/test/java/org/dspace/curate/ItemHandleCheckerIT.java index d006c9ebccbd..4a4368f41f14 100644 --- a/dspace-api/src/test/java/org/dspace/curate/ItemHandleCheckerIT.java +++ b/dspace-api/src/test/java/org/dspace/curate/ItemHandleCheckerIT.java @@ -19,7 +19,6 @@ import java.io.IOException; import java.sql.SQLException; import java.util.List; -import java.util.Random; import okhttp3.mockwebserver.Dispatcher; import okhttp3.mockwebserver.MockResponse; @@ -58,7 +57,9 @@ public class ItemHandleCheckerIT extends AbstractIntegrationTestWithDatabase { private static final String TASK_NAME = "checkhandles"; - private static final String HANDLE_COLLECTION = "123456789/" + randomString(); + // Non-numeric suffix the handle sequence can never mint, unique to this class so it cannot + // clash with leftover handle rows of other tests either. + private static final String HANDLE_COLLECTION = "123456789/handle-checker-test"; private static final String HANDLE_ITEM1 = HANDLE_COLLECTION + "-1"; private static final String HANDLE_ITEM2 = HANDLE_COLLECTION + "-2"; @@ -280,10 +281,4 @@ private String getIdentifierUri(Item item) { private List getIdentifierUris(Item item) { return itemService.getMetadata(item, "dc", "identifier", "uri", Item.ANY); } - - private static String randomString() { - Random r = new Random(); - // Generate random integers in range 1000 to 1999 - return String.valueOf(1000 + r.nextInt(1000)); - } } diff --git a/dspace-api/src/test/java/org/dspace/curate/RequiredMetadataIT.java b/dspace-api/src/test/java/org/dspace/curate/RequiredMetadataIT.java index ca9be2329513..d015ce18cb37 100644 --- a/dspace-api/src/test/java/org/dspace/curate/RequiredMetadataIT.java +++ b/dspace-api/src/test/java/org/dspace/curate/RequiredMetadataIT.java @@ -15,7 +15,6 @@ import java.io.IOException; import java.sql.SQLException; -import java.util.Random; import org.dspace.AbstractIntegrationTestWithDatabase; import org.dspace.authorize.AuthorizeException; @@ -44,7 +43,9 @@ public class RequiredMetadataIT extends AbstractIntegrationTestWithDatabase { private static final String TASK_NAME = "requiredmetadata"; - private static final String HANDLE_COLLECTION = "123456789/" + randomString(); + // Non-numeric suffix the handle sequence can never mint, unique to this class so it cannot + // clash with leftover handle rows of other tests either. + private static final String HANDLE_COLLECTION = "123456789/required-metadata-test"; private static final String HANDLE_ITEM1 = HANDLE_COLLECTION + "-1"; private static final String HANDLE_ITEM2 = HANDLE_COLLECTION + "-2"; private static final String HANDLE_ITEM3 = HANDLE_COLLECTION + "-3"; @@ -154,10 +155,4 @@ private static String successResultForItem(Item item) { return "Item: " + item.getHandle() + " has all required fields"; } - private static String randomString() { - Random r = new Random(); - // Generate random integers in range 1000 to 1999 - return String.valueOf(1000 + r.nextInt(1000)); - } - } diff --git a/dspace-api/src/test/java/org/dspace/xmlworkflow/XmlWorkflowFactoryTest.java b/dspace-api/src/test/java/org/dspace/xmlworkflow/XmlWorkflowFactoryTest.java index 03a6a0e949b0..631d8ee56818 100644 --- a/dspace-api/src/test/java/org/dspace/xmlworkflow/XmlWorkflowFactoryTest.java +++ b/dspace-api/src/test/java/org/dspace/xmlworkflow/XmlWorkflowFactoryTest.java @@ -70,7 +70,9 @@ public void init() { this.owningCommunity = communityService.create(null, context); this.mappedCollection = this.collectionService.create(context, owningCommunity, "123456789/workflow-test-1"); - this.nonMappedCollection = this.collectionService.create(context, owningCommunity, "123456789/999"); + // Non-numeric suffix so the handle sequence can never mint a colliding handle + this.nonMappedCollection = + this.collectionService.create(context, owningCommunity, "123456789/workflow-test-2"); //we need to commit the changes so we don't block the table for testing context.restoreAuthSystemState(); } catch (SQLException e) { diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamFormatRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamFormatRestRepositoryIT.java index af963696f0b7..601b152b79d4 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamFormatRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamFormatRestRepositoryIT.java @@ -21,7 +21,6 @@ import java.util.Arrays; import java.util.Map; -import java.util.Random; import java.util.concurrent.atomic.AtomicReference; import com.fasterxml.jackson.databind.ObjectMapper; @@ -39,7 +38,9 @@ import org.dspace.eperson.EPerson; import org.hamcrest.Matchers; import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TestName; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.web.servlet.MvcResult; @@ -56,7 +57,8 @@ public class BitstreamFormatRestRepositoryIT extends AbstractControllerIntegrati @Autowired private BitstreamFormatConverter bitstreamFormatConverter; - private final int DEFAULT_AMOUNT_FORMATS = 95; + @Rule + public TestName testName = new TestName(); @Test public void findAllPaginationTest() throws Exception { @@ -136,7 +138,7 @@ public void findOneNonExistentIDInURL() throws Exception { @Test public void createAdminAccess() throws Exception { ObjectMapper mapper = new ObjectMapper(); - BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false); + BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest(); //Create bitstream format String token = getAuthToken(admin.getEmail(), password); @@ -145,10 +147,15 @@ public void createAdminAccess() throws Exception { AtomicReference idRef = new AtomicReference<>(); try { + // Capture the id right after the status check, so the format is cleaned up even when + // a later assertion in this test fails. MvcResult mvcResult = getClient(token).perform(post("/api/core/bitstreamformats/") .content(mapper.writeValueAsBytes(bitstreamFormatRest)) .contentType(contentType)) .andExpect(status().isCreated()) + .andDo(result -> idRef + .set(read(result.getResponse().getContentAsString(), + "$.id"))) .andExpect(jsonPath("$", HalMatcher.matchNoEmbeds())) .andReturn(); @@ -168,13 +175,13 @@ public void createAdminAccess() throws Exception { bitstreamFormatRest.getMimetype(), bitstreamFormatRest.getDescription(), bitstreamFormatRest.getShortDescription()) - ))) - .andDo(result -> idRef - .set(read(result.getResponse().getContentAsString(), "$.id"))); + ))); } finally { - // Delete the created community (cleanup after ourselves!) - BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get()); + // Delete the created bitstream format (cleanup after ourselves!) + if (idRef.get() != null) { + BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get()); + } } @@ -183,8 +190,9 @@ public void createAdminAccess() throws Exception { @Test public void createNonValidSupportLevel() throws Exception { ObjectMapper mapper = new ObjectMapper(); - BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false); + BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest(); bitstreamFormatRest.setSupportLevel("NONVALID SUPPORT LVL"); + int totalFormatsBefore = currentTotalFormats(); //Attempt to create bitstream with a non-valid support lvl String token = getAuthToken(admin.getEmail(), password); getClient(token).perform(post("/api/core/bitstreamformats/") @@ -194,13 +202,14 @@ public void createNonValidSupportLevel() throws Exception { // Check that no new bitstreamformat was created getClient().perform(get("/api/core/bitstreamformats/")) .andExpect(status().isOk()) - .andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS))); + .andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore))); } @Test public void createNoAccess() throws Exception { ObjectMapper mapper = new ObjectMapper(); - BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false); + BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest(); + int totalFormatsBefore = currentTotalFormats(); //Try to create bitstreamFormat without auth token getClient(null).perform(post("/api/core/bitstreamformats/") @@ -210,13 +219,14 @@ public void createNoAccess() throws Exception { // Check that no new bitstreamformat was created getClient().perform(get("/api/core/bitstreamformats/")) .andExpect(status().isOk()) - .andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS))); + .andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore))); } @Test public void createNonAdminAccess() throws Exception { ObjectMapper mapper = new ObjectMapper(); - BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false); + BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest(); + int totalFormatsBefore = currentTotalFormats(); context.turnOffAuthorisationSystem(); EPerson user = EPersonBuilder.createEPerson(context) .withNameInMetadata("first", "last") @@ -234,13 +244,14 @@ public void createNonAdminAccess() throws Exception { // Check that no new bitstreamformat was created getClient().perform(get("/api/core/bitstreamformats/")) .andExpect(status().isOk()) - .andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS))); + .andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore))); } @Test public void createAlreadyExisting() throws Exception { ObjectMapper mapper = new ObjectMapper(); - BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(true); + BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest(); + int totalFormatsBefore = currentTotalFormats(); // Capture the Id of the created BitstreamFormat (see andDo() below) AtomicReference idRef = new AtomicReference<>(); @@ -264,12 +275,14 @@ public void createAlreadyExisting() throws Exception { // Check that the new bitstreamformat was created only once getClient().perform(get("/api/core/bitstreamformats/")) .andExpect(status().isOk()) - .andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS + 1))); + .andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore + 1))); } finally { - // Delete the created community (cleanup after ourselves!) - BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get()); + // Delete the created bitstream format (cleanup after ourselves!) + if (idRef.get() != null) { + BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get()); + } } } @@ -645,14 +658,13 @@ public void deleteNonAdminAccess() throws Exception { ))); } - private BitstreamFormatRest createRandomMockBitstreamRest(boolean withRand) { + /** + * Short descriptions are unique in the format registry, so derive them from the test method + * name: deterministic and collision-free, unlike the random numbers used before. + */ + private BitstreamFormatRest createMockBitstreamRest() { BitstreamFormatRest bitstreamFormatRest = new BitstreamFormatRest(); - String random = null; - if (withRand) { - Random rand = new Random(); - random = String.valueOf(rand.nextInt(100) + 1); - } - bitstreamFormatRest.setShortDescription("Test short" + random); + bitstreamFormatRest.setShortDescription("Test short " + testName.getMethodName()); bitstreamFormatRest.setDescription("Full description of Test short"); bitstreamFormatRest.setMimetype("text/plain"); bitstreamFormatRest.setSupportLevel("KNOWN"); @@ -660,4 +672,11 @@ private BitstreamFormatRest createRandomMockBitstreamRest(boolean withRand) { bitstreamFormatRest.setExtensions(Arrays.asList("txt", "asc")); return bitstreamFormatRest; } + + private int currentTotalFormats() throws Exception { + String response = getClient().perform(get("/api/core/bitstreamformats/")) + .andExpect(status().isOk()) + .andReturn().getResponse().getContentAsString(); + return read(response, "$.page.totalElements"); + } } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/PreviewContentServiceImplIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/PreviewContentServiceImplIT.java index 47f7564b8eb3..8f4db96f9fc0 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/PreviewContentServiceImplIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/PreviewContentServiceImplIT.java @@ -21,6 +21,7 @@ import org.dspace.app.rest.test.AbstractControllerIntegrationTest; import org.dspace.authorize.AuthorizeException; import org.dspace.builder.BitstreamBuilder; +import org.dspace.builder.BitstreamFormatBuilder; import org.dspace.builder.BundleBuilder; import org.dspace.builder.CollectionBuilder; import org.dspace.builder.CommunityBuilder; @@ -33,7 +34,6 @@ import org.dspace.content.Community; import org.dspace.content.Item; import org.dspace.content.PreviewContent; -import org.dspace.content.service.BitstreamFormatService; import org.dspace.content.service.PreviewContentService; import org.dspace.util.FileInfo; import org.junit.After; @@ -46,8 +46,6 @@ public class PreviewContentServiceImplIT extends AbstractControllerIntegrationTe @Autowired PreviewContentService previewContentService; - @Autowired - BitstreamFormatService bitstreamFormatService; PreviewContent previewContent0; PreviewContent previewContent1; @@ -256,11 +254,15 @@ public void destroy() throws Exception { BitstreamBuilder.deleteBitstream(tarXzFileWithIncorrectMimeType.getID()); // removing custom mime type format created for tarXGzipFile and tgzFileWithGzipMimeType files - if (customMimeTypeFormat != null) { - bitstreamFormatService.delete(context, customMimeTypeFormat); - } + Integer customMimeTypeFormatId = customMimeTypeFormat == null ? null : customMimeTypeFormat.getID(); super.destroy(); + + // Must run in its own Context after super.destroy(): a delete through the shared test + // context is never committed here, which leaked one format row per test. + if (customMimeTypeFormatId != null) { + BitstreamFormatBuilder.deleteBitstreamFormat(customMimeTypeFormatId); + } } @Test diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java index 716ccf8b8de6..1a7ea691febb 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java @@ -403,6 +403,10 @@ public void totalVisitsReport_Item_Visited() throws Exception { .contentType(contentType)) .andExpect(status().isCreated()); + // Commit the view event waiting for a new searcher so it is visible to the report query + // below (same race and fix as in topCountriesReport_Community_Visited). + StatisticsServiceFactory.getInstance().getSolrLoggerService().commit(); + // And request that collection's TotalVisits stat report getClient(adminToken).perform( get("/api/statistics/usagereports/" + itemVisited.getID() + "_" + TOTAL_VISITS_REPORT_ID))