Merge pull request #16039 from amcghie

* pr/16039:
  Polish "Complete support for customizing Tomcat's access log"
  Complete support for customizing Tomcat's access log

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java

@@ -56,6 +56,7 @@ import org.springframework.util.unit.DataSize;
  * @author Olivier Lamy
  * @author Chentao Qu
  * @author Artsiom Yudovin
+ * @author Andrew McGhie
  */
 @ConfigurationProperties(prefix = "server", ignoreUnknownFields = true)
 public class ServerProperties {
@@ -545,6 +546,18 @@ public class ServerProperties {
 			 */
 			private boolean enabled = false;
 
+			/**
+			 * Whether logging of the request will only be enabled if
+			 * "ServletRequest.getAttribute(conditionIf)" does not yield null.
+			 */
+			private String conditionIf;
+
+			/**
+			 * Whether logging of the request will only be enabled if
+			 * "ServletRequest.getAttribute(conditionUnless)" yield null.
+			 */
+			private String conditionUnless;
+
 			/**
 			 * Format pattern for access logs.
 			 */
@@ -566,6 +579,24 @@ public class ServerProperties {
 			 */
 			private String suffix = ".log";
 
+			/**
+			 * Character set used by the log file. Default to the system default character
+			 * set.
+			 */
+			private String encoding;
+
+			/**
+			 * Locale used to format timestamps in log entries and in log file name
+			 * suffix. Default to the default locale of the Java process.
+			 */
+			private String locale;
+
+			/**
+			 * Whether to check for log file existence so it can be recreated it if an
+			 * external process has renamed it.
+			 */
+			private boolean checkExists = false;
+
 			/**
 			 * Whether to enable access log rotation.
 			 */
@@ -587,6 +618,11 @@ public class ServerProperties {
 			 */
 			private String fileDateFormat = ".yyyy-MM-dd";
 
+			/**
+			 * Whether to use IPv6 canonical representation format as defined by RFC 5952.
+			 */
+			private boolean ipv6Canonical = false;
+
 			/**
 			 * Set request attributes for the IP address, Hostname, protocol, and port
 			 * used for the request.
@@ -606,6 +642,22 @@ public class ServerProperties {
 				this.enabled = enabled;
 			}
 
+			public String getConditionIf() {
+				return this.conditionIf;
+			}
+
+			public void setConditionIf(String conditionIf) {
+				this.conditionIf = conditionIf;
+			}
+
+			public String getConditionUnless() {
+				return this.conditionUnless;
+			}
+
+			public void setConditionUnless(String conditionUnless) {
+				this.conditionUnless = conditionUnless;
+			}
+
 			public String getPattern() {
 				return this.pattern;
 			}
@@ -638,6 +690,30 @@ public class ServerProperties {
 				this.suffix = suffix;
 			}
 
+			public String getEncoding() {
+				return this.encoding;
+			}
+
+			public void setEncoding(String encoding) {
+				this.encoding = encoding;
+			}
+
+			public String getLocale() {
+				return this.locale;
+			}
+
+			public void setLocale(String locale) {
+				this.locale = locale;
+			}
+
+			public boolean isCheckExists() {
+				return this.checkExists;
+			}
+
+			public void setCheckExists(boolean checkExists) {
+				this.checkExists = checkExists;
+			}
+
 			public boolean isRotate() {
 				return this.rotate;
 			}
@@ -670,6 +746,14 @@ public class ServerProperties {
 				this.fileDateFormat = fileDateFormat;
 			}
 
+			public boolean isIpv6Canonical() {
+				return this.ipv6Canonical;
+			}
+
+			public void setIpv6Canonical(boolean ipv6Canonical) {
+				this.ipv6Canonical = ipv6Canonical;
+			}
+
 			public boolean isRequestAttributesEnabled() {
 				return this.requestAttributesEnabled;
 			}

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java

@@ -30,6 +30,7 @@ import org.springframework.boot.autoconfigure.web.ErrorProperties;
 import org.springframework.boot.autoconfigure.web.ErrorProperties.IncludeStacktrace;
 import org.springframework.boot.autoconfigure.web.ServerProperties;
 import org.springframework.boot.autoconfigure.web.ServerProperties.Tomcat;
+import org.springframework.boot.autoconfigure.web.ServerProperties.Tomcat.Accesslog;
 import org.springframework.boot.cloud.CloudPlatform;
 import org.springframework.boot.context.properties.PropertyMapper;
 import org.springframework.boot.web.embedded.tomcat.ConfigurableTomcatWebServerFactory;
@@ -49,6 +50,7 @@ import org.springframework.util.unit.DataSize;
  * @author Phillip Webb
  * @author Artsiom Yudovin
  * @author Chentao Qu
+ * @author Andrew McGhie
  * @since 2.0.0
  */
 public class TomcatWebServerFactoryCustomizer implements
@@ -246,17 +248,25 @@ public class TomcatWebServerFactoryCustomizer implements
 	private void customizeAccessLog(ConfigurableTomcatWebServerFactory factory) {
 		ServerProperties.Tomcat tomcatProperties = this.serverProperties.getTomcat();
 		AccessLogValve valve = new AccessLogValve();
-		valve.setPattern(tomcatProperties.getAccesslog().getPattern());
-		valve.setDirectory(tomcatProperties.getAccesslog().getDirectory());
-		valve.setPrefix(tomcatProperties.getAccesslog().getPrefix());
-		valve.setSuffix(tomcatProperties.getAccesslog().getSuffix());
-		valve.setRenameOnRotate(tomcatProperties.getAccesslog().isRenameOnRotate());
-		valve.setMaxDays(tomcatProperties.getAccesslog().getMaxDays());
-		valve.setFileDateFormat(tomcatProperties.getAccesslog().getFileDateFormat());
-		valve.setRequestAttributesEnabled(
-				tomcatProperties.getAccesslog().isRequestAttributesEnabled());
-		valve.setRotatable(tomcatProperties.getAccesslog().isRotate());
-		valve.setBuffered(tomcatProperties.getAccesslog().isBuffered());
+		PropertyMapper map = PropertyMapper.get();
+		Accesslog accessLogConfig = tomcatProperties.getAccesslog();
+		map.from(accessLogConfig.getConditionIf()).to(valve::setConditionIf);
+		map.from(accessLogConfig.getConditionUnless()).to(valve::setConditionUnless);
+		map.from(accessLogConfig.getPattern()).to(valve::setPattern);
+		map.from(accessLogConfig.getDirectory()).to(valve::setDirectory);
+		map.from(accessLogConfig.getPrefix()).to(valve::setPrefix);
+		map.from(accessLogConfig.getSuffix()).to(valve::setSuffix);
+		map.from(accessLogConfig.getEncoding()).whenHasText().to(valve::setEncoding);
+		map.from(accessLogConfig.getLocale()).whenHasText().to(valve::setLocale);
+		map.from(accessLogConfig.isCheckExists()).to(valve::setCheckExists);
+		map.from(accessLogConfig.isRotate()).to(valve::setRotatable);
+		map.from(accessLogConfig.isRenameOnRotate()).to(valve::setRenameOnRotate);
+		map.from(accessLogConfig.getMaxDays()).to(valve::setMaxDays);
+		map.from(accessLogConfig.getFileDateFormat()).to(valve::setFileDateFormat);
+		map.from(accessLogConfig.isIpv6Canonical()).to(valve::setIpv6Canonical);
+		map.from(accessLogConfig.isRequestAttributesEnabled())
+				.to(valve::setRequestAttributesEnabled);
+		map.from(accessLogConfig.isBuffered()).to(valve::setBuffered);
 		factory.addEngineValves(valve);
 	}
 

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java

@@ -42,6 +42,7 @@ import org.eclipse.jetty.server.HttpChannel;
 import org.eclipse.jetty.server.Request;
 import org.junit.Test;
 
+import org.springframework.boot.autoconfigure.web.ServerProperties.Tomcat.Accesslog;
 import org.springframework.boot.context.properties.bind.Bindable;
 import org.springframework.boot.context.properties.bind.Binder;
 import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
@@ -72,6 +73,7 @@ import static org.assertj.core.api.Assertions.assertThat;
  * @author Eddú Meléndez
  * @author Quinten De Swaef
  * @author Venil Noronha
+ * @author Andrew McGhie
  */
 public class ServerPropertiesTests {
 
@@ -111,24 +113,37 @@ public class ServerPropertiesTests {
 	@Test
 	public void testTomcatBinding() {
 		Map<String, String> map = new HashMap<>();
+		map.put("server.tomcat.accesslog.conditionIf", "foo");
+		map.put("server.tomcat.accesslog.conditionUnless", "bar");
 		map.put("server.tomcat.accesslog.pattern", "%h %t '%r' %s %b");
 		map.put("server.tomcat.accesslog.prefix", "foo");
+		map.put("server.tomcat.accesslog.suffix", "-bar.log");
+		map.put("server.tomcat.accesslog.encoding", "UTF-8");
+		map.put("server.tomcat.accesslog.locale", "en-AU");
+		map.put("server.tomcat.accesslog.checkExists", "true");
 		map.put("server.tomcat.accesslog.rotate", "false");
 		map.put("server.tomcat.accesslog.rename-on-rotate", "true");
+		map.put("server.tomcat.accesslog.ipv6Canonical", "true");
 		map.put("server.tomcat.accesslog.request-attributes-enabled", "true");
-		map.put("server.tomcat.accesslog.suffix", "-bar.log");
 		map.put("server.tomcat.protocol-header", "X-Forwarded-Protocol");
 		map.put("server.tomcat.remote-ip-header", "Remote-Ip");
 		map.put("server.tomcat.internal-proxies", "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}");
 		map.put("server.tomcat.background-processor-delay", "10");
 		bind(map);
 		ServerProperties.Tomcat tomcat = this.properties.getTomcat();
-		assertThat(tomcat.getAccesslog().getPattern()).isEqualTo("%h %t '%r' %s %b");
-		assertThat(tomcat.getAccesslog().getPrefix()).isEqualTo("foo");
-		assertThat(tomcat.getAccesslog().isRotate()).isFalse();
-		assertThat(tomcat.getAccesslog().isRenameOnRotate()).isTrue();
-		assertThat(tomcat.getAccesslog().isRequestAttributesEnabled()).isTrue();
-		assertThat(tomcat.getAccesslog().getSuffix()).isEqualTo("-bar.log");
+		Accesslog accesslog = tomcat.getAccesslog();
+		assertThat(accesslog.getConditionIf()).isEqualTo("foo");
+		assertThat(accesslog.getConditionUnless()).isEqualTo("bar");
+		assertThat(accesslog.getPattern()).isEqualTo("%h %t '%r' %s %b");
+		assertThat(accesslog.getPrefix()).isEqualTo("foo");
+		assertThat(accesslog.getSuffix()).isEqualTo("-bar.log");
+		assertThat(accesslog.getEncoding()).isEqualTo("UTF-8");
+		assertThat(accesslog.getLocale()).isEqualTo("en-AU");
+		assertThat(accesslog.isCheckExists()).isEqualTo(true);
+		assertThat(accesslog.isRotate()).isFalse();
+		assertThat(accesslog.isRenameOnRotate()).isTrue();
+		assertThat(accesslog.isIpv6Canonical()).isTrue();
+		assertThat(accesslog.isRequestAttributesEnabled()).isTrue();
 		assertThat(tomcat.getRemoteIpHeader()).isEqualTo("Remote-Ip");
 		assertThat(tomcat.getProtocolHeader()).isEqualTo("X-Forwarded-Protocol");
 		assertThat(tomcat.getInternalProxies())

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.boot.autoconfigure.web.embedded;
 
+import java.util.Locale;
 import java.util.function.Consumer;
 
 import org.apache.catalina.Context;
@@ -49,6 +50,7 @@ import static org.assertj.core.api.Assertions.assertThat;
  * @author Rob Tompkins
  * @author Artsiom Yudovin
  * @author Stephane Nicoll
+ * @author Andrew McGhie
  */
 public class TomcatWebServerFactoryCustomizerTests {
 
@@ -324,6 +326,75 @@ public class TomcatWebServerFactoryCustomizerTests {
 						this.serverProperties.getTomcat().getAccesslog().getMaxDays());
 	}
 
+	@Test
+	public void accessLogConditionCanBeSpecified() {
+		bind("server.tomcat.accesslog.enabled=true",
+				"server.tomcat.accesslog.conditionIf=foo",
+				"server.tomcat.accesslog.conditionUnless=bar");
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.getConditionIf()).isEqualTo("foo");
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.getConditionUnless()).isEqualTo("bar");
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.getCondition()).describedAs(
+						"value of condition should equal conditionUnless - provided for backwards compatibility")
+						.isEqualTo("bar");
+	}
+
+	@Test
+	public void accessLogEncodingIsNullWhenNotSpecified() {
+		bind("server.tomcat.accesslog.enabled=true");
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.getEncoding()).isNull();
+	}
+
+	@Test
+	public void accessLogEncodingCanBeSpecified() {
+		bind("server.tomcat.accesslog.enabled=true",
+				"server.tomcat.accesslog.encoding=UTF-8");
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.getEncoding()).isEqualTo("UTF-8");
+	}
+
+	@Test
+	public void accessLogWithDefaultLocale() {
+		bind("server.tomcat.accesslog.enabled=true");
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.getLocale()).isEqualTo(Locale.getDefault().toString());
+	}
+
+	@Test
+	public void accessLogLocaleCanBeSpecified() {
+		String locale = "en_AU".equals(Locale.getDefault().toString()) ? "en_US"
+				: "en_AU";
+		bind("server.tomcat.accesslog.enabled=true",
+				"server.tomcat.accesslog.locale=" + locale);
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.getLocale()).isEqualTo(locale);
+	}
+
+	@Test
+	public void accessLogCheckExistsDefault() {
+		bind("server.tomcat.accesslog.enabled=true");
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.isCheckExists()).isFalse();
+	}
+
+	@Test
+	public void accessLogCheckExistsSpecified() {
+		bind("server.tomcat.accesslog.enabled=true",
+				"server.tomcat.accesslog.check-exists=true");
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.isCheckExists()).isTrue();
+	}
+
 	@Test
 	public void accessLogMaxDaysCanBeRedefined() {
 		bind("server.tomcat.accesslog.enabled=true",
@@ -333,6 +404,23 @@ public class TomcatWebServerFactoryCustomizerTests {
 				.getMaxDays()).isEqualTo(20);
 	}
 
+	@Test
+	public void accessLogDoesNotUseIpv6CanonicalFormatByDefault() {
+		bind("server.tomcat.accesslog.enabled=true");
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.getIpv6Canonical()).isFalse();
+	}
+
+	@Test
+	public void accessLogwithIpv6CanonicalSet() {
+		bind("server.tomcat.accesslog.enabled=true",
+				"server.tomcat.accesslog.ipv6-canonical=true");
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
+				.getIpv6Canonical()).isTrue();
+	}
+
 	private void bind(String... inlinedProperties) {
 		TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.environment,
 				inlinedProperties);