Merge pull request #1182 from square/jwilson.0730.simplify

Simplify the internals of PolymorphicJsonAdapterFactory

adapters/src/main/java/com/squareup/moshi/adapters/PolymorphicJsonAdapterFactory.java

@@ -100,30 +100,22 @@ import javax.annotation.Nullable;
  *
  * <p>If an unknown subtype is encountered when decoding:
  *  <ul>
- *    <li> if {@link #withDefaultValue(Object)} is used, then {@code defaultValue} will be returned
+ *    <li>If {@link #withDefaultValue(Object)} is used, then {@code defaultValue} will be returned.
- *    <li> if {@link #withFallbackJsonAdapter(JsonAdapter)} is used, then the
+ *    <li>If {@link #withFallbackJsonAdapter(JsonAdapter)} is used, then the
- *        {@code fallbackJsonAdapter.fromJson(reader)} result will be returned
+ *        {@code fallbackJsonAdapter.fromJson(reader)} result will be returned.
- *    <li> otherwise a {@link JsonDataException} will be thrown
+ *    <li>Otherwise a {@link JsonDataException} will be thrown.
  *  </ul>
  *
  * <p>If an unknown type is encountered when encoding:
  *  <ul>
- *    <li> if {@link #withFallbackJsonAdapter(JsonAdapter)} is used, then the
+ *    <li>If {@link #withFallbackJsonAdapter(JsonAdapter)} is used, then the
- *        {@code fallbackJsonAdapter.toJson(writer, value)} result will be returned
+ *        {@code fallbackJsonAdapter.toJson(writer, value)} result will be returned.
- *    <li> otherwise a {@link IllegalArgumentException} will be thrown
+ *    <li>Otherwise a {@link IllegalArgumentException} will be thrown.
  *  </ul>
  *
  * <p>If the same subtype has multiple labels the first one is used when encoding.
  */
 public final class PolymorphicJsonAdapterFactory<T> implements JsonAdapter.Factory {
-  /**
-   * Thin wrapper around {@link JsonAdapter} to allow {@link PolymorphicJsonAdapter} to
-   * distinguish between {@code JsonAdapter} added due to a {@code defaultValue} or added
-   * by users of {@link PolymorphicJsonAdapterFactory}.
-   */
-  private abstract static class DefaultJsonAdapter<T> extends JsonAdapter<T> {
-  }
-
   final Class<T> baseType;
   final String labelKey;
   final List<String> labels;
@@ -205,19 +197,15 @@ public final class PolymorphicJsonAdapterFactory<T> implements JsonAdapter.Facto
   }
 
   private JsonAdapter<Object> buildFallbackJsonAdapter(final T defaultValue) {
-    return new DefaultJsonAdapter<Object>() {
+    return new JsonAdapter<Object>() {
-      @Nullable
+      @Override public @Nullable Object fromJson(JsonReader reader) throws IOException {
-      @Override
-      public Object fromJson(JsonReader reader) throws IOException {
         reader.skipValue();
         return defaultValue;
       }
 
-      @Override
+      @Override public void toJson(JsonWriter writer, Object value) throws IOException {
-      public void toJson(JsonWriter writer, @Nullable Object value) throws IOException {
+        throw new IllegalArgumentException("Expected one of " + subtypes + " but found " + value
-        throw new IOException("This method should never be called. "
+            + ", a " + value.getClass() + ". Register this subtype.");
-            + "If you find this on your stacktraces please report it "
-            + "to the https://github.com/square/moshi project");
       }
     };
   }
@@ -295,13 +283,8 @@ public final class PolymorphicJsonAdapterFactory<T> implements JsonAdapter.Facto
 
         int labelIndex = reader.selectString(labelOptions);
         if (labelIndex == -1 && this.fallbackJsonAdapter == null) {
-          throw new JsonDataException("Expected one of "
+          throw new JsonDataException("Expected one of " + labels + " for key '" + labelKey
-              + labels
+              + "' but found '" + reader.nextString() + "'. Register a subtype for this label.");
-              + " for key '"
-              + labelKey
-              + "' but found '"
-              + reader.nextString()
-              + "'. Register a subtype for this label.");
         }
         return labelIndex;
       }
@@ -314,17 +297,11 @@ public final class PolymorphicJsonAdapterFactory<T> implements JsonAdapter.Facto
       int labelIndex = subtypes.indexOf(type);
       final JsonAdapter<Object> adapter;
       if (labelIndex == -1) {
-        if (fallbackJsonAdapter == null || fallbackJsonAdapter instanceof DefaultJsonAdapter) {
+        if (fallbackJsonAdapter == null) {
-          throw new IllegalArgumentException("Expected one of "
+          throw new IllegalArgumentException("Expected one of " + subtypes + " but found " + value
-              + subtypes
+              + ", a " + value.getClass() + ". Register this subtype.");
-              + " but found "
-              + value
-              + ", a "
-              + value.getClass()
-              + ". Register this subtype.");
-        } else {
-          adapter = fallbackJsonAdapter;
         }
+        adapter = fallbackJsonAdapter;
       } else {
         adapter = jsonAdapters.get(labelIndex);
       }

adapters/src/test/java/com/squareup/moshi/adapters/PolymorphicJsonAdapterFactoryTest.java

@@ -23,11 +23,10 @@ import com.squareup.moshi.Moshi;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.Map;
+import javax.annotation.Nullable;
 import okio.Buffer;
 import org.junit.Test;
 
-import javax.annotation.Nullable;
-
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 
@@ -114,15 +113,18 @@ public final class PolymorphicJsonAdapterFactoryTest {
             .withSubtype(Success.class, "success")
             .withSubtype(Error.class, "error")
             .withFallbackJsonAdapter(new JsonAdapter<Object>() {
-              @Override
+              @Override public Object fromJson(JsonReader reader) throws IOException {
-              public Object fromJson(JsonReader reader) throws IOException {
+                reader.beginObject();
-                reader.skipValue();
+                assertThat(reader.nextName()).isEqualTo("type");
+                assertThat(reader.nextString()).isEqualTo("data");
+                assertThat(reader.nextName()).isEqualTo("value");
+                assertThat(reader.nextString()).isEqualTo("Okay!");
+                reader.endObject();
                 return new EmptyMessage();
               }
 
-              @Override
+              @Override public void toJson(JsonWriter writer, @Nullable Object value) {
-              public void toJson(JsonWriter writer, @Nullable Object value) {
+                throw new AssertionError();
-                throw new RuntimeException("Not implemented as not needed for the test");
               }
             })
         )
@@ -185,14 +187,11 @@ public final class PolymorphicJsonAdapterFactoryTest {
             .withSubtype(Success.class, "success")
             .withSubtype(Error.class, "error")
             .withFallbackJsonAdapter(new JsonAdapter<Object>() {
-              @Nullable
+              @Override public Object fromJson(JsonReader reader) {
-              @Override
-              public Object fromJson(JsonReader reader) {
                 throw new RuntimeException("Not implemented as not needed for the test");
               }
 
-              @Override
+              @Override public void toJson(JsonWriter writer, Object value) throws IOException {
-              public void toJson(JsonWriter writer, @Nullable Object value) throws IOException {
                 writer.name("type").value("injected by fallbackJsonAdapter");
               }
             }))