From f7a4dbff71c814fec412f1cb9fed70490e88c391 Mon Sep 17 00:00:00 2001 From: Ralf Wondratschek Date: Sun, 10 Jun 2018 15:12:16 -0700 Subject: [PATCH] Don't use the serializable type for parcelable arrays, see #53 --- CHANGELOG.md | 1 + .../android/state/test/BundlingTest.java | 17 ++++++++ .../state/test/TestParcelableArray.java | 36 +++++++++++++++++ .../android/state/test/TestTypes.java | 4 ++ .../android/state/test/KotlinBundlingTest.kt | 30 ++++++++++++-- .../state/test/TestKotlinParcelableArray.kt | 24 +++++++++++ .../android/state/StateProcessor.java | 39 ++++++++++++++++-- .../evernote/android/state/TestProcessor.java | 40 +++++++++++++++---- 8 files changed, 175 insertions(+), 16 deletions(-) create mode 100644 library/src/test/java/com/evernote/android/state/test/TestParcelableArray.java create mode 100644 library/src/test/kotlin/com/evernote/android/state/test/TestKotlinParcelableArray.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index f3d3c6a..d67feda 100644 --- 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 index baf5c74..c8c7d33 100644 --- 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 createSavedInstance(Class 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 new file mode 100644 index 0000000..141d93a --- /dev/null +++ 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 index b1096eb..08fb393 100644 --- 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 index d3780ea..7d29c7b 100644 --- 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,8 +195,8 @@ 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) @@ -204,7 +204,7 @@ class KotlinBundlingTest { item.setToB() StateSaver.restoreInstanceState(item, bundle) - assertThat(item.isA).isTrue() + assertThat(item.isA()).isTrue() item.setToB() StateSaver.saveInstanceState(item, bundle) @@ -212,6 +212,28 @@ class KotlinBundlingTest { 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() } } \ No newline at end of file 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 new file mode 100644 index 0000000..a526ce0 --- /dev/null +++ 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 = arrayOf(TestTypes.ParcelableImpl(0)) + + @StateReflection + private var mParcelableArrayImpl2: Array = 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) + } +} \ No newline at end of file diff --git a/processor/src/main/java/com/evernote/android/state/StateProcessor.java b/processor/src/main/java/com/evernote/android/state/StateProcessor.java index a60e40f..d30be42 100644 --- 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 GENERIC_SUPER_TYPES = Collections.unmodifiableSet(new HashSet() {{ @@ -317,6 +319,13 @@ public boolean process(Set 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 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 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 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> getMapGeneratedFileToOriginatingElements() { + /*package*/ HashMap> 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 index 9480ffa..29c1b82 100644 --- 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,7 +780,7 @@ 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(); @@ -761,7 +788,6 @@ public void testAllTypes() { } @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) 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) 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();