From bcc86148561ad9ab4a0365ff00d28ff7ee71d57c Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 1 Apr 2014 10:53:25 -0400 Subject: [PATCH] [FIXED JENKINS-18364] Functions.getRelativeLinkTo is called many times during rendering, so skip checking View.getItems which is expensive. --- changelog.html | 4 ++- core/src/main/java/hudson/Functions.java | 11 ++----- core/src/test/java/hudson/FunctionsTest.java | 30 +++++++++++++++----- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/changelog.html b/changelog.html index d9a9cd58c7..45703f22db 100644 --- a/changelog.html +++ b/changelog.html @@ -55,7 +55,9 @@ Upcoming changes diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java index 4899606ae4..f6730b44ca 100644 --- a/core/src/main/java/hudson/Functions.java +++ b/core/src/main/java/hudson/Functions.java @@ -1014,20 +1014,15 @@ public class Functions { Item i=p; String url = ""; - Collection viewItems; - if (view != null) { - viewItems = view.getItems(); - } else { - viewItems = Collections.emptyList(); - } while(true) { ItemGroup ig = i.getParent(); url = i.getShortUrl()+url; if(ig== Jenkins.getInstance() || (view != null && ig == view.getOwnerItemGroup())) { assert i instanceof TopLevelItem; - if(viewItems.contains((TopLevelItem)i)) { - // if p and the current page belongs to the same view, then return a relative path + if (view != null) { + // assume p and the current page belong to the same view, so return a relative path + // (even if they did not, View.getItem does not by default verify ownership) return normalizeURI(ancestors.get(view)+'/'+url); } else { // otherwise return a path from the root Hudson diff --git a/core/src/test/java/hudson/FunctionsTest.java b/core/src/test/java/hudson/FunctionsTest.java index a35cce9240..6bc0b11664 100644 --- a/core/src/test/java/hudson/FunctionsTest.java +++ b/core/src/test/java/hudson/FunctionsTest.java @@ -23,31 +23,30 @@ */ package hudson; -import static org.junit.Assert.assertEquals; -import static org.powermock.api.mockito.PowerMockito.mock; -import static org.powermock.api.mockito.PowerMockito.mockStatic; -import static org.powermock.api.mockito.PowerMockito.when; import hudson.model.Action; +import hudson.model.Computer; import hudson.model.Item; import hudson.model.ItemGroup; import hudson.model.TopLevelItem; import hudson.model.View; - import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.logging.Level; import java.util.logging.LogRecord; - import jenkins.model.Jenkins; - +import static org.junit.Assert.assertEquals; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.jvnet.hudson.test.Bug; import org.kohsuke.stapler.Ancestor; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; +import static org.powermock.api.mockito.PowerMockito.mock; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.when; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; @@ -135,6 +134,23 @@ public class FunctionsTest { assertEquals("job/i/", result); } + @Test + @PrepareForTest({Stapler.class, Jenkins.class}) + public void testGetRelativeLinkTo_JobFromComputer() throws Exception{ + Jenkins j = createMockJenkins(); + ItemGroup parent = j; + String contextPath = "/jenkins"; + StaplerRequest req = createMockRequest(contextPath); + mockStatic(Stapler.class); + when(Stapler.getCurrentRequest()).thenReturn(req); + Computer computer = mock(Computer.class); + createMockAncestors(req, createAncestor(computer, "."), createAncestor(j, "../..")); + TopLevelItem i = createMockItem(parent, "job/i/"); + String result = Functions.getRelativeLinkTo(i); + assertEquals("/jenkins/job/i/", result); + } + + @Ignore("too expensive to make it correct") @Test @PrepareForTest({Stapler.class, Jenkins.class}) public void testGetRelativeLinkTo_JobNotContainedInView() throws Exception{ -- GitLab