More Refactoring

mhalbritter · Feb 29, 2024 · 0c04fa0 · 0c04fa0
1 parent 6a26e23
commit 0c04fa0
Show file tree

Hide file tree

Showing 17 changed files with 129 additions and 42 deletions.
diff --git a/...jarmode-layertools/src/main/java/org/springframework/boot/jarmode/layertools/Command.java b/...jarmode-layertools/src/main/java/org/springframework/boot/jarmode/layertools/Command.java
@@ -16,6 +16,7 @@
 
 package org.springframework.boot.jarmode.layertools;
 
+import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -32,6 +33,7 @@
  *
  * @author Phillip Webb
  * @author Scott Frederick
+ * @author Moritz Halbritter
  */
 abstract class Command {
 
@@ -91,9 +93,10 @@ Parameters getParameters() {
 
 	/**
 	 * Run the command by processing the remaining arguments.
+	 * @param out stream for command output
 	 * @param args a mutable deque of the remaining arguments
 	 */
-	final void run(Deque<String> args) {
+	final void run(PrintStream out, Deque<String> args) {
 		List<String> parameters = new ArrayList<>();
 		Map<Option, String> options = new HashMap<>();
 		while (!args.isEmpty()) {
@@ -106,15 +109,32 @@ final void run(Deque<String> args) {
 				parameters.add(arg);
 			}
 		}
-		run(options, parameters);
+		run(out, options, parameters);
 	}
 
 	/**
 	 * Run the actual command.
+	 * @param out stream for command output
 	 * @param options any options extracted from the arguments
 	 * @param parameters any parameters extracted from the arguments
 	 */
-	protected abstract void run(Map<Option, String> options, List<String> parameters);
+	protected abstract void run(PrintStream out, Map<Option, String> options, List<String> parameters);
+
+	/**
+	 * Whether the command is deprecated.
+	 * @return whether the command is deprecated
+	 */
+	protected boolean isDeprecated() {
+		return false;
+	}
+
+	/**
+	 * Returns the deprecation message.
+	 * @return the deprecation message
+	 */
+	protected String getDeprecationMessage() {
+		return null;
+	}
 
 	/**
 	 * Static method that can be used to find a single command from a collection.

diff --git a/...-layertools/src/main/java/org/springframework/boot/jarmode/layertools/ExtractCommand.java b/...-layertools/src/main/java/org/springframework/boot/jarmode/layertools/ExtractCommand.java
@@ -21,6 +21,7 @@
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.io.PrintStream;
 import java.io.UncheckedIOException;
 import java.nio.file.Files;
 import java.nio.file.attribute.BasicFileAttributeView;
@@ -86,7 +87,7 @@ class ExtractCommand extends Command {
 	}
 
 	@Override
-	public void run(Map<Option, String> options, List<String> parameters) {
+	public void run(PrintStream out, Map<Option, String> options, List<String> parameters) {
 		try {
 			File destination = getWorkingDirectory(options);
 			Layers layers = getLayers(options);

diff --git a/...tools/src/main/java/org/springframework/boot/jarmode/layertools/ExtractLayersCommand.java b/...tools/src/main/java/org/springframework/boot/jarmode/layertools/ExtractLayersCommand.java
@@ -16,6 +16,7 @@
 
 package org.springframework.boot.jarmode.layertools;
 
+import java.io.PrintStream;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -45,14 +46,24 @@ class ExtractLayersCommand extends Command {
 	}
 
 	@Override
-	protected void run(Map<Option, String> options, List<String> parameters) {
+	protected boolean isDeprecated() {
+		return true;
+	}
+
+	@Override
+	protected String getDeprecationMessage() {
+		return "Use '-Djarmode=tools extract --layers --launcher' instead.";
+	}
+
+	@Override
+	protected void run(PrintStream out, Map<Option, String> options, List<String> parameters) {
 		Map<Option, String> rewrittenOptions = new HashMap<>();
 		if (options.containsKey(DESTINATION_OPTION)) {
 			rewrittenOptions.put(ExtractCommand.DESTINATION_OPTION, options.get(DESTINATION_OPTION));
 		}
 		rewrittenOptions.put(ExtractCommand.LAYERS_OPTION, StringUtils.collectionToCommaDelimitedString(parameters));
 		rewrittenOptions.put(ExtractCommand.LAUNCHER_OPTION, null);
-		this.delegate.run(rewrittenOptions, Collections.emptyList());
+		this.delegate.run(out, rewrittenOptions, Collections.emptyList());
 	}
 
 }
diff --git a/...ode-layertools/src/main/java/org/springframework/boot/jarmode/layertools/HelpCommand.java b/...ode-layertools/src/main/java/org/springframework/boot/jarmode/layertools/HelpCommand.java
@@ -25,6 +25,7 @@
  * Implicit {@code 'help'} command.
  *
  * @author Phillip Webb
+ * @author Moritz Halbritter
  */
 class HelpCommand extends Command {
 
@@ -39,24 +40,34 @@ class HelpCommand extends Command {
 	}
 
 	HelpCommand(Context context, List<Command> commands, String jarMode) {
-		super("help", "Help about any command", Options.none(), Parameters.of("[<command]"));
+		super("help", "Help about any command", Options.none(), Parameters.of("[<command>]"));
 		this.context = context;
 		this.commands = commands;
 		this.jarMode = (jarMode != null) ? jarMode : "tools";
 	}
 
 	@Override
-	protected void run(Map<Option, String> options, List<String> parameters) {
-		run(System.out, parameters);
+	protected void run(PrintStream out, Map<Option, String> options, List<String> parameters) {
+		run(out, parameters);
 	}
 
 	void run(PrintStream out, List<String> parameters) {
-		Command command = (!parameters.isEmpty()) ? Command.find(this.commands, parameters.get(0)) : null;
-		if (command != null) {
-			printCommandHelp(out, command);
+		String commandName = (parameters.isEmpty()) ? null : parameters.get(0);
+		if (commandName == null) {
+			printUsageAndCommands(null, out);
+			return;
+		}
+		if (getName().equals(commandName)) {
+			printCommandHelp(out, this);
 			return;
 		}
-		printUsageAndCommands(out);
+		Command command = Command.find(this.commands, commandName);
+		if (command == null) {
+			printUsageAndCommands(commandName, out);
+		}
+		else {
+			printCommandHelp(out, command);
+		}
 	}
 
 	private void printCommandHelp(PrintStream out, Command command) {
@@ -86,7 +97,10 @@ private String getUsage(Command command) {
 		return usage.toString();
 	}
 
-	private void printUsageAndCommands(PrintStream out) {
+	private void printUsageAndCommands(String commandName, PrintStream out) {
+		if (commandName != null) {
+			printError(out, "Unknown command \"%s\"".formatted(commandName));
+		}
 		out.println("Usage:");
 		out.println("  " + getJavaCommand());
 		out.println();
@@ -108,4 +122,9 @@ private String getJavaCommand() {
 		return "java -Djarmode=" + this.jarMode + " -jar " + this.context.getArchiveFile().getName();
 	}
 
+	private void printError(PrintStream out, String errorMessage) {
+		out.println("Error: " + errorMessage);
+		out.println();
+	}
+
 }
diff --git a/...yertools/src/main/java/org/springframework/boot/jarmode/layertools/LayerToolsJarMode.java b/...yertools/src/main/java/org/springframework/boot/jarmode/layertools/LayerToolsJarMode.java
@@ -40,7 +40,7 @@ public boolean accepts(String mode) {
 	public void run(String mode, String[] args) {
 		try {
 			Context context = (contextOverride != null) ? contextOverride : new Context();
-			new Runner(context, getCommands(context)).run(args);
+			new Runner(System.out, context, getCommands(context)).run(args);
 		}
 		catch (Exception ex) {
 			throw new IllegalStateException(ex);

diff --git a/...ode-layertools/src/main/java/org/springframework/boot/jarmode/layertools/ListCommand.java b/...ode-layertools/src/main/java/org/springframework/boot/jarmode/layertools/ListCommand.java
@@ -38,12 +38,22 @@ class ListCommand extends Command {
 	}
 
 	@Override
-	protected void run(Map<Option, String> options, List<String> parameters) {
-		this.delegate.run(options, parameters);
+	protected boolean isDeprecated() {
+		return true;
+	}
+
+	@Override
+	protected String getDeprecationMessage() {
+		return "Use '-Djarmode=tools list-layers' instead.";
+	}
+
+	@Override
+	protected void run(PrintStream out, Map<Option, String> options, List<String> parameters) {
+		this.delegate.run(out, options, parameters);
 	}
 
 	void printLayers(Layers layers, PrintStream out) {
-		this.delegate.printLayers(layers, out);
+		this.delegate.printLayers(out, layers);
 	}
 
 }
diff --git a/...yertools/src/main/java/org/springframework/boot/jarmode/layertools/ListLayersCommand.java b/...yertools/src/main/java/org/springframework/boot/jarmode/layertools/ListLayersCommand.java
@@ -35,11 +35,11 @@ class ListLayersCommand extends Command {
 	}
 
 	@Override
-	public void run(Map<Option, String> options, List<String> parameters) {
-		printLayers(Layers.get(this.context), System.out);
+	public void run(PrintStream out, Map<Option, String> options, List<String> parameters) {
+		printLayers(out, Layers.get(this.context));
 	}
 
-	void printLayers(Layers layers, PrintStream out) {
+	void printLayers(PrintStream out, Layers layers) {
 		layers.forEach(out::println);
 	}
 

diff --git a/...-jarmode-layertools/src/main/java/org/springframework/boot/jarmode/layertools/Runner.java b/...-jarmode-layertools/src/main/java/org/springframework/boot/jarmode/layertools/Runner.java
@@ -16,7 +16,9 @@
 
 package org.springframework.boot.jarmode.layertools;
 
+import java.io.PrintStream;
 import java.util.ArrayDeque;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Deque;
 import java.util.List;
@@ -28,13 +30,17 @@
  */
 class Runner {
 
-	private final List<Command> commands;
+	private final PrintStream out;
+
+	private final List<Command> commands = new ArrayList<>();
 
 	private final HelpCommand help;
 
-	Runner(Context context, List<Command> commands) {
-		this.commands = commands;
+	Runner(PrintStream out, Context context, List<Command> commands) {
+		this.out = out;
+		this.commands.addAll(commands);
 		this.help = new HelpCommand(context, commands);
+		this.commands.add(this.help);
 	}
 
 	void run(String... args) {
@@ -51,26 +57,34 @@ private void run(Deque<String> args) {
 			}
 			printError("Unknown command \"" + commandName + "\"");
 		}
-		this.help.run(args);
+		this.help.run(this.out, args);
 	}
 
 	private void runCommand(Command command, Deque<String> args) {
+		if (command.isDeprecated()) {
+			printWarning("This command is deprecated. " + command.getDeprecationMessage());
+		}
 		try {
-			command.run(args);
+			command.run(this.out, args);
 		}
 		catch (UnknownOptionException ex) {
 			printError("Unknown option \"" + ex.getMessage() + "\" for the " + command.getName() + " command");
-			this.help.run(dequeOf(command.getName()));
+			this.help.run(this.out, dequeOf(command.getName()));
 		}
 		catch (MissingValueException ex) {
 			printError("Option \"" + ex.getMessage() + "\" for the " + command.getName() + " command requires a value");
-			this.help.run(dequeOf(command.getName()));
+			this.help.run(this.out, dequeOf(command.getName()));
 		}
 	}
 
-	private void printError(String errorMessage) {
-		System.out.println("Error: " + errorMessage);
-		System.out.println();
+	private void printWarning(String message) {
+		this.out.println("Warning: " + message);
+		this.out.println();
+	}
+
+	private void printError(String message) {
+		this.out.println("Error: " + message);
+		this.out.println();
 	}
 
 	private Deque<String> dequeOf(String... args) {

diff --git a/...de-layertools/src/main/java/org/springframework/boot/jarmode/layertools/ToolsJarMode.java b/...de-layertools/src/main/java/org/springframework/boot/jarmode/layertools/ToolsJarMode.java
@@ -39,7 +39,7 @@ public boolean accepts(String mode) {
 	public void run(String mode, String[] args) {
 		try {
 			Context context = (contextOverride != null) ? contextOverride : new Context();
-			new Runner(context, getCommands(context)).run(args);
+			new Runner(System.out, context, getCommands(context)).run(args);
 		}
 		catch (Exception ex) {
 			throw new IllegalStateException(ex);

diff --git a/...de-layertools/src/test/java/org/springframework/boot/jarmode/layertools/CommandTests.java b/...de-layertools/src/test/java/org/springframework/boot/jarmode/layertools/CommandTests.java
@@ -16,6 +16,7 @@
 
 package org.springframework.boot.jarmode.layertools;
 
+import java.io.PrintStream;
 import java.util.ArrayDeque;
 import java.util.Arrays;
 import java.util.List;
@@ -159,7 +160,7 @@ void shouldNotParseFollowingOptionAsValue() {
 	}
 
 	private void run(TestCommand command, String... args) {
-		command.run(new ArrayDeque<>(Arrays.asList(args)));
+		command.run(System.out, new ArrayDeque<>(Arrays.asList(args)));
 	}
 
 	static class TestCommand extends Command {
@@ -177,7 +178,7 @@ static class TestCommand extends Command {
 		}
 
 		@Override
-		protected void run(Map<Option, String> options, List<String> parameters) {
+		protected void run(PrintStream out, Map<Option, String> options, List<String> parameters) {
 			this.runOptions = options;
 			this.runParameters = parameters;
 		}

diff --git a/.../src/test/java/org/springframework/boot/jarmode/layertools/ExtractLayersCommandTests.java b/.../src/test/java/org/springframework/boot/jarmode/layertools/ExtractLayersCommandTests.java
@@ -96,7 +96,7 @@ void setup() throws Exception {
 	void runExtractsLayers() {
 		given(this.context.getArchiveFile()).willReturn(this.jarFile);
 		given(this.context.getWorkingDir()).willReturn(this.extract);
-		this.command.run(Collections.emptyMap(), Collections.emptyList());
+		this.command.run(System.out, Collections.emptyMap(), Collections.emptyList());
 		assertThat(this.extract.list()).containsOnly("a", "b", "c", "d");
 		assertThat(new File(this.extract, "a/a/a.jar")).exists().satisfies(this::timeAttributes);
 		assertThat(new File(this.extract, "b/b/b.jar")).exists().satisfies(this::timeAttributes);
@@ -145,7 +145,8 @@ private FileTime expectedCreationTime() {
 	void runWhenHasDestinationOptionExtractsLayers() {
 		given(this.context.getArchiveFile()).willReturn(this.jarFile);
 		File out = new File(this.extract, "out");
-		this.command.run(Collections.singletonMap(ExtractLayersCommand.DESTINATION_OPTION, out.getAbsolutePath()),
+		this.command.run(System.out,
+				Collections.singletonMap(ExtractLayersCommand.DESTINATION_OPTION, out.getAbsolutePath()),
 				Collections.emptyList());
 		assertThat(this.extract.list()).containsOnly("out");
 		assertThat(new File(this.extract, "out/a/a/a.jar")).exists().satisfies(this::timeAttributes);
@@ -157,7 +158,7 @@ void runWhenHasDestinationOptionExtractsLayers() {
 	void runWhenHasLayerParamsExtractsLimitedLayers() {
 		given(this.context.getArchiveFile()).willReturn(this.jarFile);
 		given(this.context.getWorkingDir()).willReturn(this.extract);
-		this.command.run(Collections.emptyMap(), Arrays.asList("a", "c"));
+		this.command.run(System.out, Collections.emptyMap(), Arrays.asList("a", "c"));
 		assertThat(this.extract.list()).containsOnly("a", "c");
 		assertThat(new File(this.extract, "a/a/a.jar")).exists().satisfies(this::timeAttributes);
 		assertThat(new File(this.extract, "c/c/c.jar")).exists().satisfies(this::timeAttributes);
@@ -173,7 +174,7 @@ void runWithJarFileContainingNoEntriesFails() throws IOException {
 		given(this.context.getArchiveFile()).willReturn(file);
 		given(this.context.getWorkingDir()).willReturn(this.extract);
 		assertThatIllegalStateException()
-			.isThrownBy(() -> this.command.run(Collections.emptyMap(), Collections.emptyList()))
+			.isThrownBy(() -> this.command.run(System.out, Collections.emptyMap(), Collections.emptyList()))
 			.withMessageContaining("not compatible");
 	}
 
@@ -191,7 +192,7 @@ void runWithJarFileThatWouldWriteEntriesOutsideDestinationFails() throws Excepti
 		given(this.context.getArchiveFile()).willReturn(this.jarFile);
 		given(this.context.getWorkingDir()).willReturn(this.extract);
 		assertThatIllegalStateException()
-			.isThrownBy(() -> this.command.run(Collections.emptyMap(), Collections.emptyList()))
+			.isThrownBy(() -> this.command.run(System.out, Collections.emptyMap(), Collections.emptyList()))
 			.withMessageContaining("Entry 'e/../../e.jar' would be written");
 	}