1. Show the shared group to a list of other groups. (still incomplete) JM-119

2. Show shared group in everyone's roster. JM-118


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

src/java/org/jivesoftware/messenger/SessionManager.java

@@ -721,9 +721,8 @@ public class SessionManager extends BasicModule implements ConnectionCloseListen
 
     private void copyUserSessions(List sessions) {
         // Get a copy of the sessions from all users
-        Iterator users = getSessionUsers();
-        while (users.hasNext()) {
-            Collection<Session> usrSessions = getSessions((String)users.next());
+        for (String username : getSessionUsers()) {
+            Collection<Session> usrSessions = getSessions(username);
             for (Session session : usrSessions) {
                 sessions.add(session);
             }
@@ -759,9 +758,8 @@ public class SessionManager extends BasicModule implements ConnectionCloseListen
 
     public int getSessionCount() {
         int sessionCount = 0;
-        Iterator users = getSessionUsers();
-        while (users.hasNext()) {
-            sessionCount += getSessionCount((String)users.next());
+        for (String username : getSessionUsers()) {
+            sessionCount += getSessionCount(username);
         }
         sessionCount += anonymousSessions.size();
         return sessionCount;
@@ -780,8 +778,8 @@ public class SessionManager extends BasicModule implements ConnectionCloseListen
         return sessionCount;
     }
 
-    public Iterator getSessionUsers() {
-        return Arrays.asList(sessions.keySet().toArray()).iterator();
+    public Collection<String> getSessionUsers() {
+        return Collections.unmodifiableCollection(sessions.keySet());
     }
 
     /**

src/java/org/jivesoftware/messenger/roster/Roster.java

@@ -73,13 +73,7 @@ public class Roster implements Cacheable {
         Collection<Group> sharedGroups = null;
         try {
             User rosterUser = UserManager.getInstance().getUser(getUsername());
-            sharedGroups = GroupManager.getInstance().getGroups(rosterUser);
-            // Remove groups that are not being shown in group members' rosters
-            for (Iterator<Group> it=sharedGroups.iterator(); it.hasNext();) {
-                if (!"true".equals(it.next().getProperties().get("showInRoster"))) {
-                    it.remove();
-                }
-            }
+            sharedGroups = XMPPServer.getInstance().getRosterManager().getSharedGroups(rosterUser);
         }
         catch (UserNotFoundException e) {
             sharedGroups = new ArrayList<Group>();
@@ -95,22 +89,32 @@ public class Roster implements Cacheable {
             for (Group group : sharedGroups) {
                 if (group.isUser(item.getJid().getNode())) {
                     // TODO Group name conflicts are not being considered (do we need this?)
-                    item.addSharedGroup(group.getProperties().get("displayName"));
+                    item.addSharedGroup(group.getProperties().get("sharedRoster.displayName"));
                 }
             }
             rosterItems.put(item.getJid().toBareJID(), item);
         }
         // Add RosterItems that belong only to shared groups
-        Map<JID,List<String>> sharedUsers = getSharedUsers(sharedGroups);
+        Map<JID,List<Group>> sharedUsers = getSharedUsers(sharedGroups);
         for (JID jid : sharedUsers.keySet()) {
             try {
+                RosterItem.SubType type = RosterItem.SUB_TO;
                 User user = UserManager.getInstance().getUser(jid.getNode());
                 String nickname = "".equals(user.getName()) ? jid.getNode() : user.getName();
                 RosterItem item = new RosterItem(jid, RosterItem.SUB_BOTH, RosterItem.ASK_NONE,
                         RosterItem.RECV_NONE, nickname , null);
-                for (String group : sharedUsers.get(jid)) {
-                    item.addSharedGroup(group);
+                if (sharedUsers.get(jid).isEmpty()) {
+                    type = RosterItem.SUB_FROM;
+                }
+                else {
+                    for (Group group : sharedUsers.get(jid)) {
+                        item.addSharedGroup(group.getProperties().get("sharedRoster.displayName"));
+                        if (group.isUser(username)) {
+                            type = RosterItem.SUB_BOTH;
+                        }
+                    }
                 }
+                item.setSubStatus(type);
                 rosterItems.put(item.getJid().toBareJID(), item);
             }
             catch (UserNotFoundException e) {
@@ -231,7 +235,7 @@ public class Roster implements Cacheable {
             Collection<Group> sharedGroups = GroupManager.getInstance().getGroups();
             for (String group : groups) {
                 for (Group sharedGroup : sharedGroups) {
-                    if (group.equals(sharedGroup.getProperties().get("displayName"))) {
+                    if (group.equals(sharedGroup.getProperties().get("sharedRoster.displayName"))) {
                         throw new SharedGroupException("Cannot add an item to a shared group");
                     }
                 }
@@ -400,26 +404,29 @@ public class Roster implements Cacheable {
      * @param sharedGroups the shared groups of this user.
      * @return the list of users that belong ONLY to a shared group of this user.
      */
-    private Map<JID,List<String>> getSharedUsers(Collection<Group> sharedGroups) {
+    private Map<JID,List<Group>> getSharedUsers(Collection<Group> sharedGroups) {
         // Get the users to process from the shared groups. Users that belong to different groups
         // will have one entry in the map associated with all the groups
-        Map<JID,List<String>> sharedGroupUsers = new HashMap<JID,List<String>>();
+        Map<JID,List<Group>> sharedGroupUsers = new HashMap<JID,List<Group>>();
         for (Group group : sharedGroups) {
-            // Get all the group users
-            Collection<String> users = new ArrayList<String>(group.getMembers());
-            users.addAll(group.getAdmins());
+            // Get all the users that can have this group in their rosters
+            Collection<String> users = XMPPServer.getInstance().getRosterManager().getRelatedUsers(group, false);
             // Add the users of the group to the general list of users to process
-            for (String groupUser : users) {
+            for (String user : users) {
                 // Add the user to the answer if the user doesn't belong to the personal roster
                 // (since we have already added the user to the answer)
-                JID jid = XMPPServer.getInstance().createJID(groupUser, null);
-                if (!isRosterItem(jid) && !getUsername().equals(groupUser)) {
-                    List<String> groups = sharedGroupUsers.get(jid);
+                JID jid = XMPPServer.getInstance().createJID(user, null);
+                if (!isRosterItem(jid) && !getUsername().equals(user)) {
+                    List<Group> groups = sharedGroupUsers.get(jid);
                     if (groups == null) {
-                        groups = new ArrayList<String>();
+                        groups = new ArrayList<Group>();
                         sharedGroupUsers.put(jid, groups);
                     }
-                    groups.add(group.getProperties().get("displayName"));
+                    // Only associate the group with the user if the user is an actual user of
+                    // the group
+                    if (group.isUser(user)) {
+                        groups.add(group);
+                    }
                 }
             }
         }
@@ -475,17 +482,19 @@ public class Roster implements Cacheable {
      * group will be added to the shared groups lists. In any case an update broadcast will be sent
      * to all the users logged resources.
      *
-     * @param sharedGroup the shared group where the user was added.
+     * @param group the shared group where the user was added.
      * @param addedUser the contact to update in the roster.
      */
-    void addSharedUser(String sharedGroup, String addedUser) {
+    void addSharedUser(Group group, String addedUser) {
         RosterItem item = null;
         JID jid = XMPPServer.getInstance().createJID(addedUser, "");
+        // Get the display name of the group
+        String groupName = group.getProperties().get("sharedRoster.displayName");
         try {
             // Get the RosterItem for the *local* user to add
             item = getRosterItem(jid);
             // Do nothing if the item already includes the shared group
-            if (item.getSharedGroups().contains(sharedGroup)) {
+            if (item.getSharedGroups().contains(groupName)) {
                 return;
             }
         }
@@ -501,17 +510,31 @@ public class Roster implements Cacheable {
                 rosterItems.put(item.getJid().toBareJID(), item);
             }
             catch (UserNotFoundException ex) {
-                Log.error("Group (" + sharedGroup + ") includes non-existent username (" +
+                Log.error("Group (" + groupName + ") includes non-existent username (" +
                         addedUser +
                         ")");
             }
         }
-        // Add the shared group to the list of shared groups
-        item.addSharedGroup(sharedGroup);
+        // Update the subscription status depending on the group membership of the new user and
+        // this user
+        if (group.isUser(addedUser) && !group.isUser(getUsername())) {
+            item.setSubStatus(RosterItem.SUB_TO);
+            // Add the shared group to the list of shared groups
+            item.addSharedGroup(groupName);
+        }
+        else if (!group.isUser(addedUser) && group.isUser(getUsername())) {
+            item.setSubStatus(RosterItem.SUB_FROM);
+        }
+        else {
+            // Add the shared group to the list of shared groups
+            item.addSharedGroup(groupName);
+        }
         // Brodcast to all the user resources of the updated roster item
         broadcast(item);
-        // Presences of shared users are of type BOTH so probe for presences
-        presenceManager.probePresence(username, item.getJid());
+        // Probe the presence of the new group user
+        if (item.getSubStatus() == RosterItem.SUB_BOTH || item.getSubStatus() == RosterItem.SUB_TO) {
+            presenceManager.probePresence(username, item.getJid());
+        }
     }
 
     /**

src/java/org/jivesoftware/messenger/roster/RosterItem.java

@@ -209,13 +209,7 @@ public class RosterItem implements Cacheable {
      * @return The subscription status of the item
      */
     public SubType getSubStatus() {
-        if (isShared()) {
-            // Redefine the sub status since the item belongs to a shared group
-            return SUB_BOTH;
-        }
-        else {
-            return subStatus;
-        }
+        return subStatus;
     }
 
     /**

src/java/org/jivesoftware/messenger/roster/RosterManager.java

@@ -16,15 +16,17 @@ import org.jivesoftware.util.Cache;
 import org.jivesoftware.util.CacheManager;
 import org.jivesoftware.messenger.container.BasicModule;
 import org.jivesoftware.messenger.user.UserNotFoundException;
+import org.jivesoftware.messenger.user.User;
+import org.jivesoftware.messenger.user.UserManager;
 import org.jivesoftware.messenger.SharedGroupException;
+import org.jivesoftware.messenger.SessionManager;
 import org.jivesoftware.messenger.event.GroupEventListener;
 import org.jivesoftware.messenger.event.GroupEventDispatcher;
 import org.jivesoftware.messenger.group.Group;
+import org.jivesoftware.messenger.group.GroupManager;
+import org.jivesoftware.messenger.group.GroupNotFoundException;
 
-import java.util.Iterator;
-import java.util.Collection;
-import java.util.ArrayList;
-import java.util.Map;
+import java.util.*;
 
 /**
  * A simple service that allows components to retrieve a roster based solely on the ID
@@ -52,8 +54,8 @@ public class RosterManager extends BasicModule implements GroupEventListener {
      *
      * @param username the username to search for.
      * @return the roster associated with the ID.
-     * @throws org.jivesoftware.messenger.user.UserNotFoundException if the ID does not correspond to a known
-     *      entity on the server.
+     * @throws org.jivesoftware.messenger.user.UserNotFoundException if the ID does not correspond
+     *         to a known entity on the server.
      */
     public Roster getRoster(String username) throws UserNotFoundException {
         if (rosterCache == null) {
@@ -126,16 +128,77 @@ public class RosterManager extends BasicModule implements GroupEventListener {
         }
     }
 
+    /**
+     * Returns a collection with all the groups that the user may include in his roster. The
+     * following criteria will be used to select the groups: 1) Groups that are configured so that
+     * everybody can include in his roster, 2) Groups that are configured so that its users may
+     * include the group in their rosters and the user is a group user of the group and 3) User
+     * belongs to a Group that may see a Group that whose members may include the Group in their
+     * rosters.
+     *
+     * @param user the user to return his shared groups.
+     * @return a collection with all the groups that the user may include in his roster.
+     */
+    public Collection<Group> getSharedGroups(User user) {
+        Collection<Group> answer = new HashSet<Group>();
+        Collection<Group> groups = GroupManager.getInstance().getGroups();
+        for (Group group : groups) {
+            String showInRoster = group.getProperties().get("sharedRoster.showInRoster");
+            if ("onlyGroup".equals(showInRoster)) {
+                if (group.isUser(user.getUsername())) {
+                    // The user belongs to the group so add the group to the answer
+                    answer.add(group);
+                }
+                else {
+                    // Check if the user belongs to a group that may see this group
+                    Collection<Group> groupList = parseGroups(group.getProperties().get("sharedRoster.groupList"));
+                    for (Group groupInList : groupList) {
+                        if (groupInList.isUser(user.getUsername())) {
+                            answer.add(groupInList);
+                        }
+                    }
+                }
+            }
+            else if ("everybody".equals(showInRoster)) {
+                // Anyone can see this group so add the group to the answer
+                answer.add(group);
+            }
+        }
+        return answer;
+    }
+
+    /**
+     * Returns a collection of Groups obtained by parsing a comma delimited String with the name
+     * of groups.
+     *
+     * @param groupNames a comma delimited string with group names.
+     * @return a collection of Groups obtained by parsing a comma delimited String with the name
+     *         of groups.
+     */
+    private Collection<Group> parseGroups(String groupNames) {
+        Collection<Group> answer = new HashSet<Group>();
+        if (groupNames != null) {
+            StringTokenizer tokenizer = new StringTokenizer(groupNames, ",");
+            while (tokenizer.hasMoreTokens()) {
+                String groupName = tokenizer.nextToken();
+                try {
+                    answer.add(GroupManager.getInstance().getGroup(groupName));
+                }
+                catch (GroupNotFoundException e) {
+                    // Do nothing. Silently ignore the invalid reference to the group
+                }
+            }
+        }
+        return answer;
+    }
+
     public void groupCreated(Group group, Map params) {
         //Do nothing
     }
 
     public void groupDeleting(Group group, Map params) {
-        // Get all the group users
-        Collection<String> users = new ArrayList<String>(group.getMembers());
-        users.addAll(group.getAdmins());
         // Iterate on all the group users and update their rosters
-        for (String deletedUser : users) {
+        for (String deletedUser : getRelatedUsers(group, true)) {
             groupUserDeleted(group, deletedUser);
         }
     }
@@ -145,46 +208,57 @@ public class RosterManager extends BasicModule implements GroupEventListener {
         if (!"propertyModified".equals(params.get("type"))) {
              return;
         }
+        String keyChanged = (String) params.get("propertyKey");
         String originalValue = (String) params.get("originalValue");
 
-        if ("showInRoster".equals(params.get("propertyKey"))) {
-            String currentValue = group.getProperties().get("showInRoster");
+
+        if ("sharedRoster.showInRoster".equals(keyChanged)) {
+            String currentValue = group.getProperties().get("sharedRoster.showInRoster");
             // Nothing has changed so do nothing.
             if (currentValue.equals(originalValue)) {
                 return;
             }
-            // Get all the group users
-            Collection<String> users = new ArrayList<String>(group.getMembers());
-            users.addAll(group.getAdmins());
-            if ("true".equals(currentValue)) {
-                // We must show group in group members' rosters
-                // Iterate on all the group users and update their rosters
-                for (String addedUser : users) {
-                    groupUserAdded(group, addedUser);
-                }
+            // Remove the group from all users's rosters (if the group was a shared group)
+            Collection<String> users = getRelatedUsers(group, originalValue,
+                    group.getProperties().get("sharedRoster.groupList"), true);
+            for (String deletedUser : users) {
+                groupUserDeleted(group, users, deletedUser);
             }
-            else {
-                // We must remove group from group members' rosters
-                // Iterate on all the group users and update their rosters
-                for (String deletedUser : users) {
-                    groupUserDeleted(group, deletedUser);
-                }
+            // Add the group to all users's rosters (if the group is a shared group)
+            for (String addedUser : getRelatedUsers(group, true)) {
+                groupUserAdded(group, addedUser);
             }
         }
-        else if ("displayName".equals(params.get("propertyKey"))) {
-            String currentValue = group.getProperties().get("displayName");
+        else if ("sharedRoster.groupList".equals(keyChanged)) {
+            String currentValue = group.getProperties().get("sharedRoster.groupList");
             // Nothing has changed so do nothing.
             if (currentValue.equals(originalValue)) {
                 return;
             }
-            // Do nothing if the group is not being shown in group members' rosters
-            if (!"true".equals(group.getProperties().get("showInRoster"))) {
+            // Remove the group from all users's rosters (if the group was a shared group)
+            Collection<String> users = getRelatedUsers(group,
+                    group.getProperties().get("sharedRoster.showInRoster"), originalValue, true);
+            for (String deletedUser : users) {
+                groupUserDeleted(group, users, deletedUser);
+            }
+            // Add the group to all users's rosters (if the group is a shared group)
+            for (String addedUser : getRelatedUsers(group, true)) {
+                groupUserAdded(group, addedUser);
+            }
+        }
+        else if ("sharedRoster.displayName".equals(keyChanged)) {
+            String currentValue = group.getProperties().get("sharedRoster.displayName");
+            // Nothing has changed so do nothing.
+            if (currentValue.equals(originalValue)) {
                 return;
             }
-            // Get all the group users
-            Collection<String> users = new ArrayList<String>(group.getMembers());
-            users.addAll(group.getAdmins());
-            // Iterate on all the group users and update their rosters
+            // Do nothing if the group is not being shown in users' rosters
+            if (!isSharedGroup(group)) {
+                return;
+            }
+            // Get all the affected users
+            Collection<String> users = getRelatedUsers(group, true);
+            // Iterate on all the affected users and update their rosters
             for (String updatedUser : users) {
                 // Get the roster to update.
                 Roster roster = (Roster) CacheManager.getCache("username2roster").get(updatedUser);
@@ -196,9 +270,24 @@ public class RosterManager extends BasicModule implements GroupEventListener {
         }
     }
 
+    /**
+     * Returns true if the specified Group may be included in a user roster. The decision is made
+     * based on the group properties that are configurable through the Admin Console.
+     *
+     * @param group the group to check if it may be considered a shared group.
+     * @return true if the specified Group may be included in a user roster.
+     */
+    public boolean isSharedGroup(Group group) {
+        String showInRoster = group.getProperties().get("sharedRoster.showInRoster");
+        if ("onlyGroup".equals(showInRoster) || "everybody".equals(showInRoster)) {
+            return true;
+        }
+        return false;
+    }
+
     public void memberAdded(Group group, Map params) {
         // Do nothing if the group is not being shown in group members' rosters
-        if (!"true".equals(group.getProperties().get("showInRoster"))) {
+        if (!isSharedGroup(group)) {
             return;
         }
         String addedUser = (String) params.get("member");
@@ -207,7 +296,7 @@ public class RosterManager extends BasicModule implements GroupEventListener {
 
     public void memberRemoved(Group group, Map params) {
         // Do nothing if the group is not being shown in group members' rosters
-        if (!"true".equals(group.getProperties().get("showInRoster"))) {
+        if (!isSharedGroup(group)) {
             return;
         }
         String addedUser = (String) params.get("member");
@@ -216,7 +305,7 @@ public class RosterManager extends BasicModule implements GroupEventListener {
 
     public void adminAdded(Group group, Map params) {
         // Do nothing if the group is not being shown in group members' rosters
-        if (!"true".equals(group.getProperties().get("showInRoster"))) {
+        if (!isSharedGroup(group)) {
             return;
         }
         String addedUser = (String) params.get("admin");
@@ -225,7 +314,7 @@ public class RosterManager extends BasicModule implements GroupEventListener {
 
     public void adminRemoved(Group group, Map params) {
         // Do nothing if the group is not being shown in group members' rosters
-        if (!"true".equals(group.getProperties().get("showInRoster"))) {
+        if (!isSharedGroup(group)) {
             return;
         }
         String addedUser = (String) params.get("admin");
@@ -239,26 +328,24 @@ public class RosterManager extends BasicModule implements GroupEventListener {
      * @param addedUser the username of the user that has been added to the group.
      */
     private void groupUserAdded(Group group, String addedUser) {
-        // Get all the group users
-        Collection<String> users = new ArrayList<String>(group.getMembers());
-        users.addAll(group.getAdmins());
+        // Get all the affected users
+        Collection<String> users = getRelatedUsers(group, true);
         // Get the roster of the added user.
         Roster addedUserRoster = (Roster) CacheManager.getCache("username2roster").get(addedUser);
-        // Get the display name of the group
-        String groupName = group.getProperties().get("displayName");
 
-        // Iterate on all the group users and update their rosters
+        // Iterate on all the affected users and update their rosters
         for (String userToUpdate : users) {
             if (!addedUser.equals(userToUpdate)) {
                 // Get the roster to update
                 Roster roster = (Roster)CacheManager.getCache("username2roster").get(userToUpdate);
                 // Only update rosters in memory
                 if (roster != null) {
-                    roster.addSharedUser(groupName, addedUser);
+                    roster.addSharedUser(group, addedUser);
                 }
-                // Update the roster of the newly added group user
-                if (addedUserRoster != null) {
-                    addedUserRoster.addSharedUser(groupName, userToUpdate);
+                // Update the roster of the newly added group user. Only add users of the group to
+                // the roster of the new group user
+                if (addedUserRoster != null && group.isUser(userToUpdate)) {
+                    addedUserRoster.addSharedUser(group, userToUpdate);
                 }
             }
         }
@@ -271,15 +358,23 @@ public class RosterManager extends BasicModule implements GroupEventListener {
      * @param deletedUser the username of the user that has been deleted from the group.
      */
     private void groupUserDeleted(Group group, String deletedUser) {
-        // Get all the group users
-        Collection<String> users = new ArrayList<String>(group.getMembers());
-        users.addAll(group.getAdmins());
+        groupUserDeleted(group, getRelatedUsers(group, true), deletedUser);
+    }
+
+    /**
+     * Notification that a Group user has been deleted. Update the group users' roster accordingly.
+     *
+     * @param group the group from where the user was deleted.
+     * @param users the users to update their rosters
+     * @param deletedUser the username of the user that has been deleted from the group.
+     */
+    private void groupUserDeleted(Group group, Collection<String> users, String deletedUser) {
         // Get the roster of the deleted user.
         Roster deletedUserRoster = (Roster) CacheManager.getCache("username2roster").get(deletedUser);
         // Get the display name of the group
-        String groupName = group.getProperties().get("displayName");
+        String groupName = group.getProperties().get("sharedRoster.displayName");
 
-        // Iterate on all the group users and update their rosters
+        // Iterate on all the affected users and update their rosters
         for (String userToUpdate : users) {
             // Get the roster to update
             Roster roster = (Roster)CacheManager.getCache("username2roster").get(userToUpdate);
@@ -287,10 +382,67 @@ public class RosterManager extends BasicModule implements GroupEventListener {
             if (roster != null) {
                 roster.deleteSharedUser(groupName, deletedUser);
             }
-            // Update the roster of the newly deleted group user
-            if (deletedUserRoster != null) {
+            // Update the roster of the newly deleted group user. Only remove users of the group
+            // from the roster of the deleted group user
+            if (deletedUserRoster != null && group.isUser(userToUpdate)) {
                 deletedUserRoster.deleteSharedUser(groupName, userToUpdate);
             }
         }
     }
+
+    /**
+     * Returns all the users that are related to a shared group. This is the logic that we are
+     * using: 1) If the group visiblity is configured as "Everybody" then all users in the system or
+     * all logged users in the system will be returned (configurable thorugh the "filterOffline"
+     * flag), 2) if the group visiblity is configured as "onlyGroup" then all the group users will
+     * be included in the answer and 3) if the group visiblity is configured as "onlyGroup" and
+     * the group allows other groups to include the group in the groups users' roster then all
+     * the users of the allowed groups will be included in the answer.
+     */
+    Collection<String> getRelatedUsers(Group group, boolean filterOffline) {
+        return getRelatedUsers(group, group.getProperties().get("sharedRoster.showInRoster"),
+                group.getProperties().get("sharedRoster.groupList"), filterOffline);
+    }
+
+    /**
+     * This method is similar to {@link #getRelatedUsers(Group, boolean)} except that it receives
+     * some group properties. The group properties are passed as parameters since the called of this
+     * method may want to obtain the related users of the group based in some properties values.
+     *
+     * This is useful when the group is being edited and some properties has changed and we need to
+     * obtain the related users of the group based on the previous group state.
+     */ 
+    private Collection<String> getRelatedUsers(Group group, String showInRoster, String groupNames,
+            boolean filterOffline) {
+        // Answer an empty collection if the group is not being shown in users' rosters
+        if (!"onlyGroup".equals(showInRoster) && !"everybody".equals(showInRoster)) {
+            return new ArrayList<String>();
+        }
+        // Add the users of the group
+        Collection<String> users = new HashSet<String>(group.getMembers());
+        users.addAll(group.getAdmins());
+        // Check if anyone can see this shared group
+        if ("everybody".equals(showInRoster)) {
+            if (filterOffline) {
+                // Add all logged users. We don't need to add all users in the system since only the
+                // logged ones will be affected.
+                users.addAll(SessionManager.getInstance().getSessionUsers());
+            }
+            else {
+                // Add all users in the system
+                for (User user : UserManager.getInstance().getUsers()) {
+                    users.add(user.getUsername());
+                }
+            }
+        }
+        else {
+            // Add the users that may see the group
+            Collection<Group> groupList = parseGroups(groupNames);
+            for (Group groupInList : groupList) {
+                users.addAll(groupInList.getMembers());
+                users.addAll(groupInList.getAdmins());
+            }
+        }
+        return users;
+    }
 }
\ No newline at end of file

src/web/group-create.jsp

@@ -38,8 +38,10 @@ errorPage="error.jsp"%>
     boolean cancel = request.getParameter("cancel") != null;
     String  name = ParamUtils.getParameter(request, "name");
     String  description = ParamUtils.getParameter(request, "description");
-    boolean showInRoster = ParamUtils.getBooleanParameter(request, "show", false);
+    String showInRosterType = ParamUtils.getParameter(request, "show");
+    boolean showInRoster = "onlyGroup".equals(showInRosterType) || "everybody".equals(showInRosterType);
     String  displayName = ParamUtils.getParameter(request, "display");
+    String groupList = ParamUtils.getParameter(request, "groupList");
     String  users = ParamUtils.getParameter(request, "users", true);
     // Handle a cancel
     if (cancel) {
@@ -63,12 +65,14 @@ errorPage="error.jsp"%>
                     newGroup.setDescription(description);
                 }
                 if (showInRoster) {
-                    newGroup.getProperties().put("showInRoster", "true");
-                    newGroup.getProperties().put("displayName", displayName);
+                    newGroup.getProperties().put("sharedRoster.showInRoster", showInRosterType);
+                    newGroup.getProperties().put("sharedRoster.displayName", displayName);
+                    newGroup.getProperties().put("sharedRoster.groupList", groupList == null ? "" : groupList);
                 }
                 else {
-                    newGroup.getProperties().put("showInRoster", "false");
-                    newGroup.getProperties().put("displayName", "");
+                    newGroup.getProperties().put("sharedRoster.showInRoster", "nobody");
+                    newGroup.getProperties().put("sharedRoster.displayName", "");
+                    newGroup.getProperties().put("sharedRoster.groupList", "");
                 }
 
                 if(users.length() > 0){
@@ -115,7 +119,18 @@ errorPage="error.jsp"%>
     <!--
     function refreshDisplayName(showCheck)
     {
-        document.forms.f.display.disabled=!showCheck.checked;
+        if ("onlyGroup" == showCheck.value) {
+            document.forms.f.newDisplay.disabled=false;
+            document.forms.f.newGroupList.disabled=false;
+        }
+        else if ("everybody" == showCheck.value) {
+            document.forms.f.newDisplay.disabled=false;
+            document.forms.f.newGroupList.disabled=true;
+        }
+        else {
+            document.forms.f.newDisplay.disabled=true;
+            document.forms.f.newGroupList.disabled=true;
+        }
     }
     function setDisplayName()
     {
@@ -206,10 +221,16 @@ errorPage="error.jsp"%>
 					<tr><td height="15" colspan="3"><img src="images/blank.gif"></td>
                     <tr>
                         <td width="1%" nowrap>
-                            <label for="gshow">Show group in group members' rosters:</label>
+                            <label for="gshow">Show group in rosters for:</label>
                         </td>
                         <td width="99%">
-                            <input type="checkbox" name="show" value="true" id="gshow" onclick="refreshDisplayName(this)"/>
+                            <label for="onlyGroup">Only group users</label>
+                            <input type="radio" name="show" id="onlyGroup" value="onlyGroup" onclick="refreshDisplayName(this)"/>&nbsp;&nbsp;
+                            <label for="everybody">Everybody</label>
+                            <input type="radio" name="show" id="everybody" value="everybody" onclick="refreshDisplayName(this)"/>&nbsp;&nbsp;
+                            <label for="nobody">Nobody</label>
+                            <input type="radio" name="show" id="nobody" value="nobody" checked onclick="refreshDisplayName(this)"/>
+
                         </td>
                     </tr>
                     <tr>
@@ -227,6 +248,14 @@ errorPage="error.jsp"%>
 %>
                         </td>
                     </tr>
+                    <tr>
+                        <td nowrap width="1%">
+                            <label for="gGroupList">Viewable by groups:</label>
+                        </td>
+                        <td width="99%">
+                            <textarea name="groupList" cols="30" rows="2" id="gGroupList"><%= ((groupList != null) ? groupList : "") %></textarea>
+                        </td>
+                    </tr>
                 </table>
                 <br>
                 <span class="jive-description"> * Required fields </span>

src/web/group-edit.jsp

@@ -31,8 +31,10 @@ import="java.text.DateFormat,
     boolean edit = ParamUtils.getBooleanParameter(request, "edit", false);
     String newName = ParamUtils.getParameter(request, "newName");
     String newDescription = ParamUtils.getParameter(request, "newDescription");
-    boolean newShowInRoster = ParamUtils.getBooleanParameter(request, "newShow", false);
+    String newShowInRosterType = ParamUtils.getParameter(request, "newShow");
+    boolean newShowInRoster = "onlyGroup".equals(newShowInRosterType) || "everybody".equals(newShowInRosterType);
     String  newDisplayName = ParamUtils.getParameter(request, "newDisplay");
+    String newGroupList = ParamUtils.getParameter(request, "newGroupList");
     boolean groupInfoChanged = ParamUtils.getBooleanParameter(request, "groupChanged", false);
 
     Group group = groupManager.getGroup(groupName);
@@ -48,12 +50,14 @@ import="java.text.DateFormat,
             group.setName(newName);
             group.setDescription(newDescription);
             if (newShowInRoster) {
-                group.getProperties().put("showInRoster", "true");
-                group.getProperties().put("displayName", newDisplayName);
+                group.getProperties().put("sharedRoster.showInRoster", newShowInRosterType);
+                group.getProperties().put("sharedRoster.displayName", newDisplayName);
+                group.getProperties().put("sharedRoster.groupList", newGroupList == null ? "" : newGroupList);
             }
             else {
-                group.getProperties().put("showInRoster", "false");
-                group.getProperties().put("displayName", "");
+                group.getProperties().put("sharedRoster.showInRoster", "nobody");
+                group.getProperties().put("sharedRoster.displayName", "");
+                group.getProperties().put("sharedRoster.groupList", "");
             }
             groupName = newName;
             groupInfoChanged = true;
@@ -169,7 +173,18 @@ import="java.text.DateFormat,
     <!--
     function refreshDisplayName(showCheck)
     {
-    document.forms.ff.newDisplay.disabled=!showCheck.checked;
+    if ("onlyGroup" == showCheck.value) {
+        document.forms.ff.newDisplay.disabled=false;
+        document.forms.ff.newGroupList.disabled=false;
+    }
+    else if ("everybody" == showCheck.value) {
+        document.forms.ff.newDisplay.disabled=false;
+        document.forms.ff.newGroupList.disabled=true;
+    }
+    else {
+        document.forms.ff.newDisplay.disabled=true;
+        document.forms.ff.newGroupList.disabled=true;
+    }
     }
     //-->
     </script>
@@ -258,7 +273,7 @@ import="java.text.DateFormat,
                 <% } else { %>
 
                 <td>
-                <input type="text" name="newDescription" value="<%= group.getDescription() != null ? group.getDescription() : "" %>">
+                <textarea name="newDescription" cols="40" rows="4"><%= group.getDescription() != null ? group.getDescription() : "" %></textarea>
                 </td>
 
                 <% } %>
@@ -266,17 +281,36 @@ import="java.text.DateFormat,
             <tr><td height="15" colspan="3"><img src="images/blank.gif"></td>
             <tr>
                 <td width="1%" nowrap>
-                    Show group in group members' rosters:
+                    Show group in rosters for:
                 </td>
-                <%  boolean showInRoster = "true".equals(group.getProperties().get("showInRoster")) || errors.get("display") != null;
+                <%  boolean showInRoster = webManager.getRosterManager().isSharedGroup(group) || errors.get("display") != null;
                     if(!edit) { %>
                 <td colspan="2">
-                    <%= (showInRoster ? "Enabled" : "Disabled") %>
+                    <% if ("onlyGroup".equals(group.getProperties().get("sharedRoster.showInRoster"))) {
+                           out.print("Only group users");
+                       }
+                       else if ("everybody".equals(group.getProperties().get("sharedRoster.showInRoster"))) {
+                           out.print("Everybody");
+                       }
+                       else {
+                           out.print("Nobody");
+                       }
+                    %>
                 </td>
-                <% } else { %>
+                <% } else {
+                        String showInRosterType = group.getProperties().get("sharedRoster.showInRoster");
+                        if (errors.get("display") != null && newShowInRosterType != null) {
+                            showInRosterType = newShowInRosterType;
+                        }
+                %>
 
                 <td>
-                <input type="checkbox" name="newShow" value="true" <%= (showInRoster ? "checked" : "") %> onclick="refreshDisplayName(this)"/>
+                <label for="onlyGroup">Only group users</label>
+                <input type="radio" name="newShow" id="onlyGroup" value="onlyGroup" <%= ("onlyGroup".equals(showInRosterType) ? "checked" : "") %> onclick="refreshDisplayName(this)"/>&nbsp;&nbsp;
+                <label for="everybody">Everybody</label>
+                <input type="radio" name="newShow" id="everybody" value="everybody" <%= ("everybody".equals(showInRosterType) ? "checked" : "") %> onclick="refreshDisplayName(this)"/>&nbsp;&nbsp;
+                <label for="nobody">Nobody</label>
+                <input type="radio" name="newShow" id="nobody" value="nobody" <%= (!"onlyGroup".equals(showInRosterType) && !"everybody".equals(showInRosterType) ? "checked" : "") %> onclick="refreshDisplayName(this)"/>
                 </td>
 
                 <% } %>
@@ -285,7 +319,7 @@ import="java.text.DateFormat,
                 <td width="1%" nowrap>
                     Display name in roster:
                 </td>
-                <%  String displayName = (group.getProperties().get("displayName") == null ? "" : group.getProperties().get("displayName"));
+                <%  String displayName = (group.getProperties().get("sharedRoster.displayName") == null ? "" : group.getProperties().get("sharedRoster.displayName"));
                     if(!edit) { %>
                 <td colspan="2">
                     <%= displayName %>
@@ -305,6 +339,31 @@ import="java.text.DateFormat,
 
                 <% } %>
             </tr>
+            <tr>
+                <td width="1%" nowrap>
+                    Viewable by groups:
+                </td>
+                <%  boolean enableGroupList = "onlyGroup".equals(group.getProperties().get("sharedRoster.showInRoster")) || errors.get("display") != null;
+                    String groupList = (group.getProperties().get("sharedRoster.groupList") == null ? "" : group.getProperties().get("sharedRoster.groupList"));
+                    if(!edit) { %>
+                <td colspan="2">
+                    <%= groupList %>
+                </td>
+                <% } else { %>
+
+                <td>
+                <textarea name="newGroupList" cols="40" rows="2" <%= enableGroupList ? "" : "disabled"%>><%= groupList %></textarea>
+<%
+                if (errors.get("groupList") != null) {
+%>
+                <span class="jive-error-text"> Please enter existing group names separated by commas. </span>
+<%
+                }
+%>
+                </td>
+
+                <% } %>
+            </tr>
             <% if(edit) { %>
             <tr>
             <td colspan="3">