Resolve or eliminate Environment-related TODOs

Issue: SPR-8031, SPR-7508

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<String, String> getModifiableSystemEnvironment() {
 		Class<?>[] classes = Collections.class.getDeclaredClasses();

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 {
 

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

org.springframework.core/src/main/java/org/springframework/core/env/PropertySource.java

@@ -214,9 +214,11 @@ public abstract class PropertySource<T> {
 			super(name, new Object());
 		}
 
+		/**
+		 * Always return {@code null}.
+		 */
 		@Override
 		public String getProperty(String key) {
-			// TODO SPR-7408: logging
 			return null;
 		}
 	}

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<String, String> getModifiableSystemEnvironment() throws Exception {
+	private static Map<String, String> getModifiableSystemEnvironment() {
 		Class<?>[] classes = Collections.class.getDeclaredClasses();
-		Map<String, String> systemEnv = System.getenv();
+		Map<String, String> env = System.getenv();
 		for (Class<?> cl : classes) {
 			if ("java.util.Collections$UnmodifiableMap".equals(cl.getName())) {
-				Field field = cl.getDeclaredField("m");
+				try {
-				field.setAccessible(true);
+					Field field = cl.getDeclaredField("m");
-				Object obj = field.get(systemEnv);
+					field.setAccessible(true);
-				return (Map<String, String>) obj;
+					Object obj = field.get(env);
+					return (Map<String, String>) obj;
+				} catch (Exception ex) {
+					throw new RuntimeException(ex);
+				}
 			}
 		}
 		throw new IllegalStateException();

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));

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);