Merge pull request #74 from jglick/GitSCM.extensions-JENKINS-26619

[JENKINS-26619] GitSCM.extensions reflection fix
jenkinsci · Mar 5, 2015 · 0e956a5 · 0e956a5
2 parents 13f7b07 + 70b91be
commit 0e956a5
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 3 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -5,6 +5,7 @@ Only noting significant user-visible or major API changes, not internal code cle
 ## 1.4 (upcoming)
 
 ### User changes
+* JENKINS-26619: _Snippet Generator_ did not work on Git SCM extensions.
 
 ## 1.3 (Mar 04 2015)
 

diff --git a/step-api/pom.xml b/step-api/pom.xml
@@ -39,6 +39,11 @@
             <groupId>org.mockito</groupId>
             <artifactId>mockito-all</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins</groupId>
+            <artifactId>git</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
     <build>
         <plugins>

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
@@ -40,6 +40,7 @@
 import java.lang.reflect.Type;
 import java.net.URL;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -243,15 +244,16 @@ private static Object coerce(String context, Type type, @Nonnull Object o) throw
             Class<?> componentType = ((Class) type).getComponentType();
             List<Object> list = mapList(context, componentType, (List) o);
             return list.toArray((Object[]) Array.newInstance(componentType, list.size()));
-        } else if (o instanceof List && isList(type)) {
+        } else if (o instanceof List && acceptsList(type)) {
             return mapList(context, ((ParameterizedType) type).getActualTypeArguments()[0], (List) o);
         } else {
             throw new ClassCastException(context + " expects " + type + " but received " + o.getClass());
         }
     }
 
+    /** Whether this type is generic of {@link List} or a supertype thereof (such as {@link Collection}). */
     @SuppressWarnings("unchecked")
-    private static boolean isList(Type type) {
+    private static boolean acceptsList(Type type) {
         return type instanceof ParameterizedType && ((ParameterizedType) type).getRawType() instanceof Class && ((Class) ((ParameterizedType) type).getRawType()).isAssignableFrom(List.class);
     }
 
@@ -315,6 +317,19 @@ private static void injectSetters(Object o, Map<String,?> arguments) throws Exce
     private static void inspect(Map<String, Object> r, Object o, Class<?> clazz, String field) {
         AtomicReference<Type> type = new AtomicReference<Type>();
         Object value = inspect(o, clazz, field, type);
+        try {
+            String[] names = new ClassDescriptor(clazz).loadConstructorParamNames();
+            int idx = Arrays.asList(names).indexOf(field);
+            if (idx >= 0) {
+                Type ctorType = findConstructor(clazz, names.length).getGenericParameterTypes()[idx];
+                if (!type.get().equals(ctorType)) {
+                    LOG.log(Level.WARNING, "For {0}.{1}, preferring constructor type {2} to differing getter type {3}", new Object[] {clazz.getName(), field, ctorType, type});
+                    type.set(ctorType);
+                }
+            }
+        } catch (IllegalArgumentException x) {
+            // From loadConstructorParamNames or findConstructor; ignore
+        }
         r.put(field, uncoerce(value, type.get()));
     }
 
@@ -332,7 +347,7 @@ private static Object uncoerce(Object o, Type type) {
                 list.add(uncoerce(elt, array.getClass().getComponentType()));
             }
             return list;
-        } else if (o instanceof List && isList(type)) {
+        } else if (o instanceof List && acceptsList(type)) {
             List<Object> list = new ArrayList<Object>();
             for (Object elt : (List<?>) o) {
                 list.add(uncoerce(elt, ((ParameterizedType) type).getActualTypeArguments()[0]));

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
@@ -30,6 +30,8 @@
 import hudson.model.BooleanParameterValue;
 import hudson.model.Descriptor;
 import hudson.model.ParameterValue;
+import hudson.plugins.git.GitSCM;
+import hudson.plugins.git.extensions.impl.CleanBeforeCheckout;
 import java.net.URL;
 import java.util.Arrays;
 import java.util.Collection;
@@ -461,6 +463,17 @@ public static final class TakesParams {
         }
     }
 
+    @Issue("JENKINS-26619")
+    @Test public void getterDescribableList() throws Exception {
+        roundTrip(GitSCM.class, map(
+            "extensions", Arrays.asList(map("$class", CleanBeforeCheckout.class.getSimpleName())),
+            // Default values for these things do not work because GitSCM fails to use @DataBoundSetter:
+            "branches", Arrays.asList(map("name", "*/master")),
+            "doGenerateSubmoduleConfigurations", false,
+            "submoduleCfg", Collections.emptyList(),
+            "userRemoteConfigs", Collections.emptyList()));
+    }
+
     private static Map<String,Object> map(Object... keysAndValues) {
         if (keysAndValues.length % 2 != 0) {
             throw new IllegalArgumentException();