-
-
Notifications
You must be signed in to change notification settings - Fork 584
Explore constructor/method validation using AspectJ or ByteBuddy #937
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
Conversation
- added new module - created an aspect to validate methods and constructors - added validator factory producer to get initialized validation factory
Hey @marko-bekhta, what is the "ByteBuddy" approach? Can BB also be used to alter existing classes and weave validation code into them?
FWIW, such feature has been under discussion for a long time, it was something Hardy always wanted to implement. I have to admit I'm not a huge fan (never saw the need really, can't remember any user at all asking for it), so I closed the feature request in JIRA at some point. On AspectJ, is this still a thing? Haven't heard about it in a long time.
This all is not to say I'm strictly against it, but we should carefully decide whether we want to add this piece of code and the related maintenance efforts (e.g. my experience with AspectJ is close to nil).
Hi @gunnarmorling! From time to time there are these kind of questions https://stackoverflow.com/questions/43859895/hibernate-validator-method-or-constructor-validation when somebody tries to do constructor validation and this is not really covered by containers, right? Also I wanted to expend this approach to static methods. Something like adding a configuration option to collect static method metadata and then do the validation of those method calls with the aspects or any other interceptors...
As for the AspectJ itself - I used it a couple of times on projects, but usually nothing big. I also asked around a couple colleges and most of the time what I heard back was something like - "Ah yes AspectJ - isn't it that thing that Spring uses under the hood ?" 😃
As for the BB approach I haven't started really working on it just checked a couple of ideas. And for now it looks like the approach would be to create a java agent that would use instrumentation to add validation logic. This actually would also work with the aspects as well if used with load-time weaving.
With that said I think it would be nice if we could validate static methods and do constructor validation for simple beans, but I'd probably would delay the decision on including this stuff for later when we would have a BB approach implemented. I think then we would see if want to release such additional artifacts or if we just would convert this work to a post describing how users can do this on their own.
I've finally experimented a bit with the ByteBuddy and created a java agent that adds validation. The part of agent code looks like:
new AgentBuilder.Default() .type( ElementMatchers.any() ) .transform( (builder, typeDescription, classLoader, module) -> builder .visit( Advice .withCustomMapping().bind( Groups.ForGroups.Factory.INSTANCE ) .to( ConstructorParametersValidationAdvice.class ) .on( ElementMatchers.isConstructor() .and( ElementMatchers.hasParameters( ElementMatchers.any() ) ) .and( ElementMatchers.isAnnotatedWith( Validate.class ) ) ) ) .visit( Advice .withCustomMapping().bind( Groups.ForGroups.Factory.INSTANCE ) .to( ConstructorReturnValueValidationAdvice.class ) .on( ElementMatchers.isConstructor() .and( ElementMatchers.isAnnotatedWith( Validate.class ) ) ) ) .visit( Advice .withCustomMapping().bind( Groups.ForGroups.Factory.INSTANCE ) .to( MethodParametersValidationAdvice.class ) .on( ElementMatchers.isMethod() .and( ElementMatchers.hasParameters( ElementMatchers.any() ) ) .and( ElementMatchers.isAnnotatedWith( Validate.class ) ) ) ) .visit( Advice .withCustomMapping().bind( Groups.ForGroups.Factory.INSTANCE ) .to( MethodReturnValueValidationAdvice.class ) .on( ElementMatchers.isMethod() // visit only non void methods .and( ElementMatchers.returns( ElementMatchers.not( ElementMatchers.is( void.class ) ) ) ) .and( ElementMatchers.isAnnotatedWith( Validate.class ) ) ) ) ).installOn( instrumentation );
where the Advice classes simply call the utility class to perform validation:
public static class ConstructorParametersValidationAdvice { @Advice.OnMethodEnter private static void exit( @Advice.AllArguments Object[] parameters, @Advice.Origin Constructor<?> constructor, @Groups Class<?>[] groups) { ValidationEntryPoint.validateConstructorParameters( constructor, parameters, groups ); } }
I've later re-used this same utility ValidationEntryPoint class to do the validation in the aspect. This way we do the validation the same way for both cases. I've tested this on a very simple "client" jar that has next in it:
/** * Bean class */ public class BankAccount { private int num; @Validate public BankAccount(@Positive int num) { this.num = num; } @Validate @Min(0) public int add(@Range(min = -100, max = 100) int num) { if ( this.num + num > -1 ) { this.num += num; return this.num; } return this.num + num; } } /** * Main method of runnable jar */ public static void main(String[] args) { BankAccount account = new BankAccount( 10 ); try { // should fail on return value as return min is 0 account.add( -90 ); } catch (ConstraintViolationException e) { e.printStackTrace(); } try { // should fail on add parameter validation. |value| < 100 account.add( 1000 ); } catch (ConstraintViolationException e) { e.printStackTrace(); } try { // should fail on creation - negative initial amount new BankAccount( -1 ); } catch (ConstraintViolationException e) { e.printStackTrace(); } }
running this jar with attached agent
java -javaagent:hibernate-validator-bb-agent.jar -jar hibernate-validator-test-client.jar
gives next result:
May 01, 2018 2:51:45 PM org.hibernate.validator.internal.util.Version <clinit> INFO: HV000001: Hibernate Validator 6.0.9-SNAPSHOT javax.validation.ConstraintViolationException: add.<return value>: must be greater than or equal to 0 at org.hibernate.validator.executable.validation.internal.ValidationEntryPoint.validateReturnValue(ValidationEntryPoint.java:79) at org.hibernate.validator.sample.client.validation.BankAccount.add(BankAccount.java:33) at org.hibernate.validator.sample.client.validation.BankOperationsRunner.main(BankOperationsRunner.java:25) javax.validation.ConstraintViolationException: add.arg0: must be between -100 and 100 at org.hibernate.validator.executable.validation.internal.ValidationEntryPoint.validateParameters(ValidationEntryPoint.java:58) at org.hibernate.validator.sample.client.validation.BankAccount.add(BankAccount.java:29) at org.hibernate.validator.sample.client.validation.BankOperationsRunner.main(BankOperationsRunner.java:32) javax.validation.ConstraintViolationException: BankAccount.arg0: must be greater than 0 at org.hibernate.validator.executable.validation.internal.ValidationEntryPoint.validateConstructorParameters(ValidationEntryPoint.java:99) at org.hibernate.validator.sample.client.validation.BankAccount.<init>(BankAccount.java:22) at org.hibernate.validator.sample.client.validation.BankOperationsRunner.main(BankOperationsRunner.java:39)
Running it without the agent obviously outputs nothing.
I wasn't sure on how to add proper tests for the agent, hence all of the above code is in another branch right now.
@marko-bekhta interesting. About how to test the agent, I suppose ORM has some testing infrastructure for the bytecode instrumentation recently transitioned to Bytebuddy. Might be worthwhile to take a look at how they test it.
@marko-bekhta so ORM doesn't use an agent but instruments the classes at compile time. Wondering if we could do the same and have a similar infrastructure (probably with a Maven plugin using some sort of shared infrastructure so people could build Gradle plugins if they wanted to).
I think the JSON work has higher priority but it's definitely something worth pursuing.
Hi @gsmet ! Here's a separate module with an AspectJ validation that we talked earlier. I've:
ServiceLoaderto get initializedValidationFactory. I haven't figured a different way to pass it to the aspect yet.If we would want to proceed in this direction I would probably work on some documentation to explain how this can be used by the users and also I would really like to add an option to HV to read metadata from static methods and use it in combination with aspects to validate calls of static methods.