diff --git a/app.yml b/app.yml index c21b3e7..4fac823 100644 --- a/app.yml +++ b/app.yml @@ -34,5 +34,6 @@ actions: build: ./mvnw spotless:apply package -DskipTests run: ./target/binary/bin/jpm test: ./mvnw test + format: ./mvnw spotless:apply buildj: javac -cp {{deps}} -d classes --source-path src/main/java src/main/java/org/codejive/jpm/Main.java runj: java -cp classes:{{deps}} org.codejive.jpm.Main diff --git a/src/main/java/org/codejive/jpm/Main.java b/src/main/java/org/codejive/jpm/Main.java index 437fedd..16c1be0 100644 --- a/src/main/java/org/codejive/jpm/Main.java +++ b/src/main/java/org/codejive/jpm/Main.java @@ -16,6 +16,7 @@ import java.io.*; import java.net.MalformedURLException; import java.net.URL; +import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.util.*; @@ -23,6 +24,7 @@ import java.util.stream.Collectors; import org.codejive.jpm.config.UserConfig; import org.codejive.jpm.search.Search.Backends; +import org.codejive.jpm.util.FileUtils; import org.codejive.jpm.util.SyncResult; import org.codejive.jpm.util.Version; import org.jline.consoleui.elements.InputValue; @@ -612,13 +614,15 @@ String actionName() { } static class DepsMixin { + private static final String DEFAULT_DIRECTORY = "deps"; + // Cached user configuration loaded from ~/.config/jpm/config.yml or ~/.jpmcfg.yml private transient UserConfig userConfig; @Option( names = {"-d", "--directory"}, description = "Directory to copy artifacts to (default: 'deps')") - Path directory; + String directory; @Option( names = {"-L", "--no-links"}, @@ -636,7 +640,7 @@ static class DepsMixin { names = {"-c", "--cache"}, description = "Directory where downloaded artifacts will be cached (default: value of JPM_CACHE environment variable; whatever is set in Maven's settings.xml or $HOME/.m2/repository") - Path cacheDir; + String cacheDir; /** * Loads and caches the user configuration. Priority: --config option > JPM_CONFIG env var > @@ -658,21 +662,19 @@ UserConfig getUserConfig() { * @return The cache directory path or null to use Maven default */ Path getCacheDir() { - if (cacheDir != null) { - return cacheDir; + String dir = cacheDir; + if (dir == null) { + dir = getUserConfig().cache(); } - Path userConfigCache = getUserConfig().cache(); - if (userConfigCache != null) { - return userConfigCache; + if (dir == null) { + dir = System.getenv("JPM_CACHE"); } - String envCache = System.getenv("JPM_CACHE"); - if (envCache != null && !envCache.isEmpty()) { + if (dir != null && !dir.isEmpty()) { try { - return Path.of(envCache); + return FileUtils.expandHomePath(dir); } catch (InvalidPathException e) { System.err.println( - "Warning: Invalid path in JPM_CACHE environment variable, ignoring: " - + envCache); + "Warning: The specified cache path is invalid, ignoring: " + dir); } } return null; @@ -685,14 +687,28 @@ Path getCacheDir() { * @return The directory path */ Path getDirectory() { - if (directory != null) { - return directory; // User explicitly set via CLI + String dir = directory; + if (dir == null) { + dir = getUserConfig().directory(); + } + if (dir == null) { + dir = DEFAULT_DIRECTORY; + } + if (dir == null || dir.isEmpty()) { + return null; + } + Path result; + try { + result = FileUtils.expandHomePath(dir); + } catch (InvalidPathException e) { + throw new IllegalArgumentException( + "The specified output directory path is invalid: " + dir); } - Path userConfigDir = getUserConfig().directory(); - if (userConfigDir != null) { - return userConfigDir; // From user config + if (Files.isRegularFile(result)) { + throw new IllegalArgumentException( + "The specified output directory is a file, not a directory: " + dir); } - return Path.of("deps"); // Hardcoded default + return result; } /** @@ -812,7 +828,7 @@ private static void printStats(SyncResult stats) { static CommandLine.IExecutionExceptionHandler errorHandler = (ex, commandLine, parseResult) -> { - System.err.println("Error: " + ex.getMessage()); + System.err.println("Error: " + ex); if (verbose) { ex.printStackTrace(); } else { diff --git a/src/main/java/org/codejive/jpm/config/UserConfig.java b/src/main/java/org/codejive/jpm/config/UserConfig.java index e2aa8d7..40af344 100644 --- a/src/main/java/org/codejive/jpm/config/UserConfig.java +++ b/src/main/java/org/codejive/jpm/config/UserConfig.java @@ -15,8 +15,8 @@ * options. */ public class UserConfig { - private Path cache; - private Path directory; + private String cache; + private String directory; private Boolean noLinks; private final Map repositories = new LinkedHashMap<>(); @@ -26,11 +26,11 @@ public class UserConfig { /** The fallback user config file location. */ public static final String USER_CONFIG_FILE_FALLBACK = ".jpmcfg.yml"; - public Path cache() { + public String cache() { return cache; } - public Path directory() { + public String directory() { return directory; } @@ -152,8 +152,7 @@ private static UserConfig readFromReader(Reader in) { if (config.containsKey("cache")) { Object cacheObj = config.get("cache"); if (cacheObj instanceof String) { - String cachePath = (String) cacheObj; - userConfig.cache = expandHomePath(cachePath); + userConfig.cache = (String) cacheObj; } else { System.err.println( "Warning: 'cache' must be a string path, ignoring: " + cacheObj); @@ -164,8 +163,7 @@ private static UserConfig readFromReader(Reader in) { if (config.containsKey("directory")) { Object dirObj = config.get("directory"); if (dirObj instanceof String) { - String dirPath = (String) dirObj; - userConfig.directory = expandHomePath(dirPath); + userConfig.directory = (String) dirObj; } else { System.err.println( "Warning: 'directory' must be a string path, ignoring: " + dirObj); @@ -207,19 +205,4 @@ private static UserConfig readFromReader(Reader in) { return userConfig; } - - /** - * Expands a path starting with "~/" to use the user's home directory. Handles cross-platform - * paths. - * - * @param pathStr The path string to expand - * @return A Path with "~/" expanded to the user's home directory - */ - private static Path expandHomePath(String pathStr) { - if (pathStr.startsWith("~/")) { - String userHome = System.getProperty("user.home"); - return Paths.get(userHome, pathStr.substring(2)); - } - return Paths.get(pathStr); - } } diff --git a/src/main/java/org/codejive/jpm/util/FileUtils.java b/src/main/java/org/codejive/jpm/util/FileUtils.java index 925053d..93c172e 100644 --- a/src/main/java/org/codejive/jpm/util/FileUtils.java +++ b/src/main/java/org/codejive/jpm/util/FileUtils.java @@ -105,4 +105,18 @@ public static Path safePath(String w) { return null; } } + + /** + * Expands a path starting with "~/" to use the user's home directory. + * + * @param pathStr The path string to expand + * @return A Path with "~/" expanded to the user's home directory + */ + public static Path expandHomePath(String pathStr) { + if (pathStr.startsWith("~/")) { + String userHome = System.getProperty("user.home"); + return Paths.get(userHome, pathStr.substring(2)); + } + return Paths.get(pathStr); + } } diff --git a/src/test/java/org/codejive/jpm/MainCacheOptionsTest.java b/src/test/java/org/codejive/jpm/MainCacheOptionsTest.java index 1e773b0..6233d9a 100644 --- a/src/test/java/org/codejive/jpm/MainCacheOptionsTest.java +++ b/src/test/java/org/codejive/jpm/MainCacheOptionsTest.java @@ -3,8 +3,11 @@ import static org.assertj.core.api.Assertions.*; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.PrintStream; +import java.nio.file.Files; import java.nio.file.Path; +import org.codejive.jpm.config.UserConfig; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -18,6 +21,7 @@ class MainCacheOptionsTest { @TempDir Path tempCacheDir1; @TempDir Path tempCacheDir2; + @TempDir Path tempDir; private PrintStream originalErr; private ByteArrayOutputStream errContent; @@ -39,7 +43,7 @@ void tearDown() { @ClearEnvironmentVariable(key = "JPM_CACHE") void testGetCacheDirWithCommandLineOption() { Main.DepsMixin mixin = new Main.DepsMixin(); - mixin.cacheDir = tempCacheDir1; + mixin.cacheDir = tempCacheDir1.toString(); Path result = mixin.getCacheDir(); @@ -65,7 +69,7 @@ void testGetCacheDirFromEnvironmentVariable() { @SetEnvironmentVariable(key = "JPM_CACHE", value = "/tmp/jpm-cache-from-env") void testCommandLineOptionTakesPrecedenceOverEnvironmentVariable() { Main.DepsMixin mixin = new Main.DepsMixin(); - mixin.cacheDir = tempCacheDir1; // Command line option is set + mixin.cacheDir = tempCacheDir1.toString(); // Command line option is set Path result = mixin.getCacheDir(); @@ -101,7 +105,7 @@ void testGetCacheDirWithEmptyEnvironmentVariable() { @ClearEnvironmentVariable(key = "JPM_CACHE") void testGetCacheDirMultipleCalls() { Main.DepsMixin mixin = new Main.DepsMixin(); - mixin.cacheDir = tempCacheDir1; + mixin.cacheDir = tempCacheDir1.toString(); // Call multiple times to ensure consistent behavior Path result1 = mixin.getCacheDir(); @@ -112,4 +116,147 @@ void testGetCacheDirMultipleCalls() { assertThat(result1).isEqualTo(result2); assertThat(errContent.toString()).isEmpty(); // No warnings } + + @Test + @ClearEnvironmentVariable(key = "JPM_CACHE") + void testGetCacheDirWithInvalidPath() { + Main.DepsMixin mixin = new Main.DepsMixin(); + // Use a path with null bytes which is invalid on most filesystems + mixin.cacheDir = "/invalid\0path/cache"; + + Path result = mixin.getCacheDir(); + + // Should return null when path is invalid + assertThat(result).isNull(); + // Should print warning to stderr + assertThat(errContent.toString()) + .contains("Warning: The specified cache path is invalid, ignoring:"); + } + + @Test + @ClearEnvironmentVariable(key = "JPM_CACHE") + void testGetCacheDirWithHomePathExpansion() { + Main.DepsMixin mixin = new Main.DepsMixin(); + mixin.cacheDir = "~/my-cache"; + + Path result = mixin.getCacheDir(); + + // Should expand ~/ to user home + assertThat(result).isNotNull(); + String userHome = System.getProperty("user.home"); + assertThat(result.toString()).isEqualTo(Path.of(userHome, "my-cache").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + @SetEnvironmentVariable(key = "JPM_CACHE", value = "~/env-cache") + void testGetCacheDirWithHomePathExpansionFromEnv() { + Main.DepsMixin mixin = new Main.DepsMixin(); + // Don't set cacheDir - should use env var + + Path result = mixin.getCacheDir(); + + // Should expand ~/ in environment variable + assertThat(result).isNotNull(); + String userHome = System.getProperty("user.home"); + assertThat(result.toString()).isEqualTo(Path.of(userHome, "env-cache").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + @ClearEnvironmentVariable(key = "JPM_CACHE") + void testGetCacheDirWithUserConfig() throws IOException { + // Create a temporary config file with cache setting + Path configFile = tempDir.resolve("config.yml"); + Files.writeString(configFile, "config:\n cache: /tmp/cache-from-config\n"); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + + Path result = mixin.getCacheDir(); + + // Should use the cache from UserConfig + assertThat(result).isNotNull(); + assertThat(result.toString()).isEqualTo(Path.of("/tmp/cache-from-config").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + @ClearEnvironmentVariable(key = "JPM_CACHE") + void testGetCacheDirCliOptionOverridesUserConfig() throws IOException { + // Create a temporary config file with cache setting + Path configFile = tempDir.resolve("config.yml"); + Files.writeString(configFile, "config:\n cache: /tmp/cache-from-config\n"); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + mixin.cacheDir = tempCacheDir1.toString(); // CLI option should take precedence + + Path result = mixin.getCacheDir(); + + // Should use CLI option, not UserConfig + assertThat(result).isEqualTo(tempCacheDir1); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + @SetEnvironmentVariable(key = "JPM_CACHE", value = "/tmp/cache-from-env") + void testGetCacheDirUserConfigOverridesEnv() throws IOException { + // Create a temporary config file with cache setting + Path configFile = tempDir.resolve("config.yml"); + Files.writeString(configFile, "config:\n cache: /tmp/cache-from-config\n"); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + + Path result = mixin.getCacheDir(); + + // UserConfig should override environment variable + assertThat(result).isNotNull(); + assertThat(result.toString()).isEqualTo(Path.of("/tmp/cache-from-config").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + @ClearEnvironmentVariable(key = "JPM_CACHE") + void testGetCacheDirWithInvalidPathFromUserConfig() throws IOException { + // Create a temporary config file with invalid cache path + Path configFile = tempDir.resolve("config.yml"); + Files.writeString(configFile, "config:\n cache: \"/invalid\\0path/cache\"\n"); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + + Path result = mixin.getCacheDir(); + + // Should return null when path from UserConfig is invalid + assertThat(result).isNull(); + // Should print warning to stderr + assertThat(errContent.toString()) + .contains("Warning: The specified cache path is invalid, ignoring:"); + } } diff --git a/src/test/java/org/codejive/jpm/MainDirectoryOptionsTest.java b/src/test/java/org/codejive/jpm/MainDirectoryOptionsTest.java new file mode 100644 index 0000000..25f0687 --- /dev/null +++ b/src/test/java/org/codejive/jpm/MainDirectoryOptionsTest.java @@ -0,0 +1,294 @@ +package org.codejive.jpm; + +import static org.assertj.core.api.Assertions.*; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.nio.file.Files; +import java.nio.file.Path; +import org.codejive.jpm.config.UserConfig; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junitpioneer.jupiter.SetEnvironmentVariable; + +/** Tests for Main class CLI directory option parsing. */ +@SetEnvironmentVariable(key = "JPM_CONFIG", value = "/nonexistent/config.yml") +class MainDirectoryOptionsTest { + + @TempDir Path tempDir; + + private PrintStream originalErr; + private ByteArrayOutputStream errContent; + + @BeforeEach + void setUp() { + // Capture stderr to verify warning messages + originalErr = System.err; + errContent = new ByteArrayOutputStream(); + System.setErr(new PrintStream(errContent)); + } + + @AfterEach + void tearDown() { + System.setErr(originalErr); + } + + @Test + void testGetDirectoryWithCommandLineOption() { + Main.DepsMixin mixin = new Main.DepsMixin(); + mixin.directory = "/tmp/my-deps"; + + Path result = mixin.getDirectory(); + + assertThat(result).isNotNull(); + assertThat(result.toString()).isEqualTo(Path.of("/tmp/my-deps").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryWithDefault() { + Main.DepsMixin mixin = new Main.DepsMixin(); + // Don't set directory - should use default 'deps' + + Path result = mixin.getDirectory(); + + assertThat(result).isNotNull(); + assertThat(result.toString()).isEqualTo(Path.of("deps").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryWithUserConfig() throws IOException { + // Create a temporary config file with directory setting + Path configFile = tempDir.resolve("config.yml"); + Files.writeString(configFile, "config:\n directory: /tmp/dir-from-config\n"); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + + Path result = mixin.getDirectory(); + + // Should use the directory from UserConfig + assertThat(result).isNotNull(); + assertThat(result.toString()).isEqualTo(Path.of("/tmp/dir-from-config").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryCliOptionOverridesUserConfig() throws IOException { + // Create a temporary config file with directory setting + Path configFile = tempDir.resolve("config.yml"); + Files.writeString(configFile, "config:\n directory: /tmp/dir-from-config\n"); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + mixin.directory = "/tmp/cli-dir"; // CLI option should take precedence + + Path result = mixin.getDirectory(); + + // Should use CLI option, not UserConfig + assertThat(result).isNotNull(); + assertThat(result.toString()).isEqualTo(Path.of("/tmp/cli-dir").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryUserConfigOverridesDefault() throws IOException { + // Create a temporary config file with directory setting + Path configFile = tempDir.resolve("config.yml"); + Files.writeString(configFile, "config:\n directory: /tmp/dir-from-config\n"); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + + Path result = mixin.getDirectory(); + + // UserConfig should override default value + assertThat(result).isNotNull(); + assertThat(result.toString()).isEqualTo(Path.of("/tmp/dir-from-config").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryWithInvalidPath() { + Main.DepsMixin mixin = new Main.DepsMixin(); + // Use a path with null bytes which is invalid on most filesystems + mixin.directory = "/invalid\0path/deps"; + + assertThatThrownBy(() -> mixin.getDirectory()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("The specified output directory path is invalid:"); + } + + @Test + void testGetDirectoryWithEmptyString() { + Main.DepsMixin mixin = new Main.DepsMixin(); + mixin.directory = ""; + + Path result = mixin.getDirectory(); + + // Empty string should return null + assertThat(result).isNull(); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryWithHomePathExpansion() { + Main.DepsMixin mixin = new Main.DepsMixin(); + mixin.directory = "~/my-deps"; + + Path result = mixin.getDirectory(); + + // Should expand ~/ to user home + assertThat(result).isNotNull(); + String userHome = System.getProperty("user.home"); + assertThat(result.toString()).isEqualTo(Path.of(userHome, "my-deps").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryWithHomePathExpansionFromUserConfig() throws IOException { + // Create a temporary config file with directory containing home path + Path configFile = tempDir.resolve("config.yml"); + Files.writeString(configFile, "config:\n directory: ~/config-deps\n"); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + + Path result = mixin.getDirectory(); + + // Should expand ~/ in UserConfig value + assertThat(result).isNotNull(); + String userHome = System.getProperty("user.home"); + assertThat(result.toString()).isEqualTo(Path.of(userHome, "config-deps").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryWhenPathIsFile() throws IOException { + // Create a regular file + Path regularFile = tempDir.resolve("not-a-directory.txt"); + Files.writeString(regularFile, "test content"); + + Main.DepsMixin mixin = new Main.DepsMixin(); + mixin.directory = regularFile.toString(); + + assertThatThrownBy(() -> mixin.getDirectory()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("The specified output directory is a file, not a directory:"); + } + + @Test + void testGetDirectoryWhenPathIsFileFromUserConfig() throws IOException { + // Create a regular file + Path regularFile = tempDir.resolve("not-a-directory.txt"); + Files.writeString(regularFile, "test content"); + + // Create a temporary config file with directory pointing to a file + Path configFile = tempDir.resolve("config.yml"); + Files.writeString( + configFile, String.format("config:\n directory: %s\n", regularFile.toString())); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + + assertThatThrownBy(() -> mixin.getDirectory()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("The specified output directory is a file, not a directory:"); + } + + @Test + void testGetDirectoryWithInvalidPathFromUserConfig() throws IOException { + // Create a temporary config file with invalid directory path + Path configFile = tempDir.resolve("config.yml"); + Files.writeString(configFile, "config:\n directory: \"/invalid\\0path/deps\"\n"); + + // Use a test subclass to inject custom UserConfig + Main.DepsMixin mixin = + new Main.DepsMixin() { + @Override + UserConfig getUserConfig() { + return UserConfig.read(configFile); + } + }; + + // Should throw IllegalArgumentException for invalid path from UserConfig + assertThatThrownBy(() -> mixin.getDirectory()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("The specified output directory path is invalid:"); + } + + @Test + void testGetDirectoryMultipleCalls() { + Main.DepsMixin mixin = new Main.DepsMixin(); + mixin.directory = "/tmp/my-deps"; + + // Call multiple times to ensure consistent behavior + Path result1 = mixin.getDirectory(); + Path result2 = mixin.getDirectory(); + + assertThat(result1).isNotNull(); + assertThat(result2).isNotNull(); + assertThat(result1.toString()).isEqualTo(result2.toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryWithRelativePath() { + Main.DepsMixin mixin = new Main.DepsMixin(); + mixin.directory = "my-deps"; + + Path result = mixin.getDirectory(); + + // Should accept relative paths + assertThat(result).isNotNull(); + assertThat(result.toString()).isEqualTo(Path.of("my-deps").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } + + @Test + void testGetDirectoryWithAbsolutePath() { + Main.DepsMixin mixin = new Main.DepsMixin(); + mixin.directory = "/tmp/absolute-deps"; + + Path result = mixin.getDirectory(); + + // Should accept absolute paths + assertThat(result).isNotNull(); + assertThat(result.toString()).isEqualTo(Path.of("/tmp/absolute-deps").toString()); + assertThat(errContent.toString()).isEmpty(); // No warnings + } +}