Return null correctly from MutablePropertySources#get

Prior to this commit, MutablePropertySources#get(String) would throw
IndexArrayOutOfBoundsException if the named property source does not
actually exist. This is a violation of the PropertySource#get contract
as described in its Javadoc.

The implementation now correctly checks for the existence of the named
property source, returning null if non-existent and otherwise returning
the associated PropertySource.

Other changes

 - Rename PropertySourcesTests => MutablePropertySourcesTests
 - Polish MutablePropertySourcesTests for style, formatting
 - Refactor MutablePropertySources for consistency

Issue: SPR-9179
This commit is contained in:
Chris Beams 2012-02-29 14:29:13 +01:00
parent 43b4997e3f
commit 15d1d824b5
2 changed files with 32 additions and 23 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2011 the original author or authors.
* Copyright 2002-2012 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.
@ -79,7 +79,8 @@ public class MutablePropertySources implements PropertySources {
}
public PropertySource<?> get(String name) {
return this.propertySourceList.get(this.propertySourceList.indexOf(PropertySource.named(name)));
int index = this.propertySourceList.indexOf(PropertySource.named(name));
return index == -1 ? null : this.propertySourceList.get(index);
}
public Iterator<PropertySource<?>> iterator() {
@ -146,10 +147,7 @@ public class MutablePropertySources implements PropertySources {
public PropertySource<?> remove(String name) {
logger.debug(String.format("Removing [%s] PropertySource", name));
int index = this.propertySourceList.indexOf(PropertySource.named(name));
if (index >= 0) {
return this.propertySourceList.remove(index);
}
return null;
return index == -1 ? null : this.propertySourceList.remove(index);
}
/**
@ -210,11 +208,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
* @throws IllegalArgumentException if the named property source is not present
*/
private int assertPresentAndGetIndex(String propertySourceName) {
int index = this.propertySourceList.indexOf(PropertySource.named(propertySourceName));
Assert.isTrue(index >= 0, String.format(NON_EXISTENT_PROPERTY_SOURCE_MESSAGE, propertySourceName));
private int assertPresentAndGetIndex(String name) {
int index = this.propertySourceList.indexOf(PropertySource.named(name));
Assert.isTrue(index >= 0, String.format(NON_EXISTENT_PROPERTY_SOURCE_MESSAGE, name));
return index;
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2010 the original author or authors.
* Copyright 2002-2012 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.
@ -16,18 +16,21 @@
package org.springframework.core.env;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import org.junit.Test;
import org.springframework.mock.env.MockPropertySource;
public class PropertySourcesTests {
import static java.lang.String.*;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
import static org.springframework.core.env.MutablePropertySources.*;
/**
* Unit tests for {@link MutablePropertySources}
*
* @author Chris Beams
*/
public class MutablePropertySourcesTests {
@Test
public void test() {
MutablePropertySources sources = new MutablePropertySources();
@ -104,7 +107,7 @@ public class PropertySourcesTests {
fail("expected non-existent PropertySource exception");
} catch (IllegalArgumentException ex) {
assertThat(ex.getMessage(),
equalTo(String.format(MutablePropertySources.NON_EXISTENT_PROPERTY_SOURCE_MESSAGE, bogusPS)));
equalTo(format(NON_EXISTENT_PROPERTY_SOURCE_MESSAGE, bogusPS)));
}
sources.addFirst(new MockPropertySource("a"));
@ -126,7 +129,7 @@ public class PropertySourcesTests {
fail("expected non-existent PropertySource exception");
} catch (IllegalArgumentException ex) {
assertThat(ex.getMessage(),
equalTo(String.format(MutablePropertySources.NON_EXISTENT_PROPERTY_SOURCE_MESSAGE, bogusPS)));
equalTo(format(NON_EXISTENT_PROPERTY_SOURCE_MESSAGE, bogusPS)));
}
try {
@ -134,7 +137,7 @@ public class PropertySourcesTests {
fail("expected exception");
} catch (IllegalArgumentException ex) {
assertThat(ex.getMessage(),
equalTo(String.format(MutablePropertySources.ILLEGAL_RELATIVE_ADDITION_MESSAGE, "b")));
equalTo(format(ILLEGAL_RELATIVE_ADDITION_MESSAGE, "b")));
}
try {
@ -142,8 +145,14 @@ public class PropertySourcesTests {
fail("expected exception");
} catch (IllegalArgumentException ex) {
assertThat(ex.getMessage(),
equalTo(String.format(MutablePropertySources.ILLEGAL_RELATIVE_ADDITION_MESSAGE, "b")));
equalTo(format(ILLEGAL_RELATIVE_ADDITION_MESSAGE, "b")));
}
}
@Test
public void getNonExistentPropertySourceReturnsNull() {
MutablePropertySources sources = new MutablePropertySources();
assertThat(sources.get("bogus"), nullValue());
}
}