From 57111aba22a65dd3707a5ba69b3340004e957858 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 1 Mar 2017 20:40:20 -0800 Subject: [PATCH 1/7] Get published DataSource from EntityManager Update DataSourceInitializedPublisher to always attempt to obtain the published DataSource directly from the EntityManager. In the case where the EntityManager doesn't provide a DataSource, the previous logic is used. Fixes gh-8296 --- .../jpa/DataSourceInitializedPublisher.java | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DataSourceInitializedPublisher.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DataSourceInitializedPublisher.java index 0a714869c1..74810e47a2 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DataSourceInitializedPublisher.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DataSourceInitializedPublisher.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2017 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. @@ -64,20 +64,33 @@ class DataSourceInitializedPublisher implements BeanPostProcessor { if (bean instanceof JpaProperties) { this.properties = (JpaProperties) bean; } - if (bean instanceof EntityManagerFactory && this.dataSource != null - && isInitializingDatabase()) { - this.applicationContext - .publishEvent(new DataSourceInitializedEvent(this.dataSource)); + if (bean instanceof EntityManagerFactory) { + publishEventIfRequired((EntityManagerFactory) bean); } return bean; } - private boolean isInitializingDatabase() { + private void publishEventIfRequired(EntityManagerFactory entityManagerFactory) { + DataSource dataSource = findDataSource(entityManagerFactory); + if (dataSource != null && isInitializingDatabase(dataSource)) { + this.applicationContext + .publishEvent(new DataSourceInitializedEvent(dataSource)); + } + } + + private DataSource findDataSource(EntityManagerFactory entityManagerFactory) { + Object dataSource = entityManagerFactory.getProperties() + .get("javax.persistence.nonJtaDataSource"); + return (dataSource != null && dataSource instanceof DataSource + ? (DataSource) dataSource : this.dataSource); + } + + private boolean isInitializingDatabase(DataSource dataSource) { if (this.properties == null) { return true; // better safe than sorry } Map hibernate = this.properties - .getHibernateProperties(this.dataSource); + .getHibernateProperties(dataSource); if (hibernate.containsKey("hibernate.hbm2ddl.auto")) { return true; } From 987b6c956e8bd9391f2411595191338c06bba4da Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 1 Mar 2017 20:43:04 -0800 Subject: [PATCH 2/7] Polish --- .../boot/autoconfigure/condition/OnBeanCondition.java | 2 +- .../boot/autoconfigure/condition/ConditionalOnBeanTests.java | 2 +- .../autoconfigure/condition/ConditionalOnMissingBeanTests.java | 2 +- .../boot/context/embedded/ApplicationBuilder.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java index e91af9b788..55bad88b22 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java index 7298fec979..336ff31d73 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java index 824b0e14d3..eab16b80b0 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/ApplicationBuilder.java b/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/ApplicationBuilder.java index 5d14ae2923..3073b64677 100644 --- a/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/ApplicationBuilder.java +++ b/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/ApplicationBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. From c06a9771c2bd1f9a84f68e411c729f994d7b690f Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 1 Mar 2017 21:33:45 -0800 Subject: [PATCH 3/7] Support list based role properties Update `HealthMvcEndpoint` to respect `ManagementServerProperties` roles. Prior to this commit the `HealthMvcEndpoint` directly loaded roles rather than using bound properties. This meant that list values from yaml were not respected. Fixes gh-8314 --- ...tWebMvcManagementContextConfiguration.java | 7 ++--- .../endpoint/mvc/HealthMvcEndpoint.java | 26 ++++++++++++++----- ...althMvcEndpointAutoConfigurationTests.java | 19 +++++++++++++- .../endpoint/mvc/HealthMvcEndpointTests.java | 16 +++++++++++- 4 files changed, 57 insertions(+), 11 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcManagementContextConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcManagementContextConfiguration.java index 67f8f1891e..04e4d37d9c 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcManagementContextConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcManagementContextConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -144,9 +144,10 @@ public class EndpointWebMvcManagementContextConfiguration { @Bean @ConditionalOnBean(HealthEndpoint.class) @ConditionalOnEnabledEndpoint("health") - public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate) { + public HealthMvcEndpoint healthMvcEndpoint(HealthEndpoint delegate, + ManagementServerProperties managementServerProperties) { HealthMvcEndpoint healthMvcEndpoint = new HealthMvcEndpoint(delegate, - isHealthSecure()); + isHealthSecure(), managementServerProperties.getSecurity().getRoles()); if (this.healthMvcEndpointProperties.getMapping() != null) { healthMvcEndpoint .addStatusMapping(this.healthMvcEndpointProperties.getMapping()); diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java index d6e55375d9..9be5eac950 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -57,6 +57,8 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter roles; + private Map statusMapping = new HashMap(); private RelaxedPropertyResolver healthPropertyResolver; @@ -70,13 +72,19 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter roles) { super(delegate); this.secure = secure; setupDefaultStatusMapping(); + this.roles = roles; } private void setupDefaultStatusMapping() { @@ -192,12 +200,9 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter roles = Arrays.asList(StringUtils - .trimArrayElements(StringUtils.commaDelimitedListToStringArray( - this.roleResolver.getProperty("roles", "ROLE_ADMIN")))); for (GrantedAuthority authority : authentication.getAuthorities()) { String name = authority.getAuthority(); - for (String role : roles) { + for (String role : getRoles()) { if (role.equals(name) || ("ROLE_" + role).equals(name)) { return true; } @@ -207,6 +212,15 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter getRoles() { + if (this.roles != null) { + return this.roles; + } + return Arrays.asList( + StringUtils.trimArrayElements(StringUtils.commaDelimitedListToStringArray( + this.roleResolver.getProperty("roles", "ROLE_ADMIN")))); + } + private boolean isSpringSecurityAuthentication(Principal principal) { return ClassUtils.isPresent("org.springframework.security.core.Authentication", null) && (principal instanceof Authentication); diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java index e1f9285e4b..2fe464c068 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthMvcEndpointAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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,6 +16,8 @@ package org.springframework.boot.actuate.autoconfigure; +import java.util.Arrays; + import org.junit.After; import org.junit.Test; @@ -33,6 +35,7 @@ import org.springframework.boot.test.util.EnvironmentTestUtils; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.mock.web.MockServletContext; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import static org.assertj.core.api.Assertions.assertThat; @@ -81,6 +84,20 @@ public class HealthMvcEndpointAutoConfigurationTests { assertThat(map.getDetails().get("foo")).isEqualTo("bar"); } + @Test + public void testSetRoles() throws Exception { + // gh-8314 + this.context = new AnnotationConfigWebApplicationContext(); + this.context.setServletContext(new MockServletContext()); + this.context.register(TestConfiguration.class); + EnvironmentTestUtils.addEnvironment(this.context, + "management.security.roles[0]=super"); + this.context.refresh(); + HealthMvcEndpoint health = this.context.getBean(HealthMvcEndpoint.class); + assertThat(ReflectionTestUtils.getField(health, "roles")) + .isEqualTo(Arrays.asList("super")); + } + @Configuration @ImportAutoConfiguration({ SecurityAutoConfiguration.class, JacksonAutoConfiguration.class, WebMvcAutoConfiguration.class, diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java index f6bd0c3d25..75b7bc0f61 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpointTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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,6 +16,7 @@ package org.springframework.boot.actuate.endpoint.mvc; +import java.util.Arrays; import java.util.Collections; import org.junit.Before; @@ -164,6 +165,19 @@ public class HealthMvcEndpointTests { assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); } + @Test + public void secureCustomRoleList() { + // gh-8314 + this.mvc = new HealthMvcEndpoint(this.endpoint, true, + Arrays.asList("HERO", "USER")); + given(this.endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); + Object result = this.mvc.invoke(this.hero); + assertThat(result instanceof Health).isTrue(); + assertThat(((Health) result).getStatus() == Status.UP).isTrue(); + assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); + } + @Test public void secureCustomRoleNoAccess() { this.environment.getPropertySources().addLast(SECURITY_ROLES); From 757aa647cfa44196c93f5730d0f7b33032b5981f Mon Sep 17 00:00:00 2001 From: "Michael K. Werle" Date: Tue, 7 Feb 2017 17:20:06 -0600 Subject: [PATCH 4/7] Ensure web containers are stopped after close Update `EmbeddedServletContainer` implementations to ensure that stop can be called even if start has not. This allows servers that are partially started during `initialize()` to still be shut down. This commit fixes a regression caused by commit 0af53b361f. See gh-8036 Fixes gh-8224 Closes gh-8227 --- .../embedded/jetty/JettyEmbeddedServletContainer.java | 3 --- .../tomcat/TomcatEmbeddedServletContainer.java | 3 --- .../JettyEmbeddedServletContainerFactoryTests.java | 10 ++++++++++ .../TomcatEmbeddedServletContainerFactoryTests.java | 10 ++++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java index a52a789078..8941c3d614 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java @@ -205,9 +205,6 @@ public class JettyEmbeddedServletContainer implements EmbeddedServletContainer { @Override public void stop() { synchronized (this.monitor) { - if (!this.started) { - return; - } this.started = false; try { this.server.stop(); diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java index 27ab3dc021..b97a66574d 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java @@ -279,9 +279,6 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer @Override public void stop() throws EmbeddedServletContainerException { synchronized (this.monitor) { - if (!this.started) { - return; - } try { this.started = false; try { diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java index cedeaae784..265b7e573b 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java @@ -143,6 +143,16 @@ public class JettyEmbeddedServletContainerFactoryTests .isEmpty(); } + @Test + public void stopNoStart() throws Exception { + JettyEmbeddedServletContainerFactory factory = getFactory(); + this.container = factory + .getEmbeddedServletContainer(exampleServletRegistration()); + this.container.stop(); + Server server = ((JettyEmbeddedServletContainer) this.container).getServer(); + assertThat(server.isStopped()).isTrue(); + } + @Override protected void addConnector(final int port, AbstractEmbeddedServletContainerFactory factory) { diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java index ac3690d905..cbb54278b7 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java @@ -352,6 +352,16 @@ public class TomcatEmbeddedServletContainerFactoryTests .doesNotContain("appears to have started a thread named [main]"); } + @Test + public void stopNoStart() throws Exception { + TomcatEmbeddedServletContainerFactory factory = getFactory(); + this.container = factory + .getEmbeddedServletContainer(exampleServletRegistration()); + this.container.stop(); + Tomcat tomcat = ((TomcatEmbeddedServletContainer) this.container).getTomcat(); + assertThat(tomcat.getServer().getState()).isSameAs(LifecycleState.DESTROYED); + } + @Override protected void addConnector(int port, AbstractEmbeddedServletContainerFactory factory) { From 7fda9c162e16580735bbf39dcf96c0a4874d459c Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 1 Mar 2017 22:02:25 -0800 Subject: [PATCH 5/7] Polish web containers stop contribution See gh-8227 --- ...tEmbeddedServletContainerFactoryTests.java | 20 +++++++++---------- ...yEmbeddedServletContainerFactoryTests.java | 2 +- ...tEmbeddedServletContainerFactoryTests.java | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java index 11482e73a9..6f30f6dd10 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java @@ -170,6 +170,16 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { assertThat(this.output.toString()).containsOnlyOnce("started on port"); } + @Test + public void stopCalledTwice() throws Exception { + AbstractEmbeddedServletContainerFactory factory = getFactory(); + this.container = factory + .getEmbeddedServletContainer(exampleServletRegistration()); + this.container.start(); + this.container.stop(); + this.container.stop(); + } + @Test public void emptyServerWhenPortIsMinusOne() throws Exception { AbstractEmbeddedServletContainerFactory factory = getFactory(); @@ -311,16 +321,6 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { getFactory().setContextPath("/"); } - @Test - public void doubleStop() throws Exception { - AbstractEmbeddedServletContainerFactory factory = getFactory(); - this.container = factory - .getEmbeddedServletContainer(exampleServletRegistration()); - this.container.start(); - this.container.stop(); - this.container.stop(); - } - @Test public void multipleConfigurations() throws Exception { AbstractEmbeddedServletContainerFactory factory = getFactory(); diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java index 265b7e573b..def6a3f8bd 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java @@ -144,7 +144,7 @@ public class JettyEmbeddedServletContainerFactoryTests } @Test - public void stopNoStart() throws Exception { + public void stopCalledWithoutStart() throws Exception { JettyEmbeddedServletContainerFactory factory = getFactory(); this.container = factory .getEmbeddedServletContainer(exampleServletRegistration()); diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java index cbb54278b7..1fa3534209 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java @@ -353,7 +353,7 @@ public class TomcatEmbeddedServletContainerFactoryTests } @Test - public void stopNoStart() throws Exception { + public void stopCalledWithoutStart() throws Exception { TomcatEmbeddedServletContainerFactory factory = getFactory(); this.container = factory .getEmbeddedServletContainer(exampleServletRegistration()); From 5aafbc2a3b1d633f8aa436025943146b10fae5ff Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 1 Mar 2017 22:36:14 -0800 Subject: [PATCH 6/7] Refine engine counter logic Update counter logic to prevent negative values. Since the stop method can now be called more than once, it was possible for the counter to move into negative values. See gh-8227 --- .../TomcatEmbeddedServletContainer.java | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java index b97a66574d..b9a59b4fc2 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java @@ -90,28 +90,34 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer synchronized (this.monitor) { try { addInstanceIdToEngineName(); + try { + // Remove service connectors to that protocol binding doesn't happen + // yet + removeServiceConnectors(); - // Remove service connectors to that protocol binding doesn't happen yet - removeServiceConnectors(); + // Start the server to trigger initialization listeners + this.tomcat.start(); - // Start the server to trigger initialization listeners - this.tomcat.start(); + // We can re-throw failure exception directly in the main thread + rethrowDeferredStartupExceptions(); - // We can re-throw failure exception directly in the main thread - rethrowDeferredStartupExceptions(); + Context context = findContext(); + try { + ContextBindings.bindClassLoader(context, getNamingToken(context), + getClass().getClassLoader()); + } + catch (NamingException ex) { + // Naming is not enabled. Continue + } - Context context = findContext(); - try { - ContextBindings.bindClassLoader(context, getNamingToken(context), - getClass().getClassLoader()); + // Unlike Jetty, all Tomcat threads are daemon threads. We create a + // blocking non-daemon to stop immediate shutdown + startDaemonAwaitThread(); } - catch (NamingException ex) { - // Naming is not enabled. Continue + catch (Exception ex) { + containerCounter.decrementAndGet(); + throw ex; } - - // Unlike Jetty, all Tomcat threads are daemon threads. We create a - // blocking non-daemon to stop immediate shutdown - startDaemonAwaitThread(); } catch (Exception ex) { throw new EmbeddedServletContainerException( @@ -279,6 +285,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer @Override public void stop() throws EmbeddedServletContainerException { synchronized (this.monitor) { + boolean wasStarted = this.started; try { this.started = false; try { @@ -294,7 +301,9 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer "Unable to stop embedded Tomcat", ex); } finally { - containerCounter.decrementAndGet(); + if (wasStarted) { + containerCounter.decrementAndGet(); + } } } } From f1012c104a9c2c0211682cdf9174a1a355fbd63d Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 1 Mar 2017 23:28:59 -0800 Subject: [PATCH 7/7] Polish --- .../mvc/MvcEndpointSecurityInterceptor.java | 5 ++-- .../MvcEndpointSecurityInterceptorTests.java | 14 ++++++---- ...tyMvcEndpointSecurityInterceptorTests.java | 9 ++++--- .../condition/ConditionalOnBeanTests.java | 2 +- .../WebSocketAutoConfigurationTests.java | 2 +- .../context/embedded/ApplicationBuilder.java | 2 +- .../maven/AbstractDependencyFilterMojo.java | 26 +++++-------------- .../boot/maven/DependencyFilterMojoTests.java | 5 ++-- 8 files changed, 30 insertions(+), 35 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java index d60b271912..aab244dcaa 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -128,7 +128,8 @@ public class MvcEndpointSecurityInterceptor extends HandlerInterceptorAdapter { private class AuthoritiesValidator { private boolean hasAuthority(String role) { - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + Authentication authentication = SecurityContextHolder.getContext() + .getAuthentication(); if (authentication != null) { for (GrantedAuthority authority : authentication.getAuthorities()) { if (authority.getAuthority().equals(role)) { diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java index 1aaedad86a..167ccd35be 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/MvcEndpointSecurityInterceptorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -130,11 +130,13 @@ public class MvcEndpointSecurityInterceptorTests { } @Test - public void sensitiveEndpointIfRoleNotCorrectShouldCheckAuthorities() throws Exception { + public void sensitiveEndpointIfRoleNotCorrectShouldCheckAuthorities() + throws Exception { Principal principal = mock(Principal.class); this.request.setUserPrincipal(principal); Authentication authentication = mock(Authentication.class); - Set authorities = Collections.singleton(new SimpleGrantedAuthority("SUPER_HERO")); + Set authorities = Collections + .singleton(new SimpleGrantedAuthority("SUPER_HERO")); doReturn(authorities).when(authentication).getAuthorities(); SecurityContextHolder.getContext().setAuthentication(authentication); assertThat(this.securityInterceptor.preHandle(this.request, this.response, @@ -142,11 +144,13 @@ public class MvcEndpointSecurityInterceptorTests { } @Test - public void sensitiveEndpointIfRoleAndAuthoritiesNotCorrectShouldNotAllowAccess() throws Exception { + public void sensitiveEndpointIfRoleAndAuthoritiesNotCorrectShouldNotAllowAccess() + throws Exception { Principal principal = mock(Principal.class); this.request.setUserPrincipal(principal); Authentication authentication = mock(Authentication.class); - Set authorities = Collections.singleton(new SimpleGrantedAuthority("HERO")); + Set authorities = Collections + .singleton(new SimpleGrantedAuthority("HERO")); doReturn(authorities).when(authentication).getAuthorities(); SecurityContextHolder.getContext().setAuthentication(authentication); assertThat(this.securityInterceptor.preHandle(this.request, this.response, diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/NoSpringSecurityMvcEndpointSecurityInterceptorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/NoSpringSecurityMvcEndpointSecurityInterceptorTests.java index 6d5ad0ceef..6515265831 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/NoSpringSecurityMvcEndpointSecurityInterceptorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/NoSpringSecurityMvcEndpointSecurityInterceptorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -39,6 +39,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; /** + * Tests for {@link MvcEndpointSecurityInterceptor} when Spring Security is not available. + * * @author Madhura Bhave */ @RunWith(ModifiedClassPathRunner.class) @@ -77,7 +79,8 @@ public class NoSpringSecurityMvcEndpointSecurityInterceptorTests { } @Test - public void sensitiveEndpointIfRoleNotPresentShouldNotValidateAuthorities() throws Exception { + public void sensitiveEndpointIfRoleNotPresentShouldNotValidateAuthorities() + throws Exception { Principal principal = mock(Principal.class); this.request.setUserPrincipal(principal); this.servletContext.declareRoles("HERO"); @@ -105,5 +108,5 @@ public class NoSpringSecurityMvcEndpointSecurityInterceptorTests { } } -} +} diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java index 7298fec979..336ff31d73 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/WebSocketAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/WebSocketAutoConfigurationTests.java index 050fae10cd..1ad3738297 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/WebSocketAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/WebSocketAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/ApplicationBuilder.java b/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/ApplicationBuilder.java index 5d14ae2923..3073b64677 100644 --- a/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/ApplicationBuilder.java +++ b/spring-boot-integration-tests/spring-boot-integration-tests-embedded-servlet-container/src/test/java/org/springframework/boot/context/embedded/ApplicationBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/AbstractDependencyFilterMojo.java b/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/AbstractDependencyFilterMojo.java index 0cb0ee4a27..bcb5b2dbed 100644 --- a/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/AbstractDependencyFilterMojo.java +++ b/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/AbstractDependencyFilterMojo.java @@ -16,7 +16,7 @@ package org.springframework.boot.maven; -import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import java.util.StringTokenizer; @@ -87,29 +87,15 @@ public abstract class AbstractDependencyFilterMojo extends AbstractMojo { this.excludeArtifactIds = excludeArtifactIds; } - @SuppressWarnings("unchecked") protected Set filterDependencies(Set dependencies, FilterArtifacts filters) throws MojoExecutionException { - List artifactsFilters = filters.getFilters(); try { - for (ArtifactsFilter filter : artifactsFilters) { - Set result = filter.filter(dependencies); - applyFiltering(dependencies, result); - } - return dependencies; - } - catch (ArtifactFilterException e) { - throw new MojoExecutionException(e.getMessage(), e); + Set filtered = new LinkedHashSet(dependencies); + filtered.retainAll(filters.filter(dependencies)); + return filtered; } - } - - private void applyFiltering(Set original, Set filtered) { - Iterator iterator = original.iterator(); - while (iterator.hasNext()) { - Artifact element = iterator.next(); - if (!filtered.contains(element)) { - iterator.remove(); - } + catch (ArtifactFilterException ex) { + throw new MojoExecutionException(ex.getMessage(), ex); } } diff --git a/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/DependencyFilterMojoTests.java b/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/DependencyFilterMojoTests.java index daa49766ae..212e711d12 100644 --- a/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/DependencyFilterMojoTests.java +++ b/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/DependencyFilterMojoTests.java @@ -103,7 +103,7 @@ public class DependencyFilterMojoTests { } @Test - public void filterExcludeKeepOrder() throws MojoExecutionException { + public void filterExcludeKeepOrder() throws MojoExecutionException { Exclude exclude = new Exclude(); exclude.setGroupId("com.bar"); exclude.setArtifactId("two"); @@ -121,7 +121,8 @@ public class DependencyFilterMojoTests { return createArtifact(groupId, artifactId, null); } - private static Artifact createArtifact(String groupId, String artifactId, String scope) { + private static Artifact createArtifact(String groupId, String artifactId, + String scope) { Artifact a = mock(Artifact.class); given(a.getGroupId()).willReturn(groupId); given(a.getArtifactId()).willReturn(artifactId);