Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/598#discussion_r106369844
--- Diff:
core/src/main/java/org/apache/brooklyn/util/core/xstream/ClassRenamingMapper.java
---
@@ -22,32 +22,168 @@
import java.util.Map;
+import org.apache.brooklyn.core.mgmt.persist.OsgiClassPrefixer;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.Reflections;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.base.Supplier;
+import com.thoughtworks.xstream.mapper.CannotResolveClassException;
import com.thoughtworks.xstream.mapper.Mapper;
import com.thoughtworks.xstream.mapper.MapperWrapper;
+/**
+ * An xstream mapper that handles class-renames, so we can rebind to
historic persisted state.
+ */
public class ClassRenamingMapper extends MapperWrapper {
+
+ /*
+ * TODO There is a strange relationship between this and
XmlMementoSerializer$OsgiClassnameMapper.
+ * Should these be perhaps merged?
+ *
+ * TODO For class-loading on deserialzation, should we push the
class-rename logic into
+ * org.apache.brooklyn.util.core.ClassLoaderUtils instead? Does the
xstream mapper do
+ * anything else important, beyond that class-loading responsibility?
It's registration
+ * in XmlSerializer makes it look a bit scary:
wrapMapperForAllLowLevelMentions().
+ *
+ * ---
+ * TODO This code feels overly complicated, and deserves a cleanup.
+ *
+ * The aim is to handle two use-cases in the
deserializingClassRenames.properties:
+ *
+ * 1. A very explicit rename that includes bundle prefixes (e.g. so
as to limit scope, or to support
+ * moving a class from one bundle to another).
+ *
+ * 2. Just the class-rename (e.g. `com.acme.Foo: com.acme.Bar`).
+ * This would rename "acme-bundle:com.acme.Foo" to
"acme-bundle:com.acme.Bar".
+ *
+ * However, to achieve that is fiddly for several reasons:
+ *
+ * 1. We might be passed qualified or unqualified names (e.g.
"com.acme.Foo" or "acme-bundle:com.acme.Foo"),
+ * depending how old the persisted state is, where OSGi was used
previously, and whether
+ * whitelabelled bundles were used.
+ *
+ * 2. Calling `super.realClass(name)` must return a class that has
exactly the same name as
+ * was passed in. This is because xstream will subsequently use
`Class.forName` which is
+ * fussy about that. However, if we're passed
"acme-bundle:com.acme.Foo" then we'd expect
+ * to return a class named "com.acme.Foo". The final classloading
in our
+ * `XmlMementoSerializer$OsgiClassLoader.findClass()` will handle
stripping out the bundle
+ * name, and using the right bundle.
+ *
+ * In the case where we haven't changed the name, then we can
leave it up to
+ * `XmlMementoSerializer$OsgiClassnameMapper.realClass()` to do
sort this out. But if we've
+ * done a rename, then unforutnately it's currently this class'
responsibility!
+ *
+ * That means it has to fallback to calling
classLoader.loadClass().
+ *
+ * 3. As mentioned under the use-cases, the rename could include the
full bundle name prefix,
+ * or it might just be the classname. We want to handle both, so
need to implement yet
+ * more fallback behaviour.
+ *
+ * ---
+ * TODO Wanted to pass xstream, rather than Supplier<ClassLoader>, in
constructor. However,
+ * this caused NPE because of how this is constructed from inside
+ * XmlMementoSerializer.wrapMapperForNormalUsage, called from within
an anonymous subclass of XStream!
+ */
+
public static final Logger LOG =
LoggerFactory.getLogger(ClassRenamingMapper.class);
private final Map<String, String> nameToType;
-
- public ClassRenamingMapper(Mapper wrapped, Map<String, String>
nameToType) {
+ private final Supplier<? extends ClassLoader> classLoaderSupplier;
+
+ public ClassRenamingMapper(Mapper wrapped, Map<String, String>
nameToType, Supplier<? extends ClassLoader> classLoaderSupplier) {
super(wrapped);
this.nameToType = checkNotNull(nameToType, "nameToType");
+ this.classLoaderSupplier = checkNotNull(classLoaderSupplier,
"classLoaderSupplier");
}
@Override
public Class<?> realClass(String elementName) {
+ String elementNamOrig = elementName;
--- End diff --
Typo elementNam**e**Orig
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
