Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] [JAVA] Referencing Object with additionalPorperties does not import HashMap in Model #3456

Open
5 of 6 tasks
FloKaemmerer opened this issue Jul 25, 2019 · 11 comments
Open
5 of 6 tasks

Comments

@FloKaemmerer
Copy link

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

Generating Model for a Object that references an object that has "additionalProperties" fails to generate the import for HashMap

openapi-generator version

affected Version(s): 4.0.0 to 4.0.3
Not affected: 3.3.4 (sets the field to null, so no import is needed here)

OpenAPI declaration file content or url

Snipped:

    WithAdditinalPropertiesDTO:
      type: object
      properties:
        id:
          type: string
      additionalProperties: {}

    BrokenDTO:
      type: object
      properties:
        id:
          type: string
        withAdditinalProperties:
          $ref: '#/components/schemas/WithAdditinalPropertiesDTO'

gist: https://gist.github.com/FloKaemmerer/54a9663183688b0d5658a8064c32f695

Command line used for generation

mvn clean compile

used pom: https://gist.github.com/FloKaemmerer/9789a0fc5bfca47b6c4cf50c7573434b

Steps to reproduce
Related issues/PRs
Suggest a fix
@FloKaemmerer
Copy link
Author

Why didn't the Bot pick this issue up and added Tags to it?
What did i miss :-/

@FelixA
Copy link

FelixA commented Sep 4, 2019

I ran into the same issue. Would be amazing to see some progress here

@FloKaemmerer
Copy link
Author

I there any possible update on this?

@jimschubert
Copy link
Member

Hey, I've just tested this in master and it's working as I would expect (HashMap is imported, but the dynamic object has no initializer and would default to null). Would you mind giving it a try if possible?

image

I think it was fixed as a result of #6052 which I'd implemented as a result of conversations on Slack, iirc.

@FloKaemmerer
Copy link
Author

@jimschubert Thanks for looking into this.

I tried the 5.0.0-Beta

The HashMap is now imported correctly, but the Object with the additional Properties does not extend HashMap any more.

My Objects look like this:

/**
 * BrokenDTO
 */
@JsonPropertyOrder({
  BrokenDTO.JSON_PROPERTY_ID,
  BrokenDTO.JSON_PROPERTY_WITH_ADDITINAL_PROPERTIES
})
@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen")
public class BrokenDTO {
  public static final String JSON_PROPERTY_ID = "id";
  private String id;

  public static final String JSON_PROPERTY_WITH_ADDITINAL_PROPERTIES = "withAdditinalProperties";
  private WithAdditinalPropertiesDTO withAdditinalProperties = new HashMap<>();


  public BrokenDTO id(String id) {
    this.id = id;
    return this;
  }


/**
 * WithAdditinalPropertiesDTO
 */
@JsonPropertyOrder({
  WithAdditinalPropertiesDTO.JSON_PROPERTY_ID
})
@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen")
public class WithAdditinalPropertiesDTO {
  public static final String JSON_PROPERTY_ID = "id";
  private String id;


  public WithAdditinalPropertiesDTO id(String id) {
    this.id = id;
    return this;
  }

@jimschubert
Copy link
Member

@FloKaemmerer The objects are no longer treated as a HashMap, but rather as a regular object where additionalProperties are stored in the "bag" of additionalProperties as introduced via #6518

image

I don't have context around the change from #6518, but I assume it has to do in part with how additionalProperties are defined in the OpenAPI Specification and in part with the complexity around maintaining HashMap contracts if schemas with additionalProperties are implemented as a map.

You could refer to my comment on additionalProperties in #6849 to understand some issues with that property itself. As OpenAPI Specification is a schema validation specification and not a structural definition specification, the implementation in #6518 using a property bag arguably matches the intent of the specification more closely.

If schemas containing additionalProperties extend HashMap, this would mean:

  • Every property would need to be a getter/setter into the map and not a backing field
  • The type would have to match the equals/hashcode implementation of HashMap
  • Every schema in a composed model (oneOf, allOf, noneOf, anyOf) would be overly complex. For example, if you had anyOf with Cat and Dog as options but both Cat and Dog extended HashMap<String, Object>, you'd now have a condition which can't be verified

The new approach also follows the best practice to "prefer composition over inheritance"

If you have specific questions about the new approach I think @wing328 would be happy to discuss.

@FloKaemmerer
Copy link
Author

@jimschubert Thanks for clearing that up, I missed that in the changelog.

But the problem is that the generator tries to initialize the Object that has the additional properties as a HashMap. (See the Code in my previous comment)

@jimschubert
Copy link
Member

@FloKaemmerer I apologize that I didn't make it more clear in my first comment on this thread, but the changes I've shown above are on master, not in 5.0.0-beta. This is what BrokenDTO would look like on master:

image

We'll have a second beta release later this week, but if you want to try things out now you could use the latest docker image.

@jass-trix
Copy link

@jimschubert hi i tried to use open api generator version 5.1.1
but when i use additionalProperties to generate Map and ref it to another model it does not import HashMap
I've already set generateAliasAsModel= true
but i saw in PR #6052 it has been resolved, im not sure whether this one is the same issue or not
Screenshot from 2021-07-07 20-41-35

@Praveen2518
Copy link

Praveen2518 commented Sep 6, 2021

@jass-trix hi did you fix that hashMap import error i am also getting the same error in 5.1.0 version.please reply to me if you have fixed it

@jass-trix
Copy link

@jass-trix hi did you fix that hashMap import error i am also getting the same error in 5.1.0 version.please reply to me if you have fixed it

hi, recently i've just tried to recreate once again to create map in my model

"userProfiling": {
            "type": "object",
            "additionalProperties": {
              "type": "string"
            }
          }

i don't know what happen, but this time it works for me and it show no error or missing import from my side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants