Default value ambiguity in oneOf / Support for discriminators?

[catadch]

I can't work out if discriminators are supposed to be supported, or not. I certainly can't get them to be honored. If I use express-ajv-swagger-validation then they are honored, but default values are not. Kind of hoping express-openapi-validator might be a one-stop-shop as everything else works a treat.

[catadch]

Actually, it could be that oneOf isn't working. I have schema types with different properties and they're not being distinguished from one another.

[gonenduk]

Can you give an example? allOf is working well for me.

[catadch]

So my schema is pretty big, but way down in it I have something like this:

When I POST an instance of SimilarTypeOne, we're all set. When I POST an instance of SimilarTypeTwo the validator complains uniqueOne is missing.

If I switch the order in the oneOf, instances of SimilarTypeOne now get rejected for not having uniqueTwo.

  SimilarTypeOne:
     allOf:   
       - $ref: '#/components/schemas/SimilarSuperType'
     type: object
     properties:
       type:
         type: string
         enum: [Some Value One]
       uniqueOne:
         type: string
     required:
       - type
      - uniqueOne

   SimilarTypeTwo:
     allOf:   
       - $ref: '#/components/schemas/SimilarSuperType'
     type: object
     properties:
       type:
         type: string
         enum: [Some Value Tow]
       uniqueTwo:
         type: string
     required:
       - type
      - uniqueTwo

   SomeType:
     allOf:   
       - $ref: '#/components/schemas/SomeSuperTYpe'
     type: object
     properties:
       something:
         type: array
         minItems: 1
         maxItems: 2
         items:
           oneOf:
             - $ref: "#/components/schemas/SimilarTypeOne"
             - $ref: "#/components/schemas/SimilarTypeTwo"
           discriminator:
             propertyName: type

[cdimascio]

@catadch would you mind posting your full spec as well as the request body you are POSTing. i'd like to repro this to investigate.

if you do not wish to post the entire spec here publicly, you can direct message it to me on gitter - just hover over my avatar and select "Chat Privately"

[catadch]

@cdimascio can't send you the whole schema for copyright / ip reasons and in simplifying / anonymizing for a reproducible test case, the problem goes away : (

If I figure out what the problem is, I'll let you know.

[cdimascio]

@catadch i created a few tests to exercise the schema you sent. it appears to work in that i'm able to use the two oneOf instances
see here -> https://github.com/cdimascio/express-openapi-validator/pull/105/files

there may be some other element that is contributing to the issue.
it would be great if you can provide a snippet that can repro the issue.

feel free to modify my tests as well

[catadch]

@cdimascio - I found the problem.

In the schema below, it's the enums / defaults for the two "value" number type properties that causes the problem.

Comment either one out and the problem goes away.

I have a stand-alone project to demonstrate this is you want it.

openapi: '3.0.0'

info:
  description: express-openapi-validator-test
  version: 1.0.0
  title: express-openapi-validator-test
  license:
    name: MIT License
    url: https://opensource.org/licenses/MIT

servers:
  - url: http://localhost:3010/v0

paths:
  /typethrees:
    post:
      requestBody:
        required: true
        content:
          application/json:
            schema:
              oneOf:
                - $ref: '#/components/schemas/TypeThree'
      responses:
        '201':
          description: Application response
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/TypeThree'
        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Error'

components:
  schemas:
    Error:
      required:
        - code
        - message
      properties:
        code:
          type: integer
          format: int32
        message:
          type: string

    SuperTypeOne:
      type: object
      properties:
        type:
          type: string
        value:
          type: number
          format: int32
      discriminator:
        propertyName: type
      required:
        - value
        - type

    TypeOne:
      allOf:   
        - $ref: '#/components/schemas/SuperTypeOne'
      type: object
      properties:
        type:
          type: string
          enum: [TypeOne]
        value:
          type: number
          format: int32
          enum: [ 1 ]
          default: 1
        uniqueOne:
          type: string
          default: Unique One
      required:
        - type
        - uniqueOne

    TypeTwo:
      allOf:   
        - $ref: '#/components/schemas/SuperTypeOne'
      type: object
      properties:
        type:
          type: string
          enum: [TypeTwo]
        value:
          type: number
          format: int32
          enum: [ 2 ]
          default: 2
        uniqueTwo:
          type: string
          default: Unique Two
      required:
        - uniqueTwo

    SuperTypeTwo:
      type: object
      properties:
        id:
          type: string
          format: uuid
        date:
          type: string
          format: date-time
        someStrings:
          type: array
          items:
            type: string

    TypeThree:
      allOf:   
        - $ref: '#/components/schemas/SuperTypeTwo'
      type: object
      properties:
        type:
          type: string
          enum: [TypeThree]
          default: TypeThree
        somethings:
          type: array
          minItems: 1
          maxItems: 2
          items:
            oneOf:
              - $ref: "#/components/schemas/TypeOne"
              - $ref: "#/components/schemas/TypeTwo"
            discriminator:
              propertyName: type
      required:
        - type
        - somethings

[cdimascio]

hmm. i added a new test that adds the value properties in a manner similar to what you show above
same PR here -> https://github.com/cdimascio/express-openapi-validator/pull/105/files

I'm still not able to reproduce

please send along your sample project that is able to trigger the issue.

also, feel free to review my test to make sure i'm exercising your case properly

[catadch]

@cdimascio try this:

https://github.com/catadch/express-openapi-validator-test

Let me know when you'e cloned it and I'll make it private again.

[cdimascio]

i cloned it, ran npm start, but appears index.js is missing

[catadch]

$ npm test

As in the README : )

[cdimascio]

gotcha. just picked up the readme change. i can run the test. thanks!
(fyi, i'm heading out shortly, but will definitely have a look tomorrow)
thanks for your help on this

[catadch]

No worries.

It's 01:00 where I am and I've been working on this for 48 hours straight, so i'm out of here too.

[cdimascio]

get some sleep :) hopefully we'll have some answers soon.

[cdimascio]

This is odd. I added your spec and tests into the test suite and they pass O.o
See this PR: #107

the api spec: https://github.com/cdimascio/express-openapi-validator/pull/107/files#diff-ed3d36f532fcf4bdd12dfce177b18308R1

the tests: https://github.com/cdimascio/express-openapi-validator/pull/107/files#diff-b66e599ff6cac0a23a25a49f6f19e2cbR1

There must be something else in the project that's playing a role. will poke around further in your example.

[cdimascio]

nevermind. it's now reproducible.

[cdimascio]

This looks like it may be an issue with Ajv and how defaults are handled. When in the context of oneOf, allOf, there is some ambiguity. This ambiguity may explain why we see a proliferation of errors in this particular case, that is, we see mix of errors, some related to each oneOf type.

The details of the Ajv feature and ambiguity is described here https://github.com/epoberezkin/ajv/issues/42

Getting back to your example, I observed a couple of behaviors:

1. By removing the default keyword on the enum valued property, value, the validation errors no longer occur. Of course, you clearly want to provide a default. But seems to indicate the problem is with the default that is used.

2. Note that in your example, you have two fields where you provide a default:

• uniqueOne and uniqueTwo. These defaults are always properly applied. I believe that this is because the property is not used in both oneOf types, thus there is no confusion surrounding the default value. In the case of value, the property exists in both types, but has a different default value. I believe that this is a cause of ambiguity and that Ajv is unsure which default to use.

Ultimately, the best course of action may be to try to reproduce this using Ajv only.
If we can show Ajv is choosing the incorrect default it may be appropriate to file an issue there.

Other options might be to:

• modify the api to ensure the default values are the same for overlapping properties of oneOf types
• avoid oneOf in cases where the default values are different for shared oneOf properties (this will cause some duplication across models)
• keep the behavior you want, but provide a hacky workaround using middleware to remove the ambiguity. (Note that adding this middleware func to your test project will fix cause your tests to pass)

  // Install middleware prior to the validator to handle these ambiguous defaults and inject their values prior to validation
  app.use((req, res, next) => {
    if (Array.isArray(req.body.somethings)) {
      req.body.somethings.forEach(s => {
        if (s.type === 'TypeOne' && !s.value) {
          s.value = 1;
        }
        if (s.type === 'TypeTwo' && !s.value) {
          s.value = 2;
        }
      })
    }
    next();
  });
  // install validator
  new OpenApiValidator(opts).install(app);

  // actual routes below here
  // ....

[cdimascio]

All in all, discriminators should work for most cases, however, as we see here, when ambiguity is present in the schema and the discriminator is necessary to inform a choice, the validator may fail.

Ideally, Ajv would support this. However, in the case that it will not, a performant solution is non trivial. I'm open to ideas and suggestions.

On the bright side, this issue is limited to cases where oneOf or anyOf are used AND conflicting defaults exist in that oneOf or anyOf branch.

[cdimascio]

Keeping this open. Will investigate possible solutions for this case

[cdimascio]

@catadch i've added some notes to the Ajv discussion here. Feel free to add to the discussion. Perhaps, we'll get some traction or, at least, better understand how we might be able to solve this case

[cdimascio]

@catadch curious to know your thoughts on my comments above and how you are currently planning to proceed

[cdimascio]

@catadch initial support for discriminators is available in v4.7.0 via PR #461.
it doesn't satisfy all use cases, but it's a start.
please give it a try, i'm glad expand support to more use cases. please post them here and i'll prioritize

examples:

• tests: https://github.com/cdimascio/express-openapi-validator/blob/master/test/one.of.2.spec.ts#L41
• specs: https://github.com/cdimascio/express-openapi-validator/blob/master/test/resources/one.of.2.yaml

[cdimascio]

@catadch we now have support for top level discriminators. See the discriminator example here

[michaelmelody91]

Hey @cdimascio, did you have any thoughts on the direction you'd see being taken to support discriminators within the request body?

We're currently using the validator (and loving it so far) where we're supporting a request body which contains a top level attribute where the value is a map of elements where each could inherit from a base schema. Each one of those elements has their own set of required fields (though they share common fields which are required across each). To use the validation provided by this middleware, we're using anyOf to match against any one of those elements. Something like below:

anyOf:
  - $ref: '#/components/schemas/Question'
  - $ref: '#/components/schemas/SubclassQuestionA'
  - $ref: '#/components/schemas/SubclassQuestionB'

While the validation works as we'd expect, we do have an amount of duplication as we're not using polymorphism - not a biggie. As we're using the anyOf approach and not using discriminators, we do receive a verbose error output which we'd like to reduce, when sending an invalid request body as the validator is validating the invalid question against each schema. Each schema has a type attribute which we could use if it were supported.

I'm happy to take a stab at this, just wondering had you any thoughts on how to go about this? Is there any gotchas to be aware of?