astm-utm / Protocol

ASTM UTM Protocol (API and sequence diagrams)
15 stars 10 forks source link

if 'anyOf' is imported to AWS API Gateway, schema-level validations are silently omitted by Gateway #25

Open issmith1 opened 4 years ago

issmith1 commented 4 years ago

This is a note for awareness. 'anyOf' definition is not supported in AWS API Gateway.
A workaround is to pre-process the interface spec to remove the anyOf structure. https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-known-issues.html

Also note that for 'anyOf', neither swagger nor OpenAPI generators (openapi-generator-cli-4.3.1, swagger-codegen-cli-3.2.0 and 3.2.1) create correct code for the language spring. I didn't test other languages.

OpenAPI Codegen had issues with inheritance which appear to be closed for some languages but other languages seem to be lacking contributors. https://github.com/OpenAPITools/openapi-generator/issues/2845

It seems that anyOf is being used so that industry can augment the spec which makes sense.

BenjaminPelletier commented 4 years ago

anyOf is in use solely because OpenAPI does not allow description to be defined along side $ref. Either anyOf or allOf could be used as workarounds (as suggested on the linked issue), but we have already identified two tools that work with anyOf but don't work with allOf (ReDoc is one of those tools, and the primary viewer used to consume the API). We should not write invalid OpenAPI in the API, which is what description + $ref would be, but we would welcome any alternate suggestions you may have.

issmith1 commented 4 years ago

Gosh knowing anyOf is used for description, the cost/benefit includes people who are confused about anyOf's for an array of length 1. AWS API Gateway documents its lack of support, but for the open source generators, determining if your language is broken for inheritance is time consuming, involving looking at both generators (OpenAPI/swagger), looking for regressions by generator version, etc.

Perhaps use the Scheme-level description block to enhance its child/properties descriptions?

When we worked with Swagger v2 spec we were frustrated about the limitations in descriptions. Our workaround was to use the parent/schema level description blocks. People learned to review that along with the properties.

in Swaggerhub, you can see specs with extensive description blocks for both OA3 and swagger2, eg) https://app.swaggerhub.com/apis/BetaSwagger/AgreementChangeRequest/partner-3-RC1

BenjaminPelletier commented 4 years ago

I'm just following up on this now -- could you elaborate on your "Scheme-level description block" suggestion? I don't seem to be able to view your link.

To be concrete, this is what we would like to do, but aren't allowed to do because of OpenAPI specifications: (example 1)

addresses:
  homeAddress:
    description: The premises at which electricity is provided.
    $ref: "#/definitions/Address"
  invoiceAddress": {
    description: The billing address - must be where the customer's credit card is registered.
    $ref: "#/definitions/Address"

The allOf workaround is this: (example 2)

addresses:
  homeAddress:
    description: The premises at which electricity is provided.
    allOf:
      - $ref: "#/definitions/Address"
  invoiceAddress": {
    description: The billing address - must be where the customer's credit card is registered.
    allOf:
      - $ref: "#/definitions/Address"

The anyOf workaround is nearly the same, but the difference is that it seems like more tools correctly support anyOf than correctly support allOf, though neither is universally supported correctly. One primary driver of "anyOf" over "allOf" is that ReDoc handles anyOf more correctly than allOf, and that is a primary consumption mechanism of the API. (example 3)

addresses:
  homeAddress:
    description: The premises at which electricity is provided.
    anyOf:
      - $ref: "#/definitions/Address"
  invoiceAddress": {
    description: The billing address - must be where the customer's credit card is registered.
    anyOf:
      - $ref: "#/definitions/Address"

We certainly wouldn't want to make a different data type in each place where the data type was used: (example 4)

addresses:
  homeAddress:
    $ref: "#/definitions/HomeAddress"
  invoiceAddress": {
    $ref: "#/definitions/InvoiceAddress"
...
HomeAddress:
  description: The premises at which electricity is provided.
  [copy-pasted content]
InvoiceAddress:
  description: The billing address - must be where the customer's credit card is registered.
  [copy-pasted content]

Sometimes we could add descriptions of fields in the description of the data type: (example 5)

addresses:
  description: |-
    homeAddress is the premises at which electricity is provided.
    invoiceAddress is the billing address - must be where the customer's credit card is registered.
  homeAddress:
    $ref: "#/definitions/Address"
  invoiceAddress": {
    $ref: "#/definitions/Address"

...but that would be confusing when documentation for some fields are in one place and documentation for sibling fields are in a different place: (example 6)

addresses:
  description: |-
    (place 1)
    homeAddress is the premises at which electricity is provided.
    invoiceAddress is the billing address - must be where the customer's credit card is registered.
  homeAddress:
    $ref: "#/definitions/Address"
  invoiceAddress": {
    $ref: "#/definitions/Address"
  primaryAddress:
    description: The address at which the customer primarily conducts business (place 2)
    type: string
    enum:
      - homeAddress
      - invoiceAddress

...and when there are multiple fields with very long descriptions, the parent description becomes very, very long and separated from the fields to which they apply.

How would inheritance work? (example 7)

URL:
  type: string
  regex: '^https?:\/\/.+'
USSBaseURL:
  description: |-
    The base URL of a USS implementation of part or all of the USS-USS API. Per the USS-USS API, the full URL
    of a particular resource can be constructed by appending, e.g., `/uss/v1/{resource}/{id}` to this base URL.
    Accordingly, this URL may not have a trailing '/' character.
  $ref: '#/definitions/UUIDv4'
OperationsBaseURL:
  description: |-
    The base URL of a USS implementation that implements the parts of the USS-USS API necessary for
    providing the details of this Operation, and telemetry during non-conformance or contingency,
    if applicable.
  $ref: '#/definitions/USSBaseURL'
SubscriptionBaseURL:
  description: |-
    The base URL of a USS implementation of the parts of the USS-USS API necessary for
    receiving the notifications requested by this Subscription.
  $ref: '#/definitions/USSBaseURL'
issmith1 commented 4 years ago

I should have stated that when a spec containing 'anyOf' is imported to AWS API Gateway (AWSGW), the result is that some AWSGW-performed bean validations are silently omitted. I am retitling this issue to help other developers discover this issue, rather than having to debug its pernicious effect.

As mentioned, NASA developers (especially those new to codegen or AWSGW) spent time looking into whether 'anyOf' is providing runtime functional effects. Knowing anyOf is merely a workaround (I just discovered the swagger editor gens a warning, below) would have allowed devs to skip the investigation before implementing the anyOf strip before AWSGW import.

swag-warning

Thank you for your detailed examples. The suggested workaround is #5 with limitation #6, which uses CommonMark 0.27 formatting in the parent schema's description.

  addresses:
      type: object
      description: >
        Property definitions vary depending its context:
          > *homeAddress:*  The premise at which electricity is provided.

          > *invoiceAddress:*  The billing address. It must be where the customer's credit card is registered.

      properties:
         homeAddress:
            $ref: "#/components/schemas/Address"
         invoiceAddress:
            $ref: "#/components/schemas/Address"

In #7 if inheritance is needed then oneOf/anyOf/allOf must be used.

BenjaminPelletier commented 3 years ago

Update: The best solution to this problem is to use $ref siblings allowed in OpenAPI 3.1.0 (via JSON Schema 2020-12), however as of today, ReDoc does not quite support OpenAPI 3.1.0 and SwaggerHub does not support OpenAPI 3.1.0, so the anyOf workaround for 3.0.x will remain in place for now.