aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRoy Varghese2014-05-19 17:12:32 (EDT)
committerRoy Varghese2014-05-19 17:12:32 (EDT)
commit7376d3ba4ad8d9051babac8cca7d82479b88e3c5 (patch)
tree36e4a722d0fc71a3e9c0fed52d927747e24b6cfb
parent202e5a6950dbafbacc8c11d87639bdb6f5ab75d8 (diff)
downloadorg.eclipse.hudson.core-7376d3ba4ad8d9051babac8cca7d82479b88e3c5.zip
org.eclipse.hudson.core-7376d3ba4ad8d9051babac8cca7d82479b88e3c5.tar.gz
org.eclipse.hudson.core-7376d3ba4ad8d9051babac8cca7d82479b88e3c5.tar.bz2
Removed redundant checks of request parameters from job configuration submit.refs/changes/84/26884/1
The json data submitted by the form has all necessary values, no need to check for form fields. Signed-off-by: Roy Varghese <rovarghe@gmail.com>
-rw-r--r--hudson-core/src/main/java/hudson/model/AbstractProject.java33
-rw-r--r--hudson-core/src/main/java/hudson/model/FreeStyleProject.java13
-rw-r--r--hudson-core/src/main/java/hudson/model/Job.java77
3 files changed, 77 insertions, 46 deletions
diff --git a/hudson-core/src/main/java/hudson/model/AbstractProject.java b/hudson-core/src/main/java/hudson/model/AbstractProject.java
index 5e26901..650af9c 100644
--- a/hudson-core/src/main/java/hudson/model/AbstractProject.java
+++ b/hudson-core/src/main/java/hudson/model/AbstractProject.java
@@ -2044,16 +2044,29 @@ public abstract class AbstractProject<P extends AbstractProject<P, R>, R extends
@Override
protected void submit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, FormException {
super.submit(req, rsp);
+
+ JSONObject json = req.getSubmittedForm();
- makeDisabled(null != req.getParameter("disable"));
- setJDK(req.getParameter("jdk"));
- setQuietPeriod(null != req.getParameter(HAS_QUIET_PERIOD_PROPERTY_NAME)
- ? req.getParameter("quiet_period") : null);
- setScmCheckoutRetryCount(null != req.getParameter(HAS_SCM_CHECKOUT_RETRY_COUNT_PROPERTY_NAME)
- ? req.getParameter("scmCheckoutRetryCount") : null);
- setBlockBuildWhenDownstreamBuilding(null != req.getParameter("blockBuildWhenDownstreamBuilding"));
- setBlockBuildWhenUpstreamBuilding(null != req.getParameter("blockBuildWhenUpstreamBuilding"));
+ makeDisabled(json.has("disable"));
+
+ setJDK(json.has("jdk") ?json.getString("jdk") : null);
+
+ JSONObject hasQuietPeriod = json.has(HAS_QUIET_PERIOD_PROPERTY_NAME) ?
+ json.getJSONObject(HAS_QUIET_PERIOD_PROPERTY_NAME):null;
+ setQuietPeriod(hasQuietPeriod != null ?
+ hasQuietPeriod.getString("quiet_period") : null);
+
+ JSONObject hasScmCheckoutRetryCount = json.has(HAS_SCM_CHECKOUT_RETRY_COUNT_PROPERTY_NAME)?
+ json.getJSONObject(HAS_SCM_CHECKOUT_RETRY_COUNT_PROPERTY_NAME):null;
+ setScmCheckoutRetryCount(hasScmCheckoutRetryCount != null ?
+ hasScmCheckoutRetryCount.getString("scmCheckoutRetryCount"): null);
+
+ // Following values should be using a boolean property
+ // but keep using truthy test for now to preserve compatibility
+ setBlockBuildWhenDownstreamBuilding(json.has("blockBuildWhenDownstreamBuilding"));
+ setBlockBuildWhenUpstreamBuilding(json.has("blockBuildWhenUpstreamBuilding"));
+ // TODO: Convert to JSON property
if (req.getParameter(APPOINTED_NODE_PROPERTY_NAME) != null) {
// New logic for handling whether this choice came from the dropdown or textfield.
if (BASIC_KEY.equals(req.getParameter(AFFINITY_CHO0SER_KEY))) {
@@ -2068,9 +2081,9 @@ public abstract class AbstractProject<P extends AbstractProject<P, R>, R extends
setAppointedNode(null);
}
- setCleanWorkspaceRequired(null != req.getParameter("cleanWorkspaceRequired"));
- setConcurrentBuild(req.getSubmittedForm().has("concurrentBuild"));
+ setCleanWorkspaceRequired(json.has("cleanWorkspaceRequired"));
+ setConcurrentBuild(json.has("concurrentBuild"));
authToken = BuildAuthorizationToken.create(req);
diff --git a/hudson-core/src/main/java/hudson/model/FreeStyleProject.java b/hudson-core/src/main/java/hudson/model/FreeStyleProject.java
index 034c7f8..97376fe 100644
--- a/hudson-core/src/main/java/hudson/model/FreeStyleProject.java
+++ b/hudson-core/src/main/java/hudson/model/FreeStyleProject.java
@@ -17,16 +17,14 @@
package hudson.model;
import hudson.Extension;
-
import hudson.util.CascadingUtil;
import java.io.IOException;
-
+import javax.servlet.ServletException;
+import net.sf.json.JSONObject;
import org.eclipse.hudson.api.model.IFreeStyleProject;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
-import javax.servlet.ServletException;
-
/**
* Free-style software project.
*
@@ -78,8 +76,11 @@ public class FreeStyleProject extends Project<FreeStyleProject, FreeStyleBuild>
protected void submit(StaplerRequest req, StaplerResponse rsp)
throws IOException, ServletException, Descriptor.FormException {
super.submit(req, rsp);
- setCustomWorkspace(
- req.hasParameter("customWorkspace") ? req.getParameter("customWorkspace.directory") : null);
+ JSONObject json = req.getSubmittedForm();
+ JSONObject customWorkspace = json.has("customWorkspace")?
+ json.getJSONObject("customWorkspace"):null;
+ setCustomWorkspace( customWorkspace != null?
+ customWorkspace.getString("directory") : null);
}
@Override
diff --git a/hudson-core/src/main/java/hudson/model/Job.java b/hudson-core/src/main/java/hudson/model/Job.java
index 89f8c77..5d3d600 100644
--- a/hudson-core/src/main/java/hudson/model/Job.java
+++ b/hudson-core/src/main/java/hudson/model/Job.java
@@ -13,7 +13,7 @@
* Stephen Connolly, Tom Huybrechts, Anton Kozak, Nikita Levyankov,
* Roy Varghese
*
- *******************************************************************************/
+ *******************************************************************************/
package hudson.model;
@@ -142,10 +142,10 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
private volatile LogRotator logRotator;
private transient ConcurrentMap<String, IProjectProperty> jobProperties = new ConcurrentHashMap<String, IProjectProperty>();
/**
- * This field is used for persisting only the overridenJobProperties in the config.xml
+ * This field is used for persisting only the overridenJobProperties in the config.xml
*/
private ConcurrentMap<String, IProjectProperty> persistableJobProperties = new ConcurrentHashMap<String, IProjectProperty>();
-
+
/**
* Not all plugins are good at calculating their health report quickly.
* These fields are used to cache the health reports to speed up rendering
@@ -206,17 +206,17 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
protected Job(ItemGroup parent, String name) {
super(parent, name);
}
-
+
public Object readResolve() {
if (persistableJobProperties == null) {
persistableJobProperties = new ConcurrentHashMap<String, IProjectProperty>();
}
return this;
}
-
+
//Bug Fix: 406889 - Non overriden job properties or properties with no values should not be written to config.xml
private Object writeReplace() throws ObjectStreamException, IOException {
-
+
persistableJobProperties.clear();
for (String key : jobProperties.keySet()) {
persistableJobProperties.put(key, jobProperties.get(key));
@@ -349,7 +349,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
cascadingChildrenNames.remove(cascadingChildName);
save();
}
-
+
//Cleanup any cascading mess (called during Hudson startup)
public void cleanCascading() throws IOException{
Set<String> cascadingChildrenToRemove = new HashSet();
@@ -456,7 +456,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
}
save();
}
-
+
protected void initAllowSave() {
// No need to init, ThreadLocal is now a final static variable
// allowSave = new ThreadLocal<Boolean>() {
@@ -651,7 +651,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
public boolean isNameEditable() {
return true;
}
-
+
/**
* If team enabled, allow user to edit only the unqualified portion
* of the job name; otherwise, edit the full name.
@@ -983,8 +983,8 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
protected HistoryWidget createHistoryWidget() {
return new HistoryWidget(this, getBuildHistoryData(), HISTORY_ADAPTER);
}
-
- protected static final HistoryWidget.Adapter<BuildHistory.Record> HISTORY_ADAPTER =
+
+ protected static final HistoryWidget.Adapter<BuildHistory.Record> HISTORY_ADAPTER =
new Adapter<BuildHistory.Record>() {
public int compare(BuildHistory.Record record, String key) {
try {
@@ -1190,9 +1190,9 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
return new File(getRootDir(), BUILDS_DIRNAME);
}
}
-
+
public abstract BuildHistory<JobT,RunT> getBuildHistoryData();
-
+
/**
* Gets all the runs.
*
@@ -1479,10 +1479,10 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
String newName = req.getParameter("name");
if (newName != null && !newName.equals(getEditableName())) {
-
+
// check this error early to avoid HTTP response splitting.
newName = Hudson.checkGoodJobName(newName);
-
+
if (Hudson.getInstance().isTeamManagementEnabled()) {
// Make the name qualified in the same team before confirm
TeamManager teamManager = Hudson.getInstance().getTeamManager();
@@ -1516,10 +1516,27 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
*/
protected void submit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, FormException {
JSONObject json = req.getSubmittedForm();
- description = req.getParameter("description");
- keepDependencies = req.getParameter("keepDependencies") != null;
+
+ description = json.getString("description");
+
+ // Support both ways of setting dependencies
+ // 1. directly in the root of the json
+ // 2. within the fingerprinter section - for compatibility
+ keepDependencies = false;
+ if (json.has("keepDependencies")) {
+ keepDependencies = json.getBoolean("keepDependencies");
+ }
+ else {
+ if ( json.has("hudson-tasks-Fingerprinter")) {
+ JSONObject fingerPrinter = json.getJSONObject("hudson-tasks-Fingerprinter");
+ if ( fingerPrinter.has("keepDependencies")) {
+ keepDependencies = fingerPrinter.getBoolean("keepDependencies");
+ }
+ }
+ }
+
properties.clear();
- setCascadingProjectName(StringUtils.trimToNull(req.getParameter("cascadingProjectName")));
+ setCascadingProjectName(StringUtils.trimToNull(json.getString("cascadingProjectName")));
CopyOnWriteList parameterDefinitionProperties = new CopyOnWriteList();
int i = 0;
for (JobPropertyDescriptor d : JobPropertyDescriptor.getPropertyDescriptors(Job.this.getClass())) {
@@ -1549,7 +1566,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
}
setParameterDefinitionProperties(parameterDefinitionProperties);
LogRotator logRotator = null;
- if (null != req.getParameter("logrotate")) {
+ if (json.has("logrotate")) {
logRotator = LogRotator.DESCRIPTOR.newInstance(req, json.getJSONObject("logrotate"));
}
setLogRotator(logRotator);
@@ -1606,13 +1623,13 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
String newName = req.getParameter("newName");
String checkName = newName;
-
+
if (isBuilding()) {
// redirect to page explaining that we can't rename now
rsp.sendRedirect("rename?newName=" + URLEncoder.encode(newName, "UTF-8"));
return;
}
-
+
if (Hudson.getInstance().isTeamManagementEnabled()) {
// Do this before rename or team will change to default for user
TeamManager teamManager = Hudson.getInstance().getTeamManager();
@@ -1624,14 +1641,14 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
throw new Failure("Job name "+newName+" is improperly qualified");
}
checkName = teamManager.getUnqualifiedJobName(team, newName);
-
+
teamManager.addJob(team, newName);
} catch (TeamManager.TeamNotFoundException ex) {
// Can't happen with non-null team
}
}
}
-
+
Hudson.checkGoodJobName(checkName);
renameTo(newName);
@@ -1825,18 +1842,18 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
Graph graph = new Graph(getLastBuild().getTimestamp(), 500, 400);
DataSet<String, ChartLabel> data = new DataSet<String, ChartLabel>();
-
+
StaplerRequest req = Stapler.getCurrentRequest();
GraphSeries<String> xSeries = new GraphSeries<String>("Build No.");
data.setXSeries(xSeries);
GraphSeries<Number> ySeriesAborted = new GraphSeries<Number>(GraphSeries.TYPE_AREA, "Aborted", ColorPalette.GREY, false, false);
- ySeriesAborted.setBaseURL(getRelPath(req) + "/${buildNo}");
+ ySeriesAborted.setBaseURL(getRelPath(req) + "/${buildNo}");
data.addYSeries(ySeriesAborted);
GraphSeries<Number> ySeriesFailed = new GraphSeries<Number>(GraphSeries.TYPE_AREA, "Failed", ColorPalette.RED, false, false);
- ySeriesFailed.setBaseURL(getRelPath(req) + "/${buildNo}");
+ ySeriesFailed.setBaseURL(getRelPath(req) + "/${buildNo}");
data.addYSeries(ySeriesFailed);
GraphSeries<Number> ySeriesUnstable = new GraphSeries<Number>(GraphSeries.TYPE_AREA, "Unstable", ColorPalette.YELLOW, false, false);
@@ -1880,7 +1897,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
data.add(duration, "min",
new TimeTrendChartLabel(run));
}
-
+
// We want to display the build result from older to latest
data.reverseOrder();
graph.setYAxisLabel(Messages.Job_minutes());
@@ -1889,7 +1906,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
return graph;
}
-
+
// For backward compatibility with JFreechart
private class TimeTrendChartLabel extends ChartLabel {
@@ -1957,7 +1974,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
return run.getDisplayName() + " : " + run.getDurationString();
}
}
-
+
private String getRelPath(StaplerRequest req) {
String relPath = req.getParameter("rel");
if (relPath == null) {
@@ -1965,5 +1982,5 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
}
return relPath;
}
-
+
}