Merge branch 'master' into bigmerge

Conflicts:
	OpenSim/Region/Framework/Scenes/SceneCommunicationService.cs

3 files changed, 104 insertions, 38 deletions

diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs
index 0c81bb2..5d94ff7 100644
--- a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs
@@ -29,6 +29,7 @@ using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Reflection;
+using System.Threading;
 using log4net;
 using Nini.Config;
 using Nwc.XmlRpc;
@@ -79,10 +80,20 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
         protected IFriendsService m_FriendsService = null;
         protected FriendsSimConnector m_FriendsSimConnector;
 
-        protected Dictionary<UUID, UserFriendData> m_Friends =
-                new Dictionary<UUID, UserFriendData>();
+        /// <summary>
+        /// Cache friends lists for users.
+        /// </summary>
+        /// <remarks>
+        /// This is a complex and error-prone thing to do.  At the moment, we assume that the efficiency gained in
+        /// permissions checks outweighs the disadvantages of that complexity.
+        /// </remarks>
+        protected Dictionary<UUID, UserFriendData> m_Friends = new Dictionary<UUID, UserFriendData>();
 
-        protected HashSet<UUID> m_NeedsListOfFriends = new HashSet<UUID>();
+        /// <summary>
+        /// Maintain a record of viewers that need to be sent notifications for friends that are online.  This only
+        /// needs to be done on login.  Subsequent online/offline friend changes are sent by a different mechanism.
+        /// </summary>
+        protected HashSet<UUID> m_NeedsListOfOnlineFriends = new HashSet<UUID>();
 
         protected IPresenceService PresenceService
         {
@@ -189,6 +200,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
         {
             if (!m_Enabled)
                 return;
+
             m_log.DebugFormat("[FRIENDS MODULE]: AddRegion on {0}", Name);
 
             m_Scenes.Add(scene);
@@ -241,16 +253,25 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             client.OnInstantMessage += OnInstantMessage;
             client.OnApproveFriendRequest += OnApproveFriendRequest;
             client.OnDenyFriendRequest += OnDenyFriendRequest;
-            client.OnTerminateFriendship += OnTerminateFriendship;
+            client.OnTerminateFriendship += (thisClient, agentID, exfriendID) => RemoveFriendship(thisClient, exfriendID);
             client.OnGrantUserRights += OnGrantUserRights;
 
-            Util.FireAndForget(delegate { FetchFriendslist(client); });
+            // Do not do this asynchronously.  If we do, then subsequent code can outrace CacheFriends() and
+            // return misleading results from the still empty friends cache.
+            // If we absolutely need to do this asynchronously, then a signalling mechanism is needed so that calls
+            // to GetFriends() will wait until CacheFriends() completes.  Locks are insufficient.
+            CacheFriends(client);
         }
 
-        /// Fetch the friends list or increment the refcount for the existing 
-        /// friends list
+        /// <summary>
+        /// Cache the friends list or increment the refcount for the existing friends list.
+        /// </summary>
+        /// <param name="client">
+        /// </param>
+        /// <returns>
         /// Returns true if the list was fetched, false if it wasn't
-        protected virtual bool FetchFriendslist(IClientAPI client)
+        /// </returns>
+        protected virtual bool CacheFriends(IClientAPI client)
         {
             UUID agentID = client.AgentId;
             lock (m_Friends)
@@ -297,7 +318,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
 
         private void OnMakeRootAgent(ScenePresence sp)
         {
-            RefetchFriends(sp.ControllingClient);
+            RecacheFriends(sp.ControllingClient);
         }
 
         private void OnClientLogin(IClientAPI client)
@@ -309,8 +330,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             StatusChange(agentID, true);
 
             // Register that we need to send the list of online friends to this user
-            lock (m_NeedsListOfFriends)
-                m_NeedsListOfFriends.Add(agentID);
+            lock (m_NeedsListOfOnlineFriends)
+                m_NeedsListOfOnlineFriends.Add(agentID);
         }
 
         public virtual bool SendFriendsOnlineIfNeeded(IClientAPI client)
@@ -318,9 +339,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             UUID agentID = client.AgentId;
 
             // Check if the online friends list is needed
-            lock (m_NeedsListOfFriends)
+            lock (m_NeedsListOfOnlineFriends)
             {
-                if (!m_NeedsListOfFriends.Remove(agentID))
+                if (!m_NeedsListOfOnlineFriends.Remove(agentID))
                     return false;
             }
 
@@ -328,7 +349,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             List<UUID> online = GetOnlineFriends(agentID);
             if (online.Count > 0)
             {
-                m_log.DebugFormat("[FRIENDS MODULE]: User {0} in region {1} has {2} friends online", client.AgentId, client.Scene.RegionInfo.RegionName, online.Count);
+                m_log.DebugFormat(
+                    "[FRIENDS MODULE]: User {0} in region {1} has {2} friends online",
+                    client.Name, client.Scene.RegionInfo.RegionName, online.Count);
+
                 client.SendAgentOnline(online.ToArray());
             }
 
@@ -586,7 +610,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             }
 
             // Update the local cache
-            RefetchFriends(client);
+            RecacheFriends(client);
 
             //
             // Notify the friend
@@ -641,14 +665,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                 }
             }
         }
-
-        private void OnTerminateFriendship(IClientAPI client, UUID agentID, UUID exfriendID)
+        
+        public void RemoveFriendship(IClientAPI client, UUID exfriendID)
         {
-            if (!DeleteFriendship(agentID, exfriendID))
+            if (!DeleteFriendship(client.AgentId, exfriendID))
                 client.SendAlertMessage("Unable to terminate friendship on this sim.");
 
             // Update local cache
-            RefetchFriends(client);
+            RecacheFriends(client);
 
             client.SendTerminateFriend(exfriendID);
 
@@ -667,9 +691,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                 if (friendSession != null)
                 {
                     GridRegion region = GridService.GetRegionByUUID(m_Scenes[0].RegionInfo.ScopeID, friendSession.RegionID);
-                    m_FriendsSimConnector.FriendshipTerminated(region, agentID, exfriendID);
+                    m_FriendsSimConnector.FriendshipTerminated(region, client.AgentId, exfriendID);
                 }
-            }
+            }            
         }
 
         private void OnGrantUserRights(IClientAPI remoteClient, UUID requester, UUID target, int rights)
@@ -769,7 +793,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
 
 
                 // Update the local cache
-                RefetchFriends(friendClient);
+                RecacheFriends(friendClient);
 
                 // we're done
                 return true;
@@ -802,7 +826,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                 // the friend in this sim as root agent
                 friendClient.SendTerminateFriend(exfriendID);
                 // update local cache
-                RefetchFriends(friendClient);
+                RecacheFriends(friendClient);
                 // we're done
                 return true;
             }
@@ -819,16 +843,15 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                 if (onlineBitChanged)
                 {
                     if ((rights & (int)FriendRights.CanSeeOnline) == 1)
-                        friendClient.SendAgentOnline(new UUID[] { new UUID(userID) });
+                        friendClient.SendAgentOnline(new UUID[] { userID });
                     else
-                        friendClient.SendAgentOffline(new UUID[] { new UUID(userID) });
+                        friendClient.SendAgentOffline(new UUID[] { userID });
                 }
                 else
                 {
                     bool canEditObjectsChanged = ((rights ^ userFlags) & (int)FriendRights.CanModifyObjects) != 0;
                     if (canEditObjectsChanged)
                         friendClient.SendChangeUserRights(userID, friendID, rights);
-
                 }
 
                 // Update local cache
@@ -847,7 +870,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             IClientAPI friendClient = LocateClientObject(friendID);
             if (friendClient != null)
             {
-                // the  friend in this sim as root agent
+                // the friend in this sim as root agent
                 if (online)
                     friendClient.SendAgentOnline(new UUID[] { userID });
                 else
@@ -902,8 +925,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             return FriendsService.GetFriends(client.AgentId);
         }
 
-        private void RefetchFriends(IClientAPI client)
+        private void RecacheFriends(IClientAPI client)
         {
+            // FIXME: Ideally, we want to avoid doing this here since it sits the EventManager.OnMakeRootAgent event
+            // is on the critical path for transferring an avatar from one region to another.
             UUID agentID = client.AgentId;
             lock (m_Friends)
             {
diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs b/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs
index 02b417f..9a97925 100644
--- a/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs
@@ -30,7 +30,6 @@ using System.Collections;
 using System.Collections.Generic;
 using System.Reflection;
 using System.Threading;
-
 using log4net;
 using Nini.Config;
 using Nwc.XmlRpc;
@@ -84,9 +83,11 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
 
         #endregion
 
-        protected override bool FetchFriendslist(IClientAPI client)
+        protected override bool CacheFriends(IClientAPI client)
         {
-            if (base.FetchFriendslist(client))
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Entered CacheFriends for {0}", client.Name);
+
+            if (base.CacheFriends(client))
             {
                 UUID agentID = client.AgentId;
                 // we do this only for the root agent
@@ -110,14 +111,20 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                             }
                         }
                     }
+
+//                    m_log.DebugFormat("[HGFRIENDS MODULE]: Exiting CacheFriends for {0} since detected root agent", client.Name);
                     return true;
                 }
             }
+
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Exiting CacheFriends for {0} since detected not root agent", client.Name);
             return false;
         }
 
         public override bool SendFriendsOnlineIfNeeded(IClientAPI client)
         {
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Entering SendFriendsOnlineIfNeeded for {0}", client.Name);
+
             if (base.SendFriendsOnlineIfNeeded(client))
             {
                 AgentCircuitData aCircuit = ((Scene)client.Scene).AuthenticateHandler.GetAgentCircuitData(client.AgentId);
@@ -134,11 +141,15 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                     }
                 }
             }
+
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Exiting SendFriendsOnlineIfNeeded for {0}", client.Name);
             return false;
         }
 
         protected override void GetOnlineFriends(UUID userID, List<string> friendList, /*collector*/ List<UUID> online)
         {
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Entering GetOnlineFriends for {0}", userID);
+
             List<string> fList = new List<string>();
             foreach (string s in friendList)
             {
@@ -157,6 +168,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                 if (UUID.TryParse(pi.UserID, out presenceID))
                     online.Add(presenceID);
             }
+
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Exiting GetOnlineFriends for {0}", userID);
         }
 
         //protected override void GetOnlineFriends(UUID userID, List<string> friendList, /*collector*/ List<UUID> online)
@@ -246,6 +259,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
 
         protected override void StatusNotify(List<FriendInfo> friendList, UUID userID, bool online)
         {
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Entering StatusNotify for {0}", userID);
+
             // First, let's divide the friends on a per-domain basis
             Dictionary<string, List<FriendInfo>> friendsPerDomain = new Dictionary<string, List<FriendInfo>>();
             foreach (FriendInfo friend in friendList)
@@ -298,6 +313,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                     }
                 }
             }
+
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Exiting StatusNotify for {0}", userID);
         }
 
         protected override bool GetAgentInfo(UUID scopeID, string fid, out UUID agentID, out string first, out string last)
@@ -351,6 +368,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
 
         protected override FriendInfo[] GetFriendsFromService(IClientAPI client)
         {
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Entering GetFriendsFromService for {0}", client.Name);
+
             UserAccount account1 = UserAccountService.GetUserAccount(m_Scenes[0].RegionInfo.ScopeID, client.AgentId);
             if (account1 != null)
                 return base.GetFriendsFromService(client);
@@ -365,6 +384,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                 finfos = FriendsService.GetFriends(agentUUI);
                 m_log.DebugFormat("[HGFRIENDS MODULE]: Fetched {0} local friends for visitor {1}", finfos.Length, agentUUI);
             }
+
+//            m_log.DebugFormat("[HGFRIENDS MODULE]: Exiting GetFriendsFromService for {0}", client.Name);
+
             return finfos;
         }
 
@@ -401,7 +423,6 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             }
 
             return false;
-
         }
 
         protected override void StoreBackwards(UUID friendID, UUID agentID)
@@ -627,4 +648,4 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             }
         }
     }
-}
+}
\ No newline at end of file
diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs
index c945dcf..682fbab 100644
--- a/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs
@@ -71,12 +71,12 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends.Tests
 
             ScenePresence sp = SceneHelpers.AddScenePresence(m_scene, userId);
 
-            Assert.That(((TestClient)sp.ControllingClient).OfflineNotificationsReceived.Count, Is.EqualTo(0));
-            Assert.That(((TestClient)sp.ControllingClient).OnlineNotificationsReceived.Count, Is.EqualTo(0));
+            Assert.That(((TestClient)sp.ControllingClient).ReceivedOfflineNotifications.Count, Is.EqualTo(0));
+            Assert.That(((TestClient)sp.ControllingClient).ReceivedOnlineNotifications.Count, Is.EqualTo(0));
         }
 
         [Test]
-        public void TestAddFriendWhileOnline()
+        public void TestAddFriendshipWhileOnline()
         {
             TestHelpers.InMethod();
 //            log4net.Config.XmlConfigurator.Configure();
@@ -91,8 +91,28 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends.Tests
             // notification.
             m_fm.AddFriendship(sp.ControllingClient, user2Id);
 
-            Assert.That(((TestClient)sp.ControllingClient).OfflineNotificationsReceived.Count, Is.EqualTo(0));
-            Assert.That(((TestClient)sp.ControllingClient).OnlineNotificationsReceived.Count, Is.EqualTo(1));
+            Assert.That(((TestClient)sp.ControllingClient).ReceivedOfflineNotifications.Count, Is.EqualTo(0));
+            Assert.That(((TestClient)sp.ControllingClient).ReceivedOnlineNotifications.Count, Is.EqualTo(1));
+        }
+
+        [Test]
+        public void TestRemoveFriendshipWhileOnline()
+        {
+            TestHelpers.InMethod();
+//            log4net.Config.XmlConfigurator.Configure();
+
+            UUID user1Id = TestHelpers.ParseTail(0x1);
+            UUID user2Id = TestHelpers.ParseTail(0x2);
+
+            ScenePresence sp = SceneHelpers.AddScenePresence(m_scene, user1Id);
+            SceneHelpers.AddScenePresence(m_scene, user2Id);
+
+            m_fm.AddFriendship(sp.ControllingClient, user2Id);
+            m_fm.RemoveFriendship(sp.ControllingClient, user2Id);
+
+            TestClient user1Client = sp.ControllingClient as TestClient;
+            Assert.That(user1Client.ReceivedFriendshipTerminations.Count, Is.EqualTo(1));
+            Assert.That(user1Client.ReceivedFriendshipTerminations[0], Is.EqualTo(user2Id));
         }
     }
 }
\ No newline at end of file