Apply TomcatConnectorCustomizer and TomcatContextCustomizer beans automatically

[philwebb]

Hi, this is a first-timers-only issue. This means we've worked to make it more legible to folks who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.

If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open source software!

If you have contributed before, consider leaving this one for someone new, and looking through our general ideal-for-contribution issues. Thanks!

Background

Spring Boot provides a number of callback interfaces that can be used to customize the web server. For Tomcat, a TomcatConnectorCustomizer can be used to customize a Tomcat Connector and a TomcatContextCustomizer can be used to customize a Tomcat Context.

Problem

The customizers can be configured via the addConnectorCustomizers() and addContextCustomizers() methods on the ConfigurableTomcatWebServerFactory. This is bit tedious as can be seen from the example below:

@Bean
public WebServerFactoryCustomizer<TomcatServletWebServerFactory> connectorCustomizer() {
    return (tomcat) -> {
        tomcat.addConnectorCustomizers(
            (connector) -> ((AbstractHttp11Protocol<?>) connector
                .getProtocolHandler()).setProcessorCache(250)); // example connector customization
    };
}

Instead, if we applied TomcatConnectorCustomizer and TomcatContextCustomizer beans automatically, that would simplify the configuration to the following:

@Bean
public TomcatConnectorCustomizer processorCacheCustomizer() {
    return (connector) ->  (connector) -> ((AbstractHttp11Protocol<?>) connector
                .getProtocolHandler()).setProcessorCache(250)); // example connector customization
}

Solution

Beans of both the TomcatContextCustomizer and TomcatConnectorCustomizer types should be applied automatically to TomcatServletWebServerFactory and TomcatReactiveWebServerFactory. For TomcatServletWebServerFactory, this can be done here and the configuration for the reactive one can be found here.

The ObjectProvider interface can be used for injecting a dependency. Here is an example that can be used to inject customizers when there can be 0..n of them.

Tests for the servlet and reactive versions are here and here respectively.

Steps to Fix

• [x] Claim this issue with a comment below and ask any clarifying questions you need
• [ ] Set up a repository locally following the Contributing Guidelines
• [ ] Try to fix the issue following the steps above
• [ ] Commit your changes and start a pull request.

[philwebb]

See also #15035

[govi20]

I would like to work on this issue.

[mbhave]

@govi20 Sure! Feel free to ask any questions you might have. We'll look forward to your Pull Request.

[wilkinsona]

We already have a PR from you, @govi20. Thank you for that. In the spirit of first-timers only can we please carry on working with you on that and leave this issue for someone who hasn’t contributed before?

[nishantraut]

Hi @wilkinsona can i work on this?

[wilkinsona]

@nishantraut Thank you for the offer, but you have already contributed as well. This issue is intended especially for someone new.

[nishantraut]

Yes @wilkinsona that was documentation update. How can i identify and pickup small issues in code.

[wilkinsona]

@nishantraut Please look at the ideal-for-contribution label as recommended above.

[xoronet]

Hi, i would like to try it!

[wilkinsona]

Great stuff. Thank you, @xoronet. Please feel free to asking any questions you may have and we’ll look forward to your pull request.

[govi20]

I am yet to make the contribution, as per Stephane my pull request does not cover all the requirements and issue is very complex. Anyways, will work on something else.

[sadath42]

Should i create a branch from the master or should i fork the master

[wilkinsona]

@sadath42 Thanks for offering to work on this but @xoronet is already doing so. Please keep an eye out for other first-timers-only issues in the future and we'll be happy to guide you through the process once you've claimed an issue.

[wilkinsona]

How's it going, @xoronet? If you have any questions, please feel free to ask them here and we'll do our best to help.

[ihrigb]

If @xoronet is no longer available to look into this, I could take a shot. Just let me know.

[TheCloutEngineer]

Do you have any other first-timers issues?

[xoronet]

Hi @wilkinsona, fine so far. I have forked, cloned and build the project. I also digged into the topic as well as into the code. Unfortunately I had to focus on a work related project in the last days. Furthermore I had to check if my employee allows me to sign the CLA. No problems from his side. I will go back to solving the issue on Monday. Is there a date until the issue should be finished?

[wilkinsona]

Great, thank you. The issue's currently in the 2.1.x milestone and 2.1.1 is scheduled for 27 November. It would be nice, although it's not essential, for the issue to be fixed before then.

[wilkinsona]

@Ksaboor You can find other first-timer-only issues by clicking on the label. At the time of writing this is the only such open first-timers issue.

[Razi007]

can i try to contribute here now.

[xoronet]

Hi team, I am terrible sorry but I am not able to work on the topic because of too much other tasks at work. I dont find enough time, so I think it would be the best to return the issue to the crowd. I am terrible sorry for wasting your time! @philwebb @wilkinsona @Razi007

[wilkinsona]

Thanks for letting us know, @xoronet.

@ihrigb I think you were next in line. Would you like to take this one on?

[ihrigb]

@wilkinsona, yes I will take care of it.

[wilkinsona]

Sorry, @ihrigb. I've just noticed that you've already made two contributions to the project. Thank you again for those. As a first timers only issue, this issue is intended for someone who has never contributed to the project before so that we can help them to get started and find their feet.

@Ksaboor I think you're next in line. Would you like to take this one on?

[TheCloutEngineer]

Yes I'll take this on. Thanks for this opportunity!

[wilkinsona]

Great! Thank you, @Ksaboor. If you have any questions, please don't hesitate to ask here.

[ihrigb]

@wilkinsona, ok. Although these must be years back. I guess then "ideal-for-contribution" is what I rather should look for?

[wilkinsona]

@ihrigb Yes, that's right. Thank you.

[Raheela1024]

Hi, can I work on it now?

[wilkinsona]

Thank you for the offer, @Raheela1024, but @Ksaboor is working on this one.

[TheCloutEngineer]

@wilkinsona @philwebb

Sorry for the delay on working this.

• Ok, I signed the Contributor License Agreement.

• fork, clone and created a separate branch for my work "Dev branch"

• But now I'm rereading the problem but I'm still having trouble try understanding it so I could provide a solution.

• Could I get some help understanding the issue on how the two blocks of code help me?

From my understanding, I'm applying the TomcatConnectorCustomizer and TomcatContextCustomizer beans to the ServletWebServerFactoryConfiguration and ReactiveWebServerFactoryConfiguration.

Am I implementing the two blocks of code into ServletWebServerFactoryConfiguration class and the ReactiveWebServerFactoryConfiguration class?

[wilkinsona]

The two blocks of code above are an illustration of how the changes proposed here will improve things for users. In terms of implementing the proposed changes, you're right that they should be made in ServletWebServerFactoryConfiguration and ReactiveWebServerFactoryConfiguration. For example, the changes to apply connector customisers to the Servlet-based Tomcat factory will leave its bean method looking something like this:

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

[tiwarivikash]

@Ksaboor Are you still working on it?

[TheCloutEngineer]

Yes, I'm sorry I had to pause this week for interviews. I will continue working on it today.

[mbhave]

@Ksaboor Do you think you can give us an estimate on when you can submit a PR for this, please? If for some reason you aren't able to work on it by then, we can give someone else a chance.

[TheCloutEngineer]

yeah sure, I'm working on it right now I should have a PR by Tuesday of this week. If not I'll happily give it away to the next person. :)

[TheCloutEngineer]

Hey sorry but with the stress of job hunting and studying technical challenges. I'm going to have step away from this issue.

[tiwarivikash]

Can I work on it?

[wilkinsona]

@Ksaboor Thanks for letting us know. Best of luck with your job hunting.

Thanks for the offer, @tiwarivikash, but, looking back through the comments I think @Raheela1024 is next in line. @Raheela1024 are you still interested in working on this?

[Raheela1024]

@wilkinsona Yes i will work on it. Thanks for this opportunity!

[Razi007]

@wilkinsona I guess i raised request before @Raheela1024.But no issue @Raheela1024 please continue.

[Raheela1024]

@wilkinsonaI I have changed in ServletWebServerFactoryConfiguration.java and ReactiveWebServerFactoryConfiguration.java files. Below change is for ServletWebServerFactoryConfiguration.java but i need to know am in correct direction ?. After changing that do I need to update test cases in ServletWebServerFactoryAutoConfigurationTests.java and ReactiveWebServerFactoryAutoConfigurationTests.java ?. There is any other area which I missed ?

ServletWebServerFactoryConfiguration.java

@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;
}

[wilkinsona]

@Raheela1024 Yes, that's exactly the right direction. Thank you.

It would be good to add 4 tests that verify that connector customiser and context customiser beans are added to the reactive and servlet factories. A test that verifies that a connector customiser bean is added to the servlet factory would like something like this:

@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);
    });
}

@Configuration
static class TomcatConnectorCustomizerConfiguration {

    @Bean
    public TomcatConnectorCustomizer connectorCustomizer() {
        return (connector) -> {
        };
    }

}

[Raheela1024]

@wilkinsona I have created the PR Please review and let me know if missed something or doing wrongly.

[wilkinsona]

Awesome! Thank you very much, @Raheela1024. I'll close this issue in favour of your PR.