Be more aggressive about canonicalizing types.

Unfortunately we shouldn't be relying on equals() and hashCode() of the
default implementations anywhere.
This commit is contained in:
jwilson
2016-04-24 10:02:42 -04:00
parent d203cf741a
commit 13ec26a96d
4 changed files with 56 additions and 17 deletions

View File

@@ -228,7 +228,7 @@ final class AdapterMethodsFactory implements JsonAdapter.Factory {
List<AdapterMethod> adapterMethods, Type type, Set<? extends Annotation> annotations) { List<AdapterMethod> adapterMethods, Type type, Set<? extends Annotation> annotations) {
for (int i = 0, size = adapterMethods.size(); i < size; i++) { for (int i = 0, size = adapterMethods.size(); i < size; i++) {
AdapterMethod adapterMethod = adapterMethods.get(i); AdapterMethod adapterMethod = adapterMethods.get(i);
if (Types.equals(adapterMethod.type, type) && adapterMethod.annotations.equals(annotations)) { if (adapterMethod.type.equals(type) && adapterMethod.annotations.equals(annotations)) {
return adapterMethod; return adapterMethod;
} }
} }
@@ -244,7 +244,7 @@ final class AdapterMethodsFactory implements JsonAdapter.Factory {
public AdapterMethod(Type type, public AdapterMethod(Type type,
Set<? extends Annotation> annotations, Object adapter, Method method, boolean nullable) { Set<? extends Annotation> annotations, Object adapter, Method method, boolean nullable) {
this.type = type; this.type = Types.canonicalize(type);
this.annotations = annotations; this.annotations = annotations;
this.adapter = adapter; this.adapter = adapter;
this.method = method; this.method = method;

View File

@@ -22,7 +22,6 @@ import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
@@ -63,6 +62,8 @@ public final class Moshi {
@SuppressWarnings("unchecked") // Factories are required to return only matching JsonAdapters. @SuppressWarnings("unchecked") // Factories are required to return only matching JsonAdapters.
public <T> JsonAdapter<T> adapter(Type type, Set<? extends Annotation> annotations) { public <T> JsonAdapter<T> adapter(Type type, Set<? extends Annotation> annotations) {
type = Types.canonicalize(type);
// If there's an equivalent adapter in the cache, we're done! // If there's an equivalent adapter in the cache, we're done!
Object cacheKey = cacheKey(type, annotations); Object cacheKey = cacheKey(type, annotations);
synchronized (adapterCache) { synchronized (adapterCache) {
@@ -111,6 +112,8 @@ public final class Moshi {
@SuppressWarnings("unchecked") // Factories are required to return only matching JsonAdapters. @SuppressWarnings("unchecked") // Factories are required to return only matching JsonAdapters.
public <T> JsonAdapter<T> nextAdapter(JsonAdapter.Factory skipPast, Type type, public <T> JsonAdapter<T> nextAdapter(JsonAdapter.Factory skipPast, Type type,
Set<? extends Annotation> annotations) { Set<? extends Annotation> annotations) {
type = Types.canonicalize(type);
int skipPastIndex = factories.indexOf(skipPast); int skipPastIndex = factories.indexOf(skipPast);
if (skipPastIndex == -1) { if (skipPastIndex == -1) {
throw new IllegalArgumentException("Unable to skip past unknown factory " + skipPast); throw new IllegalArgumentException("Unable to skip past unknown factory " + skipPast);

View File

@@ -76,15 +76,18 @@ public final class Types {
return c.isArray() ? new GenericArrayTypeImpl(canonicalize(c.getComponentType())) : c; return c.isArray() ? new GenericArrayTypeImpl(canonicalize(c.getComponentType())) : c;
} else if (type instanceof ParameterizedType) { } else if (type instanceof ParameterizedType) {
if (type instanceof ParameterizedTypeImpl) return type;
ParameterizedType p = (ParameterizedType) type; ParameterizedType p = (ParameterizedType) type;
return new ParameterizedTypeImpl(p.getOwnerType(), return new ParameterizedTypeImpl(p.getOwnerType(),
p.getRawType(), p.getActualTypeArguments()); p.getRawType(), p.getActualTypeArguments());
} else if (type instanceof GenericArrayType) { } else if (type instanceof GenericArrayType) {
if (type instanceof GenericArrayTypeImpl) return type;
GenericArrayType g = (GenericArrayType) type; GenericArrayType g = (GenericArrayType) type;
return new GenericArrayTypeImpl(g.getGenericComponentType()); return new GenericArrayTypeImpl(g.getGenericComponentType());
} else if (type instanceof WildcardType) { } else if (type instanceof WildcardType) {
if (type instanceof WildcardTypeImpl) return type;
WildcardType w = (WildcardType) type; WildcardType w = (WildcardType) type;
return new WildcardTypeImpl(w.getUpperBounds(), w.getLowerBounds()); return new WildcardTypeImpl(w.getUpperBounds(), w.getLowerBounds());
@@ -172,7 +175,7 @@ public final class Types {
&& va.getName().equals(vb.getName()); && va.getName().equals(vb.getName());
} else { } else {
// This isn't a supported type. Could be a generic array type, wildcard type, etc. // This isn't a supported type.
return false; return false;
} }
} }

View File

@@ -369,19 +369,7 @@ public final class AdapterMethodsTest {
.build(); .build();
// This class doesn't implement equals() and hashCode() as it should. // This class doesn't implement equals() and hashCode() as it should.
ParameterizedType listOfStringType = new ParameterizedType() { ParameterizedType listOfStringType = brokenParameterizedType(0, List.class, String.class);
@Override public Type[] getActualTypeArguments() {
return new Type[] { String.class };
}
@Override public Type getRawType() {
return List.class;
}
@Override public Type getOwnerType() {
return null;
}
};
JsonAdapter<List<String>> jsonAdapter = moshi.adapter(listOfStringType); JsonAdapter<List<String>> jsonAdapter = moshi.adapter(listOfStringType);
assertThat(jsonAdapter.toJson(Arrays.asList("a", "b", "c"))).isEqualTo("\"a|b|c\""); assertThat(jsonAdapter.toJson(Arrays.asList("a", "b", "c"))).isEqualTo("\"a|b|c\"");
@@ -403,6 +391,21 @@ public final class AdapterMethodsTest {
} }
} }
/**
* Even when the types we use to look up JSON adapters are not equal, if they're equivalent they
* should return the same JsonAdapter instance.
*/
@Test public void parameterizedTypeCacheKey() throws Exception {
Moshi moshi = new Moshi.Builder().build();
Type a = brokenParameterizedType(0, List.class, String.class);
Type b = brokenParameterizedType(1, List.class, String.class);
Type c = brokenParameterizedType(2, List.class, String.class);
assertThat(moshi.adapter(b)).isSameAs(moshi.adapter(a));
assertThat(moshi.adapter(c)).isSameAs(moshi.adapter(a));
}
static class Point { static class Point {
final int x; final int x;
final int y; final int y;
@@ -424,4 +427,34 @@ public final class AdapterMethodsTest {
interface Shape { interface Shape {
String draw(); String draw();
} }
/**
* Returns a new parameterized type that doesn't implement {@link Object#equals} or {@link
* Object#hashCode} by value. These implementation defects are consistent with the parameterized
* type that shipped in some older versions of Android.
*/
ParameterizedType brokenParameterizedType(
final int hashCode, final Class<?> rawType, final Type... typeArguments) {
return new ParameterizedType() {
@Override public Type[] getActualTypeArguments() {
return typeArguments;
}
@Override public Type getRawType() {
return rawType;
}
@Override public Type getOwnerType() {
return null;
}
@Override public boolean equals(Object other) {
return other == this;
}
@Override public int hashCode() {
return hashCode;
}
};
}
} }