Make ArchaiusType equals / hashCode compatible with ParameterizedType…

…Impl Make ArchaiusType's equals / hashCode / toString match what JDK's ParameterizedTypeImpl does, so that equivalent ParameterizedType instances hash and compare equal with ArchaiusType. Update toString to match what ParameterizedTypeImpl does as well. Additionally, add do a defensive copy of the returned array in getActualTypeArguments, and add basic unit tests.
Netflix · Feb 24, 2024 · 01bf4dc · 01bf4dc
1 parent 86b7d29
commit 01bf4dc
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 29 deletions.
diff --git a/archaius2-api/build.gradle b/archaius2-api/build.gradle
@@ -19,6 +19,7 @@ apply plugin: 'java-library'
 dependencies {
     api     'javax.inject:javax.inject:1'
     implementation     'org.slf4j:slf4j-api:1.7.36'
+    testImplementation 'junit:junit:4.13.2'
 }
 
 eclipse {

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
@@ -9,7 +9,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 /**
  * An implementation of {@link ParameterizedType} that can represent the collection types that Archaius can
@@ -21,24 +20,24 @@
  * @see Config#get(Type, String)
  * @see Config#get(Type, String, Object)
  */
-public class ArchaiusType implements ParameterizedType {
+public final class ArchaiusType implements ParameterizedType {
 
     /** Return a parameterizedType to represent a {@code List<listValuesType>} */
     public static ParameterizedType forListOf(Class<?> listValuesType) {
-        Class<?> maybeWrappedType = PRIMITIVE_WRAPPERS.getOrDefault(listValuesType, listValuesType);
+        Class<?> maybeWrappedType = listValuesType.isPrimitive() ? PRIMITIVE_WRAPPERS.getOrDefault(listValuesType, listValuesType) : listValuesType;
         return new ArchaiusType(List.class, new Class<?>[] { maybeWrappedType });
     }
 
     /** Return a parameterizedType to represent a {@code Set<setValuesType>} */
     public static ParameterizedType forSetOf(Class<?> setValuesType) {
-        Class<?> maybeWrappedType = PRIMITIVE_WRAPPERS.getOrDefault(setValuesType, setValuesType);
+        Class<?> maybeWrappedType = setValuesType.isPrimitive() ? PRIMITIVE_WRAPPERS.getOrDefault(setValuesType, setValuesType) : setValuesType;
         return new ArchaiusType(Set.class, new Class<?>[] { maybeWrappedType });
     }
 
     /** Return a parameterizedType to represent a {@code Map<mapKeysType, mapValuesType>} */
-    public static ParameterizedType forMapOf(Class<?> mapKeysTpe, Class<?> mapValuesType) {
-        Class<?> maybeWrappedKeyType = PRIMITIVE_WRAPPERS.getOrDefault(mapKeysTpe, mapKeysTpe);
-        Class<?> maybeWrappedValuesType = PRIMITIVE_WRAPPERS.getOrDefault(mapValuesType, mapValuesType);
+    public static ParameterizedType forMapOf(Class<?> mapKeysType, Class<?> mapValuesType) {
+        Class<?> maybeWrappedKeyType = mapKeysType.isPrimitive() ? PRIMITIVE_WRAPPERS.getOrDefault(mapKeysType, mapKeysType) : mapKeysType;
+        Class<?> maybeWrappedValuesType = mapValuesType.isPrimitive() ? PRIMITIVE_WRAPPERS.getOrDefault(mapValuesType, mapValuesType) : mapValuesType;
 
         return new ArchaiusType(Map.class, new Class<?>[] {maybeWrappedKeyType, maybeWrappedValuesType});
     }
@@ -74,7 +73,7 @@ private ArchaiusType(Class<?> rawType, Class<?>[] typeArguments) {
 
     @Override
     public Type[] getActualTypeArguments() {
-        return typeArguments;
+        return typeArguments.clone();
     }
 
     @Override
@@ -89,16 +88,23 @@ public Type getOwnerType() {
 
     @Override
     public String toString() {
-        String typeArgumentNames = Arrays.stream(typeArguments).map(Class::getSimpleName).collect(Collectors.joining(","));
-        return String.format("parameterizedType for %s<%s>", rawType.getSimpleName(), typeArgumentNames);
+        StringBuilder sb = new StringBuilder(rawType.getName());
+        sb.append('<');
+        boolean first = true;
+        for (Type t : typeArguments) {
+            if (!first) {
+                sb.append(", ");
+            }
+            sb.append(t.getTypeName());
+            first = false;
+        }
+        sb.append('>');
+        return sb.toString();
     }
 
     @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;
+        return Arrays.hashCode(typeArguments) ^ rawType.hashCode();
     }
 
     @Override
@@ -107,23 +113,13 @@ public boolean equals(Object 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)) {
+        } else if (!(obj instanceof ParameterizedType)) {
             return false;
         }
 
-        return true;
+        ParameterizedType other = (ParameterizedType) obj;
+        return other.getOwnerType() == null &&
+                Objects.equals(rawType, other.getRawType()) &&
+                Arrays.equals(typeArguments, other.getActualTypeArguments());
     }
 }
diff --git a/archaius2-api/src/test/java/com/netflix/archaius/api/ArchaiusTypeTest.java b/archaius2-api/src/test/java/com/netflix/archaius/api/ArchaiusTypeTest.java
@@ -0,0 +1,70 @@
+package com.netflix.archaius.api;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+public class ArchaiusTypeTest {
+    @Test
+    public void testEquals() {
+        ParameterizedType archaiusType = ArchaiusType.forListOf(String.class);
+        Assert.assertEquals(archaiusType, listOfString);
+        Assert.assertEquals(listOfString, archaiusType);
+        Assert.assertEquals(archaiusType, ArchaiusType.forListOf(String.class));
+        Assert.assertNotEquals(archaiusType, ArchaiusType.forListOf(Integer.class));
+        Assert.assertNotEquals(archaiusType, setOfLong);
+    }
+
+    @Test
+    public void testHashCode() {
+        Assert.assertEquals(listOfString.hashCode(), ArchaiusType.forListOf(String.class).hashCode());
+        Assert.assertEquals(ArchaiusType.forListOf(String.class).hashCode(), ArchaiusType.forListOf(String.class).hashCode());
+        Assert.assertEquals(setOfLong.hashCode(), ArchaiusType.forSetOf(Long.class).hashCode());
+        Assert.assertEquals(ArchaiusType.forMapOf(Integer.class, CharSequence.class).hashCode(), mapOfIntToCharSequence.hashCode());
+    }
+
+    @Test
+    public void testToString() {
+        Assert.assertEquals("java.util.List<java.lang.String>", ArchaiusType.forListOf(String.class).toString());
+        Assert.assertEquals(listOfString.toString(), ArchaiusType.forListOf(String.class).toString());
+        Assert.assertEquals(setOfLong.toString(), ArchaiusType.forSetOf(Long.class).toString());
+        Assert.assertEquals(mapOfIntToCharSequence.toString(), ArchaiusType.forMapOf(Integer.class, CharSequence.class).toString());
+    }
+
+    @Test
+    public void testPrimitiveType() {
+        Assert.assertEquals(setOfLong, ArchaiusType.forSetOf(long.class));
+    }
+
+    @Test
+    public void testGetTypeParameters() {
+        ParameterizedType archaiusType = ArchaiusType.forSetOf(Long.class);
+        Type[] typeArguments = archaiusType.getActualTypeArguments();
+        // check that returned array is defensively copied
+        Assert.assertNotSame(typeArguments, archaiusType.getActualTypeArguments());
+        Assert.assertEquals(1, typeArguments.length);
+        Assert.assertEquals(Long.class, typeArguments[0]);
+    }
+
+    private static List<String> listOfString() { throw new AssertionError(); }
+    private static Set<Long> setOfLong() { throw new AssertionError(); }
+    private static Map<Integer, CharSequence> mapOfIntToCharSequence() { throw new AssertionError(); }
+    private static final ParameterizedType listOfString;
+    private static final ParameterizedType setOfLong;
+    private static final ParameterizedType mapOfIntToCharSequence;
+
+    static {
+        try {
+            listOfString = (ParameterizedType) ArchaiusTypeTest.class.getDeclaredMethod("listOfString").getGenericReturnType();
+            setOfLong = (ParameterizedType) ArchaiusTypeTest.class.getDeclaredMethod("setOfLong").getGenericReturnType();
+            mapOfIntToCharSequence = (ParameterizedType) ArchaiusTypeTest.class.getDeclaredMethod("mapOfIntToCharSequence").getGenericReturnType();
+        } catch (NoSuchMethodException exc) {
+            throw new AssertionError("Method not found", exc);
+        }
+    }
+}