Many bug fixes for shared groups. JM-148

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

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

@@ -19,6 +19,7 @@ import org.jivesoftware.messenger.user.UserManager;
 import org.jivesoftware.messenger.*;
 import org.jivesoftware.messenger.group.GroupManager;
 import org.jivesoftware.messenger.group.Group;
+import org.jivesoftware.messenger.group.GroupNotFoundException;
 import org.jivesoftware.util.Cacheable;
 import org.jivesoftware.util.CacheSizes;
 import org.jivesoftware.util.Log;
@@ -51,6 +52,10 @@ public class Roster implements Cacheable {
     private XMPPServer server;
     private RoutingTable routingTable;
     private PresenceManager presenceManager;
+    /**
+     * Note: Used only for shared groups logic.
+     */
+    private RosterManager rosterManager;
 
 
     /**
@@ -66,14 +71,17 @@ public class Roster implements Cacheable {
      */
     public Roster(String username) {
         presenceManager = XMPPServer.getInstance().getPresenceManager();
+        rosterManager = XMPPServer.getInstance().getRosterManager();
         sessionManager = SessionManager.getInstance();
         this.username = username;
 
         // Get the shared groups of this user
         Collection<Group> sharedGroups = null;
+        Collection<Group> userGroups = null;
         try {
             User rosterUser = UserManager.getInstance().getUser(getUsername());
-            sharedGroups = XMPPServer.getInstance().getRosterManager().getSharedGroups(rosterUser);
+            sharedGroups = rosterManager.getSharedGroups(rosterUser);
+            userGroups = GroupManager.getInstance().getGroups(rosterUser);
         }
         catch (UserNotFoundException e) {
             sharedGroups = new ArrayList<Group>();
@@ -98,23 +106,40 @@ public class Roster implements Cacheable {
         Map<JID,List<Group>> sharedUsers = getSharedUsers(sharedGroups);
         for (JID jid : sharedUsers.keySet()) {
             try {
-                RosterItem.SubType type = RosterItem.SUB_TO;
+                Collection<Group> itemGroups = new ArrayList<Group>();
                 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 item = new RosterItem(jid, RosterItem.SUB_TO, RosterItem.ASK_NONE,
                         RosterItem.RECV_NONE, nickname , null);
-                if (sharedUsers.get(jid).isEmpty()) {
-                    type = RosterItem.SUB_FROM;
+                // Add the shared groups to the new roster item
+                for (Group group : sharedUsers.get(jid)) {
+                    if (group.isUser(jid.getNode())) {
+                        item.addSharedGroup(group.getProperties().get("sharedRoster.displayName"));
+                        itemGroups.add(group);
+                    }
+                    else {
+                        item.addInvisibleSharedGroup(group.getProperties().get("sharedRoster.displayName"));
+                    }
+                }
+                // Set subscription type to BOTH if the roster user belongs to a shared group
+                // that is mutually visible with a shared group of the new roster item
+                if (rosterManager.hasMutualVisibility(username, userGroups, jid.getNode(),
+                        itemGroups)) {
+                    item.setSubStatus(RosterItem.SUB_BOTH);
                 }
                 else {
+                    // Set subscription type to FROM if the contact does not belong to any of
+                    // the associated shared groups
+                    boolean belongsToGroup = false;
                     for (Group group : sharedUsers.get(jid)) {
-                        item.addSharedGroup(group.getProperties().get("sharedRoster.displayName"));
-                        if (group.isUser(username)) {
-                            type = RosterItem.SUB_BOTH;
+                        if (group.isUser(jid.getNode())) {
+                            belongsToGroup = true;
                         }
                     }
+                    if (!belongsToGroup) {
+                        item.setSubStatus(RosterItem.SUB_FROM);
+                    }
                 }
-                item.setSubStatus(type);
                 rosterItems.put(item.getJid().toBareJID(), item);
             }
             catch (UserNotFoundException e) {
@@ -410,8 +435,7 @@ public class Roster implements Cacheable {
         Map<JID,List<Group>> sharedGroupUsers = new HashMap<JID,List<Group>>();
         for (Group group : sharedGroups) {
             // Get all the users that should be in this roster
-            Collection<String> users = XMPPServer.getInstance().getRosterManager()
-                    .getSharedUsersForRoster(group, this);
+            Collection<String> users = rosterManager.getSharedUsersForRoster(group, this);
             // Add the users of the group to the general list of users to process
             for (String user : users) {
                 // Add the user to the answer if the user doesn't belong to the personal roster
@@ -423,11 +447,7 @@ public class Roster implements Cacheable {
                         groups = new ArrayList<Group>();
                         sharedGroupUsers.put(jid, groups);
                     }
-                    // Only associate the group with the user if the user is an actual user of
-                    // the group
-                    if (group.isUser(user)) {
-                        groups.add(group);
-                    }
+                    groups.add(group);
                 }
             }
         }
@@ -487,6 +507,7 @@ public class Roster implements Cacheable {
      * @param addedUser the contact to update in the roster.
      */
     void addSharedUser(Group group, String addedUser) {
+        boolean newItem = false;
         RosterItem item = null;
         JID jid = XMPPServer.getInstance().createJID(addedUser, "");
         // Get the display name of the group
@@ -498,6 +519,7 @@ public class Roster implements Cacheable {
             if (item.getSharedGroups().contains(groupName)) {
                 return;
             }
+            newItem = false;
         }
         catch (UserNotFoundException e) {
             try {
@@ -509,6 +531,7 @@ public class Roster implements Cacheable {
                                 RosterItem.RECV_NONE, nickname, null);
                 // Add the new item to the list of items
                 rosterItems.put(item.getJid().toBareJID(), item);
+                newItem = true;
             }
             catch (UserNotFoundException ex) {
                 Log.error("Group (" + groupName + ") includes non-existent username (" +
@@ -516,19 +539,134 @@ public class Roster implements Cacheable {
                         ")");
             }
         }
-        // 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);
+        // Update the subscription of the item **based on the item groups**
+        if (newItem || item.isOnlyShared()) {
+            Collection<Group> userGroups = null;
+            Collection<Group> sharedGroups = new ArrayList<Group>();
+            try {
+                User rosterUser = UserManager.getInstance().getUser(getUsername());
+                GroupManager groupManager = GroupManager.getInstance();
+                userGroups = groupManager.getGroups(rosterUser);
+                for (String name : item.getSharedGroups()) {
+                    try {
+                        sharedGroups.add(groupManager.getGroup(name));
+                    }
+                    catch (GroupNotFoundException e) {
+                    }
+                }
+                // Add the new group to the list of groups to check
+                sharedGroups.add(groupManager.getGroup(groupName));
+                // Set subscription type to BOTH if the roster user belongs to a shared group
+                // that is mutually visible with a shared group of the new roster item
+                if (rosterManager.hasMutualVisibility(getUsername(), userGroups, jid.getNode(),
+                        sharedGroups)) {
+                    item.setSubStatus(RosterItem.SUB_BOTH);
+                }
+                // Update the subscription status depending on the group membership of the new
+                // user and this user
+                else if (group.isUser(addedUser) && !group.isUser(getUsername())) {
+                    item.setSubStatus(RosterItem.SUB_TO);
+                }
+                else if (!group.isUser(addedUser) && group.isUser(getUsername())) {
+                    item.setSubStatus(RosterItem.SUB_FROM);
+                }
+            }
+            catch (UserNotFoundException e) {
+            }
+            catch (GroupNotFoundException e) {
+            }
         }
-        else if (!group.isUser(addedUser) && group.isUser(getUsername())) {
-            item.setSubStatus(RosterItem.SUB_FROM);
+
+        // Add the shared group to the list of shared groups
+        if (item.getSubStatus() != RosterItem.SUB_FROM) {
+            item.addSharedGroup(groupName);
         }
         else {
-            // Add the shared group to the list of shared groups
-            item.addSharedGroup(groupName);
+            item.addInvisibleSharedGroup(groupName);
+        }
+        // Brodcast to all the user resources of the updated roster item
+        broadcast(item);
+        // Probe the presence of the new group user
+        if (item.getSubStatus() == RosterItem.SUB_BOTH || item.getSubStatus() == RosterItem.SUB_TO) {
+            presenceManager.probePresence(username, item.getJid());
+        }
+    }
+
+    /**
+     * Adds a new contact that belongs to a certain list of groups to the roster. Depending on
+     * the contact's groups and this user's groups, the presence subscription of the roster item may
+     * vary.
+     *
+     * @param addedUser the new contact to add to the roster
+     * @param groups the groups where the contact is a member
+     */
+    void addSharedUser(String addedUser, Collection<Group> groups, Group addedGroup) {
+        boolean newItem = false;
+        RosterItem item = null;
+        JID jid = XMPPServer.getInstance().createJID(addedUser, "");
+        try {
+            // Get the RosterItem for the *local* user to add
+            item = getRosterItem(jid);
+            newItem = false;
+        }
+        catch (UserNotFoundException e) {
+            try {
+                // Create a new RosterItem for this new user
+                User user = UserManager.getInstance().getUser(addedUser);
+                String nickname = "".equals(user.getName()) ? jid.getNode() : user.getName();
+                item =
+                        new RosterItem(jid, RosterItem.SUB_BOTH, RosterItem.ASK_NONE,
+                                RosterItem.RECV_NONE, nickname, null);
+                // Add the new item to the list of items
+                rosterItems.put(item.getJid().toBareJID(), item);
+                newItem = true;
+            }
+            catch (UserNotFoundException ex) {
+                Log.error("Couldn't find a user with username (" + addedUser + ")");
+            }
+        }
+        // Update the subscription of the item **based on the item groups**
+        if (newItem || item.isOnlyShared()) {
+            Collection<Group> userGroups = null;
+            try {
+                User rosterUser = UserManager.getInstance().getUser(getUsername());
+                GroupManager groupManager = GroupManager.getInstance();
+                userGroups = groupManager.getGroups(rosterUser);
+                // Set subscription type to BOTH if the roster user belongs to a shared group
+                // that is mutually visible with a shared group of the new roster item
+                if (rosterManager.hasMutualVisibility(getUsername(), userGroups, jid.getNode(),
+                        groups)) {
+                    item.setSubStatus(RosterItem.SUB_BOTH);
+                    for (Group group : groups) {
+                        if (rosterManager.isGroupVisible(group, getUsername())) {
+                            // Get the display name of the group
+                            String groupName = group.getProperties().get("sharedRoster.displayName");
+                            // Add the shared group to the list of shared groups
+                            item.addSharedGroup(groupName);
+                        }
+                    }
+                }
+                else {
+                    // Assume by default that the contact has subscribed from the presence of
+                    // this user
+                    item.setSubStatus(RosterItem.SUB_FROM);
+                    // Check if the user may see the new contact in a shared group
+                    for (Group group : groups) {
+                        if (rosterManager.isGroupVisible(group, getUsername())) {
+                            // Get the display name of the group
+                            String groupName = group.getProperties().get("sharedRoster.displayName");
+                            // Add the shared group to the list of shared groups
+                            item.addSharedGroup(groupName);
+                            item.setSubStatus(RosterItem.SUB_TO);
+                        }
+                    }
+                    if (item.getSubStatus() == RosterItem.SUB_FROM) {
+                        item.addInvisibleSharedGroup(addedGroup.getProperties().get("sharedRoster.displayName"));
+                    }
+                }
+            }
+            catch (UserNotFoundException e) {
+            }
         }
         // Brodcast to all the user resources of the updated roster item
         broadcast(item);
@@ -553,7 +691,8 @@ public class Roster implements Cacheable {
         try {
             // Get the RosterItem for the *local* user to remove
             RosterItem item = getRosterItem(jid);
-            if (item.isOnlyShared() && item.getSharedGroups().size() == 1) {
+            int groupSize = item.getSharedGroups().size() + item.getInvisibleSharedGroups().size();
+            if (item.isOnlyShared() && groupSize == 1) {
                 // Do nothing if the existing shared group is not the sharedGroup to remove
                 if (!item.getSharedGroups().contains(sharedGroup)) {
                     return;
@@ -565,6 +704,102 @@ public class Roster implements Cacheable {
             else {
                 // Remove the removed shared group from the list of shared groups
                 item.removeSharedGroup(sharedGroup);
+                // Update the subscription of the item based on the remaining groups
+                if (item.isOnlyShared()) {
+                    Collection<Group> userGroups = null;
+                    Collection<Group> sharedGroups = new ArrayList<Group>();
+                    try {
+                        User rosterUser = UserManager.getInstance().getUser(getUsername());
+                        GroupManager groupManager = GroupManager.getInstance();
+                        userGroups = groupManager.getGroups(rosterUser);
+                        for (String groupName : item.getSharedGroups()) {
+                            try {
+                                sharedGroups.add(groupManager.getGroup(groupName));
+                            }
+                            catch (GroupNotFoundException e) {
+                            }
+                        }
+                        // Set subscription type to BOTH if the roster user belongs to a shared group
+                        // that is mutually visible with a shared group of the new roster item
+                        if (rosterManager.hasMutualVisibility(getUsername(), userGroups,
+                                jid.getNode(), sharedGroups)) {
+                            item.setSubStatus(RosterItem.SUB_BOTH);
+                        }
+                        else if (item.getSharedGroups().isEmpty() &&
+                                !item.getInvisibleSharedGroups().isEmpty()) {
+                            item.setSubStatus(RosterItem.SUB_FROM);
+                        }
+                        else {
+                            item.setSubStatus(RosterItem.SUB_TO);
+                        }
+
+                    }
+                    catch (UserNotFoundException e) {
+                    }
+                }
+                // Brodcast to all the user resources of the updated roster item
+                broadcast(item);
+            }
+        }
+        catch (SharedGroupException e) {
+            // Do nothing. Checkings are disabled so this exception should never happen.
+        }
+        catch (UserNotFoundException e) {
+            // Do nothing since the contact does not exist in the user's roster. (strange case!)
+        }
+    }
+
+    void deleteSharedUser(String deletedUser, Collection<Group> groups, Group deletedGroup) {
+        JID jid = XMPPServer.getInstance().createJID(deletedUser, "");
+        try {
+            // Get the RosterItem for the *local* user to remove
+            RosterItem item = getRosterItem(jid);
+            int groupSize = item.getSharedGroups().size() + item.getInvisibleSharedGroups().size();
+            if (item.isOnlyShared() && groupSize == 1) {
+                // Delete the roster item from the roster since it exists only because of this
+                // group which is being removed
+                deleteRosterItem(jid, false);
+            }
+            else {
+                item.removeSharedGroup(deletedGroup.getProperties().get("sharedRoster.displayName"));
+                // Remove all invalid shared groups from the roster item
+                for (Group group : groups) {
+                    if (!rosterManager.isGroupVisible(group, getUsername())) {
+                        // Get the display name of the group
+                        String groupName = group.getProperties().get("sharedRoster.displayName");
+                        // Remove the shared group from the list of shared groups
+                        item.removeSharedGroup(groupName);
+                    }
+                }
+
+                // Update the subscription of the item **based on the item groups**
+                if (item.isOnlyShared()) {
+                    Collection<Group> userGroups = null;
+                    try {
+                        User rosterUser = UserManager.getInstance().getUser(getUsername());
+                        GroupManager groupManager = GroupManager.getInstance();
+                        userGroups = groupManager.getGroups(rosterUser);
+                        // Set subscription type to BOTH if the roster user belongs to a shared group
+                        // that is mutually visible with a shared group of the new roster item
+                        if (rosterManager.hasMutualVisibility(getUsername(), userGroups,
+                                jid.getNode(), groups)) {
+                            item.setSubStatus(RosterItem.SUB_BOTH);
+                        }
+                        else {
+                            // Assume by default that the contact has subscribed from the presence of
+                            // this user
+                            item.setSubStatus(RosterItem.SUB_FROM);
+                            // Check if the user may see the new contact in a shared group
+                            for (Group group : groups) {
+                                if (rosterManager.isGroupVisible(group, getUsername())) {
+                                    item.setSubStatus(RosterItem.SUB_TO);
+                                }
+                            }
+                        }
+                    }
+                    catch (UserNotFoundException e) {
+                    }
+                }
                 // Brodcast to all the user resources of the updated roster item
                 broadcast(item);
             }

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

@@ -125,6 +125,7 @@ public class RosterItem implements Cacheable {
     protected String nickname;
     protected List<String> groups;
     protected Set<String> sharedGroups = new HashSet<String>();
+    protected Set<String> invisibleSharedGroups = new HashSet<String>();
     protected SubType subStatus;
     protected AskType askStatus;
     private long rosterID;
@@ -343,6 +344,17 @@ public class RosterItem implements Cacheable {
         return sharedGroups;
     }
 
+    /**
+     * Returns the invisible shared groups for the item. These groups are for internal use
+     * and help track the reason why a roster item has a presence subscription of type FROM
+     * when using shared groups.
+     *
+     * @return The shared groups this item belongs to.
+     */
+    public Collection<String> getInvisibleSharedGroups() {
+        return invisibleSharedGroups;
+    }
+
     /**
      * Adds a new group to the shared groups list.
      *
@@ -350,6 +362,18 @@ public class RosterItem implements Cacheable {
      */
     public void addSharedGroup(String sharedGroup) {
         sharedGroups.add(sharedGroup);
+        invisibleSharedGroups.remove(sharedGroup);
+    }
+
+    /**
+     * Adds a new group to the list shared groups that won't be sent to the user. These groups
+     * are for internal use and help track the reason why a roster item has a presence
+     * subscription of type FROM when using shared groups.
+     *
+     * @param sharedGroup The shared group to add to the list of shared groups.
+     */
+    public void addInvisibleSharedGroup(String sharedGroup) {
+        invisibleSharedGroups.add(sharedGroup);
     }
 
     /**
@@ -359,6 +383,7 @@ public class RosterItem implements Cacheable {
      */
     public void removeSharedGroup(String sharedGroup) {
         sharedGroups.remove(sharedGroup);
+        invisibleSharedGroups.remove(sharedGroup);
     }
 
     /**
@@ -368,7 +393,7 @@ public class RosterItem implements Cacheable {
      * @return true if this item belongs to a shared group.
      */
     public boolean isShared() {
-        return !sharedGroups.isEmpty();
+        return !sharedGroups.isEmpty() || !invisibleSharedGroups.isEmpty();
     }
 
     /**
@@ -379,7 +404,7 @@ public class RosterItem implements Cacheable {
      * @return true if this item belongs ONLY to shared groups.
      */
     public boolean isOnlyShared() {
-        return !sharedGroups.isEmpty() && groups.isEmpty();
+        return isShared() && groups.isEmpty();
     }
 
     /**

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

@@ -19,7 +19,6 @@ 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;
@@ -218,20 +217,25 @@ public class RosterManager extends BasicModule implements GroupEventListener {
             if (currentValue.equals(originalValue)) {
                 return;
             }
-            // Remove the group from all users's rosters (if the group was a shared group)
-            Collection<String> users = getAffectedUsers(group, originalValue,
+            // Get the users of the group
+            Collection<String> users = new HashSet<String>(group.getMembers());
+            users.addAll(group.getAdmins());
+            // Get the users whose roster will be affected
+            Collection<String> affectedUsers = getAffectedUsers(group, originalValue,
                     group.getProperties().get("sharedRoster.groupList"));
+            // Remove the group members from the affected rosters
             for (String deletedUser : users) {
-                groupUserDeleted(group, users, deletedUser);
+                groupUserDeleted(group, affectedUsers, deletedUser);
             }
 
-            // Get the users of the group
-            users = new HashSet<String>(group.getMembers());
-            users.addAll(group.getAdmins());
             // Simulate that the group users has been added to the group. This will cause to push
             // roster items to the "affected" users for the group users
+            //Collection<Group> visibleGroups = getVisibleGroups(group);
             for (String user : users) {
                 groupUserAdded(group, user);
+                /*for (Group visibleGroup : visibleGroups) {
+                    addSharedGroupToRoster(visibleGroup, user);
+                }*/
             }
         }
         else if ("sharedRoster.groupList".equals(keyChanged)) {
@@ -240,20 +244,25 @@ public class RosterManager extends BasicModule implements GroupEventListener {
             if (currentValue.equals(originalValue)) {
                 return;
             }
-            // Remove the group from all users's rosters (if the group was a shared group)
-            Collection<String> users = getAffectedUsers(group,
+            // Get the users of the group
+            Collection<String> users = new HashSet<String>(group.getMembers());
+            users.addAll(group.getAdmins());
+            // Get the users whose roster will be affected
+            Collection<String> affectedUsers = getAffectedUsers(group,
                     group.getProperties().get("sharedRoster.showInRoster"), originalValue);
+            // Remove the group members from the affected rosters
             for (String deletedUser : users) {
-                groupUserDeleted(group, users, deletedUser);
+                groupUserDeleted(group, affectedUsers, deletedUser);
             }
 
-            // Get the users of the group
-            users = new HashSet<String>(group.getMembers());
-            users.addAll(group.getAdmins());
             // Simulate that the group users has been added to the group. This will cause to push
             // roster items to the "affected" users for the group users
+            //Collection<Group> visibleGroups = getVisibleGroups(group);
             for (String user : users) {
                 groupUserAdded(group, user);
+                /*for (Group visibleGroup : visibleGroups) {
+                    addSharedGroupToRoster(visibleGroup, user);
+                }*/
             }
         }
         else if ("sharedRoster.displayName".equals(keyChanged)) {
@@ -365,10 +374,14 @@ public class RosterManager extends BasicModule implements GroupEventListener {
                 if (roster != null) {
                     roster.addSharedUser(group, addedUser);
                 }
-                // 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);
+                // Update the roster of the newly added group user.
+                if (addedUserRoster != null) {
+                    try {
+                        User user = UserManager.getInstance().getUser(userToUpdate);
+                        Collection<Group> groups = GroupManager.getInstance().getGroups(user);
+                        addedUserRoster.addSharedUser(userToUpdate, groups, group);
+                    }
+                    catch (UserNotFoundException e) {}
                 }
             }
         }
@@ -405,10 +418,14 @@ public class RosterManager extends BasicModule implements GroupEventListener {
             if (roster != null) {
                 roster.deleteSharedUser(groupName, deletedUser);
             }
-            // 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);
+            // Update the roster of the newly deleted group user.
+            if (deletedUserRoster != null) {
+                try {
+                    User user = UserManager.getInstance().getUser(userToUpdate);
+                    Collection<Group> groups = GroupManager.getInstance().getGroups(user);
+                    deletedUserRoster.deleteSharedUser(userToUpdate, groups, group);
+                }
+                catch (UserNotFoundException e) {}
             }
         }
     }
@@ -417,6 +434,9 @@ public class RosterManager extends BasicModule implements GroupEventListener {
         Collection<Group> answer = new HashSet<Group>();
         Collection<Group> groups = GroupManager.getInstance().getGroups();
         for (Group group : groups) {
+            if (groupToCheck == group) {
+                continue;
+            }
             String showInRoster = group.getProperties().get("sharedRoster.showInRoster");
             if ("onlyGroup".equals(showInRoster)) {
                 // Check if the user belongs to a group that may see this group
@@ -426,10 +446,48 @@ public class RosterManager extends BasicModule implements GroupEventListener {
                     answer.add(group);
                 }
             }
+            else if ("everybody".equals(showInRoster)) {
+                answer.add(group);
+            }
         }
         return answer;
     }
 
+    /**
+     * Returns true if a given group is visible to a given user. That means, if the user can
+     * see the group in his roster.
+     *
+     * @param group the group to check if the user can see.
+     * @param username the user to check if he may see the group.
+     * @return true if a given group is visible to a given user.
+     */
+    boolean isGroupVisible(Group group, String username) {
+        String showInRoster = group.getProperties().get("sharedRoster.showInRoster");
+        if ("everybody".equals(showInRoster)) {
+            return true;
+        }
+        else if ("onlyGroup".equals(showInRoster)) {
+            if (group.isUser(username)) {
+                 return true;
+            }
+            // 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(username)) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Adds the group users of the given shared group to the roster of the specified user.
+     *
+     * @param group the shared group to add to the roster of a user.
+     * @param username the name of the user to add a shared group to his roster.
+     */
     private void addSharedGroupToRoster(Group group, String username) {
         // Get the group users to add to the user's roster
         Collection<String> users = new HashSet<String>(group.getMembers());
@@ -448,7 +506,12 @@ public class RosterManager extends BasicModule implements GroupEventListener {
             }
             // Update the roster of the user
             if (userRoster != null) {
-                userRoster.addSharedUser(group, userToAdd);
+                try {
+                    User user = UserManager.getInstance().getUser(userToAdd);
+                    Collection<Group> groups = GroupManager.getInstance().getGroups(user);
+                    userRoster.addSharedUser(userToAdd, groups, group);
+                }
+                catch (UserNotFoundException e) {}
             }
         }
     }
@@ -473,7 +536,12 @@ public class RosterManager extends BasicModule implements GroupEventListener {
             }
             // Update the roster of the user
             if (userRoster != null) {
-                userRoster.deleteSharedUser(groupName, userToRemove);
+                try {
+                    User user = UserManager.getInstance().getUser(userToRemove);
+                    Collection<Group> groups = GroupManager.getInstance().getGroups(user);
+                    userRoster.deleteSharedUser(userToRemove, groups, group);
+                }
+                catch (UserNotFoundException e) {}
             }
         }
     }
@@ -493,7 +561,7 @@ public class RosterManager extends BasicModule implements GroupEventListener {
     }
 
     /**
-     * This method is similar to {@link #getRelatedUsers(Group, boolean)} except that it receives
+     * This method is similar to {@link #getAffectedUsers(Group)} 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.
      *
@@ -510,9 +578,13 @@ public class RosterManager extends BasicModule implements GroupEventListener {
         users.addAll(group.getAdmins());
         // Check if anyone can see this shared group
         if ("everybody".equals(showInRoster)) {
+            // Add all users in the system
+            for (User user : UserManager.getInstance().getUsers()) {
+                users.add(user.getUsername());
+            }
             // 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());
+            //users.addAll(SessionManager.getInstance().getSessionUsers());
         }
         else {
             // Add the users that may see the group
@@ -559,4 +631,50 @@ public class RosterManager extends BasicModule implements GroupEventListener {
         }
         return users;
     }
+
+    /**
+     * Returns true if a group in the first collection may mutually see a group of the
+     * second collection. More precisely, return true if both collections contain a public
+     * group (i.e. anybody can see the group) or if both collection have a group that may see
+     * each other and the users are members of those groups.
+     *
+     * @param user the name of the user associated to the first collection of groups.
+     * @param groups a collection of groups to check against the other collection of groups.
+     * @param otherUser the name of the user associated to the second collection of groups.
+     * @param otherGroups the other collection of groups to check against the first collection.
+     * @return true if a group in the first collection may mutually see a group of the
+     *         second collection.
+     */
+    boolean hasMutualVisibility(String user, Collection<Group> groups, String otherUser,
+            Collection<Group> otherGroups) {
+        for (Group group : groups) {
+            for (Group otherGroup : otherGroups) {
+                // Skip this groups if the users are not group users of the groups
+                if (!group.isUser(user) || !otherGroup.isUser(otherUser)) {
+                    continue;
+                }
+                if (group == otherGroup) {
+                     return true;
+                }
+                String showInRoster = group.getProperties().get("sharedRoster.showInRoster");
+                String otherShowInRoster = otherGroup.getProperties().get("sharedRoster.showInRoster");
+                // Return true if both groups are public groups (i.e. anybody can see them)
+                if ("everybody".equals(showInRoster) && "everybody".equals(otherShowInRoster)) {
+                    return true;
+                }
+                else if ("onlyGroup".equals(showInRoster) && "onlyGroup".equals(otherShowInRoster)) {
+                    String groupNames = group.getProperties().get("sharedRoster.groupList");
+                    String otherGroupNames = otherGroup.getProperties().get("sharedRoster.groupList");
+                    // Return true if each group may see the other group
+                    if (groupNames != null && otherGroupNames != null) {
+                        if (groupNames.contains(otherGroup.getName()) &&
+                                otherGroupNames.contains(group.getName())) {
+                            return true;
+                        }
+                    }
+                }
+            }
+        }
+        return false;
+    }
 }
\ No newline at end of file