quarkiverse / quarkus-renarde

Server-side Web Framework with Qute templating, magic/easier controllers, auth, reverse-routing
Apache License 2.0
78 stars 19 forks source link

@Valid not being called in REST endpoints #175

Open tmulle opened 1 year ago

tmulle commented 1 year ago

I originally posted this issue in the first comments of https://github.com/quarkusio/quarkus/issues/36276

Since then, I noticed there are two issues related to ConstraintViolationException handling, one in Quarkus core, and another in Renarde.

  1. In Renarde it looks like the @Valid param on a attribute is not being called automatically, I have to manually perform the validation.

  2. In Quarkus core, it looks like the @Valid IS being properly called automatically, but then it never triggers my custom exception mapper.

So, this ticket is about the valid annotation being called automatically like it is in quarkus.

Hope this makes sense and sorry for all the changes in tickets.

FroMage commented 1 year ago

In Renarde, validation is called, but you still get in the endpoint, so you have to check for valition with validation.hasErrors(). This allows you to add more custom validation than you can express in the annotations, and to define the outcome of the validation failure on a per-endpoint basis.

See https://docs.quarkiverse.io/quarkus-renarde/dev/advanced.html#_validation

Is this what you're referring to?

tmulle commented 1 year ago

Ah ok.. yes I remember seeing that now. Sorry.

So I am finding something odd though, and maybe it's the way our site is working (I didn't write the orignal, just converting it from Play/Scala to Quarkus/Renarde.

In our upload form, if I post a title value of longer than 10 characters the validation on the server catches it like you said and I can check validation.validationFailed() and handle it.

This form was POSTED via AJAX to the backend from the UI. I return a BAD_REQUEST exception to the front end to be displayed.

I see the error on the form like I should.

Now, if I correct the error on the form (Without closing it) and resend the new values, I see the new values come into the server BUT the validation still thinks it is failed.

The only way for me to clear the validation is to physically refresh the page in the browser and open up the upload form again and send the new values.

Since I'm using AJAX and not HTML redirects, is there a way I can clear the validation state after I send the JSON response back?

Hope this makes sense.. the website I'm converting wasn't written by a web developer :(

FroMage commented 12 months ago

Are you using boolean Controller.validationFailed()? If yes, then it will call prepareForErrorRedirect(), which will only work if you plan to redirect. For AJAX POST calls, you should call validation.hasErrors() which will not flash the errors and form values.

tmulle commented 11 months ago

Yes, I guess I was wondering if there was a way to handle the AJAX case also.

I guess the only way to handle the AJAX case is to return a JSON structure that has an error section and I then check in the JS code on the browser, and if found, then set the error classes to highlight the errors.

Or maybe I just reload the current dialog when I get the AJAX response back and it contains errors.

I want to be able to automatically display the errors like in the redirect case.

FroMage commented 11 months ago

It cannot be automatic, that depends too much on what client framework you use, when using AJAX. I could definitely add something to generate a JSON error structure from the current validation errors, but you'll have to either call it explicitely, if you know you're calling from AJAX, or you have to set a custom header to let Renarde know you're doing AJAX. We can't guess this from the request, that's the problem.

Then on the client side you'll have to handle the error structure yourself.

tmulle commented 11 months ago

Yeah, I understand.

I can handle it on my side with a custom JSON response containing the errors.

I am using Renarde on the front end and like how the automatic error field processing works with the flash context with redirect, but since there is no flash context in a AJAX response I guess I'll have to manually check each field and apply the styling.

FroMage commented 11 months ago

Well, yes, but again, we could make it easier and define a header you can use to let Renarde know and a method to return a JSON error. Would you like that?

preslavrachev commented 7 months ago

This is something I am interested in as well. As mentioned in my previous issue, our app is a mix of public endpoints showing HTML to users, and a set of internal API endpoints which change all the time. While I agree that granular validation is better for the front-facing part, our internal API endpoints can fail right away when the validations aren't met.

Is aynthing in the plans for allowing when to use automatic vs explicit validation checks? I'd rather see this on the controller level - like an annotation that you apply to a controller, which says, "This is a plan old Quarkus resource, don't apply any changes to it, whatsoever." Actually, this is what I expected it to be - anything that extends Renarde's Controller class gets the Renarde overrides, any other resources remains bound to the rules set by Quarkus.

FroMage commented 6 months ago

anything that extends Renarde's Controller class gets the Renarde overrides, any other resources remains bound to the rules set by Quarkus

That might be a strategy indeed. I have to check if I can detect that, though.