javers / javers

JaVers - object auditing and diff framework for Java
http://javers.org
Apache License 2.0
1.39k stars 361 forks source link

Javers in a Spring boot application causes runtime errors when enabling CGLIB proxy instead of Interface proxy #181

Closed deepu105 closed 9 years ago

deepu105 commented 9 years ago

I have tried out Javers in default application generated by JHipster and it works well, but if I enable proxyTargetClass = true in spring @EnableGlobalMethodSecurity Javers is causing run time exception in the repository

as per spring docs

    /**
     * Indicate whether subclass-based (CGLIB) proxies are to be created ({@code true}) as
     * opposed to standard Java interface-based proxies ({@code false}). The default is
     * {@code false}. <strong>Applicable only if {@link #mode()} is set to
     * {@link AdviceMode#PROXY}</strong>.
     *
     * <p>
     * Note that setting this attribute to {@code true} will affect <em>all</em>
     * Spring-managed beans requiring proxying, not just those marked with the Security
     * annotations. For example, other beans marked with Spring's {@code @Transactional}
     * annotation will be upgraded to subclass proxying at the same time. This approach
     * has no negative impact in practice unless one is explicitly expecting one type of
     * proxy vs another, e.g. in tests.
     *
     * @return true if CGILIB proxies should be created instead of interface based
     * proxies, else false
     */

so will you guys be able to check if you are explicitly expecting Java interface-based proxies ?

bartoszwalacik commented 9 years ago

hi, first, are you talking about @JaversSpringDataAuditable or @JaversAuditable aspects?

I'm not an expert in Spring AOP, take a look at our impl and maybe you can say what's wrong https://github.com/javers/javers/blob/69ff9949086038be8e7e72c1f41a6c8996243e4a/javers-spring/src/main/java/org/javers/spring/auditable/aspect/JaversAuditableRepositoryAspect.java

bartoszwalacik commented 9 years ago

could you provide a failing test which which reproduces this issue?

deepu105 commented 9 years ago

Im talking about JaversSpringDataAuditable coz i was using that in a project generated by jHipster. A failing test can be reproduced with any spring boot application with spring data repository just set the proxyTargetClass param true on any supported annotation

bartoszwalacik commented 9 years ago

Will check it

pszymczyk commented 9 years ago

Hi

Look at here: https://github.com/pawelszymczyk/organization-structure - this application based on spring-boot and using javers-spring, I hope this could help you.

deepu105 commented 9 years ago

@pawelszymczyk I dont have an issue in getting it working with spring boot the issue arises only if i do @EnableAspectJAutoProxy(proxyTargetClass = true) instead of @EnableAspectJAutoProxy

In my app I was using @EnableGlobalMethodSecurity(prePostEnabled = true, securedEnabled = true, proxyTargetClass = true, mode = AdviceMode.PROXY) which cause the issue so for now i made proxyTargetClass = false as i can workaround my need of having it true in the first place

So in your demo app try using @EnableAspectJAutoProxy(proxyTargetClass = true) instead of @EnableAspectJAutoProxy

bartoszwalacik commented 9 years ago

@pawelszymczyk I'll check it

bartoszwalacik commented 9 years ago

I've wrote some spring-boot tests with cglib proxies enabled and I got this exception: Cannot subclass final class class com.sun.proxy.$Proxy60

Is this the same exception that you have?

Not sure if it is a JaVers bug, as far as I understand, this exception says that Spring tries to create proxy on proxy and fails because cglib proxies are final.

I guess that this could be some general problem in Spring, when you add aspects to spring-data CrudRepositories (which are auto-generated).

bartoszwalacik commented 9 years ago

see http://stackoverflow.com/questions/13977093/how-to-use-jparepositories-with-proxy-target-class-true

deepu105 commented 9 years ago

@bartoszwalacik yes its the same error. Not sure if a spring issue but this happens only when introducing the javers configuration classes, i have tried a fresh app with cglib first ot works then added javers and it fails. You can try as well

deepu105 commented 9 years ago

To be specific if i comment the javers aspect class it works, im using other aspects on repository and i dont have the issue

bartoszwalacik commented 9 years ago

could you publish your test with any working aspect on CrudRepository? In my test when I change JaversAuditableAspect to any other aspect, even with simple log statement, Spring fails in the same way.

I've asked on stack about it: http://stackoverflow.com/questions/31679228/unable-to-create-spring-aop-aspect-on-spring-data-jpa-repository-when-cglib-prox Take a look at @manish response...

bartoszwalacik commented 9 years ago

this is a Spring issue, see https://jira.spring.io/browse/SPR-12870, looks like it's fixed in Spring 4.2 RC1

bartoszwalacik commented 9 years ago

I've tested JaVers auditable aspect with the latest Spring milestone 4.2.0.RC2 and it works in both cases: Java and CGLIB. Use these versions:

repositories {
    maven {
        url "http://repo.spring.io/milestone"
    }
}

ext {
    springVersion = '4.2.0.RC2'
    springBootVersion = '1.3.0.M2'
}
deepu105 commented 9 years ago

Thanks guys, thats good On 31 Jul 2015 05:56, "Bartosz Walacik" notifications@github.com wrote:

I've tested JaVers auditable aspect with the latest Spring milestone 4.2.0.RC2 and it works in both cases: Java and CGLIB. Use these versions:

repositories { maven { url "http://repo.spring.io/milestone" } }

ext { springVersion = '4.2.0.RC2' springBootVersion = '1.3.0.M2' }

— Reply to this email directly or view it on GitHub https://github.com/javers/javers/issues/181#issuecomment-126504631.