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

Better support for polymorphism(replacement for discriminator) #707

Closed
IvanGoncharov opened this issue Jun 1, 2016 · 7 comments
Closed

Comments

@IvanGoncharov
Copy link
Contributor

IvanGoncharov commented Jun 1, 2016

Many people including myself asked for full support of JSON Schema.
But I totally understand why oneOf & anyOf are causing a lot of problems:
It was never intended to use for anything beyond validation.

discriminator was good attempt to solve most of the common use cases.
We tried it with our customers and even have added support for it in our documentation engine.
But we have discovered a few issues that limit usability of discriminator:

  1. The value of discriminator field should match one of the keys in definitions object.
    If you use multiple discriminator inside multiple payloads you will experience name clashes.
    It also breaks namestyle, since a lot of people name schema in camelcase and field values using underscores.
    And with added clarification on namespaces, characters in definitions name #634 merged(schema names limited to a-zA-Z0-9.-_\) it creates a big problem for non-English field values.
  2. There is no support for the default value of discriminator.
  3. Limited support in tooling; IMHO limiting factor is that it very hard to detect which types are inherited.
    You need to go through all schemas and inspect allOfs and it becomes especially problematic when inherited schemas exist in separate files.
  4. I'm not aware of any validator that supports discriminator. A solution would be to provide translator of discriminator into oneOf and use standard JSON Schema validator afterwards.
    But translation is hard to implement because of the third point.

My proposal is to replace discriminator with a new valueSwitch keyword.
Here is modifiend discriminator example:

definitions:
  Pet:
    type: object
    properties:
      name:
        type: string
      petType:
        type: string
    required:
    - name
    - petType
    # Proposed keyword
    valueSwitch:
      # Name of the property which values are used to determinate apropriate subschema
      property: petType
      # Map of property values on Schema Objects
      values:
        Cat:
           $ref: '#/definitions/CatMixin'
        Dog:
           $ref: '#/definitions/DogMixin'
      # Schema which should be used when property is missing or
      # it's value doesn't match any key from `values` object
      default:
        description: Use specified pet type
        properties:
          ICZN:
            description: International Code of Zoological Nomenclature
            type: string
        required:
          - ICZN
  CatMixin:
    description: A representation of a cat
    type: object
    properties:
      huntingSkill:
        type: string
        description: The measured skill for hunting
        default: lazy
        enum:
        - clueless
        - lazy
        - adventurous
        - aggressive
    required:
    - huntingSkill
  DogMixin:
    description: A representation of a dog
    type: object
    properties:
      packSize:
        type: integer
        format: int32
        description: the size of the pack the dog is from
        default: 0
        minimum: 0
    required:
    - packSize

In the example I also added support for user-defined petType to illustrate usage of default keyword.
And here is how it solves problems mentioned above:

  1. field values from payload are used only as keys of values object.
  2. Support for default, see above example.
  3. To fully understand schema you need to go only through schema itself and all schemas referenced from it.
  4. You can translate above valueSwitch into pure JSON Schema Draft4 using straightforward algorithm, for example:
oneOf:
  - type: object
    required:
      - petType
    properties:
      petType:
        enum:
          - Cat
    allOf:
      - $ref: '#/definitions/CatMixin'
  - type: object
    required:
      - petType
    properties:
      petType:
        enum:
          - Dog
    allOf:
      - $ref: '#/definitions/DogMixin'
  - properties:
      petType:
        not:
          enum:
            - Cat
            - Dog
    allOf:
      description: Use specified pet type
      properties:
        ICZN:
          description: International Code of Zoological Nomenclature
          type: string
      required:
        - ICZN

Moreover it is possible to convert discriminator into valueSwitch for all existing Swagger specs.

@ralfhandl
Copy link
Contributor

I like the basic idea, but wonder how to express the multiple levels of inheritance: how would I specialize dogs into hunting dogs and shepherd dogs? Would I have to "flatten" the inheritance hierarchy by adding members HuntingDog and ShepherdDog to the valueSwitch/values object and provide an array of incremental mixins for them?

    valueSwitch:
      # Name of the property which values are used to determinate apropriate subschema
      property: petType
      # Map of property values on Schema Objects
      values:
        Cat:
           $ref: '#/definitions/CatMixin'
        Dog:
           $ref: '#/definitions/DogMixin'
        HuntingDog:
           - $ref: '#/definitions/DogMixin'
           - $ref: '#/definitions/HuntingDogMixin'
        ShepherdDog:
           - $ref: '#/definitions/DogMixin'
           - $ref: '#/definitions/ShepherdDogMixin'

@wparad
Copy link

wparad commented Jun 6, 2016

This feels strange. Strong-typed languages will most likely have a problem with this sort of specification, given that on-the-fly framework conversions will fail. For example what should the request object type actually be? (Or what is the response object type?) Although, it would be nice to be able to specify different validations--i.e. C if A or D if B, but I'm not sure that this is the best approach.

@IvanGoncharov
Copy link
Contributor Author

IvanGoncharov commented Jun 6, 2016

Would I have to "flatten" the inheritance hierarchy by adding members HuntingDog and ShepherdDog to the valueSwitch/values object and provide an array of incremental mixins for them?

@ralfhandl You just need to use allOf for it:

    valueSwitch:
      # Name of the property which values are used to determinate apropriate subschema
      property: petType
      # Map of property values on Schema Objects
      values:
        Cat:
           $ref: '#/definitions/CatMixin'
        Dog:
           $ref: '#/definitions/DogMixin'
        HuntingDog:
           allOf:
             - $ref: '#/definitions/DogMixin'
             - $ref: '#/definitions/HuntingDogMixin'
        ShepherdDog:
           allOf:
             - $ref: '#/definitions/DogMixin'
             - $ref: '#/definitions/ShepherdDogMixin'

values has normal Schema Objects as values, so you can use everything that Swagger support:
allOf, $ref, type, etc. You can even add nested valueSwitch 😄

This feels strange. Strong-typed languages will most likely have a problem with this sort of specification, given that on-the-fly framework conversions will fail. For example what should the request object type actually be? (Or what is the response object type?)

@wparad It possible to implement in all typed-languages I know of, most of them natively support polymorphism and the rest support "union"-like types. You can even implement it in pure C, using union type. What's important you always have access to property used for switching, petType in above example. On implementation side it very simple you just add single switch which executed in runtime and it exactly the same as current discriminator.

It would be nice to be able to specify different validations--i.e. C if A or D if B, but I'm not sure that this is the best approach.

In really most of the time developers used only two patterns for dynamic data in JSON:

  1. Attribute where different values signalize different payloads, covered by this proposal
  2. Different types, for example, single string or array of strings. I working on a similar proposal, which adds new typeSwitch keyword.

To handle exotic cases you need to add something very similar to oneOf and that's clearly not going to happen. So instead of creating some theoretical all in one solution let's solve real-life problems without introducing too much complexity into tooling and the spec itself.

@fehguy
Copy link
Contributor

fehguy commented Jun 9, 2016

I'd like to suggest another approach to handling the general idea here. How about we get some examples of different modeling challenges and then put together a comprehensive solution from them? Maybe it's been done but It would be great to start linking to them here.

@pdo400
Copy link

pdo400 commented Jun 17, 2016

I believe this has already been done quite thoroughly, resulting in the widely accepted json-schema draft 4.

@webron
Copy link
Member

webron commented Jul 21, 2016

Tackling PR: #741

@handrews
Copy link
Member

There have been several developments related to this in more recent drafts of JSON Schema.

draft-07's "if"/"then"/"else" can be used to make switching conditions explicit. You can use those keywords with anyOf or oneOf to implement an n-way switch.

The vocabulary definition and management proposals for draft-08 are primarily intended to allow adding a set of code generation keywords to augment the validation keywords. The two use cases are quite different, which has resulted in very awkward attempts to cram data definition into the validation constraint system. See json-schema-org/json-schema-spec#561 if you want to upvote the vocabulary support proposal.

Note that here in the OAS repo, #1443 is tracking whether/how to support newer drafts.

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

7 participants