CLARIN-DSpace v9/Port #1346 + #1344 (CI test de-flakes) to the v9 base (#1379)

* test: de-flake ItemHandleCheckerIT (mock the live handle resolver) (#1346)

(cherry picked from commit 867e43d on dtq-dev)

Clean conflict-free pick, no v9 adaptations needed (test-only; okhttp3
mockwebserver 4.12.0 already a test-scope dep on v9-base).

Fulfils CLARIN_V9_POST_SNAPSHOT_SYNC_ACCEPTANCE.md §5 / 867e43d (BE-2, Vlna 1).

* test: de-flake ORCID cache tests and ZIP-download IT (#1344)

(cherry picked from commit f6f1356 on dtq-dev — Part 1 of 2)

v9 adaptations / conflict resolution:
- CachingOrcidRestConnectorTest.java and MetadataBitstreamControllerIT.java do not
  exist on dtq-dev-9-base (delete/modify conflicts) — resolved with git rm to DEFER
  them; when the backlog test-port items land (CLARIN_V9_PARITY_BACKLOG.md lines
  217/514/530), those files MUST be sourced from f6f1356 state (== fork head),
  never from snapshot 8f4b80d, or the flaky byte-for-byte ZIP assert and the live
  ORCID sandbox dependency get re-introduced.
- AuthorizationRestRepositoryIT (SSR authz de-flake via JVM system props +
  reloadConfig + @after clearSsrObjectResolution) and StatisticsRestRepositoryIT
  (solrLoggerService.commit() before TopCountries report) merged clean, no adaptation.

Fulfils CLARIN_V9_POST_SNAPSHOT_SYNC_ACCEPTANCE.md §5 / f6f1356 (BE-2, Vlna 1),
ACs 8-9 DEFERRED per the card.

Author: milanmajchrak

dspace-api/src/test/java/org/dspace/curate/ItemHandleCheckerIT.java

@@ -21,6 +21,10 @@
 import java.util.List;
 import java.util.Random;
 
+import okhttp3.mockwebserver.Dispatcher;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import okhttp3.mockwebserver.RecordedRequest;
 import org.dspace.AbstractIntegrationTestWithDatabase;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.builder.ItemBuilder;
@@ -43,6 +47,12 @@
 /**
  * Test for checkhandles curation task.
  *
+ * <p>The handle URLs are served by a local {@link MockWebServer} instead of the live handle resolver
+ * (http://hdl.handle.net/), which the task contacts over HTTP. Hitting the live resolver made these tests
+ * flaky: when the network was slow the HEAD request timed out and the task reported {@code 617}
+ * (SocketTimeoutException) instead of the expected status. The mock dispatcher returns deterministic
+ * responses keyed by path.</p>
+ *
  * @author mkuchtiak
  */
 public class ItemHandleCheckerIT extends AbstractIntegrationTestWithDatabase {
@@ -55,11 +65,10 @@ public class ItemHandleCheckerIT extends AbstractIntegrationTestWithDatabase {
     private static final String HANDLE_ITEM3 = HANDLE_COLLECTION + "-3";
     private static final String HANDLE_ITEM4 = HANDLE_COLLECTION + "-4";
     private static final String HANDLE_NON_EXISTING = HANDLE_COLLECTION + "-999";
-    private static final String HANDLE_URL_REAL = "http://hdl.handle.net/20.1000/5555";
-    private static final String HANDLE_INVALID = HANDLE_URL_REAL + "/..??^^/";
+    // Path (relative to the mock server) of a handle that the resolver answers with a 302 redirect to a 200 page.
+    private static final String HANDLE_REDIRECT_PATH = "20.1000/5555";
     private static final String HANDLE_IGNORED_1 = "11234/998";
     private static final String HANDLE_IGNORED_2 = "11234/999";
-    private static final String HANDLE_URL_IGNORED = "http://hdl.handle.net/" + HANDLE_IGNORED_2;
 
     protected CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService();
     protected CollectionService collectionService = ContentServiceFactory.getInstance().getCollectionService();
@@ -77,15 +86,50 @@ public class ItemHandleCheckerIT extends AbstractIntegrationTestWithDatabase {
     private Curator curator;
     private CuratorReportTest.ListReporter reporter;
 
+    // Local stand-in for the handle resolver. Started in setUp(); its base URL becomes handle.canonical.prefix.
+    private MockWebServer mockHandleServer;
+    // Previous handle.canonical.prefix, captured in setUp and restored in destroy so the shared config is not
+    // left pointing at the now-closed mock server for later tests in the same JVM.
+    private String originalHandlePrefix;
+    // URLs that point at the mock server (computed from its dynamic port in setUp).
+    private String handleUrlReal;
+    private String handleUrlRedirectTarget;
+    private String handleInvalid;
+    private String handleUrlIgnored;
+
     @Before
     @Override
     public void setUp() throws Exception {
         super.setUp();
         CoreServiceFactory.getInstance().getPluginService().clearNamedPluginClasses();
         try {
+            // Serve handle URLs from a local mock server so the task never contacts the live resolver.
+            mockHandleServer = new MockWebServer();
+            String baseUrl = mockHandleServer.url("/").toString();
+            handleUrlReal = baseUrl + HANDLE_REDIRECT_PATH;
+            handleUrlRedirectTarget = baseUrl + HANDLE_REDIRECT_PATH + "-target";
+            handleInvalid = handleUrlReal + "/..??^^/";
+            handleUrlIgnored = baseUrl + HANDLE_IGNORED_2;
+            mockHandleServer.setDispatcher(new Dispatcher() {
+                @Override
+                public MockResponse dispatch(RecordedRequest request) {
+                    String path = request.getPath();
+                    if (("/" + HANDLE_REDIRECT_PATH).equals(path)) {
+                        // a "real" handle: 302 redirect; the task follows redirects manually via the Location header
+                        return new MockResponse().setResponseCode(302).setHeader("Location", handleUrlRedirectTarget);
+                    }
+                    if (("/" + HANDLE_REDIRECT_PATH + "-target").equals(path)) {
+                        return new MockResponse().setResponseCode(200);
+                    }
+                    // any other (well-formed, non-ignored) handle URL is treated as "not found"
+                    return new MockResponse().setResponseCode(404);
+                }
+            });
+
             //we have to create a new community in the database
             context.turnOffAuthorisationSystem();
-            cfg.setProperty("handle.canonical.prefix", "http://hdl.handle.net/");
+            originalHandlePrefix = cfg.getProperty("handle.canonical.prefix");
+            cfg.setProperty("handle.canonical.prefix", baseUrl);
             cfg.setProperty("curate.checklist.ignore", HANDLE_IGNORED_1 + "," + HANDLE_IGNORED_2);
 
             this.parentCommunity = communityService.create(null, context);
@@ -131,7 +175,7 @@ public void testItemHandleNotFound() throws IOException {
 
     @Test
     public void testItemHandleRedirected() throws IOException {
-        replaceHandleUrl(item2, HANDLE_URL_REAL);
+        replaceHandleUrl(item2, handleUrlReal);
         curator.curate(context, HANDLE_ITEM2);
         assertEquals("Curation should succeed", Curator.CURATE_SUCCESS, curator.getStatus(TASK_NAME));
         assertTrue(curator.getResult(TASK_NAME).contains(redirectedResultForItem(item2)));
@@ -150,18 +194,18 @@ public void testNonExistingHandle() throws IOException {
 
     @Test
     public void testInvalidHandleUrl() throws IOException {
-        replaceHandleUrl(item3, HANDLE_INVALID);
+        replaceHandleUrl(item3, handleInvalid);
         curator.curate(context, HANDLE_ITEM3);
         assertEquals("Curation should fail", Curator.CURATE_FAIL, curator.getStatus(TASK_NAME));
         String singleReport = reporter.getReport().get(0);
-        assertTrue(singleReport.contains(HANDLE_INVALID + " = 500 - FAILED\n"));
+        assertTrue(singleReport.contains(handleInvalid + " = 500 - FAILED\n"));
         assertTrue(singleReport.contains("Error: java.net.URISyntaxException: Illegal character"));
         reporter.getReport().clear();
     }
 
     @Test
     public void testHandleUrlIgnored() throws IOException {
-        replaceHandleUrl(item4, HANDLE_URL_IGNORED);
+        replaceHandleUrl(item4, handleUrlIgnored);
         curator.curate(context, HANDLE_ITEM4);
         assertEquals("Curation should skip", Curator.CURATE_SKIP, curator.getStatus(TASK_NAME));
         assertEquals("Item: " + HANDLE_ITEM4 + "\n", reporter.getReport().get(0));
@@ -170,9 +214,9 @@ public void testHandleUrlIgnored() throws IOException {
 
     @Test
     public void testCurateCollection() throws IOException {
-        replaceHandleUrl(item2, HANDLE_URL_REAL);
-        replaceHandleUrl(item3, HANDLE_INVALID);
-        replaceHandleUrl(item4, HANDLE_URL_IGNORED);
+        replaceHandleUrl(item2, handleUrlReal);
+        replaceHandleUrl(item3, handleInvalid);
+        replaceHandleUrl(item4, handleUrlIgnored);
         curator.curate(context, HANDLE_COLLECTION);
         // the final curator status is derived from the status of the latest checked item
         // so the final curator status is unpredictable
@@ -184,14 +228,21 @@ public void testCurateCollection() throws IOException {
                 both(
                      containsString("Item: " + item2.getHandle())).and(containsString(" = 200 - OK\n")
                 ), // item2
-                containsString(HANDLE_INVALID + " = 500 - FAILED"), // item 3
+                containsString(handleInvalid + " = 500 - FAILED"), // item 3
                 is("Item: " + HANDLE_ITEM4 + "\n") // item 4 (ignored)
         ));
     }
 
     @After
     @Override
     public void destroy() throws Exception {
+        if (mockHandleServer != null) {
+            mockHandleServer.close();
+        }
+        // restore the shared config so a later test is not left pointing at the now-closed mock server
+        if (originalHandlePrefix != null) {
+            cfg.setProperty("handle.canonical.prefix", originalHandlePrefix);
+        }
         // remove all registered handles properly
         identifierService.delete(context, item1, HANDLE_ITEM1);
         identifierService.delete(context, item2, HANDLE_ITEM2);

dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java

@@ -63,6 +63,7 @@
 import org.dspace.eperson.Group;
 import org.dspace.services.ConfigurationService;
 import org.hamcrest.Matchers;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -134,6 +135,42 @@ public class AuthorizationRestRepositoryIT extends AbstractControllerIntegration
      */
     private AuthorizationFeature trueForUsersInGroupTest;
 
+    private static final String SSR_URL_KEY = "dspace.server.ssr.url";
+    private static final String SSR_URL = "http://ssr.example.com/api";
+    private static final String ALWAYS_THROW_TURNOFF_KEY =
+            "org.dspace.app.rest.authorization.AlwaysThrowExceptionFeature.turnoff";
+
+    /**
+     * Configure the SSR object-by-URI resolution used by the {@code search/object} tests (disarm the
+     * AlwaysThrowExceptionFeature and define the SSR base URL), setting both as JVM <em>system properties</em>
+     * (+ {@link ConfigurationService#reloadConfig()}) instead of {@code configurationService.setProperty(...)}.
+     *
+     * <p>A plain {@code setProperty(...)} override only lives in the in-memory combined-config view and is
+     * silently dropped whenever that view is rebuilt by the auto-reload listener (which fires when any
+     * reloadable cfg file's mtime changes, e.g. another test writing {@code local.cfg}). When that rebuild
+     * lands mid-test the SSR url disappears and {@code search/object} no longer resolves the uri -> 400 (and a
+     * dropped turnoff would let {@code alwaysexception} throw -> 500). A system property sits in the
+     * highest-precedence override layer and is re-read on every rebuild, so it survives auto-reload; it is
+     * cleared in {@link #clearSsrObjectResolution()}. Same pattern as
+     * AuthenticationRestControllerIT#setAuthenticationMethodSequence.</p>
+     */
+    private void enableSsrObjectResolution() {
+        System.setProperty(ALWAYS_THROW_TURNOFF_KEY, "true");
+        System.setProperty(SSR_URL_KEY, SSR_URL);
+        configurationService.reloadConfig();
+    }
+
+    /**
+     * Remove the system-property overrides set by {@link #enableSsrObjectResolution()} so they do not leak into
+     * other tests in the same JVM. Runs before the superclass {@code @After}, whose {@code reloadConfig()} then
+     * restores the on-disk defaults.
+     */
+    @After
+    public void clearSsrObjectResolution() {
+        System.clearProperty(SSR_URL_KEY);
+        System.clearProperty(ALWAYS_THROW_TURNOFF_KEY);
+    }
+
     @Override
     @Before
     public void setUp() throws Exception {
@@ -852,9 +889,9 @@ public void findByObjectSSRTest() throws Exception {
         SiteRest siteRest = siteConverter.convert(site, DefaultProjection.DEFAULT);
         String siteUri = "http://ssr.example.com/api/core/sites/" + siteRest.getId();
 
-        // disarm the alwaysThrowExceptionFeature
-        configurationService.setProperty("org.dspace.app.rest.authorization.AlwaysThrowExceptionFeature.turnoff", true);
-        configurationService.setProperty("dspace.server.ssr.url", "http://ssr.example.com/api");
+        // Disarm the alwaysThrowExceptionFeature and define the SSR base URL via system properties so the
+        // overrides survive a mid-test config auto-reload (see enableSsrObjectResolution).
+        enableSsrObjectResolution();
 
         String adminToken = getAuthToken(admin.getEmail(), password);
         String epersonToken = getAuthToken(eperson.getEmail(), password);
@@ -969,9 +1006,9 @@ public void findByObjectSSRTest() throws Exception {
      */
     @Test
     public void findByObjectBadRequestSSRTest() throws Exception {
-        // disarm the alwaysThrowExceptionFeature
-        configurationService.setProperty("org.dspace.app.rest.authorization.AlwaysThrowExceptionFeature.turnoff", true);
-        configurationService.setProperty("dspace.server.ssr.url", "http://ssr.example.com/api");
+        // Disarm the alwaysThrowExceptionFeature and define the SSR base URL via system properties so the
+        // overrides survive a mid-test config auto-reload (see enableSsrObjectResolution).
+        enableSsrObjectResolution();
         String[] invalidUris = new String[] {
             "invalid-uri",
             "",
@@ -1050,9 +1087,9 @@ public void findByObjectBadRequestSSRTest() throws Exception {
     public void findByNotExistingObjectSSSTest() throws Exception {
         String wrongSiteUri = "http://localhost/api/core/sites/" + UUID.randomUUID();
 
-        // disarm the alwaysThrowExceptionFeature
-        configurationService.setProperty("org.dspace.app.rest.authorization.AlwaysThrowExceptionFeature.turnoff", true);
-        configurationService.setProperty("dspace.server.ssr.url", "http://ssr.example.com/api");
+        // Disarm the alwaysThrowExceptionFeature and define the SSR base URL via system properties so the
+        // overrides survive a mid-test config auto-reload (see enableSsrObjectResolution).
+        enableSsrObjectResolution();
 
         String adminToken = getAuthToken(admin.getEmail(), password);
 

dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java

@@ -924,6 +924,13 @@ public void topCountriesReport_Community_Visited() throws Exception {
             .contentType(contentType))
                                 .andExpect(status().isCreated());
 
+        // Force a commit that waits for a new searcher so the two just-posted view events are guaranteed to be
+        // flushed and visible to the report query below. This covers both ways the report could otherwise come
+        // back empty: postView()'s own commit uses waitSearcher=false (can return before the searcher reopens),
+        // and if the solr-statistics.autoCommit=false override were dropped by a mid-test config reload then
+        // postView() skips its commit entirely (Solr's own autoCommit only fires after 10s).
+        StatisticsServiceFactory.getInstance().getSolrLoggerService().commit();
+
         // And request that collection's TopCountries report
         getClient(adminToken).perform(
             get("/api/statistics/usagereports/" + communityVisited.getID() + "_" + TOP_COUNTRIES_REPORT_ID))