Merge commit 'ea954349'

changelog.html

@@ -67,6 +67,9 @@ Upcoming changes</a>
   <li class=bug>
     Incorrect redirect after deleting a folder.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-23375">issue 23375</a>)
+  <li class=bug>
+    Incorrect links from Build History page inside a folder.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-19310">issue 19310</a>)
   <li class=rfe>
     API changes allowing new job types to use SCM plugins.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-23365">issue 23365</a>)

core/src/main/java/hudson/model/AbstractItem.java

@@ -54,6 +54,8 @@ import java.io.IOException;
 import java.util.Collection;
 import java.util.List;
 import java.util.ListIterator;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import javax.annotation.Nonnull;
 
 import org.kohsuke.stapler.StaplerRequest;
@@ -84,6 +86,9 @@ import org.kohsuke.stapler.Ancestor;
 // Java doesn't let multiple inheritance.
 @ExportedBean
 public abstract class AbstractItem extends Actionable implements Item, HttpDeletable, AccessControlled, DescriptorByNameOwner {
+
+    private static final Logger LOGGER = Logger.getLogger(AbstractItem.class.getName());
+
     /**
      * Project name.
      */
@@ -395,16 +400,41 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet
     public final String getUrl() {
         // try to stick to the current view if possible
         StaplerRequest req = Stapler.getCurrentRequest();
+        String shortUrl = getShortUrl();
+        String uri = req == null ? null : req.getRequestURI();
         if (req != null) {
             String seed = Functions.getNearestAncestorUrl(req,this);
+            LOGGER.log(Level.FINER, "seed={0} for {1} from {2}", new Object[] {seed, this, uri});
             if(seed!=null) {
                 // trim off the context path portion and leading '/', but add trailing '/'
                 return seed.substring(req.getContextPath().length()+1)+'/';
             }
+            List<Ancestor> ancestors = req.getAncestors();
+            if (!ancestors.isEmpty()) {
+                Ancestor last = ancestors.get(ancestors.size() - 1);
+                if (last.getObject() instanceof View) {
+                    View view = (View) last.getObject();
+                    if (view.getOwnerItemGroup() == getParent() && !view.isDefault()) {
+                        // Showing something inside a view, so should use that as the base URL.
+                        String base = last.getUrl().substring(req.getContextPath().length() + 1) + '/';
+                        LOGGER.log(Level.FINER, "using {0}{1} for {2} from {3}", new Object[] {base, shortUrl, this, uri});
+                        return base + shortUrl;
+                    } else {
+                        LOGGER.log(Level.FINER, "irrelevant {0} for {1} from {2}", new Object[] {last.getObject(), this, uri});
+                    }
+                } else {
+                    LOGGER.log(Level.FINER, "inapplicable {0} for {1} from {2}", new Object[] {last.getObject(), this, uri});
+                }
+            } else {
+                LOGGER.log(Level.FINER, "no ancestors for {0} from {1}", new Object[] {this, uri});
+            }
+        } else {
+            LOGGER.log(Level.FINER, "no current request for {0}", this);
         }
-
         // otherwise compute the path normally
-        return getParent().getUrl()+getShortUrl();
+        String base = getParent().getUrl();
+        LOGGER.log(Level.FINER, "falling back to {0}{1} for {2} from {3}", new Object[] {base, shortUrl, this, uri});
+        return base + shortUrl;
     }
 
     public String getShortUrl() {

core/src/main/java/jenkins/model/Jenkins.java

@@ -180,7 +180,6 @@ import hudson.util.MultipartFormDataParser;
 import hudson.util.NamingThreadFactory;
 import hudson.util.RemotingDiagnostics;
 import hudson.util.RemotingDiagnostics.HeapDump;
-import hudson.util.StreamTaskListener;
 import hudson.util.TextFile;
 import hudson.util.TimeUnit2;
 import hudson.util.VersionNumber;
@@ -223,7 +222,6 @@ import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.args4j.Argument;
 import org.kohsuke.args4j.Option;
-import org.kohsuke.stapler.Ancestor;
 import org.kohsuke.stapler.HttpRedirect;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
@@ -666,24 +664,6 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
         protected File getRootDirFor(String name) {
             return Jenkins.this.getRootDirFor(name);
         }
-
-        /**
-         * Send the browser to the config page.
-         * use View to trim view/{default-view} from URL if possible
-         */
-        @Override
-        protected String redirectAfterCreateItem(StaplerRequest req, TopLevelItem result) throws IOException {
-            String redirect = result.getUrl()+"configure";
-            List<Ancestor> ancestors = req.getAncestors();
-            for (int i = ancestors.size() - 1; i >= 0; i--) {
-                Object o = ancestors.get(i).getObject();
-                if (o instanceof View) {
-                    redirect = req.getContextPath() + '/' + ((View)o).getUrl() + redirect;
-                    break;
-                }
-            }
-            return redirect;
-        }
     };
 
 

core/src/main/java/jenkins/util/ProgressiveRendering.java

@@ -25,16 +25,28 @@
 package jenkins.util;
 
 import edu.umd.cs.findbugs.annotations.SuppressWarnings;
+import hudson.model.AbstractItem;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nonnull;
+import javax.servlet.http.HttpServletRequest;
 import net.sf.json.JSON;
 import net.sf.json.JSONObject;
 import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
+import org.kohsuke.stapler.Ancestor;
+import org.kohsuke.stapler.RequestImpl;
 import org.kohsuke.stapler.Stapler;
-import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.TokenList;
 import org.kohsuke.stapler.bind.JavaScriptMethod;
 
 /**
@@ -68,13 +80,11 @@ public abstract class ProgressiveRendering {
     private double status = 0;
     private long lastNewsTime;
     /** just for logging */
-    private final String uri;
+    private String uri;
     private long start;
 
     /** Constructor for subclasses. */
     protected ProgressiveRendering() {
-        StaplerRequest currentRequest = Stapler.getCurrentRequest();
-        uri = currentRequest != null ? currentRequest.getRequestURI() : "?";
     }
 
     /**
@@ -83,9 +93,12 @@ public abstract class ProgressiveRendering {
     @SuppressWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE")
     public final void start() {
         final SecurityContext securityContext = SecurityContextHolder.getContext();
+        final RequestImpl request = createMockRequest();
+        uri = request.getRequestURI();
         executorService().submit(new Runnable() {
-            public void run() {
+            @Override public void run() {
                 lastNewsTime = start = System.currentTimeMillis();
+                setCurrentRequest(request);
                 SecurityContext orig = SecurityContextHolder.getContext();
                 try {
                     SecurityContextHolder.setContext(securityContext);
@@ -98,15 +111,67 @@ public abstract class ProgressiveRendering {
                     status = ERROR;
                 } finally {
                     SecurityContextHolder.setContext(orig);
+                    setCurrentRequest(null);
                     LOG.log(Level.FINE, "{0} finished in {1}msec with status {2}", new Object[] {uri, System.currentTimeMillis() - start, status});
                 }
             }
         });
     }
 
+    /**
+     * Copies important fields from the current HTTP request and makes them available during {@link #compute}.
+     * This is necessary because some model methods such as {@link AbstractItem#getUrl} behave differently when called from a request.
+     */
+    @java.lang.SuppressWarnings({"rawtypes", "unchecked"}) // public RequestImpl ctor requires List<AncestorImpl> yet AncestorImpl is not public! API design flaw
+    private static RequestImpl createMockRequest() {
+        RequestImpl currentRequest = (RequestImpl) Stapler.getCurrentRequest();
+        HttpServletRequest original = (HttpServletRequest) currentRequest.getRequest();
+        final Map<String,Object> getters = new HashMap<String,Object>();
+        for (Method method : HttpServletRequest.class.getMethods()) {
+            String m = method.getName();
+            if ((m.startsWith("get") || m.startsWith("is")) && method.getParameterTypes().length == 0) {
+                Class<?> type = method.getReturnType();
+                // TODO could add other types which are known to be safe to copy: Cookie[], Principal, HttpSession, etc.
+                if (type.isPrimitive() || type == String.class || type == Locale.class) {
+                    try {
+                        getters.put(m, method.invoke(original));
+                    } catch (Exception x) {
+                        LOG.log(Level.WARNING, "cannot mock Stapler request " + method, x);
+                    }
+                }
+            }
+        }
+        List/*<AncestorImpl>*/ ancestors = currentRequest.ancestors;
+        LOG.log(Level.FINE, "mocking ancestors {0} using {1}", new Object[] {ancestors, getters});
+        TokenList tokens = currentRequest.tokens;
+        return new RequestImpl(Stapler.getCurrent(), (HttpServletRequest) Proxy.newProxyInstance(ProgressiveRendering.class.getClassLoader(), new Class<?>[] {HttpServletRequest.class}, new InvocationHandler() {
+            @Override public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+                String m = method.getName();
+                if (getters.containsKey(m)) {
+                    return getters.get(m);
+                } else { // TODO implement other methods as needed
+                    throw new UnsupportedOperationException(m);
+                }
+            }
+        }), ancestors, tokens);
+    }
+
+    @java.lang.SuppressWarnings("unchecked")
+    private static void setCurrentRequest(RequestImpl request) {
+        try {
+            Field field = Stapler.class.getDeclaredField("CURRENT_REQUEST");
+            field.setAccessible(true);
+            ((ThreadLocal<RequestImpl>) field.get(null)).set(request);
+        } catch (Exception x) {
+            LOG.log(Level.WARNING, "cannot mock Stapler request", x);
+        }
+    }
+
     /**
      * Actually do the work.
      * <p>The security context will be that in effect when the web request was made.
+     * {@link Stapler#getCurrentRequest} will also be similar to that in effect when the web request was made;
+     * at least, {@link Ancestor}s and basic request properties (URI, locale, and so on) will be available.
      * @throws Exception whenever you like; the progress bar will indicate that an error occurred but details go to the log only
      */
     protected abstract void compute() throws Exception;

core/src/main/resources/hudson/model/Computer/builds.jelly

@@ -35,7 +35,7 @@ THE SOFTWARE.
       <st:include page="control.jelly" it="${it.timeline}" />
       <div style="height:2em"/><!-- spacer -->
 
-      <t:buildListTable builds="${it.builds}" jobBaseUrl="${rootURL}/" />
+      <t:buildListTable builds="${it.builds}"/>
     </l:main-panel>
   </l:layout>
 </j:jelly>

core/src/main/resources/hudson/model/User/builds.jelly

@@ -31,7 +31,7 @@ THE SOFTWARE.
         ${%title(it)}
       </h1>
       <!-- TODO consider adding a BuildTimelineWidget (cf. Job, View, Computer) -->
-      <t:buildListTable builds="${it.builds}" jobBaseUrl="${rootURL}/" />
+      <t:buildListTable builds="${it.builds}"/>
     </l:main-panel>
   </l:layout>
 </j:jelly>

core/src/main/resources/hudson/model/View/builds.jelly

@@ -41,8 +41,7 @@ THE SOFTWARE.
       <div>
           <a href="cc.xml">${%Export as plain XML}</a>
       </div>
-      <!-- set @jobBaseUrl="" so that links to jobs will be under this view. -->
-      <t:buildListTable builds="${it.builds}" jobBaseUrl="" />
+      <t:buildListTable builds="${it.builds}"/>
     </l:main-panel>
   </l:layout>
 </j:jelly>

core/src/main/resources/lib/hudson/buildListTable.jelly

@@ -29,9 +29,6 @@ THE SOFTWARE.
     <st:attribute name="builds" use="required">
       A collection of builds to be displayed.
     </st:attribute>
-    <st:attribute name="jobBaseUrl" use="required">
-      The base URL of all job/build links. Normally ${rootURL}/
-    </st:attribute>
   </st:documentation>
 
   <t:setIconSize/>
@@ -42,20 +39,20 @@ THE SOFTWARE.
               var e = data[x];
               var tr = new Element('tr');
               tr.insert(new Element('td', {data: e.iconColorOrdinal}).
-                  insert(new Element('a', {href: '${jobBaseUrl}' + e.url}).
+                  insert(new Element('a', {href: '${rootURL}/' + e.url}).
                       insert(new Element('img', {src: '${imagesURL}/${iconSize}/' + e.buildStatusUrl, alt: e.iconColorDescription, 'class': 'icon${iconSize}'}))));
               tr.insert(new Element('td').
-                  insert(new Element('a', {href: '${jobBaseUrl}' + e.parentUrl, 'class': 'model-link'}).
+                  insert(new Element('a', {href: '${rootURL}/' + e.parentUrl, 'class': 'model-link'}).
                       update(e.parentFullDisplayName)).
                   insert('\u00A0').
-                  insert(new Element('a', {href: '${jobBaseUrl}' + e.url, 'class': 'model-link inside'}).
+                  insert(new Element('a', {href: '${rootURL}/' + e.url, 'class': 'model-link inside'}).
                       update(e.displayName.escapeHTML())));
               tr.insert(new Element('td', {data: e.timestampString2, tooltip: '${%Click to center timeline on event}', onclick: 'javascript:tl.getBand(0).scrollToCenter(Timeline.DateTime.parseGregorianDateTime("' + e.timestampString2 + '"))'}).
                   update(e.timestampString.escapeHTML()));
               tr.insert(new Element('td', {style: e.buildStatusSummaryWorse ? 'color: red' : ''}).
                   update(e.buildStatusSummaryMessage.escapeHTML()));
               tr.insert(new Element('td').
-                  insert(new Element('a', {href: '${jobBaseUrl}' + e.url + 'console'}).
+                  insert(new Element('a', {href: '${rootURL}/' + e.url + 'console'}).
                       insert(new Element('img', {src: '${imagesURL}/${subIconSize}/terminal.png', alt: '${%Console output}', border: 0}))));
               p.insert(tr);
               Behaviour.applySubtree(tr);

test/src/test/java/hudson/model/MyViewTest.java

@@ -25,15 +25,18 @@ package hudson.model;
 
 import org.junit.Before;
 import org.junit.Test;
-import org.jvnet.hudson.test.JenkinsRule.WebClient;
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import hudson.security.GlobalMatrixAuthorizationStrategy;
-import hudson.security.HudsonPrivateSecurityRealm;
 import java.io.IOException;
+import java.util.logging.ConsoleHandler;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import org.acegisecurity.context.SecurityContextHolder;
 import org.junit.Rule;
 import org.jvnet.hudson.test.JenkinsRule;
 import static org.junit.Assert.*;
+import org.junit.BeforeClass;
 
 /**
  *
@@ -49,6 +52,14 @@ public class MyViewTest {
         rule.jenkins.setSecurityRealm(rule.createDummySecurityRealm());
     }
     
+    private static final Logger logger = Logger.getLogger(AbstractItem.class.getName());
+    @BeforeClass public static void logging() {
+        logger.setLevel(Level.ALL);
+        Handler handler = new ConsoleHandler();
+        handler.setLevel(Level.ALL);
+        logger.addHandler(handler);
+    }
+
     @Test
     public void testContains() throws IOException, Exception{
         

test/src/test/java/jenkins/widgets/BuildListTableTest.java

@@ -32,7 +32,6 @@ import java.net.URI;
 import java.net.URL;
 import org.junit.Test;
 import static org.junit.Assert.*;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
@@ -42,7 +41,6 @@ public class BuildListTableTest {
 
     @Rule public JenkinsRule r = new JenkinsRule();
 
-    @Ignore("TODO")
     @Issue("JENKINS-19310")
     @Test public void linksFromFolders() throws Exception {
         MockFolder d = r.createFolder("d");
@@ -62,8 +60,15 @@ public class BuildListTableTest {
         HtmlAnchor anchor = page.getAnchorByText("d » d2 » p");
         String href = anchor.getHrefAttribute();
         URL target = URI.create(page.getDocumentURI()).resolve(href).toURL();
+        wc.getPage(target);
         assertEquals(href, r.getURL() + "view/v1/job/d/view/v2/job/d2/job/p/", target.toString());
+        page = wc.goTo("job/d/view/All/builds?suppressTimelineControl=true");
+        assertEquals(0, wc.waitForBackgroundJavaScript(120000));
+        anchor = page.getAnchorByText("d » d2 » p");
+        href = anchor.getHrefAttribute();
+        target = URI.create(page.getDocumentURI()).resolve(href).toURL();
         wc.getPage(target);
+        assertEquals(href, r.getURL() + "job/d/job/d2/job/p/", target.toString());
     }
 
 }