Changes after review

Author: mccabd

README.md

@@ -108,7 +108,8 @@ already.defined.key+=value2,value3
 ### Profiles
 Profiles allow you to map properties to different profiles - for example, dev, test, prod or mock.
 We can activate these profiles in different environments to set(override) the properties we need. 
-The profile property can be defined as either an OS environment variable or a JVM system property. 
+The profile property is generally to be defined as either an OS environment variable or a JVM system property. 
+However, it can be set in a properties file which is useful in unit testing or testing on a local environment.
 
 When a property with the key `bordertech.config.profile` is set, it is used as the suffix for each property lookup:
 
@@ -241,8 +242,8 @@ If the `bordertech.config.spi.append.default` is true the Default Configuration
 
 ### Best Practice
 
-THe Default Configuration allows you to set the profile `bordertech.config.profile` but you would normally set the profile through a
-JVM System Property or an OS environment variable.  
+When using java-config in a container and setting specific properties for that container instance, 
+this can be achieved by property file(s) placed in the container that can be referred to and included within the application at runtime. 
 
 ## Contributing
 

src/main/java/com/github/bordertech/config/DefaultConfiguration.java

@@ -37,6 +37,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
 import java.util.TreeSet;
@@ -100,7 +101,9 @@ public class DefaultConfiguration implements Configuration {
 	public static final String DUMP_FILE = "bordertech.config.parameters.dump.file";
 	/**
 	 * If this parameter is set, it will be used as the environment suffix for each property lookup.
+	 * @deprecated Use {@link #PROFILE_PROPERTY} to define the profile property
 	 */
+	@Deprecated
 	public static final String ENVIRONMENT_PROPERTY = "bordertech.config.environment";
 	/**
 	 * If this parameter is set, it will be used as the environment suffix for each property lookup.
@@ -166,14 +169,9 @@ public class DefaultConfiguration implements Configuration {
 	private final String[] resourceLoadOrder;
 
 	/**
-	 * Holds the current environment suffix (if set).
-	 *
-	 * @deprecated - Replaced by current Profile
+	 * Hold the current profile (if set).
 	 */
-	@Deprecated
-	private String currentEnvironment = null;
-
-	private String environmentProfile = null;
+	private String currentProfile = null;
 
 	/**
 	 * Our backing store is a Map object.
@@ -196,6 +194,11 @@ public class DefaultConfiguration implements Configuration {
 	 */
 	private Map<String, Properties> subcontextCache;
 
+	/**
+	 * Properties added at runtime.
+	 */
+	private IncludeProperties runtimeProperties;
+
 	/**
 	 * Creates a Default Configuration.
 	 */
@@ -264,7 +267,8 @@ private void initialiseInstanceVariables() {
 
 		// subContextCache is updated on the fly so ensure no concurrent modification.
 		subcontextCache = Collections.synchronizedMap(new HashMap<>());
-		currentEnvironment = null;
+		runtimeProperties = new IncludeProperties("Runtime: property added at runtime");
+		currentProfile = null;
 	}
 
 	/**
@@ -297,10 +301,7 @@ private void load() {
 			loadEnvironmentProperties();
 		}
 
-		setEnvironmentProfile();
-
-		// Check if environment set
-		checkEnvironmentProperty();
+		checkProfileProperty();
 
 		// Now perform variable substitution.
 		for (String key : backing.keySet()) {
@@ -699,7 +700,7 @@ private ClassLoader getParamsClassLoader() {
 	 */
 	private void loadSystemProperties() {
 		boolean overWriteOnly = getBoolean(USE_SYSTEM_OVERWRITEONLY, false);
-		List<Object> allowedPrefixes = getList(USE_SYSTEM_PREFIXES);
+		List<String> allowedPrefixes = Arrays.asList(getStringArray(USE_SYSTEM_PREFIXES));
 		System
 			.getProperties()
 			.forEach((key, value) -> mergeExternalProperty("System Properties",
@@ -713,27 +714,12 @@ private void loadSystemProperties() {
 	 * Load the OS Environment Properties into Config.
 	 */
 	private void loadEnvironmentProperties() {
-		List<Object> allowedPrefixes = getList(USE_OSENV_PREFIXES);
+		List<String> allowedPrefixes = Arrays.asList(getStringArray(USE_OSENV_PREFIXES));
 		System
 			.getenv()
 			.forEach((key, value) -> mergeExternalProperty("Environment Properties", key, value, false, allowedPrefixes));
 	}
 
-	/**
-	 * Set the Environment Profile if it has been set as a system or environment property.
-	 * If both are defined, system property overrides and environment property
-	 */
-	private void setEnvironmentProfile() {
-
-		environmentProfile = ObjectUtils.firstNonNull(backing.get(PROFILE_PROPERTY), System.getProperty(PROFILE_PROPERTY), System.getenv().get(PROFILE_PROPERTY));
-
-		if (StringUtils.isBlank(environmentProfile)) {
-			recordMessage("Environment Profile Property <" + PROFILE_PROPERTY + "> has not been defined.");
-		} else {
-			recordMessage("Environment Profile Property <" + PROFILE_PROPERTY + "> has been defined as " + environmentProfile);
-		}
-	}
-
 	/**
 	 * Merge the external property.
 	 *
@@ -748,7 +734,7 @@ private void mergeExternalProperty(
 		final String key,
 		final String value,
 		final boolean overWriteOnly,
-		final List<Object> allowedPrefixes) {
+		final List<String> allowedPrefixes) {
 
 		// Check for "include" keys (should not come from System or Environment Properties)
 		if (INCLUDE.equals(key) || INCLUDE_AFTER.equals(key)) {
@@ -776,16 +762,16 @@ private void mergeExternalProperty(
 	 * @param key             the key to check
 	 * @return true if the key is an allowed prefix
 	 */
-	private boolean isAllowedKeyPrefix(final List<Object> allowedPrefixes, final String key) {
+	private boolean isAllowedKeyPrefix(final List<String> allowedPrefixes, final String key) {
 
 		// If no prefixes defined, then ALL keys are allowed
 		if (allowedPrefixes == null || allowedPrefixes.isEmpty()) {
 			return true;
 		}
 
 		// Check allowed prefixes
-		for (Object prefix : allowedPrefixes) {
-			if (key.startsWith(prefix.toString())) {
+		for (String prefix : allowedPrefixes) {
+			if (key.startsWith(prefix)) {
 				return true;
 			}
 		}
@@ -963,7 +949,10 @@ public void clearProperty(final String key) {
 
 	@Override
 	public boolean containsKey(final String key) {
-		return backing.containsKey(getKey(key));
+		if (useProfileKey(key) && backing.containsKey(getProfileKey(key))) {
+			return true;
+		}
+		return backing.containsKey(key);
 	}
 
 	@Override
@@ -1008,7 +997,10 @@ public BigInteger getBigInteger(final String key, final BigInteger defaultValue)
 
 	@Override
 	public boolean getBoolean(final String key) {
-		return booleanBacking.contains(getKey(key));
+		if (useProfileKey(key) && booleanBacking.contains(getProfileKey(key))) {
+			return true;
+		}
+		return booleanBacking.contains(key);
 	}
 
 	@Override
@@ -1318,18 +1310,20 @@ protected String get(final String key, final String defolt) {
 	}
 
 	/**
+	 *
 	 * @param key the property key
 	 * @return the property value or null
 	 */
 	protected String get(final String key) {
-		String result = backing.get(getKey(key));
-
-		if (StringUtils.isNotBlank(result)) {
-			//Final substitution check
-			result = StringSubstitutor.replace(result, backing);
+		// Check environment property
+		if (useProfileKey(key)) {
+			String result = backing.get(getProfileKey(key));
+			if (result != null) {
+				return StringSubstitutor.replace(result, backing);
+			}
 		}
-
-		return result;
+		//Final substitution check
+		return StringSubstitutor.replace(backing.get(key), backing);
 	}
 
 	/**
@@ -1353,7 +1347,7 @@ protected void addOrModifyProperty(final String name, final String value) {
 
 		recordMessage("modifyProperties() - Adding property '" + name + "' with the value '" + value + "'.");
 
-		new IncludeProperties("Runtime: added at runtime").put(name, value);
+		runtimeProperties.put(name, value);
 
 		handlePropertiesChanged();
 	}
@@ -1364,20 +1358,60 @@ protected void addOrModifyProperty(final String name, final String value) {
 	protected void handlePropertiesChanged() {
 		// clear the subContext cache, it's now invalid
 		subcontextCache.clear();
-		// Check if environment changed
-		checkEnvironmentProperty();
-		setEnvironmentProfile();
+		// Check if profile changed
+		checkProfileProperty();
+	}
+
+	/**
+	 * Set the current Profile if it has been set as property. An application defined property overrides,
+	 * a JVM System property which overrides a OS environment variable
+	 */
+	protected void checkProfileProperty() {
+
+		currentProfile = ObjectUtils.firstNonNull(backing.get(PROFILE_PROPERTY), System.getProperty(PROFILE_PROPERTY), System.getenv().get(PROFILE_PROPERTY));
+
+		//Temporarily exists until deprecated Environment Property is removed
+		if (StringUtils.isBlank(currentProfile)) {
+			//Might not be using profile property, try the environment property for backwards compatibility
+			currentProfile = ObjectUtils.firstNonNull(backing.get(ENVIRONMENT_PROPERTY), System.getProperty(ENVIRONMENT_PROPERTY), System.getenv().get(ENVIRONMENT_PROPERTY));
+		}
+
+		if (StringUtils.isBlank(currentProfile)) {
+			recordMessage("Profile Property <" + PROFILE_PROPERTY + "> has not been defined.");
+		} else {
+			recordMessage("Profile Property <" + PROFILE_PROPERTY + "> has been defined as " + currentProfile);
+		}
+	}
+
+	/**
+	 * @param key the property key
+	 * @return true if check suffix
+	 */
+	protected boolean useProfileKey(final String key) {
+		// Has environment and is not the environment property
+		return currentProfile != null && !ENVIRONMENT_PROPERTY.equals(key) && !PROFILE_PROPERTY.equals(key);
+	}
+
+	/**
+	 * @param key the property key
+	 * @return the property key with the environment suffix
+	 */
+	protected String getProfileKey(final String key) {
+		if (useProfileKey(key)) {
+			return key + "." + currentProfile;
+		} else {
+			return key;
+		}
 	}
 
 	/**
 	 * Check if the environment property has been set.
 	 *
-	 * @deprecated - Replaced by Environment Profile
+	 * @deprecated - Replaced by see checkProfileProperty
 	 */
 	@Deprecated
 	protected void checkEnvironmentProperty() {
-		String env = backing.get(ENVIRONMENT_PROPERTY);
-		currentEnvironment = (env == null || env.isEmpty()) ? null : env;
+		checkProfileProperty();
 	}
 
 	/**
@@ -1387,8 +1421,7 @@ protected void checkEnvironmentProperty() {
 	 */
 	@Deprecated
 	protected boolean useEnvironmentKey(final String key) {
-		// Has environment and is not the environment property
-		return currentEnvironment != null && !ENVIRONMENT_PROPERTY.equals(key);
+		return useProfileKey(key);
 	}
 
 	/**
@@ -1398,25 +1431,7 @@ protected boolean useEnvironmentKey(final String key) {
 	 */
 	@Deprecated
 	protected String getEnvironmentKey(final String key) {
-		if (useEnvironmentKey(key)) {
-			return key + "." + currentEnvironment;
-		} else {
-			return key;
-		}
-	}
-
-	private String getKey(final String key) {
-
-		final String profileKey = key + "." + environmentProfile;
-		final String currentEnvKey = key + "." + currentEnvironment;
-
-		if (StringUtils.isNotBlank(environmentProfile) && backing.containsKey(profileKey)) {
-			return profileKey;
-		} else if (StringUtils.isNotBlank(currentEnvironment) && backing.containsKey(currentEnvKey)) {
-			return currentEnvKey;
-		} else {
-			return key;
-		}
+		return getProfileKey(key);
 	}
 
 	/**
@@ -1425,7 +1440,7 @@ private String getKey(final String key) {
 	 * <p>This is used to ensure on the call of put(key, value) is immediately loaded into the
 	 * {@link DefaultConfiguration} to respect the order hierarchy for the configuration.</p>
 	 */
-	class IncludeProperties extends Properties {
+	private class IncludeProperties extends Properties {
 
 		/**
 		 * The properties file location (if applicable).
@@ -1486,5 +1501,17 @@ public synchronized Object put(final Object aKey, final Object aValue) {
 				return super.put(key, value);
 			}
 		}
+
+		@Override
+		public synchronized int hashCode() {
+			int hash = 5;
+			hash = 97 * hash + Objects.hashCode(this.location);
+			return hash;
+		}
+
+		@Override
+		public synchronized boolean equals(final Object obj) {
+			return obj instanceof IncludeProperties && Objects.equals(this.location, ((IncludeProperties) obj).location);
+		}
 	}
 }

src/test/java/com/github/bordertech/config/DefaultConfigurationEnvSuffixTest.java

@@ -4,6 +4,7 @@
 import org.junit.Test;
 
 import static com.github.bordertech.config.DefaultConfiguration.ENVIRONMENT_PROPERTY;
+import static com.github.bordertech.config.DefaultConfiguration.PROFILE_PROPERTY;
 
 /**
  * DefaultConfiguration_EnvironmentSuffix_Test - JUnit tests for {@link DefaultConfiguration}.
@@ -41,6 +42,40 @@ public void testEnvSuffixProperties() {
 
 		//Test for the suffix 2 is set
 		assertPropertyEquals(config, SUFFIX2_PROPERTY_VALUE);
+
+		System.clearProperty(ENVIRONMENT_PROPERTY);
+		config.refresh();
+	}
+
+	@Test
+	public void testProfileSuffixProperties() {
+		DefaultConfiguration config = new DefaultConfiguration(
+			"com/github/bordertech/config/DefaultConfigurationEnvSuffixTest.properties");
+
+		System.clearProperty(PROFILE_PROPERTY);
+		config.refresh();
+
+		Assert.assertNull(System.getProperty(PROFILE_PROPERTY));
+
+		//Test the default value
+		assertPropertyEquals(config, DEFAULT_PROPERTY_VALUE);
+
+		//Set the environment suffix 1 and reload
+		System.setProperty(PROFILE_PROPERTY, "suffix1");
+		config.refresh();
+
+		//Test for the suffix 1 is set
+		assertPropertyEquals(config, SUFFIX1_PROPERTY_VALUE);
+
+		//Set the environment suffix 2 and reload
+		System.setProperty(PROFILE_PROPERTY, "suffix2");
+		config.refresh();
+
+		//Test for the suffix 2 is set
+		assertPropertyEquals(config, SUFFIX2_PROPERTY_VALUE);
+
+		System.clearProperty(PROFILE_PROPERTY);
+		config.refresh();
 	}
 
 	/**