diff --git a/gson/src/main/java/com/google/gson/Gson.java b/gson/src/main/java/com/google/gson/Gson.java index 0219c1a186..4ffda1ee7f 100644 --- a/gson/src/main/java/com/google/gson/Gson.java +++ b/gson/src/main/java/com/google/gson/Gson.java @@ -16,6 +16,7 @@ package com.google.gson; +import com.google.gson.annotations.JsonAdapter; import com.google.gson.internal.ConstructorConstructor; import com.google.gson.internal.Excluder; import com.google.gson.internal.GsonBuildConfig; @@ -604,42 +605,50 @@ public TypeAdapter getAdapter(TypeToken type) { * adapter that does a little bit of work but then delegates further processing to the Gson * default type adapter. Here is an example: *

Let's say we want to write a type adapter that counts the number of objects being read - * from or written to JSON. We can achieve this by writing a type adapter factory that uses - * the getDelegateAdapter method: - *

 {@code
-   *  class StatsTypeAdapterFactory implements TypeAdapterFactory {
-   *    public int numReads = 0;
-   *    public int numWrites = 0;
-   *    public  TypeAdapter create(Gson gson, TypeToken type) {
-   *      final TypeAdapter delegate = gson.getDelegateAdapter(this, type);
-   *      return new TypeAdapter() {
-   *        public void write(JsonWriter out, T value) throws IOException {
-   *          ++numWrites;
-   *          delegate.write(out, value);
-   *        }
-   *        public T read(JsonReader in) throws IOException {
-   *          ++numReads;
-   *          return delegate.read(in);
-   *        }
-   *      };
-   *    }
-   *  }
-   *  } 
- * This factory can now be used like this: - *
 {@code
-   *  StatsTypeAdapterFactory stats = new StatsTypeAdapterFactory();
-   *  Gson gson = new GsonBuilder().registerTypeAdapterFactory(stats).create();
-   *  // Call gson.toJson() and fromJson methods on objects
-   *  System.out.println("Num JSON reads" + stats.numReads);
-   *  System.out.println("Num JSON writes" + stats.numWrites);
-   *  }
- * Note that this call will skip all factories registered before {@code skipPast}. In case of - * multiple TypeAdapterFactories registered it is up to the caller of this function to insure - * that the order of registration does not prevent this method from reaching a factory they - * would expect to reply from this call. - * Note that since you can not override type adapter factories for String and Java primitive - * types, our stats factory will not count the number of String or primitives that will be - * read or written. + * from or written to JSON. We can achieve this by writing a type adapter factory that uses + * the getDelegateAdapter method: + *
{@code
+   * class StatsTypeAdapterFactory implements TypeAdapterFactory {
+   *   public int numReads = 0;
+   *   public int numWrites = 0;
+   *   public  TypeAdapter create(Gson gson, TypeToken type) {
+   *     final TypeAdapter delegate = gson.getDelegateAdapter(this, type);
+   *     return new TypeAdapter() {
+   *       public void write(JsonWriter out, T value) throws IOException {
+   *         ++numWrites;
+   *         delegate.write(out, value);
+   *       }
+   *       public T read(JsonReader in) throws IOException {
+   *         ++numReads;
+   *         return delegate.read(in);
+   *       }
+   *     };
+   *   }
+   * }
+   * }
+ * This factory can now be used like this: + *
{@code
+   * StatsTypeAdapterFactory stats = new StatsTypeAdapterFactory();
+   * Gson gson = new GsonBuilder().registerTypeAdapterFactory(stats).create();
+   * // Call gson.toJson() and fromJson methods on objects
+   * System.out.println("Num JSON reads: " + stats.numReads);
+   * System.out.println("Num JSON writes: " + stats.numWrites);
+   * }
+ * Note that this call will skip all factories registered before {@code skipPast}. In case of + * multiple TypeAdapterFactories registered it is up to the caller of this function to insure + * that the order of registration does not prevent this method from reaching a factory they + * would expect to reply from this call. + * Note that since you can not override the type adapter factories for some types, see + * {@link GsonBuilder#registerTypeAdapter(Type, Object)}, our stats factory will not count + * the number of instances of those types that will be read or written. + * + *

If {@code skipPast} is a factory which has neither been registered on the {@link GsonBuilder} + * nor specified with the {@link JsonAdapter @JsonAdapter} annotation on a class, then this + * method behaves as if {@link #getAdapter(TypeToken)} had been called. This also means that + * for fields with {@code @JsonAdapter} annotation this method behaves normally like {@code getAdapter} + * (except for corner cases where a custom {@link InstanceCreator} is used to create an + * instance of the factory). + * * @param skipPast The type adapter factory that needs to be skipped while searching for * a matching type adapter. In most cases, you should just pass this (the type adapter * factory from where {@code getDelegateAdapter} method is being invoked). @@ -648,9 +657,10 @@ public TypeAdapter getAdapter(TypeToken type) { * @since 2.2 */ public TypeAdapter getDelegateAdapter(TypeAdapterFactory skipPast, TypeToken type) { - // Hack. If the skipPast factory isn't registered, assume the factory is being requested via - // our @JsonAdapter annotation. - if (!factories.contains(skipPast)) { + Objects.requireNonNull(skipPast, "skipPast must not be null"); + Objects.requireNonNull(type, "type must not be null"); + + if (jsonAdapterFactory.isClassJsonAdapterFactory(type, skipPast)) { skipPast = jsonAdapterFactory; } @@ -668,7 +678,13 @@ public TypeAdapter getDelegateAdapter(TypeAdapterFactory skipPast, TypeTo return candidate; } } - throw new IllegalArgumentException("GSON cannot serialize " + type); + + if (skipPastFound) { + throw new IllegalArgumentException("GSON cannot serialize " + type); + } else { + // Probably a factory from @JsonAdapter on a field + return getAdapter(type); + } } /** diff --git a/gson/src/main/java/com/google/gson/InstanceCreator.java b/gson/src/main/java/com/google/gson/InstanceCreator.java index d5096a07a3..486f250482 100644 --- a/gson/src/main/java/com/google/gson/InstanceCreator.java +++ b/gson/src/main/java/com/google/gson/InstanceCreator.java @@ -63,7 +63,7 @@ * * *

Note that it does not matter what the fields of the created instance contain since Gson will - * overwrite them with the deserialized values specified in Json. You should also ensure that a + * overwrite them with the deserialized values specified in JSON. You should also ensure that a * new object is returned, not a common object since its fields will be overwritten. * The developer will need to register {@code IdInstanceCreator} with Gson as follows:

* @@ -73,6 +73,8 @@ * * @param the type of object that will be created by this implementation. * + * @see GsonBuilder#registerTypeAdapter(Type, Object) + * * @author Inderjeet Singh * @author Joel Leitch */ @@ -81,7 +83,7 @@ public interface InstanceCreator { /** * Gson invokes this call-back method during deserialization to create an instance of the * specified type. The fields of the returned instance are overwritten with the data present - * in the Json. Since the prior contents of the object are destroyed and overwritten, do not + * in the JSON. Since the prior contents of the object are destroyed and overwritten, do not * return an instance that is useful elsewhere. In particular, do not return a common instance, * always use {@code new} to create a new instance. * diff --git a/gson/src/main/java/com/google/gson/annotations/JsonAdapter.java b/gson/src/main/java/com/google/gson/annotations/JsonAdapter.java index d168575940..5a5da72e13 100644 --- a/gson/src/main/java/com/google/gson/annotations/JsonAdapter.java +++ b/gson/src/main/java/com/google/gson/annotations/JsonAdapter.java @@ -17,6 +17,8 @@ package com.google.gson.annotations; import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.InstanceCreator; import com.google.gson.JsonDeserializer; import com.google.gson.JsonSerializer; import com.google.gson.TypeAdapter; @@ -35,11 +37,13 @@ * @JsonAdapter(UserJsonAdapter.class) * public class User { * public final String firstName, lastName; + * * private User(String firstName, String lastName) { * this.firstName = firstName; * this.lastName = lastName; * } * } + * * public class UserJsonAdapter extends TypeAdapter<User> { * @Override public void write(JsonWriter out, User user) throws IOException { * // implement write: combine firstName and lastName into name @@ -47,8 +51,8 @@ * out.name("name"); * out.value(user.firstName + " " + user.lastName); * out.endObject(); - * // implement the write method * } + * * @Override public User read(JsonReader in) throws IOException { * // implement read: split name into firstName and lastName * in.beginObject(); @@ -60,14 +64,15 @@ * } * * - * Since User class specified UserJsonAdapter.class in @JsonAdapter annotation, it - * will automatically be invoked to serialize/deserialize User instances. + * Since {@code User} class specified {@code UserJsonAdapter.class} in {@code @JsonAdapter} + * annotation, it will automatically be invoked to serialize/deserialize {@code User} instances. * - *

Here is an example of how to apply this annotation to a field. + *

Here is an example of how to apply this annotation to a field. *

  * private static final class Gadget {
- *   @JsonAdapter(UserJsonAdapter2.class)
+ *   @JsonAdapter(UserJsonAdapter.class)
  *   final User user;
+ *
  *   Gadget(User user) {
  *     this.user = user;
  *   }
@@ -75,15 +80,30 @@
  * 
* * It's possible to specify different type adapters on a field, that - * field's type, and in the {@link com.google.gson.GsonBuilder}. Field - * annotations take precedence over {@code GsonBuilder}-registered type + * field's type, and in the {@link GsonBuilder}. Field annotations + * take precedence over {@code GsonBuilder}-registered type * adapters, which in turn take precedence over annotated types. * *

The class referenced by this annotation must be either a {@link * TypeAdapter} or a {@link TypeAdapterFactory}, or must implement one * or both of {@link JsonDeserializer} or {@link JsonSerializer}. * Using {@link TypeAdapterFactory} makes it possible to delegate - * to the enclosing {@link Gson} instance. + * to the enclosing {@link Gson} instance. By default the specified + * adapter will not be called for {@code null} values; set {@link #nullSafe()} + * to {@code false} to let the adapter handle {@code null} values itself. + * + *

The type adapter is created in the same way Gson creates instances of + * custom classes during deserialization, that means: + *

    + *
  1. If a custom {@link InstanceCreator} has been registered for the + * adapter class, it will be used to create the instance + *
  2. Otherwise, if the adapter class has a no-args constructor + * (regardless of which visibility), it will be invoked to create + * the instance + *
  3. Otherwise, JDK {@code Unsafe} will be used to create the instance; + * see {@link GsonBuilder#disableJdkUnsafe()} for the unexpected + * side-effects this might have + *
* *

{@code Gson} instances might cache the adapter they create for * a {@code @JsonAdapter} annotation. It is not guaranteed that a new @@ -104,7 +124,13 @@ /** Either a {@link TypeAdapter} or {@link TypeAdapterFactory}, or one or both of {@link JsonDeserializer} or {@link JsonSerializer}. */ Class value(); - /** false, to be able to handle {@code null} values within the adapter, default value is true. */ + /** + * Whether the adapter referenced by {@link #value()} should be made {@linkplain TypeAdapter#nullSafe() null-safe}. + * + *

If {@code true} (the default), it will be made null-safe and Gson will handle {@code null} Java objects + * on serialization and JSON {@code null} on deserialization without calling the adapter. If {@code false}, + * the adapter will have to handle the {@code null} values. + */ boolean nullSafe() default true; } diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java index 9cd5649e9f..8b528b7ff4 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java @@ -24,6 +24,9 @@ import com.google.gson.annotations.JsonAdapter; import com.google.gson.internal.ConstructorConstructor; import com.google.gson.reflect.TypeToken; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; /** * Given a type T, looks for the annotation {@link JsonAdapter} and uses an instance of the @@ -32,35 +35,85 @@ * @since 2.3 */ public final class JsonAdapterAnnotationTypeAdapterFactory implements TypeAdapterFactory { + private static class DummyTypeAdapterFactory implements TypeAdapterFactory { + @Override public TypeAdapter create(Gson gson, TypeToken type) { + throw new AssertionError("Factory should not be used"); + } + } + + /** + * Factory used for {@link TreeTypeAdapter}s created for {@code @JsonAdapter} + * on a class. + */ + private static final TypeAdapterFactory TREE_TYPE_CLASS_DUMMY_FACTORY = new DummyTypeAdapterFactory(); + + /** + * Factory used for {@link TreeTypeAdapter}s created for {@code @JsonAdapter} + * on a field. + */ + private static final TypeAdapterFactory TREE_TYPE_FIELD_DUMMY_FACTORY = new DummyTypeAdapterFactory(); + private final ConstructorConstructor constructorConstructor; + /** + * For a class, if it is annotated with {@code @JsonAdapter} and refers to a {@link TypeAdapterFactory}, + * stores the factory instance in case it has been requested already. + * Has to be a {@link ConcurrentMap} because {@link Gson} guarantees to be thread-safe. + */ + // Note: In case these strong reference to TypeAdapterFactory instances are considered + // a memory leak in the future, could consider switching to WeakReference + private final ConcurrentMap, TypeAdapterFactory> adapterFactoryMap; + public JsonAdapterAnnotationTypeAdapterFactory(ConstructorConstructor constructorConstructor) { this.constructorConstructor = constructorConstructor; + this.adapterFactoryMap = new ConcurrentHashMap<>(); + } + + // Separate helper method to make sure callers retrieve annotation in a consistent way + private JsonAdapter getAnnotation(Class rawType) { + return rawType.getAnnotation(JsonAdapter.class); } @SuppressWarnings("unchecked") // this is not safe; requires that user has specified correct adapter class for @JsonAdapter @Override public TypeAdapter create(Gson gson, TypeToken targetType) { Class rawType = targetType.getRawType(); - JsonAdapter annotation = rawType.getAnnotation(JsonAdapter.class); + JsonAdapter annotation = getAnnotation(rawType); if (annotation == null) { return null; } - return (TypeAdapter) getTypeAdapter(constructorConstructor, gson, targetType, annotation); + return (TypeAdapter) getTypeAdapter(constructorConstructor, gson, targetType, annotation, true); } - TypeAdapter getTypeAdapter(ConstructorConstructor constructorConstructor, Gson gson, - TypeToken type, JsonAdapter annotation) { + // Separate helper method to make sure callers create adapter in a consistent way + private static Object createAdapter(ConstructorConstructor constructorConstructor, Class adapterClass) { // TODO: The exception messages created by ConstructorConstructor are currently written in the context of // deserialization and for example suggest usage of TypeAdapter, which would not work for @JsonAdapter usage - Object instance = constructorConstructor.get(TypeToken.get(annotation.value())).construct(); + return constructorConstructor.get(TypeToken.get(adapterClass)).construct(); + } + + private TypeAdapterFactory putFactoryAndGetCurrent(Class rawType, TypeAdapterFactory factory) { + // Uses putIfAbsent in case multiple threads concurrently create factory + TypeAdapterFactory existingFactory = adapterFactoryMap.putIfAbsent(rawType, factory); + return existingFactory != null ? existingFactory : factory; + } + + TypeAdapter getTypeAdapter(ConstructorConstructor constructorConstructor, Gson gson, + TypeToken type, JsonAdapter annotation, boolean isClassAnnotation) { + Object instance = createAdapter(constructorConstructor, annotation.value()); TypeAdapter typeAdapter; boolean nullSafe = annotation.nullSafe(); if (instance instanceof TypeAdapter) { typeAdapter = (TypeAdapter) instance; } else if (instance instanceof TypeAdapterFactory) { - typeAdapter = ((TypeAdapterFactory) instance).create(gson, type); + TypeAdapterFactory factory = (TypeAdapterFactory) instance; + + if (isClassAnnotation) { + factory = putFactoryAndGetCurrent(type.getRawType(), factory); + } + + typeAdapter = factory.create(gson, type); } else if (instance instanceof JsonSerializer || instance instanceof JsonDeserializer) { JsonSerializer serializer = instance instanceof JsonSerializer ? (JsonSerializer) instance @@ -69,10 +122,19 @@ TypeAdapter getTypeAdapter(ConstructorConstructor constructorConstructor, Gso ? (JsonDeserializer) instance : null; + // Uses dummy factory instances because TreeTypeAdapter needs a 'skipPast' factory for `Gson.getDelegateAdapter` + // call and has to differentiate there whether TreeTypeAdapter was created for @JsonAdapter on class or field + TypeAdapterFactory skipPast; + if (isClassAnnotation) { + skipPast = TREE_TYPE_CLASS_DUMMY_FACTORY; + } else { + skipPast = TREE_TYPE_FIELD_DUMMY_FACTORY; + } @SuppressWarnings({ "unchecked", "rawtypes" }) - TypeAdapter tempAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, null, nullSafe); + TypeAdapter tempAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, skipPast, nullSafe); typeAdapter = tempAdapter; + // TreeTypeAdapter handles nullSafe; don't additionally call `nullSafe()` nullSafe = false; } else { throw new IllegalArgumentException("Invalid attempt to bind an instance of " @@ -87,4 +149,45 @@ TypeAdapter getTypeAdapter(ConstructorConstructor constructorConstructor, Gso return typeAdapter; } + + /** + * Returns whether {@code factory} is a type adapter factory created for {@code @JsonAdapter} + * placed on {@code type}. + */ + public boolean isClassJsonAdapterFactory(TypeToken type, TypeAdapterFactory factory) { + Objects.requireNonNull(type); + Objects.requireNonNull(factory); + + if (factory == TREE_TYPE_CLASS_DUMMY_FACTORY) { + return true; + } + + // Using raw type to match behavior of `create(Gson, TypeToken)` above + Class rawType = type.getRawType(); + + TypeAdapterFactory existingFactory = adapterFactoryMap.get(rawType); + if (existingFactory != null) { + // Checks for reference equality, like it is done by `Gson.getDelegateAdapter` + return existingFactory == factory; + } + + // If no factory has been created for the type yet check manually for a @JsonAdapter annotation + // which specifies a TypeAdapterFactory + // Otherwise behavior would not be consistent, depending on whether or not adapter had been requested + // before call to `isClassJsonAdapterFactory` was made + JsonAdapter annotation = getAnnotation(rawType); + if (annotation == null) { + return false; + } + + Class adapterClass = annotation.value(); + if (!TypeAdapterFactory.class.isAssignableFrom(adapterClass)) { + return false; + } + + Object adapter = createAdapter(constructorConstructor, adapterClass); + TypeAdapterFactory newFactory = (TypeAdapterFactory) adapter; + + return putFactoryAndGetCurrent(rawType, newFactory) == factory; + } } diff --git a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java index 6a43b817c9..d981f2c5fd 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java @@ -156,7 +156,7 @@ private BoundField createBoundField( if (annotation != null) { // This is not safe; requires that user has specified correct adapter class for @JsonAdapter mapped = jsonAdapterFactory.getTypeAdapter( - constructorConstructor, context, fieldType, annotation); + constructorConstructor, context, fieldType, annotation, false); } final boolean jsonAdapterPresent = mapped != null; if (mapped == null) mapped = context.getAdapter(fieldType); diff --git a/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java index a1a5f760d4..f4d6eedca8 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java +++ b/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java @@ -43,11 +43,18 @@ public final class TreeTypeAdapter extends SerializationDelegatingTypeAdapter private final JsonDeserializer deserializer; final Gson gson; private final TypeToken typeToken; - private final TypeAdapterFactory skipPast; + /** + * Only intended as {@code skipPast} for {@link Gson#getDelegateAdapter(TypeAdapterFactory, TypeToken)}, + * must not be used in any other way. + */ + private final TypeAdapterFactory skipPastForGetDelegateAdapter; private final GsonContextImpl context = new GsonContextImpl(); private final boolean nullSafe; - /** The delegate is lazily created because it may not be needed, and creating it may fail. */ + /** + * The delegate is lazily created because it may not be needed, and creating it may fail. + * Field has to be {@code volatile} because {@link Gson} guarantees to be thread-safe. + */ private volatile TypeAdapter delegate; public TreeTypeAdapter(JsonSerializer serializer, JsonDeserializer deserializer, @@ -56,7 +63,7 @@ public TreeTypeAdapter(JsonSerializer serializer, JsonDeserializer deseria this.deserializer = deserializer; this.gson = gson; this.typeToken = typeToken; - this.skipPast = skipPast; + this.skipPastForGetDelegateAdapter = skipPast; this.nullSafe = nullSafe; } @@ -94,7 +101,7 @@ private TypeAdapter delegate() { TypeAdapter d = delegate; return d != null ? d - : (delegate = gson.getDelegateAdapter(skipPast, typeToken)); + : (delegate = gson.getDelegateAdapter(skipPastForGetDelegateAdapter, typeToken)); } /** diff --git a/gson/src/main/java/com/google/gson/stream/JsonWriter.java b/gson/src/main/java/com/google/gson/stream/JsonWriter.java index 07848b0682..bb285bd0c1 100644 --- a/gson/src/main/java/com/google/gson/stream/JsonWriter.java +++ b/gson/src/main/java/com/google/gson/stream/JsonWriter.java @@ -498,6 +498,9 @@ public JsonWriter name(String name) throws IOException { if (stackSize == 0) { throw new IllegalStateException("JsonWriter is closed."); } + if (stackSize == 1 && (peek() == EMPTY_DOCUMENT || peek() == NONEMPTY_DOCUMENT)) { + throw new IllegalStateException("Please begin an object before this."); + } deferredName = name; return this; } diff --git a/gson/src/main/resources/META-INF/proguard/gson.pro b/gson/src/main/resources/META-INF/proguard/gson.pro index 59d3bb441d..8f5a69b30f 100644 --- a/gson/src/main/resources/META-INF/proguard/gson.pro +++ b/gson/src/main/resources/META-INF/proguard/gson.pro @@ -15,13 +15,13 @@ # Note: Cannot perform finer selection here to only cover Gson annotations, see also https://stackoverflow.com/q/47515093 -keepattributes RuntimeVisibleAnnotations,AnnotationDefault - ### The following rules are needed for R8 in "full mode" which only adheres to `-keepattribtues` if ### the corresponding class or field is matches by a `-keep` rule as well, see ### https://r8.googlesource.com/r8/+/refs/heads/main/compatibility-faq.md#r8-full-mode -# Keep class TypeToken (respectively its generic signature) --keep class com.google.gson.reflect.TypeToken { *; } +# Keep class TypeToken (respectively its generic signature) if present +-if class com.google.gson.reflect.TypeToken +-keep,allowobfuscation class com.google.gson.reflect.TypeToken # Keep any (anonymous) classes extending TypeToken -keep,allowobfuscation class * extends com.google.gson.reflect.TypeToken @@ -29,11 +29,6 @@ # Keep classes with @JsonAdapter annotation -keep,allowobfuscation,allowoptimization @com.google.gson.annotations.JsonAdapter class * -# Keep fields with @SerializedName annotation, but allow obfuscation of their names --keepclassmembers,allowobfuscation class * { - @com.google.gson.annotations.SerializedName ; -} - # Keep fields with any other Gson annotation # Also allow obfuscation, assuming that users will additionally use @SerializedName or # other means to preserve the field names @@ -59,12 +54,19 @@ (); } -# If a class is used in some way by the application, and has fields annotated with @SerializedName -# and a no-args constructor, keep those fields and the constructor -# Based on https://issuetracker.google.com/issues/150189783#comment11 -# See also https://github.com/google/gson/pull/2420#discussion_r1241813541 for a more detailed explanation +# Keep fields annotated with @SerializedName for classes which are referenced. +# If classes with fields annotated with @SerializedName have a no-args +# constructor keep that as well. Based on +# https://issuetracker.google.com/issues/150189783#comment11. +# See also https://github.com/google/gson/pull/2420#discussion_r1241813541 +# for a more detailed explanation. -if class * --keepclasseswithmembers,allowobfuscation,allowoptimization class <1> { - (); +-keepclasseswithmembers,allowobfuscation class <1> { + @com.google.gson.annotations.SerializedName ; +} +-if class * { @com.google.gson.annotations.SerializedName ; } +-keepclassmembers,allowobfuscation,allowoptimization class <1> { + (); +} diff --git a/gson/src/test/java/com/google/gson/GsonTest.java b/gson/src/test/java/com/google/gson/GsonTest.java index 22932a55fd..2be0e388ec 100644 --- a/gson/src/test/java/com/google/gson/GsonTest.java +++ b/gson/src/test/java/com/google/gson/GsonTest.java @@ -272,6 +272,90 @@ public void run() { assertThat(otherThreadAdapter.get().toJson(null)).isEqualTo("[[\"wrapped-nested\"]]"); } + @Test + public void testGetDelegateAdapter() { + class DummyAdapter extends TypeAdapter { + private final int number; + + DummyAdapter(int number) { + this.number = number; + } + + @Override + public Number read(JsonReader in) throws IOException { + throw new AssertionError("not needed for test"); + } + + @Override + public void write(JsonWriter out, Number value) throws IOException { + throw new AssertionError("not needed for test"); + } + + // Override toString() for better assertion error messages + @Override + public String toString() { + return "adapter-" + number; + } + } + + class DummyFactory implements TypeAdapterFactory { + private final DummyAdapter adapter; + + DummyFactory(DummyAdapter adapter) { + this.adapter = adapter; + } + + @SuppressWarnings("unchecked") + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + return (TypeAdapter) adapter; + } + + // Override equals to verify that reference equality check is performed by Gson, + // and this method is ignored + @Override + public boolean equals(Object obj) { + return obj instanceof DummyFactory && ((DummyFactory) obj).adapter.equals(adapter); + } + + @Override + public int hashCode() { + return adapter.hashCode(); + } + } + + DummyAdapter adapter1 = new DummyAdapter(1); + DummyFactory factory1 = new DummyFactory(adapter1); + DummyAdapter adapter2 = new DummyAdapter(2); + DummyFactory factory2 = new DummyFactory(adapter2); + + Gson gson = new GsonBuilder() + // Note: This is 'last in, first out' order; Gson will first use factory2, then factory1 + .registerTypeAdapterFactory(factory1) + .registerTypeAdapterFactory(factory2) + .create(); + + TypeToken type = TypeToken.get(Number.class); + + assertThrows(NullPointerException.class, () -> gson.getDelegateAdapter(null, type)); + assertThrows(NullPointerException.class, () -> gson.getDelegateAdapter(factory1, null)); + + // For unknown factory the first adapter for that type should be returned + assertThat(gson.getDelegateAdapter(new DummyFactory(new DummyAdapter(0)), type)).isEqualTo(adapter2); + + assertThat(gson.getDelegateAdapter(factory2, type)).isEqualTo(adapter1); + // Default Gson adapter should be returned + assertThat(gson.getDelegateAdapter(factory1, type)).isNotInstanceOf(DummyAdapter.class); + + DummyFactory factory1Eq = new DummyFactory(adapter1); + // Verify that test setup is correct + assertThat(factory1.equals(factory1Eq)).isTrue(); + // Should only consider reference equality and ignore that custom `equals` method considers + // factories to be equal, therefore returning `adapter2` which came from `factory2` instead + // of skipping past `factory1` + assertThat(gson.getDelegateAdapter(factory1Eq, type)).isEqualTo(adapter2); + } + @Test public void testNewJsonWriter_Default() throws IOException { StringWriter writer = new StringWriter(); diff --git a/gson/src/test/java/com/google/gson/functional/InstanceCreatorTest.java b/gson/src/test/java/com/google/gson/functional/InstanceCreatorTest.java index ea3f979983..e228ef1553 100644 --- a/gson/src/test/java/com/google/gson/functional/InstanceCreatorTest.java +++ b/gson/src/test/java/com/google/gson/functional/InstanceCreatorTest.java @@ -33,7 +33,7 @@ import org.junit.Test; /** - * Functional Test exercising custom serialization only. When test applies to both + * Functional Test exercising custom deserialization only. When test applies to both * serialization and deserialization then add it to CustomTypeAdapterTest. * * @author Inderjeet Singh diff --git a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnClassesTest.java b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnClassesTest.java index 3edd8d79d4..b2b8dc0abf 100644 --- a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnClassesTest.java +++ b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnClassesTest.java @@ -22,6 +22,7 @@ import com.google.common.base.Splitter; import com.google.gson.Gson; import com.google.gson.GsonBuilder; +import com.google.gson.InstanceCreator; import com.google.gson.JsonDeserializationContext; import com.google.gson.JsonDeserializer; import com.google.gson.JsonElement; @@ -36,13 +37,14 @@ import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonWriter; import java.io.IOException; +import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.List; import java.util.Locale; import org.junit.Test; /** - * Functional tests for the {@link com.google.gson.annotations.JsonAdapter} annotation on classes. + * Functional tests for the {@link JsonAdapter} annotation on classes. */ public final class JsonAdapterAnnotationOnClassesTest { @@ -146,10 +148,84 @@ public void testSuperclassTypeAdapterNotInvoked() { } @Test - public void testNullSafeObjectFromJson() { + public void testNullSafeObject() { Gson gson = new Gson(); NullableClass fromJson = gson.fromJson("null", NullableClass.class); assertThat(fromJson).isNull(); + + fromJson = gson.fromJson("\"ignored\"", NullableClass.class); + assertThat(fromJson).isNotNull(); + + String json = gson.toJson(null, NullableClass.class); + assertThat(json).isEqualTo("null"); + + json = gson.toJson(new NullableClass()); + assertThat(json).isEqualTo("\"nullable\""); + } + + /** + * Tests behavior when a {@link TypeAdapterFactory} registered with {@code @JsonAdapter} returns + * {@code null}, indicating that it cannot handle the type and Gson should try a different factory + * instead. + */ + @Test + public void testFactoryReturningNull() { + Gson gson = new Gson(); + + assertThat(gson.fromJson("null", WithNullReturningFactory.class)).isNull(); + assertThat(gson.toJson(null, WithNullReturningFactory.class)).isEqualTo("null"); + + TypeToken> stringTypeArg = new TypeToken>() {}; + WithNullReturningFactory deserialized = gson.fromJson("\"a\"", stringTypeArg); + assertThat(deserialized.t).isEqualTo("custom-read:a"); + assertThat(gson.fromJson("null", stringTypeArg)).isNull(); + assertThat(gson.toJson(new WithNullReturningFactory<>("b"), stringTypeArg.getType())).isEqualTo("\"custom-write:b\""); + assertThat(gson.toJson(null, stringTypeArg.getType())).isEqualTo("null"); + + // Factory should return `null` for this type and Gson should fall back to reflection-based adapter + TypeToken> numberTypeArg = new TypeToken>() {}; + deserialized = gson.fromJson("{\"t\":1}", numberTypeArg); + assertThat(deserialized.t).isEqualTo(1); + assertThat(gson.toJson(new WithNullReturningFactory<>(2), numberTypeArg.getType())).isEqualTo("{\"t\":2}"); + } + // Also set `nullSafe = true` to verify that this does not cause a NullPointerException if the + // factory would accidentally call `nullSafe()` on null adapter + @JsonAdapter(value = WithNullReturningFactory.NullReturningFactory.class, nullSafe = true) + private static class WithNullReturningFactory { + T t; + + public WithNullReturningFactory(T t) { + this.t = t; + } + + static class NullReturningFactory implements TypeAdapterFactory { + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + // Don't handle raw (non-parameterized) type + if (type.getType() instanceof Class) { + return null; + } + ParameterizedType parameterizedType = (ParameterizedType) type.getType(); + // Makes this test a bit more realistic by only conditionally returning null (instead of always) + if (parameterizedType.getActualTypeArguments()[0] != String.class) { + return null; + } + + @SuppressWarnings("unchecked") + TypeAdapter adapter = (TypeAdapter) new TypeAdapter>() { + @Override + public void write(JsonWriter out, WithNullReturningFactory value) throws IOException { + out.value("custom-write:" + value.t); + } + + @Override + public WithNullReturningFactory read(JsonReader in) throws IOException { + return new WithNullReturningFactory<>("custom-read:" + in.nextString()); + } + }; + return adapter; + } + } } @JsonAdapter(A.JsonAdapter.class) @@ -222,7 +298,6 @@ private static class UserJsonAdapter extends TypeAdapter { out.name("name"); out.value(user.firstName + " " + user.lastName); out.endObject(); - // implement the write method } @Override public User read(JsonReader in) throws IOException { // implement read: split name into firstName and lastName @@ -234,6 +309,7 @@ private static class UserJsonAdapter extends TypeAdapter { } } + // Implicit `nullSafe=true` @JsonAdapter(value = NullableClassJsonAdapter.class) private static class NullableClass { } @@ -274,4 +350,396 @@ public void testIncorrectJsonAdapterType() { private static final class D { @SuppressWarnings("unused") final String value = "a"; } + + /** + * Verifies that {@link TypeAdapterFactory} specified by {@code @JsonAdapter} can + * call {@link Gson#getDelegateAdapter} without any issues, despite the factory + * not being directly registered on Gson. + */ + @Test + public void testDelegatingAdapterFactory() { + @SuppressWarnings("unchecked") + WithDelegatingFactory deserialized = new Gson().fromJson("{\"custom\":{\"f\":\"de\"}}", WithDelegatingFactory.class); + assertThat(deserialized.f).isEqualTo("de"); + + deserialized = new Gson().fromJson("{\"custom\":{\"f\":\"de\"}}", new TypeToken>() {}); + assertThat(deserialized.f).isEqualTo("de"); + + WithDelegatingFactory serialized = new WithDelegatingFactory<>("se"); + assertThat(new Gson().toJson(serialized)).isEqualTo("{\"custom\":{\"f\":\"se\"}}"); + } + @JsonAdapter(WithDelegatingFactory.Factory.class) + private static class WithDelegatingFactory { + T f; + + WithDelegatingFactory(T f) { + this.f = f; + } + + static class Factory implements TypeAdapterFactory { + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + if (type.getRawType() != WithDelegatingFactory.class) { + return null; + } + + TypeAdapter delegate = gson.getDelegateAdapter(this, type); + + return new TypeAdapter() { + @Override + public T read(JsonReader in) throws IOException { + // Perform custom deserialization + in.beginObject(); + assertThat(in.nextName()).isEqualTo("custom"); + T t = delegate.read(in); + in.endObject(); + + return t; + } + + @Override + public void write(JsonWriter out, T value) throws IOException { + // Perform custom serialization + out.beginObject(); + out.name("custom"); + delegate.write(out, value); + out.endObject(); + } + }; + } + } + } + + /** + * Similar to {@link #testDelegatingAdapterFactory}, except that the delegate is not + * looked up in {@code create} but instead in the adapter methods. + */ + @Test + public void testDelegatingAdapterFactory_Delayed() { + WithDelayedDelegatingFactory deserialized = new Gson().fromJson("{\"custom\":{\"f\":\"de\"}}", WithDelayedDelegatingFactory.class); + assertThat(deserialized.f).isEqualTo("de"); + + WithDelayedDelegatingFactory serialized = new WithDelayedDelegatingFactory("se"); + assertThat(new Gson().toJson(serialized)).isEqualTo("{\"custom\":{\"f\":\"se\"}}"); + } + @JsonAdapter(WithDelayedDelegatingFactory.Factory.class) + private static class WithDelayedDelegatingFactory { + String f; + + WithDelayedDelegatingFactory(String f) { + this.f = f; + } + + static class Factory implements TypeAdapterFactory { + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + return new TypeAdapter() { + @SuppressWarnings("SameNameButDifferent") // suppress Error Prone warning; should be clear that `Factory` refers to enclosing class + private TypeAdapter delegate() { + return gson.getDelegateAdapter(Factory.this, type); + } + + @Override + public T read(JsonReader in) throws IOException { + // Perform custom deserialization + in.beginObject(); + assertThat(in.nextName()).isEqualTo("custom"); + T t = delegate().read(in); + in.endObject(); + + return t; + } + + @Override + public void write(JsonWriter out, T value) throws IOException { + // Perform custom serialization + out.beginObject(); + out.name("custom"); + delegate().write(out, value); + out.endObject(); + } + }; + } + } + } + + /** + * Tests behavior of {@link Gson#getDelegateAdapter} when different instances of the same + * factory class are used; one registered on the {@code GsonBuilder} and the other implicitly + * through {@code @JsonAdapter}. + */ + @Test + public void testDelegating_SameFactoryClass() { + Gson gson = new GsonBuilder() + .registerTypeAdapterFactory(new WithDelegatingFactory.Factory()) + .create(); + + // Should use both factories, and therefore have `{"custom": ... }` twice + WithDelegatingFactory deserialized = gson.fromJson("{\"custom\":{\"custom\":{\"f\":\"de\"}}}", WithDelegatingFactory.class); + assertThat(deserialized.f).isEqualTo("de"); + + WithDelegatingFactory serialized = new WithDelegatingFactory<>("se"); + assertThat(gson.toJson(serialized)).isEqualTo("{\"custom\":{\"custom\":{\"f\":\"se\"}}}"); + } + + /** + * Tests behavior of {@link Gson#getDelegateAdapter} when the same instance of a factory + * is used (through {@link InstanceCreator}). + * + *

Important: This situation is likely a rare corner case; the purpose of this test is + * to verify that Gson behaves reasonable, mainly that it does not cause a {@link StackOverflowError} + * due to infinite recursion. This test is not intended to dictate an expected behavior. + */ + @Test + public void testDelegating_SameFactoryInstance() { + WithDelegatingFactory.Factory factory = new WithDelegatingFactory.Factory(); + + Gson gson = new GsonBuilder() + .registerTypeAdapterFactory(factory) + // Always provides same instance for factory + .registerTypeAdapter(WithDelegatingFactory.Factory.class, (InstanceCreator) type -> factory) + .create(); + + // Current Gson.getDelegateAdapter implementation cannot tell when call is related to @JsonAdapter + // or not, it can only work based on the `skipPast` factory, so if the same factory instance is used + // the one registered with `GsonBuilder.registerTypeAdapterFactory` actually skips past the @JsonAdapter + // one, so the JSON string is `{"custom": ...}` instead of `{"custom":{"custom":...}}` + WithDelegatingFactory deserialized = gson.fromJson("{\"custom\":{\"f\":\"de\"}}", WithDelegatingFactory.class); + assertThat(deserialized.f).isEqualTo("de"); + + WithDelegatingFactory serialized = new WithDelegatingFactory<>("se"); + assertThat(gson.toJson(serialized)).isEqualTo("{\"custom\":{\"f\":\"se\"}}"); + } + + /** + * Tests behavior of {@link Gson#getDelegateAdapter} when different instances of the same + * factory class are used; one specified with {@code @JsonAdapter} on a class, and the other specified + * with {@code @JsonAdapter} on a field of that class. + * + *

Important: This situation is likely a rare corner case; the purpose of this test is + * to verify that Gson behaves reasonable, mainly that it does not cause a {@link StackOverflowError} + * due to infinite recursion. This test is not intended to dictate an expected behavior. + */ + @Test + public void testDelegating_SameFactoryClass_OnClassAndField() { + Gson gson = new GsonBuilder() + .registerTypeAdapter(String.class, new TypeAdapter() { + @Override + public String read(JsonReader in) throws IOException { + return in.nextString() + "-str"; + } + + @Override + public void write(JsonWriter out, String value) throws IOException { + out.value(value + "-str"); + } + }) + .create(); + + // Should use both factories, and therefore have `{"custom": ... }` once for class and once for the field, + // and for field also properly delegate to custom String adapter defined above + WithDelegatingFactoryOnClassAndField deserialized = gson.fromJson("{\"custom\":{\"f\":{\"custom\":\"de\"}}}", + WithDelegatingFactoryOnClassAndField.class); + assertThat(deserialized.f).isEqualTo("de-str"); + + WithDelegatingFactoryOnClassAndField serialized = new WithDelegatingFactoryOnClassAndField("se"); + assertThat(gson.toJson(serialized)).isEqualTo("{\"custom\":{\"f\":{\"custom\":\"se-str\"}}}"); + } + + /** + * Tests behavior of {@link Gson#getDelegateAdapter} when the same instance of a factory + * is used (through {@link InstanceCreator}); specified with {@code @JsonAdapter} on a class, + * and also specified with {@code @JsonAdapter} on a field of that class. + * + *

Important: This situation is likely a rare corner case; the purpose of this test is + * to verify that Gson behaves reasonable, mainly that it does not cause a {@link StackOverflowError} + * due to infinite recursion. This test is not intended to dictate an expected behavior. + */ + @Test + public void testDelegating_SameFactoryInstance_OnClassAndField() { + WithDelegatingFactoryOnClassAndField.Factory factory = new WithDelegatingFactoryOnClassAndField.Factory(); + + Gson gson = new GsonBuilder() + .registerTypeAdapter(String.class, new TypeAdapter() { + @Override + public String read(JsonReader in) throws IOException { + return in.nextString() + "-str"; + } + + @Override + public void write(JsonWriter out, String value) throws IOException { + out.value(value + "-str"); + } + }) + // Always provides same instance for factory + .registerTypeAdapter(WithDelegatingFactoryOnClassAndField.Factory.class, (InstanceCreator) type -> factory) + .create(); + + // Because field type (`String`) differs from declaring class, JsonAdapterAnnotationTypeAdapterFactory does + // not confuse factories and this behaves as expected: Both the declaring class and the field each have + // `{"custom": ...}` and delegation for the field to the custom String adapter defined above works properly + WithDelegatingFactoryOnClassAndField deserialized = gson.fromJson("{\"custom\":{\"f\":{\"custom\":\"de\"}}}", + WithDelegatingFactoryOnClassAndField.class); + assertThat(deserialized.f).isEqualTo("de-str"); + + WithDelegatingFactoryOnClassAndField serialized = new WithDelegatingFactoryOnClassAndField("se"); + assertThat(gson.toJson(serialized)).isEqualTo("{\"custom\":{\"f\":{\"custom\":\"se-str\"}}}"); + } + // Same factory class specified on class and one of its fields + @JsonAdapter(WithDelegatingFactoryOnClassAndField.Factory.class) + private static class WithDelegatingFactoryOnClassAndField { + @SuppressWarnings("SameNameButDifferent") // suppress Error Prone warning; should be clear that `Factory` refers to nested class + @JsonAdapter(Factory.class) + String f; + + WithDelegatingFactoryOnClassAndField(String f) { + this.f = f; + } + + static class Factory implements TypeAdapterFactory { + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + TypeAdapter delegate = gson.getDelegateAdapter(this, type); + + return new TypeAdapter() { + @Override + public T read(JsonReader in) throws IOException { + // Perform custom deserialization + in.beginObject(); + assertThat(in.nextName()).isEqualTo("custom"); + T t = delegate.read(in); + in.endObject(); + + return t; + } + + @Override + public void write(JsonWriter out, T value) throws IOException { + // Perform custom serialization + out.beginObject(); + out.name("custom"); + delegate.write(out, value); + out.endObject(); + } + }; + } + } + } + + /** + * Tests usage of {@link JsonSerializer} as {@link JsonAdapter} value + */ + @Test + public void testJsonSerializer() { + Gson gson = new Gson(); + // Verify that delegate deserializer (reflection deserializer) is used + WithJsonSerializer deserialized = gson.fromJson("{\"f\":\"test\"}", WithJsonSerializer.class); + assertThat(deserialized.f).isEqualTo("test"); + + String json = gson.toJson(new WithJsonSerializer()); + // Uses custom serializer which always returns `true` + assertThat(json).isEqualTo("true"); + } + @JsonAdapter(WithJsonSerializer.Serializer.class) + private static class WithJsonSerializer { + String f = ""; + + static class Serializer implements JsonSerializer { + @Override + public JsonElement serialize(WithJsonSerializer src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive(true); + } + } + } + + /** + * Tests usage of {@link JsonDeserializer} as {@link JsonAdapter} value + */ + @Test + public void testJsonDeserializer() { + Gson gson = new Gson(); + WithJsonDeserializer deserialized = gson.fromJson("{\"f\":\"test\"}", WithJsonDeserializer.class); + // Uses custom deserializer which always uses "123" as field value + assertThat(deserialized.f).isEqualTo("123"); + + // Verify that delegate serializer (reflection serializer) is used + String json = gson.toJson(new WithJsonDeserializer("abc")); + assertThat(json).isEqualTo("{\"f\":\"abc\"}"); + } + @JsonAdapter(WithJsonDeserializer.Deserializer.class) + private static class WithJsonDeserializer { + String f; + + WithJsonDeserializer(String f) { + this.f = f; + } + + static class Deserializer implements JsonDeserializer { + @Override + public WithJsonDeserializer deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) { + return new WithJsonDeserializer("123"); + } + } + } + + /** + * Tests creation of the adapter referenced by {@code @JsonAdapter} using an {@link InstanceCreator}. + */ + @Test + public void testAdapterCreatedByInstanceCreator() { + CreatedByInstanceCreator.Serializer serializer = new CreatedByInstanceCreator.Serializer("custom"); + Gson gson = new GsonBuilder() + .registerTypeAdapter(CreatedByInstanceCreator.Serializer.class, (InstanceCreator) t -> serializer) + .create(); + + String json = gson.toJson(new CreatedByInstanceCreator()); + assertThat(json).isEqualTo("\"custom\""); + } + @JsonAdapter(CreatedByInstanceCreator.Serializer.class) + private static class CreatedByInstanceCreator { + static class Serializer implements JsonSerializer { + private final String value; + + @SuppressWarnings("unused") + public Serializer() { + throw new AssertionError("should not be called"); + } + + public Serializer(String value) { + this.value = value; + } + + @Override + public JsonElement serialize(CreatedByInstanceCreator src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive(value); + } + } + } + + /** + * Tests creation of the adapter referenced by {@code @JsonAdapter} using JDK Unsafe. + */ + @Test + public void testAdapterCreatedByJdkUnsafe() { + String json = new Gson().toJson(new CreatedByJdkUnsafe()); + assertThat(json).isEqualTo("false"); + } + @JsonAdapter(CreatedByJdkUnsafe.Serializer.class) + private static class CreatedByJdkUnsafe { + static class Serializer implements JsonSerializer { + // JDK Unsafe leaves this at default value `false` + private boolean wasInitialized = true; + + // Explicit constructor with args to remove implicit no-args constructor + @SuppressWarnings("unused") + public Serializer(int i) { + throw new AssertionError("should not be called"); + } + + @Override + public JsonElement serialize(CreatedByJdkUnsafe src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive(wasInitialized); + } + } + } } diff --git a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java index b322ad8bff..6601b2d36b 100644 --- a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java +++ b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java @@ -18,21 +18,32 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.gson.ExclusionStrategy; +import com.google.gson.FieldAttributes; import com.google.gson.Gson; import com.google.gson.GsonBuilder; +import com.google.gson.JsonDeserializationContext; +import com.google.gson.JsonDeserializer; +import com.google.gson.JsonElement; +import com.google.gson.JsonPrimitive; +import com.google.gson.JsonSerializationContext; +import com.google.gson.JsonSerializer; import com.google.gson.TypeAdapter; import com.google.gson.TypeAdapterFactory; import com.google.gson.annotations.JsonAdapter; +import com.google.gson.internal.bind.ReflectiveTypeAdapterFactory; import com.google.gson.reflect.TypeToken; import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonWriter; import java.io.IOException; +import java.lang.reflect.Type; import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.junit.Test; /** - * Functional tests for the {@link com.google.gson.annotations.JsonAdapter} annotation on fields. + * Functional tests for the {@link JsonAdapter} annotation on fields. */ public final class JsonAdapterAnnotationOnFieldsTest { @Test @@ -313,4 +324,339 @@ private static class Gizmo2PartTypeAdapterFactory implements TypeAdapterFactory }; } } + + /** + * Verify that {@link JsonAdapter} annotation can overwrite adapters which + * can normally not be overwritten (in this case adapter for {@link JsonElement}). + */ + @Test + public void testOverwriteBuiltIn() { + BuiltInOverwriting obj = new BuiltInOverwriting(); + obj.f = new JsonPrimitive(true); + String json = new Gson().toJson(obj); + assertThat(json).isEqualTo("{\"f\":\"" + JsonElementAdapter.SERIALIZED + "\"}"); + + BuiltInOverwriting deserialized = new Gson().fromJson("{\"f\": 2}", BuiltInOverwriting.class); + assertThat(deserialized.f).isEqualTo(JsonElementAdapter.DESERIALIZED); + } + + private static class BuiltInOverwriting { + @JsonAdapter(JsonElementAdapter.class) + JsonElement f; + } + + private static class JsonElementAdapter extends TypeAdapter { + static final JsonPrimitive DESERIALIZED = new JsonPrimitive("deserialized hardcoded"); + @Override public JsonElement read(JsonReader in) throws IOException { + in.skipValue(); + return DESERIALIZED; + } + + static final String SERIALIZED = "serialized hardcoded"; + @Override public void write(JsonWriter out, JsonElement value) throws IOException { + out.value(SERIALIZED); + } + } + + /** + * Verify that exclusion strategy preventing serialization has higher precedence than + * {@link JsonAdapter} annotation. + */ + @Test + public void testExcludeSerializePrecedence() { + Gson gson = new GsonBuilder() + .addSerializationExclusionStrategy(new ExclusionStrategy() { + @Override public boolean shouldSkipField(FieldAttributes f) { + return true; + } + @Override public boolean shouldSkipClass(Class clazz) { + return false; + } + }) + .create(); + + DelegatingAndOverwriting obj = new DelegatingAndOverwriting(); + obj.f = 1; + obj.f2 = new JsonPrimitive(2); + obj.f3 = new JsonPrimitive(true); + String json = gson.toJson(obj); + assertThat(json).isEqualTo("{}"); + + DelegatingAndOverwriting deserialized = gson.fromJson("{\"f\":1,\"f2\":2,\"f3\":3}", DelegatingAndOverwriting.class); + assertThat(deserialized.f).isEqualTo(Integer.valueOf(1)); + assertThat(deserialized.f2).isEqualTo(new JsonPrimitive(2)); + // Verify that for deserialization type adapter specified by @JsonAdapter is used + assertThat(deserialized.f3).isEqualTo(JsonElementAdapter.DESERIALIZED); + } + + /** + * Verify that exclusion strategy preventing deserialization has higher precedence than + * {@link JsonAdapter} annotation. + */ + @Test + public void testExcludeDeserializePrecedence() { + Gson gson = new GsonBuilder() + .addDeserializationExclusionStrategy(new ExclusionStrategy() { + @Override public boolean shouldSkipField(FieldAttributes f) { + return true; + } + @Override public boolean shouldSkipClass(Class clazz) { + return false; + } + }) + .create(); + + DelegatingAndOverwriting obj = new DelegatingAndOverwriting(); + obj.f = 1; + obj.f2 = new JsonPrimitive(2); + obj.f3 = new JsonPrimitive(true); + String json = gson.toJson(obj); + // Verify that for serialization type adapters specified by @JsonAdapter are used + assertThat(json).isEqualTo("{\"f\":1,\"f2\":2,\"f3\":\"" + JsonElementAdapter.SERIALIZED + "\"}"); + + DelegatingAndOverwriting deserialized = gson.fromJson("{\"f\":1,\"f2\":2,\"f3\":3}", DelegatingAndOverwriting.class); + assertThat(deserialized.f).isNull(); + assertThat(deserialized.f2).isNull(); + assertThat(deserialized.f3).isNull(); + } + + /** + * Verify that exclusion strategy preventing serialization and deserialization has + * higher precedence than {@link JsonAdapter} annotation. + * + *

This is a separate test method because {@link ReflectiveTypeAdapterFactory} handles + * this case differently. + */ + @Test + public void testExcludePrecedence() { + Gson gson = new GsonBuilder() + .setExclusionStrategies(new ExclusionStrategy() { + @Override public boolean shouldSkipField(FieldAttributes f) { + return true; + } + @Override public boolean shouldSkipClass(Class clazz) { + return false; + } + }) + .create(); + + DelegatingAndOverwriting obj = new DelegatingAndOverwriting(); + obj.f = 1; + obj.f2 = new JsonPrimitive(2); + obj.f3 = new JsonPrimitive(true); + String json = gson.toJson(obj); + assertThat(json).isEqualTo("{}"); + + DelegatingAndOverwriting deserialized = gson.fromJson("{\"f\":1,\"f2\":2,\"f3\":3}", DelegatingAndOverwriting.class); + assertThat(deserialized.f).isNull(); + assertThat(deserialized.f2).isNull(); + assertThat(deserialized.f3).isNull(); + } + + private static class DelegatingAndOverwriting { + @JsonAdapter(DelegatingAdapterFactory.class) + Integer f; + @JsonAdapter(DelegatingAdapterFactory.class) + JsonElement f2; + // Also have non-delegating adapter to make tests handle both cases + @JsonAdapter(JsonElementAdapter.class) + JsonElement f3; + + static class DelegatingAdapterFactory implements TypeAdapterFactory { + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + return gson.getDelegateAdapter(this, type); + } + } + } + + /** + * Verifies that {@link TypeAdapterFactory} specified by {@code @JsonAdapter} can + * call {@link Gson#getDelegateAdapter} without any issues, despite the factory + * not being directly registered on Gson. + */ + @Test + public void testDelegatingAdapterFactory() { + @SuppressWarnings("unchecked") + WithDelegatingFactory deserialized = new Gson().fromJson("{\"f\":\"test\"}", WithDelegatingFactory.class); + assertThat(deserialized.f).isEqualTo("test-custom"); + + deserialized = new Gson().fromJson("{\"f\":\"test\"}", new TypeToken>() {}); + assertThat(deserialized.f).isEqualTo("test-custom"); + + WithDelegatingFactory serialized = new WithDelegatingFactory<>(); + serialized.f = "value"; + assertThat(new Gson().toJson(serialized)).isEqualTo("{\"f\":\"value-custom\"}"); + } + private static class WithDelegatingFactory { + @SuppressWarnings("SameNameButDifferent") // suppress Error Prone warning; should be clear that `Factory` refers to nested class + @JsonAdapter(Factory.class) + T f; + + static class Factory implements TypeAdapterFactory { + @SuppressWarnings("unchecked") + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + TypeAdapter delegate = (TypeAdapter) gson.getDelegateAdapter(this, type); + + return (TypeAdapter) new TypeAdapter() { + @Override + public String read(JsonReader in) throws IOException { + // Perform custom deserialization + return delegate.read(in) + "-custom"; + } + + @Override + public void write(JsonWriter out, String value) throws IOException { + // Perform custom serialization + delegate.write(out, value + "-custom"); + } + }; + } + } + } + + /** + * Similar to {@link #testDelegatingAdapterFactory}, except that the delegate is not + * looked up in {@code create} but instead in the adapter methods. + */ + @Test + public void testDelegatingAdapterFactory_Delayed() { + WithDelayedDelegatingFactory deserialized = new Gson().fromJson("{\"f\":\"test\"}", WithDelayedDelegatingFactory.class); + assertThat(deserialized.f).isEqualTo("test-custom"); + + WithDelayedDelegatingFactory serialized = new WithDelayedDelegatingFactory(); + serialized.f = "value"; + assertThat(new Gson().toJson(serialized)).isEqualTo("{\"f\":\"value-custom\"}"); + } + @SuppressWarnings("SameNameButDifferent") // suppress Error Prone warning; should be clear that `Factory` refers to nested class + private static class WithDelayedDelegatingFactory { + @JsonAdapter(Factory.class) + String f; + + static class Factory implements TypeAdapterFactory { + @SuppressWarnings("unchecked") + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + return (TypeAdapter) new TypeAdapter() { + private TypeAdapter delegate() { + return (TypeAdapter) gson.getDelegateAdapter(Factory.this, type); + } + + @Override + public String read(JsonReader in) throws IOException { + // Perform custom deserialization + return delegate().read(in) + "-custom"; + } + + @Override + public void write(JsonWriter out, String value) throws IOException { + // Perform custom serialization + delegate().write(out, value + "-custom"); + } + }; + } + } + } + + /** + * Tests usage of {@link Gson#getAdapter(TypeToken)} in the {@code create} method of the factory. + * Existing code was using that as workaround because {@link Gson#getDelegateAdapter} previously + * did not work in combination with {@code @JsonAdapter}, see https://github.com/google/gson/issues/1028. + */ + @Test + public void testGetAdapterDelegation() { + Gson gson = new Gson(); + GetAdapterDelegation deserialized = gson.fromJson("{\"f\":\"de\"}", GetAdapterDelegation.class); + assertThat(deserialized.f).isEqualTo("de-custom"); + + String json = gson.toJson(new GetAdapterDelegation("se")); + assertThat(json).isEqualTo("{\"f\":\"se-custom\"}"); + } + private static class GetAdapterDelegation { + @SuppressWarnings("SameNameButDifferent") // suppress Error Prone warning; should be clear that `Factory` refers to nested class + @JsonAdapter(Factory.class) + String f; + + GetAdapterDelegation(String f) { + this.f = f; + } + + static class Factory implements TypeAdapterFactory { + @SuppressWarnings("unchecked") + @Override + public TypeAdapter create(Gson gson, TypeToken type) { + // Uses `Gson.getAdapter` instead of `Gson.getDelegateAdapter` + TypeAdapter delegate = (TypeAdapter) gson.getAdapter(type); + + return (TypeAdapter) new TypeAdapter() { + @Override + public String read(JsonReader in) throws IOException { + return delegate.read(in) + "-custom"; + } + + @Override + public void write(JsonWriter out, String value) throws IOException { + delegate.write(out, value + "-custom"); + } + }; + } + } + } + + /** + * Tests usage of {@link JsonSerializer} as {@link JsonAdapter} value on a field + */ + @Test + public void testJsonSerializer() { + Gson gson = new Gson(); + // Verify that delegate deserializer for List is used + WithJsonSerializer deserialized = gson.fromJson("{\"f\":[1,2,3]}", WithJsonSerializer.class); + assertThat(deserialized.f).isEqualTo(Arrays.asList(1, 2, 3)); + + String json = gson.toJson(new WithJsonSerializer()); + // Uses custom serializer which always returns `true` + assertThat(json).isEqualTo("{\"f\":true}"); + } + private static class WithJsonSerializer { + @JsonAdapter(Serializer.class) + List f = Collections.emptyList(); + + static class Serializer implements JsonSerializer> { + @Override + public JsonElement serialize(List src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive(true); + } + } + } + + /** + * Tests usage of {@link JsonDeserializer} as {@link JsonAdapter} value on a field + */ + @Test + public void testJsonDeserializer() { + Gson gson = new Gson(); + WithJsonDeserializer deserialized = gson.fromJson("{\"f\":[5]}", WithJsonDeserializer.class); + // Uses custom deserializer which always returns `[3, 2, 1]` + assertThat(deserialized.f).isEqualTo(Arrays.asList(3, 2, 1)); + + // Verify that delegate serializer for List is used + String json = gson.toJson(new WithJsonDeserializer(Arrays.asList(4, 5, 6))); + assertThat(json).isEqualTo("{\"f\":[4,5,6]}"); + } + private static class WithJsonDeserializer { + @JsonAdapter(Deserializer.class) + List f; + + WithJsonDeserializer(List f) { + this.f = f; + } + + static class Deserializer implements JsonDeserializer> { + @Override + public List deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) { + return Arrays.asList(3, 2, 1); + } + } + } } diff --git a/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java b/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java index 20081f68cf..1e4af6a0f9 100644 --- a/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java +++ b/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java @@ -20,6 +20,7 @@ import com.google.errorprone.annotations.Keep; import com.google.gson.Gson; +import com.google.gson.GsonBuilder; import com.google.gson.JsonDeserializationContext; import com.google.gson.JsonDeserializer; import com.google.gson.JsonElement; @@ -27,7 +28,11 @@ import com.google.gson.JsonPrimitive; import com.google.gson.JsonSerializationContext; import com.google.gson.JsonSerializer; +import com.google.gson.TypeAdapter; import com.google.gson.annotations.JsonAdapter; +import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonWriter; +import java.io.IOException; import java.lang.reflect.Type; import org.junit.Test; @@ -43,7 +48,7 @@ public void testJsonSerializerDeserializerBasedJsonAdapterOnFields() { String json = gson.toJson(new Computer(new User("Inderjeet Singh"), null, new User("Jesse Wilson"))); assertThat(json).isEqualTo("{\"user1\":\"UserSerializer\",\"user3\":\"UserSerializerDeserializer\"}"); Computer computer = gson.fromJson("{'user2':'Jesse Wilson','user3':'Jake Wharton'}", Computer.class); - assertThat(computer.user2.name).isEqualTo("UserSerializer"); + assertThat(computer.user2.name).isEqualTo("UserDeserializer"); assertThat(computer.user3.name).isEqualTo("UserSerializerDeserializer"); } @@ -82,7 +87,7 @@ private static final class UserDeserializer implements JsonDeserializer { @Override public User deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException { - return new User("UserSerializer"); + return new User("UserDeserializer"); } } @@ -178,20 +183,48 @@ private static final class BaseIntegerAdapter implements JsonSerializer() { + @Override + public User read(JsonReader in) throws IOException { + in.nextNull(); + return new User("fallback-read"); + } + @Override + public void write(JsonWriter out, User value) throws IOException { + assertThat(value).isNull(); + out.value("fallback-write"); + } + }) + .serializeNulls() + .create(); + + String json = gson.toJson(new WithNullSafe(null, null, null, null)); + // Only nullSafe=true serializer writes null; for @JsonAdapter with deserializer nullSafe is ignored when serializing + assertThat(json).isEqualTo("{\"userS\":\"UserSerializer\",\"userSN\":null,\"userD\":\"fallback-write\",\"userDN\":\"fallback-write\"}"); + + WithNullSafe deserialized = gson.fromJson("{\"userS\":null,\"userSN\":null,\"userD\":null,\"userDN\":null}", WithNullSafe.class); + // For @JsonAdapter with serializer nullSafe is ignored when deserializing + assertThat(deserialized.userS.name).isEqualTo("fallback-read"); + assertThat(deserialized.userSN.name).isEqualTo("fallback-read"); + assertThat(deserialized.userD.name).isEqualTo("UserDeserializer"); + assertThat(deserialized.userDN).isNull(); + } + + private static final class WithNullSafe { + // "userS..." uses JsonSerializer + @JsonAdapter(value = UserSerializer.class, nullSafe = false) final User userS; + @JsonAdapter(value = UserSerializer.class, nullSafe = true) final User userSN; + + // "userD..." uses JsonDeserializer + @JsonAdapter(value = UserDeserializer.class, nullSafe = false) final User userD; + @JsonAdapter(value = UserDeserializer.class, nullSafe = true) final User userDN; + + WithNullSafe(User userS, User userSN, User userD, User userDN) { + this.userS = userS; + this.userSN = userSN; + this.userD = userD; + this.userDN = userDN; } } } diff --git a/gson/src/test/java/com/google/gson/regression/JsonAdapterNullSafeTest.java b/gson/src/test/java/com/google/gson/regression/JsonAdapterNullSafeTest.java deleted file mode 100644 index 10c9a9a560..0000000000 --- a/gson/src/test/java/com/google/gson/regression/JsonAdapterNullSafeTest.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright (C) 2016 Google Inc. - * - * Licensed 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 com.google.gson.regression; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.gson.Gson; -import com.google.gson.TypeAdapter; -import com.google.gson.TypeAdapterFactory; -import com.google.gson.annotations.JsonAdapter; -import com.google.gson.reflect.TypeToken; -import org.junit.Test; - -public class JsonAdapterNullSafeTest { - private final Gson gson = new Gson(); - - @Test - public void testNullSafeBugSerialize() { - Device device = new Device("ec57803e"); - String unused = gson.toJson(device); - } - - @Test - public void testNullSafeBugDeserialize() { - Device device = gson.fromJson("{'id':'ec57803e2'}", Device.class); - assertThat(device.id).isEqualTo("ec57803e2"); - } - - @JsonAdapter(Device.JsonAdapterFactory.class) - private static final class Device { - String id; - Device(String id) { - this.id = id; - } - - static final class JsonAdapterFactory implements TypeAdapterFactory { - // The recursiveCall in {@link Device.JsonAdapterFactory} is the source of this bug - // because we use it to return a null type adapter on a recursive call. - private static final ThreadLocal recursiveCall = new ThreadLocal<>(); - - @Override public TypeAdapter create(final Gson gson, TypeToken type) { - if (type.getRawType() != Device.class || recursiveCall.get() != null) { - recursiveCall.set(null); // clear for subsequent use - return null; - } - recursiveCall.set(Boolean.TRUE); - return gson.getDelegateAdapter(this, type); - } - } - } -} diff --git a/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java b/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java index ae4194d745..80bf15f66e 100644 --- a/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java +++ b/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java @@ -28,6 +28,8 @@ import java.math.BigDecimal; import java.math.BigInteger; import org.junit.Test; +import java.util.Arrays; +import java.util.List; @SuppressWarnings("resource") public final class JsonWriterTest { @@ -114,13 +116,17 @@ public void testTopLevelValueTypes() throws IOException { public void testInvalidTopLevelTypes() throws IOException { StringWriter stringWriter = new StringWriter(); JsonWriter jsonWriter = new JsonWriter(stringWriter); - jsonWriter.name("hello"); // TODO: This should throw, see https://github.com/google/gson/issues/2407 - try { - jsonWriter.value("world"); - fail(); - } catch (IllegalStateException expected) { - assertThat(expected).hasMessageThat().isEqualTo("Nesting problem."); - } + assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + } + + @Test + public void closeAllObjectsAndTryToAddElements() throws IOException { + JsonWriter jsonWriterForNameAddition = getJsonWriterWithObjects(); + assertThrows(IllegalStateException.class, () -> jsonWriterForNameAddition.name("this_throw_exception_as_all_objects_are_closed")); + jsonWriterForNameAddition.close(); + JsonWriter jsonWriterForValueAddition = getJsonWriterWithObjects(); + assertThrows(IllegalStateException.class, () -> jsonWriterForValueAddition.value("this_throw_exception_as_only_one_top_level_entry")); + jsonWriterForValueAddition.close(); } @Test @@ -973,4 +979,33 @@ public void testIndentOverwritesFormattingStyle() throws IOException { + "}"; assertThat(stringWriter.toString()).isEqualTo(expected); } + + /** + * This method wites a json object and return a jsonwriter object + * that we can use for the testing purpose + * @return JsonWriter Object with nested object and an array + */ + private JsonWriter getJsonWriterWithObjects() throws IOException { + StringWriter stringWriter = new StringWriter(); + JsonWriter jsonWriter = new JsonWriter(stringWriter); + jsonWriter.beginObject(); + jsonWriter.name("a").value(20); + jsonWriter.name("age").value(30); + + // Start the nested "address" object + jsonWriter.name("address").beginObject(); + jsonWriter.name("city").value("New York"); + jsonWriter.name("country").value("USA"); + jsonWriter.endObject(); // End the nested "address" object + jsonWriter.name("random_prop").value(78); + // Add an array of phone numbers (list of numbers) + List phoneNumbers = Arrays.asList(1234567890, 98989, 9909); + jsonWriter.name("phoneNumbers").beginArray(); + for (Integer phoneNumber : phoneNumbers) { + jsonWriter.value(phoneNumber); + } + jsonWriter.endArray(); // End the array + jsonWriter.endObject(); // End the outer object + return jsonWriter; + } } diff --git a/metrics/src/main/java/com/google/gson/metrics/ParseBenchmark.java b/metrics/src/main/java/com/google/gson/metrics/ParseBenchmark.java index 63522af2e7..bbf541719d 100644 --- a/metrics/src/main/java/com/google/gson/metrics/ParseBenchmark.java +++ b/metrics/src/main/java/com/google/gson/metrics/ParseBenchmark.java @@ -351,7 +351,7 @@ static class User { @JsonProperty boolean geo_enabled; @JsonProperty boolean verified; @JsonProperty String profile_background_image_url; - @JsonProperty boolean defalut_profile_image; + @JsonProperty boolean default_profile_image; @JsonProperty int friends_count; @JsonProperty int statuses_count; @JsonProperty String screen_name; diff --git a/pom.xml b/pom.xml index 73924ac5e0..0a992d0063 100644 --- a/pom.xml +++ b/pom.xml @@ -100,7 +100,7 @@ org.apache.maven.plugins maven-enforcer-plugin - 3.3.0 + 3.4.0 enforce-versions diff --git a/shrinker-test/common.pro b/shrinker-test/common.pro new file mode 100644 index 0000000000..b30faa1329 --- /dev/null +++ b/shrinker-test/common.pro @@ -0,0 +1,44 @@ +### Common rules for ProGuard and R8 +### Should only contains rules needed specifically for the integration test; +### any general rules which are relevant for all users should not be here but in `META-INF/proguard` of Gson + +-allowaccessmodification + +# On Windows mixed case class names might cause problems +-dontusemixedcaseclassnames + +# Ignore notes about duplicate JDK classes +-dontnote module-info,jdk.internal.** + + +# Keep test entrypoints +-keep class com.example.Main { + public static void runTests(...); +} +-keep class com.example.NoSerializedNameMain { + public static java.lang.String runTest(); + public static java.lang.String runTestNoJdkUnsafe(); + public static java.lang.String runTestNoDefaultConstructor(); +} + + +### Test data setup + +# Keep fields without annotations which should be preserved +-keepclassmembers class com.example.ClassWithNamedFields { + !transient ; +} + +-keepclassmembernames class com.example.ClassWithExposeAnnotation { + ; +} +-keepclassmembernames class com.example.ClassWithJsonAdapterAnnotation { + ** f; +} +-keepclassmembernames class com.example.ClassWithVersionAnnotations { + ; +} + +# Keep the name of the class to allow using reflection to check if this class still exists +# after shrinking +-keepnames class com.example.UnusedClass diff --git a/shrinker-test/proguard.pro b/shrinker-test/proguard.pro index 3e8a812041..ee96073372 100644 --- a/shrinker-test/proguard.pro +++ b/shrinker-test/proguard.pro @@ -1,48 +1,17 @@ -### Common rules for ProGuard and R8 -### Should only contains rules needed specifically for the integration test; -### any general rules which are relevant for all users should not be here but in `META-INF/proguard` of Gson +# Include common rules +-include common.pro --allowaccessmodification +### ProGuard specific rules -# On Windows mixed case class names might cause problems --dontusemixedcaseclassnames - -# Ignore notes about duplicate JDK classes --dontnote module-info,jdk.internal.** - - -# Keep test entrypoints --keep class com.example.Main { - public static void runTests(...); -} --keep class com.example.DefaultConstructorMain { - public static java.lang.String runTest(); - public static java.lang.String runTestNoJdkUnsafe(); - public static java.lang.String runTestNoDefaultConstructor(); -} - - -### Test data setup - -# Keep fields without annotations which should be preserved --keepclassmembers class com.example.ClassWithNamedFields { - !transient ; -} - --keepclassmembernames class com.example.ClassWithExposeAnnotation { +# Unlike R8, ProGuard does not perform aggressive optimization which makes classes abstract, +# therefore for ProGuard can successfully perform deserialization, and for that need to +# preserve the field names +-keepclassmembernames class com.example.NoSerializedNameMain$TestClass { ; } --keepclassmembernames class com.example.ClassWithJsonAdapterAnnotation { - ** f; -} --keepclassmembernames class com.example.ClassWithVersionAnnotations { - ; -} - - --keepclassmembernames class com.example.DefaultConstructorMain$TestClass { +-keepclassmembernames class com.example.NoSerializedNameMain$TestClassNotAbstract { ; } --keepclassmembernames class com.example.DefaultConstructorMain$TestClassNotAbstract { +-keepclassmembernames class com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor { ; } diff --git a/shrinker-test/r8.pro b/shrinker-test/r8.pro index 392f0f0d9c..01b8c84ee2 100644 --- a/shrinker-test/r8.pro +++ b/shrinker-test/r8.pro @@ -1,5 +1,5 @@ -# Extend the ProGuard rules --include proguard.pro +# Include common rules +-include common.pro ### The following rules are needed for R8 in "full mode", which performs more aggressive optimizations than ProGuard ### See https://r8.googlesource.com/r8/+/refs/heads/main/compatibility-faq.md#r8-full-mode @@ -10,11 +10,11 @@ -keep,allowshrinking,allowoptimization,allowobfuscation,allowaccessmodification class com.example.GenericClasses$GenericUsingGenericClass # Don't obfuscate class name, to check it in exception message --keep,allowshrinking,allowoptimization class com.example.DefaultConstructorMain$TestClass --keep,allowshrinking,allowoptimization class com.example.DefaultConstructorMain$TestClassWithoutDefaultConstructor +-keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClass +-keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor # This rule has the side-effect that R8 still removes the no-args constructor, but does not make the class abstract --keep class com.example.DefaultConstructorMain$TestClassNotAbstract { +-keep class com.example.NoSerializedNameMain$TestClassNotAbstract { @com.google.gson.annotations.SerializedName ; } diff --git a/shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java index 6296237f66..6cff119011 100644 --- a/shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java +++ b/shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java @@ -2,6 +2,10 @@ import com.google.gson.annotations.SerializedName; +/** + * Class with no-args default constructor and with field annotated with + * {@link SerializedName}. + */ public class ClassWithDefaultConstructor { @SerializedName("myField") public int i; diff --git a/shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java new file mode 100644 index 0000000000..2af573677a --- /dev/null +++ b/shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java @@ -0,0 +1,17 @@ +package com.example; + +import com.google.gson.annotations.SerializedName; + +/** + * Class without no-args default constructor, but with field annotated with + * {@link SerializedName}. + */ +public class ClassWithoutDefaultConstructor { + @SerializedName("myField") + public int i; + + // Specify explicit constructor with args to remove implicit no-args default constructor + public ClassWithoutDefaultConstructor(int i) { + this.i = i; + } +} diff --git a/shrinker-test/src/main/java/com/example/Main.java b/shrinker-test/src/main/java/com/example/Main.java index 55bbb6377d..488bc03b82 100644 --- a/shrinker-test/src/main/java/com/example/Main.java +++ b/shrinker-test/src/main/java/com/example/Main.java @@ -32,6 +32,7 @@ public static void runTests(BiConsumer outputConsumer) { testNamedFields(outputConsumer); testSerializedName(outputConsumer); + testNoDefaultConstructor(outputConsumer); testNoJdkUnsafe(outputConsumer); testEnum(outputConsumer); @@ -89,6 +90,16 @@ private static void testSerializedName(BiConsumer outputConsumer }); } + private static void testNoDefaultConstructor(BiConsumer outputConsumer) { + Gson gson = new GsonBuilder().setPrettyPrinting().create(); + TestExecutor.run(outputConsumer, "Write: No default constructor", () -> toJson(gson, new ClassWithoutDefaultConstructor(2))); + // This most likely relies on JDK Unsafe (unless the shrinker rewrites the constructor in some way) + TestExecutor.run(outputConsumer, "Read: No default constructor", () -> { + ClassWithoutDefaultConstructor deserialized = fromJson(gson, "{\"myField\": 3}", ClassWithoutDefaultConstructor.class); + return Integer.toString(deserialized.i); + }); + } + private static void testNoJdkUnsafe(BiConsumer outputConsumer) { Gson gson = new GsonBuilder().disableJdkUnsafe().create(); TestExecutor.run(outputConsumer, "Read: No JDK Unsafe; initial constructor value", () -> { diff --git a/shrinker-test/src/main/java/com/example/DefaultConstructorMain.java b/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java similarity index 60% rename from shrinker-test/src/main/java/com/example/DefaultConstructorMain.java rename to shrinker-test/src/main/java/com/example/NoSerializedNameMain.java index 4f0152f7d1..cd70af34a7 100644 --- a/shrinker-test/src/main/java/com/example/DefaultConstructorMain.java +++ b/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java @@ -4,32 +4,32 @@ import com.google.gson.Gson; import com.google.gson.GsonBuilder; -import com.google.gson.annotations.SerializedName; -public class DefaultConstructorMain { +/** + * Covers cases of classes which don't use {@code @SerializedName} on their fields, and are + * therefore not matched by the default {@code gson.pro} rules. + */ +public class NoSerializedNameMain { static class TestClass { public String s; } - // R8 rule for this class still removes no-args constructor, but doesn't make class abstract + // R8 test rule in r8.pro for this class still removes no-args constructor, but doesn't make class abstract static class TestClassNotAbstract { public String s; } - // Current Gson ProGuard rules only keep default constructor (and only then prevent R8 from - // making class abstract); other constructors are ignored to suggest to user adding default - // constructor instead of implicitly relying on JDK Unsafe static class TestClassWithoutDefaultConstructor { - @SerializedName("s") public String s; + // Specify explicit constructor with args to remove implicit no-args default constructor public TestClassWithoutDefaultConstructor(String s) { this.s = s; } } /** - * Main entrypoint, called by {@code ShrinkingIT.testDefaultConstructor()}. + * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_DefaultConstructor()}. */ public static String runTest() { TestClass deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClass.class)); @@ -37,7 +37,7 @@ public static String runTest() { } /** - * Main entrypoint, called by {@code ShrinkingIT.testDefaultConstructorNoJdkUnsafe()}. + * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_DefaultConstructorNoJdkUnsafe()}. */ public static String runTestNoJdkUnsafe() { Gson gson = new GsonBuilder().disableJdkUnsafe().create(); @@ -46,7 +46,7 @@ public static String runTestNoJdkUnsafe() { } /** - * Main entrypoint, called by {@code ShrinkingIT.testNoDefaultConstructor()}. + * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_NoDefaultConstructor()}. */ public static String runTestNoDefaultConstructor() { TestClassWithoutDefaultConstructor deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClassWithoutDefaultConstructor.class)); diff --git a/shrinker-test/src/main/java/com/example/UnusedClass.java b/shrinker-test/src/main/java/com/example/UnusedClass.java new file mode 100644 index 0000000000..18d452f949 --- /dev/null +++ b/shrinker-test/src/main/java/com/example/UnusedClass.java @@ -0,0 +1,17 @@ +package com.example; + +import com.google.gson.annotations.SerializedName; + +/** + * Class with no-args constructor and field annotated with {@code @SerializedName}, + * but which is not actually used anywhere in the code. + * + *

The default ProGuard rules should not keep this class. + */ +public class UnusedClass { + public UnusedClass() { + } + + @SerializedName("i") + public int i; +} diff --git a/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java b/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java index 3304570197..be5a6213f7 100644 --- a/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java +++ b/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java @@ -19,7 +19,9 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; +import com.example.UnusedClass; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.net.URL; @@ -127,6 +129,14 @@ public void test() throws Exception { "Read: SerializedName", "3", "===", + "Write: No default constructor", + "{", + " \"myField\": 2", + "}", + "===", + "Read: No default constructor", + "3", + "===", "Read: No JDK Unsafe; initial constructor value", "-3", "===", @@ -181,8 +191,8 @@ public void test() throws Exception { } @Test - public void testDefaultConstructor() throws Exception { - runTest("com.example.DefaultConstructorMain", c -> { + public void testNoSerializedName_DefaultConstructor() throws Exception { + runTest("com.example.NoSerializedNameMain", c -> { Method m = c.getMethod("runTest"); if (jarToTest.equals(PROGUARD_RESULT_PATH)) { @@ -193,7 +203,7 @@ public void testDefaultConstructor() throws Exception { Exception e = assertThrows(InvocationTargetException.class, () -> m.invoke(null)); assertThat(e).hasCauseThat().hasMessageThat().isEqualTo( "Abstract classes can't be instantiated! Adjust the R8 configuration or register an InstanceCreator" - + " or a TypeAdapter for this type. Class name: com.example.DefaultConstructorMain$TestClass" + + " or a TypeAdapter for this type. Class name: com.example.NoSerializedNameMain$TestClass" + "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class" ); } @@ -201,8 +211,8 @@ public void testDefaultConstructor() throws Exception { } @Test - public void testDefaultConstructorNoJdkUnsafe() throws Exception { - runTest("com.example.DefaultConstructorMain", c -> { + public void testNoSerializedName_DefaultConstructorNoJdkUnsafe() throws Exception { + runTest("com.example.NoSerializedNameMain", c -> { Method m = c.getMethod("runTestNoJdkUnsafe"); if (jarToTest.equals(PROGUARD_RESULT_PATH)) { @@ -212,7 +222,7 @@ public void testDefaultConstructorNoJdkUnsafe() throws Exception { // R8 performs more aggressive optimizations Exception e = assertThrows(InvocationTargetException.class, () -> m.invoke(null)); assertThat(e).hasCauseThat().hasMessageThat().isEqualTo( - "Unable to create instance of class com.example.DefaultConstructorMain$TestClassNotAbstract;" + "Unable to create instance of class com.example.NoSerializedNameMain$TestClassNotAbstract;" + " usage of JDK Unsafe is disabled. Registering an InstanceCreator or a TypeAdapter for this type," + " adding a no-args constructor, or enabling usage of JDK Unsafe may fix this problem. Or adjust" + " your R8 configuration to keep the no-args constructor of the class." @@ -222,8 +232,8 @@ public void testDefaultConstructorNoJdkUnsafe() throws Exception { } @Test - public void testNoDefaultConstructor() throws Exception { - runTest("com.example.DefaultConstructorMain", c -> { + public void testNoSerializedName_NoDefaultConstructor() throws Exception { + runTest("com.example.NoSerializedNameMain", c -> { Method m = c.getMethod("runTestNoDefaultConstructor"); if (jarToTest.equals(PROGUARD_RESULT_PATH)) { @@ -234,10 +244,24 @@ public void testNoDefaultConstructor() throws Exception { Exception e = assertThrows(InvocationTargetException.class, () -> m.invoke(null)); assertThat(e).hasCauseThat().hasMessageThat().isEqualTo( "Abstract classes can't be instantiated! Adjust the R8 configuration or register an InstanceCreator" - + " or a TypeAdapter for this type. Class name: com.example.DefaultConstructorMain$TestClassWithoutDefaultConstructor" + + " or a TypeAdapter for this type. Class name: com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor" + "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class" ); } }); } + + @Test + public void testUnusedClassRemoved() throws Exception { + // For some reason this test only works for R8 but not for ProGuard; ProGuard keeps the unused class + assumeTrue(jarToTest.equals(R8_RESULT_PATH)); + + String className = UnusedClass.class.getName(); + ClassNotFoundException e = assertThrows(ClassNotFoundException.class, () -> { + runTest(className, c -> { + fail("Class should have been removed during shrinking: " + c); + }); + }); + assertThat(e).hasMessageThat().contains(className); + } }