Support conversion of x-discriminator even if refs don't start with "#/definitions/"

[leonardehrenfried]

In the commit 9155cd4 conversion of x-discriminator was introduced.

In it you an see that in order for a reference to be resolved successfully, it needs to start with #/definitions/.

We have a pretty big Swagger file, which is split into several files and there the references almost never start with that.

I have put a reproduction case into a gist: https://gist.github.com/leonardehrenfried/6615e92e818e3f7b22690db7ad237137

I'm hoping that helps diagnosing the issue.

Would it maybe be possible to replace the ref check with a contains("/#")?

[MikeRalphson]

I do not believe this is the right thing to do, for the following reason.

When (in your gist) you $ref defs.yml#/Transaction the only things which are in scope are:

  type: object
  properties:
    id:
      type: string
      example: A11234
      pattern: '[A-Z][0-9]{1,32}'
      readOnly: true
    account_id:
      type: string
      example: 05a2106b-92a4-4ee3-a6a3-c32f8b609de1
      format: uuid
      readOnly: true
    operation_type:
      type: string
      enum:
        - payment
        - refund
      example: payment
    payment_method:
      $ref: '#/PaymentMethod'

directly, and the value of #/PaymentMethod (indirectly).

PaymentMethod is defined as

  type: object
  x-discriminator:
    propertyName: type
    mapping:
      visa: "#/paymentMethodVisa"
      mastercard: "#/paymentMethodMastercard"
      amex: "#/paymentMethodAmex"
  properties:
    type:
      description: The type of Payment Method used for Transaction
type: string

In OAS 3.0, it is not defined if the discriminator object mapping values, when they contain a $ref-like value resolve against the current document or the root document.

Because they are not a JSON Schema $ref object, our resolver effectively can't see them, and I'm reluctant to add a special case for discriminator.mapping, especially here, where we're actually looking at x-discriminator.

Personally, I would change the x-discriminator.mapping values to be plain schema names (which are presumably resolved against #/components/schemas/ in the root/resolved document, and $ref the schema definitions from your root document.

It might also be worth raising this issue on the OAS repository (in OAS v3 terms and referencing discriminator and not x-discriminator), because it is confusing, and I say that as an OAI TSC member 😄

[leonardehrenfried]

Thanks for mulling it over and giving a detailed response. I agree that it's very confusing.

Personally, I would change the x-discriminator.mapping values to be plain schema names (which are presumably resolved against #/components/schemas/ in the root/resolved document, and $ref the schema definitions from your root document.

Are you suggesting that references should be expressed the following way?

PaymentMethod:
  type: object
  x-discriminator:
    propertyName: type
    mapping:
      visa: 
         $ref: "foo.yml#/paymentMethodVisa"
      mastercard: 
         $ref: "foo.yml#/paymentMethodMastercard"
      amex: 
         $ref: "foo.yml#/paymentMethodAmex"
  properties:
    type:
      description: The type of Payment Method used for Transaction
    type: string

[MikeRalphson]

Not quite - though that might be my preferred structure of discriminator in OAS v4!

I would (for now) do something like this:

swagger.yml:

swagger: "2.0"
info:
  title: API
  version: 1.0.0
paths:
  /:
    get:
      responses:
        default:
          description: OK
          schema:
            $ref: 'defs.yml#/Transaction'
definitions:
  paymentMethodVisa:
    $ref: 'defs.yml#/paymentMethodVisa'

defs.yml:

Transaction:
   ...
   x-discriminator:
     propertyName: type
     mapping:
       visa: "paymentMethodVisa"
       ...
paymentMethodVisa:
  ...
...

I hope I've explained the approach ok?

[leonardehrenfried]

Ah ok, I think I get it. Your suggestion is worth a try.

Do you think it's worth me suggesting the ref syntax this as an improvement for the next version of OAS? I love returning polymorphic types but so far Swagger/OAS has made it quite hard to do so.

[MikeRalphson]

Do you think it's worth me suggesting the ref syntax this as an improvement for the next version of OAS?

I think it would be worth getting some clarity on why it wasn't done this way, and around the expected resolution rules when discriminator occurs inside a sub-document.

[MikeRalphson]

Closing due to inactivity. Will reopen if workable changes come out of discussions on OAI/OpenAPI-Specification#1661 or any new issue you chose to open over at https://github.com/OAI/OpenAPI-Specification