提交 5b4dbb39 编写于 作者: K Kohsuke Kawaguchi

Make BulkChange auto-closeable

so that it can be used as below:

    try (BulkChange bc = new BulkChange(o)) {
       ...
       bc.commit();
    }
上级 533156c7
...@@ -25,6 +25,7 @@ package hudson; ...@@ -25,6 +25,7 @@ package hudson;
import hudson.model.Saveable; import hudson.model.Saveable;
import java.io.Closeable;
import java.io.IOException; import java.io.IOException;
/** /**
...@@ -35,28 +36,13 @@ import java.io.IOException; ...@@ -35,28 +36,13 @@ import java.io.IOException;
* The usage of {@link BulkChange} needs to follow a specific closure-like pattern, namely: * The usage of {@link BulkChange} needs to follow a specific closure-like pattern, namely:
* *
* <pre> * <pre>
* BulkChange bc = new BulkChange(someObject); * try (BulkChange bc = new BulkChange(someObject)) {
* try {
* ... make changes to 'someObject' * ... make changes to 'someObject'
* } finally {
* bc.commit(); * bc.commit();
* } * }
* </pre> * </pre>
* *
* <p> * <p>
* ... or if you'd like to avoid saving when something bad happens:
*
* <pre>
* BulkChange bc = new BulkChange(someObject);
* try {
* ... make changes to 'someObject'
* bc.commit();
* } finally {
* bc.abort();
* }
* </pre>
*
* <p>
* Use of this method is optional. If {@link BulkChange} is not used, individual mutator * Use of this method is optional. If {@link BulkChange} is not used, individual mutator
* will perform the save operation, and things will just run somewhat slower. * will perform the save operation, and things will just run somewhat slower.
* *
...@@ -82,7 +68,7 @@ import java.io.IOException; ...@@ -82,7 +68,7 @@ import java.io.IOException;
* @author Kohsuke Kawaguchi * @author Kohsuke Kawaguchi
* @since 1.249 * @since 1.249
*/ */
public class BulkChange { public class BulkChange implements Closeable {
private final Saveable saveable; private final Saveable saveable;
public final Exception allocator; public final Exception allocator;
private final BulkChange parent; private final BulkChange parent;
...@@ -112,6 +98,14 @@ public class BulkChange { ...@@ -112,6 +98,14 @@ public class BulkChange {
saveable.save(); 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. * Exits the scope of {@link BulkChange} without saving the changes.
* *
......
...@@ -436,15 +436,12 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R ...@@ -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 { public synchronized void setBuildDiscarder(BuildDiscarder bd) throws IOException {
BulkChange bc = new BulkChange(this); try (BulkChange bc = new BulkChange(this)) {
try {
removeProperty(BuildDiscarderProperty.class); removeProperty(BuildDiscarderProperty.class);
if (bd != null) { if (bd != null) {
addProperty(new BuildDiscarderProperty(bd)); addProperty(new BuildDiscarderProperty(bd));
} }
bc.commit(); bc.commit();
} finally {
bc.abort();
} }
} }
......
...@@ -2306,13 +2306,10 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run ...@@ -2306,13 +2306,10 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
@RequirePOST @RequirePOST
public @Nonnull HttpResponse doConfigSubmit( StaplerRequest req ) throws IOException, ServletException, FormException { public @Nonnull HttpResponse doConfigSubmit( StaplerRequest req ) throws IOException, ServletException, FormException {
checkPermission(UPDATE); checkPermission(UPDATE);
BulkChange bc = new BulkChange(this); try (BulkChange bc = new BulkChange(this)) {
try {
JSONObject json = req.getSubmittedForm(); JSONObject json = req.getSubmittedForm();
submit(json); submit(json);
bc.commit(); bc.commit();
} finally {
bc.abort();
} }
return FormApply.success("."); return FormApply.success(".");
} }
......
...@@ -96,8 +96,7 @@ public class SetupWizard extends PageDecorator { ...@@ -96,8 +96,7 @@ public class SetupWizard extends PageDecorator {
// difficult password // difficult password
FilePath iapf = getInitialAdminPasswordFile(); FilePath iapf = getInitialAdminPasswordFile();
if(jenkins.getSecurityRealm() == null || jenkins.getSecurityRealm() == SecurityRealm.NO_AUTHENTICATION) { // this seems very fragile if(jenkins.getSecurityRealm() == null || jenkins.getSecurityRealm() == SecurityRealm.NO_AUTHENTICATION) { // this seems very fragile
BulkChange bc = new BulkChange(jenkins); try (BulkChange bc = new BulkChange(jenkins)) {
try{
HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null); HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null);
jenkins.setSecurityRealm(securityRealm); jenkins.setSecurityRealm(securityRealm);
String randomUUID = UUID.randomUUID().toString().replace("-", "").toLowerCase(Locale.ENGLISH); String randomUUID = UUID.randomUUID().toString().replace("-", "").toLowerCase(Locale.ENGLISH);
...@@ -130,8 +129,6 @@ public class SetupWizard extends PageDecorator { ...@@ -130,8 +129,6 @@ public class SetupWizard extends PageDecorator {
jenkins.save(); // !! jenkins.save(); // !!
bc.commit(); bc.commit();
} finally {
bc.abort();
} }
} }
......
...@@ -35,6 +35,8 @@ import java.util.concurrent.CountDownLatch; ...@@ -35,6 +35,8 @@ import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Future; import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeoutException;
import static org.bouncycastle.asn1.bc.BCObjectIdentifiers.bc;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
...@@ -86,8 +88,7 @@ public class NodeProvisionerTest { ...@@ -86,8 +88,7 @@ public class NodeProvisionerTest {
*/ */
// TODO fragile // TODO fragile
@Test public void autoProvision() throws Exception { @Test public void autoProvision() throws Exception {
BulkChange bc = new BulkChange(r.jenkins); try (BulkChange bc = new BulkChange(r.jenkins)) {
try {
DummyCloudImpl cloud = initHudson(10); DummyCloudImpl cloud = initHudson(10);
...@@ -98,8 +99,6 @@ public class NodeProvisionerTest { ...@@ -98,8 +99,6 @@ public class NodeProvisionerTest {
// since there's only one job, we expect there to be just one slave // since there's only one job, we expect there to be just one slave
assertEquals(1,cloud.numProvisioned); assertEquals(1,cloud.numProvisioned);
} finally {
bc.abort();
} }
} }
...@@ -108,8 +107,7 @@ public class NodeProvisionerTest { ...@@ -108,8 +107,7 @@ public class NodeProvisionerTest {
*/ */
// TODO fragile // TODO fragile
@Test public void loadSpike() throws Exception { @Test public void loadSpike() throws Exception {
BulkChange bc = new BulkChange(r.jenkins); try (BulkChange bc = new BulkChange(r.jenkins)) {
try {
DummyCloudImpl cloud = initHudson(0); DummyCloudImpl cloud = initHudson(0);
verifySuccessfulCompletion(buildAll(create5SlowJobs(new Latch(5)))); verifySuccessfulCompletion(buildAll(create5SlowJobs(new Latch(5))));
...@@ -117,8 +115,6 @@ public class NodeProvisionerTest { ...@@ -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 // 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. // a new slave, so in this scenario we end up allocating 5 slaves for 5 jobs.
assertEquals(5,cloud.numProvisioned); assertEquals(5,cloud.numProvisioned);
} finally {
bc.abort();
} }
} }
...@@ -127,8 +123,7 @@ public class NodeProvisionerTest { ...@@ -127,8 +123,7 @@ public class NodeProvisionerTest {
*/ */
// TODO fragile // TODO fragile
@Test public void baselineSlaveUsage() throws Exception { @Test public void baselineSlaveUsage() throws Exception {
BulkChange bc = new BulkChange(r.jenkins); try (BulkChange bc = new BulkChange(r.jenkins)) {
try {
DummyCloudImpl cloud = initHudson(0); DummyCloudImpl cloud = initHudson(0);
// add slaves statically upfront // add slaves statically upfront
r.createSlave().toComputer().connect(false).get(); r.createSlave().toComputer().connect(false).get();
...@@ -138,8 +133,6 @@ public class NodeProvisionerTest { ...@@ -138,8 +133,6 @@ public class NodeProvisionerTest {
// we should have used two static slaves, thus only 3 slaves should have been provisioned // we should have used two static slaves, thus only 3 slaves should have been provisioned
assertEquals(3,cloud.numProvisioned); assertEquals(3,cloud.numProvisioned);
} finally {
bc.abort();
} }
} }
...@@ -148,8 +141,7 @@ public class NodeProvisionerTest { ...@@ -148,8 +141,7 @@ public class NodeProvisionerTest {
*/ */
// TODO fragile // TODO fragile
@Test public void labels() throws Exception { @Test public void labels() throws Exception {
BulkChange bc = new BulkChange(r.jenkins); try (BulkChange bc = new BulkChange(r.jenkins)) {
try {
DummyCloudImpl cloud = initHudson(0); DummyCloudImpl cloud = initHudson(0);
Label blue = r.jenkins.getLabel("blue"); Label blue = r.jenkins.getLabel("blue");
Label red = r.jenkins.getLabel("red"); Label red = r.jenkins.getLabel("red");
...@@ -175,8 +167,6 @@ public class NodeProvisionerTest { ...@@ -175,8 +167,6 @@ public class NodeProvisionerTest {
// and all blue jobs should be still stuck in the queue // and all blue jobs should be still stuck in the queue
for (Future<FreeStyleBuild> bb : blueBuilds) for (Future<FreeStyleBuild> bb : blueBuilds)
assertFalse(bb.isDone()); assertFalse(bb.isDone());
} finally {
bc.abort();
} }
} }
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册