Operation validator: add request validation from URL only about openapi4j HOT 11 CLOSED

Hi and welcome,

You're right, this feature was on my roadmap before 1.0.

I thought about making this lookup in RequestValidator via a new validate method:
RequestParameters validate(final Request request) throws ValidationException.

This method would look at the URL of the request (to add) to make the successive matches:

• server path parameters variables if any (we can't arbitrarily skip this) to detect the "prefixing" fragments
• the path of the operation from the remaining path (after removal of server "prefixing" fragments).

In this way, I expect to keep only one OperationValidator per Operation, the laziness as behaviour and avoid performance downgrade.

Also, we must honor server variable parameter which can be open or close with enum.

By the way, is the sample code written for illustration or are you really using directly OperationValidator instead of RequestValidator?

from openapi4j.

Hi, and thanks for your project.
I noticed there is a 'PathResolver' class that may be useful for doing this matching.

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.
b. RequestValidator has no way to return the validation results if there is no error.

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;

• Register a custom 'converter' that converts the data in addition to validating it. This is particularly important for types with a 'format'. But may also be needed for converting values to 'reals' when the Json data only has an integer value for example.
• Allow processing Map<String,Object> data without first converting it to JsonNode.

I have workarounds for the above, but it might be nice to see it in-built.

from openapi4j.

Hi,

I merged all this in master branch, now we can lookup to operation from request URL only.
This is done from RequestValidator not OperationValidator. This last needs all the information to perform the subsequent validations.

Now to talk about your needs, the naming could be misleading but you should use a shared RequestValidator. It stores a OperationValidator collection and caches them for reusability.

Before reading further, it would be nice if you can validate the changes with latest 0.8-SNAPSHOT.
Let me know...

That said, if I understand well what you're trying to achieve, you should use directly openapi-schema-validator to validate the body saving you the recontruction of all this (internally or externally).

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 openapi-schema-validator resolution proposal, you can do this already with business (or override) validator: https://github.com/openapi4j/openapi4j/tree/master/openapi-schema-validator#business-validator
You can do everything you need with, I just trigger it.

I hope these lines are helpful to you.

from openapi4j.

I tried out the changes and it seemed to work sometimes, but was matching paths inconsistently. I had paths of:

path: /v1/abc
{
   "post"
}
path: /v1/abc/{Id}/{Age}
{
  "get"
}

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.

from openapi4j.

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 public static Body from(Map<String, Object> body). The conversion will occur internally.

Finally, for your feature validator/converter, are you available to contribute to it? At least as an experiment?

from openapi4j.

PR #63 should make this lookup accurate now.
To make this, the last changes handle now all servers definitions (absolute, relative and none) to match exact path template.

Can you confirm the issue is fixed in latest 0.8-SNAPSHOT?

from openapi4j.

I will try find time to confirm the changes soon and will get back to you.

Matching templated strings to templated strings will never be possible here. I need named parameters fit to the path or/and operation parameters

This is fine. One could always just create a URL with dummy values to do the match if needed.

Also, you can already wrap your body with public static Body from(Map<String, Object> body). The conversion will occur internally.

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

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.

from openapi4j.

It also makes a copy of the data so mutation during validation is not possible

Half incorrect. Object reference is passed by value, there's no deep copy around there.

(but perhaps that's not a good idea anyway).

True :)

It does not convert the content twice as you suggest, getContentAsNode will return JsonNode directly if this is what is wrapped. Schema parameter is mostly needed for un-structured content types (i.e. form, ...) to construct the common form of readable content.

Let me know if the issue still persist on your side.
Thanks.

from openapi4j.

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.
e.g

• numbers that should be strings get converted to strings by 'getContentAsNode' and it validates fine.
• passing strings that should be numbers get converted to 'null' and you get "Null value is not allowed." errors instead of "Type expected 'number', found 'string'. errors.

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.

from openapi4j.

I tested the changes out and they seem to work ok.
It might be nice to expose the "buildPathPatterns" and "findPath" methods publically as they are useful utility methods. It could perhaps live on the "OpenApi3" class instead of RequestValidator.

But otherwise the patch matching seems to be working ok for simple test cases.

from openapi4j.

Thanks for the feedback.

Both methods are already available from PathResolver.

from openapi4j.