[BUG][CLI][GENERATOR] NullPointer when not setting outputDir (updated) (#3752)

* Fixes NPE when no outputDir is set

* Fix behaviors of default values for values not provided by user

* Easier handling of default behavior in settings.

* Fixes for dynamic config deserialization (specifically, ruby client sample fix)

* Tests for WorkflowSettings (defaults, modified defaults, nulls)

* Test modification of WorkflowSettings defaults for both class constructor and builder

Author: jimschubert

bin/ruby-petstore-faraday.json

@@ -2,5 +2,6 @@
   "gemName": "petstore",
   "moduleName": "Petstore",
   "library": "faraday",
-  "gemVersion": "1.0.0"
+  "gemVersion": "1.0.0",
+  "strictSpecBehavior": false
 }

bin/ruby-petstore.json

@@ -2,5 +2,6 @@
   "gemName": "petstore",
   "library": "typhoeus",
   "moduleName": "Petstore",
-  "gemVersion": "1.0.0"
+  "gemVersion": "1.0.0",
+  "strictSpecBehavior": false
 }

modules/openapi-generator-core/src/main/java/org/openapitools/codegen/config/GeneratorSettings.java

@@ -421,13 +421,27 @@ public static Builder newBuilder(GeneratorSettings copy) {
         builder.artifactId = copy.getArtifactId();
         builder.artifactVersion = copy.getArtifactVersion();
         builder.library = copy.getLibrary();
-        builder.instantiationTypes = new HashMap<>(copy.getInstantiationTypes());
-        builder.typeMappings = new HashMap<>(copy.getTypeMappings());
-        builder.additionalProperties = new HashMap<>(copy.getAdditionalProperties());
-        builder.importMappings = new HashMap<>(copy.getImportMappings());
-        builder.languageSpecificPrimitives = new HashSet<>(copy.getLanguageSpecificPrimitives());
-        builder.reservedWordMappings = new HashMap<>(copy.getReservedWordMappings());
-        builder.serverVariables = new HashMap<>(copy.getServerVariables());
+        if (copy.getInstantiationTypes() != null) {
+            builder.instantiationTypes.putAll(copy.getInstantiationTypes());
+        }
+        if (copy.getTypeMappings() != null) {
+            builder.typeMappings.putAll(copy.getTypeMappings());
+        }
+        if (copy.getAdditionalProperties() != null) {
+            builder.additionalProperties.putAll(copy.getAdditionalProperties());
+        }
+        if (copy.getImportMappings() != null) {
+            builder.importMappings.putAll(copy.getImportMappings());
+        }
+        if (copy.getLanguageSpecificPrimitives() != null) {
+            builder.languageSpecificPrimitives.addAll(copy.getLanguageSpecificPrimitives());
+        }
+        if (copy.getReservedWordMappings() != null) {
+            builder.reservedWordMappings.putAll(copy.getReservedWordMappings());
+        }
+        if (copy.getServerVariables() != null) {
+            builder.serverVariables.putAll(copy.getServerVariables());
+        }
         builder.gitUserId = copy.getGitUserId();
         builder.gitRepoId = copy.getGitRepoId();
         builder.releaseNote = copy.getReleaseNote();

modules/openapi-generator-core/src/main/java/org/openapitools/codegen/config/WorkflowSettings.java

@@ -33,61 +33,64 @@
 public class WorkflowSettings {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(WorkflowSettings.class);
+    public static final String DEFAULT_OUTPUT_DIR = ".";
+    public static final boolean DEFAULT_VERBOSE = false;
+    public static final boolean DEFAULT_SKIP_OVERWRITE = false;
+    public static final boolean DEFAULT_REMOVE_OPERATION_ID_PREFIX = false;
+    public static final boolean DEFAULT_LOG_TO_STDERR = false;
+    public static final boolean DEFAULT_VALIDATE_SPEC = true;
+    public static final boolean DEFAULT_ENABLE_POST_PROCESS_FILE = false;
+    public static final boolean DEFAULT_ENABLE_MINIMAL_UPDATE = false;
+    public static final boolean DEFAULT_STRICT_SPEC_BEHAVIOR = true;
+    public static final String DEFAULT_TEMPLATING_ENGINE_NAME = "mustache";
+    public static final ImmutableMap<String, String> DEFAULT_SYSTEM_PROPERTIES = ImmutableMap.of();
 
     private String inputSpec;
-    private String outputDir;
-    private boolean verbose;
-    private boolean skipOverwrite;
-    private boolean removeOperationIdPrefix;
-    private boolean logToStderr;
-    private boolean validateSpec;
-    private boolean enablePostProcessFile;
-    private boolean enableMinimalUpdate;
-    private boolean strictSpecBehavior;
+    private String outputDir = DEFAULT_OUTPUT_DIR;
+    private boolean verbose = DEFAULT_VERBOSE;
+    private boolean skipOverwrite = DEFAULT_SKIP_OVERWRITE;
+    private boolean removeOperationIdPrefix = DEFAULT_REMOVE_OPERATION_ID_PREFIX;
+    private boolean logToStderr = DEFAULT_LOG_TO_STDERR;
+    private boolean validateSpec = DEFAULT_VALIDATE_SPEC;
+    private boolean enablePostProcessFile = DEFAULT_ENABLE_POST_PROCESS_FILE;
+    private boolean enableMinimalUpdate = DEFAULT_ENABLE_MINIMAL_UPDATE;
+    private boolean strictSpecBehavior = DEFAULT_STRICT_SPEC_BEHAVIOR;
     private String templateDir;
-    private String templatingEngineName;
+    private String templatingEngineName = DEFAULT_TEMPLATING_ENGINE_NAME;
     private String ignoreFileOverride;
-    private ImmutableMap<String, String> systemProperties;
+    private ImmutableMap<String, String> systemProperties = DEFAULT_SYSTEM_PROPERTIES;
 
     private WorkflowSettings(Builder builder) {
-        setDefaults();
-        inputSpec = builder.inputSpec;
-        outputDir = builder.outputDir;
-        verbose = builder.verbose;
-        skipOverwrite = builder.skipOverwrite;
-        removeOperationIdPrefix = builder.removeOperationIdPrefix;
-        logToStderr = builder.logToStderr;
-        validateSpec = builder.validateSpec;
-        enablePostProcessFile = builder.enablePostProcessFile;
-        enableMinimalUpdate = builder.enableMinimalUpdate;
-        strictSpecBehavior = builder.strictSpecBehavior;
-        templateDir = builder.templateDir;
-        templatingEngineName = builder.templatingEngineName;
-        ignoreFileOverride = builder.ignoreFileOverride;
-        systemProperties = ImmutableMap.copyOf(builder.systemProperties);
+        this.inputSpec = builder.inputSpec;
+        this.outputDir = builder.outputDir;
+        this.verbose = builder.verbose;
+        this.skipOverwrite = builder.skipOverwrite;
+        this.removeOperationIdPrefix = builder.removeOperationIdPrefix;
+        this.logToStderr = builder.logToStderr;
+        this.validateSpec = builder.validateSpec;
+        this.enablePostProcessFile = builder.enablePostProcessFile;
+        this.enableMinimalUpdate = builder.enableMinimalUpdate;
+        this.strictSpecBehavior = builder.strictSpecBehavior;
+        this.templateDir = builder.templateDir;
+        this.templatingEngineName = builder.templatingEngineName;
+        this.ignoreFileOverride = builder.ignoreFileOverride;
+        this.systemProperties = ImmutableMap.copyOf(builder.systemProperties);
     }
 
     /**
      * Instantiates a new workflow settings.
      */
     @SuppressWarnings("unused")
     public WorkflowSettings() {
-        setDefaults();
-        systemProperties = ImmutableMap.of();
-    }
 
-    private void setDefaults(){
-        validateSpec = true;
-        strictSpecBehavior = true;
-        outputDir = ".";
     }
 
     public static Builder newBuilder() {
         return new Builder();
     }
 
     public static Builder newBuilder(WorkflowSettings copy) {
-        Builder builder = new Builder();
+        Builder builder = newBuilder();
         builder.inputSpec = copy.getInputSpec();
         builder.outputDir = copy.getOutputDir();
         builder.verbose = copy.isVerbose();
@@ -257,32 +260,34 @@ public Map<String, String> getSystemProperties() {
     @SuppressWarnings("unused")
     public static final class Builder {
         private String inputSpec;
-        private String outputDir;
-        private boolean verbose;
-        private boolean skipOverwrite;
-        private boolean removeOperationIdPrefix;
-        private boolean logToStderr;
-        private boolean validateSpec;
-        private boolean enablePostProcessFile;
-        private boolean enableMinimalUpdate;
-        private boolean strictSpecBehavior;
+        private String outputDir = DEFAULT_OUTPUT_DIR;
+        private Boolean verbose = DEFAULT_VERBOSE;
+        private Boolean skipOverwrite = DEFAULT_SKIP_OVERWRITE;
+        private Boolean removeOperationIdPrefix = DEFAULT_REMOVE_OPERATION_ID_PREFIX;
+        private Boolean logToStderr = DEFAULT_LOG_TO_STDERR;
+        private Boolean validateSpec = DEFAULT_VALIDATE_SPEC;
+        private Boolean enablePostProcessFile = DEFAULT_ENABLE_POST_PROCESS_FILE;
+        private Boolean enableMinimalUpdate = DEFAULT_ENABLE_MINIMAL_UPDATE;
+        private Boolean strictSpecBehavior = DEFAULT_STRICT_SPEC_BEHAVIOR;
         private String templateDir;
-        private String templatingEngineName;
+        private String templatingEngineName = DEFAULT_TEMPLATING_ENGINE_NAME;
         private String ignoreFileOverride;
-        private Map<String, String> systemProperties;
+        private Map<String, String> systemProperties = new HashMap<>();;
 
         private Builder() {
-            systemProperties = new HashMap<>();
         }
 
+
         /**
          * Sets the {@code inputSpec} and returns a reference to this Builder so that the methods can be chained together.
          *
          * @param inputSpec the {@code inputSpec} to set
          * @return a reference to this Builder
          */
         public Builder withInputSpec(String inputSpec) {
-            this.inputSpec = inputSpec;
+            if (inputSpec != null) {
+                this.inputSpec = inputSpec;
+            }
             return this;
         }
 
@@ -293,7 +298,11 @@ public Builder withInputSpec(String inputSpec) {
          * @return a reference to this Builder
          */
         public Builder withOutputDir(String outputDir) {
-            this.outputDir = Paths.get(outputDir).toAbsolutePath().toString();;
+            if (outputDir != null ) {
+                this.outputDir = Paths.get(outputDir).toAbsolutePath().toString();
+            } else {
+                this.outputDir = DEFAULT_OUTPUT_DIR;
+            }
             return this;
         }
 
@@ -303,8 +312,8 @@ public Builder withOutputDir(String outputDir) {
          * @param verbose the {@code verbose} to set
          * @return a reference to this Builder
          */
-        public Builder withVerbose(boolean verbose) {
-            this.verbose = verbose;
+        public Builder withVerbose(Boolean verbose) {
+            this.verbose = verbose != null ? verbose : Boolean.valueOf(DEFAULT_VERBOSE);
             return this;
         }
 
@@ -314,8 +323,8 @@ public Builder withVerbose(boolean verbose) {
          * @param skipOverwrite the {@code skipOverwrite} to set
          * @return a reference to this Builder
          */
-        public Builder withSkipOverwrite(boolean skipOverwrite) {
-            this.skipOverwrite = skipOverwrite;
+        public Builder withSkipOverwrite(Boolean skipOverwrite) {
+            this.skipOverwrite = skipOverwrite != null ? skipOverwrite : Boolean.valueOf(DEFAULT_SKIP_OVERWRITE);
             return this;
         }
 
@@ -325,8 +334,8 @@ public Builder withSkipOverwrite(boolean skipOverwrite) {
          * @param removeOperationIdPrefix the {@code removeOperationIdPrefix} to set
          * @return a reference to this Builder
          */
-        public Builder withRemoveOperationIdPrefix(boolean removeOperationIdPrefix) {
-            this.removeOperationIdPrefix = removeOperationIdPrefix;
+        public Builder withRemoveOperationIdPrefix(Boolean removeOperationIdPrefix) {
+            this.removeOperationIdPrefix = removeOperationIdPrefix != null ? removeOperationIdPrefix : Boolean.valueOf(DEFAULT_REMOVE_OPERATION_ID_PREFIX);
             return this;
         }
 
@@ -336,8 +345,8 @@ public Builder withRemoveOperationIdPrefix(boolean removeOperationIdPrefix) {
          * @param logToStderr the {@code logToStderr} to set
          * @return a reference to this Builder
          */
-        public Builder withLogToStderr(boolean logToStderr) {
-            this.logToStderr = logToStderr;
+        public Builder withLogToStderr(Boolean logToStderr) {
+            this.logToStderr = logToStderr != null ? logToStderr : Boolean.valueOf(DEFAULT_LOG_TO_STDERR);
             return this;
         }
 
@@ -347,8 +356,8 @@ public Builder withLogToStderr(boolean logToStderr) {
          * @param validateSpec the {@code validateSpec} to set
          * @return a reference to this Builder
          */
-        public Builder withValidateSpec(boolean validateSpec) {
-            this.validateSpec = validateSpec;
+        public Builder withValidateSpec(Boolean validateSpec) {
+            this.validateSpec = validateSpec != null ? validateSpec : Boolean.valueOf(DEFAULT_VALIDATE_SPEC);
             return this;
         }
 
@@ -358,8 +367,8 @@ public Builder withValidateSpec(boolean validateSpec) {
          * @param enablePostProcessFile the {@code enablePostProcessFile} to set
          * @return a reference to this Builder
          */
-        public Builder withEnablePostProcessFile(boolean enablePostProcessFile) {
-            this.enablePostProcessFile = enablePostProcessFile;
+        public Builder withEnablePostProcessFile(Boolean enablePostProcessFile) {
+            this.enablePostProcessFile = enablePostProcessFile != null ? enablePostProcessFile : Boolean.valueOf(DEFAULT_ENABLE_POST_PROCESS_FILE);
             return this;
         }
 
@@ -369,8 +378,8 @@ public Builder withEnablePostProcessFile(boolean enablePostProcessFile) {
          * @param enableMinimalUpdate the {@code enableMinimalUpdate} to set
          * @return a reference to this Builder
          */
-        public Builder withEnableMinimalUpdate(boolean enableMinimalUpdate) {
-            this.enableMinimalUpdate = enableMinimalUpdate;
+        public Builder withEnableMinimalUpdate(Boolean enableMinimalUpdate) {
+            this.enableMinimalUpdate = enableMinimalUpdate != null ? enableMinimalUpdate : Boolean.valueOf(DEFAULT_ENABLE_MINIMAL_UPDATE);
             return this;
         }
 
@@ -380,8 +389,8 @@ public Builder withEnableMinimalUpdate(boolean enableMinimalUpdate) {
          * @param strictSpecBehavior the {@code strictSpecBehavior} to set
          * @return a reference to this Builder
          */
-        public Builder withStrictSpecBehavior(boolean strictSpecBehavior) {
-            this.strictSpecBehavior = strictSpecBehavior;
+        public Builder withStrictSpecBehavior(Boolean strictSpecBehavior) {
+            this.strictSpecBehavior = strictSpecBehavior != null ? strictSpecBehavior : Boolean.valueOf(DEFAULT_STRICT_SPEC_BEHAVIOR);
             return this;
         }
 
@@ -392,9 +401,7 @@ public Builder withStrictSpecBehavior(boolean strictSpecBehavior) {
          * @return a reference to this Builder
          */
         public Builder withTemplateDir(String templateDir) {
-            if (templateDir == null) {
-                this.templateDir = null;
-            } else {
+            if (templateDir != null) {
                 File f = new File(templateDir);
 
                 // check to see if the folder exists
@@ -416,7 +423,7 @@ public Builder withTemplateDir(String templateDir) {
          * @return a reference to this Builder
          */
         public Builder withTemplatingEngineName(String templatingEngineName) {
-            this.templatingEngineName = templatingEngineName;
+            this.templatingEngineName = templatingEngineName != null ? templatingEngineName : DEFAULT_TEMPLATING_ENGINE_NAME;
             return this;
         }
 
@@ -438,7 +445,9 @@ public Builder withIgnoreFileOverride(String ignoreFileOverride) {
          * @return a reference to this Builder
          */
         public Builder withSystemProperties(Map<String, String> systemProperties) {
-            this.systemProperties = systemProperties;
+            if (systemProperties != null) {
+                this.systemProperties = systemProperties;
+            }
             return this;
         }