From 4853e6a7af9f0eeeea42e03916900f86022f9f5f Mon Sep 17 00:00:00 2001 From: Rahul Ahuja Date: Thu, 15 Nov 2018 20:12:06 +0530 Subject: [PATCH 1/2] Skip scoped targets when determining endpoints Update `EndpointDiscoverer` to filter out scoped target beans when finding endpoints. Closes gh-15182 --- .../annotation/EndpointDiscoverer.java | 14 ++++++---- .../annotation/EndpointDiscovererTests.java | 28 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java index 92d59d3d03..e153bfe85a 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java @@ -29,6 +29,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; import java.util.stream.Collectors; +import org.springframework.aop.scope.ScopedProxyUtils; import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.boot.actuate.endpoint.EndpointFilter; @@ -131,11 +132,14 @@ public abstract class EndpointDiscoverer, O exten this.applicationContext, Endpoint.class); for (String beanName : beanNames) { EndpointBean endpointBean = createEndpointBean(beanName); - EndpointBean previous = byId.putIfAbsent(endpointBean.getId(), endpointBean); - Assert.state(previous == null, - () -> "Found two endpoints with the id '" + endpointBean.getId() - + "': '" + endpointBean.getBeanName() + "' and '" - + previous.getBeanName() + "'"); + if (!ScopedProxyUtils.isScopedTarget(beanName)) { + EndpointBean previous = byId.putIfAbsent(endpointBean.getId(), + endpointBean); + Assert.state(previous == null, + () -> "Found two endpoints with the id '" + endpointBean.getId() + + "': '" + endpointBean.getBeanName() + "' and '" + + previous.getBeanName() + "'"); + } } return byId.values(); } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscovererTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscovererTests.java index c5d508085f..1d8a0506fd 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscovererTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscovererTests.java @@ -46,6 +46,7 @@ import org.springframework.boot.actuate.endpoint.invoke.ParameterValueMapper; import org.springframework.boot.actuate.endpoint.invoke.convert.ConversionServiceParameterValueMapper; import org.springframework.boot.actuate.endpoint.invoker.cache.CachingOperationInvoker; import org.springframework.boot.actuate.endpoint.invoker.cache.CachingOperationInvokerAdvisor; +import org.springframework.boot.actuate.endpoint.jmx.EndpointMBean; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -148,6 +149,18 @@ public class EndpointDiscovererTests { }); } + @Test + public void getEndpointsWhenEndpointsArePrefixedWithScopedTargetShouldRegisterOnlyOneEndpoint() { + load(ScopedTargetEndpointConfiguration.class, ( + context) -> { + Collection endpoints = + new TestEndpointDiscoverer(context).getEndpoints(); + assertThat(endpoints).hasSize(1); + assertThat(endpoints.iterator().next().getEndpointBean()).isSameAs(context + .getBean(ScopedTargetEndpointConfiguration.class).testEndpoint()); + }); + } + @Test public void getEndpointsWhenTtlSetToZeroShouldNotCacheInvokeCalls() { load(TestEndpointConfiguration.class, (context) -> { @@ -393,6 +406,21 @@ public class EndpointDiscovererTests { } + @Configuration + static class ScopedTargetEndpointConfiguration { + + @Bean + public TestEndpoint testEndpoint() { + return new TestEndpoint(); + } + + @Bean(name = "scopedTarget.testEndpoint") + public TestEndpoint scopedTargetTestEndpoint() { + return new TestEndpoint(); + } + + } + @Import({ TestEndpoint.class, SpecializedTestEndpoint.class, SpecializedExtension.class }) static class SpecializedEndpointsConfiguration { From e4d5714d50ca7fb4dbcb8cc1a033b579362719ea Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 15 Nov 2018 10:37:55 -0800 Subject: [PATCH 2/2] Polish "Skip scoped targets when determining endpoints" See gh-15182 --- .../endpoint/annotation/EndpointDiscoverer.java | 2 +- .../annotation/EndpointDiscovererTests.java | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java index e153bfe85a..af195b3325 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java @@ -131,8 +131,8 @@ public abstract class EndpointDiscoverer, O exten String[] beanNames = BeanFactoryUtils.beanNamesForAnnotationIncludingAncestors( this.applicationContext, Endpoint.class); for (String beanName : beanNames) { - EndpointBean endpointBean = createEndpointBean(beanName); if (!ScopedProxyUtils.isScopedTarget(beanName)) { + EndpointBean endpointBean = createEndpointBean(beanName); EndpointBean previous = byId.putIfAbsent(endpointBean.getId(), endpointBean); Assert.state(previous == null, diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscovererTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscovererTests.java index 1d8a0506fd..25c3d85606 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscovererTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscovererTests.java @@ -46,7 +46,6 @@ import org.springframework.boot.actuate.endpoint.invoke.ParameterValueMapper; import org.springframework.boot.actuate.endpoint.invoke.convert.ConversionServiceParameterValueMapper; import org.springframework.boot.actuate.endpoint.invoker.cache.CachingOperationInvoker; import org.springframework.boot.actuate.endpoint.invoker.cache.CachingOperationInvokerAdvisor; -import org.springframework.boot.actuate.endpoint.jmx.EndpointMBean; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -151,13 +150,13 @@ public class EndpointDiscovererTests { @Test public void getEndpointsWhenEndpointsArePrefixedWithScopedTargetShouldRegisterOnlyOneEndpoint() { - load(ScopedTargetEndpointConfiguration.class, ( - context) -> { - Collection endpoints = - new TestEndpointDiscoverer(context).getEndpoints(); - assertThat(endpoints).hasSize(1); - assertThat(endpoints.iterator().next().getEndpointBean()).isSameAs(context - .getBean(ScopedTargetEndpointConfiguration.class).testEndpoint()); + load(ScopedTargetEndpointConfiguration.class, (context) -> { + TestEndpoint expectedEndpoint = context + .getBean(ScopedTargetEndpointConfiguration.class).testEndpoint(); + Collection endpoints = new TestEndpointDiscoverer( + context).getEndpoints(); + assertThat(endpoints).flatExtracting(TestExposableEndpoint::getEndpointBean) + .containsOnly(expectedEndpoint); }); }