Merge pull request #86 from square/jwilson_0926_ergonomics

Adapter caching, plus other ergonomic features.

moshi/src/main/java/com/squareup/moshi/AdapterMethodsFactory.java

@@ -34,7 +34,7 @@ final class AdapterMethodsFactory implements JsonAdapter.Factory {
   }
 
   @Override public JsonAdapter<?> create(
-      Type type, Set<? extends Annotation> annotations, final Moshi moshi) {
+      final Type type, final Set<? extends Annotation> annotations, final Moshi moshi) {
     final AdapterMethod toAdapter = get(toAdapters, type, annotations);
     final AdapterMethod fromAdapter = get(fromAdapters, type, annotations);
     if (toAdapter == null && fromAdapter == null) return null;
@@ -87,6 +87,10 @@ final class AdapterMethodsFactory implements JsonAdapter.Factory {
           }
         }
       }
+
+      @Override public String toString() {
+        return "JsonAdapter" + annotations + "(" + type + ")";
+      }
     };
   }
 

moshi/src/main/java/com/squareup/moshi/ClassFactory.java

@@ -44,6 +44,9 @@ abstract class ClassFactory<T> {
           Object[] args = null;
           return (T) constructor.newInstance(args);
         }
+        @Override public String toString() {
+          return rawType.getName();
+        }
       };
     } catch (NoSuchMethodException ignored) {
       // No no-args constructor. Fall back to something more magical...
@@ -64,6 +67,9 @@ abstract class ClassFactory<T> {
         @Override public T newInstance() throws InvocationTargetException, IllegalAccessException {
           return (T) allocateInstance.invoke(unsafe, rawType);
         }
+        @Override public String toString() {
+          return rawType.getName();
+        }
       };
     } catch (IllegalAccessException e) {
       throw new AssertionError();
@@ -89,6 +95,9 @@ abstract class ClassFactory<T> {
         @Override public T newInstance() throws InvocationTargetException, IllegalAccessException {
           return (T) newInstance.invoke(null, rawType, constructorId);
         }
+        @Override public String toString() {
+          return rawType.getName();
+        }
       };
     } catch (IllegalAccessException e) {
       throw new AssertionError();

moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.java

@@ -166,6 +166,10 @@ final class ClassJsonAdapter<T> extends JsonAdapter<T> {
     }
   }
 
+  @Override public String toString() {
+    return "JsonAdapter(" + classFactory + ")";
+  }
+
   static class FieldBinding<T> {
     private final Field field;
     private final JsonAdapter<T> adapter;

moshi/src/main/java/com/squareup/moshi/CollectionJsonAdapter.java

@@ -85,4 +85,8 @@ abstract class CollectionJsonAdapter<C extends Collection<T>, T> extends JsonAda
     }
     writer.endArray();
   }
+
+  @Override public String toString() {
+    return elementAdapter + ".collection()";
+  }
 }

moshi/src/main/java/com/squareup/moshi/JsonAdapter.java

@@ -75,6 +75,9 @@ public abstract class JsonAdapter<T> {
           delegate.toJson(writer, value);
         }
       }
+      @Override public String toString() {
+        return delegate + ".nullSafe()";
+      }
     };
   }
 
@@ -100,6 +103,9 @@ public abstract class JsonAdapter<T> {
           writer.setLenient(lenient);
         }
       }
+      @Override public String toString() {
+        return delegate + ".lenient()";
+      }
     };
   }
 
@@ -124,6 +130,9 @@ public abstract class JsonAdapter<T> {
       @Override public void toJson(JsonWriter writer, T value) throws IOException {
         delegate.toJson(writer, value);
       }
+      @Override public String toString() {
+        return delegate + ".failOnUnknown()";
+      }
     };
   }
 

moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java

@@ -75,4 +75,8 @@ final class MapJsonAdapter<K, V> extends JsonAdapter<Map<K, V>> {
     reader.endObject();
     return result;
   }
+
+  @Override public String toString() {
+    return "JsonAdapter(" + keyAdapter + "=" + valueAdapter + ")";
+  }
 }

moshi/src/main/java/com/squareup/moshi/Moshi.java

@@ -19,8 +19,11 @@ import java.io.IOException;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -29,6 +32,7 @@ import java.util.Set;
 public final class Moshi {
   private final List<JsonAdapter.Factory> factories;
   private final ThreadLocal<List<DeferredAdapter<?>>> reentrantCalls = new ThreadLocal<>();
+  private final Map<Object, JsonAdapter<?>> adapterCache = new LinkedHashMap<>();
 
   private Moshi(Builder builder) {
     List<JsonAdapter.Factory> factories = new ArrayList<>();
@@ -47,52 +51,77 @@ public final class Moshi {
   }
 
   public <T> JsonAdapter<T> adapter(Class<T> type) {
-    // TODO: cache created JSON adapters.
     return adapter(type, Util.NO_ANNOTATIONS);
   }
 
-  public <T> JsonAdapter<T> adapter(Type type, Set<? extends Annotation> annotations) {
-    return createAdapter(0, type, annotations);
-  }
-
-  public <T> JsonAdapter<T> nextAdapter(JsonAdapter.Factory skipPast, Type type,
-      Set<? extends Annotation> annotations) {
-    return createAdapter(factories.indexOf(skipPast) + 1, type, annotations);
-  }
-
   @SuppressWarnings("unchecked") // Factories are required to return only matching JsonAdapters.
-  private <T> JsonAdapter<T> createAdapter(
-      int firstIndex, Type type, Set<? extends Annotation> annotations) {
+  public <T> JsonAdapter<T> adapter(Type type, Set<? extends Annotation> annotations) {
+    // If there's an equivalent adapter in the cache, we're done!
+    Object cacheKey = cacheKey(type, annotations);
+    synchronized (adapterCache) {
+      JsonAdapter<?> result = adapterCache.get(cacheKey);
+      if (result != null) return (JsonAdapter<T>) result;
+    }
+
+    // Short-circuit if this is a reentrant call.
     List<DeferredAdapter<?>> deferredAdapters = reentrantCalls.get();
-    if (deferredAdapters == null) {
-      deferredAdapters = new ArrayList<>();
-      reentrantCalls.set(deferredAdapters);
-    } else if (firstIndex == 0) {
-      // If this is a regular adapter lookup, check that this isn't a reentrant call.
-      for (DeferredAdapter<?> deferredAdapter : deferredAdapters) {
-        if (deferredAdapter.type.equals(type) && deferredAdapter.annotations.equals(annotations)) {
+    if (deferredAdapters != null) {
+      for (int i = 0, size = deferredAdapters.size(); i < size; i++) {
+        DeferredAdapter<?> deferredAdapter = deferredAdapters.get(i);
+        if (deferredAdapter.cacheKey.equals(cacheKey)) {
           return (JsonAdapter<T>) deferredAdapter;
         }
       }
+    } else {
+      deferredAdapters = new ArrayList<>();
+      reentrantCalls.set(deferredAdapters);
     }
 
-    DeferredAdapter<T> deferredAdapter = new DeferredAdapter<>(type, annotations);
+    // Prepare for re-entrant calls, then ask each factory to create a type adapter.
+    DeferredAdapter<T> deferredAdapter = new DeferredAdapter<>(cacheKey);
     deferredAdapters.add(deferredAdapter);
     try {
-      for (int i = firstIndex, size = factories.size(); i < size; i++) {
+      for (int i = 0, size = factories.size(); i < size; i++) {
         JsonAdapter<T> result = (JsonAdapter<T>) factories.get(i).create(type, annotations, this);
         if (result != null) {
           deferredAdapter.ready(result);
+          synchronized (adapterCache) {
+            adapterCache.put(cacheKey, result);
+          }
           return result;
         }
       }
     } finally {
       deferredAdapters.remove(deferredAdapters.size() - 1);
+      if (deferredAdapters.isEmpty()) {
+        reentrantCalls.remove();
+      }
     }
 
     throw new IllegalArgumentException("No JsonAdapter for " + type + " annotated " + annotations);
   }
 
+  @SuppressWarnings("unchecked") // Factories are required to return only matching JsonAdapters.
+  public <T> JsonAdapter<T> nextAdapter(JsonAdapter.Factory skipPast, Type type,
+      Set<? extends Annotation> annotations) {
+    int skipPastIndex = factories.indexOf(skipPast);
+    if (skipPastIndex == -1) {
+      throw new IllegalArgumentException("Unable to skip past unknown factory " + skipPast);
+    }
+    for (int i = skipPastIndex + 1, size = factories.size(); i < size; i++) {
+      JsonAdapter<T> result = (JsonAdapter<T>) factories.get(i).create(type, annotations, this);
+      if (result != null) return result;
+    }
+    throw new IllegalArgumentException("No next JsonAdapter for "
+        + type + " annotated " + annotations);
+  }
+
+  /** Returns an opaque object that's equal if the type and annotations are equal. */
+  private Object cacheKey(Type type, Set<? extends Annotation> annotations) {
+    if (annotations.isEmpty()) return type;
+    return Arrays.asList(type, annotations);
+  }
+
   public static final class Builder {
     private final List<JsonAdapter.Factory> factories = new ArrayList<>();
 
@@ -116,22 +145,24 @@ public final class Moshi {
       if (!annotation.isAnnotationPresent(JsonQualifier.class)) {
         throw new IllegalArgumentException(annotation + " does not have @JsonQualifier");
       }
+      if (annotation.getDeclaredMethods().length > 0) {
+        throw new IllegalArgumentException("Use JsonAdapter.Factory for annotations with elements");
+      }
 
       return add(new JsonAdapter.Factory() {
         @Override public JsonAdapter<?> create(
             Type targetType, Set<? extends Annotation> annotations, Moshi moshi) {
-          if (!Util.typesMatch(type, targetType)) return null;
-
-          // TODO: check for an annotations exact match.
-          if (!Util.isAnnotationPresent(annotations, annotation)) return null;
-
-          return jsonAdapter;
+          if (Util.typesMatch(type, targetType)
+              && annotations.size() == 1
+              && Util.isAnnotationPresent(annotations, annotation)) {
+            return jsonAdapter;
+          }
+          return null;
         }
       });
     }
 
     public Builder add(JsonAdapter.Factory jsonAdapter) {
-      // TODO: define precedence order. Last added wins? First added wins?
       factories.add(jsonAdapter);
       return this;
     }
@@ -154,21 +185,16 @@ public final class Moshi {
    * class that has a {@code List<Employee>} field for an organization's management hierarchy.
    */
   private static class DeferredAdapter<T> extends JsonAdapter<T> {
-    private Type type;
-    private Set<? extends Annotation> annotations;
+    private Object cacheKey;
     private JsonAdapter<T> delegate;
 
-    public DeferredAdapter(Type type, Set<? extends Annotation> annotations) {
-      this.type = type;
-      this.annotations = annotations;
+    public DeferredAdapter(Object cacheKey) {
+      this.cacheKey = cacheKey;
     }
 
     public void ready(JsonAdapter<T> delegate) {
       this.delegate = delegate;
-
-      // Null out the type and annotations so they can be garbage collected.
-      this.type = null;
-      this.annotations = null;
+      this.cacheKey = null;
     }
 
     @Override public T fromJson(JsonReader reader) throws IOException {

moshi/src/main/java/com/squareup/moshi/StandardJsonAdapters.java

@@ -78,15 +78,23 @@ final class StandardJsonAdapters {
     @Override public void toJson(JsonWriter writer, Boolean value) throws IOException {
       writer.value(value);
     }
+
+    @Override public String toString() {
+      return "JsonAdapter(Boolean)";
+    }
   };
 
   static final JsonAdapter<Byte> BYTE_JSON_ADAPTER = new JsonAdapter<Byte>() {
     @Override public Byte fromJson(JsonReader reader) throws IOException {
-      return (byte) rangeCheckNextInt(reader, "a byte", Byte.MIN_VALUE, 0xFF);
+      return (byte) rangeCheckNextInt(reader, "a byte", Byte.MIN_VALUE, 0xff);
     }
 
     @Override public void toJson(JsonWriter writer, Byte value) throws IOException {
-      writer.value(value.intValue() & 0xFF);
+      writer.value(value.intValue() & 0xff);
+    }
+
+    @Override public String toString() {
+      return "JsonAdapter(Byte)";
     }
   };
 
@@ -103,6 +111,10 @@ final class StandardJsonAdapters {
     @Override public void toJson(JsonWriter writer, Character value) throws IOException {
       writer.value(value.toString());
     }
+
+    @Override public String toString() {
+      return "JsonAdapter(Character)";
+    }
   };
 
   static final JsonAdapter<Double> DOUBLE_JSON_ADAPTER = new JsonAdapter<Double>() {
@@ -113,6 +125,10 @@ final class StandardJsonAdapters {
     @Override public void toJson(JsonWriter writer, Double value) throws IOException {
       writer.value(value.doubleValue());
     }
+
+    @Override public String toString() {
+      return "JsonAdapter(Double)";
+    }
   };
 
   static final JsonAdapter<Float> FLOAT_JSON_ADAPTER = new JsonAdapter<Float>() {
@@ -134,6 +150,10 @@ final class StandardJsonAdapters {
       // Use the Number overload so we write out float precision instead of double precision.
       writer.value(value);
     }
+
+    @Override public String toString() {
+      return "JsonAdapter(Float)";
+    }
   };
 
   static final JsonAdapter<Integer> INTEGER_JSON_ADAPTER = new JsonAdapter<Integer>() {
@@ -144,6 +164,10 @@ final class StandardJsonAdapters {
     @Override public void toJson(JsonWriter writer, Integer value) throws IOException {
       writer.value(value.intValue());
     }
+
+    @Override public String toString() {
+      return "JsonAdapter(Integer)";
+    }
   };
 
   static final JsonAdapter<Long> LONG_JSON_ADAPTER = new JsonAdapter<Long>() {
@@ -154,6 +178,10 @@ final class StandardJsonAdapters {
     @Override public void toJson(JsonWriter writer, Long value) throws IOException {
       writer.value(value.longValue());
     }
+
+    @Override public String toString() {
+      return "JsonAdapter(Long)";
+    }
   };
 
   static final JsonAdapter<Short> SHORT_JSON_ADAPTER = new JsonAdapter<Short>() {
@@ -164,6 +192,10 @@ final class StandardJsonAdapters {
     @Override public void toJson(JsonWriter writer, Short value) throws IOException {
       writer.value(value.intValue());
     }
+
+    @Override public String toString() {
+      return "JsonAdapter(Short)";
+    }
   };
 
   static final JsonAdapter<String> STRING_JSON_ADAPTER = new JsonAdapter<String>() {
@@ -174,6 +206,10 @@ final class StandardJsonAdapters {
     @Override public void toJson(JsonWriter writer, String value) throws IOException {
       writer.value(value);
     }
+
+    @Override public String toString() {
+      return "JsonAdapter(String)";
+    }
   };
 
   static <T extends Enum<T>> JsonAdapter<T> enumAdapter(final Class<T> enumType) {
@@ -192,6 +228,10 @@ final class StandardJsonAdapters {
       @Override public void toJson(JsonWriter writer, T value) throws IOException {
         writer.value(value.name());
       }
+
+      @Override public String toString() {
+        return "JsonAdapter(" + enumType.getName() + ")";
+      }
     };
   }
 
@@ -271,5 +311,9 @@ final class StandardJsonAdapters {
       if (Collection.class.isAssignableFrom(valueClass)) return Collection.class;
       return valueClass;
     }
+
+    @Override public String toString() {
+      return "JsonAdapter(Object)";
+    }
   }
 }

moshi/src/test/java/com/squareup/moshi/MoshiTest.java

@@ -735,6 +735,70 @@ public final class MoshiTest {
     }
   }
 
+  @Test public void qualifierWithElementsMayNotBeDirectlyRegistered() throws IOException {
+    try {
+      new Moshi.Builder()
+          .add(Boolean.class, Localized.class, StandardJsonAdapters.BOOLEAN_JSON_ADAPTER);
+      fail();
+    } catch (IllegalArgumentException expected) {
+      assertThat(expected).hasMessage("Use JsonAdapter.Factory for annotations with elements");
+    }
+  }
+
+  @Test public void qualifierWithElements() throws IOException {
+    Moshi moshi = new Moshi.Builder()
+        .add(LocalizedBooleanAdapter.FACTORY)
+        .build();
+
+    Baguette baguette = new Baguette();
+    baguette.avecBeurre = true;
+    baguette.withButter = true;
+
+    JsonAdapter<Baguette> adapter = moshi.adapter(Baguette.class);
+    assertThat(adapter.toJson(baguette))
+        .isEqualTo("{\"avecBeurre\":\"oui\",\"withButter\":\"yes\"}");
+
+    Baguette decoded = adapter.fromJson("{\"avecBeurre\":\"oui\",\"withButter\":\"yes\"}");
+    assertThat(decoded.avecBeurre).isTrue();
+    assertThat(decoded.withButter).isTrue();
+  }
+
+  /** Note that this is the opposite of Gson's behavior, where later adapters are preferred. */
+  @Test public void adaptersRegisteredInOrderOfPrecedence() throws Exception {
+    JsonAdapter<String> adapter1 = new JsonAdapter<String>() {
+      @Override public String fromJson(JsonReader reader) throws IOException {
+        throw new AssertionError();
+      }
+      @Override public void toJson(JsonWriter writer, String value) throws IOException {
+        writer.value("one!");
+      }
+    };
+
+    JsonAdapter<String> adapter2 = new JsonAdapter<String>() {
+      @Override public String fromJson(JsonReader reader) throws IOException {
+        throw new AssertionError();
+      }
+      @Override public void toJson(JsonWriter writer, String value) throws IOException {
+        writer.value("two!");
+      }
+    };
+
+    Moshi moshi = new Moshi.Builder()
+        .add(String.class, adapter1)
+        .add(String.class, adapter2)
+        .build();
+    JsonAdapter<String> adapter = moshi.adapter(String.class).lenient();
+    assertThat(adapter.toJson("a")).isEqualTo("\"one!\"");
+  }
+
+  @Test public void cachingJsonAdapters() throws Exception {
+    Moshi moshi = new Moshi.Builder().build();
+
+    JsonAdapter<MealDeal> adapter1 = moshi.adapter(MealDeal.class);
+    JsonAdapter<MealDeal> adapter2 = moshi.adapter(MealDeal.class);
+    assertThat(adapter1).isSameAs(adapter2);
+  }
+
   static class Pizza {
     final int diameter;
     final boolean extraCheese;
@@ -858,4 +922,52 @@ public final class MoshiTest {
     PAPER,
     SCISSORS
   }
+
+  @Retention(RUNTIME)
+  @JsonQualifier
+  @interface Localized {
+    String value();
+  }
+
+  static class Baguette {
+    @Localized("en") boolean withButter;
+    @Localized("fr") boolean avecBeurre;
+  }
+
+  static class LocalizedBooleanAdapter extends JsonAdapter<Boolean> {
+    private static final JsonAdapter.Factory FACTORY = new JsonAdapter.Factory() {
+      @Override public JsonAdapter<?> create(
+          Type type, Set<? extends Annotation> annotations, Moshi moshi) {
+        if (type == boolean.class) {
+          for (Annotation annotation : annotations) {
+            if (annotation instanceof Localized) {
+              return new LocalizedBooleanAdapter(((Localized) annotation).value());
+            }
+          }
+        }
+        return null;
+      }
+    };
+
+    private final String trueString;
+    private final String falseString;
+
+    public LocalizedBooleanAdapter(String language) {
+      if (language.equals("fr")) {
+        trueString = "oui";
+        falseString = "non";
+      } else {
+        trueString = "yes";
+        falseString = "no";
+      }
+    }
+
+    @Override public Boolean fromJson(JsonReader reader) throws IOException {
+      return reader.nextString().equals(trueString);
+    }
+
+    @Override public void toJson(JsonWriter writer, Boolean value) throws IOException {
+      writer.value(value ? trueString : falseString);
+    }
+  }
 }