From fd444366146d8ac308c64e124f73b916a7129e0c Mon Sep 17 00:00:00 2001 From: Jim Schubert Date: Mon, 28 May 2018 21:19:14 -0400 Subject: [PATCH 1/3] Errors in Generate/Validate print to stderr/exit 1 Generate and Validate exposed exceptions rather than user-friendly messages when an error occurred. In generate, this could happen for numerous reasons, but the most likely is a user typing (or guessing) an invalid generator name. In Validate, an error was exposed if there were any validation errors in a spec. New behavior: * Generate now exposes a typed exception when a generator cannot be loaded by name. This allows consistent messaging for load failures. * Generate now presents guidance on failure (check the spelling and try again). This is purely a usability improvement. * Validate now writes validation errors to stderr and exits with code 1. --- .../openapitools/codegen/cmd/Generate.java | 11 ++- .../openapitools/codegen/cmd/Validate.java | 18 +++-- .../codegen/CodegenConfigLoader.java | 2 +- .../codegen/GeneratorNotFoundException.java | 80 +++++++++++++++++++ 4 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 modules/openapi-generator/src/main/java/org/openapitools/codegen/GeneratorNotFoundException.java diff --git a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java index 59eeb282a5a5..4410b80c1c45 100644 --- a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java +++ b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java @@ -22,6 +22,7 @@ import org.openapitools.codegen.ClientOptInput; import org.openapitools.codegen.CodegenConstants; import org.openapitools.codegen.DefaultGenerator; +import org.openapitools.codegen.GeneratorNotFoundException; import org.openapitools.codegen.config.CodegenConfigurator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -309,8 +310,14 @@ public void run() { applyAdditionalPropertiesKvpList(additionalProperties, configurator); applyLanguageSpecificPrimitivesCsvList(languageSpecificPrimitives, configurator); applyReservedWordsMappingsKvpList(reservedWordsMappings, configurator); - final ClientOptInput clientOptInput = configurator.toClientOptInput(); - new DefaultGenerator().opts(clientOptInput).generate(); + try { + final ClientOptInput clientOptInput = configurator.toClientOptInput(); + new DefaultGenerator().opts(clientOptInput).generate(); + } catch (GeneratorNotFoundException e) { + System.err.println(e.getMessage()); + System.err.println("Check the spelling of the generator's name and try again."); + System.exit(1); + } } } diff --git a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java index 2e79a929685e..ff01912306b0 100644 --- a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java +++ b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java @@ -36,18 +36,24 @@ public class Validate implements Runnable { @Override public void run() { - System.out.println("Validating spec file (" + spec + ")"); + System.out.println("Validating spec (" + spec + ")"); SwaggerParseResult result = new OpenAPIParser().readLocation(spec, null, null); List messageList = result.getMessages(); Set messages = new HashSet(messageList); - for (String message : messages) { - System.out.println(message); - } - if (messages.size() > 0) { - throw new ValidateException(); + StringBuilder sb = new StringBuilder(); + sb.append("\n"); + for (String message : messages) { + sb.append(String.format("\t- %s\n", message)); + } + sb.append("\n"); + sb.append("Spec has errors."); + System.err.println(sb.toString()); + System.exit(1); + } else { + System.out.println("No validation errors detected."); } } } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConfigLoader.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConfigLoader.java index 29ee94eb52af..29c0898e568e 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConfigLoader.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConfigLoader.java @@ -47,7 +47,7 @@ public static CodegenConfig forName(String name) { try { return (CodegenConfig) Class.forName(name).newInstance(); } catch (Exception e) { - throw new RuntimeException("Can't load config class with name '".concat(name) + "'\nAvailable:\n" + availableConfigs.toString()); + throw new GeneratorNotFoundException("Can't load config class with name '".concat(name) + "'\nAvailable:\n" + availableConfigs.toString(), e); } } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/GeneratorNotFoundException.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/GeneratorNotFoundException.java new file mode 100644 index 000000000000..f01d89cce268 --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/GeneratorNotFoundException.java @@ -0,0 +1,80 @@ +package org.openapitools.codegen; + +/** + * Typed exception exposing issues with loading generators (e.g. by name). + */ +@SuppressWarnings("unused") +public class GeneratorNotFoundException extends RuntimeException { + /** + * Constructs a new runtime exception with {@code null} as its + * detail message. The cause is not initialized, and may subsequently be + * initialized by a call to {@link #initCause}. + */ + public GeneratorNotFoundException() { + } + + /** + * Constructs a new runtime exception with the specified detail message. + * The cause is not initialized, and may subsequently be initialized by a + * call to {@link #initCause}. + * + * @param message the detail message. The detail message is saved for + * later retrieval by the {@link #getMessage()} method. + */ + public GeneratorNotFoundException(String message) { + super(message); + } + + /** + * Constructs a new runtime exception with the specified detail message and + * cause.

Note that the detail message associated with + * {@code cause} is not automatically incorporated in + * this runtime exception's detail message. + * + * @param message the detail message (which is saved for later retrieval + * by the {@link #getMessage()} method). + * @param cause the cause (which is saved for later retrieval by the + * {@link #getCause()} method). (A null value is + * permitted, and indicates that the cause is nonexistent or + * unknown.) + * @since 1.4 + */ + public GeneratorNotFoundException(String message, Throwable cause) { + super(message, cause); + } + + /** + * Constructs a new runtime exception with the specified cause and a + * detail message of (cause==null ? null : cause.toString()) + * (which typically contains the class and detail message of + * cause). This constructor is useful for runtime exceptions + * that are little more than wrappers for other throwables. + * + * @param cause the cause (which is saved for later retrieval by the + * {@link #getCause()} method). (A null value is + * permitted, and indicates that the cause is nonexistent or + * unknown.) + * @since 1.4 + */ + public GeneratorNotFoundException(Throwable cause) { + super(cause); + } + + /** + * Constructs a new runtime exception with the specified detail + * message, cause, suppression enabled or disabled, and writable + * stack trace enabled or disabled. + * + * @param message the detail message. + * @param cause the cause. (A {@code null} value is permitted, + * and indicates that the cause is nonexistent or unknown.) + * @param enableSuppression whether or not suppression is enabled + * or disabled + * @param writableStackTrace whether or not the stack trace should + * be writable + * @since 1.7 + */ + public GeneratorNotFoundException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) { + super(message, cause, enableSuppression, writableStackTrace); + } +} From 13843ada9437f4f48067e24f62802c63be601b91 Mon Sep 17 00:00:00 2001 From: Jim Schubert Date: Mon, 28 May 2018 21:41:03 -0400 Subject: [PATCH 2/3] Improve err messages: config-help/required opts. config-help now presents same error for invalid generator names as the 'generate' command. Options which are required, and those which require a value, now present a user-friendly hint at the error and exit with code 1 (rather than an uncaught exception). --- .../codegen/OpenAPIGenerator.java | 11 +++++++-- .../openapitools/codegen/cmd/ConfigHelp.java | 21 ++++++++++------ .../openapitools/codegen/cmd/Generate.java | 2 +- .../openapitools/codegen/cmd/Validate.java | 8 +++---- .../codegen/cmd/ValidateException.java | 24 ------------------- 5 files changed, 28 insertions(+), 38 deletions(-) delete mode 100644 modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ValidateException.java diff --git a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/OpenAPIGenerator.java b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/OpenAPIGenerator.java index 779ee3c90544..85b6379857fe 100644 --- a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/OpenAPIGenerator.java +++ b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/OpenAPIGenerator.java @@ -19,6 +19,8 @@ import io.airlift.airline.Cli; import io.airlift.airline.Help; +import io.airlift.airline.ParseOptionMissingException; +import io.airlift.airline.ParseOptionMissingValueException; import org.openapitools.codegen.cmd.*; import java.util.Arrays; @@ -53,8 +55,6 @@ public static void main(String[] args) { Version.class ); - builder.build().parse(args).run(); - // If CLI is run without a command, consider this an error. // We can check against empty args because unrecognized arguments/commands result in an exception. // This is useful to exit with status 1, for example, so that misconfigured scripts fail fast. @@ -64,5 +64,12 @@ public static void main(String[] args) { if (args.length == 0) { System.exit(1); } + + try { + builder.build().parse(args).run(); + } catch (ParseOptionMissingException | ParseOptionMissingValueException e) { + System.err.printf("[error] %s%n", e.getMessage()); + System.exit(1); + } } } diff --git a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ConfigHelp.java b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ConfigHelp.java index 76898b760801..ce2dff5c61b6 100644 --- a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ConfigHelp.java +++ b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ConfigHelp.java @@ -22,6 +22,7 @@ import org.openapitools.codegen.CliOption; import org.openapitools.codegen.CodegenConfig; import org.openapitools.codegen.CodegenConfigLoader; +import org.openapitools.codegen.GeneratorNotFoundException; @Command(name = "config-help", description = "Config help for chosen lang") public class ConfigHelp implements Runnable { @@ -32,14 +33,20 @@ public class ConfigHelp implements Runnable { @Override public void run() { - System.out.println(); - CodegenConfig config = CodegenConfigLoader.forName(lang); - System.out.println("CONFIG OPTIONS"); - for (CliOption langCliOption : config.cliOptions()) { - System.out.println("\t" + langCliOption.getOpt()); - System.out.println("\t " - + langCliOption.getOptionHelp().replaceAll("\n", "\n\t ")); + try { + CodegenConfig config = CodegenConfigLoader.forName(lang); System.out.println(); + System.out.println("CONFIG OPTIONS"); + for (CliOption langCliOption : config.cliOptions()) { + System.out.println("\t" + langCliOption.getOpt()); + System.out.println("\t " + + langCliOption.getOptionHelp().replaceAll("\n", System.lineSeparator() + "\t ")); + System.out.println(); + } + } catch (GeneratorNotFoundException e) { + System.err.println(e.getMessage()); + System.err.println("[error] Check the spelling of the generator's name and try again."); + System.exit(1); } } } diff --git a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java index 4410b80c1c45..e0cc402d4248 100644 --- a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java +++ b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java @@ -316,7 +316,7 @@ public void run() { new DefaultGenerator().opts(clientOptInput).generate(); } catch (GeneratorNotFoundException e) { System.err.println(e.getMessage()); - System.err.println("Check the spelling of the generator's name and try again."); + System.err.println("[error] Check the spelling of the generator's name and try again."); System.exit(1); } } diff --git a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java index ff01912306b0..4da5b318c6a1 100644 --- a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java +++ b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java @@ -44,12 +44,12 @@ public void run() { if (messages.size() > 0) { StringBuilder sb = new StringBuilder(); - sb.append("\n"); + sb.append(System.lineSeparator()); for (String message : messages) { - sb.append(String.format("\t- %s\n", message)); + sb.append(String.format("\t- %s%s", message, System.lineSeparator())); } - sb.append("\n"); - sb.append("Spec has errors."); + sb.append(System.lineSeparator()); + sb.append("[error] Spec is invalid."); System.err.println(sb.toString()); System.exit(1); } else { diff --git a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ValidateException.java b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ValidateException.java deleted file mode 100644 index 193553ab0643..000000000000 --- a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ValidateException.java +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright 2018 OpenAPI-Generator Contributors (https://openapi-generator.tech) - * Copyright 2018 SmartBear Software - * - * 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 - * - * http://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.openapitools.codegen.cmd; - -/** - * Created by takuro on 2017/05/02. - */ -public class ValidateException extends RuntimeException { -} From a08ec69991686d00bf937ceb68b94d8a2247a674 Mon Sep 17 00:00:00 2001 From: Jim Schubert Date: Mon, 28 May 2018 21:52:32 -0400 Subject: [PATCH 3/3] Log missing -g error to stderr rather than LOGGER --- .../src/main/java/org/openapitools/codegen/cmd/Generate.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java index e0cc402d4248..7382a6d55956 100644 --- a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java +++ b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java @@ -27,7 +27,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.apache.commons.lang3.StringUtils.isEmpty; import static org.openapitools.codegen.config.CodegenConfiguratorUtils.*; import static org.apache.commons.lang3.StringUtils.isNotEmpty; @@ -227,7 +226,7 @@ public void run() { LOGGER.warn("The '--lang' and '-l' are deprecated and may reference language names only in the next major release (4.0). Please use --generator-name /-g instead."); configurator.setGeneratorName(lang); } else { - LOGGER.error("A generator name (--generator-name / -g) is required. "); + System.err.println("[error] A generator name (--generator-name / -g) is required."); System.exit(1); }