diff options
5 files changed, 76 insertions, 5 deletions
diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoAnnotationTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoAnnotationTest.java index 10ef4cba..fd2b62d6 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoAnnotationTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoAnnotationTest.java @@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.testing.EqualsTester; import com.google.common.testing.SerializableTester; +import java.io.ObjectStreamClass; import java.lang.annotation.Annotation; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -414,6 +415,28 @@ public class AutoAnnotationTest { } } + @Test + @SuppressWarnings("GetClassOnAnnotation") // yes, we really do want the implementation classes + public void testSerialVersionUid() { + Class<? extends Everything> everythingImpl = EVERYTHING_FROM_AUTO.getClass(); + Class<? extends Everything> everythingFromCollectionsImpl = + EVERYTHING_FROM_AUTO_COLLECTIONS.getClass(); + assertThat(everythingImpl).isNotEqualTo(everythingFromCollectionsImpl); + long everythingUid = ObjectStreamClass.lookup(everythingImpl).getSerialVersionUID(); + long everythingFromCollectionsUid = + ObjectStreamClass.lookup(everythingFromCollectionsImpl).getSerialVersionUID(); + // Two different implementations of the same annotation with the same members being provided + // (not defaulted) should have the same serialVersionUID. They won't be serial-compatible, of + // course, because their classes are different. So we're really just checking that the + // serialVersionUID depends only on the names and types of those members. + assertThat(everythingFromCollectionsUid).isEqualTo(everythingUid); + Class<? extends StringValues> stringValuesImpl = newStringValues(new String[0]).getClass(); + long stringValuesUid = ObjectStreamClass.lookup(stringValuesImpl).getSerialVersionUID(); + // The previous assertion would be vacuously true if every implementation had the same + // serialVersionUID, so check that that's not true. + assertThat(stringValuesUid).isNotEqualTo(everythingUid); + } + public static class IntList extends ArrayList<Integer> { IntList(Collection<Integer> c) { super(c); diff --git a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java index d3cd9bd5..8f01a01e 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java @@ -17,6 +17,9 @@ package com.google.auto.value.processor; import static com.google.auto.common.GeneratedAnnotations.generatedAnnotation; import static com.google.auto.value.processor.ClassNames.AUTO_ANNOTATION_NAME; +import static com.google.common.collect.Maps.immutableEntry; +import static java.util.Comparator.comparing; +import static java.util.stream.Collectors.joining; import com.google.auto.common.MoreElements; import com.google.auto.common.SuperficialValidation; @@ -25,6 +28,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.hash.Hashing; import com.google.common.primitives.Primitives; import com.google.errorprone.annotations.FormatMethod; import java.io.IOException; @@ -172,6 +176,7 @@ public class AutoAnnotationProcessor extends AbstractProcessor { vars.pkg = pkg; vars.wrapperTypesUsedInCollections = wrapperTypesUsedInCollections; vars.gwtCompatible = isGwtCompatible(annotationElement); + vars.serialVersionUID = computeSerialVersionUid(members, parameters); ImmutableMap<String, Integer> invariableHashes = invariableHashes(members, parameters.keySet()); vars.invariableHashSum = 0; for (int h : invariableHashes.values()) { @@ -448,6 +453,43 @@ public class AutoAnnotationProcessor extends AbstractProcessor { return pkg.isEmpty() ? cls : pkg + "." + cls; } + /** + * We compute a {@code serialVersionUID} for the generated class based on the names and types of + * the annotation members that the {@code @AutoAnnotation} method defines. These are exactly the + * names and types of the instance fields in the generated class. So in the common case where the + * annotation acquires a new member with a default value, if the {@code @AutoAnnotation} method is + * not changed then the generated class will acquire an implementation of the new member method + * which just returns the default value. The {@code serialVersionUID} will not change, which makes + * sense because the instance fields haven't changed, and instances that were serialized before + * the new member was added should deserialize fine. On the other hand, if you then add a + * parameter to the {@code @AutoAnnotation} method for the new member, the implementation class + * will acquire a new instance field, and we will compute a different {@code serialVersionUID}. + * That's because an instance serialized before that change would not have a value for the new + * instance field, which would end up zero or null. Users don't expect annotation methods to + * return null so that would be bad. + * + * <p>We could instead add a {@code readObject(ObjectInputStream)} method that would check that + * all of the instance fields are really present in the deserialized instance, and perhaps + * replace them with their default values from the annotation if not. That seems a lot more + * complicated than is justified, though, especially since the instance fields are final and + * would have to be set in the deserialized object through reflection. + */ + private static long computeSerialVersionUid( + ImmutableMap<String, Member> members, ImmutableMap<String, Parameter> parameters) { + // TypeMirror.toString() isn't fully specified so it could potentially differ between + // implementations. Our member.getType() string comes from TypeEncoder and is predictable, but + // it includes `...` markers around fully-qualified type names, which are used to handle + // imports. So we remove those markers below. + String namesAndTypesString = + members.entrySet().stream() + .filter(e -> parameters.containsKey(e.getKey())) + .map(e -> immutableEntry(e.getKey(), e.getValue().getType().replace("`", ""))) + .sorted(comparing(Map.Entry::getKey)) + .map(e -> e.getKey() + ":" + e.getValue()) + .collect(joining(";")); + return Hashing.murmur3_128().hashUnencodedChars(namesAndTypesString).asLong(); + } + private void writeSourceFile(String className, String text, TypeElement originatingType) { try { JavaFileObject sourceFile = diff --git a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationTemplateVars.java b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationTemplateVars.java index 4db59c78..dc39e06b 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationTemplateVars.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationTemplateVars.java @@ -74,6 +74,12 @@ class AutoAnnotationTemplateVars extends TemplateVars { /** The sum of the hash code contributions from the members in {@link #invariableHashes}. */ Integer invariableHashSum; + /** + * A computed {@code serialVersionUID} based on the names and types of the {@code @AutoAnnotation} + * method parameters. + */ + Long serialVersionUID; + private static final Template TEMPLATE = parsedTemplateForResource("autoannotation.vm"); @Override diff --git a/value/src/main/java/com/google/auto/value/processor/autoannotation.vm b/value/src/main/java/com/google/auto/value/processor/autoannotation.vm index 5d697d22..590992ee 100644 --- a/value/src/main/java/com/google/auto/value/processor/autoannotation.vm +++ b/value/src/main/java/com/google/auto/value/processor/autoannotation.vm @@ -46,7 +46,7 @@ package $pkg; // Generated by com.google.auto.value.processor.AutoAnnotationProcessor #end final class $className implements $annotationName, `java.io.Serializable` { - private static final long serialVersionUID = 1L; + private static final long serialVersionUID = ${serialVersionUID}L; ## Fields diff --git a/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java index f30f519d..f8070245 100644 --- a/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java @@ -80,7 +80,7 @@ public class AutoAnnotationCompilationTest { "@Generated(\"" + AutoAnnotationProcessor.class.getName() + "\")", "final class AutoAnnotation_AnnotationFactory_newMyAnnotation", " implements MyAnnotation, Serializable {", - " private static final long serialVersionUID = 1L;", + " private static final long serialVersionUID = -7473814294717163169L;", " private final MyEnum value;", " private static final int defaultedValue = 23;", "", @@ -165,7 +165,7 @@ public class AutoAnnotationCompilationTest { "@Generated(\"" + AutoAnnotationProcessor.class.getName() + "\")", "final class AutoAnnotation_AnnotationFactory_newMyAnnotation", " implements MyAnnotation, Serializable {", - " private static final long serialVersionUID = 1L;", + " private static final long serialVersionUID = 0L;", " AutoAnnotation_AnnotationFactory_newMyAnnotation() {", " }", "", @@ -248,7 +248,7 @@ public class AutoAnnotationCompilationTest { "@Generated(\"" + AutoAnnotationProcessor.class.getName() + "\")", "final class AutoAnnotation_AnnotationFactory_newMyAnnotation implements MyAnnotation," + " Serializable {", - " private static final long serialVersionUID = 1L;", + " private static final long serialVersionUID = -8116050813861599066L;", " private final int[] value;", "", " AutoAnnotation_AnnotationFactory_newMyAnnotation(int[] value) {", @@ -359,7 +359,7 @@ public class AutoAnnotationCompilationTest { "@Generated(\"" + AutoAnnotationProcessor.class.getName() + "\")", "final class AutoAnnotation_AnnotationFactory_newMyAnnotation implements MyAnnotation," + " Serializable {", - " private static final long serialVersionUID = 1L;", + " private static final long serialVersionUID = -2102364343628921304L;", " private final int[] value;", " private final MyEnum[] enums;", "", |