Compiler independent outer class field deserialization Different compilers generate different names for the outer class synthetic fields. The change tries to infer the new name where possible.
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/ba0e6480 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/ba0e6480 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/ba0e6480 Branch: refs/heads/master Commit: ba0e648058bd861122c6ccf9d6dcfe0fdfbfd6f3 Parents: 96a5ab1 Author: Svetoslav Neykov <[email protected]> Authored: Thu Feb 19 14:26:35 2015 +0200 Committer: Svetoslav Neykov <[email protected]> Committed: Tue Mar 3 17:42:04 2015 +0200 ---------------------------------------------------------------------- .../rebind/persister/XmlMementoSerializer.java | 6 +- ...ompilerIndependentOuterClassFieldMapper.java | 150 ++++++++++++++++++ .../brooklyn/util/xstream/XmlSerializer.java | 2 +- .../util/xstream/CompilerCompatibilityTest.java | 154 +++++++++++++++++++ .../rebind/compiler_compatibility_eclipse.xml | 41 +++++ .../rebind/compiler_compatibility_oracle.xml | 41 +++++ 6 files changed, 391 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ba0e6480/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java b/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java index 0d88bea..4bd1a20 100644 --- a/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java +++ b/core/src/main/java/brooklyn/entity/rebind/persister/XmlMementoSerializer.java @@ -135,8 +135,10 @@ public class XmlMementoSerializer<T> extends XmlSerializer<T> implements Memento // Warning: this is called in the super-class constuctor, so before this constructor! @Override protected MapperWrapper wrapMapper(MapperWrapper next) { - MapperWrapper result = new CustomMapper(next, Entity.class, "entityProxy"); - return new CustomMapper(result, Location.class, "locationProxy"); + MapperWrapper mapper = super.wrapMapper(next); + mapper = new CustomMapper(mapper, Entity.class, "entityProxy"); + mapper = new CustomMapper(mapper, Location.class, "locationProxy"); + return mapper; } @Override http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ba0e6480/core/src/main/java/brooklyn/util/xstream/CompilerIndependentOuterClassFieldMapper.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/util/xstream/CompilerIndependentOuterClassFieldMapper.java b/core/src/main/java/brooklyn/util/xstream/CompilerIndependentOuterClassFieldMapper.java new file mode 100644 index 0000000..7faeb02 --- /dev/null +++ b/core/src/main/java/brooklyn/util/xstream/CompilerIndependentOuterClassFieldMapper.java @@ -0,0 +1,150 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 brooklyn.util.xstream; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.thoughtworks.xstream.mapper.Mapper; +import com.thoughtworks.xstream.mapper.MapperWrapper; + +/** + * <p>Compiler independent outer class field mapper.</p> + * <p>Different compilers generate different indexes for the names of outer class reference + * field (this$N) leading to deserialization errors.</p> + * <ul> + * <li> eclipse-[groovy-]compiler counts all outer static classes + * <li> OpenJDK/Oracle/IBM compiler starts at 0, regardless of the nesting level + * </ul> + * <p>The mapper will be able to update field names for instances with a single this$N + * field only (including those from parent classes).</p> + * <p>For difference between generated field names compare + * {@code src/test/java/brooklyn/util/xstream/compiler_compatibility_eclipse.xml} and + * {@code src/test/java/brooklyn/util/xstream/compiler_compatibility_oracle.xml}, + * generated from {@code brooklyn.util.xstream.CompilerCompatibilityTest}</p> + * <p>JLS 1.1 relevant section, copied verbatim for a lack of reliable URL:</p> + * <blockquote> + * <p>Java 1.1 compilers are strongly encouraged, though not required, to use the + * following naming conventions when implementing inner classes. Compilers may + * not use synthetic names of the forms defined here for any other purposes.</p> + * <p>A synthetic field pointing to the outermost enclosing instance is named this$0. + * The next-outermost enclosing instance is this$1, and so forth. (At most one such + * field is necessary in any given inner class.) A synthetic field containing a copy + * of a constant v is named val$v. These fields are final.</p> + * </blockquote> + * <p>Currently available at + * http://web.archive.org/web/20000830111107/http://java.sun.com/products/jdk/1.1/docs/guide/innerclasses/spec/innerclasses.doc10.html</p> + */ +public class CompilerIndependentOuterClassFieldMapper extends MapperWrapper { + public static final Logger LOG = LoggerFactory.getLogger(CompilerIndependentOuterClassFieldMapper.class); + + private static final String OUTER_CLASS_FIELD_PREFIX = "this$"; + + public CompilerIndependentOuterClassFieldMapper(Mapper wrapped) { + super(wrapped); + } + + @Override + public String realMember(@SuppressWarnings("rawtypes") Class type, String serialized) { + // Let com.thoughtworks.xstream.mapper.OuterClassMapper also run on the input. + String serializedFieldName = super.realMember(type, serialized); + + if (serializedFieldName.startsWith(OUTER_CLASS_FIELD_PREFIX)) { + Collection<String> compiledFieldNames = findOuterClassFieldNames(type); + if (compiledFieldNames.size() == 0) { + throw new IllegalStateException("Unable to find any outer class fields in " + type + ", searching specifically for " + serializedFieldName); + } + + Set<String> uniqueFieldNames = new HashSet<String>(compiledFieldNames); + String deserializeFieldName; + if (!compiledFieldNames.contains(serializedFieldName)) { + String msg = + "Unable to find outer class field " + serializedFieldName + " in class " + type + ". " + + "This could be caused by " + + "1) changing the class (or one of its parents) to a static or " + + "2) moving the class to a different lexical level (enclosing classes) or " + + "3) using a different compiler (i.e eclipse vs oracle) at the time the object was serialized. "; + if (uniqueFieldNames.size() == 1) { + // Try to fix the field naming only for the case with a single field or + // multiple fields with the same name, in which case XStream puts defined-in + // for the field declared in super. + // + // We don't have access to the XML elements from here to check for same name + // so we check the target class instead. This should work most of the time, but + // if code is recompiled in such a way that the new instance has fields with + // different names, where only the field of the extending class is renamed and + // the super field is not, then the instance will be deserialized incorrectly - + // the super field will be assigned both times. If the field type is incompatible + // then a casting exception will be thrown, if it's the same then only the warning + // below will indicate of a possible problem down the line - most probably NPE on + // the this$N field. + deserializeFieldName = compiledFieldNames.iterator().next(); + LOG.warn(msg + "Will use the field " + deserializeFieldName + " instead."); + } else { + // Multiple fields with differing names case - don't try to fix it. + // Better fail with an explicit error, and have someone fix it manually, + // than try to fix it here non-reliably and have it fail down the line + // with some unrelated error. + // XStream will fail later with a field not found exception. + LOG.error(msg + "Will fail with a field not found exception. " + + "Edit the persistence state manually and update the field names. "+ + "Existing field names are " + uniqueFieldNames); + deserializeFieldName = serializedFieldName; + } + } else { + if (uniqueFieldNames.size() > 1) { + // Log at debug level as the actual problem would occur in very specific cases. Only + // useful when the compiler is changed, otherwise leads to false positives. + LOG.debug("Deserializing the non-static class " + type + " with multiple outer class fields " + uniqueFieldNames + ". " + + "When changing compilers it's possible that the instance won't be able to be deserialized due to changed outer class field names. " + + "In those cases deserialization could fail with field not found exception or class cast exception following this log line."); + } + deserializeFieldName = serializedFieldName; + } + + return deserializeFieldName; + } else { + return serializedFieldName; + } + } + + private Collection<String> findOuterClassFieldNames(Class<?> type) { + Collection<String> fields = new ArrayList<String>(); + addOuterClassFields(type, fields); + return fields; + } + + private void addOuterClassFields(Class<?> type, Collection<String> fields) { + for (Field field : type.getDeclaredFields()) { + if (field.isSynthetic()) { + fields.add(field.getName()); + } + } + if (type.getSuperclass() != null) { + addOuterClassFields(type.getSuperclass(), fields); + } + } + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ba0e6480/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java b/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java index ed1dc79..7220254 100644 --- a/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java +++ b/core/src/main/java/brooklyn/util/xstream/XmlSerializer.java @@ -72,7 +72,7 @@ public class XmlSerializer<T> { } protected MapperWrapper wrapMapper(MapperWrapper next) { - return next; + return new CompilerIndependentOuterClassFieldMapper(next); } public void serialize(Object object, Writer writer) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ba0e6480/core/src/test/java/brooklyn/util/xstream/CompilerCompatibilityTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/util/xstream/CompilerCompatibilityTest.java b/core/src/test/java/brooklyn/util/xstream/CompilerCompatibilityTest.java new file mode 100644 index 0000000..5a1f844 --- /dev/null +++ b/core/src/test/java/brooklyn/util/xstream/CompilerCompatibilityTest.java @@ -0,0 +1,154 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 brooklyn.util.xstream; + +import static org.testng.Assert.assertTrue; + +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Field; + +import org.testng.annotations.Test; + +import brooklyn.util.xstream.CompilerCompatibilityTest.EnclosingClass.DynamicClass; +import brooklyn.util.xstream.CompilerCompatibilityTest.EnclosingClass.DynamicExtendingClass; +import brooklyn.util.xstream.CompilerCompatibilityTest.EnclosingClass.EnclosingDynamicClass; +import brooklyn.util.xstream.CompilerCompatibilityTest.EnclosingClass.EnclosingDynamicClass.NestedDynamicClass; + +import com.thoughtworks.xstream.XStream; +import com.thoughtworks.xstream.mapper.MapperWrapper; + +// To get the generated synthetic fields use the command: +/* + find core/target/test-classes -name CompilerCompatibilityTest\$EnclosingClass\$* | \ + sed s@core/target/test-classes/@@ | sed '[email protected]$@@' | sed s@/@.@g | \ + xargs javap -classpath core/target/test-classes/ | grep -B1 this +*/ +@SuppressWarnings("unused") +public class CompilerCompatibilityTest { + private EnclosingClass enclosingClass = new EnclosingClass(); + private DynamicClass dynamicClass = enclosingClass.new DynamicClass(); + private DynamicExtendingClass dynamicExtendingClass = enclosingClass.new DynamicExtendingClass(); + private EnclosingDynamicClass enclosingDynamicClass = enclosingClass.new EnclosingDynamicClass(); + private NestedDynamicClass nestedDynamicClass = enclosingDynamicClass.new NestedDynamicClass(); +// NOT SUPPORTED +// private DynamicExtendingClassWithDifferentScope dynamicExtendingClassWithDifferentScope = +// enclosingClass.new DynamicExtendingClassWithDifferentScope(enclosingDynamicClass); + + public static class EnclosingClass { + public class DynamicClass { + //Oracle/OpenJDK/IBM generates + //final EnclosingClass this$0; + + //eclipse-[groovy-]compiler generates + //final EnclosingClass this$1; + } + + public class DynamicExtendingClass extends DynamicClass { + //The field here masks the parent field + + //Oracle/OpenJDK/IBM generates + //final EnclosingClass this$0; + + //eclipse-[groovy-]compiler generates + //final EnclosingClass this$1; + } + + public class EnclosingDynamicClass { + //Oracle/OpenJDK/IBM generates + //final EnclosingClass this$0; + + //eclipse-[groovy-]compiler generates + //final EnclosingClass this$1; + + public class NestedDynamicClass { + //Oracle/OpenJDK/IBM generates + //final EnclosingClass this$1; + + //eclipse-[groovy-]compiler generates + //final EnclosingClass this$2; + } + } + +// WARNING: Combination NOT SUPPORTED. Not enough information in XML to deserialize reliably, +// having in mind that different compilers could be used for parent/child classes. +// If we really need to, we can extend the heuristic to check for field types or assume that +// only one compiler was used for the whole class hierarchy covering some more cases. +// +// The problem is that we have two fields with different names, without relation between the +// indexes in each one. Changing compilers (or combination of compilers) could change the +// indexes independently in each field. This makes it impossible to infer which field in the xml +// maps to which field in the object. +// When having identical field names with parent classes XStream will put a defined-in attribute +// which makes it possible to deserialize, but it can't be forced to put it in each element. +// + public class DynamicExtendingClassWithDifferentScope extends NestedDynamicClass { + //Oracle/OpenJDK/IBM generates + //final EnclosingClass this$0; + + //eclipse-[groovy-]compiler generates + //final EnclosingClass this$1; + + //constructor required to compile + public DynamicExtendingClassWithDifferentScope(EnclosingDynamicClass superEnclosingScope) { + superEnclosingScope.super(); + } + } + } + + @Test + public void testXStreamDeserialize() throws Exception { + deserialize("/brooklyn/entity/rebind/compiler_compatibility_eclipse.xml"); + deserialize("/brooklyn/entity/rebind/compiler_compatibility_oracle.xml"); + } + + private void deserialize(String inputUrl) throws Exception { + XStream xstream = new XStream() { + @Override + protected MapperWrapper wrapMapper(MapperWrapper next) { + return new CompilerIndependentOuterClassFieldMapper(super.wrapMapper(next)); + } + }; + + InputStream in = this.getClass().getResourceAsStream(inputUrl); + try { + Object obj = xstream.fromXML(in); + assertNonNullOuterFields(obj); + } finally { + in.close(); + } + } + + private void assertNonNullOuterFields(Object obj) throws Exception { + Field[] testInstances = obj.getClass().getDeclaredFields(); + for (Field instanceField : testInstances) { + Object instance = instanceField.get(obj); + Class<?> type = instance.getClass(); + do { + for (Field field : type.getDeclaredFields()) { + if (field.getName().startsWith("this$")) { + Object value = field.get(instance); + assertTrue(value != null, field + " should not be null"); + } + } + type = type.getSuperclass(); + } while (type != null); + } + } +} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ba0e6480/core/src/test/resources/brooklyn/entity/rebind/compiler_compatibility_eclipse.xml ---------------------------------------------------------------------- diff --git a/core/src/test/resources/brooklyn/entity/rebind/compiler_compatibility_eclipse.xml b/core/src/test/resources/brooklyn/entity/rebind/compiler_compatibility_eclipse.xml new file mode 100644 index 0000000..dc972ef --- /dev/null +++ b/core/src/test/resources/brooklyn/entity/rebind/compiler_compatibility_eclipse.xml @@ -0,0 +1,41 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you 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. +--> +<brooklyn.util.xstream.CompilerCompatibilityTest> + <enclosingClass/> + <dynamicClass> + <this_-1 reference="../../enclosingClass"/> + </dynamicClass> + <dynamicExtendingClass> + <this_-1 defined-in="brooklyn.util.xstream.CompilerCompatibilityTest$EnclosingClass$DynamicClass" reference="../../enclosingClass"/> + <this_-1 reference="../../enclosingClass"/> + </dynamicExtendingClass> + <enclosingDynamicClass> + <this_-1 reference="../../enclosingClass"/> + </enclosingDynamicClass> + <nestedDynamicClass> + <this_-2 reference="../../enclosingDynamicClass"/> + </nestedDynamicClass> +<!-- NOT SUPPORTED + <dynamicExtendingClassWithDifferentScope> + <this_-2 reference="../../enclosingDynamicClass"/> + <this_-1 reference="../../enclosingClass"/> + </dynamicExtendingClassWithDifferentScope> +--> +</brooklyn.util.xstream.CompilerCompatibilityTest> http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/ba0e6480/core/src/test/resources/brooklyn/entity/rebind/compiler_compatibility_oracle.xml ---------------------------------------------------------------------- diff --git a/core/src/test/resources/brooklyn/entity/rebind/compiler_compatibility_oracle.xml b/core/src/test/resources/brooklyn/entity/rebind/compiler_compatibility_oracle.xml new file mode 100644 index 0000000..a01692e --- /dev/null +++ b/core/src/test/resources/brooklyn/entity/rebind/compiler_compatibility_oracle.xml @@ -0,0 +1,41 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you 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. +--> +<brooklyn.util.xstream.CompilerCompatibilityTest> + <enclosingClass/> + <dynamicClass> + <outer-class reference="../../enclosingClass"/> + </dynamicClass> + <dynamicExtendingClass> + <outer-class defined-in="brooklyn.util.xstream.CompilerCompatibilityTest$EnclosingClass$DynamicClass" reference="../../enclosingClass"/> + <outer-class reference="../../enclosingClass"/> + </dynamicExtendingClass> + <enclosingDynamicClass> + <outer-class reference="../../enclosingClass"/> + </enclosingDynamicClass> + <nestedDynamicClass> + <this_-1 reference="../../enclosingDynamicClass"/> + </nestedDynamicClass> +<!-- NOT SUPPORTED + <dynamicExtendingClassWithDifferentScope> + <this_-1 reference="../../enclosingDynamicClass"/> + <outer-class reference="../../enclosingClass"/> + </dynamicExtendingClassWithDifferentScope> +--> +</brooklyn.util.xstream.CompilerCompatibilityTest>
