[BUG] Wrong imports for maps

[mwilby]

Bug Report Checklist

• [ ] Have you provided a full/minimal spec to reproduce the issue?
• [ ] Have you validated the input using an OpenAPI validator (example)?
• [ ] What's the version of OpenAPI Generator used?
• [ ] Have you search for related issues/PRs?
• [ ] What's the actual output vs expected output?
• [ ] [Optional] Bounty to sponsor the fix (example)

Description

While fixing the import paths on one of the python generators, I came across an odd problem.

Its to do with the use of https://swagger.io/docs/specification/data-models/dictionaries/

You get library imports for arrays and maps and it should just be maps.

openapi-generator version

All of them

OpenAPI declaration file content or url

Typical example:

reporting_state:
  type: object
  properties:
    file_list:
      type: object
      additionalProperties:
        $ref: '#/file_path'
file_path:      # Can be referenced via '#/file_path'
  type: object
  properties:
    path:
      type: string

Command line used for generation

N/A

Steps to reproduce

N/A

Related issues/PRs

Issue observed in #6065

Suggest a fix

Its found in the

addVars(CodegenModel m, List<CodegenProperty> vars, Map<String, Schema> properties, Set<String> mandatory)

method in DefaultCodegen.

The method contains the following snippet

// TODO revise the logic to include map
if (cp.isContainer) {
    addImport(m, typeMapping.get("array"));
}

Which courteously tells you whats wrong. The fix for the python generators was very simple

if (cp.isContainer) {
    if (cp.isListContainer) {
        addImport(m, typeMapping.get("array"));
    }
    else if (cp.isMapContainer) {
        addImport(m, typeMapping.get("map"));
    }
}

However, I don't know if this is really valid for the other generators, so I don't want to add the change that effects everything.

It does looks like it may be OK, but it looks too simple not to be there, unless it effects other languages differently. I'm reporting it, just in case it can be used.

[jimschubert]

I think both of these would have to import the instantiation type as well, but I doubt every generator is using instantiation type correctly.

[jimschubert]

cc @OpenAPITools/generator-core-team thoughts on this approach, and maybe including instantiation type as well before the 5.0 release?

[mwilby]

Its unlikely its a problem. Basically the bug is writing in a dependency as an array, when in fact it should be a map. In theory this would cause generated code not to compile/run as the wrong import will be included. No one seems to be complaining, even in my case it was just an ugliness in the generate code I didn't like.

I think the reason its not a problem is, further down the process, the actual dictionary object has to be built, this necessitates the inclusion of the appropriate map type for the language, so the result is no worse than the inclusion of a redundant array type when you don't need it and languages that have maps and arrays as native object, wont even see that.

I don't think you need to map it onto the instantiation types, as that is/should be done when the language specific import list is built.

Assuming that there are use cases out there that use swagger dictionaries, it has been already tested and and any problems found and fixed. If the lack of map import was a real problem, it would have shown up already. I would suggest that, as it has a very low probability of it being a real problem, just chuck it in and see if anyone complains.

I did think that, just in case you might change it to be ultra conservative and throw in the a default assignment

if (cp.isContainer) {
    if (cp.isListContainer) {
        addImport(m, typeMapping.get("array"));
    }
    else if (cp.isMapContainer) {
        addImport(m, typeMapping.get("map"));
    }
    else {
        addImport(m, typeMapping.get("array"));
    }
}

It should not be necessary, but just in case the spec. gets changed to include differentiate list types.