From 2d6093198f8c5131a6b5216c1ec15c93d06c3527 Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Tue, 18 Aug 2026 12:07:14 +0200 Subject: [PATCH 1/2] test: use explicit handles the sequence can never mint RequiredMetadataIT and ItemHandleCheckerIT planted a random numeric handle (1000-1999) that handle_seq reaches late in the module run; the sequence mint path does no existence check, so the flush of whichever test was minting at that moment died on the handle unique index. Replace the random handles with deterministic non-numeric suffixes. Co-Authored-By: Claude Fable 5 --- .../java/org/dspace/curate/ItemHandleCheckerIT.java | 11 +++-------- .../java/org/dspace/curate/RequiredMetadataIT.java | 11 +++-------- .../dspace/xmlworkflow/XmlWorkflowFactoryTest.java | 4 +++- 3 files changed, 9 insertions(+), 17 deletions(-) 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) { From 58a2ff9644e679e4a4b0c62ab1c70f290c0acc93 Mon Sep 17 00:00:00 2001 From: milanmajchrak Date: Tue, 18 Aug 2026 12:07:14 +0200 Subject: [PATCH 2/2] test: stop bitstream-format leak, de-randomize format names, commit view event PreviewContentServiceImplIT deleted its custom format through the shared test context after earlier builder deletes had detached it from the thread-bound session, so the delete was silently lost - one leaked row per test broke BitstreamFormatRestRepositoryIT counts when class order put Preview first. totalVisitsReport_Item_Visited could query a stale Solr searcher before its posted view event became visible. Co-Authored-By: Claude Fable 5 --- .../rest/BitstreamFormatRestRepositoryIT.java | 69 ++++++++++++------- .../app/rest/PreviewContentServiceImplIT.java | 14 ++-- .../app/rest/StatisticsRestRepositoryIT.java | 4 ++ 3 files changed, 56 insertions(+), 31 deletions(-) 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))