brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #718: improvement to OSGi serialization strateg...
Date Wed, 07 Jun 2017 16:35:17 GMT
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/718#discussion_r120678060
  
    --- Diff: core/src/main/java/org/apache/brooklyn/util/core/xstream/XmlSerializer.java
---
    @@ -89,33 +96,49 @@ protected MapperWrapper wrapMapper(MapperWrapper next) {
         }
     
         /**
    -     * JCC is used when class names are serialized/deserialized and no alias is defined;
    -     * it is configured in XStream *without* access to the XStream mapper.
    +     * JCC is used when Class instances are serialized/deserialized as a value 
    +     * (not as tags) and there are no aliases configured for that type.
    +     * It is configured in XStream default *without* access to the XStream mapper,
    +     * which is meant to apply when serializing the type name for instances of that type.
    +     * <p>
          * However we need a few selected mappers (see {@link #wrapMapperForAllLowLevelMentions(Mapper)}
)
    -     * in order to effect renames at the low level, but many of the mappers must NOT
be used,
    +     * to apply to all class renames, but many of the mappers must NOT be used,
          * e.g. because some might intercept all Class<? extends Entity> references
          * (and that interception is only wanted when serializing <i>instances</i>,
          * as in {@link #wrapMapperForNormalUsage(Mapper)}).
          * <p>
    -     * This can typically be done simply by registering our own instance (due to order
guarantee of PrioritizedList),
    +     * This can typically be done simply by registering our own instance of this (due
to order guarantee of PrioritizedList),
          * after the instance added by XStream.setupConverters()
          */
         private JavaClassConverter newCustomJavaClassConverter() {
             return new JavaClassConverter(wrapMapperForAllLowLevelMentions(new DefaultMapper(xstream.getClassLoaderReference())))
{};
         }
         
    -    /** Adds mappers needed for *any* reference to a class, e.g. when names are used
for inner classes, or classes are renamed;
    -     * this *excludes* basic mentions, however, because most rewrites should *not* be
applied at this deep level;
    -     * mappers which effect aliases or intercept references to entities are usually NOT
be invoked in this low-level pathway.
    -     * See {@link #newCustomJavaClassConverter()}. */
    +    /** Adds mappers needed for *any* reference to a class, both "normal" usage (when
xstream wants a mapper)
    +     * and Class conversion (when xstream needs to serialize an instance of Class and
doesn't have an alias).
    +     * <p>
    +     * This should apply when nice names are used for inner classes, or classes are renamed;
    +     * however mappers which affect aliases or intercept references to entities are usually

    +     * NOT be invoked in this low-level pathway. See {@link #newCustomJavaClassConverter()}.
*/
    +    // Developer note - this is called by the xstream subclass constructor in the constructor
of this class,
    +    // so very few fields are populated
         protected MapperWrapper wrapMapperForAllLowLevelMentions(Mapper next) {
             MapperWrapper result = new CompilerIndependentOuterClassFieldMapper(next);
    +        
             Supplier<ClassLoader> classLoaderSupplier = new Supplier<ClassLoader>()
{
                 @Override public ClassLoader get() {
                     return xstream.getClassLoaderReference().getReference();
                 }
             };
    -        return new ClassRenamingMapper(result, deserializingClassRenames, classLoaderSupplier);
    +        result = new ClassRenamingMapper(result, deserializingClassRenames, classLoaderSupplier);
    +        result = new OsgiClassnameMapper(new Supplier<XStream>() {
    +            @Override public XStream get() { return xstream; } }, result);
    --- End diff --
    
    @neykov I think @ahgittin said it will be null at this point, because we're calling this
method from inside the `XmlSerializer` constructor (yuck!), and worse it gets called from
inside the constructor of the `XStream` object.
    
    The `new XStream()` constructor calls `buildMapper()`, which calls `wrapMapper()`, which
we override in the anonymous XStream class to call `wrapMapperForNormalUsage`, which calls
`wrapMapperForAllLowLevelMentions`. So when we get to this line of code, we haven't managed
to set the `xstream` field yet. Crazy!


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message