From 975e986fef1517b80eaf280edf24409ff79c5f8e Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 4 May 2018 19:04:28 -0700 Subject: [PATCH] Polish --- .../security/servlet/EndpointRequest.java | 157 +++++++++--------- .../actuate/endpoint/jmx/EndpointMBean.java | 9 +- .../endpoint/jmx/EndpointMBeanTests.java | 2 + .../DispatcherServletAutoConfiguration.java | 2 +- .../servlet/WebMvcAutoConfigurationTests.java | 1 - .../src/main/asciidoc/howto.adoc | 2 +- 6 files changed, 83 insertions(+), 90 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java index bee0dedb867..8ce38af30a9 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/security/servlet/EndpointRequest.java @@ -113,19 +113,56 @@ public final class EndpointRequest { } /** - * The request matcher used to match against {@link Endpoint actuator endpoints}. + * Base class for supported request matchers. */ - public static final class EndpointRequestMatcher + private static abstract class AbstractRequestMatcher extends ApplicationContextRequestMatcher { + private volatile RequestMatcher delegate; + + public AbstractRequestMatcher() { + super(WebApplicationContext.class); + } + + @Override + protected final void initialized(Supplier context) { + this.delegate = createDelegate(context.get()); + } + + @Override + protected final boolean matches(HttpServletRequest request, + Supplier context) { + return this.delegate.matches(request); + } + + private RequestMatcher createDelegate(WebApplicationContext context) { + try { + RequestMatcherFactory requestMatcherFactory = new RequestMatcherFactory( + context.getBean(DispatcherServletPathProvider.class) + .getServletPath()); + return createDelegate(context, requestMatcherFactory); + } + catch (NoSuchBeanDefinitionException ex) { + return EMPTY_MATCHER; + } + } + + protected abstract RequestMatcher createDelegate(WebApplicationContext context, + RequestMatcherFactory requestMatcherFactory); + + } + + /** + * The request matcher used to match against {@link Endpoint actuator endpoints}. + */ + public static final class EndpointRequestMatcher extends AbstractRequestMatcher { + private final List includes; private final List excludes; private final boolean includeLinks; - private volatile RequestMatcher delegate; - private EndpointRequestMatcher(boolean includeLinks) { this(Collections.emptyList(), Collections.emptyList(), includeLinks); } @@ -142,7 +179,6 @@ public final class EndpointRequest { private EndpointRequestMatcher(List includes, List excludes, boolean includeLinks) { - super(WebApplicationContext.class); this.includes = includes; this.excludes = excludes; this.includeLinks = includeLinks; @@ -165,40 +201,22 @@ public final class EndpointRequest { } @Override - protected void initialized( - Supplier webApplicationContext) { - this.delegate = createDelegate(webApplicationContext); - } - - private RequestMatcher createDelegate( - Supplier webApplicationContext) { - try { - WebApplicationContext context = webApplicationContext.get(); - PathMappedEndpoints pathMappedEndpoints = context - .getBean(PathMappedEndpoints.class); - DispatcherServletPathProvider pathProvider = context - .getBean(DispatcherServletPathProvider.class); - return createDelegate(pathMappedEndpoints, pathProvider.getServletPath()); - } - catch (NoSuchBeanDefinitionException ex) { - return EMPTY_MATCHER; - } - } - - private RequestMatcher createDelegate(PathMappedEndpoints pathMappedEndpoints, - String servletPath) { + protected RequestMatcher createDelegate(WebApplicationContext context, + RequestMatcherFactory requestMatcherFactory) { + PathMappedEndpoints pathMappedEndpoints = context + .getBean(PathMappedEndpoints.class); Set paths = new LinkedHashSet<>(); if (this.includes.isEmpty()) { paths.addAll(pathMappedEndpoints.getAllPaths()); } streamPaths(this.includes, pathMappedEndpoints).forEach(paths::add); streamPaths(this.excludes, pathMappedEndpoints).forEach(paths::remove); - List delegateMatchers = getDelegateMatchers(servletPath, - paths); + List delegateMatchers = getDelegateMatchers( + requestMatcherFactory, paths); if (this.includeLinks && StringUtils.hasText(pathMappedEndpoints.getBasePath())) { - delegateMatchers.add(new AntPathRequestMatcher( - computePath(servletPath, pathMappedEndpoints.getBasePath()))); + delegateMatchers.add( + requestMatcherFactory.antPath(pathMappedEndpoints.getBasePath())); } return new OrRequestMatcher(delegateMatchers); } @@ -226,75 +244,50 @@ public final class EndpointRequest { return annotation.id(); } - private List getDelegateMatchers(String servletPath, - Set paths) { + private List getDelegateMatchers( + RequestMatcherFactory requestMatcherFactory, Set paths) { return paths.stream() - .map((path) -> new AntPathRequestMatcher(computePath(servletPath, path) + "/**")) + .map((path) -> requestMatcherFactory.antPath(path, "/**")) .collect(Collectors.toList()); } - private String computePath(String servletPath, String path) { - if (servletPath.equals("/")) { - return path; - } - return servletPath + path; - } - - @Override - protected boolean matches(HttpServletRequest request, - Supplier context) { - return this.delegate.matches(request); - } - } /** * The request matcher used to match against the links endpoint. */ - public static final class LinksRequestMatcher - extends ApplicationContextRequestMatcher { - - private RequestMatcher delegate; - - private LinksRequestMatcher() { - super(WebApplicationContext.class); - } + public static final class LinksRequestMatcher extends AbstractRequestMatcher { @Override - protected void initialized( - Supplier webApplicationContext) { - try { - WebApplicationContext context = webApplicationContext.get(); - WebEndpointProperties properties = context - .getBean(WebEndpointProperties.class); - DispatcherServletPathProvider pathProvider = context - .getBean(DispatcherServletPathProvider.class); - this.delegate = createDelegate(pathProvider.getServletPath(), properties); - } - catch (NoSuchBeanDefinitionException ex) { - this.delegate = EMPTY_MATCHER; - } - } - - private RequestMatcher createDelegate(String path, - WebEndpointProperties properties) { + protected RequestMatcher createDelegate(WebApplicationContext context, + RequestMatcherFactory requestMatcherFactory) { + WebEndpointProperties properties = context + .getBean(WebEndpointProperties.class); if (StringUtils.hasText(properties.getBasePath())) { - return new AntPathRequestMatcher(computePath(path, properties.getBasePath())); + return requestMatcherFactory.antPath(properties.getBasePath()); } return EMPTY_MATCHER; } - private String computePath(String servletPath, String path) { - if (servletPath.equals("/")) { - return path; - } - return servletPath + path; + } + + /** + * Factory used to create a {@link RequestMatcher}. + */ + private static class RequestMatcherFactory { + + private final String servletPath; + + RequestMatcherFactory(String servletPath) { + this.servletPath = servletPath; } - @Override - protected boolean matches(HttpServletRequest request, - Supplier context) { - return this.delegate.matches(request); + public RequestMatcher antPath(String... parts) { + String pattern = (this.servletPath.equals("/") ? "" : this.servletPath); + for (String part : parts) { + pattern += part; + } + return new AntPathRequestMatcher(pattern); } } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java index f6f50945707..8a3a79e668e 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java @@ -99,8 +99,9 @@ public class EndpointMBean implements DynamicMBean { String[] parameterNames = operation.getParameters().stream() .map(JmxOperationParameter::getName).toArray(String[]::new); Map arguments = getArguments(parameterNames, params); - Object result = operation - .invoke(new InvocationContext(SecurityContext.NONE, arguments)); + InvocationContext context = new InvocationContext(SecurityContext.NONE, + arguments); + Object result = operation.invoke(context); if (REACTOR_PRESENT) { result = ReactiveHandler.handle(result); } @@ -119,9 +120,7 @@ public class EndpointMBean implements DynamicMBean { if (exception.getClass().getName().startsWith("java.")) { return exception; } - else { - return new IllegalStateException(exception.getMessage()); - } + return new IllegalStateException(exception.getMessage()); } private Map getArguments(String[] parameterNames, Object[] params) { diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java index 481f9b85a96..f851e3febc5 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanTests.java @@ -130,10 +130,12 @@ public class EndpointMBeanTests { public void invokeWhenOperationIsInvalidShouldThrowException() throws MBeanException, ReflectionException { TestJmxOperation operation = new TestJmxOperation() { + @Override public Object invoke(InvocationContext context) { throw new InvalidEndpointRequestException("test failure", "test"); } + }; TestExposableJmxEndpoint endpoint = new TestExposableJmxEndpoint(operation); EndpointMBean bean = new EndpointMBean(this.responseMapper, endpoint); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfiguration.java index cd2ac0dd333..61e0a1a4e59 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfiguration.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-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfigurationTests.java index c69b711a13e..d1e30f4fb0c 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfigurationTests.java @@ -831,7 +831,6 @@ public class WebMvcAutoConfigurationTests { ContentNegotiationStrategy delegate = mock(ContentNegotiationStrategy.class); ContentNegotiationStrategy strategy = new WebMvcAutoConfiguration.OptionalPathExtensionContentNegotiationStrategy( delegate); - MockHttpServletRequest request = new MockHttpServletRequest(); request.setAttribute( PathExtensionContentNegotiationStrategy.class.getName() + ".SKIP", diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc index 3bd9ebff46f..f51a6505457 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc @@ -2600,7 +2600,7 @@ the artifact yourself instead of overriding the property. WARNING: Each Spring Boot release is designed and tested against this specific set of third-party dependencies. Overriding versions may cause compatibility issues. -To override dependency versions in Gradle, see {spring-boot-gradle-plugin-reference}/#managing-dependencies-customizing[ this section ] +To override dependency versions in Gradle, see {spring-boot-gradle-plugin-reference}/#managing-dependencies-customizing[this section] of the Gradle plugin's documentation. [[howto-create-an-executable-jar-with-maven]]