equinix-labs / metal-java

Experimental OpenAPI Generated Java SDK for Equinix Metal
https://deploy.equinix.com/labs/metal-java/
Apache License 2.0
2 stars 5 forks source link

Wrapper/Model to handle anyOf over VirtualCircuit and VrfVirtualCircuit #58

Open VamshikShetty opened 1 year ago

VamshikShetty commented 1 year ago

We have multiple models/schema representing anyOf structure between VirtualCircuit and VrfVirtualCircuit.

VirtualCircuitList in non-stitched oas 3.0 as following schema structure :

properties:
  virtual_circuits:
    items:
      anyOf:
      - $ref: './VirtualCircuit.yaml'
      - $ref: './VrfVirtualCircuit.yaml'
    type: array
type: object

ref : https://github.com/equinix-labs/metal-java/blob/main/spec/oas3.fetched/openapi/public/components/schemas/VirtualCircuitList.yaml

Whereas after stitching it with openapi generator we get following :

VirtualCircuitList:
      properties:
        virtual_circuits:
          items:
            $ref: '#/components/schemas/VirtualCircuitList_virtual_circuits_inner'
          type: array
      type: object

ref : https://github.com/equinix-labs/metal-java/blob/main/spec/oas3.stitched/oas3.stitched.metal.yaml#L13421

Same is visible in create request and response schema :

  requestBody:
    content:
      application/json:
        schema:
          oneOf:
          - $ref: '../../../../../components/schemas/VirtualCircuitCreateInput.yaml'
          - $ref: '../../../../../components/schemas/VrfVirtualCircuitCreateInput.yaml'
    description: Virtual Circuit details
    required: true
  responses:
    "201":
      content:
        application/json:
          schema:
            oneOf:
            - $ref: '../../../../../components/schemas/VirtualCircuit.yaml'
            - $ref: '../../../../../components/schemas/VrfVirtualCircuit.yaml'

ref: https://github.com/equinix-labs/metal-java/blob/main/spec/oas3.fetched/openapi/public/paths/connections/connection_id/ports/port_id/virtual-circuits.yaml#L62

Stitched flavour :

      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/createInterconnectionPortVirtualCircuit_request'
        description: Virtual Circuit details
        required: true
      responses:
        "201":
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/createInterconnectionPortVirtualCircuit_201_response'

ref : https://github.com/equinix-labs/metal-java/blob/main/spec/oas3.stitched/oas3.stitched.metal.yaml#L1088

createInterconnectionPortVirtualCircuit_201_response and VirtualCircuitList_virtual_circuits_inner in stitched spec represent the same schema modelling and is generated as a result of anyOf not being wrapped in a model before consumption. This creates inconsistent flavours when java bindings are generated.

Such example exists in multiple other places and should be rectified :

  1. IPReservationList_ip_addresses_inner : https://github.com/equinix-labs/metal-java/blob/main/spec/oas3.stitched/oas3.stitched.metal.yaml#L14129
  2. MetalGatewayList_metal_gateways_inner : https://github.com/equinix-labs/metal-java/blob/main/spec/oas3.stitched/oas3.stitched.metal.yaml#L14192
  3. findIPAddressById_200_response : https://github.com/equinix-labs/metal-java/blob/main/spec/oas3.stitched/oas3.stitched.metal.yaml#L13986
  4. InstancesBatchCreateInput_batches_inner_allOf : https://github.com/equinix-labs/metal-java/blob/main/spec/oas3.stitched/oas3.stitched.metal.yaml#L14133
displague commented 1 year ago

@ctreatma Is this something that you noticed improvement with in the attempt to use redoc-cli for stitching?

ctreatma commented 1 year ago

No, switching to redoc for bundling wouldn't have an impact here. For this sort of thing, we need to extract the anyOf, oneOf, etc., to separate files so that we can replace these inline, complex models with appropriately-named external references to make the common functionality more explicit.

For example, we could create a file components/schemas/OneOfVirtualCircuit.yaml (name is an example, not necessarily committing to that) with the following contents:

oneOf:
- $ref: '../../../../../components/schemas/VirtualCircuitCreate.yaml'
- $ref: '../../../../../components/schemas/VrfVirtualCircuitCreate.yaml'

Given that file, we would update the VirtualCircuitList schema and the operation snippet in the issue description as follows:

properties:
  virtual_circuits:
    items:
      $ref: './OneOfVirtualCircuit.yaml
    type: array
type: object
requestBody:
  content:
    application/json:
      schema:
          oneOf:
          - $ref: '../../../../../components/schemas/VirtualCircuitCreateInput.yaml'
          - $ref: '../../../../../components/schemas/VrfVirtualCircuitCreateInput.yaml'
  description: Virtual Circuit details
  required: true
responses:
  "201":
    content:
      application/json:
        schema:
          $ref: '../../../../../components/schemas/OneOfVirtualCircuit.yaml'

That change would replace both the VirtualCircuitList_virtual_circuits_inner and createInterconnectionPortVirtualCircuit_201_response types with OneOfVirtualCircuit.

ctreatma commented 1 year ago

More broadly, we need to establish a pattern/practice of avoiding nested anyOf, oneOf, allOf, etc.; anywhere we use those it's an indication that there's a model that needs its own name.