Support polymorphic validation

[whitlockjc]

Originally reported here: apigee-127/swagger-tools#241

[matthewspivey]

Is this blocked by the ambiguities in the spec (OAI/OpenAPI-Specification#403)?

[whitlockjc]

Yes and no. I think in the linked swagger-tools issue I got a good enough feel for how to do this but it's just not been a high priority. I do plan on it being done by v1.0.0 which is my current effort.

[IvanGoncharov]

@whitlockjc Do you have any progress on this?
This issue blocks me, so I can help by submitting PR for it.
I will start to work on it tomorrow, do you have any suggestion or concerns?

[whitlockjc]

I'm up for any help I can get, I've punted on this long enough. I've thought about this a few times but I have to tell you that this likely won't be easy. I've thought about it a few times but not recently. Let me give this some thought and I'll get back to you.

[IvanGoncharov]

I experience UNUSED_DEFINITION warnings, so I will try to fix it.
My idea is to find all schemes under definitions which has discriminator in them.
And then remove from unused list all schemes which have allOf with discriminator's schemes.
@whitlockjc Do you see any potential problems with such approach?

[whitlockjc]

Wait...so you don't want to implement discriminator validation, you just want to stop indirectly used references (discriminator) from being marked as UNUSED_DEFINITION? That is welcome too but that is not what it would take to do this work. :) Do feel free to reference this issue in your PR and commit message but don't use Fixes because it will not fix this issue completely. Make sense?

[whitlockjc]

As I think about this, I realize that the easiest way to do this might be to leverage JSON Schema, like we do for non-body parameters.

[IvanGoncharov]

@whitlockjc Do you mean converting discriminator into oneOf?

[IvanGoncharov]

Wait...so you don't want to implement discriminator validation, you just want to stop indirectly used references (discriminator) from being marked as UNUSED_DEFINITION?

You closed #58 as duplicate of this and it referenced swagger-api/swagger-editor#765 which is exactly what I want to fix.

[whitlockjc]

Ah, maybe I assumed on #58. "Support Discriminator" is a duplicate of this, as this issue is about general support for discriminator. I'm just saying that there are multiple parts to this:

• Handling definitions that are indirectly referenced via discriminators (Your current case)
• Handling discriminators during the request/response validation

Did that clear things up? As for how we'll do it, my though of using discriminator via oneOf, that was one idea but I've not done the work to prove that's the right approach. I'm all ears.

[ahultgren]

Any progress on this? (I'm also only interested in the unused definition warning)

[whitlockjc]

I thought @IvanGoncharov had an incoming PR for that but maybe I was wrong. I'm working on json-refs stuff now with hopes of releasing a new sway version within the next few days. At that time, I'll revisit this if Ivan hasn't.

[IvanGoncharov]

@whitlockjc We working on an alternative approach for Swagger validation.
IMHO primary limiting factor is the usage of oneOf/anyOf inside official schema this prevents generation of readable error messages.
Also, we want to build linter framework which allows for 3rd-party checks without the performance drop.

Currently, our implementation is in proof-of-concept stage and we still working on defining architecture.
Meantime, we continue to use sway to validate Swagger files but we decided to focus on developing our validator.

[whitlockjc]

That sucks. It would had been cool to make sway better instead of ending up with a competitor but such is life. Let me know if I can help.

[IvanGoncharov]

@whitlockjc It highly experimental so we don't know if this approach will work or not.
What we trying to do is to make dialect of JSON Schema in which if subschema fail it mean root schema also fail, so we need replace oneOf, anyOf, not and finally type with array as value. This allows us to produce flat error messages and also implement normal auto-completion.
But more importantly, it will allow us to bind custom handlers on particular sub-schema.
For example, if you want to validate header parameters you need to bind your callback to #/definitions/HeaderParameter subschema.
We need to write our own validator on top of this dialect and it should do all validation(including 3rd-party) in a single pass.

So as you see it very different from sway + very ambition so we not sure if it even possible.
If/When we will have a working prototype, I will send you a link, and we will discuss how we can collaborate.

[whitlockjc]

That would be great. You've been a great help with the Swagger tooling I've written and I'd rather us work together. So if I can help, let me know. Also, if there is anything in Sway that could be done better/different to make it more useful, please don't hesitate.

[ahamednijamudeensa]

Any update on the validation part? I want my request and response to be validated based on the discriminator field. Currently it validate only against the Base definitions. It would be great if this gets fixed. Let me know the current progress please

[whitlockjc]

There is nothing to report yet. I've been on vacation and I haven't worked on anything in a few weeks. I'm back and I hope to get to this soon.

[wbbradley]

I'm interested in helping out. Here's where I'm unsure of how to proceed.

Is the problem here that the Swagger Spec is ambiguous in terms of validation, or in terms of code generation? Seems like there is some confusion as to what the blocking issue is. I'm seeing a few goals that may be colliding.

• The ability to validate an API specification that includes types with discriminators, and their subtypes. (validation of the schema)
• The ability to validate data at run-time in the actual implementation of an API specified using Swagger 2.0 (validation of data coherence to schema)
• Performance considerations of writing a linter using anyOf/oneOf semantics (not sure how anyOf factors in here.)

My difficulty in understanding is coming from some ambiguity around whether the Swagger 2.0 Spec actually supports substitutability, aka "polymorphism through inheritance." According to the "discriminator" property specifier, I believe the construction is sound, and doable. However, I'm lacking some background in the semantics of JSON Schema's allOf, and whether or where anyOf comes into play.

Perhaps one issue here is the combination of composition with sum types. This type of construction makes it tricky to deal with disambiguation of members, and allows for strange partial slicing of base types, or double type checking. Although, if I'm not mistaken, the usage of allOf should ensure that a subtype is a superset of the dimensions of its supertype.

[whitlockjc]

While the Swagger Specification is not very clear on things, there are a lot of details in apigee-127/swagger-tools#241 that help explain how this should work. I think the reason this hasn't been done yet is really about the scope of the work. Right now, we can rely on a JSON Schema validator to do the work. Since discriminator support is not something a JSON Schema validator can use, we have to come up with a way to basically write our own validator. If discriminators were only at one level, this would be really simple but since any schema object could have a discriminator, it very quickly becomes quite painful to support this.

I'm not saying it can't be done, I'm just saying this quickly grows in terms of difficulty and scope. That being said, I have thought about this some. One of the options that seems to have the most potential, and the least amount of reinventing of the wheel, is attempting to use JSON Schema primitives like anyOf and/or oneOf and/or others to fill the void. We could process the document, find all places where discriminators are used, find the sub-types and use the JSON Schema primitives to fill in the gap. This might be a quick win but we could quickly cause issues because the whole idea of discriminators is to do type-specific validation and so some generic solution might not be 100% accurate.

I'd love to talk about it more.

[wbbradley]

Sounds like you are focused primarily on the validation side of the equation, which makes sense. That's probably more important than my main concern, which is document generation.

This may be better suited for the swagger-spec issues board, but just to keep the conversation alive, I looked into how RAML is specifying this specific construct. It looks like they:

1. have discriminator on the supertype to specify the expected discriminator field (like Swagger).
2. the supertype's discriminator property can be defined with an enum of the valid subtypes.
3. have a discriminatorValue field on the subtype which specifies the subtype's type name.
4. also, the subtype actually declares itself as being of the supertype's type. Like follows:

types:
  Animal:
    type: object
    discriminator: kind
    properties:
      kind:
        description: The type of animal this is.
        type: string
        enum:
          - cat
          - dog
  Cat:
    type: Animal
    discriminatorValue: cat
    properties:
      # naturally inherits Animal's properties, but can extend them here...
  Dog:
    type: Animal
    discriminatorValue: dog
    properties:
      # naturally inherits Animal's properties, but can extend them here...

[whitlockjc]

Yeah, we're into the realm of the OpenAPI Specification now. As for the context that matters to swagger-tools/sway, it's purely from a validation perspective.

[dan-kez]

Any update on this?

[amarzavery]

Although we have implemented polymorphic validation in our tool oav using the oneOf approach and we use this for validating request/response, the error messages are nested as pointed earlier in one of the comments. It takes some time to understand what is incorrect.

If we do not want to use oneOf then there is another approach that can be used, a little cumbersome but will give better error messages.

1. Maintain a tree of the polymorphic types.
2. At runtime, walk through (dfs/bfs) the model schema that needs to be validated and
   1. if there is a reference to any polymorphic type then
      1. find the value of that property that is present in the example/request/response.
      2. Update the reference of that property with the reference to the child model (from the example).
      3. If the value in the example is not present in the tree maintained for polymorphic types then you can throw an error "Unknown polymorphic type".

After making the modification at runtime, give the updated schema to z-schema for validation. This should then do the validation correctly.

The difficult thing to validate in this approach is collection of polymorphic types. If you have an array/dictionary of polymorphic types then the first item could be a Cat and the second item could be a Dog. So one has to modify the schema at runtime for every item in the collection and then validate only that sub schema using the z-schema validator.

This becomes way more complex. Hence validating using oneOf is a much better option.

Another important point to model validation is setting "additionalProperties": false for every model definition unless explicitly specified to not set it. If this is not done then z-schema would not throw an error for extra or missing properties from the parent or the child schema.

[amarzavery]

Suggestion: Sway could maintain different copies of the spec internally for semantic and model validation, thereby avoiding the problem of using non 2.0 open api specification keywords like oneOf.