From 5b4dbb3970b7e559e32aee6dbd159e66bf7a68a8 Mon Sep 17 00:00:00 2001
From: Kohsuke Kawaguchi <kk@kohsuke.org>
Date: Tue, 28 Jun 2016 13:56:32 -0700
Subject: [PATCH] Make BulkChange auto-closeable

so that it can be used as below:

    try (BulkChange bc = new BulkChange(o)) {
       ...
       bc.commit();
    }
---
 core/src/main/java/hudson/BulkChange.java     | 28 ++++++++-----------
 core/src/main/java/hudson/model/Job.java      |  5 +---
 core/src/main/java/hudson/model/Run.java      |  5 +---
 .../java/jenkins/install/SetupWizard.java     |  5 +---
 .../hudson/slaves/NodeProvisionerTest.java    | 22 ++++-----------
 5 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/core/src/main/java/hudson/BulkChange.java b/core/src/main/java/hudson/BulkChange.java
index db48db3de7..3c7da451f6 100644
--- a/core/src/main/java/hudson/BulkChange.java
+++ b/core/src/main/java/hudson/BulkChange.java
@@ -25,6 +25,7 @@ package hudson;
 
 import hudson.model.Saveable;
 
+import java.io.Closeable;
 import java.io.IOException;
 
 /**
@@ -35,24 +36,9 @@ import java.io.IOException;
  * The usage of {@link BulkChange} needs to follow a specific closure-like pattern, namely:
  *
  * <pre>
- * BulkChange bc = new BulkChange(someObject);
- * try {
- *    ... make changes to 'someObject'
- * } finally {
- *    bc.commit();
- * }
- * </pre>
- *
- * <p>
- * ... or if you'd like to avoid saving when something bad happens:
- *
- * <pre>
- * BulkChange bc = new BulkChange(someObject);
- * try {
+ * try (BulkChange bc = new BulkChange(someObject)) {
  *    ... make changes to 'someObject'
  *    bc.commit();
- * } finally {
- *    bc.abort();
  * }
  * </pre>
  *
@@ -82,7 +68,7 @@ import java.io.IOException;
  * @author Kohsuke Kawaguchi
  * @since 1.249
  */
-public class BulkChange {
+public class BulkChange implements Closeable {
     private final Saveable saveable;
     public final Exception allocator;
     private final BulkChange parent;
@@ -112,6 +98,14 @@ public class BulkChange {
         saveable.save();
     }
 
+    /**
+     * Alias for {@link #abort()} to make {@link BulkChange} auto-closeable.
+     */
+    @Override
+    public void close() {
+        abort();
+    }
+
     /**
      * Exits the scope of {@link BulkChange} without saving the changes.
      *
diff --git a/core/src/main/java/hudson/model/Job.java b/core/src/main/java/hudson/model/Job.java
index 69cc02708e..1a61359972 100644
--- a/core/src/main/java/hudson/model/Job.java
+++ b/core/src/main/java/hudson/model/Job.java
@@ -436,15 +436,12 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
     }
 
     public synchronized void setBuildDiscarder(BuildDiscarder bd) throws IOException {
-        BulkChange bc = new BulkChange(this);
-        try {
+        try (BulkChange bc = new BulkChange(this)) {
             removeProperty(BuildDiscarderProperty.class);
             if (bd != null) {
                 addProperty(new BuildDiscarderProperty(bd));
             }
             bc.commit();
-        } finally {
-            bc.abort();
         }
     }
 
diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java
index 8af5c09cc7..dad840bf3a 100644
--- a/core/src/main/java/hudson/model/Run.java
+++ b/core/src/main/java/hudson/model/Run.java
@@ -2306,13 +2306,10 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
     @RequirePOST
     public @Nonnull HttpResponse doConfigSubmit( StaplerRequest req ) throws IOException, ServletException, FormException {
         checkPermission(UPDATE);
-        BulkChange bc = new BulkChange(this);
-        try {
+        try (BulkChange bc = new BulkChange(this)) {
             JSONObject json = req.getSubmittedForm();
             submit(json);
             bc.commit();
-        } finally {
-            bc.abort();
         }
         return FormApply.success(".");
     }
diff --git a/core/src/main/java/jenkins/install/SetupWizard.java b/core/src/main/java/jenkins/install/SetupWizard.java
index c3bfac930e..9d91e686d4 100644
--- a/core/src/main/java/jenkins/install/SetupWizard.java
+++ b/core/src/main/java/jenkins/install/SetupWizard.java
@@ -96,8 +96,7 @@ public class SetupWizard extends PageDecorator {
             // difficult password
             FilePath iapf = getInitialAdminPasswordFile();
             if(jenkins.getSecurityRealm() == null || jenkins.getSecurityRealm() == SecurityRealm.NO_AUTHENTICATION) { // this seems very fragile
-                BulkChange bc = new BulkChange(jenkins);
-                try{
+                try (BulkChange bc = new BulkChange(jenkins)) {
                     HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null);
                     jenkins.setSecurityRealm(securityRealm);
                     String randomUUID = UUID.randomUUID().toString().replace("-", "").toLowerCase(Locale.ENGLISH);
@@ -130,8 +129,6 @@ public class SetupWizard extends PageDecorator {
                 
                     jenkins.save(); // !!
                     bc.commit();
-                } finally {
-                    bc.abort();
                 }
             }
     
diff --git a/test/src/test/java/hudson/slaves/NodeProvisionerTest.java b/test/src/test/java/hudson/slaves/NodeProvisionerTest.java
index 7374eb8b77..b5d631399c 100644
--- a/test/src/test/java/hudson/slaves/NodeProvisionerTest.java
+++ b/test/src/test/java/hudson/slaves/NodeProvisionerTest.java
@@ -35,6 +35,8 @@ import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+
+import static org.bouncycastle.asn1.bc.BCObjectIdentifiers.bc;
 import static org.junit.Assert.*;
 import org.junit.Rule;
 import org.junit.Test;
@@ -86,8 +88,7 @@ public class NodeProvisionerTest {
      */
     // TODO fragile
     @Test public void autoProvision() throws Exception {
-        BulkChange bc = new BulkChange(r.jenkins);
-        try {
+        try (BulkChange bc = new BulkChange(r.jenkins)) {
             DummyCloudImpl cloud = initHudson(10);
 
 
@@ -98,8 +99,6 @@ public class NodeProvisionerTest {
 
             // since there's only one job, we expect there to be just one slave
             assertEquals(1,cloud.numProvisioned);
-        } finally {
-            bc.abort();
         }
     }
 
@@ -108,8 +107,7 @@ public class NodeProvisionerTest {
      */
     // TODO fragile
     @Test public void loadSpike() throws Exception {
-        BulkChange bc = new BulkChange(r.jenkins);
-        try {
+        try (BulkChange bc = new BulkChange(r.jenkins)) {
             DummyCloudImpl cloud = initHudson(0);
 
             verifySuccessfulCompletion(buildAll(create5SlowJobs(new Latch(5))));
@@ -117,8 +115,6 @@ public class NodeProvisionerTest {
             // the time it takes to complete a job is eternally long compared to the time it takes to launch
             // a new slave, so in this scenario we end up allocating 5 slaves for 5 jobs.
             assertEquals(5,cloud.numProvisioned);
-        } finally {
-            bc.abort();
         }
     }
 
@@ -127,8 +123,7 @@ public class NodeProvisionerTest {
      */
     // TODO fragile
     @Test public void baselineSlaveUsage() throws Exception {
-        BulkChange bc = new BulkChange(r.jenkins);
-        try {
+        try (BulkChange bc = new BulkChange(r.jenkins)) {
             DummyCloudImpl cloud = initHudson(0);
             // add slaves statically upfront
             r.createSlave().toComputer().connect(false).get();
@@ -138,8 +133,6 @@ public class NodeProvisionerTest {
 
             // we should have used two static slaves, thus only 3 slaves should have been provisioned
             assertEquals(3,cloud.numProvisioned);
-        } finally {
-            bc.abort();
         }
     }
 
@@ -148,8 +141,7 @@ public class NodeProvisionerTest {
      */
     // TODO fragile
     @Test public void labels() throws Exception {
-        BulkChange bc = new BulkChange(r.jenkins);
-        try {
+        try (BulkChange bc = new BulkChange(r.jenkins)) {
             DummyCloudImpl cloud = initHudson(0);
             Label blue = r.jenkins.getLabel("blue");
             Label red = r.jenkins.getLabel("red");
@@ -175,8 +167,6 @@ public class NodeProvisionerTest {
             // and all blue jobs should be still stuck in the queue
             for (Future<FreeStyleBuild> bb : blueBuilds)
                 assertFalse(bb.isDone());
-        } finally {
-            bc.abort();
         }
     }