This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new d3680544f3 minor s3 issues in file/folder names, fixes #8841 (#8843)
d3680544f3 is described below
commit d3680544f3433e496b86accb306f37a13b259468
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Sat Oct 10 16:31:15 2026 +0200
minor s3 issues in file/folder names, fixes #8841 (#8843)
---
.../apache/hop/vfs/s3/s3/vfs/S3FileProvider.java | 151 ++++++-----
.../hop/vfs/s3/s3common/S3CommonFileObject.java | 48 +++-
.../vfs/s3/vfs/S3FileObjectStoreSemanticsTest.java | 286 +++++++++++++++++++++
.../apache/hop/vfs/s3/vfs/S3FileObjectTest.java | 8 +-
.../s3/vfs/S3FileProviderFileSystemReuseTest.java | 114 ++++++++
5 files changed, 532 insertions(+), 75 deletions(-)
diff --git
a/plugins/tech/aws/src/main/java/org/apache/hop/vfs/s3/s3/vfs/S3FileProvider.java
b/plugins/tech/aws/src/main/java/org/apache/hop/vfs/s3/s3/vfs/S3FileProvider.java
index 9f8a324fca..676f5cb236 100644
---
a/plugins/tech/aws/src/main/java/org/apache/hop/vfs/s3/s3/vfs/S3FileProvider.java
+++
b/plugins/tech/aws/src/main/java/org/apache/hop/vfs/s3/s3/vfs/S3FileProvider.java
@@ -20,6 +20,7 @@ package org.apache.hop.vfs.s3.s3.vfs;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.vfs2.FileName;
import org.apache.commons.vfs2.FileSystem;
+import org.apache.commons.vfs2.FileSystemException;
import org.apache.commons.vfs2.FileSystemOptions;
import org.apache.hop.core.variables.IVariables;
import org.apache.hop.vfs.s3.metadata.S3AuthType;
@@ -38,6 +39,9 @@ public class S3FileProvider extends S3CommonFileProvider {
private final IVariables variables;
private final S3Meta s3Meta;
+ /** A named connection's options, built once; see {@link
#options(FileSystemOptions)}. */
+ private FileSystemOptions connectionOptions;
+
public S3FileProvider() {
super();
this.variables = null;
@@ -53,80 +57,101 @@ public class S3FileProvider extends S3CommonFileProvider {
setFileNameParser(S3FileNameParser.getInstance());
}
+ /**
+ * Look file systems up by the options they are created with. Commons VFS
caches a file system
+ * under its own options but looks it up under the caller's, which are empty
when resolving
+ * through HopVfs; a named connection's options never matched, so every
resolve built a new file
+ * system and S3 client, and the provider kept all of them.
+ */
+ @Override
+ protected synchronized FileSystem getFileSystem(
+ final FileName rootFileName, final FileSystemOptions fileSystemOptions)
+ throws FileSystemException {
+ return super.getFileSystem(rootFileName, options(fileSystemOptions));
+ }
+
@Override
public FileSystem doCreateFileSystem(
final FileName name, final FileSystemOptions fileSystemOptions) {
- // Named connections (s3test:// etc.) must use a dedicated options
instance so we never
- // mutate the default options used by s3://. Otherwise using s3test://
would overwrite
- // the default S3 configuration.
- final FileSystemOptions options;
- if (s3Meta != null && variables != null) {
- options = new FileSystemOptions();
- } else {
- options = fileSystemOptions != null ? fileSystemOptions :
getDefaultFileSystemOptions();
+ return new S3FileSystem(name, options(fileSystemOptions));
+ }
+
+ /**
+ * The options of the file system for the given request. For s3:// the
caller's options, or the
+ * defaults. For a named connection one options instance, built from the
connection the first time
+ * it is needed: dedicated, so it never changes the defaults s3:// uses, and
the same every time,
+ * so its file system is found again.
+ */
+ private synchronized FileSystemOptions options(final FileSystemOptions
fileSystemOptions) {
+ if (s3Meta == null || variables == null) {
+ return fileSystemOptions != null ? fileSystemOptions :
getDefaultFileSystemOptions();
}
+ if (connectionOptions == null) {
+ connectionOptions = buildConnectionOptions();
+ }
+ return connectionOptions;
+ }
- if (s3Meta != null && variables != null) {
- S3CommonFileSystemConfigBuilder config = new
S3CommonFileSystemConfigBuilder(options);
- String authType = s3Meta.getAuthenticationType();
- if (StringUtils.isEmpty(authType)) {
- if (StringUtils.isNotEmpty(s3Meta.getAccessKey())
- || StringUtils.isNotEmpty(s3Meta.getSecretKey())) {
- authType = S3AuthType.ACCESS_KEYS.name();
- } else if (StringUtils.isNotEmpty(s3Meta.getCredentialsFile())) {
- authType = S3AuthType.CREDENTIALS_FILE.name();
- } else if
(S3AuthType.ANONYMOUS.name().equals(s3Meta.getAuthenticationType())) {
- authType = S3AuthType.ANONYMOUS.name();
- } else {
- authType = S3AuthType.DEFAULT.name();
- }
+ private FileSystemOptions buildConnectionOptions() {
+ final FileSystemOptions options = new FileSystemOptions();
+ S3CommonFileSystemConfigBuilder config = new
S3CommonFileSystemConfigBuilder(options);
+ String authType = s3Meta.getAuthenticationType();
+ if (StringUtils.isEmpty(authType)) {
+ if (StringUtils.isNotEmpty(s3Meta.getAccessKey())
+ || StringUtils.isNotEmpty(s3Meta.getSecretKey())) {
+ authType = S3AuthType.ACCESS_KEYS.name();
+ } else if (StringUtils.isNotEmpty(s3Meta.getCredentialsFile())) {
+ authType = S3AuthType.CREDENTIALS_FILE.name();
+ } else if
(S3AuthType.ANONYMOUS.name().equals(s3Meta.getAuthenticationType())) {
+ authType = S3AuthType.ANONYMOUS.name();
+ } else {
+ authType = S3AuthType.DEFAULT.name();
}
- String region = variables.resolve(s3Meta.getRegion());
- String endpoint = variables.resolve(s3Meta.getEndpoint());
+ }
+ String region = variables.resolve(s3Meta.getRegion());
+ String endpoint = variables.resolve(s3Meta.getEndpoint());
- if (StringUtils.isNotEmpty(region)) {
- config.setRegion(region);
+ if (StringUtils.isNotEmpty(region)) {
+ config.setRegion(region);
+ }
+ if (StringUtils.isNotEmpty(endpoint)) {
+ config.setEndpoint(endpoint);
+ }
+ if (S3AuthType.ACCESS_KEYS.name().equals(authType)) {
+ String accessKey = variables.resolve(s3Meta.getAccessKey());
+ String secretKey = variables.resolve(s3Meta.getSecretKey());
+ String sessionToken = variables.resolve(s3Meta.getSessionToken());
+ if (StringUtils.isNotEmpty(accessKey)) {
+ config.setAccessKey(accessKey);
+ }
+ if (StringUtils.isNotEmpty(secretKey)) {
+ config.setSecretKey(secretKey);
}
- if (StringUtils.isNotEmpty(endpoint)) {
- config.setEndpoint(endpoint);
+ if (StringUtils.isNotEmpty(sessionToken)) {
+ config.setSessionToken(sessionToken);
}
- if (S3AuthType.ACCESS_KEYS.name().equals(authType)) {
- String accessKey = variables.resolve(s3Meta.getAccessKey());
- String secretKey = variables.resolve(s3Meta.getSecretKey());
- String sessionToken = variables.resolve(s3Meta.getSessionToken());
- if (StringUtils.isNotEmpty(accessKey)) {
- config.setAccessKey(accessKey);
- }
- if (StringUtils.isNotEmpty(secretKey)) {
- config.setSecretKey(secretKey);
- }
- if (StringUtils.isNotEmpty(sessionToken)) {
- config.setSessionToken(sessionToken);
- }
- } else if (S3AuthType.CREDENTIALS_FILE.name().equals(authType)) {
- String credentialsFile =
variables.resolve(s3Meta.getCredentialsFile());
- String profileName = variables.resolve(s3Meta.getProfileName());
- if (StringUtils.isNotEmpty(credentialsFile)) {
- config.setCredentialsFile(credentialsFile);
- }
- if (StringUtils.isNotEmpty(profileName)) {
- config.setProfileName(profileName);
- }
- } else if (S3AuthType.DEFAULT.name().equals(authType)) {
- String profileName = variables.resolve(s3Meta.getProfileName());
- if (StringUtils.isNotEmpty(profileName)) {
- config.setProfileName(profileName);
- }
- } else if (S3AuthType.ANONYMOUS.name().equals(authType)) {
- config.setUseAnonymousAccess(true);
+ } else if (S3AuthType.CREDENTIALS_FILE.name().equals(authType)) {
+ String credentialsFile = variables.resolve(s3Meta.getCredentialsFile());
+ String profileName = variables.resolve(s3Meta.getProfileName());
+ if (StringUtils.isNotEmpty(credentialsFile)) {
+ config.setCredentialsFile(credentialsFile);
}
- config.setPathStyleAccess(String.valueOf(s3Meta.isPathStyleAccess()));
- String cacheTtl = variables.resolve(s3Meta.getCacheTtlSeconds());
- if (StringUtils.isNotEmpty(cacheTtl)) {
- config.setCacheTtlSeconds(cacheTtl);
+ if (StringUtils.isNotEmpty(profileName)) {
+ config.setProfileName(profileName);
}
+ } else if (S3AuthType.DEFAULT.name().equals(authType)) {
+ String profileName = variables.resolve(s3Meta.getProfileName());
+ if (StringUtils.isNotEmpty(profileName)) {
+ config.setProfileName(profileName);
+ }
+ } else if (S3AuthType.ANONYMOUS.name().equals(authType)) {
+ config.setUseAnonymousAccess(true);
}
-
- return new S3FileSystem(name, options);
+ config.setPathStyleAccess(String.valueOf(s3Meta.isPathStyleAccess()));
+ String cacheTtl = variables.resolve(s3Meta.getCacheTtlSeconds());
+ if (StringUtils.isNotEmpty(cacheTtl)) {
+ config.setCacheTtlSeconds(cacheTtl);
+ }
+ return options;
}
}
diff --git
a/plugins/tech/aws/src/main/java/org/apache/hop/vfs/s3/s3common/S3CommonFileObject.java
b/plugins/tech/aws/src/main/java/org/apache/hop/vfs/s3/s3common/S3CommonFileObject.java
index 3af4107a54..ea32080aa2 100644
---
a/plugins/tech/aws/src/main/java/org/apache/hop/vfs/s3/s3common/S3CommonFileObject.java
+++
b/plugins/tech/aws/src/main/java/org/apache/hop/vfs/s3/s3common/S3CommonFileObject.java
@@ -31,6 +31,7 @@ import org.apache.commons.vfs2.FileSystemException;
import org.apache.commons.vfs2.FileType;
import org.apache.commons.vfs2.provider.AbstractFileName;
import org.apache.commons.vfs2.provider.AbstractFileObject;
+import org.apache.commons.vfs2.provider.UriParser;
import org.apache.hop.core.logging.LogChannel;
import software.amazon.awssdk.core.ResponseInputStream;
import software.amazon.awssdk.core.sync.RequestBody;
@@ -75,8 +76,20 @@ public abstract class S3CommonFileObject extends
AbstractFileObject {
@Override
protected InputStream doGetInputStream() throws Exception {
LogChannel.GENERAL.logDebug("Accessing content {0}", getQualifiedName());
+ // A folder or a missing object has no content: say so, not "could not
read"
+ if (!getType().hasContent()) {
+ throw new org.apache.commons.vfs2.FileNotFoundException(getName());
+ }
closeS3Object();
- ResponseInputStream<GetObjectResponse> stream =
getObjectStream(bucketName, key);
+ ResponseInputStream<GetObjectResponse> stream;
+ try {
+ stream = getObjectStream(bucketName, key);
+ } catch (S3Exception e) {
+ if (isNotFound(e)) {
+ throw new org.apache.commons.vfs2.FileNotFoundException(getName(), e);
+ }
+ throw e;
+ }
return new S3CommonFileInputStream(stream, stream);
}
@@ -87,15 +100,27 @@ public abstract class S3CommonFileObject extends
AbstractFileObject {
@Override
protected String[] doListChildren() throws Exception {
- List<String> childrenList = new ArrayList<>();
- if (getType() == FileType.FOLDER || isRootBucket()) {
- childrenList = getS3ObjectsFromVirtualFolder(key, bucketName);
+ if (getType() != FileType.FOLDER && !isRootBucket()) {
+ // null tells VFS this is not a folder, as for local files, instead of
an empty folder
+ return null;
+ }
+ return getS3ObjectsFromVirtualFolder(key, bucketName).toArray(new
String[0]);
+ }
+
+ /**
+ * The path with its %nn escapes decoded: the object key "file%.txt" is the
name "file%25.txt".
+ * The parser already validated the escapes, so decoding can't fail on a
parsed name.
+ */
+ private String decodedPath() {
+ try {
+ return getName().getPathDecoded();
+ } catch (FileSystemException e) {
+ return getName().getPath();
}
- return childrenList.toArray(new String[0]);
}
protected String getS3BucketName() {
- String path = getName().getPath();
+ String path = decodedPath();
if (path == null) {
return "";
}
@@ -120,7 +145,7 @@ public abstract class S3CommonFileObject extends
AbstractFileObject {
List<Bucket> buckets = fileSystem.getS3Client().listBuckets().buckets();
if (buckets != null) {
for (Bucket b : buckets) {
- childrenList.add(b.name() + DELIMITER);
+ childrenList.add(UriParser.encode(b.name()) + DELIMITER);
}
}
} else {
@@ -146,7 +171,7 @@ public abstract class S3CommonFileObject extends
AbstractFileObject {
for (S3Object s3o : response.contents()) {
if (!s3o.key().equals(realKey)) {
String childName = s3o.key().substring(prefix.length());
- childrenList.add(childName);
+ childrenList.add(UriParser.encode(childName));
cacheEntries.put(
s3o.key(),
new S3ListCache.ChildInfo(
@@ -160,7 +185,7 @@ public abstract class S3CommonFileObject extends
AbstractFileObject {
for (CommonPrefix cp : response.commonPrefixes()) {
String p = cp.prefix();
if (!p.equals(realKey)) {
- childrenList.add(p.substring(prefix.length()));
+ childrenList.add(UriParser.encode(p.substring(prefix.length())));
cacheEntries.put(p, new S3ListCache.ChildInfo(FileType.FOLDER, 0,
Instant.EPOCH));
}
}
@@ -174,7 +199,7 @@ public abstract class S3CommonFileObject extends
AbstractFileObject {
}
protected String getBucketRelativeS3Path() {
- String path = getName().getPath();
+ String path = decodedPath();
if (path == null) {
return "";
}
@@ -217,6 +242,9 @@ public abstract class S3CommonFileObject extends
AbstractFileObject {
@Override
public void doAttach() throws Exception {
LogChannel.GENERAL.logDebug("Attach called on {0}", getQualifiedName());
+ // An earlier attach may have appended '/' to a folder's key; starting
from it would look up
+ // the wrong parent in the list cache and probe "folder//", turning the
folder IMAGINARY.
+ this.key = getBucketRelativeS3Path();
injectType(FileType.IMAGINARY);
if (isRootBucket()) {
diff --git
a/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileObjectStoreSemanticsTest.java
b/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileObjectStoreSemanticsTest.java
new file mode 100644
index 0000000000..273d80310e
--- /dev/null
+++
b/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileObjectStoreSemanticsTest.java
@@ -0,0 +1,286 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.vfs.s3.vfs;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.TreeMap;
+import java.util.TreeSet;
+import org.apache.commons.vfs2.CacheStrategy;
+import org.apache.commons.vfs2.FileName;
+import org.apache.commons.vfs2.FileObject;
+import org.apache.commons.vfs2.FileSystem;
+import org.apache.commons.vfs2.FileSystemException;
+import org.apache.commons.vfs2.FileSystemOptions;
+import org.apache.commons.vfs2.FileType;
+import org.apache.commons.vfs2.cache.SoftRefFilesCache;
+import org.apache.commons.vfs2.impl.DefaultFileSystemManager;
+import org.apache.commons.vfs2.provider.UriParser;
+import org.apache.hop.core.HopEnvironment;
+import org.apache.hop.vfs.s3.s3.vfs.S3FileProvider;
+import org.apache.hop.vfs.s3.s3.vfs.S3FileSystem;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import software.amazon.awssdk.awscore.exception.AwsErrorDetails;
+import software.amazon.awssdk.core.ResponseInputStream;
+import software.amazon.awssdk.http.AbortableInputStream;
+import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.model.CommonPrefix;
+import software.amazon.awssdk.services.s3.model.GetObjectRequest;
+import software.amazon.awssdk.services.s3.model.GetObjectResponse;
+import software.amazon.awssdk.services.s3.model.HeadObjectRequest;
+import software.amazon.awssdk.services.s3.model.HeadObjectResponse;
+import software.amazon.awssdk.services.s3.model.ListObjectsV2Request;
+import software.amazon.awssdk.services.s3.model.ListObjectsV2Response;
+import software.amazon.awssdk.services.s3.model.NoSuchKeyException;
+import software.amazon.awssdk.services.s3.model.S3Object;
+
+/**
+ * The S3 provider against an in-memory bucket that answers like S3: keys are
flat, a "folder" is
+ * only a key prefix unless a marker object exists, and a missing key is a
404. Runs through a real
+ * file system manager with the ON_RESOLVE cache strategy Hop uses.
+ */
+class S3FileObjectStoreSemanticsTest {
+
+ private static final String BUCKET = "bucket";
+
+ /** The bucket's objects: key to content. */
+ private final TreeMap<String, byte[]> objects = new TreeMap<>();
+
+ private DefaultFileSystemManager manager;
+
+ @BeforeAll
+ static void initHop() throws Exception {
+ HopEnvironment.init();
+ }
+
+ @BeforeEach
+ void setUp() throws Exception {
+ S3Client client = inMemoryClient();
+ manager = new DefaultFileSystemManager();
+ manager.setFilesCache(new SoftRefFilesCache());
+ manager.setCacheStrategy(CacheStrategy.ON_RESOLVE);
+ manager.addProvider(
+ "s3",
+ new S3FileProvider() {
+ @Override
+ public FileSystem doCreateFileSystem(FileName name,
FileSystemOptions options) {
+ return new S3FileSystem(name, options) {
+ @Override
+ public S3Client getS3Client() {
+ return client;
+ }
+ };
+ }
+ });
+ manager.init();
+ }
+
+ @AfterEach
+ void tearDown() {
+ manager.close();
+ }
+
+ private void put(String key, String content) {
+ objects.put(key, content.getBytes(StandardCharsets.UTF_8));
+ }
+
+ private FileObject resolve(String path) throws FileSystemException {
+ return manager.resolveFile("s3://" + BUCKET + "/" + path);
+ }
+
+ private static List<String> names(FileObject[] children) throws
FileSystemException {
+ List<String> names = new ArrayList<>();
+ for (FileObject child : children) {
+ // Base names keep VFS's %nn escapes, as they do for local files
+ names.add(UriParser.decode(child.getName().getBaseName()));
+ }
+ return names;
+ }
+
+ private static NoSuchKeyException notFound() {
+ return (NoSuchKeyException)
+ NoSuchKeyException.builder()
+ .statusCode(404)
+
.awsErrorDetails(AwsErrorDetails.builder().errorCode("NoSuchKey").build())
+ .build();
+ }
+
+ private S3Client inMemoryClient() {
+ S3Client client = mock(S3Client.class);
+ when(client.headObject(any(HeadObjectRequest.class)))
+ .thenAnswer(
+ call -> {
+ byte[] content =
objects.get(call.<HeadObjectRequest>getArgument(0).key());
+ if (content == null) {
+ throw notFound();
+ }
+ return HeadObjectResponse.builder().contentLength((long)
content.length).build();
+ });
+ when(client.getObject(any(GetObjectRequest.class)))
+ .thenAnswer(
+ call -> {
+ byte[] content =
objects.get(call.<GetObjectRequest>getArgument(0).key());
+ if (content == null) {
+ throw notFound();
+ }
+ return new ResponseInputStream<>(
+ GetObjectResponse.builder().contentLength((long)
content.length).build(),
+ AbortableInputStream.create(new
ByteArrayInputStream(content)));
+ });
+ when(client.listObjectsV2(any(ListObjectsV2Request.class)))
+ .thenAnswer(call -> list(call.getArgument(0)));
+ return client;
+ }
+
+ /** ListObjectsV2 with a delimiter: direct objects as contents, deeper keys
as common prefixes. */
+ private ListObjectsV2Response list(ListObjectsV2Request request) {
+ String prefix = request.prefix() == null ? "" : request.prefix();
+ List<S3Object> contents = new ArrayList<>();
+ TreeSet<String> prefixes = new TreeSet<>();
+ for (String key : objects.keySet()) {
+ if (!key.startsWith(prefix)) {
+ continue;
+ }
+ int slash = key.indexOf('/', prefix.length());
+ if (slash >= 0) {
+ prefixes.add(key.substring(0, slash + 1));
+ } else {
+ contents.add(S3Object.builder().key(key).size((long)
objects.get(key).length).build());
+ }
+ }
+ return ListObjectsV2Response.builder()
+ .contents(contents)
+ .commonPrefixes(
+ prefixes.stream().map(p ->
CommonPrefix.builder().prefix(p).build()).toList())
+ .build();
+ }
+
+ @Test
+ void prefixFolderStaysAFolderAfterItsParentIsListedAgain() throws Exception {
+ put("data/sub/file.txt", "x");
+
+ FileObject data = resolve("data");
+ assertEquals(List.of("sub"), names(data.getChildren()));
+ FileObject sub = resolve("data/sub");
+ assertEquals(FileType.FOLDER, sub.getType());
+
+ data.getChildren();
+ FileObject again = resolve("data/sub");
+
+ assertEquals(FileType.FOLDER, again.getType());
+ assertEquals(List.of("file.txt"), names(again.getChildren()));
+ }
+
+ @Test
+ void prefixFolderStaysAFolderAfterARefresh() throws Exception {
+ put("data/sub/file.txt", "x");
+ resolve("data").getChildren();
+ FileObject sub = resolve("data/sub");
+ assertEquals(FileType.FOLDER, sub.getType());
+
+ sub.refresh();
+
+ assertEquals(FileType.FOLDER, sub.getType());
+ }
+
+ @Test
+ void listsAndReadsAnObjectWithAPercentInItsName() throws Exception {
+ put("data/file%.txt", "percent");
+ put("data/100% sure/inner.txt", "inner");
+
+ FileObject data = resolve("data");
+ assertEquals(
+ new TreeSet<>(List.of("file%.txt", "100% sure")), new
TreeSet<>(names(data.getChildren())));
+
+ for (FileObject child : data.getChildren()) {
+ if (child.getName().getBaseName().equals("file%25.txt")) {
+ assertEquals(FileType.FILE, child.getType());
+ try (InputStream in = child.getContent().getInputStream()) {
+ assertEquals("percent", new String(in.readAllBytes(),
StandardCharsets.UTF_8));
+ }
+ } else {
+ assertEquals(FileType.FOLDER, child.getType());
+ assertEquals(List.of("inner.txt"), names(child.getChildren()));
+ }
+ }
+ }
+
+ @Test
+ void resolvesAPercentNameByItsEscapedForm() throws Exception {
+ put("data/file%.txt", "percent");
+
+ FileObject file = resolve("data/file%25.txt");
+
+ assertTrue(file.exists());
+ assertEquals("percent",
file.getContent().getString(StandardCharsets.UTF_8));
+ }
+
+ @Test
+ void readingAFolderSaysItIsNotAFile() throws Exception {
+ put("data/sub/file.txt", "x");
+ FileObject sub = resolve("data/sub");
+
+ FileSystemException e =
+ assertThrows(FileSystemException.class, () ->
sub.getContent().getInputStream());
+
+ assertEquals("vfs.provider/read-not-file.error", e.getCode());
+ }
+
+ @Test
+ void readingAMissingObjectSaysItIsNotAFile() throws Exception {
+ FileObject missing = resolve("data/missing.txt");
+
+ FileSystemException e =
+ assertThrows(FileSystemException.class, () ->
missing.getContent().getInputStream());
+
+ assertEquals("vfs.provider/read-not-file.error", e.getCode());
+ }
+
+ @Test
+ void listingAFileSaysItIsNotAFolder() throws Exception {
+ put("data/plain.txt", "content");
+ FileObject file = resolve("data/plain.txt");
+
+ FileSystemException e = assertThrows(FileSystemException.class,
file::getChildren);
+
+ assertEquals("vfs.provider/list-children-not-folder.error", e.getCode());
+ }
+
+ @Test
+ void stillReadsAPlainFile() throws Exception {
+ put("data/plain.txt", "content");
+
+ assertEquals(
+ "content",
resolve("data/plain.txt").getContent().getString(StandardCharsets.UTF_8));
+ assertEquals(Arrays.asList("plain.txt"),
names(resolve("data").getChildren()));
+ }
+}
diff --git
a/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileObjectTest.java
b/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileObjectTest.java
index 94f13f6f07..10937e699b 100644
---
a/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileObjectTest.java
+++
b/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileObjectTest.java
@@ -182,7 +182,10 @@ class S3FileObjectTest {
@Test
void testGetInputStream() throws Exception {
- assertNotNull(s3FileObjectBucketSpy.getInputStream());
+ // A bucket is a folder: it has no content to read
+ FileSystemException e =
+ assertThrows(FileSystemException.class, () ->
s3FileObjectBucketSpy.getInputStream());
+ assertEquals("vfs.provider/read-not-file.error", e.getCode());
}
@Test
@@ -258,7 +261,8 @@ class S3FileObjectTest {
@Test
void testDoGetInputStream() throws Exception {
- assertNotNull(s3FileObjectBucketSpy.getInputStream());
+ assertThrows(
+ FileSystemException.class, () ->
s3FileObjectBucketSpy.getContent().getInputStream());
}
@Test
diff --git
a/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileProviderFileSystemReuseTest.java
b/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileProviderFileSystemReuseTest.java
new file mode 100644
index 0000000000..27c110a940
--- /dev/null
+++
b/plugins/tech/aws/src/test/java/org/apache/hop/vfs/s3/vfs/S3FileProviderFileSystemReuseTest.java
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.vfs.s3.vfs;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+import org.apache.commons.vfs2.FileSystem;
+import org.apache.commons.vfs2.impl.DefaultFileSystemManager;
+import org.apache.hop.core.HopEnvironment;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.vfs.s3.metadata.S3AuthType;
+import org.apache.hop.vfs.s3.metadata.S3Meta;
+import org.apache.hop.vfs.s3.s3.vfs.S3FileProvider;
+import org.apache.hop.vfs.s3.s3common.S3CommonFileProvider;
+import org.apache.hop.vfs.s3.s3common.S3CommonFileSystemConfigBuilder;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Resolving files reuses the provider's file system, as HopVfs resolves them:
without options.
+ * Building a file system means building an S3 client, so a new one per file
is expensive, and the
+ * provider would keep every one of them. Nothing here connects to S3.
+ */
+class S3FileProviderFileSystemReuseTest {
+
+ private static final String ENDPOINT = "http://127.0.0.1:9";;
+
+ private DefaultFileSystemManager manager;
+
+ @BeforeAll
+ static void initHop() throws Exception {
+ HopEnvironment.init();
+ }
+
+ @BeforeEach
+ void setUp() throws Exception {
+ S3Meta connection = new S3Meta();
+ connection.setName("s3reuse");
+ connection.setAuthenticationType(S3AuthType.ACCESS_KEYS.name());
+ connection.setAccessKey("access");
+ connection.setSecretKey("secret");
+ connection.setRegion("eu-west-1");
+ connection.setEndpoint(ENDPOINT);
+ connection.setPathStyleAccess(true);
+
+ manager = new DefaultFileSystemManager();
+ manager.addProvider("s3", new S3FileProvider());
+ manager.addProvider("s3reuse", new S3FileProvider(new Variables(),
connection));
+ manager.init();
+ }
+
+ @AfterEach
+ void tearDown() {
+ manager.close();
+ }
+
+ private FileSystem fileSystemOf(String uri) throws Exception {
+ return manager.resolveFile(uri).getFileSystem();
+ }
+
+ @Test
+ void defaultSchemeReusesItsFileSystem() throws Exception {
+ FileSystem first = fileSystemOf("s3://bucket/a.txt");
+
+ assertSame(first, fileSystemOf("s3://bucket/folder/b.txt"));
+ assertSame(first, fileSystemOf("s3://other-bucket/c.txt"));
+ }
+
+ @Test
+ void namedConnectionReusesItsFileSystem() throws Exception {
+ FileSystem first = fileSystemOf("s3reuse://bucket/a.txt");
+
+ assertSame(first, fileSystemOf("s3reuse://bucket/folder/b.txt"));
+ assertSame(first, fileSystemOf("s3reuse://other-bucket/c.txt"));
+ }
+
+ @Test
+ void namedConnectionKeepsItsOwnSettings() throws Exception {
+ FileSystem named = fileSystemOf("s3reuse://bucket/a.txt");
+ FileSystem standard = fileSystemOf("s3://bucket/a.txt");
+
+ assertNotSame(standard, named);
+ S3CommonFileSystemConfigBuilder config =
+ new S3CommonFileSystemConfigBuilder(named.getFileSystemOptions());
+ assertEquals(ENDPOINT, config.getEndpoint());
+ assertEquals("eu-west-1", config.getRegion());
+ assertEquals("access", config.getAccessKey());
+
+ // The named connection must never write its settings into the defaults
s3:// uses
+ assertNull(
+ new
S3CommonFileSystemConfigBuilder(S3CommonFileProvider.getDefaultFileSystemOptions())
+ .getEndpoint());
+ }
+}
