From 7d72ab59ae1841fecb0f33c938b37234dde75a8f Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Wed, 29 Feb 2012 14:29:13 +0100 Subject: [PATCH] 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-9185 Backport-Issue: SPR-9179 Backport-Commit: 15d1d824b54b2ec36a7c926e12644b391b310930 --- .../core/env/MutablePropertySources.java | 18 ++++----- ....java => MutablePropertySourcesTests.java} | 37 ++++++++++++------- 2 files changed, 32 insertions(+), 23 deletions(-) rename org.springframework.core/src/test/java/org/springframework/core/env/{PropertySourcesTests.java => MutablePropertySourcesTests.java} (87%) diff --git a/org.springframework.core/src/main/java/org/springframework/core/env/MutablePropertySources.java b/org.springframework.core/src/main/java/org/springframework/core/env/MutablePropertySources.java index 64614536c6b..6ddc48a2a46 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/env/MutablePropertySources.java +++ b/org.springframework.core/src/main/java/org/springframework/core/env/MutablePropertySources.java @@ -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> 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; } diff --git a/org.springframework.core/src/test/java/org/springframework/core/env/PropertySourcesTests.java b/org.springframework.core/src/test/java/org/springframework/core/env/MutablePropertySourcesTests.java similarity index 87% rename from org.springframework.core/src/test/java/org/springframework/core/env/PropertySourcesTests.java rename to org.springframework.core/src/test/java/org/springframework/core/env/MutablePropertySourcesTests.java index a404445639e..747c30b1ef9 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/env/PropertySourcesTests.java +++ b/org.springframework.core/src/test/java/org/springframework/core/env/MutablePropertySourcesTests.java @@ -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()); + } + }