[FIXED HUDSON-5610] Improved the form validation mechanism to support multiple controls.

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@27436 71c3de6d-444a-0410-be80-ed276b4c234a

core/pom.xml

@@ -333,7 +333,7 @@ THE SOFTWARE.
     <dependency>
       <groupId>org.kohsuke.stapler</groupId>
       <artifactId>stapler-jelly</artifactId>
-      <version>1.133</version>
+      <version>1.134</version>
       <exclusions>
         <exclusion>
           <groupId>commons-jelly</groupId>
@@ -745,6 +745,13 @@ THE SOFTWARE.
       <version>1.4</version>
     </dependency>
 
+    <dependency>
+      <!-- with this, stapler can load parameter names from the debug info -->
+      <groupId>asm</groupId>
+      <artifactId>asm-commons</artifactId>
+      <version>2.2.3</version>
+    </dependency>
+
     <!-- offline profiler API to put in the classpath if we need it -->
     <!--dependency>
       <groupId>com.yourkit.api</groupId>

core/src/main/java/hudson/model/Descriptor.java

@@ -31,6 +31,8 @@ import hudson.model.listeners.SaveableListener;
 import hudson.views.ListViewColumn;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
+import org.kohsuke.stapler.ClassDescriptor;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerResponse;
@@ -47,8 +49,10 @@ import javax.servlet.RequestDispatcher;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.annotation.Annotation;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Hashtable;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -129,7 +133,7 @@ public abstract class Descriptor<T extends Describable<T>> implements Saveable {
      */
     public transient final Class<? extends T> clazz;
 
-    private transient final Map<String,Method> checkMethods = new ConcurrentHashMap<String,Method>();
+    private transient final Map<String,String> checkMethods = new ConcurrentHashMap<String,String>();
 
     /**
      * Lazily computed list of properties on {@link #clazz}.
@@ -252,30 +256,62 @@ public abstract class Descriptor<T extends Describable<T>> implements Saveable {
      * This method is used to hook up the form validation method to
      */
     public String getCheckUrl(String fieldName) {
+        String method = checkMethods.get(fieldName);
+        if(method==null) {
+            method = calcCheckUrl(fieldName);
+            checkMethods.put(fieldName,method);
+        }
+
+        return method==NONE ? null : method;
+    }
+
+    private String calcCheckUrl(String fieldName) {
         String capitalizedFieldName = StringUtils.capitalize(fieldName);
 
-        Method method = checkMethods.get(fieldName);
-        if(method==null) {
-            method = NONE;
-            String methodName = "doCheck"+ capitalizedFieldName;
-            for (Class c=getClass(); c!=null; c=c.getSuperclass()) {
-                for( Method m : c.getMethods() ) {
-                    if(m.getName().equals(methodName)) {
-                        method = m;
-                        break;
-                    }
+        Method method = null;
+        String methodName = "doCheck"+ capitalizedFieldName;
+        for (Class c=getClass(); c!=null; c=c.getSuperclass()) {
+            for( Method m : c.getMethods() ) {
+                if(m.getName().equals(methodName)) {
+                    method = m;
+                    break;
                 }
             }
-            checkMethods.put(fieldName,method);
         }
 
-        if(method==NONE)
-            return null;
+        if(method==null)
+            return NONE;
+
+        // build query parameter line by figuring out what should be submitted
+        StringBuilder query = new StringBuilder();
+        String[] names = ClassDescriptor.loadParameterNames(method);
+        boolean first = true;
+        Annotation[][] all = method.getParameterAnnotations();
+        for (int i = 0, allLength = all.length; i < allLength; i++) {
+            for (Annotation a : all[i]) {
+                if (a instanceof QueryParameter) {
+                    QueryParameter qp = (QueryParameter) a;
+                    String name = qp.value();
+                    if (name.length()==0 && i<names.length) name = names[i];
+                    if (name.length()==0)
+                        continue;   // unknown parameter name. we'll report the error when the form is submitted.
+
+                    if (first)  first = false;
+                    else        query.append('+').append(singleQuote("&"));
+                    if (name.equals("value")) {
+                        // The special 'value' parameter binds to the the current field
+                        query.append('+').append(singleQuote("value=")).append("+toValue(this)");
+                    } else {
+                        query.append('+').append(singleQuote(name+'=')).append("+toValue(findNearBy(this,'"+name+"'))");
+                    }
+                }
+            }
+        }
 
         StaplerRequest req = Stapler.getCurrentRequest();
         Ancestor a = req.findAncestor(DescriptorByNameOwner.class);
         // a is always non-null because we already have Hudson as the sentinel
-        return singleQuote(a.getUrl()+"/descriptorByName/"+clazz.getName()+"/check"+capitalizedFieldName+"?value=")+"+toValue(this)";
+        return singleQuote(a.getUrl()+"/descriptorByName/"+clazz.getName()+"/check"+capitalizedFieldName+"?")+query;
     }
 
     /**
@@ -704,13 +740,5 @@ public abstract class Descriptor<T extends Describable<T>> implements Saveable {
     /**
      * Used in {@link #checkMethods} to indicate that there's no check method.
      */
-    private static final Method NONE;
-
-    static {
-        try {
-            NONE = Object.class.getMethod("toString");
-        } catch (NoSuchMethodException e) {
-            throw new AssertionError();
-        }
-    }
+    private static final String NONE = "\u0000";
 }

war/resources/scripts/hudson-behavior.js

@@ -125,6 +125,24 @@ var FormChecker = {
     }
 }
 
+/**
+ * Find the sibling (in the sense of the structured form submission) form item of the given name,
+ * and returns that DOM node.
+ */
+function findNearBy(e,name) {
+    var owner = findFormParent(e,null);
+
+    var p = findPreviousFormItem(e,name);
+    if (p!=null && findFormParent(p,null)==owner)
+        return p;
+
+    var n = findNextFormItem(e,name);
+    if (n!=null && findFormParent(n,null)==owner)
+        return n;
+
+    return null; // not found
+}
+
 function toValue(e) {
     // compute the form validation value to be sent to the server
     var type = e.getAttribute("type");
@@ -162,33 +180,54 @@ function findFollowingTR(input, className) {
     return tr;
 }
 
+function find(src,filter,traversalF) {
+    while(src!=null) {
+        src = traversalF(src);
+        if(src==null)   break;
+        if(filter(src))
+            return src;
+    }
+    return null;
+}
+
 /**
  * Traverses a form in the reverse document order starting from the given element (but excluding it),
  * until the given filter matches, or run out of an element.
  */
 function findPrevious(src,filter) {
-    function prev(e) {
+    return find(src,filter,function (e) {
         var p = e.previousSibling;
         if(p==null) return e.parentNode;
         while(p.lastChild!=null)
             p = p.lastChild;
         return p;
-    }
+    });
+}
 
-    while(src!=null) {
-        src = prev(src);
-        if(src==null)   break;
-        if(filter(src))
-            return src;
-    }
-    return null;
+function findNext(src,filter) {
+    return find(src,filter,function (e) {
+        var n = e.nextSibling;
+        if(n==null) return e.parentNode;
+        while(n.firstChild!=null)
+            n = n.firstChild;
+        return n;
+    });
 }
+
+function findFormItem(src,name,directionF) {
+    var name2 = "_."+name; // handles <textbox field="..." /> notation silently
+    return directionF(src,function(e){ return (e.tagName=="INPUT" || e.tagName=="TEXTAREA" || e.tagName=="SELECT") && (e.name==name || e.name==name2); });
+}
+
 /**
  * Traverses a form in the reverse document order and finds an INPUT element that matches the given name.
  */
 function findPreviousFormItem(src,name) {
-    var name2 = "_."+name; // handles <textbox field="..." /> notation silently
-    return findPrevious(src,function(e){ return (e.tagName=="INPUT" || e.tagName=="TEXTAREA" || e.tagName=="SELECT") && (e.name==name || e.name==name2); });
+    return findFormItem(src,name,findPrevious);
+}
+
+function findNextFormItem(src,name) {
+    return findFormItem(src,name,findNext);
 }
 
 
@@ -1256,6 +1295,41 @@ function createSearchBox(searchURL) {
 }
 
 
+/**
+ * Finds the DOM node of the given DOM node that acts as a parent in the form submission.
+ *
+ * @return null
+ *      if the given element shouldn't be a part of the final submission.
+ */
+function findFormParent(e,form) {
+    if (form==null) // caller can pass in null to have this method compute the owning form
+        form = findAncestor(e,"FORM");
+
+    while(e!=form) {
+        e = e.parentNode;
+
+        // this is used to create a group where no single containing parent node exists,
+        // like <optionalBlock>
+        var nameRef = e.getAttribute("nameRef");
+        if(nameRef!=null)
+            e = $(nameRef);
+
+        if(e.getAttribute("field-disabled")!=null)
+            return null;  // this field shouldn't contribute to the final result
+
+        var name = e.getAttribute("name");
+        if(name!=null) {
+            if(e.tagName=="INPUT" && !xor(e.checked,Element.hasClassName(e,"negative")))
+                return null;  // field is not active
+
+            return e;
+        }
+    }
+
+    return form;
+}
+
+
 //
 // structured form submission handling
 //   see http://hudson.gotdns.com/wiki/display/HUDSON/Structured+Form+Submission
@@ -1294,35 +1368,17 @@ function buildFormTree(form) {
         // find the grouping parent node, which will have @name.
         // then return the corresponding object in the map
         function findParent(e) {
-            while(e!=form) {
-                e = e.parentNode;
-
-                // this is used to create a group where no single containing parent node exists,
-                // like <optionalBlock>
-                var nameRef = e.getAttribute("nameRef");
-                if(nameRef!=null)
-                    e = $(nameRef);
-
-                if(e.getAttribute("field-disabled")!=null)
-                    return {};  // this field shouldn't contribute to the final result
-
-                var name = e.getAttribute("name");
-                if(name!=null) {
-                    if(e.tagName=="INPUT" && !xor(e.checked,Element.hasClassName(e,"negative")))
-                        return {};  // field is not active
-
-                    var m = e.formDom;
-                    if(m==null) {
-                        // this is a new grouping node
-                        doms.push(e);
-                        e.formDom = m = {};
-                        addProperty(findParent(e), name, m);
-                    }
-                    return m;
-                }
+            var p = findFormParent(e,form);
+            if (p==null)    return {};
+
+            var m = p.formDom;
+            if(m==null) {
+                // this is a new grouping node
+                doms.push(p);
+                p.formDom = m = {};
+                addProperty(findParent(p), name, m);
             }
-
-            return form.formDom; // guaranteed non-null
+            return m;
         }
 
         var jsonElement = null;