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

Explore constructor/method validation using AspectJ or ByteBuddy #937

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

marko-bekhta
Copy link
Member

Hi @gsmet ! Here's a separate module with an AspectJ validation that we talked earlier. I've:

  • added new module. It is under a new parent module as I would also like to investigate the ByteBuddy approach as well and see if we could give users different options to choose from.
  • created an aspect to validate methods and constructors. It is using a different syntax from the aspect that I've showed previously as it turned out that the initial approach caused issues with return value validation.
  • added validator factory producer to get initialized validation factor. This uses the ServiceLoader to get initialized ValidationFactory. I haven't figured a different way to pass it to the aspect yet.
  • there are 4 simple tests to show how the validation would work with such aspect.

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.

- added new module
- created an aspect to validate methods and constructors
- added validator factory producer to get initialized validation factory
@gunnarmorling
Copy link
Member

gunnarmorling commented Mar 29, 2018

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).

@marko-bekhta
Copy link
Member Author

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.

@marko-bekhta
Copy link
Member Author

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.

@gsmet
Copy link
Member

gsmet commented May 3, 2018

@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.

@gsmet
Copy link
Member

gsmet commented May 3, 2018

@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.

@gsmet gsmet changed the title Added new module with validation aspect Explore constructor/method validation using AspectJ or ByteBuddy May 3, 2018
Base automatically changed from master to main March 19, 2021 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants