This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git


The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
     new 0666ddb3dd Better handling of directory on S3 storage.
0666ddb3dd is described below

commit 0666ddb3dd389cb8c2e97efc284b1bd9633bed9d
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Mon May 9 13:07:32 2022 +0200

    Better handling of directory on S3 storage.
---
 .../java/org/apache/sis/cloud/aws/s3/KeyPath.java  |  7 +++---
 .../apache/sis/cloud/aws/s3/ObjectAttributes.java  |  2 +-
 .../org/apache/sis/cloud/aws/s3/KeyPathTest.java   | 24 ++++++++++----------
 .../sis/internal/storage/io/IOUtilities.java       | 26 ++++++++++++++++------
 .../sis/internal/storage/io/IOUtilitiesTest.java   |  1 +
 5 files changed, 36 insertions(+), 24 deletions(-)

diff --git 
a/cloud/sis-cloud-S3/src/main/java/org/apache/sis/cloud/aws/s3/KeyPath.java 
b/cloud/sis-cloud-S3/src/main/java/org/apache/sis/cloud/aws/s3/KeyPath.java
index 907dad7407..f95010b0c1 100644
--- a/cloud/sis-cloud-S3/src/main/java/org/apache/sis/cloud/aws/s3/KeyPath.java
+++ b/cloud/sis-cloud-S3/src/main/java/org/apache/sis/cloud/aws/s3/KeyPath.java
@@ -716,7 +716,6 @@ search:     if (key != null) {
 
     /**
      * Returns a string representation of this path.
-     * By convention, a trailing {@link ClientFileSystem#separator} is 
appended to directories.
      */
     @Override
     public String toString() {
@@ -729,13 +728,13 @@ search:     if (key != null) {
             if (fs.accessKey != null) {
                 sb.append(fs.accessKey).append('@');
             }
-            sb.append(bucket).append(fs.separator);
+            sb.append(bucket);
         }
         if (key != null) {
-            sb.append(key);
-            if (isDirectory) {
+            if (bucket != null) {
                 sb.append(fs.separator);
             }
+            sb.append(key);
         }
         return sb.toString();
     }
diff --git 
a/cloud/sis-cloud-S3/src/main/java/org/apache/sis/cloud/aws/s3/ObjectAttributes.java
 
b/cloud/sis-cloud-S3/src/main/java/org/apache/sis/cloud/aws/s3/ObjectAttributes.java
index c8fee55a52..ee62fa7cea 100644
--- 
a/cloud/sis-cloud-S3/src/main/java/org/apache/sis/cloud/aws/s3/ObjectAttributes.java
+++ 
b/cloud/sis-cloud-S3/src/main/java/org/apache/sis/cloud/aws/s3/ObjectAttributes.java
@@ -200,7 +200,7 @@ final class ObjectAttributes implements 
BasicFileAttributeView {
          */
         @Override
         public boolean isRegularFile() {
-            return !isDirectory;
+            return !isDirectory && size != 0;
         }
 
         /**
diff --git 
a/cloud/sis-cloud-S3/src/test/java/org/apache/sis/cloud/aws/s3/KeyPathTest.java 
b/cloud/sis-cloud-S3/src/test/java/org/apache/sis/cloud/aws/s3/KeyPathTest.java
index 36356381a0..b78b2565d6 100644
--- 
a/cloud/sis-cloud-S3/src/test/java/org/apache/sis/cloud/aws/s3/KeyPathTest.java
+++ 
b/cloud/sis-cloud-S3/src/test/java/org/apache/sis/cloud/aws/s3/KeyPathTest.java
@@ -143,14 +143,14 @@ public final strictfp class KeyPathTest extends TestCase {
      */
     @Test
     public void testGetName() {
-        assertEquals("S3://the-bucket/", absolute.getName(0).toString());
-        assertEquals("first/",           absolute.getName(1).toString());
-        assertEquals("second/",          absolute.getName(2).toString());
-        assertEquals("third/",           absolute.getName(3).toString());
-        assertEquals("the-file",         absolute.getName(4).toString());
-        assertEquals("second/",          relative.getName(0).toString());
-        assertEquals("third/",           relative.getName(1).toString());
-        assertEquals("the-file",         relative.getName(2).toString());
+        assertEquals("S3://the-bucket", absolute.getName(0).toString());
+        assertEquals("first",           absolute.getName(1).toString());
+        assertEquals("second",          absolute.getName(2).toString());
+        assertEquals("third",           absolute.getName(3).toString());
+        assertEquals("the-file",        absolute.getName(4).toString());
+        assertEquals("second",          relative.getName(0).toString());
+        assertEquals("third",           relative.getName(1).toString());
+        assertEquals("the-file",        relative.getName(2).toString());
         try {
             absolute.getName(5);
             fail("Expected an exception.");
@@ -164,10 +164,10 @@ public final strictfp class KeyPathTest extends TestCase {
      */
     @Test
     public void testSubpath() {
-        assertEquals("S3://the-bucket/first/second/", absolute.subpath(0, 
3).toString());
-        assertEquals("second/third/the-file",         absolute.subpath(2, 
5).toString());
-        assertEquals("second/third/",                 absolute.subpath(2, 
4).toString());
-        assertEquals("third/the-file",                relative.subpath(1, 
3).toString());
+        assertEquals("S3://the-bucket/first/second", absolute.subpath(0, 
3).toString());
+        assertEquals("second/third/the-file",        absolute.subpath(2, 
5).toString());
+        assertEquals("second/third",                 absolute.subpath(2, 
4).toString());
+        assertEquals("third/the-file",               relative.subpath(1, 
3).toString());
         assertSame(absolute, absolute.subpath(0, 5));
         assertSame(relative, relative.subpath(0, 3));
     }
diff --git 
a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/IOUtilities.java
 
b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/IOUtilities.java
index cc7b2a8b55..ce0ae12174 100644
--- 
a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/IOUtilities.java
+++ 
b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/IOUtilities.java
@@ -122,10 +122,13 @@ public final class IOUtilities extends Static {
     private static String part(final Object path, final boolean extension) {
         int fromIndex = 0;
         final String name;
+        int end;
         if (path instanceof File) {
             name = ((File) path).getName();
+            end  = name.length();
         } else if (path instanceof Path) {
             name = ((Path) path).getFileName().toString();
+            end  = name.length();
         } else {
             char separator = '/';
             if (path instanceof URL) {
@@ -139,20 +142,29 @@ public final class IOUtilities extends Static {
             } else {
                 return null;
             }
-            fromIndex = name.lastIndexOf('/') + 1;
-            if (separator != '/') {
-                // Search for platform-specific character only if the object 
is neither a URL or a URI.
-                fromIndex = Math.max(fromIndex, 
CharSequences.lastIndexOf(name, separator, fromIndex, name.length()) + 1);
-            }
+            /*
+             * Search for the last '/' separator character (looking also for 
'\' on Windows).
+             * If the separator is the very last character of the name, search 
for the previous one.
+             * The intent is to ignore the trailing separator in "foo/".
+             */
+            end = name.length();
+            do {
+                fromIndex = name.lastIndexOf('/', --end) + 1;
+                if (separator != '/') {
+                    // Search for platform-specific character only if the 
object is neither a URL or a URI.
+                    fromIndex = Math.max(fromIndex, 
CharSequences.lastIndexOf(name, separator, fromIndex, end+1) + 1);
+                }
+            } while (fromIndex > end);
+            end++;
         }
         if (extension) {
-            fromIndex = CharSequences.lastIndexOf(name, '.', fromIndex, 
name.length()) + 1;
+            fromIndex = CharSequences.lastIndexOf(name, '.', fromIndex, end) + 
1;
             if (fromIndex <= 1) {
                 // If the dot is the first character, do not consider as a 
filename extension.
                 return "";
             }
         }
-        return name.substring(fromIndex);
+        return name.substring(fromIndex, end);
     }
 
     /**
diff --git 
a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/IOUtilitiesTest.java
 
b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/IOUtilitiesTest.java
index 09b0d56f10..073da5abeb 100644
--- 
a/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/IOUtilitiesTest.java
+++ 
b/storage/sis-storage/src/test/java/org/apache/sis/internal/storage/io/IOUtilitiesTest.java
@@ -52,6 +52,7 @@ public final strictfp class IOUtilitiesTest extends TestCase {
         assertEquals("Map.png", IOUtilities.filename(new File(     
"/Users/name/Map.png")));
         assertEquals("Map.png", IOUtilities.filename(new URI 
("file:/Users/name/Map.png")));
         assertEquals("Map.png", IOUtilities.filename(new URL 
("file:/Users/name/Map.png")));
+        assertEquals("name",    IOUtilities.filename(new URI 
("file:/Users/name/")));
         assertNull(IOUtilities.filename(Boolean.FALSE));
         assertNull(IOUtilities.filename(null));
     }

Reply via email to