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

Named @RestForm and error mapping of the violations #248

Open
jmini opened this issue Sep 19, 2024 · 7 comments
Open

Named @RestForm and error mapping of the violations #248

jmini opened this issue Sep 19, 2024 · 7 comments

Comments

@jmini
Copy link
Contributor

jmini commented Sep 19, 2024

The @RestForm annotation is accepting a value which is the name of the parameter.

So it is valid to name the the parameter:

@POST
public void procced(@RestForm("u") @NotBlank String userName) {
    // do something with the form parameters
}

But in this case the the validation is executed (hasError() returns true) but the error is placed on the "userName" key. I would have expected to have them on "u".

Is that expected?

The returned violations here do not seems to contains this information:

Set<ConstraintViolation<Object>> violations = executableValidator.validateParameters(ctx.getTarget(),
ctx.getMethod(), ctx.getParameters());

So the mapping to the lastNode key seems difficult to change:

public void addErrors(Set<ConstraintViolation<Object>> violations) {
for (ConstraintViolation<Object> violation : violations) {
Iterator<Node> iterator = violation.getPropertyPath().iterator();
String lastNode = null;
while (iterator.hasNext()) {
lastNode = iterator.next().getName();
}
addError(lastNode, violation.getMessage());
}
}

@jmini jmini changed the title Validation is not working with named @RestForm Named @RestForm and error mapping of the violations Sep 19, 2024
jmini added a commit to jmini/quarkus-renarde that referenced this issue Sep 19, 2024
@FroMage
Copy link
Contributor

FroMage commented Sep 19, 2024

Ah yes, good point. We need to add logic to find the original form name. If you're up for making a PR, I can help you.

@jmini
Copy link
Contributor Author

jmini commented Sep 19, 2024

I just implemented the unit test in a10cd25, but I would need some pointer on "logic to find the original form name" because right now I have no clue how it is supposed to be done.

@FroMage
Copy link
Contributor

FroMage commented Sep 20, 2024

Unfortunately, we don't have any logic in Quarkus REST to reverse-map parameters back to their HTTP form name (or query, or header, or cookie).

It should be doable for simple cases, though, in MyValidationInterceptor, because when you get a validation error on method parameters, the constraint violation's property path is {<method name>, <parameter name>} so you can ignore the first element, only keep the last member (as we do now) and then iterate the method parameters to find the parameter with the right name using ctx.getMethod().getParameters().

Once you get the right parameter, you can load its annotations and if you see a @RestForm or a @FormParam annotation with a non-null value, you can use that for the error key.

This won't work for bean params, though, but we can add support for them later, let's not over-engineer this for now.

@jmini
Copy link
Contributor Author

jmini commented Sep 23, 2024

This won't work for bean params, though, but we can add support for them later, let's not over-engineer this for now.

This is an other potential issue, but right now if you use FormData class approach (which every field annotated with @RestForm as suggested in this section), according to my tests there is no validation.

@FroMage
Copy link
Contributor

FroMage commented Sep 23, 2024

Are you sure? Did you add @Valid to the endpoint parameter?

@jmini
Copy link
Contributor Author

jmini commented Sep 24, 2024

Thank you for the pointer, I was not aware of the @Valid annotation.

So for the second example of the page https://docs.quarkiverse.io/quarkus-renarde/dev/advanced.html#_the_endpoint (with the FormData). This applies the validations:

@Consumes(MediaType.MULTIPART_FORM_DATA)
@POST
public void complete(@RestQuery String confirmationCode,
        @Valid FormData form) {
    // do something with the form parameters
}
public static class FormData {
        @RestForm @NotBlank @Length(max = Util.VARCHAR_SIZE) String userName;
        @RestForm @NotBlank @Length(min = 8, max = Util.VARCHAR_SIZE) String password;
        @RestForm @NotBlank @Length(max = Util.VARCHAR_SIZE) String password2;
        @RestForm @NotBlank @Length(max = Util.VARCHAR_SIZE) String firstName;
        @RestForm @NotBlank @Length(max = Util.VARCHAR_SIZE) String lastName;
}

With the same limitation as the one discussed in this issue:

public static class FormData {
        @RestForm("foo") @NotBlank @Length(max = Util.VARCHAR_SIZE) String userName;
        //...
}

The validation works, but is set on errors map with the key "userName" and not "foo".

@FroMage
Copy link
Contributor

FroMage commented Sep 24, 2024

OK, so same problem, I suspected.

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

No branches or pull requests

2 participants