Thanks for the ping and sorry this fell off my radar. Even more, thanks for the work!
If we threw some kind of exception, this can work. It would be a little bit expensive in practice as it would be happening every time, which means anyone who uses this feature will pay the cost of triggering a stack trace each call. This could still be someone what livable as they're theoretically also doing an RSA public key check in the same call, which I'd expect to be more expensive (untested assumption). For this to work fully, we'd need some way to tell the BValInterceptor to catch the exception and keep going. Right around here we'd need some logic that catches the "no valid constraints" exception and still keeps going: - https://github.com/apache/bval/blob/master/bval-jsr/src/main/java/org/apache/bval/cdi/BValInterceptor.java#L163 Which TCK test failed and is there a PR that has the code you two tried that caused it to fail? Thinking out loud, the most ideal outcome is that the constraints that can apply are applied. Those that cannot apply are simply ignored. Theoretically, a user could have a few constraints on the method: some aimed at the return value, some aimed at contextual data like the JWT. Let's imagine this scenario: @OrangeToken @GreenReturn public Green paint() { //... } We'll say under the covers the validator impls are: - @OrangeToken validatedBy = class OrangeTokenConstraint implements ConstraintValidator<OrangeToken, JsonWebToken> - @GreenReturn validatedBy = class GreenReturnConstraint implements ConstraintValidator<GreenReturn, Green> Here's how throwing an exception could play out: - BValInterceptor sees `OrangeTokenConstraint` cannot apply to `Green` and throws an exception. `GreenReturnConstraint` is therefore not enforced, but should. - MPJWTValidatorInterceptor sees `GreenReturnConstraint` cannot apply to `JsonWebToken` and throws an exception. `OrangeTokenConstraint` is therefore not enforced, but should. The call is allowed to complete even though both the return value and contextual @RequestScoped JsonWebToken are potentially not valid. I'd have to look around the failing TCK test to figure out what is most in spirit with the Bean Validation TCK, but it might be the case there's some future standardization work that has to be done and some meta-data we need to invent. Maybe the opportunity for us to pave some new ground. The basic concept is: - There might be CDI contextual objects a user might want to validate before a method is called (new feature) - There might be a return value a user might want to validate after a method is called (the existing feature) Essentially, some of them are before advice and some after advice. One flavor of the before advice could be to validate an object that is found in a CDI Scope. Short term, could we perhaps require someone put some kind of annotation on their Constraint annotation or ConstraintValidator that basically says "don't include me in the return value constraints", so those do not show up in `ReturnValueDescriptor`. It could be something very blunt and direct like @DescriptorType This is not really a complete idea, but want to inch the conversation forward. -- David Blevins http://twitter.com/dblevins http://www.tomitribe.com > On Jun 28, 2019, at 12:35 AM, Thomas Andraschko <[email protected]> > wrote: > > ping :D > > Am Fr., 7. Juni 2019 um 14:08 Uhr schrieb Thomas Andraschko < > [email protected]>: > >> Hi David, >> >> i worked together with Romain on the issue and already commited it. >> >> Our first idea was to just remove the constraint, if there is no matching >> validator for the validated return type (void in your case). >> This worked fine and fixed your example as the constraint was ignored. >> The problem however is, that the TCK forces us to not remove the >> constraint. >> So our "solution" is now to validate the constraint when constructing >> ReturnD and throw a exception, to indicate that there is no validator >> and the constraint on the method doesn't make any sense. >> >> WDYT? >> >> Best regards, >> Thomas >> >> >> >> Am Do., 23. Mai 2019 um 07:13 Uhr schrieb David Blevins < >> [email protected]>: >> >>>> On May 22, 2019, at 12:34 AM, Romain Manni-Bucau <[email protected]> >>> wrote: >>>> >>>> Have to admit i also see it as a good opportunity to enter the >>>> codebase to maybe a good call for contribution ;) >>> >>> Agree. Also provides some potential way to add committers and future >>> potential binding votes to help get releases out the door. >>> >>> Thomas, I gave you write permission to my bval fork so you can push >>> commits into PR #3. You up for hacking on this together? >>> >>> - https://github.com/dblevins/bval/tree/bval-174-voidreturns >>> >>> I'm not going to have time till the TomEE 8.0.0-M3 release is out, but if >>> you add Romain's test case I can rejoin the fun next week perhaps. If you >>> are up for some pair-programming fun, just push commits right into the >>> fork, no need to request a review. Yay the power of git :) >>> >>> >>> -David >>> >>>
