From 3df097f1b8f41444e5c684a9afae0695c208f8d7 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 13 Oct 2023 17:22:28 +0200 Subject: [PATCH] [JENKINS-72107] Introduce `Loadable` interface (#8544) * Introduce `Loadable` interface CloudBees CI uses `load()` method to reload selectively parts of the Jenkins object model, without reloading everything. This formalizes the interface that gets used for this purpose, even if most of the Saveable implementations were using a `load()` method by convention. * Add missing @Override * Jenkins#load can throw IOException * Inline loadConfig into load * Make Plugin#load/save synchronized (same as Descriptor) --- core/src/main/java/hudson/Plugin.java | 8 +++++--- .../main/java/hudson/logging/LogRecorder.java | 4 +++- .../main/java/hudson/model/AbstractItem.java | 8 +++++++- .../main/java/hudson/model/Descriptor.java | 4 +++- .../hudson/model/PersistentDescriptor.java | 4 +++- .../main/java/hudson/model/UpdateCenter.java | 4 +++- core/src/main/java/hudson/model/User.java | 8 +++++++- core/src/main/java/jenkins/model/Jenkins.java | 7 ++++--- .../src/main/java/jenkins/model/Loadable.java | 19 +++++++++++++++++++ 9 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 core/src/main/java/jenkins/model/Loadable.java diff --git a/core/src/main/java/hudson/Plugin.java b/core/src/main/java/hudson/Plugin.java index 41eafeabf545..c87a2cbe9b3c 100644 --- a/core/src/main/java/hudson/Plugin.java +++ b/core/src/main/java/hudson/Plugin.java @@ -44,6 +44,7 @@ import javax.servlet.http.HttpServletResponse; import jenkins.model.GlobalConfiguration; import jenkins.model.Jenkins; +import jenkins.model.Loadable; import jenkins.util.SystemProperties; import net.sf.json.JSONObject; import org.kohsuke.accmod.Restricted; @@ -84,7 +85,7 @@ * @author Kohsuke Kawaguchi * @since 1.42 */ -public abstract class Plugin implements Saveable, StaplerProxy { +public abstract class Plugin implements Loadable, Saveable, StaplerProxy { private static final Logger LOGGER = Logger.getLogger(Plugin.class.getName()); @@ -265,7 +266,8 @@ public void doDynamic(StaplerRequest req, StaplerResponse rsp) throws IOExceptio * * @since 1.245 */ - protected void load() throws IOException { + @Override + public synchronized void load() throws IOException { XmlFile xml = getConfigXml(); if (xml.exists()) xml.unmarshal(this); @@ -277,7 +279,7 @@ protected void load() throws IOException { * @since 1.245 */ @Override - public void save() throws IOException { + public synchronized void save() throws IOException { if (BulkChange.contains(this)) return; XmlFile config = getConfigXml(); config.write(this); diff --git a/core/src/main/java/hudson/logging/LogRecorder.java b/core/src/main/java/hudson/logging/LogRecorder.java index a01684da8973..23ebc72cde6a 100644 --- a/core/src/main/java/hudson/logging/LogRecorder.java +++ b/core/src/main/java/hudson/logging/LogRecorder.java @@ -73,6 +73,7 @@ import java.util.logging.Logger; import javax.servlet.ServletException; import jenkins.model.Jenkins; +import jenkins.model.Loadable; import jenkins.security.MasterToSlaveCallable; import jenkins.util.MemoryReductionUtil; import net.sf.json.JSONObject; @@ -99,7 +100,7 @@ * @author Kohsuke Kawaguchi * @see LogRecorderManager */ -public class LogRecorder extends AbstractModelObject implements Saveable { +public class LogRecorder extends AbstractModelObject implements Loadable, Saveable { private volatile String name; /** @@ -476,6 +477,7 @@ public HttpResponse doClear() throws IOException { /** * Loads the settings from a file. */ + @Override public synchronized void load() throws IOException { getConfigFile().unmarshal(this); loggers.forEach(Target::enable); diff --git a/core/src/main/java/hudson/model/AbstractItem.java b/core/src/main/java/hudson/model/AbstractItem.java index ef9b97319053..05a45da743fb 100644 --- a/core/src/main/java/hudson/model/AbstractItem.java +++ b/core/src/main/java/hudson/model/AbstractItem.java @@ -76,6 +76,7 @@ import javax.xml.transform.stream.StreamSource; import jenkins.model.DirectlyModifiableTopLevelItemGroup; import jenkins.model.Jenkins; +import jenkins.model.Loadable; import jenkins.model.queue.ItemDeletion; import jenkins.security.NotReallyRoleSensitiveCallable; import jenkins.util.SystemProperties; @@ -111,7 +112,7 @@ // Item doesn't necessarily have to be Actionable, but // Java doesn't let multiple inheritance. @ExportedBean -public abstract class AbstractItem extends Actionable implements Item, HttpDeletable, AccessControlled, DescriptorByNameOwner, StaplerProxy { +public abstract class AbstractItem extends Actionable implements Loadable, Item, HttpDeletable, AccessControlled, DescriptorByNameOwner, StaplerProxy { private static final Logger LOGGER = Logger.getLogger(AbstractItem.class.getName()); @@ -934,6 +935,11 @@ public void updateByXml(Source source) throws IOException { */ @RequirePOST public void doReload() throws IOException { + load(); + } + + @Override + public void load() throws IOException { checkPermission(CONFIGURE); // try to reflect the changes by reloading diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java index 213642a5aae6..a2cc91cba576 100644 --- a/core/src/main/java/hudson/model/Descriptor.java +++ b/core/src/main/java/hudson/model/Descriptor.java @@ -75,6 +75,7 @@ import jenkins.model.GlobalConfiguration; import jenkins.model.GlobalConfigurationCategory; import jenkins.model.Jenkins; +import jenkins.model.Loadable; import jenkins.security.RedactSecretJsonInErrorMessageSanitizer; import jenkins.util.io.OnMaster; import net.sf.json.JSONArray; @@ -143,7 +144,7 @@ * @author Kohsuke Kawaguchi * @see Describable */ -public abstract class Descriptor> implements Saveable, OnMaster { +public abstract class Descriptor> implements Loadable, Saveable, OnMaster { /** * The class being described by this descriptor. */ @@ -924,6 +925,7 @@ public synchronized void save() { * (If we do that in the base class, the derived class won't * get a chance to set default values.) */ + @Override public synchronized void load() { XmlFile file = getConfigFile(); if (!file.exists()) diff --git a/core/src/main/java/hudson/model/PersistentDescriptor.java b/core/src/main/java/hudson/model/PersistentDescriptor.java index 908752151279..3c4da088b6f0 100644 --- a/core/src/main/java/hudson/model/PersistentDescriptor.java +++ b/core/src/main/java/hudson/model/PersistentDescriptor.java @@ -1,6 +1,7 @@ package hudson.model; import jakarta.annotation.PostConstruct; +import jenkins.model.Loadable; /** * Marker interface for Descriptors which use xml persistent data, and as such need to load from disk when instantiated. @@ -10,8 +11,9 @@ * @author Nicolas De Loof * @since 2.140 */ -public interface PersistentDescriptor extends Saveable { +public interface PersistentDescriptor extends Loadable, Saveable { @PostConstruct + @Override void load(); } diff --git a/core/src/main/java/hudson/model/UpdateCenter.java b/core/src/main/java/hudson/model/UpdateCenter.java index 0e04bf1db53a..58d9982dbbd1 100644 --- a/core/src/main/java/hudson/model/UpdateCenter.java +++ b/core/src/main/java/hudson/model/UpdateCenter.java @@ -110,6 +110,7 @@ import jenkins.install.InstallUtil; import jenkins.management.Badge; import jenkins.model.Jenkins; +import jenkins.model.Loadable; import jenkins.security.stapler.StaplerDispatchable; import jenkins.util.SystemProperties; import jenkins.util.Timer; @@ -156,7 +157,7 @@ * @since 1.220 */ @ExportedBean -public class UpdateCenter extends AbstractModelObject implements Saveable, OnMaster, StaplerProxy { +public class UpdateCenter extends AbstractModelObject implements Loadable, Saveable, OnMaster, StaplerProxy { private static final Logger LOGGER; private static final String UPDATE_CENTER_URL; @@ -986,6 +987,7 @@ public synchronized void save() { /** * Loads the data from the disk into this object. */ + @Override public synchronized void load() throws IOException { XmlFile file = getConfigFile(); if (file.exists()) { diff --git a/core/src/main/java/hudson/model/User.java b/core/src/main/java/hudson/model/User.java index c41b28f2c1a0..6d8b83a793e3 100644 --- a/core/src/main/java/hudson/model/User.java +++ b/core/src/main/java/hudson/model/User.java @@ -70,6 +70,7 @@ import javax.servlet.http.HttpServletResponse; import jenkins.model.IdStrategy; import jenkins.model.Jenkins; +import jenkins.model.Loadable; import jenkins.model.ModelObjectWithContextMenu; import jenkins.scm.RunWithSCM; import jenkins.security.ImpersonatingUserDetailsService2; @@ -120,7 +121,7 @@ * @author Kohsuke Kawaguchi */ @ExportedBean -public class User extends AbstractModelObject implements AccessControlled, DescriptorByNameOwner, Saveable, Comparable, ModelObjectWithContextMenu, StaplerProxy { +public class User extends AbstractModelObject implements AccessControlled, DescriptorByNameOwner, Loadable, Saveable, Comparable, ModelObjectWithContextMenu, StaplerProxy { public static final XStream2 XSTREAM = new XStream2(); private static final Logger LOGGER = Logger.getLogger(User.class.getName()); @@ -191,6 +192,11 @@ private User(String id, String fullName) { load(id); } + @Override + public void load() { + load(id); + } + private void load(String userId) { clearExistingProperties(); loadFromUserConfigFile(userId); diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index b620d35b739d..fa5d14b4032c 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -344,7 +344,7 @@ @ExportedBean public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLevelItemGroup, StaplerProxy, StaplerFallback, ModifiableViewGroup, AccessControlled, DescriptorByNameOwner, - ModelObjectWithContextMenu, ModelObjectWithChildren, OnMaster { + ModelObjectWithContextMenu, ModelObjectWithChildren, OnMaster, Loadable { private final transient Queue queue; // flag indicating if we have loaded the jenkins configuration or not yet. @@ -3367,7 +3367,8 @@ public Computer createComputer() { return new Hudson.MasterComputer(); } - private void loadConfig() throws IOException { + @Override + public void load() throws IOException { XmlFile cfg = getConfigFile(); if (cfg.exists()) { // reset some data that may not exist in the disk file @@ -3488,7 +3489,7 @@ private synchronized TaskBuilder loadTasks() throws IOException { Handle loadJenkins = g.requires(EXTENSIONS_AUGMENTED).attains(SYSTEM_CONFIG_LOADED).add("Loading global config", new Executable() { @Override public void run(Reactor session) throws Exception { - loadConfig(); + load(); // if we are loading old data that doesn't have this field if (slaves != null && !slaves.isEmpty() && nodes.isLegacy()) { nodes.setNodes(slaves); diff --git a/core/src/main/java/jenkins/model/Loadable.java b/core/src/main/java/jenkins/model/Loadable.java new file mode 100644 index 000000000000..a8fd2eca4c56 --- /dev/null +++ b/core/src/main/java/jenkins/model/Loadable.java @@ -0,0 +1,19 @@ +package jenkins.model; + +import hudson.model.Saveable; +import java.io.IOException; + +/** + * Object whose state can be loaded from disk. In general, also implements {@link Saveable}. + * + * @since TODO + */ +public interface Loadable { + + /** + * Loads the state of this object from disk. + * + * @throws IOException The state could not be loaded. + */ + void load() throws IOException; +}