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

Update imports generated in java layer to use one helper for all locations

Open spacether opened this issue 3 years ago • 0 comments

Bug Report Checklist

  • [X] Have you provided a full/minimal spec to reproduce the issue?
  • [ ] Have you validated the input using an OpenAPI validator (example)?
  • [X] Have you tested with the latest master to confirm the issue still exists?
  • [X] Have you searched for related issues/PRs?
  • [X] What's the actual output vs expected output?
  • [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

This is needed to ensure the referenced schemas in not are added to the model/endpoint imports import missing for ref in not

Feature: jsonschema.$ref/not

openapi-generator version

6.0.0

OpenAPI declaration file content or url
openapi: 3.0.3
info:
  title: openapi 3.0.3 sample spec
  description: sample spec for testing openapi functionality, built from json schema
    tests for draft6
  version: 0.0.1
paths: {}
components:
  schemas:
    PropertyNamedRefThatIsNotAReference:
      properties:
        $ref:
          type: string
    RefInNot:
      not:
        $ref: '#/components/schemas/PropertyNamedRefThatIsNotAReference'

In RefInNot there is no import for PropertyNamedRefThatIsNotAReference

Generation Details

python-experimental

Steps to reproduce

generate w/ the above generator

Related issues/PRs

Found when working on: https://github.com/OpenAPITools/openapi-generator/pull/12619

Suggest a fix

Add the import in defaultcodegen.java

spacether avatar Jul 01 '22 19:07 spacether

From my PR:

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