From 505917cb975e521f9095194366b7acc3e18ce85a Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 13 Aug 2014 17:12:50 +0100 Subject: [PATCH] Use new Federation Query API to implement HS->HS fetching of remote users' profile information instead of (ab)using the client-side REST API --- synapse/handlers/profile.py | 66 ++++++++++++++++++----------- tests/handlers/test_presencelike.py | 3 ++ tests/handlers/test_profile.py | 35 +++++++++++---- 3 files changed, 71 insertions(+), 33 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 976b8cfcdc..950648cc12 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -26,15 +26,16 @@ import logging logger = logging.getLogger(__name__) -PREFIX = "/matrix/client/api/v1" - class ProfileHandler(BaseHandler): def __init__(self, hs): super(ProfileHandler, self).__init__(hs) - self.client = hs.get_http_client() + self.federation = hs.get_replication_layer() + self.federation.register_query_handler( + "profile", self.on_profile_query + ) distributor = hs.get_distributor() self.distributor = distributor @@ -57,17 +58,14 @@ class ProfileHandler(BaseHandler): defer.returnValue(displayname) elif not local_only: - # TODO(paul): This should use the server-server API to ask another - # HS. For now we'll just have it use the http client to talk to the - # other HS's REST client API - path = PREFIX + "/profile/%s/displayname?local_only=1" % ( - target_user.to_string() - ) - try: - result = yield self.client.get_json( + result = yield self.federation.make_query( destination=target_user.domain, - path=path + query_type="profile", + args={ + "user_id": target_user.to_string(), + "field": "displayname", + } ) except CodeMessageException as e: if e.code != 404: @@ -76,8 +74,8 @@ class ProfileHandler(BaseHandler): raise except: logger.exception("Failed to get displayname") - - defer.returnValue(result["displayname"]) + else: + defer.returnValue(result["displayname"]) else: raise SynapseError(400, "User is not hosted on this Home Server") @@ -110,18 +108,14 @@ class ProfileHandler(BaseHandler): defer.returnValue(avatar_url) elif not local_only: - # TODO(paul): This should use the server-server API to ask another - # HS. For now we'll just have it use the http client to talk to the - # other HS's REST client API - destination = target_user.domain - path = PREFIX + "/profile/%s/avatar_url?local_only=1" % ( - target_user.to_string(), - ) - try: - result = yield self.client.get_json( - destination=destination, - path=path + result = yield self.federation.make_query( + destination=target_user.domain, + query_type="profile", + args={ + "user_id": target_user.to_string(), + "field": "avatar_url", + } ) except CodeMessageException as e: if e.code != 404: @@ -168,3 +162,25 @@ class ProfileHandler(BaseHandler): state["avatar_url"] = avatar_url defer.returnValue(None) + + @defer.inlineCallbacks + def on_profile_query(self, args): + user = self.hs.parse_userid(args["user_id"]) + if not user.is_mine: + raise SynapseError(400, "User is not hosted on this Home Server") + + just_field = args.get("field", None) + + response = {} + + if just_field is None or just_field == "displayname": + response["displayname"] = yield self.store.get_profile_displayname( + user.localpart + ) + + if just_field is None or just_field == "avatar_url": + response["avatar_url"] = yield self.store.get_profile_avatar_url( + user.localpart + ) + + defer.returnValue(response) diff --git a/tests/handlers/test_presencelike.py b/tests/handlers/test_presencelike.py index 224cf646f4..f244ab6007 100644 --- a/tests/handlers/test_presencelike.py +++ b/tests/handlers/test_presencelike.py @@ -43,6 +43,9 @@ class MockReplication(object): def register_edu_handler(self, edu_type, handler): self.edu_handlers[edu_type] = handler + def register_query_handler(self, query_type, handler): + pass + def received_edu(self, origin, edu_type, content): self.edu_handlers[edu_type](origin, content) diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 34af7afd01..eb1df2a4cf 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -37,13 +37,18 @@ class ProfileTestCase(unittest.TestCase): """ Tests profile management. """ def setUp(self): - self.mock_client = Mock(spec=[ - "get_json", + self.mock_federation = Mock(spec=[ + "make_query", ]) + self.query_handlers = {} + def register_query_handler(query_type, handler): + self.query_handlers[query_type] = handler + self.mock_federation.register_query_handler = register_query_handler + hs = HomeServer("test", db_pool=None, - http_client=self.mock_client, + http_client=None, datastore=Mock(spec=[ "get_profile_displayname", "set_profile_displayname", @@ -52,6 +57,7 @@ class ProfileTestCase(unittest.TestCase): ]), handlers=None, http_server=Mock(), + replication_layer=self.mock_federation, ) hs.handlers = ProfileHandlers(hs) @@ -93,18 +99,31 @@ class ProfileTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_other_name(self): - self.mock_client.get_json.return_value = defer.succeed( - {"displayname": "Alice"}) + self.mock_federation.make_query.return_value = defer.succeed( + {"displayname": "Alice"} + ) displayname = yield self.handler.get_displayname(self.alice) self.assertEquals(displayname, "Alice") - self.mock_client.get_json.assert_called_with( + self.mock_federation.make_query.assert_called_with( destination="remote", - path="/matrix/client/api/v1/profile/@alice:remote/displayname" - "?local_only=1" + query_type="profile", + args={"user_id": "@alice:remote", "field": "displayname"} ) + @defer.inlineCallbacks + def test_incoming_fed_query(self): + mocked_get = self.datastore.get_profile_displayname + mocked_get.return_value = defer.succeed("Caroline") + + response = yield self.query_handlers["profile"]( + {"user_id": "@caroline:test", "field": "displayname"} + ) + + self.assertEquals({"displayname": "Caroline"}, response) + mocked_get.assert_called_with("caroline") + @defer.inlineCallbacks def test_get_my_avatar(self): mocked_get = self.datastore.get_profile_avatar_url