diff --git a/dspace-api/src/test/java/org/dspace/external/CachingOrcidRestConnectorTest.java b/dspace-api/src/test/java/org/dspace/external/CachingOrcidRestConnectorTest.java index 7e8cbc6c94fc..47d8af106079 100644 --- a/dspace-api/src/test/java/org/dspace/external/CachingOrcidRestConnectorTest.java +++ b/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() { diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java index 365e88597d8d..28bbf74a00dd 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java +++ b/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 system properties + * (+ {@link ConfigurationService#reloadConfig()}) instead of {@code configurationService.setProperty(...)}. + * + *

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.

+ */ + 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); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/MetadataBitstreamControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/MetadataBitstreamControllerIT.java index 63f7e62206f8..7fb2f4cecdf8 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/MetadataBitstreamControllerIT.java +++ b/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 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())); } } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java index 6989c208617d..716ccf8b8de6 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java +++ b/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))