Bugfix in URI conversion when working with directories

Base/src/main/java/io/deephaven/base/FileUtils.java

@@ -82,7 +82,7 @@ public static void deleteRecursively(File file) {
     /**
      * Move files accepted by a filter from their relative path under source to the same relative path under
      * destination. Creates missing destination subdirectories as needed.
-     * 
+     *
      * @param source Must be a directory.
      * @param destination Must be a directory if it exists.
      * @param filter Applied to normal files, only. We recurse on directories automatically.
@@ -129,7 +129,7 @@ private static void moveRecursivelyInternal(File source, File destination, FileF
 
     /**
      * Recursive delete method that copes with .nfs files. Uses the file's parent as the trash directory.
-     * 
+     *
      * @param file
      */
     public static void deleteRecursivelyOnNFS(File file) {
@@ -138,7 +138,7 @@ public static void deleteRecursivelyOnNFS(File file) {
 
     /**
      * Recursive delete method that copes with .nfs files.
-     * 
+     *
      * @param trashFile Filename to move regular files to before deletion. .nfs files may be created in its parent
      *        directory.
      * @param fileToBeDeleted File or directory at which to begin recursive deletion.
@@ -169,7 +169,7 @@ public static void deleteRecursivelyOnNFS(final File trashFile, final File fileT
 
     /**
      * Scan directory recursively to find all files
-     * 
+     *
      * @param dir
      * @return
      */
@@ -282,21 +282,33 @@ public static URI convertToURI(final String source, final boolean isDirectory) {
         URI uri;
         try {
             uri = new URI(source);
+            if (uri.getScheme() == null) {
+                // Convert to a "file" URI
+                return convertToURI(new File(source), isDirectory);
+            }
+            String path = uri.getPath();
+            final boolean endsWithSlash = path.charAt(path.length() - 1) == URI_SEPARATOR_CHAR;
+            if (!isDirectory && endsWithSlash) {
+                throw new IllegalArgumentException("Non-directory URI should not end with a slash: " + uri);
+            }
+            boolean isUpdated = false;
+            if (isDirectory && !endsWithSlash) {
+                path = path + URI_SEPARATOR_CHAR;
+                isUpdated = true;
+            }
             // Replace two or more consecutive slashes in the path with a single slash
-            final String path = uri.getPath();
             if (path.contains(REPEATED_URI_SEPARATOR)) {
-                final String canonicalizedPath = REPEATED_URI_SEPARATOR_PATTERN.matcher(path).replaceAll(URI_SEPARATOR);
-                uri = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(), canonicalizedPath,
-                        uri.getQuery(), uri.getFragment());
+                path = REPEATED_URI_SEPARATOR_PATTERN.matcher(path).replaceAll(URI_SEPARATOR);
+                isUpdated = true;
+            }
+            if (isUpdated) {
+                uri = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(), path, uri.getQuery(),
+                        uri.getFragment());
             }
         } catch (final URISyntaxException e) {
             // If the URI is invalid, assume it's a file path
             return convertToURI(new File(source), isDirectory);
         }
-        if (uri.getScheme() == null) {
-            // Convert to a "file" URI
-            return convertToURI(new File(source), isDirectory);
-        }
         return uri;
     }
 
@@ -314,15 +326,9 @@ public static URI convertToURI(final File file, final boolean isDirectory) {
         if (File.separatorChar != URI_SEPARATOR_CHAR) {
             absPath = absPath.replace(File.separatorChar, URI_SEPARATOR_CHAR);
         }
-        if (absPath.charAt(0) != URI_SEPARATOR_CHAR) {
-            absPath = URI_SEPARATOR_CHAR + absPath;
-        }
         if (isDirectory && absPath.charAt(absPath.length() - 1) != URI_SEPARATOR_CHAR) {
             absPath = absPath + URI_SEPARATOR_CHAR;
         }
-        if (absPath.startsWith(REPEATED_URI_SEPARATOR)) {
-            absPath = REPEATED_URI_SEPARATOR + absPath;
-        }
         try {
             return new URI("file", null, absPath, null);
         } catch (final URISyntaxException e) {

Base/src/test/java/io/deephaven/base/FileUtilsTest.java

@@ -0,0 +1,65 @@
+//
+// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
+//
+package io.deephaven.base;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.nio.file.Path;
+
+import junit.framework.TestCase;
+import org.junit.Assert;
+
+public class FileUtilsTest extends TestCase {
+
+    public void testConvertToFileURI() throws IOException {
+        final File currentDir = new File("").getAbsoluteFile();
+        fileUriTestHelper(currentDir.toString(), true, currentDir.toURI().toString());
+
+        final File someFile = new File(currentDir, "tempFile");
+        fileUriTestHelper(someFile.getPath(), false, someFile.toURI().toString());
+
+        // Check if trailing slash gets added for a directory
+        final String expectedURI = "file:" + currentDir.getPath() + "/path/to/directory/";
+        fileUriTestHelper(currentDir.getPath() + "/path/to/directory", true, expectedURI);
+
+        // Check if multiple slashes get normalized
+        fileUriTestHelper(currentDir.getPath() + "////path///to////directory////", true, expectedURI);
+
+        // Check if multiple slashes in the beginning get normalized
+        fileUriTestHelper("////" + currentDir.getPath() + "/path/to/directory", true, expectedURI);
+
+        // Check for bad inputs
+        fileUriTestHelper(someFile.getPath() + "/", false, someFile.toURI().toString());
+    }
+
+    private static void fileUriTestHelper(final String filePath, final boolean isDirectory,
+            final String expectedURIString) {
+        Assert.assertEquals(expectedURIString, FileUtils.convertToURI(filePath, isDirectory).toString());
+        Assert.assertEquals(expectedURIString, FileUtils.convertToURI(new File(filePath), isDirectory).toString());
+        Assert.assertEquals(expectedURIString, FileUtils.convertToURI(Path.of(filePath), isDirectory).toString());
+    }
+
+    public void testConvertToS3URI() throws URISyntaxException {
+        Assert.assertEquals("s3://bucket/key", FileUtils.convertToURI("s3://bucket/key", false).toString());
+
+        // Check if trailing slash gets added for a directory
+        Assert.assertEquals("s3://bucket/key/".toString(), FileUtils.convertToURI("s3://bucket/key", true).toString());
+
+        // Check if multiple slashes get normalized
+        Assert.assertEquals("s3://bucket/key/", FileUtils.convertToURI("s3://bucket///key///", true).toString());
+
+        try {
+            FileUtils.convertToURI("", false);
+            Assert.fail("Expected IllegalArgumentException");
+        } catch (IllegalArgumentException expected) {
+        }
+
+        try {
+            FileUtils.convertToURI("s3://bucket/key/", false);
+            Assert.fail("Expected IllegalArgumentException");
+        } catch (IllegalArgumentException expected) {
+        }
+    }
+}

extensions/parquet/base/src/main/java/io/deephaven/parquet/base/ParquetFileWriter.java

@@ -18,7 +18,6 @@
 import org.apache.parquet.schema.MessageType;
 import org.jetbrains.annotations.NotNull;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;