Mappers should be given more context to resolve a class

Hi Dariusz,

can you tell me how you gain the information from the type and what you have to do to load it again? Personally I have not much OSGi experience.

Your proposal is not really realizable, the mapper should not get access to the writer or reader, these objects have completely different concerns. In the mapper you have no idea in what state the reader or writer is and you should not make any assumptions about it. You will never be sure if you can currently write or read attributes without destroying the state.

Actually the idea to manipulate the class name is not bad. But it would require a special ClassLoader for the DefaultMapper where you can register the next OSGi bundle info to use. For the name manipulation you should be able to write a custom mapper. Something along this:

class BundleClassLoader extends ClassLoader {
  ThreadLocal nextBundleInfo = new ThreadLocal();
  Class loadClass(String name) {
    // use bundle info here
  }
}
class OSGiMapper extends MapperWrapper {
  BundleClassLoader bcl;
  OSGiMapper(MapperWrapper next, BundleClassLoader bcl) {
    super(next);
    this.bcl = bcl;
  }
  String serializedClass(Class type) {
    String name = super.serializedClass(type);
    return name + getBundleInfo(type);
  }
  Class realClass(String name) {
    name = stripAndRegisterBundleInfo(name);
    return super.realClass(name);
  }
  ...
  String stripAndRegisterBundleInfo(name) {
    ...
    bcl.nextBundleInfo.set(info);
    return strippedName;
  }
} 

final BundleClassLoader bcl = new BundleClassLoader()
XStream xstream = new XStream(null, new XppDriver(), bcl) {
  MapperWrapper wrapMapper(MapperWrapper next) {
    return new OSGiMapper(next, bcl);
  }
}

The problem is that at deserialization time you do not yet have the real class name in OSGiMapper.realClass. The call has to go down the complete chain to the DefautlMapper where the real class name actually arrives. However, the bundle info must be stripped first from the name. Therefore this 'hack' with the ThreadLocal.

And if you really want to separate in the XML the type name from the bundle info into element name and attributes, you may additionally implement a special HierarchicalStreamWriter (and HSReader) that extends from WriterWrapper and can strip off the bundle info from the element name again and add it as attributes. You would then use this writer and reader directly with XStream's marshal and unmarshal calls:

XStream xstream = new XStream(null, null, bcl) {
 ...
};
HierachricalStreamDriver driver = new XppDriver();
StringWriter w = new StringWriter();
xstream.marshal(object, new OSGiWriter(driver.createWriter(w)));
...

Does this help?

BTW: Did you have realized that XStream uses Bamboo for CI?

Hi Joerg,

thanks for your detailed answer! And sorry for answering so late - I was on holiday for a while

So what we do at the moment is in some ways similar to your suggestion. Except that:

• we don't really need bundle class loader to figure out that bundle symbolic name (that's the unique identifier of the bundle which in our case is Atlassian plugin's key) - you can get that straight frtom the object, using the fact that if an object comes from a bundle, it's class' classloader can be cast to BundleReference that has a reference to the bundle
• The other thing we're doing in realClass is (if we detect that given name matches the format containing bundle symbolic name) is just go straight to our plugin manager, get the related plugin (-> bundle) and load class from there.
• we also need to encode plugin (bunlde) version to detect potential incompatibilities

This is the core of the code we're using, just to visualize things (where Plugin is really just a wrapper interface around OSGi Bundle):

    public static String resolvePluginKey(Class<?> type)
    {
        if (type == null)
        {
            return null;
        }
        final ClassLoader loader = type.getClassLoader();
        if (loader instanceof BundleReference)
        {
            BundleReference bundleReference = (BundleReference) loader;
            return bundleReference.getBundle().getSymbolicName();
        }
        else
        {
            return null;
        }
    }

// .... in the mapper:

    @Override
    public String serializedClass(Class type)
    {
        final String pluginKey = resolvePluginKey(type);
        if (pluginKey != null)
        {
            return formatPluginClassName(super.serializedClass(type), pluginKey, resolveVersion(pluginKey));
        }
        else
        {
            return super.serializedClass(type);
        }
    }

    @Override
    public Class realClass(String elementName)
    {
        if (elementName.contains(PLUGIN_MARKER))
        {
            final ClassFromPlugin classFromPlugin = resolveClassFromPlugin(elementName);
            final Plugin plugin = pluginAccessor.getPlugin(classFromPlugin.pluginKey);
            return loadFromPlugin(classFromPlugin, plugin);
        }
        else
        {
            return super.realClass(elementName);
        }
    }

So it's quite similar to what you suggested and works rather nicely, the problem I see is that we and up with this kind of XMLs:

<com.atlassian.pluginxyz.ClassFromPluginXyz-pluginKey-com.atlassian.plugin.xyz-pluginVersion-1.0>

or

<instance class="com.atlassian.pluginxyz.ClassFromPluginXyz-pluginKey-com.atlassian.plugin.xyz-pluginVersion-1.0">

This is not optimal and I hope you agree that it would look much better like this:

<com.atlassian.pluginxyz.ClassFromPluginXyz pluginKey="com.atlassian.plugin.xyz" pluginVersion="1.0">

And I don't really mean aesthetics here, I'm more concerned about fragility (XML names have restrictions over what characters are allowed in them) and maintainability (if we ever resign from that model, we will still have to use a mapper that deals with 'legacy' XML names).

I understand that passing Readers/Writers is not a good choice, though I'm not sure if I follow you on why writing/reading attributes of the current element by the mapper would be unsafe. What if instead of full Writers/Readers we would pass something like this (the example is for reading - basically a subset of HierarchicalStreamReader), which doesn't really allow for state modifications:

public interface ClassReadContext {
   
    String getNodeName();

    String getAttribute(String name);
}

would that be safe/possible to implement?

Anyway, it seems that your last suggestion with custom HierarchicalStreamWriter/Reader would work for us! Still in my opinion that would be quite a workaround and the fact that I needed feedback from you to point me at this workaround suggests that the improvement I'm proposing might be useful

We're glad that XStream uses Bamboo! What it really means is that XStream is relying heavily on Xstream for its CI

Just tried the approach with the writer/reader wrapper... The problem is that sometimes the serialized class is written to attributes instead of node names: like 'class', 'resolves-to' etc. Again having to handle all that cases in the custom reader/wrapper seems hard (I guess I would need to go through usage of Mapper.serializedClass(..) and Mapper.realClass(...) in the whole XStream code base) and a bit fragile...

Yeah, class names are written in tag names, attribute values and sometimes even into text (e.g. serializing a Field). And some of the attributes may appear at same time.

Different approach. How big is your data? Can you keep it a 2nd time in memory? You might then separate the creation of the XML representation from the actual writing. But it would allow you to create at serialization time a map of all used classes in the tree with their bundle name incl. version and write that map as first information to the XML.

class ObjectGraph {
  BundleMap<String, BundleInfo> mapClassName2BundleInfo;
  transient Object root;
};

class BundleInfoMapper extends MapperWrapper {
  ObjectGraph graph; // threadlocal?
  BundleInfoMapper(MapperWrapper next, ObjectGraph graph) {
    super(next);
    this.graph=graph;
  }
  // ... fill mapClassName2BundleInfo in serializedClass
  // ... read from mapClassName2BundleInfo in realClass
};

class ObjectGraphConverter implements Converter {
  BundleInfoMapper mapper;
  XStream xsteam;
  BinaryStreamDriver driver = new BinaryStreamDriver(); // driver since 1.4.2, reader/writer since 1.3.x
  ObjectGraphConverter(XStream xstream) { // normally anti-pattern ;-)
    this.mapper = xstream.getMapper().lookupMapperOfType(BundleInfoMapper.class);
    this.xsteam = xsteam;
  }
  marshal(Object obj, HierarchicalStreamWriter writer, MarshallingContext context) {
    ObjectGraph graph = obj;
    // you may have to write a resolves-to attribute here for Object
    ByteArrayOutputStream baos = new ByteArrayOutputStream();
    xstream.marshal(graph.root, driver.createWriter(baos)); // convert, but fill also map
    writer.startNode("bundleInfo");
    context.convertAnother(graph.mapClassName2BundleInfo);
    writer.endNode();
    // transform data without 2nd conversion into XML
    new HierarchicalStreamCopier().copy(
      driver.createReader(new ByteArrayInputStream(baos.toBytes())), writer);
  }
  Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) {
    reader.moveDown();
    mapper.mapClassName2BundleInfo = context.convertAnother(null, BundleMap.class);
    reader.moveUp();
    return context.convertAnother(null, Object.class);
  }
}

ObjectGraph graph = new ObjectGraph();
graph.root = objectToSerialize;
XStream xstream = new XStream() {
  MapperWrapper wrapMapper(MapperWrapper next) {
    return new BundleInfoMapper(next);
  }
};
String xml = xstream.toXml(graph);
Object restored = xml.fromXml(xml);

Hope you get the idea.
And bear with me, for it's ~4 a.m.