-
Notifications
You must be signed in to change notification settings - Fork 43
Operation validator: add request validation from URL only #59
Comments
Hi and welcome, You're right, this feature was on my roadmap before 1.0. I thought about making this lookup in This method would look at the URL of the request (to add) to make the successive matches:
In this way, I expect to keep only one Also, we must honor server variable parameter which can be open or close with By the way, is the sample code written for illustration or are you really using directly |
Hi, and thanks for your project. I am using org.restlet and have an object per route. This object knows the templated path and method, thus I create and store an OperationValidator per end-point. I am yet to deal with validating responses from the requests. I am using OperationValidator because; a. I know the path up front, so don't need to create a new one per request. I need the validation results for 'info' messages. I need this because I am using a custom 'format' validator to collect all the crumbs to the nodes with "format: datetime". I then use this collection to perform string to date conversion on the data. This allows generic date conversion of Json payloads by simply having an openapi spec defined. Along these lines I do have a couple other feature requests;
I have workarounds for the above, but it might be nice to see it in-built. |
Hi, I merged all this in master branch, now we can lookup to operation from request URL only. Now to talk about your needs, the naming could be misleading but you should use a shared Before reading further, it would be nice if you can validate the changes with latest 0.8-SNAPSHOT. That said, if I understand well what you're trying to achieve, you should use directly This would be like the following: // Setup this block once
////////////////////////////////
// coming from parser or not, you're choice.
JsonNode schemaNode = operation.getRequestBody().getContentMediaType("application/json").getSchema();
// you can also setup directly the schema from URL
JsonNode schemaNode = TreeUtil.load(getResource("my-schema.yaml"));
OAI3Context apiContext = new OAI3Context(URI.create("/"), schemaNode);
ValidationContext<OAI3> validationContext = new ValidationContext<>(apiContext);
SchemaValidator schemaValidator = new SchemaValidator(validationContext, "schema", schemaNode);
////////////////////////////////
// On each request do the following
////////////////////////////////
ValidationResults results = new ValidationResults();
schemaValidator.validate(YOUR_BODY_JsonNode, results);
//////////////////////////////// About registering a custom validator, which will be available for I hope these lines are helpful to you. |
I tried out the changes and it seemed to work sometimes, but was matching paths inconsistently. I had paths of:
And a GET request of "/v1/abc/123/123" would occasionally match the path of "/v1/abc" instead, throwing an error regarding no "get" operation. Perhaps you don't wish to support it, but it may also be useful to match templated strings to templated strings where the named params differ. e.g "/v1/abc/{p1}/{p2}" would match "/v1/abc/{Id}/{Age}" This is useful for routers that are trying to match end points to the spec up front prior to receiving a real request. I'm unclear how this might work if the types of the params change, e.g. integers and string params. Thanks for your tips on using SchemaValidator. I create a OperationValidator per end point and re-use them for each request. But unfortunately I am working with a Map<String,Object> rather than JsonNode, so the conversion needs to occur anyway. If openapi4j could work with native Map<String,Object> that would be a great feature, for me at least. I am using a validator to override the "format" handling. However there doesn't appear to be anything I can do other than record a validation error. I am using this to record all the points "format" occurs then going back through the data and performing conversion. I was suggesting openapi4j could allow conversion of the data via a validator/converter. However for my case that would require supporting validating the original Map<String,Object> rather than a JsonNode copy of the data. |
Thanks for the report. I can confirm the issue you noticed. I'll make further internal changes to be fully accurate here. Matching templated strings to templated strings will never be possible here. I need named parameters fit to the path or/and operation parameters. Also, you can already wrap your body with Finally, for your feature validator/converter, are you available to contribute to it? At least as an experiment? |
PR #63 should make this lookup accurate now. Can you confirm the issue is fixed in latest 0.8-SNAPSHOT? |
I will try find time to confirm the changes soon and will get back to you.
This is fine. One could always just create a URL with dummy values to do the match if needed.
Yes this is what I am doing. However there is a performance/memory cost associated with doing this. It also makes a copy of the data, so mutation during validation is not possible (but perhaps that's not a good idea anyway). I am also unsure the conversion is very accurate. It seems to use the schema to do the conversion but has no support for discriminators/anyof etc. So will not convert correctly. And if it did read the schema correctly it would essentially be doing that schema processing twice (see Line 51 in bc7d613
Is there any reason you cannot simply use JsonGenerator/JsonFactory/Objectmapper etc? Java objects already encode the type information needed to convert to Json, there's no need to use the schema. If openapi4j were to support Map<String,Object> natively, I think it might require using generics. The conversion helper I was suggesting would probably best be left as a post validation step, to avoid mutation during validation. In hindsight the way I am doing it is probably close to optimal - I just wish I could avoid the unnecessary copies of the data. |
Half incorrect. Object reference is passed by value, there's no deep copy around there.
True :) It does not convert the content twice as you suggest, Let me know if the issue still persist on your side. |
ok thanks for the confirmation. However I can see there is significant difference between passing a Map<String,Object> vs a JsonNode. If a value does not match the schema it gets converted incorrectly, thus validated incorrectly.
However converting to JsonNode manually prior to validation works as expected: ObjectMapper OBJECT_MAPPER = new ObjectMapper();
JsonNode jsonNodeMap = OBJECT_MAPPER.convertValue(objMap, JsonNode.class);
builder.body(Body.from(jsonNodeMap)); It shouldn't be doing conversion using the schema prior to validation as it attempts to coerce the values to the target type. It also has the problem of not being able to convert correctly if it encounters a discriminator. As it stands only JsonNode is correctly supported, it's not practical to use Map<String,Object>. I will raise a separate issue for this. |
I tested the changes out and they seem to work ok. But otherwise the patch matching seems to be working ok for simple test cases. |
Thanks for the feedback. Both methods are already available from PathResolver. |
Is your feature request related to a problem? Please describe.
I would like to validate a request/response by path rather than a known operation id. To validate requests as they arrive to an end-point the only information we have is the path, headers and method. We don't know what the "operation" is. We can do this with code similar to:
However this only works for paths with no parameters. I would like to find paths such as "/pet/{petId}".
Describe the solution you'd like
I believe the "api.getPath" could take an overload which converts all paths with "{xxx}" to regexes similar to: "/pet/.*/" and find a match using this instead.
It would be nice if it also took the 'servers:url' component into account as well. Perhaps an api.findPath(url) that correctly finds the path by skipping over the server part and regexing through the parameters parts.
Describe alternatives you've considered
I can write my own iteration over the map of paths but it would be nice to have this part of the API. I suspect many people would like to validate using the path+method/response code rather than a known operation id.
The text was updated successfully, but these errors were encountered: