Skip to content

Commit

Permalink
[FIXED JENKINS-15817] XStream form of projects excessively strict abo…
Browse files Browse the repository at this point in the history 
…ut null fields.
  • Loading branch information
@jglick
jglick committed Nov 13, 2012
1 parent 2dbf8c5 commit 7d2c0ef
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 49 deletions.
3 changes: 3 additions & 0 deletions changelog.html
View file
Expand Up @@ -55,6 +55,9 @@
<!-- Record your changes in the trunk here. -->
<div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
<ul class=image>
<li class=rfe>
XStream form of projects excessively strict about null fields.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-15817">issue 15817</a>)
<li class=bug>
Build records were broken if timezone was changed while running.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-15816">issue 15816</a>)
Expand Down
27 changes: 13 additions & 14 deletions core/src/main/java/hudson/model/AbstractProject.java
View file
Expand Up @@ -39,7 +39,6 @@
import hudson.Util;
import hudson.cli.declarative.CLIMethod;
import hudson.cli.declarative.CLIResolver;
import hudson.diagnosis.OldDataMonitor;
import hudson.model.Cause.LegacyCodeCause;
import hudson.model.Cause.RemoteCause;
import hudson.model.Cause.UserIdCause;
Expand Down Expand Up @@ -80,7 +79,6 @@
import hudson.util.DescribableList;
import hudson.util.EditDistance;
import hudson.util.FormValidation;
import hudson.util.RunList;
import hudson.widgets.BuildHistoryWidget;
import hudson.widgets.HistoryWidget;
import jenkins.model.Jenkins;
Expand Down Expand Up @@ -285,13 +283,7 @@ public R create(File dir) throws IOException {
}
}
this.builds = builds;

if(triggers==null) {
// it didn't exist in < 1.28
triggers = new Vector<Trigger<?>>();
OldDataMonitor.report(this, "1.28");
}
for (Trigger t : triggers)
for (Trigger t : triggers())
t.start(this, Items.updatingByXml.get());
if(scm==null)
scm = new NullSCM(); // perhaps it was pointing to a plugin that no longer exists.
Expand All @@ -301,6 +293,13 @@ public R create(File dir) throws IOException {
updateTransientActions();
}

private synchronized List<Trigger<?>> triggers() {
if (triggers == null) {
triggers = new Vector<Trigger<?>>();
}
return triggers;
}

@Override
public EnvVars getEnvironment(Node node, TaskListener listener) throws IOException, InterruptedException {
EnvVars env = super.getEnvironment(node, listener);
Expand Down Expand Up @@ -1531,11 +1530,11 @@ public void setScm(SCM scm) throws IOException {
* Adds a new {@link Trigger} to this {@link Project} if not active yet.
*/
public void addTrigger(Trigger<?> trigger) throws IOException {
addToList(trigger,triggers);
addToList(trigger,triggers());
}

public void removeTrigger(TriggerDescriptor trigger) throws IOException {
removeFromList(trigger,triggers);
removeFromList(trigger,triggers());
}

protected final synchronized <T extends Describable<T>>
Expand Down Expand Up @@ -1569,14 +1568,14 @@ void removeFromList(Descriptor<T> item, List<T> collection) throws IOException {

@SuppressWarnings("unchecked")
public synchronized Map<TriggerDescriptor,Trigger> getTriggers() {
return (Map)Descriptor.toMap(triggers);
return (Map)Descriptor.toMap(triggers());
}

/**
* Gets the specific trigger, or null if the propert is not configured for this job.
*/
public <T extends Trigger> T getTrigger(Class<T> clazz) {
for (Trigger p : triggers) {
for (Trigger p : triggers()) {
if(clazz.isInstance(p))
return clazz.cast(p);
}
Expand Down Expand Up @@ -1862,7 +1861,7 @@ protected void submit(StaplerRequest req, StaplerResponse rsp) throws IOExceptio

setScm(SCMS.parseSCM(req,this));

for (Trigger t : triggers)
for (Trigger t : triggers())
t.stop();
triggers = buildDescribable(req, Trigger.for_(this));
for (Trigger t : triggers)
Expand Down
68 changes: 33 additions & 35 deletions core/src/main/java/hudson/model/Project.java
View file
Expand Up @@ -25,10 +25,8 @@
package hudson.model;

import hudson.Util;
import hudson.diagnosis.OldDataMonitor;
import hudson.model.Descriptor.FormException;
import hudson.tasks.BuildStep;
import hudson.tasks.BuildStepDescriptor;
import hudson.tasks.BuildWrapper;
import hudson.tasks.BuildWrappers;
import hudson.tasks.Builder;
Expand Down Expand Up @@ -61,20 +59,17 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
/**
* List of active {@link Builder}s configured for this project.
*/
private DescribableList<Builder,Descriptor<Builder>> builders =
new DescribableList<Builder,Descriptor<Builder>>(this);
private DescribableList<Builder,Descriptor<Builder>> builders;

/**
* List of active {@link Publisher}s configured for this project.
*/
private DescribableList<Publisher,Descriptor<Publisher>> publishers =
new DescribableList<Publisher,Descriptor<Publisher>>(this);
private DescribableList<Publisher,Descriptor<Publisher>> publishers;

/**
* List of active {@link BuildWrapper}s configured for this project.
*/
private DescribableList<BuildWrapper,Descriptor<BuildWrapper>> buildWrappers =
new DescribableList<BuildWrapper,Descriptor<BuildWrapper>>(this);
private DescribableList<BuildWrapper,Descriptor<BuildWrapper>> buildWrappers;

/**
* Creates a new project.
Expand All @@ -86,23 +81,17 @@ public Project(ItemGroup parent,String name) {
@Override
public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOException {
super.onLoad(parent, name);

if (buildWrappers==null) {
// it didn't exist in < 1.64
buildWrappers = new DescribableList<BuildWrapper, Descriptor<BuildWrapper>>(this);
OldDataMonitor.report(this, "1.64");
}
builders.setOwner(this);
publishers.setOwner(this);
buildWrappers.setOwner(this);
getBuildersList().setOwner(this);
getPublishersList().setOwner(this);
getBuildWrappersList().setOwner(this);
}

public AbstractProject<?, ?> asProject() {
return this;
}

public List<Builder> getBuilders() {
return builders.toList();
return getBuildersList().toList();
}

/**
Expand All @@ -111,22 +100,31 @@ public List<Builder> getBuilders() {
* Use {@link #getPublishersList()} instead.
*/
public Map<Descriptor<Publisher>,Publisher> getPublishers() {
return publishers.toMap();
return getPublishersList().toMap();
}

public DescribableList<Builder,Descriptor<Builder>> getBuildersList() {
public synchronized DescribableList<Builder,Descriptor<Builder>> getBuildersList() {
if (builders == null) {
builders = new DescribableList<Builder,Descriptor<Builder>>(this);
}
return builders;
}

public DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
public synchronized DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
if (publishers == null) {
publishers = new DescribableList<Publisher,Descriptor<Publisher>>(this);
}
return publishers;
}

public Map<Descriptor<BuildWrapper>,BuildWrapper> getBuildWrappers() {
return buildWrappers.toMap();
return getBuildWrappersList().toMap();
}

public DescribableList<BuildWrapper, Descriptor<BuildWrapper>> getBuildWrappersList() {
public synchronized DescribableList<BuildWrapper, Descriptor<BuildWrapper>> getBuildWrappersList() {
if (buildWrappers == null) {
buildWrappers = new DescribableList<BuildWrapper,Descriptor<BuildWrapper>>(this);
}
return buildWrappers;
}

Expand All @@ -135,9 +133,9 @@ protected Set<ResourceActivity> getResourceActivities() {
final Set<ResourceActivity> activities = new HashSet<ResourceActivity>();

activities.addAll(super.getResourceActivities());
activities.addAll(Util.filter(builders,ResourceActivity.class));
activities.addAll(Util.filter(publishers,ResourceActivity.class));
activities.addAll(Util.filter(buildWrappers,ResourceActivity.class));
activities.addAll(Util.filter(getBuildersList(),ResourceActivity.class));
activities.addAll(Util.filter(getPublishersList(),ResourceActivity.class));
activities.addAll(Util.filter(getBuildWrappersList(),ResourceActivity.class));

return activities;
}
Expand All @@ -149,7 +147,7 @@ protected Set<ResourceActivity> getResourceActivities() {
* Use {@code getPublishersList().add(x)}
*/
public void addPublisher(Publisher buildStep) throws IOException {
publishers.add(buildStep);
getPublishersList().add(buildStep);
}

/**
Expand All @@ -159,21 +157,21 @@ public void addPublisher(Publisher buildStep) throws IOException {
* Use {@code getPublishersList().remove(x)}
*/
public void removePublisher(Descriptor<Publisher> descriptor) throws IOException {
publishers.remove(descriptor);
getPublishersList().remove(descriptor);
}

public Publisher getPublisher(Descriptor<Publisher> descriptor) {
for (Publisher p : publishers) {
for (Publisher p : getPublishersList()) {
if(p.getDescriptor()==descriptor)
return p;
}
return null;
}

protected void buildDependencyGraph(DependencyGraph graph) {
publishers.buildDependencyGraph(this,graph);
builders.buildDependencyGraph(this,graph);
buildWrappers.buildDependencyGraph(this,graph);
getPublishersList().buildDependencyGraph(this,graph);
getBuildersList().buildDependencyGraph(this,graph);
getBuildWrappersList().buildDependencyGraph(this,graph);
}

@Override
Expand All @@ -198,9 +196,9 @@ protected void submit( StaplerRequest req, StaplerResponse rsp ) throws IOExcept

JSONObject json = req.getSubmittedForm();

buildWrappers.rebuild(req,json, BuildWrappers.getFor(this));
builders.rebuildHetero(req,json, Builder.all(), "builder");
publishers.rebuildHetero(req, json, Publisher.all(), "publisher");
getBuildWrappersList().rebuild(req,json, BuildWrappers.getFor(this));
getBuildersList().rebuildHetero(req,json, Builder.all(), "builder");
getPublishersList().rebuildHetero(req, json, Publisher.all(), "publisher");
}

@Override
Expand Down
24 changes: 24 additions & 0 deletions test/src/test/java/hudson/model/FreeStyleProjectTest.java
View file
Expand Up @@ -25,8 +25,10 @@

import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import edu.umd.cs.findbugs.annotations.SuppressWarnings;
import hudson.tasks.Builder;
import hudson.tasks.Shell;
import java.io.ByteArrayInputStream;
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.HudsonTestCase;

Expand Down Expand Up @@ -121,4 +123,26 @@ public void testCustomWorkspaceVariableExpansion() throws Exception {
assertFalse(path.contains("${JOB_NAME}"));
assertEquals(b.getWorkspace().getName(),f.getName());
}

@Bug(15817)
@SuppressWarnings("DM_DEFAULT_ENCODING")
public void testMinimalConfigXml() throws Exception {
// Make sure it can be created without exceptions:
FreeStyleProject project = (FreeStyleProject) jenkins.createProjectFromXML("stuff", new ByteArrayInputStream("<project/>".getBytes()));
System.out.println(project.getConfigFile().asString());
// and round-tripped:
Shell shell = new Shell("echo hello");
project.getBuildersList().add(shell);
WebClient webClient = new WebClient();
HtmlPage page = webClient.getPage(project,"configure");
HtmlForm form = page.getFormByName("config");
submit(form);
List<Builder> builders = project.getBuilders();
assertEquals(1,builders.size());
assertEquals(Shell.class,builders.get(0).getClass());
assertEquals("echo hello",((Shell)builders.get(0)).getCommand().trim());
assertTrue(builders.get(0)!=shell);
System.out.println(project.getConfigFile().asString());
}

}

0 comments on commit 7d2c0ef

Please sign in to comment.