diff --git a/archaius2-api/src/main/java/com/netflix/archaius/api/ArchaiusType.java b/archaius2-api/src/main/java/com/netflix/archaius/api/ArchaiusType.java index b2223600..18c041d7 100644 --- a/archaius2-api/src/main/java/com/netflix/archaius/api/ArchaiusType.java +++ b/archaius2-api/src/main/java/com/netflix/archaius/api/ArchaiusType.java @@ -92,4 +92,38 @@ public String toString() { String typeArgumentNames = Arrays.stream(typeArguments).map(Class::getSimpleName).collect(Collectors.joining(",")); return String.format("parameterizedType for %s<%s>", rawType.getSimpleName(), typeArgumentNames); } + + @Override + public int hashCode() { + int result = 1; + result = 31 * result + (this.rawType == null ? 0 : this.rawType.hashCode()); + result = 31 * result + Arrays.hashCode(this.typeArguments); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } else if (obj == null) { + return false; + } else if (this.getClass() != obj.getClass()) { + return false; + } + + ArchaiusType other = (ArchaiusType) obj; + if ((this.rawType == null) && (other.rawType != null)) { + return false; + } else if (this.rawType != null && !this.rawType.equals(other.rawType)) { + return false; + } + + if ((this.typeArguments == null) && (other.typeArguments != null)) { + return false; + } else if (this.typeArguments != null && !Arrays.equals(this.typeArguments, other.typeArguments)) { + return false; + } + + return true; + } } diff --git a/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java b/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java index 1c331ffc..282a97e9 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java @@ -15,6 +15,7 @@ */ package com.netflix.archaius.property; +import java.lang.reflect.Field; import java.math.BigDecimal; import java.math.BigInteger; import java.time.Duration; @@ -45,6 +46,10 @@ import com.netflix.archaius.config.DefaultSettableConfig; import com.netflix.archaius.config.MapConfig; +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.fail; +import static org.junit.Assert.assertNotEquals; + @SuppressWarnings("deprecation") public class PropertyTest { static class MyService { @@ -534,4 +539,101 @@ public void customMappingWithError() { Integer value = factory.getProperty("a").asType(Integer::parseInt, "1").get(); Assert.assertEquals(1, value.intValue()); } + + @Test + public void getListShouldReuseKey() { + SettableConfig config = new DefaultSettableConfig(); + DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); + config.setProperty("geralt", "of,rivia"); + + List expectedList = Arrays.asList("of", "rivia"); + + Property> firstReference = factory.getList("geralt", String.class); + assertEquals(expectedList, firstReference.get()); + + Property> secondReference = factory.getList("geralt", String.class); + assertEquals(expectedList, secondReference.get()); + + ensureReferencesMatch(firstReference, secondReference); + } + + @Test + public void getSetShouldReuseKey() { + SettableConfig config = new DefaultSettableConfig(); + DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); + config.setProperty("geralt", "of,rivia"); + + Set expectedSet = new HashSet<>(Arrays.asList("of", "rivia")); + + Property> firstReference = factory.getSet("geralt", String.class); + assertEquals(expectedSet, firstReference.get()); + + Property> secondReference = factory.getSet("geralt", String.class); + assertEquals(expectedSet, secondReference.get()); + + ensureReferencesMatch(firstReference, secondReference); + } + + @Test + public void getMapShouldReuseKey() { + SettableConfig config = new DefaultSettableConfig(); + DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); + config.setProperty("geralt", "of=rivia"); + + Map expectedMap = new HashMap<>(); + expectedMap.put("of", "rivia"); + + Property> firstReference = factory.getMap("geralt", String.class, String.class); + assertEquals(expectedMap, firstReference.get()); + + Property> secondReference = factory.getMap("geralt", String.class, String.class); + assertEquals(expectedMap, secondReference.get()); + + ensureReferencesMatch(firstReference, secondReference); + } + + @Test + public void getCollectionShouldNotReuseKeyWithDifferentTypes() { + SettableConfig config = new DefaultSettableConfig(); + DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); + config.setProperty("geralt", "of,rivia"); + + Property> firstReference = factory.getList("geralt", String.class); + assertEquals(Arrays.asList("of", "rivia"), firstReference.get()); + + Property> secondReference = factory.getSet("geralt", String.class); + assertEquals(new HashSet<>(Arrays.asList("of", "rivia")), secondReference.get()); + + ensureReferencesDoNotMatch(firstReference, secondReference); + } + + private void ensureReferencesMatch(Property firstReference, Property secondReference) { + ensureReferencesMatch(firstReference, secondReference, true); + } + + private void ensureReferencesDoNotMatch(Property firstReference, Property secondReference) { + ensureReferencesMatch(firstReference, secondReference, false); + } + + private void ensureReferencesMatch(Property firstReference, Property secondReference, boolean shouldMatch) { + try { + // inspect the keyAndType private field within DefaultPropertyFactory + // to validate that we hold the same reference to the key when caching keys + // this ensures that we don't leak references to keys where the key and types match + Field keyAndType = firstReference.getClass().getDeclaredField("keyAndType"); + keyAndType.setAccessible(true); + Object firstReferenceValue = keyAndType.get(firstReference); + Object secondReferenceValue = keyAndType.get(secondReference); + if (shouldMatch) { + assertEquals(firstReferenceValue, secondReferenceValue); + } else { + assertNotEquals(firstReferenceValue, secondReferenceValue); + } + } catch (Exception e) { + fail(String.format( + "Expect references [%s] and [%s] keyAndType to be %s " + + "- this can cause memory leaks and inefficient allocations: %s", + firstReference, secondReference, shouldMatch ? "equal" : "not equal", e.getMessage())); + } + } }