Cleanup of group caching logic.

git-svn-id: http://svn.igniterealtime.org/svn/repos/wildfire/trunk@5249 b35dd754-fafc-0310-a699-88a17e54d16e

src/java/org/jivesoftware/wildfire/container/PluginManager.java

@@ -80,7 +80,7 @@ public class PluginManager {
     public void start() {
         executor = new ScheduledThreadPoolExecutor(1);
         // See if we're in development mode. If so, check for new plugins once every 5 seconds.
-        // Otherwise, default to every 30 seconds.
+        // Otherwise, default to every 20 seconds.
         if (Boolean.getBoolean("developmentMode")) {
             executor.scheduleWithFixedDelay(pluginMonitor, 1, 5, TimeUnit.SECONDS);
         }
@@ -817,8 +817,10 @@ public class PluginManager {
                             // Ask the system to clean up references.
                             System.gc();
                         }
-                        // Now unzip the plugin.
-                        unzipPlugin(pluginName, jarFile, dir);
+                        // If the delete operation was a success, unzip the plugin.
+                        if (count != 5) {
+                            unzipPlugin(pluginName, jarFile, dir);
+                        }
                     }
                 }
 
@@ -969,6 +971,7 @@ public class PluginManager {
                 for (String file : children) {
                     boolean success = deleteDir(new File(dir, file));
                     if (!success) {
+                        System.out.println("could not delete: " + new File(dir, file));
                         return false;
                     }
                 }

src/java/org/jivesoftware/wildfire/group/DefaultGroupProvider.java

@@ -261,7 +261,7 @@ public class DefaultGroupProvider implements GroupProvider {
         return count;
     }
 
-    public Collection<Group> getGroups() {
+    public Collection<String> getGroupNames() {
         List<String> groupNames = new ArrayList<String>();
         Connection con = null;
         PreparedStatement pstmt = null;
@@ -289,14 +289,10 @@ public class DefaultGroupProvider implements GroupProvider {
                 } }
             catch (Exception e) { Log.error(e); }
         }
-        return new GroupCollection(groupNames.toArray(new String[groupNames.size()]));
+        return groupNames;
     }
 
-    public Collection<Group> getGroups(Set<String> groupNames) {
-        return new GroupCollection(groupNames.toArray(new String[groupNames.size()]));
-    }
-
-    public Collection<Group> getGroups(int startIndex, int numResults) {
+    public Collection<String> getGroupNames(int startIndex, int numResults) {
         List<String> groupNames = new ArrayList<String>();
         Connection con = null;
         PreparedStatement pstmt = null;
@@ -327,10 +323,10 @@ public class DefaultGroupProvider implements GroupProvider {
                 } }
             catch (Exception e) { Log.error(e); }
         }
-        return new GroupCollection(groupNames.toArray(new String[groupNames.size()]));
+        return groupNames;
     }
 
-    public Collection<Group> getGroups(JID user) {
+    public Collection<String> getGroupNames(JID user) {
         List<String> groupNames = new ArrayList<String>();
         Connection con = null;
         PreparedStatement pstmt = null;
@@ -351,15 +347,17 @@ public class DefaultGroupProvider implements GroupProvider {
             try {
                 if (pstmt != null) {
                     pstmt.close();
-                } }
+                }
+            }
             catch (Exception e) { Log.error(e); }
             try {
                 if (con != null) {
                     con.close();
-                } }
+                }
+            }
             catch (Exception e) { Log.error(e); }
         }
-        return new GroupCollection(groupNames.toArray(new String[groupNames.size()]));
+        return groupNames;
     }
 
     public void addMember(String groupName, JID user, boolean administrator) {
@@ -449,7 +447,7 @@ public class DefaultGroupProvider implements GroupProvider {
         return false;
     }
 
-    public Collection<Group> search(String query) {
+    public Collection<String> search(String query) {
         if (query == null || "".equals(query)) {
             return Collections.emptyList();
         }
@@ -483,10 +481,10 @@ public class DefaultGroupProvider implements GroupProvider {
         finally {
             DbConnectionManager.closeConnection(rs, stmt, con);
         }
-        return new GroupCollection(groupNames.toArray(new String[groupNames.size()]));
+        return groupNames;
     }
 
-    public Collection<Group> search(String query, int startIndex, int numResults) {
+    public Collection<String> search(String query, int startIndex, int numResults) {
         if (query == null || "".equals(query)) {
             return Collections.emptyList();
         }
@@ -524,7 +522,7 @@ public class DefaultGroupProvider implements GroupProvider {
         finally {
             DbConnectionManager.closeConnection(rs, stmt, con);
         }
-        return new GroupCollection(groupNames.toArray(new String[groupNames.size()]));
+        return groupNames;
     }
 
     public boolean isSearchSupported() {

src/java/org/jivesoftware/wildfire/group/Group.java

@@ -67,6 +67,7 @@ public class Group implements Cacheable {
      * @return the name of the groups that are shared groups.
      */
     static Set<String> getSharedGroupsNames() {
+        // TODO: add caching
         Set<String> groupNames = new HashSet<String>();
         Connection con = null;
         PreparedStatement pstmt = null;

src/java/org/jivesoftware/wildfire/group/GroupCollection.java

@@ -13,6 +13,7 @@ package org.jivesoftware.wildfire.group;
 import java.util.AbstractCollection;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
+import java.util.Collection;
 
 /**
  * Provides a view of an array of group names as a Collection of Group objects. If
@@ -25,6 +26,13 @@ public class GroupCollection extends AbstractCollection {
 
     private String[] elements;
 
+    /**
+     * Constructs a new GroupCollection.
+     */
+    public GroupCollection(Collection<String> collection) {
+        this.elements = collection.toArray(new String[collection.size()]);
+    }
+
     /**
      * Constructs a new GroupCollection.
      */

src/java/org/jivesoftware/wildfire/group/GroupManager.java

@@ -30,10 +30,6 @@ public class GroupManager {
 
     Cache<String, Group> groupCache;
     Cache<String, Collection<Group>> userGroupCache;
-    // Holds the cache key for the global group in the users groups cache
-    private final String globalGroupKey = "ALL GROUPS";
-    // Holds the cache key for the shared groups in the users groups cache
-    private final String sharedGroupsKey = "SHARED GROUPS";
     private GroupProvider provider;
 
     private static GroupManager instance = new GroupManager();
@@ -140,7 +136,7 @@ public class GroupManager {
         if (group == null) {
             synchronized (name.intern()) {
                 group = groupCache.get(name);
-                // If ID wan't found in cache, load it up and put it there.
+                // If group wan't found in cache, load it up and put it there.
                 if (group == null) {
                     group = provider.getGroup(name);
                     groupCache.put(name, group);
@@ -179,13 +175,18 @@ public class GroupManager {
         JID userJID = XMPPServer.getInstance().createJID(user.getUsername(), null);
         for (Group group : getGroups(userJID)) {
             if (group.getAdmins().contains(userJID)) {
-                group.getAdmins().remove(userJID);
+                if (group.getAdmins().remove(userJID)) {
+                    // Remove the group from cache.
+                    groupCache.remove(group.getName());
+                }
             }
             else {
-                group.getMembers().remove(userJID);
+                if (group.getMembers().remove(userJID)) {
+                    // Remove the group from cache.
+                    groupCache.remove(group.getName());
+                }
             }
         }
-        userGroupCache.clear();
     }
 
     /**
@@ -194,11 +195,8 @@ public class GroupManager {
      * @return the total number of groups.
      */
     public int getGroupCount() {
-        Collection<Group> groups = userGroupCache.get(globalGroupKey);
-        if(groups == null) {
-            return provider.getGroupCount();
-        }
-        return groups.size();
+        // TODO: add caching.
+        return provider.getGroupCount();
     }
 
     /**
@@ -207,16 +205,9 @@ public class GroupManager {
      * @return an unmodifiable Collection of all groups.
      */
     public Collection<Group> getGroups() {
-        synchronized (globalGroupKey) {
-            Collection<Group> groups = userGroupCache.get(globalGroupKey);
-            if (groups == null) {
-                groups = provider.getGroups();
-                // Add to cache and ensure correct identity
-                groups = cacheAndEnsureIdentity(groups);
-                userGroupCache.put(globalGroupKey, groups);
-            }
-            return groups;
-        }
+        // TODO: add caching.
+        Collection<String> groupNames = provider.getGroupNames();
+        return new GroupCollection(groupNames);
     }
 
     /**
@@ -225,38 +216,25 @@ public class GroupManager {
      * @return an unmodifiable Collection of all shared groups.
      */
     public Collection<Group> getSharedGroups() {
-        synchronized (sharedGroupsKey) {
-            Collection<Group> groups = userGroupCache.get(sharedGroupsKey);
-            if (groups == null) {
-                Set<String> groupsNames = Group.getSharedGroupsNames();
-                groups = provider.getGroups(groupsNames);
-                // Add to cache and ensure correct identity
-                groups = cacheAndEnsureIdentity(groups);
-                userGroupCache.put(sharedGroupsKey, groups);
-            }
-            return groups;
-        }
+        Collection<String> groupNames = Group.getSharedGroupsNames();
+        return new GroupCollection(groupNames);
     }
 
     /**
-     * Returns an iterator for all groups according to a filter.
-     * <p/>
-     * This is useful to support
-     * pagination in a GUI where you may only want to display a certain
-     * number of results per page. It is possible that the
-     * number of results returned will be less than that specified by
-     * numResults if numResults is greater than the number of records left in
-     * the system to display.
+     * Returns all groups given a start index and desired number of results. This is
+     * useful to support pagination in a GUI where you may only want to display a certain
+     * number of results per page. It is possible that the number of results returned will
+     * be less than that specified by numResults if numResults is greater than the number
+     * of records left in the system to display.
      *
      * @param startIndex start index in results.
      * @param numResults number of results to return.
      * @return an Iterator for all groups in the specified range.
      */
     public Collection<Group> getGroups(int startIndex, int numResults) {
-        Collection<Group> groups = provider.getGroups(startIndex, numResults);
-        // Add to cache and ensure correct identity
-        groups = cacheAndEnsureIdentity(groups);
-        return groups;
+        // TODO: add caching
+        Collection<String> groupNames = provider.getGroupNames(startIndex, numResults);
+        return new GroupCollection(groupNames);
     }
 
     /**
@@ -276,17 +254,9 @@ public class GroupManager {
      * @return all groups that an entity belongs to.
      */
     public Collection<Group> getGroups(JID user) {
-        String userID = user.toString();
-        synchronized (userID.intern()) {
-            Collection<Group> groups = userGroupCache.get(userID);
-            if (groups == null) {
-                groups = provider.getGroups(user);
-                // Add to cache and ensure correct identity
-                groups = cacheAndEnsureIdentity(groups);
-                userGroupCache.put(userID, groups);
-            }
-            return groups;
-        }
+        // TODO: add caching
+        Collection<String> groupNames = provider.getGroupNames(user);
+        return new GroupCollection(groupNames);
     }
 
     /**
@@ -320,7 +290,8 @@ public class GroupManager {
      * @return all groups that match the search.
      */
     public Collection<Group> search(String query) {
-        return provider.search(query);
+        Collection<String> groupNames = provider.search(query);
+        return new GroupCollection(groupNames);
     }
 
     /**
@@ -337,7 +308,8 @@ public class GroupManager {
      * @return all groups that match the search.
      */
     public Collection<Group> search(String query, int startIndex, int numResults) {
-        return provider.search(query, startIndex, numResults);
+        Collection<String> groupNames = provider.search(query, startIndex, numResults);
+        return new GroupCollection(groupNames);
     }
 
     /**
@@ -346,33 +318,7 @@ public class GroupManager {
      *
      * @return the group provider.
      */
-    GroupProvider getProvider() {
+    public GroupProvider getProvider() {
         return provider;
     }
-
-    /**
-     * Caches groups present in the specified collection that are not already cached and
-     * ensures correct identity of already cached groups.
-     *
-     * @param groups loaded groups from the backend store.
-     * @return a list containing the correct and valid groups (i.e. ensuring identity).
-     */
-    private Collection<Group> cacheAndEnsureIdentity(Collection<Group> groups) {
-        Collection<Group> answer = new ArrayList<Group>(groups.size());
-        for (Group group : groups) {
-            synchronized (group.getName().intern()) {
-                Group existingGroup = groupCache.get(group.getName());
-                if (existingGroup == null) {
-                    // Add loaded group to the cache
-                    groupCache.put(group.getName(), group);
-                    answer.add(group);
-                }
-                else {
-                    // Replace loaded group with the cached one to ensure correct identity
-                    answer.add(existingGroup);
-                }
-            }
-        }
-        return answer;
-    }
 }
\ No newline at end of file

src/java/org/jivesoftware/wildfire/group/GroupProvider.java

@@ -14,7 +14,6 @@ package org.jivesoftware.wildfire.group;
 import org.xmpp.packet.JID;
 
 import java.util.Collection;
-import java.util.Set;
 
 /**
  * Provider interface for groups. Users that wish to integrate with
@@ -98,35 +97,29 @@ public interface GroupProvider {
     int getGroupCount();
 
     /**
-     * Returns the Collection of all groups in the system.
+     * Returns the Collection of all group names in the system.
      *
      * @return the Collection of all groups.
      */
-    Collection<Group> getGroups();
-
-    /**
-     * Returns the Collection of groups for the specified groups names.
-     *
-     * @return the Collection with the requested groups.
-     */
-    Collection<Group> getGroups(Set<String> groupNames);
+    Collection<String> getGroupNames();
 
     /**
      * Returns the Collection of all groups in the system.
      *
      * @param startIndex start index in results.
      * @param numResults number of results to return.
-     * @return the Collection of all groups given the <tt>startIndex</tt> and <tt>numResults</tt>.
+     * @return the Collection of all group names given the
+     *      <tt>startIndex</tt> and <tt>numResults</tt>.
      */
-    Collection<Group> getGroups(int startIndex, int numResults);
+    Collection<String> getGroupNames(int startIndex, int numResults);
 
     /**
-     * Returns the Collection of Groups that an entity belongs to.
+     * Returns the Collection of group names that an entity belongs to.
      *
      * @param user the JID of the entity.
-     * @return the Collection of groups that the user belongs to.
+     * @return the Collection of group names that the user belongs to.
      */
-    Collection<Group> getGroups(JID user);
+    Collection<String> getGroupNames(JID user);
 
     /**
      * Adds an entity to a group (optional operation).
@@ -171,7 +164,7 @@ public interface GroupProvider {
     boolean isReadOnly();
 
     /**
-     * Returns the groups that match the search. The search is over group names and
+     * Returns the group names that match a search. The search is over group names and
      * implicitly uses wildcard matching (although the exact search semantics are left
      * up to each provider implementation). For example, a search for "HR" should match
      * the groups "HR", "HR Department", and "The HR People".<p>
@@ -182,10 +175,10 @@ public interface GroupProvider {
      * @param query the search string for group names.
      * @return all groups that match the search.
      */
-    Collection<Group> search(String query);
+    Collection<String> search(String query);
 
     /**
-     * Returns the groups that match the search given a start index and desired number of results.
+     * Returns the group names that match a search given a start index and desired number of results.
      * The search is over group names and implicitly uses wildcard matching (although the
      * exact search semantics are left up to each provider implementation). For example, a
      * search for "HR" should match the groups "HR", "HR Department", and "The HR People".<p>
@@ -196,7 +189,7 @@ public interface GroupProvider {
      * @param query the search string for group names.
      * @return all groups that match the search.
      */
-    Collection<Group> search(String query, int startIndex, int numResults);
+    Collection<String> search(String query, int startIndex, int numResults);
 
     /**
      * Returns true if group searching is supported by the provider.

src/java/org/jivesoftware/wildfire/group/JDBCGroupProvider.java

@@ -227,7 +227,7 @@ public class JDBCGroupProvider implements GroupProvider {
         return count;
     }
 
-    public Collection<Group> getGroups() {
+    public Collection<String> getGroupNames() {
         List<String> groupNames = new ArrayList<String>();
         Connection con = null;
         PreparedStatement pstmt = null;
@@ -246,32 +246,10 @@ public class JDBCGroupProvider implements GroupProvider {
         finally {
             DbConnectionManager.closeConnection(rs, pstmt, con);
         }
-        List<Group> groups = new ArrayList<Group>(groupNames.size());
-        for (String groupName : groupNames) {
-            try {
-                groups.add(getGroup(groupName));
-            }
-            catch (GroupNotFoundException e) {
-                Log.error(e);
-            }
-        }
-        return groups;
-    }
-
-    public Collection<Group> getGroups(Set<String> groupNames) {
-        List<Group> groups = new ArrayList<Group>(groupNames.size());
-        for (String groupName : groupNames) {
-            try {
-                groups.add(getGroup(groupName));
-            }
-            catch (GroupNotFoundException e) {
-                Log.error(e);
-            }
-        }
-        return groups;
+        return groupNames;
     }
 
-    public Collection<Group> getGroups(int start, int num) {
+    public Collection<String> getGroupNames(int start, int num) {
         List<String> groupNames = new ArrayList<String>();
         Connection con = null;
         PreparedStatement pstmt = null;
@@ -293,19 +271,10 @@ public class JDBCGroupProvider implements GroupProvider {
         finally {
             DbConnectionManager.closeConnection(rs, pstmt, con);
         }
-        List<Group> groups = new ArrayList<Group>(groupNames.size());
-        for (String groupName : groupNames) {
-            try {
-                groups.add(getGroup(groupName));
-            }
-            catch (GroupNotFoundException e) {
-                Log.error(e);
-            }
-        }
-        return groups;
+        return groupNames;
     }
 
-    public Collection<Group> getGroups(JID user) {
+    public Collection<String> getGroupNames(JID user) {
         List<String> groupNames = new ArrayList<String>();
         Connection con = null;
         PreparedStatement pstmt = null;
@@ -325,16 +294,7 @@ public class JDBCGroupProvider implements GroupProvider {
         finally {
             DbConnectionManager.closeConnection(rs, pstmt, con);
         }
-        List<Group> groups = new ArrayList<Group>(groupNames.size());
-        for (String groupName : groupNames) {
-            try {
-                groups.add(getGroup(groupName));
-            }
-            catch (GroupNotFoundException e) {
-                Log.error(e);
-            }
-        }
-        return groups;
+        return groupNames;
     }
 
     /**
@@ -360,7 +320,8 @@ public class JDBCGroupProvider implements GroupProvider {
      * @throws UnsupportedOperationException when called.
      */
     public void updateMember(String groupName, JID user, boolean administrator)
-            throws UnsupportedOperationException {
+            throws UnsupportedOperationException
+    {
         throw new UnsupportedOperationException();
     }
 
@@ -372,7 +333,8 @@ public class JDBCGroupProvider implements GroupProvider {
      * @throws UnsupportedOperationException when called.
      */
     public void deleteMember(String groupName, JID user)
-            throws UnsupportedOperationException {
+            throws UnsupportedOperationException
+    {
         throw new UnsupportedOperationException();
     }
 
@@ -385,11 +347,11 @@ public class JDBCGroupProvider implements GroupProvider {
         return true;
     }
 
-    public Collection<Group> search(String query) {
+    public Collection<String> search(String query) {
         return Collections.emptyList();
     }
 
-    public Collection<Group> search(String query, int startIndex, int numResults) {
+    public Collection<String> search(String query, int startIndex, int numResults) {
         return Collections.emptyList();
     }
 

src/java/org/jivesoftware/wildfire/ldap/LdapGroupProvider.java

@@ -18,7 +18,6 @@ import org.jivesoftware.wildfire.XMPPServer;
 import org.jivesoftware.wildfire.group.Group;
 import org.jivesoftware.wildfire.group.GroupNotFoundException;
 import org.jivesoftware.wildfire.group.GroupProvider;
-import org.jivesoftware.wildfire.group.GroupCollection;
 import org.jivesoftware.wildfire.user.UserManager;
 import org.jivesoftware.wildfire.user.UserNotFoundException;
 import org.xmpp.packet.JID;
@@ -34,13 +33,12 @@ import java.text.MessageFormat;
 import java.util.*;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.io.IOException;
 
 /**
- * LDAP implementation of the GroupProvider interface.  All data in the directory is treated as read-only so any set
- * operations will result in an exception.
+ * LDAP implementation of the GroupProvider interface.  All data in the directory is treated as
+ * read-only so any set operations will result in an exception.
  *
- * @author Greg Ferguson and Cameron Moore
+ * @author Matt Tucker, Greg Ferguson and Cameron Moore
  */
 public class LdapGroupProvider implements GroupProvider {
 
@@ -51,7 +49,7 @@ public class LdapGroupProvider implements GroupProvider {
     private String[] standardAttributes;
 
     /**
-     * Constructor of the LdapGroupProvider class. Gets an LdapManager instance from the LdapManager class.
+     * Constructs a new LDAP group provider.
      */
     public LdapGroupProvider() {
         manager = LdapManager.getInstance();
@@ -84,27 +82,50 @@ public class LdapGroupProvider implements GroupProvider {
         throw new UnsupportedOperationException();
     }
 
-    public Group getGroup(String group) throws GroupNotFoundException {
-        String filter = MessageFormat.format(manager.getGroupSearchFilter(), "*");
-        String searchFilter = "(&" + filter + "(" +
-                manager.getGroupNameField() + "=" + group + "))";
-        Collection<Group> groups;
+    public Group getGroup(String groupName) throws GroupNotFoundException {
+        LdapContext ctx = null;
         try {
-            groups = populateGroups(searchForGroups(searchFilter, standardAttributes));
+            ctx = manager.getContext();
+
+            // Search for the dn based on the group name.
+            SearchControls searchControls = new SearchControls();
+            // See if recursive searching is enabled. Otherwise, only search one level.
+            if (manager.isSubTreeSearch()) {
+                searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);
+            }
+            else {
+                searchControls.setSearchScope(SearchControls.ONELEVEL_SCOPE);
+            }
+            searchControls.setReturningAttributes(standardAttributes);
+            String filter = MessageFormat.format(manager.getGroupSearchFilter(), groupName);
+            NamingEnumeration<SearchResult> answer = ctx.search("", filter, searchControls);
+
+            Collection<Group> groups = populateGroups(answer);
+            if (groups.size() > 1) {
+                // If multiple groups found, throw exception.
+                throw new GroupNotFoundException("Too many groups with name " + groupName + " were found.");
+            }
+            else if (groups.isEmpty()) {
+                throw new GroupNotFoundException("Group with name " + groupName + " not found.");
+            }
+            else {
+                return groups.iterator().next();
+            }
         }
         catch (Exception e) {
-            Log.error("Error populating groups from LDAP", e);
-            throw new GroupNotFoundException("Error populating groups from LDAP", e);
-        }
-        if (groups.size() > 1) {
-            // If multiple groups found, throw exception.
-            throw new GroupNotFoundException("Too many groups with name " + group + " were found.");
-        }
-        else if (groups.isEmpty()) {
-            throw new GroupNotFoundException("Group with name " + group + " not found.");
+            Log.error(e);
+            throw new GroupNotFoundException(e);
         }
-        else {
-            return groups.iterator().next();
+        finally {
+            try {
+                if (ctx != null) {
+                    ctx.setRequestControls(null);
+                    ctx.close();
+                }
+            }
+            catch (Exception ignored) {
+                // Ignore.
+            }
         }
     }
 
@@ -127,7 +148,8 @@ public class LdapGroupProvider implements GroupProvider {
      * @throws UnsupportedOperationException when called.
      */
     public void setDescription(String name, String description)
-            throws UnsupportedOperationException {
+            throws UnsupportedOperationException
+    {
         throw new UnsupportedOperationException();
     }
 
@@ -136,130 +158,173 @@ public class LdapGroupProvider implements GroupProvider {
         if (groupCount != -1 && System.currentTimeMillis() < expiresStamp) {
             return groupCount;
         }
-        int count = 0;
-
         if (manager.isDebugEnabled()) {
             Log.debug("Trying to get the number of groups in the system.");
         }
 
-        String searchFilter = MessageFormat.format(manager.getGroupSearchFilter(), "*");
-        String returningAttributes[] = {manager.getGroupNameField()};
+        int count = 0;
+        LdapContext ctx = null;
         try {
-            NamingEnumeration<SearchResult> answer = searchForGroups(searchFilter, returningAttributes);
-            for (; answer.hasMoreElements(); count++) {
-                try {
-                    answer.next();
-                }
-                catch (Exception e) {
-                    // Ignore.
-                }
-            }
+            ctx = manager.getContext();
 
+            SearchControls searchControls = new SearchControls();
+            // See if recursive searching is enabled. Otherwise, only search one level.
+            if (manager.isSubTreeSearch()) {
+                searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);
+            }
+            else {
+                searchControls.setSearchScope(SearchControls.ONELEVEL_SCOPE);
+            }
+            searchControls.setReturningAttributes(new String[] { manager.getGroupNameField() });
+            String filter = MessageFormat.format(manager.getGroupSearchFilter(), "*");
+            NamingEnumeration answer = ctx.search("", filter, searchControls);
+            while (answer.hasMoreElements()) {
+                answer.next();
+                count++;
+            }
+            // Cache the group count.
             this.groupCount = count;
             this.expiresStamp = System.currentTimeMillis() + JiveConstants.MINUTE * 5;
         }
-        catch (Exception ex) {
-            Log.error("Error searching for groups in LDAP", ex);
+        catch (Exception e) {
+            Log.error(e);
+        }
+        finally {
+            try {
+                if (ctx != null) {
+                    ctx.setRequestControls(null);
+                    ctx.close();
+                }
+            }
+            catch (Exception ignored) {
+                // Ignore.
+            }
         }
+
         return count;
     }
 
-    public Collection<Group> getGroups() {
-        String filter = MessageFormat.format(manager.getGroupSearchFilter(), "*");
+    public Collection<String> getGroupNames() {
+        List<String> groupNames = new ArrayList<String>();
+        LdapContext ctx = null;
         try {
-            return populateGroups(searchForGroups(filter, standardAttributes));
-        }
-        catch (Exception ex) {
-            return Collections.emptyList();
-        }
-    }
+            ctx = manager.getContext();
+            // Sort on group name field.
+            Control[] searchControl = new Control[]{
+                new SortControl(new String[]{manager.getGroupNameField()}, Control.NONCRITICAL)
+            };
+            ctx.setRequestControls(searchControl);
 
-    public Collection<Group> getGroups(Set<String> groupNames) {
-        if (groupNames.isEmpty()) {
-            return Collections.emptyList();
-        }
-        Collection<Group> groups = new ArrayList<Group>(groupNames.size());
-
-        String filter = MessageFormat.format(manager.getGroupSearchFilter(), "*");
-        // Instead of loading all groups at once which may not work for super big collections
-        // of group names, we are going to make many queries and load by 10 groups at onces
-        Collection<String> searchFilters = new ArrayList<String>(groupNames.size());
-        List<String> names = new ArrayList<String>(groupNames);
-        int i = 0;
-        int range = 10;
-        do {
-            List<String> subset = names.subList(i, Math.min(i + range, groupNames.size()));
-
-            if (subset.size() == 1) {
-                String searchFilter = "(&" + filter + "(" +
-                        manager.getGroupNameField() + "=" + subset.get(0) + "))";
-                searchFilters.add(searchFilter);
+            SearchControls searchControls = new SearchControls();
+            // See if recursive searching is enabled. Otherwise, only search one level.
+            if (manager.isSubTreeSearch()) {
+                searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);
             }
             else {
-                StringBuilder sb = new StringBuilder(300);
-                sb.append("(&").append(filter).append("(|");
-                for (String groupName : subset) {
-                    sb.append("(").append(manager.getGroupNameField()).append("=");
-                    sb.append(groupName).append(")");
-                }
-                sb.append("))");
-                searchFilters.add(sb.toString());
+                searchControls.setSearchScope(SearchControls.ONELEVEL_SCOPE);
+            }
+            searchControls.setReturningAttributes(new String[] { manager.getGroupNameField() });
+            String filter = MessageFormat.format(manager.getGroupSearchFilter(), "*");
+            NamingEnumeration answer = ctx.search("", filter, searchControls);
+            while (answer.hasMoreElements()) {
+                // Get the next group.
+                String groupName = (String)((SearchResult)answer.next()).getAttributes().get(
+                        manager.getGroupNameField()).get();
+                // Escape group name and add to results.
+                groupNames.add(JID.escapeNode(groupName));
+            }
+            // If client-side sorting is enabled, sort.
+            if (Boolean.valueOf(JiveGlobals.getXMLProperty("ldap.clientSideSorting"))) {
+                Collections.sort(groupNames);
             }
-            // Increment counter to get next range
-            i = i + range;
         }
-        while (groupNames.size() > i);
-
-        // Perform all required queries to load all requested groups
-        for (String searchFilter : searchFilters) {
+        catch (Exception e) {
+            Log.error(e);
+        }
+        finally {
             try {
-                groups.addAll(populateGroups(searchForGroups(searchFilter, standardAttributes)));
+                if (ctx != null) {
+                    ctx.setRequestControls(null);
+                    ctx.close();
+                }
             }
-            catch (Exception e) {
-                Log.error("Error populating groups from LDAP", e);
-                return Collections.emptyList();
+            catch (Exception ignored) {
+                // Ignore.
             }
         }
-        return new ArrayList<Group>(groups);
+        return groupNames;
     }
 
-    public Collection<Group> getGroups(int startIndex, int numResults) {
-        // Get an enumeration of all groups in the system
-        String searchFilter = MessageFormat.format(manager.getGroupSearchFilter(), "*");
-        NamingEnumeration<SearchResult> answer;
+    public Collection<String> getGroupNames(int startIndex, int numResults) {
+        List<String> groupNames = new ArrayList<String>();
+        LdapContext ctx = null;
         try {
-            answer = searchForGroups(searchFilter, standardAttributes);
+            ctx = manager.getContext();
+            // Sort on group name field.
+            Control[] searchControl = new Control[]{
+                new SortControl(new String[]{manager.getGroupNameField()}, Control.NONCRITICAL)
+            };
+            ctx.setRequestControls(searchControl);
+
+            SearchControls searchControls = new SearchControls();
+            // See if recursive searching is enabled. Otherwise, only search one level.
+            if (manager.isSubTreeSearch()) {
+                searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);
+            }
+            else {
+                searchControls.setSearchScope(SearchControls.ONELEVEL_SCOPE);
+            }
+            searchControls.setReturningAttributes(new String[] { manager.getGroupNameField() });
+            String filter = MessageFormat.format(manager.getGroupSearchFilter(), "*");
+
+            // TODO: used paged results is supported by LDAP server.
+            NamingEnumeration answer = ctx.search("", filter, searchControls);
+            for (int i=0; i < startIndex; i++) {
+                if (answer.hasMoreElements()) {
+                    answer.next();
+                }
+                else {
+                    return Collections.emptyList();
+                }
+            }
+            // Now read in desired number of results (or stop if we run out of results).
+            for (int i = 0; i < numResults; i++) {
+                if (answer.hasMoreElements()) {
+                    // Get the next group.
+                    String groupName = (String)((SearchResult)answer.next()).getAttributes().get(
+                            manager.getGroupNameField()).get();
+                    // Escape group name and add to results.
+                    groupNames.add(JID.escapeNode(groupName));
+                }
+                else {
+                    break;
+                }
+            }
+            // If client-side sorting is enabled, sort.
+            if (Boolean.valueOf(JiveGlobals.getXMLProperty("ldap.clientSideSorting"))) {
+                Collections.sort(groupNames);
+            }
         }
         catch (Exception e) {
-            Log.error("Error searching for groups in LDAP", e);
-            return Collections.emptyList();
+            Log.error(e);
         }
-
-        // Place all groups that are wanted into an enumeration
-        Vector<SearchResult> v = new Vector<SearchResult>();
-        for (int i = 1; answer.hasMoreElements() && i <= (startIndex + numResults); i++) {
+        finally {
             try {
-                SearchResult sr = answer.next();
-                if (i >= startIndex) {
-                    v.add(sr);
+                if (ctx != null) {
+                    ctx.setRequestControls(null);
+                    ctx.close();
                 }
             }
-            catch (Exception e) {
+            catch (Exception ignored) {
                 // Ignore.
             }
         }
-
-        try {
-            return populateGroups(v.elements());
-        }
-        catch (NamingException e) {
-            Log.error("Error populating groups recieved from LDAP", e);
-            return Collections.emptyList();
-        }
+        return groupNames;
     }
 
-    public Collection<Group> getGroups(JID user) {
-        XMPPServer server = XMPPServer.getInstance();
+    public Collection<String> getGroupNames(JID user) {
+        return Collections.emptyList();
+        /* XMPPServer server = XMPPServer.getInstance();
         String username;
         if (!manager.isPosixMode()) {
             // Check if the user exists (only if user is a local user)
@@ -278,15 +343,17 @@ public class LdapGroupProvider implements GroupProvider {
         else {
             username = server.isLocal(user) ? JID.unescapeNode(user.getNode()) : user.toString();
         }
+        System.out.println("Username for search: " + username);
 
-        String filter = MessageFormat.format(manager.getGroupSearchFilter(), username);
+        String filter = MessageFormat.format(manager.getSearchFilter(), username);
+        System.out.println("Filter: " + filter);
         try {
             return populateGroups(searchForGroups(filter, standardAttributes));
         }
         catch (Exception e) {
             Log.error("Error populating groups recieved from LDAP", e);
             return Collections.emptyList();
-        }
+        }*/
     }
 
     /**
@@ -298,7 +365,8 @@ public class LdapGroupProvider implements GroupProvider {
      * @throws UnsupportedOperationException when called.
      */
     public void addMember(String groupName, JID user, boolean administrator)
-            throws UnsupportedOperationException {
+            throws UnsupportedOperationException
+    {
         throw new UnsupportedOperationException();
     }
 
@@ -335,7 +403,7 @@ public class LdapGroupProvider implements GroupProvider {
         return true;
     }
 
-    public Collection<Group> search(String query) {
+    public Collection<String> search(String query) {
         if (query == null || "".equals(query)) {
             return Collections.emptyList();
         }
@@ -393,10 +461,10 @@ public class LdapGroupProvider implements GroupProvider {
                 // Ignore.
             }
         }
-        return new GroupCollection(groupNames.toArray(new String[groupNames.size()]));
+        return groupNames;
     }
 
-    public Collection<Group> search(String query, int startIndex, int numResults) {
+    public Collection<String> search(String query, int startIndex, int numResults) {
         if (query == null || "".equals(query)) {
             return Collections.emptyList();
         }
@@ -470,67 +538,13 @@ public class LdapGroupProvider implements GroupProvider {
                 // Ignore.
             }
         }
-        return new GroupCollection(groupNames.toArray(new String[groupNames.size()]));
+        return groupNames;
     }
 
     public boolean isSearchSupported() {
         return true;
     }
 
-    /**
-     * An auxilary method used to perform LDAP queries based on a provided LDAP search filter.
-     *
-     * @param searchFilter LDAP search filter used to query.
-     * @return an enumeration of SearchResult.
-     */
-    private NamingEnumeration<SearchResult> searchForGroups(String searchFilter,
-            String[] returningAttributes) throws NamingException, IOException
-    {
-        if (manager.isDebugEnabled()) {
-            Log.debug("Trying to find all groups in the system.");
-        }
-        LdapContext ctx = null;
-        NamingEnumeration<SearchResult> answer;
-        try {
-            ctx = manager.getContext();
-            // Sort on username field.
-            Control[] searchControl = new Control[]{
-                new SortControl(new String[]{manager.getGroupNameField()}, Control.NONCRITICAL)
-            };
-            ctx.setRequestControls(searchControl);
-            if (manager.isDebugEnabled()) {
-                Log.debug("Starting LDAP search...");
-                Log.debug("Using groupSearchFilter: " + searchFilter);
-            }
-
-            // Search for the dn based on the groupname.
-            SearchControls searchControls = new SearchControls();
-            searchControls.setReturningAttributes(returningAttributes);
-            // See if recursive searching is enabled. Otherwise, only search one level.
-            if (manager.isSubTreeSearch()) {
-                searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);
-            }
-            else {
-                searchControls.setSearchScope(SearchControls.ONELEVEL_SCOPE);
-            }
-            answer = ctx.search("", searchFilter, searchControls);
-
-            if (manager.isDebugEnabled()) {
-                Log.debug("... search finished");
-            }
-
-            return answer;
-        }
-        finally {
-            if (ctx != null) {
-                try {
-                    ctx.close();
-                }
-                catch (Exception ex) { /* do nothing */ }
-            }
-        }
-    }
-
     /**
      * An auxilary method used to populate LDAP groups based on a provided LDAP search result.
      *

src/web/group-edit.jsp

@@ -31,9 +31,6 @@
 <!-- Define Administration Bean -->
 <jsp:useBean id="webManager" class="org.jivesoftware.util.WebManager"/>
 <%  webManager.init(pageContext); %>
-<jsp:useBean id="errors" class="java.util.HashMap"/>
-
-
 
 <%  // Get parameters
     boolean add = request.getParameter("add") != null;
@@ -50,6 +47,8 @@
     String newDescription = ParamUtils.getParameter(request, "newDescription");
     boolean groupInfoChanged = ParamUtils.getBooleanParameter(request, "groupChanged", false);
 
+    Map<String,String> errors = new HashMap<String,String>();
+
     // Get the presence manager
     PresenceManager presenceManager = webManager.getPresenceManager();
     UserManager userManager = webManager.getUserManager();
@@ -98,7 +97,6 @@
                 }
 
             groupName = newName;
-            groupInfoChanged = true;
              // Get admin list and compare it the admin posted list.
             response.sendRedirect("group-edit.jsp?group=" + URLEncoder.encode(groupName, "UTF-8") + "&groupChanged=true");
             return;
@@ -113,8 +111,8 @@
 
     if (update) {
         Set<JID> adminIDSet = new HashSet<JID>();
-        for (int i = 0; i < adminIDs.length; i++) {
-            JID newAdmin = new JID(adminIDs[i]);
+        for (String adminID : adminIDs) {
+            JID newAdmin = new JID(adminID);
             adminIDSet.add(newAdmin);
             boolean isAlreadyAdmin = group.getAdmins().contains(newAdmin);
             if (!isAlreadyAdmin) {
@@ -122,18 +120,15 @@
                 group.getAdmins().add(newAdmin);
             }
         }
-        Iterator<JID> groupIter = Collections.unmodifiableCollection(group.getAdmins()).iterator();
+        Collection<JID> admins = Collections.unmodifiableCollection(group.getAdmins());
         Set<JID> removeList = new HashSet<JID>();
-        while (groupIter.hasNext()) {
-            JID m = (JID) groupIter.next();
-            if (!adminIDSet.contains(m)) {
-                removeList.add(m);
+        for (JID admin : admins) {
+            if (!adminIDSet.contains(admin)) {
+                removeList.add(admin);
             }
         }
-        Iterator<JID> i = removeList.iterator();
-        while (i.hasNext()) {
-            JID m = (JID) i.next();
-            group.getMembers().add(m);
+        for (JID member : removeList) {
+            group.getMembers().add(member);
         }
         // Get admin list and compare it the admin posted list.
         response.sendRedirect("group-edit.jsp?group=" + URLEncoder.encode(groupName, "UTF-8") + "&updatesuccess=true");
@@ -191,8 +186,8 @@
         add = false;
     }
     else if (delete) {
-        for (int i = 0; i < deleteMembers.length; i++) {
-            JID member = new JID(deleteMembers[i]);
+        for (String deleteMember : deleteMembers) {
+            JID member = new JID(deleteMember);
             group.getMembers().remove(member);
             group.getAdmins().remove(member);
         }
@@ -586,7 +581,7 @@
             }
 %>
         </table>
-        </div>
+
     </form>
 
     <script type="text/javascript">