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

Incorrect BodyProcessorException: [Bad Request] Validation error for body application/json: More than one schema valid #1820

Closed
ylemoigne opened this issue Dec 14, 2020 · 6 comments

Comments

@ylemoigne
Copy link

Version

4.0.0

Context

Given an openapi schema like :

      "Version": {
        "title": "[simple type, class  Version]",
        "required": [ "@type"],
        "properties": {
          "@type": {
            "type": "string",
            "enum": [ "Version$Branch", "Version$Tag"]
          }
        },
        "discriminator": {
          "propertyName": "@type",
          "mapping": {
            "Version$Branch": "#/components/schemas/Version_Branch",
            "Version$Tag": "#/components/schemas/Version_Tag"
          }
        },
        "oneOf": [ {
          "$ref": "#/components/schemas/Version_Branch"
        }, {
          "$ref": "#/components/schemas/Version_Tag"
        } ]
      },
      "Version_Branch": {
        "title": "[simple type, class  Version$Branch]",
        "required": [ "name"],
        "type": "object",
        "properties": {
          "name": {
            "type": "string",
            "nullable": false
          }
        }
      },
      "Version_Tag": {
        "title": "[simple type, class  Version$Tag]",
        "required": [ "name"],
        "type": "object",
        "properties": {
          "name": {
            "type": "string",
            "nullable": false
          }
        }
      }

And a body :

{...,
"version":{"name":"release_8.9","@type":"Version$Tag"},
...}

I expect vertx to be able to match the correct type but I got :

io.vertx.ext.web.validation.BodyProcessorException: [Bad Request] Validation error for body application/json: More than one schema valid
	at io.vertx.ext.web.validation.BodyProcessorException.createValidationError(BodyProcessorException.java:64) ~[vertx-web-validation-4.0.0.jar:4.0.0]
	at io.vertx.ext.web.validation.impl.body.JsonBodyProcessorImpl.lambda$process$0(JsonBodyProcessorImpl.java:40) ~[vertx-web-validation-4.0.0.jar:4.0.0]
	at io.vertx.core.impl.future.ComposeTransformation.onFailure(ComposeTransformation.java:50) ~[vertx-core-4.0.0.jar:4.0.0]
	at io.vertx.core.impl.future.FutureBase.emitFailure(FutureBase.java:78) ~[vertx-core-4.0.0.jar:4.0.0]
	at io.vertx.core.impl.future.FailedFuture.addListener(FailedFuture.java:92) ~[vertx-core-4.0.0.jar:4.0.0]
	at io.vertx.core.impl.future.FutureBase.compose(FutureBase.java:104) ~[vertx-core-4.0.0.jar:4.0.0]
	at io.vertx.core.impl.future.FailedFuture.compose(FailedFuture.java:27) ~[vertx-core-4.0.0.jar:4.0.0]
	at io.vertx.core.Future.recover(Future.java:210) ~[vertx-core-4.0.0.jar:4.0.0]
	at io.vertx.ext.web.validation.impl.body.JsonBodyProcessorImpl.process(JsonBodyProcessorImpl.java:39) ~[vertx-web-validation-4.0.0.jar:4.0.0]
	at io.vertx.ext.web.validation.impl.ValidationHandlerImpl.validateBody(ValidationHandlerImpl.java:247) ~[vertx-web-validation-4.0.0.jar:4.0.0]
	at io.vertx.ext.web.validation.impl.ValidationHandlerImpl.handle(ValidationHandlerImpl.java:143) ~[vertx-web-validation-4.0.0.jar:4.0.0]
	at io.vertx.ext.web.validation.impl.ValidationHandlerImpl.handle(ValidationHandlerImpl.java:18) ~[vertx-web-validation-4.0.0.jar:4.0.0]
	at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1104) ~[vertx-web-4.0.0.jar:4.0.0]

Do you have a reproducer?

No

Others

The specification (https://swagger.io/specification/#discriminator-object) say that the implementor MAY use discriminator, so it's questionnable, but I think that when the discriminator is here, vertx should use it to see that only one type match (and to speedup matching)

@ylemoigne ylemoigne added the bug Something isn't working label Dec 14, 2020
@vietj vietj added this to the 4.0.1 milestone Dec 14, 2020
@slinkydeveloper
Copy link
Member

slinkydeveloper commented Dec 16, 2020

tl;dr discriminator is ignored from the vertx-json-schema validator, on each field type definition use enum to match the type name:

      "Version_Branch": {
        "title": "[simple type, class  Version$Branch]",
        "required": [ "name"],
        "type": "object",
        "properties": {
          "name": {
            "type": "string",
            "enum": ["Version$Branch"]
          }
        }
      },
      "Version_Tag": {
        "title": "[simple type, class  Version$Tag]",
        "required": [ "name"],
        "type": "object",
        "properties": {
          "name": {
            "type": "string",
            "enum": ["Version$Tag"]
          }
        }
      }

The long story here is that the discriminator is still not supported, there is a WIP PR here, where I still have some concerns about it, but in general the discriminator is a weird feature of openapi 3, because it goes a bit against the design philosophy of json schema (and making it weird to implement). If that's something people is interested in, I'll try to get back to that PR and fix it, in order to support discriminator properly.

In any case, I think your schemes miss the enum (or, in OpenAPI 3.1, you'll be able to use directly const) anyway, because type is a constant value for your 2 schemes.

@slinkydeveloper slinkydeveloper added enhancement and removed bug Something isn't working labels Dec 16, 2020
@ylemoigne
Copy link
Author

I use the workaround you provided (more exactly, I have added the property @type to each subtype, with the enum and only value possible for this type).

I think that discriminator is very usefull, there is this use-case of course (type discrimination), but also, in another project, the business model was event sourced with almost 200 type of event and in this case discrimator is a shortcut to determine quickly which of oneOf schema has to be used.

Not supporting it and relying on property declaration can lead to weird issue (on weird serialization case, I admit). In this sample a collection of Version_Tag doesn't include @type, and a collection of Version include @type. It happen on some configuration/usage of Jackson with polymorphism.

Also use the add-property-with-enum way make harder to use some code generator based on openapi schema which treat this property as 'real' property rather 'for-serialization-only` property.

But as I said in initial post, the OpenAPI spec say MAY, so I understand if you don't want to add the burden of supporting it.

@slinkydeveloper
Copy link
Member

Also use the add-property-with-enum way make harder to use some code generator based on openapi schema which treat this property as 'real' property rather 'for-serialization-only` property.

Well you can have both discriminator and the add-property-with-enum way, they certainly don't exclude each other. I see how the discriminator works very well for serde tools, but they make more complex the validation, because in json schema validation keywords are meant to be independent but discriminator creates a "bond" between different keywords, hence my issue with supporting it properly.

But as I said in initial post, the OpenAPI spec say MAY, so I understand if you don't want to add the burden of supporting it.

I see that this is in OpenAPI 3.1 too, so I eventually won't have any choice than supporting it 😄. I'll see what I can do, starting from eclipse-vertx/vertx-json-schema#18

@ylemoigne
Copy link
Author

I see that this is in OpenAPI 3.1 too, so I eventually won't have any choice than supporting it 😄. I'll see what I can do, starting from eclipse-vertx/vertx-json-schema#18

Great News ! (easy to say when you're not the maintainer).
Thanks again for all vertx's team great work.

@slinkydeveloper slinkydeveloper modified the milestones: 4.0.1, 4.1.0 Dec 16, 2020
@slinkydeveloper
Copy link
Member

I've opened an issue in json schema to discuss about that: eclipse-vertx/vertx-json-schema#31

@slinkydeveloper
Copy link
Member

I close this and mark it as wontfix, as discussed in eclipse-vertx/vertx-json-schema#31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants