[JENKINS-48638] Deprecate Jenkins.getInstance in favor of (usually) g…

…et (#3195) * Deprecate Jenkins.getInstance in favor of (usually) getActiveInstance. * Introduced get() as a concise non-null accessor. * May as well suggest rewrites of getActiveInstance too.
jenkinsci · Dec 22, 2017 · 001a33c · 001a33c
1 parent b5f24e7
commit 001a33c
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 36 deletions.
diff --git a/core/src/main/java/hudson/model/Hudson.java b/core/src/main/java/hudson/model/Hudson.java
@@ -34,7 +34,6 @@
 import hudson.slaves.ComputerListener;
 import hudson.util.CopyOnWriteList;
 import hudson.util.FormValidation;
-import javax.annotation.Nonnull;
 import jenkins.model.Jenkins;
 import org.jvnet.hudson.reactor.ReactorException;
 import org.kohsuke.stapler.QueryParameter;
@@ -52,7 +51,7 @@
 import java.util.List;
 
 import static hudson.Util.fixEmpty;
-import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
 
 public class Hudson extends Jenkins {
 
@@ -70,10 +69,10 @@ public class Hudson extends Jenkins {
     @Deprecated
     private transient final CopyOnWriteList<ComputerListener> computerListeners = ExtensionListView.createCopyOnWriteList(ComputerListener.class);
 
-    /** @deprecated Here only for compatibility. Use {@link Jenkins#getInstance} instead. */
+    /** @deprecated Here only for compatibility. Use {@link Jenkins#get} instead. */
     @Deprecated
     @CLIResolver
-    @Nonnull
+    @Nullable
     public static Hudson getInstance() {
         return (Hudson)Jenkins.getInstance();
     }

diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
@@ -179,7 +179,6 @@
 import jenkins.ExtensionRefreshException;
 import jenkins.InitReactorRunner;
 import jenkins.install.InstallState;
-import jenkins.install.InstallUtil;
 import jenkins.install.SetupWizard;
 import jenkins.model.ProjectNamingStrategy.DefaultProjectNamingStrategy;
 import jenkins.security.ConfidentialKey;
@@ -739,56 +738,57 @@ public interface JenkinsHolder {
 
     /**
      * Gets the {@link Jenkins} singleton.
-     * {@link #getInstanceOrNull()} provides the unchecked versions of the method.
      * @return {@link Jenkins} instance
-     * @throws IllegalStateException {@link Jenkins} has not been started, or was already shut down
-     * @since 1.590
-     * @deprecated use {@link #getInstance()}
+     * @throws IllegalStateException for the reasons that {@link #getInstanceOrNull} might return null
+     * @since FIXME
      */
-    @Deprecated
     @Nonnull
-    public static Jenkins getActiveInstance() throws IllegalStateException {
-        Jenkins instance = HOLDER.getInstance();
+    public static Jenkins get() throws IllegalStateException {
+        Jenkins instance = getInstanceOrNull();
         if (instance == null) {
-            throw new IllegalStateException("Jenkins has not been started, or was already shut down");
+            throw new IllegalStateException("Jenkins.instance is missing. Read the documentation of Jenkins.getInstanceOrNull to see what you are doing wrong.");
         }
         return instance;
     }
 
+    /**
+     * @deprecated This is a verbose historical alias for {@link #get}.
+     * @since 1.590
+     */
+    @Deprecated
+    @Nonnull
+    public static Jenkins getActiveInstance() throws IllegalStateException {
+        return get();
+    }
+
     /**
      * Gets the {@link Jenkins} singleton.
-     * {@link #getActiveInstance()} provides the checked versions of the method.
-     * @return The instance. Null if the {@link Jenkins} instance has not been started,
-     * or was already shut down
+     * {@link #get} is what you normally want.
+     * <p>In certain rare cases you may have code that is intended to run before Jenkins starts or while Jenkins is being shut down.
+     * For those rare cases use this method.
+     * <p>In other cases you may have code that might end up running on a remote JVM and not on the Jenkins master.
+     * For those cases you really should rewrite your code so that when the {@link Callable} is sent over the remoting channel
+     * it can do whatever it needs without ever referring to {@link Jenkins};
+     * for example, gather any information you need on the master side before constructing the callable.
+     * If you must do a runtime check whether you are in the master or agent, use {@link JenkinsJVM} rather than this method,
+     * as merely loading the {@link Jenkins} class file into an agent JVM can cause linkage errors under some conditions.
+     * @return The instance. Null if the {@link Jenkins} service has not been started, or was already shut down,
+     *         or we are running on an unrelated JVM, typically an agent.
      * @since 1.653
      */
+    @CLIResolver
     @CheckForNull
     public static Jenkins getInstanceOrNull() {
         return HOLDER.getInstance();
     }
 
     /**
-     * Gets the {@link Jenkins} singleton. In certain rare cases you may have code that is intended to run before
-     * Jenkins starts or while Jenkins is being shut-down. For those rare cases use {@link #getInstanceOrNull()}.
-     * In other cases you may have code that might end up running on a remote JVM and not on the Jenkins master,
-     * for those cases you really should rewrite your code so that when the {@link Callable} is sent over the remoting
-     * channel it uses a {@code writeReplace} method or similar to ensure that the {@link Jenkins} class is not being
-     * loaded into the remote class loader
-     * @return The instance.
-     * @throws IllegalStateException {@link Jenkins} has not been started, or was already shut down
+     * @deprecated This is a historical alias for {@link #getInstanceOrNull} but with ambiguous nullability. Use {@link #get} in typical cases.
      */
-    @CLIResolver
-    @Nonnull
+    @Nullable
+    @Deprecated
     public static Jenkins getInstance() {
-        Jenkins instance = HOLDER.getInstance();
-        if (instance == null) {
-            if(SystemProperties.getBoolean(Jenkins.class.getName()+".enableExceptionOnNullInstance")) {
-                // TODO: remove that second block around 2.20 (that is: ~20 versions to battle test it)
-                // See https://github.com/jenkinsci/jenkins/pull/2297#issuecomment-216710150
-                throw new IllegalStateException("Jenkins has not been started, or was already shut down");
-            }
-        }
-        return instance;
+        return getInstanceOrNull();
     }
 
     /**

diff --git a/core/src/main/resources/META-INF/upgrade/Hudson.hint b/core/src/main/resources/META-INF/upgrade/Hudson.hint
@@ -1 +1 @@
-hudson.model.Hudson.getInstance() => jenkins.model.Jenkins.getInstance();;
+hudson.model.Hudson.getInstance() => jenkins.model.Jenkins.get();;
diff --git a/core/src/main/resources/META-INF/upgrade/Jenkins.hint b/core/src/main/resources/META-INF/upgrade/Jenkins.hint
@@ -0,0 +1,2 @@
+jenkins.model.Jenkins.getInstance() => jenkins.model.Jenkins.get();;
+jenkins.model.Jenkins.getActiveInstance() => jenkins.model.Jenkins.get();;