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

Document how to rely on ServletContext with an embedded container setup #24561

Closed
zaynetro opened this issue Dec 18, 2020 · 8 comments
Closed
Labels
type: documentation A documentation update
Milestone

Comments

@zaynetro
Copy link
Contributor

If Configuration class implements both ServletContextAware and DestructionAwareBeanPostProcessor then setServletContext will not be called.

Steps to reproduce:

  1. Init a project https://start.spring.io/
  2. Maven project, Spring boot 2.4.1, Java 8
  3. Add spring-boot-starter-web dependency
  4. Add two configuration classes
DemoConfig
package com.example.demo;

import javax.annotation.PostConstruct;
import javax.servlet.ServletContext;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.config.DestructionAwareBeanPostProcessor;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.context.ServletContextAware;

@Configuration
public class DemoConfig implements DestructionAwareBeanPostProcessor, ServletContextAware {
    private static final Logger LOG = LoggerFactory.getLogger( DemoConfig.class );

    private ServletContext servletContext;

    @PostConstruct
    public void init() {
        LOG.warn( "init() {}", servletContext );
    }

    @Override
    public void postProcessBeforeDestruction( Object bean, String beanName ) throws BeansException {
    }

    @Override
    public void setServletContext( ServletContext servletContext ) {
        LOG.warn( "setServletContext() {}", servletContext );
        this.servletContext = servletContext;
    }

}
AnotherConfig
package com.example.demo;

import javax.annotation.PostConstruct;
import javax.servlet.ServletContext;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.context.ServletContextAware;

@Configuration
public class AnotherConfig implements ServletContextAware {
    private static final Logger LOG = LoggerFactory.getLogger( AnotherConfig.class );

    private ServletContext servletContext;

    @PostConstruct
    public void init() {
        LOG.warn( "init() {}", servletContext );
    }

    @Override
    public void setServletContext( ServletContext servletContext ) {
        LOG.warn( "setServletContext() {}", servletContext );
        this.servletContext = servletContext;
    }

}

Logs:

2020-12-18 22:17:01.742  INFO 331634 --- [           main] com.example.demo.DemoApplication         : Starting DemoApplication using Java 1.8.0_265 on zaynetro-thinkpad with PID 331634 (/home/zaynetro/Code/trial/demo-spring-boot/target/classes started by zaynetro in /home/zaynetro/Code/trial/demo-spring-boot)
2020-12-18 22:17:01.745 DEBUG 331634 --- [           main] com.example.demo.DemoApplication         : Running with Spring Boot v2.4.1, Spring v5.3.2
2020-12-18 22:17:01.746  INFO 331634 --- [           main] com.example.demo.DemoApplication         : No active profile set, falling back to default profiles: default
2020-12-18 22:17:02.367  WARN 331634 --- [           main] com.example.demo.DemoConfig              : init() null
2020-12-18 22:17:02.614  INFO 331634 --- [           main] o.s.b.w.embedded.tomcat.TomcatWebServer  : Tomcat initialized with port(s): 8080 (http)
2020-12-18 22:17:02.623  INFO 331634 --- [           main] o.apache.catalina.core.StandardService   : Starting service [Tomcat]
2020-12-18 22:17:02.623  INFO 331634 --- [           main] org.apache.catalina.core.StandardEngine  : Starting Servlet engine: [Apache Tomcat/9.0.41]
2020-12-18 22:17:02.668  INFO 331634 --- [           main] o.a.c.c.C.[Tomcat].[localhost].[/]       : Initializing Spring embedded WebApplicationContext
2020-12-18 22:17:02.668  INFO 331634 --- [           main] w.s.c.ServletWebServerApplicationContext : Root WebApplicationContext: initialization completed in 873 ms
2020-12-18 22:17:02.713  WARN 331634 --- [           main] com.example.demo.AnotherConfig           : setServletContext() org.apache.catalina.core.ApplicationContextFacade@299266e2
2020-12-18 22:17:02.713  WARN 331634 --- [           main] com.example.demo.AnotherConfig           : init() org.apache.catalina.core.ApplicationContextFacade@299266e2
2020-12-18 22:17:02.842  INFO 331634 --- [           main] o.s.s.concurrent.ThreadPoolTaskExecutor  : Initializing ExecutorService 'applicationTaskExecutor'
2020-12-18 22:17:02.985  INFO 331634 --- [           main] o.s.b.w.embedded.tomcat.TomcatWebServer  : Tomcat started on port(s): 8080 (http) with context path ''

setServletContext is called correctly for AnotherConfig but never called for DemoConfig.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 18, 2020
@philwebb
Copy link
Member

Having a @Configuration class implement DestructionAwareBeanPostProcessor is quite an unusual pattern and not something I'd recommend. Have you tried creating a dedicated class for the DestructionAwareBeanPostProcessor functionality and creating it from a static @Bean method?

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Dec 18, 2020
@zaynetro
Copy link
Contributor Author

Thank you for your quick response!

We are migrating our Spring (5.2) Jetty Servlet (9.4) application to spring boot. The same configuration class used to work in the previous setup for us hence I decided to create an issue. We are actually following a recommended way to configure CometD library with Spring.

I will try to figure out if I can use your suggestion to achieve the same.

@snicoll
Copy link
Member

snicoll commented Dec 19, 2020

We are actually following a recommended way to configure CometD library with Spring.

Unfortunately, that doesn't change the fact that this is unusual and should be avoided. I've created an issue to ask them to consider reviewing this part of the code and doc.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Dec 19, 2020
@zaynetro
Copy link
Contributor Author

Sadly, I am a bit of at loss in here.

My problem is that my class implementing DestructionAwareBeanPostProcessor depends on a bean that requires ServletContext. It seems that ServletContext is not initialized until DestructionAwareBeanPostProcessor bean is constructed.

Could you share more info regarding:

creating it from a static @bean method

I don't think I fully understand how to utilize that to solve my problem.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 21, 2020
@snicoll
Copy link
Member

snicoll commented Dec 21, 2020

@zaynetro thanks for the update.

When you start a web application with the Spring Framework, the server is already started by the time the ApplicationContext is bootstrapped. As such the ServletContext is set very early on and any ServletContextAware callback can be honoured as you've experienced.

That said, this is mixing two different phase of the application. A @Configuration is meant to defines the components of the application and should not implement any "runtime" business logic. With Spring Boot and its embedded server support, the server is starting as part of the application startup and the ServletContext may or may not available. As startup happens asynchronously, you have no guarantee that the context is available if you don't express a dependency on it.

Steps to reproduce:

Going forward, could you please move that to an actual GitHub repository? This is what I had to do to reproduce the problem and can be a good base for discussion. I've pushed your change and then an additional commit that describes what we were discussing here. Please note in particular that the access to ServletContext is delayed to the actual use rather than storing the variable on startup.

Can you please give that DemoPostProcessor a try and let us know how it goes?

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Dec 21, 2020
@zaynetro
Copy link
Contributor Author

As startup happens asynchronously, you have no guarantee that the context is available if you don't express a dependency on it.

How can you express a dependency on ServletContext? ServletContextAware interface isn't really a precondition. Is there other ways?

I need for a DestructionAwareBeanPostProcessor to depend on a bean that depends on ServletContext. Unfortunately, when I do that currently my bean is initialized before ServletContext is set.

I have tried to express my problem here: snicoll-scratches/spring-boot-issue-24561#1

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 22, 2020
@snicoll
Copy link
Member

snicoll commented Dec 22, 2020

How can you express a dependency on ServletContext? ServletContextAware interface isn't really a precondition.

That is a good point. ServletContextAware will be honoured if the servlet context is available by the time the bean is actually created. We've already discussed you can't really do this as part of the context initialisation since the servlet context itself is set when starting the server.

Rather, you should get a callback once the server has started (and the servlet context is set). You could do so via ApplicationListener<ApplicationStartedEvent> for instance.

I need for a DestructionAwareBeanPostProcessor to depend on a bean that depends on ServletContext. Unfortunately, when I do that currently my bean is initialized before ServletContext is set.

With an embedded server support, I don't see a way for a bean to reliably depend on the ServletContext using regular dependency injection style. Perhaps that bean could have the servletContext injected at a later stage (via the listener I've just expressed above?).

I have tried to express my problem here: snicoll-scratches/spring-boot-issue-24561#1

Great. Let's continue the brainstorm over there.

@snicoll snicoll changed the title setServletContext is not called when class also implements DestructionAwareBeanPostProcessor Document how to rely on ServletContext with an embedded container setup Dec 23, 2020
@snicoll snicoll added type: documentation A documentation update and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Dec 23, 2020
@snicoll snicoll added this to the 2.3.x milestone Dec 23, 2020
@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Dec 23, 2020
@zaynetro
Copy link
Contributor Author

zaynetro commented Dec 28, 2020

Alright, I think I have found a workaround solution that works for me. I create my bean that depends on servlet context and create post processor as a static bean.

AnotherConfig.java
@Configuration
public class AnotherConfig implements ServletContextAware {
	private static final Logger LOG = LoggerFactory.getLogger( AnotherConfig.class );

	private ServletContext servletContext;

	@Override
	public void setServletContext( ServletContext servletContext ) {
		this.servletContext = servletContext;
	}

	@Bean
	public Hello hello() {
		return new Hello(servletContext);
	}

	@Bean
	public static DemoPostProcessor demoPostProcessor() {
		return new DemoPostProcessor();
	}
}

And then in my post processor I try to get a bean after it has been initialized.

DemoPostProcessor.java
public class DemoPostProcessor implements DestructionAwareBeanPostProcessor, ApplicationContextAware {

	private static final Logger LOG = LoggerFactory.getLogger( DemoPostProcessor.class );

	private WebApplicationContext applicationContext;

	Optional<Hello> getHello() {
		if( applicationContext.getServletContext() != null ) {
			// Once we have servletContext I want to instantiate my Hello bean. This could fail with:
			//
			// > Caused by: org.springframework.beans.factory.BeanCurrentlyInCreationException:
			// > Error creating bean with name 'hello':
			// > Requested bean is currently in creation: Is there an unresolvable circular reference?

			try {
				return Optional.of( applicationContext.getBean( Hello.class ) );
			} catch( BeanCurrentlyInCreationException e ) {
				// Ignore
			}
		}

		return Optional.empty();
	}

	@Override
	public Object postProcessBeforeInitialization( Object bean, String name ) throws BeansException {
		Optional<Hello> found = getHello();
		if( found.isPresent() ) {
			found.get().process( bean, name );
		}
		return bean;
	}

	@Override
	public void postProcessBeforeDestruction( Object bean, String name ) throws BeansException {
		LOG.warn("Destruction of {} with servletContext {}", name, getHello().get().servletContext);
	}

	@Override
	public void setApplicationContext( ApplicationContext applicationContext ) throws BeansException {
		this.applicationContext = (WebApplicationContext) applicationContext;
	}
}

Note that I don't particularly like this solution but it works for my use case. There is an option to use CometD library without processing the annotations so I might switch to that if this solution becomes unreliable.

As a final note it would be nice for a bean to be able to depend on a servlet context but I understand that it might be undesirable with post processor beans.

Feel free to close this issue.

UPD: this solution is ugly and not recommended. I have switched to post processing beans manually instead.

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Jan 6, 2021
@scottfrederick scottfrederick self-assigned this May 27, 2021
@wilkinsona wilkinsona removed this from the 2.3.x milestone Jun 10, 2021
@wilkinsona wilkinsona added this to the 2.4.x milestone Jun 10, 2021
@scottfrederick scottfrederick removed their assignment Nov 17, 2021
@wilkinsona wilkinsona modified the milestones: 2.4.x, 2.5.x Nov 18, 2021
@mbhave mbhave modified the milestones: 2.5.x, 2.5.11 Mar 9, 2022
@mbhave mbhave closed this as completed in d240e29 Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

7 participants