Merge pull request #541 from marklogic/feature/maybe-file-fix

MLE-23330 Trimming off scheme when writing URIs to files

Author: rjrudin

marklogic-spark-connector/src/main/java/com/marklogic/spark/writer/file/FileUtil.java

@@ -3,11 +3,31 @@
  */
 package com.marklogic.spark.writer.file;
 
+import com.marklogic.spark.Util;
 import org.apache.hadoop.fs.Path;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+
 public interface FileUtil {
 
     static Path makePathFromDocumentURI(String basePath, String documentURI) {
+        // Mostly copied from MLCP.
+        try {
+            URI uri = new URI(documentURI);
+            // As of 1.4.0, an opaque URI will not cause a problem due to the inclusion of "./" below.
+            // We still parse the URI to get the path in case there is a scheme (e.g. file://).
+            if (!uri.isOpaque()) {
+                documentURI = uri.getPath();
+            }
+        } catch (URISyntaxException e) {
+            // MLCP logs errors from parsing the URI at the "WARN" level. That seems noisy, as large numbers of URIs
+            // could e.g. have spaces in them. So DEBUG is used instead.
+            if (Util.MAIN_LOGGER.isDebugEnabled()) {
+                Util.MAIN_LOGGER.debug("Unable to parse document URI: {}; will use unparsed URI as file path.", documentURI);
+            }
+        }
+
         if (documentURI.charAt(0) == '/') {
             return new Path(basePath + documentURI);
         }

tests/src/test/java/com/marklogic/spark/writer/file/FileUtilTest.java

@@ -14,6 +14,16 @@ void makePathFromRegularURI() {
         assertEquals("/base/path/to/doc.xml", makePath("/path/to/doc.xml"));
     }
 
+    @Test
+    void noForwardSlash() {
+        assertEquals("/base/path/to/doc.xml", makePath("path/to/doc.xml"));
+    }
+
+    @Test
+    void justFilename() {
+        assertEquals("/base/doc.xml", makePath("doc.xml"));
+    }
+
     /**
      * This was altered in the 2.7.0 to fix a bug where a URI with two or more colons and
      * no leading slash causing a URISyntaxException when the Hadoop Path constructor was called. This
@@ -38,7 +48,20 @@ void uriWithSpace() {
     void allKindsOfStuff() {
         assertEquals("/base/has+lots of&/stuff_in-it.json", makePath("has+lots of&/stuff_in-it.json"));
     }
-    
+
+    @Test
+    void fileBasedUri() {
+        assertEquals("/base/hey.json", makePath("file://tmp/hey.json"), "Per the fixes for 1.4.0, " +
+            "we're retaining the behavior where the scheme is removed for a non-opaque URI. This ensures we don't " +
+            "try to files with e.g. 'http://' or 'file://' in the URI. This won't prevent issues like writing files " +
+            "to Azure Storage, which doesn't allow colons - a colon can appear anywhere in the URI and still be valid.");
+    }
+
+    @Test
+    void httpBasedUri() {
+        assertEquals("/base/path/to/file.xml", makePath("https://www.exampple.org/path/to/file.xml"));
+    }
+
     private String makePath(String uri) {
         return FileUtil.makePathFromDocumentURI("/base", uri).toString();
     }

tests/src/test/java/com/marklogic/spark/writer/file/WriteDocumentZipFilesTest.java

@@ -85,9 +85,36 @@ void opaqueURI(@TempDir Path tempDir) throws IOException {
         assertEquals(1, tempDir.toFile().listFiles().length);
         File file = tempDir.toFile().listFiles()[0];
         ZipFile zipFile = new ZipFile(file);
-        assertNotNull(zipFile.getEntry("some:org:example/123.xml"), "some:org:example/123.xml is considered an 'opaque' URI per " +
-            "the definition of java.net.URI:isOpaque. Per MLCP behavior, the URI is expected to be set to the " +
-            "'schema-specific part', which is just example/123.xml.");
+        assertNotNull(zipFile.getEntry(uri), "some:org:example/123.xml is considered an 'opaque' URI per " +
+            "the definition of java.net.URI:isOpaque. As of 1.4.0 now, we no longer modify those when the document " +
+            "is written to a zip, as zip entry names can be opaque URIs.");
+    }
+
+    @Test
+    void uriWithScheme(@TempDir Path tempDir) throws IOException {
+        final String uri = "http://example.org/some/file.xml";
+
+        getDatabaseClient().newXMLDocumentManager().write(uri,
+            TestUtil.withDefaultPermissions(new DocumentMetadataHandle()).withCollections("http-test"),
+            new StringHandle("<hello>world</hello>"));
+
+        newSparkSession().read()
+            .format(CONNECTOR_IDENTIFIER)
+            .option(Options.CLIENT_URI, makeClientUri())
+            .option(Options.READ_DOCUMENTS_COLLECTIONS, "http-test")
+            .load()
+            .repartition(1)
+            .write()
+            .format(CONNECTOR_IDENTIFIER)
+            .option(Options.WRITE_FILES_COMPRESSION, "zip")
+            .mode(SaveMode.Append)
+            .save(tempDir.toFile().getAbsolutePath());
+
+        assertEquals(1, tempDir.toFile().listFiles().length);
+        File file = tempDir.toFile().listFiles()[0];
+        ZipFile zipFile = new ZipFile(file);
+        assertNotNull(zipFile.getEntry(uri), "When writing URIs to a zip, we don't need to modify them at all, " +
+            "unless we later find out there's some set of characters that are not allowed in zip entry names.");
     }
 
     @Test
@@ -113,7 +140,7 @@ void uriWithSpaces(@TempDir Path tempDir) throws IOException {
         assertEquals(1, tempDir.toFile().listFiles().length);
         File file = tempDir.toFile().listFiles()[0];
         ZipFile zipFile = new ZipFile(file);
-        assertNotNull(zipFile.getEntry("has multiple spaces.xml"));
+        assertNotNull(zipFile.getEntry(uri));
     }
 
     /**