From 49a21ded7ac8f9926271852d8fa06d76fb1fd5c7 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 23 Apr 2020 20:44:06 -0700 Subject: [PATCH] Create endpoint beans as late as possible Update `EndpointDiscoverer` so that `@Endpoint` and `@EndpointExtension` beans are created as late as possible. Prior to this commit, endpoint beans and extension beans would be created during the discovery phase which could cause early bean initialization. The problem was especially nasty when using an embedded servlet container since `ServletEndpointRegistrar` is loaded as the container is initialized. This would trigger discovery and load all endpoint beans, including the health endpoint, and all health indicator beans. Fixes gh-20714 --- .../CloudFoundryWebEndpointDiscoverer.java | 12 +-- .../annotation/EndpointDiscoverer.java | 78 ++++++++++++++----- .../ControllerEndpointDiscoverer.java | 7 +- .../annotation/ServletEndpointDiscoverer.java | 7 +- 4 files changed, 71 insertions(+), 33 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointDiscoverer.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointDiscoverer.java index 7f51058956..650f975375 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointDiscoverer.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointDiscoverer.java @@ -58,21 +58,21 @@ public class CloudFoundryWebEndpointDiscoverer extends WebEndpointDiscoverer { } @Override - protected boolean isExtensionExposed(Object extensionBean) { - if (isHealthEndpointExtension(extensionBean) && !isCloudFoundryHealthEndpointExtension(extensionBean)) { + protected boolean isExtensionTypeExposed(Class extensionBeanType) { + if (isHealthEndpointExtension(extensionBeanType) && !isCloudFoundryHealthEndpointExtension(extensionBeanType)) { // Filter regular health endpoint extensions so a CF version can replace them return false; } return true; } - private boolean isHealthEndpointExtension(Object extensionBean) { - return MergedAnnotations.from(extensionBean.getClass()).get(EndpointWebExtension.class) + private boolean isHealthEndpointExtension(Class extensionBeanType) { + return MergedAnnotations.from(extensionBeanType).get(EndpointWebExtension.class) .getValue("endpoint", Class.class).map(HealthEndpoint.class::isAssignableFrom).orElse(false); } - private boolean isCloudFoundryHealthEndpointExtension(Object extensionBean) { - return MergedAnnotations.from(extensionBean.getClass()).isPresent(EndpointCloudFoundryExtension.class); + private boolean isCloudFoundryHealthEndpointExtension(Class extensionBeanType) { + return MergedAnnotations.from(extensionBeanType).isPresent(EndpointCloudFoundryExtension.class); } } 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 bbdf544f66..4ee0094463 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 @@ -49,6 +49,7 @@ import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; import org.springframework.core.env.Environment; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -140,8 +141,9 @@ public abstract class EndpointDiscoverer, O exten } private EndpointBean createEndpointBean(String beanName) { - Object bean = this.applicationContext.getBean(beanName); - return new EndpointBean(this.applicationContext.getEnvironment(), beanName, bean); + Class beanType = ClassUtils.getUserClass(this.applicationContext.getType(beanName, false)); + Supplier beanSupplier = () -> this.applicationContext.getBean(beanName); + return new EndpointBean(this.applicationContext.getEnvironment(), beanName, beanType, beanSupplier); } private void addExtensionBeans(Collection endpointBeans) { @@ -159,8 +161,9 @@ public abstract class EndpointDiscoverer, O exten } private ExtensionBean createExtensionBean(String beanName) { - Object bean = this.applicationContext.getBean(beanName); - return new ExtensionBean(this.applicationContext.getEnvironment(), beanName, bean); + Class beanType = ClassUtils.getUserClass(this.applicationContext.getType(beanName)); + Supplier beanSupplier = () -> this.applicationContext.getBean(beanName); + return new ExtensionBean(this.applicationContext.getEnvironment(), beanName, beanType, beanSupplier); } private void addExtensionBean(EndpointBean endpointBean, ExtensionBean extensionBean) { @@ -233,7 +236,8 @@ public abstract class EndpointDiscoverer, O exten } private boolean isExtensionExposed(EndpointBean endpointBean, ExtensionBean extensionBean) { - return isFilterMatch(extensionBean.getFilter(), endpointBean) && isExtensionExposed(extensionBean.getBean()); + return isFilterMatch(extensionBean.getFilter(), endpointBean) + && isExtensionTypeExposed(extensionBean.getBeanType()); } /** @@ -242,10 +246,21 @@ public abstract class EndpointDiscoverer, O exten * @param extensionBean the extension bean * @return {@code true} if the extension is exposed */ + @Deprecated protected boolean isExtensionExposed(Object extensionBean) { return true; } + /** + * Determine if an extension bean should be exposed. Subclasses can override this + * method to provide additional logic. + * @param extensionBeanType the extension bean type + * @return {@code true} if the extension is exposed + */ + protected boolean isExtensionTypeExposed(Class extensionBeanType) { + return true; + } + private boolean isEndpointExposed(EndpointBean endpointBean) { return isFilterMatch(endpointBean.getFilter(), endpointBean) && !isEndpointFiltered(endpointBean) && isEndpointExposed(endpointBean.getBean()); @@ -257,10 +272,21 @@ public abstract class EndpointDiscoverer, O exten * @param endpointBean the endpoint bean * @return {@code true} if the endpoint is exposed */ + @Deprecated protected boolean isEndpointExposed(Object endpointBean) { return true; } + /** + * Determine if an endpoint bean should be exposed. Subclasses can override this + * method to provide additional logic. + * @param beanType the endpoint bean type + * @return {@code true} if the endpoint is exposed + */ + protected boolean isEndpointTypeExposed(Class beanType) { + return true; + } + private boolean isEndpointFiltered(EndpointBean endpointBean) { for (EndpointFilter filter : this.filters) { if (!isFilterMatch(filter, endpointBean)) { @@ -272,7 +298,7 @@ public abstract class EndpointDiscoverer, O exten @SuppressWarnings("unchecked") private boolean isFilterMatch(Class filter, EndpointBean endpointBean) { - if (!isEndpointExposed(endpointBean.getBean())) { + if (!isEndpointTypeExposed(endpointBean.getBeanType())) { return false; } if (filter == null) { @@ -392,7 +418,9 @@ public abstract class EndpointDiscoverer, O exten private final String beanName; - private final Object bean; + private final Class beanType; + + private final Supplier beanSupplier; private final EndpointId id; @@ -402,17 +430,18 @@ public abstract class EndpointDiscoverer, O exten private Set extensions = new LinkedHashSet<>(); - EndpointBean(Environment environment, String beanName, Object bean) { - MergedAnnotation annotation = MergedAnnotations - .from(bean.getClass(), SearchStrategy.TYPE_HIERARCHY).get(Endpoint.class); + EndpointBean(Environment environment, String beanName, Class beanType, Supplier beanSupplier) { + MergedAnnotation annotation = MergedAnnotations.from(beanType, SearchStrategy.TYPE_HIERARCHY) + .get(Endpoint.class); String id = annotation.getString("id"); Assert.state(StringUtils.hasText(id), - () -> "No @Endpoint id attribute specified for " + bean.getClass().getName()); + () -> "No @Endpoint id attribute specified for " + beanType.getName()); this.beanName = beanName; - this.bean = bean; + this.beanType = beanType; + this.beanSupplier = beanSupplier; this.id = EndpointId.of(environment, id); this.enabledByDefault = annotation.getBoolean("enableByDefault"); - this.filter = getFilter(this.bean.getClass()); + this.filter = getFilter(beanType); } void addExtension(ExtensionBean extensionBean) { @@ -432,8 +461,12 @@ public abstract class EndpointDiscoverer, O exten return this.beanName; } + Class getBeanType() { + return this.beanType; + } + Object getBean() { - return this.bean; + return this.beanSupplier.get(); } EndpointId getId() { @@ -457,17 +490,20 @@ public abstract class EndpointDiscoverer, O exten private final String beanName; - private final Object bean; + private final Class beanType; + + private final Supplier beanSupplier; private final EndpointId endpointId; private final Class filter; - ExtensionBean(Environment environment, String beanName, Object bean) { - this.bean = bean; + ExtensionBean(Environment environment, String beanName, Class beanType, Supplier beanSupplier) { this.beanName = beanName; + this.beanType = beanType; + this.beanSupplier = beanSupplier; MergedAnnotation extensionAnnotation = MergedAnnotations - .from(bean.getClass(), SearchStrategy.TYPE_HIERARCHY).get(EndpointExtension.class); + .from(beanType, SearchStrategy.TYPE_HIERARCHY).get(EndpointExtension.class); Class endpointType = extensionAnnotation.getClass("endpoint"); MergedAnnotation endpointAnnotation = MergedAnnotations .from(endpointType, SearchStrategy.TYPE_HIERARCHY).get(Endpoint.class); @@ -481,8 +517,12 @@ public abstract class EndpointDiscoverer, O exten return this.beanName; } + Class getBeanType() { + return this.beanType; + } + Object getBean() { - return this.bean; + return this.beanSupplier.get(); } EndpointId getEndpointId() { diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/annotation/ControllerEndpointDiscoverer.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/annotation/ControllerEndpointDiscoverer.java index 45c672a680..87b4c67411 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/annotation/ControllerEndpointDiscoverer.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/annotation/ControllerEndpointDiscoverer.java @@ -30,7 +30,7 @@ import org.springframework.boot.actuate.endpoint.invoke.ParameterValueMapper; import org.springframework.boot.actuate.endpoint.web.PathMapper; import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.MergedAnnotations; -import org.springframework.util.ClassUtils; +import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; /** * {@link EndpointDiscoverer} for {@link ExposableControllerEndpoint controller @@ -57,9 +57,8 @@ public class ControllerEndpointDiscoverer extends EndpointDiscoverer type = ClassUtils.getUserClass(endpointBean.getClass()); - MergedAnnotations annotations = MergedAnnotations.from(type); + protected boolean isEndpointTypeExposed(Class beanType) { + MergedAnnotations annotations = MergedAnnotations.from(beanType, SearchStrategy.SUPERCLASS); return annotations.isPresent(ControllerEndpoint.class) || annotations.isPresent(RestControllerEndpoint.class); } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/annotation/ServletEndpointDiscoverer.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/annotation/ServletEndpointDiscoverer.java index 5c7dc7bd67..2c568ac6bc 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/annotation/ServletEndpointDiscoverer.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/annotation/ServletEndpointDiscoverer.java @@ -31,7 +31,7 @@ import org.springframework.boot.actuate.endpoint.web.ExposableServletEndpoint; import org.springframework.boot.actuate.endpoint.web.PathMapper; import org.springframework.context.ApplicationContext; import org.springframework.core.annotation.MergedAnnotations; -import org.springframework.util.ClassUtils; +import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; /** * {@link EndpointDiscoverer} for {@link ExposableServletEndpoint servlet endpoints}. @@ -57,9 +57,8 @@ public class ServletEndpointDiscoverer extends EndpointDiscoverer type = ClassUtils.getUserClass(endpointBean.getClass()); - return MergedAnnotations.from(type).isPresent(ServletEndpoint.class); + protected boolean isEndpointTypeExposed(Class beanType) { + return MergedAnnotations.from(beanType, SearchStrategy.SUPERCLASS).isPresent(ServletEndpoint.class); } @Override