Merge pull request #3 from jglick/JENKINS-32925-redux

Fixed DescribableModel.toString to stop recursion
jenkinsci · Apr 7, 2016 · 29ee8d0 · 29ee8d0
2 parents 34b22f0 + e1b1d10
commit 29ee8d0
Show file tree

Hide file tree

Showing 10 changed files with 187 additions and 61 deletions.
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/ArrayType.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/ArrayType.java
@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.structs.describable;
 
 import java.lang.reflect.Type;
+import java.util.Stack;
 
 /**
  * @author Jesse Glick
@@ -26,7 +27,8 @@ public ParameterType getElementType() {
     }
 
     @Override
-    public String toString() {
-        return elementType + "[]";
+    void toString(StringBuilder b, Stack<Class<?>> modelTypes) {
+        elementType.toString(b, modelTypes);
+        b.append("[]");
     }
 }
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/AtomicType.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/AtomicType.java
@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.structs.describable;
 
 import com.google.common.primitives.Primitives;
+import java.util.Stack;
 
 /**
  * @author Jesse Glick
@@ -16,7 +17,7 @@ public Class<?> getType() {
     }
 
     @Override
-    public String toString() {
-        return Primitives.unwrap((Class) getActualType()).getSimpleName();
+    void toString(StringBuilder b, Stack<Class<?>> modelTypes) {
+        b.append(Primitives.unwrap((Class<?>) getActualType()).getSimpleName());
     }
 }
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/DescribableModel.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/DescribableModel.java
@@ -13,7 +13,6 @@
 import net.java.sezpoz.IndexItem;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.ObjectUtils;
-import org.apache.commons.lang.StringUtils;
 import org.codehaus.groovy.reflection.ReflectionCache;
 import org.jvnet.tiger_types.Types;
 import org.kohsuke.stapler.ClassDescriptor;
@@ -40,12 +39,13 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.Stack;
 import java.util.TreeMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
- * Introspects a {@link Describable} with {@link DataBoundConstructor} and {@link DataBoundSetter}
+ * Introspects a {@link Describable} with {@link DataBoundConstructor} and {@link DataBoundSetter}.
  *
  * <p>
  * Provides such operations like
@@ -61,11 +61,15 @@
  *         either through {@link DataBoundConstructor} or {@link DataBoundSetter}.
  *         See {@link DescribableParameter} for more details
  *     <li>
- *         {@linkplain #getHelp() qccess help file}
+ *         {@linkplain #getHelp() access help file}
  * </ul>
  *
+ * Note that some structures are recursive or mutually recursive.
+ * It is up a caller to defend against stack overflows when traversing a model graph,
+ * for example by keeping a stack of types which have already been encountered.
+ *
  * @author Jesse Glick
- * @author Anderw Bayer
+ * @author Andrew Bayer
  * @author Kohsuke Kawaguchi
  */
 public final class DescribableModel<T> {
@@ -91,6 +95,11 @@ public final class DescribableModel<T> {
      */
     private final String[] constructorParamNames;
 
+    /** binds type parameter */
+    static <T> DescribableModel<T> of(Class<T> clazz) {
+        return new DescribableModel<T>(clazz);
+    }
+
     /**
      * Loads a definition of the structure of a class: what kind of data
      * you might get back from {@link #uninstantiate} on an instance,
@@ -461,9 +470,11 @@ public Map<String,Object> uninstantiate(T o) throws UnsupportedOperationExceptio
     /**
      * In case if you just need to uninstantiate one object and be done with it.
      */
-    @SuppressWarnings("unchecked")
     public static Map<String,Object> uninstantiate_(Object o) {
-        return new DescribableModel(o.getClass()).uninstantiate(o);
+        return uninstantiate__(o, o.getClass());
+    }
+    private static <T> Map<String,Object> uninstantiate__(Object o, Class<T> clazz) {
+        return of(clazz).uninstantiate(clazz.cast(o));
     }
 
     /**
@@ -487,8 +498,34 @@ String getHelp(String name) throws IOException {
         return null;
     }
 
+    void toString(StringBuilder b, Stack<Class<?>> modelTypes) {
+        b.append(type.getSimpleName());
+        if (modelTypes.contains(type)) {
+            b.append('…');
+        } else {
+            modelTypes.push(type);
+            try {
+                b.append('(');
+                boolean first = true;
+                for (DescribableParameter dp : getParameters()) {
+                    if (first) {
+                        first = false;
+                    } else {
+                        b.append(", ");
+                    }
+                    dp.toString(b, modelTypes);
+                }
+                b.append(')');
+            } finally {
+                modelTypes.pop();
+            }
+        }
+    }
+
     @Override public String toString() {
-        return "("+StringUtils.join(getParameters(), ", ") + ")";
+        StringBuilder b = new StringBuilder();
+        toString(b, new Stack<Class<?>>());
+        return b.toString();
     }
 
     public static final String CLAZZ = "$class";

diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/DescribableParameter.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/DescribableParameter.java
@@ -15,6 +15,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Stack;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -27,7 +28,7 @@
  * @see DescribableModel#getParameter(String)
  */
 public final class DescribableParameter {
-    private final DescribableModel parent;
+    private final DescribableModel<?> parent;
     private ParameterType type;
     private final String name;
 
@@ -42,7 +43,7 @@ public final class DescribableParameter {
      */
     /*package*/ final Setter setter;
 
-    /*package*/ DescribableParameter(DescribableModel parent, Type type, String name, Setter setter) {
+    /*package*/ DescribableParameter(DescribableModel<?> parent, Type type, String name, Setter setter) {
         this.parent = parent;
         this.rawType = type;
         this.name = name;
@@ -98,11 +99,20 @@ String getHelp() throws IOException {
         return parent.getHelp("help-" + name + ".html");
     }
 
+    void toString(StringBuilder b, Stack<Class<?>> modelTypes) {
+        b.append(name);
+        if (!isRequired()) {
+            b.append('?');
+        }
+        b.append(": ");
+        getType().toString(b, modelTypes);
+    }
+
     @Override
     public String toString() {
-        StringBuilder sb = new StringBuilder().append(name);
-        if (!isRequired())   sb.append('?');
-        return sb.append(": ").append(getType()).toString();
+        StringBuilder b = new StringBuilder();
+        toString(b, new Stack<Class<?>>());
+        return b.toString();
     }
 
     /**

diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/EnumType.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/EnumType.java
@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.structs.describable;
 
 import java.util.Arrays;
+import java.util.Stack;
 
 /**
  * @author Jesse Glick
@@ -26,7 +27,7 @@ public String[] getValues() {
     }
 
     @Override
-    public String toString() {
-        return ((Class) getActualType()).getSimpleName() + Arrays.toString(values);
+    void toString(StringBuilder b, Stack<Class<?>> modelTypes) {
+        b.append(((Class) getActualType()).getSimpleName()).append(Arrays.toString(values));
     }
 }
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/ErrorType.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/ErrorType.java
@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.structs.describable;
 
 import java.lang.reflect.Type;
+import java.util.Stack;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -22,8 +23,8 @@ public Exception getError() {
     }
 
     @Override
-    public String toString() {
-        return error.toString();
+    void toString(StringBuilder b, Stack<Class<?>> modelTypes) {
+        b.append(error);
     }
 
     private static final Logger LOGGER = Logger.getLogger(ErrorType.class.getName());

diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/HeterogeneousObjectType.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/HeterogeneousObjectType.java
@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.structs.describable;
 
 import java.util.Map;
+import java.util.Stack;
 
 /**
  * A parameter (or array element) which could take any of the indicated concrete object types.
@@ -9,8 +10,8 @@
  * @author Anderw Bayer
  */
 public final class HeterogeneousObjectType extends ParameterType {
-    private final Map<String,DescribableModel> types;
-    HeterogeneousObjectType(Class<?> supertype, Map<String, DescribableModel> types) {
+    private final Map<String,DescribableModel<?>> types;
+    HeterogeneousObjectType(Class<?> supertype, Map<String,DescribableModel<?>> types) {
         super(supertype);
         this.types = types;
     }
@@ -22,10 +23,38 @@ public Class<?> getType() {
     /**
      * A map from names which could be passed to {@link DescribableModel#CLAZZ} to types of allowable nested objects.
      */
-    public Map<String,DescribableModel> getTypes() {
+    public Map<String,DescribableModel<?>> getTypes() {
         return types;
     }
-    @Override public String toString() {
-        return getType().getSimpleName() + types;
+
+    @Override
+    void toString(StringBuilder b, Stack<Class<?>> modelTypes) {
+        Class<?> type = getType();
+        b.append(type.getSimpleName());
+        if (modelTypes.contains(type)) {
+            b.append('…');
+        } else {
+            modelTypes.push(type);
+            try {
+                b.append('{');
+                boolean first = true;
+                for (Map.Entry<String, DescribableModel<?>> entry : types.entrySet()) {
+                    if (first) {
+                        first = false;
+                    } else {
+                        b.append(" | ");
+                    }
+                    String key = entry.getKey();
+                    DescribableModel<?> model = entry.getValue();
+                    if (!key.equals(model.getType().getSimpleName())) {
+                        b.append(key).append('~');
+                    }
+                    model.toString(b, modelTypes);
+                }
+                b.append('}');
+            } finally {
+                modelTypes.pop();
+            }
+        }
     }
 }
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/HomogeneousObjectType.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/HomogeneousObjectType.java
@@ -1,16 +1,18 @@
 package org.jenkinsci.plugins.structs.describable;
 
+import java.util.Stack;
+
 /**
  *
  * @author Jesse Glick
  * @author Anderw Bayer
  */
 public final class HomogeneousObjectType extends ParameterType {
-    private final DescribableModel type;
+    private final DescribableModel<?> type;
 
     HomogeneousObjectType(Class<?> actualClass) {
         super(actualClass);
-        this.type = new DescribableModel(actualClass);
+        this.type = DescribableModel.of(actualClass);
     }
 
     public Class<?> getType() {
@@ -20,15 +22,15 @@ public Class<?> getType() {
     /**
      * The schema representing a type of nested object.
      */
-    public DescribableModel getSchemaType() {
+    public DescribableModel<?> getSchemaType() {
         return type;
     }
 
     /**
      * The actual class underlying the type.
      */
     @Override
-    public String toString() {
-        return type.getType().getSimpleName() + type;
+    void toString(StringBuilder b, Stack<Class<?>> modelTypes) {
+        type.toString(b, modelTypes);
     }
 }
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/ParameterType.java b/plugin/src/main/java/org/jenkinsci/plugins/structs/describable/ParameterType.java
@@ -14,6 +14,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.Stack;
 import java.util.TreeMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -72,18 +73,18 @@ static ParameterType of(Type type) {
                         }
                         bySimpleName.add(subtype);
                     }
-                    Map<String,DescribableModel> types = new TreeMap<String,DescribableModel>();
+                    Map<String,DescribableModel<?>> types = new TreeMap<String,DescribableModel<?>>();
                     for (Map.Entry<String,List<Class<?>>> entry : subtypesBySimpleName.entrySet()) {
                         if (entry.getValue().size() == 1) { // normal case: unambiguous via simple name
                             try {
-                                types.put(entry.getKey(), new DescribableModel(entry.getValue().get(0)));
+                                types.put(entry.getKey(), DescribableModel.of(entry.getValue().get(0)));
                             } catch (Exception x) {
                                 LOGGER.log(Level.FINE, "skipping subtype", x);
                             }
                         } else { // have to diambiguate via FQN
                             for (Class<?> subtype : entry.getValue()) {
                                 try {
-                                    types.put(subtype.getName(), new DescribableModel(subtype));
+                                    types.put(subtype.getName(), DescribableModel.of(subtype));
                                 } catch (Exception x) {
                                     LOGGER.log(Level.FINE, "skipping subtype", x);
                                 }
@@ -102,5 +103,14 @@ static ParameterType of(Type type) {
         }
     }
 
+    abstract void toString(StringBuilder b, Stack<Class<?>> modelTypes);
+
+    @Override
+    public final String toString() {
+        StringBuilder b = new StringBuilder();
+        toString(b, new Stack<Class<?>>());
+        return b.toString();
+    }
+
     private static final Logger LOGGER = Logger.getLogger(ParameterType.class.getName());
 }