Replace or remove discriminator

[MikeRalphson]

There are a number of issues with the current definition of discriminator (as well as a number of open issues).

• It is not JSON Schema keyword-independent
• It claims to hint/short-circuit the evaluation of the allOf/oneOf/anyOf keywords
• It has an ill-defined short-hand notation for schema names, where the scoping rules are undefined
• It can only use schema names or $refs not full sub-schemas
• Implementation ranges from patchy to non-existent

I think we can replace it with either JSON Schema if/then/else or a compact form such as:

  switch:
    property: myType
    case:
      Cat:
        ... sub-schema
      Dog:
        ... sub-schema
      Lizard:
        ... sub-schema

[jdesrosiers]

There's a new keyword called propertyDependencies that's likely to land in the next release of JSON Schema that is intended to cover discriminator use cases while fixing a lot of its problems. I'd suggest removing discriminator because propertyDependencies should make it not necessary anymore. If propertyDependencies isn't sufficient, I suggest we work it out in JSON Schema so all JSON Schema can benefit rather than just OpenAPI users.

[ahl]

Are there uses of the discriminator that cannot be modeled in JSON Schema (i.e. without the use of that field)? While it (maybe) affords the possibility of optimization, I haven't seen any implementations that are significantly more efficient than they would be using extant JSON Schema structures.

Further, I have found the description of discriminator confusing and complex. If the goal of this draft is simplicity, eliminating discriminator seems like a clear benefit with little to no down-side.

[sm-Fifteen]

There was also a proposed keyword by @handrews way back in OAI/OpenAPI-Specification#2143 (comment) that merely used a JSON pointer to designate which field to use as the discriminating field, without requiring some kind of duplication of the validation functionnality.

{
  "selectBy": "0/class",
  "oneOf": [
    {
      "if": {"properties": {"class": {"const": "foo"}}},
      "then": {...}
    },
    {
      "if": {"properties": {"class": {"const": "bar"}}},
      "then": {...}
    },
    ...
}

I do realize that it's very late to bring this up given propertyDependencies has already been merged into JSON Schema and it would be a lot more practical for everyone to not add too much stuff on top of JSON Schema (on top of the syntax being less cumbersome than if-then), but I still figured it was worth bringing up.

[mikekistler]

For the benefit of others reading this thread that aren't familiar with propertyDependencies, here's a link to the original proposal:

json-schema-org/json-schema-spec#1082 (lots of discussion here)

and here is the PR that adds the keyword:

https://github.com/json-schema-org/json-schema-spec/pull/1143/files

[mikekistler]

I think propertyDependencies is a good addition to JSON Schema, but I guess I'm not quite ready to completely abandon discriminator in OpenAPI. But I agree that it should be simplified and some of its problems fixed. Here's an idea that came to me while reviewing OAI/OpenAPI-Specification#3846 to clarify some details about discriminator in 3.1.x: ditch the "mapping" component and instead require each element of the allOf or anyOf to define its allowed values.

Here's an example:

components:
  schemas:
    Pet:
      type: object
      discriminator: petType
      properties:
        name:
          type: string
      required:
      - name
      oneOf:
      - description: A representation of a cat
        type: object
        properties:
          petType:
            type: string
            const: "cat"
          huntingSkill:
            type: string
            description: The measured skill for hunting
            enum:
            - clueless
            - lazy
            - adventurous
            - aggressive
        required:
        - petType
        - huntingSkill
      - description: A representation of a dog
        type: object
        properties:
          petType:
            type: string
            const: "dog"
          packSize:
            type: integer
            format: int32
            description: the size of the pack the dog is from
            default: 0
            minimum: 0
        required:
        - petType
        - packSize

Note that each of the subschemas in the oneOf define the value(s) of petType that are allowed / map to that subschema.

The discriminator goes back to simply naming the property that will determine which variant to use. It is now purely a "hint" -- and normal JSON schema validators can simply ignore it since the schemas already contain the essential information.

I suppose this is not terribly different than propertyDependencies, but in my view it is simpler and only uses features of JSON schema that have been around for many versions / years, so should have good tooling support.

[handrews]

This is essentially the selectBy proposal that was considered (but obviously not accepted) by JSON Schema quite a few years ago. The main benefit I can see of this is that it would support non-string values. But it would leave us in the business of extending JSON Schema again.

[mikekistler]

But it would be a very small and completely compatible extension, and as I mentioned tools could safely ignore it as it is only a hint. If we chose to keep discriminator at all, I think this design maintains the spirit of the original keyword and addresses nearly all the issues of the current design.

[handrews]

@mikekistler yes, if we decide to keep it at all I do prefer this version (selectBy was one of my proposals and I was always a bit sad it didn't get more traction - I can't remember if it was exactly the same, I probably made it a relative JSON Pointer or did something else to make it more complicated TBH).

[sm-Fifteen]

@handrews: I haven't seen anything relating to selectBy since the comment I quoted in my previous post. I didn't know it had been officially considered and not accepted; I thought it ended up being just a throwaway suggestion that nobody else really noticed. Do you have a link to the discussions?

[handrews]

@sm-Fifteen it was actually selectWith or selector and I lost that argument in favor of if/then/else. I prefer not to look back at JSON Schema but you can probably dig it up if you really want to.

[mikekistler]

One more point to raise here -- if we keep the discriminator in some form in Moonwalk -- the 3.x specs say that the discriminator must be a required property, but this is overly restrictive. Allowing the absence of the discriminator to "hint" at a particular variant should also be allowed and would better match the behavior in some language frameworks, such as .NET.