From ecf393461e7c3f57f54c76234fc1f65c99350b71 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 24 Sep 2019 16:11:15 -0700 Subject: [PATCH] Skip management context ResourceConfigCustomizers Update `JerseyManagementContextConfiguration` so that customizer beans are not longer applied. The endpoint resource endpoints are now added with a registrar bean `@PostConstruct` method. Prior to this commit, when running the management server on a different port a `Resource` added by a customizer could be added two different `ResourceConfig` instance. This breaks the singleton contract expected by Jersey. Fixes gh-17801 Co-authored-by: Phillip Webb --- ...ndpointManagementContextConfiguration.java | 80 ++++++++++++--- .../JerseyManagementContextConfiguration.java | 8 +- ...seySameManagementContextConfiguration.java | 2 +- ...ntManagementContextConfigurationTests.java | 8 +- ...ldManagementContextConfigurationTests.java | 25 ----- ...meManagementContextConfigurationTests.java | 22 ----- .../jersey/JerseyManagementPortTests.java | 98 +++++++++++++++++++ 7 files changed, 171 insertions(+), 72 deletions(-) create mode 100644 spring-boot-samples/spring-boot-sample-jersey/src/test/java/sample/jersey/JerseyManagementPortTests.java diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/jersey/JerseyWebEndpointManagementContextConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/jersey/JerseyWebEndpointManagementContextConfiguration.java index 41c30c3b008..d44d327fa8d 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/jersey/JerseyWebEndpointManagementContextConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/jersey/JerseyWebEndpointManagementContextConfiguration.java @@ -18,12 +18,15 @@ package org.springframework.boot.actuate.autoconfigure.endpoint.web.jersey; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; import java.util.List; -import org.glassfish.jersey.server.ResourceConfig; +import javax.annotation.PostConstruct; +import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.server.model.Resource; + +import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointProperties; import org.springframework.boot.actuate.autoconfigure.web.ManagementContextConfiguration; import org.springframework.boot.actuate.endpoint.ExposableEndpoint; @@ -31,6 +34,7 @@ import org.springframework.boot.actuate.endpoint.annotation.Endpoint; import org.springframework.boot.actuate.endpoint.web.EndpointLinksResolver; import org.springframework.boot.actuate.endpoint.web.EndpointMapping; import org.springframework.boot.actuate.endpoint.web.EndpointMediaTypes; +import org.springframework.boot.actuate.endpoint.web.ExposableServletEndpoint; import org.springframework.boot.actuate.endpoint.web.ExposableWebEndpoint; import org.springframework.boot.actuate.endpoint.web.WebEndpointsSupplier; import org.springframework.boot.actuate.endpoint.web.annotation.ServletEndpointsSupplier; @@ -59,21 +63,69 @@ import org.springframework.context.annotation.Bean; class JerseyWebEndpointManagementContextConfiguration { @Bean - public ResourceConfigCustomizer webEndpointRegistrar(WebEndpointsSupplier webEndpointsSupplier, + JerseyWebEndpointsResourcesRegistrar jerseyWebEndpointsResourcesRegistrar( + ObjectProvider resourceConfig, WebEndpointsSupplier webEndpointsSupplier, ServletEndpointsSupplier servletEndpointsSupplier, EndpointMediaTypes endpointMediaTypes, WebEndpointProperties webEndpointProperties) { - List> allEndpoints = new ArrayList<>(); - allEndpoints.addAll(webEndpointsSupplier.getEndpoints()); - allEndpoints.addAll(servletEndpointsSupplier.getEndpoints()); - return (resourceConfig) -> { + return new JerseyWebEndpointsResourcesRegistrar(resourceConfig.getIfAvailable(), webEndpointsSupplier, + servletEndpointsSupplier, endpointMediaTypes, webEndpointProperties.getBasePath()); + } + + /** + * Register endpoints with the {@link ResourceConfig}. The + * {@link ResourceConfigCustomizer} cannot be used because we don't want to apply + */ + static class JerseyWebEndpointsResourcesRegistrar { + + private final ResourceConfig resourceConfig; + + private final WebEndpointsSupplier webEndpointsSupplier; + + private final ServletEndpointsSupplier servletEndpointsSupplier; + + private final EndpointMediaTypes mediaTypes; + + private final String basePath; + + JerseyWebEndpointsResourcesRegistrar(ResourceConfig resourceConfig, WebEndpointsSupplier webEndpointsSupplier, + ServletEndpointsSupplier servletEndpointsSupplier, EndpointMediaTypes endpointMediaTypes, + String basePath) { + super(); + this.resourceConfig = resourceConfig; + this.webEndpointsSupplier = webEndpointsSupplier; + this.servletEndpointsSupplier = servletEndpointsSupplier; + this.mediaTypes = endpointMediaTypes; + this.basePath = basePath; + } + + @PostConstruct + void register() { + // We can't easily use @ConditionalOnBean because @AutoConfigureBefore is + // not an option for management contexts. Instead we manually check if + // the resource config bean exists + if (this.resourceConfig == null) { + return; + } + Collection webEndpoints = this.webEndpointsSupplier.getEndpoints(); + Collection servletEndpoints = this.servletEndpointsSupplier.getEndpoints(); + EndpointLinksResolver linksResolver = getLinksResolver(webEndpoints, servletEndpoints); + EndpointMapping mapping = new EndpointMapping(this.basePath); JerseyEndpointResourceFactory resourceFactory = new JerseyEndpointResourceFactory(); - String basePath = webEndpointProperties.getBasePath(); - EndpointMapping endpointMapping = new EndpointMapping(basePath); - Collection webEndpoints = Collections - .unmodifiableCollection(webEndpointsSupplier.getEndpoints()); - resourceConfig.registerResources(new HashSet<>(resourceFactory.createEndpointResources(endpointMapping, - webEndpoints, endpointMediaTypes, new EndpointLinksResolver(allEndpoints, basePath)))); - }; + register(resourceFactory.createEndpointResources(mapping, webEndpoints, this.mediaTypes, linksResolver)); + } + + private EndpointLinksResolver getLinksResolver(Collection webEndpoints, + Collection servletEndpoints) { + List> endpoints = new ArrayList<>(webEndpoints.size() + servletEndpoints.size()); + endpoints.addAll(webEndpoints); + endpoints.addAll(servletEndpoints); + return new EndpointLinksResolver(endpoints, this.basePath); + } + + private void register(Collection resources) { + this.resourceConfig.registerResources(new HashSet<>(resources)); + } + } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseyManagementContextConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseyManagementContextConfiguration.java index b3162ceb1cd..a62f0a8d43a 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseyManagementContextConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseyManagementContextConfiguration.java @@ -18,8 +18,6 @@ package org.springframework.boot.actuate.autoconfigure.web.jersey; import org.glassfish.jersey.server.ResourceConfig; import org.glassfish.jersey.servlet.ServletContainer; -import org.springframework.beans.factory.ObjectProvider; -import org.springframework.boot.autoconfigure.jersey.ResourceConfigCustomizer; import org.springframework.boot.autoconfigure.web.servlet.JerseyApplicationPath; import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.context.annotation.Bean; @@ -41,10 +39,8 @@ class JerseyManagementContextConfiguration { } @Bean - public ResourceConfig resourceConfig(ObjectProvider resourceConfigCustomizers) { - ResourceConfig resourceConfig = new ResourceConfig(); - resourceConfigCustomizers.orderedStream().forEach((customizer) -> customizer.customize(resourceConfig)); - return resourceConfig; + public ResourceConfig resourceConfig() { + return new ResourceConfig(); } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseySameManagementContextConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseySameManagementContextConfiguration.java index bbee55bb696..6909d2d06f0 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseySameManagementContextConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseySameManagementContextConfiguration.java @@ -38,9 +38,9 @@ import org.springframework.context.annotation.Import; * @since 2.1.0 */ @ManagementContextConfiguration(ManagementContextType.SAME) -@ConditionalOnMissingBean(ResourceConfig.class) @Import(JerseyManagementContextConfiguration.class) @EnableConfigurationProperties(JerseyProperties.class) +@ConditionalOnMissingBean(ResourceConfig.class) @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET) @ConditionalOnClass(ResourceConfig.class) @ConditionalOnMissingClass("org.springframework.web.servlet.DispatcherServlet") diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/jersey/JerseyWebEndpointManagementContextConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/jersey/JerseyWebEndpointManagementContextConfigurationTests.java index 83a9528b360..1f321241723 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/jersey/JerseyWebEndpointManagementContextConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/jersey/JerseyWebEndpointManagementContextConfigurationTests.java @@ -22,10 +22,10 @@ import org.glassfish.jersey.server.ResourceConfig; import org.junit.Test; import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointAutoConfiguration; +import org.springframework.boot.actuate.autoconfigure.endpoint.web.jersey.JerseyWebEndpointManagementContextConfiguration.JerseyWebEndpointsResourcesRegistrar; import org.springframework.boot.actuate.autoconfigure.web.jersey.JerseySameManagementContextConfiguration; import org.springframework.boot.actuate.endpoint.web.WebEndpointsSupplier; import org.springframework.boot.autoconfigure.AutoConfigurations; -import org.springframework.boot.autoconfigure.jersey.ResourceConfigCustomizer; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; @@ -45,11 +45,11 @@ public class JerseyWebEndpointManagementContextConfigurationTests { private final WebApplicationContextRunner runner = new WebApplicationContextRunner() .withConfiguration(AutoConfigurations.of(WebEndpointAutoConfiguration.class, JerseyWebEndpointManagementContextConfiguration.class)) - .withUserConfiguration(WebEndpointsSupplierConfig.class); + .withUserConfiguration(ResourceConfig.class, WebEndpointsSupplierConfig.class); @Test - public void resourceConfigCustomizerForEndpointsIsAutoConfigured() { - this.runner.run((context) -> assertThat(context).hasSingleBean(ResourceConfigCustomizer.class)); + public void jerseyWebEndpointsResourcesRegistrarForEndpointsIsAutoConfigured() { + this.runner.run((context) -> assertThat(context).hasSingleBean(JerseyWebEndpointsResourcesRegistrar.class)); } @Test diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseyChildManagementContextConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseyChildManagementContextConfigurationTests.java index 895d76e5921..808a3de5487 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseyChildManagementContextConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseyChildManagementContextConfigurationTests.java @@ -22,7 +22,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.boot.autoconfigure.AutoConfigurations; -import org.springframework.boot.autoconfigure.jersey.ResourceConfigCustomizer; import org.springframework.boot.autoconfigure.web.servlet.JerseyApplicationPath; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.runner.ApplicationContextRunner; @@ -30,12 +29,8 @@ import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.boot.testsupport.runner.classpath.ClassPathExclusions; import org.springframework.boot.testsupport.runner.classpath.ModifiedClassPathRunner; import org.springframework.boot.web.servlet.ServletRegistrationBean; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; /** * Tests for {@link JerseyChildManagementContextConfiguration}. @@ -64,16 +59,6 @@ public class JerseyChildManagementContextConfigurationTests { .run((context) -> assertThat(context).doesNotHaveBean(JerseySameManagementContextConfiguration.class)); } - @Test - public void resourceConfigIsCustomizedWithResourceConfigCustomizerBean() { - this.contextRunner.withUserConfiguration(CustomizerConfiguration.class).run((context) -> { - assertThat(context).hasSingleBean(ResourceConfig.class); - ResourceConfig config = context.getBean(ResourceConfig.class); - ResourceConfigCustomizer customizer = context.getBean(ResourceConfigCustomizer.class); - verify(customizer).customize(config); - }); - } - @Test public void jerseyApplicationPathIsAutoConfigured() { this.contextRunner.run((context) -> { @@ -96,14 +81,4 @@ public class JerseyChildManagementContextConfigurationTests { this.contextRunner.run((context) -> assertThat(context).hasSingleBean(ResourceConfig.class)); } - @Configuration - static class CustomizerConfiguration { - - @Bean - ResourceConfigCustomizer resourceConfigCustomizer() { - return mock(ResourceConfigCustomizer.class); - } - - } - } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseySameManagementContextConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseySameManagementContextConfigurationTests.java index 9a7f6e7ddd0..c42a5644ad0 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseySameManagementContextConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/jersey/JerseySameManagementContextConfigurationTests.java @@ -21,7 +21,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.boot.autoconfigure.AutoConfigurations; -import org.springframework.boot.autoconfigure.jersey.ResourceConfigCustomizer; import org.springframework.boot.autoconfigure.web.servlet.DefaultJerseyApplicationPath; import org.springframework.boot.autoconfigure.web.servlet.JerseyApplicationPath; import org.springframework.boot.test.context.FilteredClassLoader; @@ -35,7 +34,6 @@ import org.springframework.context.annotation.Configuration; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; /** * Tests for {@link JerseySameManagementContextConfiguration}. @@ -63,16 +61,6 @@ public class JerseySameManagementContextConfigurationTests { .run((context) -> assertThat(context).doesNotHaveBean(JerseySameManagementContextConfiguration.class)); } - @Test - public void resourceConfigIsCustomizedWithResourceConfigCustomizerBean() { - this.contextRunner.withUserConfiguration(CustomizerConfiguration.class).run((context) -> { - assertThat(context).hasSingleBean(ResourceConfig.class); - ResourceConfig config = context.getBean(ResourceConfig.class); - ResourceConfigCustomizer customizer = context.getBean(ResourceConfigCustomizer.class); - verify(customizer).customize(config); - }); - } - @Test public void jerseyApplicationPathIsAutoConfiguredWhenNeeded() { this.contextRunner.run((context) -> assertThat(context).hasSingleBean(DefaultJerseyApplicationPath.class)); @@ -125,14 +113,4 @@ public class JerseySameManagementContextConfigurationTests { } - @Configuration - static class CustomizerConfiguration { - - @Bean - ResourceConfigCustomizer resourceConfigCustomizer() { - return mock(ResourceConfigCustomizer.class); - } - - } - } diff --git a/spring-boot-samples/spring-boot-sample-jersey/src/test/java/sample/jersey/JerseyManagementPortTests.java b/spring-boot-samples/spring-boot-sample-jersey/src/test/java/sample/jersey/JerseyManagementPortTests.java new file mode 100644 index 00000000000..05c0b6b9bd2 --- /dev/null +++ b/spring-boot-samples/spring-boot-sample-jersey/src/test/java/sample/jersey/JerseyManagementPortTests.java @@ -0,0 +1,98 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package sample.jersey; + +import javax.ws.rs.GET; +import javax.ws.rs.Path; + +import org.glassfish.jersey.server.ResourceConfig; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.actuate.autoconfigure.web.server.LocalManagementPort; +import org.springframework.boot.autoconfigure.jersey.ResourceConfigCustomizer; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.boot.web.server.LocalServerPort; +import org.springframework.context.annotation.Bean; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests for separate management and main service ports. + * + * @author Madhura Bhave + */ +@RunWith(SpringRunner.class) +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, properties = "management.server.port=0") +public class JerseyManagementPortTests { + + @LocalServerPort + private int port; + + @LocalManagementPort + private int managementPort; + + @Autowired + private TestRestTemplate testRestTemplate; + + @Test + public void resourceShouldBeAvailableOnMainPort() { + ResponseEntity entity = this.testRestTemplate.getForEntity("http://localhost:" + this.port + "/test", + String.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(entity.getBody()).contains("test"); + } + + @Test + public void resourceShouldNotBeAvailableOnManagementPort() { + ResponseEntity entity = this.testRestTemplate + .getForEntity("http://localhost:" + this.managementPort + "/test", String.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + } + + @TestConfiguration + static class ResourceConfigConfiguration { + + @Bean + public ResourceConfigCustomizer customizer() { + return new ResourceConfigCustomizer() { + @Override + public void customize(ResourceConfig config) { + config.register(TestEndpoint.class); + } + }; + } + + @Path("/test") + public static class TestEndpoint { + + @GET + public String test() { + return "test"; + } + + } + + } + +}