From 3f9c3e3d770087f0b6e3b157a5d000981dce818f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 3 Jan 2013 13:21:59 -0500 Subject: [PATCH] Stack trace printed while running UseRecipesWithJenkinsRuleTest. Default constructor of RobustReflectionConverter ought to use PluginManager. java.lang.AssertionError at hudson.util.RobustReflectionConverter.(RobustReflectionConverter.java:82) at hudson.util.RobustReflectionConverter.(RobustReflectionConverter.java:77) at hudson.security.ProjectMatrixAuthorizationStrategy$ConverterImpl.(ProjectMatrixAuthorizationStrategy.java:103) --- .../util/RobustReflectionConverter.java | 7 +-- core/src/main/java/hudson/util/XStream2.java | 49 +++++++++++-------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/hudson/util/RobustReflectionConverter.java b/core/src/main/java/hudson/util/RobustReflectionConverter.java index ef9e2f76eb..42269140f8 100755 --- a/core/src/main/java/hudson/util/RobustReflectionConverter.java +++ b/core/src/main/java/hudson/util/RobustReflectionConverter.java @@ -53,6 +53,7 @@ import java.util.Map; import java.util.Set; import static java.util.logging.Level.WARNING; import java.util.logging.Logger; +import javax.annotation.Nonnull; /** * Custom {@link ReflectionConverter} that handle errors more gracefully. @@ -71,10 +72,10 @@ public class RobustReflectionConverter implements Converter { protected final Mapper mapper; protected transient SerializationMethodInvoker serializationMethodInvoker; private transient ReflectionProvider pureJavaReflectionProvider; - private final XStream2.ClassOwnership classOwnership; + private final @Nonnull XStream2.ClassOwnership classOwnership; public RobustReflectionConverter(Mapper mapper, ReflectionProvider reflectionProvider) { - this(mapper, reflectionProvider, null); + this(mapper, reflectionProvider, new XStream2().new PluginClassOwnership()); } RobustReflectionConverter(Mapper mapper, ReflectionProvider reflectionProvider, XStream2.ClassOwnership classOwnership) { this.mapper = mapper; @@ -96,7 +97,7 @@ public class RobustReflectionConverter implements Converter { } OwnerContext oc = OwnerContext.find(context); - oc.startVisiting(writer, classOwnership == null ? null : classOwnership.ownerOf(original.getClass())); + oc.startVisiting(writer, classOwnership.ownerOf(original.getClass())); try { doMarshal(source, writer, context); } finally { diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java index f772f2a498..44327b8009 100644 --- a/core/src/main/java/hudson/util/XStream2.java +++ b/core/src/main/java/hudson/util/XStream2.java @@ -41,6 +41,7 @@ import com.thoughtworks.xstream.io.HierarchicalStreamDriver; import com.thoughtworks.xstream.io.HierarchicalStreamReader; import com.thoughtworks.xstream.io.HierarchicalStreamWriter; import com.thoughtworks.xstream.mapper.CannotResolveClassException; +import edu.umd.cs.findbugs.annotations.SuppressWarnings; import hudson.PluginManager; import hudson.PluginWrapper; import hudson.diagnosis.OldDataMonitor; @@ -67,7 +68,7 @@ import javax.annotation.CheckForNull; public class XStream2 extends XStream { private RobustReflectionConverter reflectionConverter; private final ThreadLocal oldData = new ThreadLocal(); - private final ClassOwnership classOwnership; + private final @CheckForNull ClassOwnership classOwnership; private final Map> compatibilityAliases = new ConcurrentHashMap>(); /** @@ -111,26 +112,7 @@ public class XStream2 extends XStream { @Override protected Converter createDefaultConverter() { // replace default reflection converter - reflectionConverter = new RobustReflectionConverter(getMapper(),new JVM().bestReflectionProvider(), new ClassOwnership() { - PluginManager pm; - @Override public String ownerOf(Class clazz) { - if (classOwnership != null) { - return classOwnership.ownerOf(clazz); - } - if (pm == null) { - Jenkins j = Jenkins.getInstance(); - if (j != null) { - pm = j.getPluginManager(); - } - } - if (pm == null) { - return null; - } - // TODO: possibly recursively scan super class to discover dependencies - PluginWrapper p = pm.whichPlugin(clazz); - return p != null ? p.getShortName() + '@' + trimVersion(p.getVersion()) : null; - } - }); + reflectionConverter = new RobustReflectionConverter(getMapper(),new JVM().bestReflectionProvider(), new PluginClassOwnership()); return reflectionConverter; } @@ -377,5 +359,30 @@ public class XStream2 extends XStream { */ @CheckForNull String ownerOf(Class clazz); } + + class PluginClassOwnership implements ClassOwnership { + + private PluginManager pm; + + @SuppressWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE") // classOwnership checked for null so why does FB complain? + @Override public String ownerOf(Class clazz) { + if (classOwnership != null) { + return classOwnership.ownerOf(clazz); + } + if (pm == null) { + Jenkins j = Jenkins.getInstance(); + if (j != null) { + pm = j.getPluginManager(); + } + } + if (pm == null) { + return null; + } + // TODO: possibly recursively scan super class to discover dependencies + PluginWrapper p = pm.whichPlugin(clazz); + return p != null ? p.getShortName() + '@' + trimVersion(p.getVersion()) : null; + } + + } } -- GitLab