From 3f4c32fcddec094787e356238d48adf9e9a41a7e Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Dec 2016 11:27:57 -0800 Subject: [PATCH 01/16] Polish --- .../OAuth2RestOperationsConfiguration.java | 61 +++++++------------ .../oauth2/OAuth2AutoConfigurationTests.java | 23 +++---- 2 files changed, 32 insertions(+), 52 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfiguration.java index 55e30f672e..86866d662f 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/client/OAuth2RestOperationsConfiguration.java @@ -16,12 +16,6 @@ package org.springframework.boot.autoconfigure.security.oauth2.client; -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - import javax.annotation.Resource; import org.springframework.beans.factory.annotation.Qualifier; @@ -76,7 +70,7 @@ import org.springframework.util.StringUtils; public class OAuth2RestOperationsConfiguration { @Configuration - @ConditionalOnClientCredentials + @Conditional(ClientCredentialsCondition.class) protected static class SingletonScopedConfiguration { @Bean @@ -96,7 +90,7 @@ public class OAuth2RestOperationsConfiguration { @Configuration @ConditionalOnBean(OAuth2ClientConfiguration.class) - @ConditionalOnNotClientCredentials + @Conditional(NoClientCredentialsCondition.class) @Import(OAuth2ProtectedResourceDetailsConfiguration.class) protected static class SessionScopedConfiguration { @@ -126,15 +120,13 @@ public class OAuth2RestOperationsConfiguration { } - /* - * When the authentication is per cookie but the stored token is an oauth2 one, we can - * pass that on to a client that wants to call downstream. We don't even need an - * OAuth2ClientContextFilter until we need to refresh the access token. To handle - * refresh tokens you need to {@code @EnableOAuth2Client} - */ + // When the authentication is per cookie but the stored token is an oauth2 one, we can + // pass that on to a client that wants to call downstream. We don't even need an + // OAuth2ClientContextFilter until we need to refresh the access token. To handle + // refresh tokens you need to @EnableOAuth2Client @Configuration @ConditionalOnMissingBean(OAuth2ClientConfiguration.class) - @ConditionalOnNotClientCredentials + @Conditional(NoClientCredentialsCondition.class) @Import(OAuth2ProtectedResourceDetailsConfiguration.class) protected static class RequestScopedConfiguration { @@ -182,22 +174,24 @@ public class OAuth2RestOperationsConfiguration { } - @Conditional(ClientCredentialsCondition.class) - @Target({ ElementType.TYPE, ElementType.METHOD }) - @Retention(RetentionPolicy.RUNTIME) - @Documented - public static @interface ConditionalOnClientCredentials { - - } - - @Conditional(NotClientCredentialsCondition.class) - @Target({ ElementType.TYPE, ElementType.METHOD }) - @Retention(RetentionPolicy.RUNTIME) - @Documented - public static @interface ConditionalOnNotClientCredentials { + /** + * Condition to check for no client credentials. + */ + static class NoClientCredentialsCondition extends NoneNestedConditions { + + NoClientCredentialsCondition() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @Conditional(ClientCredentialsCondition.class) + static class ClientCredentialsActivated { + } } + /** + * Condition to check for client credentials. + */ static class ClientCredentialsCondition extends AnyNestedCondition { ClientCredentialsCondition() { @@ -211,17 +205,6 @@ public class OAuth2RestOperationsConfiguration { @ConditionalOnNotWebApplication static class NoWebApplication { } - } - - static class NotClientCredentialsCondition extends NoneNestedConditions { - - NotClientCredentialsCondition() { - super(ConfigurationPhase.PARSE_CONFIGURATION); - } - - @ConditionalOnClientCredentials - static class ClientCredentialsActivated { - } } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/OAuth2AutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/OAuth2AutoConfigurationTests.java index 750f009b5f..22bad3fbcc 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/OAuth2AutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/oauth2/OAuth2AutoConfigurationTests.java @@ -21,7 +21,6 @@ import java.util.Arrays; import java.util.List; import com.fasterxml.jackson.databind.JsonNode; - import org.junit.Test; import org.springframework.aop.support.AopUtils; @@ -196,8 +195,8 @@ public class OAuth2AutoConfigurationTests { "security.oauth2.client.clientId=client", "security.oauth2.client.grantType=client_credentials"); this.context.refresh(); - assertThat(this.context.getBean(OAuth2ClientContext.class).getAccessTokenRequest()) - .isNotNull(); + OAuth2ClientContext bean = this.context.getBean(OAuth2ClientContext.class); + assertThat(bean.getAccessTokenRequest()).isNotNull(); assertThat(countBeans(ClientCredentialsResourceDetails.class)).isEqualTo(1); assertThat(countBeans(OAuth2ClientContext.class)).isEqualTo(1); } @@ -211,17 +210,15 @@ public class OAuth2AutoConfigurationTests { "security.oauth2.client.clientId=client", "security.oauth2.client.grantType=client_credentials"); this.context.refresh(); - // Thr primary context is fine (not session scoped): - assertThat(this.context.getBean(OAuth2ClientContext.class).getAccessTokenRequest()) - .isNotNull(); + // The primary context is fine (not session scoped): + OAuth2ClientContext bean = this.context.getBean(OAuth2ClientContext.class); + assertThat(bean.getAccessTokenRequest()).isNotNull(); assertThat(countBeans(ClientCredentialsResourceDetails.class)).isEqualTo(1); - /* - * Kind of a bug (should ideally be 1), but the cause is in Spring OAuth2 (there - * is no need for the extra session-scoped bean). What this test proves is that - * even if the user screws up and does @EnableOAuth2Client for client credentials, - * it will still just about work (because of the @Primary annotation on the - * Boot-created instance of OAuth2ClientContext). - */ + // Kind of a bug (should ideally be 1), but the cause is in Spring OAuth2 (there + // is no need for the extra session-scoped bean). What this test proves is that + // even if the user screws up and does @EnableOAuth2Client for client credentials, + // it will still just about work (because of the @Primary annotation on the + // Boot-created instance of OAuth2ClientContext). assertThat(countBeans(OAuth2ClientContext.class)).isEqualTo(2); } From 1fc2e870530af15301d3e69b3dd506f606a3c177 Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Wed, 5 Oct 2016 23:50:42 +1300 Subject: [PATCH 02/16] Enable custom Reservoir with Dropwizard metrics Uses the ReservoirFactory to customize the implementation of the Reservoir that will be used when creating Timer and Histogram in the DropwizardMetricServices. Fixes gh-5199 Closes gh-7105 --- .../MetricsDropwizardAutoConfiguration.java | 12 ++- .../dropwizard/DropwizardMetricServices.java | 61 ++++++++++++- .../metrics/dropwizard/ReservoirFactory.java | 39 ++++++++ ...tricsDropwizardAutoConfigurationTests.java | 89 +++++++++++++++++++ .../DropwizardMetricServicesTests.java | 44 +++++++++ 5 files changed, 242 insertions(+), 3 deletions(-) create mode 100644 spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java create mode 100644 spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java index ecd2e99ef0..577b21a87c 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java @@ -18,10 +18,12 @@ package org.springframework.boot.actuate.autoconfigure; import com.codahale.metrics.MetricRegistry; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.MetricReaderPublicMetrics; import org.springframework.boot.actuate.metrics.CounterService; import org.springframework.boot.actuate.metrics.GaugeService; import org.springframework.boot.actuate.metrics.dropwizard.DropwizardMetricServices; +import org.springframework.boot.actuate.metrics.dropwizard.ReservoirFactory; import org.springframework.boot.actuate.metrics.reader.MetricRegistryMetricReader; import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -41,6 +43,9 @@ import org.springframework.context.annotation.Configuration; @AutoConfigureBefore(MetricRepositoryAutoConfiguration.class) public class MetricsDropwizardAutoConfiguration { + @Autowired(required = false) + private ReservoirFactory reservoirFactory; + @Bean @ConditionalOnMissingBean public MetricRegistry metricRegistry() { @@ -52,7 +57,12 @@ public class MetricsDropwizardAutoConfiguration { GaugeService.class }) public DropwizardMetricServices dropwizardMetricServices( MetricRegistry metricRegistry) { - return new DropwizardMetricServices(metricRegistry); + if (this.reservoirFactory == null) { + return new DropwizardMetricServices(metricRegistry); + } + else { + return new DropwizardMetricServices(metricRegistry, this.reservoirFactory); + } } @Bean diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java index 93a6a16ee9..846c51ff48 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java @@ -24,7 +24,9 @@ import com.codahale.metrics.Counter; import com.codahale.metrics.Gauge; import com.codahale.metrics.Histogram; import com.codahale.metrics.Meter; +import com.codahale.metrics.Metric; import com.codahale.metrics.MetricRegistry; +import com.codahale.metrics.Reservoir; import com.codahale.metrics.Timer; import org.springframework.boot.actuate.metrics.CounterService; @@ -53,6 +55,8 @@ public class DropwizardMetricServices implements CounterService, GaugeService { private final MetricRegistry registry; + private ReservoirFactory reservoirFactory; + private final ConcurrentMap gauges = new ConcurrentHashMap(); private final ConcurrentHashMap names = new ConcurrentHashMap(); @@ -65,6 +69,18 @@ public class DropwizardMetricServices implements CounterService, GaugeService { this.registry = registry; } + /** + * Create a new {@link DropwizardMetricServices} instance. + * @param registry the underlying metric registry + * @param reservoirFactory the factory that instantiates the {@link Reservoir} that + * will be used on Timers and Histograms + */ + public DropwizardMetricServices(MetricRegistry registry, + ReservoirFactory reservoirFactory) { + this.registry = registry; + this.reservoirFactory = reservoirFactory; + } + @Override public void increment(String name) { incrementInternal(name, 1L); @@ -91,12 +107,12 @@ public class DropwizardMetricServices implements CounterService, GaugeService { public void submit(String name, double value) { if (name.startsWith("histogram")) { long longValue = (long) value; - Histogram metric = this.registry.histogram(name); + Histogram metric = registerHistogram(name); metric.update(longValue); } else if (name.startsWith("timer")) { long longValue = (long) value; - Timer metric = this.registry.timer(name); + Timer metric = registerTimer(name); metric.update(longValue, TimeUnit.MILLISECONDS); } else { @@ -105,6 +121,43 @@ public class DropwizardMetricServices implements CounterService, GaugeService { } } + private Histogram registerHistogram(String name) { + if (this.reservoirFactory == null) { + return this.registry.histogram(name); + } + else { + Histogram histogram = new Histogram(this.reservoirFactory.getObject()); + return getOrAddMetric(name, histogram); + } + } + + private Timer registerTimer(String name) { + if (this.reservoirFactory == null) { + return this.registry.timer(name); + } + else { + Timer timer = new Timer(this.reservoirFactory.getObject()); + return getOrAddMetric(name, timer); + } + } + + @SuppressWarnings("unchecked") + private T getOrAddMetric(String name, T newMetric) { + Metric metric = this.registry.getMetrics().get(name); + if (metric == null) { + return this.registry.register(name, newMetric); + } + else { + if (metric.getClass().equals(newMetric.getClass())) { + return (T) metric; + } + else { + throw new IllegalArgumentException( + name + " is already used for a different type of metric"); + } + } + } + private void setGaugeValue(String name, double value) { // NOTE: Dropwizard provides no way to do this atomically SimpleGauge gauge = this.gauges.get(name); @@ -148,6 +201,10 @@ public class DropwizardMetricServices implements CounterService, GaugeService { this.registry.remove(name); } + void setReservoirFactory(ReservoirFactory reservoirFactory) { + this.reservoirFactory = reservoirFactory; + } + /** * Simple {@link Gauge} implementation to {@literal double} value. */ diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java new file mode 100644 index 0000000000..b1de0bb628 --- /dev/null +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java @@ -0,0 +1,39 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.metrics.dropwizard; + +import com.codahale.metrics.Reservoir; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.ObjectFactory; + +/** + * A {@link Reservoir} factory to instantiate the Reservoir that will be set as default + * for the {@link DropwizardMetricServices}. + * The Reservoir instances can't be shared across {@link com.codahale.metrics.Metric}. + * + * @author Lucas Saldanha + */ +public abstract class ReservoirFactory implements ObjectFactory { + + protected abstract Reservoir defaultReservoir(); + + @Override + public Reservoir getObject() throws BeansException { + return defaultReservoir(); + } +} diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java new file mode 100644 index 0000000000..5f032deea6 --- /dev/null +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java @@ -0,0 +1,89 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.autoconfigure; + +import com.codahale.metrics.Reservoir; +import com.codahale.metrics.UniformReservoir; + +import org.junit.After; +import org.junit.Test; + +import org.springframework.boot.actuate.metrics.dropwizard.DropwizardMetricServices; +import org.springframework.boot.actuate.metrics.dropwizard.ReservoirFactory; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +import org.springframework.test.util.ReflectionTestUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link MetricsDropwizardAutoConfiguration}. + * + * @author Lucas Saldanha + */ +public class MetricsDropwizardAutoConfigurationTests { + + private AnnotationConfigApplicationContext context; + + @After + public void after() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + public void dropwizardWithoutCustomReservoirConfigured() { + this.context = new AnnotationConfigApplicationContext( + MetricsDropwizardAutoConfiguration.class); + + DropwizardMetricServices dropwizardMetricServices = this.context + .getBean(DropwizardMetricServices.class); + + assertThat(ReflectionTestUtils.getField(dropwizardMetricServices, "reservoirFactory")) + .isNull(); + } + + @Test + public void dropwizardWithCustomReservoirConfigured() { + this.context = new AnnotationConfigApplicationContext( + MetricsDropwizardAutoConfiguration.class, Config.class); + + DropwizardMetricServices dropwizardMetricServices = this.context + .getBean(DropwizardMetricServices.class); + + assertThat(ReflectionTestUtils.getField(dropwizardMetricServices, "reservoirFactory")) + .isNotNull(); + } + + @Configuration + static class Config { + + @Bean + public ReservoirFactory reservoirFactory() { + return new ReservoirFactory() { + @Override + protected Reservoir defaultReservoir() { + return new UniformReservoir(); + } + }; + } + } + +} diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java index ec745b1646..04f4c1d65a 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java @@ -20,15 +20,22 @@ import java.util.ArrayList; import java.util.List; import com.codahale.metrics.Gauge; +import com.codahale.metrics.Histogram; import com.codahale.metrics.MetricRegistry; +import com.codahale.metrics.Reservoir; +import com.codahale.metrics.Timer; +import com.codahale.metrics.UniformReservoir; import org.junit.Test; +import org.springframework.test.util.ReflectionTestUtils; + import static org.assertj.core.api.Assertions.assertThat; /** * Tests for {@link DropwizardMetricServices}. * * @author Dave Syer + * @author Lucas Saldanha */ public class DropwizardMetricServicesTests { @@ -78,6 +85,26 @@ public class DropwizardMetricServicesTests { assertThat(this.registry.timer("timer.foo").getCount()).isEqualTo(2); } + @Test + public void setCustomReservoirTimer() { + this.writer.setReservoirFactory(new ReservoirFactory() { + @Override + protected Reservoir defaultReservoir() { + return new UniformReservoir(); + } + }); + + this.writer.submit("timer.foo", 200); + this.writer.submit("timer.foo", 300); + assertThat(this.registry.timer("timer.foo").getCount()).isEqualTo(2); + + Timer timer = (Timer) this.registry.getMetrics().get("timer.foo"); + Histogram histogram = (Histogram) ReflectionTestUtils + .getField(timer, "histogram"); + assertThat(ReflectionTestUtils.getField(histogram, "reservoir").getClass() + .equals(UniformReservoir.class)).isTrue(); + } + @Test public void setPredefinedHistogram() { this.writer.submit("histogram.foo", 2.1); @@ -85,6 +112,23 @@ public class DropwizardMetricServicesTests { assertThat(this.registry.histogram("histogram.foo").getCount()).isEqualTo(2); } + @Test + public void setCustomReservoirHistogram() { + this.writer.setReservoirFactory(new ReservoirFactory() { + @Override + protected Reservoir defaultReservoir() { + return new UniformReservoir(); + } + }); + + this.writer.submit("histogram.foo", 2.1); + this.writer.submit("histogram.foo", 2.3); + assertThat(this.registry.histogram("histogram.foo").getCount()).isEqualTo(2); + assertThat(ReflectionTestUtils + .getField(this.registry.getMetrics().get("histogram.foo"), "reservoir") + .getClass().equals(UniformReservoir.class)).isTrue(); + } + /** * Test the case where a given writer is used amongst several threads where each * thread is updating the same set of metrics. This would be an example case of the From 06a7ab0cd5f3036e7ece4edf7c80428874afc362 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Dec 2016 13:52:53 -0800 Subject: [PATCH 03/16] Polish ReservoirFactory support Polish Dropwizrd reservoir support including a refactor of `ReservoirFactory` to allow reservoirs to be created based on a metric name. See gh-5199 See gh-7105 --- .../MetricsDropwizardAutoConfiguration.java | 10 +- .../dropwizard/DropwizardMetricServices.java | 133 ++++++++++++------ .../metrics/dropwizard/ReservoirFactory.java | 36 +++-- ...tricsDropwizardAutoConfigurationTests.java | 34 ++--- .../DropwizardMetricServicesTests.java | 43 +++--- 5 files changed, 162 insertions(+), 94 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java index 577b21a87c..a53bc9db45 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java @@ -18,7 +18,7 @@ package org.springframework.boot.actuate.autoconfigure; import com.codahale.metrics.MetricRegistry; -import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.actuate.endpoint.MetricReaderPublicMetrics; import org.springframework.boot.actuate.metrics.CounterService; import org.springframework.boot.actuate.metrics.GaugeService; @@ -43,8 +43,12 @@ import org.springframework.context.annotation.Configuration; @AutoConfigureBefore(MetricRepositoryAutoConfiguration.class) public class MetricsDropwizardAutoConfiguration { - @Autowired(required = false) - private ReservoirFactory reservoirFactory; + private final ReservoirFactory reservoirFactory; + + public MetricsDropwizardAutoConfiguration( + ObjectProvider reservoirFactory) { + this.reservoirFactory = reservoirFactory.getIfAvailable(); + } @Bean @ConditionalOnMissingBean diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java index 846c51ff48..98a70ad8cc 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java @@ -31,6 +31,8 @@ import com.codahale.metrics.Timer; import org.springframework.boot.actuate.metrics.CounterService; import org.springframework.boot.actuate.metrics.GaugeService; +import org.springframework.core.ResolvableType; +import org.springframework.util.Assert; /** * A {@link GaugeService} and {@link CounterService} that sends data to a Dropwizard @@ -55,7 +57,7 @@ public class DropwizardMetricServices implements CounterService, GaugeService { private final MetricRegistry registry; - private ReservoirFactory reservoirFactory; + private final ReservoirFactory reservoirFactory; private final ConcurrentMap gauges = new ConcurrentHashMap(); @@ -66,19 +68,20 @@ public class DropwizardMetricServices implements CounterService, GaugeService { * @param registry the underlying metric registry */ public DropwizardMetricServices(MetricRegistry registry) { - this.registry = registry; + this(registry, null); } /** * Create a new {@link DropwizardMetricServices} instance. * @param registry the underlying metric registry * @param reservoirFactory the factory that instantiates the {@link Reservoir} that - * will be used on Timers and Histograms + * will be used on Timers and Histograms */ public DropwizardMetricServices(MetricRegistry registry, ReservoirFactory reservoirFactory) { this.registry = registry; - this.reservoirFactory = reservoirFactory; + this.reservoirFactory = (reservoirFactory == null ? ReservoirFactory.NONE + : reservoirFactory); } @Override @@ -106,14 +109,10 @@ public class DropwizardMetricServices implements CounterService, GaugeService { @Override public void submit(String name, double value) { if (name.startsWith("histogram")) { - long longValue = (long) value; - Histogram metric = registerHistogram(name); - metric.update(longValue); + submitHistogram(name, value); } else if (name.startsWith("timer")) { - long longValue = (long) value; - Timer metric = registerTimer(name); - metric.update(longValue, TimeUnit.MILLISECONDS); + submitTimer(name, value); } else { name = wrapGaugeName(name); @@ -121,40 +120,36 @@ public class DropwizardMetricServices implements CounterService, GaugeService { } } - private Histogram registerHistogram(String name) { - if (this.reservoirFactory == null) { - return this.registry.histogram(name); - } - else { - Histogram histogram = new Histogram(this.reservoirFactory.getObject()); - return getOrAddMetric(name, histogram); - } + private void submitTimer(String name, double value) { + long longValue = (long) value; + Timer metric = register(name, new TimerMetricRegistrar()); + metric.update(longValue, TimeUnit.MILLISECONDS); } - private Timer registerTimer(String name) { - if (this.reservoirFactory == null) { - return this.registry.timer(name); - } - else { - Timer timer = new Timer(this.reservoirFactory.getObject()); - return getOrAddMetric(name, timer); - } + private void submitHistogram(String name, double value) { + long longValue = (long) value; + Histogram metric = register(name, new HistogramMetricRegistrar()); + metric.update(longValue); } @SuppressWarnings("unchecked") - private T getOrAddMetric(String name, T newMetric) { - Metric metric = this.registry.getMetrics().get(name); - if (metric == null) { - return this.registry.register(name, newMetric); + private T register(String name, MetricRegistrar registrar) { + Reservoir reservoir = this.reservoirFactory.getReservoir(name); + if (reservoir == null) { + return registrar.register(this.registry, name); } - else { - if (metric.getClass().equals(newMetric.getClass())) { - return (T) metric; - } - else { - throw new IllegalArgumentException( - name + " is already used for a different type of metric"); - } + Metric metric = this.registry.getMetrics().get(name); + if (metric != null) { + registrar.checkExisting(metric); + return (T) metric; + } + try { + return this.registry.register(name, registrar.createForReservoir(reservoir)); + } + catch (IllegalArgumentException ex) { + Metric added = this.registry.getMetrics().get(name); + registrar.checkExisting(metric); + return (T) added; } } @@ -201,10 +196,6 @@ public class DropwizardMetricServices implements CounterService, GaugeService { this.registry.remove(name); } - void setReservoirFactory(ReservoirFactory reservoirFactory) { - this.reservoirFactory = reservoirFactory; - } - /** * Simple {@link Gauge} implementation to {@literal double} value. */ @@ -227,4 +218,62 @@ public class DropwizardMetricServices implements CounterService, GaugeService { } + /** + * Strategy used to register metrics. + */ + private static abstract class MetricRegistrar { + + private final Class type; + + @SuppressWarnings("unchecked") + MetricRegistrar() { + this.type = (Class) ResolvableType + .forClass(MetricRegistrar.class, getClass()).resolveGeneric(); + } + + public void checkExisting(Metric metric) { + Assert.isInstanceOf(this.type, metric, + "Different metric type already registered"); + } + + protected abstract T register(MetricRegistry registry, String name); + + protected abstract T createForReservoir(Reservoir reservoir); + + } + + /** + * {@link MetricRegistrar} for {@link Timer} metrics. + */ + private static class TimerMetricRegistrar extends MetricRegistrar { + + @Override + protected Timer register(MetricRegistry registry, String name) { + return registry.timer(name); + } + + @Override + protected Timer createForReservoir(Reservoir reservoir) { + return new Timer(reservoir); + } + + } + + /** + * {@link MetricRegistrar} for {@link Histogram} metrics. + */ + private static class HistogramMetricRegistrar extends MetricRegistrar { + + @Override + protected Histogram register(MetricRegistry registry, String name) { + return registry.histogram(name); + } + + @Override + protected Histogram createForReservoir(Reservoir reservoir) { + return new Histogram(reservoir); + } + + } + } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java index b1de0bb628..64396043c5 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java @@ -18,22 +18,34 @@ package org.springframework.boot.actuate.metrics.dropwizard; import com.codahale.metrics.Reservoir; -import org.springframework.beans.BeansException; -import org.springframework.beans.factory.ObjectFactory; - /** - * A {@link Reservoir} factory to instantiate the Reservoir that will be set as default - * for the {@link DropwizardMetricServices}. - * The Reservoir instances can't be shared across {@link com.codahale.metrics.Metric}. + * Factory interface that can be used by {@link DropwizardMetricServices} to create a + * custom {@link Reservoir}. * * @author Lucas Saldanha + * @author Phillip Webb + * @since 1.5.0 */ -public abstract class ReservoirFactory implements ObjectFactory { +public interface ReservoirFactory { - protected abstract Reservoir defaultReservoir(); + /** + * Default empty {@link ReservoirFactory} implementation. + */ + ReservoirFactory NONE = new ReservoirFactory() { + + @Override + public Reservoir getReservoir(String name) { + return null; + } + + }; + + /** + * Return the {@link Reservoir} instance to use or {@code null} if a custom reservoir + * is not needed. + * @param name the name of the metric + * @return a reservoir instance or {@code null} + */ + Reservoir getReservoir(String name); - @Override - public Reservoir getObject() throws BeansException { - return defaultReservoir(); - } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java index 5f032deea6..f878306b97 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java @@ -18,7 +18,6 @@ package org.springframework.boot.actuate.autoconfigure; import com.codahale.metrics.Reservoir; import com.codahale.metrics.UniformReservoir; - import org.junit.After; import org.junit.Test; @@ -27,7 +26,6 @@ import org.springframework.boot.actuate.metrics.dropwizard.ReservoirFactory; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; - import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -52,24 +50,23 @@ public class MetricsDropwizardAutoConfigurationTests { public void dropwizardWithoutCustomReservoirConfigured() { this.context = new AnnotationConfigApplicationContext( MetricsDropwizardAutoConfiguration.class); - DropwizardMetricServices dropwizardMetricServices = this.context .getBean(DropwizardMetricServices.class); - - assertThat(ReflectionTestUtils.getField(dropwizardMetricServices, "reservoirFactory")) - .isNull(); + ReservoirFactory reservoirFactory = (ReservoirFactory) ReflectionTestUtils + .getField(dropwizardMetricServices, "reservoirFactory"); + assertThat(reservoirFactory.getReservoir("test")).isNull(); } @Test public void dropwizardWithCustomReservoirConfigured() { this.context = new AnnotationConfigApplicationContext( MetricsDropwizardAutoConfiguration.class, Config.class); - DropwizardMetricServices dropwizardMetricServices = this.context .getBean(DropwizardMetricServices.class); - - assertThat(ReflectionTestUtils.getField(dropwizardMetricServices, "reservoirFactory")) - .isNotNull(); + ReservoirFactory reservoirFactory = (ReservoirFactory) ReflectionTestUtils + .getField(dropwizardMetricServices, "reservoirFactory"); + assertThat(reservoirFactory.getReservoir("test")) + .isInstanceOf(UniformReservoir.class); } @Configuration @@ -77,13 +74,18 @@ public class MetricsDropwizardAutoConfigurationTests { @Bean public ReservoirFactory reservoirFactory() { - return new ReservoirFactory() { - @Override - protected Reservoir defaultReservoir() { - return new UniformReservoir(); - } - }; + return new UniformReservoirFactory(); } + + } + + private static class UniformReservoirFactory implements ReservoirFactory { + + @Override + public Reservoir getReservoir(String name) { + return new UniformReservoir(); + } + } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java index 04f4c1d65a..535797bc5c 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java @@ -22,14 +22,18 @@ import java.util.List; import com.codahale.metrics.Gauge; import com.codahale.metrics.Histogram; import com.codahale.metrics.MetricRegistry; -import com.codahale.metrics.Reservoir; import com.codahale.metrics.Timer; import com.codahale.metrics.UniformReservoir; +import org.junit.Before; import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.anyString; /** * Tests for {@link DropwizardMetricServices}. @@ -39,10 +43,18 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class DropwizardMetricServicesTests { - private final MetricRegistry registry = new MetricRegistry(); + private MetricRegistry registry = new MetricRegistry(); - private final DropwizardMetricServices writer = new DropwizardMetricServices( - this.registry); + @Mock + private ReservoirFactory reservoirFactory; + + private DropwizardMetricServices writer; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + this.writer = new DropwizardMetricServices(this.registry, this.reservoirFactory); + } @Test public void incrementCounter() { @@ -87,20 +99,14 @@ public class DropwizardMetricServicesTests { @Test public void setCustomReservoirTimer() { - this.writer.setReservoirFactory(new ReservoirFactory() { - @Override - protected Reservoir defaultReservoir() { - return new UniformReservoir(); - } - }); - + given(this.reservoirFactory.getReservoir(anyString())) + .willReturn(new UniformReservoir()); this.writer.submit("timer.foo", 200); this.writer.submit("timer.foo", 300); assertThat(this.registry.timer("timer.foo").getCount()).isEqualTo(2); - Timer timer = (Timer) this.registry.getMetrics().get("timer.foo"); - Histogram histogram = (Histogram) ReflectionTestUtils - .getField(timer, "histogram"); + Histogram histogram = (Histogram) ReflectionTestUtils.getField(timer, + "histogram"); assertThat(ReflectionTestUtils.getField(histogram, "reservoir").getClass() .equals(UniformReservoir.class)).isTrue(); } @@ -114,13 +120,8 @@ public class DropwizardMetricServicesTests { @Test public void setCustomReservoirHistogram() { - this.writer.setReservoirFactory(new ReservoirFactory() { - @Override - protected Reservoir defaultReservoir() { - return new UniformReservoir(); - } - }); - + given(this.reservoirFactory.getReservoir(anyString())) + .willReturn(new UniformReservoir()); this.writer.submit("histogram.foo", 2.1); this.writer.submit("histogram.foo", 2.3); assertThat(this.registry.histogram("histogram.foo").getCount()).isEqualTo(2); From 534a9db6fdebc3b1b933144e9932b69753fda3af Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Mon, 17 Oct 2016 23:56:05 +1300 Subject: [PATCH 04/16] Make stop wait time in the launch script configurable Create a parameter `STOP_WAIT_TIME` for the startup script that configures the time in seconds to wait for a normal shutdown. Because of #4941 we also send a shutdown half way the countdown. Fixes gh-7121 --- spring-boot-docs/src/main/asciidoc/deployment.adoc | 8 ++++++++ .../springframework/boot/loader/tools/launch.script | 7 +++++-- .../boot/loader/tools/DefaultLaunchScriptTests.java | 12 ++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/spring-boot-docs/src/main/asciidoc/deployment.adoc b/spring-boot-docs/src/main/asciidoc/deployment.adoc index 714efaf4ce..c8aa0fc681 100644 --- a/spring-boot-docs/src/main/asciidoc/deployment.adoc +++ b/spring-boot-docs/src/main/asciidoc/deployment.adoc @@ -634,6 +634,10 @@ for Gradle and to `${project.name}` for Maven. |`useStartStopDaemon` |If the `start-stop-daemon` command, when it's available, should be used to control the process. Defaults to `true`. + +|`stopWaitTime` +|The default value for `STOP_WAIT_TIME`. Only valid for an `init.d` service. + Defaults to 60 seconds. |=== @@ -694,6 +698,10 @@ The following environment properties are supported with the default script: |`DEBUG` |if not empty will set the `-x` flag on the shell process, making it easy to see the logic in the script. + +|`STOP_WAIT_TIME` +|The time in seconds to wait when stopping the application before forcing a shutdown + (`60` by default). |=== NOTE: The `PID_FOLDER`, `LOG_FOLDER` and `LOG_FILENAME` variables are only valid for an diff --git a/spring-boot-tools/spring-boot-loader-tools/src/main/resources/org/springframework/boot/loader/tools/launch.script b/spring-boot-tools/spring-boot-loader-tools/src/main/resources/org/springframework/boot/loader/tools/launch.script index 84cadd780a..feb9f538a7 100755 --- a/spring-boot-tools/spring-boot-loader-tools/src/main/resources/org/springframework/boot/loader/tools/launch.script +++ b/spring-boot-tools/spring-boot-loader-tools/src/main/resources/org/springframework/boot/loader/tools/launch.script @@ -73,6 +73,9 @@ fi # Initialize log file name if not provided by the config file [[ -z "$LOG_FILENAME" ]] && LOG_FILENAME="{{logFilename:${identity}.log}}" +# Initialize stop wait time if not provided by the config file +[[ -z "$STOP_WAIT_TIME" ]] && STOP_WAIT_TIME={{stopWaitTime:60}} + # ANSI Colors echoRed() { echo $'\e[0;31m'"$1"$'\e[0m'; } echoGreen() { echo $'\e[0;32m'"$1"$'\e[0m'; } @@ -191,9 +194,9 @@ stop() { do_stop() { kill "$1" &> /dev/null || { echoRed "Unable to kill process $1"; return 1; } - for i in $(seq 1 60); do + for i in $(seq 1 $STOP_WAIT_TIME); do isRunning "$1" || { echoGreen "Stopped [$1]"; rm -f "$2"; return 0; } - [[ $i -eq 30 ]] && kill "$1" &> /dev/null + [[ $i -eq STOP_WAIT_TIME/2 ]] && kill "$1" &> /dev/null sleep 1 done echoRed "Unable to kill process $1"; diff --git a/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/DefaultLaunchScriptTests.java b/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/DefaultLaunchScriptTests.java index 278acaa6f8..92c1012a52 100644 --- a/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/DefaultLaunchScriptTests.java +++ b/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/DefaultLaunchScriptTests.java @@ -111,6 +111,11 @@ public class DefaultLaunchScriptTests { assertThatPlaceholderCanBeReplaced("confFolder"); } + @Test + public void stopWaitTimeCanBeReplaced() throws Exception { + assertThatPlaceholderCanBeReplaced("stopWaitTime"); + } + @Test public void defaultForUseStartStopDaemonIsTrue() throws Exception { DefaultLaunchScript script = new DefaultLaunchScript(null, null); @@ -125,6 +130,13 @@ public class DefaultLaunchScriptTests { assertThat(content).contains("MODE=\"auto\""); } + @Test + public void defaultForStopWaitTimeIs60() throws Exception { + DefaultLaunchScript script = new DefaultLaunchScript(null, null); + String content = new String(script.toByteArray()); + assertThat(content).contains("STOP_WAIT_TIME=60"); + } + @Test public void loadFromFile() throws Exception { File file = this.temporaryFolder.newFile(); From 80eee6b30fe4604fba84a036d238896afe359d4d Mon Sep 17 00:00:00 2001 From: Kazuki Shimizu Date: Sat, 3 Dec 2016 18:49:15 +0900 Subject: [PATCH 05/16] Support spring transaction manager properties Add Spring TransactionManager properties to allow timeout and rollback settings to be configured. See gh-7561 --- .../batch/BasicBatchConfigurer.java | 22 ++++-- .../batch/BatchAutoConfiguration.java | 14 +++- .../neo4j/Neo4jDataAutoConfiguration.java | 13 ++- ...ceTransactionManagerAutoConfiguration.java | 10 ++- .../orm/jpa/JpaBaseConfiguration.java | 10 ++- .../transaction/TransactionProperties.java | 79 +++++++++++++++++++ .../jta/AtomikosJtaConfiguration.java | 12 ++- .../jta/BitronixJtaConfiguration.java | 12 ++- .../transaction/jta/JndiJtaConfiguration.java | 14 +++- .../jta/NarayanaJtaConfiguration.java | 12 ++- .../batch/BatchAutoConfigurationTests.java | 38 +++++++++ .../Neo4jDataAutoConfigurationTests.java | 12 +++ ...nsactionManagerAutoConfigurationTests.java | 14 ++++ .../HibernateJpaAutoConfigurationTests.java | 13 +++ .../jta/JtaAutoConfigurationTests.java | 26 ++++++ 15 files changed, 275 insertions(+), 26 deletions(-) create mode 100644 spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/TransactionProperties.java diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BasicBatchConfigurer.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BasicBatchConfigurer.java index 251601ba28..d8f07295fe 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BasicBatchConfigurer.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BasicBatchConfigurer.java @@ -30,6 +30,7 @@ import org.springframework.batch.core.launch.JobLauncher; import org.springframework.batch.core.launch.support.SimpleJobLauncher; import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.core.repository.support.JobRepositoryFactoryBean; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.jdbc.datasource.DataSourceTransactionManager; import org.springframework.orm.jpa.JpaTransactionManager; import org.springframework.transaction.PlatformTransactionManager; @@ -40,6 +41,7 @@ import org.springframework.util.StringUtils; * * @author Dave Syer * @author Andy Wilkinson + * @author Kazuki Shimizu */ public class BasicBatchConfigurer implements BatchConfigurer { @@ -47,6 +49,8 @@ public class BasicBatchConfigurer implements BatchConfigurer { private final BatchProperties properties; + private final TransactionProperties transactionProperties; + private final DataSource dataSource; private final EntityManagerFactory entityManagerFactory; @@ -62,21 +66,24 @@ public class BasicBatchConfigurer implements BatchConfigurer { /** * Create a new {@link BasicBatchConfigurer} instance. * @param properties the batch properties + * @param transactionProperties the transaction properties * @param dataSource the underlying data source */ - protected BasicBatchConfigurer(BatchProperties properties, DataSource dataSource) { - this(properties, dataSource, null); + protected BasicBatchConfigurer(BatchProperties properties, TransactionProperties transactionProperties, DataSource dataSource) { + this(properties, transactionProperties, dataSource, null); } /** * Create a new {@link BasicBatchConfigurer} instance. * @param properties the batch properties + * @param transactionProperties the transaction properties * @param dataSource the underlying data source * @param entityManagerFactory the entity manager factory (or {@code null}) */ - protected BasicBatchConfigurer(BatchProperties properties, DataSource dataSource, + protected BasicBatchConfigurer(BatchProperties properties, TransactionProperties transactionProperties, DataSource dataSource, EntityManagerFactory entityManagerFactory) { this.properties = properties; + this.transactionProperties = transactionProperties; this.entityManagerFactory = entityManagerFactory; this.dataSource = dataSource; } @@ -150,10 +157,15 @@ public class BasicBatchConfigurer implements BatchConfigurer { } protected PlatformTransactionManager createTransactionManager() { + PlatformTransactionManager txManager; if (this.entityManagerFactory != null) { - return new JpaTransactionManager(this.entityManagerFactory); + txManager = new JpaTransactionManager(this.entityManagerFactory); } - return new DataSourceTransactionManager(this.dataSource); + else { + txManager = new DataSourceTransactionManager(this.dataSource); + } + this.transactionProperties.applyTo(txManager); + return txManager; } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfiguration.java index cb9c3515c8..44eefb73f7 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfiguration.java @@ -37,11 +37,13 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.io.ResourceLoader; import org.springframework.jdbc.core.JdbcOperations; +import org.springframework.transaction.PlatformTransactionManager; import org.springframework.util.StringUtils; /** @@ -57,6 +59,7 @@ import org.springframework.util.StringUtils; * * @author Dave Syer * @author Eddú Meléndez + * @author Kazuki Shimizu */ @Configuration @ConditionalOnClass({ JobLauncher.class, DataSource.class, JdbcOperations.class }) @@ -133,15 +136,18 @@ public class BatchAutoConfiguration { return factory; } - @ConditionalOnClass(name = "javax.persistence.EntityManagerFactory") + @EnableConfigurationProperties({BatchProperties.class, TransactionProperties.class}) + @ConditionalOnClass(value = PlatformTransactionManager.class, name = "javax.persistence.EntityManagerFactory") @ConditionalOnMissingBean(BatchConfigurer.class) @Configuration protected static class JpaBatchConfiguration { private final BatchProperties properties; + private final TransactionProperties transactionProperties; - protected JpaBatchConfiguration(BatchProperties properties) { + protected JpaBatchConfiguration(BatchProperties properties, TransactionProperties transactionProperties) { this.properties = properties; + this.transactionProperties = transactionProperties; } // The EntityManagerFactory may not be discoverable by type when this condition @@ -151,14 +157,14 @@ public class BatchAutoConfiguration { @ConditionalOnBean(name = "entityManagerFactory") public BasicBatchConfigurer jpaBatchConfigurer(DataSource dataSource, EntityManagerFactory entityManagerFactory) { - return new BasicBatchConfigurer(this.properties, dataSource, + return new BasicBatchConfigurer(this.properties, this.transactionProperties, dataSource, entityManagerFactory); } @Bean @ConditionalOnMissingBean(name = "entityManagerFactory") public BasicBatchConfigurer basicBatchConfigurer(DataSource dataSource) { - return new BasicBatchConfigurer(this.properties, dataSource); + return new BasicBatchConfigurer(this.properties, this.transactionProperties, dataSource); } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfiguration.java index e5743e2595..19bbc4004e 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfiguration.java @@ -29,6 +29,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.domain.EntityScanPackages; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; @@ -48,12 +49,13 @@ import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter * @author Josh Long * @author Vince Bickers * @author Stephane Nicoll + * @author Kazuki Shimizu * @since 1.4.0 */ @Configuration -@ConditionalOnClass(SessionFactory.class) +@ConditionalOnClass({SessionFactory.class, PlatformTransactionManager.class}) @ConditionalOnMissingBean(SessionFactory.class) -@EnableConfigurationProperties(Neo4jProperties.class) +@EnableConfigurationProperties({Neo4jProperties.class, TransactionProperties.class}) @SuppressWarnings("deprecation") public class Neo4jDataAutoConfiguration { @@ -87,8 +89,11 @@ public class Neo4jDataAutoConfiguration { @Bean @ConditionalOnMissingBean(PlatformTransactionManager.class) - public Neo4jTransactionManager transactionManager(SessionFactory sessionFactory) { - return new Neo4jTransactionManager(sessionFactory); + public Neo4jTransactionManager transactionManager(SessionFactory sessionFactory, + TransactionProperties transactionProperties) { + Neo4jTransactionManager transactionManager = new Neo4jTransactionManager(sessionFactory); + transactionProperties.applyTo(transactionManager); + return transactionManager; } private String[] getPackagesToScan(ApplicationContext applicationContext) { diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfiguration.java index d5feaf1352..7632f16196 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfiguration.java @@ -23,6 +23,8 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnSingleCandidate; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.Ordered; @@ -39,6 +41,7 @@ import org.springframework.transaction.annotation.EnableTransactionManagement; * @author Dave Syer * @author Stephane Nicoll * @author Andy Wilkinson + * @author Kazuki Shimizu */ @Configuration @ConditionalOnClass({ JdbcTemplate.class, PlatformTransactionManager.class }) @@ -47,6 +50,7 @@ public class DataSourceTransactionManagerAutoConfiguration { @Configuration @ConditionalOnSingleCandidate(DataSource.class) + @EnableConfigurationProperties(TransactionProperties.class) static class DataSourceTransactionManagerConfiguration { private final DataSource dataSource; @@ -57,8 +61,10 @@ public class DataSourceTransactionManagerAutoConfiguration { @Bean @ConditionalOnMissingBean(PlatformTransactionManager.class) - public DataSourceTransactionManager transactionManager() { - return new DataSourceTransactionManager(this.dataSource); + public DataSourceTransactionManager transactionManager(TransactionProperties transactionProperties) { + DataSourceTransactionManager transactionManager = new DataSourceTransactionManager(this.dataSource); + transactionProperties.applyTo(transactionManager); + return transactionManager; } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaBaseConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaBaseConfiguration.java index 67d1ccbd96..a4d05fd6b8 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaBaseConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaBaseConfiguration.java @@ -34,6 +34,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.domain.EntityScanPackages; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.orm.jpa.EntityManagerFactoryBuilder; import org.springframework.context.annotation.Bean; @@ -59,8 +60,9 @@ import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter * @author Dave Syer * @author Oliver Gierke * @author Andy Wilkinson + * @author Kazuki Shimizu */ -@EnableConfigurationProperties(JpaProperties.class) +@EnableConfigurationProperties({JpaProperties.class, TransactionProperties.class}) @Import(DataSourceInitializedPublisher.Registrar.class) public abstract class JpaBaseConfiguration implements BeanFactoryAware { @@ -81,8 +83,10 @@ public abstract class JpaBaseConfiguration implements BeanFactoryAware { @Bean @ConditionalOnMissingBean(PlatformTransactionManager.class) - public PlatformTransactionManager transactionManager() { - return new JpaTransactionManager(); + public PlatformTransactionManager transactionManager(TransactionProperties transactionProperties) { + JpaTransactionManager transactionManager = new JpaTransactionManager(); + transactionProperties.applyTo(transactionManager); + return transactionManager; } @Bean diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/TransactionProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/TransactionProperties.java new file mode 100644 index 0000000000..41a3499080 --- /dev/null +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/TransactionProperties.java @@ -0,0 +1,79 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.autoconfigure.transaction; + +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.AbstractPlatformTransactionManager; + +/** + * External configuration properties for a {@link org.springframework.transaction.PlatformTransactionManager} created by + * Spring. All {@literal spring.transaction.} properties are also applied to the {@code PlatformTransactionManager}. + * + * @author Kazuki Shimizu + * @since 1.5.0 + */ +@ConfigurationProperties(prefix = "spring.transaction") +public class TransactionProperties { + + /** + * The default transaction timeout (sec). + */ + private Integer defaultTimeout; + + /** + * The indicating flag whether perform the rollback processing on commit failure (If perform rollback, set to the true). + */ + private Boolean rollbackOnCommitFailure; + + public Integer getDefaultTimeout() { + return this.defaultTimeout; + } + + public void setDefaultTimeout(Integer defaultTimeout) { + this.defaultTimeout = defaultTimeout; + } + + public Boolean getRollbackOnCommitFailure() { + return this.rollbackOnCommitFailure; + } + + public void setRollbackOnCommitFailure(Boolean rollbackOnCommitFailure) { + this.rollbackOnCommitFailure = rollbackOnCommitFailure; + } + + /** + * Apply all transaction custom properties to a specified {@link PlatformTransactionManager} instance. + * + * @param transactionManager the target transaction manager + * @see AbstractPlatformTransactionManager#setDefaultTimeout(int) + * @see AbstractPlatformTransactionManager#setRollbackOnCommitFailure(boolean) + */ + public void applyTo(PlatformTransactionManager transactionManager) { + if (transactionManager instanceof AbstractPlatformTransactionManager) { + AbstractPlatformTransactionManager abstractPlatformTransactionManager = + (AbstractPlatformTransactionManager) transactionManager; + if (this.defaultTimeout != null) { + abstractPlatformTransactionManager.setDefaultTimeout(this.defaultTimeout); + } + if (this.rollbackOnCommitFailure != null) { + abstractPlatformTransactionManager.setRollbackOnCommitFailure(this.rollbackOnCommitFailure); + } + } + } + +} diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/AtomikosJtaConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/AtomikosJtaConfiguration.java index bde242ed0b..e5b6d7b37c 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/AtomikosJtaConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/AtomikosJtaConfiguration.java @@ -30,6 +30,7 @@ import com.atomikos.icatch.jta.UserTransactionManager; import org.springframework.boot.ApplicationHome; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.jta.XAConnectionFactoryWrapper; import org.springframework.boot.jta.XADataSourceWrapper; @@ -50,18 +51,21 @@ import org.springframework.util.StringUtils; * @author Phillip Webb * @author Andy Wilkinson * @author Stephane Nicoll + * @author Kazuki Shimizu * @since 1.2.0 */ @Configuration -@EnableConfigurationProperties(AtomikosProperties.class) +@EnableConfigurationProperties({AtomikosProperties.class, JtaProperties.class, TransactionProperties.class}) @ConditionalOnClass({ JtaTransactionManager.class, UserTransactionManager.class }) @ConditionalOnMissingBean(PlatformTransactionManager.class) class AtomikosJtaConfiguration { private final JtaProperties jtaProperties; + private final TransactionProperties transactionProperties; - AtomikosJtaConfiguration(JtaProperties jtaProperties) { + AtomikosJtaConfiguration(JtaProperties jtaProperties, TransactionProperties transactionProperties) { this.jtaProperties = jtaProperties; + this.transactionProperties = transactionProperties; } @Bean(initMethod = "init", destroyMethod = "shutdownForce") @@ -111,7 +115,9 @@ class AtomikosJtaConfiguration { @Bean public JtaTransactionManager transactionManager(UserTransaction userTransaction, TransactionManager transactionManager) { - return new JtaTransactionManager(userTransaction, transactionManager); + JtaTransactionManager jtaTransactionManager = new JtaTransactionManager(userTransaction, transactionManager); + this.transactionProperties.applyTo(jtaTransactionManager); + return jtaTransactionManager; } @Configuration diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/BitronixJtaConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/BitronixJtaConfiguration.java index c10098ce14..efcb403b94 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/BitronixJtaConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/BitronixJtaConfiguration.java @@ -28,7 +28,9 @@ import bitronix.tm.jndi.BitronixContext; import org.springframework.boot.ApplicationHome; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.jta.XAConnectionFactoryWrapper; import org.springframework.boot.jta.XADataSourceWrapper; import org.springframework.boot.jta.bitronix.BitronixDependentBeanFactoryPostProcessor; @@ -46,17 +48,21 @@ import org.springframework.util.StringUtils; * @author Josh Long * @author Phillip Webb * @author Andy Wilkinson + * @author Kazuki Shimizu * @since 1.2.0 */ @Configuration +@EnableConfigurationProperties({JtaProperties.class, TransactionProperties.class}) @ConditionalOnClass({ JtaTransactionManager.class, BitronixContext.class }) @ConditionalOnMissingBean(PlatformTransactionManager.class) class BitronixJtaConfiguration { private final JtaProperties jtaProperties; + private final TransactionProperties transactionProperties; - BitronixJtaConfiguration(JtaProperties jtaProperties) { + BitronixJtaConfiguration(JtaProperties jtaProperties, TransactionProperties transactionProperties) { this.jtaProperties = jtaProperties; + this.transactionProperties = transactionProperties; } @Bean @@ -105,7 +111,9 @@ class BitronixJtaConfiguration { @Bean public JtaTransactionManager transactionManager( TransactionManager transactionManager) { - return new JtaTransactionManager(transactionManager); + JtaTransactionManager jtaTransactionManager = new JtaTransactionManager(transactionManager); + this.transactionProperties.applyTo(jtaTransactionManager); + return jtaTransactionManager; } @ConditionalOnClass(Message.class) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JndiJtaConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JndiJtaConfiguration.java index 12f20dd39f..c35c62db5b 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JndiJtaConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JndiJtaConfiguration.java @@ -19,6 +19,8 @@ package org.springframework.boot.autoconfigure.transaction.jta; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnJndi; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.transaction.PlatformTransactionManager; @@ -30,6 +32,7 @@ import org.springframework.transaction.jta.JtaTransactionManager; * * @author Phillip Webb * @author Stephane Nicoll + * @author Kazuki Shimizu * @since 1.2.0 */ @Configuration @@ -38,11 +41,20 @@ import org.springframework.transaction.jta.JtaTransactionManager; "java:comp/TransactionManager", "java:appserver/TransactionManager", "java:pm/TransactionManager", "java:/TransactionManager" }) @ConditionalOnMissingBean(PlatformTransactionManager.class) +@EnableConfigurationProperties(TransactionProperties.class) class JndiJtaConfiguration { + private final TransactionProperties transactionProperties; + + JndiJtaConfiguration(TransactionProperties transactionProperties) { + this.transactionProperties = transactionProperties; + } + @Bean public JtaTransactionManager transactionManager() { - return new JtaTransactionManagerFactoryBean().getObject(); + JtaTransactionManager transactionManager = new JtaTransactionManagerFactoryBean().getObject(); + this.transactionProperties.applyTo(transactionManager); + return transactionManager; } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/NarayanaJtaConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/NarayanaJtaConfiguration.java index 31394d6ed9..99c98f7f8f 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/NarayanaJtaConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/NarayanaJtaConfiguration.java @@ -28,6 +28,8 @@ import org.jboss.tm.XAResourceRecoveryRegistry; import org.springframework.boot.ApplicationHome; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.jta.XAConnectionFactoryWrapper; import org.springframework.boot.jta.XADataSourceWrapper; import org.springframework.boot.jta.narayana.NarayanaBeanFactoryPostProcessor; @@ -47,18 +49,22 @@ import org.springframework.util.StringUtils; * JTA Configuration for Narayana. * * @author Gytis Trikleris + * @author Kazuki Shimizu * @since 1.4.0 */ @Configuration @ConditionalOnClass({ JtaTransactionManager.class, com.arjuna.ats.jta.UserTransaction.class, XAResourceRecoveryRegistry.class }) @ConditionalOnMissingBean(PlatformTransactionManager.class) +@EnableConfigurationProperties({JtaProperties.class, TransactionProperties.class}) public class NarayanaJtaConfiguration { private final JtaProperties jtaProperties; + private final TransactionProperties transactionProperties; - public NarayanaJtaConfiguration(JtaProperties jtaProperties) { + public NarayanaJtaConfiguration(JtaProperties jtaProperties, TransactionProperties transactionProperties) { this.jtaProperties = jtaProperties; + this.transactionProperties = transactionProperties; } @Bean @@ -116,7 +122,9 @@ public class NarayanaJtaConfiguration { @Bean public JtaTransactionManager transactionManager(UserTransaction userTransaction, TransactionManager transactionManager) { - return new JtaTransactionManager(userTransaction, transactionManager); + JtaTransactionManager jtaTransactionManager = new JtaTransactionManager(userTransaction, transactionManager); + this.transactionProperties.applyTo(jtaTransactionManager); + return jtaTransactionManager; } @Bean diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java index c668df5019..e793ef3e44 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java @@ -58,6 +58,8 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.jdbc.BadSqlGrammarException; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.datasource.DataSourceTransactionManager; +import org.springframework.orm.jpa.JpaTransactionManager; import org.springframework.transaction.PlatformTransactionManager; import static org.assertj.core.api.Assertions.assertThat; @@ -267,6 +269,42 @@ public class BatchAutoConfigurationTests { .queryForList("select * from BATCH_JOB_EXECUTION"); } + @Test + public void testCustomizeJpaTransactionManagerUsingProperties() throws Exception { + this.context = new AnnotationConfigApplicationContext(); + EnvironmentTestUtils.addEnvironment(this.context, + "spring.transaction.default-timeout:30", + "spring.transaction.rollback-on-commit-failure:true"); + this.context.register(TestConfiguration.class, + EmbeddedDataSourceConfiguration.class, + HibernateJpaAutoConfiguration.class, BatchAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class); + this.context.refresh(); + this.context.getBean(BatchConfigurer.class); + JpaTransactionManager transactionManager = JpaTransactionManager.class.cast( + this.context.getBean(BatchConfigurer.class).getTransactionManager()); + assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); + assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); + } + + @Test + public void testCustomizeDataSourceTransactionManagerUsingProperties() throws Exception { + this.context = new AnnotationConfigApplicationContext(); + EnvironmentTestUtils.addEnvironment(this.context, + "spring.transaction.default-timeout:30", + "spring.transaction.rollback-on-commit-failure:true"); + this.context.register(TestConfiguration.class, + EmbeddedDataSourceConfiguration.class, + BatchAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class); + this.context.refresh(); + this.context.getBean(BatchConfigurer.class); + DataSourceTransactionManager transactionManager = DataSourceTransactionManager.class.cast( + this.context.getBean(BatchConfigurer.class).getTransactionManager()); + assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); + assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); + } + @Configuration protected static class EmptyConfiguration { diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfigurationTests.java index 25fc2b5662..49031a48bd 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfigurationTests.java @@ -37,6 +37,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.data.neo4j.mapping.Neo4jMappingContext; import org.springframework.data.neo4j.template.Neo4jOperations; +import org.springframework.data.neo4j.transaction.Neo4jTransactionManager; import org.springframework.data.neo4j.web.support.OpenSessionInViewInterceptor; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; @@ -73,10 +74,21 @@ public class Neo4jDataAutoConfigurationTests { .hasSize(1); assertThat(this.context.getBeansOfType(SessionFactory.class)).hasSize(1); assertThat(this.context.getBeansOfType(Neo4jOperations.class)).hasSize(1); + assertThat(this.context.getBeansOfType(Neo4jTransactionManager.class)).hasSize(1); assertThat(this.context.getBeansOfType(OpenSessionInViewInterceptor.class)) .isEmpty(); } + @Test + public void customNeo4jTransactionManagerUsingProperties() { + load(null, + "spring.transaction.default-timeout=30", + "spring.transaction.rollback-on-commit-failure:true"); + Neo4jTransactionManager transactionManager = this.context.getBean(Neo4jTransactionManager.class); + assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); + assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); + } + @Test public void customSessionFactory() { load(CustomSessionFactory.class); diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfigurationTests.java index 93b44a1414..70ea23679e 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfigurationTests.java @@ -20,6 +20,7 @@ import javax.sql.DataSource; import org.junit.Test; +import org.springframework.boot.test.util.EnvironmentTestUtils; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -106,6 +107,19 @@ public class DataSourceTransactionManagerAutoConfigurationTests { .isNotNull(); } + @Test + public void testCustomizeDataSourceTransactionManagerUsingProperties() throws Exception { + EnvironmentTestUtils.addEnvironment(this.context, + "spring.transaction.default-timeout:30", + "spring.transaction.rollback-on-commit-failure:true"); + this.context.register(EmbeddedDataSourceConfiguration.class, + DataSourceTransactionManagerAutoConfiguration.class); + this.context.refresh(); + DataSourceTransactionManager transactionManager = this.context.getBean(DataSourceTransactionManager.class); + assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); + assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); + } + @EnableTransactionManagement protected static class SwitchTransactionsOn { diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java index 1c2b4798ac..ff67fcf246 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java @@ -38,6 +38,7 @@ import org.springframework.boot.autoconfigure.transaction.jta.JtaAutoConfigurati import org.springframework.boot.orm.jpa.hibernate.SpringJtaPlatform; import org.springframework.boot.test.util.EnvironmentTestUtils; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.orm.jpa.JpaTransactionManager; import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; import static org.assertj.core.api.Assertions.assertThat; @@ -171,6 +172,18 @@ public class HibernateJpaAutoConfigurationTests .isEqualTo(TestJtaPlatform.class.getName()); } + @Test + public void testCustomJpaTransactionManagerUsingProperties() throws Exception { + EnvironmentTestUtils.addEnvironment(this.context, + "spring.transaction.default-timeout:30", + "spring.transaction.rollback-on-commit-failure:true"); + setupTestConfiguration(); + this.context.refresh(); + JpaTransactionManager transactionManager = context.getBean(JpaTransactionManager.class); + assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); + assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); + } + public static class TestJtaPlatform implements JtaPlatform { @Override diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/transaction/jta/JtaAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/transaction/jta/JtaAutoConfigurationTests.java index f57b84e2fa..2193e59155 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/transaction/jta/JtaAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/transaction/jta/JtaAutoConfigurationTests.java @@ -245,6 +245,32 @@ public class JtaAutoConfigurationTests { assertThat(dataSource.getMaxPoolSize()).isEqualTo(10); } + @Test + public void atomikosCustomizeJtaTransactionManagerUsingProperties() throws Exception { + this.context = new AnnotationConfigApplicationContext(); + EnvironmentTestUtils.addEnvironment(this.context, + "spring.transaction.default-timeout:30", + "spring.transaction.rollback-on-commit-failure:true"); + this.context.register(AtomikosJtaConfiguration.class); + this.context.refresh(); + JtaTransactionManager transactionManager = this.context.getBean(JtaTransactionManager.class); + assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); + assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); + } + + @Test + public void bitronixCustomizeJtaTransactionManagerUsingProperties() throws Exception { + this.context = new AnnotationConfigApplicationContext(); + EnvironmentTestUtils.addEnvironment(this.context, + "spring.transaction.default-timeout:30", + "spring.transaction.rollback-on-commit-failure:true"); + this.context.register(BitronixJtaConfiguration.class); + this.context.refresh(); + JtaTransactionManager transactionManager = this.context.getBean(JtaTransactionManager.class); + assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); + assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); + } + @Configuration @EnableConfigurationProperties(JtaProperties.class) public static class JtaPropertiesConfiguration { From 99e72664d9eda43c170d339ca7b5c5986f6b8a33 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Dec 2016 15:27:41 -0800 Subject: [PATCH 06/16] Polish spring transaction manager properties Polish and update contribution so that TransactionManager properties can be defined per technology, rather than globally. Closes gh-7561 --- .../batch/BasicBatchConfigurer.java | 28 +++++++--------- .../batch/BatchAutoConfiguration.java | 11 +++---- .../autoconfigure/batch/BatchProperties.java | 21 ++++++++---- .../neo4j/Neo4jDataAutoConfiguration.java | 12 +++---- .../data/neo4j/Neo4jProperties.java | 9 ++++++ .../jdbc/DataSourceProperties.java | 9 ++++++ ...ceTransactionManagerAutoConfiguration.java | 11 ++++--- .../orm/jpa/JpaBaseConfiguration.java | 7 ++-- .../autoconfigure/orm/jpa/JpaProperties.java | 8 +++++ .../transaction/TransactionProperties.java | 32 +++++++------------ .../jta/AtomikosJtaConfiguration.java | 12 +++---- .../jta/BitronixJtaConfiguration.java | 12 +++---- .../transaction/jta/JndiJtaConfiguration.java | 14 ++++---- .../transaction/jta/JtaProperties.java | 9 ++++++ .../jta/NarayanaJtaConfiguration.java | 12 +++---- .../batch/BatchAutoConfigurationTests.java | 20 ++++++------ .../Neo4jDataAutoConfigurationTests.java | 9 +++--- ...nsactionManagerAutoConfigurationTests.java | 10 +++--- .../HibernateJpaAutoConfigurationTests.java | 8 +++-- .../jta/JtaAutoConfigurationTests.java | 15 +++++---- .../appendix-application-properties.adoc | 5 +++ 21 files changed, 154 insertions(+), 120 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BasicBatchConfigurer.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BasicBatchConfigurer.java index d8f07295fe..5437f5e407 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BasicBatchConfigurer.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BasicBatchConfigurer.java @@ -30,10 +30,10 @@ import org.springframework.batch.core.launch.JobLauncher; import org.springframework.batch.core.launch.support.SimpleJobLauncher; import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.core.repository.support.JobRepositoryFactoryBean; -import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.jdbc.datasource.DataSourceTransactionManager; import org.springframework.orm.jpa.JpaTransactionManager; import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.AbstractPlatformTransactionManager; import org.springframework.util.StringUtils; /** @@ -49,8 +49,6 @@ public class BasicBatchConfigurer implements BatchConfigurer { private final BatchProperties properties; - private final TransactionProperties transactionProperties; - private final DataSource dataSource; private final EntityManagerFactory entityManagerFactory; @@ -66,24 +64,21 @@ public class BasicBatchConfigurer implements BatchConfigurer { /** * Create a new {@link BasicBatchConfigurer} instance. * @param properties the batch properties - * @param transactionProperties the transaction properties * @param dataSource the underlying data source */ - protected BasicBatchConfigurer(BatchProperties properties, TransactionProperties transactionProperties, DataSource dataSource) { - this(properties, transactionProperties, dataSource, null); + protected BasicBatchConfigurer(BatchProperties properties, DataSource dataSource) { + this(properties, dataSource, null); } /** * Create a new {@link BasicBatchConfigurer} instance. * @param properties the batch properties - * @param transactionProperties the transaction properties * @param dataSource the underlying data source * @param entityManagerFactory the entity manager factory (or {@code null}) */ - protected BasicBatchConfigurer(BatchProperties properties, TransactionProperties transactionProperties, DataSource dataSource, + protected BasicBatchConfigurer(BatchProperties properties, DataSource dataSource, EntityManagerFactory entityManagerFactory) { this.properties = properties; - this.transactionProperties = transactionProperties; this.entityManagerFactory = entityManagerFactory; this.dataSource = dataSource; } @@ -157,15 +152,16 @@ public class BasicBatchConfigurer implements BatchConfigurer { } protected PlatformTransactionManager createTransactionManager() { - PlatformTransactionManager txManager; + AbstractPlatformTransactionManager transactionManager = createAppropriateTransactionManager(); + this.properties.getTransaction().applyTo(transactionManager); + return transactionManager; + } + + private AbstractPlatformTransactionManager createAppropriateTransactionManager() { if (this.entityManagerFactory != null) { - txManager = new JpaTransactionManager(this.entityManagerFactory); + return new JpaTransactionManager(this.entityManagerFactory); } - else { - txManager = new DataSourceTransactionManager(this.dataSource); - } - this.transactionProperties.applyTo(txManager); - return txManager; + return new DataSourceTransactionManager(this.dataSource); } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfiguration.java index 44eefb73f7..2586c8621a 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfiguration.java @@ -37,7 +37,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration; -import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -136,18 +135,16 @@ public class BatchAutoConfiguration { return factory; } - @EnableConfigurationProperties({BatchProperties.class, TransactionProperties.class}) + @EnableConfigurationProperties(BatchProperties.class) @ConditionalOnClass(value = PlatformTransactionManager.class, name = "javax.persistence.EntityManagerFactory") @ConditionalOnMissingBean(BatchConfigurer.class) @Configuration protected static class JpaBatchConfiguration { private final BatchProperties properties; - private final TransactionProperties transactionProperties; - protected JpaBatchConfiguration(BatchProperties properties, TransactionProperties transactionProperties) { + protected JpaBatchConfiguration(BatchProperties properties) { this.properties = properties; - this.transactionProperties = transactionProperties; } // The EntityManagerFactory may not be discoverable by type when this condition @@ -157,14 +154,14 @@ public class BatchAutoConfiguration { @ConditionalOnBean(name = "entityManagerFactory") public BasicBatchConfigurer jpaBatchConfigurer(DataSource dataSource, EntityManagerFactory entityManagerFactory) { - return new BasicBatchConfigurer(this.properties, this.transactionProperties, dataSource, + return new BasicBatchConfigurer(this.properties, dataSource, entityManagerFactory); } @Bean @ConditionalOnMissingBean(name = "entityManagerFactory") public BasicBatchConfigurer basicBatchConfigurer(DataSource dataSource) { - return new BasicBatchConfigurer(this.properties, this.transactionProperties, dataSource); + return new BasicBatchConfigurer(this.properties, dataSource); } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchProperties.java index 9582a05f34..998098416d 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchProperties.java @@ -16,7 +16,9 @@ package org.springframework.boot.autoconfigure.batch; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.NestedConfigurationProperty; /** * Configuration properties for Spring Batch. @@ -46,6 +48,9 @@ public class BatchProperties { private final Job job = new Job(); + @NestedConfigurationProperty + private final TransactionProperties transaction = new TransactionProperties(); + public String getSchema() { return this.schema; } @@ -54,6 +59,14 @@ public class BatchProperties { this.schema = schema; } + public String getTablePrefix() { + return this.tablePrefix; + } + + public void setTablePrefix(String tablePrefix) { + this.tablePrefix = tablePrefix; + } + public Initializer getInitializer() { return this.initializer; } @@ -62,12 +75,8 @@ public class BatchProperties { return this.job; } - public void setTablePrefix(String tablePrefix) { - this.tablePrefix = tablePrefix; - } - - public String getTablePrefix() { - return this.tablePrefix; + public TransactionProperties getTransaction() { + return this.transaction; } public class Initializer { diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfiguration.java index 19bbc4004e..f0a069b1b9 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfiguration.java @@ -29,7 +29,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.domain.EntityScanPackages; -import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; @@ -53,9 +52,9 @@ import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter * @since 1.4.0 */ @Configuration -@ConditionalOnClass({SessionFactory.class, PlatformTransactionManager.class}) +@ConditionalOnClass({ SessionFactory.class, PlatformTransactionManager.class }) @ConditionalOnMissingBean(SessionFactory.class) -@EnableConfigurationProperties({Neo4jProperties.class, TransactionProperties.class}) +@EnableConfigurationProperties(Neo4jProperties.class) @SuppressWarnings("deprecation") public class Neo4jDataAutoConfiguration { @@ -90,9 +89,10 @@ public class Neo4jDataAutoConfiguration { @Bean @ConditionalOnMissingBean(PlatformTransactionManager.class) public Neo4jTransactionManager transactionManager(SessionFactory sessionFactory, - TransactionProperties transactionProperties) { - Neo4jTransactionManager transactionManager = new Neo4jTransactionManager(sessionFactory); - transactionProperties.applyTo(transactionManager); + Neo4jProperties properties) { + Neo4jTransactionManager transactionManager = new Neo4jTransactionManager( + sessionFactory); + properties.getTransaction().applyTo(transactionManager); return transactionManager; } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jProperties.java index e58bda102b..030b69cee6 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jProperties.java @@ -23,7 +23,9 @@ import org.neo4j.ogm.config.Configuration; import org.neo4j.ogm.config.DriverConfiguration; import org.springframework.beans.BeansException; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.NestedConfigurationProperty; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.util.ClassUtils; @@ -69,6 +71,9 @@ public class Neo4jProperties implements ApplicationContextAware { private final Embedded embedded = new Embedded(); + @NestedConfigurationProperty + private final TransactionProperties transaction = new TransactionProperties(); + private ClassLoader classLoader = Neo4jProperties.class.getClassLoader(); public String getUri() { @@ -107,6 +112,10 @@ public class Neo4jProperties implements ApplicationContextAware { return this.embedded; } + public TransactionProperties getTransaction() { + return this.transaction; + } + @Override public void setApplicationContext(ApplicationContext ctx) throws BeansException { this.classLoader = ctx.getClassLoader(); diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceProperties.java index 6b79062f89..be1b5a130d 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceProperties.java @@ -27,7 +27,9 @@ import javax.sql.DataSource; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.InitializingBean; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.NestedConfigurationProperty; import org.springframework.boot.jdbc.DatabaseDriver; import org.springframework.context.EnvironmentAware; import org.springframework.core.env.Environment; @@ -157,6 +159,9 @@ public class DataSourceProperties private String uniqueName; + @NestedConfigurationProperty + private final TransactionProperties transaction = new TransactionProperties(); + @Override public void setBeanClassLoader(ClassLoader classLoader) { this.classLoader = classLoader; @@ -473,6 +478,10 @@ public class DataSourceProperties this.xa = xa; } + public TransactionProperties getTransaction() { + return this.transaction; + } + /** * XA Specific datasource settings. */ diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfiguration.java index 7632f16196..931da19c9c 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfiguration.java @@ -23,7 +23,6 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnSingleCandidate; -import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -46,11 +45,11 @@ import org.springframework.transaction.annotation.EnableTransactionManagement; @Configuration @ConditionalOnClass({ JdbcTemplate.class, PlatformTransactionManager.class }) @AutoConfigureOrder(Ordered.LOWEST_PRECEDENCE) +@EnableConfigurationProperties(DataSourceProperties.class) public class DataSourceTransactionManagerAutoConfiguration { @Configuration @ConditionalOnSingleCandidate(DataSource.class) - @EnableConfigurationProperties(TransactionProperties.class) static class DataSourceTransactionManagerConfiguration { private final DataSource dataSource; @@ -61,9 +60,11 @@ public class DataSourceTransactionManagerAutoConfiguration { @Bean @ConditionalOnMissingBean(PlatformTransactionManager.class) - public DataSourceTransactionManager transactionManager(TransactionProperties transactionProperties) { - DataSourceTransactionManager transactionManager = new DataSourceTransactionManager(this.dataSource); - transactionProperties.applyTo(transactionManager); + public DataSourceTransactionManager transactionManager( + DataSourceProperties properties) { + DataSourceTransactionManager transactionManager = new DataSourceTransactionManager( + this.dataSource); + properties.getTransaction().applyTo(transactionManager); return transactionManager; } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaBaseConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaBaseConfiguration.java index a4d05fd6b8..48f79d2ff0 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaBaseConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaBaseConfiguration.java @@ -34,7 +34,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.domain.EntityScanPackages; -import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.orm.jpa.EntityManagerFactoryBuilder; import org.springframework.context.annotation.Bean; @@ -62,7 +61,7 @@ import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter * @author Andy Wilkinson * @author Kazuki Shimizu */ -@EnableConfigurationProperties({JpaProperties.class, TransactionProperties.class}) +@EnableConfigurationProperties(JpaProperties.class) @Import(DataSourceInitializedPublisher.Registrar.class) public abstract class JpaBaseConfiguration implements BeanFactoryAware { @@ -83,9 +82,9 @@ public abstract class JpaBaseConfiguration implements BeanFactoryAware { @Bean @ConditionalOnMissingBean(PlatformTransactionManager.class) - public PlatformTransactionManager transactionManager(TransactionProperties transactionProperties) { + public PlatformTransactionManager transactionManager() { JpaTransactionManager transactionManager = new JpaTransactionManager(); - transactionProperties.applyTo(transactionManager); + this.properties.getTransaction().applyTo(transactionManager); return transactionManager; } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java index ecb118b4ff..aee5121b81 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java @@ -22,6 +22,7 @@ import java.util.Map; import javax.sql.DataSource; import org.springframework.boot.autoconfigure.jdbc.EmbeddedDatabaseConnection; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.NestedConfigurationProperty; import org.springframework.orm.jpa.vendor.Database; @@ -67,6 +68,9 @@ public class JpaProperties { private Hibernate hibernate = new Hibernate(); + @NestedConfigurationProperty + private final TransactionProperties transaction = new TransactionProperties(); + public Map getProperties() { return this.properties; } @@ -125,6 +129,10 @@ public class JpaProperties { return this.hibernate.getAdditionalProperties(this.properties, dataSource); } + public TransactionProperties getTransaction() { + return this.transaction; + } + public static class Hibernate { private static final String USE_NEW_ID_GENERATOR_MAPPINGS = "hibernate.id." diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/TransactionProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/TransactionProperties.java index 41a3499080..619c368b84 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/TransactionProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/TransactionProperties.java @@ -16,27 +16,24 @@ package org.springframework.boot.autoconfigure.transaction; -import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.support.AbstractPlatformTransactionManager; /** - * External configuration properties for a {@link org.springframework.transaction.PlatformTransactionManager} created by - * Spring. All {@literal spring.transaction.} properties are also applied to the {@code PlatformTransactionManager}. + * Nested configuration properties that can be applied to an + * {@link AbstractPlatformTransactionManager}. * * @author Kazuki Shimizu * @since 1.5.0 */ -@ConfigurationProperties(prefix = "spring.transaction") public class TransactionProperties { /** - * The default transaction timeout (sec). + * Default transaction timeout in seconds. */ private Integer defaultTimeout; /** - * The indicating flag whether perform the rollback processing on commit failure (If perform rollback, set to the true). + * Perform the rollback on commit failurures. */ private Boolean rollbackOnCommitFailure; @@ -57,22 +54,15 @@ public class TransactionProperties { } /** - * Apply all transaction custom properties to a specified {@link PlatformTransactionManager} instance. - * + * Apply all transaction custom properties to the specified transaction manager. * @param transactionManager the target transaction manager - * @see AbstractPlatformTransactionManager#setDefaultTimeout(int) - * @see AbstractPlatformTransactionManager#setRollbackOnCommitFailure(boolean) */ - public void applyTo(PlatformTransactionManager transactionManager) { - if (transactionManager instanceof AbstractPlatformTransactionManager) { - AbstractPlatformTransactionManager abstractPlatformTransactionManager = - (AbstractPlatformTransactionManager) transactionManager; - if (this.defaultTimeout != null) { - abstractPlatformTransactionManager.setDefaultTimeout(this.defaultTimeout); - } - if (this.rollbackOnCommitFailure != null) { - abstractPlatformTransactionManager.setRollbackOnCommitFailure(this.rollbackOnCommitFailure); - } + public void applyTo(AbstractPlatformTransactionManager transactionManager) { + if (this.defaultTimeout != null) { + transactionManager.setDefaultTimeout(this.defaultTimeout); + } + if (this.rollbackOnCommitFailure != null) { + transactionManager.setRollbackOnCommitFailure(this.rollbackOnCommitFailure); } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/AtomikosJtaConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/AtomikosJtaConfiguration.java index e5b6d7b37c..308fe36e7c 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/AtomikosJtaConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/AtomikosJtaConfiguration.java @@ -30,7 +30,6 @@ import com.atomikos.icatch.jta.UserTransactionManager; import org.springframework.boot.ApplicationHome; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.jta.XAConnectionFactoryWrapper; import org.springframework.boot.jta.XADataSourceWrapper; @@ -55,17 +54,15 @@ import org.springframework.util.StringUtils; * @since 1.2.0 */ @Configuration -@EnableConfigurationProperties({AtomikosProperties.class, JtaProperties.class, TransactionProperties.class}) +@EnableConfigurationProperties({ AtomikosProperties.class, JtaProperties.class }) @ConditionalOnClass({ JtaTransactionManager.class, UserTransactionManager.class }) @ConditionalOnMissingBean(PlatformTransactionManager.class) class AtomikosJtaConfiguration { private final JtaProperties jtaProperties; - private final TransactionProperties transactionProperties; - AtomikosJtaConfiguration(JtaProperties jtaProperties, TransactionProperties transactionProperties) { + AtomikosJtaConfiguration(JtaProperties jtaProperties) { this.jtaProperties = jtaProperties; - this.transactionProperties = transactionProperties; } @Bean(initMethod = "init", destroyMethod = "shutdownForce") @@ -115,8 +112,9 @@ class AtomikosJtaConfiguration { @Bean public JtaTransactionManager transactionManager(UserTransaction userTransaction, TransactionManager transactionManager) { - JtaTransactionManager jtaTransactionManager = new JtaTransactionManager(userTransaction, transactionManager); - this.transactionProperties.applyTo(jtaTransactionManager); + JtaTransactionManager jtaTransactionManager = new JtaTransactionManager( + userTransaction, transactionManager); + this.jtaProperties.getTransaction().applyTo(jtaTransactionManager); return jtaTransactionManager; } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/BitronixJtaConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/BitronixJtaConfiguration.java index efcb403b94..36e27bc738 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/BitronixJtaConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/BitronixJtaConfiguration.java @@ -28,7 +28,6 @@ import bitronix.tm.jndi.BitronixContext; import org.springframework.boot.ApplicationHome; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.jta.XAConnectionFactoryWrapper; @@ -52,17 +51,15 @@ import org.springframework.util.StringUtils; * @since 1.2.0 */ @Configuration -@EnableConfigurationProperties({JtaProperties.class, TransactionProperties.class}) +@EnableConfigurationProperties(JtaProperties.class) @ConditionalOnClass({ JtaTransactionManager.class, BitronixContext.class }) @ConditionalOnMissingBean(PlatformTransactionManager.class) class BitronixJtaConfiguration { private final JtaProperties jtaProperties; - private final TransactionProperties transactionProperties; - BitronixJtaConfiguration(JtaProperties jtaProperties, TransactionProperties transactionProperties) { + BitronixJtaConfiguration(JtaProperties jtaProperties) { this.jtaProperties = jtaProperties; - this.transactionProperties = transactionProperties; } @Bean @@ -111,8 +108,9 @@ class BitronixJtaConfiguration { @Bean public JtaTransactionManager transactionManager( TransactionManager transactionManager) { - JtaTransactionManager jtaTransactionManager = new JtaTransactionManager(transactionManager); - this.transactionProperties.applyTo(jtaTransactionManager); + JtaTransactionManager jtaTransactionManager = new JtaTransactionManager( + transactionManager); + this.jtaProperties.getTransaction().applyTo(jtaTransactionManager); return jtaTransactionManager; } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JndiJtaConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JndiJtaConfiguration.java index c35c62db5b..478a898a45 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JndiJtaConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JndiJtaConfiguration.java @@ -19,8 +19,6 @@ package org.springframework.boot.autoconfigure.transaction.jta; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnJndi; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.boot.autoconfigure.transaction.TransactionProperties; -import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.transaction.PlatformTransactionManager; @@ -41,19 +39,19 @@ import org.springframework.transaction.jta.JtaTransactionManager; "java:comp/TransactionManager", "java:appserver/TransactionManager", "java:pm/TransactionManager", "java:/TransactionManager" }) @ConditionalOnMissingBean(PlatformTransactionManager.class) -@EnableConfigurationProperties(TransactionProperties.class) class JndiJtaConfiguration { - private final TransactionProperties transactionProperties; + private final JtaProperties jtaProperties; - JndiJtaConfiguration(TransactionProperties transactionProperties) { - this.transactionProperties = transactionProperties; + JndiJtaConfiguration(JtaProperties jtaProperties) { + this.jtaProperties = jtaProperties; } @Bean public JtaTransactionManager transactionManager() { - JtaTransactionManager transactionManager = new JtaTransactionManagerFactoryBean().getObject(); - this.transactionProperties.applyTo(transactionManager); + JtaTransactionManager transactionManager = new JtaTransactionManagerFactoryBean() + .getObject(); + this.jtaProperties.getTransaction().applyTo(transactionManager); return transactionManager; } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JtaProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JtaProperties.java index 124bba9434..bbf63e4b53 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JtaProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/JtaProperties.java @@ -16,7 +16,9 @@ package org.springframework.boot.autoconfigure.transaction.jta; +import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.NestedConfigurationProperty; import org.springframework.transaction.jta.JtaTransactionManager; /** @@ -42,6 +44,9 @@ public class JtaProperties { */ private String transactionManagerId; + @NestedConfigurationProperty + private final TransactionProperties transaction = new TransactionProperties(); + public void setLogDir(String logDir) { this.logDir = logDir; } @@ -58,4 +63,8 @@ public class JtaProperties { this.transactionManagerId = transactionManagerId; } + public TransactionProperties getTransaction() { + return this.transaction; + } + } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/NarayanaJtaConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/NarayanaJtaConfiguration.java index 99c98f7f8f..ec97194374 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/NarayanaJtaConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/transaction/jta/NarayanaJtaConfiguration.java @@ -28,7 +28,6 @@ import org.jboss.tm.XAResourceRecoveryRegistry; import org.springframework.boot.ApplicationHome; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.boot.autoconfigure.transaction.TransactionProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.jta.XAConnectionFactoryWrapper; import org.springframework.boot.jta.XADataSourceWrapper; @@ -56,15 +55,13 @@ import org.springframework.util.StringUtils; @ConditionalOnClass({ JtaTransactionManager.class, com.arjuna.ats.jta.UserTransaction.class, XAResourceRecoveryRegistry.class }) @ConditionalOnMissingBean(PlatformTransactionManager.class) -@EnableConfigurationProperties({JtaProperties.class, TransactionProperties.class}) +@EnableConfigurationProperties(JtaProperties.class) public class NarayanaJtaConfiguration { private final JtaProperties jtaProperties; - private final TransactionProperties transactionProperties; - public NarayanaJtaConfiguration(JtaProperties jtaProperties, TransactionProperties transactionProperties) { + public NarayanaJtaConfiguration(JtaProperties jtaProperties) { this.jtaProperties = jtaProperties; - this.transactionProperties = transactionProperties; } @Bean @@ -122,8 +119,9 @@ public class NarayanaJtaConfiguration { @Bean public JtaTransactionManager transactionManager(UserTransaction userTransaction, TransactionManager transactionManager) { - JtaTransactionManager jtaTransactionManager = new JtaTransactionManager(userTransaction, transactionManager); - this.transactionProperties.applyTo(jtaTransactionManager); + JtaTransactionManager jtaTransactionManager = new JtaTransactionManager( + userTransaction, transactionManager); + this.jtaProperties.getTransaction().applyTo(jtaTransactionManager); return jtaTransactionManager; } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java index e793ef3e44..327ee070ca 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java @@ -70,6 +70,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Dave Syer * @author Stephane Nicoll * @author Vedran Pavic + * @author Kazuki Shimizu */ public class BatchAutoConfigurationTests { @@ -273,8 +274,8 @@ public class BatchAutoConfigurationTests { public void testCustomizeJpaTransactionManagerUsingProperties() throws Exception { this.context = new AnnotationConfigApplicationContext(); EnvironmentTestUtils.addEnvironment(this.context, - "spring.transaction.default-timeout:30", - "spring.transaction.rollback-on-commit-failure:true"); + "spring.batch.transaction.default-timeout:30", + "spring.batch.transaction.rollback-on-commit-failure:true"); this.context.register(TestConfiguration.class, EmbeddedDataSourceConfiguration.class, HibernateJpaAutoConfiguration.class, BatchAutoConfiguration.class, @@ -288,19 +289,20 @@ public class BatchAutoConfigurationTests { } @Test - public void testCustomizeDataSourceTransactionManagerUsingProperties() throws Exception { + public void testCustomizeDataSourceTransactionManagerUsingProperties() + throws Exception { this.context = new AnnotationConfigApplicationContext(); EnvironmentTestUtils.addEnvironment(this.context, - "spring.transaction.default-timeout:30", - "spring.transaction.rollback-on-commit-failure:true"); + "spring.batch.transaction.default-timeout:30", + "spring.batch.transaction.rollback-on-commit-failure:true"); this.context.register(TestConfiguration.class, - EmbeddedDataSourceConfiguration.class, - BatchAutoConfiguration.class, + EmbeddedDataSourceConfiguration.class, BatchAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class); this.context.refresh(); this.context.getBean(BatchConfigurer.class); - DataSourceTransactionManager transactionManager = DataSourceTransactionManager.class.cast( - this.context.getBean(BatchConfigurer.class).getTransactionManager()); + DataSourceTransactionManager transactionManager = DataSourceTransactionManager.class + .cast(this.context.getBean(BatchConfigurer.class) + .getTransactionManager()); assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfigurationTests.java index 49031a48bd..38044b7488 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/neo4j/Neo4jDataAutoConfigurationTests.java @@ -54,6 +54,7 @@ import static org.mockito.Mockito.verify; * @author Michael Hunger * @author Vince Bickers * @author Andy Wilkinson + * @author Kazuki Shimizu */ @SuppressWarnings("deprecation") public class Neo4jDataAutoConfigurationTests { @@ -81,10 +82,10 @@ public class Neo4jDataAutoConfigurationTests { @Test public void customNeo4jTransactionManagerUsingProperties() { - load(null, - "spring.transaction.default-timeout=30", - "spring.transaction.rollback-on-commit-failure:true"); - Neo4jTransactionManager transactionManager = this.context.getBean(Neo4jTransactionManager.class); + load(null, "spring.data.neo4j.transaction.default-timeout=30", + "spring.data.neo4j.transaction.rollback-on-commit-failure:true"); + Neo4jTransactionManager transactionManager = this.context + .getBean(Neo4jTransactionManager.class); assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfigurationTests.java index 70ea23679e..16c331edc7 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceTransactionManagerAutoConfigurationTests.java @@ -108,14 +108,16 @@ public class DataSourceTransactionManagerAutoConfigurationTests { } @Test - public void testCustomizeDataSourceTransactionManagerUsingProperties() throws Exception { + public void testCustomizeDataSourceTransactionManagerUsingProperties() + throws Exception { EnvironmentTestUtils.addEnvironment(this.context, - "spring.transaction.default-timeout:30", - "spring.transaction.rollback-on-commit-failure:true"); + "spring.datasource.transaction.default-timeout:30", + "spring.datasource.transaction.rollback-on-commit-failure:true"); this.context.register(EmbeddedDataSourceConfiguration.class, DataSourceTransactionManagerAutoConfiguration.class); this.context.refresh(); - DataSourceTransactionManager transactionManager = this.context.getBean(DataSourceTransactionManager.class); + DataSourceTransactionManager transactionManager = this.context + .getBean(DataSourceTransactionManager.class); assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java index ff67fcf246..a45cbba8d5 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java @@ -49,6 +49,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Dave Syer * @author Phillip Webb * @author Andy Wilkinson + * @author Kazuki Shimizu */ public class HibernateJpaAutoConfigurationTests extends AbstractJpaAutoConfigurationTests { @@ -175,11 +176,12 @@ public class HibernateJpaAutoConfigurationTests @Test public void testCustomJpaTransactionManagerUsingProperties() throws Exception { EnvironmentTestUtils.addEnvironment(this.context, - "spring.transaction.default-timeout:30", - "spring.transaction.rollback-on-commit-failure:true"); + "spring.jpa.transaction.default-timeout:30", + "spring.jpa.transaction.rollback-on-commit-failure:true"); setupTestConfiguration(); this.context.refresh(); - JpaTransactionManager transactionManager = context.getBean(JpaTransactionManager.class); + JpaTransactionManager transactionManager = this.context + .getBean(JpaTransactionManager.class); assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/transaction/jta/JtaAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/transaction/jta/JtaAutoConfigurationTests.java index 2193e59155..70239874a6 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/transaction/jta/JtaAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/transaction/jta/JtaAutoConfigurationTests.java @@ -69,6 +69,7 @@ import static org.mockito.Mockito.mock; * @author Josh Long * @author Phillip Webb * @author Andy Wilkinson + * @author Kazuki Shimizu */ public class JtaAutoConfigurationTests { @@ -249,11 +250,12 @@ public class JtaAutoConfigurationTests { public void atomikosCustomizeJtaTransactionManagerUsingProperties() throws Exception { this.context = new AnnotationConfigApplicationContext(); EnvironmentTestUtils.addEnvironment(this.context, - "spring.transaction.default-timeout:30", - "spring.transaction.rollback-on-commit-failure:true"); + "spring.jta.transaction.default-timeout:30", + "spring.jta.transaction.rollback-on-commit-failure:true"); this.context.register(AtomikosJtaConfiguration.class); this.context.refresh(); - JtaTransactionManager transactionManager = this.context.getBean(JtaTransactionManager.class); + JtaTransactionManager transactionManager = this.context + .getBean(JtaTransactionManager.class); assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); } @@ -262,11 +264,12 @@ public class JtaAutoConfigurationTests { public void bitronixCustomizeJtaTransactionManagerUsingProperties() throws Exception { this.context = new AnnotationConfigApplicationContext(); EnvironmentTestUtils.addEnvironment(this.context, - "spring.transaction.default-timeout:30", - "spring.transaction.rollback-on-commit-failure:true"); + "spring.jta.transaction.default-timeout:30", + "spring.jta.transaction.rollback-on-commit-failure:true"); this.context.register(BitronixJtaConfiguration.class); this.context.refresh(); - JtaTransactionManager transactionManager = this.context.getBean(JtaTransactionManager.class); + JtaTransactionManager transactionManager = this.context + .getBean(JtaTransactionManager.class); assertThat(transactionManager.getDefaultTimeout()).isEqualTo(30); assertThat(transactionManager.isRollbackOnCommitFailure()).isTrue(); } diff --git a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index 43267fbfce..196d51c2f4 100644 --- a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -582,6 +582,7 @@ content into your application; rather pick only the properties that you need. spring.data.neo4j.open-in-view=false # Register OpenSessionInViewInterceptor. Binds a Neo4j Session to the thread for the entire processing of the request. spring.data.neo4j.password= # Login password of the server. spring.data.neo4j.repositories.enabled=true # Enable Neo4j repositories. + spring.data.neo4j.transaction.*= # Transaction manager settings spring.data.neo4j.uri= # URI used by the driver. Auto-detected by default. spring.data.neo4j.username= # Login user of the server. @@ -623,6 +624,7 @@ content into your application; rather pick only the properties that you need. spring.datasource.separator=; # Statement separator in SQL initialization scripts. spring.datasource.sql-script-encoding= # SQL scripts encoding. spring.datasource.tomcat.*= # Tomcat datasource specific settings + spring.datasource.transaction.*= # Transaction manager settings spring.datasource.type= # Fully qualified name of the connection pool implementation to use. By default, it is auto-detected from the classpath. spring.datasource.url= # JDBC url of the database. spring.datasource.username= @@ -658,10 +660,12 @@ content into your application; rather pick only the properties that you need. spring.jpa.open-in-view=true # Register OpenEntityManagerInViewInterceptor. Binds a JPA EntityManager to the thread for the entire processing of the request. spring.jpa.properties.*= # Additional native properties to set on the JPA provider. spring.jpa.show-sql=false # Enable logging of SQL statements. + spring.jpa.transaction.*= # Transaction manager settings # JTA ({sc-spring-boot-autoconfigure}/transaction/jta/JtaAutoConfiguration.{sc-ext}[JtaAutoConfiguration]) spring.jta.enabled=true # Enable JTA support. spring.jta.log-dir= # Transaction logs directory. + spring.jta.transaction.*= # Transaction manager settings spring.jta.transaction-manager-id= # Transaction manager unique identifier. # ATOMIKOS ({sc-spring-boot}/jta/atomikos/AtomikosProperties.{sc-ext}[AtomikosProperties]) @@ -844,6 +848,7 @@ content into your application; rather pick only the properties that you need. spring.batch.job.names= # Comma-separated list of job names to execute on startup (For instance `job1,job2`). By default, all Jobs found in the context are executed. spring.batch.schema=classpath:org/springframework/batch/core/schema-@@platform@@.sql # Path to the SQL file to use to initialize the database schema. spring.batch.table-prefix= # Table prefix for all the batch meta-data tables. + spring.batch.transaction.*= # Transaction manager settings # JMS ({sc-spring-boot-autoconfigure}/jms/JmsProperties.{sc-ext}[JmsProperties]) spring.jms.jndi-name= # Connection factory JNDI name. When set, takes precedence to others connection factory auto-configurations. From bdda4703050b3d8dbac2312a44174e5e5c55716b Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 16 Dec 2016 12:23:06 -0500 Subject: [PATCH 07/16] Support arbitrary Kafka properties Add support for arbitrary Kafka properties via `spring.kafka.properties.*` and also a `spring.kafka.max.poll.records` property. See gh-7672 --- .../autoconfigure/kafka/KafkaProperties.java | 32 +++++ .../kafka/KafkaAutoConfigurationTests.java | 9 ++ .../appendix-application-properties.adoc | 2 + .../main/asciidoc/spring-boot-features.adoc | 22 +-- ...aSpecialProducerConsumerConfigExample.java | 125 ++++++++++++++++++ 5 files changed, 179 insertions(+), 11 deletions(-) create mode 100644 spring-boot-docs/src/main/java/org/springframework/boot/kafka/KafkaSpecialProducerConsumerConfigExample.java diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/kafka/KafkaProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/kafka/KafkaProperties.java index c6ed49f490..c7e9fd4529 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/kafka/KafkaProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/kafka/KafkaProperties.java @@ -71,6 +71,11 @@ public class KafkaProperties { */ private String clientId; + /** + * Additional properties used to configure the client. + */ + private Map properties = new HashMap(); + public Consumer getConsumer() { return this.consumer; } @@ -107,6 +112,14 @@ public class KafkaProperties { this.clientId = clientId; } + public Map getProperties() { + return this.properties; + } + + public void setProperties(Map properties) { + this.properties = properties; + } + private Map buildCommonProperties() { Map properties = new HashMap(); if (this.bootstrapServers != null) { @@ -135,6 +148,9 @@ public class KafkaProperties { properties.put(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG, this.ssl.getTruststorePassword()); } + if (this.properties != null && this.properties.size() > 0) { + properties.putAll(this.properties); + } return properties; } @@ -240,6 +256,11 @@ public class KafkaProperties { */ private Class valueDeserializer = StringDeserializer.class; + /** + * Maximum number of records returned in a single call to poll(). + */ + private Integer maxPollRecords; + public Ssl getSsl() { return this.ssl; } @@ -332,6 +353,14 @@ public class KafkaProperties { this.valueDeserializer = valueDeserializer; } + public Integer getMaxPollRecords() { + return this.maxPollRecords; + } + + public void setMaxPollRecords(Integer maxPollRecords) { + this.maxPollRecords = maxPollRecords; + } + public Map buildProperties() { Map properties = new HashMap(); if (this.autoCommitInterval != null) { @@ -395,6 +424,9 @@ public class KafkaProperties { properties.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, this.valueDeserializer); } + if (this.maxPollRecords != null) { + properties.put(ConsumerConfig.MAX_POLL_RECORDS_CONFIG, this.maxPollRecords); + } return properties; } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/kafka/KafkaAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/kafka/KafkaAutoConfigurationTests.java index db3a8d8bae..63b92b8a5d 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/kafka/KafkaAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/kafka/KafkaAutoConfigurationTests.java @@ -61,12 +61,16 @@ public class KafkaAutoConfigurationTests { @Test public void consumerProperties() { load("spring.kafka.bootstrap-servers=foo:1234", + "spring.kafka.properties.foo=bar", + "spring.kafka.properties.baz=qux", + "spring.kafka.properties.foo.bar.baz=qux.fiz.buz", "spring.kafka.ssl.key-password=p1", "spring.kafka.ssl.keystore-location=classpath:ksLoc", "spring.kafka.ssl.keystore-password=p2", "spring.kafka.ssl.truststore-location=classpath:tsLoc", "spring.kafka.ssl.truststore-password=p3", "spring.kafka.consumer.auto-commit-interval=123", + "spring.kafka.consumer.max-poll-records=42", "spring.kafka.consumer.auto-offset-reset=earliest", "spring.kafka.consumer.client-id=ccid", // test override common "spring.kafka.consumer.enable-auto-commit=false", @@ -109,6 +113,11 @@ public class KafkaAutoConfigurationTests { .isEqualTo(LongDeserializer.class); assertThat(configs.get(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG)) .isEqualTo(IntegerDeserializer.class); + assertThat(configs.get(ConsumerConfig.MAX_POLL_RECORDS_CONFIG)) + .isEqualTo(42); + assertThat(configs.get("foo")).isEqualTo("bar"); + assertThat(configs.get("baz")).isEqualTo("qux"); + assertThat(configs.get("foo.bar.baz")).isEqualTo("qux.fiz.buz"); } @Test diff --git a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index 196d51c2f4..47008f6b67 100644 --- a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -878,6 +878,7 @@ content into your application; rather pick only the properties that you need. spring.kafka.consumer.group-id= # Unique string that identifies the consumer group this consumer belongs to. spring.kafka.consumer.heartbeat-interval= # Expected time in milliseconds between heartbeats to the consumer coordinator. spring.kafka.consumer.key-deserializer= # Deserializer class for keys. + spring.kafka.consumer.max-poll-messages= # Maximum number of records returned in a single call to poll(). spring.kafka.consumer.value-deserializer= # Deserializer class for values. spring.kafka.listener.ack-count= # Number of records between offset commits when ackMode is "COUNT" or "COUNT_TIME". spring.kafka.listener.ack-mode= # Listener AckMode; see the spring-kafka documentation. @@ -893,6 +894,7 @@ content into your application; rather pick only the properties that you need. spring.kafka.producer.key-serializer= # Serializer class for keys. spring.kafka.producer.retries= # When greater than zero, enables retrying of failed sends. spring.kafka.producer.value-serializer= # Serializer class for values. + spring.kafka.properties.*= # Additional properties used to configure the client. spring.kafka.ssl.key-password= # Password of the private key in the key store file. spring.kafka.ssl.keystore-location= # Location of the key store file. spring.kafka.ssl.keystore-password= # Store password for the key store file. diff --git a/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index 1ae03b95ab..3e9c4d5cfa 100644 --- a/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -4643,22 +4643,22 @@ auto configuration supports all HIGH importance properties, some selected MEDIUM and any that do not have a default value. Only a subset of the properties supported by Kafka are available via the `KafkaProperties` -class. If you wish to configure the producer or consumer with additional properties, you -can override the producer factory and/or consumer factory bean, adding additional -properties, for example: +class. If you wish to configure the producer or consumer with additional properties that +are not directly supported, use the following: + +`spring.kafka.properties.foo.bar=baz` + +This sets the common `foo.bar` kafka property to `baz`. + +These properties will be shared by both the consumer and producer factory beans. +If you wish to customize these components with different properties, such as to use a +different metrics reader for each, you can override the bean definitions, as follows: [source,java,indent=0] ---- - @Bean - public ProducerFactory kafkaProducerFactory(KafkaProperties properties) { - Map producerProperties = properties.buildProducerProperties(); - producerProperties.put("some.property", "some.value"); - return new DefaultKafkaProducerFactory(producerProperties); - } +include::{code-examples}/kafka/KafkaSpecialProducerConsumerConfigExample.java[tag=configuration] ---- - - [[boot-features-restclient]] == Calling REST services If you need to call remote REST services from your application, you can use Spring diff --git a/spring-boot-docs/src/main/java/org/springframework/boot/kafka/KafkaSpecialProducerConsumerConfigExample.java b/spring-boot-docs/src/main/java/org/springframework/boot/kafka/KafkaSpecialProducerConsumerConfigExample.java new file mode 100644 index 0000000000..43dfc60ce6 --- /dev/null +++ b/spring-boot-docs/src/main/java/org/springframework/boot/kafka/KafkaSpecialProducerConsumerConfigExample.java @@ -0,0 +1,125 @@ +/* + * Copyright 2016-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.kafka; + +import java.util.List; +import java.util.Map; + +import org.apache.kafka.clients.CommonClientConfigs; +import org.apache.kafka.common.metrics.KafkaMetric; +import org.apache.kafka.common.metrics.MetricsReporter; + +import org.springframework.boot.autoconfigure.kafka.KafkaProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.kafka.core.ConsumerFactory; +import org.springframework.kafka.core.DefaultKafkaConsumerFactory; +import org.springframework.kafka.core.DefaultKafkaProducerFactory; +import org.springframework.kafka.core.ProducerFactory; + +/** + * Example custom kafka configuration beans used when the user wants to + * apply different common properties to the producer and consumer. + * + * @author Gary Russell + * @since 1.5 + * + */ +public class KafkaSpecialProducerConsumerConfigExample { + + // tag::configuration[] + @Configuration + public static class CustomKafkaBeans { + + /** + * Customized ProducerFactory bean. + * @param properties the kafka properties. + * @return the bean. + */ + @Bean + public ProducerFactory kafkaProducerFactory(KafkaProperties properties) { + Map producerProperties = properties.buildProducerProperties(); + producerProperties.put(CommonClientConfigs.METRIC_REPORTER_CLASSES_CONFIG, + MyProducerMetricsReporter.class); + return new DefaultKafkaProducerFactory(producerProperties); + } + + /** + * Customized ConsumerFactory bean. + * @param properties the kafka properties. + * @return the bean. + */ + @Bean + public ConsumerFactory kafkaConsumerFactory(KafkaProperties properties) { + Map consumererProperties = properties.buildConsumerProperties(); + consumererProperties.put(CommonClientConfigs.METRIC_REPORTER_CLASSES_CONFIG, + MyConsumerMetricsReporter.class); + return new DefaultKafkaConsumerFactory(consumererProperties); + } + + } + // end::configuration[] + + public static class MyConsumerMetricsReporter implements MetricsReporter { + + @Override + public void configure(Map configs) { + } + + @Override + public void init(List metrics) { + } + + @Override + public void metricChange(KafkaMetric metric) { + } + + @Override + public void metricRemoval(KafkaMetric metric) { + } + + @Override + public void close() { + } + + } + + public static class MyProducerMetricsReporter implements MetricsReporter { + + @Override + public void configure(Map configs) { + } + + @Override + public void init(List metrics) { + } + + @Override + public void metricChange(KafkaMetric metric) { + } + + @Override + public void metricRemoval(KafkaMetric metric) { + } + + @Override + public void close() { + } + + } + +} From 1f7b3cad4515c9836753afc16f4dcf1875784a9c Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Dec 2016 18:14:14 -0800 Subject: [PATCH 08/16] Polish Kafka properties Closes gh-7672 --- .../autoconfigure/kafka/KafkaProperties.java | 70 +++++++++---------- .../kafka/KafkaAutoConfigurationTests.java | 6 +- .../main/asciidoc/spring-boot-features.adoc | 9 ++- ...aSpecialProducerConsumerConfigExample.java | 8 +-- 4 files changed, 48 insertions(+), 45 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/kafka/KafkaProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/kafka/KafkaProperties.java index c7e9fd4529..d4e0774400 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/kafka/KafkaProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/kafka/KafkaProperties.java @@ -33,6 +33,7 @@ import org.apache.kafka.common.serialization.StringSerializer; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.core.io.Resource; import org.springframework.kafka.listener.AbstractMessageListenerContainer.AckMode; +import org.springframework.util.CollectionUtils; /** * Configuration properties for Spring for Apache Kafka. @@ -47,18 +48,6 @@ import org.springframework.kafka.listener.AbstractMessageListenerContainer.AckMo @ConfigurationProperties(prefix = "spring.kafka") public class KafkaProperties { - private final Consumer consumer = new Consumer(); - - private final Producer producer = new Producer(); - - private final Listener listener = new Listener(); - - private final Template template = new Template(); - - private final Ssl ssl = new Ssl(); - - // Apache Kafka Common Properties - /** * Comma-delimited list of host:port pairs to use for establishing the initial * connection to the Kafka cluster. @@ -76,25 +65,15 @@ public class KafkaProperties { */ private Map properties = new HashMap(); - public Consumer getConsumer() { - return this.consumer; - } + private final Consumer consumer = new Consumer(); - public Producer getProducer() { - return this.producer; - } + private final Producer producer = new Producer(); - public Listener getListener() { - return this.listener; - } + private final Listener listener = new Listener(); - public Ssl getSsl() { - return this.ssl; - } + private final Ssl ssl = new Ssl(); - public Template getTemplate() { - return this.template; - } + private final Template template = new Template(); public List getBootstrapServers() { return this.bootstrapServers; @@ -120,6 +99,26 @@ public class KafkaProperties { this.properties = properties; } + public Consumer getConsumer() { + return this.consumer; + } + + public Producer getProducer() { + return this.producer; + } + + public Listener getListener() { + return this.listener; + } + + public Ssl getSsl() { + return this.ssl; + } + + public Template getTemplate() { + return this.template; + } + private Map buildCommonProperties() { Map properties = new HashMap(); if (this.bootstrapServers != null) { @@ -148,7 +147,7 @@ public class KafkaProperties { properties.put(SslConfigs.SSL_TRUSTSTORE_PASSWORD_CONFIG, this.ssl.getTruststorePassword()); } - if (this.properties != null && this.properties.size() > 0) { + if (!CollectionUtils.isEmpty(this.properties)) { properties.putAll(this.properties); } return properties; @@ -163,9 +162,9 @@ public class KafkaProperties { * instance */ public Map buildConsumerProperties() { - Map props = buildCommonProperties(); - props.putAll(this.consumer.buildProperties()); - return props; + Map properties = buildCommonProperties(); + properties.putAll(this.consumer.buildProperties()); + return properties; } /** @@ -177,9 +176,9 @@ public class KafkaProperties { * instance */ public Map buildProducerProperties() { - Map props = buildCommonProperties(); - props.putAll(this.producer.buildProperties()); - return props; + Map properties = buildCommonProperties(); + properties.putAll(this.producer.buildProperties()); + return properties; } private static String resourceToPath(Resource resource) { @@ -425,7 +424,8 @@ public class KafkaProperties { this.valueDeserializer); } if (this.maxPollRecords != null) { - properties.put(ConsumerConfig.MAX_POLL_RECORDS_CONFIG, this.maxPollRecords); + properties.put(ConsumerConfig.MAX_POLL_RECORDS_CONFIG, + this.maxPollRecords); } return properties; } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/kafka/KafkaAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/kafka/KafkaAutoConfigurationTests.java index 63b92b8a5d..dc27e38da6 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/kafka/KafkaAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/kafka/KafkaAutoConfigurationTests.java @@ -60,8 +60,7 @@ public class KafkaAutoConfigurationTests { @Test public void consumerProperties() { - load("spring.kafka.bootstrap-servers=foo:1234", - "spring.kafka.properties.foo=bar", + load("spring.kafka.bootstrap-servers=foo:1234", "spring.kafka.properties.foo=bar", "spring.kafka.properties.baz=qux", "spring.kafka.properties.foo.bar.baz=qux.fiz.buz", "spring.kafka.ssl.key-password=p1", @@ -113,8 +112,7 @@ public class KafkaAutoConfigurationTests { .isEqualTo(LongDeserializer.class); assertThat(configs.get(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG)) .isEqualTo(IntegerDeserializer.class); - assertThat(configs.get(ConsumerConfig.MAX_POLL_RECORDS_CONFIG)) - .isEqualTo(42); + assertThat(configs.get(ConsumerConfig.MAX_POLL_RECORDS_CONFIG)).isEqualTo(42); assertThat(configs.get("foo")).isEqualTo("bar"); assertThat(configs.get("baz")).isEqualTo("qux"); assertThat(configs.get("foo.bar.baz")).isEqualTo("qux.fiz.buz"); diff --git a/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index 3e9c4d5cfa..a10482548e 100644 --- a/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -4646,9 +4646,12 @@ Only a subset of the properties supported by Kafka are available via the `KafkaP class. If you wish to configure the producer or consumer with additional properties that are not directly supported, use the following: -`spring.kafka.properties.foo.bar=baz` +[source,properties,indent=0] +---- + spring.kafka.properties.foo.bar=baz +---- -This sets the common `foo.bar` kafka property to `baz`. +This sets the common `foo.bar` Kafka property to `baz`. These properties will be shared by both the consumer and producer factory beans. If you wish to customize these components with different properties, such as to use a @@ -4659,6 +4662,8 @@ different metrics reader for each, you can override the bean definitions, as fol include::{code-examples}/kafka/KafkaSpecialProducerConsumerConfigExample.java[tag=configuration] ---- + + [[boot-features-restclient]] == Calling REST services If you need to call remote REST services from your application, you can use Spring diff --git a/spring-boot-docs/src/main/java/org/springframework/boot/kafka/KafkaSpecialProducerConsumerConfigExample.java b/spring-boot-docs/src/main/java/org/springframework/boot/kafka/KafkaSpecialProducerConsumerConfigExample.java index 43dfc60ce6..23a7aa139d 100644 --- a/spring-boot-docs/src/main/java/org/springframework/boot/kafka/KafkaSpecialProducerConsumerConfigExample.java +++ b/spring-boot-docs/src/main/java/org/springframework/boot/kafka/KafkaSpecialProducerConsumerConfigExample.java @@ -32,12 +32,11 @@ import org.springframework.kafka.core.DefaultKafkaProducerFactory; import org.springframework.kafka.core.ProducerFactory; /** - * Example custom kafka configuration beans used when the user wants to - * apply different common properties to the producer and consumer. + * Example custom kafka configuration beans used when the user wants to apply different + * common properties to the producer and consumer. * * @author Gary Russell * @since 1.5 - * */ public class KafkaSpecialProducerConsumerConfigExample { @@ -65,7 +64,8 @@ public class KafkaSpecialProducerConsumerConfigExample { */ @Bean public ConsumerFactory kafkaConsumerFactory(KafkaProperties properties) { - Map consumererProperties = properties.buildConsumerProperties(); + Map consumererProperties = properties + .buildConsumerProperties(); consumererProperties.put(CommonClientConfigs.METRIC_REPORTER_CLASSES_CONFIG, MyConsumerMetricsReporter.class); return new DefaultKafkaConsumerFactory(consumererProperties); From 34712cbf76b2f965abc95dcc947e41c98f405c1a Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Thu, 15 Dec 2016 03:09:10 -0800 Subject: [PATCH 09/16] Switch CF management skip SSL to opt-in Change CloudFoundryActuatorAutoConfiguration so that skipping of SSL verification is now opt-in rather than enabled by default. Fixes gh-7629 Closes gh-7655 --- ...CloudFoundryActuatorAutoConfiguration.java | 6 +++-- .../CloudFoundrySecurityService.java | 9 ++++--- ...FoundryActuatorAutoConfigurationTests.java | 17 ++++++++++++ .../CloudFoundrySecurityServiceTests.java | 27 ++++++++++++++++++- 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfiguration.java index 9df978a816..6026654ecd 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfiguration.java @@ -84,9 +84,11 @@ public class CloudFoundryActuatorAutoConfiguration { private CloudFoundrySecurityService getCloudFoundrySecurityService( RestTemplateBuilder restTemplateBuilder, Environment environment) { String cloudControllerUrl = environment.getProperty("vcap.application.cf_api"); + boolean skipSslValidation = Boolean.parseBoolean( + environment.getProperty("management.cloudfoundry.skipSslValidation")); return cloudControllerUrl == null ? null - : new CloudFoundrySecurityService(restTemplateBuilder, - cloudControllerUrl); + : new CloudFoundrySecurityService(restTemplateBuilder, cloudControllerUrl, + skipSslValidation); } private CorsConfiguration getCorsConfiguration() { diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityService.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityService.java index ce6126ba88..723b53a7e9 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityService.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityService.java @@ -46,11 +46,14 @@ class CloudFoundrySecurityService { private String uaaUrl; CloudFoundrySecurityService(RestTemplateBuilder restTemplateBuilder, - String cloudControllerUrl) { + String cloudControllerUrl, boolean skipSslValidation) { Assert.notNull(restTemplateBuilder, "RestTemplateBuilder must not be null"); Assert.notNull(cloudControllerUrl, "CloudControllerUrl must not be null"); - this.restTemplate = restTemplateBuilder - .requestFactory(SkipSslVerificationHttpRequestFactory.class).build(); + if (skipSslValidation) { + restTemplateBuilder = restTemplateBuilder + .requestFactory(SkipSslVerificationHttpRequestFactory.class); + } + this.restTemplate = restTemplateBuilder.build(); this.cloudControllerUrl = cloudControllerUrl; } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfigurationTests.java index 5de230a353..374bdb977b 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfigurationTests.java @@ -42,6 +42,7 @@ import org.springframework.mock.web.MockServletContext; import org.springframework.security.config.annotation.web.builders.WebSecurity.IgnoredRequestConfigurer; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.client.RestTemplate; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import org.springframework.web.cors.CorsConfiguration; @@ -117,6 +118,22 @@ public class CloudFoundryActuatorAutoConfigurationTests { assertThat(cloudControllerUrl).isEqualTo("http://my-cloud-controller.com"); } + @Test + public void skipSslValidation() throws Exception { + EnvironmentTestUtils.addEnvironment(this.context, + "management.cloudfoundry.skipSslValidation:true"); + this.context.refresh(); + CloudFoundryEndpointHandlerMapping handlerMapping = getHandlerMapping(); + Object interceptor = ReflectionTestUtils.getField(handlerMapping, + "securityInterceptor"); + Object interceptorSecurityService = ReflectionTestUtils.getField(interceptor, + "cloudFoundrySecurityService"); + RestTemplate restTemplate = (RestTemplate) ReflectionTestUtils + .getField(interceptorSecurityService, "restTemplate"); + assertThat(restTemplate.getRequestFactory()) + .isInstanceOf(SkipSslVerificationHttpRequestFactory.class); + } + @Test public void cloudFoundryPlatformActiveAndCloudControllerUrlNotPresent() throws Exception { diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityServiceTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityServiceTests.java index 585b69a759..0f80c90fef 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityServiceTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityServiceTests.java @@ -28,7 +28,9 @@ import org.springframework.boot.test.web.client.MockServerRestTemplateCustomizer import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.test.web.client.MockRestServiceServer; +import org.springframework.web.client.RestTemplate; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.client.match.MockRestRequestMatchers.header; @@ -63,10 +65,33 @@ public class CloudFoundrySecurityServiceTests { public void setup() throws Exception { MockServerRestTemplateCustomizer mockServerCustomizer = new MockServerRestTemplateCustomizer(); RestTemplateBuilder builder = new RestTemplateBuilder(mockServerCustomizer); - this.securityService = new CloudFoundrySecurityService(builder, CLOUD_CONTROLLER); + this.securityService = new CloudFoundrySecurityService(builder, CLOUD_CONTROLLER, + false); this.server = mockServerCustomizer.getServer(); } + @Test + public void skipSslValidationWhenTrue() throws Exception { + RestTemplateBuilder builder = new RestTemplateBuilder(); + this.securityService = new CloudFoundrySecurityService(builder, CLOUD_CONTROLLER, + true); + RestTemplate restTemplate = (RestTemplate) ReflectionTestUtils + .getField(this.securityService, "restTemplate"); + assertThat(restTemplate.getRequestFactory()) + .isInstanceOf(SkipSslVerificationHttpRequestFactory.class); + } + + @Test + public void doNotskipSslValidationWhenFalse() throws Exception { + RestTemplateBuilder builder = new RestTemplateBuilder(); + this.securityService = new CloudFoundrySecurityService(builder, CLOUD_CONTROLLER, + false); + RestTemplate restTemplate = (RestTemplate) ReflectionTestUtils + .getField(this.securityService, "restTemplate"); + assertThat(restTemplate.getRequestFactory()) + .isNotInstanceOf(SkipSslVerificationHttpRequestFactory.class); + } + @Test public void getAccessLevelWhenSpaceDeveloperShouldReturnFull() throws Exception { String responseBody = "{\"read_sensitive_data\": true,\"read_basic_data\": true}"; From dba8ef2ba83ac75b94a3aeb992724f3225eb1823 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Dec 2016 18:25:32 -0800 Subject: [PATCH 10/16] Polish CF management skip SSL opt-in See gh-7629 See gh-7655 --- .../CloudFoundryActuatorAutoConfiguration.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfiguration.java index 6026654ecd..460891dec2 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryActuatorAutoConfiguration.java @@ -30,6 +30,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnCloudPlatform; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.security.IgnoredRequestCustomizer; +import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.boot.cloud.CloudPlatform; import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.context.annotation.Bean; @@ -83,9 +84,11 @@ public class CloudFoundryActuatorAutoConfiguration { private CloudFoundrySecurityService getCloudFoundrySecurityService( RestTemplateBuilder restTemplateBuilder, Environment environment) { + RelaxedPropertyResolver cloudFoundryProperties = new RelaxedPropertyResolver( + environment, "management.cloudfoundry."); String cloudControllerUrl = environment.getProperty("vcap.application.cf_api"); - boolean skipSslValidation = Boolean.parseBoolean( - environment.getProperty("management.cloudfoundry.skipSslValidation")); + boolean skipSslValidation = cloudFoundryProperties + .getProperty("skip-ssl-validation", Boolean.class, false); return cloudControllerUrl == null ? null : new CloudFoundrySecurityService(restTemplateBuilder, cloudControllerUrl, skipSslValidation); From 38eeae216604cca7bca730847a8bf45fe85392e8 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 13 Dec 2016 00:27:40 -0800 Subject: [PATCH 11/16] Send error with message from Endpoint MVC security Update `MvcEndpointSecurityInterceptor` to that it sends an error in the same way as Spring Security. Prior to this commit the `ErrorController` would not handle endpoint security errors. Fixes gh-7605 Closes gh-7634 --- .../mvc/MvcEndpointSecurityInterceptor.java | 18 +++++++++---- .../MvcEndpointSecurityInterceptorTests.java | 27 ++++++++++++++++--- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java index 08d4dba5c0..7800dd5e0c 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java @@ -59,17 +59,25 @@ public class MvcEndpointSecurityInterceptor extends HandlerInterceptorAdapter { return true; } } - setFailureResponseStatus(request, response); + sendFailureResponse(request, response); return false; } - private void setFailureResponseStatus(HttpServletRequest request, - HttpServletResponse response) { + private void sendFailureResponse(HttpServletRequest request, + HttpServletResponse response) throws Exception { if (request.getUserPrincipal() != null) { - response.setStatus(HttpStatus.FORBIDDEN.value()); + StringBuilder message = new StringBuilder(); + for (String role : this.roles) { + message.append(role).append(" "); + } + response.sendError(HttpStatus.FORBIDDEN.value(), + "Access is denied. User must have one of the these roles: " + + message.toString().trim()); } else { - response.setStatus(HttpStatus.UNAUTHORIZED.value()); + response.sendError(HttpStatus.UNAUTHORIZED.value(), + "Full authentication is required to access this resource. " + + "Consider adding Spring Security or set management.security.enabled to false."); } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java index 8586bdd7f7..bc8f347763 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java @@ -16,19 +16,24 @@ package org.springframework.boot.actuate.endpoint.mvc; +import java.security.Principal; import java.util.Arrays; import java.util.List; +import javax.servlet.http.HttpServletResponse; + import org.junit.Before; import org.junit.Test; import org.springframework.boot.actuate.endpoint.AbstractEndpoint; +import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockServletContext; import org.springframework.web.method.HandlerMethod; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * Tests for {@link MvcEndpointSecurityInterceptor}. @@ -47,7 +52,7 @@ public class MvcEndpointSecurityInterceptorTests { private MockHttpServletRequest request; - private MockHttpServletResponse response; + private HttpServletResponse response; private MockServletContext servletContext; @@ -62,7 +67,7 @@ public class MvcEndpointSecurityInterceptorTests { this.handlerMethod = new HandlerMethod(this.mvcEndpoint, "invoke"); this.servletContext = new MockServletContext(); this.request = new MockHttpServletRequest(this.servletContext); - this.response = new MockHttpServletResponse(); + this.response = mock(HttpServletResponse.class); } @Test @@ -87,11 +92,25 @@ public class MvcEndpointSecurityInterceptorTests { } @Test - public void sensitiveEndpointIfRoleIsNotPresentShouldNotAllowAccess() + public void sensitiveEndpointIfNotAuthenticatedShouldNotAllowAccess() throws Exception { + assertThat(this.securityInterceptor.preHandle(this.request, this.response, + this.handlerMethod)).isFalse(); + verify(this.response).sendError(HttpStatus.UNAUTHORIZED.value(), + "Full authentication is required to access this resource. " + + "Consider adding Spring Security or set management.security.enabled to false."); + } + + @Test + public void sensitiveEndpointIfRoleIsNotCorrectShouldNotAllowAccess() + throws Exception { + Principal principal = mock(Principal.class); + this.request.setUserPrincipal(principal); this.servletContext.declareRoles("HERO"); assertThat(this.securityInterceptor.preHandle(this.request, this.response, this.handlerMethod)).isFalse(); + verify(this.response).sendError(HttpStatus.FORBIDDEN.value(), + "Access is denied. User must have one of the these roles: SUPER_HERO"); } private static class TestEndpoint extends AbstractEndpoint { From c76bd2d81e633292e43b3f583f75e435375593ab Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Dec 2016 18:47:39 -0800 Subject: [PATCH 12/16] Refine error message from Endpoint MVC security Update the error message to return less information to the client. Details of how to disable security are now written to the log instead. See gh-7605 See gh-7634 --- .../mvc/MvcEndpointSecurityInterceptor.java | 31 ++++++++++++++----- .../MvcEndpointSecurityInterceptorTests.java | 14 +++++++-- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java index 7800dd5e0c..e9c6d3b785 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java @@ -17,11 +17,16 @@ package org.springframework.boot.actuate.endpoint.mvc; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.http.HttpStatus; +import org.springframework.util.StringUtils; import org.springframework.web.cors.CorsUtils; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; @@ -34,10 +39,15 @@ import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; */ public class MvcEndpointSecurityInterceptor extends HandlerInterceptorAdapter { + private static final Log logger = LogFactory + .getLog(MvcEndpointSecurityInterceptor.class); + private final boolean secure; private final List roles; + private AtomicBoolean loggedUnauthorizedAttempt = new AtomicBoolean(); + public MvcEndpointSecurityInterceptor(boolean secure, List roles) { this.secure = secure; this.roles = roles; @@ -66,18 +76,23 @@ public class MvcEndpointSecurityInterceptor extends HandlerInterceptorAdapter { private void sendFailureResponse(HttpServletRequest request, HttpServletResponse response) throws Exception { if (request.getUserPrincipal() != null) { - StringBuilder message = new StringBuilder(); - for (String role : this.roles) { - message.append(role).append(" "); - } + String roles = StringUtils.collectionToDelimitedString(this.roles, " "); response.sendError(HttpStatus.FORBIDDEN.value(), - "Access is denied. User must have one of the these roles: " - + message.toString().trim()); + "Access is denied. User must have one of the these roles: " + roles); } else { + logUnauthorizedAttempt(); response.sendError(HttpStatus.UNAUTHORIZED.value(), - "Full authentication is required to access this resource. " - + "Consider adding Spring Security or set management.security.enabled to false."); + "Full authentication is required to access this resource."); + } + } + + private void logUnauthorizedAttempt() { + if (this.loggedUnauthorizedAttempt.compareAndSet(false, true) + && logger.isInfoEnabled()) { + logger.info("Full authentication is required to access " + + "actuator endpoints. Consider adding Spring Security " + + "or set 'management.security.enabled' to false."); } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java index bc8f347763..cbfa698133 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java @@ -23,9 +23,11 @@ import java.util.List; import javax.servlet.http.HttpServletResponse; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.springframework.boot.actuate.endpoint.AbstractEndpoint; +import org.springframework.boot.test.rule.OutputCapture; import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockServletContext; @@ -42,6 +44,9 @@ import static org.mockito.Mockito.verify; */ public class MvcEndpointSecurityInterceptorTests { + @Rule + public OutputCapture output = new OutputCapture(); + private MvcEndpointSecurityInterceptor securityInterceptor; private TestMvcEndpoint mvcEndpoint; @@ -97,8 +102,13 @@ public class MvcEndpointSecurityInterceptorTests { assertThat(this.securityInterceptor.preHandle(this.request, this.response, this.handlerMethod)).isFalse(); verify(this.response).sendError(HttpStatus.UNAUTHORIZED.value(), - "Full authentication is required to access this resource. " - + "Consider adding Spring Security or set management.security.enabled to false."); + "Full authentication is required to access this resource."); + assertThat(this.securityInterceptor.preHandle(this.request, this.response, + this.handlerMethod)).isFalse(); + assertThat(this.output.toString()) + .containsOnlyOnce("Full authentication is required to access actuator " + + "endpoints. Consider adding Spring Security or set " + + "'management.security.enabled' to false"); } @Test From 38f7389eab7b2dd4f3758214a6aca743941658aa Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Dec 2016 19:07:16 -0800 Subject: [PATCH 13/16] Polish loggers --- .../CloudFoundrySecurityInterceptor.java | 5 ++-- .../resource/SpringSocialTokenServices.java | 5 ---- ...gurationReportLoggingInitializerTests.java | 4 ++++ .../org/springframework/boot/ImageBanner.java | 6 ++--- .../bind/PropertiesConfigurationFactory.java | 18 +++++++------- .../boot/bind/YamlConfigurationFactory.java | 24 +++++++++++-------- .../boot/diagnostics/FailureAnalyzers.java | 4 ++-- .../ClasspathLoggingApplicationListener.java | 10 ++++---- 8 files changed, 41 insertions(+), 35 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptor.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptor.java index ab13a0e148..38d0e47433 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptor.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityInterceptor.java @@ -38,7 +38,8 @@ import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; */ class CloudFoundrySecurityInterceptor extends HandlerInterceptorAdapter { - protected final Log logger = LogFactory.getLog(getClass()); + private static final Log logger = LogFactory + .getLog(CloudFoundrySecurityInterceptor.class); private final TokenValidator tokenValidator; @@ -74,7 +75,7 @@ class CloudFoundrySecurityInterceptor extends HandlerInterceptorAdapter { check(request, mvcEndpoint); } catch (CloudFoundryAuthorizationException ex) { - this.logger.error(ex); + logger.error(ex); response.setContentType(MediaType.APPLICATION_JSON.toString()); response.getWriter() .write("{\"security_error\":\"" + ex.getMessage() + "\"}"); diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/SpringSocialTokenServices.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/SpringSocialTokenServices.java index 6cadf238c3..f453493626 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/SpringSocialTokenServices.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/oauth2/resource/SpringSocialTokenServices.java @@ -18,9 +18,6 @@ package org.springframework.boot.autoconfigure.security.oauth2.resource; import java.util.List; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.GrantedAuthority; @@ -43,8 +40,6 @@ import org.springframework.social.oauth2.AccessGrant; */ public class SpringSocialTokenServices implements ResourceServerTokenServices { - protected final Log logger = LogFactory.getLog(getClass()); - private final OAuth2ConnectionFactory connectionFactory; private final String clientId; diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/logging/AutoConfigurationReportLoggingInitializerTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/logging/AutoConfigurationReportLoggingInitializerTests.java index 4e38cd9bba..5727d6cead 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/logging/AutoConfigurationReportLoggingInitializerTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/logging/AutoConfigurationReportLoggingInitializerTests.java @@ -81,20 +81,24 @@ public class AutoConfigurationReportLoggingInitializerTests { given(this.log.isDebugEnabled()).willReturn(debug); willAnswer(new Answer() { + @Override public Object answer(InvocationOnMock invocation) throws Throwable { return AutoConfigurationReportLoggingInitializerTests.this.debugLog .add(String.valueOf(invocation.getArguments()[0])); } + }).given(this.log).debug(anyObject()); given(this.log.isInfoEnabled()).willReturn(info); willAnswer(new Answer() { + @Override public Object answer(InvocationOnMock invocation) throws Throwable { return AutoConfigurationReportLoggingInitializerTests.this.infoLog .add(String.valueOf(invocation.getArguments()[0])); } + }).given(this.log).info(anyObject()); LogFactory.releaseAll(); diff --git a/spring-boot/src/main/java/org/springframework/boot/ImageBanner.java b/spring-boot/src/main/java/org/springframework/boot/ImageBanner.java index c6b2133e0b..0404b16ca3 100644 --- a/spring-boot/src/main/java/org/springframework/boot/ImageBanner.java +++ b/spring-boot/src/main/java/org/springframework/boot/ImageBanner.java @@ -49,7 +49,7 @@ import org.springframework.util.Assert; */ public class ImageBanner implements Banner { - private static final Log log = LogFactory.getLog(ImageBanner.class); + private static final Log logger = LogFactory.getLog(ImageBanner.class); private static final double[] RGB_WEIGHT = { 0.2126d, 0.7152d, 0.0722d }; @@ -76,9 +76,9 @@ public class ImageBanner implements Banner { printBanner(environment, out); } catch (Throwable ex) { - log.warn("Image banner not printable: " + this.image + " (" + ex.getClass() + logger.warn("Image banner not printable: " + this.image + " (" + ex.getClass() + ": '" + ex.getMessage() + "')"); - log.debug("Image banner printing failure", ex); + logger.debug("Image banner printing failure", ex); } finally { if (headless == null) { diff --git a/spring-boot/src/main/java/org/springframework/boot/bind/PropertiesConfigurationFactory.java b/spring-boot/src/main/java/org/springframework/boot/bind/PropertiesConfigurationFactory.java index f8bcaf0242..13f305da05 100644 --- a/spring-boot/src/main/java/org/springframework/boot/bind/PropertiesConfigurationFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/bind/PropertiesConfigurationFactory.java @@ -58,7 +58,8 @@ public class PropertiesConfigurationFactory private static final char[] TARGET_NAME_DELIMITERS = { '_', '.' }; - private final Log logger = LogFactory.getLog(getClass()); + private static final Log logger = LogFactory + .getLog(PropertiesConfigurationFactory.class); private boolean ignoreUnknownFields = true; @@ -228,8 +229,8 @@ public class PropertiesConfigurationFactory public void bindPropertiesToTarget() throws BindException { Assert.state(this.propertySources != null, "PropertySources should not be null"); try { - if (this.logger.isTraceEnabled()) { - this.logger.trace("Property Sources: " + this.propertySources); + if (logger.isTraceEnabled()) { + logger.trace("Property Sources: " + this.propertySources); } this.hasBeenBound = true; @@ -239,8 +240,9 @@ public class PropertiesConfigurationFactory if (this.exceptionIfInvalid) { throw ex; } - this.logger.error("Failed to load Properties validation bean. " - + "Your Properties may be invalid.", ex); + PropertiesConfigurationFactory.logger + .error("Failed to load Properties validation bean. " + + "Your Properties may be invalid.", ex); } } @@ -340,10 +342,10 @@ public class PropertiesConfigurationFactory dataBinder.validate(); BindingResult errors = dataBinder.getBindingResult(); if (errors.hasErrors()) { - this.logger.error("Properties configuration failed validation"); + logger.error("Properties configuration failed validation"); for (ObjectError error : errors.getAllErrors()) { - this.logger - .error(this.messageSource != null + logger.error( + this.messageSource != null ? this.messageSource.getMessage(error, Locale.getDefault()) + " (" + error + ")" : error); diff --git a/spring-boot/src/main/java/org/springframework/boot/bind/YamlConfigurationFactory.java b/spring-boot/src/main/java/org/springframework/boot/bind/YamlConfigurationFactory.java index 6ab2e551ea..fd45de3296 100644 --- a/spring-boot/src/main/java/org/springframework/boot/bind/YamlConfigurationFactory.java +++ b/spring-boot/src/main/java/org/springframework/boot/bind/YamlConfigurationFactory.java @@ -52,7 +52,7 @@ import org.springframework.validation.Validator; public class YamlConfigurationFactory implements FactoryBean, MessageSourceAware, InitializingBean { - private final Log logger = LogFactory.getLog(getClass()); + private static final Log logger = LogFactory.getLog(YamlConfigurationFactory.class); private final Class type; @@ -137,8 +137,8 @@ public class YamlConfigurationFactory Assert.state(this.yaml != null, "Yaml document should not be null: " + "either set it directly or set the resource to load it from"); try { - if (this.logger.isTraceEnabled()) { - this.logger.trace(String.format("Yaml document is %n%s", this.yaml)); + if (logger.isTraceEnabled()) { + logger.trace(String.format("Yaml document is %n%s", this.yaml)); } Constructor constructor = new YamlJavaBeanPropertyConstructor(this.type, this.propertyAliases); @@ -151,7 +151,7 @@ public class YamlConfigurationFactory if (this.exceptionIfInvalid) { throw ex; } - this.logger.error("Failed to load YAML validation bean. " + logger.error("Failed to load YAML validation bean. " + "Your YAML file may be invalid.", ex); } } @@ -161,13 +161,9 @@ public class YamlConfigurationFactory "configuration"); this.validator.validate(this.configuration, errors); if (errors.hasErrors()) { - this.logger.error("YAML configuration failed validation"); + logger.error("YAML configuration failed validation"); for (ObjectError error : errors.getAllErrors()) { - this.logger - .error(this.messageSource != null - ? this.messageSource.getMessage(error, - Locale.getDefault()) + " (" + error + ")" - : error); + logger.error(getErrorMessage(error)); } if (this.exceptionIfInvalid) { BindException summary = new BindException(errors); @@ -176,6 +172,14 @@ public class YamlConfigurationFactory } } + private Object getErrorMessage(ObjectError error) { + if (this.messageSource != null) { + Locale locale = Locale.getDefault(); + return this.messageSource.getMessage(error, locale) + " (" + error + ")"; + } + return error; + } + @Override public Class getObjectType() { if (this.configuration == null) { diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java index 75e19c9e07..2f9f800e91 100644 --- a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java @@ -48,7 +48,7 @@ import org.springframework.util.ReflectionUtils; */ public final class FailureAnalyzers { - private static final Log log = LogFactory.getLog(FailureAnalyzers.class); + private static final Log logger = LogFactory.getLog(FailureAnalyzers.class); private final ClassLoader classLoader; @@ -82,7 +82,7 @@ public final class FailureAnalyzers { analyzers.add((FailureAnalyzer) constructor.newInstance()); } catch (Throwable ex) { - log.trace("Failed to load " + analyzerName, ex); + logger.trace("Failed to load " + analyzerName, ex); } } AnnotationAwareOrderComparator.sort(analyzers); diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/ClasspathLoggingApplicationListener.java b/spring-boot/src/main/java/org/springframework/boot/logging/ClasspathLoggingApplicationListener.java index 3dcd654818..e31eea4080 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/ClasspathLoggingApplicationListener.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/ClasspathLoggingApplicationListener.java @@ -42,17 +42,17 @@ public final class ClasspathLoggingApplicationListener private static final int ORDER = LoggingApplicationListener.DEFAULT_ORDER + 1; - private final Log logger = LogFactory.getLog(getClass()); + private static final Log logger = LogFactory + .getLog(ClasspathLoggingApplicationListener.class); @Override public void onApplicationEvent(ApplicationEvent event) { - if (this.logger.isDebugEnabled()) { + if (logger.isDebugEnabled()) { if (event instanceof ApplicationEnvironmentPreparedEvent) { - this.logger - .debug("Application started with classpath: " + getClasspath()); + logger.debug("Application started with classpath: " + getClasspath()); } else if (event instanceof ApplicationFailedEvent) { - this.logger.debug( + logger.debug( "Application failed to start with classpath: " + getClasspath()); } } From c2992e3736eebe061e88b84f087fedd768f70af0 Mon Sep 17 00:00:00 2001 From: Hrishikesh Joshi Date: Thu, 1 Dec 2016 23:35:53 +1100 Subject: [PATCH 14/16] Add more debug logging to DevTools Add debug logging for the included and excluded URL patterns and matching URLs. Fixes gh-7478 Closes gh-7544 --- .../boot/devtools/restart/ChangeableUrls.java | 8 ++++++++ .../boot/devtools/settings/DevToolsSettings.java | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/ChangeableUrls.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/ChangeableUrls.java index 3b43a450ab..7d68419769 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/ChangeableUrls.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/ChangeableUrls.java @@ -30,6 +30,9 @@ import java.util.jar.Attributes; import java.util.jar.JarFile; import java.util.jar.Manifest; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.boot.devtools.settings.DevToolsSettings; import org.springframework.util.StringUtils; @@ -41,6 +44,8 @@ import org.springframework.util.StringUtils; */ final class ChangeableUrls implements Iterable { + private static final Log logger = LogFactory.getLog(ChangeableUrls.class); + private final List urls; private ChangeableUrls(URL... urls) { @@ -52,6 +57,9 @@ final class ChangeableUrls implements Iterable { reloadableUrls.add(url); } } + if (logger.isDebugEnabled()) { + logger.debug("Matching URLs for reloading : " + reloadableUrls); + } this.urls = Collections.unmodifiableList(reloadableUrls); } diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/settings/DevToolsSettings.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/settings/DevToolsSettings.java index 525ab6cd60..332bfeb756 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/settings/DevToolsSettings.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/settings/DevToolsSettings.java @@ -24,6 +24,9 @@ import java.util.List; import java.util.Map; import java.util.regex.Pattern; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.core.io.UrlResource; import org.springframework.core.io.support.PropertiesLoaderUtils; @@ -40,6 +43,8 @@ public class DevToolsSettings { */ public static final String SETTINGS_RESOURCE_LOCATION = "META-INF/spring-devtools.properties"; + private static final Log logger = LogFactory.getLog(DevToolsSettings.class); + private static DevToolsSettings settings; private final List restartIncludePatterns = new ArrayList(); @@ -105,6 +110,12 @@ public class DevToolsSettings { settings.add(PropertiesLoaderUtils .loadProperties(new UrlResource(urls.nextElement()))); } + if (logger.isDebugEnabled()) { + logger.debug("Included patterns for restart : " + + settings.restartIncludePatterns); + logger.debug("Excluded patterns for restart : " + + settings.restartExcludePatterns); + } return settings; } catch (Exception ex) { From 90eb58252e678f6ea37b321e9377ece254fc9473 Mon Sep 17 00:00:00 2001 From: Marco Aust Date: Tue, 15 Nov 2016 19:44:34 +0100 Subject: [PATCH 15/16] Add support for `spring.redis.url` property Update `RedisAutoConfiguration` to optionally configure Redis using a `spring.redis.url` property`. Closes gh-7395 --- .../data/redis/RedisAutoConfiguration.java | 36 ++++++++++++++++--- .../data/redis/RedisProperties.java | 27 ++++++++++++++ .../redis/RedisAutoConfigurationTests.java | 16 +++++++++ .../appendix-application-properties.adoc | 2 ++ 4 files changed, 77 insertions(+), 4 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.java index 618377f889..5a42e7fd81 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.java @@ -16,10 +16,14 @@ package org.springframework.boot.autoconfigure.data.redis; +import java.net.URI; +import java.net.URISyntaxException; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.List; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.commons.pool2.impl.GenericObjectPool; import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPoolConfig; @@ -55,12 +59,15 @@ import org.springframework.util.StringUtils; * @author Phillip Webb * @author Eddú Meléndez * @author Stephane Nicoll + * @author Marco Aust */ @Configuration @ConditionalOnClass({ JedisConnection.class, RedisOperations.class, Jedis.class }) @EnableConfigurationProperties(RedisProperties.class) public class RedisAutoConfiguration { + private static final Log logger = LogFactory.getLog(RedisAutoConfiguration.class); + /** * Redis connection configuration. */ @@ -91,10 +98,31 @@ public class RedisAutoConfiguration { protected final JedisConnectionFactory applyProperties( JedisConnectionFactory factory) { - factory.setHostName(this.properties.getHost()); - factory.setPort(this.properties.getPort()); - if (this.properties.getPassword() != null) { - factory.setPassword(this.properties.getPassword()); + if (StringUtils.hasText(this.properties.getUrl())) { + if (this.properties.getUrl().startsWith("rediss://")) { + factory.setUseSsl(true); + } + try { + URI redisURI = new URI(this.properties.getUrl()); + factory.setHostName(redisURI.getHost()); + factory.setPort(redisURI.getPort()); + if (redisURI.getUserInfo() != null) { + factory.setPassword(redisURI.getUserInfo().split(":", 2)[1]); + } + } + catch (URISyntaxException e) { + logger.error("Incorrect spring.redis.url", e); + } + } + else { + factory.setHostName(this.properties.getHost()); + factory.setPort(this.properties.getPort()); + if (this.properties.getPassword() != null) { + factory.setPassword(this.properties.getPassword()); + } + } + if (this.properties.isSsl()) { + factory.setUseSsl(true); } factory.setDatabase(this.properties.getDatabase()); if (this.properties.getTimeout() > 0) { diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java index 8c6d3ddfdf..cd182a485d 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java @@ -26,6 +26,7 @@ import org.springframework.boot.context.properties.ConfigurationProperties; * @author Dave Syer * @author Christoph Strobl * @author Eddú Meléndez + * @author Marco Aust */ @ConfigurationProperties(prefix = "spring.redis") public class RedisProperties { @@ -35,6 +36,11 @@ public class RedisProperties { */ private int database = 0; + /** + * Redis url, which will overrule host, port and password if set. + */ + private String url; + /** * Redis server host. */ @@ -50,6 +56,11 @@ public class RedisProperties { */ private int port = 6379; + /** + * Enable SSL. + */ + private boolean ssl; + /** * Connection timeout in milliseconds. */ @@ -69,6 +80,14 @@ public class RedisProperties { this.database = database; } + public String getUrl() { + return this.url; + } + + public void setUrl(String url) { + this.url = url; + } + public String getHost() { return this.host; } @@ -93,6 +112,14 @@ public class RedisProperties { this.port = port; } + public boolean isSsl() { + return this.ssl; + } + + public void setSsl(boolean ssl) { + this.ssl = ssl; + } + public void setTimeout(int timeout) { this.timeout = timeout; } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java index e75f3e9e85..831e7b2809 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java @@ -75,6 +75,22 @@ public class RedisAutoConfigurationTests { .isEqualTo(1); } + @Test + public void testOverrideURLRedisConfiguration() throws Exception { + load("spring.redis.host:foo", "spring.redis.password:xyz", + "spring.redis.port:1000", + "spring.redis.ssl:true", + "spring.redis.url:redis://user:password@example:33"); + assertThat(this.context.getBean(JedisConnectionFactory.class).getHostName()) + .isEqualTo("example"); + assertThat(this.context.getBean(JedisConnectionFactory.class).getPort()) + .isEqualTo(33); + assertThat(this.context.getBean(JedisConnectionFactory.class).getPassword()) + .isEqualTo("password"); + assertThat(this.context.getBean(JedisConnectionFactory.class).isUseSsl()) + .isEqualTo(true); + } + @Test public void testRedisConfigurationWithPool() throws Exception { load("spring.redis.host:foo", "spring.redis.pool.max-idle:1"); diff --git a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index 47008f6b67..ab2b8b3220 100644 --- a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -799,8 +799,10 @@ content into your application; rather pick only the properties that you need. spring.redis.cluster.max-redirects= # Maximum number of redirects to follow when executing commands across the cluster. spring.redis.cluster.nodes= # Comma-separated list of "host:port" pairs to bootstrap from. spring.redis.database=0 # Database index used by the connection factory. + spring.redis.url= # Connection URL, will override host, port and password (user will be ignored), e.g. redis://user:password@example.com:6379 spring.redis.host=localhost # Redis server host. spring.redis.password= # Login password of the redis server. + spring.redis.ssl=false # Enable SSL support. spring.redis.pool.max-active=8 # Max number of connections that can be allocated by the pool at a given time. Use a negative value for no limit. spring.redis.pool.max-idle=8 # Max number of "idle" connections in the pool. Use a negative value to indicate an unlimited number of idle connections. spring.redis.pool.max-wait=-1 # Maximum amount of time (in milliseconds) a connection allocation should block before throwing an exception when the pool is exhausted. Use a negative value to block indefinitely. From 8f7efbe12a9ee1a564b4a1959b057e1d92430105 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Dec 2016 20:57:50 -0800 Subject: [PATCH 16/16] Polish `spring.redis.url` support See gh-7395 --- .../data/redis/RedisAutoConfiguration.java | 65 +++++++++++-------- .../redis/RedisAutoConfigurationTests.java | 6 +- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.java index 5a42e7fd81..1c770888d5 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.java @@ -22,8 +22,6 @@ import java.net.UnknownHostException; import java.util.ArrayList; import java.util.List; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.commons.pool2.impl.GenericObjectPool; import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPoolConfig; @@ -66,8 +64,6 @@ import org.springframework.util.StringUtils; @EnableConfigurationProperties(RedisProperties.class) public class RedisAutoConfiguration { - private static final Log logger = LogFactory.getLog(RedisAutoConfiguration.class); - /** * Redis connection configuration. */ @@ -98,29 +94,7 @@ public class RedisAutoConfiguration { protected final JedisConnectionFactory applyProperties( JedisConnectionFactory factory) { - if (StringUtils.hasText(this.properties.getUrl())) { - if (this.properties.getUrl().startsWith("rediss://")) { - factory.setUseSsl(true); - } - try { - URI redisURI = new URI(this.properties.getUrl()); - factory.setHostName(redisURI.getHost()); - factory.setPort(redisURI.getPort()); - if (redisURI.getUserInfo() != null) { - factory.setPassword(redisURI.getUserInfo().split(":", 2)[1]); - } - } - catch (URISyntaxException e) { - logger.error("Incorrect spring.redis.url", e); - } - } - else { - factory.setHostName(this.properties.getHost()); - factory.setPort(this.properties.getPort()); - if (this.properties.getPassword() != null) { - factory.setPassword(this.properties.getPassword()); - } - } + configureConnection(factory); if (this.properties.isSsl()) { factory.setUseSsl(true); } @@ -131,6 +105,43 @@ public class RedisAutoConfiguration { return factory; } + private void configureConnection(JedisConnectionFactory factory) { + if (StringUtils.hasText(this.properties.getUrl())) { + configureConnectionFromUrl(factory); + } + else { + factory.setHostName(this.properties.getHost()); + factory.setPort(this.properties.getPort()); + if (this.properties.getPassword() != null) { + factory.setPassword(this.properties.getPassword()); + } + } + } + + private void configureConnectionFromUrl(JedisConnectionFactory factory) { + String url = this.properties.getUrl(); + if (url.startsWith("rediss://")) { + factory.setUseSsl(true); + } + try { + URI uri = new URI(url); + factory.setHostName(uri.getHost()); + factory.setPort(uri.getPort()); + if (uri.getUserInfo() != null) { + String password = uri.getUserInfo(); + int index = password.lastIndexOf(":"); + if (index >= 0) { + password = password.substring(index + 1); + } + factory.setPassword(password); + } + } + catch (URISyntaxException ex) { + throw new IllegalArgumentException("Malformed 'spring.redis.url' " + url, + ex); + } + } + protected final RedisSentinelConfiguration getSentinelConfig() { if (this.sentinelConfiguration != null) { return this.sentinelConfiguration; diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java index 831e7b2809..69be81f6dc 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java @@ -41,6 +41,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Christian Dupuis * @author Christoph Strobl * @author Eddú Meléndez + * @author Marco Aust */ public class RedisAutoConfigurationTests { @@ -76,10 +77,9 @@ public class RedisAutoConfigurationTests { } @Test - public void testOverrideURLRedisConfiguration() throws Exception { + public void testOverrideUrlRedisConfiguration() throws Exception { load("spring.redis.host:foo", "spring.redis.password:xyz", - "spring.redis.port:1000", - "spring.redis.ssl:true", + "spring.redis.port:1000", "spring.redis.ssl:true", "spring.redis.url:redis://user:password@example:33"); assertThat(this.context.getBean(JedisConnectionFactory.class).getHostName()) .isEqualTo("example");