Skip to content

Can optional properties be made required when "allOf" and "discriminator" are used? #1870

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

Closed
jeff9finger opened this issue Mar 19, 2019 · 6 comments

Comments

@jeff9finger
Copy link

swagger-api/swagger-editor#1212 seem to imply that the following is legal in Open API v2 and v3.
(This is a little different than the example in the git issue because the one there is not technically correct)

  Foo:
    type: object
    properties:
      req2:
        type: string
      req3:
        type: string
      opt1:
        type: string
      opt2:
        type: string

  Bar:
    allOf:
      - $ref: '#/definitions/Foo'
      - type: object
        required:
          - req1
          - req2
          - req3
        properties:
          req1:
            type: string

But I am wondering if anyone knows whether adding a discriminator into the mix changes the behavior.

  FooType:
    type: string
    enum:
    - a
    - b
    - c

  Foo:
    type: object
    properties:
      id:
        type: string
  
  Bar:
    allOf:
      - $ref: '#/definitions/Foo'
      - type: object
        discriminator: foo_type
        required:
          - foo_type
        properties:
          foo_type:
            $ref: '#/definitions/FooType'
          optional1:
            type: string
  
  FooBar:
    allOf:
      - $ref: '#/definitions/Bar'
      - type: object
        required:
          - optional1
        properties:
          optional2:
            type: string

#1428 provides a lot of good information, but does not seem to explain this particular scenario - FooBar declares a property in the "required" list that exists in one of the other objects.

Is the above spec valid? Would it set "optional1" as required in FooBar and not required in Bar?

Thanks for any clarification.

@kh0ma
Copy link

kh0ma commented Apr 1, 2019

I also faced with this question. Could somebody clarify?

@tedepstein
Copy link
Contributor

tedepstein commented Apr 1, 2019

@jeff9finger and @kh0ma , the presence of a discriminator doesn't change the basic rules of OpenAPI's schema object. Those rules are defined in the underlying JSON Schema specification.

It works the same way here. The required keyword is a constraint, and only FooBar specifies that optional1 is required. So when you're validating an object against FooBar, the constraint applies. When you're validating an object against Bar, there is no such constraint, so optional1 is optional, not required.

What discriminator does is determine which subtype schema, if any, is used to validate the value.

You can think of discriminator as an instruction to validate the instance against another schema, depending on the discriminator property value. It says, "Find the schema whose name is the same as the foo_type value, validate the instance against that schema, and consider the result of that validation alongside all of the other constraints in this schema." In that respect, it's similar to allOf. But whereas allOf specifies subschemas at design time, discriminator specifies the subschema dynamically at runtime, depending on the discriminator property value.

I hope that helps.

@tedepstein
Copy link
Contributor

@jeff9finger , BTW, it looks like your example is using OpenAPI v2.0.

I'm just highlighting that because discriminators are defined a bit differently in OpenAPI 3.0, to allow for mapping of discriminator values to arbitrary schema names or references.

@jeff9finger
Copy link
Author

@tedepstein Thanks this does explain things.

One of my additional questions is around code generation tools. I am assuming the most of the popular code generation tools would generate this correctly.

I would assume so, because the "required keyword is a constraint".

@tedepstein
Copy link
Contributor

@jeff9finger, I think most code generators should get the inheritance hierarchy right, with or without the discriminator keyword. But I can't say whether any given code generator will observe the discriminator value to instantiate the right subclass during message deserialization, or whether it will do anything meaningful with the required constraint.

The OpenAPI specification doesn't put any restrictions on what code generators can or can't do. And it's quite possible that you'll see different code generators handling these things in very different ways.

@jeff9finger
Copy link
Author

That is what I would expect. Thanks

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

No branches or pull requests

3 participants