Merge pull request #11064 from DSpace/backport-11063-to-dspace-9_x

[Port dspace-9_x] Fix several unit/integration test failures that only occur on Windows

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;
+    }
 }

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));

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<String> pathElements = new ArrayList<>();
+        Paths.get(stringPath).forEach(p -> pathElements.add(p.toString()));
+        return pathElements.size();
+    }
+
 }

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 {
+    public void cleanUp() {
-        FileUtils.deleteDirectory(s3Directory);
         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<String> pathElements = new ArrayList<>();
+        Paths.get(stringPath).forEach(p -> pathElements.add(p.toString()));
+        return pathElements.size();
+    }
+
 }

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");

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<MetadatumDTO> list, List<MetadatumDTO> 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));

pom.xml

@@ -531,10 +531,10 @@
     </build>
 
     <profiles>
-        <!-- Allow for passing extra memory to Unit/Integration tests.
+        <!-- Profile which sets our default system properties for all Unit/Integration tests.
-             By default this gives unit tests 1GB of memory max (when tests are enabled),
+             By default, this sets UTF-8 encoding for all tests and gives tests 1GB of memory max.
-             unless tweaked on commandline (e.g. "-Dtest.argLine=-Xmx512m"). Since
+             Both m-surefire-p and m-failsafe-p are unable to use MAVEN_OPTS because they fork a new JVM for testing.
-             m-surefire-p and m-failsafe-p both fork a new JVM for testing, they ignores MAVEN_OPTS. -->
+             These default settings may be overridden via the commandline (e.g. "-Dtest.argLine=-Xmx512m"). -->
         <profile>
             <id>test-argLine</id>
             <activation>
@@ -543,7 +543,7 @@
                 </property>
             </activation>
             <properties>
-                <test.argLine>-Xmx1024m</test.argLine>
+                <test.argLine>-Xmx1024m -Dfile.encoding=UTF-8</test.argLine>
             </properties>
         </profile>