Consistent logging in Environment and PropertySource implementations

Issue: SPR-15825

spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -121,9 +121,8 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 	 */
 	public AbstractEnvironment() {
 		customizePropertySources(this.propertySources);
-		if (this.logger.isDebugEnabled()) {
-			this.logger.debug(String.format(
-					"Initialized %s with PropertySources %s", getClass().getSimpleName(), this.propertySources));
+		if (logger.isDebugEnabled()) {
+			logger.debug("Initialized " + getClass().getSimpleName() + " with PropertySources " + this.propertySources);
 		}
 	}
 
@@ -262,8 +261,8 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 
 	@Override
 	public void addActiveProfile(String profile) {
-		if (this.logger.isDebugEnabled()) {
-			this.logger.debug(String.format("Activating profile '%s'", profile));
+		if (logger.isDebugEnabled()) {
+			logger.debug("Activating profile '" + profile + "'");
 		}
 		validateProfile(profile);
 		doGetActiveProfiles();
@@ -393,9 +392,8 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 					}
 					catch (AccessControlException ex) {
 						if (logger.isInfoEnabled()) {
-							logger.info(String.format("Caught AccessControlException when accessing system " +
-									"environment variable [%s]; its value will be returned [null]. Reason: %s",
-									attributeName, ex.getMessage()));
+							logger.info("Caught AccessControlException when accessing system environment variable '" +
+									attributeName + "'; its value will be returned [null]. Reason: " + ex.getMessage());
 						}
 						return null;
 					}
@@ -434,9 +432,8 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 					}
 					catch (AccessControlException ex) {
 						if (logger.isInfoEnabled()) {
-							logger.info(String.format("Caught AccessControlException when accessing system " +
-									"property [%s]; its value will be returned [null]. Reason: %s",
-									attributeName, ex.getMessage()));
+							logger.info("Caught AccessControlException when accessing system property '" +
+									attributeName + "'; its value will be returned [null]. Reason: " + ex.getMessage());
 						}
 						return null;
 					}
@@ -569,9 +566,8 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
 
 	@Override
 	public String toString() {
-		return String.format("%s {activeProfiles=%s, defaultProfiles=%s, propertySources=%s}",
-				getClass().getSimpleName(), this.activeProfiles, this.defaultProfiles,
-				this.propertySources);
+		return getClass().getSimpleName() + " {activeProfiles=" + this.activeProfiles +
+				", defaultProfiles=" + this.defaultProfiles + ", propertySources=" + this.propertySources + "}";
 	}
 
 }

spring-core/src/main/java/org/springframework/core/env/AbstractPropertyResolver.java

@@ -180,7 +180,7 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
 	public String getRequiredProperty(String key) throws IllegalStateException {
 		String value = getProperty(key);
 		if (value == null) {
-			throw new IllegalStateException(String.format("required key [%s] not found", key));
+			throw new IllegalStateException("Required key '" + key + "' not found");
 		}
 		return value;
 	}
@@ -189,7 +189,7 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
 	public <T> T getRequiredProperty(String key, Class<T> valueType) throws IllegalStateException {
 		T value = getProperty(key, valueType);
 		if (value == null) {
-			throw new IllegalStateException(String.format("required key [%s] not found", key));
+			throw new IllegalStateException("Required key '" + key + "' not found");
 		}
 		return value;
 	}
@@ -233,7 +233,7 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
 	}
 
 	private String doResolvePlaceholders(String text, PropertyPlaceholderHelper helper) {
-		return helper.replacePlaceholders(text, placeholderName -> getPropertyAsRawString(placeholderName));
+		return helper.replacePlaceholders(text, this::getPropertyAsRawString);
 	}
 
 	/**

spring-core/src/main/java/org/springframework/core/env/CompositePropertySource.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -119,8 +119,7 @@ public class CompositePropertySource extends EnumerablePropertySource<Object> {
 
 	@Override
 	public String toString() {
-		return String.format("%s [name='%s', propertySources=%s]",
-				getClass().getSimpleName(), this.name, this.propertySources);
+		return getClass().getSimpleName() + " {name='" + this.name + "', propertySources=" + this.propertySources + "}";
 	}
 
 }

spring-core/src/main/java/org/springframework/core/env/MissingRequiredPropertiesException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -33,6 +33,17 @@ public class MissingRequiredPropertiesException extends IllegalStateException {
 
 	private final Set<String> missingRequiredProperties = new LinkedHashSet<>();
 
+
+	void addMissingRequiredProperty(String key) {
+		this.missingRequiredProperties.add(key);
+	}
+
+	@Override
+	public String getMessage() {
+		return "The following properties were declared as required but could not be resolved: " +
+				getMissingRequiredProperties();
+	}
+
 	/**
 	 * Return the set of properties marked as required but not present
 	 * upon validation.
@@ -40,17 +51,7 @@ public class MissingRequiredPropertiesException extends IllegalStateException {
 	 * @see ConfigurablePropertyResolver#validateRequiredProperties()
 	 */
 	public Set<String> getMissingRequiredProperties() {
-		return missingRequiredProperties;
-	}
-
-	void addMissingRequiredProperty(String key) {
-		missingRequiredProperties.add(key);
+		return this.missingRequiredProperties;
 	}
 
-	@Override
-	public String getMessage() {
-		return String.format(
-				"The following properties were declared as required but could " +
-				"not be resolved: %s", this.getMissingRequiredProperties());
-	}
 }

spring-core/src/main/java/org/springframework/core/env/MutablePropertySources.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,7 +24,6 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.lang.Nullable;
-import org.springframework.util.StringUtils;
 
 /**
  * Default implementation of the {@link PropertySources} interface.
@@ -95,8 +94,7 @@ public class MutablePropertySources implements PropertySources {
 	 */
 	public void addFirst(PropertySource<?> propertySource) {
 		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Adding [%s] PropertySource with highest search precedence",
-					propertySource.getName()));
+			logger.debug("Adding PropertySource '" + propertySource.getName() + "' with highest search precedence");
 		}
 		removeIfPresent(propertySource);
 		this.propertySourceList.add(0, propertySource);
@@ -107,8 +105,7 @@ public class MutablePropertySources implements PropertySources {
 	 */
 	public void addLast(PropertySource<?> propertySource) {
 		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Adding [%s] PropertySource with lowest search precedence",
-					propertySource.getName()));
+			logger.debug("Adding PropertySource '" + propertySource.getName() + "' with lowest search precedence");
 		}
 		removeIfPresent(propertySource);
 		this.propertySourceList.add(propertySource);
@@ -120,8 +117,8 @@ public class MutablePropertySources implements PropertySources {
 	 */
 	public void addBefore(String relativePropertySourceName, PropertySource<?> propertySource) {
 		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Adding [%s] PropertySource with search precedence immediately higher than [%s]",
-					propertySource.getName(), relativePropertySourceName));
+			logger.debug("Adding PropertySource '" + propertySource.getName() +
+					"' with search precedence immediately higher than '" + relativePropertySourceName + "'");
 		}
 		assertLegalRelativeAddition(relativePropertySourceName, propertySource);
 		removeIfPresent(propertySource);
@@ -135,8 +132,8 @@ public class MutablePropertySources implements PropertySources {
 	 */
 	public void addAfter(String relativePropertySourceName, PropertySource<?> propertySource) {
 		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Adding [%s] PropertySource with search precedence immediately lower than [%s]",
-					propertySource.getName(), relativePropertySourceName));
+			logger.debug("Adding PropertySource '" + propertySource.getName() +
+					"' with search precedence immediately lower than '" + relativePropertySourceName + "'");
 		}
 		assertLegalRelativeAddition(relativePropertySourceName, propertySource);
 		removeIfPresent(propertySource);
@@ -158,7 +155,7 @@ public class MutablePropertySources implements PropertySources {
 	@Nullable
 	public PropertySource<?> remove(String name) {
 		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Removing [%s] PropertySource", name));
+			logger.debug("Removing PropertySource '" + name + "'");
 		}
 		int index = this.propertySourceList.indexOf(PropertySource.named(name));
 		return (index != -1 ? this.propertySourceList.remove(index) : null);
@@ -173,8 +170,7 @@ public class MutablePropertySources implements PropertySources {
 	 */
 	public void replace(String name, PropertySource<?> propertySource) {
 		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Replacing [%s] PropertySource with [%s]",
-					name, propertySource.getName()));
+			logger.debug("Replacing PropertySource '" + name + "' with '" + propertySource.getName() + "'");
 		}
 		int index = assertPresentAndGetIndex(name);
 		this.propertySourceList.set(index, propertySource);
@@ -189,11 +185,7 @@ public class MutablePropertySources implements PropertySources {
 
 	@Override
 	public String toString() {
-		String[] names = new String[this.size()];
-		for (int i = 0; i < size(); i++) {
-			names[i] = this.propertySourceList.get(i).getName();
-		}
-		return String.format("[%s]", StringUtils.arrayToCommaDelimitedString(names));
+		return this.propertySourceList.toString();
 	}
 
 	/**
@@ -203,7 +195,7 @@ public class MutablePropertySources implements PropertySources {
 		String newPropertySourceName = propertySource.getName();
 		if (relativePropertySourceName.equals(newPropertySourceName)) {
 			throw new IllegalArgumentException(
-					String.format("PropertySource named [%s] cannot be added relative to itself", newPropertySourceName));
+					"PropertySource named '" + newPropertySourceName + "' cannot be added relative to itself");
 		}
 	}
 
@@ -224,14 +216,13 @@ public class MutablePropertySources implements PropertySources {
 
 	/**
 	 * Assert that the named property source is present and return its index.
-	 * @param name the {@linkplain PropertySource#getName() name of the property source}
-	 * to find
+	 * @param name {@linkplain PropertySource#getName() name of the property source} to find
 	 * @throws IllegalArgumentException if the named property source is not present
 	 */
 	private int assertPresentAndGetIndex(String name) {
 		int index = this.propertySourceList.indexOf(PropertySource.named(name));
 		if (index == -1) {
-			throw new IllegalArgumentException(String.format("PropertySource named [%s] does not exist", name));
+			throw new IllegalArgumentException("PropertySource named '" + name + "' does not exist");
 		}
 		return index;
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -157,11 +157,11 @@ public abstract class PropertySource<T> {
 	@Override
 	public String toString() {
 		if (logger.isDebugEnabled()) {
-			return String.format("%s@%s [name='%s', properties=%s]",
-					getClass().getSimpleName(), System.identityHashCode(this), this.name, this.source);
+			return getClass().getSimpleName() + "@" + System.identityHashCode(this) +
+					" {name='" + this.name + "', properties=" + this.source + "}";
 		}
 		else {
-			return String.format("%s [name='%s']", getClass().getSimpleName(), this.name);
+			return getClass().getSimpleName() + " {name='" + this.name + "'}";
 		}
 	}
 
@@ -242,11 +242,6 @@ public abstract class PropertySource<T> {
 		public String getProperty(String name) {
 			throw new UnsupportedOperationException(USAGE_ERROR);
 		}
-
-		@Override
-		public String toString() {
-			return String.format("%s [name='%s']", getClass().getSimpleName(), this.name);
-		}
 	}
 
 }

spring-core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -76,7 +76,8 @@ public class PropertySourcesPropertyResolver extends AbstractPropertyResolver {
 		if (this.propertySources != null) {
 			for (PropertySource<?> propertySource : this.propertySources) {
 				if (logger.isTraceEnabled()) {
-					logger.trace(String.format("Searching for key '%s' in [%s]", key, propertySource.getName()));
+					logger.trace("Searching for key '" + key + "' in PropertySource '" +
+							propertySource.getName() + "'");
 				}
 				Object value = propertySource.getProperty(key);
 				if (value != null) {
@@ -89,7 +90,7 @@ public class PropertySourcesPropertyResolver extends AbstractPropertyResolver {
 			}
 		}
 		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Could not find key '%s' in any property source", key));
+			logger.debug("Could not find key '" + key + "' in any property source");
 		}
 		return null;
 	}
@@ -108,8 +109,8 @@ public class PropertySourcesPropertyResolver extends AbstractPropertyResolver {
 	 */
 	protected void logKeyFound(String key, PropertySource<?> propertySource, Object value) {
 		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Found key '%s' in [%s] with type [%s]",
-					key, propertySource.getName(), value.getClass().getSimpleName()));
+			logger.debug("Found key '" + key + "' in PropertySource '" + propertySource.getName() +
+					"' with value of type " + value.getClass().getSimpleName());
 		}
 	}
 

spring-core/src/main/java/org/springframework/core/env/SystemEnvironmentPropertySource.java

@@ -91,8 +91,8 @@ public class SystemEnvironmentPropertySource extends MapPropertySource {
 	public Object getProperty(String name) {
 		String actualName = resolvePropertyName(name);
 		if (logger.isDebugEnabled() && !name.equals(actualName)) {
-			logger.debug(String.format("PropertySource [%s] does not contain '%s', but found equivalent '%s'",
-					getName(), name, actualName));
+			logger.debug("PropertySource '" + getName() + "' does not contain property '" + name +
+					"', but found equivalent '" + actualName + "'");
 		}
 		return super.getProperty(actualName);
 	}