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

JSONField adds empty additionalProperties causing failures with client generation libraries #238

Closed
rvinzent opened this issue Dec 22, 2020 · 5 comments
Labels
fix confirmation pending issue has been fixed and confirmation from issue reporter is pending

Comments

@rvinzent
Copy link

rvinzent commented Dec 22, 2020

Describe the bug
Using a JSON field correctly sets the OpenAPI type to object, but it also adds an empty additionalProperties. This causes some client generators to fail, and I can't see the benefit of having an empty additionalProperties property anyway.

To Reproduce
Use a serializers.JSONField and generate a component.

The generated spec is

field_name:
  type: object
  additionalProperties: {}

Expected behavior
the field should be this equivalent, and not break the Java/Kotlin client generators

field_name:
  type: object

Setting additionalProperties: true should also work, but this is the default

@tfranzel
Copy link
Owner

hi @rvinzent, your point is absolutely valid. interpretation of this slightly changed over time. i can remember multiple discussions in the OAP repo issue section (which i can't find at the moment).

But it is also true that the json-schema and the official documentation consider both acceptable:
https://swagger.io/docs/specification/data-models/data-types/#free-form

for me personally, it is a hint that the object will definitely contain extra fields even though the default is already true. i'm open to changing this, but we have to make sure that SwaggerUI, ReDoc, and the most common client targets are not negatively impacted by the change.

@rvinzent
Copy link
Author

rvinzent commented Dec 28, 2020

@tfranzel I can confirm the Swagger UI is unaffected by the change, and IMO if there are any issues with any other consumers of OpenAPI spec, that is a bug on their end because the change here should be equivalent when strictly adhering to the spec.

In that sense, It's also technically a bug on the client generation side ¯\(ツ)

@tfranzel
Copy link
Owner

@rvinzent so i checked this again. it turns out it is even worse. according to the specification, all these fields should be identical. correct me if i'm wrong here.

openapi-generator==5.0.1 target typescript-fetch generated 3 versions. and field_b (true) is definitely broken in typescript. some targets don't care either way. kotlin looked better to me with true

i'm afraid we have to introduce a setting for this as it looks like there is no one-size-fits-all.

    Beta:
      type: object
      properties:
        field_a:
          type: object
        field_b:
          type: object
          additionalProperties: true
        field_c:
          type: object
          additionalProperties: {}
      required:
      - field_a
      - field_b
      - field_c
export interface Beta {
    /**
     * 
     * @type {object}
     * @memberof Beta
     */
    fieldA: object;
    /**
     * 
     * @type {{ [key: string]: object; }}
     * @memberof Beta
     */
    fieldB: { [key: string]: object; };
    /**
     * 
     * @type {{ [key: string]: any; }}
     * @memberof Beta
     */
    fieldC: { [key: string]: any; };
}

@tfranzel tfranzel added the fix confirmation pending issue has been fixed and confirmation from issue reporter is pending label Jan 13, 2021
@tfranzel
Copy link
Owner

introduced a setting for this. let me know if that works for you

@tfranzel
Copy link
Owner

i consider this resolved with the added setting. feel free to comment if there is something missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix confirmation pending issue has been fixed and confirmation from issue reporter is pending
Projects
None yet
Development

No branches or pull requests

2 participants