Skip to content

Fix Boolean option without value does not properly work with argument #1309 #1310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
czpilar wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix Boolean option without value does not properly work with argument #1309 #1310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .../main/java/org/springframework/shell/core/autoconfigure/ShellRunnerAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.core.convert.support.ConfigurableConversionService;
import org.springframework.core.env.Environment;
import org.springframework.shell.core.ConsoleInputProvider;
import org.springframework.shell.core.NonInteractiveShellRunner;
Expand All @@ -30,6 +31,8 @@
import org.springframework.shell.core.command.CommandRegistry;
import org.springframework.shell.core.command.DefaultCommandParser;

import java.util.Optional;

@AutoConfiguration
public class ShellRunnerAutoConfiguration {

Expand Down Expand Up @@ -66,8 +69,9 @@ public ConsoleInputProvider consoleInputProvider() {

@Bean
@ConditionalOnMissingBean
public CommandParser commandParser(CommandRegistry commandRegistry) {
return new DefaultCommandParser(commandRegistry);
public CommandParser commandParser(CommandRegistry commandRegistry,
Optional<ConfigurableConversionService> conversionService) {
return new DefaultCommandParser(commandRegistry, conversionService);
}

}
8 changes: 6 additions & 2 deletions .../java/org/springframework/shell/core/autoconfigure/StandardCommandsAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@
import org.springframework.boot.info.BuildProperties;
import org.springframework.boot.info.GitProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.core.convert.support.ConfigurableConversionService;
import org.springframework.shell.core.command.*;

import java.util.Optional;

/**
* Creates beans for standard commands.
*
Expand Down Expand Up @@ -73,8 +76,9 @@ public Command versionCommand(SpringShellProperties shellProperties,

@Bean
@ConditionalOnProperty(value = "spring.shell.command.script.enabled", havingValue = "true", matchIfMissing = true)
public Command scriptCommand(CommandRegistry commandRegistry) {
return new Script(commandRegistry);
public Command scriptCommand(CommandRegistry commandRegistry,
Optional<ConfigurableConversionService> conversionService) {
return new Script(commandRegistry, conversionService);
}

}
21 changes: 20 additions & 1 deletion ...shell-core/src/main/java/org/springframework/shell/core/command/DefaultCommandParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.core.convert.ConversionFailedException;
import org.springframework.core.convert.support.ConfigurableConversionService;
import org.springframework.core.convert.support.DefaultConversionService;

/**
* Default implementation of {@link CommandParser}. Supports options in the long form of
Expand Down Expand Up @@ -48,8 +51,12 @@ public class DefaultCommandParser implements CommandParser {

private final CommandRegistry commandRegistry;

public DefaultCommandParser(CommandRegistry commandRegistry) {
private final ConfigurableConversionService conversionService;

public DefaultCommandParser(CommandRegistry commandRegistry,
Optional<ConfigurableConversionService> conversionService) {
this.commandRegistry = commandRegistry;
this.conversionService = conversionService.orElse(new DefaultConversionService());
}

@Override
Expand Down Expand Up @@ -121,6 +128,9 @@ public ParsedInput parse(String input) {
}
nextWord = "true";
}
else if (isBooleanOption(commandName, currentWord) && !isBooleanValue(nextWord)) {
nextWord = "true";
}
else {
i++; // skip next word as it was used as option value
}
Expand Down Expand Up @@ -199,4 +209,13 @@ private boolean isBooleanOption(String commandName, String currentWord) {
.anyMatch(o -> o.type() == boolean.class || o.type() == Boolean.class);
}

private boolean isBooleanValue(String rawValue) {
try {
return conversionService.convert(rawValue, boolean.class) != null;
}
catch (ConversionFailedException e) {
return false;
}
}

}
6 changes: 4 additions & 2 deletions spring-shell-core/src/main/java/org/springframework/shell/core/command/Script.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
import java.io.File;
import java.util.List;
import java.util.Objects;
import java.util.Optional;

import org.springframework.core.convert.support.ConfigurableConversionService;
import org.springframework.shell.core.FileInputProvider;
import org.springframework.shell.core.NonInteractiveShellRunner;

Expand All @@ -34,8 +36,8 @@ public class Script implements Command {

private CommandParser commandParser;

public Script(CommandRegistry commandRegistry) {
this.commandParser = new DefaultCommandParser(commandRegistry);
public Script(CommandRegistry commandRegistry, Optional<ConfigurableConversionService> conversionService) {
this.commandParser = new DefaultCommandParser(commandRegistry, conversionService);
}

@Override
Expand Down
55 changes: 54 additions & 1 deletion ...-core/src/test/java/org/springframework/shell/core/command/DefaultCommandParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;

import java.util.Optional;
import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -36,7 +37,7 @@ class DefaultCommandParserTests {
@BeforeEach
void setUp() {
this.commandRegistry = new CommandRegistry();
this.parser = new DefaultCommandParser(this.commandRegistry);
this.parser = new DefaultCommandParser(this.commandRegistry, Optional.empty());
}

@Test
Expand Down Expand Up @@ -315,6 +316,58 @@ static Stream<Arguments> parseWithBooleanOptionData() {
Arguments.of("mycommand -o", "", 'o', Boolean.class, "true"));
}

@ParameterizedTest
@MethodSource("parseWithBooleanOptionAndArgumentData")
void testParseWithBooleanOptionAndArgument(String input, String expectedValue, int expectedArguments,
String expectedArgumentValue) {
// given
Command command = createCommand("mycommand", "My test command");
command.getOptions().add(CommandOption.with().longName("option").type(boolean.class).build());
commandRegistry.registerCommand(command);
// when
ParsedInput parsedInput = parser.parse(input);

// then
assertEquals("mycommand", parsedInput.commandName());
assertEquals(1, parsedInput.options().size());
assertEquals(expectedValue, parsedInput.options().get(0).value());
assertEquals(expectedArguments, parsedInput.arguments().size());
if (expectedArguments > 0) {
assertEquals(expectedArgumentValue, parsedInput.arguments().get(0).value());
}
}

static Stream<Arguments> parseWithBooleanOptionAndArgumentData() {
return Stream.of(Arguments.of("mycommand --option=false", "false", 0, null),
Arguments.of("mycommand --option=true", "true", 0, null),
Arguments.of("mycommand --option false", "false", 0, null),
Arguments.of("mycommand --option true", "true", 0, null),
Arguments.of("mycommand --option", "true", 0, null),

Arguments.of("mycommand --option=false argument", "false", 1, "argument"),
Arguments.of("mycommand --option=true argument", "true", 1, "argument"),
Arguments.of("mycommand --option false argument", "false", 1, "argument"),
Arguments.of("mycommand --option true argument", "true", 1, "argument"),
Arguments.of("mycommand --option argument", "true", 1, "argument"),

Arguments.of("mycommand argument --option=false", "false", 1, "argument"),
Arguments.of("mycommand argument --option=true", "true", 1, "argument"),
Arguments.of("mycommand argument --option false", "false", 1, "argument"),
Arguments.of("mycommand argument --option true", "true", 1, "argument"),
Arguments.of("mycommand argument --option", "true", 1, "argument"),

Arguments.of("mycommand --option=false true", "false", 1, "true"),
Arguments.of("mycommand --option=true true", "true", 1, "true"),
Arguments.of("mycommand --option false false", "false", 1, "false"),
Arguments.of("mycommand --option true false", "true", 1, "false"),

Arguments.of("mycommand false --option=false", "false", 1, "false"),
Arguments.of("mycommand false --option=true", "true", 1, "false"),
Arguments.of("mycommand true --option false", "false", 1, "true"),
Arguments.of("mycommand true --option true", "true", 1, "true"),
Arguments.of("mycommand true --option", "true", 1, "true"));
}

@ParameterizedTest
@ValueSource(strings = { "mycommand --help", "mycommand -h" })
void testParseWithHelpOption(String input) {
Expand Down
8 changes: 6 additions & 2 deletions ...l-jline/src/main/java/org/springframework/shell/jline/DefaultJLineShellConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.springframework.shell.jline;

import java.io.IOException;
import java.util.Optional;

import org.jline.reader.LineReader;
import org.jline.reader.LineReaderBuilder;
Expand All @@ -10,6 +11,7 @@

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.convert.support.ConfigurableConversionService;
import org.springframework.shell.core.ShellConfigurationException;
import org.springframework.shell.core.ShellRunner;
import org.springframework.shell.core.command.CommandRegistry;
Expand All @@ -19,8 +21,10 @@
public class DefaultJLineShellConfiguration {

@Bean
public ShellRunner shellRunner(JLineInputProvider inputProvider, CommandRegistry commandRegistry) {
return new JLineShellRunner(inputProvider, new DefaultCommandParser(commandRegistry), commandRegistry);
public ShellRunner shellRunner(JLineInputProvider inputProvider, CommandRegistry commandRegistry,
Optional<ConfigurableConversionService> conversionService) {
return new JLineShellRunner(inputProvider, new DefaultCommandParser(commandRegistry, conversionService),
commandRegistry);
}

@Bean
Expand Down
8 changes: 6 additions & 2 deletions ...n/java/org/springframework/shell/test/autoconfigure/ShellTestClientAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@
import org.springframework.boot.autoconfigure.AutoConfiguration;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.context.annotation.Bean;
import org.springframework.core.convert.support.ConfigurableConversionService;
import org.springframework.shell.core.command.CommandParser;
import org.springframework.shell.core.command.CommandRegistry;
import org.springframework.shell.core.command.DefaultCommandParser;
import org.springframework.shell.test.ShellTestClient;

import java.util.Optional;

/**
* @author Janne Valkealahti
* @author Mahmoud Ben Hassine
Expand All @@ -44,8 +47,9 @@ public CommandRegistry commandRegistry() {

@Bean
@ConditionalOnMissingBean
public CommandParser commandParser(CommandRegistry commandRegistry) {
return new DefaultCommandParser(commandRegistry);
public CommandParser commandParser(CommandRegistry commandRegistry,
Optional<ConfigurableConversionService> conversionService) {
return new DefaultCommandParser(commandRegistry, conversionService);
}

}
8 changes: 6 additions & 2 deletions spring-shell-test/src/test/java/org/springframework/shell/test/ShellTestClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.convert.support.ConfigurableConversionService;
import org.springframework.shell.core.command.AbstractCommand;
import org.springframework.shell.core.command.Command;
import org.springframework.shell.core.command.CommandContext;
Expand All @@ -17,6 +18,8 @@
import org.springframework.shell.core.command.ExitStatus;
import org.springframework.test.context.junit.jupiter.SpringExtension;

import java.util.Optional;

@ExtendWith(SpringExtension.class)
class ShellTestClientTests {

Expand Down Expand Up @@ -56,8 +59,9 @@ public CommandRegistry commandRegistry() {
}

@Bean
public CommandParser commandParser(CommandRegistry commandRegistry) {
return new DefaultCommandParser(commandRegistry);
public CommandParser commandParser(CommandRegistry commandRegistry,
Optional<ConfigurableConversionService> conversionService) {
return new DefaultCommandParser(commandRegistry, conversionService);
}

@Bean
Expand Down