From 3172d434a7b232763d6a18659db832a804ff7938 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 16 Sep 2016 11:54:50 -0700 Subject: [PATCH 1/4] Polish Closes gh-6835 --- .../boot/actuate/hypermedia/EndpointDocumentation.java | 6 +++--- .../autoconfigure/MetricFilterAutoConfiguration.java | 2 +- .../boot/actuate/health/JmsHealthIndicatorTests.java | 6 ++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/spring-boot-actuator-docs/src/restdoc/java/org/springframework/boot/actuate/hypermedia/EndpointDocumentation.java b/spring-boot-actuator-docs/src/restdoc/java/org/springframework/boot/actuate/hypermedia/EndpointDocumentation.java index 1a4b5b2b6d..8ff3b06c1a 100644 --- a/spring-boot-actuator-docs/src/restdoc/java/org/springframework/boot/actuate/hypermedia/EndpointDocumentation.java +++ b/spring-boot-actuator-docs/src/restdoc/java/org/springframework/boot/actuate/hypermedia/EndpointDocumentation.java @@ -80,8 +80,8 @@ public class EndpointDocumentation { private MvcEndpoints mvcEndpoints; @Autowired - @Qualifier("metricFilter") - private Filter metricFilter; + @Qualifier("metricsFilter") + private Filter metricsFilter; @Autowired @Qualifier("webRequestLoggingFilter") @@ -95,7 +95,7 @@ public class EndpointDocumentation { @Before public void setUp() { this.mockMvc = MockMvcBuilders.webAppContextSetup(this.context) - .addFilters(this.metricFilter, this.traceFilter) + .addFilters(this.metricsFilter, this.traceFilter) .apply(documentationConfiguration(this.restDocumentation)).build(); } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricFilterAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricFilterAutoConfiguration.java index 20d0453e1c..8c27975df6 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricFilterAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricFilterAutoConfiguration.java @@ -55,7 +55,7 @@ public class MetricFilterAutoConfiguration { private GaugeService gaugeService; @Bean - public MetricsFilter metricFilter() { + public MetricsFilter metricsFilter() { return new MetricsFilter(this.counterService, this.gaugeService); } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/JmsHealthIndicatorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/JmsHealthIndicatorTests.java index 76f2beffea..8221c8c251 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/JmsHealthIndicatorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/JmsHealthIndicatorTests.java @@ -86,10 +86,8 @@ public class JmsHealthIndicatorTests { given(connectionMetaData.getJMSProviderName()).willReturn("JMS test provider"); Connection connection = mock(Connection.class); given(connection.getMetaData()).willReturn(connectionMetaData); - willThrow(new JMSException("Could not start", "123")) - .given(connection).start(); - given(connectionFactory.createConnection()) - .willReturn(connection); + willThrow(new JMSException("Could not start", "123")).given(connection).start(); + given(connectionFactory.createConnection()).willReturn(connection); JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); Health health = indicator.health(); assertEquals(Status.DOWN, health.getStatus()); From a2e4127d4f7be77b4c7ea0428dbe663f0320f949 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 16 Sep 2016 13:07:44 -0700 Subject: [PATCH 2/4] Fix multi-annotation nested condition logic Update `AbstractNestedCondition` to correctly group nested conditions on members. Fixes gh-6672 --- .../condition/AbstractNestedCondition.java | 62 +++++++++++++++---- .../condition/AnyNestedConditionTests.java | 4 +- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/AbstractNestedCondition.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/AbstractNestedCondition.java index 4422b97548..4d0d043b44 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/AbstractNestedCondition.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/AbstractNestedCondition.java @@ -162,25 +162,65 @@ abstract class AbstractNestedCondition extends SpringBootCondition for (Map.Entry> entry : this.memberConditions .entrySet()) { AnnotationMetadata metadata = entry.getKey(); - for (Condition condition : entry.getValue()) { - outcomes.add(getConditionOutcome(metadata, condition)); - } + List conditions = entry.getValue(); + outcomes.add(new MemberOutcomes(this.context, metadata, conditions) + .getUltimateOutcome()); } return Collections.unmodifiableList(outcomes); } + } + + private static class MemberOutcomes { + + private final ConditionContext context; + + private final AnnotationMetadata metadata; + + private final List outcomes; + + MemberOutcomes(ConditionContext context, AnnotationMetadata metadata, + List conditions) { + this.context = context; + this.metadata = metadata; + this.outcomes = new ArrayList(conditions.size()); + for (Condition condition : conditions) { + this.outcomes.add(getConditionOutcome(metadata, condition)); + } + } + private ConditionOutcome getConditionOutcome(AnnotationMetadata metadata, Condition condition) { - String messagePrefix = "member condition on " + metadata.getClassName(); if (condition instanceof SpringBootCondition) { - ConditionOutcome outcome = ((SpringBootCondition) condition) - .getMatchOutcome(this.context, metadata); - String message = outcome.getMessage(); - return new ConditionOutcome(outcome.isMatch(), messagePrefix - + (StringUtils.hasLength(message) ? " : " + message : "")); + return ((SpringBootCondition) condition).getMatchOutcome(this.context, + metadata); } - boolean matches = condition.matches(this.context, metadata); - return new ConditionOutcome(matches, messagePrefix); + return new ConditionOutcome(condition.matches(this.context, metadata), null); + } + + public ConditionOutcome getUltimateOutcome() { + if (this.outcomes.size() == 1) { + ConditionOutcome outcome = this.outcomes.get(0); + StringBuilder message = new StringBuilder( + "member condition on " + this.metadata.getClassName()); + if (StringUtils.hasLength(outcome.getMessage())) { + message.append(" " + outcome.getMessage()); + } + return new ConditionOutcome(outcome.isMatch(), message.toString()); + } + StringBuilder message = new StringBuilder( + "member conditions on " + this.metadata.getClassName()); + boolean match = true; + boolean hasMessage = false; + for (ConditionOutcome outcome : this.outcomes) { + match &= outcome.isMatch(); + if (StringUtils.hasLength(outcome.getMessage())) { + message.append(hasMessage ? ", " : " : "); + message.append(outcome.getMessage()); + hasMessage = true; + } + } + return new ConditionOutcome(match, message.toString()); } } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/AnyNestedConditionTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/AnyNestedConditionTests.java index 69e8fa86bf..07ff223fb4 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/AnyNestedConditionTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/AnyNestedConditionTests.java @@ -31,12 +31,13 @@ import static org.junit.Assert.assertThat; * Tests for {@link AnyNestedCondition}. * * @author Phillip Webb + * @author Dave Syer */ public class AnyNestedConditionTests { @Test public void neither() throws Exception { - AnnotationConfigApplicationContext context = load(OnPropertyAorBCondition.class); + AnnotationConfigApplicationContext context = load(Config.class); assertThat(context.containsBean("myBean"), equalTo(false)); context.close(); } @@ -92,6 +93,7 @@ public class AnyNestedConditionTests { } + @ConditionalOnExpression("true") @ConditionalOnProperty("b") static class HasPropertyB { From b97e0bd4718551ffad369fd1b493ac491de48949 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 16 Sep 2016 14:14:21 -0700 Subject: [PATCH 3/4] Allow HttpPutFormContentFilter to be disabled Add `spring.mvc.formcontent.putfilter.enabled` property to allow the HttpPutFormContentFilter to be disabled. Fixes gh-6519 --- .../boot/autoconfigure/web/WebMvcAutoConfiguration.java | 1 + .../META-INF/additional-spring-configuration-metadata.json | 6 ++++++ .../autoconfigure/web/WebMvcAutoConfigurationTests.java | 7 +++++++ .../src/main/asciidoc/appendix-application-properties.adoc | 1 + 4 files changed, 15 insertions(+) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java index bfa1adf029..ba198d3a37 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java @@ -122,6 +122,7 @@ public class WebMvcAutoConfiguration { @Bean @ConditionalOnMissingBean(HttpPutFormContentFilter.class) + @ConditionalOnProperty(prefix = "spring.mvc.formcontent.putfilter", name = "enabled", matchIfMissing = true) public OrderedHttpPutFormContentFilter httpPutFormContentFilter() { return new OrderedHttpPutFormContentFilter(); } diff --git a/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json index f5034020e3..4fefa5a20c 100644 --- a/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -148,6 +148,12 @@ "description": "Enable resolution of favicon.ico.", "defaultValue": true }, + { + "name": "spring.mvc.formcontent.putfilter.enabled", + "type": "java.lang.Boolean", + "description": "Enable Spring's HttpPutFormContentFilter.", + "defaultValue": true + }, { "name": "spring.rabbitmq.dynamic", "type": "java.lang.Boolean", diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfigurationTests.java index acd2d82d05..8b40cb7acb 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfigurationTests.java @@ -476,6 +476,13 @@ public class WebMvcAutoConfigurationTests { is(equalTo(1))); } + @Test + public void httpPutFormContentFilterCanBeDisabled() throws Exception { + load((Class) null, "spring.mvc.formcontent.putfilter.enabled=false"); + assertThat(this.context.getBeansOfType(HttpPutFormContentFilter.class).size(), + is(equalTo(0))); + } + @Test public void customConfigurableWebBindingInitializer() { load(CustomConfigurableWebBindingInitializer.class); 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 d10ea5eb7c..7b9aff6c0e 100644 --- a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -307,6 +307,7 @@ content into your application; rather pick only the properties that you need. spring.mvc.dispatch-trace-request=false # Dispatch TRACE requests to the FrameworkServlet doService method. spring.mvc.dispatch-options-request=false # Dispatch OPTIONS requests to the FrameworkServlet doService method. spring.mvc.favicon.enabled=true # Enable resolution of favicon.ico. + spring.mvc.formcontent.putfilter.enabled=true # Enable Spring's HttpPutFormContentFilter. spring.mvc.ignore-default-model-on-redirect=true # If the content of the "default" model should be ignored during redirect scenarios. spring.mvc.locale= # Locale to use. spring.mvc.media-types.*= # Maps file extensions to media types for content negotiation. From 3b52909fc2b1f0dc8a0c9e0c32612229433aa662 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 16 Sep 2016 14:35:21 -0700 Subject: [PATCH 4/4] Don't change ownership of PID_FOLDER Update the launch script so that it no longer changes ownership of the PID_FOLDER. Commit b24e736cfed77be5902f30b79548f051e91d56c9 had changed the chown line from: chown "$run_user" "$PID_FOLDER/${identity}" to: chown "$run_user" "$PID_FOLDER" This meant that it was possible for the launch script to change ownership of `/var/run` and prevent later processes from writing to the folder. Since PID_FOLDER is created before the chown statement, and that the `checkPermissions` function runs to ensure that the PID file can be written, it appears that the chown is not even required. Fixes gh-6532 --- .../org/springframework/boot/loader/tools/launch.script | 1 - 1 file changed, 1 deletion(-) 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 8e2ce5d74c..db3fa0087d 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 @@ -143,7 +143,6 @@ do_start() { mkdir "$PID_FOLDER" &> /dev/null if [[ -n "$run_user" ]]; then checkPermissions || return $? - chown "$run_user" "$PID_FOLDER" chown "$run_user" "$pid_file" chown "$run_user" "$log_file" if [ $USE_START_STOP_DAEMON = true ] && type start-stop-daemon > /dev/null 2>&1; then