From 71960e541503360c7c2415b30af417d0436c1f46 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Thu, 2 May 2024 08:40:17 +0100 Subject: [PATCH] Use string array in ShellRunner - In ShellRunner interface replace use of boot's ApplicationArguments into plain string array. - Deprecate old methods with some fallbacks to give time for users to do updates. - NonInteractiveShellRunner contains one breaking change due to its public api to set function doing conversion from ApplicationArguments which was potentially used via customizer hooks. - Deprecations planned to get removed in 3.4.x. - Fixes #1057 --- .../shell/DefaultShellApplicationRunner.java | 23 +++++++- .../springframework/shell/ShellRunner.java | 31 ++++++++++- .../shell/jline/InteractiveShellRunner.java | 7 +-- .../jline/NonInteractiveShellRunner.java | 31 +++++------ .../shell/jline/ScriptShellRunner.java | 25 ++++----- .../jline/InteractiveShellRunnerTests.java | 37 +++++++++++++ .../jline/NonInteractiveShellRunnerTests.java | 54 +++++++++++++----- .../shell/jline/ScriptShellRunnerTests.java | 55 ++++++++++++++++--- .../shell/test/ShellTestClient.java | 9 ++- 9 files changed, 208 insertions(+), 64 deletions(-) diff --git a/spring-shell-core/src/main/java/org/springframework/shell/DefaultShellApplicationRunner.java b/spring-shell-core/src/main/java/org/springframework/shell/DefaultShellApplicationRunner.java index e2d422590..7d8dac91c 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/DefaultShellApplicationRunner.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/DefaultShellApplicationRunner.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2022 the original author or authors. + * Copyright 2021-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -56,6 +56,27 @@ public DefaultShellApplicationRunner(List shellRunners) { @Override public void run(ApplicationArguments args) throws Exception { log.debug("Checking shell runners {}", shellRunners); + + // Handle new ShellRunner api + String[] sourceArgs = args.getSourceArgs(); + boolean canRun = false; + for (ShellRunner runner : shellRunners) { + try { + canRun = runner.run(sourceArgs); + } catch (Exception e) { + break; + } + if (canRun) { + break; + } + } + + if (canRun) { + // new api handled execution + return; + } + + // Handle old deprecated ShellRunner api Optional optional = shellRunners.stream() .filter(sh -> sh.canRun(args)) .findFirst(); diff --git a/spring-shell-core/src/main/java/org/springframework/shell/ShellRunner.java b/spring-shell-core/src/main/java/org/springframework/shell/ShellRunner.java index 7ca6c523d..6792ae3b3 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/ShellRunner.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/ShellRunner.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 the original author or authors. + * Copyright 2021-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,16 +27,41 @@ public interface ShellRunner { /** * Checks if a particular shell runner can execute. * + * For {@link #canRun(ApplicationArguments)} and + * {@link #run(ApplicationArguments)} prefer {@link #run(String[])}. + * * @param args the application arguments * @return true if shell runner can execute */ - boolean canRun(ApplicationArguments args); + @Deprecated(since = "3.3.0", forRemoval = true) + default boolean canRun(ApplicationArguments args) { + return false; + } /** * Execute application. * + * For {@link #canRun(ApplicationArguments)} and + * {@link #run(ApplicationArguments)} prefer {@link #run(String[])}. + * * @param args the application argumets * @throws Exception in errors */ - void run(ApplicationArguments args) throws Exception; + @Deprecated(since = "3.3.0", forRemoval = true) + default void run(ApplicationArguments args) throws Exception { + throw new UnsupportedOperationException("Should get implemented together with canRun"); + } + + /** + * Execute {@code ShellRunner} with given args. Return value indicates if run + * operation happened and no further runners should be used. + * + * @param args the raw arguments + * @return true if run execution happened + * @throws Exception possible error during run + */ + default boolean run(String[] args) throws Exception { + return false; + } + } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/jline/InteractiveShellRunner.java b/spring-shell-core/src/main/java/org/springframework/shell/jline/InteractiveShellRunner.java index cabdfaff5..c29c8f98d 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/jline/InteractiveShellRunner.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/jline/InteractiveShellRunner.java @@ -21,7 +21,6 @@ import org.jline.reader.UserInterruptException; import org.jline.utils.AttributedString; -import org.springframework.boot.ApplicationArguments; import org.springframework.core.annotation.Order; import org.springframework.shell.ExitRequest; import org.springframework.shell.Input; @@ -67,14 +66,10 @@ public InteractiveShellRunner(LineReader lineReader, PromptProvider promptProvid } @Override - public void run(ApplicationArguments args) throws Exception { + public boolean run(String[] args) throws Exception { shellContext.setInteractionMode(InteractionMode.INTERACTIVE); InputProvider inputProvider = new JLineInputProvider(lineReader, promptProvider); shell.run(inputProvider); - } - - @Override - public boolean canRun(ApplicationArguments args) { return true; } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/jline/NonInteractiveShellRunner.java b/spring-shell-core/src/main/java/org/springframework/shell/jline/NonInteractiveShellRunner.java index 7abcd1fa5..13399c7a2 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/jline/NonInteractiveShellRunner.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/jline/NonInteractiveShellRunner.java @@ -1,5 +1,5 @@ /* - * Copyright 2021-2023 the original author or authors. + * Copyright 2021-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,7 +25,6 @@ import org.jline.reader.Parser; import org.jline.reader.impl.DefaultParser; -import org.springframework.boot.ApplicationArguments; import org.springframework.core.annotation.Order; import org.springframework.shell.Input; import org.springframework.shell.InputProvider; @@ -34,6 +33,7 @@ import org.springframework.shell.Utils; import org.springframework.shell.context.InteractionMode; import org.springframework.shell.context.ShellContext; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -65,8 +65,8 @@ public class NonInteractiveShellRunner implements ShellRunner { private static final String SINGLE_QUOTE = "\'"; private static final String DOUBLE_QUOTE = "\""; - private Function> commandsFromInputArgs = args -> { - if (args.getSourceArgs().length == 0) { + private Function> commandsFromArgs = args -> { + if (ObjectUtils.isEmpty(args)) { if (StringUtils.hasText(primaryCommand)) { Collections.singletonList(primaryCommand); } @@ -75,7 +75,7 @@ public class NonInteractiveShellRunner implements ShellRunner { } } // re-quote if needed having whitespace - String raw = Arrays.stream(args.getSourceArgs()) + String raw = Arrays.stream(args) .map(a -> { if (!isQuoted(a) && StringUtils.containsWhitespace(a)) { return "\"" + a + "\""; @@ -113,12 +113,12 @@ public NonInteractiveShellRunner(Shell shell, ShellContext shellContext, String /** * Sets the function that creates the command() to run from the input application arguments. * - * @param commandsFromInputArgs function that takes input application arguments and creates zero or more commands + * @param commandsFromArgs function that takes input application arguments and creates zero or more commands * where each command is a string that specifies the command and options * (eg. 'history --file myHistory.txt') */ - public void setCommandsFromInputArgs(Function> commandsFromInputArgs) { - this.commandsFromInputArgs = commandsFromInputArgs; + public void setCommandsFromArgs(Function> commandsFromArgs) { + this.commandsFromArgs = commandsFromArgs; } /** @@ -131,19 +131,18 @@ public void setLineParser(Parser lineParser) { } @Override - public boolean canRun(ApplicationArguments args) { - return !commandsFromInputArgs.apply(args).isEmpty(); - } - - @Override - public void run(ApplicationArguments args) throws Exception { - shellContext.setInteractionMode(InteractionMode.NONINTERACTIVE); - List commands = this.commandsFromInputArgs.apply(args); + public boolean run(String[] args) throws Exception { + List commands = commandsFromArgs.apply(args); + if (commands.isEmpty()) { + return false; + } List parsedLines = commands.stream() .map(rawCommandLine -> lineParser.parse(rawCommandLine, rawCommandLine.length() + 1)) .collect(Collectors.toList()); MultiParsedLineInputProvider inputProvider = new MultiParsedLineInputProvider(parsedLines); + shellContext.setInteractionMode(InteractionMode.NONINTERACTIVE); shell.run(inputProvider); + return true; } /** diff --git a/spring-shell-core/src/main/java/org/springframework/shell/jline/ScriptShellRunner.java b/spring-shell-core/src/main/java/org/springframework/shell/jline/ScriptShellRunner.java index d437c5e4f..30fa1aa03 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/jline/ScriptShellRunner.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/jline/ScriptShellRunner.java @@ -19,15 +19,16 @@ import java.io.File; import java.io.FileReader; import java.io.Reader; +import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; import org.jline.reader.Parser; -import org.springframework.boot.ApplicationArguments; import org.springframework.core.annotation.Order; import org.springframework.shell.Shell; import org.springframework.shell.ShellRunner; +import org.springframework.util.ObjectUtils; /** * A {@link ShellRunner} that looks for process arguments that start with {@literal @}, which are then interpreted as @@ -59,19 +60,16 @@ public ScriptShellRunner(Parser parser, Shell shell) { } @Override - public boolean canRun(ApplicationArguments args) { - String[] sourceArgs = args.getSourceArgs(); - if (sourceArgs.length > 0 && sourceArgs[0].startsWith("@") && sourceArgs[0].length() > 1) { - return true; + public boolean run(String[] args) throws Exception { + String[] sourceArgs = args; + if (ObjectUtils.isEmpty(sourceArgs)) { + return false; + } + if (!(sourceArgs[0].startsWith("@") && sourceArgs[0].length() > 1)) { + return false; } - return false; - } - - //tag::documentation[] - @Override - public void run(ApplicationArguments args) throws Exception { - List scriptsToRun = args.getNonOptionArgs().stream() + List scriptsToRun = Arrays.asList(args).stream() .filter(s -> s.startsWith("@")) .map(s -> new File(s.substring(1))) .collect(Collectors.toList()); @@ -82,7 +80,8 @@ public void run(ApplicationArguments args) throws Exception { shell.run(inputProvider); } } + + return true; } - //end::documentation[] } diff --git a/spring-shell-core/src/test/java/org/springframework/shell/jline/InteractiveShellRunnerTests.java b/spring-shell-core/src/test/java/org/springframework/shell/jline/InteractiveShellRunnerTests.java index 17c517e77..8d0b5b44d 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/jline/InteractiveShellRunnerTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/jline/InteractiveShellRunnerTests.java @@ -1,3 +1,18 @@ +/* + * Copyright 2022-2024 the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.shell.jline; import java.io.ByteArrayOutputStream; @@ -14,6 +29,8 @@ import org.jline.utils.AttributedStyle; import org.junit.jupiter.api.Test; +import org.springframework.boot.ApplicationArguments; +import org.springframework.boot.DefaultApplicationArguments; import org.springframework.shell.ExitRequest; import static org.assertj.core.api.Assertions.assertThat; @@ -132,4 +149,24 @@ public void testExitWithCtrlD() throws Exception { readThread.join(); writeThread.join(); } + + + @Test + void oldApiCanRunReturnFalse() { + InteractiveShellRunner runner = new InteractiveShellRunner(null, null, null, null); + assertThat(runner.canRun(ofApplicationArguments())).isFalse(); + } + + @Test + void oldApiRunThrows() { + InteractiveShellRunner runner = new InteractiveShellRunner(null, null, null, null); + assertThatThrownBy(() -> { + runner.run(ofApplicationArguments()); + }); + } + + private static ApplicationArguments ofApplicationArguments(String... args) { + return new DefaultApplicationArguments(args); + } + } diff --git a/spring-shell-core/src/test/java/org/springframework/shell/jline/NonInteractiveShellRunnerTests.java b/spring-shell-core/src/test/java/org/springframework/shell/jline/NonInteractiveShellRunnerTests.java index 7d2fcdac5..e8faa0c71 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/jline/NonInteractiveShellRunnerTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/jline/NonInteractiveShellRunnerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2022 the original author or authors. + * Copyright 2022-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,12 +23,14 @@ import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.boot.ApplicationArguments; import org.springframework.boot.DefaultApplicationArguments; import org.springframework.shell.InputProvider; import org.springframework.shell.Shell; import org.springframework.shell.context.DefaultShellContext; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; @ExtendWith(MockitoExtension.class) public class NonInteractiveShellRunnerTests { @@ -38,26 +40,25 @@ public class NonInteractiveShellRunnerTests { private Shell shell; @Test - public void testEmptyArgsDontRun() { + public void testEmptyArgsDontRun() throws Exception { NonInteractiveShellRunner runner = new NonInteractiveShellRunner(null, null); - DefaultApplicationArguments args = new DefaultApplicationArguments(); - assertThat(runner.canRun(args)).isFalse(); + assertThat(runner.run(new String[0])).isFalse(); } @Test - public void testNonEmptyArgsRun() { - NonInteractiveShellRunner runner = new NonInteractiveShellRunner(null, null); - DefaultApplicationArguments args = new DefaultApplicationArguments("hi"); - assertThat(runner.canRun(args)).isTrue(); + public void testNonEmptyArgsRun() throws Exception { + NonInteractiveShellRunner runner = new NonInteractiveShellRunner(shell, new DefaultShellContext()); + ArgumentCaptor valueCapture = ArgumentCaptor.forClass(InputProvider.class); + Mockito.doNothing().when(shell).run(valueCapture.capture()); + assertThat(runner.run(ofArgs("hi"))).isTrue(); } @Test public void shouldQuoteWithWhitespace() throws Exception { NonInteractiveShellRunner runner = new NonInteractiveShellRunner(shell, new DefaultShellContext()); - DefaultApplicationArguments args = new DefaultApplicationArguments("foo bar"); ArgumentCaptor valueCapture = ArgumentCaptor.forClass(InputProvider.class); Mockito.doNothing().when(shell).run(valueCapture.capture()); - runner.run(args); + assertThat(runner.run(ofArgs("foo bar"))).isTrue(); InputProvider value = valueCapture.getValue(); assertThat(value.readInput().rawText()).isEqualTo("\"foo bar\""); } @@ -65,10 +66,9 @@ public void shouldQuoteWithWhitespace() throws Exception { @Test public void shouldNotQuoteIfQuoted() throws Exception { NonInteractiveShellRunner runner = new NonInteractiveShellRunner(shell, new DefaultShellContext()); - DefaultApplicationArguments args = new DefaultApplicationArguments("'foo bar'"); ArgumentCaptor valueCapture = ArgumentCaptor.forClass(InputProvider.class); Mockito.doNothing().when(shell).run(valueCapture.capture()); - runner.run(args); + assertThat(runner.run(ofArgs("'foo bar'"))).isTrue(); InputProvider value = valueCapture.getValue(); assertThat(value.readInput().rawText()).isEqualTo("'foo bar'"); } @@ -76,11 +76,37 @@ public void shouldNotQuoteIfQuoted() throws Exception { @Test public void shouldNotQuoteWithoutWhitespace() throws Exception { NonInteractiveShellRunner runner = new NonInteractiveShellRunner(shell, new DefaultShellContext()); - DefaultApplicationArguments args = new DefaultApplicationArguments("foobar"); ArgumentCaptor valueCapture = ArgumentCaptor.forClass(InputProvider.class); Mockito.doNothing().when(shell).run(valueCapture.capture()); - runner.run(args); + assertThat(runner.run(ofArgs("foobar"))).isTrue(); InputProvider value = valueCapture.getValue(); assertThat(value.readInput().rawText()).isEqualTo("foobar"); } + + @Test + void oldApiCanRunReturnFalse() { + NonInteractiveShellRunner runner = new NonInteractiveShellRunner(shell, null); + assertThat(runner.canRun(ofApplicationArguments())).isFalse(); + } + + @Test + void oldApiRunThrows() { + NonInteractiveShellRunner runner = new NonInteractiveShellRunner(shell, null); + assertThatThrownBy(() -> { + runner.run(ofApplicationArguments()); + }); + } + + private static ApplicationArguments ofApplicationArguments(String... args) { + return new DefaultApplicationArguments(args); + } + + private static String[] ofArgs(String... args) { + String[] a = new String[args.length]; + for (int i = 0; i < args.length; i++) { + a[i] = args[i]; + } + return a; + } + } diff --git a/spring-shell-core/src/test/java/org/springframework/shell/jline/ScriptShellRunnerTests.java b/spring-shell-core/src/test/java/org/springframework/shell/jline/ScriptShellRunnerTests.java index 5ecd2894b..c5b06c21c 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/jline/ScriptShellRunnerTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/jline/ScriptShellRunnerTests.java @@ -15,38 +15,75 @@ */ package org.springframework.shell.jline; +import java.nio.file.Files; +import java.nio.file.Path; + import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.boot.ApplicationArguments; import org.springframework.boot.DefaultApplicationArguments; +import org.springframework.shell.Shell; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +@ExtendWith(MockitoExtension.class) public class ScriptShellRunnerTests { + @Mock + Shell shell; + private ScriptShellRunner runner = new ScriptShellRunner(null, null); @Test - void shouldNotRunWhenNoArgs() { - assertThat(runner.canRun(of())).isFalse(); + void shouldNotRunWhenNoArgs() throws Exception { + assertThat(runner.run(ofArgs())).isFalse(); + } + + @Test + void shouldNotRunWhenInOptionValue() throws Exception { + assertThat(runner.run(ofArgs("--foo", "@"))).isFalse(); } @Test - void shouldNotRunWhenInOptionValue() { - assertThat(runner.canRun(of("--foo", "@"))).isFalse(); + void shouldNotRunWhenJustFirstArgWithoutFile() throws Exception { + assertThat(runner.run(ofArgs("@"))).isFalse(); } @Test - void shouldNotRunWhenJustFirstArgWithoutFile() { - assertThat(runner.canRun(of("@"))).isFalse(); + void shouldRunWhenFirstArgHavingFile(@TempDir Path workingDir) throws Exception { + Path path = workingDir.resolve("test"); + Path file = Files.createFile(path); + String pathStr = file.toAbsolutePath().toString(); + ScriptShellRunner runner = new ScriptShellRunner(null, shell); + assertThat(runner.run(new String[]{"@" + pathStr})).isTrue(); } @Test - void shouldRunWhenFirstArgHavingFile() { - assertThat(runner.canRun(of("@file"))).isTrue(); + void oldApiCanRunReturnFalse() { + assertThat(runner.canRun(ofApplicationArguments())).isFalse(); } - private static ApplicationArguments of(String... args) { + @Test + void oldApiRunThrows() { + assertThatThrownBy(() -> { + runner.run(ofApplicationArguments()); + }); + } + + private static ApplicationArguments ofApplicationArguments(String... args) { return new DefaultApplicationArguments(args); } + + private static String[] ofArgs(String... args) { + String[] a = new String[args.length]; + for (int i = 0; i < args.length; i++) { + a[i] = args[i]; + } + return a; + } } diff --git a/spring-shell-test/src/main/java/org/springframework/shell/test/ShellTestClient.java b/spring-shell-test/src/main/java/org/springframework/shell/test/ShellTestClient.java index 3ea66bce6..2ffcd9b43 100644 --- a/spring-shell-test/src/main/java/org/springframework/shell/test/ShellTestClient.java +++ b/spring-shell-test/src/main/java/org/springframework/shell/test/ShellTestClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2022 the original author or authors. + * Copyright 2022-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -340,7 +340,12 @@ public void run() { try { log.trace("Running {}", data.runner()); data.state().set(-1); - data.runner().run(data.args()); + if (data.runner().canRun(data.args)) { + data.runner().run(data.args()); + } + else { + data.runner().run(data.args().getSourceArgs()); + } data.state().set(0); log.trace("Running done {}", data.runner()); } catch (Exception e) {