You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2015/03/03 17:19:51 UTC
[2/4] incubator-brooklyn git commit: Compiler independent outer class
field deserialization
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 <sv...@cloudsoftcorp.com>
Authored: Thu Feb 19 14:26:35 2015 +0200
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
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 's@.class$@@' | 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>