[JENKINS-22967] Fix item name semantics for DirectlyModifiableView we…

…b methods
jenkinsci · May 15, 2014 · 5f443a7 · 5f443a7
1 parent 21ec2a7
commit 5f443a7
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 7 deletions.
diff --git a/core/src/main/java/hudson/model/DirectlyModifiableView.java b/core/src/main/java/hudson/model/DirectlyModifiableView.java
@@ -62,18 +62,18 @@ public interface DirectlyModifiableView {
     /**
      * Handle addJobToView web method.
      *
-     * This method should @RequirePOST.
+     * This method should {@link RequirePOST}.
      *
-     * @param name Item name.
+     * @param name Item name. This can be either full name relative to owner item group or full item name prefixed with '/'.
      */
     HttpResponse doAddJobToView(@QueryParameter String name) throws IOException, ServletException;
 
     /**
      * Handle removeJobFromView web method.
      *
-     * This method should @RequirePOST.
+     * This method should {@link RequirePOST}.
      *
-     * @param name Item name.
+     * @param name Item name. This can be either full name relative to owner item group or full item name prefixed with '/'.
      */
     HttpResponse doRemoveJobFromView(@QueryParameter String name) throws IOException, ServletException;
 }
diff --git a/core/src/main/java/hudson/model/ListView.java b/core/src/main/java/hudson/model/ListView.java
@@ -312,7 +312,7 @@ public HttpResponse doAddJobToView(@QueryParameter String name) throws IOExcepti
         if(name==null)
             throw new Failure("Query parameter 'name' is required");
 
-        TopLevelItem item = getOwnerItemGroup().getItem(name);
+        TopLevelItem item = resolveName(name);
         if (item == null)
             throw new Failure("Query parameter 'name' does not correspond to a known item");
 
@@ -331,13 +331,22 @@ public HttpResponse doRemoveJobFromView(@QueryParameter String name) throws IOEx
         if(name==null)
             throw new Failure("Query parameter 'name' is required");
 
-        TopLevelItem item = getOwnerItemGroup().getItem(name);
+        TopLevelItem item = resolveName(name);
         if (remove(item))
             owner.save();
 
         return HttpResponses.ok();
     }
 
+    private TopLevelItem resolveName(String name) {
+        TopLevelItem item = getOwnerItemGroup().getItem(name);
+        if (item == null) {
+            name = Items.getCanonicalName(getOwnerItemGroup(), name);
+            item = Jenkins.getInstance().getItemByFullName(name, TopLevelItem.class);
+        }
+        return item;
+    }
+
     /**
      * Handles the configuration submission.
      *

diff --git a/test/src/test/java/hudson/model/DirectlyModifiableViewTest.java b/test/src/test/java/hudson/model/DirectlyModifiableViewTest.java
@@ -28,10 +28,14 @@
 import java.io.IOException;
 import java.net.URL;
 
+import javax.annotation.Nonnull;
+
+import org.hamcrest.Matchers;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.JenkinsRule.WebClient;
+import org.jvnet.hudson.test.MockFolder;
 
 import com.gargoylesoftware.htmlunit.HttpMethod;
 import com.gargoylesoftware.htmlunit.Page;
@@ -89,6 +93,51 @@ public void doAddJobToView() throws Exception {
         assertTrue(view.contains(project));
     }
 
+    @Test
+    public void doAddNestedJobToRecursiveView() throws Exception {
+        ListView view = new ListView("a_view", j.jenkins);
+        view.setRecurse(true);
+        j.jenkins.addView(view);
+
+        MockFolder folder = j.createFolder("folder");
+        FreeStyleProject np = folder.createProject(FreeStyleProject.class, "nested_project");
+
+        view.add(np);
+        assertTrue(view.contains(np));
+        view.remove(np);
+        assertFalse(view.contains(np));
+
+        Page page = doPost(view, "addJobToView?name=folder/nested_project");
+        j.assertGoodStatus(page);
+        assertTrue(view.contains(np));
+
+        page = doPost(view, "removeJobFromView?name=folder/nested_project");
+        j.assertGoodStatus(page);
+        assertFalse(view.contains(np));
+
+        MockFolder nf = folder.createProject(MockFolder.class, "nested_folder");
+        FreeStyleProject nnp = nf.createProject(FreeStyleProject.class, "nested_nested_project");
+        ListView nestedView = new ListView("nested_view", folder);
+        nestedView.setRecurse(true);
+        folder.addView(nestedView);
+
+        page = doPost(nestedView, "addJobToView?name=nested_folder/nested_nested_project");
+        j.assertGoodStatus(page);
+        assertTrue(nestedView.contains(nnp));
+
+        page = doPost(nestedView, "removeJobFromView?name=nested_folder/nested_nested_project");
+        j.assertGoodStatus(page);
+        assertFalse(nestedView.contains(nnp));
+
+        page = doPost(nestedView, "addJobToView?name=/folder/nested_folder/nested_nested_project");
+        j.assertGoodStatus(page);
+        assertTrue(nestedView.contains(nnp));
+
+        page = doPost(nestedView, "removeJobFromView?name=/folder/nested_folder/nested_nested_project");
+        j.assertGoodStatus(page);
+        assertFalse(nestedView.contains(nnp));
+    }
+
     @Test
     public void doRemoveJobFromView() throws Exception {
         FreeStyleProject project = j.createFreeStyleProject("a_project");
@@ -124,6 +173,15 @@ public void failWebMethodForIllegalRequest() throws Exception {
                 doPost(view, "removeJobFromView"),
                 "Query parameter 'name' is required"
         );
+
+        MockFolder folder = j.createFolder("folder");
+        ListView folderView = new ListView("folder_view", folder);
+        folder.addView(folderView);
+
+        assertBadStatus( // Item is scoped to different ItemGroup
+                doPost(folderView, "addJobToView?name=top_project"),
+                "Query parameter 'name' does not correspond to a known item"
+        );
     }
 
     private Page doPost(View view, String path) throws Exception {
@@ -140,6 +198,6 @@ private Page doPost(View view, String path) throws Exception {
     private void assertBadStatus(Page page, String message) {
         WebResponse rsp = page.getWebResponse();
         assertFalse("Status: " + rsp.getStatusCode(), j.isGoodHttpStatus(rsp.getStatusCode()));
-        assertTrue(rsp.getContentAsString().contains(message));
+        assertThat(rsp.getContentAsString(), Matchers.containsString(message));
     }
 }