Translate cli parameter exceptions (#1997)

Translate CLI parameter exceptions to german. We explictly do not translate the default english exceptions to custom ones since the default messages are more specific/helpful.

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);
   }
 }

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;
   }

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);
+  }
+}
+
+
+

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);
   }
 

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 = "<name>")
   private String repository;
 
   @CommandLine.Option(names = {"--description", "-d"}, descriptionKey = "scm.repo.create.desc")

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
 

scm-webapp/src/main/resources/sonia/scm/cli/i18n_de.properties

@@ -22,12 +22,22 @@
 # SOFTWARE.
 #
 
+# Exception
 errorCommandFailed= ____________Befehl fehlgeschlagen____________
 errorUnknownError= Unbekannter Fehler. Pr<50>fen Sie Ihre Server Logs
 errorLabel= FEHLER
 errorExecutionFailed = Fehler
 transactionId = Transaktions-ID
 
+## Parameter Exception
+exception.typeConversion = Ung<EFBFBD>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<69>ende Argumente <20>bergeben
+exception.unmatchedArguments = Ung<EFBFBD>ltige Argumente:\u0020
+exception.missingSubCommand = Weiteres Kommando ben<65>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
 

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("<name>");
+
+      callParamHandler(new CommandLine.OverwrittenOptionException(commandLine, argSpec, "Option overwritten"), "<name>");
+
+      assertThat(errorStream.toString())
+        .contains("FEHLER: Option wurde mehrfach gesetzt: <name>");
+    }
   }
 
-  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();
     }

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");
     }
   }