From b50ac7489b63673cd3dd4b7dfa7bd791a41fa7af Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Tue, 15 Mar 2011 12:57:12 +0000 Subject: [PATCH] Resolve or eliminate Environment-related TODOs Issue: SPR-8031, SPR-7508 --- .../PropertyPlaceholderConfigurerTests.java | 1 - .../xml/ProfileXmlBeanDefinitionTests.java | 4 +++- .../core/env/EnumerablePropertySource.java | 18 ++++++++++++++++- .../core/env/PropertySource.java | 4 +++- .../core/env/EnvironmentTests.java | 20 ++++++++++--------- .../core/env/PropertySourceTests.java | 1 - .../core/env/EnvironmentIntegrationTests.java | 4 ---- 7 files changed, 34 insertions(+), 18 deletions(-) diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/factory/config/PropertyPlaceholderConfigurerTests.java b/org.springframework.beans/src/test/java/org/springframework/beans/factory/config/PropertyPlaceholderConfigurerTests.java index 9e039550a7..4080268853 100644 --- a/org.springframework.beans/src/test/java/org/springframework/beans/factory/config/PropertyPlaceholderConfigurerTests.java +++ b/org.springframework.beans/src/test/java/org/springframework/beans/factory/config/PropertyPlaceholderConfigurerTests.java @@ -243,7 +243,6 @@ public class PropertyPlaceholderConfigurerTests { } - // TODO SPR-7508: duplicated from EnvironmentPropertyResolutionSearchTests @SuppressWarnings("unchecked") private static Map getModifiableSystemEnvironment() { Class[] classes = Collections.class.getDeclaredClasses(); diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/ProfileXmlBeanDefinitionTests.java b/org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/ProfileXmlBeanDefinitionTests.java index c6cb1e408c..eab10be5d4 100644 --- a/org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/ProfileXmlBeanDefinitionTests.java +++ b/org.springframework.beans/src/test/java/org/springframework/beans/factory/xml/ProfileXmlBeanDefinitionTests.java @@ -30,9 +30,11 @@ import org.springframework.core.env.DefaultEnvironment; import org.springframework.core.io.ClassPathResource; /** - * TODO SPR-7508: document + * Tests various combinations of profile declarations against various profile + * activation and profile default scenarios. * * @author Chris Beams + * @since 3.1 */ public class ProfileXmlBeanDefinitionTests { diff --git a/org.springframework.core/src/main/java/org/springframework/core/env/EnumerablePropertySource.java b/org.springframework.core/src/main/java/org/springframework/core/env/EnumerablePropertySource.java index d2960513a5..1d8861edeb 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/env/EnumerablePropertySource.java +++ b/org.springframework.core/src/main/java/org/springframework/core/env/EnumerablePropertySource.java @@ -19,7 +19,23 @@ package org.springframework.core.env; import org.springframework.util.Assert; /** - * TODO SPR-7508: document + * A {@link PropertySource} implementation capable of interrogating its + * underlying source object to enumerate all possible property key/value + * pairs. Exposes the {@link #getPropertyNames()} method to allow callers + * to introspect available properties without having to access the underlying + * source object. This also facilitates a more efficient implementation of + * {@link #containsProperty(String)}, in that it can call {@link #getPropertyNames()} + * and iterate through the returned array rather than attempting a call to + * {@link #getProperty(String)} which may be more expensive. Implementations may + * consider caching the result of {@link #getPropertyNames()} to fully exploit this + * performance opportunity. + * + * Most framework-provided {@code PropertySource} implementations are enumerable; + * a counter-example would be {@code JndiPropertySource} where, due to the + * nature of JNDI it is not possible to determine all possible property names at + * any given time; rather it is only possible to try to access a property + * (via {@link #getProperty(String)}) in order to evaluate whether it is present + * or not. * * @author Chris Beams * @since 3.1 diff --git a/org.springframework.core/src/main/java/org/springframework/core/env/PropertySource.java b/org.springframework.core/src/main/java/org/springframework/core/env/PropertySource.java index a46ceb14ef..3aba39a59f 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/env/PropertySource.java +++ b/org.springframework.core/src/main/java/org/springframework/core/env/PropertySource.java @@ -214,9 +214,11 @@ public abstract class PropertySource { super(name, new Object()); } + /** + * Always return {@code null}. + */ @Override public String getProperty(String key) { - // TODO SPR-7408: logging return null; } } diff --git a/org.springframework.core/src/test/java/org/springframework/core/env/EnvironmentTests.java b/org.springframework.core/src/test/java/org/springframework/core/env/EnvironmentTests.java index 387e2dcd88..7bad6d85a0 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/env/EnvironmentTests.java +++ b/org.springframework.core/src/test/java/org/springframework/core/env/EnvironmentTests.java @@ -41,7 +41,6 @@ import java.util.Map; import org.junit.Test; import org.springframework.mock.env.MockPropertySource; - /** * Unit tests for {@link DefaultEnvironment}. * @@ -230,7 +229,7 @@ public class EnvironmentTests { } @Test - public void getSystemEnvironment_withAndWithoutSecurityManager() throws Exception { + public void getSystemEnvironment_withAndWithoutSecurityManager() { getModifiableSystemEnvironment().put(ALLOWED_PROPERTY_NAME, ALLOWED_PROPERTY_VALUE); getModifiableSystemEnvironment().put(DISALLOWED_PROPERTY_NAME, DISALLOWED_PROPERTY_VALUE); @@ -270,17 +269,20 @@ public class EnvironmentTests { getModifiableSystemEnvironment().remove(DISALLOWED_PROPERTY_NAME); } - // TODO SPR-7508: duplicated from EnvironmentPropertyResolutionSearchTests @SuppressWarnings("unchecked") - private static Map getModifiableSystemEnvironment() throws Exception { + private static Map getModifiableSystemEnvironment() { Class[] classes = Collections.class.getDeclaredClasses(); - Map systemEnv = System.getenv(); + Map env = System.getenv(); for (Class cl : classes) { if ("java.util.Collections$UnmodifiableMap".equals(cl.getName())) { - Field field = cl.getDeclaredField("m"); - field.setAccessible(true); - Object obj = field.get(systemEnv); - return (Map) obj; + try { + Field field = cl.getDeclaredField("m"); + field.setAccessible(true); + Object obj = field.get(env); + return (Map) obj; + } catch (Exception ex) { + throw new RuntimeException(ex); + } } } throw new IllegalStateException(); diff --git a/org.springframework.core/src/test/java/org/springframework/core/env/PropertySourceTests.java b/org.springframework.core/src/test/java/org/springframework/core/env/PropertySourceTests.java index dfe740c361..3717edb38e 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/env/PropertySourceTests.java +++ b/org.springframework.core/src/test/java/org/springframework/core/env/PropertySourceTests.java @@ -72,7 +72,6 @@ public class PropertySourceTests { assertThat(propertySources.contains(ps1), is(true)); assertThat(propertySources.contains(PropertySource.named("ps1")), is(true)); - // TODO SPR-7508: consider disallowing duplicates somehow (in the actual data structure used by environment) PropertySource ps1replacement = new MapPropertySource("ps1", map2); // notice - different map assertThat(propertySources.add(ps1replacement), is(true)); // true because linkedlist allows duplicates assertThat(propertySources.size(), is(2)); diff --git a/org.springframework.integration-tests/src/test/java/org/springframework/core/env/EnvironmentIntegrationTests.java b/org.springframework.integration-tests/src/test/java/org/springframework/core/env/EnvironmentIntegrationTests.java index 16ef332880..ca1a4dc202 100644 --- a/org.springframework.integration-tests/src/test/java/org/springframework/core/env/EnvironmentIntegrationTests.java +++ b/org.springframework.integration-tests/src/test/java/org/springframework/core/env/EnvironmentIntegrationTests.java @@ -354,9 +354,6 @@ public class EnvironmentIntegrationTests { assertEnvironmentAwareInvoked(ctx, prodEnv); } - // TODO SPR-7508: need to think about how a custom environment / custom property sources - // would be specified in an actual webapp using XmlWebApplicationContext. What do the - // context params look like, etc. @Test public void xmlWebApplicationContext() { AbstractRefreshableWebApplicationContext ctx = new XmlWebApplicationContext(); @@ -541,7 +538,6 @@ public class EnvironmentIntegrationTests { public void resourceAdapterApplicationContext() { ResourceAdapterApplicationContext ctx = new ResourceAdapterApplicationContext(new SimpleBootstrapContext(new SimpleTaskWorkManager())); - // TODO SPR-7508: should be a JCA-specific environment? assertHasDefaultEnvironment(ctx); registerEnvironmentBeanDefinition(ctx);