1) Users are not being loaded when using public shared groups. JM-222

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

src/java/org/jivesoftware/wildfire/handler/IQSharedGroupHandler.java

@@ -6,8 +6,6 @@ import org.jivesoftware.wildfire.XMPPServer;
 import org.jivesoftware.wildfire.auth.UnauthorizedException;
 import org.jivesoftware.wildfire.group.Group;
 import org.jivesoftware.wildfire.roster.RosterManager;
-import org.jivesoftware.wildfire.user.UserManager;
-import org.jivesoftware.wildfire.user.UserNotFoundException;
 import org.xmpp.packet.IQ;
 import org.xmpp.packet.PacketError;
 
@@ -24,7 +22,6 @@ public class IQSharedGroupHandler extends IQHandler {
 
     private IQHandlerInfo info;
     private String serverName;
-    private UserManager userManager;
     private RosterManager rosterManager;
 
     public IQSharedGroupHandler() {
@@ -44,8 +41,7 @@ public class IQSharedGroupHandler extends IQHandler {
             return result;
         }
 
-        try {
-            Collection<Group> groups = rosterManager.getSharedGroups(userManager.getUser(username));
+        Collection<Group> groups = rosterManager.getSharedGroups(username);
         Element sharedGroups = result.setChildElement("sharedgroup",
                 "http://www.jivesoftware.org/protocol/sharedgroup");
         for (Group sharedGroup : groups) {
@@ -54,12 +50,6 @@ public class IQSharedGroupHandler extends IQHandler {
                 sharedGroups.addElement("group").setText(displayName);
             }
         }
-        }
-        catch (UserNotFoundException e) {
-            // User not found return an error. This case should never happen.
-            result.setChildElement(packet.getChildElement().createCopy());
-            result.setError(PacketError.Condition.not_allowed);
-        }
         return result;
     }
 
@@ -70,7 +60,6 @@ public class IQSharedGroupHandler extends IQHandler {
     public void initialize(XMPPServer server) {
         super.initialize(server);
         serverName = server.getServerInfo().getName();
-        userManager = server.getUserManager();
         rosterManager = server.getRosterManager();
     }
 }

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

@@ -22,7 +22,9 @@ import org.jivesoftware.wildfire.group.Group;
 import org.jivesoftware.wildfire.group.GroupManager;
 import org.jivesoftware.wildfire.privacy.PrivacyList;
 import org.jivesoftware.wildfire.privacy.PrivacyListManager;
-import org.jivesoftware.wildfire.user.*;
+import org.jivesoftware.wildfire.user.UserAlreadyExistsException;
+import org.jivesoftware.wildfire.user.UserNameManager;
+import org.jivesoftware.wildfire.user.UserNotFoundException;
 import org.xmpp.packet.IQ;
 import org.xmpp.packet.JID;
 import org.xmpp.packet.Presence;
@@ -79,16 +81,8 @@ public class Roster implements Cacheable {
         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 = rosterManager.getSharedGroups(rosterUser);
-            userGroups = GroupManager.getInstance().getGroups(getUserJID());
-        }
-        catch (UserNotFoundException e) {
-            sharedGroups = new ArrayList<Group>();
-        }
+        Collection<Group> sharedGroups = rosterManager.getSharedGroups(username);
+        Collection<Group> userGroups = GroupManager.getInstance().getGroups(getUserJID());;
 
         // Add RosterItems that belong to the personal roster
         rosterItemProvider =  RosterItemProvider.getInstance();
@@ -111,7 +105,7 @@ public class Roster implements Cacheable {
         for (JID jid : sharedUsers.keySet()) {
             try {
                 Collection<Group> itemGroups = new ArrayList<Group>();
-                String nickname = UserNameManager.getUserName(jid);
+                String nickname = "";
                 RosterItem item = new RosterItem(jid, RosterItem.SUB_TO, RosterItem.ASK_NONE,
                         RosterItem.RECV_NONE, nickname , null);
                 // Add the shared groups to the new roster item
@@ -142,8 +136,17 @@ public class Roster implements Cacheable {
                         item.setSubStatus(RosterItem.SUB_FROM);
                     }
                 }
+                // Set nickname and store in memory only if subscription type is not FROM.
+                // Roster items with subscription type FROM that exist only because of shared
+                // groups will be recreated on demand in #getRosterItem(JID) and #isRosterItem()
+                // but will never be stored in memory nor in the database. This is an important
+                // optimization to reduce objects in memory and avoid loading users in memory
+                // to get their nicknames that will never be shown
+                if (item.getSubStatus() != RosterItem.SUB_FROM) {
+                    item.setNickname(UserNameManager.getUserName(jid));
                     rosterItems.put(item.getJid().toBareJID(), item);
                 }
+            }
             catch (UserNotFoundException e) {
                 Log.error("Groups (" + sharedUsers.get(jid) + ") include non-existent username (" +
                         jid.getNode() +
@@ -159,11 +162,16 @@ public class Roster implements Cacheable {
      * @return true if the specified user is a member of the roster, false otherwise.
      */
     public boolean isRosterItem(JID user) {
-        return rosterItems.containsKey(user.toBareJID());
+        // Optimization: Check if the contact has a FROM subscription due to shared groups
+        // (only when not present in the rosterItems collection)
+        return rosterItems.containsKey(user.toBareJID()) || getImplicitRosterItem(user) != null;
     }
 
     /**
-     * Returns a collection of users in this roster.
+     * Returns a collection of users in this roster.<p>
+     *
+     * Note: Roster items with subscription type FROM that exist only because of shared groups
+     * are not going to be returned.
      *
      * @return a collection of users in this roster.
      */
@@ -171,15 +179,6 @@ public class Roster implements Cacheable {
         return Collections.unmodifiableCollection(rosterItems.values());
     }
 
-    /**
-     * Returns the total number of users in the roster.
-     *
-     * @return the number of online users in the roster.
-     */
-    public int getTotalRosterItemCount() {
-        return rosterItems.size();
-    }
-
     /**
      * Gets a user from the roster. If the roster item does not exist, an empty one is created.
      * The new roster item is not stored in the roster until it is added using
@@ -190,12 +189,59 @@ public class Roster implements Cacheable {
      */
     public RosterItem getRosterItem(JID user) throws UserNotFoundException {
         RosterItem item = rosterItems.get(user.toBareJID());
+        if (item == null) {
+            // Optimization: Check if the contact has a FROM subscription due to shared groups
+            item = getImplicitRosterItem(user);
             if (item == null) {
                 throw new UserNotFoundException(user.toBareJID());
             }
+        }
         return item;
     }
 
+    /**
+     * Returns a roster item if the specified user has a subscription of type FROM to this
+     * user and the susbcription only exists due to some shared groups or otherwise
+     * <tt>null</tt>. This method assumes that this user does not have a subscription to
+     * the contact. In other words, this method will not check if there should be a subscription
+     * of type TO ot BOTH.
+     *
+     * @param user the contact to check if he is subscribed to the presence of this user.
+     * @return a roster item if the specified user has a subscription of type FROM to this
+     *         user and the susbcription only exists due to some shared groups or otherwise null.
+     */
+    private RosterItem getImplicitRosterItem(JID user) {
+        // Get the groups of this user
+        Collection<Group> userGroups = GroupManager.getInstance().getGroups(getUserJID());
+        Collection<Group> sharedGroups = new ArrayList<Group>(userGroups.size());
+        // Check if there is a public shared group. That means that anyone can see this user.
+        for (Group group : userGroups) {
+            if (rosterManager.isPulicSharedGroup(group)) {
+                return new RosterItem(user, RosterItem.SUB_FROM, RosterItem.ASK_NONE,
+                        RosterItem.RECV_NONE, "", null);
+            }
+            else if (rosterManager.isSharedGroup(group)) {
+                // This is a shared group that can be seen only by group members or
+                // (maybe) by other groups
+                sharedGroups.add(group);
+            }
+        }
+        if (!sharedGroups.isEmpty()) {
+            // Check if any group of contact may see a shared group of this user
+            Collection<Group> contactGroups = GroupManager.getInstance().getGroups(user);
+            if (contactGroups == null) {
+                return null;
+            }
+            for (Group sharedGroup : sharedGroups) {
+                if (rosterManager.isSharedGroupVisible(sharedGroup, contactGroups)) {
+                    return new RosterItem(user, RosterItem.SUB_FROM, RosterItem.ASK_NONE,
+                            RosterItem.RECV_NONE, "", null);
+                }
+            }
+        }
+        return null;
+    }
+
     /**
      * Create a new item to the roster. Roster items may not be created that contain the same user
      * address as an existing item.
@@ -517,7 +563,8 @@ public class Roster implements Cacheable {
             for (JID jid : 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)
-                if (!isRosterItem(jid) && !userJID.equals(jid)) {
+                boolean isRosterItem = rosterItems.containsKey(jid.toBareJID());
+                if (!isRosterItem && !userJID.equals(jid)) {
                     List<Group> groups = sharedGroupUsers.get(jid);
                     if (groups == null) {
                         groups = new ArrayList<Group>();
@@ -681,13 +728,21 @@ public class Roster implements Cacheable {
             }
         }
 
+        // Optimization: Check if we do not need to keep the item in memory
+        if (item.isOnlyShared() && item.getSubStatus() == RosterItem.SUB_FROM) {
+            // Remove from memory and do nothing else
+            rosterItems.remove(item.getJid().toBareJID());
+        }
+        else {
             // Brodcast to all the user resources of the updated roster item
             broadcast(item, true);
             // Probe the presence of the new group user
-        if (item.getSubStatus() == RosterItem.SUB_BOTH || item.getSubStatus() == RosterItem.SUB_TO) {
+            if (item.getSubStatus() == RosterItem.SUB_BOTH ||
+                    item.getSubStatus() == RosterItem.SUB_TO) {
                 probePresence(item.getJid());
             }
         }
+    }
 
     /**
      * Adds a new contact that belongs to a certain list of groups to the roster. Depending on
@@ -778,13 +833,21 @@ public class Roster implements Cacheable {
                 }
             }
         }
+        // Optimization: Check if we do not need to keep the item in memory
+        if (item.isOnlyShared() && item.getSubStatus() == RosterItem.SUB_FROM) {
+            // Remove from memory and do nothing else
+            rosterItems.remove(item.getJid().toBareJID());
+        }
+        else {
             // Brodcast to all the user resources of the updated roster item
             broadcast(item, true);
             // Probe the presence of the new group user
-        if (item.getSubStatus() == RosterItem.SUB_BOTH || item.getSubStatus() == RosterItem.SUB_TO) {
+            if (item.getSubStatus() == RosterItem.SUB_BOTH ||
+                    item.getSubStatus() == RosterItem.SUB_TO) {
                 probePresence(item.getJid());
             }
         }
+    }
 
     /**
      * Update the roster since a group user has been deleted from a shared group. If the RosterItem

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

@@ -25,7 +25,6 @@ import org.jivesoftware.wildfire.event.GroupEventListener;
 import org.jivesoftware.wildfire.group.Group;
 import org.jivesoftware.wildfire.group.GroupManager;
 import org.jivesoftware.wildfire.group.GroupNotFoundException;
-import org.jivesoftware.wildfire.user.User;
 import org.jivesoftware.wildfire.user.UserManager;
 import org.jivesoftware.wildfire.user.UserNotFoundException;
 import org.xmpp.packet.JID;
@@ -162,16 +161,16 @@ public class RosterManager extends BasicModule implements GroupEventListener {
      * 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.
+     * @param username the username of 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) {
+    public Collection<Group> getSharedGroups(String username) {
         Collection<Group> answer = new HashSet<Group>();
         Collection<Group> groups = GroupManager.getInstance().getSharedGroups();
         for (Group group : groups) {
             String showInRoster = group.getProperties().get("sharedRoster.showInRoster");
             if ("onlyGroup".equals(showInRoster)) {
-                if (group.isUser(user.getUsername())) {
+                if (group.isUser(username)) {
                     // The user belongs to the group so add the group to the answer
                     answer.add(group);
                 }
@@ -179,7 +178,7 @@ public class RosterManager extends BasicModule implements GroupEventListener {
                     // 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())) {
+                        if (groupInList.isUser(username)) {
                             answer.add(group);
                         }
                     }
@@ -334,6 +333,42 @@ public class RosterManager extends BasicModule implements GroupEventListener {
         return false;
     }
 
+    /**
+     * Returns true if the specified Group may be seen by all users in the system. 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 seen by all users in the system.
+     * @return true if the specified Group may be seen by all users in the system.
+     */
+    public boolean isPulicSharedGroup(Group group) {
+        String showInRoster = group.getProperties().get("sharedRoster.showInRoster");
+        if ("everybody".equals(showInRoster)) {
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Returns true if a shared group may be seen by any of the specified groups. The groups
+     * contained in the collection may or may not be a shared groups.
+     *
+     * @param sharedGroup the shared group to check its visibility.
+     * @param groups the groups to check if any of them can see the shared group.
+     * @return true if any of the specified groups can see the shared group.
+     */
+    public boolean isSharedGroupVisible(Group sharedGroup, Collection<Group> groups) {
+        Map<String, String> properties = sharedGroup.getProperties();
+        String showInRoster = properties.get("sharedRoster.showInRoster");
+        if ("everybody".equals(showInRoster)) {
+            return true;
+        }
+        if ("onlyGroup".equals(showInRoster) ) {
+            Collection<Group> visibleGroups = parseGroups(properties.get("sharedRoster.groupList"));
+            return !Collections.disjoint(visibleGroups, groups);
+        }
+        return false;
+    }
+
     public void memberAdded(Group group, Map params) {
         JID addedUser = new JID((String) params.get("member"));
         // Do nothing if the user was an admin that became a member
@@ -661,8 +696,8 @@ public class RosterManager extends BasicModule implements GroupEventListener {
         // 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(server.createJID(user.getUsername(), null));
+            for (String username : UserManager.getInstance().getUsernames()) {
+                users.add(server.createJID(username, null));
             }
             // Add all logged users. We don't need to add all users in the system since only the
             // logged ones will be affected.
@@ -698,8 +733,8 @@ public class RosterManager extends BasicModule implements GroupEventListener {
             // in the system since they all need to be in the roster with subscription "from"
             if (group.isUser(roster.getUsername())) {
                 // Add all users in the system
-                for (User user : UserManager.getInstance().getUsers()) {
-                    users.add(server.createJID(user.getUsername(),null));
+                for (String username : UserManager.getInstance().getUsernames()) {
+                    users.add(server.createJID(username, null));
                 }
             }
         }

src/java/org/jivesoftware/wildfire/user/UserManager.java

@@ -194,6 +194,15 @@ public class UserManager implements IQResultListener {
         return provider.getUsers();
     }
 
+    /**
+     * Returns an unmodifiable Collection of usernames of all users in the system.
+     *
+     * @return an unmodifiable Collection of all usernames in the system.
+     */
+    public Collection<String> getUsernames() {
+        return provider.getUsernames();
+    }
+
     /**
      * Returns an unmodifiable Collection of all users starting at <tt>startIndex</tt>
      * with the given number of results. This is useful to support pagination in a GUI