Repository: jclouds Updated Branches: refs/heads/master 1e35c0fe1 -> 45fd59f4b
JCLOUDS-750 Revert 5b6f1e929ef4b6438facc06df0f081ddef8c9cf6 in favor of tighter contract on @SerializedNames. Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/45fd59f4 Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/45fd59f4 Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/45fd59f4 Branch: refs/heads/master Commit: 45fd59f4b7065ca7d4e73dd979901505fac11990 Parents: 1e35c0f Author: Adrian Cole <[email protected]> Authored: Sat Nov 1 10:20:50 2014 -0700 Committer: Adrian Cole <[email protected]> Committed: Sat Nov 1 10:56:04 2014 -0700 ---------------------------------------------------------------------- .../java/org/jclouds/json/SerializedNames.java | 27 +++++- .../org/jclouds/json/config/GsonModule.java | 15 +--- .../jclouds/json/internal/NamingStrategies.java | 34 +++++-- .../java/org/jclouds/reflect/Reflection2.java | 2 +- .../test/java/org/jclouds/json/JsonTest.java | 93 ++++++++++---------- ...ctorAndReflectiveTypeAdapterFactoryTest.java | 3 +- .../json/internal/NamingStrategiesTest.java | 11 +-- 7 files changed, 107 insertions(+), 78 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jclouds/blob/45fd59f4/core/src/main/java/org/jclouds/json/SerializedNames.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/json/SerializedNames.java b/core/src/main/java/org/jclouds/json/SerializedNames.java index 35a32ce..b6dbb8f 100644 --- a/core/src/main/java/org/jclouds/json/SerializedNames.java +++ b/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(); } http://git-wip-us.apache.org/repos/asf/jclouds/blob/45fd59f4/core/src/main/java/org/jclouds/json/config/GsonModule.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/json/config/GsonModule.java b/core/src/main/java/org/jclouds/json/config/GsonModule.java index 4c62be7..934541b 100644 --- a/core/src/main/java/org/jclouds/json/config/GsonModule.java +++ b/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, - FallbackFieldNamingStrategy fallbackFieldNamingStrategy) { + ImmutableMapTypeAdapterFactory immutableMap, DefaultExclusionStrategy exclusionStrategy) { 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()); http://git-wip-us.apache.org/repos/asf/jclouds/blob/45fd59f4/core/src/main/java/org/jclouds/json/internal/NamingStrategies.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/json/internal/NamingStrategies.java b/core/src/main/java/org/jclouds/json/internal/NamingStrategies.java index 5f4ed89..cf3c5ac 100644 --- a/core/src/main/java/org/jclouds/json/internal/NamingStrategies.java +++ b/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, - FieldNamingStrategy fallback) { + public AnnotationFieldNamingStrategy(Iterable<? extends NameExtractor<?>> extractors) { 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, - FieldNamingStrategy fallback) { - super(extractors, fallback); + public AnnotationOrNameFieldNamingStrategy(Iterable<? extends NameExtractor<?>> extractors) { + super(extractors); } @Override http://git-wip-us.apache.org/repos/asf/jclouds/blob/45fd59f4/core/src/main/java/org/jclouds/reflect/Reflection2.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/reflect/Reflection2.java b/core/src/main/java/org/jclouds/reflect/Reflection2.java index 4c08b08..f592466 100644 --- a/core/src/main/java/org/jclouds/reflect/Reflection2.java +++ b/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()} http://git-wip-us.apache.org/repos/asf/jclouds/blob/45fd59f4/core/src/test/java/org/jclouds/json/JsonTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/jclouds/json/JsonTest.java b/core/src/test/java/org/jclouds/json/JsonTest.java index cf477b4..dc8e3ee 100644 --- a/core/src/test/java/org/jclouds/json/JsonTest.java +++ b/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) { - return new AutoValue_JsonTest_UpperCamelCasedType(id, volumes); + private static SerializedNamesType create(String id, Map<String, String> volumes) { + return new AutoValue_JsonTest_SerializedNamesType(id, volumes); } } - public void upperCamelCaseWithSerializedNames() { - Json json = Guice.createInjector(new GsonModule(), new AbstractModule() { - @Override protected void configure() { - bind(FieldNamingStrategy.class).toInstance(FieldNamingPolicy.UPPER_CAMEL_CASE); - } - }).getInstance(Json.class); + public void autoValueSerializedNames() { + Json json = Guice.createInjector(new GsonModule()).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); + @AutoValue + abstract static class SerializedNamesTooFewType { + abstract String id(); + + @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); + } + } - assertEquals(json.fromJson("{\"Id\":\"1234\",\"Volumes\":null}", UpperCamelCasedType.class), - UpperCamelCasedType.create("1234", null)); + /** 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 NestedCamelCasedType { - abstract UpperCamelCasedType item(); + abstract static class NestedSerializedNamesType { + abstract SerializedNamesType item(); - abstract List<UpperCamelCasedType> items(); + 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) { - return new AutoValue_JsonTest_NestedCamelCasedType(item, items); + private static NestedSerializedNamesType create(SerializedNamesType item, List<SerializedNamesType> items) { + return new AutoValue_JsonTest_NestedSerializedNamesType(item, items); } } - private final NestedCamelCasedType nested = NestedCamelCasedType - .create(UpperCamelCasedType.create("1234", Collections.<String, String>emptyMap()), - Arrays.asList(UpperCamelCasedType.create("5678", ImmutableMap.of("Foo", "Bar")))); + private final NestedSerializedNamesType nested = NestedSerializedNamesType + .create(SerializedNamesType.create("1234", Collections.<String, String>emptyMap()), + Arrays.asList(SerializedNamesType.create("5678", ImmutableMap.of("Foo", "Bar")))); - public void nestedCamelCasedType() { - Json json = Guice.createInjector(new GsonModule(), new AbstractModule() { - @Override protected void configure() { - bind(FieldNamingStrategy.class).toInstance(FieldNamingPolicy.UPPER_CAMEL_CASE); - } - }).getInstance(Json.class); + public void autoValueSerializedNames_nestedType() { + Json json = Guice.createInjector(new GsonModule()).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(); http://git-wip-us.apache.org/repos/asf/jclouds/blob/45fd59f4/core/src/test/java/org/jclouds/json/internal/DeserializationConstructorAndReflectiveTypeAdapterFactoryTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/jclouds/json/internal/DeserializationConstructorAndReflectiveTypeAdapterFactoryTest.java b/core/src/test/java/org/jclouds/json/internal/DeserializationConstructorAndReflectiveTypeAdapterFactoryTest.java index b4f0202..1d54d09 100644 --- a/core/src/test/java/org/jclouds/json/internal/DeserializationConstructorAndReflectiveTypeAdapterFactoryTest.java +++ b/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())); http://git-wip-us.apache.org/repos/asf/jclouds/blob/45fd59f4/core/src/test/java/org/jclouds/json/internal/NamingStrategiesTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/jclouds/json/internal/NamingStrategiesTest.java b/core/src/test/java/org/jclouds/json/internal/NamingStrategiesTest.java index 20c6046..917d0bf 100644 --- a/core/src/test/java/org/jclouds/json/internal/NamingStrategiesTest.java +++ b/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()), - FieldNamingPolicy.UPPER_CAMEL_CASE); + FieldNamingStrategy strategy = new AnnotationFieldNamingStrategy(ImmutableSet.of(new ExtractNamed())); - assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("a")), "A"); // Per fallback! - assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("b")), "B"); // Per fallback! + assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("a")), null); + 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()), - FieldNamingPolicy.IDENTITY); + FieldNamingStrategy strategy = new AnnotationOrNameFieldNamingStrategy(ImmutableSet.of(new ExtractNamed())); assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("a")), "a"); assertEquals(strategy.translateName(SimpleTest.class.getDeclaredField("b")), "b");
