Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/718#discussion_r120765196
--- 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) {
--- End diff --
either should be extensible. it's not a `lowLevelWrapping` but a
`wrapperExtensionPointForLowLevelActivities`.
in fact the name low-level is pissing me off. i'm tempted to rename it
`wrapMapperForLotsOfShitImNotSureWhat` and "normal usage" of course becomes
`wrapMapperForSlightlyMoreSpecificUsageButAgainImNotTellingYouWhat`.
(no prizes for guessing what numpty was responsible for the original names
:) )
---
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.
---
