introduced a new scheme to the form field validation, that makes the check...

introduced a new scheme to the form field validation, that makes the check feel more like a function.

Using it in a few places to begin with

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

core/pom.xml

@@ -403,8 +403,22 @@ THE SOFTWARE.
     </dependency>
     <dependency>
       <groupId>org.kohsuke.stapler</groupId>
-      <artifactId>stapler</artifactId>
-      <version>1.96</version>
+      <artifactId>stapler-jelly</artifactId>
+      <version>1.97</version>
+      <exclusions>
+        <exclusion>
+          <groupId>dom4j</groupId>
+          <artifactId>dom4j</artifactId>
+        </exclusion>
+        <exclusion>
+          <groupId>commons-jelly</groupId>
+          <artifactId>commons-jelly</artifactId>
+        </exclusion>
+        <exclusion>
+          <groupId>commons-jexl</groupId>
+          <artifactId>commons-jexl</artifactId>
+        </exclusion>
+      </exclusions>
     </dependency>
     <dependency>
       <groupId>org.jvnet.localizer</groupId>

core/src/main/java/hudson/WebAppMain.java

@@ -43,6 +43,7 @@ import hudson.util.AWTProblem;
 import org.jvnet.localizer.LocaleProvider;
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.jelly.JellyFacet;
 import org.apache.tools.ant.types.FileSet;
 
 import javax.naming.Context;
@@ -221,7 +222,7 @@ public final class WebAppMain implements ServletContextListener {
     }
 
     public static void installExpressionFactory(ServletContextEvent event) {
-        Stapler.setExpressionFactory(event, new ExpressionFactory2());
+        JellyFacet.setExpressionFactory(event, new ExpressionFactory2());
     }
 
 	/**

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

@@ -104,6 +104,7 @@ import hudson.util.DescribableList;
 import hudson.util.Futures;
 import hudson.util.Memoizer;
 import hudson.util.Iterators;
+import hudson.util.FormValidation;
 import hudson.widgets.Widget;
 import net.sf.json.JSONObject;
 import org.acegisecurity.*;
@@ -121,6 +122,7 @@ import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.StaplerFallback;
 import org.kohsuke.stapler.WebApp;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.jelly.JellyClassLoaderTearOff;
 import org.kohsuke.stapler.jelly.JellyRequestDispatcher;
 import org.kohsuke.stapler.framework.adjunct.AdjunctManager;
@@ -2687,90 +2689,68 @@ public final class Hudson extends Node implements ItemGroup<TopLevelItem>, Stapl
     /**
      * Checks if the JAVA_HOME is a valid JAVA_HOME path.
      */
-    public void doJavaHomeCheck( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
+    public FormValidation doJavaHomeCheck(@QueryParameter File value) {
         // this can be used to check the existence of a file on the server, so needs to be protected
-        new FormFieldValidator(req,rsp,true) {
-            public void check() throws IOException, ServletException {
-                File f = getFileParameter("value");
-                if(!f.isDirectory()) {
-                    error(Messages.Hudson_NotADirectory(f));
-                    return;
-                }
+        checkPermission(ADMINISTER);
 
-                File toolsJar = new File(f,"lib/tools.jar");
-                File mac = new File(f,"lib/dt.jar");
-                if(!toolsJar.exists() && !mac.exists()) {
-                    error(Messages.Hudson_NotJDKDir(f));
-                    return;
-                }
+        if(!value.isDirectory())
+            return FormValidation.error(Messages.Hudson_NotADirectory(value));
 
-                ok();
-            }
-        }.process();
+        File toolsJar = new File(value,"lib/tools.jar");
+        File mac = new File(value,"lib/dt.jar");
+        if(!toolsJar.exists() && !mac.exists())
+            return FormValidation.error(Messages.Hudson_NotJDKDir(value));
+
+        return FormValidation.ok();
     }
 
     /**
      * If the user chose the default JDK, make sure we got 'java' in PATH.
      */
-    public void doDefaultJDKCheck( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
-        new FormFieldValidator(req,rsp,false) {
-            public void check() throws IOException, ServletException {
-                String v = request.getParameter("value");
-                if(!v.equals("(Default)"))
-                    // assume the user configured named ones properly in system config ---
-                    // or else system config should have reported form field validation errors.
-                    ok();
-                else {
-                    // default JDK selected. Does such java really exist?
-                    if(JDK.isDefaultJDKValid(Hudson.this))
-                        ok();
-                    else
-                        errorWithMarkup(Messages.Hudson_NoJavaInPath(request.getContextPath()));
-                }
-            }
-        }.process();
+    public FormValidation doDefaultJDKCheck(StaplerRequest request, @QueryParameter String value) {
+        if(!value.equals("(Default)"))
+            // assume the user configured named ones properly in system config ---
+            // or else system config should have reported form field validation errors.
+            return FormValidation.ok();
+
+        // default JDK selected. Does such java really exist?
+        if(JDK.isDefaultJDKValid(Hudson.this))
+            return FormValidation.ok();
+        else
+            return FormValidation.errorWithMarkup(Messages.Hudson_NoJavaInPath(request.getContextPath()));
     }
 
     /**
      * Checks if the top-level item with the given name exists.
      */
-    public void doItemExistsCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
+    public FormValidation doItemExistsCheck(@QueryParameter String value) {
         // this method can be used to check if a file exists anywhere in the file system,
         // so it should be protected.
-        new FormFieldValidator(req,rsp,Item.CREATE) {
-            protected void check() throws IOException, ServletException {
-                String job = fixEmpty(request.getParameter("value"));
-                if(job==null) {
-                    ok(); // nothing is entered yet
-                    return;
-                }
-
-                if(getItem(job)==null)
-                    ok();
-                else
-                    error(Messages.Hudson_JobAlreadyExists(job));
-            }
-        }.process();
+        checkPermission(Item.CREATE);
+        
+        String job = fixEmpty(value);
+        if(job==null)
+            return FormValidation.ok(); // nothing is entered yet
+
+        if(getItem(job)==null)
+            return FormValidation.ok();
+        else
+            return FormValidation.error(Messages.Hudson_JobAlreadyExists(job));
     }
 
     /**
      * Checks if a top-level view with the given name exists.
      */
-    public void doViewExistsCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
-        new FormFieldValidator(req,rsp,View.CREATE) {
-            protected void check() throws IOException, ServletException {
-                String view = fixEmpty(request.getParameter("value"));
-                if(view==null) {
-                    ok(); // nothing is entered yet
-                    return;
-                }
+    public FormValidation doViewExistsCheck(@QueryParameter String value) {
+        checkPermission(View.CREATE);
 
-                if(getView(view)==null)
-                    ok();
-                else
-                    error(Messages.Hudson_ViewAlreadyExists(view));
-            }
-        }.process();
+        String view = fixEmpty(value);
+        if(view==null)  return FormValidation.ok(); // nothing is entered yet
+
+        if(getView(view)==null)
+            return FormValidation.ok();
+        else
+            return FormValidation.error(Messages.Hudson_ViewAlreadyExists(view));
     }
 
 

core/src/main/java/hudson/scm/CVSSCM.java

@@ -32,7 +32,6 @@ import hudson.Util;
 import hudson.Extension;
 import static hudson.Util.fixEmpty;
 import static hudson.Util.fixNull;
-import static hudson.Util.fixEmptyAndTrim;
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
 import hudson.model.BuildListener;
@@ -51,6 +50,7 @@ import hudson.util.ArgumentListBuilder;
 import hudson.util.ForkOutputStream;
 import hudson.util.FormFieldValidator;
 import hudson.util.IOException2;
+import hudson.util.FormValidation;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.tools.ant.BuildException;
@@ -60,6 +60,7 @@ import org.apache.tools.zip.ZipOutputStream;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.framework.io.ByteBuffer;
 
 import javax.servlet.ServletException;
@@ -1165,17 +1166,13 @@ public class CVSSCM extends SCM implements Serializable {
         /**
          * Checks the correctness of the branch name.
          */
-        public void doCheckBranch(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
-            new FormFieldValidator(req,rsp,false) {
-                protected void check() throws IOException, ServletException {
-                    String v = fixNull(request.getParameter("value"));
+        public FormValidation doCheckBranch(@QueryParameter String value) {
+            String v = fixNull(value);
 
-                    if(v.equals("HEAD"))
-                        error(Messages.CVSSCM_HeadIsNotBranch());
-                    else
-                        ok();
-                }
-            }.process();
+            if(v.equals("HEAD"))
+                return FormValidation.error(Messages.CVSSCM_HeadIsNotBranch());
+
+            return FormValidation.ok();
         }
 
         /**
@@ -1183,82 +1180,61 @@ public class CVSSCM extends SCM implements Serializable {
          * <p>
          * Also checks if .cvspass file contains the entry for this.
          */
-        public void doCheckCvsRoot(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
-            new FormFieldValidator(req,rsp,false) {
-                protected void check() throws IOException, ServletException {
-                    String v = fixEmpty(request.getParameter("value"));
-                    if(v==null) {
-                        error(Messages.CVSSCM_MissingCvsroot());
-                        return;
-                    }
-
-                    Matcher m = CVSROOT_PSERVER_PATTERN.matcher(v);
+        public FormValidation doCheckCvsRoot(@QueryParameter String value) throws IOException {
+            String v = fixEmpty(value);
+            if(v==null)
+                return FormValidation.error(Messages.CVSSCM_MissingCvsroot());
+
+            Matcher m = CVSROOT_PSERVER_PATTERN.matcher(v);
+
+            // CVSROOT format isn't really that well defined. So it's hard to check this rigorously.
+            if(v.startsWith(":pserver") || v.startsWith(":ext")) {
+                if(!m.matches())
+                    return FormValidation.error(Messages.CVSSCM_InvalidCvsroot());
+                // I can't really test if the machine name exists, either.
+                // some cvs, such as SOCKS-enabled cvs can resolve host names that Hudson might not
+                // be able to. If :ext is used, all bets are off anyway.
+            }
 
-                    // CVSROOT format isn't really that well defined. So it's hard to check this rigorously.
-                    if(v.startsWith(":pserver") || v.startsWith(":ext")) {
-                        if(!m.matches()) {
-                            error(Messages.CVSSCM_InvalidCvsroot());
-                            return;
-                        }
-                        // I can't really test if the machine name exists, either.
-                        // some cvs, such as SOCKS-enabled cvs can resolve host names that Hudson might not
-                        // be able to. If :ext is used, all bets are off anyway.
+            // check .cvspass file to see if it has entry.
+            // CVS handles authentication only if it's pserver.
+            if(v.startsWith(":pserver")) {
+                if(m.group(2)==null) {// if password is not specified in CVSROOT
+                    String cvspass = getCvspassFile();
+                    File passfile;
+                    if(cvspass.equals("")) {
+                        passfile = new File(new File(System.getProperty("user.home")),".cvspass");
+                    } else {
+                        passfile = new File(cvspass);
                     }
 
-                    // check .cvspass file to see if it has entry.
-                    // CVS handles authentication only if it's pserver.
-                    if(v.startsWith(":pserver")) {
-                        if(m.group(2)==null) {// if password is not specified in CVSROOT
-                            String cvspass = getCvspassFile();
-                            File passfile;
-                            if(cvspass.equals("")) {
-                                passfile = new File(new File(System.getProperty("user.home")),".cvspass");
-                            } else {
-                                passfile = new File(cvspass);
-                            }
-
-                            if(passfile.exists()) {
-                                // It's possible that we failed to locate the correct .cvspass file location,
-                                // so don't report an error if we couldn't locate this file.
-                                //
-                                // if this is explicitly specified, then our system config page should have
-                                // reported an error.
-                                if(!scanCvsPassFile(passfile, v)) {
-                                    error(Messages.CVSSCM_PasswordNotSet());
-                                    return;
-                                }
-                            }
-                        }
+                    if(passfile.exists()) {
+                        // It's possible that we failed to locate the correct .cvspass file location,
+                        // so don't report an error if we couldn't locate this file.
+                        //
+                        // if this is explicitly specified, then our system config page should have
+                        // reported an error.
+                        if(!scanCvsPassFile(passfile, v))
+                            return FormValidation.error(Messages.CVSSCM_PasswordNotSet());
                     }
-
-                    // all tests passed so far
-                    ok();
                 }
-            }.process();
+            }
+            return FormValidation.ok();
         }
 
-	     /**
+	    /**
          * Validates the excludeRegions Regex
          */
-        public void doExcludeRegionsCheck(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
-            new FormFieldValidator(req,rsp,false) {
-                protected void check() throws IOException, ServletException {
-                    String v = fixEmptyAndTrim(request.getParameter("value"));
-
-                    if(v != null) {
-	                    String[] regions = v.split("[\\r\\n]+");
-	                    for (String region : regions) {
-		                    try {
-			                    Pattern.compile(region);
-		                    }
-		                    catch (PatternSyntaxException e) {
-			                    error("Invalid regular expression. " + e.getMessage());
-		                    }
-	                    }
-                    }
-                    ok();
+        public FormValidation doExcludeRegionsCheck(@QueryParameter String value) {
+            String v = fixNull(value).trim();
+
+            for (String region : v.split("[\\r\\n]+"))
+                try {
+                    Pattern.compile(region);
+                } catch (PatternSyntaxException e) {
+                    return FormValidation.error("Invalid regular expression. " + e.getMessage());
                 }
-            }.process();
+            return FormValidation.ok();
         }
 
         /**

core/src/main/java/hudson/util/FormValidation.java

+/*
+ * The MIT License
+ *
+ * Copyright (c) 2004-2009, Sun Microsystems, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package hudson.util;
+
+import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
+
+import javax.servlet.ServletException;
+import java.io.IOException;
+
+import hudson.Util;
+import hudson.scm.CVSSCM;
+import hudson.model.Hudson;
+
+/**
+ * Represents the result of the form field validation.
+ *
+ * <p>
+ * Use one of the factory methods to create an instance, then return it from your <tt>doCheckXyz</tt>
+ * method. (Via {@link HttpResponse}, the returned object will render the result into {@link StaplerResponse}.)
+ * This way of designing form field validation allows you to reuse {@code doCheckXyz()} methods
+ * programmatically as well (by using {@link #kind}.
+ *
+ * <p>
+ * See {@link CVSSCM.DescriptorImpl#doCheckCvsRoot(String)} as an example.
+ *
+ * @author Kohsuke Kawaguchi
+ * @since 1.294
+ */
+public abstract class FormValidation implements HttpResponse {
+    /**
+     * Indicates the kind of result.
+     */
+    public enum Kind {
+        /**
+         * Form field value was OK and no problem was detected.
+         */
+        OK,
+        /**
+         * Form field value contained something suspicious. For some limited use cases
+         * the value could be valid, but we suspect the user made a mistake.
+         */
+        WARNING,
+        /**
+         * Form field value contained a problem that should be corrected.
+         */
+        ERROR
+    }
+
+    public static FormValidation ok() {
+        return OK;
+    }
+
+    /**
+     * Sends out a string error message that indicates an error.
+     *
+     * @param message
+     *      Human readable message to be sent. <tt>error(null)</tt>
+     *      can be used as <tt>ok()</tt>.
+     */
+    public static FormValidation error(String message) {
+        return errorWithMarkup(message==null?null: Util.escape(message));
+    }
+
+    public static FormValidation warning(String message) {
+        return warningWithMarkup(message==null?null:Util.escape(message));
+    }
+
+    public static FormValidation ok(String message) {
+        return okWithMarkup(message==null?null:Util.escape(message));
+    }
+
+    /**
+     * Sends out a string error message that indicates an error,
+     * by formatting it with {@link String#format(String, Object[])}
+     */
+    public static FormValidation error(String format, Object... args) {
+        return error(String.format(format,args));
+    }
+
+    public static FormValidation warning(String format, Object... args) {
+        return warning(String.format(format,args));
+    }
+
+    public static FormValidation ok(String format, Object... args) {
+        return ok(String.format(format,args));
+    }
+
+    /**
+     * Sends out an HTML fragment that indicates an error.
+     *
+     * <p>
+     * This method must be used with care to avoid cross-site scripting
+     * attack.
+     *
+     * @param message
+     *      Human readable message to be sent. <tt>error(null)</tt>
+     *      can be used as <tt>ok()</tt>.
+     */
+    public static FormValidation errorWithMarkup(String message) {
+        return _errorWithMarkup(message,Kind.ERROR);
+    }
+
+    public static FormValidation warningWithMarkup(String message) {
+        return _errorWithMarkup(message,Kind.WARNING);
+    }
+
+    public static FormValidation okWithMarkup(String message) {
+        return _errorWithMarkup(message,Kind.OK);
+    }
+
+    private static FormValidation _errorWithMarkup(final String message, final Kind kind) {
+        if(message==null)
+            return ok();
+        return new FormValidation(kind) {
+            public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
+                // 1x16 spacer needed for IE since it doesn't support min-height
+                respond(rsp,"<div class="+ kind.name().toLowerCase() +"><img src='"+
+                        req.getContextPath()+ Hudson.RESOURCE_PATH+"/images/none.gif' height=16 width=1>"+
+                        message+"</div>");
+            }
+        };
+    }
+
+    public final Kind kind;
+
+    /**
+     * Instances should be created via one of the factory methods above.
+     * @param kind
+     */
+    private FormValidation(Kind kind) {
+        this.kind = kind;
+    }
+
+    /**
+     * Sends out an arbitrary HTML fragment as the output.
+     */
+    protected void respond(StaplerResponse rsp, String html) throws IOException, ServletException {
+        rsp.setContentType("text/html");
+        rsp.getWriter().print(html);
+    }
+
+    /**
+     * Singleton instance that represents "OK" without any message. By far the most common case.
+     */
+    private static final FormValidation OK = new FormValidation(Kind.OK) {
+        public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
+            respond(rsp,"<div/>");
+        }
+    };
+}