Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-36282] Add support for exporting jobs in folders recursively #1

Closed
wants to merge 8 commits into from
3 changes: 1 addition & 2 deletions pom.xml
Expand Up @@ -6,7 +6,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>2.19</version>
<version>2.35</version>
<relativePath/>
</parent>

Expand All @@ -18,7 +18,6 @@
<!-- TODO FIXME Adjust after release that dropped cc.xml from core -->
<jenkins.version>2.41</jenkins.version>
<java.level>7</java.level>
<jenkins-test-harness.version>2.13</jenkins-test-harness.version>
</properties>

<name>CCtray XML (cc.xml) Plugin</name>
Expand Down
44 changes: 43 additions & 1 deletion src/main/java/org/jenkinsci/plugins/ccxml/CCXMLAction.java
Expand Up @@ -25,14 +25,25 @@

import hudson.model.Action;
import hudson.model.Item;
import hudson.model.ItemGroup;
import hudson.model.Job;
import hudson.model.TopLevelItem;
import hudson.model.View;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.Stapler;

@Restricted(NoExternalUse.class)
public class CCXMLAction implements Action {

public static final String URL_NAME = "cc.xml2";

private transient View view;

CCXMLAction(View view) {
Expand All @@ -51,13 +62,44 @@ public String getDisplayName() {

@Override
public String getUrlName() {
return "cc.xml2";
return URL_NAME;
}

public View getView() {
return this.view;
}

/**
* @return A map containing the items in the view object. If the request
* contains a query parameter named "recursive", then folders in the view
* are traversed recursively and all items in those folders are returned
* as well. The map is keyed by the folder the item is in, with top-level
* items having an empty key.
*/
public Map<String, Collection<TopLevelItem>> getItems() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is considered as a backportable bugfix, new public methods should be marked as @Restricted(NoExternalUse.class)

String recursive = Stapler.getCurrentRequest().getParameter("recursive");
if (recursive == null) {
return Collections.singletonMap("", view.getItems());
} else {
return Collections.unmodifiableMap(getItemsRecursive("", view.getItems()));
}
}

private Map<String, Collection<TopLevelItem>> getItemsRecursive(String namePrefix, Collection<TopLevelItem> items) {
Map<String, Collection<TopLevelItem>> result = new HashMap<>();
List<TopLevelItem> currentLevelItems = new ArrayList<>();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on the number of folders and projects on the Jenkins instance, and how sparse the folders are, these ArrayList and HashMap objects might have a lot of unused space. Does anyone think I should make them a smaller size to start, or use a LinkedList instead or anything?

for (TopLevelItem i : items) {
if (i instanceof ItemGroup) {
ItemGroup g = (ItemGroup) i;
result.putAll(getItemsRecursive(namePrefix + g.getDisplayName() + "/", g.getItems()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about Display Name here. It causes some concerns since you duplicate the core's logic here. I would rather suggest to use Item#getFullDisplayName though it decreases the performance of the call

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know about Item#getFullDisplayName, so I will update it to use that.

} else {
currentLevelItems.add(i);
}
}
result.put(namePrefix, currentLevelItems);
return result;
}

/**
* Converts the Hudson build status to CruiseControl build status,
* which is either Success, Failure, Exception, or Unknown.
Expand Down
Expand Up @@ -28,19 +28,21 @@ THE SOFTWARE.
-->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt">
<st:contentType value="text/xml;charset=UTF-8" />
<st:contentType value="application/xml;charset=UTF-8" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to RFC referenced in https://stackoverflow.com/questions/4832357/whats-the-difference-between-text-xml-vs-application-xml-for-webservice-respons . I would argue that "text/xml" was correct since the output is really "readable by casual users". It is also not related to the bug from what I see

Copy link
Member Author

@dwnusbaum dwnusbaum Sep 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I changed it to make the testing logic simpler as JenkinsRule#createWebClient#goToXml requires application/xml (Maybe that method should also accept text/xml?). I will change it back and update the tests.

<Projects>
<j:forEach var="p" items="${it.view.items}">
<j:set var="lb" value="${p.lastCompletedBuild}"/>
<j:if test="${lb!=null}">
<Project name="${p.displayName}"
activity="${p.isBuilding() ? 'Building' : 'Sleeping'}"
lastBuildStatus="${it.toCCStatus(p)}"
lastBuildLabel="${lb.number}"
lastBuildTime="${lb.timestampString2}"
webUrl="${app.rootUrl}${p.url}"
/>
</j:if>
<j:forEach var="me" items="${it.items}">
<j:forEach var="p" items="${me.value}">
<j:set var="lb" value="${p.lastCompletedBuild}"/>
<j:if test="${lb!=null}">
<Project name="${me.key + p.displayName}"
activity="${p.isBuilding() ? 'Building' : 'Sleeping'}"
lastBuildStatus="${it.toCCStatus(p)}"
lastBuildLabel="${lb.number}"
lastBuildTime="${lb.timestampString2}"
webUrl="${app.rootUrl}${p.url}"
/>
</j:if>
</j:forEach>
</j:forEach>
</Projects>
</j:jelly>
</j:jelly>
98 changes: 98 additions & 0 deletions src/test/java/org/jenkinsci/plugins/ccxml/CCXMLActionTest.java
@@ -0,0 +1,98 @@
/*
* The MIT License
*
* Copyright 2017 CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.jenkinsci.plugins.ccxml;

import com.gargoylesoftware.htmlunit.xml.XmlPage;
import hudson.model.FreeStyleProject;
import hudson.model.Item;
import java.util.List;
import static org.junit.Assert.*;
import org.junit.Test;
import org.junit.Rule;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.MockFolder;

public class CCXMLActionTest {

@Rule
public JenkinsRule j = new JenkinsRule();

@Test
public void testGetItemsNonRecursive() throws Exception {
FreeStyleProject p1 = j.createFreeStyleProject("project1");
j.buildAndAssertSuccess(p1);

XmlPage xml = getPrimaryViewCCXMLPage();
assertXPathNodeCount(xml, getXPathForItem(p1), 1);

MockFolder f1 = j.createFolder("folder1");
FreeStyleProject p2 = f1.createProject(FreeStyleProject.class, "project2");
j.buildAndAssertSuccess(p2);
xml = getPrimaryViewCCXMLPage();
assertXPathNodeCount(xml, getXPathForItem(p1), 1);
assertXPathNodeCount(xml, getXPathForItem(p2), 0);
}

@Test
public void testGetItemsRecursive() throws Exception {
FreeStyleProject p1 = j.createFreeStyleProject("project1");
j.buildAndAssertSuccess(p1);

XmlPage xml = getPrimaryViewCCXMLPage("recursive");
assertXPathNodeCount(xml, getXPathForItem(p1), 1);

MockFolder f1 = j.createFolder("folder1");
FreeStyleProject p2 = f1.createProject(FreeStyleProject.class, "project2");
j.buildAndAssertSuccess(p2);
xml = getPrimaryViewCCXMLPage("recursive");
assertXPathNodeCount(xml, getXPathForItem(p1), 1);
assertXPathNodeCount(xml, getXPathForItem(f1, p2), 1);

MockFolder f2 = f1.createProject(MockFolder.class, "folder2");
FreeStyleProject p3 = f2.createProject(FreeStyleProject.class, "project3");
j.buildAndAssertSuccess(p3);
xml = getPrimaryViewCCXMLPage("recursive");
assertXPathNodeCount(xml, getXPathForItem(p1), 1);
assertXPathNodeCount(xml, getXPathForItem(f1, p2), 1);
assertXPathNodeCount(xml, getXPathForItem(f1, f2, p3), 1);
}

private XmlPage getPrimaryViewCCXMLPage(String... queryParameters) throws Exception {
return j.createWebClient().goToXml("view/all/" + CCXMLAction.URL_NAME + "/?" + String.join("&", queryParameters));
}

private void assertXPathNodeCount(XmlPage xml, String xPath, int expectedNodes) {
List<?> nodes = xml.getByXPath(xPath);
assertEquals("incorrect number of nodes in xpath", expectedNodes, nodes.size());
}

private String getXPathForItem(Item... pathToItem) {
String[] itemNames = new String[pathToItem.length];
for (int i = 0; i < pathToItem.length; i++) {
itemNames[i] = pathToItem[i].getDisplayName();
}
return "/Projects/Project[@name='" + String.join("/", itemNames) + "']";
}

}