From 7d5e41f7dcb5a6edddfcd6742049342fb23c0000 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 1 Feb 2018 10:24:53 -0800 Subject: [PATCH] Polish --- .../endpoint/invoke/OperationParameter.java | 6 +- .../endpoint/invoke/OperationParameters.java | 9 ++ .../reflect/OperationMethodParameter.java | 4 +- .../reflect/ReflectiveOperationInvoker.java | 4 +- .../cache/CachingOperationInvoker.java | 7 +- .../cache/CachingOperationInvokerAdvisor.java | 12 +- .../OperationMethodParameterTests.java | 8 +- .../SecurityAutoConfigurationTests.java | 144 +++++++++++------- .../GettingStartedDocumentationTests.java | 2 +- .../gradle/tasks/bundling/BootWarTests.java | 2 +- .../logging/logback/LogbackLoggingSystem.java | 2 +- .../logback/LogbackLoggingSystemTests.java | 2 +- 12 files changed, 117 insertions(+), 85 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/OperationParameter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/OperationParameter.java index 4bef3a74a85..28754722962 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/OperationParameter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/OperationParameter.java @@ -37,9 +37,9 @@ public interface OperationParameter { Class getType(); /** - * Return if the parameter accepts null values. - * @return if the parameter is nullable + * Return if the parameter is mandatory (does not accept null values). + * @return if the parameter is mandatory */ - boolean isNullable(); + boolean isMandatory(); } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/OperationParameters.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/OperationParameters.java index 42046d4378f..f0b5d63cd9c 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/OperationParameters.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/OperationParameters.java @@ -40,6 +40,15 @@ public interface OperationParameters extends Iterable { */ int getParameterCount(); + /** + * Return if any of the contained parameters are + * {@link OperationParameter#isMandatory() mandatory}. + * @return if any parameters are mandatory + */ + default boolean hasMandatoryParameter() { + return stream().anyMatch(OperationParameter::isMandatory); + } + /** * Return the parameter at the specified index. * @param index the parameter index diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameter.java index 85b81908c77..0755f880866 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameter.java @@ -54,8 +54,8 @@ class OperationMethodParameter implements OperationParameter { } @Override - public boolean isNullable() { - return !ObjectUtils.isEmpty(this.parameter.getAnnotationsByType(Nullable.class)); + public boolean isMandatory() { + return ObjectUtils.isEmpty(this.parameter.getAnnotationsByType(Nullable.class)); } @Override diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/ReflectiveOperationInvoker.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/ReflectiveOperationInvoker.java index f1dd3be18a3..4f437b2227d 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/ReflectiveOperationInvoker.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoke/reflect/ReflectiveOperationInvoker.java @@ -85,8 +85,8 @@ public class ReflectiveOperationInvoker implements OperationInvoker { private boolean isMissing(Map arguments, OperationParameter parameter) { - if (parameter.isNullable()) { - return false; + if (!parameter.isMandatory()) { + return true; } return arguments.get(parameter.getName()) == null; } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java index c284f940e85..2668bfc98d8 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvoker.java @@ -17,6 +17,7 @@ package org.springframework.boot.actuate.endpoint.invoker.cache; import java.util.Map; +import java.util.Objects; import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker; import org.springframework.util.Assert; @@ -74,11 +75,7 @@ public class CachingOperationInvoker implements OperationInvoker { private boolean hasArgument(Map arguments) { if (!ObjectUtils.isEmpty(arguments)) { - for (Object value : arguments.values()) { - if (value != null) { - return true; - } - } + return arguments.values().stream().anyMatch(Objects::nonNull); } return false; } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java index 639930472dd..d7ef9dd14b2 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/invoker/cache/CachingOperationInvokerAdvisor.java @@ -21,7 +21,6 @@ import java.util.function.Function; import org.springframework.boot.actuate.endpoint.OperationType; import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker; import org.springframework.boot.actuate.endpoint.invoke.OperationInvokerAdvisor; -import org.springframework.boot.actuate.endpoint.invoke.OperationParameter; import org.springframework.boot.actuate.endpoint.invoke.OperationParameters; /** @@ -41,7 +40,7 @@ public class CachingOperationInvokerAdvisor implements OperationInvokerAdvisor { @Override public OperationInvoker apply(String endpointId, OperationType operationType, OperationParameters parameters, OperationInvoker invoker) { - if (operationType == OperationType.READ && !hasMandatoryParameter(parameters)) { + if (operationType == OperationType.READ && !parameters.hasMandatoryParameter()) { Long timeToLive = this.endpointIdTimeToLive.apply(endpointId); if (timeToLive != null && timeToLive > 0) { return new CachingOperationInvoker(invoker, timeToLive); @@ -50,13 +49,4 @@ public class CachingOperationInvokerAdvisor implements OperationInvokerAdvisor { return invoker; } - private boolean hasMandatoryParameter(OperationParameters parameters) { - for (OperationParameter parameter : parameters) { - if (!parameter.isNullable()) { - return true; - } - } - return false; - } - } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameterTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameterTests.java index fb95661ce28..a2ba89fce29 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameterTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/invoke/reflect/OperationMethodParameterTests.java @@ -50,17 +50,17 @@ public class OperationMethodParameterTests { } @Test - public void isNullableWhenNoAnnotationShouldReturnFalse() { + public void isMandatoryWhenNoAnnotationShouldReturnTrue() { OperationMethodParameter parameter = new OperationMethodParameter("name", this.method.getParameters()[0]); - assertThat(parameter.isNullable()).isFalse(); + assertThat(parameter.isMandatory()).isTrue(); } @Test - public void isNullableWhenNullableAnnotationShouldReturnTrue() { + public void isMandatoryWhenNullableAnnotationShouldReturnFalse() { OperationMethodParameter parameter = new OperationMethodParameter("name", this.method.getParameters()[1]); - assertThat(parameter.isNullable()).isTrue(); + assertThat(parameter.isMandatory()).isFalse(); } void example(String one, @Nullable String two) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfigurationTests.java index 14ff1ab741e..6dea9c23229 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfigurationTests.java @@ -87,10 +87,14 @@ public class SecurityAutoConfigurationTests { @Test public void testDefaultFilterOrderWithSecurityAdapter() { - this.contextRunner.withConfiguration(AutoConfigurations.of(WebSecurity.class, SecurityFilterAutoConfiguration.class)) - .run(context -> assertThat(context.getBean("securityFilterChainRegistration", - DelegatingFilterProxyRegistrationBean.class).getOrder()).isEqualTo( - FilterRegistrationBean.REQUEST_WRAPPER_FILTER_MAX_ORDER - 100)); + this.contextRunner.withConfiguration(AutoConfigurations.of(WebSecurity.class, + SecurityFilterAutoConfiguration.class)).run( + context -> assertThat(context + .getBean("securityFilterChainRegistration", + DelegatingFilterProxyRegistrationBean.class) + .getOrder()).isEqualTo( + FilterRegistrationBean.REQUEST_WRAPPER_FILTER_MAX_ORDER + - 100)); } @Test @@ -115,30 +119,41 @@ public class SecurityAutoConfigurationTests { @Test public void defaultAuthenticationEventPublisherIsConditionalOnMissingBean() { - this.contextRunner.withUserConfiguration(AuthenticationEventPublisherConfiguration.class).run(context -> { - assertThat(context.getBean(AuthenticationEventPublisher.class)) - .isInstanceOf(AuthenticationEventPublisherConfiguration.TestAuthenticationEventPublisher.class); - }); + this.contextRunner + .withUserConfiguration(AuthenticationEventPublisherConfiguration.class) + .run(context -> { + assertThat(context.getBean(AuthenticationEventPublisher.class)) + .isInstanceOf( + AuthenticationEventPublisherConfiguration.TestAuthenticationEventPublisher.class); + }); } @Test public void testDefaultFilterOrder() { - this.contextRunner.withConfiguration(AutoConfigurations.of(SecurityFilterAutoConfiguration.class)).run(context -> { - assertThat(context.getBean("securityFilterChainRegistration", - DelegatingFilterProxyRegistrationBean.class).getOrder()).isEqualTo( - FilterRegistrationBean.REQUEST_WRAPPER_FILTER_MAX_ORDER - 100); - }); + this.contextRunner + .withConfiguration( + AutoConfigurations.of(SecurityFilterAutoConfiguration.class)) + .run(context -> { + assertThat(context + .getBean("securityFilterChainRegistration", + DelegatingFilterProxyRegistrationBean.class) + .getOrder()).isEqualTo( + FilterRegistrationBean.REQUEST_WRAPPER_FILTER_MAX_ORDER + - 100); + }); } @Test public void testCustomFilterOrder() { - this.contextRunner.withConfiguration(AutoConfigurations.of(SecurityFilterAutoConfiguration.class)) - .withPropertyValues("spring.security.filter.order:12345") - .run(context -> { - assertThat(context.getBean("securityFilterChainRegistration", - DelegatingFilterProxyRegistrationBean.class).getOrder()).isEqualTo( - 12345); - }); + this.contextRunner + .withConfiguration( + AutoConfigurations.of(SecurityFilterAutoConfiguration.class)) + .withPropertyValues("spring.security.filter.order:12345").run(context -> { + assertThat(context + .getBean("securityFilterChainRegistration", + DelegatingFilterProxyRegistrationBean.class) + .getOrder()).isEqualTo(12345); + }); } @Test @@ -153,60 +168,75 @@ public class SecurityAutoConfigurationTests { @Test public void defaultUserNotCreatedIfAuthenticationManagerBeanPresent() { - this.contextRunner.withUserConfiguration(TestAuthenticationManagerConfiguration.class).run(context -> { - AuthenticationManager manager = context.getBean(AuthenticationManager.class); - assertThat(manager).isEqualTo(context.getBean( - TestAuthenticationManagerConfiguration.class).authenticationManager); - assertThat(this.outputCapture.toString()) - .doesNotContain("Using generated security password: "); - TestingAuthenticationToken token = new TestingAuthenticationToken("foo", "bar"); - assertThat(manager.authenticate(token)).isNotNull(); - }); + this.contextRunner + .withUserConfiguration(TestAuthenticationManagerConfiguration.class) + .run(context -> { + AuthenticationManager manager = context + .getBean(AuthenticationManager.class); + assertThat(manager).isEqualTo(context.getBean( + TestAuthenticationManagerConfiguration.class).authenticationManager); + assertThat(this.outputCapture.toString()) + .doesNotContain("Using generated security password: "); + TestingAuthenticationToken token = new TestingAuthenticationToken( + "foo", "bar"); + assertThat(manager.authenticate(token)).isNotNull(); + }); } @Test public void defaultUserNotCreatedIfUserDetailsServiceBeanPresent() { - this.contextRunner.withUserConfiguration(TestUserDetailsServiceConfiguration.class).run(context -> { - UserDetailsService userDetailsService = context - .getBean(UserDetailsService.class); - assertThat(this.outputCapture.toString()) - .doesNotContain("Using default security password: "); - assertThat(userDetailsService.loadUserByUsername("foo")).isNotNull(); - }); + this.contextRunner + .withUserConfiguration(TestUserDetailsServiceConfiguration.class) + .run(context -> { + UserDetailsService userDetailsService = context + .getBean(UserDetailsService.class); + assertThat(this.outputCapture.toString()) + .doesNotContain("Using default security password: "); + assertThat(userDetailsService.loadUserByUsername("foo")).isNotNull(); + }); } @Test public void defaultUserNotCreatedIfAuthenticationProviderBeanPresent() { - this.contextRunner.withUserConfiguration(TestAuthenticationProviderConfiguration.class).run(context -> { - AuthenticationProvider provider = context - .getBean(AuthenticationProvider.class); - assertThat(this.outputCapture.toString()) - .doesNotContain("Using default security password: "); - TestingAuthenticationToken token = new TestingAuthenticationToken("foo", "bar"); - assertThat(provider.authenticate(token)).isNotNull(); - }); + this.contextRunner + .withUserConfiguration(TestAuthenticationProviderConfiguration.class) + .run(context -> { + AuthenticationProvider provider = context + .getBean(AuthenticationProvider.class); + assertThat(this.outputCapture.toString()) + .doesNotContain("Using default security password: "); + TestingAuthenticationToken token = new TestingAuthenticationToken( + "foo", "bar"); + assertThat(provider.authenticate(token)).isNotNull(); + }); } @Test public void testJpaCoexistsHappily() { - this.contextRunner.withPropertyValues("spring.datasource.url:jdbc:hsqldb:mem:testsecdb", + this.contextRunner + .withPropertyValues("spring.datasource.url:jdbc:hsqldb:mem:testsecdb", "spring.datasource.initialization-mode:never") .withUserConfiguration(EntityConfiguration.class) - .withConfiguration(AutoConfigurations.of(HibernateJpaAutoConfiguration.class, DataSourceAutoConfiguration.class)) - .run(context -> assertThat(context.getBean(JpaTransactionManager.class)).isNotNull()); + .withConfiguration( + AutoConfigurations.of(HibernateJpaAutoConfiguration.class, + DataSourceAutoConfiguration.class)) + .run(context -> assertThat(context.getBean(JpaTransactionManager.class)) + .isNotNull()); // This can fail if security @Conditionals force early instantiation of the // HibernateJpaAutoConfiguration (e.g. the EntityManagerFactory is not found) } @Test public void testSecurityEvaluationContextExtensionSupport() { - this.contextRunner.run(context -> - assertThat(context).getBean(SecurityEvaluationContextExtension.class).isNotNull()); + this.contextRunner.run(context -> assertThat(context) + .getBean(SecurityEvaluationContextExtension.class).isNotNull()); } @Test public void defaultFilterDispatcherTypes() { - this.contextRunner.withConfiguration(AutoConfigurations.of(SecurityFilterAutoConfiguration.class)) + this.contextRunner + .withConfiguration( + AutoConfigurations.of(SecurityFilterAutoConfiguration.class)) .run(context -> { DelegatingFilterProxyRegistrationBean bean = context.getBean( "securityFilterChainRegistration", @@ -216,13 +246,16 @@ public class SecurityAutoConfigurationTests { .getField(bean, "dispatcherTypes"); assertThat(dispatcherTypes).containsOnly(DispatcherType.ASYNC, DispatcherType.ERROR, DispatcherType.REQUEST); - }); + }); } @Test public void customFilterDispatcherTypes() { - this.contextRunner.withPropertyValues("spring.security.filter.dispatcher-types:INCLUDE,ERROR") - .withConfiguration(AutoConfigurations.of(SecurityFilterAutoConfiguration.class)) + this.contextRunner + .withPropertyValues( + "spring.security.filter.dispatcher-types:INCLUDE,ERROR") + .withConfiguration( + AutoConfigurations.of(SecurityFilterAutoConfiguration.class)) .run(context -> { DelegatingFilterProxyRegistrationBean bean = context.getBean( "securityFilterChainRegistration", @@ -257,10 +290,13 @@ public class SecurityAutoConfigurationTests { } @Override - public void publishAuthenticationFailure(AuthenticationException exception, Authentication authentication) { + public void publishAuthenticationFailure(AuthenticationException exception, + Authentication authentication) { } + } + } @Configuration diff --git a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/docs/GettingStartedDocumentationTests.java b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/docs/GettingStartedDocumentationTests.java index 412b8fe1f23..26d3ad61958 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/docs/GettingStartedDocumentationTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/docs/GettingStartedDocumentationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. diff --git a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootWarTests.java b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootWarTests.java index c40c095fc0a..c9592e3be5f 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootWarTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootWarTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java index 85f4aa500d4..d25ad8850c1 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java index b1cfa51be2f..ea9638000e5 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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.