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

Question on the anyOf in DeviceIpv4Addr #276

Closed jimbobbennett closed 5 months ago

jimbobbennett commented 6 months ago

Problem description

I'm looking at the DeviceIpv4Addr schema, and I'm unsure about the anyOf and if it is valid OpenAPI:

(trimmed description/examples for brevity)

DeviceIpv4Addr:
  type: object
  properties:
    publicAddress:
      $ref: "#/components/schemas/SingleIpv4Addr"
    privateAddress:
      $ref: "#/components/schemas/SingleIpv4Addr"
    publicPort:
      $ref: "#/components/schemas/Port"
  anyOf:
    - required: [publicAddress, privateAddress]
    - required: [publicAddress, publicPort]

My understanding is that the intention of this anyOf is to enforce that either publicAddress and privateAddress are set, or publicAddress and publicPort.

Whilst this passes swagger validation using the action in this repo, when I view this in the swagger editor, it doesn't render this correctly:

image

Notice the anyOf is empty.

This suggests that it is not valid OpenAPI.

Expected behavior

This should be valid OpenAPI and be able to be interpreted by standard tools

Alternative solution

This is what I feel is more correct (though not as nice to read):

DeviceIpv4Addr:
  anyOf:
    - allOf:
      - $ref: "#/components/schemas/DeviceIpv4AddrObject"
      - required: [publicAddress, privateAddress]
    - allOf:
      - $ref: "#/components/schemas/DeviceIpv4AddrObject"
      - required: [publicAddress, publicPort]

DeviceIpv4AddrObject:
  type: object
  properties:
    publicAddress:
      $ref: "#/components/schemas/SingleIpv4Addr"
    privateAddress:
      $ref: "#/components/schemas/SingleIpv4Addr"
    publicPort:
      $ref: "#/components/schemas/Port"

This comes out in the swagger editor like this:

image

The anyOf now lists everything correctly, and has red asterisks to show the required fields.

I'd appreciate your thoughts on this to help my understanding.

jimbobbennett commented 6 months ago

In addition - when I run the spectral validation action that is configured for manual workflow, I also see a warning about this schema:

  977:20     hint  camara-discriminator-use        Ensure that API definition YAML files with oneOf or anyOf sections include a discriminator object for serialization, deserialization, and validation.  components.schemas.DeviceIpv4Addr
jlurien commented 6 months ago

Hi,

This is not a very common use of anyOf, but I wouldn't assume that lack of support from Swagger editor means that spec is not compliant with OAS. ,You can check that Redocly understands it, and it currently shows something very similar to what Swagger editor shows after your alternative version:

image image

Regarding the linter warning, I think that it does not apply to this case as there is no discriminator property to use.

patrice-conil commented 5 months ago

Hi @jimbobbennett ,@jlurien, I've tried both versions with openapi-generator (tagetting jaxrs-spec). Current version that uses anyOf only in required section is usable (constraints are not embedded in generated DeviceIpv4Addr but model is consistent) Jim's proposed version that uses a combination of anyOf and allOf produces an unusable model (No fields present in DeviceIpv4Addr)... probably due to the lack of a discriminator in the spec. So my preference is to continue with actual definition.

hdamker commented 5 months ago

Based on comments and the discussion within QoD Call on April 19th added "won't fix". Will be closed on April 26th if there are no further comments.