Improve generation of selected models with dependent models (#18462)

* Issue-18444: recursively trace variables and support of new option

* Issue-18444: suppoting inheritance, but interfaces

* Issue-18444: build project instructions executed

* code review from wing328: tab-spaces removed

* code review by wing328: added a line of comment for the private method

Author: 0x0-pavel-0x0

modules/openapi-generator-maven-plugin/src/main/java/org/openapitools/codegen/plugin/CodeGenMojo.java

@@ -443,6 +443,12 @@ public class CodeGenMojo extends AbstractMojo {
     @Parameter(name = "generateModels", property = "openapi.generator.maven.plugin.generateModels")
     private Boolean generateModels = true;
 
+    /**
+     * Generate the models recursively if models should generate selectively (see modelsToGenerate) and all dependent models are to generate
+     */
+    @Parameter(name = "generateRecursiveDependentModels", property = "openapi.generator.maven.plugin.generateRecursiveDependentModels")
+    private Boolean generateRecursiveDependentModels = false;
+
     /**
      * A comma separated list of models to generate. All models is the default.
      */
@@ -795,6 +801,7 @@ public void execute() throws MojoExecutionException {
             GlobalSettings.setProperty(CodegenConstants.API_TESTS, generateApiTests.toString());
             GlobalSettings.setProperty(CodegenConstants.API_DOCS, generateApiDocumentation.toString());
             GlobalSettings.setProperty(CodegenConstants.WITH_XML, withXml.toString());
+            GlobalSettings.setProperty(CodegenConstants.GENERATE_RECURSIVE_DEPENDENT_MODELS, generateRecursiveDependentModels.toString());
 
             if (configOptions != null) {
                 // Retained for backwards-compatibility with configOptions -> instantiation-types

modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java

@@ -25,6 +25,7 @@ public class CodegenConstants {
     // NOTE: We may want to move these to a separate class to avoid confusion or modification.
     public static final String APIS = "apis";
     public static final String MODELS = "models";
+    public static final String GENERATE_RECURSIVE_DEPENDENT_MODELS = "generateRecursiveDependentModels";
     public static final String SUPPORTING_FILES = "supportingFiles";
     public static final String MODEL_TESTS = "modelTests";
     public static final String MODEL_DOCS = "modelDocs";

modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java

@@ -29,6 +29,7 @@
 import io.swagger.v3.oas.models.parameters.Parameter;
 import io.swagger.v3.oas.models.security.*;
 import io.swagger.v3.oas.models.tags.Tag;
+
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.io.comparator.PathFileComparator;
 import org.apache.commons.lang3.ObjectUtils;
@@ -65,6 +66,7 @@
 import java.util.*;
 import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import static org.apache.commons.lang3.StringUtils.removeStart;
@@ -81,6 +83,7 @@ public class DefaultGenerator implements Generator {
     protected CodegenIgnoreProcessor ignoreProcessor;
     private Boolean generateApis = null;
     private Boolean generateModels = null;
+    private Boolean generateRecursiveDependentModels = null;
     private Boolean generateSupportingFiles = null;
     private Boolean generateWebhooks = null;
     private Boolean generateApiTests = null;
@@ -232,6 +235,7 @@ void configureGeneratorProperties() {
         generateModelDocumentation = GlobalSettings.getProperty(CodegenConstants.MODEL_DOCS) != null ? Boolean.valueOf(GlobalSettings.getProperty(CodegenConstants.MODEL_DOCS)) : getGeneratorPropertyDefaultSwitch(CodegenConstants.MODEL_DOCS, true);
         generateApiTests = GlobalSettings.getProperty(CodegenConstants.API_TESTS) != null ? Boolean.valueOf(GlobalSettings.getProperty(CodegenConstants.API_TESTS)) : getGeneratorPropertyDefaultSwitch(CodegenConstants.API_TESTS, true);
         generateApiDocumentation = GlobalSettings.getProperty(CodegenConstants.API_DOCS) != null ? Boolean.valueOf(GlobalSettings.getProperty(CodegenConstants.API_DOCS)) : getGeneratorPropertyDefaultSwitch(CodegenConstants.API_DOCS, true);
+        generateRecursiveDependentModels = GlobalSettings.getProperty(CodegenConstants.GENERATE_RECURSIVE_DEPENDENT_MODELS) != null ? Boolean.valueOf(GlobalSettings.getProperty(CodegenConstants.GENERATE_RECURSIVE_DEPENDENT_MODELS)) : getGeneratorPropertyDefaultSwitch(CodegenConstants.GENERATE_RECURSIVE_DEPENDENT_MODELS, false);
 
         // Additional properties added for tests to exclude references in project related files
         config.additionalProperties().put(CodegenConstants.GENERATE_API_TESTS, generateApiTests);
@@ -243,6 +247,7 @@ void configureGeneratorProperties() {
         config.additionalProperties().put(CodegenConstants.GENERATE_APIS, generateApis);
         config.additionalProperties().put(CodegenConstants.GENERATE_MODELS, generateModels);
         config.additionalProperties().put(CodegenConstants.GENERATE_WEBHOOKS, generateWebhooks);
+        config.additionalProperties().put(CodegenConstants.GENERATE_RECURSIVE_DEPENDENT_MODELS, generateRecursiveDependentModels);
 
         if (!generateApiTests && !generateModelTests) {
             config.additionalProperties().put(CodegenConstants.EXCLUDE_TESTS, true);
@@ -443,36 +448,21 @@ private void generateModel(List<File> files, Map<String, Object> models, String
     }
 
     void generateModels(List<File> files, List<ModelMap> allModels, List<String> unusedModels, List<ModelMap> aliasModels) {
+        generateModels(files, allModels, unusedModels, aliasModels, new ArrayList<>(), DefaultGenerator.this::modelKeys);
+    }
+
+    void generateModels(List<File> files, List<ModelMap> allModels, List<String> unusedModels, List<ModelMap> aliasModels, List<String> processedModels, Supplier<Set<String>> modelKeysSupplier) {
         if (!generateModels) {
             // TODO: Process these anyway and add to dryRun info
             LOGGER.info("Skipping generation of models.");
             return;
         }
 
-        final Map<String, Schema> schemas = ModelUtils.getSchemas(this.openAPI);
-        if (schemas == null) {
-            LOGGER.warn("Skipping generation of models because specification document has no schemas.");
+        Set<String> modelKeys = modelKeysSupplier.get();
+        if(modelKeys.isEmpty()) {
             return;
         }
 
-        String modelNames = GlobalSettings.getProperty("models");
-        Set<String> modelsToGenerate = null;
-        if (modelNames != null && !modelNames.isEmpty()) {
-            modelsToGenerate = new HashSet<>(Arrays.asList(modelNames.split(",")));
-        }
-
-        Set<String> modelKeys = schemas.keySet();
-        if (modelsToGenerate != null && !modelsToGenerate.isEmpty()) {
-            Set<String> updatedKeys = new HashSet<>();
-            for (String m : modelKeys) {
-                if (modelsToGenerate.contains(m)) {
-                    updatedKeys.add(m);
-                }
-            }
-
-            modelKeys = updatedKeys;
-        }
-
         // store all processed models
         Map<String, ModelsMap> allProcessedModels = new TreeMap<>((o1, o2) -> ObjectUtils.compare(config.toModelName(o1), config.toModelName(o2)));
 
@@ -508,7 +498,7 @@ void generateModels(List<File> files, List<ModelMap> allModels, List<String> unu
                     }
                 }
 
-                Schema schema = schemas.get(name);
+                Schema schema = ModelUtils.getSchemas(this.openAPI).get(name);
 
                 if (schema.getExtensions() != null && Boolean.TRUE.equals(schema.getExtensions().get("x-internal"))) {
                     LOGGER.info("Model {} not generated since x-internal is set to true", name);
@@ -549,6 +539,24 @@ void generateModels(List<File> files, List<ModelMap> allModels, List<String> unu
         // post process all processed models
         allProcessedModels = config.postProcessAllModels(allProcessedModels);
 
+        if (generateRecursiveDependentModels) {
+            for(ModelsMap modelsMap : allProcessedModels.values()) {
+                for(ModelMap mm: modelsMap.getModels()) {
+                    CodegenModel cm = mm.getModel();
+                    if (cm != null) {
+                        for(CodegenProperty variable : cm.getVars()) {
+                            generateModelsForVariable(files, allModels, unusedModels, aliasModels, processedModels, variable);
+                        }
+                        //TODO:  handle interfaces
+                        String parentSchema = cm.getParentSchema();
+                        if (parentSchema != null && !processedModels.contains(parentSchema) && ModelUtils.getSchemas(this.openAPI).containsKey(parentSchema)) {
+                            generateModels(files, allModels, unusedModels, aliasModels, processedModels, () -> Set.of(parentSchema));
+                        }
+                    }
+                }
+            }
+        }
+        
         // generate files based on processed models
         for (String modelName : allProcessedModels.keySet()) {
             ModelsMap models = allProcessedModels.get(modelName);
@@ -592,7 +600,71 @@ void generateModels(List<File> files, List<ModelMap> allModels, List<String> unu
             LOGGER.info("############ Model info ############");
             Json.prettyPrint(allModels);
         }
+    }
+
+    /**
+     * this method guesses the schema type of in parent model used variable and if the schema type is available it let the generate the model for the type of this variable
+     */
+    private void generateModelsForVariable(List<File> files, List<ModelMap> allModels, List<String> unusedModels, List<ModelMap> aliasModels, List<String> processedModels, CodegenProperty variable) {
+        if (variable == null) {
+            return;
+        }
+
+        final String schemaKey = calculateModelKey(variable.getOpenApiType(), variable.getRef());
+        Map<String, Schema> allSchemas = ModelUtils.getSchemas(this.openAPI);
+        if (!processedModels.contains(schemaKey) && allSchemas.containsKey(schemaKey)) {
+            generateModels(files, allModels, unusedModels, aliasModels, processedModels, () -> Set.of(schemaKey));
+        } else if (variable.getComplexType() != null && variable.getComposedSchemas() == null) {
+            String ref = variable.getHasItems() ? variable.getItems().getRef() : variable.getRef();
+            final String key = calculateModelKey(variable.getComplexType(), ref);
+            if (allSchemas.containsKey(key)) {
+                generateModels(files, allModels, unusedModels, aliasModels, processedModels, () -> Set.of(key));
+            } else {
+                LOGGER.info("Type " + variable.getComplexType()+" of variable " + variable.getName() + " could not be resolve because it is not declared as a model.");
+            }
+        } else {
+            LOGGER.info("Type " + variable.getOpenApiType()+" of variable " + variable.getName() + " could not be resolve because it is not declared as a model.");
+        }
+    }
+
+    private String calculateModelKey(String type, String ref) {
+        Map<String, Schema> schemaMap = ModelUtils.getSchemas(this.openAPI);
+        Set<String> keys = schemaMap.keySet();
+        String simpleRef;
+        if(keys.contains(type)) {
+            return type;
+        } else if (keys.contains(simpleRef = ModelUtils.getSimpleRef(ref))) {
+            return simpleRef;
+        } else {
+            return type;
+        }
+    }
+
+    private Set<String> modelKeys() {
+        final Map<String, Schema> schemas = ModelUtils.getSchemas(this.openAPI);
+        if (schemas == null) {
+            LOGGER.warn("Skipping generation of models because specification document has no schemas.");
+            return Collections.emptySet();
+        }
 
+        String modelNames = GlobalSettings.getProperty("models");
+        Set<String> modelsToGenerate = null;
+        if (modelNames != null && !modelNames.isEmpty()) {
+            modelsToGenerate = new HashSet<>(Arrays.asList(modelNames.split(",")));
+        }
+
+        Set<String> modelKeys = schemas.keySet();
+        if (modelsToGenerate != null && !modelsToGenerate.isEmpty()) {
+            Set<String> updatedKeys = new HashSet<>();
+            for (String m : modelKeys) {
+                if (modelsToGenerate.contains(m)) {
+                    updatedKeys.add(m);
+                }
+            }
+
+            modelKeys = updatedKeys;
+        }
+        return modelKeys;
     }
 
     @SuppressWarnings("unchecked")

modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultGeneratorTest.java

@@ -775,4 +775,137 @@ public void testRecursionBug4650() {
         generator.generateModels(files, allModels, filteredSchemas, aliasModels);
         // all fine, we have passed
     }
+
+    
+    private DefaultGenerator generatorGenerateRecursiveDependentModelsBackwardCompatibility(String recursively) throws IOException {
+        DefaultGenerator generator = new DefaultGenerator(false);
+        generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
+        generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_TESTS, "true");
+        generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_DOCS, "false");
+        generator.setGeneratorPropertyDefault(CodegenConstants.APIS, "true");
+        generator.setGeneratorPropertyDefault(CodegenConstants.SUPPORTING_FILES, "true");
+        generator.setGeneratorPropertyDefault(CodegenConstants.API_DOCS, "false");
+        generator.setGeneratorPropertyDefault(CodegenConstants.API_TESTS, "false");
+        generator.setGeneratorPropertyDefault(CodegenConstants.GENERATE_RECURSIVE_DEPENDENT_MODELS, recursively);
+        return generator;
+    }
+
+    private ClientOptInput createOptInputIssue18444(Path target) {
+        final CodegenConfigurator configurator = new CodegenConfigurator()
+                .setGeneratorName("spring")
+                .setInputSpec("src/test/resources/bugs/issue_18444.json")
+                .setOutputDir(target.toAbsolutePath().toString());
+        return configurator.toClientOptInput();
+    }
+
+    @Test
+    public void testGenerateRecursiveDependentModelsBackwardCompatibilityIssue18444() throws IOException {
+        Path target = Files.createTempDirectory("test");
+        File output = target.toFile();
+        String oldModelsProp = GlobalSettings.getProperty("models");
+
+        try {
+            DefaultGenerator generator = generatorGenerateRecursiveDependentModelsBackwardCompatibility("false");
+            GlobalSettings.setProperty("models", "RQ1,RS1");
+            ClientOptInput clientOptInput = createOptInputIssue18444(target);
+            List<File> files = generator.opts(clientOptInput ).generate();
+            Assert.assertEquals(files.size(), 17);
+
+            // Check expected generated files
+            // api sanity check
+            String apiJavaFileName = "src/main/java/org/openapitools/api/ApiApi.java"; 
+            TestUtils.ensureContainsFile(files, output, apiJavaFileName);
+            Assert.assertTrue(new File(output, apiJavaFileName).exists());
+
+            // model sanity check
+            String rq1FileName = "src/main/java/org/openapitools/model/RQ1.java";
+            TestUtils.ensureContainsFile(files, output, rq1FileName);
+            Assert.assertTrue(new File(output, rq1FileName).exists());
+
+            String rs1FileName = "src/main/java/org/openapitools/model/RS1.java";
+            TestUtils.ensureContainsFile(files, output, rs1FileName );
+            Assert.assertTrue(new File(output, rs1FileName).exists());
+
+            // Check not generated cause backwards compatibility files
+            String ft1FileName = "src/main/java/org/openapitools/model/FT1.java";
+            TestUtils.ensureDoesNotContainsFile(files, output, ft1FileName);
+            Assert.assertFalse(new File(output, ft1FileName).exists());
+
+            String ft2FileName = "src/main/java/org/openapitools/model/FT2.java";
+            TestUtils.ensureDoesNotContainsFile(files, output, ft2FileName);
+            Assert.assertFalse(new File(output, ft2FileName).exists());
+
+            String ft3FileName = "src/main/java/org/openapitools/model/FT3.java";
+            TestUtils.ensureDoesNotContainsFile(files, output, ft3FileName);
+            Assert.assertFalse(new File(output, ft3FileName).exists());
+
+            String bttFileName = "src/main/java/org/openapitools/model/BTT.java";
+            TestUtils.ensureDoesNotContainsFile(files, output, bttFileName);
+            Assert.assertFalse(new File(output, bttFileName).exists());
+
+        } finally {
+            output.deleteOnExit();
+            if (oldModelsProp != null) {
+                GlobalSettings.setProperty("models", oldModelsProp);
+            } else {
+                GlobalSettings.clearProperty("models");
+            }
+        }
+    }
+
+    @Test
+    public void testGenerateRecursiveDependentModelsIssue18444() throws IOException {
+        Path target = Files.createTempDirectory("test");
+        File output = target.toFile();
+        String oldModelsProp = GlobalSettings.getProperty("models");
+
+        try {
+            DefaultGenerator generator = generatorGenerateRecursiveDependentModelsBackwardCompatibility("true");
+            GlobalSettings.setProperty("models", "RQ1,RS1");
+            ClientOptInput clientOptInput = createOptInputIssue18444(target);
+            List<File> files = generator.opts(clientOptInput ).generate();
+            Assert.assertEquals(files.size(), 21);
+
+            // Check expected generated files
+            // api sanity check
+            String apiJavaFileName = "src/main/java/org/openapitools/api/ApiApi.java"; 
+            TestUtils.ensureContainsFile(files, output, apiJavaFileName);
+            Assert.assertTrue(new File(output, apiJavaFileName).exists());
+
+            // model sanity check
+            String rq1FileName = "src/main/java/org/openapitools/model/RQ1.java";
+            TestUtils.ensureContainsFile(files, output, rq1FileName);
+            Assert.assertTrue(new File(output, rq1FileName).exists());
+
+            String rs1FileName = "src/main/java/org/openapitools/model/RS1.java";
+            TestUtils.ensureContainsFile(files, output, rs1FileName );
+            Assert.assertTrue(new File(output, rs1FileName).exists());
+
+            // Check generated cause RQ1 and RS1 dependents of FT1,FT2,FT3 files
+            String ft1FileName = "src/main/java/org/openapitools/model/FT1.java";
+            TestUtils.ensureContainsFile(files, output, ft1FileName);
+            Assert.assertTrue(new File(output, ft1FileName).exists());
+
+            String ft2FileName = "src/main/java/org/openapitools/model/FT2.java";
+            TestUtils.ensureContainsFile(files, output, ft2FileName);
+            Assert.assertTrue(new File(output, ft2FileName).exists());
+
+            String ft3FileName = "src/main/java/org/openapitools/model/FT3.java";
+            TestUtils.ensureContainsFile(files, output, ft3FileName);
+            Assert.assertTrue(new File(output, ft3FileName).exists());
+
+            String bttFileName = "src/main/java/org/openapitools/model/BTT.java";
+            TestUtils.ensureContainsFile(files, output, bttFileName);
+            Assert.assertTrue(new File(output, bttFileName).exists());
+
+        } finally {
+            output.deleteOnExit();
+            if (oldModelsProp != null) {
+                GlobalSettings.setProperty("models", oldModelsProp);
+            } else {
+                GlobalSettings.clearProperty("models");
+            }
+        }
+    }
+
 }