Added a mechanism to perform a bulk change without intermediate save().

This allows us to define more setter methods on configuration values, which in turn improves the ability to make programatic changes to the configuration, which in turn improves testability.

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@11515 71c3de6d-444a-0410-be80-ed276b4c234a

core/src/main/java/hudson/BulkChange.java

+package hudson;
+
+import hudson.model.Saveable;
+import hudson.model.Hudson;
+
+import java.io.IOException;
+
+/**
+ * Transaction-like object that can be used to make a bunch of changes to an object, and defer the
+ * {@link Saveable#save()} until the end.
+ *
+ * <p>
+ * 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 {
+ *    ... 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
+ * will perform the save operation, and things will just run somewhat slower.
+ *
+ *
+ * <h2>Cooperation from {@link Saveable}</h2>
+ * <p>
+ * For this class to work as intended, {@link Saveable} implementations need to co-operate.
+ * Namely,
+ *
+ * <ol>
+ * <li>
+ * Mutater methods should invoke {@code this.save()} so that if the method is called outside
+ * a {@link BulkChange}, the result will be saved immediately.
+ *
+ * <li>
+ * In the {@code save()} method implementation, use {@link #contains(Saveable)} and
+ * only perform the actual I/O operation when this method returns false.
+ * </ol>
+ *
+ * <p>
+ * See {@link Hudson#save()} as an example if you are not sure how to implement {@link Saveable}.
+ *
+ * @author Kohsuke Kawaguchi
+ * @since 1.249
+ */
+public class BulkChange {
+    private final Saveable saveable;
+    public final Exception allocator;
+    private final BulkChange parent;
+
+    private boolean completed;
+
+    public BulkChange(Saveable saveable) {
+        this.parent = current();
+        this.saveable = saveable;
+        // rememeber who allocated this object in case
+        // someone forgot to call save() at the end.
+        allocator = new Exception();
+
+        // in effect at construction
+        INSCOPE.set(this);
+    }
+
+    /**
+     * Saves the accumulated changes.
+     */
+    public void commit() throws IOException {
+        if(completed)   return;
+        completed = true;
+
+        // move this object out of the scope first before save, or otherwise the save() method will do nothing.
+        pop();
+        saveable.save();
+    }
+
+    /**
+     * Exits the scope of {@link BulkChange} without saving the changes.
+     *
+     * <p>
+     * This can be used when a bulk change fails in the middle.
+     * Note that unlike a real transaction, this will not roll back the state of the object.
+     *
+     * <p>
+     * The abort method can be called after the commit method, in which case this method does nothing.
+     * This is so that {@link BulkChange} can be used naturally in the try/finally block.
+     */
+    public void abort() {
+        if(completed)   return;
+        completed = true;
+        pop();
+    }
+
+    private void pop() {
+        if(current()!=this)
+            throw new AssertionError("Trying to save BulkChange that's not in scope");
+        INSCOPE.set(parent);
+    }
+
+    /**
+     * {@link BulkChange}s that are effective currently.
+     */
+    private static final ThreadLocal<BulkChange> INSCOPE = new ThreadLocal<BulkChange>();
+
+    /**
+     * Gets the {@link BulkChange} instance currently in scope for the current thread.
+     */
+    public static BulkChange current() {
+        return INSCOPE.get();
+    }
+
+    /**
+     * Checks if the given {@link Saveable} is currently in the bulk change.
+     *
+     * <p>
+     * The expected usage is from the {@link Saveable#save()} implementation to check
+     * if the actual persistence should happen now or not.
+     */
+    public static boolean contains(Saveable s) {
+        for(BulkChange b=current(); b!=null; b=b.parent)
+            if(b.saveable== s)
+                return true;
+        return false;
+    }
+}

core/src/main/java/hudson/Plugin.java

@@ -2,6 +2,7 @@ package hudson;
 
 import hudson.model.Hudson;
 import hudson.model.Descriptor;
+import hudson.model.Saveable;
 import hudson.model.Descriptor.FormException;
 import hudson.scm.SCM;
 import hudson.tasks.Builder;
@@ -49,7 +50,7 @@ import com.thoughtworks.xstream.XStream;
  * @author Kohsuke Kawaguchi
  * @since 1.42
  */
-public abstract class Plugin {
+public abstract class Plugin implements Saveable {
 
     /**
      * Set by the {@link PluginManager}.
@@ -187,7 +188,8 @@ public abstract class Plugin {
      *
      * @since 1.245
      */
-    protected void save() throws IOException {
+    public void save() throws IOException {
+        if(BulkChange.contains(this))   return;
         getConfigXml().write(this);
     }
 

core/src/main/java/hudson/ProxyConfiguration.java

@@ -2,6 +2,7 @@ package hudson;
 
 import com.thoughtworks.xstream.XStream;
 import hudson.model.Hudson;
+import hudson.model.Saveable;
 import hudson.util.XStream2;
 import hudson.util.Scrambler;
 
@@ -20,7 +21,7 @@ import java.net.URL;
  *
  * @see Hudson#proxy
  */
-public final class ProxyConfiguration {
+public final class ProxyConfiguration implements Saveable {
     public final String name;
     public final int port;
 
@@ -54,6 +55,7 @@ public final class ProxyConfiguration {
     }
 
     public void save() throws IOException {
+        if(BulkChange.contains(this))   return;
         getXmlFile().write(this);
     }
 

core/src/main/java/hudson/model/AbstractItem.java

@@ -3,6 +3,7 @@ package hudson.model;
 import hudson.XmlFile;
 import hudson.Util;
 import hudson.Functions;
+import hudson.BulkChange;
 import hudson.security.AccessControlled;
 import hudson.security.Permission;
 import hudson.security.ACL;
@@ -187,6 +188,7 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet
      * Save the settings to a file.
      */
     public synchronized void save() throws IOException {
+        if(BulkChange.contains(this))   return;
         getConfigFile().write(this);
     }
 

core/src/main/java/hudson/model/Descriptor.java

 package hudson.model;
 
 import hudson.XmlFile;
+import hudson.BulkChange;
 import hudson.util.CopyOnWriteList;
 import hudson.scm.CVSSCM;
 import net.sf.json.JSONArray;
@@ -65,7 +66,7 @@ import java.lang.reflect.Modifier;
  * @author Kohsuke Kawaguchi
  * @see Describable
  */
-public abstract class Descriptor<T extends Describable<T>> {
+public abstract class Descriptor<T extends Describable<T>> implements Saveable {
     /**
      * Up to Hudson 1.61 this was used as the primary persistence mechanism.
      * Going forward Hudson simply persists all the non-transient fields
@@ -261,7 +262,8 @@ public abstract class Descriptor<T extends Describable<T>> {
     /**
      * Saves the configuration info to the disk.
      */
-    protected synchronized void save() {
+    public synchronized void save() {
+        if(BulkChange.contains(this))   return;
         try {
             getConfigFile().write(this);
         } catch (IOException e) {

core/src/main/java/hudson/model/Fingerprint.java

@@ -9,6 +9,7 @@ import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
 import hudson.Util;
 import hudson.XmlFile;
+import hudson.BulkChange;
 import hudson.maven.MavenModule;
 import hudson.maven.MavenModuleSet;
 import hudson.util.HexBinaryConverter;
@@ -35,7 +36,7 @@ import java.util.logging.Logger;
  * @author Kohsuke Kawaguchi
  */
 @ExportedBean
-public class Fingerprint implements ModelObject {
+public class Fingerprint implements ModelObject, Saveable {
     /**
      * Pointer to a {@link Build}.
      */
@@ -637,6 +638,8 @@ public class Fingerprint implements ModelObject {
      * Save the settings to a file.
      */
     public synchronized void save() throws IOException {
+        if(BulkChange.contains(this))   return;
+
         long start=0;
         if(logger.isLoggable(Level.FINE))
             start = System.currentTimeMillis();

core/src/main/java/hudson/model/Hudson.java

@@ -15,6 +15,7 @@ import hudson.TcpSlaveAgentListener;
 import hudson.Util;
 import static hudson.Util.fixEmpty;
 import hudson.XmlFile;
+import hudson.BulkChange;
 import hudson.model.Descriptor.FormException;
 import hudson.model.listeners.ItemListener;
 import hudson.model.listeners.JobListener;
@@ -1444,6 +1445,7 @@ public final class Hudson extends View implements ItemGroup<TopLevelItem>, Node,
      * Save the settings to a file.
      */
     public synchronized void save() throws IOException {
+        if(BulkChange.contains(this))   return;
         getConfigFile().write(this);
     }
 

core/src/main/java/hudson/model/PersistenceRoot.java

@@ -8,7 +8,7 @@ import java.io.File;
  *
  * @author Kohsuke Kawaguchi
  */
-public interface PersistenceRoot {
+public interface PersistenceRoot extends Saveable {
     /**
      * Gets the root directory on the file system that this
      * {@link Item} can use freely fore storing the configuration data.

core/src/main/java/hudson/model/Queue.java

@@ -2,6 +2,7 @@ package hudson.model;
 
 import hudson.Util;
 import hudson.XmlFile;
+import hudson.BulkChange;
 import hudson.model.Node.Mode;
 import hudson.triggers.SafeTimerTask;
 import hudson.triggers.Trigger;
@@ -62,7 +63,7 @@ import com.thoughtworks.xstream.converters.basic.AbstractSingleValueConverter;
  * @author Kohsuke Kawaguchi
  */
 @ExportedBean
-public class Queue extends ResourceController {
+public class Queue extends ResourceController implements Saveable {
     /**
      * Items that are waiting for its quiet period to pass.
      *
@@ -241,6 +242,8 @@ public class Queue extends ResourceController {
      * Persists the queue contents to the disk.
      */
     public synchronized void save() {
+        if(BulkChange.contains(this))  return;
+        
         // write out the tasks on the queue
     	ArrayList<Task> tasks = new ArrayList<Task>();
     	for (Item item: getItems()) {

core/src/main/java/hudson/model/Run.java

@@ -10,6 +10,7 @@ import hudson.Util;
 import static hudson.Util.combine;
 import hudson.XmlFile;
 import hudson.AbortException;
+import hudson.BulkChange;
 import hudson.matrix.MatrixBuild;
 import hudson.matrix.MatrixRun;
 import hudson.model.listeners.RunListener;
@@ -901,6 +902,7 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
      * Save the settings to a file.
      */
     public synchronized void save() throws IOException {
+        if(BulkChange.contains(this))   return;
         getDataFile().write(this);
     }
 

core/src/main/java/hudson/model/Saveable.java

+package hudson.model;
+
+import hudson.BulkChange;
+
+import java.io.IOException;
+
+/**
+ * Object whose state is persisted to XML.
+ *
+ * @author Kohsuke Kawaguchi
+ * @see BulkChange
+ * @since 1.249
+ */
+public interface Saveable {
+    /**
+     * Persists the state of this object into XML.
+     *
+     * <p>
+     * For making a bulk change efficiently, see {@link BulkChange}.
+     *
+     * @throws IOException
+     *      if the persistence failed.
+     */
+    void save() throws IOException;
+}

core/src/main/java/hudson/model/User.java

@@ -5,6 +5,7 @@ import hudson.CopyOnWrite;
 import hudson.FeedAdapter;
 import hudson.Util;
 import hudson.XmlFile;
+import hudson.BulkChange;
 import hudson.tasks.Mailer;
 import hudson.model.Descriptor.FormException;
 import hudson.security.ACL;
@@ -65,7 +66,7 @@ import java.util.logging.Logger;
  * @author Kohsuke Kawaguchi
  */
 @ExportedBean
-public class User extends AbstractModelObject implements AccessControlled {
+public class User extends AbstractModelObject implements AccessControlled, Saveable {
 
     private transient final String id;
 
@@ -364,6 +365,7 @@ public class User extends AbstractModelObject implements AccessControlled {
      * Save the settings to a file.
      */
     public synchronized void save() throws IOException {
+        if(BulkChange.contains(this))   return;
         getConfigFile().write(this);
     }
 

core/src/main/java/hudson/util/DescribableList.java

@@ -21,6 +21,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Collections;
+import java.security.acl.Owner;
 
 /**
  * Persisted list of {@link Describable}s with some operations specific
@@ -38,19 +39,35 @@ import java.util.Collections;
  */
 public class DescribableList<T extends Describable<T>, D extends Descriptor<T>> implements Iterable<T> {
     private final CopyOnWriteList<T> data = new CopyOnWriteList<T>();
-    private Owner owner;
+    private Saveable owner;
 
     private DescribableList() {
     }
 
+    /**
+     * @deprecated
+     *      Use {@link #DescribableList(Saveable)} 
+     */
     public DescribableList(Owner owner) {
         setOwner(owner);
     }
 
+    public DescribableList(Saveable owner) {
+        setOwner(owner);
+    }
+
+    /**
+     * @deprecated
+     *      Use {@link #setOwner(Saveable)}
+     */
     public void setOwner(Owner owner) {
         this.owner = owner;
     }
 
+    public void setOwner(Saveable owner) {
+        this.owner = owner;
+    }
+
     public void add(T item) throws IOException {
         data.add(item);
         owner.save();
@@ -152,11 +169,11 @@ public class DescribableList<T extends Describable<T>, D extends Descriptor<T>>
         }
     }
 
-    public interface Owner {
-        /**
-         * Called whenever the list is changed, so that it can be saved.
-         */
-        void save() throws IOException;
+    /**
+     * @deprecated 
+     *      Just implement {@link Saveable}.
+     */
+    public interface Owner extends Saveable {
     }
 
     /**