[JENKINS-27177] Minor fixes

SimoHyHa414 · Oct 15, 2018 · 1aa7dbb · 1aa7dbb
1 parent a609502
commit 1aa7dbb
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 44 deletions.
diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java
@@ -29,16 +29,35 @@
 import hudson.init.InitMilestone;
 import hudson.init.InitStrategy;
 import hudson.init.InitializerFinder;
-import hudson.model.*;
+import hudson.model.AbstractItem;
+import hudson.model.AbstractModelObject;
+import hudson.model.AdministrativeMonitor;
+import hudson.model.Api;
+import hudson.model.Descriptor;
+import hudson.model.DownloadService;
+import hudson.model.Failure;
+import hudson.model.ItemGroupMixIn;
+import hudson.model.UpdateCenter;
 import hudson.model.UpdateCenter.DownloadJob;
 import hudson.model.UpdateCenter.InstallationJob;
+import hudson.model.UpdateSite;
 import hudson.security.ACL;
 import hudson.security.ACLContext;
 import hudson.security.Permission;
 import hudson.security.PermissionScope;
-import hudson.util.*;
+import hudson.util.CyclicGraphDetector;
 import hudson.util.CyclicGraphDetector.CycleDetectedException;
-import jenkins.*;
+import hudson.util.FormValidation;
+import hudson.util.PersistedList;
+import hudson.util.Service;
+import hudson.util.VersionNumber;
+import hudson.util.XStream2;
+import jenkins.ClassLoaderReflectionToolkit;
+import jenkins.ExtensionRefreshException;
+import jenkins.InitReactorRunner;
+import jenkins.MissingDependencyException;
+import jenkins.RestartRequiredException;
+import jenkins.YesNoMaybe;
 import jenkins.install.InstallState;
 import jenkins.install.InstallUtil;
 import jenkins.model.Jenkins;
@@ -60,12 +79,22 @@
 import org.apache.commons.logging.LogFactory;
 import org.jenkinsci.Symbol;
 import org.jenkinsci.bytecode.Transformer;
-import org.jvnet.hudson.reactor.*;
+import org.jvnet.hudson.reactor.Executable;
+import org.jvnet.hudson.reactor.Reactor;
+import org.jvnet.hudson.reactor.ReactorException;
+import org.jvnet.hudson.reactor.TaskBuilder;
+import org.jvnet.hudson.reactor.TaskGraphBuilder;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.DoNotUse;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
-import org.kohsuke.stapler.*;
+import org.kohsuke.stapler.HttpRedirect;
+import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
+import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.StaplerOverridable;
+import org.kohsuke.stapler.StaplerProxy;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 import org.kohsuke.stapler.interceptor.RequirePOST;
@@ -80,11 +109,34 @@
 import javax.servlet.ServletException;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParserFactory;
-import java.io.*;
+import java.io.ByteArrayInputStream;
+import java.io.Closeable;
+import java.io.File;
+import java.io.FilenameFilter;
+import java.io.IOException;
+import java.io.InputStream;
 import java.lang.ref.WeakReference;
 import java.lang.reflect.Method;
-import java.net.*;
-import java.util.*;
+import java.net.JarURLConnection;
+import java.net.MalformedURLException;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.net.URLConnection;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.ServiceLoader;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -1789,10 +1841,10 @@ public MetadataCache createCache() {
     }
 
     /**
-     * Disable a list of plugins using a strategy for its dependants plugins.
+     * Disable a list of plugins using a strategy for their dependants plugins.
      * @param strategy the strategy regarding how the dependant plugins are processed
      * @param plugins the list of plugins
-     * @return the list of results for every plugin and its dependant plugins.
+     * @return the list of results for every plugin and their dependant plugins.
      * @throws IOException see {@link PluginWrapper#disable()}
      */
     public List<PluginWrapper.PluginDisableResult> disablePlugins(PluginWrapper.PluginDisableStrategy strategy, List<String> plugins) throws IOException {

diff --git a/core/src/main/java/hudson/PluginWrapper.java b/core/src/main/java/hudson/PluginWrapper.java
@@ -27,17 +27,16 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import hudson.PluginManager.PluginInstanceStore;
-import hudson.cli.DisablePluginCommand;
 import hudson.model.AdministrativeMonitor;
 import hudson.model.Api;
 import hudson.model.ModelObject;
-import java.nio.file.Files;
-import java.nio.file.InvalidPathException;
-import jenkins.YesNoMaybe;
-import jenkins.model.Jenkins;
 import hudson.model.UpdateCenter;
 import hudson.model.UpdateSite;
 import hudson.util.VersionNumber;
+import jenkins.YesNoMaybe;
+import jenkins.model.Jenkins;
+import org.apache.commons.lang.StringUtils;
+import org.apache.commons.logging.LogFactory;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.DoNotUse;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
@@ -49,16 +48,27 @@
 import org.kohsuke.stapler.export.ExportedBean;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 
-import org.apache.commons.lang.StringUtils;
-import org.apache.commons.logging.LogFactory;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.URL;
-import java.util.*;
+import java.nio.file.Files;
+import java.nio.file.InvalidPathException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
 import java.util.function.Predicate;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
@@ -233,7 +243,6 @@ public void setOptionalDependants(@Nonnull Set<String> optionalDependants) {
     }
 
     /**
-     * Get the list of components that depend optionally on this plugin.
      * @return The list of components that depend optionally on this plugin.
      */
     public @Nonnull Set<String> getOptionalDependants() {
@@ -571,8 +580,7 @@ private void disableWithoutCheck() throws IOException {
      * @throws IOException If this plugin or one dependant plugin raise this exception during its disablement. It could
      * be raised because the disable file cannot be created.
      */
-    @Nonnull
-    public PluginDisableResult disable(@Nonnull PluginDisableStrategy strategy) throws IOException {
+    public @Nonnull PluginDisableResult disable(@Nonnull PluginDisableStrategy strategy) throws IOException {
         PluginDisableResult result = new PluginDisableResult(shortName);
 
         if (!this.isEnabled()) {
@@ -587,21 +595,7 @@ public PluginDisableResult disable(@Nonnull PluginDisableStrategy strategy) thro
 
         // List of dependants plugins to 'check'. 'Check' means disable for mandatory or all strategies, or review if
         // this dependant-mandatory plugin is enabled in order to return an error for the NONE strategy.
-        Set<String> dependantsToCheck;
-        switch (strategy) {
-            case MANDATORY:
-                // As of getDependants has all the dependants, we get the difference between them and only the optionals
-                dependantsToCheck = Sets.difference(this.getDependants(), this.getOptionalDependants());
-                break;
-            case ALL:
-                // getDependants returns all the dependant plugins, mandatory or optional.
-                dependantsToCheck = this.getDependants();
-                break;
-            case NONE:
-            default:
-                // with NONE, the process only fail if mandatory dependant plugins exists
-                dependantsToCheck = Sets.difference(this.getDependants(), this.getOptionalDependants());
-        }
+        Set<String> dependantsToCheck = dependantsToCheck(strategy);
 
         // Review all the dependants and add to the plugin result what happened with its dependants
         for (String dependant : dependantsToCheck) {
@@ -653,6 +647,25 @@ public PluginDisableResult disable(@Nonnull PluginDisableStrategy strategy) thro
         return result;
     }
 
+    private Set<String> dependantsToCheck(PluginDisableStrategy strategy) {
+        Set<String> dependantsToCheck;
+        switch (strategy) {
+            case MANDATORY:
+                // As of getDependants has all the dependants, we get the difference between them and only the optionals
+                dependantsToCheck = Sets.difference(this.getDependants(), this.getOptionalDependants());
+                break;
+            case ALL:
+                // getDependants returns all the dependant plugins, mandatory or optional.
+                dependantsToCheck = this.getDependants();
+                break;
+            case NONE:
+            default:
+                // with NONE, the process only fail if mandatory dependant plugins exists
+                dependantsToCheck = Sets.difference(this.getDependants(), this.getOptionalDependants());
+        }
+        return dependantsToCheck;
+    }
+
     /**
      * Returns true if this plugin is enabled for this session.
      */
@@ -1011,7 +1024,7 @@ public void addDependantDisableStatus(PluginDisableResult dependantDisableStatus
     /**
      * An enum to hold the status of a disabling action against a plugin. To do it more reader-friendly.
      */
-    public static enum PluginDisableStatus {
+    public enum PluginDisableStatus {
         NO_SUCH_PLUGIN,
         DISABLED,
         ALREADY_DISABLED,

diff --git a/core/src/main/resources/hudson/cli/Messages.properties b/core/src/main/resources/hudson/cli/Messages.properties
@@ -119,4 +119,4 @@ Disable one or more installed plugins.
 DisablePluginCommand.NoSuchStrategy=This strategy ({0}) does not exist. Allowed strategies are {1}
 DisablePluginCommand.PrintUsageSummary=\
 Disable the plugins with the given short names. You can define how to proceed with the dependant plugins and if a restart after should be done. You can also set the quiet mode to avoid extra info in the console.
-DisablePluginCommand.StatusMessage=Disabling {0}: {1} ({2})
+DisablePluginCommand.StatusMessage=Disabling {0}: {1} ({2})
diff --git a/core/src/main/resources/hudson/cli/Messages_es.properties b/core/src/main/resources/hudson/cli/Messages_es.properties
@@ -64,4 +64,4 @@ Deshabilita uno o m
 DisablePluginCommand.NoSuchStrategy=Esta estrategia ({0}) no existe. Las estrategias permitidas son {1}
 DisablePluginCommand.PrintUsageSummary=\
 Deshabilita los plugins con los nombre cortos dados. Puede definir cómo proceder con los plugins dependientes y si se realiza un reinicio posterior.
-DisablePluginCommand.StatusMessage=Deshabilitando {0}: {1} ({2})
+DisablePluginCommand.StatusMessage=Deshabilitando {0}: {1} ({2})
diff --git a/test/src/test/java/hudson/cli/DisablePluginCommandTest.java b/test/src/test/java/hudson/cli/DisablePluginCommandTest.java
@@ -58,15 +58,15 @@ public class DisablePluginCommandTest {
     @Test
     @Issue("JENKINS-27177")
     @WithPlugin({"depender-0.0.2.hpi", "dependee-0.0.2.hpi"})
-    public void canDisablePluginWithOptionalDependerStNone() {
+    public void canDisablePluginWithOptionalDependerStrategyNone() {
         assertThat(disablePluginsCLiCommand("-strategy", "NONE", "dependee"), succeeded());
         assertPluginDisabled("dependee");
     }
 
     @Test
     @Issue("JENKINS-27177")
     @WithPlugin({"depender-0.0.2.hpi", "dependee-0.0.2.hpi", "mandatory-depender-0.0.2.hpi"})
-    public void canDisablePluginWithDependantsDisabledStNone() throws IOException {
+    public void canDisablePluginWithDependantsDisabledStrategyNone() throws IOException {
         disablePlugin("mandatory-depender");
         CLICommandInvoker.Result result = disablePluginsCLiCommand("-strategy", "NONE", "dependee");
 
@@ -82,7 +82,7 @@ public void canDisablePluginWithDependantsDisabledStNone() throws IOException {
     @Test
     @Issue("JENKINS-27177")
     @WithPlugin({"mandatory-depender-0.0.2.hpi", "dependee-0.0.2.hpi"})
-    public void cannotDisablePluginWithMandatoryDependerStNone() {
+    public void cannotDisablePluginWithMandatoryDependerStrategyNone() {
         assertThat(disablePluginsCLiCommand("dependee"), failedWith(RETURN_CODE_NOT_DISABLED_DEPENDANTS));
         assertPluginEnabled("dependee");
     }
@@ -94,7 +94,7 @@ public void cannotDisablePluginWithMandatoryDependerStNone() {
     @Test
     @Issue("JENKINS-27177")
     @WithPlugin({"mandatory-depender-0.0.2.hpi", "dependee-0.0.2.hpi"})
-    public void cannotDisableDependentPluginWrongOrderStNone() {
+    public void cannotDisableDependentPluginWrongOrderStrategyNone() {
         assertThat(disablePluginsCLiCommand("dependee", "mandatory-depender"), failedWith(RETURN_CODE_NOT_DISABLED_DEPENDANTS));
         assertPluginDisabled("mandatory-depender");
         assertPluginEnabled("dependee");
@@ -107,7 +107,7 @@ public void cannotDisableDependentPluginWrongOrderStNone() {
     @Test
     @Issue("JENKINS-27177")
     @WithPlugin({"mandatory-depender-0.0.2.hpi", "dependee-0.0.2.hpi"})
-    public void canDisableDependentPluginsRightOrderStNone() {
+    public void canDisableDependentPluginsRightOrderStrategyNone() {
         assertThat(disablePluginsCLiCommand("mandatory-depender", "dependee"), succeeded());
         assertPluginDisabled("dependee");
         assertPluginDisabled("mandatory-depender");