test: de-flake ORCID cache tests and ZIP-download IT

dspace-api/src/test/java/org/dspace/external/CachingOrcidRestConnectorTest.java

@@ -19,7 +19,10 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
 import org.dspace.AbstractDSpaceTest;
 import org.dspace.external.provider.orcid.xml.ExpandedSearchConverter;
 import org.dspace.utils.DSpace;
@@ -53,6 +56,19 @@ private InputStream cannedResponse(String resource) {
         return is;
     }
 
+    /**
+     * Build a canned 200 OK ORCID "expanded-search" response for the mock HTTP server, so the cache-aware
+     * tests below exercise the real Spring {@code @Cacheable} bean without depending on the live ORCID sandbox.
+     */
+    private MockResponse cannedOrcidResponse() throws IOException {
+        try (InputStream is = cannedResponse(EXPANDED_SEARCH_XML)) {
+            return new MockResponse()
+                    .setResponseCode(200)
+                    .setHeader("Content-Type", "application/vnd.orcid+xml")
+                    .setBody(new String(is.readAllBytes(), StandardCharsets.UTF_8));
+        }
+    }
+
     @Before
     public void setup() {
         sut = new CachingOrcidRestConnector();
@@ -134,7 +150,7 @@ public void search_fail() throws Exception {
     }
 
     @Test
-    public void testCachable() {
+    public void testCachable() throws IOException {
         CachingOrcidRestConnector c = new DSpace().getServiceManager().getServiceByName(
                 "CachingOrcidRestConnector", CachingOrcidRestConnector.class);
 
@@ -148,43 +164,56 @@ public void testCachable() {
         verify(c, times(1)).getLabel(orcid);
         */
 
-        c.setApiURL("https://pub.sandbox.orcid.org/v3.0");
-        c.forceAccessToken(sandboxToken);
-
-        String r1 = c.getLabel(orcid);
-        assertEquals(expectedLabel, r1);
-        String r2 = c.getLabel(orcid);
-        assertEquals(expectedLabel, r2);
-        //get the orcid-labels cache and verify that the label is there
-        assertEquals(expectedLabel, cache.get(orcid).get());
+        // Drive the real Spring @Cacheable bean against a local mock HTTP server instead of the live ORCID
+        // sandbox, whose dataset is periodically reset and previously caused intermittent CI failures.
+        try (MockWebServer server = new MockWebServer()) {
+            // Two responses are enqueued, but with caching working only the FIRST getLabel() hits the server;
+            // the second is served from the "orcid-labels" cache (asserted via getRequestCount() below).
+            server.enqueue(cannedOrcidResponse());
+            server.enqueue(cannedOrcidResponse());
+
+            c.setApiURL(server.url("/v3.0").toString());
+            c.forceAccessToken(sandboxToken);
+
+            String r1 = c.getLabel(orcid);
+            assertEquals(expectedLabel, r1);
+            String r2 = c.getLabel(orcid);
+            assertEquals(expectedLabel, r2);
+            //get the orcid-labels cache and verify that the label is there
+            assertEquals(expectedLabel, cache.get(orcid).get());
+            //caching means two getLabel() calls produced a single ORCID API request
+            assertEquals("Expected getLabel to be cached after the first call", 1, server.getRequestCount());
+        }
     }
 
     @Test
-    public void testCacheableWithError() {
+    public void testCacheableWithError() throws IOException {
         CachingOrcidRestConnector c = new DSpace().getServiceManager().getServiceByName(
                 "CachingOrcidRestConnector", CachingOrcidRestConnector.class);
 
         Cache cache = prepareCache();
         assertNull(cache.get(orcid));
 
-        //skip init
-        c.forceAccessToken(sandboxToken);
-        //set bad ApiURL to provoke an error
-        c.setApiURL("https://api.sandbox.orcid.org/");
-        String r1 = c.getLabel(orcid);
-        //on error, getLabel should return null
-        assertNull(r1);
-        //the cache should not contain a value for this id
-        assertNull(cache.get(orcid));
-
-        //fix the error
-        c.setApiURL("https://pub.sandbox.orcid.org/v3.0");
-        // the error flipped the initialized flag, this reset it
-        c.forceAccessToken(sandboxToken);
-        String r2 = c.getLabel(orcid);
-        assertEquals(expectedLabel, r2);
-        //the cache should now contain a value for this id
-        assertEquals(expectedLabel, cache.get(orcid).get());
+        try (MockWebServer server = new MockWebServer()) {
+            //the (mock) ORCID API returns an error first, then a valid response
+            server.enqueue(new MockResponse().setResponseCode(500));
+            server.enqueue(cannedOrcidResponse());
+
+            //skip init (force a token so getAccessToken/init never reaches out to the network)
+            c.forceAccessToken(sandboxToken);
+            c.setApiURL(server.url("/v3.0").toString());
+            String r1 = c.getLabel(orcid);
+            //on error, getLabel should return null
+            assertNull(r1);
+            //a null result must NOT be cached (see @Cacheable(unless = "#result == null"))
+            assertNull(cache.get(orcid));
+
+            //the second call gets the valid (200) response; the error never cleared the token, so no re-init needed
+            String r2 = c.getLabel(orcid);
+            assertEquals(expectedLabel, r2);
+            //the cache should now contain a value for this id
+            assertEquals(expectedLabel, cache.get(orcid).get());
+        }
     }
 
     private Cache prepareCache() {

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/MetadataBitstreamControllerIT.java

@@ -7,17 +7,20 @@
  */
 package org.dspace.app.rest;
 
+import static org.junit.Assert.assertEquals;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
-import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
-import java.io.ByteArrayOutputStream;
+import java.io.ByteArrayInputStream;
 import java.io.InputStream;
-import java.util.zip.Deflater;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
 
 import org.apache.commons.codec.CharEncoding;
-import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
-import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
 import org.apache.commons.io.IOUtils;
 import org.dspace.app.rest.model.ItemRest;
 import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
@@ -29,7 +32,6 @@
 import org.dspace.content.Bitstream;
 import org.dspace.content.Collection;
 import org.dspace.content.Item;
-import org.dspace.content.service.BitstreamService;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 
@@ -38,6 +40,7 @@ public class MetadataBitstreamControllerIT extends AbstractControllerIntegration
     private static final String ALL_ZIP_PATH = "allzip";
     private static final String HANDLE_PARAM = "handleId";
     private static final String AUTHOR = "Test author name";
+    private static final String BITSTREAM_CONTENT = "ThisIsSomeDummyText";
     private Collection col;
 
     private Item publicItem;
@@ -46,9 +49,6 @@ public class MetadataBitstreamControllerIT extends AbstractControllerIntegration
     @Autowired
     AuthorizeService authorizeService;
 
-    @Autowired
-    BitstreamService bitstreamService;
-
 
     @Override
     public void setUp() throws Exception {
@@ -64,7 +64,7 @@ public void setUp() throws Exception {
                 .withAuthor(AUTHOR)
                 .build();
 
-        String bitstreamContent = "ThisIsSomeDummyText";
+        String bitstreamContent = BITSTREAM_CONTENT;
         try (InputStream is = IOUtils.toInputStream(bitstreamContent, CharEncoding.UTF_8)) {
             bts = BitstreamBuilder.
                     createBitstream(context, publicItem, is)
@@ -78,23 +78,30 @@ public void setUp() throws Exception {
 
     @Test
     public void downloadAllZip() throws Exception {
-        ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
-        ZipArchiveOutputStream zip = new ZipArchiveOutputStream(byteArrayOutputStream);
-        zip.setCreateUnicodeExtraFields(ZipArchiveOutputStream.UnicodeExtraFieldPolicy.ALWAYS);
-        zip.setLevel(Deflater.NO_COMPRESSION);
-        ZipArchiveEntry ze = new ZipArchiveEntry(bts.getName());
-        zip.putArchiveEntry(ze);
-        InputStream is = bitstreamService.retrieve(context, bts);
-        org.apache.commons.compress.utils.IOUtils.copy(is, zip);
-        zip.closeArchiveEntry();
-        is.close();
-        zip.close();
-
         String token = getAuthToken(admin.getEmail(), password);
-        getClient(token).perform(get(METADATABITSTREAM_ENDPOINT + "/" + publicItem.getID() +
+        byte[] zipBytes = getClient(token).perform(get(METADATABITSTREAM_ENDPOINT + "/" + publicItem.getID() +
                         "/" + ALL_ZIP_PATH).param(HANDLE_PARAM, publicItem.getHandle()))
                 .andExpect(status().isOk())
-                .andExpect(content().bytes(byteArrayOutputStream.toByteArray()));
+                .andReturn().getResponse().getContentAsByteArray();
+
+        // A ZIP entry stores a DOS last-modified timestamp that defaults to "now" at 2-second resolution, so
+        // comparing the response byte-for-byte against a locally-built ZIP intermittently failed when the server
+        // and the test happened to build their entries in different time buckets. Assert the meaningful payload
+        // instead: the archive must contain exactly the item's bitstream, with the expected content.
+        Map<String, String> entries = new HashMap<>();
+        int entryCount = 0;
+        try (ZipInputStream zis = new ZipInputStream(new ByteArrayInputStream(zipBytes))) {
+            ZipEntry entry;
+            while ((entry = zis.getNextEntry()) != null) {
+                entryCount++;
+                entries.put(entry.getName(), new String(IOUtils.toByteArray(zis), StandardCharsets.UTF_8));
+                zis.closeEntry();
+            }
+        }
 
+        // count tracked separately so a duplicate entry name can't be masked by the map
+        assertEquals(1, entryCount);
+        assertEquals(Set.of(bts.getName()), entries.keySet());
+        assertEquals(BITSTREAM_CONTENT, entries.get(bts.getName()));
     }
 }

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

@@ -938,6 +938,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))