[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.
This commit is contained in:
Jim Schubert 2020-04-01 23:11:02 -04:00 committed by GitHub
parent e14e5fccf3
commit 2957dd4d45
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 58 additions and 12 deletions

View File

@ -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<String> systemProperties = new ArrayList<>();
private List<String> 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);

View File

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

View File

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

View File

@ -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<String, String> environmentVariables = new HashMap<>();

View File

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

View File

@ -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<String> 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<String> globalProperties, CodegenConfigurator configurator) {
for(String propString : globalProperties) {
applyGlobalPropertiesKvp(propString, configurator);
}
}
public static void applySystemPropertiesKvp(String systemProperties, CodegenConfigurator configurator) {
final Map<String, String> map = createMapFromKeyValuePairs(systemProperties);
public static void applyGlobalPropertiesKvp(String globalProperties, CodegenConfigurator configurator) {
final Map<String, String> map = createMapFromKeyValuePairs(globalProperties);
for (Map.Entry<String, String> entry : map.entrySet()) {
configurator.addSystemProperty(entry.getKey(), entry.getValue());
}