[FIXED JENKINS-18680] Update members of a ListView using a regular li…

…stener, not View.onJobRenamed which does not work with folders.
jenkinsci · Dec 19, 2013 · 6d3c2e0 · daniel-beck · Apr 24, 2014 · jglick
1 parent a11df3f
commit 6d3c2e0
Show file tree

Hide file tree

Showing 11 changed files with 97 additions and 67 deletions.
diff --git a/core/src/main/java/hudson/model/AllView.java b/core/src/main/java/hudson/model/AllView.java
@@ -81,11 +81,6 @@ public String getPostConstructLandingPage() {
         return ""; // there's no configuration page
     }
 
-    @Override
-    public void onJobRenamed(Item item, String oldName, String newName) {
-        // noop
-    }
-
     @Override
     protected void submit(StaplerRequest req) throws IOException, ServletException, FormException {
         // noop

diff --git a/core/src/main/java/hudson/model/ItemGroup.java b/core/src/main/java/hudson/model/ItemGroup.java
@@ -83,7 +83,6 @@ public interface ItemGroup<T extends Item> extends PersistenceRoot, ModelObject
 
     /**
      * Internal method. Called by {@link Item}s when they are deleted by users.
-     * Should normally call {@link ItemListener#fireOnDeleted} and {@link View#onJobRenamed} (with a null {@code newName}).
      */
     void onDeleted(T item) throws IOException;
 }
diff --git a/core/src/main/java/hudson/model/ListView.java b/core/src/main/java/hudson/model/ListView.java
@@ -28,6 +28,7 @@
 import hudson.Util;
 import hudson.diagnosis.OldDataMonitor;
 import hudson.model.Descriptor.FormException;
+import hudson.model.listeners.ItemListener;
 import hudson.util.CaseInsensitiveComparator;
 import hudson.util.DescribableList;
 import hudson.util.FormValidation;
@@ -37,13 +38,18 @@
 
 import java.io.IOException;
 import java.util.*;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
 
 import javax.annotation.concurrent.GuardedBy;
 import javax.servlet.ServletException;
+import jenkins.model.Jenkins;
 
 import net.sf.json.JSONObject;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.HttpResponse;
@@ -302,12 +308,6 @@ public HttpResponse doRemoveJobFromView(@QueryParameter String name) throws IOEx
         return HttpResponses.ok();
     }
 
-    @Override
-    public synchronized void onJobRenamed(Item item, String oldName, String newName) {
-        if(jobNames.remove(oldName) && newName!=null)
-            jobNames.add(newName);
-    }
-
     /**
      * Handles the configuration submission.
      *
@@ -387,4 +387,56 @@ public FormValidation doCheckIncludeRegex( @QueryParameter String value ) throws
     public static List<ListViewColumn> getDefaultColumns() {
         return ListViewColumn.createDefaultInitialColumnList();
     }
+
+    @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()) {
+                if (g instanceof ViewGroup) {
+                    ViewGroup vg = (ViewGroup) g;
+                    for (View v : vg.getViews()) {
+                        if (v instanceof ListView) {
+                            ListView lv = (ListView) v;
+                            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()));
+                                }
+                                if (!oldJobNames.equals(lv.jobNames)) {
+                                    try {
+                                        g.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()) {
+                if (g instanceof ViewGroup) {
+                    ViewGroup vg = (ViewGroup) g;
+                    for (View v : vg.getViews()) {
+                        if (v instanceof ListView) {
+                            ListView lv = (ListView) v;
+                            synchronized (lv) {
+                                if (lv.jobNames.remove(item.getRelativeNameFrom(vg.getItemGroup()))) {
+                                    try {
+                                        g.save();
+                                    } catch (IOException x) {
+                                        Logger.getLogger(ListView.class.getName()).log(Level.WARNING, null, x);
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+
 }
diff --git a/core/src/main/java/hudson/model/MyView.java b/core/src/main/java/hudson/model/MyView.java
@@ -86,11 +86,6 @@ public String getPostConstructLandingPage() {
         return ""; // there's no configuration page
     }
 
-    @Override
-    public void onJobRenamed(Item item, String oldName, String newName) {
-        // noop
-    }
-
     @Override
     protected void submit(StaplerRequest req) throws IOException, ServletException, FormException {
         // noop

diff --git a/core/src/main/java/hudson/model/ProxyView.java b/core/src/main/java/hudson/model/ProxyView.java
@@ -89,13 +89,6 @@ public boolean contains(TopLevelItem item) {
         return getProxiedView().contains(item);
     }
 
-    @Override
-    public void onJobRenamed(Item item, String oldName, String newName) {
-        if (oldName.equals(proxiedViewName)) {
-            proxiedViewName = newName;
-        }
-    }
-
     @Override
     protected void submit(StaplerRequest req) throws IOException, ServletException, FormException {
         String proxiedViewName = req.getSubmittedForm().getString("proxiedViewName");

diff --git a/core/src/main/java/hudson/model/TreeView.java b/core/src/main/java/hudson/model/TreeView.java
@@ -119,14 +119,7 @@ public TopLevelItem doCreateItem(StaplerRequest req, StaplerResponse rsp) throws
         return null;
     }
 
-    @Override
-    public synchronized void onJobRenamed(Item item, String oldName, String newName) {
-        if(jobNames.remove(oldName) && newName!=null)
-            jobNames.add(newName);
-        // forward to children
-        for (View v : views)
-            v.onJobRenamed(item,oldName,newName);
-    }
+    // TODO listen for changes that might affect jobNames
 
     protected void submit(StaplerRequest req) throws IOException, ServletException, FormException {
     }

diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java
@@ -581,21 +581,9 @@ public boolean hasPermission(Permission p) {
         return getACL().hasPermission(p);
     }
 
-    /**
-     * Called when a job name is changed or deleted.
-     *
-     * <p>
-     * If this view contains this job, it should update the view membership so that
-     * the renamed job will remain in the view, and the deleted job is removed.
-     *
-     * @param item
-     *      The item whose name is being changed.
-     * @param oldName
-     *      Old name of the item. Always non-null.
-     * @param newName
-     *      New name of the item, if the item is renamed. Or null, if the item is removed.
-     */
-    public abstract void onJobRenamed(Item item, String oldName, String newName);
+    /** @deprecated Does not work properly with moved jobs. Use {@link ItemListener#onLocationChanged} instead. */
+    @Deprecated
+    public void onJobRenamed(Item item, String oldName, String newName) {}
 
     @ExportedBean(defaultVisibility=2)
     public static final class UserInfo implements Comparable<UserInfo> {

diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
@@ -2434,6 +2434,7 @@ public void onRenamed(TopLevelItem job, String oldName, String newName) throws I
         items.remove(oldName);
         items.put(newName,job);
 
+        // For compatibility with old views:
         for (View v : views)
             v.onJobRenamed(job, oldName, newName);
         save();
@@ -2446,6 +2447,7 @@ public void onDeleted(TopLevelItem item) throws IOException {
         ItemListener.fireOnDeleted(item);
 
         items.remove(item.getName());
+        // For compatibility with old views:
         for (View v : views)
             v.onJobRenamed(item, item.getName(), null);
         save();

diff --git a/core/src/test/java/hudson/model/ViewTest.java b/core/src/test/java/hudson/model/ViewTest.java
@@ -177,10 +177,6 @@ public boolean contains(TopLevelItem item) {
             return false;
         }
 
-        @Override
-        public void onJobRenamed(Item item, String oldName, String newName) {
-        }
-
         @Override
         protected void submit(StaplerRequest req) throws IOException, ServletException, FormException {
         }

diff --git a/test/src/main/java/org/jvnet/hudson/test/MockFolder.java b/test/src/main/java/org/jvnet/hudson/test/MockFolder.java
@@ -181,9 +181,6 @@ public <T extends TopLevelItem> T createProject(Class<T> type, String name) thro
     @Override public void onRenamed(TopLevelItem item, String oldName, String newName) throws IOException {
         items.remove(oldName);
         items.put(newName, item);
-        for (View v : views) {
-            v.onJobRenamed(item, oldName, newName);
-        }
     }
 
     @Override public void renameTo(String newName) throws IOException {
@@ -193,9 +190,6 @@ public <T extends TopLevelItem> T createProject(Class<T> type, String name) thro
     @Override public void onDeleted(TopLevelItem item) throws IOException {
         ItemListener.fireOnDeleted(item);
         items.remove(item.getName());
-        for (View v : views) {
-            v.onJobRenamed(item, item.getName(), null);
-        }
     }
 
     @Override public boolean canAdd(TopLevelItem item) {

diff --git a/test/src/test/java/hudson/model/ListViewTest.java b/test/src/test/java/hudson/model/ListViewTest.java
@@ -24,11 +24,17 @@
 
 package hudson.model;
 
-import static org.junit.Assert.*;
+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;
@@ -38,13 +44,6 @@
 import org.jvnet.hudson.test.recipes.LocalData;
 import org.xml.sax.SAXException;
 
-import com.gargoylesoftware.htmlunit.html.HtmlAnchor;
-import com.gargoylesoftware.htmlunit.html.HtmlPage;
-import hudson.matrix.AxisList;
-import hudson.matrix.MatrixProject;
-import hudson.matrix.TextAxis;
-import java.util.Collections;
-
 public class ListViewTest {
 
     @Rule public JenkinsRule j = new JenkinsRule();
@@ -112,4 +111,28 @@ private void checkLinkFromItemExistsAndIsValid(Item item, ItemGroup ig, Item top
         assertEquals(Collections.singletonList(mp), v.getItems());
     }
 
+    @Bug(18680)
+    @Test public void renamesMovesAndDeletes() throws Exception {
+        MockFolder top = j.createFolder("top");
+        MockFolder sub = top.createProject(MockFolder.class, "sub");
+        FreeStyleProject p1 = top.createProject(FreeStyleProject.class, "p1");
+        FreeStyleProject p2 = sub.createProject(FreeStyleProject.class, "p2");
+        FreeStyleProject p3 = top.createProject(FreeStyleProject.class, "p3");
+        ListView v = new ListView("v");
+        top.addView(v);
+        v.add(p1);
+        v.add(p2);
+        v.add(p3);
+        assertEquals(new HashSet<TopLevelItem>(Arrays.asList(p1, p2, p3)), new HashSet<TopLevelItem>(v.getItems()));
+        sub.renameTo("lower");
+        MockFolder stuff = top.createProject(MockFolder.class, "stuff");
+        Items.move(p1, stuff);
+        p3.delete();
+        Thread.sleep(500); // TODO working around crappy JENKINS-19446 fix
+        top.createProject(FreeStyleProject.class, "p3");
+        assertEquals(new HashSet<TopLevelItem>(Arrays.asList(p1, p2)), new HashSet<TopLevelItem>(v.getItems()));
+        top.renameTo("upper");
+        assertEquals(new HashSet<TopLevelItem>(Arrays.asList(p1, p2)), new HashSet<TopLevelItem>(v.getItems()));
+    }
+
 }