AWS2OPENAPI-45 fix: remove generic 'allOf' composites
- only keep them where we actually have multiple types that are accepted
Hi @MikeRalphson Thanks for looking at this PR. Is there any work left to do until it can be merged? Can I help somehow?
Can you expand a little on why an allOf in a schema object would be "unexpected"?
https://github.com/APIs-guru/openapi-directory/blob/1772399f6fedfd650b6eff7c9a6b35c4c04b732d/APIs/amazonaws.com/runtime.lex.v2/2020-08-07/openapi.yaml#L393-L395
Your PR appears to replace these with
ProgressUpdateStream:
$ref: '#/components/schemas/ProgressUpdateStream'
description: 'The name of the ProgressUpdateStream. '
which is less valid according to the spec, because in OAS 3.0 sibling properties of a $ref MUST be ignored.
There are 5 pages of open issues on swagger-codegen about $ref and allOf and I don't think working around them by tweaking the output of this project is really practical.
Could you test to see if something like
description: foo
allOf:
- $ref: '#/components/schemas/bar'
solves the problem you're having with code-gen?
Thanks for the update @MikeRalphson I wasn't aware of the restriction, that $ref schema should not have sibling properties. Therefore indeed your current schema looks more compliant. And I see that it doesn't make too much sense to bend the world too much around swagger-codegen and rather suggest a fix in the swagger repos.
I've tested manually your proposal, to place the description on the same level as the 'allOf' property. openapi-lex2.yaml And yes, this solves the most severe problem of the swagger-codegen stack: I no longer get invalid java sources that can't be compiled. So I'd vote to change the AWSOPENAPI generator towards that end. Should I fire another PR for this?
However, due to the generic 'allOf' reference, I still loose all the 'type' support in the swagger generated java classes. I'm left with only 'object' references for the available properties. Which makes the generated output less attractive, especially in the complex type hierarchy of this amazon.lex service.
I've tested manually your proposal, to place the description on the same level as the 'allOf' property. And yes, this solves the most severe problem of the swagger-codegen stack: I no longer get invalid java sources that can't be compiled. So I'd vote to change the AWSOPENAPI generator towards that end. Should I fire another PR for this?
Yes, a new or updated PR to use this pattern for $ref + description would be welcome.
However, due to the generic 'allOf' reference, I still loose all the 'type' support in the swagger generated java classes. I'm left with only 'object' references for the available properties. Which makes the generated output less attractive, especially in the complex type hierarchy of this amazon.lex service.
That definitely sounds like a bug in the codegen tool.
Update the PR as discussed: There are still some structures that actually have properties other than $ref within the 'allOf' composites. See the 'xml' properties in ... amazonaws.com/apigateway/2015-07-09/openapi.yaml
items:
description: The current page of elements from this collection.
allOf:
- $ref: '#/components/schemas/ListOfApiKey'
- xml:
name: item
I guess this is ok and still valid.
Anything left to do here @MikeRalphson ?