Skip to content

Commit 57fbdc0

Browse files
jhoellerjhoeller
committed
Consistent logging in Environment and PropertySource implementations
Issue: SPR-15825 (cherry picked from commit fac83b2)
1 parent f5d689e commit 57fbdc0

File tree

9 files changed

+53
-102
lines changed

9 files changed

+53
-102
lines changed

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

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -120,9 +120,8 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
120120
*/
121121
public AbstractEnvironment() {
122122
customizePropertySources(this.propertySources);
123-
if (this.logger.isDebugEnabled()) {
124-
this.logger.debug(String.format(
125-
"Initialized %s with PropertySources %s", getClass().getSimpleName(), this.propertySources));
123+
if (logger.isDebugEnabled()) {
124+
logger.debug("Initialized " + getClass().getSimpleName() + " with PropertySources " + this.propertySources);
126125
}
127126
}
128127

@@ -261,8 +260,8 @@ public void setActiveProfiles(String... profiles) {
261260

262261
@Override
263262
public void addActiveProfile(String profile) {
264-
if (this.logger.isDebugEnabled()) {
265-
this.logger.debug(String.format("Activating profile '%s'", profile));
263+
if (logger.isDebugEnabled()) {
264+
logger.debug("Activating profile '" + profile + "'");
266265
}
267266
validateProfile(profile);
268267
doGetActiveProfiles();
@@ -392,9 +391,8 @@ protected String getSystemAttribute(String attributeName) {
392391
}
393392
catch (AccessControlException ex) {
394393
if (logger.isInfoEnabled()) {
395-
logger.info(String.format("Caught AccessControlException when accessing system " +
396-
"environment variable [%s]; its value will be returned [null]. Reason: %s",
397-
attributeName, ex.getMessage()));
394+
logger.info("Caught AccessControlException when accessing system environment variable '" +
395+
attributeName + "'; its value will be returned [null]. Reason: " + ex.getMessage());
398396
}
399397
return null;
400398
}
@@ -433,9 +431,8 @@ protected String getSystemAttribute(String attributeName) {
433431
}
434432
catch (AccessControlException ex) {
435433
if (logger.isInfoEnabled()) {
436-
logger.info(String.format("Caught AccessControlException when accessing system " +
437-
"property [%s]; its value will be returned [null]. Reason: %s",
438-
attributeName, ex.getMessage()));
434+
logger.info("Caught AccessControlException when accessing system property '" +
435+
attributeName + "'; its value will be returned [null]. Reason: " + ex.getMessage());
439436
}
440437
return null;
441438
}
@@ -574,9 +571,8 @@ public String resolveRequiredPlaceholders(String text) throws IllegalArgumentExc
574571

575572
@Override
576573
public String toString() {
577-
return String.format("%s {activeProfiles=%s, defaultProfiles=%s, propertySources=%s}",
578-
getClass().getSimpleName(), this.activeProfiles, this.defaultProfiles,
579-
this.propertySources);
574+
return getClass().getSimpleName() + " {activeProfiles=" + this.activeProfiles +
575+
", defaultProfiles=" + this.defaultProfiles + ", propertySources=" + this.propertySources + "}";
580576
}
581577

582578
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -180,7 +180,7 @@ public <T> Class<T> getPropertyAsClass(String key, Class<T> targetValueType) {
180180
public String getRequiredProperty(String key) throws IllegalStateException {
181181
String value = getProperty(key);
182182
if (value == null) {
183-
throw new IllegalStateException(String.format("required key [%s] not found", key));
183+
throw new IllegalStateException("Required key '" + key + "' not found");
184184
}
185185
return value;
186186
}
@@ -189,7 +189,7 @@ public String getRequiredProperty(String key) throws IllegalStateException {
189189
public <T> T getRequiredProperty(String key, Class<T> valueType) throws IllegalStateException {
190190
T value = getProperty(key, valueType);
191191
if (value == null) {
192-
throw new IllegalStateException(String.format("required key [%s] not found", key));
192+
throw new IllegalStateException("Required key '" + key + "' not found");
193193
}
194194
return value;
195195
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -33,24 +33,25 @@ public class MissingRequiredPropertiesException extends IllegalStateException {
3333

3434
private final Set<String> missingRequiredProperties = new LinkedHashSet<String>();
3535

36+
37+
void addMissingRequiredProperty(String key) {
38+
this.missingRequiredProperties.add(key);
39+
}
40+
41+
@Override
42+
public String getMessage() {
43+
return "The following properties were declared as required but could not be resolved: " +
44+
getMissingRequiredProperties();
45+
}
46+
3647
/**
3748
* Return the set of properties marked as required but not present
3849
* upon validation.
3950
* @see ConfigurablePropertyResolver#setRequiredProperties(String...)
4051
* @see ConfigurablePropertyResolver#validateRequiredProperties()
4152
*/
4253
public Set<String> getMissingRequiredProperties() {
43-
return missingRequiredProperties;
44-
}
45-
46-
void addMissingRequiredProperty(String key) {
47-
missingRequiredProperties.add(key);
54+
return this.missingRequiredProperties;
4855
}
4956

50-
@Override
51-
public String getMessage() {
52-
return String.format(
53-
"The following properties were declared as required but could " +
54-
"not be resolved: %s", this.getMissingRequiredProperties());
55-
}
5657
}

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

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -23,8 +23,6 @@
2323
import org.apache.commons.logging.Log;
2424
import org.apache.commons.logging.LogFactory;
2525

26-
import org.springframework.util.StringUtils;
27-
2826
/**
2927
* Default implementation of the {@link PropertySources} interface.
3028
* Allows manipulation of contained property sources and provides a constructor
@@ -94,8 +92,7 @@ public Iterator<PropertySource<?>> iterator() {
9492
*/
9593
public void addFirst(PropertySource<?> propertySource) {
9694
if (logger.isDebugEnabled()) {
97-
logger.debug(String.format("Adding [%s] PropertySource with highest search precedence",
98-
propertySource.getName()));
95+
logger.debug("Adding PropertySource '" + propertySource.getName() + "' with highest search precedence");
9996
}
10097
removeIfPresent(propertySource);
10198
this.propertySourceList.add(0, propertySource);
@@ -106,8 +103,7 @@ public void addFirst(PropertySource<?> propertySource) {
106103
*/
107104
public void addLast(PropertySource<?> propertySource) {
108105
if (logger.isDebugEnabled()) {
109-
logger.debug(String.format("Adding [%s] PropertySource with lowest search precedence",
110-
propertySource.getName()));
106+
logger.debug("Adding PropertySource '" + propertySource.getName() + "' with lowest search precedence");
111107
}
112108
removeIfPresent(propertySource);
113109
this.propertySourceList.add(propertySource);
@@ -119,8 +115,8 @@ public void addLast(PropertySource<?> propertySource) {
119115
*/
120116
public void addBefore(String relativePropertySourceName, PropertySource<?> propertySource) {
121117
if (logger.isDebugEnabled()) {
122-
logger.debug(String.format("Adding [%s] PropertySource with search precedence immediately higher than [%s]",
123-
propertySource.getName(), relativePropertySourceName));
118+
logger.debug("Adding PropertySource '" + propertySource.getName() +
119+
"' with search precedence immediately higher than '" + relativePropertySourceName + "'");
124120
}
125121
assertLegalRelativeAddition(relativePropertySourceName, propertySource);
126122
removeIfPresent(propertySource);
@@ -134,8 +130,8 @@ public void addBefore(String relativePropertySourceName, PropertySource<?> prope
134130
*/
135131
public void addAfter(String relativePropertySourceName, PropertySource<?> propertySource) {
136132
if (logger.isDebugEnabled()) {
137-
logger.debug(String.format("Adding [%s] PropertySource with search precedence immediately lower than [%s]",
138-
propertySource.getName(), relativePropertySourceName));
133+
logger.debug("Adding PropertySource '" + propertySource.getName() +
134+
"' with search precedence immediately lower than '" + relativePropertySourceName + "'");
139135
}
140136
assertLegalRelativeAddition(relativePropertySourceName, propertySource);
141137
removeIfPresent(propertySource);
@@ -156,7 +152,7 @@ public int precedenceOf(PropertySource<?> propertySource) {
156152
*/
157153
public PropertySource<?> remove(String name) {
158154
if (logger.isDebugEnabled()) {
159-
logger.debug(String.format("Removing [%s] PropertySource", name));
155+
logger.debug("Removing PropertySource '" + name + "'");
160156
}
161157
int index = this.propertySourceList.indexOf(PropertySource.named(name));
162158
return (index != -1 ? this.propertySourceList.remove(index) : null);
@@ -171,8 +167,7 @@ public PropertySource<?> remove(String name) {
171167
*/
172168
public void replace(String name, PropertySource<?> propertySource) {
173169
if (logger.isDebugEnabled()) {
174-
logger.debug(String.format("Replacing [%s] PropertySource with [%s]",
175-
name, propertySource.getName()));
170+
logger.debug("Replacing PropertySource '" + name + "' with '" + propertySource.getName() + "'");
176171
}
177172
int index = assertPresentAndGetIndex(name);
178173
this.propertySourceList.set(index, propertySource);
@@ -187,11 +182,7 @@ public int size() {
187182

188183
@Override
189184
public String toString() {
190-
String[] names = new String[this.size()];
191-
for (int i = 0; i < size(); i++) {
192-
names[i] = this.propertySourceList.get(i).getName();
193-
}
194-
return String.format("[%s]", StringUtils.arrayToCommaDelimitedString(names));
185+
return this.propertySourceList.toString();
195186
}
196187

197188
/**
@@ -201,7 +192,7 @@ protected void assertLegalRelativeAddition(String relativePropertySourceName, Pr
201192
String newPropertySourceName = propertySource.getName();
202193
if (relativePropertySourceName.equals(newPropertySourceName)) {
203194
throw new IllegalArgumentException(
204-
String.format("PropertySource named [%s] cannot be added relative to itself", newPropertySourceName));
195+
"PropertySource named '" + newPropertySourceName + "' cannot be added relative to itself");
205196
}
206197
}
207198

@@ -222,14 +213,13 @@ private void addAtIndex(int index, PropertySource<?> propertySource) {
222213

223214
/**
224215
* Assert that the named property source is present and return its index.
225-
* @param name the {@linkplain PropertySource#getName() name of the property source}
226-
* to find
216+
* @param name {@linkplain PropertySource#getName() name of the property source} to find
227217
* @throws IllegalArgumentException if the named property source is not present
228218
*/
229219
private int assertPresentAndGetIndex(String name) {
230220
int index = this.propertySourceList.indexOf(PropertySource.named(name));
231221
if (index == -1) {
232-
throw new IllegalArgumentException(String.format("PropertySource named [%s] does not exist", name));
222+
throw new IllegalArgumentException("PropertySource named '" + name + "' does not exist");
233223
}
234224
return index;
235225
}

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -155,11 +155,11 @@ public int hashCode() {
155155
@Override
156156
public String toString() {
157157
if (logger.isDebugEnabled()) {
158-
return String.format("%s@%s [name='%s', properties=%s]",
159-
getClass().getSimpleName(), System.identityHashCode(this), this.name, this.source);
158+
return getClass().getSimpleName() + "@" + System.identityHashCode(this) +
159+
" {name='" + this.name + "', properties=" + this.source + "}";
160160
}
161161
else {
162-
return String.format("%s [name='%s']", getClass().getSimpleName(), this.name);
162+
return getClass().getSimpleName() + " {name='" + this.name + "'}";
163163
}
164164
}
165165

@@ -240,11 +240,6 @@ public boolean containsProperty(String name) {
240240
public String getProperty(String name) {
241241
throw new UnsupportedOperationException(USAGE_ERROR);
242242
}
243-
244-
@Override
245-
public String toString() {
246-
return String.format("%s [name='%s']", getClass().getSimpleName(), this.name);
247-
}
248243
}
249244

250245
}

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -75,7 +75,8 @@ protected <T> T getProperty(String key, Class<T> targetValueType, boolean resolv
7575
if (this.propertySources != null) {
7676
for (PropertySource<?> propertySource : this.propertySources) {
7777
if (logger.isTraceEnabled()) {
78-
logger.trace(String.format("Searching for key '%s' in [%s]", key, propertySource.getName()));
78+
logger.trace("Searching for key '" + key + "' in PropertySource '" +
79+
propertySource.getName() + "'");
7980
}
8081
Object value = propertySource.getProperty(key);
8182
if (value != null) {
@@ -88,7 +89,7 @@ protected <T> T getProperty(String key, Class<T> targetValueType, boolean resolv
8889
}
8990
}
9091
if (logger.isDebugEnabled()) {
91-
logger.debug(String.format("Could not find key '%s' in any property source", key));
92+
logger.debug("Could not find key '" + key + "' in any property source");
9293
}
9394
return null;
9495
}
@@ -148,8 +149,8 @@ else if (value instanceof Class) {
148149
*/
149150
protected void logKeyFound(String key, PropertySource<?> propertySource, Object value) {
150151
if (logger.isDebugEnabled()) {
151-
logger.debug(String.format("Found key '%s' in [%s] with type [%s]",
152-
key, propertySource.getName(), value.getClass().getSimpleName()));
152+
logger.debug("Found key '" + key + "' in PropertySource '" + propertySource.getName() +
153+
"' with value of type " + value.getClass().getSimpleName());
153154
}
154155
}
155156

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ public boolean containsProperty(String name) {
9090
public Object getProperty(String name) {
9191
String actualName = resolvePropertyName(name);
9292
if (logger.isDebugEnabled() && !name.equals(actualName)) {
93-
logger.debug(String.format("PropertySource [%s] does not contain '%s', but found equivalent '%s'",
94-
getName(), name, actualName));
93+
logger.debug("PropertySource '" + getName() + "' does not contain property '" + name +
94+
"', but found equivalent '" + actualName + "'");
9595
}
9696
return super.getProperty(actualName);
9797
}

‎spring-core/src/test/java/org/springframework/core/env/PropertySourceTests.java

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import java.util.Map;
2323
import java.util.Properties;
2424

25-
import org.apache.log4j.Level;
26-
import org.apache.log4j.Logger;
2725
import org.junit.Test;
2826

2927
import static org.hamcrest.CoreMatchers.*;
@@ -90,34 +88,4 @@ public void collectionsOperations() {
9088
propertySources.clear();
9189
}
9290

93-
@Test @SuppressWarnings("serial")
94-
public void toString_verbosityVariesOnLogLevel() {
95-
String name = "props";
96-
Map<String, Object> map = new HashMap<String, Object>() {{ put("k1", "v1"); }};
97-
MapPropertySource ps = new MapPropertySource(name, map);
98-
Logger logger = Logger.getLogger(ps.getClass());
99-
Level original = logger.getLevel();
100-
101-
try {
102-
logger.setLevel(Level.DEBUG);
103-
assertThat("PropertySource.toString() should render verbose output for log levels <= DEBUG",
104-
ps.toString(),
105-
equalTo(String.format("%s@%s [name='%s', properties=%s]",
106-
ps.getClass().getSimpleName(),
107-
System.identityHashCode(ps),
108-
name,
109-
map)));
110-
111-
logger.setLevel(Level.INFO);
112-
assertThat("PropertySource.toString() should render concise output for log levels >= INFO",
113-
ps.toString(),
114-
equalTo(String.format("%s [name='%s']",
115-
ps.getClass().getSimpleName(),
116-
name)));
117-
}
118-
finally {
119-
logger.setLevel(original);
120-
}
121-
}
122-
12391
}

0 commit comments

Comments
 (0)