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

Detect LoadTimeWeaver bean when declared through @Bean method as well [SPR-10856] #15483

Closed
spring-issuemaster opened this Issue Aug 24, 2013 · 6 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Aug 24, 2013

Nick Williams opened SPR-10856 and commented

This may be a Java-config problem only. I haven't tried it with XML config.

I'm configuring a LocalContainerEntityManagerFactoryBean, and I want to enable load time weaving. @EnableLoadTimeWeaving works (I see in the log that it found addTransformer on the ClassLoader and created a load time weaver), but setLoadTimeWeaver is never called on the LocalContainerEntityManagerFactoryBean.

LocalContainerEntityManagerFactoryBean implements LoadTimeWeaverAware, so my (possibly incorrect) assumption was that Spring should set the LoadTimeWeaver property, but it does not. If my assumption was incorrect, the documentation should be updated to indicate that you must call this method manually. If my assumption was correct, there is a bug here, because Spring is not calling this method.

Instead, I have to do this in my configuration:

@Configuration
...
@EnableLoadTimeWeaving
...
public class RootContextConfiguration
{
    ...
    @Inject LoadTimeWeaver loadTimeWeaver;
    ...

    @Bean
    public LocalContainerEntityManagerFactoryBean entityManagerFactoryBean()
    {
        ...
        LocalContainerEntityManagerFactoryBean factory =
                new LocalContainerEntityManagerFactoryBean();
        ...
        factory.setLoadTimeWeaver(this.loadTimeWeaver);
        ...
        return factory;
    }
    ...
}

That code works. The LoadTimeWeaver is injected and I successfully add it to my factory, then the JPA provider starts instrumenting my classes. However, without this the LoadTimeWeaver is never added to the LocalContainerEntityManagerFactoryBean and the JPA provider cannot instrument my classes.


Affects: 3.2.4, 4.0 M2

Issue Links:

  • #15365 LoadTimeWeaving is working with XML config and not with Java Config ("is duplicated by")
  • #19498 NPE in LoadTimeWeavingConfiguration: loadTimeWeaver() called too early
  • #18459 Proper load-time weaving support for Hibernate 5

Referenced from: commits 437ffa6

9 votes, 14 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 22, 2013

Nick Williams commented

Juergen Hoeller, are you not going to be able to get this fixed for 4.0.0? That's disappointing. :-(

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Dec 29, 2013

Nick Williams commented

Any possibility this gets fixed in 4.0.1 or 4.0.2?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Dec 11, 2014

Steve McKay commented

I encountered this issue with Spring 4.1.1, and it's caused by the way the ApplicationContext registers LoadTimeWeaverAwareProcessor. LoadTimeWeaver definition is checked for in AbstractApplicationContext.prepareBeanFactory(), which runs before BeanFactoryPostProcessors. Because @Configuration classes are handled by a BeanFactoryPostProcessor, the LoadTimeWeaver registered by @EnableLoadTimeWeaving is added to late too trigger registration of LoadTimeWeaverAwareProcessor.

Because I'm using Spring Boot, I used a SpringApplicationRunListener to register a LoadTimeWeaver in the contextLoaded() callback. Of course that only works with Spring Boot so anyone not using it will have to find some other workaround. The important thing is to have a LoadTimeWeaver definition available before calling ApplicationContext.refresh().

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 10, 2016

Michael Simons commented

Here's a snippet that works with Spring Boot in case anybody lands here:

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.event.ApplicationPreparedEvent;
import org.springframework.context.ApplicationListener;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.instrument.classloading.InstrumentationLoadTimeWeaver;
 
@SpringBootApplication
public class Application {    
    public static void main(final String... args) {
        final SpringApplication springApplication = new SpringApplication(Application.class);
        springApplication.addListeners(new ApplicationListener<ApplicationPreparedEvent>() {
            @Override
            public void onApplicationEvent(final ApplicationPreparedEvent event) {
                event.getApplicationContext().getBeanFactory().registerSingleton(ConfigurableApplicationContext.LOAD_TIME_WEAVER_BEAN_NAME, new InstrumentationLoadTimeWeaver());
            }
        });
        springApplication.run(args);
    }
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 11, 2016

Juergen Hoeller commented

Thanks for the pointers! It seems that we need to detect a LoadTimeWeaver bean when declared through an @Bean method as well, after we registered all BeanFactoryPostProcessors. We did not support that at all before but we are doing it in AbstractApplicationContext as of 4.3 now.

That said, it might make sense for Spring Boot to go a step further and have first-class support for LoadTimeWeaver setup, registering an externally declared LoadTimeWeaver class at the earliest possible point.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 12, 2016

Andrei Ivanov commented

Sorry to hijack this issue, but maybe I can draw attention to my related issue too, #18466 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment