swagger-tools
swagger-tools copied to clipboard
apigee-127
Support for polymorphic validation
I've been trying to use swagger-tools to validate requests to my controllers. It seems that when I perform a request with the body parameter specified as a base class, validation happens only against the specification of the base class model, but not against the model for the extended class identified by the discriminator.
Can you give a code snippet and some example? I ask because we have heavily tested validation so I'm not sure how to reproduce.
Below a simplified version of my schema.
{
"swagger": "2.0",
"info": {
"title": "My Cloud API",
"version": "1.0"
},
"consumes": ["application/json"],
"produces": ["application/json"],
"host": "localhost:3000",
"basePath": "/api",
"paths": {
"/commands": {
"post": {
"description": "Issues a command",
"operationId": "executeCommand",
"parameters": [
{
"name": "body",
"in": "body",
"description": "Command to execute",
"required": true,
"schema": {
"$ref": "#/definitions/Command"
}
}
],
"responses": {
"200": {
"description": "Successful request.",
"schema": {
"$ref": "#/definitions/CommandResult"
}
},
"default": {
"description": "Invalid request.",
"schema": {
"$ref": "#/definitions/Error"
}
}
}
}
}
},
"definitions": {
"Error": {
"properties": {
"message": {
"type": "string"
}
},
"required": ["message"]
},
"CommandResult": {
"properties": {
"message": {
"type": "string"
}
},
"required": ["message"]
},
"Command": {
"discriminator": "type",
"properties": {
"type": {
"type": "string"
},
"propBase": {
"type": "string"
}
},
"required": ["type", "propBase"]
},
"Command1": {
"description": "Command 1",
"allOf": [
{
"$ref": "#/definitions/Command"
},
{
"properties": {
"prop1": {
"type": "string"
}
},
"required": ["prop1"]
}
]
}
}
}
The issue is that commands that are sent to /api/commands are only validated against Command, regardless of the discriminator value. If I send the following it will still pass validation and call my controller function even though it's missing the required property "prop1" for the type "Command1".
{
"type": "Command1",
"propBase": "blah"
}
I cannot reproduce this. Whenever I throw your definitions into my API and have my request/response use them, if I post only what you do, I get a request validation error (Parameter (command) failed schema validation]):
{
[Error: Parameter (pet) failed schema validation]
code: 'SCHEMA_VALIDATION_FAILED',
failedValidation: true,
results: {
errors: [
{
"code": "OBJECT_MISSING_REQUIRED_PROPERTY",
"message": "Missing required property: prop1",
"path": []
}
],
warnings: []
},
path: [ 'paths', '/pets', 'post', 'parameters', '0' ],
paramName: 'pet'
}
And when I post a full example, with the prop1 provided, the request is successful. Which version of swagger-tools are you using?
My package.json has "swagger-tools": "^0.8.7". I am using it with MEAN.js, and have the following in my express.js configuration file. I'm using the express router vs the swagger-tools one, but it since that middleware comes after the validator, and it's validating some requests I'm hope I've got it correctly configured.
var swaggerDoc = require('../app/api/swagger.json');
swaggerTools.initializeMiddleware(swaggerDoc, function (middleware) {
// Interpret Swagger resources and attach metadata to request - must be first in swagger-tools middleware chain
app.use(middleware.swaggerMetadata());
// Validate Swagger requests
app.use(middleware.swaggerValidator());
// Route validated requests to appropriate controller
//app.use(middleware.swaggerRouter(options));
// Serve the Swagger documents and Swagger UI
app.use(middleware.swaggerUi());
});
Can you run with DEBUG=swagger-tools* and paste the output of the failed request? swagger-validator should be working right based on my tests.
When I submit
{
"type": "Command1",
"propBase": "string"
}
the output of the request to the log is
NODE_ENV is not defined! Using default development environment
swagger-tools:middleware Initializing middleware +0ms
swagger-tools:middleware Identified Swagger version: 2.0 +2ms
swagger-tools:middleware Validation: succeeded +26ms
swagger-tools:middleware:metadata Initializing swagger-metadata middleware +162ms
swagger-tools:middleware:metadata Identified Swagger version: 2.0 +1ms
swagger-tools:middleware:metadata Found Path: /commands +1ms
swagger-tools:middleware:validator Initializing swagger-validator middleware +1ms
swagger-tools:middleware:validator Response validation: disabled +0ms
swagger-tools:middleware:ui Initializing swagger-ui middleware +35ms
swagger-tools:middleware:ui Using swagger-ui from: internal +1ms
swagger-tools:middleware:ui API Docs path: /api-docs +0ms
swagger-tools:middleware:ui swagger-ui path: /docs +0ms
MEAN.JS application started on port 3000
swagger-tools:middleware:metadata POST /api/commands +2m
swagger-tools:middleware:metadata Is a Swagger path: true +1ms
swagger-tools:middleware:metadata Is a Swagger operation: true +0ms
swagger-tools:middleware:metadata Processing Parameters +0ms
swagger-tools:middleware:metadata body +1ms
swagger-tools:middleware:metadata Type: object +1ms
swagger-tools:middleware:metadata Value provided: true +0ms
swagger-tools:middleware:metadata Value: [object Object] +0ms
swagger-tools:middleware:validator POST /api/commands +0ms
swagger-tools:middleware:validator Will process: yes +0ms
swagger-tools:middleware:ui POST /api/commands +5ms
swagger-tools:middleware:ui Will process: no +0ms
POST /api/commands 200 159.372 ms - 4
Can you show me the rest of your server file, replacing sensitive data with reasonable mocks so as not to change the structure of the file?
I hate to keep bugging you but I cannot reproduce it. If you'd rather not give me your server file, could you give me a recipe via a patch/PR that will showcase this issue in test/2.0/test-middleware-swagger-validator.js. Without a way to reproduce this, I cannot fix it.
I was able to reproduce it. It required me to get outside of the test suite. Let me take a look.
Awesome. I was just about to start putting together a skeleton app for you. Sorry for the delay. I was out grabbing some lunch. Let me know if you need the skeleton app anymore.
I've figured it out, swagger-tools is working right which is why my test worked but the server didn't. Your parameter references #/definitions/Command but you want it to be #/definitions/Command1. If I change the reference, it works as expected.
Right, but the goal in reality is that I want to have Command1, Command2, Command3, etc. Is that not possible?
Let me get @webron involved as I don't understand the discriminator field honestly, I thought it was only useful for swagger-codegen. I would expect that allowing a parameter, or a response, to be N different types would violate Swagger's design goals but let's see what Ron has to say.
Cool. I'm pretty new to swagger, so I may be understanding it incorrectly. I'll wait to hear what the verdict is. Thanks for being so responsive.
Okay, I hope I got everything needed from the thread. If the intention is to pass Command as a (body) parameter, and Command has a discriminator, then indeed anything that adds Command to it (via allOf) becomes a valid input (and this is recursive). So in this case, both Command and Command1 would be valid as input for the operation.
Two side notes - I'm happy to see that you've used the discriminator properly. Describing it is a bit complicated, and you've hit both spots - the discriminator is required and the sub-Command overrides the discriminator with a specific value (seems natural, but it's very easy to miss).
The other, which is slightly unrelated, for every model you should include "type": "object". While missing it will still work, technically, it allows things you don't want to allow.
Let me get this straight: If your model has an ancestor that uses the discriminator, that model is a valid value anywhere the ancestor is used as a type?
That was my understanding. Think of it like a function in a language with strong typing like Java or C++. I can declare a function that receives a base class or interface, and any function that uses that base class can also accept any subclass too.
@whitlockjc - that is correct and the explanation @daveismith provided is fairly accurate. The discriminator is used to declare inheritance between models (unlike the simple usage of allOf that's basically composition but offers not hierarchical relationship between the models). So whenever you use the parent definition, its descendants could be used as well. Hope that's clear.
@webron - So to support this, I would need to see if a schema had a discriminator and if so, find all of the ancestors of the model. For each of these ancestors, validate the provided value against its schema and if any of those ancestor schemas validate successfully, validation succeeds. If all of them fail, validation fails. Is that correct?
Wouldn't it be the reverse? If the specified schema had a discriminator you would need to look for any schemas that have an allOf that specifies a reference to the schema in question.
Actually, it's not even that complex. When you receive an object that is specified by a schema with a discriminator you would just look at the value of the discriminator field to determine the name of the schema that should be used (hopefully not too much more complex that what's already happening).
It's not a complexity thing to understand, just a potentially complex thing to implement due to how we do validation right now. I could be wrong, it might be really simple.
@webron mentioned recursion. I'm not 100% on that, I can think of two cases.
- subclass specifies another, different discriminator?
- a class specifies the subclass in an allOf, then the discriminator would inform you of the final class directly
Can you join #swagger on irc.freenode.net? It might be easier to chat in real time there.
Please do. I think I understand it but my understanding doesn't sound simple so I could be misunderstanding it. I'm jeremyw so highlight me and I'll respond.
When I was talking about recursion, there are two cases:
- There's a model (ModelX) with
allOfparent that doesn't specify thediscriminatorvalue (composition), and then there are models thatallOfModelX, and do specify thediscriminatorvalue. That's still inheritance. - (and this one is less certain) - There's a model (ModelY) with
allOfparent that does specify thediscriminatorvalue (inheritance), and then are models thatallOfModelY. That's a composition of an inherited model. I'm not sure you want to deal with this use case for now and the meaning of it would be uncertain.