Merge pull request #2146 from olivergondza/hide-management-link

[FIXED JENKINS-33683] - Fix Management link hiding
jenkinsci · Apr 13, 2016 · ac566a9 · ac566a9
2 parents b17e04c + 8fdca29
commit ac566a9
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 16 deletions.
diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java
@@ -1532,10 +1532,12 @@ public static String joinPath(String... components) {
     /**
      * Computes the hyperlink to actions, to handle the situation when the {@link Action#getUrlName()}
      * returns absolute URL.
+     *
+     * @return null in case the action should not be presented to the user.
      */
-    public static String getActionUrl(String itUrl,Action action) {
+    public static @CheckForNull String getActionUrl(String itUrl,Action action) {
         String urlName = action.getUrlName();
-        if(urlName==null)   return null;    // to avoid NPE and fail to render the whole page
+        if(urlName==null)   return null;    // Should not be displayed
         try {
             if (new URI(urlName).isAbsolute()) {
                 return urlName;

diff --git a/core/src/main/java/hudson/model/Action.java b/core/src/main/java/hudson/model/Action.java
@@ -25,6 +25,8 @@
 
 import hudson.Functions;
 
+import javax.annotation.CheckForNull;
+
 /**
  * Object that contributes additional information, behaviors, and UIs to {@link ModelObject}
  * (more specifically {@link Actionable} objects, which most interesting {@link ModelObject}s are.)
@@ -90,15 +92,17 @@ public interface Action extends ModelObject {
      * @see Functions#isAnonymous()
      * @see Functions#getIconFilePath(Action)
      */
-    String getIconFileName();
+    @CheckForNull String getIconFileName();
 
     /**
      * Gets the string to be displayed.
      *
      * The convention is to capitalize the first letter of each word,
-     * such as "Test Result". 
+     * such as "Test Result".
+     *
+     * @return Can be null in case the action is hidden.
      */
-    String getDisplayName();
+    @CheckForNull String getDisplayName();
 
     /**
      * Gets the URL path name.
@@ -124,5 +128,5 @@ public interface Action extends ModelObject {
      *      (when you do that, be sure to also return null from {@link #getIconFileName()}.
      * @see Functions#getActionUrl(String, Action)
      */
-    String getUrlName();
+    @CheckForNull String getUrlName();
 }
diff --git a/core/src/main/java/hudson/model/ManagementLink.java b/core/src/main/java/hudson/model/ManagementLink.java
@@ -33,6 +33,9 @@
 import java.util.List;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+
 /**
  * Extension point to add icon to <tt>http://server/hudson/manage</tt> page.
  *
@@ -59,7 +62,7 @@ public abstract class ManagementLink implements ExtensionPoint, Action {
      *      This is useful for defining {@link ManagementLink} that only shows up under
      *      certain circumstances.
      */
-    public abstract String getIconFileName();
+    public abstract @CheckForNull String getIconFileName();
 
     /**
      * Returns a short description of what this link does. This text
@@ -80,7 +83,7 @@ public String getDescription() {
      * so relative paths are interpreted against the root {@link Jenkins} object.
      */
     @Override
-    public abstract String getUrlName();
+    public abstract @CheckForNull String getUrlName();
 
     /**
      * Allows implementations to request that this link show a confirmation dialog, and use POST if confirmed.
@@ -102,16 +105,16 @@ public boolean getRequiresConfirmation() {
     public static final List<ManagementLink> LIST = ExtensionListView.createList(ManagementLink.class);
 
     /**
-     * All regsitered instances.
+     * All registered instances.
      */
-    public static ExtensionList<ManagementLink> all() {
+    public static @Nonnull ExtensionList<ManagementLink> all() {
         return ExtensionList.lookup(ManagementLink.class);
     }
 
     /**
      * @return permission required for user to access this management link, in addition to {@link Jenkins#ADMINISTER}
      */
-    public Permission getRequiredPermission() {
+    public @CheckForNull Permission getRequiredPermission() {
         return null;
     }
 

diff --git a/core/src/main/java/hudson/model/User.java b/core/src/main/java/hudson/model/User.java
@@ -73,6 +73,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -903,11 +904,11 @@ public Descriptor getDescriptorByName(String className) {
 
     public Object getDynamic(String token) {
         for(Action action: getTransientActions()){
-            if(action.getUrlName().equals(token))
+            if(Objects.equals(action.getUrlName(), token))
                 return action;
         }
         for(Action action: getPropertyActions()){
-            if(action.getUrlName().equals(token))
+            if(Objects.equals(action.getUrlName(), token))
                 return action;
         }
         return null;

diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java
@@ -552,7 +552,7 @@ public Object getDynamic(String token) {
         for (Action a : getActions()) {
             String url = a.getUrlName();
             if (url==null)  continue;
-            if(a.getUrlName().equals(token))
+            if (url.equals(token))
                 return a;
         }
         return null;

diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
@@ -191,6 +191,8 @@
 import hudson.views.MyViewsTabBar;
 import hudson.views.ViewsTabBar;
 import hudson.widgets.Widget;
+
+import java.util.Objects;
 import java.util.TimerTask;
 import java.util.concurrent.CountDownLatch;
 import jenkins.ExtensionComponentSet;
@@ -3333,7 +3335,7 @@ public Object getDynamic(String token) {
                 return a;
         }
         for (Action a : getManagementLinks())
-            if(a.getUrlName().equals(token))
+            if (Objects.equals(a.getUrlName(), token))
                 return a;
         return null;
     }
@@ -4329,7 +4331,9 @@ public Collection<String> getUnprotectedRootActions() {
         // TODO consider caching (expiring cache when actions changes)
         for (Action a : getActions()) {
             if (a instanceof UnprotectedRootAction) {
-                names.add(a.getUrlName());
+                String url = a.getUrlName();
+                if (url == null) continue;
+                names.add(url);
             }
         }
         return names;

diff --git a/test/src/test/java/hudson/model/ManagementLinkTest.java b/test/src/test/java/hudson/model/ManagementLinkTest.java
@@ -23,15 +23,18 @@
  */
 package hudson.model;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import com.gargoylesoftware.htmlunit.html.DomNodeUtil;
 import com.gargoylesoftware.htmlunit.html.HtmlAnchor;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.JenkinsRule.WebClient;
+import org.jvnet.hudson.test.TestExtension;
 
 import java.util.List;
 
@@ -59,4 +62,28 @@ public void links() throws Exception {
             ((HtmlAnchor)anchors.get(i)).click();
         }
     }
+
+    @Test @Issue("JENKINS-33683")
+    public void invisibleLinks() throws Exception {
+        assertEquals(null, j.jenkins.getDynamic("and_fail_trying"));
+    }
+
+    @TestExtension // Intentionally hooked in all tests
+    public static final class InvisibleManagementLink extends ManagementLink {
+
+        @Override
+        public String getIconFileName() {
+            return null;
+        }
+
+        @Override
+        public String getDisplayName() {
+            return null;
+        }
+
+        @Override
+        public String getUrlName() {
+            return null;
+        }
+    }
 }