From 2b5301f737192f98021c2e8ead1286457ea3dd2f Mon Sep 17 00:00:00 2001 From: jwilson Date: Sat, 15 Oct 2016 10:02:04 -0400 Subject: [PATCH] Fix enclosed types with adapter methods. This was broken becase reflection was providing owner types but our API didn't have a way to specify them. Closes: https://github.com/square/moshi/issues/148 --- .../main/java/com/squareup/moshi/Types.java | 24 ++++++++---- .../squareup/moshi/AdapterMethodsTest.java | 39 +++++++++++++++++++ .../java/com/squareup/moshi/TypesTest.java | 23 ++++++----- 3 files changed, 69 insertions(+), 17 deletions(-) diff --git a/moshi/src/main/java/com/squareup/moshi/Types.java b/moshi/src/main/java/com/squareup/moshi/Types.java index e383baa..67cac08 100644 --- a/moshi/src/main/java/com/squareup/moshi/Types.java +++ b/moshi/src/main/java/com/squareup/moshi/Types.java @@ -18,7 +18,6 @@ package com.squareup.moshi; import java.lang.reflect.Array; import java.lang.reflect.GenericArrayType; import java.lang.reflect.GenericDeclaration; -import java.lang.reflect.Modifier; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; @@ -37,12 +36,22 @@ public final class Types { } /** - * Returns a new parameterized type, applying {@code typeArguments} to {@code rawType}. + * Returns a new parameterized type, applying {@code typeArguments} to {@code rawType}. Use this + * method if {@code rawType} is not enclosed in another type. */ public static ParameterizedType newParameterizedType(Type rawType, Type... typeArguments) { return new ParameterizedTypeImpl(null, rawType, typeArguments); } + /** + * Returns a new parameterized type, applying {@code typeArguments} to {@code rawType}. Use this + * method if {@code rawType} is enclosed in {@code ownerType}. + */ + public static ParameterizedType newParameterizedTypeWithOwner( + Type ownerType, Type rawType, Type... typeArguments) { + return new ParameterizedTypeImpl(ownerType, rawType, typeArguments); + } + /** Returns an array type whose elements are all instances of {@code componentType}. */ public static GenericArrayType arrayOf(Type componentType) { return new GenericArrayTypeImpl(componentType); @@ -405,12 +414,11 @@ public final class Types { final Type[] typeArguments; ParameterizedTypeImpl(Type ownerType, Type rawType, Type... typeArguments) { - // require an owner type if the raw type needs it - if (rawType instanceof Class) { - Class rawTypeAsClass = (Class) rawType; - boolean isStaticOrTopLevelClass = Modifier.isStatic(rawTypeAsClass.getModifiers()) - || rawTypeAsClass.getEnclosingClass() == null; - if (ownerType == null && !isStaticOrTopLevelClass) throw new IllegalArgumentException(); + // Require an owner type if the raw type needs it. + if (rawType instanceof Class + && (ownerType == null) != (((Class) rawType).getEnclosingClass() == null)) { + throw new IllegalArgumentException( + "unexpected owner type for " + rawType + ": " + ownerType); } this.ownerType = ownerType == null ? null : canonicalize(ownerType); diff --git a/moshi/src/test/java/com/squareup/moshi/AdapterMethodsTest.java b/moshi/src/test/java/com/squareup/moshi/AdapterMethodsTest.java index d3082ee..ee378c4 100644 --- a/moshi/src/test/java/com/squareup/moshi/AdapterMethodsTest.java +++ b/moshi/src/test/java/com/squareup/moshi/AdapterMethodsTest.java @@ -22,6 +22,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -547,6 +548,44 @@ public final class AdapterMethodsTest { } } + @Test public void adaptedTypeIsEnclosedParameterizedType() throws Exception { + Moshi moshi = new Moshi.Builder() + .add(new EnclosedParameterizedTypeJsonAdapter()) + .build(); + JsonAdapter> boxAdapter = moshi.adapter(Types.newParameterizedTypeWithOwner( + AdapterMethodsTest.class, Box.class, Point.class)); + Box box = new Box<>(new Point(5, 8)); + String json = "[{\"x\":5,\"y\":8}]"; + assertThat(boxAdapter.toJson(box)).isEqualTo(json); + assertThat(boxAdapter.fromJson(json)).isEqualTo(box); + } + + static class EnclosedParameterizedTypeJsonAdapter { + @FromJson Box boxFromJson(List points) { + return new Box<>(points.get(0)); + } + + @ToJson List boxToJson(Box box) throws Exception { + return Collections.singletonList(box.data); + } + } + + static class Box { + final T data; + + public Box(T data) { + this.data = data; + } + + @Override public boolean equals(Object o) { + return o instanceof Box && ((Box) o).data.equals(data); + } + + @Override public int hashCode() { + return data.hashCode(); + } + } + static class Point { final int x; final int y; diff --git a/moshi/src/test/java/com/squareup/moshi/TypesTest.java b/moshi/src/test/java/com/squareup/moshi/TypesTest.java index fd77e27..a41070e 100644 --- a/moshi/src/test/java/com/squareup/moshi/TypesTest.java +++ b/moshi/src/test/java/com/squareup/moshi/TypesTest.java @@ -34,27 +34,32 @@ public final class TypesTest { assertThat(getFirstTypeArgument(type)).isEqualTo(A.class); // A. A is a static inner class. - type = Types.newParameterizedType(A.class, B.class); + type = Types.newParameterizedTypeWithOwner(TypesTest.class, A.class, B.class); assertThat(getFirstTypeArgument(type)).isEqualTo(B.class); + } - final class D { - } + @Test public void parameterizedTypeWithRequiredOwnerMissing() throws Exception { try { - // D is not allowed since D is not a static inner class. - Types.newParameterizedType(D.class, A.class); + Types.newParameterizedType(A.class, B.class); fail(); } catch (IllegalArgumentException expected) { + assertThat(expected).hasMessage("unexpected owner type for " + A.class + ": null"); } + } - // A is allowed. - type = Types.newParameterizedType(A.class, D.class); - assertThat(getFirstTypeArgument(type)).isEqualTo(D.class); + @Test public void parameterizedTypeWithUnnecessaryOwnerProvided() throws Exception { + try { + Types.newParameterizedTypeWithOwner(A.class, List.class, B.class); + fail(); + } catch (IllegalArgumentException expected) { + assertThat(expected).hasMessage("unexpected owner type for " + List.class + ": " + A.class); + } } @Test public void getFirstTypeArgument() throws Exception { assertThat(getFirstTypeArgument(A.class)).isNull(); - Type type = Types.newParameterizedType(A.class, B.class, C.class); + Type type = Types.newParameterizedTypeWithOwner(TypesTest.class, A.class, B.class, C.class); assertThat(getFirstTypeArgument(type)).isEqualTo(B.class); }