Use new Federation Query API to implement HS->HS fetching of remote users' profile information instead of (ab)using the client-side REST API

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