From 1a9a9abcc73ebbd14fce0f45689e4648a71d55bc Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 22 May 2015 16:11:17 +0100 Subject: [PATCH 1/2] Add a cache for getting the presence list for a user --- synapse/handlers/presence.py | 24 +++++++++++++++--------- synapse/storage/presence.py | 35 +++++++++++++++++++++++++++-------- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 670c1d353..023ad33ab 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -521,20 +521,26 @@ class PresenceHandler(BaseHandler): if not self.hs.is_mine(observer_user): raise SynapseError(400, "User is not hosted on this Home Server") - presence = yield self.store.get_presence_list( + presence_list = yield self.store.get_presence_list( observer_user.localpart, accepted=accepted ) - for p in presence: - observed_user = UserID.from_string(p.pop("observed_user_id")) - p["observed_user"] = observed_user - p.update(self._get_or_offline_usercache(observed_user).get_state()) - if "last_active" in p: - p["last_active_ago"] = int( - self.clock.time_msec() - p.pop("last_active") + results = [] + for row in presence_list: + observed_user = UserID.from_string(row["observed_user_id"]) + result = { + "observed_user": observed_user, "accepted": row["accepted"] + } + result.update( + self._get_or_offline_usercache(observed_user).get_state() + ) + if "last_active" in result: + result["last_active_ago"] = int( + self.clock.time_msec() - result.pop("last_active") ) + results.append(result) - defer.returnValue(presence) + defer.returnValue(results) @defer.inlineCallbacks @log_function diff --git a/synapse/storage/presence.py b/synapse/storage/presence.py index 22ec94bc1..fefcf6bce 100644 --- a/synapse/storage/presence.py +++ b/synapse/storage/presence.py @@ -13,7 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -from ._base import SQLBaseStore +from ._base import SQLBaseStore, cached + +from twisted.internet import defer class PresenceStore(SQLBaseStore): @@ -87,31 +89,48 @@ class PresenceStore(SQLBaseStore): desc="add_presence_list_pending", ) + @defer.inlineCallbacks def set_presence_list_accepted(self, observer_localpart, observed_userid): - return self._simple_update_one( + result = yield self._simple_update_one( table="presence_list", keyvalues={"user_id": observer_localpart, "observed_user_id": observed_userid}, updatevalues={"accepted": True}, desc="set_presence_list_accepted", ) + self.get_presence_list_accepted.invalidate(observer_localpart) + defer.returnValue(result) def get_presence_list(self, observer_localpart, accepted=None): - keyvalues = {"user_id": observer_localpart} - if accepted is not None: - keyvalues["accepted"] = accepted + if accepted: + return self.get_presence_list_accepted(observer_localpart) + else: + keyvalues = {"user_id": observer_localpart} + if accepted is not None: + keyvalues["accepted"] = accepted + return self._simple_select_list( + table="presence_list", + keyvalues=keyvalues, + retcols=["observed_user_id", "accepted"], + desc="get_presence_list", + ) + + @cached() + def get_presence_list_accepted(self, observer_localpart): return self._simple_select_list( table="presence_list", - keyvalues=keyvalues, + keyvalues={"user_id": observer_localpart, "accepted": True}, retcols=["observed_user_id", "accepted"], - desc="get_presence_list", + desc="get_presence_list_accepted", ) + @defer.inlineCallbacks def del_presence_list(self, observer_localpart, observed_userid): - return self._simple_delete_one( + yield self._simple_delete_one( table="presence_list", keyvalues={"user_id": observer_localpart, "observed_user_id": observed_userid}, desc="del_presence_list", ) + self.get_presence_list_accepted.invalidate(observer_localpart) From 17167898c816c95db8a3f4661d955f43ad6a87ce Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 22 May 2015 16:22:54 +0100 Subject: [PATCH 2/2] Fix the presence tests --- tests/handlers/test_presence.py | 16 ++++++++++------ tests/handlers/test_presencelike.py | 20 +++++++++++--------- tests/rest/client/v1/test_presence.py | 4 ++-- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 12cf5747a..29372d488 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -233,7 +233,7 @@ class MockedDatastorePresenceTestCase(PresenceTestCase): if not user_localpart in self.PRESENCE_LIST: return defer.succeed([]) return defer.succeed([ - {"observed_user_id": u} for u in + {"observed_user_id": u, "accepted": accepted} for u in self.PRESENCE_LIST[user_localpart]]) datastore.get_presence_list = get_presence_list @@ -734,10 +734,12 @@ class PresencePushTestCase(MockedDatastorePresenceTestCase): self.assertEquals( [ - {"observed_user": self.u_banana, - "presence": OFFLINE}, + {"observed_user": self.u_banana, + "presence": OFFLINE, + "accepted": True}, {"observed_user": self.u_clementine, - "presence": OFFLINE}, + "presence": OFFLINE, + "accepted": True}, ], presence ) @@ -758,9 +760,11 @@ class PresencePushTestCase(MockedDatastorePresenceTestCase): self.assertEquals([ {"observed_user": self.u_banana, "presence": ONLINE, - "last_active_ago": 2000}, + "last_active_ago": 2000, + "accepted": True}, {"observed_user": self.u_clementine, - "presence": OFFLINE}, + "presence": OFFLINE, + "accepted": True}, ], presence) (events, _) = yield self.event_source.get_new_events_for_user( diff --git a/tests/handlers/test_presencelike.py b/tests/handlers/test_presencelike.py index 1f2e66ac1..19107caee 100644 --- a/tests/handlers/test_presencelike.py +++ b/tests/handlers/test_presencelike.py @@ -101,8 +101,8 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): self.datastore.get_profile_avatar_url = get_profile_avatar_url self.presence_list = [ - {"observed_user_id": "@banana:test"}, - {"observed_user_id": "@clementine:test"}, + {"observed_user_id": "@banana:test", "accepted": True}, + {"observed_user_id": "@clementine:test", "accepted": True}, ] def get_presence_list(user_localpart, accepted=None): return defer.succeed(self.presence_list) @@ -144,8 +144,8 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): @defer.inlineCallbacks def test_set_my_state(self): self.presence_list = [ - {"observed_user_id": "@banana:test"}, - {"observed_user_id": "@clementine:test"}, + {"observed_user_id": "@banana:test", "accepted": True}, + {"observed_user_id": "@clementine:test", "accepted": True}, ] mocked_set = self.datastore.set_presence_state @@ -167,8 +167,8 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): self.mock_get_joined.side_effect = get_joined self.presence_list = [ - {"observed_user_id": "@banana:test"}, - {"observed_user_id": "@clementine:test"}, + {"observed_user_id": "@banana:test", "accepted": True}, + {"observed_user_id": "@clementine:test", "accepted": True}, ] self.datastore.set_presence_state.return_value = defer.succeed( @@ -203,9 +203,11 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): "presence": ONLINE, "last_active_ago": 0, "displayname": "Frank", - "avatar_url": "http://foo"}, + "avatar_url": "http://foo", + "accepted": True}, {"observed_user": self.u_clementine, - "presence": OFFLINE} + "presence": OFFLINE, + "accepted": True} ], presence) self.mock_update_client.assert_has_calls([ @@ -233,7 +235,7 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): @defer.inlineCallbacks def test_push_remote(self): self.presence_list = [ - {"observed_user_id": "@potato:remote"}, + {"observed_user_id": "@potato:remote", "accepted": True}, ] self.datastore.set_presence_state.return_value = defer.succeed( diff --git a/tests/rest/client/v1/test_presence.py b/tests/rest/client/v1/test_presence.py index 523b30cf8..4b32c7a20 100644 --- a/tests/rest/client/v1/test_presence.py +++ b/tests/rest/client/v1/test_presence.py @@ -183,7 +183,7 @@ class PresenceListTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_my_list(self): self.datastore.get_presence_list.return_value = defer.succeed( - [{"observed_user_id": "@banana:test"}], + [{"observed_user_id": "@banana:test", "accepted": True}], ) (code, response) = yield self.mock_resource.trigger("GET", @@ -191,7 +191,7 @@ class PresenceListTestCase(unittest.TestCase): self.assertEquals(200, code) self.assertEquals([ - {"user_id": "@banana:test", "presence": OFFLINE}, + {"user_id": "@banana:test", "presence": OFFLINE, "accepted": True}, ], response) self.datastore.get_presence_list.assert_called_with(