Skip to content

Clarification on using null+type:object in composed schemas v3.1.0 #2215

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
spacether opened this issue Apr 24, 2020 · 7 comments
Closed

Clarification on using null+type:object in composed schemas v3.1.0 #2215

spacether opened this issue Apr 24, 2020 · 7 comments

Comments

@spacether
Copy link

spacether commented Apr 24, 2020

Hi team,
In the upcoming v3.1.0 spec we will be allowed to use type: "null" definitions.
Using a discriminator allows us to quickly move through an inheritance tree using the discriminator.propertyName passed in a payload.
So if discriminator.propertyName = "animalType"
and we deserialize an Animal with the payload {"animalType: "Dog"} this works.
But sometimes users want to pass in empty payload values like null or empty dict {}.
Should we allow those types in the oneOf definition when we are in a composed schema?

Question 1: clarification on using type: "null" + object in oneOf w/ discriminator

Should this be valid?

Animal:
oneOf:
- $ref: NullType
- $ref: Pet
- $ref: ObjectType
discriminator:
  propertyName: animalType

NullType:
type: "null"

ObjectType:
type: object

Some context in python is that None and {} are both Falsey values for a variable that holds a dict.

My proposal is that we allow these null and {} empty types because some clients will expect, accept and understand these empty/zero-state values.
This would allow for rapid deserialization through an inheritance tree while also allowing servers to return null/{} empty state values.

Note about the where we would include type: object in the oneOf list:
If we do allow empty object type, then we may want to list it last.
That would be safer for generator that loop through oneOf types and deserialize if the payload matches the schema. That would allow a match on Pet before {}.
This point applies more when one includes type: object without using a discriminator in Animal.
Because order matters for that use case, it would be simplest if we require that the type: object definition be the last one when used in oneOf.

@handrews @philsturgeon @tedepstein @johncmunson

This question came up here

Question 2: clarification on using type: type: "null" + object in anyOf without discriminator

Should this be invalid?

Animal:
allOf:
- $ref: Blah
anyOf:
- type: null
- type: object

I think that null and type: object should be invalid in anyOf definition.

Question 3: clarification on using type: type: "null" + object in allOf without discriminator

What should we do for allOf?

Animal:
allOf:
- type: null
- type: object

Type null should be invalid because it does not apply to object schemas
Type: object. Not sure about how I feel on this one, it adds no new information to the schema.

@spacether spacether changed the title Clarification on using null+type:object in a oneOf schema w/ discriminator, v3.1.0 Clarification on using null+type:object in a composed schemas v3.1.0 Apr 24, 2020
@spacether spacether changed the title Clarification on using null+type:object in a composed schemas v3.1.0 Clarification on using null+type:object in composed schemas v3.1.0 Apr 24, 2020
@philsturgeon
Copy link
Contributor

I believe a lot of this is covered in #2143, where the oneOf doesn't need a discriminator to make it work.

@spacether
Copy link
Author

@philsturgeon I read through that issue and didn't see any examples or discussion that addressed these three questions. These questions hinge around mixing types in oneOf/anyOf/allOf definition. Did I miss something? Where specifically did you mean?
Can you tell me what you think we should do in each of the above 3 cases and why?

@handrews
Copy link
Member

@spacether TBH I can never remember how discriminator works, except that (from a JSON Schema processing model perspective) it's really problematic. I think because it implicitly (again, from the point of view of what JSON Schema understands) changes how validation schemas are applied.

I have no idea whether type: "null" would be allowed in OAS 3.1. I see your point about it being a use case (I personally avoid nulls but that's a stylistic choice related to available typing systems, and sometimes you have to work with what's already there so we should take that into account).

If you ignore whatever discriminator is doing, your oneOf example is perfectly valid JSON Schema aside from the $refs not looking like proper URI references. I assume that's just shorthand for the example.

The schema:

allOf:
  - type: object
  - type: "null"

will always fail validation. The type keyword applies to all data types (and constrains which ones are legal). An instance can never be both null and an object at the same time, so this always fails.

@philsturgeon
Copy link
Contributor

@philsturgeon I read through that issue and didn't see any examples or discussion that addressed these three questions.

@spacether the whole thing is super relevant, from the possibility of it being deprecated entieely, to the examples showing oneOf mixed with discriminator. This issue talks about how a discriminator can be replaced with oneOf usage, so you don't need to use discriminator in the first place.

Here's a polymorphic example I helped a client with. Actions can be one of these two types, and common action shares some common properties.

type: object
properties:
  id:
    type: string
  action:
    oneOf:
      - $ref: '#/definitions/ActionType1'
      - $ref: '#/definitions/ActionType2'
definitions:
  ActionType1:
    allOf:
      - $ref: '#/definitions/CommonAction'
      - type: object
        properties:
          type:
            type: string
            enum: [type1]
          type1SpecificField:
            type: string
            example: someValue
  ActionType2:
    allOf:
      - $ref: '#/definitions/CommonAction'
      - type: object
        properties:
          type:
            type: string
            enum: [type2]
          type2SpecificField:
            type: string
            example: someValue
  CommonAction:
    type: object
    properties:
      commonField:
        type: string
        example: someCommonValue

The enum: [type1] and enum: [type2] is the replacement for discriminator (in OAS3.1 that'll be const: type1 and const: type2.

Beyond that, if discriminator is deprecated, question 2 and 3 seem a bit moot. Question 2 is possibly either valid or invalid with or without a discriminator (depends if a solo type: null works or not), and question 3 is invalid with or without a discriminator because an allOf cannot enforce two totally different types at the same time.

Are you seeking edge cases for your code generator or are you trying to find out how to use something for a project, because maybe that sort of background would provide better context for helping you out.

@spacether
Copy link
Author

spacether commented Apr 30, 2020

@philsturgeon in the questions I say whether the discriminator exists, so questions 2 + 3 only apply to non-discriminator use cases.

Thanks for sharing that context about the possibility of the discriminator being deprecated. My question 1 is specifically about what we do with a discriminator. When will the spec committee make a decision about whether to deprecate it? The current draft spec includes the discriminator and I asked question 1 because the payloads that I write about deserializing lack the required propertyName key value pair when they are null or {}.

Thank you for letting me know what you think should happen for use cases 2 + 3.
It sounds like this is a summary of how non-object types may be used in questions 2 + 3:

  • a. VALID: oneOf w/ non-object types + NO allOF + NO anyOf + NO properties in the composed schema
  • b. VALID: anyOf w/ non-object types + NO allOF + NO oneOf + NO properties in the composed schema
  • c. INVALID: non-object types in allOf

Should the 3.1.0 spec clarify the above a+b+c summary?

a + b can include object types in their definitions also, but the non-object types trigger this invalidity if allOf or properties or the other XOf field contains information.

@sebastien-rosset
Copy link
Contributor

sebastien-rosset commented Apr 30, 2020

@spacether the whole thing is super relevant, from the possibility of it being deprecated entieely, to the examples showing oneOf mixed with discriminator. This issue talks about how a discriminator can be replaced with oneOf usage, so you don't need to use discriminator in the first place.

One unanswered question about deprecating the discriminator is the performance impact, as discussed in #2143. Until there is a solid proposal, we need to continue supporting discriminators, and even if that happens, there will be OAS documents with discriminators for a long time (there are still lots of OAS v2 documents around).

Are you seeking edge cases for your code generator or are you trying to find out how to use something for a project, because maybe that sort of background would provide better context for helping you out.

I'll let Justin respond for his use cases. In our case (Cisco), we are both authors and consumers of OAS documents. We have server-side implementation of OAS v3, clients for v2 and v3 in multiple languages (java, python, go, powershell), and we contribute to OpenAPITools such that as a goal, the tooling can work with any input OAS v3 document, not just ours. We need to be able to consume OAS documents that have been written by 3rd parties, which may or may not have edge cases. Some OAS doc tend to be using bleeding edge features, other are using basic features from OAS v2, and everything in between. We do use discriminators because without it performance is significantly worse (see #2143 for why). We have also started using the "null" type from OAS 3.1.

Related to this, I have created #2216 as an attempt to clarify the discriminator.

@handrews
Copy link
Member

To summarize: All of the examples in the questions are "valid" in the sense that they will parse and tools can try to use them. But (like many things that are legal to do in JSON Schema), they may or may not make any sense.

  • allOf two conflicting types, no matter what types they are, will always fail
  • anyOf multiple types is valid and has many use cases. You more often see this with oneOf but it could happen with anyOf if there are other branches that might be true alongside one of the branches with different types
  • trying to use discriminator with schemas that don't include the type discriminating field falls under "garbage in, garbage out" - tools can do whatever with it. As an OAD author, you probably want to anyOf the "null" type outside of the discriminator construct - discriminator only makes sense if it is not null.

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

4 participants