BVal
  1. BVal
  2. BVAL-72

NPE when doing methodvalidation

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.2-incubating
    • Fix Version/s: 0.2-incubating
    • Component/s: jsr303
    • Labels:
      None
    • Environment:

      JDK 1.5, spring 3.0.3

      Description

      I get an npe when calling this annotated method (the annotations are on the implementation class - not on the interface):
      findUser( @Size( min=1 ) String param1,
      @NotNull String param2,
      @NotNull SomeClass param3 )

      It ends up not findin a MethodDescriptor, this it's null when attempting to call method validation.

      java.lang.NullPointerException
      	at org.apache.bval.jsr303.extensions.MethodValidatorImpl.validateParameters(MethodValidatorImpl.java:86)
      	at com.edb.payment.pays.core.util.aop.validation.ValidationAspect.validate(ValidationAspect.java:67)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      	at java.lang.reflect.Method.invoke(Method.java:592)
      	at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:621)
      	at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:603)
      	at org.springframework.aop.aspectj.AspectJMethodBeforeAdvice.before(AspectJMethodBeforeAdvice.java:39)
      	at org.springframework.aop.framework.adapter.MethodBeforeAdviceInterceptor.invoke(MethodBeforeAdviceInterceptor.java:49)
      	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
      	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:110)
      	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
      	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:89)
      	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
      	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:202)
      	at $Proxy21.findUser(Unknown Source)
      	at com.edb.payment.pays.core.service.user.UserServiceImplTest.findUserThatExists(UserServiceImplTest.java:27)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      	at java.lang.reflect.Method.invoke(Method.java:592)
      	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
      	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
      	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
      	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
      	at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:74)
      	at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:82)
      	at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:72)
      	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:240)
      	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
      	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
      	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
      	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
      	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
      	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
      	at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
      	at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
      	at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
      	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:180)
      	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:49)
      	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
      	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
      	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
      	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
      	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
      
      
      1. bval72.zip
        6 kB
        David J. M. Karlsen

        Activity

        Hide
        Carlos Vara added a comment -

        Hi David,

        I can't replicate the issue with a simple standalone test involving a method declared in an interface, so I think it's very likely related to Spring's dynamic proxies. Looking at your stacktrace, you are running a junit test, could you please attach it to the issue?

        Show
        Carlos Vara added a comment - Hi David, I can't replicate the issue with a simple standalone test involving a method declared in an interface, so I think it's very likely related to Spring's dynamic proxies. Looking at your stacktrace, you are running a junit test, could you please attach it to the issue?
        Hide
        David J. M. Karlsen added a comment -

        Unfortunately I can't free the code.
        I can however provide some debugging info:

        MethodDescriptorImpl methodDescriptor =
                (MethodDescriptorImpl) beanDesc.getConstraintsForMethod(method);
            
        methodDescriptor is null so that the next line
        return validateParameters(methodDescriptor.getMetaBean(),
                methodDescriptor.getParameterDescriptors(), parameters, groupArray);
        will NPE (maybe it should be more robust?)
        
        

        The method object in the aspect is:
        public abstract UserType com.edb.payment.pays.core.service.user.UserService.findUser(java.lang.String,java.lang.String,java.lang.String)
        Which is the interface method (note the abstract - it's also missing the annotations).
        Should the interface or the implementing class be annotated - or shouldn't it matter?

        The before aspect:
        (The target type is the actual class implementing the interface.)

            @Before("anyPublicMethodExecution() && withinCoreLayer()")
            public void validate( JoinPoint joinPoint )
                throws ValidationException
            {
                MethodSignature methodSignature = (MethodSignature) joinPoint.getSignature();
                MethodValidator methodValidator = getMethodValidator();
        
                Class<?> clazz = joinPoint.getTarget().getClass();
                Method method = methodSignature.getMethod();
                Object[] args = joinPoint.getArgs();
        
                logger.trace( "Validating method {} on target {} passed args {}", new Object[] { clazz, method, args } );
        
                Set<? extends ConstraintViolation<?>> validationErrors = methodValidator.validateParameters( clazz, method,
                                                                                                             args );
                if ( !validationErrors.isEmpty() )
                {
                    throw new ValidationException( validationErrors );
                }
            }
        
        

        If you really need it I can create a case which looks the same but on code which can be freed - please ping back.

        Show
        David J. M. Karlsen added a comment - Unfortunately I can't free the code. I can however provide some debugging info: MethodDescriptorImpl methodDescriptor = (MethodDescriptorImpl) beanDesc.getConstraintsForMethod(method); methodDescriptor is null so that the next line return validateParameters(methodDescriptor.getMetaBean(), methodDescriptor.getParameterDescriptors(), parameters, groupArray); will NPE (maybe it should be more robust?) The method object in the aspect is: public abstract UserType com.edb.payment.pays.core.service.user.UserService.findUser(java.lang.String,java.lang.String,java.lang.String) Which is the interface method (note the abstract - it's also missing the annotations). Should the interface or the implementing class be annotated - or shouldn't it matter? The before aspect: (The target type is the actual class implementing the interface.) @Before("anyPublicMethodExecution() && withinCoreLayer()") public void validate( JoinPoint joinPoint ) throws ValidationException { MethodSignature methodSignature = (MethodSignature) joinPoint.getSignature(); MethodValidator methodValidator = getMethodValidator(); Class<?> clazz = joinPoint.getTarget().getClass(); Method method = methodSignature.getMethod(); Object[] args = joinPoint.getArgs(); logger.trace( "Validating method {} on target {} passed args {}", new Object[] { clazz, method, args } ); Set<? extends ConstraintViolation<?>> validationErrors = methodValidator.validateParameters( clazz, method, args ); if ( !validationErrors.isEmpty() ) { throw new ValidationException( validationErrors ); } } If you really need it I can create a case which looks the same but on code which can be freed - please ping back.
        Hide
        Carlos Vara added a comment -

        Ok, I will try to replicate the scenario this weekend. Just a quick note, you are using spring's proxies to get your aspects to work right?

        Regarding the robustness of the null pointer check, checking for null there is easy, but I would like to investigate on whether we can actually parse the annotated parameters when given a proxy object so it can actually be validated.

        Show
        Carlos Vara added a comment - Ok, I will try to replicate the scenario this weekend. Just a quick note, you are using spring's proxies to get your aspects to work right? Regarding the robustness of the null pointer check, checking for null there is easy, but I would like to investigate on whether we can actually parse the annotated parameters when given a proxy object so it can actually be validated.
        Hide
        Carlos Vara added a comment -

        I managed to replicate the problem. I will keep you updated on the progress.

        Show
        Carlos Vara added a comment - I managed to replicate the problem. I will keep you updated on the progress.
        Hide
        Carlos Vara added a comment -

        In your before aspect, add:

        method = clazz.getDeclaredMethod(method.getName(), method.getParameterTypes());

        after the line:

        Method method = methodSignature.getMethod();

        Reason is, clazz gets the bean class (not the interface), but method gets the declaration of the interface method, which is abstract. As the signatures differ, the retrieval of the method constraints fails.

        Show
        Carlos Vara added a comment - In your before aspect, add: method = clazz.getDeclaredMethod(method.getName(), method.getParameterTypes()); after the line: Method method = methodSignature.getMethod(); Reason is, clazz gets the bean class (not the interface), but method gets the declaration of the interface method, which is abstract. As the signatures differ, the retrieval of the method constraints fails.
        Hide
        David J. M. Karlsen added a comment -

        Great - now I can drop the separate commons lang call to find it by name and parameters and reflection.
        However I still do not get the anticipated result on the returnAspect.
        Attaching a full .zip of a maven project where I've isolated the aspect w/test and spring.

        Show
        David J. M. Karlsen added a comment - Great - now I can drop the separate commons lang call to find it by name and parameters and reflection. However I still do not get the anticipated result on the returnAspect. Attaching a full .zip of a maven project where I've isolated the aspect w/test and spring.
        Hide
        David J. M. Karlsen added a comment -

        example project failing on afterReturning

        Show
        David J. M. Karlsen added a comment - example project failing on afterReturning
        Hide
        David J. M. Karlsen added a comment -

        Doh - I was a little quick on the ctrl-space
        Didn't notice I had getThis() and not getTarget()
        case closed!
        Thanks for the help.

        Show
        David J. M. Karlsen added a comment - Doh - I was a little quick on the ctrl-space Didn't notice I had getThis() and not getTarget() case closed! Thanks for the help.
        Hide
        Carlos Vara added a comment -

        Solved.

        Also, in r960471 code was added to throw a ValidationException instead of a NPE in case no matching method is found on the holder object.

        Show
        Carlos Vara added a comment - Solved. Also, in r960471 code was added to throw a ValidationException instead of a NPE in case no matching method is found on the holder object.

          People

          • Assignee:
            Carlos Vara
            Reporter:
            David J. M. Karlsen
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development