Don't use the serializable type for parcelable arrays, see #53

Evernote · Jun 10, 2018 · f7a4dbf · f7a4dbf
1 parent 44f6762
commit f7a4dbf
Show file tree

Hide file tree

Showing 8 changed files with 175 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## 1.3.1
 
 * Avoid obfuscating the class name if the class contains a field annotated with `@State`, see #43
+* Don't use the serializable type for parcelable arrays, see #53
 
 ## 1.3.0 (2018-05-28)
 

diff --git a/library/src/test/java/com/evernote/android/state/test/BundlingTest.java b/library/src/test/java/com/evernote/android/state/test/BundlingTest.java
@@ -317,6 +317,23 @@ public void testPrivateInnerClass() {
         assertThat(testPrivateInnerClass.isB()).isTrue();
     }
 
+    @Test
+    public void testParcelableArray() {
+        TestParcelableArray testParcelableArray = createSavedInstance(TestParcelableArray.class);
+        testParcelableArray.setToValue(1);
+
+        StateSaver.restoreInstanceState(testParcelableArray, mBundle);
+        assertThat(testParcelableArray.isValue(0)).isTrue();
+
+        testParcelableArray.setToValue(1);
+
+        StateSaver.saveInstanceState(testParcelableArray, mBundle);
+        testParcelableArray.setToValue(0);
+
+        StateSaver.restoreInstanceState(testParcelableArray, mBundle);
+        assertThat(testParcelableArray.isValue(1)).isTrue();
+    }
+
     private <T> T createSavedInstance(Class<T> clazz) {
         try {
             T instance = clazz.newInstance();

diff --git a/library/src/test/java/com/evernote/android/state/test/TestParcelableArray.java b/library/src/test/java/com/evernote/android/state/test/TestParcelableArray.java
@@ -0,0 +1,36 @@
+package com.evernote.android.state.test;
+
+import com.evernote.android.state.State;
+import com.evernote.android.state.StateReflection;
+
+/**
+ * @author rwondratschek
+ */
+public class TestParcelableArray {
+    @State
+    public TestTypes.ParcelableImpl[] mParcelableArrayImpl1 = new TestTypes.ParcelableImpl[]{new TestTypes.ParcelableImpl(0)};
+
+    @State
+    private TestTypes.ParcelableImpl[] mParcelableArrayImpl2 = new TestTypes.ParcelableImpl[]{new TestTypes.ParcelableImpl(0)};
+
+    @StateReflection
+    private TestTypes.ParcelableImpl[] mParcelableArrayImpl3 = new TestTypes.ParcelableImpl[]{new TestTypes.ParcelableImpl(0)};
+
+    public TestTypes.ParcelableImpl[] getParcelableArrayImpl2() {
+        return mParcelableArrayImpl2;
+    }
+
+    public void setParcelableArrayImpl2(TestTypes.ParcelableImpl[] parcelableArrayImpl2) {
+        mParcelableArrayImpl2 = parcelableArrayImpl2;
+    }
+
+    public void setToValue(int value) {
+        mParcelableArrayImpl1 = new TestTypes.ParcelableImpl[]{new TestTypes.ParcelableImpl(value)};
+        mParcelableArrayImpl2 = new TestTypes.ParcelableImpl[]{new TestTypes.ParcelableImpl(value)};
+        mParcelableArrayImpl3 = new TestTypes.ParcelableImpl[]{new TestTypes.ParcelableImpl(value)};
+    }
+
+    public boolean isValue(int value) {
+        return mParcelableArrayImpl1[0].isValue(value) && mParcelableArrayImpl2[0].isValue(value) && mParcelableArrayImpl3[0].isValue(value);
+    }
+}
diff --git a/library/src/test/java/com/evernote/android/state/test/TestTypes.java b/library/src/test/java/com/evernote/android/state/test/TestTypes.java
@@ -97,6 +97,10 @@ protected ParcelableImpl(Parcel in) {
             mInt = in.readInt();
         }
 
+        public boolean isValue(int value) {
+            return mInt == value;
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;

diff --git a/library/src/test/kotlin/com/evernote/android/state/test/KotlinBundlingTest.kt b/library/src/test/kotlin/com/evernote/android/state/test/KotlinBundlingTest.kt
@@ -195,23 +195,45 @@ class KotlinBundlingTest {
 
     @Test
     fun testPrivateInnerClass() {
-        val item = TestPrivateInnerClass()
-        assertThat(item.isA).isTrue()
+        val item = TestKotlinPrivateInnerClass()
+        assertThat(item.isA()).isTrue()
 
         val bundle = Bundle()
         StateSaver.saveInstanceState(item, bundle)
 
         item.setToB()
 
         StateSaver.restoreInstanceState(item, bundle)
-        assertThat(item.isA).isTrue()
+        assertThat(item.isA()).isTrue()
 
         item.setToB()
         StateSaver.saveInstanceState(item, bundle)
 
         item.setToA()
 
         StateSaver.restoreInstanceState(item, bundle)
-        assertThat(item.isB).isTrue()
+        assertThat(item.isB()).isTrue()
+    }
+
+    @Test
+    fun testParcelableArray() {
+        val item = TestKotlinParcelableArray()
+        assertThat(item.isValue(0)).isTrue()
+
+        val bundle = Bundle()
+        StateSaver.saveInstanceState(item, bundle)
+
+        item.setToValue(1)
+
+        StateSaver.restoreInstanceState(item, bundle)
+        assertThat(item.isValue(0)).isTrue()
+
+        item.setToValue(1)
+        StateSaver.saveInstanceState(item, bundle)
+
+        item.setToValue(0)
+
+        StateSaver.restoreInstanceState(item, bundle)
+        assertThat(item.isValue(1)).isTrue()
     }
 }
diff --git a/library/src/test/kotlin/com/evernote/android/state/test/TestKotlinParcelableArray.kt b/library/src/test/kotlin/com/evernote/android/state/test/TestKotlinParcelableArray.kt
@@ -0,0 +1,24 @@
+package com.evernote.android.state.test
+
+import com.evernote.android.state.State
+import com.evernote.android.state.StateReflection
+
+/**
+ * @author rwondratschek
+ */
+class TestKotlinParcelableArray {
+    @State
+    var parcelableArrayImpl1: Array<TestTypes.ParcelableImpl> = arrayOf(TestTypes.ParcelableImpl(0))
+
+    @StateReflection
+    private var mParcelableArrayImpl2: Array<TestTypes.ParcelableImpl> = arrayOf(TestTypes.ParcelableImpl(0))
+
+    fun setToValue(value: Int) {
+        parcelableArrayImpl1 = arrayOf(TestTypes.ParcelableImpl(value))
+        mParcelableArrayImpl2 = arrayOf(TestTypes.ParcelableImpl(value))
+    }
+
+    fun isValue(value: Int): Boolean {
+        return parcelableArrayImpl1[0].isValue(value) && mParcelableArrayImpl2[0].isValue(value)
+    }
+}
diff --git a/processor/src/main/java/com/evernote/android/state/StateProcessor.java b/processor/src/main/java/com/evernote/android/state/StateProcessor.java
@@ -57,6 +57,7 @@
 import javax.lang.model.element.Modifier;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.VariableElement;
+import javax.lang.model.type.ArrayType;
 import javax.lang.model.type.DeclaredType;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeMirror;
@@ -118,6 +119,7 @@ public int compare(Element o1, Element o2) {
     private static final String STATE_REFLECTION_CLASS_NAME = StateReflection.class.getName();
     private static final String OBJECT_CLASS_NAME = Object.class.getName();
     private static final String PARCELABLE_CLASS_NAME = Parcelable.class.getName();
+    private static final String PARCELABLE_ARRAY_CLASS_NAME = Parcelable[].class.getCanonicalName();
     private static final String SERIALIZABLE_CLASS_NAME = Serializable.class.getName();
 
     private static final Set<String> GENERIC_SUPER_TYPES = Collections.unmodifiableSet(new HashSet<String>() {{
@@ -317,6 +319,13 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
 
                 String fieldName = fieldType.getFieldName(field);
 
+                String castedType = null;
+                CompatibilityType compatibilityType = getCompatibilityType(field);
+                if (compatibilityType == CompatibilityType.PARCELABLE_ARRAY && !PARCELABLE_ARRAY_CLASS_NAME.equals(fieldTypeString)) {
+                    mMessager.printMessage(Diagnostic.Kind.NOTE, "Field " + fieldTypeString + " " + PARCELABLE_ARRAY_CLASS_NAME);
+                    castedType = fieldTypeString;
+                }
+
                 BundlerWrapper bundler = bundlers.get(field);
                 if (bundler != null) {
                     staticInitBlock = staticInitBlock.addStatement("BUNDLERS.put($S, new $T())", fieldName, bundler.mBundlerName);
@@ -326,7 +335,11 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
                 switch (fieldType) {
                     case FIELD:
                         saveMethodBuilder = saveMethodBuilder.addStatement("HELPER.put$N(state, $S, target.$N)", mapping, fieldName, fieldName);
-                        restoreMethodBuilder = restoreMethodBuilder.addStatement("target.$N = HELPER.get$N(state, $S)", fieldName, mapping, fieldName);
+                        if (castedType != null) {
+                            restoreMethodBuilder = restoreMethodBuilder.addStatement("target.$N = ($N) HELPER.get$N(state, $S)", fieldName, castedType, mapping, fieldName);
+                        } else {
+                            restoreMethodBuilder = restoreMethodBuilder.addStatement("target.$N = HELPER.get$N(state, $S)", fieldName, mapping, fieldName);
+                        }
                         break;
 
                     case BOOLEAN_PROPERTY:
@@ -356,6 +369,9 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
                             if (insertedType != null) {
                                 restoreMethodBuilder = restoreMethodBuilder.addStatement("target.set$N(HELPER.<$T>get$N(state, $S))",
                                         fieldName, ClassName.get(insertedType.mTypeMirror), mapping, fieldName);
+                            } else if (castedType != null) {
+                                restoreMethodBuilder = restoreMethodBuilder.addStatement("target.set$N(($N) HELPER.get$N(state, $S))",
+                                        fieldName, castedType, mapping, fieldName);
                             } else {
                                 restoreMethodBuilder = restoreMethodBuilder.addStatement("target.set$N(HELPER.get$N(state, $S))",
                                         fieldName, mapping, fieldName);
@@ -366,7 +382,6 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
                     case FIELD_REFLECTION:
                         String reflectionMapping = isPrimitiveMapping(mapping) ? mapping : "";
 
-                        CompatibilityType compatibilityType = getCompatibilityType(field);
                         InsertedTypeResult insertedType = getInsertedType(field, true);
                         if (compatibilityType != null && insertedType != null) {
                             // either serializable or parcelable, this could be a private inner class, so don't use the concrete type
@@ -486,7 +501,7 @@ private boolean writeJavaFile(JavaFile javaFile) {
         }
     }
 
-    public HashMap<String, List<Element>> getMapGeneratedFileToOriginatingElements() {
+    /*package*/ HashMap<String, List<Element>> getMapGeneratedFileToOriginatingElements() {
         return mMapGeneratedFileToOriginatingElements;
     }
 
@@ -786,6 +801,7 @@ private BundlerWrapper(TypeName bundlerName, TypeName genericName) {
     @SuppressWarnings("unused")
     private enum CompatibilityType {
         PARCELABLE("Parcelable", Parcelable.class, null),
+        PARCELABLE_ARRAY("ParcelableArray", Parcelable[].class, null),
         PARCELABLE_LIST("ParcelableArrayList", ArrayList.class, Parcelable.class),
         SPARSE_PARCELABLE_ARRAY("SparseParcelableArray", SparseArray.class, Parcelable.class),
         SERIALIZABLE("Serializable", Serializable.class, null);
@@ -804,7 +820,13 @@ private enum CompatibilityType {
     private CompatibilityType getCompatibilityType(Element field) {
         TypeMirror typeMirror = field.asType();
         for (CompatibilityType compatibilityType : CompatibilityType.values()) {
-            if (compatibilityType.mGenericClass == null) {
+            if (compatibilityType == CompatibilityType.PARCELABLE_ARRAY) {
+                TypeMirror arrayType = getArrayType(field);
+                if (arrayType != null && isAssignable(arrayType, Parcelable.class)) {
+                    return CompatibilityType.PARCELABLE_ARRAY;
+                }
+
+            } else if (compatibilityType.mGenericClass == null) {
                 if (isAssignable(typeMirror, compatibilityType.mClass)) {
                     return compatibilityType;
                 }
@@ -821,6 +843,15 @@ private CompatibilityType getCompatibilityType(Element field) {
         return null;
     }
 
+    private TypeMirror getArrayType(Element field) {
+        TypeMirror typeMirror = field.asType();
+        if (typeMirror instanceof ArrayType) {
+            return ((ArrayType) typeMirror).getComponentType();
+        } else {
+            return null;
+        }
+    }
+
     @SuppressWarnings("SameParameterValue")
     private boolean isAssignable(Element element, Class<?> clazz) {
         return isAssignable(element.asType(), clazz);

diff --git a/processor/src/test/java/com/evernote/android/state/TestProcessor.java b/processor/src/test/java/com/evernote/android/state/TestProcessor.java
@@ -15,7 +15,6 @@
 import com.google.testing.compile.JavaFileObjects;
 
 import org.junit.FixMethodOrder;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runners.MethodSorters;
 
@@ -651,6 +650,34 @@ public void testAllTypes() {
                 + "}\n");
 
         JavaFileObject expected = JavaFileObjects.forSourceString(getName(className, true), ""
+                + "/* *****************************************************************************\n"
+                + " * Copyright (c) 2017 Evernote Corporation.\n"
+                + " * This software was generated by Evernote’s Android-State code generator\n"
+                + " * (available at https://github.com/evernote/android-state), which is made\n"
+                + " * available under the terms of the Eclipse Public License v1.0\n"
+                + " * (available at http://www.eclipse.org/legal/epl-v10.html).\n"
+                + " * For clarification, code generated by the Android-State code generator is\n"
+                + " * not subject to the Eclipse Public License and can be used subject to the\n"
+                + " * following terms:\n"
+                + " *\n"
+                + " * Permission is hereby granted, free of charge, to any person obtaining a copy\n"
+                + " * of this software and associated documentation files (the \"Software\"), to deal\n"
+                + " * in the Software without restriction, including without limitation the rights\n"
+                + " * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell\n"
+                + " * copies of the Software, and to permit persons to whom the Software is\n"
+                + " * furnished to do so, subject to the following conditions:\n"
+                + " *\n"
+                + " * The above copyright notice and this permission notice shall be included in all\n"
+                + " * copies or substantial portions of the Software.\n"
+                + " *\n"
+                + " * THE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR\n"
+                + " * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,\n"
+                + " * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE\n"
+                + " * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER\n"
+                + " * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\n"
+                + " * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\n"
+                + " * SOFTWARE.\n"
+                + " *******************************************************************************/\n"
                 + "package com.evernote.android.state.test;\n"
                 + "\n"
                 + "import android.os.Bundle;\n"
@@ -753,15 +780,14 @@ public void testAllTypes() {
                 + "    target.mShort = HELPER.getShort(state, \"mShort\");\n"
                 + "    target.mShortArray = HELPER.getShortArray(state, \"mShortArray\");\n"
                 + "  }\n"
-                + "}\n");
+                + "}");
 
         Compilation compilation = Compiler.javac().withProcessors(new StateProcessor()).compile(javaFileObject);
         assertThat(compilation).succeeded();
         assertThat(compilation).generatedSourceFile(getName(className, true)).hasSourceEquivalentTo(expected);
     }
 
     @Test
-    @Ignore
     public void testReflection() {
         String className = "TestTypesReflection";
         JavaFileObject javaFileObject = JavaFileObjects.forSourceString(getName(className), ""
@@ -1043,8 +1069,7 @@ public void testReflection() {
                 + "      if (!accessible) {\n"
                 + "        field.setAccessible(true);\n"
                 + "      }\n"
-                + "      HELPER.putSparseParcelableArray(state, \"mParcelableSparseArray\", "
-                + "(android.util.SparseArray<com.evernote.android.state.test.TestTypes.ParcelableImpl>) field.get(target));\n"
+                + "      HELPER.putSparseParcelableArray(state, \"mParcelableSparseArray\", (android.util.SparseArray) field.get(target));\n"
                 + "      if (!accessible) {\n"
                 + "        field.setAccessible(false);\n"
                 + "      }\n"
@@ -1096,8 +1121,7 @@ public void testReflection() {
                 + "      if (!accessible) {\n"
                 + "        field.setAccessible(true);\n"
                 + "      }\n"
-                + "      HELPER.putSerializable(state, \"mParcelableArrayList\", (java.util.ArrayList<? "
-                + "extends com.evernote.android.state.test.TestTypes.ParcelableImpl>) field.get(target));\n"
+                + "      HELPER.putSerializable(state, \"mParcelableArrayList\", (java.io.Serializable) field.get(target));\n"
                 + "      if (!accessible) {\n"
                 + "        field.setAccessible(false);\n"
                 + "      }\n"
@@ -1201,7 +1225,7 @@ public void testReflection() {
                 + "      throw new RuntimeException(e);\n"
                 + "    }\n"
                 + "  }\n"
-                + "}\n");
+                + "}");
 
         Compilation compilation = Compiler.javac().withProcessors(new StateProcessor()).compile(javaFileObject, javaFileObjectInnerClass);
         assertThat(compilation).succeeded();