From ec0e9b17add9bef60b2321458a56011f674de406 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 30 Apr 2013 17:36:55 +0100 Subject: [PATCH] More ordering issues in service apps * The TraceAutoConfiguration in the service project was loading too early because it contains a BPP * It also had a Spring Security dependency without any @Conditional* configuration * Fixed by nesting the BPP in a class with @Conditional* --- .../bootstrap/cli/CleanCommand.java | 2 + spring-bootstrap-cli/src/main/scripts/spring | 20 ++- ...tdownServiceBootstrapApplicationTests.java | 120 ++++++++++++++++++ spring-bootstrap-service/pom.xml | 1 + .../service/ContainerConfiguration.java | 2 + .../service/TraceAutoConfiguration.java | 34 +++-- .../trace/SecurityFilterPostProcessor.java | 17 ++- 7 files changed, 173 insertions(+), 23 deletions(-) create mode 100644 spring-bootstrap-samples/spring-bootstrap-service-sample/src/test/java/org/springframework/bootstrap/sample/service/ShutdownServiceBootstrapApplicationTests.java diff --git a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/CleanCommand.java b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/CleanCommand.java index a5461b9812..37eedc3fce 100644 --- a/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/CleanCommand.java +++ b/spring-bootstrap-cli/src/main/java/org/springframework/bootstrap/cli/CleanCommand.java @@ -99,11 +99,13 @@ public class CleanCommand extends OptionParsingCommand { if (file.exists()) { if (options.has(this.allOption) || group.equals("org.springframework.bootstrap")) { + System.out.println("Deleting: " + file); FileUtil.forceDelete(file); } else { for (Object obj : FileUtil.listAll(file, Collections.emptyList())) { File candidate = (File) obj; if (candidate.getName().contains("SNAPSHOT")) { + System.out.println("Deleting: " + candidate); FileUtil.forceDelete(candidate); } } diff --git a/spring-bootstrap-cli/src/main/scripts/spring b/spring-bootstrap-cli/src/main/scripts/spring index 28120e8a56..19121254f5 100755 --- a/spring-bootstrap-cli/src/main/scripts/spring +++ b/spring-bootstrap-cli/src/main/scripts/spring @@ -1,4 +1,4 @@ -#!/bin/bash -x +#!/bin/bash # OS specific support (must be 'true' or 'false'). cygwin=false; @@ -48,8 +48,14 @@ fi if [ "$SPRING_HOME" == "" ]; then SPRING_HOME=`cd "$(dirname $0)"/.. && pwd` +else + if [ ! -d "${SPRING_HOME}" ]; then + echo "Not a directory: SPRING_HOME=${SPRING_HOME}" + echo "Please rectify and restart." + exit 2 + fi + SPRING_HOME=`cd "${SPRING_HOME}" && pwd` fi -SPRING_HOME=`cd "${SPRING_HOME}" && pwd` SPRING_BIN=$(dirname $0) TARGETDIR=target/classes @@ -60,15 +66,7 @@ mkdir -p "${TARGETDIR%/}" CLASSPATH="${CLASSPATH}":"${SPRING_BIN}":"${TARGETDIR}" -if [ ! "${SPRING_BIN}" == "${SPRING_HOME}" ]; then - CLASSPATH="${CLASSPATH}":"${SPRING_HOME}"/classes -fi - -for f in "${SPRING_HOME}"/lib/*.jar; do - [ -f $f ] && CLASSPATH="${CLASSPATH}":$f -done - -for f in "${SPRING_HOME}"/*.jar; do +for f in "${SPRING_HOME}"/*.jar "${SPRING_HOME}"/lib/*.jar "${SPRING_HOME}"/classes; do [ -f $f ] && CLASSPATH="${CLASSPATH}":$f done diff --git a/spring-bootstrap-samples/spring-bootstrap-service-sample/src/test/java/org/springframework/bootstrap/sample/service/ShutdownServiceBootstrapApplicationTests.java b/spring-bootstrap-samples/spring-bootstrap-service-sample/src/test/java/org/springframework/bootstrap/sample/service/ShutdownServiceBootstrapApplicationTests.java new file mode 100644 index 0000000000..9b56b20ef5 --- /dev/null +++ b/spring-bootstrap-samples/spring-bootstrap-service-sample/src/test/java/org/springframework/bootstrap/sample/service/ShutdownServiceBootstrapApplicationTests.java @@ -0,0 +1,120 @@ +package org.springframework.bootstrap.sample.service; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.Callable; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.springframework.bootstrap.SpringApplication; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.http.HttpRequest; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.http.client.ClientHttpRequestExecution; +import org.springframework.http.client.ClientHttpRequestInterceptor; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.http.client.InterceptingClientHttpRequestFactory; +import org.springframework.http.client.SimpleClientHttpRequestFactory; +import org.springframework.security.crypto.codec.Base64; +import org.springframework.web.client.DefaultResponseErrorHandler; +import org.springframework.web.client.RestTemplate; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Integration tests for separate management and main service ports. + * + * @author Dave Syer + * + */ +public class ShutdownServiceBootstrapApplicationTests { + + private static ConfigurableApplicationContext context; + + @BeforeClass + public static void start() throws Exception { + Future future = Executors + .newSingleThreadExecutor().submit( + new Callable() { + @Override + public ConfigurableApplicationContext call() throws Exception { + return (ConfigurableApplicationContext) SpringApplication + .run(ServiceBootstrapApplication.class); + } + }); + context = future.get(10, TimeUnit.SECONDS); + } + + @AfterClass + public static void stop() { + if (context != null) { + context.close(); + } + } + + @Test + public void testHome() throws Exception { + @SuppressWarnings("rawtypes") + ResponseEntity entity = getRestTemplate("user", "password").getForEntity( + "http://localhost:8080", Map.class); + assertEquals(HttpStatus.OK, entity.getStatusCode()); + @SuppressWarnings("unchecked") + Map body = entity.getBody(); + assertEquals("Hello Phil", body.get("message")); + } + + @Test + public void testShutdown() throws Exception { + @SuppressWarnings("rawtypes") + ResponseEntity entity = getRestTemplate("user", "password").postForEntity( + "http://localhost:8080/shutdown", null, Map.class); + assertEquals(HttpStatus.OK, entity.getStatusCode()); + @SuppressWarnings("unchecked") + Map body = entity.getBody(); + assertTrue("Wrong body: " + body, + ((String) body.get("message")).contains("Shutting down")); + } + + private RestTemplate getRestTemplate(final String username, final String password) { + + List interceptors = new ArrayList(); + + if (username != null) { + + interceptors.add(new ClientHttpRequestInterceptor() { + + @Override + public ClientHttpResponse intercept(HttpRequest request, byte[] body, + ClientHttpRequestExecution execution) throws IOException { + request.getHeaders().add( + "Authorization", + "Basic " + + new String(Base64 + .encode((username + ":" + password) + .getBytes()))); + return execution.execute(request, body); + } + }); + } + + RestTemplate restTemplate = new RestTemplate( + new InterceptingClientHttpRequestFactory( + new SimpleClientHttpRequestFactory(), interceptors)); + restTemplate.setErrorHandler(new DefaultResponseErrorHandler() { + @Override + public void handleError(ClientHttpResponse response) throws IOException { + } + }); + return restTemplate; + + } + +} diff --git a/spring-bootstrap-service/pom.xml b/spring-bootstrap-service/pom.xml index e0e04fb0fa..dfb133bb5a 100644 --- a/spring-bootstrap-service/pom.xml +++ b/spring-bootstrap-service/pom.xml @@ -65,6 +65,7 @@ log4j log4j true + test com.fasterxml.jackson.core diff --git a/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/autoconfigure/service/ContainerConfiguration.java b/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/autoconfigure/service/ContainerConfiguration.java index 5a49fcc3cf..ad58d67b3e 100644 --- a/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/autoconfigure/service/ContainerConfiguration.java +++ b/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/autoconfigure/service/ContainerConfiguration.java @@ -37,6 +37,7 @@ import org.springframework.bootstrap.service.properties.ContainerProperties; import org.springframework.bootstrap.service.properties.ContainerProperties.Tomcat; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.annotation.Order; import org.springframework.util.StringUtils; /** @@ -48,6 +49,7 @@ import org.springframework.util.StringUtils; // the right order @Configuration @ConditionalOnClass({ Servlet.class }) +@Order(Integer.MIN_VALUE) public class ContainerConfiguration implements BeanPostProcessor, BeanFactoryAware { private BeanFactory beanFactory; diff --git a/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/autoconfigure/service/TraceAutoConfiguration.java b/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/autoconfigure/service/TraceAutoConfiguration.java index b59aaa8ad8..0cd89ae938 100644 --- a/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/autoconfigure/service/TraceAutoConfiguration.java +++ b/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/autoconfigure/service/TraceAutoConfiguration.java @@ -18,17 +18,19 @@ package org.springframework.bootstrap.autoconfigure.service; import javax.servlet.Servlet; +import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.bootstrap.context.annotation.ConditionalOnClass; import org.springframework.bootstrap.context.annotation.ConditionalOnMissingBean; import org.springframework.bootstrap.context.annotation.EnableAutoConfiguration; -import org.springframework.bootstrap.service.properties.ContainerProperties; import org.springframework.bootstrap.service.trace.InMemoryTraceRepository; import org.springframework.bootstrap.service.trace.SecurityFilterPostProcessor; import org.springframework.bootstrap.service.trace.TraceEndpoint; import org.springframework.bootstrap.service.trace.TraceRepository; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.security.web.SecurityFilterChain; import org.springframework.web.servlet.DispatcherServlet; /** @@ -41,24 +43,34 @@ import org.springframework.web.servlet.DispatcherServlet; @ConditionalOnMissingBean({ TraceEndpoint.class }) public class TraceAutoConfiguration { - @Autowired - private ContainerProperties configuration = new ContainerProperties(); - @Autowired(required = false) private TraceRepository traceRepository = new InMemoryTraceRepository(); @Bean - @ConditionalOnMissingBean({ TraceRepository.class }) + @ConditionalOnMissingBean(TraceRepository.class) protected TraceRepository traceRepository() { return this.traceRepository; } - @Bean - public SecurityFilterPostProcessor securityFilterPostProcessor() { - SecurityFilterPostProcessor processor = new SecurityFilterPostProcessor( - traceRepository()); - processor.setDumpRequests(this.configuration.isDumpRequests()); - return processor; + @Configuration + @ConditionalOnClass(SecurityFilterChain.class) + public static class SecurityFilterPostProcessorConfiguration { + + @Autowired + private TraceRepository traceRepository; + + @Value("${container.dump_requests:false}") + private boolean dumpRequests; + + @Bean + public SecurityFilterPostProcessor securityFilterPostProcessor( + BeanFactory beanFactory) { + SecurityFilterPostProcessor processor = new SecurityFilterPostProcessor( + this.traceRepository); + processor.setDumpRequests(this.dumpRequests); + return processor; + } + } @Bean diff --git a/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/service/trace/SecurityFilterPostProcessor.java b/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/service/trace/SecurityFilterPostProcessor.java index e94d428170..6c039e9616 100644 --- a/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/service/trace/SecurityFilterPostProcessor.java +++ b/spring-bootstrap-service/src/main/java/org/springframework/bootstrap/service/trace/SecurityFilterPostProcessor.java @@ -35,6 +35,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.BeansException; import org.springframework.beans.factory.config.BeanPostProcessor; +import org.springframework.core.Ordered; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.SecurityFilterChain; import org.springframework.util.Assert; @@ -51,7 +52,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; * @author Dave Syer * */ -public class SecurityFilterPostProcessor implements BeanPostProcessor { +public class SecurityFilterPostProcessor implements BeanPostProcessor, Ordered { private final static Log logger = LogFactory .getLog(SecurityFilterPostProcessor.class); @@ -60,6 +61,20 @@ public class SecurityFilterPostProcessor implements BeanPostProcessor { private TraceRepository traceRepository = new InMemoryTraceRepository(); + private int order = Integer.MAX_VALUE; + + /** + * @param order the order to set + */ + public void setOrder(int order) { + this.order = order; + } + + @Override + public int getOrder() { + return this.order; + } + /** * @param traceRepository */