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] Composed properties are missing from allVars #3613

Closed
glennschmidt opened this issue Aug 12, 2019 · 4 comments · Fixed by #3616
Closed

[BUG] Composed properties are missing from allVars #3613

glennschmidt opened this issue Aug 12, 2019 · 4 comments · Fixed by #3616

Comments

@glennschmidt
Copy link
Contributor

Description

When a model includes properties by composition (allOf, without a discriminator), these properties end up in the vars template variable but they are missing from the allVars variable.

I would expect allVars to include, at minimum, everything in vars. Some generators only iterate allVars when producing a model class, so they will be missing these composed properties.

openapi-generator version

4.1.0

OpenAPI declaration file content or url

https://gist.github.com/glennschmidt/d5e87fe518ac5afc7c39032ae1df479e

Command line used for generation

openapi-generator generate -i openapi.json -g swift4 -o apigen

Steps to reproduce
  1. Execute the above CLI with the schema from the linked gist
  2. Look at the generated output apigen/OpenAPIClient/Classes/OpenAPIs/Models/EventCollection.swift

It will contain

public struct EventCollection: Codable {


    public init() {
    }


}

ie. The EventCollection model has no properties, but it should have

  • page
  • page_count
  • page_size
  • total_items
  • events
Suggest a fix

In DefaultCodegen.fromModel(), when adding composed properties to properties, also add them to allProperties (i will submit a PR with this).

@auto-labeler
Copy link

auto-labeler bot commented Aug 12, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@jimschubert
Copy link
Member

Seems like this might affect more than just allVars… required/optional/parent/readonly/readwrite.

@glennschmidt
Copy link
Contributor Author

@jimschubert I think because of the location of my fix those other template vars will be updated too (by the addVars() method)

@glennschmidt
Copy link
Contributor Author

I've also run into a question about this code in DefaultCodegen.java:1857

// child schema (properties owned by the schema itself)
for (Schema component : interfaces) {
    if (component.get$ref() == null) {
        if (component != null) {
            // component is the child schema
            addProperties(properties, required, component);

            // includes child's properties (all, required) in allProperties, allRequired
            addProperties(allProperties, allRequired, component);
        }
        break; // at most one child only
    }
}

It looks at the component parts to find the one that has an empty $ref, and treats that as the properties of the schema itself. In my example, I'd expect the events property to be treated this way on the EventCollection schema. But it seems that there can never be a child schema with an empty $ref, because it will have been renamed by InlineModelResolver.flattenComponents(). In this case the ref gets set to #/components/schemas/EventCollection_allOf

I'm not sure if this suggests another bug or i'm just misunderstanding the purpose of this code.

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

Successfully merging a pull request may close this issue.

2 participants