Port #1413 to dtq-dev-9-base: test: de-flake scheduled ITs (test-planted handle collisions, leaked bitstream formats, stale Solr searcher) (#1413) (#1435)

Source: 23f3786 (dtq-dev PR #1413)

Test-only. All six flake sources the source commit removed exist unchanged on
this branch, so the port is a straight carry-over:

- ItemHandleCheckerIT / RequiredMetadataIT planted a random numeric handle
  (`123456789/` + 1000..1999) that `handle_seq` reaches late in the module run;
  the sequence mint path does no existence check, so whichever test was minting
  at that moment died on the handle unique index. Replaced by deterministic
  non-numeric suffixes (`handle-checker-test`, `required-metadata-test`), and
  the now-unused `randomString()` helper and `java.util.Random` import go away.
- XmlWorkflowFactoryTest used the numeric handle `123456789/999` for the
  non-mapped collection; same collision class, now `123456789/workflow-test-2`.
- PreviewContentServiceImplIT deleted its custom bitstream 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
  `bitstreamformatregistry` row per test, which broke BitstreamFormatRestRepositoryIT's
  counts whenever class order put Preview first. The id is now captured before
  `super.destroy()` and the row deleted afterwards through
  `BitstreamFormatBuilder.deleteBitstreamFormat`, which uses its own Context.
- BitstreamFormatRestRepositoryIT no longer asserts against the hardcoded
  `DEFAULT_AMOUNT_FORMATS = 95`; each affected test reads the live baseline via
  `currentTotalFormats()`. Short descriptions are unique in the format registry,
  so `createMockBitstreamRest()` derives them from the JUnit `@Rule TestName`
  instead of a random number, and `createAdminAccess` captures the created id in
  the `andDo` attached to the status check so cleanup still runs if a later
  assertion fails.
- StatisticsRestRepositoryIT#totalVisitsReport_Item_Visited queried the usage
  report before the posted view event became visible to a new Solr searcher; it
  now commits, exactly as topCountriesReport_Community_Visited already did.

One adaptation vs the source commit, in BitstreamFormatRestRepositoryIT only:
this branch already carries an unrelated refactor that replaced the six
per-test `ObjectMapper mapper = new ObjectMapper();` locals with a single
`@Autowired private ObjectMapper mapper` field. That field is kept and the
source hunks were applied on top of it, so no `new ObjectMapper()` returns.
Every added and removed line of the source patch for this file is otherwise
byte-identical (44 added / 25 removed, verified by diffing the two patches).


(cherry picked from commit 23f3786)

Co-authored-by: Claude Opus 5 (1M context) <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;
 
@@ -59,7 +60,8 @@ public class BitstreamFormatRestRepositoryIT extends AbstractControllerIntegrati
     @Autowired
     private ObjectMapper mapper;
 
-    private final int DEFAULT_AMOUNT_FORMATS = 95;
+    @Rule
+    public TestName testName = new TestName();
 
     @Test
     public void findAllPaginationTest() throws Exception {
@@ -138,7 +140,7 @@ public void findOneNonExistentIDInURL() throws Exception {
 
     @Test
     public void createAdminAccess() throws Exception {
-        BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
+        BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
 
         //Create bitstream format
         String token = getAuthToken(admin.getEmail(), password);
@@ -147,10 +149,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();
 
@@ -170,22 +177,23 @@ 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());
+            }
         }
 
 
     }
 
     @Test
     public void createNonValidSupportLevel() throws Exception {
-        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/")
@@ -195,12 +203,13 @@ 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 {
-        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,12 +219,13 @@ 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 {
-        BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
+        BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
+        int totalFormatsBefore = currentTotalFormats();
         context.turnOffAuthorisationSystem();
         EPerson user = EPersonBuilder.createEPerson(context)
                 .withNameInMetadata("first", "last")
@@ -233,12 +243,13 @@ 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 {
-        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<>();
@@ -262,12 +273,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());
+            }
         }
     }
 
@@ -631,19 +644,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))