From 2957dd4d459165f78a0621e183860feefecc6a6c 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 e3df7e2461a..e40f3c624ac 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 class Generate extends OpenApiGeneratorCommand { 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 0fdc6c6e99c..2a0c9ac572a 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.nio.file.Paths; 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 @@ public class WorkflowSettings { 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 e0dee13cafe..93087a986e1 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 8e17356e460..aa386a2995f 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 b2ad527e2f0..a60f824278b 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 class CodegenConfigurator { 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 3c5b6c919b8..b3154db6407 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 @@ import java.util.*; */ 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()); }