quarkus-resteasy-problem
quarkus-resteasy-problem copied to clipboard
Issue regarding propertynames in constraintviolation.
When I throw a plain constraing violation, violating as an example: email address. I recieve the message in the issue, but I do not get the fieldname or what the actual value is.
I reviewed the code and found the below implementation quite weird.
Why do we visit the iterator twice to get the propertyname?
https://github.com/TietoEVRY/quarkus-resteasy-problem/blob/da642a0aad2d5892fc1ecc466edf4005c468e8db/runtime/src/main/java/com/tietoevry/quarkus/resteasy/problem/validation/ConstraintViolationExceptionMapper.java#L64-L78
Hi @bdunni, it may seem weird, but hibernate validator puts controller's class name and method name as first 2 elements of the property path - we decided we don't want to leak this implementation detail outside of REST api - api clients don't care if it's java or php, if it's handled by ResourceA, or ResourceB.
This may interfer with your use case - would you mind posting reproducer/example here?
Hey @lwitkowski thanks for the quick reply.
I will see if I can make a reproducer example.
It also has me questioning line 75. We compare the parameter name in the resource to the parameter name in violation. Am I wrong to assume that this is also interfering with my use case?
In my case I have validation on "Email" it is part of the body of my request, but all my resourceInfo method parameters delivers is the name of the DTO which email is a part of. Hence this will never be a valid?
as an example:
public class PersonDTO { @NotNull String email }
Then the parameters returned by line 75 will return PersonDTO, but never email as it is inside the object?
@lwitkowski I have to post from another account.
I reproduced the problem here: https://github.com/NissenDev/Http-problem-reproduce
Simply make a post with the following Json: { "email": 222222, "title": "45654565" }
To http://localhost:8080/hello
I used POSTMAN
So it seems to be that we do not support the @RequestBody as a param of the ResourceInfo methods. Furthermore the 2 first elements of the controller and name and class are not present when doing the post.
@NissenDev @bdunni thanks for reproducer, now I see that you use Validator programmatically, instead of simply using @Valid annotation on Book
request param.
@Valid
usage is covered with tests, and works as designed. Have a look at this test: https://github.com/TietoEVRY/quarkus-resteasy-problem/blob/master/integration-test/src/main/java/com/tietoevry/quarkus/resteasy/problem/ValidationExceptionsResource.java#L34
We would need to find a way to detect if this exception comes from @Valid
annotation or from programmatic usage of Validator
, as they produce different ConstraintViolationException
. I strongly suggest using @Valid
when possible though.
@lwitkowski It works when I use @Valid directly on the requestParam.
We do run into a problem because we're using validation groups. Which are added programatically later on. So in this case we have to use it as is. It produces the same empty fields and unknown variables as the example above.
In our case it has something to do with external and internal calls or Business or private users. They are validated differently.
I am thinking we should support: https://quarkus.io/guides/validation#validation-groups-for-rest-endpoint-or-service-method-validation.
And throwing constraintValidationException manually.
@bdunni support for validation groups is a nice feature request, I've never used them. I'll try have a closer look.
Regarding original issue, after merging #313 we could add a config flag, something like:
quarkus.resteasy.problem.constraint-violation.sanitize-parameters=true|false
This way one could disable removing first two segments of the param path, and that may potentially solve your problem.
I wouldn't be against that what so ever.
Sounds like a decent solution.
However what if they were to use the constraints programatically and from the annotation? Wouldn't this be an issue?
However what if they were to use the constraints programatically and from the annotation? Wouldn't this be an issue?
When you opt-in for not sanitizing parameters, you'll get everything that is in ConstraintViolationException.getConstraintViolations(). getPropertyPath()
, unchanged in any way. So it will work as good as the code that creats these exception :)
Would it also know when it should ignore the 2 first propertynames you mention here:
hibernate validator puts controller's class name and method name as first 2 elements of the property path
And when not to?
If so, lets go!
No, it would never ignore anything if this flag is active - it would work similar to default Quarkus implementation. The consequence would be that Class and method name will leak outside rest api, which we tried to avoid with this 'remove 2 first segments from path' logic - but I don't see other option.
I have no idea how to reliably detect different usecases without increasing complexity too much.
If I correctly recall at the moment we wrote this implementaiton there were not means to determine how exception has been triggered. We had to make "one-way" decission based on "most common use-case" for scenarios we recognized at that moment of time.
I'm not sure if API changed since then so there might not be an option with "increased complexity".