From b99c05329f6cd403dad830d841bf8bdb8245cefd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=B3mez=20D=C3=ADaz=2C=20Julio=20Jos=C3=A9?= Date: Wed, 27 Feb 2019 22:01:15 +0100 Subject: [PATCH 1/2] Add loadOnStartup property to EndpointServlet loadOnStartup property was missing from EndpointServlet and cannot be set inside ServletEndpointRegistrar. Now it can be set and register a Servlet with that integer property ready to act upon registration. See gh-16053 --- .../actuate/endpoint/web/EndpointServlet.java | 34 +++++++++++++++++-- .../web/ServletEndpointRegistrar.java | 3 +- .../endpoint/web/EndpointServletTests.java | 26 ++++++++++++++ .../web/ServletEndpointRegistrarTests.java | 24 +++++++++++++ 4 files changed, 83 insertions(+), 4 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/EndpointServlet.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/EndpointServlet.java index 44239b2fde..a72d0508fb 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/EndpointServlet.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/EndpointServlet.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -35,23 +35,31 @@ public final class EndpointServlet { private final Servlet servlet; + private final int loadOnStartup; + private final Map initParameters; public EndpointServlet(Class servlet) { Assert.notNull(servlet, "Servlet must not be null"); this.servlet = BeanUtils.instantiateClass(servlet); this.initParameters = Collections.emptyMap(); + this.loadOnStartup = -1; + } public EndpointServlet(Servlet servlet) { Assert.notNull(servlet, "Servlet must not be null"); this.servlet = servlet; this.initParameters = Collections.emptyMap(); + this.loadOnStartup = -1; } - private EndpointServlet(Servlet servlet, Map initParameters) { + private EndpointServlet(Servlet servlet, Map initParameters, + int loadOnStartup) { this.servlet = servlet; this.initParameters = Collections.unmodifiableMap(initParameters); + this.loadOnStartup = loadOnStartup; + } public EndpointServlet withInitParameter(String name, String value) { @@ -67,7 +75,23 @@ public final class EndpointServlet { Map mergedInitParameters = new LinkedHashMap<>( this.initParameters); mergedInitParameters.putAll(initParameters); - return new EndpointServlet(this.servlet, mergedInitParameters); + return new EndpointServlet(this.servlet, mergedInitParameters, + this.loadOnStartup); + } + + /** + * Sets the loadOnStartup priority that will be set on Servlet + * registration + *

+ * The default value for loadOnStartup is -1. + * @param loadOnStartup the initialization priority of the Servlet + * @return a new instance of {@link EndpointServlet} with the provided + * loadOnStartup value set + * @since 2.2.0 + * @see ServletRegistration.Dynamic#setLoadOnStartup(int) + */ + public EndpointServlet withLoadOnStartup(int loadOnStartup) { + return new EndpointServlet(this.servlet, this.initParameters, loadOnStartup); } Servlet getServlet() { @@ -78,4 +102,8 @@ public final class EndpointServlet { return this.initParameters; } + int getLoadOnStartup() { + return this.loadOnStartup; + } + } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrar.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrar.java index dec24a8794..ade3db9a9e 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrar.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrar.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -75,6 +75,7 @@ public class ServletEndpointRegistrar implements ServletContextInitializer { endpointServlet.getServlet()); registration.addMapping(urlMapping); registration.setInitParameters(endpointServlet.getInitParameters()); + registration.setLoadOnStartup(endpointServlet.getLoadOnStartup()); logger.info("Registered '" + path + "' to " + name); } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/EndpointServletTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/EndpointServletTests.java index 229d651b37..1c487bb325 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/EndpointServletTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/EndpointServletTests.java @@ -132,6 +132,32 @@ public class EndpointServletTests { } + @Test + public void withLoadOnStartupNotSetShouldReturnDefaultValue() { + EndpointServlet endpointServlet = new EndpointServlet(TestServlet.class); + assertThat(endpointServlet.getLoadOnStartup()).isEqualTo(-1); + } + + @Test + public void withLoadOnStartupSetShouldReturnValue() { + final int loadOnStartupTestValue = 3; + EndpointServlet endpointServlet = new EndpointServlet(TestServlet.class) + .withLoadOnStartup(loadOnStartupTestValue); + assertThat(endpointServlet.getLoadOnStartup()).isEqualTo(loadOnStartupTestValue); + } + + @Test + public void withLoadOnStartupAndInitParamsShouldReturnValue() { + final int loadOnStartupTestValue = 9; + EndpointServlet endpointServlet = new EndpointServlet(TestServlet.class) + .withLoadOnStartup(loadOnStartupTestValue).withInitParameter("a", "b") + .withInitParameter("c", "d"); + Map extra = new LinkedHashMap<>(); + extra.put("a", "b1"); + extra.put("e", "f"); + assertThat(endpointServlet.getLoadOnStartup()).isEqualTo(loadOnStartupTestValue); + } + private static class TestServlet extends GenericServlet { @Override diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrarTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrarTests.java index 399ace063d..3163bf8c09 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrarTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrarTests.java @@ -120,6 +120,30 @@ public class ServletEndpointRegistrarTests { verify(this.dynamic).setInitParameters(Collections.singletonMap("a", "b")); } + @Test + public void onStartupWhenHasLoadOnStartupShouldRegisterLoadOnStartup() + throws Exception { + final int loadOnStartupTestValue = 7; + ExposableServletEndpoint endpoint = mockEndpoint( + new EndpointServlet(TestServlet.class) + .withLoadOnStartup(loadOnStartupTestValue)); + ServletEndpointRegistrar registrar = new ServletEndpointRegistrar("/actuator", + Collections.singleton(endpoint)); + registrar.onStartup(this.servletContext); + verify(this.dynamic).setLoadOnStartup(loadOnStartupTestValue); + } + + @Test + public void onStartupWhenHasNotLoadOnStartupShouldRegisterDefaultValue() + throws Exception { + ExposableServletEndpoint endpoint = mockEndpoint( + new EndpointServlet(TestServlet.class)); + ServletEndpointRegistrar registrar = new ServletEndpointRegistrar("/actuator", + Collections.singleton(endpoint)); + registrar.onStartup(this.servletContext); + verify(this.dynamic).setLoadOnStartup(-1); + } + private ExposableServletEndpoint mockEndpoint(EndpointServlet endpointServlet) { ExposableServletEndpoint endpoint = mock(ExposableServletEndpoint.class); given(endpoint.getEndpointId()).willReturn(EndpointId.of("test")); From 4f029d6df90d86388251692e3b39663c603b6513 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 5 Mar 2019 10:59:18 +0100 Subject: [PATCH 2/2] Polish "Add loadOnStartup property to EndpointServlet" Closes gh-16053 --- .../actuate/endpoint/web/EndpointServlet.java | 32 +++++++++---------- .../endpoint/web/EndpointServletTests.java | 25 +++------------ .../web/ServletEndpointRegistrarTests.java | 8 ++--- 3 files changed, 22 insertions(+), 43 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/EndpointServlet.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/EndpointServlet.java index a72d0508fb..a3abd5e9ae 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/EndpointServlet.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/EndpointServlet.java @@ -21,6 +21,7 @@ import java.util.LinkedHashMap; import java.util.Map; import javax.servlet.Servlet; +import javax.servlet.ServletRegistration; import org.springframework.beans.BeanUtils; import org.springframework.util.Assert; @@ -30,36 +31,35 @@ import org.springframework.util.StringUtils; * Contains details of a servlet that is exposed as an actuator endpoint. * * @author Phillip Webb + * @author Julio José Gómez Díaz */ public final class EndpointServlet { private final Servlet servlet; - private final int loadOnStartup; - private final Map initParameters; - public EndpointServlet(Class servlet) { - Assert.notNull(servlet, "Servlet must not be null"); - this.servlet = BeanUtils.instantiateClass(servlet); - this.initParameters = Collections.emptyMap(); - this.loadOnStartup = -1; + private final int loadOnStartup; + public EndpointServlet(Class servlet) { + this(instantiateClass(servlet)); + } + + private static Servlet instantiateClass(Class servlet) { + Assert.notNull(servlet, "Servlet must not be null"); + return BeanUtils.instantiateClass(servlet); } public EndpointServlet(Servlet servlet) { - Assert.notNull(servlet, "Servlet must not be null"); - this.servlet = servlet; - this.initParameters = Collections.emptyMap(); - this.loadOnStartup = -1; + this(servlet, Collections.emptyMap(), -1); } private EndpointServlet(Servlet servlet, Map initParameters, int loadOnStartup) { + Assert.notNull(servlet, "Servlet must not be null"); this.servlet = servlet; this.initParameters = Collections.unmodifiableMap(initParameters); this.loadOnStartup = loadOnStartup; - } public EndpointServlet withInitParameter(String name, String value) { @@ -80,13 +80,11 @@ public final class EndpointServlet { } /** - * Sets the loadOnStartup priority that will be set on Servlet - * registration - *

- * The default value for loadOnStartup is -1. + * Sets the {@code loadOnStartup} priority that will be set on Servlet registration. + * The default value for {@code loadOnStartup} is {@code -1}. * @param loadOnStartup the initialization priority of the Servlet * @return a new instance of {@link EndpointServlet} with the provided - * loadOnStartup value set + * {@code loadOnStartup} value set * @since 2.2.0 * @see ServletRegistration.Dynamic#setLoadOnStartup(int) */ diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/EndpointServletTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/EndpointServletTests.java index 1c487bb325..6d03625a43 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/EndpointServletTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/EndpointServletTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -16,14 +16,12 @@ package org.springframework.boot.actuate.endpoint.web; -import java.io.IOException; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; import javax.servlet.GenericServlet; import javax.servlet.Servlet; -import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -129,7 +127,6 @@ public class EndpointServletTests { extra.put("e", "f"); assertThat(endpointServlet.withInitParameters(extra).getInitParameters()) .containsExactly(entry("a", "b1"), entry("c", "d"), entry("e", "f")); - } @Test @@ -140,29 +137,15 @@ public class EndpointServletTests { @Test public void withLoadOnStartupSetShouldReturnValue() { - final int loadOnStartupTestValue = 3; EndpointServlet endpointServlet = new EndpointServlet(TestServlet.class) - .withLoadOnStartup(loadOnStartupTestValue); - assertThat(endpointServlet.getLoadOnStartup()).isEqualTo(loadOnStartupTestValue); - } - - @Test - public void withLoadOnStartupAndInitParamsShouldReturnValue() { - final int loadOnStartupTestValue = 9; - EndpointServlet endpointServlet = new EndpointServlet(TestServlet.class) - .withLoadOnStartup(loadOnStartupTestValue).withInitParameter("a", "b") - .withInitParameter("c", "d"); - Map extra = new LinkedHashMap<>(); - extra.put("a", "b1"); - extra.put("e", "f"); - assertThat(endpointServlet.getLoadOnStartup()).isEqualTo(loadOnStartupTestValue); + .withLoadOnStartup(3); + assertThat(endpointServlet.getLoadOnStartup()).isEqualTo(3); } private static class TestServlet extends GenericServlet { @Override - public void service(ServletRequest req, ServletResponse res) - throws ServletException, IOException { + public void service(ServletRequest req, ServletResponse res) { } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrarTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrarTests.java index 3163bf8c09..62f4a44bed 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrarTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/web/ServletEndpointRegistrarTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -123,14 +123,12 @@ public class ServletEndpointRegistrarTests { @Test public void onStartupWhenHasLoadOnStartupShouldRegisterLoadOnStartup() throws Exception { - final int loadOnStartupTestValue = 7; ExposableServletEndpoint endpoint = mockEndpoint( - new EndpointServlet(TestServlet.class) - .withLoadOnStartup(loadOnStartupTestValue)); + new EndpointServlet(TestServlet.class).withLoadOnStartup(7)); ServletEndpointRegistrar registrar = new ServletEndpointRegistrar("/actuator", Collections.singleton(endpoint)); registrar.onStartup(this.servletContext); - verify(this.dynamic).setLoadOnStartup(loadOnStartupTestValue); + verify(this.dynamic).setLoadOnStartup(7); } @Test