diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java index 3008814c48..188412b3c3 100644 --- a/core/src/main/java/hudson/Functions.java +++ b/core/src/main/java/hudson/Functions.java @@ -1532,10 +1532,12 @@ public class Functions { /** * 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 index f73bd34032..2e8d2e641f 100644 --- a/core/src/main/java/hudson/model/Action.java +++ b/core/src/main/java/hudson/model/Action.java @@ -25,6 +25,8 @@ package hudson.model; 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 index dca459ca70..041fda35d0 100644 --- a/core/src/main/java/hudson/model/ManagementLink.java +++ b/core/src/main/java/hudson/model/ManagementLink.java @@ -33,6 +33,9 @@ import jenkins.model.Jenkins; import java.util.List; import org.kohsuke.stapler.interceptor.RequirePOST; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; + /** * Extension point to add icon to http://server/hudson/manage 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 abstract class ManagementLink implements ExtensionPoint, Action { * 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 abstract class ManagementLink implements ExtensionPoint, Action { public static final List LIST = ExtensionListView.createList(ManagementLink.class); /** - * All regsitered instances. + * All registered instances. */ - public static ExtensionList all() { + public static @Nonnull ExtensionList 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 index 4a16c9f153..99f1a39a4e 100644 --- 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.HashSet; 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 class User extends AbstractModelObject implements AccessControlled, Descr 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 index f9e2d5c855..4f4976f724 100644 --- a/core/src/main/java/hudson/model/View.java +++ b/core/src/main/java/hudson/model/View.java @@ -552,7 +552,7 @@ public abstract class View extends AbstractModelObject implements AccessControll 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 index ddf35cd265..0e862b6bab 100644 --- 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.DefaultViewsTabBar; 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 class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve 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 class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve // 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 index 5de5dd3d51..6df8a85752 100644 --- a/test/src/test/java/hudson/model/ManagementLinkTest.java +++ b/test/src/test/java/hudson/model/ManagementLinkTest.java @@ -23,6 +23,7 @@ */ package hudson.model; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import com.gargoylesoftware.htmlunit.html.DomNodeUtil; @@ -30,8 +31,10 @@ 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 class ManagementLinkTest { ((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; + } + } }