From 168cd1c4f5a407efdbc5f356a16c21b321270df6 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 10 Mar 2026 15:32:14 +0100 Subject: [PATCH 1/6] Only search directories, if pluginDir property is set and valid --- .../common/ClassLoaderFactory.java | 21 ++++--- .../common/ClassLoaderFactoryTest.java | 56 +++++++++++++++---- 2 files changed, 59 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java index 06abfb3..44f8614 100644 --- a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java +++ b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java @@ -18,7 +18,6 @@ class ClassLoaderFactory { private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderFactory.class); - private static final String USER_HOME = System.getProperty("user.home"); private static final String PLUGIN_DIR_KEY = "cryptomator.pluginDir"; private static final String JAR_SUFFIX = ".jar"; @@ -30,14 +29,20 @@ class ClassLoaderFactory { */ @Contract(value = "-> new", pure = true) public static URLClassLoader forPluginDir() { - String val = System.getProperty(PLUGIN_DIR_KEY, ""); - final Path p; - if (val.startsWith("~/")) { - p = Path.of(USER_HOME).resolve(val.substring(2)); - } else { - p = Path.of(val); + String val = System.getProperty(PLUGIN_DIR_KEY); + if (val == null) { + return URLClassLoader.newInstance(new URL[0]); + } + + try { + if (val.isBlank()) { + throw new IllegalArgumentException("Plugin dir path is blank"); + } + return forPluginDirWithPath(Path.of(val)); //Path.of might throw InvalidPathException + } catch (IllegalArgumentException e) { + LOG.debug("{} contains illegal value. Skipping plugin directory.", PLUGIN_DIR_KEY, e); + return URLClassLoader.newInstance(new URL[0]); } - return forPluginDirWithPath(p); } @VisibleForTesting diff --git a/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java b/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java index 2463966..94ec896 100644 --- a/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java +++ b/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java @@ -1,11 +1,16 @@ package org.cryptomator.integrations.common; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EmptySource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.MockedStatic; import org.mockito.Mockito; import java.io.ByteArrayInputStream; @@ -17,6 +22,9 @@ import java.util.Arrays; import java.util.Comparator; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; + public class ClassLoaderFactoryTest { @Nested @@ -91,21 +99,49 @@ public void testReadPluginDirFromSysProp() { } } - @Test - @DisplayName("read path from cryptomator.pluginDir and replace ~/ with user.home") - public void testReadPluginDirFromSysPropAndReplaceHome() { - var ucl = Mockito.mock(URLClassLoader.class, "ucl"); - var relPath = "~/there/will/be/plugins"; - var absPath = Path.of(System.getProperty("user.home")).resolve("there/will/be/plugins"); - try (var mockedClass = Mockito.mockStatic(ClassLoaderFactory.class)) { + @Nested + @DisplayName("when the system property contains invalid values") + public class InvalidSystemProperty { + + MockedStatic mockedClass; + + @BeforeEach + public void beforeEach() { + mockedClass = Mockito.mockStatic(ClassLoaderFactory.class); mockedClass.when(() -> ClassLoaderFactory.forPluginDir()).thenCallRealMethod(); - mockedClass.when(() -> ClassLoaderFactory.forPluginDirWithPath(absPath)).thenReturn(ucl); + mockedClass.when(() -> ClassLoaderFactory.forPluginDirWithPath(any())).thenReturn(null); + } - System.setProperty("cryptomator.pluginDir", relPath); + @AfterEach + public void afterEach() { + mockedClass.close(); + } + + + @Test + @DisplayName("Undefined cryptomator.pluginDir returns empty URLClassLoader") + public void testUndefinedSysProp() { + System.clearProperty("cryptomator.pluginDir"); var result = ClassLoaderFactory.forPluginDir(); - Assertions.assertSame(ucl, result); + mockedClass.verify(() -> ClassLoaderFactory.forPluginDirWithPath(any()), never()); + Assertions.assertNotNull(result); + Assertions.assertEquals(0, result.getURLs().length); } + + @ParameterizedTest + @DisplayName("Property cryptomator.pluginDir filled with blanks returns empty URLClassLoader") + @EmptySource + @ValueSource(strings = {"\t\t", " "}) + public void testBlankSysProp(String propValue) { + System.setProperty("cryptomator.pluginDir", propValue); + var result = ClassLoaderFactory.forPluginDir(); + + mockedClass.verify(() -> ClassLoaderFactory.forPluginDirWithPath(any()), never()); + Assertions.assertNotNull(result); + Assertions.assertEquals(0, result.getURLs().length); + } + } @Test From 8b5b3ce86abfa905e0acf6e5e17b6bc08085f3e9 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 10 Mar 2026 15:40:00 +0100 Subject: [PATCH 2/6] Cache URLClassLoader if created with system property --- .../common/ClassLoaderFactory.java | 18 ++++++++++++++++++ .../common/IntegrationsLoader.java | 4 ++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java index 44f8614..a332380 100644 --- a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java +++ b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java @@ -13,6 +13,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; class ClassLoaderFactory { @@ -20,6 +21,23 @@ class ClassLoaderFactory { private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderFactory.class); private static final String PLUGIN_DIR_KEY = "cryptomator.pluginDir"; private static final String JAR_SUFFIX = ".jar"; + private static final AtomicReference CLASSLOADER_CACHE = new AtomicReference<>(null); + + /** + * Attempts to find {@code .jar} files in the path specified in {@value #PLUGIN_DIR_KEY} system property. + * Returns the cached class loader instance. If no instance is cached, creates a new instance with {@link #forPluginDir()} and caches it. + * + * @return A URLClassLoader that is aware of all {@code .jar} files in the plugin dir + */ + @Contract(value = "-> _", pure = false) + public synchronized static URLClassLoader forPluginDirCached() { + var ucl = CLASSLOADER_CACHE.get(); + if (ucl == null) { + ucl = forPluginDir(); + CLASSLOADER_CACHE.set(ucl); + } + return ucl; + } /** * Attempts to find {@code .jar} files in the path specified in {@value #PLUGIN_DIR_KEY} system property. diff --git a/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java b/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java index c0f5116..917bd5b 100644 --- a/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java +++ b/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java @@ -44,7 +44,7 @@ public static Optional load(Class clazz) { * @param Type of the service */ public static Optional loadSpecific(Class clazz, String implementationClassName) { - return ServiceLoader.load(clazz, ClassLoaderFactory.forPluginDir()).stream() + return ServiceLoader.load(clazz, ClassLoaderFactory.forPluginDirCached()).stream() .filter(provider -> provider.type().getName().equals(implementationClassName)) .map(ServiceLoader.Provider::get) .findAny(); @@ -61,7 +61,7 @@ public static Optional loadSpecific(Class clazz, String implementation * @return An ordered stream of all suited service providers */ public static Stream loadAll(Class clazz) { - return loadAll(ServiceLoader.load(clazz, ClassLoaderFactory.forPluginDir()), clazz); + return loadAll(ServiceLoader.load(clazz, ClassLoaderFactory.forPluginDirCached()), clazz); } /** From 95cff98976a3dea8c31bbd15bf1d05528db28b1e Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 10 Mar 2026 15:41:06 +0100 Subject: [PATCH 3/6] improve logging --- .../cryptomator/integrations/common/ClassLoaderFactory.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java index a332380..08c034a 100644 --- a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java +++ b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java @@ -67,7 +67,7 @@ public static URLClassLoader forPluginDir() { @Contract(value = "_ -> new", pure = true) static URLClassLoader forPluginDirWithPath(Path path) throws UncheckedIOException { var jars = findJars(path); - if (LOG.isDebugEnabled()) { + if (LOG.isDebugEnabled() && jars.length != 0) { String jarList = Arrays.stream(jars).map(URL::getPath).collect(Collectors.joining(", ")); LOG.debug("Found jars in cryptomator.pluginDir: {}", jarList); } @@ -79,7 +79,7 @@ static URL[] findJars(Path path) { try (var stream = Files.walk(path)) { return stream.filter(ClassLoaderFactory::isJarFile).map(ClassLoaderFactory::toUrl).toArray(URL[]::new); } catch (IOException | UncheckedIOException e) { - // unable to locate any jars // TODO: log a warning? + LOG.debug("Failed to read plugin dir {}", path, e); return new URL[0]; } } From 3d11f8e760a9722c3a3d39cd115fc193c1079015 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 10 Mar 2026 15:53:59 +0100 Subject: [PATCH 4/6] simplify caching --- .../integrations/common/ClassLoaderFactory.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java index 08c034a..e58c160 100644 --- a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java +++ b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java @@ -13,7 +13,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; -import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; class ClassLoaderFactory { @@ -21,7 +20,8 @@ class ClassLoaderFactory { private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderFactory.class); private static final String PLUGIN_DIR_KEY = "cryptomator.pluginDir"; private static final String JAR_SUFFIX = ".jar"; - private static final AtomicReference CLASSLOADER_CACHE = new AtomicReference<>(null); + + private static URLClassLoader CACHED_CLASSLOADER = null; /** * Attempts to find {@code .jar} files in the path specified in {@value #PLUGIN_DIR_KEY} system property. @@ -31,12 +31,10 @@ class ClassLoaderFactory { */ @Contract(value = "-> _", pure = false) public synchronized static URLClassLoader forPluginDirCached() { - var ucl = CLASSLOADER_CACHE.get(); - if (ucl == null) { - ucl = forPluginDir(); - CLASSLOADER_CACHE.set(ucl); + if (CACHED_CLASSLOADER == null) { + CACHED_CLASSLOADER = forPluginDir(); } - return ucl; + return CACHED_CLASSLOADER; } /** From 5d1ddb81caf657df9b601e1f4503fe09b35a027c Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 11 Mar 2026 14:01:09 +0100 Subject: [PATCH 5/6] Return new ClassLoader if property changes --- .../common/ClassLoaderFactory.java | 20 ++++-- .../common/IntegrationsLoader.java | 4 +- .../common/ClassLoaderFactoryTest.java | 64 +++++++++++++++++++ 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java index e58c160..98bbea3 100644 --- a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java +++ b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java @@ -13,6 +13,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; +import java.util.Objects; import java.util.stream.Collectors; class ClassLoaderFactory { @@ -21,17 +22,24 @@ class ClassLoaderFactory { private static final String PLUGIN_DIR_KEY = "cryptomator.pluginDir"; private static final String JAR_SUFFIX = ".jar"; - private static URLClassLoader CACHED_CLASSLOADER = null; + @VisibleForTesting + static String CACHED_PLUGIN_DIR = null; + @VisibleForTesting + static URLClassLoader CACHED_CLASSLOADER = null; /** - * Attempts to find {@code .jar} files in the path specified in {@value #PLUGIN_DIR_KEY} system property. - * Returns the cached class loader instance. If no instance is cached, creates a new instance with {@link #forPluginDir()} and caches it. + * Returns the cached class loader instance. + *

+ * The returned instance does not recheck the pluginDir for updates. + * If no instance is cached or the system property changed, creates a new instance with {@link #forPluginDir()} and caches it. * - * @return A URLClassLoader that is aware of all {@code .jar} files in the plugin dir + * @return The cached URLClassLoader that is aware of all {@code .jar} files in the plugin dir at the creation time of the instance */ @Contract(value = "-> _", pure = false) - public synchronized static URLClassLoader forPluginDirCached() { - if (CACHED_CLASSLOADER == null) { + public synchronized static URLClassLoader forCachedPluginDir() { + String currentPluginDir = System.getProperty(PLUGIN_DIR_KEY); + if (CACHED_CLASSLOADER == null || !Objects.equals(CACHED_PLUGIN_DIR, currentPluginDir)) { + CACHED_PLUGIN_DIR = currentPluginDir; CACHED_CLASSLOADER = forPluginDir(); } return CACHED_CLASSLOADER; diff --git a/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java b/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java index 917bd5b..159574c 100644 --- a/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java +++ b/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java @@ -44,7 +44,7 @@ public static Optional load(Class clazz) { * @param Type of the service */ public static Optional loadSpecific(Class clazz, String implementationClassName) { - return ServiceLoader.load(clazz, ClassLoaderFactory.forPluginDirCached()).stream() + return ServiceLoader.load(clazz, ClassLoaderFactory.forCachedPluginDir()).stream() .filter(provider -> provider.type().getName().equals(implementationClassName)) .map(ServiceLoader.Provider::get) .findAny(); @@ -61,7 +61,7 @@ public static Optional loadSpecific(Class clazz, String implementation * @return An ordered stream of all suited service providers */ public static Stream loadAll(Class clazz) { - return loadAll(ServiceLoader.load(clazz, ClassLoaderFactory.forPluginDirCached()), clazz); + return loadAll(ServiceLoader.load(clazz, ClassLoaderFactory.forCachedPluginDir()), clazz); } /** diff --git a/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java b/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java index 94ec896..8c86499 100644 --- a/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java +++ b/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java @@ -144,6 +144,70 @@ public void testBlankSysProp(String propValue) { } + @Nested + @DisplayName("forCachedPluginDir()") + public class CachedPluginDir { + + MockedStatic mockedClass; + + @BeforeEach + public void beforeEach() { + ClassLoaderFactory.CACHED_PLUGIN_DIR = null; + ClassLoaderFactory.CACHED_CLASSLOADER = null; + System.clearProperty("cryptomator.pluginDir"); + mockedClass = Mockito.mockStatic(ClassLoaderFactory.class); + mockedClass.when(() -> ClassLoaderFactory.forCachedPluginDir()).thenCallRealMethod(); + } + + @AfterEach + public void afterEach() { + mockedClass.close(); + ClassLoaderFactory.CACHED_PLUGIN_DIR = null; + ClassLoaderFactory.CACHED_CLASSLOADER = null; + System.clearProperty("cryptomator.pluginDir"); + } + + @Test + @DisplayName("returns cached classloader on subsequent calls with same property") + public void testReturnsCachedInstance() { + var ucl = Mockito.mock(URLClassLoader.class, "ucl"); + mockedClass.when(() -> ClassLoaderFactory.forPluginDir()).thenReturn(ucl); + + System.setProperty("cryptomator.pluginDir", "/some/path"); + var first = ClassLoaderFactory.forCachedPluginDir(); + var second = ClassLoaderFactory.forCachedPluginDir(); + + Assertions.assertSame(first, second); + mockedClass.verify(() -> ClassLoaderFactory.forPluginDir(), Mockito.times(1)); + } + + @Test + @DisplayName("creates new classloader when property changes") + public void testInvalidatesCacheOnPropertyChange() { + var ucl1 = Mockito.mock(URLClassLoader.class, "ucl1"); + var ucl2 = Mockito.mock(URLClassLoader.class, "ucl2"); + var ucl3 = Mockito.mock(URLClassLoader.class, "ucl3"); + mockedClass.when(() -> ClassLoaderFactory.forPluginDir()).thenReturn(ucl1, ucl2, ucl3); + + System.setProperty("cryptomator.pluginDir", "/path/one"); + var first = ClassLoaderFactory.forCachedPluginDir(); + + System.setProperty("cryptomator.pluginDir", "/path/two"); + var second = ClassLoaderFactory.forCachedPluginDir(); + + System.clearProperty("cryptomator.pluginDir"); + var third = ClassLoaderFactory.forCachedPluginDir(); + + Assertions.assertSame(ucl1, first); + Assertions.assertSame(ucl2, second); + Assertions.assertSame(ucl3, third); + Assertions.assertNotSame(first, second); + Assertions.assertNotSame(second, third); + Assertions.assertNotSame(first, third); + mockedClass.verify(() -> ClassLoaderFactory.forPluginDir(), Mockito.times(3)); + } + } + @Test @DisplayName("findJars returns empty list if not containing jars") public void testFindJars1(@TempDir Path tmpDir) throws IOException { From 2613fc584070fd4a236d46aa4a5942715045e205 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 11 Mar 2026 14:57:15 +0100 Subject: [PATCH 6/6] Rework cache impl * rename method to fit better the naming scheme * use cache record to model the connection propertyValue, classloader * adjust unit tests --- .../common/ClassLoaderFactory.java | 24 ++++--- .../common/IntegrationsLoader.java | 4 +- .../common/ClassLoaderFactoryTest.java | 63 +++++++++---------- 3 files changed, 47 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java index 98bbea3..11fc9a6 100644 --- a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java +++ b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java @@ -1,6 +1,7 @@ package org.cryptomator.integrations.common; import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -22,10 +23,10 @@ class ClassLoaderFactory { private static final String PLUGIN_DIR_KEY = "cryptomator.pluginDir"; private static final String JAR_SUFFIX = ".jar"; - @VisibleForTesting - static String CACHED_PLUGIN_DIR = null; - @VisibleForTesting - static URLClassLoader CACHED_CLASSLOADER = null; + static Cache CACHE = null; + + record Cache(@Nullable String pluginDir, URLClassLoader classLoader) { + } /** * Returns the cached class loader instance. @@ -36,13 +37,12 @@ class ClassLoaderFactory { * @return The cached URLClassLoader that is aware of all {@code .jar} files in the plugin dir at the creation time of the instance */ @Contract(value = "-> _", pure = false) - public synchronized static URLClassLoader forCachedPluginDir() { + public synchronized static URLClassLoader forPluginDirFromCache() { String currentPluginDir = System.getProperty(PLUGIN_DIR_KEY); - if (CACHED_CLASSLOADER == null || !Objects.equals(CACHED_PLUGIN_DIR, currentPluginDir)) { - CACHED_PLUGIN_DIR = currentPluginDir; - CACHED_CLASSLOADER = forPluginDir(); + if (CACHE == null || !Objects.equals(CACHE.pluginDir, currentPluginDir)) { + CACHE = new Cache(currentPluginDir, forPluginDirInternal(currentPluginDir)); } - return CACHED_CLASSLOADER; + return CACHE.classLoader; } /** @@ -54,6 +54,12 @@ public synchronized static URLClassLoader forCachedPluginDir() { @Contract(value = "-> new", pure = true) public static URLClassLoader forPluginDir() { String val = System.getProperty(PLUGIN_DIR_KEY); + return forPluginDirInternal(val); + } + + @VisibleForTesting + @Contract(value = "_ -> new", pure = true) + static URLClassLoader forPluginDirInternal(@Nullable String val) { if (val == null) { return URLClassLoader.newInstance(new URL[0]); } diff --git a/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java b/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java index 159574c..b863cb9 100644 --- a/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java +++ b/src/main/java/org/cryptomator/integrations/common/IntegrationsLoader.java @@ -44,7 +44,7 @@ public static Optional load(Class clazz) { * @param Type of the service */ public static Optional loadSpecific(Class clazz, String implementationClassName) { - return ServiceLoader.load(clazz, ClassLoaderFactory.forCachedPluginDir()).stream() + return ServiceLoader.load(clazz, ClassLoaderFactory.forPluginDirFromCache()).stream() .filter(provider -> provider.type().getName().equals(implementationClassName)) .map(ServiceLoader.Provider::get) .findAny(); @@ -61,7 +61,7 @@ public static Optional loadSpecific(Class clazz, String implementation * @return An ordered stream of all suited service providers */ public static Stream loadAll(Class clazz) { - return loadAll(ServiceLoader.load(clazz, ClassLoaderFactory.forCachedPluginDir()), clazz); + return loadAll(ServiceLoader.load(clazz, ClassLoaderFactory.forPluginDirFromCache()), clazz); } /** diff --git a/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java b/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java index 8c86499..4d38819 100644 --- a/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java +++ b/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java @@ -9,6 +9,7 @@ import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EmptySource; +import org.junit.jupiter.params.provider.NullSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.MockedStatic; import org.mockito.Mockito; @@ -90,6 +91,7 @@ public void testReadPluginDirFromSysProp() { var absPath = "/there/will/be/plugins"; try (var mockedClass = Mockito.mockStatic(ClassLoaderFactory.class)) { mockedClass.when(() -> ClassLoaderFactory.forPluginDir()).thenCallRealMethod(); + mockedClass.when(() -> ClassLoaderFactory.forPluginDirInternal(any())).thenCallRealMethod(); mockedClass.when(() -> ClassLoaderFactory.forPluginDirWithPath(Path.of(absPath))).thenReturn(ucl); System.setProperty("cryptomator.pluginDir", absPath); @@ -100,16 +102,14 @@ public void testReadPluginDirFromSysProp() { } @Nested - @DisplayName("when the system property contains invalid values") - public class InvalidSystemProperty { - + @DisplayName("Method pluginDirInternal") + public class PluginDirInternal { MockedStatic mockedClass; @BeforeEach public void beforeEach() { mockedClass = Mockito.mockStatic(ClassLoaderFactory.class); - mockedClass.when(() -> ClassLoaderFactory.forPluginDir()).thenCallRealMethod(); - mockedClass.when(() -> ClassLoaderFactory.forPluginDirWithPath(any())).thenReturn(null); + mockedClass.when(() -> ClassLoaderFactory.forPluginDirInternal(any())).thenCallRealMethod(); } @AfterEach @@ -117,25 +117,24 @@ public void afterEach() { mockedClass.close(); } - @Test - @DisplayName("Undefined cryptomator.pluginDir returns empty URLClassLoader") - public void testUndefinedSysProp() { - System.clearProperty("cryptomator.pluginDir"); - var result = ClassLoaderFactory.forPluginDir(); + @DisplayName("Valid path string calls forPluginDirWithPath") + public void testValidPathString() { + var ucl = Mockito.mock(URLClassLoader.class, "ucl"); + mockedClass.when(() -> ClassLoaderFactory.forPluginDirWithPath(any())).thenReturn(ucl); - mockedClass.verify(() -> ClassLoaderFactory.forPluginDirWithPath(any()), never()); - Assertions.assertNotNull(result); - Assertions.assertEquals(0, result.getURLs().length); + var result = ClassLoaderFactory.forPluginDirInternal("some/string"); + mockedClass.verify(() -> ClassLoaderFactory.forPluginDirWithPath(any())); + Assertions.assertSame(ucl, result); } @ParameterizedTest - @DisplayName("Property cryptomator.pluginDir filled with blanks returns empty URLClassLoader") + @DisplayName("Invalid or null path strings do not call forPluginDirWithPath") @EmptySource @ValueSource(strings = {"\t\t", " "}) - public void testBlankSysProp(String propValue) { - System.setProperty("cryptomator.pluginDir", propValue); - var result = ClassLoaderFactory.forPluginDir(); + @NullSource + public void testInvalidPathString(String propValue) { + var result = ClassLoaderFactory.forPluginDirInternal(propValue); mockedClass.verify(() -> ClassLoaderFactory.forPluginDirWithPath(any()), never()); Assertions.assertNotNull(result); @@ -145,25 +144,23 @@ public void testBlankSysProp(String propValue) { } @Nested - @DisplayName("forCachedPluginDir()") - public class CachedPluginDir { + @DisplayName("Method pluginDirFromCache") + public class PluginDirFromCache { MockedStatic mockedClass; @BeforeEach public void beforeEach() { - ClassLoaderFactory.CACHED_PLUGIN_DIR = null; - ClassLoaderFactory.CACHED_CLASSLOADER = null; + ClassLoaderFactory.CACHE = null; System.clearProperty("cryptomator.pluginDir"); mockedClass = Mockito.mockStatic(ClassLoaderFactory.class); - mockedClass.when(() -> ClassLoaderFactory.forCachedPluginDir()).thenCallRealMethod(); + mockedClass.when(() -> ClassLoaderFactory.forPluginDirFromCache()).thenCallRealMethod(); } @AfterEach public void afterEach() { mockedClass.close(); - ClassLoaderFactory.CACHED_PLUGIN_DIR = null; - ClassLoaderFactory.CACHED_CLASSLOADER = null; + ClassLoaderFactory.CACHE = null; System.clearProperty("cryptomator.pluginDir"); } @@ -171,14 +168,14 @@ public void afterEach() { @DisplayName("returns cached classloader on subsequent calls with same property") public void testReturnsCachedInstance() { var ucl = Mockito.mock(URLClassLoader.class, "ucl"); - mockedClass.when(() -> ClassLoaderFactory.forPluginDir()).thenReturn(ucl); + mockedClass.when(() -> ClassLoaderFactory.forPluginDirInternal(any())).thenReturn(ucl); System.setProperty("cryptomator.pluginDir", "/some/path"); - var first = ClassLoaderFactory.forCachedPluginDir(); - var second = ClassLoaderFactory.forCachedPluginDir(); + var first = ClassLoaderFactory.forPluginDirFromCache(); + var second = ClassLoaderFactory.forPluginDirFromCache(); Assertions.assertSame(first, second); - mockedClass.verify(() -> ClassLoaderFactory.forPluginDir(), Mockito.times(1)); + mockedClass.verify(() -> ClassLoaderFactory.forPluginDirInternal("/some/path"), Mockito.times(1)); } @Test @@ -187,16 +184,16 @@ public void testInvalidatesCacheOnPropertyChange() { var ucl1 = Mockito.mock(URLClassLoader.class, "ucl1"); var ucl2 = Mockito.mock(URLClassLoader.class, "ucl2"); var ucl3 = Mockito.mock(URLClassLoader.class, "ucl3"); - mockedClass.when(() -> ClassLoaderFactory.forPluginDir()).thenReturn(ucl1, ucl2, ucl3); + mockedClass.when(() -> ClassLoaderFactory.forPluginDirInternal(any())).thenReturn(ucl1, ucl2, ucl3); System.setProperty("cryptomator.pluginDir", "/path/one"); - var first = ClassLoaderFactory.forCachedPluginDir(); + var first = ClassLoaderFactory.forPluginDirFromCache(); System.setProperty("cryptomator.pluginDir", "/path/two"); - var second = ClassLoaderFactory.forCachedPluginDir(); + var second = ClassLoaderFactory.forPluginDirFromCache(); System.clearProperty("cryptomator.pluginDir"); - var third = ClassLoaderFactory.forCachedPluginDir(); + var third = ClassLoaderFactory.forPluginDirFromCache(); Assertions.assertSame(ucl1, first); Assertions.assertSame(ucl2, second); @@ -204,7 +201,7 @@ public void testInvalidatesCacheOnPropertyChange() { Assertions.assertNotSame(first, second); Assertions.assertNotSame(second, third); Assertions.assertNotSame(first, third); - mockedClass.verify(() -> ClassLoaderFactory.forPluginDir(), Mockito.times(3)); + mockedClass.verify(() -> ClassLoaderFactory.forPluginDirInternal(any()), Mockito.times(3)); } }