From 0b88dadb8f1ae9ca893465e81dc89707eae35bec Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sun, 26 Oct 2014 09:04:05 -0700 Subject: [PATCH] JCLOUDS-750 Added @SerializedNames which can apply to either a constructor or factory method to capture the corresponding json field names. Removed wildly confusing mixed naming of constructor params feature. --- .../org/jclouds/json/SerializedNames.java | 33 +++++++++++++++ .../org/jclouds/json/config/GsonModule.java | 4 +- .../json/internal/NamingStrategies.java | 24 +++++++---- ...orAndReflectiveTypeAdapterFactoryTest.java | 15 ++++--- .../json/internal/NamingStrategiesTest.java | 40 +------------------ 5 files changed, 63 insertions(+), 53 deletions(-) create mode 100644 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 new file mode 100644 index 0000000000..35a32ceee1 --- /dev/null +++ b/core/src/main/java/org/jclouds/json/SerializedNames.java @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +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; + +import java.lang.annotation.Retention; +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) +public @interface SerializedNames { + + String[] value(); +} 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 58606fb64a..afbed11ef1 100644 --- a/core/src/main/java/org/jclouds/json/config/GsonModule.java +++ b/core/src/main/java/org/jclouds/json/config/GsonModule.java @@ -42,6 +42,7 @@ import com.google.inject.Provides; import org.jclouds.date.DateService; import org.jclouds.domain.JsonBall; import org.jclouds.json.Json; +import org.jclouds.json.SerializedNames; import org.jclouds.json.internal.DeserializationConstructorAndReflectiveTypeAdapterFactory; import org.jclouds.json.internal.EnumTypeAdapterThatReturnsFromValue; import org.jclouds.json.internal.GsonWrapper; @@ -119,7 +120,8 @@ public class GsonModule extends AbstractModule { builder.registerTypeAdapterFactory(immutableMap); AnnotationConstructorNamingStrategy deserializationPolicy = new AnnotationConstructorNamingStrategy( - ImmutableSet.of(ConstructorProperties.class, Inject.class), ImmutableSet.of(new ExtractNamed())); + ImmutableSet.of(ConstructorProperties.class, SerializedNames.class, Inject.class), + ImmutableSet.of(new ExtractNamed())); builder.registerTypeAdapterFactory(new DeserializationConstructorAndReflectiveTypeAdapterFactory( new ConstructorConstructor(ImmutableMap.>of()), serializationPolicy, Excluder.DEFAULT, deserializationPolicy)); 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 bb5c03bd13..7a94ce3c90 100644 --- a/core/src/main/java/org/jclouds/json/internal/NamingStrategies.java +++ b/core/src/main/java/org/jclouds/json/internal/NamingStrategies.java @@ -32,6 +32,8 @@ import java.util.Map; import javax.inject.Named; +import org.jclouds.json.SerializedNames; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Joiner; @@ -229,17 +231,25 @@ public class NamingStrategies { String translateName(Invokable c, int index) { String name = null; - if (markers.contains(ConstructorProperties.class) && c.getAnnotation(ConstructorProperties.class) != null) { - String[] names = c.getAnnotation(ConstructorProperties.class).value(); + if ((markers.contains(ConstructorProperties.class) && c.getAnnotation(ConstructorProperties.class) != null) + || (markers.contains(SerializedNames.class) && c.getAnnotation(SerializedNames.class) != null)) { + + String[] names = c.getAnnotation(SerializedNames.class) != null + ? c.getAnnotation(SerializedNames.class).value() + : c.getAnnotation(ConstructorProperties.class).value(); + + checkArgument(names.length == c.getParameters().size(), "Incorrect count of names on annotation of %s", c); + if (names != null && names.length > index) { name = names[index]; } - } - for (Annotation annotation : c.getParameters().get(index).getAnnotations()) { - if (annotationToNameExtractor.containsKey(annotation.annotationType())) { - name = annotationToNameExtractor.get(annotation.annotationType()).apply(annotation); - break; + } else { + for (Annotation annotation : c.getParameters().get(index).getAnnotations()) { + if (annotationToNameExtractor.containsKey(annotation.annotationType())) { + name = annotationToNameExtractor.get(annotation.annotationType()).apply(annotation); + break; + } } } return name; 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 65671256f7..1d54d09c61 100644 --- a/core/src/test/java/org/jclouds/json/internal/DeserializationConstructorAndReflectiveTypeAdapterFactoryTest.java +++ b/core/src/test/java/org/jclouds/json/internal/DeserializationConstructorAndReflectiveTypeAdapterFactoryTest.java @@ -30,6 +30,7 @@ import java.util.Map; import javax.inject.Inject; import javax.inject.Named; +import org.jclouds.json.SerializedNames; import org.jclouds.json.internal.NamingStrategies.AnnotationConstructorNamingStrategy; import org.jclouds.json.internal.NamingStrategies.AnnotationOrNameFieldNamingStrategy; import org.jclouds.json.internal.NamingStrategies.ExtractNamed; @@ -63,7 +64,8 @@ public final class DeserializationConstructorAndReflectiveTypeAdapterFactoryTest FieldNamingStrategy serializationPolicy = new AnnotationOrNameFieldNamingStrategy(ImmutableSet.of( new ExtractSerializedName(), new ExtractNamed())); AnnotationConstructorNamingStrategy deserializationPolicy = new AnnotationConstructorNamingStrategy( - ImmutableSet.of(ConstructorProperties.class, Inject.class), ImmutableSet.of(new ExtractNamed())); + ImmutableSet.of(ConstructorProperties.class, SerializedNames.class, Inject.class), + ImmutableSet.of(new ExtractNamed())); return new DeserializationConstructorAndReflectiveTypeAdapterFactory(new ConstructorConstructor(ImmutableMap.>of()), serializationPolicy, Excluder.DEFAULT, deserializationPolicy); @@ -157,8 +159,8 @@ public final class DeserializationConstructorAndReflectiveTypeAdapterFactoryTest abstract List foo(); abstract Map bar(); - @Inject - static ValueTypeWithFactory create(@Named("foo") List foo, @Named("bar") Map bar) { + @SerializedNames({ "foo", "bar" }) + static ValueTypeWithFactory create(List foo, Map bar) { return new GenericParamsCopiedIn(foo, bar); } } @@ -196,14 +198,15 @@ public final class DeserializationConstructorAndReflectiveTypeAdapterFactoryTest } private abstract static class ValueTypeWithFactoryMissingSerializedNames { - @Inject + @SerializedNames({ "foo" }) static ValueTypeWithFactoryMissingSerializedNames create(List foo, Map bar) { return null; } } - @Test(expectedExceptions = IllegalArgumentException.class, expectedExceptionsMessageRegExp = ".* parameter 0 failed to be named by AnnotationBasedNamingStrategy requiring one of javax.inject.Named") - public void testSerializedNameRequiredOnAllFactoryMethodParameters() { + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = "Incorrect count of names on annotation of .*") + public void testSerializedNamesMustHaveCorrectCountOfNames() { parameterizedCtorFactory.create(gson, TypeToken.get(ValueTypeWithFactoryMissingSerializedNames.class)); } 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 4ac77202cd..ccd947a25b 100644 --- a/core/src/test/java/org/jclouds/json/internal/NamingStrategiesTest.java +++ b/core/src/test/java/org/jclouds/json/internal/NamingStrategiesTest.java @@ -54,7 +54,7 @@ public final class NamingStrategiesTest { private String d; @ConstructorProperties({"aardvark", "bat", "coyote", "dog"}) - private SimpleTest(String aa, String bb, String cc, @Named("dingo") String dd) { + private SimpleTest(String aa, String bb, String cc, String dd) { } @Inject @@ -62,14 +62,6 @@ public final class NamingStrategiesTest { } } - private static class MixedConstructorTest { - @Inject - @ConstructorProperties("thiscanbeoverriddenbyNamed") - private MixedConstructorTest(@Named("aardvark") String aa, @Named("bat") String bb, @Named("cat") String cc, @Named("dog") String dd) { - } - } - - public void testExtractSerializedName() throws Exception { NameExtractor extractor = new ExtractSerializedName(); assertEquals(extractor.extractName(SimpleTest.class.getDeclaredField("a").getAnnotation(SerializedName.class)), @@ -137,17 +129,6 @@ public final class NamingStrategiesTest { assertEquals(strategy.translateName(constructor, 0), "aardvark"); assertEquals(strategy.translateName(constructor, 1), "bat"); assertEquals(strategy.translateName(constructor, 2), "coyote"); - // Note: @Named overrides the ConstructorProperties setting - assertEquals(strategy.translateName(constructor, 3), "dingo"); - - Invokable mixedCtor = strategy.getDeserializer(typeToken(MixedConstructorTest.class)); - assertNotNull(mixedCtor); - assertEquals(mixedCtor.getParameters().size(), 4); - - assertEquals(strategy.translateName(mixedCtor, 0), "aardvark"); - assertEquals(strategy.translateName(mixedCtor, 1), "bat"); - assertEquals(strategy.translateName(mixedCtor, 2), "cat"); - assertEquals(strategy.translateName(mixedCtor, 3), "dog"); } public void testAnnotationConstructorFieldNamingStrategyCP() throws Exception { @@ -162,15 +143,6 @@ public final class NamingStrategiesTest { assertEquals(strategy.translateName(constructor, 1), "bat"); assertEquals(strategy.translateName(constructor, 2), "coyote"); assertEquals(strategy.translateName(constructor, 3), "dog"); - - Invokable mixedCtor = strategy.getDeserializer(typeToken(MixedConstructorTest.class)); - assertNotNull(mixedCtor); - assertEquals(mixedCtor.getParameters().size(), 4); - - assertEquals(strategy.translateName(mixedCtor, 0), "thiscanbeoverriddenbyNamed"); - assertNull(strategy.translateName(mixedCtor, 1)); - assertNull(strategy.translateName(mixedCtor, 2)); - assertNull(strategy.translateName(mixedCtor, 3)); } public void testAnnotationConstructorFieldNamingStrategyInject() throws Exception { @@ -185,15 +157,5 @@ public final class NamingStrategiesTest { assertEquals(strategy.translateName(constructor, 1), "bb"); assertEquals(strategy.translateName(constructor, 2), "cc"); assertEquals(strategy.translateName(constructor, 3), "dd"); - - Invokable mixedCtor = strategy.getDeserializer(typeToken(MixedConstructorTest.class)); - assertNotNull(mixedCtor); - assertEquals(mixedCtor.getParameters().size(), 4); - - assertEquals(strategy.translateName(mixedCtor, 0), "aardvark"); - assertEquals(strategy.translateName(mixedCtor, 1), "bat"); - assertEquals(strategy.translateName(mixedCtor, 2), "cat"); - assertEquals(strategy.translateName(mixedCtor, 3), "dog"); } - }