Skip to content

Commit

Permalink
fix for awslabs#293: as per https://docs.oracle.com/javase/8/docs/api…
Browse files Browse the repository at this point in the history 
…/java/nio/file/Path.html#subpath-int-int- subpath shall return a relative path
  • Loading branch information
@stefanofornari
stefanofornari committed Mar 3, 2024
1 parent a197a93 commit 8dec680
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 27 deletions.
7 changes: 2 additions & 5 deletions src/main/java/software/amazon/nio/spi/s3/S3Path.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.InvalidPathException;
import java.nio.file.LinkOption;
import java.nio.file.Path;
Expand Down Expand Up @@ -242,7 +243,7 @@ public S3Path getName(int index) {
*/
@Override
public S3Path subpath(int beginIndex, int endIndex) {
final var size = pathRepresentation.elements().size();
FileSystems.getDefault().getRootDirectories().iterator().next();final var size = pathRepresentation.elements().size();
if (beginIndex < 0) {
throw new IllegalArgumentException("begin index may not be < 0");
}
Expand All @@ -257,10 +258,6 @@ public S3Path subpath(int beginIndex, int endIndex) {
}

var path = String.join(PATH_SEPARATOR, pathRepresentation.elements().subList(beginIndex, endIndex));
if (this.isAbsolute() && beginIndex == 0) {
path = PATH_SEPARATOR + path;
}

if (endIndex == size && !pathRepresentation.hasTrailingSeparator()) {
return from(path);
} else {
Expand Down
46 changes: 24 additions & 22 deletions src/test/java/software/amazon/nio/spi/s3/S3PathTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.nio.spi.s3;


import java.net.URI;
import java.nio.file.LinkOption;
import java.nio.file.Paths;
Expand All @@ -30,6 +28,7 @@

@ExtendWith(MockitoExtension.class)
public class S3PathTest {

final String uriString = "s3://mybucket";
final S3FileSystemProvider provider = new S3FileSystemProvider();

Expand All @@ -45,7 +44,7 @@ public class S3PathTest {
S3Path withSpecialChars;

@BeforeEach
public void init(){
public void init() {
fileSystem = provider.getFileSystem(URI.create(uriString), true);
fileSystem.clientProvider(new FixedS3ClientProvider(mockClient));
lenient().when(mockClient.headObject(anyConsumer())).thenReturn(
Expand Down Expand Up @@ -81,12 +80,12 @@ public void getPathEmptyFirst() {

@Test
public void getPathWithScheme() {
assertEquals(fileSystem.getPath("/", "foo"), Paths.get(URI.create(uriString+"/foo")));
assertEquals(fileSystem.getPath("/", "foo"), Paths.get(URI.create(uriString + "/foo")));
}

@Test
public void getFileSystem() {
assertEquals(fileSystem, root.getFileSystem());
assertEquals(fileSystem, root.getFileSystem());
}

@Test
Expand Down Expand Up @@ -139,7 +138,6 @@ public void getFileName() {
public void getParent() {
assertNull(root.getParent());


var dir1 = S3Path.getPath(fileSystem, "dir1/");
assertEquals(dir1, absoluteDirectory.getParent());

Expand Down Expand Up @@ -185,13 +183,13 @@ public void subpath() {
var up = S3Path.getPath(fileSystem, "..");
var obj = S3Path.getPath(fileSystem, "object");

assertEquals(rootDir1, absoluteDirectory.subpath(0,1));
assertEquals(rootDir1, absoluteObject.subpath(0,1));
assertEquals(dir1, relativeObject.subpath(0,1));
assertEquals(up, relativeDirectory.subpath(0,1));
assertEquals(dir2, absoluteDirectory.subpath(1,2));
assertEquals(rootDir1, absoluteDirectory.subpath(0, 1));
assertEquals(rootDir1, absoluteObject.subpath(0, 1));
assertEquals(dir1, relativeObject.subpath(0, 1));
assertEquals(up, relativeDirectory.subpath(0, 1));
assertEquals(dir2, absoluteDirectory.subpath(1, 2));

assertEquals(obj, absoluteObject.subpath(2,3));
assertEquals(obj, absoluteObject.subpath(2, 3));
}

@Test
Expand Down Expand Up @@ -286,20 +284,20 @@ public void resolve() {
final var empty = fileSystem.getPath("");
assertEquals(root, root.resolve(empty));

assertEquals(fileSystem.getPath(absoluteDirectory.toString(), "foo") , absoluteDirectory.resolve(fileSystem.getPath("foo")));
assertEquals(fileSystem.getPath(absoluteDirectory.toString(), "foo/") , absoluteDirectory.resolve(fileSystem.getPath("foo/")));
assertEquals(fileSystem.getPath(absoluteObject.toString(), "foo") , absoluteObject.resolve(fileSystem.getPath("foo")));
assertEquals(fileSystem.getPath(absoluteObject.toString(), "foo/") , absoluteObject.resolve(fileSystem.getPath("foo/")));
assertEquals(fileSystem.getPath(absoluteDirectory.toString(), "foo"), absoluteDirectory.resolve(fileSystem.getPath("foo")));
assertEquals(fileSystem.getPath(absoluteDirectory.toString(), "foo/"), absoluteDirectory.resolve(fileSystem.getPath("foo/")));
assertEquals(fileSystem.getPath(absoluteObject.toString(), "foo"), absoluteObject.resolve(fileSystem.getPath("foo")));
assertEquals(fileSystem.getPath(absoluteObject.toString(), "foo/"), absoluteObject.resolve(fileSystem.getPath("foo/")));
}

@Test
public void testResolveString() {
assertEquals(root, root.resolve(""));

assertEquals(fileSystem.getPath(absoluteDirectory.toString(), "foo") , absoluteDirectory.resolve("foo"));
assertEquals(fileSystem.getPath(absoluteDirectory.toString(), "foo/") , absoluteDirectory.resolve("foo/"));
assertEquals(fileSystem.getPath(absoluteObject.toString(), "foo") , absoluteObject.resolve("foo"));
assertEquals(fileSystem.getPath(absoluteObject.toString(), "foo/") , absoluteObject.resolve("foo/"));
assertEquals(fileSystem.getPath(absoluteDirectory.toString(), "foo"), absoluteDirectory.resolve("foo"));
assertEquals(fileSystem.getPath(absoluteDirectory.toString(), "foo/"), absoluteDirectory.resolve("foo/"));
assertEquals(fileSystem.getPath(absoluteObject.toString(), "foo"), absoluteObject.resolve("foo"));
assertEquals(fileSystem.getPath(absoluteObject.toString(), "foo/"), absoluteObject.resolve("foo/"));
}

@Test
Expand Down Expand Up @@ -336,6 +334,8 @@ public void relativize() {
var bcd = fileSystem.getPath("b/c/d/");
var bcdObject = fileSystem.getPath("b/c/d/object");

assertFalse(root.relativize(ab).isAbsolute());

assertEquals(fileSystem.getPath(""), absoluteObject.relativize(absoluteObject));
assertEquals(fileSystem.getPath("../.."), abcd.relativize(ab));
assertEquals(fileSystem.getPath("e/"), abcd.relativize(abcde));
Expand Down Expand Up @@ -462,11 +462,13 @@ public void testEquals() {
assertEquals(S3Path.getPath(fileSystem, "dir1/"), S3Path.getPath(fileSystem, "/dir1/"));

S3FileSystem fooFS = null;
try{
try {
fooFS = provider.getFileSystem(URI.create("s3://foo"), true);
assertNotEquals(S3Path.getPath(fileSystem, "dir1/"), S3Path.getPath(fooFS, "/dir1/"));
} finally {
if ( fooFS != null ) provider.closeFileSystem(fooFS);
if (fooFS != null) {
provider.closeFileSystem(fooFS);
}
}

// not equal because in s3 dir1 cannot be implied to be a directory unless it ends with "/"
Expand Down

0 comments on commit 8dec680

Please sign in to comment.