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

[milanmajchrak]

References

• Continues the de-flake series Fix flaky tests in IT pipeline #1321, test: de-flake ORCID cache tests and ZIP-download IT #1344, test: de-flake ItemHandleCheckerIT (mock the live handle resolver) #1346
• Handle randomization history: Fixing RequiredMetadataIT ufal/clarin-dspace#1273
• Failing scheduled runs: 32009094158, 31883563801, 31653657025 (dspace-api); 31147975599, 31873509292, 30976733926 (dspace-server-webapp)

Description

Scheduled integration-test builds on dtq-dev fail intermittently (~1 in 5 runs), each time in a different test. This PR removes the three sources of cross-test interference behind those failures. Test-only changes, no production code touched.

Instructions for Reviewers

List of changes in this PR:

• RequiredMetadataIT, ItemHandleCheckerIT: the explicit collection handle is a deterministic non-numeric suffix (e.g. 123456789/required-metadata-test) instead of 123456789/<1000 + Random.nextInt(1000)>. Handle rows survive object deletion and handle_seq reaches 1000+ late in the module run, so the random handle was intermittently re-minted by the sequence and killed whichever test was minting at that moment (Unique index or primary key violation ... PUBLIC.HANDLE ... '123456789/1154', session left MARKED_ROLLBACK). A non-numeric suffix can never be produced by the sequence; per-class constants keep the Fixing RequiredMetadataIT ufal/clarin-dspace#1273 protection against cross-class handle reuse. XmlWorkflowFactoryTest gets the same treatment for its 123456789/999.
• PreviewContentServiceImplIT: the custom bitstream format is deleted by id in its own Context after super.destroy(). The previous delete went through the shared test context, where it is never committed, leaking one registry row per test (13 total) and breaking BitstreamFormatRestRepositoryIT count assertions whenever class scan order put Preview first.
• BitstreamFormatRestRepositoryIT: short descriptions derived from the test method name instead of Random (also removes the accidental "Test shortnull"), the created format id is captured on the POST so the finally-block cleans up even when a later assertion fails, and count assertions compare against a baseline read at test start instead of the hardcoded registry size.
• StatisticsRestRepositoryIT#totalVisitsReport_Item_Visited: an explicit statistics-core commit (waitSearcher=true) between posting the view event and querying the report, as topCountriesReport_Community_Visited already does. The report could otherwise read a stale Solr searcher and see 0 views. The same pattern exists in the other view-then-assert tests of this class; only the one observed failing is changed here.

To reproduce the two main failures on dtq-dev (both verified in Docker, maven:3.9-eclipse-temurin-11, CI flags):

• Pin the explicit handle in RequiredMetadataIT.randomString() to "20" and run mvn -pl dspace-api verify -DskipIntegrationTests=false -Dit.test=RequiredMetadataIT,SubscribeServiceIT - fails with the exact CI signature. With this PR the planted handle is gone and the same selection is green.
• mvn -pl dspace-server-webapp verify -DskipIntegrationTests=false -Dit.test=PreviewContentServiceImplIT,BitstreamFormatRestRepositoryIT -Dfailsafe.runOrder=reversealphabetical - 4 failures (totalElements 95 vs 108). With this PR: green.

Notes:

• The mint path itself (HandleServiceImpl.createId) still trusts the sequence blindly; hardening it (skip suffixes whose handle already exists) would also cover production minting after handle imports, and is prepared as a possible follow-up PR.
• The IT job's -Dfailsafe.rerunFailingTestsCount=2 is ignored by maven-failsafe-plugin 2.22.x (it reads surefire.rerunFailingTestsCount), so IT reruns never actually ran; the one-line workflow fix comes separately.

Checklist

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & integration tests). Exceptions may be made if previously agreed upon.
• My PR passes Checkstyle validation based on the Code Style Guide.
• My PR includes Javadoc for all new (or modified) public methods and classes. It also includes Javadoc for large or complex private methods.
• My PR passes all tests and includes new/updated Unit or Integration Tests based on the Code Testing Guide.
• If my PR includes new libraries/dependencies (in any pom.xml), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
• If my PR modifies REST API endpoints, I've opened a separate REST Contract PR related to this change.
• If my PR includes new configurations, I've provided basic technical documentation in the PR itself.
• If my PR fixes an issue ticket, I've linked them together.

[Copilot]

Pull request overview

This PR aims to eliminate intermittent failures in scheduled integration-test runs by addressing three independent sources of flakiness: handle mint collisions, leaked BitstreamFormat test data, and Solr statistics visibility races. It does so via a small production change in handle minting plus several test harness adjustments to make test data and timing deterministic.

Changes:

• Make handle minting skip already-existing handle suffixes (sequence catch-up safety) and add an integration test to cover the behavior.
• Fix BitstreamFormat test flakiness by ensuring cleanup is committed and by removing Random-based uniqueness/count assumptions.
• Stabilize statistics report assertions by forcing a Solr commit (waitSearcher) after posting view events.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java	Forces a Solr commit after posting view events to avoid stale-searcher reads in report assertions.
dspace-server-webapp/src/test/java/org/dspace/app/rest/PreviewContentServiceImplIT.java	Ensures BitstreamFormat cleanup runs in its own committed Context to prevent leaked rows across tests.
dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamFormatRestRepositoryIT.java	Makes test data deterministic (method-name-based shortDescription), captures created IDs for robust cleanup, and asserts counts relative to a baseline.
dspace-api/src/test/java/org/dspace/xmlworkflow/XmlWorkflowFactoryTest.java	Switches to a non-numeric handle suffix to avoid collisions with the numeric handle sequence.
dspace-api/src/test/java/org/dspace/handle/HandleServiceIT.java	Adds IT coverage ensuring sequence-based minting skips existing handle rows (including unbound rows after deletion).
dspace-api/src/test/java/org/dspace/curate/RequiredMetadataIT.java	Replaces randomized numeric handle suffixes with deterministic non-numeric suffixes that cannot collide with the sequence.
dspace-api/src/test/java/org/dspace/curate/ItemHandleCheckerIT.java	Same as above: deterministic non-numeric handle suffixes to prevent sequence collisions.
dspace-api/src/main/java/org/dspace/handle/HandleServiceImpl.java	Updates handle minting to skip already-existing handle IDs instead of assuming sequence uniqueness.
dspace-api/src/main/java/org/dspace/handle/HandleClarinServiceImpl.java	Mirrors the handle minting skip logic for the CLARIN handle service implementation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

dspace-api/src/main/java/org/dspace/handle/HandleClarinServiceImpl.java:380

• The new regression tests only exercise HandleServiceImpl through collection creation. HandleClarinServiceImplIT never invokes either blank-handle branch that reaches this new loop, so duplicate skipping in this separate production path remains unverified. Add a test that occupies the next sequence suffix, calls createHandle(context, null) or createExternalHandle with a blank handle, and asserts that the occupied suffix is skipped.

        for (int attempt = 0; attempt < MAX_SUFFIX_ATTEMPTS; attempt++) {
            Long handleSuffix = handleDAO.getNextHandleSuffix(context);
            String handleId = handlePrefix + (handlePrefix.endsWith("/") ? "" : "/") + handleSuffix;
            if (handleDAO.findByHandle(context, handleId) == null) {
                return handleId;

dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamFormatRestRepositoryIT.java:159

• The ID is still captured only after both POST assertions run. If the POST successfully inserts the format but matchNoEmbeds() fails, andDo is never reached, idRef remains null, and this test leaks the row—the cleanup failure this change is meant to prevent. Register andDo immediately after perform, before any assertions.

                                                  .andDo(result -> idRef
                                                          .set(read(result.getResponse().getContentAsString(),
                                                                  "$.id")))

[milanmajchrak]

Restructured to test-only changes (2 commits): the mint-path hardening in HandleServiceImpl/HandleClarinServiceImpl and its HandleServiceIT regression tests were dropped from this PR and are prepared separately. The earlier resolved review threads refer to that removed code.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamFormatRestRepositoryIT.java:159

• Move the ID capture before the HAL assertion. andExpect(...) evaluates immediately, so if matchNoEmbeds() fails, this andDo is never reached, idRef remains null, and the newly created format leaks—the cleanup path this change is intended to guarantee. Keeping the status check first avoids parsing an error response while still capturing the ID before later assertions.

                                                  .andDo(result -> idRef
                                                          .set(read(result.getResponse().getContentAsString(),
                                                                  "$.id")))

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.