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 da7c91e54c collapse the 2 FileSystemManagers, fixes #7688 (#7759)
da7c91e54c is described below
commit da7c91e54cb2ef4c8b043f06141f21456fbbce97
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Mon Aug 3 12:52:42 2026 +0200
collapse the 2 FileSystemManagers, fixes #7688 (#7759)
do not register empty providers
---
.../main/java/org/apache/hop/core/vfs/HopVfs.java | 470 +++++++++++----------
.../java/org/apache/hop/core/vfs/plugin/IVfs.java | 20 +
.../hop/core/vfs/HopVfsSingleManagerTest.java | 271 ++++++++++++
.../org/apache/hop/projects/util/ProjectsUtil.java | 6 +
.../hop/vfs/databricks/DatabricksVfsPlugin.java | 2 +-
.../vfs/databricks/DatabricksVfsPluginTest.java | 9 +-
.../org/apache/hop/vfs/minio/MinioVfsPlugin.java | 2 +-
.../apache/hop/vfs/minio/MinioVfsPluginTest.java | 28 +-
8 files changed, 553 insertions(+), 255 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/vfs/HopVfs.java
b/core/src/main/java/org/apache/hop/core/vfs/HopVfs.java
index fee0d224e7..de7b547e30 100644
--- a/core/src/main/java/org/apache/hop/core/vfs/HopVfs.java
+++ b/core/src/main/java/org/apache/hop/core/vfs/HopVfs.java
@@ -29,7 +29,6 @@ import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.UUID;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.vfs2.CacheStrategy;
import org.apache.commons.vfs2.FileContent;
@@ -47,6 +46,7 @@ import org.apache.hop.core.Const;
import org.apache.hop.core.exception.HopException;
import org.apache.hop.core.exception.HopFileException;
import org.apache.hop.core.exception.HopRuntimeException;
+import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.core.plugins.IPlugin;
import org.apache.hop.core.plugins.PluginRegistry;
import org.apache.hop.core.variables.IVariables;
@@ -59,66 +59,152 @@ public class HopVfs {
public static final String TEMP_DIR = System.getProperty("java.io.tmpdir");
+ /** The one and only file system manager. */
private static DefaultFileSystemManager fsm;
- private static DefaultFileSystemManager extendedFsm;
- private static final ReentrantReadWriteLock lock = new
ReentrantReadWriteLock();
+ /**
+ * The variables used to bootstrap the metadata driven providers (the named
VFS connections).
+ * They're only needed while those providers are being registered, not to
resolve individual
+ * files.
+ */
+ private static IVariables bootstrapVariables;
- public static DefaultFileSystemManager getFileSystemManager() {
- lock.readLock().lock();
- try {
- if (fsm == null) {
- try {
- fsm = createFileSystemManager();
- fsm.init();
- } catch (Exception e) {
- throw new HopRuntimeException("Error initializing file system
manager : ", e);
- }
+ /** Set once the metadata driven providers have been registered on {@link
#fsm}. */
+ private static boolean namedProvidersRegistered;
+
+ /**
+ * Guards against re-entrant registration: reading the VFS connection
metadata resolves files
+ * through this very class.
+ */
+ private static boolean registeringNamedProviders;
+
+ /**
+ * Set the variables to look up the metadata driven VFS providers (the named
VFS connections)
+ * with. Call this when a project is loaded: its variables are what point at
the metadata holding
+ * those connections. The named connections of that project are registered
the next time the file
+ * system manager is used.
+ *
+ * @param variables the variables to bootstrap the VFS providers with
+ */
+ public static synchronized void setBootstrapVariables(IVariables variables) {
+ if (variables == bootstrapVariables) {
+ return;
+ }
+ bootstrapVariables = variables;
+
+ // Connections of a previous project are registered on the manager and
there's no unregistering
+ // them, so start over. Nothing registered yet means we can keep the
manager as it is: phase 2
+ // simply runs with these variables the next time around.
+ //
+ if (namedProvidersRegistered) {
+ reset();
+ }
+ }
+
+ /**
+ * Get the file system manager. There is only one: it knows both the fixed
schemes (file, zip, s3,
+ * ...) and the schemes of the named VFS connections in the metadata.
+ *
+ * @return the file system manager
+ */
+ public static synchronized DefaultFileSystemManager getFileSystemManager() {
+ if (fsm == null) {
+ try {
+ // Phase 1 : the standard schemes and the fixed schemes of the VFS
plugins.
+ //
+ DefaultFileSystemManager manager = createFileSystemManager();
+ manager.init();
+
+ // Publish before phase 2 : registering the metadata driven providers
below reads the
+ // metadata, and reading metadata resolves files through this very
manager.
+ //
+ fsm = manager;
+ } catch (Exception e) {
+ throw new HopRuntimeException("Error initializing file system manager
: ", e);
}
- return fsm;
- } finally {
- lock.readLock().unlock();
}
+
+ // Phase 2 : the providers of the named VFS connections in the metadata.
Only once a project
+ // handed us its variables: they're what points us at the metadata holding
those connections,
+ // and what the providers resolve their credentials with. Everything
resolving files before
+ // that (reading the configuration, loading the images of the GUI) wants a
local file and is
+ // served by phase 1 alone.
+ //
+ if (bootstrapVariables != null && !namedProvidersRegistered &&
!registeringNamedProviders) {
+ // Set before anything else: registering reads metadata, which resolves
files, which lands
+ // right back here.
+ //
+ registeringNamedProviders = true;
+ try {
+ registerNamedProviders(fsm, bootstrapVariables);
+ } finally {
+ registeringNamedProviders = false;
+ namedProvidersRegistered = true;
+ }
+ }
+
+ return fsm;
}
- public static DefaultFileSystemManager getFileSystemManager(IVariables
variables) {
- lock.readLock().lock();
- try {
- if (extendedFsm == null) {
- try {
- extendedFsm = createFileSystemManager();
- // Here are extra VFS plugins to register
- //
- PluginRegistry registry = PluginRegistry.getInstance();
- List<IPlugin> plugins = registry.getPlugins(VfsPluginType.class);
- for (IPlugin plugin : plugins) {
- IVfs iVfs = registry.loadClass(plugin, IVfs.class);
- try {
- Map<String, FileProvider> fileProviderMap =
iVfs.getProviders(variables);
- if (fileProviderMap != null) {
- for (Map.Entry<String, FileProvider> entry :
fileProviderMap.entrySet()) {
- extendedFsm.addProvider(entry.getKey(), entry.getValue());
- }
- }
- } catch (Exception e) {
- throw new HopException(
- "Error registering provider for VFS plugin "
- + plugin.getIds()[0]
- + " : "
- + plugin.getName()
- + " : ",
- e);
- }
- }
+ /**
+ * @param variables the variables to bootstrap the metadata driven providers
with, in case nothing
+ * did that yet
+ * @return the one and only file system manager
+ * @see #getFileSystemManager()
+ */
+ public static synchronized DefaultFileSystemManager
getFileSystemManager(IVariables variables) {
+ bootstrapWith(variables);
+ return getFileSystemManager();
+ }
- extendedFsm.init();
- } catch (Exception e) {
- throw new HopRuntimeException("Error initializing file system
manager : ", e);
+ /**
+ * Remember the variables to bootstrap the metadata driven providers with,
for as long as nothing
+ * bootstrapped them yet. These are the first variables we get to see, so
the named connections
+ * are looked up in the metadata they point at, the next time the manager is
used. An explicit
+ * {@link #setBootstrapVariables(IVariables)} always wins.
+ */
+ private static synchronized void bootstrapWith(IVariables variables) {
+ if (variables != null && bootstrapVariables == null) {
+ bootstrapVariables = variables;
+ }
+ }
+
+ /**
+ * Register a provider for every named VFS connection in the metadata. A
connection which can't be
+ * registered is skipped: a single bad connection should never take the
whole file system manager
+ * down with it.
+ */
+ private static void registerNamedProviders(
+ DefaultFileSystemManager manager, IVariables variables) {
+ PluginRegistry registry = PluginRegistry.getInstance();
+ for (IPlugin plugin : registry.getPlugins(VfsPluginType.class)) {
+ try {
+ IVfs iVfs = registry.loadClass(plugin, IVfs.class);
+ Map<String, FileProvider> fileProviderMap =
iVfs.getProviders(variables);
+ if (fileProviderMap == null) {
+ continue;
+ }
+ for (Map.Entry<String, FileProvider> entry :
fileProviderMap.entrySet()) {
+ String scheme = entry.getKey();
+ if (manager.hasProvider(scheme)) {
+ LogChannel.GENERAL.logError(
+ "The VFS connection '"
+ + scheme
+ + "' of plugin "
+ + plugin.getIds()[0]
+ + " is ignored: a provider is already registered for that
scheme.");
+ continue;
+ }
+ manager.addProvider(scheme, entry.getValue());
}
+ } catch (Exception e) {
+ LogChannel.GENERAL.logError(
+ "Error registering provider for VFS plugin "
+ + plugin.getIds()[0]
+ + " : "
+ + plugin.getName(),
+ e);
}
- return extendedFsm;
- } finally {
- lock.readLock().unlock();
}
}
@@ -182,7 +268,15 @@ public class HopVfs {
for (IPlugin plugin : plugins) {
IVfs iVfs = registry.loadClass(plugin, IVfs.class);
try {
- fsm.addProvider(iVfs.getUrlSchemes(), iVfs.getProvider());
+ String[] urlSchemes = iVfs.getUrlSchemes();
+ FileProvider provider = iVfs.getProvider();
+
+ // Skip plugins with no fixed scheme (Minio, Databricks): a provider
the manager has no
+ // scheme for is never reached, and never closed. Phase 2 registers
those by name.
+ if (urlSchemes == null || urlSchemes.length == 0 || provider ==
null) {
+ continue;
+ }
+ fsm.addProvider(urlSchemes, provider);
} catch (Exception e) {
throw new HopException(
"Error registering provider for VFS plugin "
@@ -199,100 +293,59 @@ public class HopVfs {
}
}
- public static synchronized FileObject getFileObject(String vfsFilename,
IVariables variables)
+ /**
+ * @param vfsFilename the name of the file to resolve
+ * @param variables the variables to bootstrap the metadata driven providers
with, in case nothing
+ * did that yet. They play no role in resolving the file itself.
+ * @return the file object
+ * @see #getFileObject(String)
+ */
+ public static FileObject getFileObject(String vfsFilename, IVariables
variables)
throws HopFileException {
- lock.readLock().lock();
- try {
- DefaultFileSystemManager fsManager = getFileSystemManager(variables);
-
- try {
- // We have one problem with VFS: if the file is in a subdirectory of
the current one:
- // somedir/somefile
- // In that case, VFS doesn't parse the file correctly.
- // We need to put file: in front of it to make it work.
- // However, how are we going to verify this?
- //
- // We are going to see if the filename starts with one of the known
protocols like file:
- // zip: ram: smb: jar: etc.
- // If not, we are going to assume it's a file.
- //
- boolean relativeFilename = true;
- String[] initialSchemes = fsManager.getSchemes();
-
- relativeFilename = checkForScheme(initialSchemes, relativeFilename,
vfsFilename);
-
- String filename;
- if (vfsFilename.startsWith("\\\\")) {
- File file = new File(vfsFilename);
- filename = file.toURI().toString();
- } else {
- if (relativeFilename) {
- File file = new File(vfsFilename);
- filename = file.getAbsolutePath();
- } else {
- filename = vfsFilename;
- }
- }
-
- return fsManager.resolveFile(filename);
- } catch (Exception e) {
- throw new HopFileException(
- "Unable to get VFS File object for filename '"
- + cleanseFilename(vfsFilename)
- + "' : "
- + e.getMessage(),
- e);
- }
- } finally {
- lock.readLock().unlock();
- }
+ bootstrapWith(variables);
+ return getFileObject(vfsFilename);
}
public static synchronized FileObject getFileObject(String vfsFilename)
throws HopFileException {
- lock.readLock().lock();
- try {
- DefaultFileSystemManager fsManager = getFileSystemManager();
+ DefaultFileSystemManager fsManager = getFileSystemManager();
- try {
- // We have one problem with VFS: if the file is in a subdirectory of
the current one:
- // somedir/somefile
- // In that case, VFS doesn't parse the file correctly.
- // We need to put file: in front of it to make it work.
- // However, how are we going to verify this?
- //
- // We are going to see if the filename starts with one of the known
protocols like file:
- // zip: ram: smb: jar: etc.
- // If not, we are going to assume it's a file.
- //
- boolean relativeFilename = true;
- String[] initialSchemes = fsManager.getSchemes();
+ try {
+ // We have one problem with VFS: if the file is in a subdirectory of the
current one:
+ // somedir/somefile
+ // In that case, VFS doesn't parse the file correctly.
+ // We need to put file: in front of it to make it work.
+ // However, how are we going to verify this?
+ //
+ // We are going to see if the filename starts with one of the known
protocols like file:
+ // zip: ram: smb: jar: etc.
+ // If not, we are going to assume it's a file.
+ //
+ boolean relativeFilename = true;
+ String[] initialSchemes = fsManager.getSchemes();
- relativeFilename = checkForScheme(initialSchemes, relativeFilename,
vfsFilename);
+ relativeFilename = checkForScheme(initialSchemes, relativeFilename,
vfsFilename);
- String filename;
- if (vfsFilename.startsWith("\\\\")) {
+ String filename;
+ if (vfsFilename.startsWith("\\\\")) {
+ File file = new File(vfsFilename);
+ filename = file.toURI().toString();
+ } else {
+ if (relativeFilename) {
File file = new File(vfsFilename);
- filename = file.toURI().toString();
+ filename = file.getAbsolutePath();
} else {
- if (relativeFilename) {
- File file = new File(vfsFilename);
- filename = file.getAbsolutePath();
- } else {
- filename = vfsFilename;
- }
+ filename = vfsFilename;
}
-
- return fsManager.resolveFile(filename);
- } catch (Exception e) {
- throw new HopFileException(
- "Unable to get VFS File object for filename '"
- + cleanseFilename(vfsFilename)
- + "' : "
- + e.getMessage(),
- e);
}
- } finally {
- lock.readLock().unlock();
+
+ return fsManager.resolveFile(filename);
+ } catch (Exception e) {
+ throw new HopFileException(
+ "Unable to get VFS File object for filename '"
+ + cleanseFilename(vfsFilename)
+ + "' : "
+ + e.getMessage(),
+ e);
}
}
@@ -362,23 +415,13 @@ public class HopVfs {
}
}
+ /**
+ * @see #fileExists(String)
+ */
public static boolean fileExists(String vfsFilename, IVariables variables)
throws HopFileException {
- FileObject fileObject = null;
- try {
- fileObject = getFileObject(vfsFilename, variables);
- return fileObject.exists();
- } catch (IOException e) {
- throw new HopFileException(e);
- } finally {
- if (fileObject != null) {
- try {
- fileObject.close();
- } catch (Exception e) {
- /* Ignore */
- }
- }
- }
+ bootstrapWith(variables);
+ return fileExists(vfsFilename);
}
public static boolean isLocalFileSystem(String path) {
@@ -405,15 +448,13 @@ public class HopVfs {
}
}
+ /**
+ * @see #getInputStream(String)
+ */
public static InputStream getInputStream(String vfsFilename, IVariables
variables)
throws HopFileException {
- try {
- FileObject fileObject = getFileObject(vfsFilename, variables);
-
- return getInputStream(fileObject);
- } catch (IOException e) {
- throw new HopFileException(e);
- }
+ bootstrapWith(variables);
+ return getInputStream(vfsFilename);
}
public static OutputStream getOutputStream(FileObject fileObject, boolean
append)
@@ -453,14 +494,13 @@ public class HopVfs {
}
}
+ /**
+ * @see #getOutputStream(String, boolean)
+ */
public static OutputStream getOutputStream(
String vfsFilename, boolean append, IVariables variables) throws
HopFileException {
- try {
- FileObject fileObject = getFileObject(vfsFilename, variables);
- return getOutputStream(fileObject, append);
- } catch (IOException e) {
- throw new HopFileException(e);
- }
+ bootstrapWith(variables);
+ return getOutputStream(vfsFilename, append);
}
public static OutputStream getOutputStream(String vfsFilename, boolean
append)
@@ -519,19 +559,12 @@ public class HopVfs {
return friendlyName;
}
+ /**
+ * @see #getFriendlyURI(String)
+ */
public static String getFriendlyURI(String filename, IVariables variables) {
- if (filename == null) {
- return null;
- }
- String friendlyName;
- try {
- friendlyName = getFriendlyURI(HopVfs.getFileObject(filename, variables));
- } catch (Exception e) {
- // unable to get a friendly name from VFS object.
- // Cleanse name of pwd before returning
- friendlyName = cleanseFilename(filename);
- }
- return friendlyName;
+ bootstrapWith(variables);
+ return getFriendlyURI(filename);
}
public static String getFriendlyURI(FileObject fileObject) {
@@ -593,40 +626,17 @@ public class HopVfs {
* @param prefix - file name
* @param suffix - file extension
* @param directory - directory where file will be created
+ * @param variables the variables to bootstrap the metadata driven providers
with, in case nothing
+ * did that yet
* @return FileObject
* @throws HopFileException
+ * @see #createTempFile(String, String, String)
*/
- public static synchronized FileObject createTempFile(
+ public static FileObject createTempFile(
String prefix, String suffix, String directory, IVariables variables)
throws HopFileException {
- try {
- FileObject fileObject;
- do {
- // Temporary files are always stored locally.
- // No other schemes besides file:// make sense
- //
- String baseUrl;
- if (directory.contains("://")) {
- baseUrl = directory;
- } else {
- File directoryFile = new File(directory);
- baseUrl = "file://" + directoryFile.getAbsolutePath();
- }
-
- // Build temporary file name using UUID to ensure uniqueness. Old
mechanism would fail using
- // Sort Rows (for example)
- // when there multiple nodes with multiple JVMs on each node. In this
case, the temp file
- // names would end up being
- // duplicated which would cause the sort to fail.
- //
- String filename = baseUrl + "/" + prefix + "_" + UUID.randomUUID() +
suffix;
-
- fileObject = getFileObject(filename, variables);
- } while (fileObject.exists());
- return fileObject;
- } catch (IOException e) {
- throw new HopFileException(e);
- }
+ bootstrapWith(variables);
+ return createTempFile(prefix, suffix, directory);
}
public static Comparator<FileObject> getComparator() {
@@ -645,24 +655,30 @@ public class HopVfs {
* @return boolean
*/
public static boolean startsWithScheme(String vfsFileName, IVariables
variables) {
- lock.readLock().lock();
- try {
-
- DefaultFileSystemManager fsManager = getFileSystemManager(variables);
+ bootstrapWith(variables);
+ return startsWithScheme(vfsFileName);
+ }
- boolean found = false;
- String[] schemes = fsManager.getSchemes();
- for (String scheme : schemes) {
- if (vfsFileName.startsWith(scheme + ":")) {
- found = true;
- break;
- }
+ /**
+ * Check if filename starts with one of the known protocols like file: zip:
ram: smb: jar: etc. If
+ * yes, return true otherwise return false
+ *
+ * @param vfsFileName
+ * @return boolean
+ */
+ public static boolean startsWithScheme(String vfsFileName) {
+ DefaultFileSystemManager fsManager = getFileSystemManager();
+
+ boolean found = false;
+ String[] schemes = fsManager.getSchemes();
+ for (String scheme : schemes) {
+ if (vfsFileName.startsWith(scheme + ":")) {
+ found = true;
+ break;
}
-
- return found;
- } finally {
- lock.readLock().unlock();
}
+
+ return found;
}
/**
@@ -728,23 +744,25 @@ public class HopVfs {
/**
* @see StandardFileSystemManager#freeUnusedResources()
*/
- public static void freeUnusedResources() {
+ public static synchronized void freeUnusedResources() {
if (fsm != null) {
fsm.freeUnusedResources();
}
}
- public static void reset() {
+ /**
+ * Drop the file system manager so it's rebuilt, providers of the named VFS
connections included,
+ * the next time it's used. The bootstrap variables are kept: use {@link
+ * #setBootstrapVariables(IVariables)} to change those.
+ */
+ public static synchronized void reset() {
if (fsm != null) {
fsm.freeUnusedResources();
fsm.close();
fsm = null;
}
- if (extendedFsm != null) {
- extendedFsm.freeUnusedResources();
- extendedFsm.close();
- extendedFsm = null;
- }
+ namedProvidersRegistered = false;
+ registeringNamedProviders = false;
}
public enum Suffix {
diff --git a/core/src/main/java/org/apache/hop/core/vfs/plugin/IVfs.java
b/core/src/main/java/org/apache/hop/core/vfs/plugin/IVfs.java
index 387f14c425..a6ccf71680 100644
--- a/core/src/main/java/org/apache/hop/core/vfs/plugin/IVfs.java
+++ b/core/src/main/java/org/apache/hop/core/vfs/plugin/IVfs.java
@@ -22,9 +22,29 @@ import org.apache.commons.vfs2.provider.FileProvider;
import org.apache.hop.core.variables.IVariables;
public interface IVfs {
+ /**
+ * The fixed URL schemes served by {@link #getProvider()}, empty for a
plugin serving named
+ * connections from the metadata only.
+ *
+ * @return the fixed URL schemes of this plugin
+ */
String[] getUrlSchemes();
+ /**
+ * The provider serving the fixed {@link #getUrlSchemes()}. Return {@code
null} when there are no
+ * fixed schemes: a provider registered under no scheme at all can never be
reached, and the file
+ * system manager can't close what it doesn't know a scheme for.
+ *
+ * @return the provider for the fixed schemes, or null if this plugin has
none
+ */
FileProvider getProvider();
+ /**
+ * The providers of the named connections in the metadata, keyed by
connection name : the name is
+ * the scheme these are registered under.
+ *
+ * @param variables the variables pointing at the metadata to load the
connections from
+ * @return a provider per named connection, empty or null if there are none
+ */
Map<String, FileProvider> getProviders(IVariables variables);
}
diff --git
a/core/src/test/java/org/apache/hop/core/vfs/HopVfsSingleManagerTest.java
b/core/src/test/java/org/apache/hop/core/vfs/HopVfsSingleManagerTest.java
new file mode 100644
index 0000000000..dddeeef810
--- /dev/null
+++ b/core/src/test/java/org/apache/hop/core/vfs/HopVfsSingleManagerTest.java
@@ -0,0 +1,271 @@
+/*
+ * 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.core.vfs;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.atLeast;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Consumer;
+import java.util.function.Predicate;
+import org.apache.commons.logging.Log;
+import org.apache.commons.vfs2.FileObject;
+import org.apache.commons.vfs2.impl.DefaultFileSystemManager;
+import org.apache.commons.vfs2.provider.FileProvider;
+import org.apache.commons.vfs2.provider.ram.RamFileProvider;
+import org.apache.hop.core.extension.IPluginMock;
+import org.apache.hop.core.plugins.IPlugin;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.core.variables.IVariables;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.core.vfs.plugin.IVfs;
+import org.apache.hop.core.vfs.plugin.VfsPluginType;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.ArgumentCaptor;
+
+/**
+ * {@link HopVfs} keeps one file system manager, knowing both the fixed
schemes and the schemes of
+ * the named VFS connections in the metadata. The variables of {@code
getFileObject(path,variables)}
+ * only bootstrap the metadata driven providers; they play no role in
resolving a file, so both
+ * entry points have to end up on that one manager.
+ */
+class HopVfsSingleManagerTest {
+
+ /** Stands in for HOP_METADATA_FOLDER: only variables carrying it see the
project metadata. */
+ private static final String PROJECT_MARKER = "TEST_PROJECT_METADATA";
+
+ private IPlugin registeredPlugin;
+
+ @AfterEach
+ void tearDown() {
+ if (registeredPlugin != null) {
+ PluginRegistry.getInstance().removePlugin(VfsPluginType.class,
registeredPlugin);
+ registeredPlugin = null;
+ }
+ // Drop the manager built with the test providers, and the variables it
was built with, so
+ // other tests start clean.
+ HopVfs.setBootstrapVariables(null);
+ HopVfs.reset();
+ }
+
+ @Test
+ @DisplayName("Both entry points return the very same file system manager")
+ void bothEntryPointsShareOneManager() {
+ DefaultFileSystemManager plain = HopVfs.getFileSystemManager();
+ DefaultFileSystemManager withVariables =
+ HopVfs.getFileSystemManager(Variables.getADefaultVariableSpace());
+
+ assertSame(plain, withVariables);
+ assertSame(withVariables, HopVfs.getFileSystemManager());
+ }
+
+ @Test
+ @DisplayName("Without variables nothing goes looking for metadata")
+ void noMetadataLookupBeforeVariablesArrive(@TempDir Path tmp) throws
Exception {
+ // Reading the Hop configuration resolves a file, and building a default
variable space reads
+ // the Hop configuration. Going looking for VFS connections here loops
forever, so resolving a
+ // file without variables must not touch the metadata at all.
+ boolean[] wentLooking = {false};
+ registerTestVfsPlugin("too-early", variables -> wentLooking[0] = true);
+
+ try (FileObject file =
+
HopVfs.getFileObject(tmp.resolve("hop-config.json").toUri().toString())) {
+ assertNotNull(file);
+ }
+
+ assertFalse(wentLooking[0], "The VFS connections were looked up before a
project was loaded");
+ }
+
+ @Test
+ @DisplayName("Files resolved with and without variables come off the same
manager")
+ void resolvedFilesShareOneManager(@TempDir Path tmp) throws Exception {
+ String url = tmp.resolve("single-manager.txt").toUri().toString();
+
+ try (FileObject withoutVariables = HopVfs.getFileObject(url);
+ FileObject withVariables =
+ HopVfs.getFileObject(url, Variables.getADefaultVariableSpace())) {
+ assertSame(
+ withoutVariables.getFileSystem().getFileSystemManager(),
+ withVariables.getFileSystem().getFileSystemManager());
+ assertSame(
+ HopVfs.getFileSystemManager(),
withoutVariables.getFileSystem().getFileSystemManager());
+ }
+ }
+
+ @Test
+ @DisplayName("Named connection providers land on the manager the no-argument
getter hands out")
+ void namedProvidersAreVisibleToTheNoArgumentGetter() {
+ registerTestVfsPlugin("named-connection-test", variables -> {});
+
+ // Loading a project is what hands over the variables to find the
connections with.
+ HopVfs.setBootstrapVariables(Variables.getADefaultVariableSpace());
+
+ // There is one manager, so the connections are there for callers passing
no variables too.
+ assertTrue(
+ HopVfs.getFileSystemManager().hasProvider("named-connection-test"),
+ "The named VFS connection is missing from the file system manager");
+
assertTrue(HopVfs.startsWithScheme("named-connection-test://some/file.txt"));
+ }
+
+ @Test
+ @DisplayName("Resolving a file while the named providers register does not
recurse")
+ void reEntrantResolutionDuringRegistration(@TempDir Path tmp) {
+ String url = tmp.resolve("metadata.json").toUri().toString();
+ FileObject[] resolvedDuringRegistration = new FileObject[1];
+
+ // Reading the VFS connection metadata resolves files through HopVfs, so
registering the
+ // providers re-enters getFileSystemManager(). That has to hand back the
manager under
+ // construction instead of building a second one or recursing forever.
+ registerTestVfsPlugin(
+ "reentrant-test",
+ variables -> {
+ try {
+ resolvedDuringRegistration[0] = HopVfs.getFileObject(url);
+ } catch (Exception e) {
+ throw new IllegalStateException("Re-entrant file resolution
failed", e);
+ }
+ });
+
+ HopVfs.setBootstrapVariables(Variables.getADefaultVariableSpace());
+ DefaultFileSystemManager manager = HopVfs.getFileSystemManager();
+
+ assertNotNull(resolvedDuringRegistration[0], "Nothing was resolved during
registration");
+ assertSame(manager,
resolvedDuringRegistration[0].getFileSystem().getFileSystemManager());
+ assertTrue(manager.hasProvider("reentrant-test"));
+ }
+
+ @Test
+ @DisplayName("Resolving a file before a project is enabled does not lock out
the named providers")
+ void earlyResolutionDoesNotLockOutNamedProviders(@TempDir Path tmp) throws
Exception {
+ // A named connection only visible to variables pointing at the metadata
of a project, the way
+ // HOP_METADATA_FOLDER points the VFS plugins at the connections of the
project being opened.
+ registerTestVfsPlugin(
+ "project-connection",
+ variables -> {},
+ variables -> "true".equals(variables.getVariable(PROJECT_MARKER)));
+
+ // HopGui resolves files long before any project is enabled: it loads its
images that way.
+ try (FileObject startup =
HopVfs.getFileObject(tmp.resolve("icon.svg").toUri().toString())) {
+ assertNotNull(startup);
+ }
+ assertFalse(
+ HopVfs.getFileSystemManager().hasProvider("project-connection"),
+ "Nothing pointed at the project metadata yet, so there is nothing to
find");
+
+ // Enabling the project hands us variables that do see its metadata.
+ IVariables projectVariables = Variables.getADefaultVariableSpace();
+ projectVariables.setVariable(PROJECT_MARKER, "true");
+
+ assertTrue(
+
HopVfs.getFileSystemManager(projectVariables).hasProvider("project-connection"),
+ "The named VFS connection of the project was never registered");
+
assertTrue(HopVfs.startsWithScheme("project-connection://container/file.txt"));
+ }
+
+ @Test
+ @DisplayName("A plugin without fixed schemes leaves no unclosed component
behind")
+ void schemelessPluginLeavesNothingUnclosed() {
+ // A provider registered under no scheme at all is adopted as a component
of the manager but
+ // never lands in its providers map, so close() can't reach it and
complains :
+ // "DefaultFilesystemManager.close: not all components are closed".
+ registerTestVfsPlugin("closes-clean", variables -> {});
+ HopVfs.setBootstrapVariables(Variables.getADefaultVariableSpace());
+
+ DefaultFileSystemManager manager = HopVfs.getFileSystemManager();
+ assertTrue(manager.hasProvider("closes-clean"));
+
+ Log log = mock(Log.class);
+ manager.setLogger(log);
+
+ HopVfs.reset();
+
+ ArgumentCaptor<Object> warnings = ArgumentCaptor.forClass(Object.class);
+ verify(log, atLeast(0)).warn(warnings.capture());
+ assertTrue(
+ warnings.getAllValues().stream()
+ .noneMatch(
+ warning -> String.valueOf(warning).contains("not all
components are closed")),
+ "The file system manager was left with components it can't close : "
+ + warnings.getAllValues());
+ }
+
+ /** Register a VFS plugin serving one named connection, and reset HopVfs so
it's picked up. */
+ private void registerTestVfsPlugin(String connectionName,
Consumer<IVariables> whileRegistering) {
+ registerTestVfsPlugin(connectionName, whileRegistering, variables -> true);
+ }
+
+ /**
+ * Register a VFS plugin serving one named connection, visible only to
variables accepted by
+ * {@code visibleTo} the way a connection is only visible to variables
pointing at the metadata
+ * folder holding it.
+ */
+ private void registerTestVfsPlugin(
+ String connectionName,
+ Consumer<IVariables> whileRegistering,
+ Predicate<IVariables> visibleTo) {
+ IVfs vfs =
+ new IVfs() {
+ @Override
+ public String[] getUrlSchemes() {
+ // Like the Minio and Databricks plugins: all schemes come from
the metadata.
+ return new String[] {};
+ }
+
+ @Override
+ public FileProvider getProvider() {
+ return new RamFileProvider();
+ }
+
+ @Override
+ public Map<String, FileProvider> getProviders(IVariables variables) {
+ whileRegistering.accept(variables);
+ Map<String, FileProvider> providers = new HashMap<>();
+ if (visibleTo.test(variables)) {
+ providers.put(connectionName, new RamFileProvider());
+ }
+ return providers;
+ }
+ };
+
+ IPluginMock plugin = mock(IPluginMock.class);
+ when(plugin.getIds()).thenReturn(new String[] {"test-vfs-plugin"});
+ when(plugin.getName()).thenReturn("Test VFS plugin");
+ when(plugin.getMainType()).thenReturn((Class) IVfs.class);
+ when(plugin.loadClass(IVfs.class)).thenReturn(vfs);
+
+ try {
+ PluginRegistry.getInstance().registerPlugin(VfsPluginType.class, plugin);
+ } catch (Exception e) {
+ throw new IllegalStateException("Unable to register the test VFS
plugin", e);
+ }
+ registeredPlugin = plugin;
+
+ // Forget any manager built before the test plugin existed.
+ HopVfs.reset();
+ }
+}
diff --git
a/plugins/misc/projects/src/main/java/org/apache/hop/projects/util/ProjectsUtil.java
b/plugins/misc/projects/src/main/java/org/apache/hop/projects/util/ProjectsUtil.java
index 64b24d775c..8fabc7cd80 100644
---
a/plugins/misc/projects/src/main/java/org/apache/hop/projects/util/ProjectsUtil.java
+++
b/plugins/misc/projects/src/main/java/org/apache/hop/projects/util/ProjectsUtil.java
@@ -133,6 +133,12 @@ public class ProjectsUtil {
project.setMetadataProvider(metadataProvider);
}
+ // The named VFS connections live in the metadata of this project, so hand
HopVfs the variables
+ // to find them with. This also resets the file system manager: the
providers of the previous
+ // project are gone and those of this one are registered the next time VFS
is used.
+ //
+ HopVfs.setBootstrapVariables(variables);
+
// We store the project in the namespace singleton (used mainly in the GUI)
//
HopNamespace.setNamespace(projectName);
diff --git
a/plugins/tech/databricks/src/main/java/org/apache/hop/vfs/databricks/DatabricksVfsPlugin.java
b/plugins/tech/databricks/src/main/java/org/apache/hop/vfs/databricks/DatabricksVfsPlugin.java
index dd6b7a4329..2df9328b9d 100644
---
a/plugins/tech/databricks/src/main/java/org/apache/hop/vfs/databricks/DatabricksVfsPlugin.java
+++
b/plugins/tech/databricks/src/main/java/org/apache/hop/vfs/databricks/DatabricksVfsPlugin.java
@@ -50,7 +50,7 @@ public class DatabricksVfsPlugin implements IVfs {
@Override
public FileProvider getProvider() {
- return new DatabricksFileProvider();
+ return null;
}
@Override
diff --git
a/plugins/tech/databricks/src/test/java/org/apache/hop/vfs/databricks/DatabricksVfsPluginTest.java
b/plugins/tech/databricks/src/test/java/org/apache/hop/vfs/databricks/DatabricksVfsPluginTest.java
index 120d4192c3..841f6747b2 100644
---
a/plugins/tech/databricks/src/test/java/org/apache/hop/vfs/databricks/DatabricksVfsPluginTest.java
+++
b/plugins/tech/databricks/src/test/java/org/apache/hop/vfs/databricks/DatabricksVfsPluginTest.java
@@ -19,6 +19,7 @@ package org.apache.hop.vfs.databricks;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.Map;
@@ -36,11 +37,11 @@ class DatabricksVfsPluginTest {
}
@Test
- void getProviderReturnsDatabricksProvider() {
+ void getProviderIsNullWithoutFixedSchemes() {
+ // Nothing to register a provider under, and a provider the manager has no
scheme for is never
+ // closed : "DefaultFilesystemManager.close: not all components are
closed".
DatabricksVfsPlugin plugin = new DatabricksVfsPlugin();
- FileProvider provider = plugin.getProvider();
- assertNotNull(provider);
- assertTrue(provider instanceof DatabricksFileProvider);
+ assertNull(plugin.getProvider());
}
@Test
diff --git
a/plugins/tech/minio/src/main/java/org/apache/hop/vfs/minio/MinioVfsPlugin.java
b/plugins/tech/minio/src/main/java/org/apache/hop/vfs/minio/MinioVfsPlugin.java
index 4dbba9ae6f..573437a110 100644
---
a/plugins/tech/minio/src/main/java/org/apache/hop/vfs/minio/MinioVfsPlugin.java
+++
b/plugins/tech/minio/src/main/java/org/apache/hop/vfs/minio/MinioVfsPlugin.java
@@ -39,7 +39,7 @@ public class MinioVfsPlugin implements IVfs {
@Override
public FileProvider getProvider() {
- return new MinioFileProvider();
+ return null;
}
@Override
diff --git
a/plugins/tech/minio/src/test/java/org/apache/hop/vfs/minio/MinioVfsPluginTest.java
b/plugins/tech/minio/src/test/java/org/apache/hop/vfs/minio/MinioVfsPluginTest.java
index 8edb68f7e3..4544f8f748 100644
---
a/plugins/tech/minio/src/test/java/org/apache/hop/vfs/minio/MinioVfsPluginTest.java
+++
b/plugins/tech/minio/src/test/java/org/apache/hop/vfs/minio/MinioVfsPluginTest.java
@@ -19,6 +19,7 @@ package org.apache.hop.vfs.minio;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.Map;
@@ -50,9 +51,10 @@ class MinioVfsPluginTest {
@Test
void testGetProvider() {
- FileProvider provider = plugin.getProvider();
- assertNotNull(provider, "Provider should not be null");
- assertTrue(provider instanceof MinioFileProvider, "Provider should be
MinioFileProvider");
+ // There are no fixed schemes, so there is nothing to register a provider
under. Handing one
+ // over anyway leaves it unreachable and unclosed on the file system
manager :
+ // "DefaultFilesystemManager.close: not all components are closed".
+ assertNull(plugin.getProvider(), "MinIO has no fixed scheme, so it has no
fixed provider");
}
@Test
@@ -81,17 +83,6 @@ class MinioVfsPluginTest {
assertNotNull(providers, "Providers map should not be null");
}
- @Test
- void testMultipleGetProviderCalls() {
- FileProvider provider1 = plugin.getProvider();
- FileProvider provider2 = plugin.getProvider();
-
- assertNotNull(provider1, "First provider should not be null");
- assertNotNull(provider2, "Second provider should not be null");
- assertTrue(provider1 instanceof MinioFileProvider, "Should be
MinioFileProvider");
- assertTrue(provider2 instanceof MinioFileProvider, "Should be
MinioFileProvider");
- }
-
@Test
void testPluginAnnotation() {
// Verify the plugin has the VfsPlugin annotation by checking class
annotations
@@ -122,15 +113,6 @@ class MinioVfsPluginTest {
assertNotNull(providers, "Should handle errors and return empty map");
}
- @Test
- void testProviderCreation() {
- // Test that provider is properly created each time
- FileProvider provider = plugin.getProvider();
-
- assertNotNull(provider, "Provider should be created");
- assertTrue(provider instanceof MinioFileProvider, "Should create
MinioFileProvider");
- }
-
@Test
void testGetProvidersReturnsMap() {
IVariables variables = new Variables();
