You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Adrian Cole <no...@github.com> on 2014/11/01 18:33:37 UTC

[jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

In  5b6f1e9, we added the ability to customize the FieldNamingStrategy, so that you can control the serialized names on auto-value types. I&#39;ve added code to propagate the same names used for deserialization to also be used for serialization. With this in, customizing FieldNamingStrategy is no longer needed, and should be removed.
You can merge this Pull Request by running:

  git pull https://github.com/adriancole/jclouds adrian.serialized-names-input

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds/pull/596

-- Commit Summary --

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

-- File Changes --

    M core/src/main/java/org/jclouds/json/SerializedNames.java (27)
    M core/src/main/java/org/jclouds/json/config/GsonModule.java (15)
    M core/src/main/java/org/jclouds/json/internal/NamingStrategies.java (34)
    M core/src/main/java/org/jclouds/reflect/Reflection2.java (2)
    M core/src/test/java/org/jclouds/json/JsonTest.java (93)
    M core/src/test/java/org/jclouds/json/internal/DeserializationConstructorAndReflectiveTypeAdapterFactoryTest.java (3)
    M core/src/test/java/org/jclouds/json/internal/NamingStrategiesTest.java (11)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/596.patch
https://github.com/jclouds/jclouds/pull/596.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1368](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1368/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596#issuecomment-61377049

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by Andrew Phillips <no...@github.com>.
>           for (Annotation annotation : f.getAnnotations()) {
>              if (annotationToNameExtractor.containsKey(annotation.annotationType())) {
>                 return annotationToNameExtractor.get(annotation.annotationType()).apply(annotation);
>              }
>           }
> -         return fallback.translateName(f);
> +         return null;

> Contract in gson says return null if you don't know the name.

Thanks for clarifying!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596/files#r19721983

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by Adrian Cole <no...@github.com>.
ok into master, and 1.8.x pending green. will cleanup the labs providers now.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596#issuecomment-61376877

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by Chris Custine <no...@github.com>.
+1 This will make things much better :)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596#issuecomment-61376316

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-7 #279](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-7/279/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596#issuecomment-61376730

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by Adrian Cole <no...@github.com>.
>           for (Annotation annotation : f.getAnnotations()) {
>              if (annotationToNameExtractor.containsKey(annotation.annotationType())) {
>                 return annotationToNameExtractor.get(annotation.annotationType()).apply(annotation);
>              }
>           }
> -         return fallback.translateName(f);
> +         return null;

Part of the revert. Contract in gson says return null if you don't know the
name.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596/files#r19712671

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by Andrew Phillips <no...@github.com>.
> @@ -25,9 +24,31 @@
>  
>  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!

[minor] Should this be `@SeriablizedNames`?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596/files#r19710059

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds #1892](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1892/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596#issuecomment-61378064

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by Andrew Phillips <no...@github.com>.
>           for (Annotation annotation : f.getAnnotations()) {
>              if (annotationToNameExtractor.containsKey(annotation.annotationType())) {
>                 return annotationToNameExtractor.get(annotation.annotationType()).apply(annotation);
>              }
>           }
> -         return fallback.translateName(f);
> +         return null;

[minor] Curious whether [this](http://google-gson.googlecode.com/svn/trunk/gson/docs/javadocs/com/google/gson/FieldNamingStrategy.html) is `nullable` or whether we should throw an exception here?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596/files#r19710115

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by Adrian Cole <no...@github.com>.
> @@ -25,9 +24,31 @@
>  
>  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!

Good catch. Will fix in followup.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596/files#r19712683

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by Adrian Cole <ad...@gmail.com>.
I hope we eventually kill this class and similar :)

Re: [jclouds] JCLOUDS-750 Revert customizing FieldNamingStrategy in favor of tighter contract on @SerializedNames. (#596)

Posted by Andrew Phillips <no...@github.com>.
> @@ -89,7 +89,7 @@
>     }
>  
>     /**
> -    * 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.

[minor] Should we also update the Javadoc for `constructor` in this case? Also, if there is going to be a follow-up to this, rename to something like `constructorsOrFactories`?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/596/files#r19710108