From 3ade716f416ad13904b212c8164c9d4f3e2f2e7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Fern=C3=A1ndez?= <31063239+fcojfernandez@users.noreply.github.com> Date: Wed, 15 Jan 2020 07:54:52 +0100 Subject: [PATCH] [JENKINS-60577] - Prevent the RSS feed in Computer page from returning an error 404 (#4411) * [JENKINS-60577] Rss Latest Builds for Computer * [JENKINS-60577] Missing javadoc * [JENKINS-60577] Add builds from the computer * Update core/src/main/java/hudson/model/Computer.java * Apply suggestions from code review Co-Authored-By: Oleg Nenashev * [JENKINS-60577] Document limitation to AbstractProject * [JENKINS-60577] Restrict the RSS methods in Computer Co-authored-by: Oleg Nenashev --- core/src/main/java/hudson/model/Computer.java | 30 +++++++-- core/src/main/java/hudson/model/Job.java | 10 +-- core/src/main/java/hudson/model/RSS.java | 34 ++++++++++ core/src/main/java/hudson/model/User.java | 11 +--- core/src/main/java/hudson/model/View.java | 12 +--- test/src/test/java/hudson/model/RSSTest.java | 63 +++++++++++++++++-- 6 files changed, 124 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index bd0e246330..bdff4ea5ee 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -1368,16 +1368,36 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces // UI // // + @Restricted(DoNotUse.class) public void doRssAll( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { - rss(req, rsp, " all builds", getBuilds()); + RSS.rss(req, rsp, getDisplayName() + " all builds", getUrl(), getBuilds()); } + @Restricted(DoNotUse.class) public void doRssFailed(StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { - rss(req, rsp, " failed builds", getBuilds().failureOnly()); + RSS.rss(req, rsp, getDisplayName() + " failed builds", getUrl(), getBuilds().failureOnly()); } - private void rss(StaplerRequest req, StaplerResponse rsp, String suffix, RunList runs) throws IOException, ServletException { - RSS.forwardToRss(getDisplayName() + suffix, getUrl(), - runs.newBuilds(), Run.FEED_ADAPTER, req, rsp); + + /** + * Retrieve the RSS feed for the last build for each project executed in this computer. + * Only the information from {@link AbstractProject} is displayed since there isn't a proper API to gather + * information about the node where the builds are executed for other sorts of projects such as Pipeline + * @since TODO + */ + @Restricted(DoNotUse.class) + public void doRssLatest( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { + final List lastBuilds = new ArrayList<>(); + for (AbstractProject p : Jenkins.get().allItems(AbstractProject.class)) { + if (p.getLastBuild() != null) { + for (AbstractBuild b = p.getLastBuild(); b != null; b = b.getPreviousBuild()) { + if (b.getBuiltOn() == getNode()) { + lastBuilds.add(b); + break; + } + } + } + } + RSS.rss(req, rsp, getDisplayName() + " last builds only", getUrl(), RunList.fromRuns(lastBuilds)); } @RequirePOST diff --git a/core/src/main/java/hudson/model/Job.java b/core/src/main/java/hudson/model/Job.java index f62ba2bd21..80b73062c4 100644 --- a/core/src/main/java/hudson/model/Job.java +++ b/core/src/main/java/hudson/model/Job.java @@ -1566,18 +1566,12 @@ public abstract class Job, RunT extends Run feedAdapter) throws IOException, ServletException { + final FeedAdapter feedAdapter_ = feedAdapter == null ? Run.FEED_ADAPTER : feedAdapter; + forwardToRss(title, url, runList, feedAdapter_, req, rsp); + } } diff --git a/core/src/main/java/hudson/model/User.java b/core/src/main/java/hudson/model/User.java index 5df13fc15f..54f9db6c6a 100644 --- a/core/src/main/java/hudson/model/User.java +++ b/core/src/main/java/hudson/model/User.java @@ -872,11 +872,11 @@ public class User extends AbstractModelObject implements AccessControlled, Descr } public void doRssAll(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { - rss(req, rsp, " all builds", getBuilds(), Run.FEED_ADAPTER); + RSS.rss(req, rsp, getDisplayName() + " all builds", getUrl(), getBuilds().newBuilds()); } public void doRssFailed(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { - rss(req, rsp, " regression builds", getBuilds().regressionOnly(), Run.FEED_ADAPTER); + RSS.rss(req, rsp, getDisplayName() + " regression builds", getUrl(), getBuilds().regressionOnly()); } public void doRssLatest(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { @@ -892,12 +892,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr // historically these have been reported sorted by project name, we switched to the lazy iteration // so we only have to sort the sublist of runs rather than the full list of irrelevant projects lastBuilds.sort((o1, o2) -> Items.BY_FULL_NAME.compare(o1.getParent(), o2.getParent())); - rss(req, rsp, " latest build", RunList.fromRuns(lastBuilds), Run.FEED_ADAPTER_LATEST); - } - - private void rss(StaplerRequest req, StaplerResponse rsp, String suffix, RunList runs, FeedAdapter adapter) - throws IOException, ServletException { - RSS.forwardToRss(getDisplayName() + suffix, getUrl(), runs.newBuilds(), adapter, req, rsp); + RSS.rss(req, rsp, getDisplayName() + " latest build", getUrl(), RunList.fromRuns(lastBuilds), Run.FEED_ADAPTER_LATEST); } @Override diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java index 032bd9f7d8..d310c5295c 100644 --- a/core/src/main/java/hudson/model/View.java +++ b/core/src/main/java/hudson/model/View.java @@ -1144,11 +1144,11 @@ public abstract class View extends AbstractModelObject implements AccessControll } public void doRssAll( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { - rss(req, rsp, " all builds", getBuilds()); + RSS.rss(req, rsp, getDisplayName() + " all builds", getUrl(), getBuilds().newBuilds()); } public void doRssFailed( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { - rss(req, rsp, " failed builds", getBuilds().failureOnly()); + RSS.rss(req, rsp, getDisplayName() + " failed builds", getUrl(), getBuilds().failureOnly().newBuilds()); } public RunList getBuilds() { @@ -1159,11 +1159,6 @@ public abstract class View extends AbstractModelObject implements AccessControll return new BuildTimelineWidget(getBuilds()); } - private void rss(StaplerRequest req, StaplerResponse rsp, String suffix, RunList runs) throws IOException, ServletException { - RSS.forwardToRss(getDisplayName()+ suffix, getUrl(), - runs.newBuilds(), Run.FEED_ADAPTER, req, rsp ); - } - public void doRssLatest( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { List lastBuilds = new ArrayList<>(); for (TopLevelItem item : getItems()) { @@ -1173,8 +1168,7 @@ public abstract class View extends AbstractModelObject implements AccessControll if(lb!=null) lastBuilds.add(lb); } } - RSS.forwardToRss(getDisplayName()+" last builds only", getUrl(), - lastBuilds, Run.FEED_ADAPTER_LATEST, req, rsp ); + RSS.rss(req, rsp, getDisplayName() + " last builds only", getUrl(), RunList.fromRuns(lastBuilds), Run.FEED_ADAPTER_LATEST); } /** diff --git a/test/src/test/java/hudson/model/RSSTest.java b/test/src/test/java/hudson/model/RSSTest.java index 3a87aac8ac..3967e94a7d 100644 --- a/test/src/test/java/hudson/model/RSSTest.java +++ b/test/src/test/java/hudson/model/RSSTest.java @@ -31,6 +31,10 @@ import org.jvnet.hudson.test.JenkinsRule; import org.w3c.dom.Node; import org.w3c.dom.NodeList; +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; @@ -43,7 +47,7 @@ public class RSSTest { @Test @Issue("JENKINS-59167") public void absoluteURLsPresentInRSS_evenWithoutRootUrlSetup() throws Exception { - XmlPage page = getRssPage(); + XmlPage page = getRssAllPage(); NodeList allLinks = page.getXmlDocument().getElementsByTagName("link"); assertEquals(1, allLinks.getLength()); @@ -52,14 +56,14 @@ public class RSSTest { FreeStyleProject p = j.createFreeStyleProject(); j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - page = getRssPage(); + page = getRssAllPage(); allLinks = page.getXmlDocument().getElementsByTagName("link"); assertEquals(2, allLinks.getLength()); assertAllRSSLinksContainRootUrl(allLinks); } - private XmlPage getRssPage() throws Exception { + private XmlPage getRssAllPage() throws Exception { return (XmlPage) j.createWebClient().goTo("rssAll?flavor=rss20", "text/xml"); } @@ -74,7 +78,7 @@ public class RSSTest { @Test @Issue("JENKINS-59167") public void absoluteURLsPresentInAtom_evenWithoutRootUrlSetup() throws Exception { - XmlPage page = getAtomPage(); + XmlPage page = getRssAllAtomPage(); NodeList allLinks = page.getXmlDocument().getElementsByTagName("link"); assertEquals(1, allLinks.getLength()); @@ -83,14 +87,14 @@ public class RSSTest { FreeStyleProject p = j.createFreeStyleProject(); j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - page = getAtomPage(); + page = getRssAllAtomPage(); allLinks = page.getXmlDocument().getElementsByTagName("link"); assertEquals(2, allLinks.getLength()); assertAllAtomLinksContainRootUrl(allLinks); } - private XmlPage getAtomPage() throws Exception { + private XmlPage getRssAllAtomPage() throws Exception { return (XmlPage) j.createWebClient().goTo("rssAll", "application/atom+xml"); } @@ -102,4 +106,51 @@ public class RSSTest { assertThat(url, containsString(j.getURL().toString())); } } + + @Issue("JENKINS-60577") + @Test + public void latestBuilds() throws Exception { + XmlPage page = getRssLatestPage(); + NodeList allLinks = page.getXmlDocument().getElementsByTagName("link"); + + assertEquals(1, allLinks.getLength()); + + FreeStyleProject p = j.createFreeStyleProject("test1"); + j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + + p = j.createFreeStyleProject("test2"); + j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + + page = getRssLatestPage(); + allLinks = page.getXmlDocument().getElementsByTagName("link"); + + assertEquals(3, allLinks.getLength()); + assertLatestRSSLinks(allLinks); + + page = getRssAllPage(); + allLinks = page.getXmlDocument().getElementsByTagName("link"); + assertEquals(6, allLinks.getLength()); + } + + private XmlPage getRssLatestPage() throws Exception { + return (XmlPage) j.createWebClient().goTo("rssLatest?flavor=rss20", "text/xml"); + } + + private void assertLatestRSSLinks(NodeList allLinks) throws Exception { + List urls = new ArrayList<>(allLinks.getLength()); + for (int i = 0; i < allLinks.getLength(); i++) { + Node item = allLinks.item(i); + String url = item.getTextContent(); + urls.add(url); + } + + assertThat(urls, containsInAnyOrder( + j.getURL().toString(), + j.getURL().toString() + "job/test1/2/", + j.getURL().toString() + "job/test2/3/" + )); + } } -- GitLab