test: de-flake scheduled ITs (test-planted handle collisions, leaked bitstream formats, stale Solr searcher) (#1413)

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: milanmajchrak

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<MetadataValue> 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));
-    }
 }

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));
-    }
-
 }

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) {

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<Integer> 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<Integer> 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,19 +658,25 @@ 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");
         bitstreamFormatRest.setInternal(false);
         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");
+    }
 }

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

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))