[FIXED JENKINS-32925] Merged #351.

jenkinsci · Mar 4, 2016 · e51ddbb · e51ddbb
2 parents 0b0b2aa + 64e8b85
commit e51ddbb
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 11 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -4,6 +4,7 @@ Only noting significant user changes, not internal code cleanups and minor bug f
 
 ## 1.15 (upcoming)
 
+* [JENKINS-32925](https://issues.jenkins-ci.org/browse/JENKINS-32925): stack overflow displaying reference documentation in certain cases.
 * [JENKINS-27152](https://issues.jenkins-ci.org/browse/JENKINS-27152): offering `tmp` option to `pwd` step.
 
 ## 1.14 (Feb 25 2016)

diff --git a/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java b/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java
@@ -52,6 +52,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.Stack;
 import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.Level;
@@ -147,6 +148,10 @@ public static Map<String,Object> uninstantiate(Object o) throws UnsupportedOpera
     public static Schema schemaFor(Class<?> clazz) {
         return new Schema(clazz);
     }
+
+    private static Schema schemaFor(Class<?> clazz, Stack<String> tracker) {
+        return new Schema(clazz, tracker);
+    }
 
     /**
      * Definition of how a particular class may be configured.
@@ -158,20 +163,27 @@ public static final class Schema {
         private final List<String> mandatoryParameters;
 
         Schema(Class<?> clazz) {
+            this(clazz, new Stack<String>());
+        }
+
+        Schema(Class<?> clazz, @Nonnull Stack<String> tracker) {
             this.type = clazz;
+            /*if(tracker == null){
+                tracker = new Stack<String>();
+            }*/
             mandatoryParameters = new ArrayList<String>();
             parameters = new TreeMap<String,ParameterType>();
             String[] names = loadConstructorParamNames(clazz);
             Type[] types = findConstructor(clazz, names.length).getGenericParameterTypes();
             for (int i = 0; i < names.length; i++) {
                 mandatoryParameters.add(names[i]);
-                parameters.put(names[i], ParameterType.of(types[i]));
+                parameters.put(names[i], ParameterType.of(types[i], tracker));
             }
             for (Class<?> c = clazz; c != null; c = c.getSuperclass()) {
                 for (Field f : c.getDeclaredFields()) {
                     if (f.isAnnotationPresent(DataBoundSetter.class)) {
                         f.setAccessible(true);
-                        parameters.put(f.getName(), ParameterType.of(f.getGenericType()));
+                        parameters.put(f.getName(), ParameterType.of(f.getGenericType(), tracker));
                     }
                 }
                 for (Method m : c.getDeclaredMethods()) {
@@ -181,7 +193,7 @@ public static final class Schema {
                             throw new IllegalStateException(m + " cannot be a @DataBoundSetter");
                         }
                         m.setAccessible(true);
-                        parameters.put(Introspector.decapitalize(m.getName().substring(3)), ParameterType.of(m.getGenericParameterTypes()[0]));
+                        parameters.put(Introspector.decapitalize(m.getName().substring(3)), ParameterType.of(m.getGenericParameterTypes()[0], tracker));
                     }
                 }
             }
@@ -282,8 +294,12 @@ public Type getActualType() {
         ParameterType(Type actualType) {
             this.actualType = actualType;
         }
+
+        static ParameterType of(Type type){
+            return of(type, new Stack<String>());
+        }
 
-        static ParameterType of(Type type) {
+        private static ParameterType of(Type type, @Nonnull Stack<String> tracker) {
             try {
                 if (type instanceof Class) {
                     Class<?> c = (Class<?>) type;
@@ -323,14 +339,24 @@ static ParameterType of(Type type) {
                         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(), schemaFor(entry.getValue().get(0)));
+                                    String key = entry.getKey();
+                                    if(tracker.search(key) < 0) {
+                                        tracker.push(key);
+                                        types.put(key, schemaFor(entry.getValue().get(0), tracker));
+                                        tracker.pop();
+                                    }
                                 } catch (Exception x) {
                                     LOG.log(Level.FINE, "skipping subtype", x);
                                 }
                             } else { // have to diambiguate via FQN
                                 for (Class<?> subtype : entry.getValue()) {
                                     try {
-                                        types.put(subtype.getName(), schemaFor(subtype));
+                                        String name = subtype.getName();
+                                        if(tracker.search(name) < 0) {
+                                            tracker.push(name);
+                                            types.put(name, schemaFor(subtype, tracker));
+                                            tracker.pop();
+                                        }
                                     } catch (Exception x) {
                                         LOG.log(Level.FINE, "skipping subtype", x);
                                     }

diff --git a/step-api/src/test/java/org/jenkinsci/plugins/workflow/structs/DescribableHelperTest.java b/step-api/src/test/java/org/jenkinsci/plugins/workflow/structs/DescribableHelperTest.java
@@ -154,7 +154,7 @@ public boolean isFlag() {
     }
 
     @Test public void findSubtypes() throws Exception {
-        assertEquals(new HashSet<Class<?>>(Arrays.asList(Impl1.class, Impl2.class)), DescribableHelper.findSubtypes(Base.class));
+        assertEquals(new HashSet<Class<?>>(Arrays.asList(Impl1.class, Impl2.class, Impl3.class)), DescribableHelper.findSubtypes(Base.class));
         assertEquals(Collections.singleton(Impl1.class), DescribableHelper.findSubtypes(Marker.class));
     }
 
@@ -172,7 +172,7 @@ public boolean isFlag() {
         roundTrip(UsesBase.class, map("base", map(CLAZZ, "Impl1", "text", "hello")));
         roundTrip(UsesBase.class, map("base", map(CLAZZ, "Impl2", "flag", true)));
         roundTrip(UsesImpl2.class, map("impl2", map()));
-        schema(UsesBase.class, "(base: Base{Impl1=(text: String), Impl2=([flag: boolean])})");
+        schema(UsesBase.class, "(base: Base{Impl1=(text: String), Impl2=([flag: boolean]), Impl3=(base: Base{Impl1=(text: String), Impl2=([flag: boolean])})})");
         schema(UsesImpl2.class, "(impl2: Impl2([flag: boolean]))");
         schema(UsesUnimplementedExtensionPoint.class, "(delegate: UnimplementedExtensionPoint{})");
         schema(UsesSomeImplsBroken.class, "(delegate: SomeImplsBroken{FineImpl=()})");
@@ -239,6 +239,25 @@ public boolean isFlag() {
         }
     }
 
+    //use to trigger recursion subcases
+    public static final class Impl3 extends Base {
+        private final Base base;
+        @DataBoundConstructor public Impl3(Base base) {
+            this.base = base;
+        }
+        public Base getBase() {
+            return base;
+        }
+        @Override public String toString() {
+            return "Impl3[" + base.toString() + "]";
+        }
+        @Extension public static final class DescriptorImpl extends Descriptor<Base> {
+            @Override public String getDisplayName() {
+                return "Impl3";
+            }
+        }
+    }
+
     public static abstract class UnimplementedExtensionPoint extends AbstractDescribableImpl<UnimplementedExtensionPoint> {}
     public static final class UsesUnimplementedExtensionPoint {
         @DataBoundConstructor public UsesUnimplementedExtensionPoint(UnimplementedExtensionPoint delegate) {}
@@ -389,7 +408,7 @@ public Collection<Impl2> getImpls() {
 
     @Test public void structArrayHetero() throws Exception {
         roundTrip(UsesStructArrayHetero.class, map("bases", Arrays.asList(map(CLAZZ, "Impl1", "text", "hello"), map(CLAZZ, "Impl2", "flag", true))), "UsesStructArrayHetero[Impl1[hello], Impl2[true]]");
-        schema(UsesStructArrayHetero.class, "(bases: Base{Impl1=(text: String), Impl2=([flag: boolean])}[])");
+        schema(UsesStructArrayHetero.class, "(bases: Base{Impl1=(text: String), Impl2=([flag: boolean]), Impl3=(base: Base{Impl1=(text: String), Impl2=([flag: boolean])})}[])");
     }
 
     public static final class UsesStructArrayHetero {
@@ -407,7 +426,7 @@ public Base[] getBases() {
 
     @Test public void structListHetero() throws Exception {
         roundTrip(UsesStructListHetero.class, map("bases", Arrays.asList(map(CLAZZ, "Impl1", "text", "hello"), map(CLAZZ, "Impl2", "flag", true))), "UsesStructListHetero[Impl1[hello], Impl2[true]]");
-        schema(UsesStructListHetero.class, "(bases: Base{Impl1=(text: String), Impl2=([flag: boolean])}[])");
+        schema(UsesStructListHetero.class, "(bases: Base{Impl1=(text: String), Impl2=([flag: boolean]), Impl3=(base: Base{Impl1=(text: String), Impl2=([flag: boolean])})}[])");
     }
 
     public static final class UsesStructListHetero {
@@ -425,7 +444,7 @@ public List<Base> getBases() {
 
     @Test public void structCollectionHetero() throws Exception {
         roundTrip(UsesStructCollectionHetero.class, map("bases", Arrays.asList(map(CLAZZ, "Impl1", "text", "hello"), map(CLAZZ, "Impl2", "flag", true))), "UsesStructCollectionHetero[Impl1[hello], Impl2[true]]");
-        schema(UsesStructCollectionHetero.class, "(bases: Base{Impl1=(text: String), Impl2=([flag: boolean])}[])");
+        schema(UsesStructCollectionHetero.class, "(bases: Base{Impl1=(text: String), Impl2=([flag: boolean]), Impl3=(base: Base{Impl1=(text: String), Impl2=([flag: boolean])})}[])");
     }
 
     public static final class UsesStructCollectionHetero {