[FIXED JENKINS-40252] Add an Iterable<T> that returns all items unsorted

core/src/main/java/hudson/DependencyRunner.java

@@ -59,7 +59,7 @@ public void run() {
             Set<AbstractProject> topLevelProjects = new HashSet<AbstractProject>();
             // Get all top-level projects
             LOGGER.fine("assembling top level projects");
-            for (AbstractProject p : Jenkins.getInstance().getAllItems(AbstractProject.class))
+            for (AbstractProject p : Jenkins.getInstance().allItems(AbstractProject.class))
                 if (p.getUpstreamProjects().size() == 0) {
                     LOGGER.fine("adding top level project " + p.getName());
                     topLevelProjects.add(p);

core/src/main/java/hudson/model/Computer.java

@@ -778,7 +778,7 @@ public List<AbstractProject> getTiedJobs() {
     }
 
     public RunList getBuilds() {
-    	return new RunList(Jenkins.getInstance().getAllItems(Job.class)).node(getNode());
+        return RunList.fromJobs(Jenkins.getInstance().allItems(Job.class)).node(getNode());
     }
 
     /**

core/src/main/java/hudson/model/DependencyGraph.java

@@ -91,7 +91,7 @@ public void build() {
         SecurityContext saveCtx = ACL.impersonate(ACL.SYSTEM);
         try {
             this.computationalData = new HashMap<Class<?>, Object>();
-            for( AbstractProject p : getAllProjects() )
+            for( AbstractProject p : Jenkins.getInstance().allItems(AbstractProject.class) )
                 p.buildDependencyGraph(this);
 
             forward = finalize(forward);
@@ -147,10 +147,6 @@ public int compare(AbstractProject<?,?> o1, AbstractProject<?,?> o2) {
         topologicallySorted = Collections.unmodifiableList(topologicallySorted);
     }
 
-    Collection<AbstractProject> getAllProjects() {
-        return Jenkins.getInstance().getAllItems(AbstractProject.class);
-    }
-
     /**
      * Special constructor for creating an empty graph
      */

core/src/main/java/hudson/model/Items.java

@@ -30,29 +30,30 @@
 import hudson.model.listeners.ItemListener;
 import hudson.remoting.Callable;
 import hudson.security.ACL;
+import hudson.security.ACLContext;
 import hudson.security.AccessControlled;
 import hudson.triggers.Trigger;
 import hudson.util.DescriptorList;
 import hudson.util.EditDistance;
 import hudson.util.XStream2;
-import jenkins.model.Jenkins;
-import org.acegisecurity.Authentication;
-import org.apache.commons.lang.StringUtils;
-
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.Iterator;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Stack;
 import java.util.StringTokenizer;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
-
 import jenkins.model.DirectlyModifiableTopLevelItemGroup;
+import jenkins.model.Jenkins;
+import org.acegisecurity.Authentication;
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang.StringUtils;
 
 /**
  * Convenience methods related to {@link Item}.
@@ -79,6 +80,44 @@ public class Items {
             return false;
         }
     };
+    /**
+     * A comparator of {@link Item} instances that uses a case-insensitive comparison of {@link Item#getName()}.
+     * If you are replacing {@link #getAllItems(ItemGroup, Class)} with {@link #allItems(ItemGroup, Class)} and
+     * need to restore the sort order of a further filtered result, you probably want {@link #BY_FULL_NAME}.
+     *
+     * @since FIXME
+     */
+    public static final Comparator<Item> BY_NAME = new Comparator<Item>() {
+        @Override public int compare(Item i1, Item i2) {
+            return name(i1).compareToIgnoreCase(name(i2));
+        }
+
+        String name(Item i) {
+            String n = i.getName();
+            if (i instanceof ItemGroup) {
+                n += '/';
+            }
+            return n;
+        }
+    };
+    /**
+     * A comparator of {@link Item} instances that uses a case-insensitive comparison of {@link Item#getFullName()}.
+     *
+     * @since FIXME
+     */
+    public static final Comparator<Item> BY_FULL_NAME = new Comparator<Item>() {
+        @Override public int compare(Item i1, Item i2) {
+            return name(i1).compareToIgnoreCase(name(i2));
+        }
+
+        String name(Item i) {
+            String n = i.getFullName();
+            if (i instanceof ItemGroup) {
+                n += '/';
+            }
+            return n;
+        }
+    };
 
     /**
      * Runs a block while making {@link #currentlyUpdatingByXml} be temporarily true.
@@ -350,7 +389,12 @@ public static XmlFile getConfigFile(Item item) {
 
     /**
      * Gets all the {@link Item}s recursively in the {@link ItemGroup} tree
-     * and filter them by the given type.
+     * and filter them by the given type. The returned list will represent a snapshot view of the items present at some
+     * time during the call. If items are moved during the call, depending on the move, it may be possible for some
+     * items to escape the snapshot entirely.
+     * <p>
+     * If you do not need to iterate all items, or if the order of the items is not required, consider using
+     * {@link #allItems(ItemGroup, Class)} instead.
      * 
      * @since 1.512
      */
@@ -361,18 +405,8 @@ public static <T extends Item> List<T> getAllItems(final ItemGroup root, Class<T
     }
     private static <T extends Item> void getAllItems(final ItemGroup root, Class<T> type, List<T> r) {
         List<Item> items = new ArrayList<Item>(((ItemGroup<?>) root).getItems());
-        Collections.sort(items, new Comparator<Item>() {
-            @Override public int compare(Item i1, Item i2) {
-                return name(i1).compareToIgnoreCase(name(i2));
-            }
-            String name(Item i) {
-                String n = i.getName();
-                if (i instanceof ItemGroup) {
-                    n += '/';
-                }
-                return n;
-            }
-        });
+        // because we add items depth first, we can use the quicker BY_NAME comparison
+        Collections.sort(items, BY_NAME);
         for (Item i : items) {
             if (type.isInstance(i)) {
                 if (i.hasPermission(Item.READ)) {
@@ -385,6 +419,41 @@ String name(Item i) {
         }
     }
 
+    /**
+     * Gets a read-only view of all the {@link Item}s recursively in the {@link ItemGroup} tree visible to
+     * {@link Jenkins#getAuthentication()} without concern for the order in which items are returned. Each iteration
+     * of the view will be "live" reflecting the items available between the time the iteration was started and the
+     * time the iteration was completed, however if items are moved during an iteration - depending on the move - it
+     * may be possible for such items to escape the entire iteration.
+     *
+     * @param root the root.
+     * @param type the type.
+     * @param <T> the type.
+     * @return An {@link Iterable} for all items.
+     * @since FIXME
+     */
+    public static <T extends Item> Iterable<T> allItems(ItemGroup root, Class<T> type) {
+        return allItems(Jenkins.getAuthentication(), root, type);
+    }
+
+
+    /**
+     * Gets a read-only view all the {@link Item}s recursively in the {@link ItemGroup} tree visible to the supplied
+     * authentication without concern for the order in which items are returned. Each iteration
+     * of the view will be "live" reflecting the items available between the time the iteration was started and the
+     * time the iteration was completed, however if items are moved during an iteration - depending on the move - it
+     * may be possible for such items to escape the entire iteration.
+     *
+     * @param root the root.
+     * @param type the type.
+     * @param <T> the type.
+     * @return An {@link Iterable} for all items.
+     * @since FIXME
+     */
+    public static <T extends Item> Iterable<T> allItems(Authentication authentication, ItemGroup root, Class<T> type) {
+        return new AllItemsIterable<>(root, authentication, type);
+    }
+
     /**
      * Finds an item whose name (when referenced from the specified context) is closest to the given name.
      * @param <T> the type of item being considered
@@ -395,10 +464,9 @@ String name(Item i) {
      * @since 1.538
      */
     public static @CheckForNull <T extends Item> T findNearest(Class<T> type, String name, ItemGroup context) {
-        List<T> projects = Jenkins.getInstance().getAllItems(type);
-        String[] names = new String[projects.size()];
-        for (int i = 0; i < projects.size(); i++) {
-            names[i] = projects.get(i).getRelativeNameFrom(context);
+        List<String> names = new ArrayList<>();
+        for (T item: Jenkins.getInstance().allItems(type)) {
+            names.add(item.getRelativeNameFrom(context));
         }
         String nearest = EditDistance.findNearest(name, names);
         return Jenkins.getInstance().getItem(nearest, context, type);
@@ -440,6 +508,121 @@ public static <I extends AbstractItem & TopLevelItem> I move(I item, DirectlyMod
         return newItem;
     }
 
+    private static class AllItemsIterable<T extends Item> implements Iterable<T> {
+
+        /**
+         * The authentication we are iterating as.
+         */
+        private final Authentication authentication;
+        /**
+         * The root we are iterating from.
+         */
+        private final ItemGroup root;
+        /**
+         * The type of item we want to return.
+         */
+        private final Class<T> type;
+
+        private AllItemsIterable(ItemGroup root, Authentication authentication, Class<T> type) {
+            this.root = root;
+            this.authentication = authentication;
+            this.type = type;
+        }
+
+        /**
+         * {@inheritDoc}
+         */
+        @Override
+        public Iterator<T> iterator() {
+            return new AllItemsIterator();
+        }
+
+        private class AllItemsIterator implements Iterator<T> {
+
+            /**
+             * The stack of {@link ItemGroup}s that we have left to descend into.
+             */
+            private final Stack<ItemGroup> stack = new Stack<>();
+            /**
+             * The iterator of the current {@link ItemGroup} we are iterating.
+             */
+            private Iterator<Item> delegate = null;
+            /**
+             * The next item.
+             */
+            private T next = null;
+
+            private AllItemsIterator() {
+                // put on the stack so that hasNext() is the only place that has to worry about authentication
+                // alternative would be to impersonate and populate delegate.
+                stack.push(root);
+            }
+
+            /**
+             * {@inheritDoc}
+             */
+            @Override
+            public void remove() {
+                throw new UnsupportedOperationException();
+            }
+
+            /**
+             * {@inheritDoc}
+             */
+            @Override
+            public boolean hasNext() {
+                if (next != null) {
+                    return true;
+                }
+                while (true) {
+                    if (delegate == null || !delegate.hasNext()) {
+                        if (stack.isEmpty()) {
+                            return false;
+                        }
+                        ItemGroup group = stack.pop();
+                        // group.getItems() is responsible for performing the permission check so we will not repeat it
+                        if (Jenkins.getAuthentication() == authentication) {
+                            delegate = group.getItems().iterator();
+                        } else {
+                            // slower path because the caller has switched authentication
+                            // we need to keep the original authentication so that allItems() can be used
+                            // like getAllItems() without the cost of building the entire list up front
+                            try (ACLContext ctx = ACL.as(authentication)) {
+                                delegate = group.getItems().iterator();
+                            }
+                        }
+                    }
+                    while (delegate.hasNext()) {
+                        Item item = delegate.next();
+                        if (item instanceof ItemGroup) {
+                            stack.push((ItemGroup) item);
+                        }
+                        if (type.isInstance(item)) {
+                            next = type.cast(item);
+                            return true;
+                        }
+                    }
+                }
+            }
+
+            /**
+             * {@inheritDoc}
+             */
+            @Override
+            public T next() {
+                if (!hasNext()) {
+                    throw new NoSuchElementException();
+                }
+                try {
+                    return next;
+                } finally {
+                    next = null;
+                }
+            }
+
+        }
+    }
+
     /**
      * Used to load/save job configuration.
      *

core/src/main/java/hudson/model/Label.java

@@ -362,10 +362,11 @@ private String toString(Collection<? extends ModelObject> model) {
     @Exported
     public List<AbstractProject> getTiedJobs() {
         List<AbstractProject> r = new ArrayList<AbstractProject>();
-        for (AbstractProject<?,?> p : Jenkins.getInstance().getAllItems(AbstractProject.class)) {
+        for (AbstractProject<?,?> p : Jenkins.getInstance().allItems(AbstractProject.class)) {
             if(p instanceof TopLevelItem && this.equals(p.getAssignedLabel()))
                 r.add(p);
         }
+        Collections.sort(r, Items.BY_FULL_NAME);
         return r;
     }
 

core/src/main/java/hudson/model/ListView.java

@@ -480,7 +480,7 @@ private void locationChanged(Item item, String oldFullName, String newFullName)
                     renameViewItem(oldFullName, newFullName, jenkins, (ListView) view);
                 }
             }
-            for (Item g : jenkins.getAllItems()) {
+            for (Item g : jenkins.allItems()) {
                 if (g instanceof ViewGroup) {
                     ViewGroup vg = (ViewGroup) g;
                     for (View v : vg.getViews()) {
@@ -524,7 +524,7 @@ private void deleted(Item item) {
                     deleteViewItem(item, jenkins, (ListView) view);
                 }
             }
-            for (Item g : jenkins.getAllItems()) {
+            for (Item g : jenkins.allItems()) {
                 if (g instanceof ViewGroup) {
                     ViewGroup vg = (ViewGroup) g;
                     for (View v : vg.getViews()) {

core/src/main/java/hudson/model/UsageStatistics.java

@@ -158,14 +158,22 @@ public String getStatData() throws IOException {
         o.put("plugins",plugins);
 
         JSONObject jobs = new JSONObject();
-        List<TopLevelItem> items = j.getAllItems(TopLevelItem.class);
-        for (TopLevelItemDescriptor d : Items.all()) {
-            int cnt=0;
-            for (TopLevelItem item : items) {
-                if(item.getDescriptor()==d)
-                    cnt++;
+        // capture the descriptors as these should be small compared with the number of items
+        // so we will walk all items only once and we can short-cut the search of descriptors
+        TopLevelItemDescriptor[] descriptors = Items.all().toArray(new TopLevelItemDescriptor[0]);
+        int counts[] = new int[descriptors.length];
+        for (TopLevelItem item: j.allItems(TopLevelItem.class)) {
+            TopLevelItemDescriptor d = item.getDescriptor();
+            for (int i = 0; i < descriptors.length; i++) {
+                if (d == descriptors[i]) {
+                    counts[i]++;
+                    // no point checking any more, we found the match
+                    break;
+                }
             }
-            jobs.put(d.getJsonSafeClassName(),cnt);
+        }
+        for (int i = 0; i < descriptors.length; i++) {
+            jobs.put(descriptors[i].getJsonSafeClassName(), counts[i]);
         }
         o.put("jobs",jobs);