Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: name of object not shown in oneOf #932

Closed
bartkummel opened this issue May 31, 2019 · 9 comments · May be fixed by #1977
Closed

Regression: name of object not shown in oneOf #932

bartkummel opened this issue May 31, 2019 · 9 comments · May be fixed by #1977

Comments

@bartkummel
Copy link

When upgrading from 2.0.0-rc.4 to 2.0.0-rc.5, the names of objects to choose from are lost. See the screenshots:

rc.4:
Schermafbeelding 2019-05-31 om 14 47 42

rc.5:
Schermafbeelding 2019-05-31 om 14 45 49

(I also tried rc.8-1, that one still has the problem.)

@RomanHotsiy
Copy link
Member

Could you share your spec? It is not the case for all the titles.

@bartkummel
Copy link
Author

@travis-supalla
Copy link

travis-supalla commented Jul 19, 2019

@bartkummel - We had a similar issue once upgrading to a higher version after 2.0.0-rc.4. The problem was caused on our end with the way we had structured the oneOfs. The issue was caused by a oneOf in an allOf.
I was able to fix the formatting from something like this:

allOf:
 - oneOf:
  - $ref: '#/components/schemas/OptionOne'
  - $ref: '#/components/schemas/OptionTwo'

to just this:

oneOf:
 - $ref: '#/components/schemas/OptionOne'
 - $ref: '#/components/schemas/OptionTwo'

As long as the title was the first thing set in the schema for each option it worked.
It might be something to look at.

@bartkummel
Copy link
Author

Hi @travis-supalla,

Thanks for your suggestion! We do have a nested oneOf inside an allOf. The example from the screenshots corresponds to this snippet from our OAS spec:

        constraint:
          allOf:
            - description: |
                In the context of an `Event`, a `Constraint` can be used to constrain the order of events. This can be useful if events don't have
                any timestamp but you need to guarantee that a certain event occurs before another event. The only constraint types that are valid in
                the context of an `Event` are:

                - `eventOrderConstraint`;
                - `andConstraint`;
                - `orConstraint`;
                - `constraintReference`.
            - oneOf:
              - $ref: '#/components/schemas/constraintReference'
              - $ref: '#/components/schemas/andConstraint'
              - $ref: '#/components/schemas/notConstraint'
              - $ref: '#/components/schemas/orConstraint'
              - $ref: '#/components/schemas/sizeConstraint'
              - $ref: '#/components/schemas/speedConstraint'
              - $ref: '#/components/schemas/weightConstraint'
              - $ref: '#/components/schemas/fuelTypeConstraint'
              - $ref: '#/components/schemas/vehicleTypeConstraint'
              - $ref: '#/components/schemas/routeConstraint'
              - $ref: '#/components/schemas/dangerousGoodsConstraint'
              - $ref: '#/components/schemas/numberOfVehiclesConstraint'
              - $ref: '#/components/schemas/eventOrderConstraint'
              - $ref: '#/components/schemas/sensorValueConstraint'
              - $ref: '#/components/schemas/sensorUpdateFrequencyConstraint'
              - $ref: '#/components/schemas/startDateTimeConstraint'
              - $ref: '#/components/schemas/endDateTimeConstraint'
              - $ref: '#/components/schemas/timeRangeConstraint'

The problem is, I don't see how I can fix this without the need to repeat the description.

Apart from that: it worked up and until rc.4, so I don't see why this can't be fixed.

@RomanHotsiy
Copy link
Member

The new behavior is technically correct. allOf-ing definitions makes modifies them so the name may be not correct. See the example:

field:
  allOf:
    - maxLength: 3
    - oneOf:
      - $ref: '#/components/schemas/nameShorterThan5
      - $ref: '#/components/schemas/surnameShorterThan5

the names namShorterThan5 here are not valid anymore because of the allOf that changed the behaviour.

In your case I would recommend you using the following schema:

constraint:
  description: |
    In the context of an `Event`, a `Constraint` can be used to constrain the order of events. This can be useful if events don't have
    any timestamp but you need to guarantee that a certain event occurs before another event. The only constraint types that are valid in
    the context of an `Event` are:

    - `eventOrderConstraint`;
    - `andConstraint`;
    - `orConstraint`;
    - `constraintReference`.
  type: object
  oneOf:
    - $ref: '#/components/schemas/constraintReference'
    - $ref: '#/components/schemas/andConstraint'
    - $ref: '#/components/schemas/notConstraint'
    - $ref: '#/components/schemas/orConstraint'
    - $ref: '#/components/schemas/sizeConstraint'
    - $ref: '#/components/schemas/speedConstraint'
    - $ref: '#/components/schemas/weightConstraint'
    - $ref: '#/components/schemas/fuelTypeConstraint'
    - $ref: '#/components/schemas/vehicleTypeConstraint'
    - $ref: '#/components/schemas/routeConstraint'
    - $ref: '#/components/schemas/dangerousGoodsConstraint'
    - $ref: '#/components/schemas/numberOfVehiclesConstraint'
    - $ref: '#/components/schemas/eventOrderConstraint'
    - $ref: '#/components/schemas/sensorValueConstraint'
    - $ref: '#/components/schemas/sensorUpdateFrequencyConstraint'
    - $ref: '#/components/schemas/startDateTimeConstraint'
    - $ref: '#/components/schemas/endDateTimeConstraint'
    - $ref: '#/components/schemas/timeRangeConstraint'
  discriminator:
      propertyName: type
      mapping:
        constraintReference: '#/components/schemas/constraintReference'
        andConstraint: '#/components/schemas/andConstraint'
        notConstraint: '#/components/schemas/notConstraint'
        orConstraint: '#/components/schemas/orConstraint'
        sizeConstraint: '#/components/schemas/sizeConstraint'
        speedConstraint: '#/components/schemas/speedConstraint'
        weightConstraint: '#/components/schemas/weightConstraint'
        fuelTypeConstraint: '#/components/schemas/fuelTypeConstraint'
        vehicleTypeConstraint: '#/components/schemas/vehicleTypeConstraint'
        routeConstraint: '#/components/schemas/routeConstraint'
        dangerousGoodsConstraint: '#/components/schemas/dangerousGoodsConstraint'
        numberOfVehiclesConstraint: '#/components/schemas/numberOfVehiclesConstraint'
        eventOrderConstraint: '#/components/schemas/eventOrderConstraint'
        sensorValueConstraint: '#/components/schemas/sensorValueConstraint'
        sensorUpdateFrequencyConstraint: '#/components/schemas/sensorUpdateFrequencyConstraint'
        startDateTimeConstraint: '#/components/schemas/startDateTimeConstraint'
        endDateTimeConstraint: '#/components/schemas/endDateTimeConstraint'
        timeRangeConstraint: '#/components/schemas/timeRangeConstraint'

@bartkummel
Copy link
Author

Thanks for the clarification! Your solution works for us.

@aleung
Copy link

aleung commented Sep 6, 2019

@RomanHotsiy

allOf-ing definitions makes modifies them so the name may be not correct.

It should be the responsibility of API designer to give it a good name, rather then Redoc hides the name in all cases.

I have an example of data model Recurrence representing recurrent period, which is used in calendar. Example of data looks like:

examples:
  repeatWeekly:
    start: '2019-09-09'
    numOfOccurrences: 5
    weekly:
      startTimeInDay: '09:45'
      endTimeInDay: '10:45'
      daysOfWeek:
        - SAT
        - SUN      

The schema contains one of 'Daily', 'Weekly' and 'Monthly' data type but right now Redoc render it to

One of 'Object' 'Object' 'Object'

Information is lost.

components:
  schemas:
    Recurrence:
      allOf:
        - oneOf:
          - $ref: '#/components/schemas/Daily'
          - $ref: '#/components/schemas/Weekly'
          - $ref: '#/components/schemas/Monthly'
        - type: object
          required:
            - start
            - numOfOccurrences
          properties:
            start:
              type: string
              format: date
            numOfOccurrences:
              type: integer
    TimeRangeInDay:
      type: object
      required:
        - startTimeInDay
        - endTimeInDay
      properties:
        startTimeInDay:
          type: string
          format: time
        endTimeInDay:
          type: string
          format: time
      example:
        startTimeInDay: '09:45'
        endTimeInDay: '10:45'
    Daily:
      type: object
      required:
        - daily
      properties:
        daily:
          description: Occuries everyday
          allOf:
            - $ref: '#/components/schemas/TimeRangeInDay'
    Weekly:
      type: object
      required:
        - weekly
      properties:
        weekly:
          description: Occuries every week
          allOf:
            - $ref: '#/components/schemas/TimeRangeInDay'
            - type: object
              required:
                - daysOfWeek
              properties:
                daysOfWeek:
                  type: array
                  items:
                    type: string
                    enum:
                      - MON
                      - TUE
                      - WED
                      - THU
                      - FRI
                      - SAT
                      - SUN
    Monthly:
      type: object
      required:
        - monthly
      properties:
        monthly:
          description: Occuries every month
          allOf:
            - $ref: '#/components/schemas/TimeRangeInDay'
            - type: object
              required:
                - dayOfMonth
              properties:
                dayOfMonth:
                  type: integer
                  maximum: 31

@alecgorge
Copy link

@RomanHotsiy @aleung I have a similar issue with a similar API. I cannot use a discriminator for my oneOf in an allOf because the options in the oneOf have completely disjoint properties.

The OpenAPI spec seems to indicate that you MAY include a discriminator when it is needed and implies that if you don't provide one the options are assumed to be disjoint (non-polymorphic).

Could it even be some sort parameter passed where I promise that all of my oneOfs are discriminated or disjoint?

Spec Code Snippet
components:
  schemas:
    addresses__create:
      title: 'Addresses: Create'
      allOf:
      - oneOf:
        - type: object
          properties:
            city:
              type: string
            country:
              "$ref": "#/components/schemas/enums__country_code_us"
            postal_code:
              "$ref": "#/components/schemas/types__us_zip_code"
            region:
              "$ref": "#/components/schemas/enums__us_state_codes"
            street_1:
              type: string
            street_2:
              type: string
          required:
          - city
          - country
          - street_1
          title: US Address
        - type: object
          properties:
            city:
              type: string
            country:
              "$ref": "#/components/schemas/enums__country_codes"
            postal_code:
              type: string
            region:
              type: string
            street_1:
              type: string
            street_2:
              type: string
          required:
          - city
          - country
          - street_1
          title: Non-US Address
      - type: object
        properties:
          contact_id:
            type: string
            pattern: uuid
        title: Address
    address:
      title: 'Address'
      allOf:
      - oneOf:
        - type: object
          properties:
            address:
              "$ref": "#/components/schemas/addresses__create"
          required:
          - beneficiary_address
          title: Inline Address
        - type: object
          properties:
            address_id:
              type: string
              pattern: uuid
          required:
          - address_id
          title: Address ID
        - type: object
          properties: {}
          title: No Address
        title: Address
    test__create:
      title: 'Test: Create'
      allOf:
      - "$ref": "#/components/schemas/address"
      - oneOf:
        - type: object
          properties:
            contact_email:
              type: string
              pattern: email
            contact_name:
              type: string
          required:
          - contact_email
          - contact_name
          title: Inline Contact Info
        - type: object
          properties:
            contact_id:
              type: string
              pattern: uuid
          required:
          - contact_id
          title: Contact
        title: Contact
      - type: object
        properties:
          ip_address:
            "$ref": "#/components/schemas/types__ipv4_address"
        title: Ip Address

@henhal
Copy link

henhal commented May 26, 2020

It may suck, but couldn't you extract the description as a separate schema and shift from 1 oneOf inside 1 allOf to N allOfs inside 1 oneOf?

Foo:
  allOf:
    - description: Foo
    - oneOf:
        - title: A
          properties:
            a: {type: string}
        - title: B
          properties:
            b: {type: string}

=>

Foo.Description:
  description: Foo

Foo:
  oneOf:
    - title: A
      allOf:
        - $ref: '#/Foo.Description'
        - properties:
            a: {type: string}  
    - title: B
      allOf:
        - $ref: '#/Foo.Description'
        - properties:
            b: {type: string}  

Even if it's just one repeated $ref, the repetition still sucks though, and personally I think ReDoc shouldn't override the title just because the presence of an allOf might make it "wrong".

A possible downside of this shift is also that some codegens, such as for typescript, will render much more verbose code if allOf + oneOf cannot be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants