Search code examples
springspring-datajavers

Spring AOP, @JaversAuditable still trying to commit a change when exception has been thrown


This is with Spring 4.1.9.RELEASE, Spring-Data 1.11.4.RELEASE, Javers 1.6.0 with spring integration

So using Spring with @JaversAuditable, I'm finding that if an exception handles when I try and insert (such as duplicate key exception), JaversTransactionalDecorator still goes ahead and tries to generate a commit.

However, since my entity doesn't have an ID due to the failed insert, Javers ends up throwing an exception (see below). Stepping through debug, I'm finding in I end up in org.springframework.aop.aspectjAspectJAfterAdvice.invoke() in the finally block. It then calls invokeAdviceMethod(JoinPointMatch,null,null), with that 3rd null being a spot for a Throwable.

According to http://docs.spring.io/spring/docs/4.1.9.RELEASE/spring-framework-reference/html/aop.html, @After advice will run as a finally, regardless if the method exists normally or with an exception. Thus, it sounds like Javers needs to be able to be able to detect if an exception has been thrown, or use a different Advice like @AfterReturning, possibly with a separate @AfterThrowing. I'm trying to find out in the current @After that Javers has ( org.javers.spring.auditable.aspect.JaversAuditableRepositoryAspect.commitAdvice(JoinPoint pjp) ) how it'd be able to detect if an exception/throwable has been thrown, but I can't seem to find anything in the docs about that.

So, is this something Javers already has but for some reason it's not correctly picking up on the thrown SQL exception, or where would some info on detecting in @After advice the thrown exception/throwable so I could go in and contribute a fix for this?

Sorry for the possibly less than extremely organized, I'm here a couple hours past normal time.

Caused by: org.javers.common.exception.JaversException: ENTITY_INSTANCE_WITH_NULL_ID Found Entity instance of class 'com.findology.api.model.publisher.Publisher' with null id
at org.javers.core.metamodel.type.EntityType.getIdOf(EntityType.java:96)
at org.javers.core.metamodel.object.InstanceId.createFromInstance(InstanceId.java:28)
at org.javers.core.metamodel.object.GlobalIdFactory.createId(GlobalIdFactory.java:47)
at org.javers.core.graph.LiveCdoFactory.create(LiveCdoFactory.java:24)
at org.javers.core.graph.LiveCdoFactory.create(LiveCdoFactory.java:9)
at org.javers.core.graph.EdgeBuilder.asCdo(EdgeBuilder.java:39)
at org.javers.core.graph.ObjectGraphBuilder.buildGraph(ObjectGraphBuilder.java:45)
at org.javers.core.graph.LiveGraphFactory.createLiveGraph(LiveGraphFactory.java:39)
at org.javers.core.commit.CommitFactory.create(CommitFactory.java:76)
at org.javers.core.JaversCore.commit(JaversCore.java:70)
at org.javers.spring.jpa.JaversTransactionalDecorator.commit(JaversTransactionalDecorator.java:52)
at org.javers.spring.jpa.JaversTransactionalDecorator$$FastClassBySpringCGLIB$$acb40bd0.invoke(<generated>)
at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:718)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:157)
at org.springframework.transaction.interceptor.TransactionInterceptor$1.proceedWithInvocation(TransactionInterceptor.java:99)
at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:281)
at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:96)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:653)
at org.javers.spring.jpa.JaversTransactionalDecorator$$EnhancerBySpringCGLIB$$c6591394.commit(<generated>)
at org.javers.spring.auditable.aspect.JaversCommitAdvice.commitMethodArguments(JaversCommitAdvice.java:24)
at org.javers.spring.auditable.aspect.JaversAuditableRepositoryAspect.commitAdvice(JaversAuditableRepositoryAspect.java:48)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
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.AspectJAfterAdvice.invoke(AspectJAfterAdvice.java:46)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:168)
at org.springframework.transaction.interceptor.TransactionInterceptor$1.proceedWithInvocation(TransactionInterceptor.java:99)
at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:281)
at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:96)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:653)
at com.findology.api.dao.PublisherDao$$EnhancerBySpringCGLIB$$8ba00510.insert(<generated>)
at com.findology.api.controller.SimpleController.createPublisher(SimpleController.java:82)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:222)
at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:137)
at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:110)
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:775)
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:705)
at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:85)
at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:959)
at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:893)
at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:965)
... 74 common frames omitted

Solution

  • So, final resolution is changing to use @AfterReturning as per Spring docs, it's in the process of being accepted as per https://github.com/javers/javers/pull/356, so a future version will incorporate the fix.