[FIXED JENKINS-23893] Top level ListView is not updated when job is r…

…enamed/deleted (cherry picked from commit d74520f)
jenkinsci · Sep 7, 2014 · b49e132 · b49e132
1 parent 65d34a5
commit b49e132
Show file tree

Hide file tree

Showing 2 changed files with 120 additions and 31 deletions.
diff --git a/core/src/main/java/hudson/model/ListView.java b/core/src/main/java/hudson/model/ListView.java
@@ -398,56 +398,75 @@ public static List<ListViewColumn> getDefaultColumns() {
     @Restricted(NoExternalUse.class)
     @Extension public static final class Listener extends ItemListener {
         @Override public void onLocationChanged(Item item, String oldFullName, String newFullName) {
-            for (Item g : Jenkins.getInstance().getAllItems()) {
+            final Jenkins jenkins = Jenkins.getInstance();
+            for (View view: jenkins.getViews()) {
+                if (view instanceof ListView) {
+                    renameViewItem(oldFullName, newFullName, jenkins, (ListView) view);
+                }
+            }
+            for (Item g : jenkins.getAllItems()) {
                 if (g instanceof ViewGroup) {
                     ViewGroup vg = (ViewGroup) g;
                     for (View v : vg.getViews()) {
                         if (v instanceof ListView) {
-                            ListView lv = (ListView) v;
-                            boolean needsSave;
-                            synchronized (lv) {
-                                Set<String> oldJobNames = new HashSet<String>(lv.jobNames);
-                                lv.jobNames.clear();
-                                for (String oldName : oldJobNames) {
-                                    lv.jobNames.add(Items.computeRelativeNamesAfterRenaming(oldFullName, newFullName, oldName, vg.getItemGroup()));
-                                }
-                                needsSave = !oldJobNames.equals(lv.jobNames);
-                            }
-                            if (needsSave) { // do not hold ListView lock at the time
-                                try {
-                                    g.save();
-                                } catch (IOException x) {
-                                    Logger.getLogger(ListView.class.getName()).log(Level.WARNING, null, x);
-                                }
-                            }
+                            renameViewItem(oldFullName, newFullName, vg, (ListView) v);
                         }
                     }
                 }
             }
         }
+
+        private void renameViewItem(String oldFullName, String newFullName, ViewGroup vg, ListView lv) {
+            boolean needsSave;
+            synchronized (lv) {
+                Set<String> oldJobNames = new HashSet<String>(lv.jobNames);
+                lv.jobNames.clear();
+                for (String oldName : oldJobNames) {
+                    lv.jobNames.add(Items.computeRelativeNamesAfterRenaming(oldFullName, newFullName, oldName, vg.getItemGroup()));
+                }
+                needsSave = !oldJobNames.equals(lv.jobNames);
+            }
+            if (needsSave) { // do not hold ListView lock at the time
+                try {
+                    lv.save();
+                } catch (IOException x) {
+                    Logger.getLogger(ListView.class.getName()).log(Level.WARNING, null, x);
+                }
+            }
+        }
+
         @Override public void onDeleted(Item item) {
-            for (Item g : Jenkins.getInstance().getAllItems()) {
+            final Jenkins jenkins = Jenkins.getInstance();
+            for (View view: jenkins.getViews()) {
+                if (view instanceof ListView) {
+                    deleteViewItem(item, jenkins, (ListView) view);
+                }
+            }
+            for (Item g : jenkins.getAllItems()) {
                 if (g instanceof ViewGroup) {
                     ViewGroup vg = (ViewGroup) g;
                     for (View v : vg.getViews()) {
                         if (v instanceof ListView) {
-                            ListView lv = (ListView) v;
-                            boolean needsSave;
-                            synchronized (lv) {
-                                needsSave = lv.jobNames.remove(item.getRelativeNameFrom(vg.getItemGroup()));
-                            }
-                            if (needsSave) {
-                                try {
-                                    g.save();
-                                } catch (IOException x) {
-                                    Logger.getLogger(ListView.class.getName()).log(Level.WARNING, null, x);
-                                }
-                            }
+                            deleteViewItem(item, vg, (ListView) v);
                         }
                     }
                 }
             }
         }
+
+        private void deleteViewItem(Item item, ViewGroup vg, ListView lv) {
+            boolean needsSave;
+            synchronized (lv) {
+                needsSave = lv.jobNames.remove(item.getRelativeNameFrom(vg.getItemGroup()));
+            }
+            if (needsSave) {
+                try {
+                    lv.save();
+                } catch (IOException x) {
+                    Logger.getLogger(ListView.class.getName()).log(Level.WARNING, null, x);
+                }
+            }
+        }
     }
 
 }
diff --git a/test/src/test/java/hudson/model/ListViewTest.java b/test/src/test/java/hudson/model/ListViewTest.java
@@ -26,15 +26,19 @@
 
 import com.gargoylesoftware.htmlunit.html.HtmlAnchor;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
+
 import hudson.Functions;
 import hudson.matrix.AxisList;
 import hudson.matrix.MatrixProject;
 import hudson.matrix.TextAxis;
+
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
+
 import static org.junit.Assert.*;
+
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Bug;
@@ -135,4 +139,70 @@ private void checkLinkFromItemExistsAndIsValid(Item item, ItemGroup ig, Item top
         assertEquals(new HashSet<TopLevelItem>(Arrays.asList(p1, p2)), new HashSet<TopLevelItem>(v.getItems()));
     }
 
+    @Bug(23893)
+    @Test public void renameJobContainedInTopLevelView() throws Exception {
+        ListView view = new ListView("view", j.jenkins);
+        j.jenkins.addView(view);
+        FreeStyleProject job = j.createFreeStyleProject("old_name");
+        view.add(job);
+
+        assertTrue(view.contains(job));
+        assertTrue(view.jobNamesContains(job));
+
+        job.renameTo("new_name");
+
+        assertFalse("old job name is still contained: " + view.jobNames, view.jobNames.contains("old_name"));
+        assertTrue(view.contains(job));
+        assertTrue(view.jobNamesContains(job));
+    }
+
+    @Test public void renameContainedJob() throws Exception {
+        MockFolder folder = j.createFolder("folder");
+        ListView view = new ListView("view", folder);
+        folder.addView(view);
+
+        FreeStyleProject job = folder.createProject(FreeStyleProject.class, "old_name");
+        view.add(job);
+
+        assertTrue(view.contains(job));
+        assertTrue(view.jobNamesContains(job));
+
+        job.renameTo("new_name");
+
+        assertFalse("old job name is still contained", view.jobNames.contains("old_name"));
+        assertTrue(view.contains(job));
+        assertTrue(view.jobNamesContains(job));
+    }
+
+    @Bug(23893)
+    @Test public void deleteJobContainedInTopLevelView() throws Exception {
+        ListView view = new ListView("view", j.jenkins);
+        j.jenkins.addView(view);
+        FreeStyleProject job = j.createFreeStyleProject("project");
+        view.add(job);
+
+        assertTrue(view.contains(job));
+        assertTrue(view.jobNamesContains(job));
+
+        job.delete();
+
+        assertFalse(view.contains(job));
+        assertFalse(view.jobNamesContains(job));
+    }
+
+    @Test public void deleteContainedJob() throws Exception {
+        MockFolder folder = j.createFolder("folder");
+        ListView view = new ListView("view", folder);
+        folder.addView(view);
+        FreeStyleProject job = folder.createProject(FreeStyleProject.class, "project");
+        view.add(job);
+
+        assertTrue(view.contains(job));
+        assertTrue(view.jobNamesContains(job));
+
+        job.delete();
+
+        assertFalse(view.contains(job));
+        assertFalse(view.jobNamesContains(job));
+    }
 }