Merge pull request #1650 from tulinkry/groups-ordering

displaying the subgroups in ascending order

Author: tulinkry

src/org/opensolaris/opengrok/configuration/Groups.java

@@ -40,11 +40,15 @@
  */
 public final class Groups {
 
+    /**
+     * Interface used to perform an action to a single group.
+     */
     private interface Walker {
 
         /**
          * @param g group
-         * @return true if traversing should end, false otherwise
+         * @return true if traversing should stop just after this group, false
+         * otherwise
          */
         boolean call(Group g);
     }
@@ -326,34 +330,54 @@ private static void deleteGroup(Set<Group> groups, String groupname) {
         }
     }
 
-    private static boolean treeTraverseGroups(Set<Group> groups, Walker f) {
+    /**
+     * Traverse the set of groups starting in top level groups (groups without a
+     * parent group) and performing depth first search in the group's subgroups.
+     *
+     * @param groups set of groups (mixed top level and other groups)
+     * @param walker an instance of {@link Walker} which is used for every
+     * traversed group
+     * @return true if {@code walker} emits true for any of the groups; false
+     * otherwise
+     *
+     * @see Walker
+     */
+    private static boolean treeTraverseGroups(Set<Group> groups, Walker walker) {
         LinkedList<Group> stack = new LinkedList<>();
         for (Group g : groups) {
+            // the flag here represents the group's depth in the group tree
             g.setFlag(0);
             if (g.getParent() == null) {
-                stack.add(g);
+                stack.addLast(g);
             }
         }
 
         while (!stack.isEmpty()) {
             Group g = stack.getFirst();
             stack.removeFirst();
 
-            if (f.call(g)) {
+            if (walker.call(g)) {
                 return true;
             }
 
-            for (Group x : g.getSubgroups()) {
-                x.setFlag(g.getFlag() + 1);
-                stack.addFirst(x);
-            }
+            g.getSubgroups().forEach((x) -> x.setFlag(g.getFlag() + 1));
+            // add all the subgroups respecting the sorted order
+            stack.addAll(0, g.getSubgroups());
         }
         return false;
     }
 
-    private static boolean linearTraverseGroups(Set<Group> groups, Walker f) {
+    /**
+     * Traverse the set of groups linearly based on the set's iterator.
+     *
+     * @param groups set of groups (mixed top level and other groups)
+     * @param walker an instance of {@link Walker} which is used for every
+     * traversed group
+     * @return true if {@code walker} emits true for any of the groups; false
+     */
+    private static boolean linearTraverseGroups(Set<Group> groups, Walker walker) {
         for (Group g : groups) {
-            if (f.call(g)) {
+            if (walker.call(g)) {
                 return true;
             }
         }

web/repos.jspf

@@ -21,11 +21,12 @@ CDDL HEADER END
 Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
 
 --%>
+<%@page import="java.util.TreeSet"%>
+<%@page import="java.util.Iterator"%>
 <%@page import="org.opensolaris.opengrok.web.Scripts"%>
 <%@page import="org.json.simple.JSONArray"%>
 <%@page import="org.opensolaris.opengrok.configuration.messages.Message"%>
 <%@page import="java.util.SortedSet"%>
-<%@page import="java.util.SortedSet"%>
 <%@page import="java.util.Set"%>
 <%@page import="java.text.ParseException"%>
 <%@page import="java.util.ArrayList"%>
@@ -80,6 +81,7 @@ Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
             LinkedList<Group> stack = new LinkedList<>();
             for ( Group x : groups ) {
                 if (x.getParent() == null && (pHelper.hasAllowedSubgroup(x) || cfg.isAllowed(x))) {
+                    // the flag here represents the state of the group - open/close
                     x.setFlag(0);
                     stack.addLast(x);
                 }
@@ -97,14 +99,19 @@ Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
                 }
 
                 stack.element().setFlag(1);
-                
-                for (Group x : pHelper.getSubgroups(group)) {
+
+                Set<Group> subgroups = new TreeSet<>(pHelper.getSubgroups(group));
+                for (Iterator<Group> it = subgroups.iterator(); it.hasNext();) {
+                    Group x = it.next();
                     if (cfg.isAllowed(x) || pHelper.hasAllowedSubgroup(x)) {
                         x.setFlag(0);
-                        stack.addFirst(x);
+                    } else {
+                        it.remove();
                     }
                 }
-                
+                // add all the subgroups to the beginning respecting the order
+                stack.addAll(0, subgroups);
+
                 %><div class="panel">
                     <div class="panel-heading-accordion">