camaraproject / QualityOnDemand

Repository to describe, develop, document and test the QualityOnDemand API family
https://wiki.camaraproject.org/x/zwOeAQ
Apache License 2.0
41 stars 59 forks source link

Move the "description" out of "allOf"-declaration #204

Closed maxl2287 closed 1 year ago

maxl2287 commented 1 year ago

Problem description "allOf" including a $ref and a 'description' is not compatible with openapi - generators for later implemantation usage.

    CreateSession:
      description: Attributes required to create a session
      type: object
      properties:
        <...>
        devicePorts:
          allOf:
            - description: The ports used locally by the device for flows to which the requested QoS profile should apply. If omitted, then the qosProfile will apply to all flows between the device and the specified application server address and ports
            - $ref: "#/components/schemas/PortsSpec" 
        applicationServerPorts:
          allOf:
            - description: A list of single ports or port ranges on the application server
            - $ref: "#/components/schemas/PortsSpec"
        <...>

Expected behavior The description should be out of the "allOf".

    CreateSession:
      <...>
        devicePorts:
          description: The ports used locally by the device for flows to which the requested QoS profile should apply. If omitted, then the qosProfile will apply to all flows between the device and the specified application server address and ports
          allOf:
            - $ref: "#/components/schemas/PortsSpec"
        applicationServerPorts:
          description: A list of single ports or port ranges on the application server
          allOf:
            - $ref: "#/components/schemas/PortsSpec"
         <...>
RandyLevensalor commented 1 year ago

@maxl2287 Thanks for sharing your issues with open api generators.

The allOf with the description was added so that when the OpenAPI documentation is rendered it displays the top level description and not the description of the $ref. We use this in both redoc and the swagger editor

To my knowledge, this is consistent with the Open API 3.0 specification. Please share if you find a reference in the spec where this isn't compliant.

Which openapi generators are you having issues with? Have you confirmed that this isn't a bug with the generator?

@jlurien I believe that you helped with this formating. Can you comment as well?

eric-murray commented 1 year ago

Both the Swagger editor and ReDoc appear happy with this alternative construct, so I would support this for v0.10.0 onwards.

For v0.9.0 however, I think that should remain as it is, as this is a tooling rather than specification issue, and a release candidate version was available for some time to give developers an opportunity to pick up on such issues with their tooling. It is not so difficult to make the changes manually if it is desired to implement v0.9.0 and your tooling objects.

Alternatively, if we update v0.10.0-wip now, then that updated version (which will remain available through github, even if subsequent changes are made) will still effectively be v0.9.0, and can be used for implementation with tooling that experiences this issue.

It was proposed in Commonalities that we adopt OAS v3.1 so that we can use reference objects that support descriptions without the need for the allOf workaround. If you have an opinion on this, please comment that issue.

maxl2287 commented 1 year ago

@RandyLevensalor

@eric-murray I guess when we will have v3.1 then maybe these "allOf"-behaviours may also be gone, yes.

patrice-conil commented 1 year ago

Hi @maxl2287, I had the same problem and offered to migrate to oas 3.1.0 in commonalities/issue#11. Please take a look and add your comments.

jlurien commented 1 year ago

The alternate proposal is OK as it preserves the principle of $ref not having sibling elements. OAS 3.1 would also solve this but migrating to it has other implications. It allows to use all the potential JSON-schema to define more complex schemas for objects, which is nice, but many tools don't support it properly, so we'd probably have to limit in the guidelines how to use it or face many issues complaining about certain tools not supporting this or that.

hdamker commented 1 year ago

I'm here with @eric-murray and @jlurien

jlurien commented 1 year ago

The new feature in OAS 3.1 is not supported by the viewers (Redoc, Swagger editor), while the alternate proposal is, so I would stick to the new alternate version. About fixing 0.9.x, it depends on the severity of the issue with the tooling. It is not a functional change so release scope is not altered.

patrice-conil commented 1 year ago

Hi all, Openapi-generator 7.0.0-beta supports allOf with description inside but generates unwanted extra models...as explained in [commons issue 11](https://github.com/camaraproject/ Commonalities/issues/11) I had to rewrite all the definitions before generating the code. I provided a rewritten version of the spec in PI2 that one could use as a workaround.

And as I said in commonalities, editor-next.swagger.io supports version 3.1 as a redocly viewer in IntelliJ. I vote to accept PR for 0.10.0-wip ... because we can live with this workaround in 0.9.0.