ng-openapi-gen icon indicating copy to clipboard operation
ng-openapi-gen copied to clipboard

Required properties on an extended schema are not handled expectedly

Open tajnymag opened this issue 5 years ago • 4 comments

Given this schema:

openapi: 3.0.1
# meta info (skipped for shorted snippet)
components:
  schemas:
    Method:
      type: string
      enum:
        - "get"
        - "post"
        - "delete"
        - "patch"
        - "put"
    Status:
      type: string
      enum:
        - "new"
        - "ok"
        - "remote"
        - "lost"
    DhcpMode:
      type: string
      enum:
        - "static"
        - "dhcp"
    Version:
      type: object
      properties:
        major:
          type: integer
        minor:
          type: integer
        revision:
          type: integer
      required:
        - major
        - minor
        - revision
    INetAddress:
      oneOf:
        - type: string
          format: ipv4
        - type: string
          format: ipv6
    ConnectionType:
      type: string
      enum:
        - "serial"
        - "slave"
        - "network"
    Profile:
      allOf:
        - $ref: "#/components/schemas/ProfilePartial"
      required:
        - name
        - active
        - connections
    ProfilePartial:
      type: object
      properties:
        name:
          type: string
        active:
          type: boolean
        connections:
          type: array
          items:
            $ref: "#/components/schemas/MasterDevice"
    Connection:
      type: object
      properties:
        id:
          type: string
        status:
          $ref: "#/components/schemas/Status"
        name:
          type: string
        last:
          type: boolean
        persistent:
          type: boolean
      required:
        - id
        - status
        - name
        - last
        - persistent
    SerialConnection:
      allOf:
        - $ref: "#/components/schemas/Connection"
        - type: object
          properties:
            port:
              type: string
            baudRate:
              type: integer
            parity:
              type: string # TODO ask Štěpán
            stopbits:
              type: string # TODO ask Štěpán
          required:
            - port
            - baudRate
            - parity
            - stopbits
    Device:
      allOf:
        - $ref: "#/components/schemas/Connection"
        - type: object
          properties:
            version:
              $ref: "#/components/schemas/Version"
            model:
              type: string
            identified:
              type: boolean
            sn:
              type: string
          required:
            - version
            - model
            - identified
            - sn
    SlaveDevice:
      allOf:
        - $ref: "#/components/schemas/Device"
        - type: object
          properties:
            address:
              type: integer
          required:
            - address
    NetworkDevice:
      allOf:
        - $ref: "#/components/schemas/Device"
        - type: object
          properties:
            address:
              $ref: "#/components/schemas/INetAddress"
            changed:
              type: boolean
            subnetCidr:
              type: integer
            subnetMask:
              $ref: "#/components/schemas/INetAddress"
            gateway:
              $ref: "#/components/schemas/INetAddress"
            dns:
              $ref: "#/components/schemas/INetAddress"
            dhcpMode:
              $ref: "#/components/schemas/DhcpMode"
            modbusConnected:
              type: boolean
            dataConnected:
              type: boolean
            cfgConnected:
              type: boolean
            artnetConnected:
              type: boolean
          required:
            - address
            - changed
            - subnetCidr
            - subnetMask
            - gateway
            - dns
            - dhcpMode
            - modbusConnected
            - cfgConnected
            - artnetConnected
    MasterDevice:
      allOf:
        - $ref: "#/components/schemas/MasterDevicePartial"
      required:
        - type
        - slaveChainExpanded
        - slaves
    MasterDevicePartial:
      allOf:
        - oneOf:
            - $ref: "#/components/schemas/SerialConnection"
            - $ref: "#/components/schemas/NetworkDevice"
        - type: object
          properties:
            type:
              allOf:
                - $ref: "#/components/schemas/ConnectionType"
            slaveChainExpanded:
              type: boolean
            slaves:
              type: array
              items:
                $ref: "#/components/schemas/SlaveDevice"
    Event:
      type: object
      properties:
        method:
          $ref: "#/components/schemas/Method"
        endpoint:
          type: string
          enum:
            - "profiles"
            - "connections"
        data:
          type: object
      required:
        - method
        - endpoint
        - data
    Interface:
      allOf:
        - $ref: "#/components/schemas/InterfacePartial"
      required:
        - id
        - name
        - active
        - scanning
    InterfacePartial:
      type: object
      properties:
        id:
          type: string
        name:
          type: string
        active:
          type: boolean
        scanning:
          type: boolean
paths:
# continues (skipped for shorter snippet)

Among other files I get these:

profile-partial.ts (expected)

/* tslint:disable */
import { MasterDevice } from './master-device';
export interface ProfilePartial {
  active?: boolean;
  connections?: Array<MasterDevice>;
  name?: string;
}

partial.ts (unexpected: in my schema, I'm setting all properties of an included InterfacePartial to be required and yet in the generated file they are not being generated as at all ==> thus they are kept optional)

/* tslint:disable */
import { ProfilePartial } from './profile-partial';
export interface Profile extends ProfilePartial {
}

Exact same behavior can be seen on master-device.ts and master-device-partial.ts.

I would expect the properties to be included in the Partial interface to set them their required status.

Probably related to this, these samples do not transform into the same interface. The first one generates correctly and the second one is generated with "address" being optional.

SlaveDevice:
      allOf:
        - $ref: "#/components/schemas/Device"
        - type: object
          properties:
            address:
              type: integer
          required:
            - address
SlaveDevice:
      allOf:
        - $ref: "#/components/schemas/Device"
        - type: object
          properties:
            address:
              type: integer
      required:
        - address

tajnymag avatar May 31 '20 14:05 tajnymag

I didn't even know it was possible to have a required for a property defined in a inherited schema. The specifications are generally not that specific. I found this bug report that indicates that swagger-ui 3 does allow such case: https://github.com/swagger-api/swagger-editor/issues/1212 I'm not sure, but wouldn't be exactly easy in the current generator design. At least would require 2 passes through the models, because there's no need for the child model to be declared before the parent model. I'll try to find a solution, but can't guarantee any ETA.

luisfpg avatar Jun 01 '20 11:06 luisfpg

@luisfpg Yes, that's how I validated I'm not going against the spec. In Swagger Editor, the properties are inherited and set correctly.

Optional multi-pass would be great, as as right now the generation process seems to be very fast anyway.

I understand that it could be a pain in the ass to implement. Thank you for considering it :)

tajnymag avatar Jun 01 '20 11:06 tajnymag

@luisfpg Any ETA for this yet? We're experiencing the same issues.

cstruter avatar Jul 06 '20 13:07 cstruter

Sorry, no idea when / if this will be implemented. I have to wait on #110, because it would change the implementation anyhow. Afterwards, I can see if I can do it.

luisfpg avatar Jul 14 '20 11:07 luisfpg