[JENKINS-34547] Switch WorkflowJob.concurrentBuild to JobProperty

src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowJob.java

@@ -140,7 +140,7 @@ public WorkflowJob(ItemGroup parent, String name) {
         for (Trigger t : triggers) {
             t.start(this, Items.currentlyUpdatingByXml());
         }
-        if (concurrentBuild != null && !concurrentBuild) {
+        if (concurrentBuild != null) {
             setConcurrentBuild(concurrentBuild);
         }
     }
@@ -343,26 +343,28 @@ public String getShortDescription() {
 
     @Exported
     @Override public boolean isConcurrentBuild() {
-        DisableConcurrentBuildsJobProperty p = getProperty(DisableConcurrentBuildsJobProperty.class);
-        if (p != null) {
-            return false;
-        } else {
-            /* settings compatibility */
-            return !Boolean.FALSE.equals(concurrentBuild);
-        }
+        return getProperty(DisableConcurrentBuildsJobProperty.class) == null;
     }
 
     public void setConcurrentBuild(boolean b) throws IOException {
         concurrentBuild = null;
-        BulkChange bc = new BulkChange(this);
-        try {
-            removeProperty(DisableConcurrentBuildsJobProperty.class);
-            if (!b) {
-                addProperty(new DisableConcurrentBuildsJobProperty());
+
+        boolean propertyExists = getProperty(DisableConcurrentBuildsJobProperty.class) == null;
+
+        // If the property exists, concurrent builds are disabled. So if the argument here is true and the
+        // property exists, we need to remove the property, while if the argument is false and the property
+        // does not exist, we need to add the property. Yay for flipping boolean values around!
+        if (propertyExists != b) {
+            BulkChange bc = new BulkChange(this);
+            try {
+                removeProperty(DisableConcurrentBuildsJobProperty.class);
+                if (!b) {
+                    addProperty(new DisableConcurrentBuildsJobProperty());
+                }
+                bc.commit();
+            } finally {
+                bc.abort();
             }
-            bc.commit();
-        } finally {
-            bc.abort();
         }
     }
 

src/test/java/org/jenkinsci/plugins/workflow/WorkflowRunTest.java

@@ -56,7 +56,6 @@
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
-import org.jenkinsci.plugins.workflow.job.properties.DisableConcurrentBuildsJobProperty;
 import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
 import static org.junit.Assert.*;
 import org.junit.ClassRule;
@@ -277,21 +276,4 @@ public void failedToStartRun() throws Exception {
         assertEquals(Collections.emptyList(), iba.getCauses());
     }
 
-    @Issue("JENKINS-34547")
-    @LocalData
-    @Test public void concurrentBuildsMigrationOnByDefault() throws Exception {
-        WorkflowJob p = r.jenkins.getItemByFullName("p", WorkflowJob.class);
-        assertNotNull(p);
-        assertNull(p.getProperty(DisableConcurrentBuildsJobProperty.class));
-        assertTrue(p.isConcurrentBuild());
-    }
-
-    @Issue("JENKINS-34547")
-    @LocalData
-    @Test public void concurrentBuildsMigrationFromFalse() throws Exception {
-        WorkflowJob p = r.jenkins.getItemByFullName("p", WorkflowJob.class);
-        assertNotNull(p);
-        assertNotNull(p.getProperty(DisableConcurrentBuildsJobProperty.class));
-        assertFalse(p.isConcurrentBuild());
-    }
 }

src/test/java/org/jenkinsci/plugins/workflow/properties/DisableConcurrentBuildsJobPropertyTest.java

@@ -0,0 +1,83 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2016, CloudBees, 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 org.jenkinsci.plugins.workflow.properties;
+
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
+import org.jenkinsci.plugins.workflow.job.properties.DisableConcurrentBuildsJobProperty;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.BuildWatcher;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.recipes.LocalData;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class DisableConcurrentBuildsJobPropertyTest {
+    @ClassRule
+    public static BuildWatcher buildWatcher = new BuildWatcher();
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+
+    @Issue("JENKINS-34547")
+    @LocalData
+    @Test
+    public void concurrentBuildsMigrationOnByDefault() throws Exception {
+        WorkflowJob p = r.jenkins.getItemByFullName("p", WorkflowJob.class);
+        assertNotNull(p);
+        assertNull(p.getProperty(DisableConcurrentBuildsJobProperty.class));
+        assertTrue(p.isConcurrentBuild());
+    }
+
+    @Issue("JENKINS-34547")
+    @LocalData
+    @Test public void concurrentBuildsMigrationFromFalse() throws Exception {
+        WorkflowJob p = r.jenkins.getItemByFullName("p", WorkflowJob.class);
+        assertNotNull(p);
+        assertNotNull(p.getProperty(DisableConcurrentBuildsJobProperty.class));
+        assertFalse(p.isConcurrentBuild());
+    }
+
+    @Issue("JENKINS-34547")
+    @Test public void configRoundTrip() throws Exception {
+        WorkflowJob defaultCase = r.jenkins.createProject(WorkflowJob.class, "defaultCase");
+        assertTrue(defaultCase.isConcurrentBuild());
+
+        WorkflowJob roundTripDefault = r.configRoundtrip(defaultCase);
+        assertTrue(roundTripDefault.isConcurrentBuild());
+
+        WorkflowJob disabledCase = r.jenkins.createProject(WorkflowJob.class, "disableCase");
+        assertTrue(disabledCase.isConcurrentBuild());
+
+        WorkflowJob roundTripDisabled = r.configRoundtrip(disabledCase);
+        assertTrue(roundTripDisabled.isConcurrentBuild());
+    }
+}