[FIXED JENKINS-19124]

Added another attribute 'checkDependsOn' on <input> elements (akin to 'fillDependsOn', etc) to list up the other dependency controls, and insert appropriate onchange events. 'checkUrl' is now just the stem portion of the URL to invoke, and the client script builds up the query parameters.
jenkinsci · Aug 8, 2013 · 7232798 · megazoll · Aug 27, 2013 · jglick
1 parent 176ce5f
commit 7232798
Show file tree

Hide file tree

Showing 13 changed files with 295 additions and 73 deletions.
diff --git a/changelog.html b/changelog.html
@@ -57,6 +57,9 @@
 <ul class=image>
   <li class=rfe>
     Command line now supports "--sessionTimeout" option for controlling session timeout
+  <li class=bug>
+    Form validation methods weren't getting triggered when one of its dependency controls change.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-19124">issue 19124</a>)
 </ul>
 </div><!--=TRUNK-END=-->
 

diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java
@@ -73,6 +73,7 @@
 import hudson.tasks.Publisher;
 import hudson.tasks.UserAvatarResolver;
 import hudson.util.Area;
+import hudson.util.FormValidation.CheckMethod;
 import hudson.util.Iterators;
 import hudson.util.Secret;
 import hudson.views.MyViewsTabBar;
@@ -1531,6 +1532,9 @@ public String getServerName() {
 
     /**
      * Determines the form validation check URL. See textbox.jelly
+     *
+     * @deprecated
+     *      Use {@link #calcCheckUrl}
      */
     public String getCheckUrl(String userDefined, Object descriptor, String field) {
         if(userDefined!=null || field==null)   return userDefined;
@@ -1541,6 +1545,23 @@ public String getCheckUrl(String userDefined, Object descriptor, String field) {
         return null;
     }
 
+    /**
+     * Determines the parameters that client-side needs for a form validation check. See prepareDatabinding.jelly
+     *
+     * @deprecated
+     *      Use {@link #calcCheckUrl}
+     */
+    public void calcCheckUrl(Map attributes, String userDefined, Object descriptor, String field) {
+        if(userDefined!=null || field==null)   return;
+
+        if (descriptor instanceof Descriptor) {
+            Descriptor d = (Descriptor) descriptor;
+            CheckMethod m = d.getCheckMethod(field);
+            attributes.put("checkUrl",m.toStemUrl());
+            attributes.put("checkDependsOn",m.getDependsOn());
+        }
+    }
+
     /**
      * If the given href link is matching the current page, return true.
      *

diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java
@@ -31,6 +31,7 @@
 import hudson.Util;
 import hudson.model.listeners.SaveableListener;
 import hudson.util.FormApply;
+import hudson.util.FormValidation.CheckMethod;
 import hudson.util.ReflectionUtils;
 import hudson.util.ReflectionUtils.Parameter;
 import hudson.views.ListViewColumn;
@@ -44,7 +45,6 @@
 import org.jvnet.tiger_types.Types;
 import org.apache.commons.io.IOUtils;
 
-import static hudson.Functions.*;
 import static hudson.util.QuotedStringTokenizer.*;
 import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
 import javax.servlet.ServletException;
@@ -127,7 +127,7 @@ public abstract class Descriptor<T extends Describable<T>> implements Saveable {
      */
     public transient final Class<? extends T> clazz;
 
-    private transient final Map<String,String> checkMethods = new ConcurrentHashMap<String,String>();
+    private transient final Map<String,CheckMethod> checkMethods = new ConcurrentHashMap<String,CheckMethod>();
 
     /**
      * Lazily computed list of properties on {@link #clazz} and on the descriptor itself.
@@ -361,69 +361,28 @@ public static String getCurrentDescriptorByNameUrl() {
         return a.getUrl();
     }
 
+    /**
+     * @deprecated since 1.528
+     *      Use {@link #getCheckMethod(String)}
+     */
+    public String getCheckUrl(String fieldName) {
+        return getCheckMethod(fieldName).toCheckUrl();
+    }
+
     /**
      * If the field "xyz" of a {@link Describable} has the corresponding "doCheckXyz" method,
-     * return the form-field validation string. Otherwise null.
+     * return the model of the check method.
      * <p>
      * This method is used to hook up the form validation method to the corresponding HTML input element.
      */
-    public String getCheckUrl(String fieldName) {
-        String method = checkMethods.get(fieldName);
+    public CheckMethod getCheckMethod(String fieldName) {
+        CheckMethod method = checkMethods.get(fieldName);
         if(method==null) {
-            method = calcCheckUrl(fieldName);
+            method = new CheckMethod(this,fieldName);
             checkMethods.put(fieldName,method);
         }
 
-        if (method.equals(NONE)) // == would do, but it makes IDE flag a warning
-            return null;
-
-        // put this under the right contextual umbrella.
-        // a is always non-null because we already have Hudson as the sentinel
-        return '\'' + jsStringEscape(getCurrentDescriptorByNameUrl()) + "/'+" + method;
-    }
-
-    private String calcCheckUrl(String fieldName) {
-        String capitalizedFieldName = StringUtils.capitalize(fieldName);
-
-        Method method = ReflectionUtils.getPublicMethodNamed(getClass(),"doCheck"+ capitalizedFieldName);
-
-        if(method==null)
-            return NONE;
-
-        return '\'' + getDescriptorUrl() + "/check" + capitalizedFieldName + '\'' + buildParameterList(method, new StringBuilder()).append(".toString()");
-    }
-
-    /**
-     * Builds query parameter line by figuring out what should be submitted
-     */
-    private StringBuilder buildParameterList(Method method, StringBuilder query) {
-        for (Parameter p : ReflectionUtils.getParameters(method)) {
-            QueryParameter qp = p.annotation(QueryParameter.class);
-            if (qp!=null) {
-                String name = qp.value();
-                if (name.length()==0) name = p.name();
-                if (name==null || name.length()==0)
-                    continue;   // unknown parameter name. we'll report the error when the form is submitted.
-
-                RelativePath rp = p.annotation(RelativePath.class);
-                if (rp!=null)
-                    name = rp.value()+'/'+name;
-
-                if (query.length()==0)  query.append("+qs(this)");
-
-                if (name.equals("value")) {
-                    // The special 'value' parameter binds to the the current field
-                    query.append(".addThis()");
-                } else {
-                    query.append(".nearBy('"+name+"')");
-                }
-                continue;
-            }
-
-            Method m = ReflectionUtils.getPublicMethodNamed(p.type(), "fromStapler");
-            if (m!=null)    buildParameterList(m,query);
-        }
-        return query;
+        return method;
     }
 
     /**
@@ -1019,11 +978,6 @@ public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object nod
 
     private static final Logger LOGGER = Logger.getLogger(Descriptor.class.getName());
 
-    /**
-     * Used in {@link #checkMethods} to indicate that there's no check method.
-     */
-    private static final String NONE = "\u0000";
-
     /**
      * Special type indicating that {@link Descriptor} describes itself.
      * @see Descriptor#Descriptor(Class)

diff --git a/core/src/main/java/hudson/util/FormValidation.java b/core/src/main/java/hudson/util/FormValidation.java
@@ -27,27 +27,38 @@
 import hudson.Functions;
 import hudson.Launcher;
 import hudson.ProxyConfiguration;
+import hudson.RelativePath;
 import hudson.Util;
 import hudson.FilePath;
 import hudson.model.AbstractBuild;
 import hudson.model.BuildListener;
+import hudson.model.Descriptor;
 import hudson.tasks.Builder;
-import static hudson.Util.fixEmpty;
+
+import hudson.util.ReflectionUtils.Parameter;
 import jenkins.model.Jenkins;
 import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.Stapler;
+import org.springframework.util.StringUtils;
 
 import javax.servlet.ServletException;
 import java.io.File;
 import java.io.IOException;
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
+import java.lang.reflect.Method;
 import java.net.URL;
 import java.net.URLConnection;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Locale;
 
+import static hudson.Functions.jsStringEscape;
+import static hudson.Util.*;
+
 /**
  * Represents the result of the form field validation.
  *
@@ -506,4 +517,111 @@ protected void respond(StaplerResponse rsp, String html) throws IOException, Ser
         rsp.setContentType("text/html;charset=UTF-8");
         rsp.getWriter().print(html);
     }
+
+    /**
+     * Builds up the check URL for the client-side JavaScript to call back.
+     */
+    public static class CheckMethod {
+        private final Descriptor descriptor;
+        private final Method method;
+        private final String capitalizedFieldName;
+
+        /**
+         * Names of the parameters to pass from the client.
+         */
+        private final List<String> names;
+
+        private volatile String checkUrl;    // cached once computed
+        private volatile String dependsOn;  //  cached once computed
+
+        public CheckMethod(Descriptor descriptor, String fieldName) {
+            this.descriptor = descriptor;
+            this.capitalizedFieldName = StringUtils.capitalize(fieldName);
+
+            method = ReflectionUtils.getPublicMethodNamed(descriptor.getClass(), "doCheck" + capitalizedFieldName);
+            if(method !=null) {
+                names = new ArrayList<String>();
+                findParameters(method);
+            } else {
+                names = null;
+            }
+        }
+
+        /**
+         * Builds query parameter line by figuring out what should be submitted
+         */
+        private void findParameters(Method method) {
+            for (Parameter p : ReflectionUtils.getParameters(method)) {
+                QueryParameter qp = p.annotation(QueryParameter.class);
+                if (qp!=null) {
+                    String name = qp.value();
+                    if (name.length()==0) name = p.name();
+                    if (name==null || name.length()==0)
+                        continue;   // unknown parameter name. we'll report the error when the form is submitted.
+                    if (name.equals("value"))
+                        continue;   // 'value' parameter is implicit
+
+                    RelativePath rp = p.annotation(RelativePath.class);
+                    if (rp!=null)
+                        name = rp.value()+'/'+name;
+
+                    names.add(name);
+                    continue;
+                }
+
+                Method m = ReflectionUtils.getPublicMethodNamed(p.type(), "fromStapler");
+                if (m!=null)    findParameters(m);
+            }
+        }
+
+        /**
+         * Obtains the 1.526-compatible single string representation.
+         *
+         * This method computes JavaScript expression, which evaluates to the URL that the client should request
+         * the validation to.
+         * A modern version depends on {@link #toStemUrl()} and {@link #getDependsOn()}
+         */
+        public String toCheckUrl() {
+            if (names==null)    return null;
+
+            if (checkUrl==null) {
+                StringBuilder buf = new StringBuilder(singleQuote(relativePath()));
+                if (!names.isEmpty()) {
+                    buf.append("+qs(this).addThis()");
+
+                    for (String name : names) {
+                        buf.append(".nearBy('"+name+"')");
+                    }
+                    buf.append(".toString()");
+                }
+                checkUrl = buf.toString();
+            }
+
+            // put this under the right contextual umbrella.
+            // 'a' in getCurrentDescriptorByNameUrl is always non-null because we already have Hudson as the sentinel
+            return '\'' + jsStringEscape(Descriptor.getCurrentDescriptorByNameUrl()) + "/'+" + checkUrl;
+        }
+
+        /**
+         * Returns the URL that the JavaScript should hit to perform form validation, except
+         * the query string portion (which is built on the client side.)
+         */
+        public String toStemUrl() {
+            if (names==null)    return null;
+            return jsStringEscape(Descriptor.getCurrentDescriptorByNameUrl()) + '/' + relativePath();
+        }
+
+        public String getDependsOn() {
+            if (names==null)    return null;
+
+            if (dependsOn==null)
+                dependsOn = join(names," ");
+            return dependsOn;
+        }
+
+        private String relativePath() {
+            return descriptor.getDescriptorUrl() + "/check" + capitalizedFieldName;
+        }
+
+    }
 }
diff --git a/core/src/main/resources/lib/form/checkbox.jelly b/core/src/main/resources/lib/form/checkbox.jelly
@@ -61,7 +61,7 @@ THE SOFTWARE.
          value="${attrs.value}"
          title="${attrs.tooltip}"
          onclick="${attrs.onclick}" id="${attrs.id}" class="${attrs.negative!=null ? 'negative' : null} ${attrs.checkUrl!=null?'validated':''}"
-         checkUrl="${attrs.checkUrl}" json="${attrs.json}"
+         checkUrl="${attrs.checkUrl}" checkDependsOn="${attrs.checkDependsOn}" json="${attrs.json}"
          checked="${(attrs.checked ?: instance[attrs.field] ?: attrs.default) ? 'true' : null}"/>
   <j:if test="${attrs.title!=null}">
     <label class="attach-previous"

diff --git a/core/src/main/resources/lib/form/editableComboBox.jelly b/core/src/main/resources/lib/form/editableComboBox.jelly
@@ -49,7 +49,7 @@ THE SOFTWARE.
   <input id="${attrs.id}" autocomplete="off" class="combobox ${attrs.clazz}${attrs.checkUrl!=null ? ' validated' : ''}"
          name="${attrs.name ?: '_.'+attrs.field}"
          value="${attrs.value ?: instance[attrs.field]}"
-         checkUrl="${attrs.checkUrl}" />
+         checkUrl="${attrs.checkUrl}" checkDependsOn="${attrs.checkDependsOn}" />
   <div class="combobox-values">
     <j:if test="${items!=null}">
       <j:forEach var="v" items="${items}">

diff --git a/core/src/main/resources/lib/form/prepareDatabinding.jelly b/core/src/main/resources/lib/form/prepareDatabinding.jelly
@@ -33,6 +33,5 @@ THE SOFTWARE.
     <!-- this looks up the ancestor <entry> set by entry.jelly -->
     <j:set target="${pattrs}" property="field" value="${entry.field}" />
   </j:if>
-  <j:set target="${pattrs}" property="checkUrl"
-         value="${h.getCheckUrl(pattrs.checkUrl,descriptor,pattrs.field)}" />
+  <j:expr value="${h.calcCheckUrl(pattrs,  pattrs.checkUrl,descriptor,pattrs.field)}" />
 </j:jelly>
diff --git a/core/src/main/resources/lib/form/readOnlyTextbox.jelly b/core/src/main/resources/lib/form/readOnlyTextbox.jelly
@@ -68,7 +68,7 @@ THE SOFTWARE.
          id="${attrs.id}"
          type="text"
          readonly="readonly"
-         checkUrl="${attrs.checkUrl}" checkMethod="${attrs.checkMethod}"
+         checkUrl="${attrs.checkUrl}" checkDependsOn="${attrs.checkDependsOn}" checkMethod="${attrs.checkMethod}"
          checkMessage="${attrs.checkMessage}"
          onchange="${attrs.onchange}" onkeyup="${attrs.onkeyup}"/>
 </j:jelly>
diff --git a/core/src/main/resources/lib/form/textarea.jelly b/core/src/main/resources/lib/form/textarea.jelly
@@ -82,7 +82,7 @@ THE SOFTWARE.
   <textarea id="${attrs.id}" style="${attrs.style}"
             name ="${attrs.name ?: '_.'+attrs.field}"
             class="setting-input ${attrs.checkUrl!=null?'validated':''} ${attrs['codemirror-mode']!=null?'codemirror':''} ${attrs.class}"
-            checkUrl="${attrs.checkUrl}" checkMethod="${attrs.checkMethod}"
+            checkUrl="${attrs.checkUrl}" checkDependsOn="${attrs.checkDependsOn}" checkMethod="${attrs.checkMethod}"
             rows="${h.determineRows(value)}"
             readonly="${attrs.readonly}"
             codemirror-mode="${attrs['codemirror-mode']}"

diff --git a/test/src/main/java/org/jvnet/hudson/test/JenkinsRule.java b/test/src/main/java/org/jvnet/hudson/test/JenkinsRule.java
@@ -567,6 +567,8 @@ protected ServletContext createWebServer() throws Exception {
 
         SocketConnector connector = new SocketConnector();
         connector.setHeaderBufferSize(12*1024); // use a bigger buffer as Stapler traces can get pretty large on deeply nested URL
+        if (System.getProperty("port")!=null)
+            connector.setPort(Integer.parseInt(System.getProperty("port")));
 
         server.setThreadPool(new ThreadPoolImpl(new ThreadPoolExecutor(10, 10, 10L, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(),new ThreadFactory() {
             public Thread newThread(Runnable r) {