openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

Feat fixes issue 12756

Open spacether opened this issue 2 years ago • 0 comments

Feat fixes issue 12756

PR checklist

  • [ ] Read the contribution guidelines.
  • [ ] Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • [ ] Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
  • [ ] File the PR against the correct branch: master (6.1.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • [ ] If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

spacether avatar Aug 09 '22 05:08 spacether

So our code + use cases to add imports is a nest of spaghetti. There are many import addition call sites and they behave differently. The general cases are:

    default Set<String> getImports(boolean importContainerType, boolean importBaseType, Map<String, String> instantiationTypes, Map<String, String> typeMapping, FeatureSet featureSet) {
        Set<String> imports = new HashSet<>();
        if (this.getComposedSchemas() != null) {
            CodegenComposedSchemas composed = this.getComposedSchemas();
            List<CodegenProperty> allOfs = Collections.emptyList();
            List<CodegenProperty> oneOfs = Collections.emptyList();
            List<CodegenProperty> anyOfs = Collections.emptyList();
            List<CodegenProperty> nots = Collections.emptyList();
            if (composed.getAllOf() != null && featureSet.getSchemaSupportFeatures().contains(SchemaSupportFeature.allOf)) {
                allOfs = composed.getAllOf();
            }
            if (composed.getOneOf() != null && featureSet.getSchemaSupportFeatures().contains(SchemaSupportFeature.oneOf)) {
                oneOfs = composed.getOneOf();
            }
            if (composed.getAnyOf() != null && featureSet.getSchemaSupportFeatures().contains(SchemaSupportFeature.anyOf)) {
                anyOfs = composed.getAnyOf();
            }
            if (composed.getNot() != null && featureSet.getSchemaSupportFeatures().contains(SchemaSupportFeature.not)) {
                nots = Arrays.asList(composed.getNot());
            }
            Stream<CodegenProperty> innerTypes = Stream.of(
                            allOfs.stream(), anyOfs.stream(), oneOfs.stream(), nots.stream())
                    .flatMap(i -> i);
            innerTypes.flatMap(cp -> cp.getImports(importContainerType, importBaseType, instantiationTypes, typeMapping, featureSet).stream()).forEach(s -> imports.add(s));
        }
        // items can exist for AnyType and type array
        if (this.getItems() != null && this.getIsArray()) {
            imports.addAll(this.getItems().getImports(importContainerType, importBaseType, instantiationTypes, typeMapping, featureSet));
        }
        // additionalProperties can exist for AnyType and type object
        if (this.getAdditionalProperties() != null) {
            imports.addAll(this.getAdditionalProperties().getImports(importContainerType, importBaseType, instantiationTypes, typeMapping, featureSet));
        }
        // vars can exist for AnyType and type object
        if (this.getVars() != null && !this.getVars().isEmpty()) {
            this.getVars().stream().flatMap(v -> v.getImports(importContainerType, importBaseType, instantiationTypes, typeMapping, featureSet).stream()).forEach(s -> imports.add(s));
        }
        if (this.getIsArray() || this.getIsMap()) {
            /*
            use-case for this complexType block:
            DefaultCodegenTest.objectQueryParamIdentifyAsObject
            DefaultCodegenTest.mapParamImportInnerObject
            */
            String complexType = this.getComplexType();
            if (complexType != null) {
                imports.add(complexType);
            }
            if (importContainerType) {
                String containerType;
                if (this.getIsArray()) {
                    if (this.getUniqueItems()) {
                        containerType = "set";
                    } else {
                        containerType = "array";
                    }
                } else {
                    containerType = "map";
                }
                /*
                 use case:
                 generator has instantiationType for array/map/set
                 */
                final String instantiationType = instantiationTypes.get(containerType);
                if (instantiationType != null) {
                    imports.add(instantiationType);
                }
                /*
                 use case:
                 generator has typeMapping for array/map/set
                 */
                final String mappedType = typeMapping.get(containerType);
                if (mappedType != null) {
                    imports.add(mappedType);
                }
            }
            /*
            use-case:
            Adding List/Map etc, Java uses this
             */
            String baseType = this.getBaseType();
            if (importBaseType && baseType != null) {
                imports.add(baseType);
            }
        } else {
            // referenced or inline schemas
            String complexType = this.getComplexType();
            if (this.getRef() != null && complexType != null) {
                /*
                use case:
                referenced components store the model name in complexType
                 */
                imports.add(complexType);
            } else if (complexType != null) {
                 /*
                 use-case:
                 c, c++, java, + others primitive type imports
                  */
                imports.add(complexType);
            }
            return imports;
        }
        return imports;
    }

Which almost works BUT Array and Map imports are sometimes added and sometime not and used in a bunch of generators especially Java ones. And Responses behave differently than Parameters than Models. So this is not generalizable for all generators. Next I will try to make a more targeted opt in import function that depends on parametersAndResponsesImportFromV3SpecLocations being true, and leaving all of the rest import code untouched.

spacether avatar Aug 17 '22 00:08 spacether