From 2c78bc53e57dc895d6c8e378afacad176746903e Mon Sep 17 00:00:00 2001 From: Jim Schubert Date: Wed, 1 Apr 2020 23:11:02 -0400 Subject: [PATCH] [cli] Add --global-property for -D replacement (#5687) -D option has been deprecated as it was previously used to: * Pass "system properties" * Pass additional properties This was confusing because we already have --additional-properties and because Java System Properties are passed as -D before program arguments. Confusion around the -D option had existed for some time, but when we introduced the thread-safe GlobalSettings to avoid overwriting Java System Properties, we created a hard break from Java System Properties in the generator. This also disconnected the previous "system properties" from accepting additional properties. Once these newly deprecated methods are removed, we will have a clear separation of concerns between: * Java System Properties * Global generator properties (used as workflow context) * Additional properties (used as generator options) This commit marks multiple places for cleanup in 5.0. These will be breaking changes, and lower effort to break in 5.0 with deprecation warnings now rather than adding sibling properties throughout the code and potentially introducing logic errors. --- .../openapitools/codegen/cmd/Generate.java | 15 +++---- .../codegen/config/WorkflowSettings.java | 11 ++++- .../gradle/plugin/tasks/GenerateTask.kt | 1 + .../codegen/plugin/CodeGenMojo.java | 1 + .../codegen/config/CodegenConfigurator.java | 1 + .../config/CodegenConfiguratorUtils.java | 41 +++++++++++++++++-- 6 files changed, 58 insertions(+), 12 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 e3df7e2461a6..e40f3c624acc 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 @@ -71,12 +71,13 @@ public class Generate extends OpenApiGeneratorCommand { + "Pass in a URL-encoded string of name:header with a comma separating multiple values") private String auth; + // TODO: Remove -D short option in 5.0 @Option( - name = {"-D"}, - title = "system properties", - description = "sets specified system properties in " + name = {"-D", "--global-property"}, + title = "global properties", + description = "sets specified global properties (previously called 'system properties') in " + "the format of name=value,name=value (or multiple options, each with name=value)") - private List systemProperties = new ArrayList<>(); + private List globalProperties = new ArrayList<>(); @Option( name = {"-c", "--config"}, @@ -403,9 +404,9 @@ public void execute() { configurator.setStrictSpecBehavior(strictSpecBehavior); } - if (systemProperties != null && !systemProperties.isEmpty()) { - System.err.println("[DEPRECATED] -D arguments after 'generate' are application arguments and not Java System Properties, please consider changing to -p, or apply your options to JAVA_OPTS, or move the -D arguments before the jar option."); - applySystemPropertiesKvpList(systemProperties, configurator); + if (globalProperties != null && !globalProperties.isEmpty()) { + System.err.println("[DEPRECATED] -D arguments after 'generate' are application arguments and not Java System Properties, please consider changing to --global-property, apply your system properties to JAVA_OPTS, or move the -D arguments before the jar option."); + applyGlobalPropertiesKvpList(globalProperties, configurator); } applyInstantiationTypesKvpList(instantiationTypes, configurator); applyImportMappingsKvpList(importMappings, configurator); diff --git a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/config/WorkflowSettings.java b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/config/WorkflowSettings.java index 0fdc6c6e99c3..2a0c9ac572af 100644 --- a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/config/WorkflowSettings.java +++ b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/config/WorkflowSettings.java @@ -28,13 +28,14 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.concurrent.atomic.AtomicLong; /** * Represents those settings applied to a generation workflow. */ @SuppressWarnings("WeakerAccess") public class WorkflowSettings { - + private static final AtomicLong lastWarning = new AtomicLong(0); private static final Logger LOGGER = LoggerFactory.getLogger(WorkflowSettings.class); public static final String DEFAULT_OUTPUT_DIR = "."; public static final boolean DEFAULT_VERBOSE = false; @@ -77,7 +78,15 @@ private WorkflowSettings(Builder builder) { this.templateDir = builder.templateDir; this.templatingEngineName = builder.templatingEngineName; this.ignoreFileOverride = builder.ignoreFileOverride; + // TODO: rename to globalProperties for 5.0 this.systemProperties = ImmutableMap.copyOf(builder.systemProperties); + if (this.systemProperties.size() > 0) { + // write no more than every 5s. This is temporary until version 5.0 as once(Logger) is not accessible here. + // thread contention may cause this to write more than once, but this is just an attempt to reduce noise + if (System.currentTimeMillis() - lastWarning.getAndUpdate(x -> System.currentTimeMillis()) > 5000) { + LOGGER.warn("systemProperties will be renamed to globalProperties in version 5.0"); + } + } } /** diff --git a/modules/openapi-generator-gradle-plugin/src/main/kotlin/org/openapitools/generator/gradle/plugin/tasks/GenerateTask.kt b/modules/openapi-generator-gradle-plugin/src/main/kotlin/org/openapitools/generator/gradle/plugin/tasks/GenerateTask.kt index e0dee13cafe4..93087a986e15 100644 --- a/modules/openapi-generator-gradle-plugin/src/main/kotlin/org/openapitools/generator/gradle/plugin/tasks/GenerateTask.kt +++ b/modules/openapi-generator-gradle-plugin/src/main/kotlin/org/openapitools/generator/gradle/plugin/tasks/GenerateTask.kt @@ -574,6 +574,7 @@ open class GenerateTask : DefaultTask() { } if (systemProperties.isPresent) { + // TODO: rename to globalProperties in 5.0 systemProperties.get().forEach { entry -> configurator.addSystemProperty(entry.key, entry.value) } diff --git a/modules/openapi-generator-maven-plugin/src/main/java/org/openapitools/codegen/plugin/CodeGenMojo.java b/modules/openapi-generator-maven-plugin/src/main/java/org/openapitools/codegen/plugin/CodeGenMojo.java index 8e17356e4601..aa386a2995f3 100644 --- a/modules/openapi-generator-maven-plugin/src/main/java/org/openapitools/codegen/plugin/CodeGenMojo.java +++ b/modules/openapi-generator-maven-plugin/src/main/java/org/openapitools/codegen/plugin/CodeGenMojo.java @@ -408,6 +408,7 @@ public class CodeGenMojo extends AbstractMojo { @Parameter(defaultValue = "true", property = "openapi.generator.maven.plugin.addCompileSourceRoot") private boolean addCompileSourceRoot = true; + // TODO: Rename to global properties in version 5.0 @Parameter protected Map environmentVariables = new HashMap<>(); diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfigurator.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfigurator.java index b2ad527e2f04..a60f824278b1 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfigurator.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfigurator.java @@ -178,6 +178,7 @@ public CodegenConfigurator addLanguageSpecificPrimitive(String value) { return this; } + // TODO: rename this and other references to "global property" rather than "system property" public CodegenConfigurator addSystemProperty(String key, String value) { this.systemProperties.put(key, value); workflowSettingsBuilder.withSystemProperty(key, value); diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfiguratorUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfiguratorUtils.java index 3c5b6c919b8f..b3154db6407a 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfiguratorUtils.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfiguratorUtils.java @@ -42,14 +42,47 @@ */ public final class CodegenConfiguratorUtils { + /** + * Applies "system" properties to the configurator as global properties. + * + * @deprecated + * This method is deprecated due to confusion around the tool's use of system properties. We called these system properties + * in the past and accepted them via CLI option -D. This lead to confusion between true Java System Properties and generator-specific + * "system level properties". They've since been renamed as "Global Properties". Please use {@link CodegenConfiguratorUtils#applyGlobalPropertiesKvpList(List, CodegenConfigurator)}. + * + * @param systemProperties List of properties to be globally available throughout the generator execution. + * @param configurator The {@link CodegenConfigurator} instance to configure. + */ + @Deprecated public static void applySystemPropertiesKvpList(List systemProperties, CodegenConfigurator configurator) { - for(String propString : systemProperties) { - applySystemPropertiesKvp(propString, configurator); + // TODO: Remove in 5.0 + applyGlobalPropertiesKvpList(systemProperties, configurator); + } + + /** + * Applies a key-value pair of strings as "system" properties to the configurator as global properties. + * + * @deprecated + * This method is deprecated due to confusing between Java Sytsem Properties and generator-specific "system-level properties". + * They've since been renamed as "Global Properties". Please use {@link CodegenConfiguratorUtils#applyGlobalPropertiesKvp(String, CodegenConfigurator)}. + * + * @param systemProperties List of properties to be globally available throughout the generator execution. + * @param configurator The {@link CodegenConfigurator} instance to configure. + */ + @Deprecated + public static void applySystemPropertiesKvp(String systemProperties, CodegenConfigurator configurator) { + // TODO: Remove in 5.0 + applyGlobalPropertiesKvp(systemProperties, configurator); + } + + public static void applyGlobalPropertiesKvpList(List globalProperties, CodegenConfigurator configurator) { + for(String propString : globalProperties) { + applyGlobalPropertiesKvp(propString, configurator); } } - public static void applySystemPropertiesKvp(String systemProperties, CodegenConfigurator configurator) { - final Map map = createMapFromKeyValuePairs(systemProperties); + public static void applyGlobalPropertiesKvp(String globalProperties, CodegenConfigurator configurator) { + final Map map = createMapFromKeyValuePairs(globalProperties); for (Map.Entry entry : map.entrySet()) { configurator.addSystemProperty(entry.getKey(), entry.getValue()); }