Skip to content

Apply TomcatConnectorCustomizer and TomcatContextCustomizer beans automatically #15492

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Apply TomcatConnectorCustomizer and TomcatContextCustomizer beans automatically #15492

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3c9bb5d
Fixes gh-15062
Raheela1024 Dec 19, 2018
98d0ec4
Concourse CI build failure error issue resolved.
Raheela1024 Dec 19, 2018
73e580b
review comments resolved.
Raheela1024 Dec 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions ...pringframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,18 @@

package org.springframework.boot.autoconfigure.web.reactive;

import java.util.stream.Collectors;

import io.undertow.Undertow;
import reactor.netty.http.server.HttpServer;

import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.web.embedded.jetty.JettyReactiveWebServerFactory;
import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory;
import org.springframework.boot.web.embedded.tomcat.TomcatConnectorCustomizer;
import org.springframework.boot.web.embedded.tomcat.TomcatContextCustomizer;
import org.springframework.boot.web.embedded.tomcat.TomcatReactiveWebServerFactory;
import org.springframework.boot.web.embedded.undertow.UndertowReactiveWebServerFactory;
import org.springframework.boot.web.reactive.server.ReactiveWebServerFactory;
Expand All @@ -38,6 +43,7 @@
* their order of execution.
*
* @author Brian Clozel
* @author Raheela Aslam
*/
abstract class ReactiveWebServerFactoryConfiguration {

Expand Down Expand Up @@ -72,6 +78,18 @@ public TomcatReactiveWebServerFactory tomcatReactiveWebServerFactory() {
return new TomcatReactiveWebServerFactory();
}

@Bean
public TomcatReactiveWebServerFactory tomcatReactiveWebServerFactory(
ObjectProvider<TomcatConnectorCustomizer> connectorCustomizers,
ObjectProvider<TomcatContextCustomizer> contextCustomizers) {
TomcatReactiveWebServerFactory factory = new TomcatReactiveWebServerFactory();
factory.getTomcatConnectorCustomizers().addAll(
connectorCustomizers.orderedStream().collect(Collectors.toList()));
factory.getTomcatContextCustomizers().addAll(
contextCustomizers.orderedStream().collect(Collectors.toList()));
return factory;
}

}

@Configuration
Expand Down
18 changes: 18 additions & 0 deletions .../springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.springframework.boot.autoconfigure.web.servlet;

import java.util.stream.Collectors;

import javax.servlet.Servlet;

import io.undertow.Undertow;
Expand All @@ -26,10 +28,13 @@
import org.eclipse.jetty.webapp.WebAppContext;
import org.xnio.SslClientAuthMode;

import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.autoconfigure.condition.SearchStrategy;
import org.springframework.boot.web.embedded.jetty.JettyServletWebServerFactory;
import org.springframework.boot.web.embedded.tomcat.TomcatConnectorCustomizer;
import org.springframework.boot.web.embedded.tomcat.TomcatContextCustomizer;
import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory;
import org.springframework.boot.web.embedded.undertow.UndertowServletWebServerFactory;
import org.springframework.boot.web.servlet.server.ServletWebServerFactory;
Expand All @@ -47,6 +52,7 @@
* @author Ivan Sopov
* @author Brian Clozel
* @author Stephane Nicoll
* @author Raheela Asalm
*/
@Configuration
class ServletWebServerFactoryConfiguration {
Expand All @@ -61,6 +67,18 @@ public TomcatServletWebServerFactory tomcatServletWebServerFactory() {
return new TomcatServletWebServerFactory();
}

@Bean
public TomcatServletWebServerFactory tomcatServletWebServerFactory(
ObjectProvider<TomcatConnectorCustomizer> connectorCustomizers,
ObjectProvider<TomcatContextCustomizer> contextCustomizers) {
TomcatServletWebServerFactory factory = new TomcatServletWebServerFactory();
factory.getTomcatConnectorCustomizers().addAll(
connectorCustomizers.orderedStream().collect(Collectors.toList()));
factory.getTomcatContextCustomizers().addAll(
contextCustomizers.orderedStream().collect(Collectors.toList()));
return factory;
}

}

/**
Expand Down
50 changes: 50 additions & 0 deletions ...ework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@

import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner;
import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
import org.springframework.boot.web.embedded.tomcat.TomcatConnectorCustomizer;
import org.springframework.boot.web.embedded.tomcat.TomcatReactiveWebServerFactory;
import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory;
import org.springframework.boot.web.reactive.context.AnnotationConfigReactiveWebServerApplicationContext;
import org.springframework.boot.web.reactive.server.ConfigurableReactiveWebServerFactory;
import org.springframework.boot.web.reactive.server.ReactiveWebServerFactory;
import org.springframework.boot.web.server.WebServerFactoryCustomizer;
import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext;
import org.springframework.context.ApplicationContextException;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand All @@ -37,6 +41,7 @@
* Tests for {@link ReactiveWebServerFactoryAutoConfiguration}.
*
* @author Brian Clozel
* @author Raheela Aslam
*/
public class ReactiveWebServerFactoryAutoConfigurationTests {

Expand Down Expand Up @@ -97,6 +102,36 @@ public void defaultWebServerIsTomcat() {
.isInstanceOf(TomcatReactiveWebServerFactory.class));
}

@Test
public void tomcatConnectorCustomizerBeanIsAddedToFactory() {
WebApplicationContextRunner runner = new WebApplicationContextRunner(
AnnotationConfigServletWebServerApplicationContext::new)
.withConfiguration(AutoConfigurations
.of(ReactiveWebServerFactoryAutoConfiguration.class))
.withUserConfiguration(
TomcatConnectorCustomizerConfiguration.class);
runner.run((context) -> {
TomcatServletWebServerFactory factory = context
.getBean(TomcatServletWebServerFactory.class);
assertThat(factory.getTomcatConnectorCustomizers()).hasSize(1);
});
}

@Test
public void tomcatContextBeanIsAddedToFactory() {
WebApplicationContextRunner runner = new WebApplicationContextRunner(
AnnotationConfigServletWebServerApplicationContext::new)
.withConfiguration(AutoConfigurations
.of(ReactiveWebServerFactoryAutoConfiguration.class))
.withUserConfiguration(
TomcatContextCustomizerConfiguration.class);
runner.run((context) -> {
TomcatServletWebServerFactory factory = context
.getBean(TomcatServletWebServerFactory.class);
assertThat(factory.getTomcatContextCustomizers()).hasSize(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this test passes then this assertion isn't quite right as TomcatContextCustomizerConfiguration doesn't define a TomcatContextCustomizer. The assertions needs to be written such that if you remove .withUserConfiguration(TomcatContextCustomizerConfiguration.class) the test will fail.

Copy link
Contributor Author

@Raheela1024 Raheela1024 Dec 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertThat(factory.getTomcatContextCustomizers()).hasSize(1); is working fine in TomcatServletWebServerFactory case. If i remove .withUserConfiguration(TomcatContextCustomizerConfiguration.class) from test assertion going to fail. If we doesn't define TomcatContextCustomizer into TomcatContextCustomizerConfiguration assertion is also failed because tomcatContextCustomizers into TomcatReactiveWebServerFactory list is zero.

});
}

@Configuration
protected static class HttpHandlerConfiguration {

Expand Down Expand Up @@ -137,4 +172,19 @@ public MockReactiveWebServerFactory mockReactiveWebServerFactory() {

}

@Configuration
static class TomcatConnectorCustomizerConfiguration {

@Bean
public TomcatConnectorCustomizer connectorCustomizer() {
return (connector) -> connector.setPort(9001);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might cause a port clash if something else is used port 9001. It would be safer if the customizer didn't do anything to the connector.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

}

@Configuration
static class TomcatContextCustomizerConfiguration {

}

}
48 changes: 48 additions & 0 deletions ...amework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import org.springframework.boot.test.context.assertj.AssertableWebApplicationContext;
import org.springframework.boot.test.context.runner.ContextConsumer;
import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
import org.springframework.boot.web.embedded.tomcat.TomcatConnectorCustomizer;
import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory;
import org.springframework.boot.web.server.WebServerFactoryCustomizer;
import org.springframework.boot.web.servlet.ServletRegistrationBean;
import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext;
Expand All @@ -51,6 +53,7 @@
* @author Dave Syer
* @author Phillip Webb
* @author Stephane Nicoll
* @author Raheela Aslam
*/
public class ServletWebServerFactoryAutoConfigurationTests {

Expand Down Expand Up @@ -138,6 +141,36 @@ public void initParametersAreConfiguredOnTheServletContext() {
});
}

@Test
public void tomcatConnectorCustomizerBeanIsAddedToFactory() {
WebApplicationContextRunner runner = new WebApplicationContextRunner(
AnnotationConfigServletWebServerApplicationContext::new)
.withConfiguration(AutoConfigurations
.of(ServletWebServerFactoryAutoConfiguration.class))
.withUserConfiguration(
TomcatConnectorCustomizerConfiguration.class);
runner.run((context) -> {
TomcatServletWebServerFactory factory = context
.getBean(TomcatServletWebServerFactory.class);
assertThat(factory.getTomcatConnectorCustomizers()).hasSize(1);
});
}

@Test
public void tomcatContextCustomizerBeanIsAddedToFactory() {
WebApplicationContextRunner runner = new WebApplicationContextRunner(
AnnotationConfigServletWebServerApplicationContext::new)
.withConfiguration(AutoConfigurations
.of(ServletWebServerFactoryAutoConfiguration.class))
.withUserConfiguration(
TomcatContextCustomizerConfiguration.class);
runner.run((context) -> {
TomcatServletWebServerFactory factory = context
.getBean(TomcatServletWebServerFactory.class);
assertThat(factory.getTomcatContextCustomizers()).hasSize(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this test passes then this assertion isn't quite right as TomcatContextCustomizerConfiguration doesn't define a TomcatContextCustomizer. The assertions needs to be written such that if you remove .withUserConfiguration(TomcatContextCustomizerConfiguration.class) the test will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TomcatServletWebServerFactory already have one TomcatContextCustomizer into tomcatConnectorCustomizers list. if i remove .withUserConfiguration(TomcatContextCustomizerConfiguration.class) assertion is not failed.

We have an option that add assert for size 2 because one is already there and other is added by our configuration class. If i am thinking wrong you can guide me towards correcting the assertion.

});
}

private ContextConsumer<AssertableWebApplicationContext> verifyContext() {
return this::verifyContext;
}
Expand Down Expand Up @@ -253,4 +286,19 @@ public void customize(ConfigurableServletWebServerFactory serverFactory) {

}

@Configuration
static class TomcatConnectorCustomizerConfiguration {

@Bean
public TomcatConnectorCustomizer connectorCustomizer() {
return (connector) -> connector.setPort(9001);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might cause a port clash if something else is used port 9001. It would be safer if the customizer didn't do anything to the connector.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

}

@Configuration
static class TomcatContextCustomizerConfiguration {

}

}