Add configuration for Tomcat's cachingAllowed property

[chtompki]

This fixes #13613 by adding the ServerProperties.Tomcat entry server.tomcat.is-web-resource-caching-allowed. Which, in turn, sets on the tomcat WebResourceRoot the value isCachingAllowed. This essentially turns off the WebResource file level caching mechanics that Tomcat performs. Generally this is a configurable option, and I would like to expose it to be configurable in application.properties.

[philwebb]

This looks like a nice addition to me. I'd perhaps suggest a different property name since the is- prefix makes for a slightly unusual setter. Perhaps we should use something like server.tomcat.webresource.allow-caching? Having a dedicated webresource nested property block might also prove useful in the future if we want to expose properties for items such as setCacheTtl.

[chtompki]

Agreed on the is prefix, I was thinking server.tomcat.use-webresource-caching but forgot to add that in (thoughts there?). Don't know if I want to go with the webresource property block because its more fiddling around with the EmbeddedTomcatContext and less about web resources. I'll make the change for is -> use. Let me know if you have any more thoughts. I'm not sure if I did the integration in the "right" place, but I do know that it works.

[wilkinsona]

+1 for a webresource nested property. It reflects the type on which the property is set and gives us a grouping for other resource-related properties in the future.

We should verify the behaviour with war packaging as we already customize the resource root in that case to hide the loader classes.

[chtompki]

Ok, group consensus wins. I’ll head that direction. I’ll try to get moving on it this afternoon (UTC-4), as I’m AFK currently. And, generally many thanks for the pleasant discussion/contribution paradigm.

[chtompki]

(Pardon the build failure there, will fix that.)

I think that last commit accomplishes what we're going for here, yeah? @wilkinsona - by "verify the behaviour with war packaging," do you mean running a .war file inside a tomcat container? I'm less familiar with the mechanics of running boot as a war. If you point me in the direction of how to test that, I'll start working on it.

(Also plan to squash commits down when you guys think we're ready to merge.)

[chtompki]

@philwebb - should I go ahead and squash the commits down now?

[philwebb]

@chtompki No, don't worry about that. We'll do it when we rebase for merge.

[wilkinsona]

This is the code that we need to double-check. I think it'll be fine but want to be sure that one customisation won't trample over the other.

[chtompki]

Gotcha. I’ll poke around in there in a simple project consuming my locally built 2.1.0-SNAPSHOT to see if I can see the interplay between the two different places. Further, might it be worth putting both customizations in the same place to ensure better maintainability?

[snicoll]

When merging this one, we should make sure to update the appendix with the new property.

[chtompki]

@snicoll - is “the appendix” here: https://github.com/spring-projects/spring-boot/blob/master/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc ? If so I’ll add that into the pull request.

[snicoll]

Yes, thank you.

[chtompki]

Curious if you think that there's anything else here for me to do, while we wait in the backlog?

[wilkinsona]

@chtompki I think we're good, thank you. Unless you'd like to rename the property to allow-caching rather than use-caching that you're currently proposing. I think the former's a better match for the underlying Tomcat property that's being set. No worries if not, we can take care of it when we get round to merging this.

[snicoll]

Turns out we already had a resource section that actually was configuring the TTL cache so I've moved the property there and simplified the customizer by using the same code path.

@chtompki thank you very much for your first contribution to Spring Boot. This is now merged in master