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

DefaultCodegen: Inheritance (allOf) parent not working when it is not at first position #340

Closed
janssen92 opened this issue Jun 18, 2018 · 12 comments · Fixed by #1169
Closed

Comments

@janssen92
Copy link
Contributor

Description

The inheritance is not working, the inherited attributes are not included in the generated files. (E.g. GetProofingConfigDto misses all ProofingConfigDto attriubtes)

Additionally, the generator warns about Objects, which SwaggerCodegen did not:
[main] WARN o.o.codegen.DefaultCodegen - Unknown type found in the schema: object

openapi-generator version

3.0.2

OpenAPI declaration file content or url

https://github.com/janssen92/swagger-feedback-service-api/blob/testOpenApiGenerator/swagger-feedback.json
https://github.com/janssen92/swagger-feedback-service-api/blob/testOpenApiGenerator/swagger-feedback-conf.json

Command line used for generation

java -jar ~/git/openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -i swagger-feedback.json -g spring -o ~/testOpenApiGen --library spring-boot --config swagger-feedback-conf.json
But also same result with less parameters:
java -jar ~/git/openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -i swagger-feedback.json -g spring -o ~/testOpenApiGen

Steps to reproduce
  • Checkout file(s)
  • Generate Code
@janssen92
Copy link
Contributor Author

Tested it with v3 as well as v2

@janssen92
Copy link
Contributor Author

It works just fine with Swagger Codegen 2.3.1

@jmini
Copy link
Member

jmini commented Jun 19, 2018

Might be a duplicate of #192

@jmini
Copy link
Member

jmini commented Jun 20, 2018

I did not investigate this issue in detail, but I think it is the same than #192 and therefore it might be fixed with PR #358.

@jmini
Copy link
Member

jmini commented Jun 22, 2018

Can you check the latest 3.0.3-SNAPSHOT version?

@janssen92
Copy link
Contributor Author

I can not find any 3.0.3-SNAPSHOT, but I just pulled and build current Master. There is still no inherited Attributes in some classes. (In others the 'extend' ist used just fine). I think the reason is: If you use 'allOf' and then a $ref as the first array element it works just fine. But if the $ref is a later array Element (e.g. the last one) the 'extend' isn't generated. This wasn't an Issue with Swagger-Gen so far.

@jmini
Copy link
Member

jmini commented Jun 25, 2018

Our SNAPSHOT builds are pushed to the sonatype snapshot repository:
https://oss.sonatype.org/content/repositories/snapshots

For example the CLI jar is the newest jar in this folder:
https://oss.sonatype.org/content/repositories/snapshots/org/openapitools/openapi-generator-cli/3.0.3-SNAPSHOT/

Or you can just use Maven to download it for you.


Anyway thanks for having tested this...

You are telling that this works:

definitions:
  SomeObject:
    type: object
    allOf:
      - $ref: "#/definitions/OtherObject"
      - type: object
        required:
          - bla
        properties:
          bla:
            type: string
  OtherObject:
    type: object
    properties:
      p1:
        type: string
      p2:
        type: string

And this one does not work:

definitions:
  SomeObject:
    type: object
    allOf:
      - type: object
        required:
          - bla
        properties:
          bla:
            type: string
      - $ref: "#/definitions/OtherObject"
  OtherObject:
    type: object
    properties:
      p1:
        type: string
      p2:
        type: string

If this is your issue, I can have a look at it.

@janssen92
Copy link
Contributor Author

janssen92 commented Jun 25, 2018

Ah thanks for the links, I will check it out.

Yes, your example is correct, didn't notice that pattern when I created the issue, but now that I know it, it seems to be correct for all problems

@jmini
Copy link
Member

jmini commented Jun 25, 2018

Well with 3.0.2 both cases are broken. 3.0.3-SNAPSHOT solves the case with $ref at first position. The remaining work to solve this issue is to handle cases where $ref is not at first position.

@jmini jmini changed the title [JAVA][Spring-Boot] Inheritance (allOf) not working [JAVA][Spring-Boot] Inheritance (allOf) not working when parent is not at first position Jun 25, 2018
@jmini
Copy link
Member

jmini commented Jun 25, 2018

The way getParentName(..) is implemented makes the case clear:

protected String getParentName(ComposedSchema composedSchema, Map<String, Schema> allSchemas) {
if (composedSchema.getAllOf() != null && !composedSchema.getAllOf().isEmpty()) {
Schema schema = composedSchema.getAllOf().get(0);
String ref = schema.get$ref();
if (StringUtils.isBlank(ref)) {
return null;
}
return ModelUtils.getSimpleRef(ref);
}
return null;
}

In a allOf construct, if the parent is not at first position it is ignored...

In my opinion we could improve this by taking the first $ref we find in the allOf-List...

@OpenAPITools/generator-core-team what do you think?

@jmini jmini changed the title [JAVA][Spring-Boot] Inheritance (allOf) not working when parent is not at first position DefaultCodegen: Inheritance (allOf) parent not working when it is not at first position Jun 25, 2018
@Shockolate
Copy link

I'm getting a bunch of these errors as well when trying to generate typescript code.
Tried with 3.2.3, 3.3.x (most recent as of posting SNAPSHOT), and 4.x.x (most recent as of posting SNAPSHOT).

[main] WARN  o.o.codegen.DefaultCodegen - Unknown type found in the schema: object
[main] WARN  o.o.codegen.DefaultCodegen - More than one inline schema specified in allOf:. Only the first one is recognized. All others are ignored.

@ackintosh
Copy link
Contributor

#340 (comment)

In my opinion we could improve this by taking the first $ref we find in the allOf-List...

I agree with that. I'll file a PR to improve so. 😉

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.

4 participants