From 88e6b2f65e0f366bfea54fec4f976acd3c413e90 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Wed, 16 Jul 2025 15:10:49 -0500 Subject: [PATCH 1/6] Fix broken Windows tests by using Paths to split file path strings instead of regex. (cherry picked from commit 4e425b09080f4a2f1f30a0f23147be9c4aac33f7) --- .../bitstore/JCloudBitStoreServiceTest.java | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/storage/bitstore/JCloudBitStoreServiceTest.java b/dspace-api/src/test/java/org/dspace/storage/bitstore/JCloudBitStoreServiceTest.java index 1ada1e20d3..7fd28b25da 100644 --- a/dspace-api/src/test/java/org/dspace/storage/bitstore/JCloudBitStoreServiceTest.java +++ b/dspace-api/src/test/java/org/dspace/storage/bitstore/JCloudBitStoreServiceTest.java @@ -16,6 +16,9 @@ import static org.mockito.Mockito.when; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; import com.google.common.io.ByteSource; import org.dspace.AbstractUnitTest; @@ -143,7 +146,7 @@ public class JCloudBitStoreServiceTest extends AbstractUnitTest { String computedPath = this.jCloudBitStoreService.getIntermediatePath(path.toString()); int slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("2"); computedPath = this.jCloudBitStoreService.getIntermediatePath(path.toString()); @@ -168,31 +171,31 @@ public class JCloudBitStoreServiceTest extends AbstractUnitTest { String computedPath = this.jCloudBitStoreService.getIntermediatePath(path.toString()); int slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("2"); computedPath = this.jCloudBitStoreService.getIntermediatePath(path.toString()); slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("3"); computedPath = this.jCloudBitStoreService.getIntermediatePath(path.toString()); slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("4"); computedPath = this.jCloudBitStoreService.getIntermediatePath(path.toString()); slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("56789"); computedPath = this.jCloudBitStoreService.getIntermediatePath(path.toString()); slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); } @Test @@ -219,4 +222,12 @@ public class JCloudBitStoreServiceTest extends AbstractUnitTest { return Math.min(slashes, S3BitStoreService.directoryLevels); } + // Count the number of elements in a Unix or Windows path. + // We use 'Paths' instead of splitting on slashes because these OSes use different path separators. + private int countPathElements(String stringPath) { + List pathElements = new ArrayList<>(); + Paths.get(stringPath).forEach(p -> pathElements.add(p.toString())); + return pathElements.size(); + } + } From f6584d0e29ae00d993d3380c40b5a40a4e0bcbbe Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Wed, 16 Jul 2025 16:27:05 -0500 Subject: [PATCH 2/6] Update test to no longer assume 127.0.0.1 will always respond with "localhost" as the hostname. On my machine it does not. (cherry picked from commit 5768f9380e9104646e28f01ff1a178e1a5ff736a) --- .../client/DSpaceHttpClientFactoryTest.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/app/client/DSpaceHttpClientFactoryTest.java b/dspace-api/src/test/java/org/dspace/app/client/DSpaceHttpClientFactoryTest.java index b518f19ff4..ca91bb5dc9 100644 --- a/dspace-api/src/test/java/org/dspace/app/client/DSpaceHttpClientFactoryTest.java +++ b/dspace-api/src/test/java/org/dspace/app/client/DSpaceHttpClientFactoryTest.java @@ -17,6 +17,7 @@ import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import java.net.InetAddress; import java.util.List; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -107,7 +108,14 @@ public class DSpaceHttpClientFactoryTest { @Test public void testBuildWithProxyConfiguredAndHostPrefixToIgnoreSet() throws Exception { - setHttpProxyOnConfigurationService("local*", "www.test.com"); + // Get hostname assigned to 127.0.0.1 (usually is "localhost", but not always) + InetAddress address = InetAddress.getByAddress(new byte[]{127, 0, 0, 1}); + String hostname = address.getHostName(); + // Take first 4 characters hostname as the prefix (e.g. "loca" in "localhost") + String hostnamePrefix = hostname.substring(0, 4); + // Save hostname prefix to our list of hosts to ignore, followed by an asterisk. + // (This should result in our Proxy ignoring our localhost) + setHttpProxyOnConfigurationService(hostnamePrefix + "*", "www.test.com"); CloseableHttpClient httpClient = httpClientFactory.build(); assertThat(mockProxy.getRequestCount(), is(0)); assertThat(mockServer.getRequestCount(), is(0)); @@ -122,7 +130,14 @@ public class DSpaceHttpClientFactoryTest { @Test public void testBuildWithProxyConfiguredAndHostSuffixToIgnoreSet() throws Exception { - setHttpProxyOnConfigurationService("www.test.com", "*host"); + // Get hostname assigned to 127.0.0.1 (usually is "localhost", but not always) + InetAddress address = InetAddress.getByAddress(new byte[]{127, 0, 0, 1}); + String hostname = address.getHostName(); + // Take last 4 characters hostname as the suffix (e.g. "host" in "localhost") + String hostnameSuffix = hostname.substring(hostname.length() - 4); + // Save hostname suffix to our list of hosts to ignore, preceded by an asterisk. + // (This should result in our Proxy ignoring our localhost) + setHttpProxyOnConfigurationService("www.test.com", "*" + hostnameSuffix); CloseableHttpClient httpClient = httpClientFactory.build(); assertThat(mockProxy.getRequestCount(), is(0)); assertThat(mockServer.getRequestCount(), is(0)); From 84182e77141d3b690b135b4a084e45157e22198f Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Thu, 17 Jul 2025 15:50:40 -0500 Subject: [PATCH 3/6] Fix broken SAML2 test on Windows by building proper file URLs for Windows. (cherry picked from commit 4b8686f82893050bda380f67f745044a9c378bcf) --- .../DSpaceRelyingPartyRegistrationRepositoryTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dspace-saml2/src/test/java/org/dspace/saml2/DSpaceRelyingPartyRegistrationRepositoryTest.java b/dspace-saml2/src/test/java/org/dspace/saml2/DSpaceRelyingPartyRegistrationRepositoryTest.java index df40c6e7f6..236236a27d 100644 --- a/dspace-saml2/src/test/java/org/dspace/saml2/DSpaceRelyingPartyRegistrationRepositoryTest.java +++ b/dspace-saml2/src/test/java/org/dspace/saml2/DSpaceRelyingPartyRegistrationRepositoryTest.java @@ -7,6 +7,7 @@ */ package org.dspace.saml2; +import static org.apache.commons.lang3.SystemUtils.IS_OS_WINDOWS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -376,11 +377,14 @@ public class DSpaceRelyingPartyRegistrationRepositoryTest extends AbstractDSpace configurationService.setProperty( "saml-relying-party.auth0.asserting-party.metadata-uri", "classpath:auth0-ap-metadata.xml"); + // Windows requires three slashes in a file URL. Linux/unix does not. + String fileUrlPrefix = IS_OS_WINDOWS ? "file:///" : "file://"; + configurationService.setProperty("saml-relying-party.auth0.signing.credentials.0.private-key-location", - "file://" + new ClassPathResource("auth0-rp-private.key").getFile().getAbsolutePath()); + fileUrlPrefix + new ClassPathResource("auth0-rp-private.key").getFile().getAbsolutePath()); configurationService.setProperty("saml-relying-party.auth0.signing.credentials.0.certificate-location", - "file://" + new ClassPathResource("auth0-rp-certificate.crt").getFile().getAbsolutePath()); + fileUrlPrefix + new ClassPathResource("auth0-rp-certificate.crt").getFile().getAbsolutePath()); DSpaceRelyingPartyRegistrationRepository repo = new DSpaceRelyingPartyRegistrationRepository(); RelyingPartyRegistration registration = repo.findByRegistrationId("auth0"); From d9e647bd962827a2941d3975aaebbb90aecdcb30 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 18 Jul 2025 11:33:41 -0500 Subject: [PATCH 4/6] Fix broken tests on Windows by using Paths to split file path instead of regex. Also switch to in-memory s3mock because Windows cannot cleanup created files successfully. (cherry picked from commit cbe87832dc0916ecdba5b97155d58d029996aaaa) --- .../storage/bitstore/S3BitStoreServiceIT.java | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/storage/bitstore/S3BitStoreServiceIT.java b/dspace-api/src/test/java/org/dspace/storage/bitstore/S3BitStoreServiceIT.java index 6ea21eac8d..63d88e9501 100644 --- a/dspace-api/src/test/java/org/dspace/storage/bitstore/S3BitStoreServiceIT.java +++ b/dspace-api/src/test/java/org/dspace/storage/bitstore/S3BitStoreServiceIT.java @@ -26,9 +26,11 @@ import static org.junit.Assert.assertTrue; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Paths; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.sql.SQLException; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -41,7 +43,6 @@ import com.amazonaws.services.s3.model.AmazonS3Exception; import com.amazonaws.services.s3.model.Bucket; import com.amazonaws.services.s3.model.ObjectMetadata; import io.findify.s3mock.S3Mock; -import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.BooleanUtils; import org.dspace.AbstractIntegrationTestWithDatabase; @@ -80,8 +81,6 @@ public class S3BitStoreServiceIT extends AbstractIntegrationTestWithDatabase { private Collection collection; - private File s3Directory; - private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); @@ -89,9 +88,8 @@ public class S3BitStoreServiceIT extends AbstractIntegrationTestWithDatabase { public void setup() throws Exception { configurationService.setProperty("assetstore.s3.enabled", "true"); - s3Directory = new File(System.getProperty("java.io.tmpdir"), "s3"); - s3Mock = S3Mock.create(8001, s3Directory.getAbsolutePath()); + s3Mock = new S3Mock.Builder().withPort(8001).withInMemoryBackend().build(); s3Mock.start(); amazonS3Client = createAmazonS3Client(); @@ -112,8 +110,7 @@ public class S3BitStoreServiceIT extends AbstractIntegrationTestWithDatabase { } @After - public void cleanUp() throws IOException { - FileUtils.deleteDirectory(s3Directory); + public void cleanUp() { s3Mock.shutdown(); } @@ -337,7 +334,7 @@ public class S3BitStoreServiceIT extends AbstractIntegrationTestWithDatabase { String computedPath = this.s3BitStoreService.getIntermediatePath(path.toString()); int slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("2"); computedPath = this.s3BitStoreService.getIntermediatePath(path.toString()); @@ -362,31 +359,31 @@ public class S3BitStoreServiceIT extends AbstractIntegrationTestWithDatabase { String computedPath = this.s3BitStoreService.getIntermediatePath(path.toString()); int slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("2"); computedPath = this.s3BitStoreService.getIntermediatePath(path.toString()); slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("3"); computedPath = this.s3BitStoreService.getIntermediatePath(path.toString()); slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("4"); computedPath = this.s3BitStoreService.getIntermediatePath(path.toString()); slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); path.append("56789"); computedPath = this.s3BitStoreService.getIntermediatePath(path.toString()); slashes = computeSlashes(path.toString()); assertThat(computedPath, Matchers.endsWith(File.separator)); - assertThat(computedPath.split(File.separator).length, Matchers.equalTo(slashes)); + assertThat(countPathElements(computedPath), Matchers.equalTo(slashes)); } @Test @@ -465,4 +462,12 @@ public class S3BitStoreServiceIT extends AbstractIntegrationTestWithDatabase { return Math.min(slashes, S3BitStoreService.directoryLevels); } + // Count the number of elements in a Unix or Windows path. + // We use 'Paths' instead of splitting on slashes because these OSes use different path separators. + private int countPathElements(String stringPath) { + List pathElements = new ArrayList<>(); + Paths.get(stringPath).forEach(p -> pathElements.add(p.toString())); + return pathElements.size(); + } + } From 65257935ddcf0b3959381bf25eb4602cf3354bdd Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 21 Jul 2025 15:57:34 -0500 Subject: [PATCH 5/6] Improve logging in AbstractLiveImportIntegrationTest (to make tests easier to debug). Replace obscure StringInputStream (from Ant) with IOUtils.toInputStream (cherry picked from commit e9f32b9a1b830b895d1d298caee6f70ca2c27720) --- .../importer/external/metadatamapping/MetadatumDTO.java | 9 +++++++++ .../app/rest/AbstractLiveImportIntegrationTest.java | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/importer/external/metadatamapping/MetadatumDTO.java b/dspace-api/src/main/java/org/dspace/importer/external/metadatamapping/MetadatumDTO.java index 265dd55eb9..b8a7507eaa 100644 --- a/dspace-api/src/main/java/org/dspace/importer/external/metadatamapping/MetadatumDTO.java +++ b/dspace-api/src/main/java/org/dspace/importer/external/metadatamapping/MetadatumDTO.java @@ -105,4 +105,13 @@ public class MetadatumDTO { public void setValue(String value) { this.value = value; } + + /** + * Return string representation of MetadatumDTO + * @return string representation of format "[schema].[element].[qualifier]=[value]" + */ + @Override + public String toString() { + return schema + "." + element + "." + qualifier + "=" + value; + } } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AbstractLiveImportIntegrationTest.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AbstractLiveImportIntegrationTest.java index cf3e125cc5..94fbce4a51 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AbstractLiveImportIntegrationTest.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AbstractLiveImportIntegrationTest.java @@ -16,12 +16,12 @@ import java.io.UnsupportedEncodingException; import java.util.ArrayList; import java.util.List; +import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.apache.http.ProtocolVersion; import org.apache.http.StatusLine; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.entity.BasicHttpEntity; -import org.apache.tools.ant.filters.StringInputStream; import org.dspace.app.rest.test.AbstractControllerIntegrationTest; import org.dspace.importer.external.datamodel.ImportRecord; import org.dspace.importer.external.metadatamapping.MetadatumDTO; @@ -43,7 +43,8 @@ public class AbstractLiveImportIntegrationTest extends AbstractControllerIntegra private void checkMetadataValue(List list, List list2) { assertEquals(list.size(), list2.size()); for (int i = 0; i < list.size(); i++) { - assertTrue(sameMetadatum(list.get(i), list2.get(i))); + assertTrue("'" + list.get(i).toString() + "' should be equal to '" + list2.get(i).toString() + "'", + sameMetadatum(list.get(i), list2.get(i))); } } @@ -70,7 +71,7 @@ public class AbstractLiveImportIntegrationTest extends AbstractControllerIntegra throws UnsupportedEncodingException { BasicHttpEntity basicHttpEntity = new BasicHttpEntity(); basicHttpEntity.setChunked(true); - basicHttpEntity.setContent(new StringInputStream(xmlExample)); + basicHttpEntity.setContent(IOUtils.toInputStream(xmlExample)); CloseableHttpResponse response = mock(CloseableHttpResponse.class); when(response.getStatusLine()).thenReturn(statusLine(statusCode, reason)); From 4678f87e0bceeb8eaf56872317fcacc7904bf4db Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 21 Jul 2025 15:58:37 -0500 Subject: [PATCH 6/6] Force UTF-8 encoding in all tests. This fixes several test failures when running tests from Windows commandline because Windows doesn't default to using UTF-8. (cherry picked from commit d5457029adc166dba2fa8b6fd3e2798baf6e670f) --- pom.xml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pom.xml b/pom.xml index 8e03c9b806..fcd178a92b 100644 --- a/pom.xml +++ b/pom.xml @@ -531,10 +531,10 @@ - + test-argLine @@ -543,7 +543,7 @@ - -Xmx1024m + -Xmx1024m -Dfile.encoding=UTF-8