From 7d0c0fc0dde8f868f6bdad92fb6cddec501b7e6d Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 11 Jul 2013 11:21:34 -0700 Subject: [PATCH] Polish --- .../HibernateJpaAutoConfigurationTests.java | 55 ++++++----- .../config/ArrayDocumentMatcher.java | 3 + .../config/DefaultProfileDocumentMatcher.java | 5 +- .../PropertiesPropertySourceLoader.java | 6 +- .../config/PropertySourceLoader.java | 6 +- .../config/SpringProfileDocumentMatcher.java | 13 ++- .../bootstrap/config/YamlMapFactoryBean.java | 5 +- .../bootstrap/config/YamlProcessor.java | 91 +++++++++++++++---- .../config/YamlPropertiesFactoryBean.java | 5 +- .../config/YamlPropertySourceLoader.java | 51 ++++++----- .../config/ArrayDocumentMatcherTests.java | 4 +- 11 files changed, 166 insertions(+), 78 deletions(-) diff --git a/spring-autoconfigure/src/test/java/org/springframework/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java b/spring-autoconfigure/src/test/java/org/springframework/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java index 70f8d0e7ca..ea034ebb89 100644 --- a/spring-autoconfigure/src/test/java/org/springframework/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java +++ b/spring-autoconfigure/src/test/java/org/springframework/autoconfigure/orm/jpa/HibernateJpaAutoConfigurationTests.java @@ -16,10 +16,6 @@ package org.springframework.autoconfigure.orm.jpa; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; - import javax.sql.DataSource; import org.junit.After; @@ -40,6 +36,10 @@ import org.springframework.orm.jpa.support.OpenEntityManagerInViewFilter; import org.springframework.orm.jpa.support.OpenEntityManagerInViewInterceptor; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + /** * Tests for {@link HibernateJpaAutoConfiguration}. * @@ -51,14 +51,15 @@ public class HibernateJpaAutoConfigurationTests { @After public void close() { - if (context != null) { - context.close(); + if (this.context != null) { + this.context.close(); } } @Test public void testEntityManagerCreated() throws Exception { - ((AnnotationConfigApplicationContext) this.context).register(ComponentScanDetectorConfiguration.class, + ((AnnotationConfigApplicationContext) this.context).register( + ComponentScanDetectorConfiguration.class, EmbeddedDatabaseConfiguration.class, HibernateJpaAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, TestConfiguration.class); this.context.refresh(); @@ -68,10 +69,11 @@ public class HibernateJpaAutoConfigurationTests { @Test public void testDataSourceTransactionManagerNotCreated() throws Exception { - ((AnnotationConfigApplicationContext) this.context).register(ComponentScanDetectorConfiguration.class, + ((AnnotationConfigApplicationContext) this.context).register( + ComponentScanDetectorConfiguration.class, EmbeddedDatabaseConfiguration.class, HibernateJpaAutoConfiguration.class, - DataSourceTransactionManagerAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, - TestConfiguration.class); + DataSourceTransactionManagerAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class, TestConfiguration.class); this.context.refresh(); assertNotNull(this.context.getBean(DataSource.class)); assertTrue(this.context.getBean("transactionManager") instanceof JpaTransactionManager); @@ -80,33 +82,44 @@ public class HibernateJpaAutoConfigurationTests { @Test public void testOpenEntityManagerInViewInterceptorCreated() throws Exception { AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); - context.register(ComponentScanDetectorConfiguration.class, EmbeddedDatabaseConfiguration.class, - HibernateJpaAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, - TestConfiguration.class); + context.register(ComponentScanDetectorConfiguration.class, + EmbeddedDatabaseConfiguration.class, HibernateJpaAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class, TestConfiguration.class); this.context = context; this.context.refresh(); assertNotNull(this.context.getBean(OpenEntityManagerInViewInterceptor.class)); } @Test - public void testOpenEntityManagerInViewInterceptorNotRegisteredWhenFilterPresent() throws Exception { + public void testOpenEntityManagerInViewInterceptorNotRegisteredWhenFilterPresent() + throws Exception { AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); - context.register(TestFilterConfiguration.class, ComponentScanDetectorConfiguration.class, EmbeddedDatabaseConfiguration.class, - HibernateJpaAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class); + context.register(ComponentScanDetectorConfiguration.class, + EmbeddedDatabaseConfiguration.class, HibernateJpaAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class, TestFilterConfiguration.class); this.context = context; this.context.refresh(); - assertEquals(0, this.context.getBeanNamesForType(OpenEntityManagerInViewInterceptor.class).length); + assertEquals( + 0, + this.context + .getBeanNamesForType(OpenEntityManagerInViewInterceptor.class).length); } @Test - public void testOpenEntityManagerInViewInterceptorNotRegisteredWhenExplicitlyOff() throws Exception { + public void testOpenEntityManagerInViewInterceptorNotRegisteredWhenExplicitlyOff() + throws Exception { AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); TestUtils.addEnviroment(context, "spring.jpa.open_in_view:false"); - context.register(TestConfiguration.class, ComponentScanDetectorConfiguration.class, EmbeddedDatabaseConfiguration.class, - HibernateJpaAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class); + context.register(TestConfiguration.class, + ComponentScanDetectorConfiguration.class, + EmbeddedDatabaseConfiguration.class, HibernateJpaAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class); this.context = context; this.context.refresh(); - assertEquals(0, this.context.getBeanNamesForType(OpenEntityManagerInViewInterceptor.class).length); + assertEquals( + 0, + this.context + .getBeanNamesForType(OpenEntityManagerInViewInterceptor.class).length); } @ComponentScan(basePackageClasses = { City.class }) diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/ArrayDocumentMatcher.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/ArrayDocumentMatcher.java index afda3a46d8..93477f97bb 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/ArrayDocumentMatcher.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/ArrayDocumentMatcher.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.bootstrap.config; import java.util.Properties; @@ -26,6 +27,8 @@ import org.springframework.util.StringUtils; * Matches a document containing a given key and where the value of that key is an array * containing one of the given values, or where one of the values matches one of the given * values (interpreted as regexes). + * + * @author Dave Syer */ public class ArrayDocumentMatcher implements DocumentMatcher { diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/DefaultProfileDocumentMatcher.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/DefaultProfileDocumentMatcher.java index 3680190f13..ce22f2a531 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/DefaultProfileDocumentMatcher.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/DefaultProfileDocumentMatcher.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.bootstrap.config; import java.util.Properties; @@ -25,7 +26,6 @@ import org.springframework.bootstrap.config.YamlProcessor.MatchStatus; * explicitly (i.e. matches if "spring.profiles" is not found and not otherwise). * * @author Dave Syer - * */ public final class DefaultProfileDocumentMatcher implements DocumentMatcher { @@ -33,7 +33,8 @@ public final class DefaultProfileDocumentMatcher implements DocumentMatcher { public MatchStatus matches(Properties properties) { if (!properties.containsKey("spring.profiles")) { return MatchStatus.FOUND; - } else { + } + else { return MatchStatus.NOT_FOUND; } } diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/PropertiesPropertySourceLoader.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/PropertiesPropertySourceLoader.java index 3368b66a6c..6bd6a8a5c7 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/PropertiesPropertySourceLoader.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/PropertiesPropertySourceLoader.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.bootstrap.config; import java.io.IOException; @@ -26,6 +27,8 @@ import org.springframework.core.io.support.PropertiesLoaderUtils; /** * Strategy to load '.properties' files into a {@link PropertySource}. + * + * @author Dave Syer */ public class PropertiesPropertySourceLoader implements PropertySourceLoader { @@ -39,7 +42,8 @@ public class PropertiesPropertySourceLoader implements PropertySourceLoader { try { Properties properties = loadProperties(resource, environment); return new PropertiesPropertySource(resource.getDescription(), properties); - } catch (IOException ex) { + } + catch (IOException ex) { throw new IllegalStateException("Could not load properties from " + resource, ex); } diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/PropertySourceLoader.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/PropertySourceLoader.java index 8f6f154572..6bf17f9742 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/PropertySourceLoader.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/PropertySourceLoader.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.bootstrap.config; import org.springframework.core.env.Environment; @@ -21,11 +22,14 @@ import org.springframework.core.io.Resource; /** * Strategy interface used to load a {@link PropertySource}. + * + * @author Dave Syer */ public interface PropertySourceLoader { /** - * @return Is this resource supported? + * Returns {@code true} if the {@link Resource} is supported. + * @return if the resource is supported */ public boolean supports(Resource resource); diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/SpringProfileDocumentMatcher.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/SpringProfileDocumentMatcher.java index 802879689e..0b1573f792 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/SpringProfileDocumentMatcher.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/SpringProfileDocumentMatcher.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.bootstrap.config; import java.util.Properties; @@ -20,19 +21,25 @@ import java.util.Properties; import org.springframework.bootstrap.config.YamlProcessor.DocumentMatcher; import org.springframework.bootstrap.config.YamlProcessor.MatchStatus; import org.springframework.core.env.Environment; +import org.springframework.util.Assert; /** - * @author Dave Syer + * {@link DocumentMatcher} backed by {@link Environment#getActiveProfiles()}. * + * @author Dave Syer */ public class SpringProfileDocumentMatcher implements DocumentMatcher { + private static final String[] DEFAULT_PROFILES = new String[] { "default" }; + private final Environment environment; /** - * @param environment + * Create a new {@link SpringProfileDocumentMatcher} instance. + * @param environment the environment */ public SpringProfileDocumentMatcher(Environment environment) { + Assert.notNull(environment, "Environment must not be null"); this.environment = environment; } @@ -40,7 +47,7 @@ public class SpringProfileDocumentMatcher implements DocumentMatcher { public MatchStatus matches(Properties properties) { String[] profiles = this.environment.getActiveProfiles(); if (profiles.length == 0) { - profiles = new String[] { "default" }; + profiles = DEFAULT_PROFILES; } return new ArrayDocumentMatcher("spring.profiles", profiles).matches(properties); } diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlMapFactoryBean.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlMapFactoryBean.java index f071278354..0e9927d0e8 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlMapFactoryBean.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlMapFactoryBean.java @@ -76,13 +76,12 @@ public class YamlMapFactoryBean extends YamlProcessor implements public Map getObject() { if (!this.singleton || this.instance == null) { final Map result = new LinkedHashMap(); - MatchCallback callback = new MatchCallback() { + process(new MatchCallback() { @Override public void process(Properties properties, Map map) { merge(result, map); } - }; - process(callback); + }); this.instance = result; } return this.instance; diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlProcessor.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlProcessor.java index 079534070a..c3c672a97c 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlProcessor.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlProcessor.java @@ -27,6 +27,7 @@ import java.util.Properties; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.core.io.Resource; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.yaml.snakeyaml.Yaml; @@ -92,17 +93,13 @@ public class YamlProcessor { /** * Method to use for resolving resources. Each resource will be converted to a Map, so * this property is used to decide which map entries to keep in the final output from - * this factory. Possible values: - *
    - *
  • OVERRIDE for replacing values from earlier in the list
    • - *
  • FIRST_FOUND if you want to take the first resource in the list - * that exists and use just that.
    • - *
+ * this factory. * - * - * @param resolutionMethod the resolution method to set. Defaults to OVERRIDE. + * @param resolutionMethod the resolution method to set (defaults to + * {@link ResolutionMethod#OVERRIDE}). */ public void setResolutionMethod(ResolutionMethod resolutionMethod) { + Assert.notNull(resolutionMethod, "ResolutionMethod must not be null"); this.resolutionMethod = resolutionMethod; } @@ -147,7 +144,8 @@ public class YamlProcessor { } this.logger.info("Loaded " + count + " document" + (count > 1 ? "s" : "") + " from YAML resource: " + resource); - } catch (IOException ex) { + } + catch (IOException ex) { handleProcessError(resource, ex); } return count > 0; @@ -175,12 +173,13 @@ public class YamlProcessor { if (this.documentMatchers.isEmpty()) { this.logger.debug("Merging document (no matchers set)" + map); callback.process(properties, map); - } else { + } + else { boolean valueFound = false; MatchStatus result = MatchStatus.ABSTAIN; for (DocumentMatcher matcher : this.documentMatchers) { MatchStatus match = matcher.matches(properties); - result = match.ordinal() < result.ordinal() ? match : result; + result = MatchStatus.getMostSpecific(match, result); if (match == MatchStatus.FOUND) { this.logger.debug("Matched document with document matcher: " + properties); @@ -193,7 +192,8 @@ public class YamlProcessor { if (result == MatchStatus.ABSTAIN && this.matchDefault) { this.logger.debug("Matched document with default matcher: " + map); callback.process(properties, map); - } else if (!valueFound) { + } + else if (!valueFound) { this.logger.debug("Unmatched document"); return false; } @@ -208,19 +208,22 @@ public class YamlProcessor { if (StringUtils.hasText(path)) { if (key.startsWith("[")) { key = path + key; - } else { + } + else { key = path + "." + key; } } Object value = entry.getValue(); if (value instanceof String) { properties.put(key, value); - } else if (value instanceof Map) { + } + else if (value instanceof Map) { // Need a compound key @SuppressWarnings("unchecked") Map map = (Map) value; assignProperties(properties, map, key); - } else if (value instanceof Collection) { + } + else if (value instanceof Collection) { // Need a compound key @SuppressWarnings("unchecked") Collection collection = (Collection) value; @@ -229,26 +232,74 @@ public class YamlProcessor { assignProperties(properties, Collections.singletonMap("[" + (count++) + "]", object), key); } - } else { + } + else { properties.put(key, value == null ? "" : value); } } } + /** + * Callback interface used to process properties in a resulting map. + */ public interface MatchCallback { + + /** + * Process the properties. + * @param properties the properties to process + * @param map a mutable result map + */ void process(Properties properties, Map map); + } + /** + * Strategy interface used the test if properties match. + */ public interface DocumentMatcher { + + /** + * Test if the given properties match. + * @param properties the properties to test + * @return the status of the match. + */ MatchStatus matches(Properties properties); - } - public static enum ResolutionMethod { - OVERRIDE, OVERRIDE_AND_IGNORE, FIRST_FOUND } + /** + * Status returned from {@link DocumentMatcher#matches(Properties)} + */ public static enum MatchStatus { - FOUND, NOT_FOUND, ABSTAIN + FOUND, NOT_FOUND, ABSTAIN; + + /** + * Compare two {@link MatchStatus} items, returning the most specific status. + */ + public static MatchStatus getMostSpecific(MatchStatus a, MatchStatus b) { + return a.ordinal() < b.ordinal() ? a : b; + } + } + + /** + * Resolution methods. + */ + public static enum ResolutionMethod { + + /** + * Replace values from earlier in the list. + */ + OVERRIDE, + + /** + * Replace values from earlier in the list, ignoring any failures. + */ + OVERRIDE_AND_IGNORE, + + /** + * Take the first resource in the list that exists and use just that. + */ + FIRST_FOUND } } diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlPropertiesFactoryBean.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlPropertiesFactoryBean.java index a0abb26e47..0a7934dd80 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlPropertiesFactoryBean.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlPropertiesFactoryBean.java @@ -77,13 +77,12 @@ public class YamlPropertiesFactoryBean extends YamlProcessor implements public Properties getObject() { if (!this.singleton || this.instance == null) { final Properties result = new Properties(); - MatchCallback callback = new MatchCallback() { + process(new MatchCallback() { @Override public void process(Properties properties, Map map) { result.putAll(properties); } - }; - process(callback); + }); this.instance = result; } return this.instance; diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlPropertySourceLoader.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlPropertySourceLoader.java index ad7012bfa5..4cc07d9fd2 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlPropertySourceLoader.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlPropertySourceLoader.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.bootstrap.config; import java.io.IOException; @@ -27,35 +28,16 @@ import org.springframework.core.io.Resource; /** * Strategy to load '.yml' files into a {@link PropertySource}. + * + * @author Dave Syer */ public class YamlPropertySourceLoader extends PropertiesPropertySourceLoader { private List matchers; /** - * A property source loader that loads all properties and matches all documents. - * - * @return a property source loader - */ - public static YamlPropertySourceLoader matchAllLoader() { - return new YamlPropertySourceLoader(); - } - - /** - * A property source loader that matches documents that have no explicit profile or - * which have an explicit "spring.profiles.active" value in the current active - * profiles. - * - * @return a property source loader - */ - public static YamlPropertySourceLoader springProfileAwareLoader( - Environment environment) { - return new YamlPropertySourceLoader(new SpringProfileDocumentMatcher(environment), - new DefaultProfileDocumentMatcher()); - } - - /** - * @param matchers + * Create a {@link YamlPropertySourceLoader} instance with the specified matchers. + * @param matchers the document matchers */ public YamlPropertySourceLoader(DocumentMatcher... matchers) { this.matchers = Arrays.asList(matchers); @@ -78,4 +60,27 @@ public class YamlPropertySourceLoader extends PropertiesPropertySourceLoader { return factory.getObject(); } + /** + * A property source loader that loads all properties and matches all documents. + * + * @return a property source loader + */ + public static YamlPropertySourceLoader matchAllLoader() { + return new YamlPropertySourceLoader(); + } + + /** + * A property source loader that matches documents that have no explicit profile or + * which have an explicit "spring.profiles.active" value in the current active + * profiles. + * + * @return a property source loader + */ + public static YamlPropertySourceLoader springProfileAwareLoader( + Environment environment) { + return new YamlPropertySourceLoader( + new SpringProfileDocumentMatcher(environment), + new DefaultProfileDocumentMatcher()); + } + } \ No newline at end of file diff --git a/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/ArrayDocumentMatcherTests.java b/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/ArrayDocumentMatcherTests.java index fd8da8e8cb..c321075612 100644 --- a/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/ArrayDocumentMatcherTests.java +++ b/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/ArrayDocumentMatcherTests.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.bootstrap.config; import java.io.IOException; @@ -26,8 +27,9 @@ import org.springframework.core.io.support.PropertiesLoaderUtils; import static org.junit.Assert.assertEquals; /** - * @author Dave Syer + * Tests for {@link ArrayDocumentMatcher}. * + * @author Dave Syer */ public class ArrayDocumentMatcherTests {