JCLOUDS-750 Revert 5b6f1e929e in favor of tighter contract on @SerializedNames.

core/src/main/java/org/jclouds/json/SerializedNames.java

@@ -16,7 +16,6 @@
  */
 package org.jclouds.json;
 
-import static java.lang.annotation.ElementType.CONSTRUCTOR;
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
@@ -25,9 +24,31 @@ import java.lang.annotation.Target;
 
 import com.google.common.annotations.Beta;
 
-/** An ordered list of json field names that correspond to factory method or constructor parameters. */
+/**
-@Beta @Target({ CONSTRUCTOR, METHOD }) @Retention(RUNTIME)
+ * This annotation identifies the canonical factory method on an {@code AutoValue} type used for json.
+ * It also dictates the serialized naming convention of the fields. This is required as there's currently
+ * no way to add annotations to the fields generated by {@code AutoValue}.
+ *
+ * <p/>Example:
+ * <pre>{@code @AutoValue class Resource {
+ *   abstract String id();
+ *   @Nullable abstract Map<String, String> metadata();
+ *
+ *   @AutoValueSerializedNames({ "Id", "Metadata" }) // Note case format is controlled here!
+ *   static Resource create(String id, Map<String, String> metadata) {
+ *     return new AutoValue_Resource(id, metadata);
+ *   }
+ * }}</pre>
+ */
+@Beta @Target(METHOD) @Retention(RUNTIME)
 public @interface SerializedNames {
 
+   /**
+    * Ordered values that dictate the naming convention for serialization.
+    *
+    * <h3>Note</h3>
+    * The order of these names must exactly match the factory method parameters and also match the order of the
+    * auto-value constructor parameters.
+    */
    String[] value();
 }

core/src/main/java/org/jclouds/json/config/GsonModule.java

@@ -65,7 +65,6 @@ import com.google.common.collect.Sets;
 import com.google.common.primitives.Bytes;
 import com.google.gson.ExclusionStrategy;
 import com.google.gson.FieldAttributes;
-import com.google.gson.FieldNamingPolicy;
 import com.google.gson.FieldNamingStrategy;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
@@ -85,12 +84,6 @@ import com.google.inject.Provides;
 
 public class GsonModule extends AbstractModule {
 
-   /** Optionally override the fallback field naming strategy when name annotations aren't on fields. */
-   private static class FallbackFieldNamingStrategy {
-      @Inject(optional = true)
-      FieldNamingStrategy fallback = FieldNamingPolicy.IDENTITY;
-   }
-
    @SuppressWarnings("rawtypes")
    @Provides
    @Singleton
@@ -101,11 +94,10 @@ public class GsonModule extends AbstractModule {
          MultimapTypeAdapterFactory multimap, IterableTypeAdapterFactory iterable,
          CollectionTypeAdapterFactory collection, ListTypeAdapterFactory list,
          ImmutableListTypeAdapterFactory immutableList, FluentIterableTypeAdapterFactory fluentIterable,
-         ImmutableMapTypeAdapterFactory immutableMap, DefaultExclusionStrategy exclusionStrategy,
+         ImmutableMapTypeAdapterFactory immutableMap, DefaultExclusionStrategy exclusionStrategy) {
-         FallbackFieldNamingStrategy fallbackFieldNamingStrategy) {
 
       FieldNamingStrategy serializationPolicy = new AnnotationOrNameFieldNamingStrategy(ImmutableSet.of(
-            new ExtractSerializedName(), new ExtractNamed()), fallbackFieldNamingStrategy.fallback);
+            new ExtractSerializedName(), new ExtractNamed()));
 
       GsonBuilder builder = new GsonBuilder().setFieldNamingStrategy(serializationPolicy)
                                              .setExclusionStrategies(exclusionStrategy);
@@ -135,7 +127,8 @@ public class GsonModule extends AbstractModule {
             ImmutableSet.of(new ExtractNamed()));
 
       builder.registerTypeAdapterFactory(new DeserializationConstructorAndReflectiveTypeAdapterFactory(
-            new ConstructorConstructor(ImmutableMap.<Type, InstanceCreator<?>>of()), serializationPolicy, Excluder.DEFAULT, deserializationPolicy));
+            new ConstructorConstructor(ImmutableMap.<Type, InstanceCreator<?>>of()), serializationPolicy,
+            Excluder.DEFAULT, deserializationPolicy));
 
       // complicated (serializers/deserializers as they need context to operate)
       builder.registerTypeHierarchyAdapter(Enum.class, new EnumTypeAdapterThatReturnsFromValue());

core/src/main/java/org/jclouds/json/internal/NamingStrategies.java

@@ -18,6 +18,7 @@ package org.jclouds.json.internal;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Predicates.in;
 import static com.google.common.collect.Iterables.transform;
 import static com.google.common.collect.Iterables.tryFind;
@@ -26,6 +27,7 @@ import static org.jclouds.reflect.Reflection2.constructors;
 import java.beans.ConstructorProperties;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Map;
@@ -162,33 +164,47 @@ public class NamingStrategies {
    public static class AnnotationFieldNamingStrategy extends AnnotationBasedNamingStrategy implements
          FieldNamingStrategy {
 
-      private final FieldNamingStrategy fallback;
+      public AnnotationFieldNamingStrategy(Iterable<? extends NameExtractor<?>> extractors) {
-
-      public AnnotationFieldNamingStrategy(Iterable<? extends NameExtractor<?>> extractors,
-            FieldNamingStrategy fallback) {
          super(extractors);
          checkArgument(extractors.iterator().hasNext(), "you must supply at least one name extractor, for example: "
                + ExtractSerializedName.class.getSimpleName());
-         this.fallback = checkNotNull(fallback, "fallback fieldNamingPolicy");
       }
 
       @Override
       public String translateName(Field f) {
+         // Determining if AutoValue is tough, since annotations are SOURCE retention.
+         if (Modifier.isAbstract(f.getDeclaringClass().getSuperclass().getModifiers())) { // AutoValue means abstract.
+            for (Invokable<?, ?> target : constructors(TypeToken.of(f.getDeclaringClass().getSuperclass()))) {
+               SerializedNames names = target.getAnnotation(SerializedNames.class);
+               if (names != null && target.isStatic()) { // == factory method
+                  // Fields and constructor params are written by AutoValue in same order as methods are declared.
+                  // By contract, SerializedNames factory methods must declare its names in the same order,
+                  Field[] fields = f.getDeclaringClass().getDeclaredFields();
+                  checkState(fields.length == names.value().length, "Incorrect number of names on " + names);
+                  for (int i = 0; i < fields.length; i++) {
+                     if (fields[i].equals(f)) {
+                        return names.value()[i];
+                     }
+                  }
+                  // The input field was not a declared field. Accidentally placed on something not AutoValue?
+                  throw new IllegalStateException("Inconsistent state. Ensure type is AutoValue on " + target);
+               }
+            }
+         }
          for (Annotation annotation : f.getAnnotations()) {
             if (annotationToNameExtractor.containsKey(annotation.annotationType())) {
                return annotationToNameExtractor.get(annotation.annotationType()).apply(annotation);
             }
          }
-         return fallback.translateName(f);
+         return null;
       }
    }
 
    public static class AnnotationOrNameFieldNamingStrategy extends AnnotationFieldNamingStrategy implements
          FieldNamingStrategy {
 
-      public AnnotationOrNameFieldNamingStrategy(Iterable<? extends NameExtractor<?>> extractors,
+      public AnnotationOrNameFieldNamingStrategy(Iterable<? extends NameExtractor<?>> extractors) {
-            FieldNamingStrategy fallback) {
+         super(extractors);
-         super(extractors, fallback);
       }
 
       @Override

core/src/main/java/org/jclouds/reflect/Reflection2.java

@@ -89,7 +89,7 @@ public class Reflection2 {
    }
 
    /**
-    * return all constructors present in the class as {@link Invokable}s.
+    * return all constructors or static factory methods present in the class as {@link Invokable}s.
     * 
     * @param ownerType
     *           corresponds to {@link Invokable#getOwnerType()}

core/src/test/java/org/jclouds/json/JsonTest.java

@@ -38,8 +38,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.gson.FieldAttributes;
-import com.google.gson.FieldNamingPolicy;
-import com.google.gson.FieldNamingStrategy;
 import com.google.gson.Gson;
 import com.google.gson.TypeAdapter;
 import com.google.gson.TypeAdapterFactory;
@@ -226,105 +224,110 @@ public class JsonTest {
    }
 
    @AutoValue
-   abstract static class UpperCamelCasedType {
+   abstract static class SerializedNamesType {
       abstract String id();
 
       @Nullable abstract Map<String, String> volumes();
 
-      // Currently, this only works for deserialization. Need to set a naming policy for serialization!
       @SerializedNames({ "Id", "Volumes" })
-      private static UpperCamelCasedType create(String id, Map<String, String> volumes) {
+      private static SerializedNamesType create(String id, Map<String, String> volumes) {
-         return new AutoValue_JsonTest_UpperCamelCasedType(id, volumes);
+         return new AutoValue_JsonTest_SerializedNamesType(id, volumes);
       }
    }
 
-   public void upperCamelCaseWithSerializedNames() {
+   public void autoValueSerializedNames() {
-      Json json = Guice.createInjector(new GsonModule(), new AbstractModule() {
+      Json json = Guice.createInjector(new GsonModule()).getInstance(Json.class);
-         @Override protected void configure() {
-            bind(FieldNamingStrategy.class).toInstance(FieldNamingPolicy.UPPER_CAMEL_CASE);
-         }
-      }).getInstance(Json.class);
 
-      UpperCamelCasedType resource = UpperCamelCasedType.create("1234", Collections.<String, String>emptyMap());
+      SerializedNamesType resource = SerializedNamesType.create("1234", Collections.<String, String>emptyMap());
       String spinalJson = "{\"Id\":\"1234\",\"Volumes\":{}}";
 
       assertEquals(json.toJson(resource), spinalJson);
-      assertEquals(json.fromJson(spinalJson, UpperCamelCasedType.class), resource);
+      assertEquals(json.fromJson(spinalJson, SerializedNamesType.class), resource);
-   }
-
-   public void upperCamelCaseWithSerializedNamesNullJsonValue() {
-      Json json = Guice.createInjector(new GsonModule(), new AbstractModule() {
-         @Override protected void configure() {
-            bind(FieldNamingStrategy.class).toInstance(FieldNamingPolicy.UPPER_CAMEL_CASE);
-         }
-      }).getInstance(Json.class);
-
-      assertEquals(json.fromJson("{\"Id\":\"1234\",\"Volumes\":null}", UpperCamelCasedType.class),
-            UpperCamelCasedType.create("1234", null));
    }
 
    @AutoValue
-   abstract static class NestedCamelCasedType {
+   abstract static class SerializedNamesTooFewType {
-      abstract UpperCamelCasedType item();
+      abstract String id();
 
-      abstract List<UpperCamelCasedType> items();
+      @Nullable abstract Map<String, String> volumes();
+
+      @SerializedNames("Id") // TODO: check things like this with error-prone, not runtime!
+      private static SerializedNamesTooFewType create(String id, Map<String, String> volumes) {
+         return new AutoValue_JsonTest_SerializedNamesTooFewType(id, volumes);
+      }
+   }
+
+   /** Common problem. Someone adds a parameter, but forgets to add it to the names list. */
+   @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Incorrect number .*")
+   public void autoValueSerializedNames_tooFew() {
+      Json json = Guice.createInjector(new GsonModule()).getInstance(Json.class);
+      json.toJson(SerializedNamesTooFewType.create("1234", null));
+   }
+
+   public void autoValueSerializedNames_nullValueInJson() {
+      Json json = Guice.createInjector(new GsonModule()).getInstance(Json.class);
+
+      assertEquals(json.fromJson("{\"Id\":\"1234\",\"Volumes\":null}", SerializedNamesType.class),
+            SerializedNamesType.create("1234", null));
+   }
+
+   @AutoValue
+   abstract static class NestedSerializedNamesType {
+      abstract SerializedNamesType item();
+
+      abstract List<SerializedNamesType> items();
 
-      // Currently, this only works for deserialization. Need to set a naming policy for serialization!
       @SerializedNames({ "Item", "Items" })
-      private static NestedCamelCasedType create(UpperCamelCasedType item, List<UpperCamelCasedType> items) {
+      private static NestedSerializedNamesType create(SerializedNamesType item, List<SerializedNamesType> items) {
-         return new AutoValue_JsonTest_NestedCamelCasedType(item, items);
+         return new AutoValue_JsonTest_NestedSerializedNamesType(item, items);
       }
    }
 
-   private final NestedCamelCasedType nested = NestedCamelCasedType
+   private final NestedSerializedNamesType nested = NestedSerializedNamesType
-         .create(UpperCamelCasedType.create("1234", Collections.<String, String>emptyMap()),
+         .create(SerializedNamesType.create("1234", Collections.<String, String>emptyMap()),
-               Arrays.asList(UpperCamelCasedType.create("5678", ImmutableMap.of("Foo", "Bar"))));
+               Arrays.asList(SerializedNamesType.create("5678", ImmutableMap.of("Foo", "Bar"))));
 
-   public void nestedCamelCasedType() {
+   public void autoValueSerializedNames_nestedType() {
-      Json json = Guice.createInjector(new GsonModule(), new AbstractModule() {
+      Json json = Guice.createInjector(new GsonModule()).getInstance(Json.class);
-         @Override protected void configure() {
-            bind(FieldNamingStrategy.class).toInstance(FieldNamingPolicy.UPPER_CAMEL_CASE);
-         }
-      }).getInstance(Json.class);
 
       String spinalJson = "{\"Item\":{\"Id\":\"1234\",\"Volumes\":{}},\"Items\":[{\"Id\":\"5678\",\"Volumes\":{\"Foo\":\"Bar\"}}]}";
 
       assertEquals(json.toJson(nested), spinalJson);
-      assertEquals(json.fromJson(spinalJson, NestedCamelCasedType.class), nested);
+      assertEquals(json.fromJson(spinalJson, NestedSerializedNamesType.class), nested);
    }
 
-   public void nestedCamelCasedTypeOverriddenTypeAdapterFactory() {
+   public void autoValueSerializedNames_overriddenTypeAdapterFactory() {
       Json json = Guice.createInjector(new GsonModule(), new AbstractModule() {
          @Override protected void configure() {
          }
 
          @Provides public Set<TypeAdapterFactory> typeAdapterFactories() {
-            return ImmutableSet.<TypeAdapterFactory>of(new NestedCamelCasedTypeAdapterFactory());
+            return ImmutableSet.<TypeAdapterFactory>of(new NestedSerializedNamesTypeAdapterFactory());
          }
       }).getInstance(Json.class);
 
       assertEquals(json.toJson(nested), "{\"id\":\"1234\",\"count\":1}");
-      assertEquals(json.fromJson("{\"id\":\"1234\",\"count\":1}", NestedCamelCasedType.class), nested);
+      assertEquals(json.fromJson("{\"id\":\"1234\",\"count\":1}", NestedSerializedNamesType.class), nested);
    }
 
-   private class NestedCamelCasedTypeAdapterFactory extends TypeAdapter<NestedCamelCasedType>
+   private class NestedSerializedNamesTypeAdapterFactory extends TypeAdapter<NestedSerializedNamesType>
          implements TypeAdapterFactory {
 
       @Override public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
-         if (!(NestedCamelCasedType.class.isAssignableFrom(typeToken.getRawType()))) {
+         if (!(NestedSerializedNamesType.class.isAssignableFrom(typeToken.getRawType()))) {
             return null;
          }
          return (TypeAdapter<T>) this;
       }
 
-      @Override public void write(JsonWriter out, NestedCamelCasedType value) throws IOException {
+      @Override public void write(JsonWriter out, NestedSerializedNamesType value) throws IOException {
          out.beginObject();
          out.name("id").value(value.item().id());
          out.name("count").value(value.items().size());
          out.endObject();
       }
 
-      @Override public NestedCamelCasedType read(JsonReader in) throws IOException {
+      @Override public NestedSerializedNamesType read(JsonReader in) throws IOException {
          in.beginObject();
          in.nextName();
          in.nextString();

core/src/test/java/org/jclouds/json/internal/DeserializationConstructorAndReflectiveTypeAdapterFactoryTest.java

@@ -43,7 +43,6 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import com.google.gson.FieldNamingPolicy;
 import com.google.gson.FieldNamingStrategy;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
@@ -63,7 +62,7 @@ public final class DeserializationConstructorAndReflectiveTypeAdapterFactoryTest
 
    static DeserializationConstructorAndReflectiveTypeAdapterFactory parameterizedCtorFactory() {
       FieldNamingStrategy serializationPolicy = new AnnotationOrNameFieldNamingStrategy(ImmutableSet.of(
-            new ExtractSerializedName(), new ExtractNamed()), FieldNamingPolicy.IDENTITY);
+            new ExtractSerializedName(), new ExtractNamed()));
       AnnotationConstructorNamingStrategy deserializationPolicy = new AnnotationConstructorNamingStrategy(
             ImmutableSet.of(ConstructorProperties.class, SerializedNames.class, Inject.class),
             ImmutableSet.of(new ExtractNamed()));

core/src/test/java/org/jclouds/json/internal/NamingStrategiesTest.java

@@ -36,7 +36,6 @@ import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.reflect.Invokable;
-import com.google.gson.FieldNamingPolicy;
 import com.google.gson.FieldNamingStrategy;
 import com.google.gson.annotations.SerializedName;
 
@@ -101,18 +100,16 @@ public final class NamingStrategiesTest {
    }
    
    public void testAnnotationFieldNamingStrategy() throws Exception {
-      FieldNamingStrategy strategy = new AnnotationFieldNamingStrategy(ImmutableSet.of(new ExtractNamed()),
+      FieldNamingStrategy strategy = new AnnotationFieldNamingStrategy(ImmutableSet.of(new ExtractNamed()));
-            FieldNamingPolicy.UPPER_CAMEL_CASE);
 
-      assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("a")), "A"); // Per fallback!
+      assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("a")), null);
-      assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("b")), "B"); // Per fallback!
+      assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("b")), null);
       assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("c")), "cat");
       assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("d")), "dog");
    }
 
    public void testAnnotationOrNameFieldNamingStrategy() throws Exception {
-      FieldNamingStrategy strategy = new AnnotationOrNameFieldNamingStrategy(ImmutableSet.of(new ExtractNamed()),
+      FieldNamingStrategy strategy = new AnnotationOrNameFieldNamingStrategy(ImmutableSet.of(new ExtractNamed()));
-            FieldNamingPolicy.IDENTITY);
 
       assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("a")), "a");
       assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("b")), "b");