[core] Fix system properties being immutable (#4447)

When WorkflowSettings was constructed from an existing instance, as is
the case when we deserialize from an external configuration file, it
would result in an error:

Caused by: java.lang.UnsupportedOperationException
        at com.google.common.collect.ImmutableMap.put(ImmutableMap.java:450)
        at org.openapitools.codegen.config.WorkflowSettings$Builder.withSystemProperty(WorkflowSettings.java:465)

This was due to an error in `newBuilder(WorkflowSettings copy)` which
assigned builder.systemProperties with an immutable map. This is
incorrect because everything in the builder should be mutable until
.build() is invoked.

This likely affects CLI/Maven plugin as well for version 4.1.1 through 4.2.0.
This commit is contained in:
Jim Schubert 2019-11-11 08:03:13 -05:00 committed by GitHub
parent 84d3562a0f
commit 7bfc53b3bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 40 additions and 1 deletions

View File

@ -103,7 +103,9 @@ public class WorkflowSettings {
builder.strictSpecBehavior = copy.isStrictSpecBehavior(); builder.strictSpecBehavior = copy.isStrictSpecBehavior();
builder.templatingEngineName = copy.getTemplatingEngineName(); builder.templatingEngineName = copy.getTemplatingEngineName();
builder.ignoreFileOverride = copy.getIgnoreFileOverride(); builder.ignoreFileOverride = copy.getIgnoreFileOverride();
builder.systemProperties = ImmutableMap.copyOf(copy.getSystemProperties());
// this, and any other collections, must be mutable in the builder.
builder.systemProperties = new HashMap<>(copy.getSystemProperties());
// force builder "with" methods to invoke side effects // force builder "with" methods to invoke side effects
builder.withTemplateDir(copy.getTemplateDir()); builder.withTemplateDir(copy.getTemplateDir());
@ -272,6 +274,8 @@ public class WorkflowSettings {
private String templateDir; private String templateDir;
private String templatingEngineName = DEFAULT_TEMPLATING_ENGINE_NAME; private String templatingEngineName = DEFAULT_TEMPLATING_ENGINE_NAME;
private String ignoreFileOverride; private String ignoreFileOverride;
// NOTE: All collections must be mutable in the builder, and copied to a new immutable collection in .build()
private Map<String, String> systemProperties = new HashMap<>();; private Map<String, String> systemProperties = new HashMap<>();;
private Builder() { private Builder() {

View File

@ -19,6 +19,7 @@ package org.openapitools.codegen.config;
import org.testng.annotations.Test; import org.testng.annotations.Test;
import java.nio.file.Paths; import java.nio.file.Paths;
import java.util.Map;
import static org.testng.Assert.*; import static org.testng.Assert.*;
@ -48,6 +49,25 @@ public class WorkflowSettingsTest {
assertTrue(settings.isStrictSpecBehavior()); assertTrue(settings.isStrictSpecBehavior());
} }
@Test
public void newBuilderFromCopyShouldMutateSystemProperties(){
WorkflowSettings original = WorkflowSettings.newBuilder()
.withOutputDir("output")
.withVerbose(true)
.withSkipOverwrite(false)
.withSystemProperty("first", "1st")
.build();
WorkflowSettings modified = WorkflowSettings.newBuilder(original)
.withSystemProperty("second", "2nd")
.build();
Map<String, String> properties = modified.getSystemProperties();
assertEquals(properties.size(), 2, "System Properties map should allow mutation when invoked via copy constructor");
assertEquals(properties.getOrDefault("first", ""), "1st");
assertEquals(properties.getOrDefault("second", ""), "2nd");
}
private void assertOnChangesToDefaults(WorkflowSettings defaults) { private void assertOnChangesToDefaults(WorkflowSettings defaults) {
WorkflowSettings settings = WorkflowSettings.newBuilder() WorkflowSettings settings = WorkflowSettings.newBuilder()
.withOutputDir("output") .withOutputDir("output")

View File

@ -11,6 +11,7 @@ gradle openApiGenerate
gradle openApiMeta gradle openApiMeta
gradle openApiValidate gradle openApiValidate
gradle buildGoSdk gradle buildGoSdk
gradle buildDotnetSdk
gradle generateGoWithInvalidSpec gradle generateGoWithInvalidSpec
``` ```

View File

@ -65,6 +65,20 @@ task buildGoSdk(type: org.openapitools.generator.gradle.plugin.tasks.GenerateTas
] ]
} }
task buildDotnetSdk(type: org.openapitools.generator.gradle.plugin.tasks.GenerateTask){
generatorName = "csharp-netcore"
inputSpec = "$rootDir/petstore-v3.0.yaml".toString()
additionalProperties = [
packageGuid: "{321C8C3F-0156-40C1-AE42-D59761FB9B6C}",
useCompareNetObjects: "true"
]
outputDir = "$buildDir/csharp-netcore".toString()
systemProperties = [
models: "",
apis : "",
]
}
task generateGoWithInvalidSpec(type: org.openapitools.generator.gradle.plugin.tasks.GenerateTask){ task generateGoWithInvalidSpec(type: org.openapitools.generator.gradle.plugin.tasks.GenerateTask){
validateSpec = true validateSpec = true
generatorName = "go" generatorName = "go"