diff --git a/scm-webapp/src/main/java/sonia/scm/cli/CliExceptionHandlerFactory.java b/scm-webapp/src/main/java/sonia/scm/cli/CliExceptionHandlerFactory.java index d0897f23f1..38be692343 100644 --- a/scm-webapp/src/main/java/sonia/scm/cli/CliExceptionHandlerFactory.java +++ b/scm-webapp/src/main/java/sonia/scm/cli/CliExceptionHandlerFactory.java @@ -37,7 +37,11 @@ class CliExceptionHandlerFactory { this.i18nCollector = i18nCollector; } - CliExceptionHandler createExceptionHandler(String languageCode) { - return new CliExceptionHandler(i18nCollector, languageCode); + CliExecutionExceptionHandler createExecutionExceptionHandler(String languageCode) { + return new CliExecutionExceptionHandler(i18nCollector, languageCode); + } + + CliParameterExceptionHandler createParameterExceptionHandler(String languageCode) { + return new CliParameterExceptionHandler(languageCode); } } diff --git a/scm-webapp/src/main/java/sonia/scm/cli/CliExceptionHandler.java b/scm-webapp/src/main/java/sonia/scm/cli/CliExecutionExceptionHandler.java similarity index 95% rename from scm-webapp/src/main/java/sonia/scm/cli/CliExceptionHandler.java rename to scm-webapp/src/main/java/sonia/scm/cli/CliExecutionExceptionHandler.java index 3af75af9dd..696a6c3d61 100644 --- a/scm-webapp/src/main/java/sonia/scm/cli/CliExceptionHandler.java +++ b/scm-webapp/src/main/java/sonia/scm/cli/CliExecutionExceptionHandler.java @@ -39,12 +39,12 @@ import java.util.ResourceBundle; import static java.util.Optional.empty; import static java.util.Optional.of; -public class CliExceptionHandler implements CommandLine.IExecutionExceptionHandler { +public class CliExecutionExceptionHandler implements CommandLine.IExecutionExceptionHandler { private final I18nCollector i18nCollector; private final String languageCode; - CliExceptionHandler(I18nCollector i18nCollector, String languageCode) { + CliExecutionExceptionHandler(I18nCollector i18nCollector, String languageCode) { this.i18nCollector = i18nCollector; this.languageCode = languageCode; } diff --git a/scm-webapp/src/main/java/sonia/scm/cli/CliParameterExceptionHandler.java b/scm-webapp/src/main/java/sonia/scm/cli/CliParameterExceptionHandler.java new file mode 100644 index 0000000000..543d978de2 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/cli/CliParameterExceptionHandler.java @@ -0,0 +1,107 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.cli; + +import picocli.CommandLine; +import picocli.CommandLine.IParameterExceptionHandler; + +import java.io.PrintWriter; +import java.util.Locale; +import java.util.ResourceBundle; +import java.util.stream.Collectors; + +public class CliParameterExceptionHandler implements IParameterExceptionHandler { + + private final String languageCode; + + public CliParameterExceptionHandler(String languageCode) { + this.languageCode = languageCode; + } + + @Override + public int handleParseException(CommandLine.ParameterException ex, String[] args) { + CommandLine cmd = ex.getCommandLine(); + PrintWriter stderr = cmd.getErr(); + + if (languageCode.equals("en")) { + // Do not use translated exceptions for english since the default ones are more specific + printError(stderr, ex.getMessage()); + if (!CommandLine.UnmatchedArgumentException.printSuggestions(ex, stderr)) { + ex.getCommandLine().usage(stderr); + } + } else { + handleTranslatedExceptions(ex, stderr); + } + return ExitCode.USAGE; + } + + private void handleTranslatedExceptions(CommandLine.ParameterException ex, PrintWriter stderr) { + if (ex.getCause() instanceof CommandLine.TypeConversionException) { + printError(stderr, getMessage("exception.typeConversion") + ex.getArgSpec().paramLabel()); + } + if (ex instanceof CommandLine.MissingParameterException) { + printError(stderr, getMessage("exception.missingParameter") + + ((CommandLine.MissingParameterException) ex).getMissing() + .stream() + .map(CommandLine.Model.ArgSpec::paramLabel) + .collect(Collectors.toList())); + } + if (ex instanceof CommandLine.OverwrittenOptionException) { + printError(stderr, getMessage("exception.overwrittenOption") + + ((CommandLine.OverwrittenOptionException) ex).getOverwritten().paramLabel()); + } + if (ex instanceof CommandLine.MaxValuesExceededException) { + printError(stderr, getMessage("exception.maxValuesExceeded")); + } + if (ex instanceof CommandLine.MutuallyExclusiveArgsException) { + printError(stderr, getMessage("exception.mutuallyExclusiveArgs")); + } + if (ex instanceof CommandLine.UnmatchedArgumentException) { + printError(stderr, getMessage("exception.unmatchedArguments") + + ((CommandLine.UnmatchedArgumentException) ex).getUnmatched()); + } + if (ex.getMessage().equals("Missing required subcommand")) { + printError(stderr, getMessage("exception.missingSubCommand")); + } + + // Print command description or suggestions to avoid this error + if (!CommandLine.UnmatchedArgumentException.printSuggestions(ex, stderr)) { + ex.getCommandLine().usage(stderr); + } + } + + private void printError(PrintWriter stderr, String message) { + stderr.println(getMessage("errorLabel") + ": " + message); + } + + private String getMessage(String key) { + return ResourceBundle + .getBundle("sonia.scm.cli.i18n", new Locale(languageCode)) + .getString(key); + } +} + + + diff --git a/scm-webapp/src/main/java/sonia/scm/cli/CliProcessor.java b/scm-webapp/src/main/java/sonia/scm/cli/CliProcessor.java index 2e512ab766..faa5779ad7 100644 --- a/scm-webapp/src/main/java/sonia/scm/cli/CliProcessor.java +++ b/scm-webapp/src/main/java/sonia/scm/cli/CliProcessor.java @@ -59,7 +59,8 @@ public class CliProcessor { cli.addSubcommand(AutoComplete.GenerateCompletion.class); cli.setErr(context.getStderr()); cli.setOut(context.getStdout()); - cli.setExecutionExceptionHandler(exceptionHandlerFactory.createExceptionHandler(context.getLocale().getLanguage())); + cli.setExecutionExceptionHandler(exceptionHandlerFactory.createExecutionExceptionHandler(context.getLocale().getLanguage())); + cli.setParameterExceptionHandler(exceptionHandlerFactory.createParameterExceptionHandler(context.getLocale().getLanguage())); return cli.execute(args); } diff --git a/scm-webapp/src/main/java/sonia/scm/repository/cli/RepositoryCreateCommand.java b/scm-webapp/src/main/java/sonia/scm/repository/cli/RepositoryCreateCommand.java index a115075c29..7accd81db1 100644 --- a/scm-webapp/src/main/java/sonia/scm/repository/cli/RepositoryCreateCommand.java +++ b/scm-webapp/src/main/java/sonia/scm/repository/cli/RepositoryCreateCommand.java @@ -54,7 +54,7 @@ class RepositoryCreateCommand implements Runnable { private String type; @RepositoryName(namespace = RepositoryName.Namespace.OPTIONAL) - @CommandLine.Parameters(descriptionKey = "scm.repo.create.repository", paramLabel = "name") + @CommandLine.Parameters(descriptionKey = "scm.repo.create.repository", paramLabel = "") private String repository; @CommandLine.Option(names = {"--description", "-d"}, descriptionKey = "scm.repo.create.desc") diff --git a/scm-webapp/src/main/resources/sonia/scm/cli/i18n.properties b/scm-webapp/src/main/resources/sonia/scm/cli/i18n.properties index ab1ff480e1..98c58542a9 100644 --- a/scm-webapp/src/main/resources/sonia/scm/cli/i18n.properties +++ b/scm-webapp/src/main/resources/sonia/scm/cli/i18n.properties @@ -43,6 +43,9 @@ repoCreationDate = Creation Date repoLastModified = Last Modified # picocli translations +usage.parameterListHeading = Parameters:%n +usage.optionListHeading = Options:%n +usage.commandListHeading = Commands:%n usage.descriptionHeading = Description:\u0020 usage.synopsisHeading = Usage:\u0020 diff --git a/scm-webapp/src/main/resources/sonia/scm/cli/i18n_de.properties b/scm-webapp/src/main/resources/sonia/scm/cli/i18n_de.properties index f84c86f992..1e4ae263a9 100644 --- a/scm-webapp/src/main/resources/sonia/scm/cli/i18n_de.properties +++ b/scm-webapp/src/main/resources/sonia/scm/cli/i18n_de.properties @@ -22,12 +22,22 @@ # SOFTWARE. # +# Exception errorCommandFailed= ____________Befehl fehlgeschlagen____________ errorUnknownError= Unbekannter Fehler. Prüfen Sie Ihre Server Logs errorLabel= FEHLER errorExecutionFailed = Fehler transactionId = Transaktions-ID +## Parameter Exception +exception.typeConversion = Ungültiger Eingabewert für Option oder Parameter:\u0020 +exception.missingParameter = Fehlende Parameter:\u0020 +exception.overwrittenOption = Option wurde mehrfach gesetzt:\u0020 +exception.maxValuesExceeded = Zu viele Optionen oder Parameter +exception.mutuallyExclusiveArgs = Gegenseitig ausschließende Argumente übergeben +exception.unmatchedArguments = Ungültige Argumente:\u0020 +exception.missingSubCommand = Weiteres Kommando benötigt + creationDate = Erstellt lastModified = Zuletzt bearbeitet yes = ja @@ -43,6 +53,9 @@ repoCreationDate = Erstellt repoLastModified = Zuletzt bearbeitet # picocli translations +usage.parameterListHeading = Parameter:%n +usage.optionListHeading = Optionen:%n +usage.commandListHeading = Kommandos:%n usage.descriptionHeading = Beschreibung:\u0020 usage.synopsisHeading = Nutzung:\u0020 diff --git a/scm-webapp/src/test/java/sonia/scm/cli/CliExceptionHandlerTest.java b/scm-webapp/src/test/java/sonia/scm/cli/CliExceptionHandlerTest.java index 9e4857c244..13eb9aa460 100644 --- a/scm-webapp/src/test/java/sonia/scm/cli/CliExceptionHandlerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/cli/CliExceptionHandlerTest.java @@ -46,6 +46,8 @@ import java.io.PrintWriter; import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -78,7 +80,8 @@ class CliExceptionHandlerTest { @Mock private CommandLine commandLine; - private CliExceptionHandler handler; + private CliExecutionExceptionHandler executionExceptionHandler; + private CliParameterExceptionHandler parameterExceptionHandler; private final ByteArrayOutputStream errorStream = new ByteArrayOutputStream(); private final PrintWriter stdErr = new PrintWriter(errorStream); @@ -93,12 +96,13 @@ class CliExceptionHandlerTest { @BeforeEach void setUpHandler() { - handler = new CliExceptionHandler(i18nCollector, "en"); + executionExceptionHandler = new CliExecutionExceptionHandler(i18nCollector, "en"); + parameterExceptionHandler = new CliParameterExceptionHandler("en"); } @Test void shouldPrintSimpleException() { - int exitCode = callHandler(new NullPointerException("expected error")); + int exitCode = callExecHandler(new NullPointerException("expected error")); assertThat(errorStream.toString()).contains("Error: expected error"); assertThat(exitCode).isEqualTo(1); @@ -108,7 +112,7 @@ class CliExceptionHandlerTest { void shouldPrintTransactionIdIfPresent() { TransactionId.set("42"); - callHandler(new NullPointerException("expected error")); + callExecHandler(new NullPointerException("expected error")); assertThat(errorStream.toString()).contains("Transaction ID: 42"); } @@ -118,7 +122,7 @@ class CliExceptionHandlerTest { when(i18nCollector.findJson("en")) .thenReturn(Optional.of(new ObjectMapper().readTree(PLUGIN_BUNDLE_EN))); - callHandler(NotFoundException.notFound(new ContextEntry.ContextBuilder())); + callExecHandler(NotFoundException.notFound(new ContextEntry.ContextBuilder())); assertThat(errorStream.toString()).contains("Error: The requested entity could not be found. It may have been deleted in another session."); } @@ -128,7 +132,7 @@ class CliExceptionHandlerTest { when(i18nCollector.findJson("en")) .thenReturn(Optional.of(new ObjectMapper().readTree(PLUGIN_BUNDLE_EN))); - callHandler(new AlreadyExistsException(new Group("test", "hog"))); + callExecHandler(new AlreadyExistsException(new Group("test", "hog"))); assertThat(errorStream.toString()).contains("Error: group with id hog already exists"); } @@ -138,10 +142,17 @@ class CliExceptionHandlerTest { when(i18nCollector.findJson("en")) .thenReturn(Optional.of(new ObjectMapper().readTree(PLUGIN_BUNDLE_EN))); - callHandler(new StoreReadOnlyException("test")); + callExecHandler(new StoreReadOnlyException("test")); assertThat(errorStream.toString()).contains("Error: Store is read only, could not write location test"); } + + @Test + void shouldUseParameterExceptionHandlerWithDefaultExceptionMessages() { + callParamHandler(new CommandLine.OverwrittenOptionException(commandLine, null, "My default exception which will not be overwritten"), ""); + + assertThat(errorStream.toString()).contains("ERROR: My default exception which will not be overwritten"); + } } @Nested @@ -149,12 +160,13 @@ class CliExceptionHandlerTest { @BeforeEach void setUpHandler() { - handler = new CliExceptionHandler(i18nCollector, "de"); + executionExceptionHandler = new CliExecutionExceptionHandler(i18nCollector, "de"); + parameterExceptionHandler = new CliParameterExceptionHandler("de"); } @Test void shouldPrintSimpleException() { - int exitCode = callHandler(new NullPointerException("expected error")); + int exitCode = callExecHandler(new NullPointerException("expected error")); assertThat(errorStream.toString()).contains("Fehler: expected error"); assertThat(exitCode).isEqualTo(1); @@ -165,7 +177,7 @@ class CliExceptionHandlerTest { void shouldPrintTransactionIdIfPresent() { TransactionId.set("42"); - callHandler(new NullPointerException("expected error")); + callExecHandler(new NullPointerException("expected error")); assertThat(errorStream.toString()).contains("Transaktions-ID: 42"); } @@ -175,7 +187,7 @@ class CliExceptionHandlerTest { when(i18nCollector.findJson("de")) .thenReturn(Optional.of(new ObjectMapper().readTree(PLUGIN_BUNDLE_DE))); - callHandler(new StoreReadOnlyException("test")); + callExecHandler(new StoreReadOnlyException("test")); assertThat(errorStream.toString()).contains("Fehler: Ein Datensatz konnte nicht gespeichert werden, da der entsprechende Speicher als schreibgeschützt markiert wurde. Weitere Hinweise finden sich im Log."); } @@ -187,15 +199,34 @@ class CliExceptionHandlerTest { when(i18nCollector.findJson("de")) .thenReturn(Optional.of(new ObjectMapper().readTree(PLUGIN_BUNDLE_DE))); - callHandler(NotFoundException.notFound(new ContextEntry.ContextBuilder())); + callExecHandler(NotFoundException.notFound(new ContextEntry.ContextBuilder())); assertThat(errorStream.toString()).contains("Fehler: The requested entity could not be found. It may have been deleted in another session."); } + + @Test + void shouldUseParamExceptionHandlerWithTranslations() { + CommandLine.Model.ArgSpec argSpec = mock(CommandLine.Model.ArgSpec.class); + when(argSpec.paramLabel()).thenReturn(""); + + callParamHandler(new CommandLine.OverwrittenOptionException(commandLine, argSpec, "Option overwritten"), ""); + + assertThat(errorStream.toString()) + .contains("FEHLER: Option wurde mehrfach gesetzt: "); + } } - private int callHandler(Exception ex) { + private int callExecHandler(Exception ex) { try { - return handler.handleExecutionException(ex, commandLine, null); + return executionExceptionHandler.handleExecutionException(ex, commandLine, null); + } finally { + stdErr.flush(); + } + } + + private void callParamHandler(CommandLine.ParameterException ex, String... args) { + try { + parameterExceptionHandler.handleParseException(ex, args); } finally { stdErr.flush(); } diff --git a/scm-webapp/src/test/java/sonia/scm/cli/CliProcessorTest.java b/scm-webapp/src/test/java/sonia/scm/cli/CliProcessorTest.java index c8f84cc766..f3cc5c342d 100644 --- a/scm-webapp/src/test/java/sonia/scm/cli/CliProcessorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/cli/CliProcessorTest.java @@ -57,7 +57,9 @@ class CliProcessorTest { @Mock private CliExceptionHandlerFactory exceptionHandlerFactory; @Mock - private CliExceptionHandler exceptionHandler; + private CliExecutionExceptionHandler executionExceptionHandler; + @Mock + private CliParameterExceptionHandler parameterExceptionHandler; @Nested class ForDefaultLanguageTest { @@ -65,7 +67,8 @@ class CliProcessorTest { @BeforeEach void setDefaultLocale() { when(context.getLocale()).thenReturn(Locale.ENGLISH); - when(exceptionHandlerFactory.createExceptionHandler("en")).thenReturn(exceptionHandler); + when(exceptionHandlerFactory.createExecutionExceptionHandler("en")).thenReturn(executionExceptionHandler); + when(exceptionHandlerFactory.createParameterExceptionHandler("en")).thenReturn(parameterExceptionHandler); } @Test @@ -119,11 +122,14 @@ class CliProcessorTest { class ForAnotherLanguageTest { @Mock - private CliExceptionHandler germanExceptionHandler; + private CliExecutionExceptionHandler germanExecutionExceptionHandler; + @Mock + private CliParameterExceptionHandler germanParamExceptionHandler; @BeforeEach void setUpOtherLanguage() { - when(exceptionHandlerFactory.createExceptionHandler("de")).thenReturn(germanExceptionHandler); + when(exceptionHandlerFactory.createParameterExceptionHandler("de")).thenReturn(germanParamExceptionHandler); + when(exceptionHandlerFactory.createExecutionExceptionHandler("de")).thenReturn(germanExecutionExceptionHandler); when(context.getLocale()).thenReturn(Locale.GERMAN); } @@ -145,7 +151,8 @@ class CliProcessorTest { void shouldUseExceptionHandlerForOtherLanguage() { executeHierarchyCommands("one", "two", "--help"); - verify(exceptionHandlerFactory).createExceptionHandler("de"); + verify(exceptionHandlerFactory).createExecutionExceptionHandler("de"); + verify(exceptionHandlerFactory).createParameterExceptionHandler("de"); } }