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