From 9f863d34663a4d58cf59cd15e0b0aa4a8e8581a5 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 14 Aug 2014 09:52:20 +0100 Subject: [PATCH 1/4] Start phasing out HttpServer: we should be using Resources instead. Added resource_for_client/federation/web_client to the HomeServer and hooked the C-S servlets to operate on resource_for_client. Dynamically construct the Resource tree. --- synapse/app/homeserver.py | 77 ++++++++++++++++++++++++++++++++++++- synapse/http/server.py | 5 +++ synapse/rest/__init__.py | 24 +++++------- synapse/rest/base.py | 4 +- synapse/server.py | 8 ++-- tests/rest/test_presence.py | 3 ++ tests/rest/test_profile.py | 1 + 7 files changed, 101 insertions(+), 21 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 82afb04c7d..1acc87e99c 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -21,8 +21,12 @@ from synapse.server import HomeServer from twisted.internet import reactor from twisted.enterprise import adbapi from twisted.python.log import PythonLoggingObserver -from synapse.http.server import TwistedHttpServer +from twisted.web.resource import Resource +from twisted.web.static import File +from twisted.web.server import Site +from synapse.http.server import TwistedHttpServer, JsonResource from synapse.http.client import TwistedHttpClient +from synapse.rest.base import CLIENT_PREFIX from daemonize import Daemonize @@ -41,6 +45,15 @@ class SynapseHomeServer(HomeServer): def build_http_client(self): return TwistedHttpClient() + def build_resource_for_client(self): + return JsonResource() + + def build_resource_for_federation(self): + return JsonResource() + + def build_resource_for_web_client(self): + return File("webclient") + def build_db_pool(self): """ Set up all the dbs. Since all the *.sql have IF NOT EXISTS, so we don't have to worry about overwriting existing content. @@ -73,6 +86,65 @@ class SynapseHomeServer(HomeServer): return pool + def create_resource_tree(self): + """Create the resource tree for this Home Server. + + This in unduly complicated because Twisted does not support putting + child resources more than 1 level deep at a time. + """ + desired_tree = ( # list of tuples containing (path_str, Resource) + ("/matrix/client", self.get_resource_for_web_client()), + (CLIENT_PREFIX, self.get_resource_for_client()) + ) + + self.root_resource = Resource() + # ideally we'd just use getChild and putChild but getChild doesn't work + # unless you give it a Request object IN ADDITION to the name :/ So + # instead, we'll store a copy of this mapping so we can actually add + # extra resources to existing nodes. See self._resource_id for the key. + resource_mappings = {} + for (full_path, resource) in desired_tree: + logging.info("Attaching %s to path %s", resource, full_path) + last_resource = self.root_resource + for path_seg in full_path.split('/')[1:-1]: + if not path_seg in last_resource.listNames(): + # resource doesn't exist + child_resource = Resource() + last_resource.putChild(path_seg, child_resource) + res_id = self._resource_id(last_resource, path_seg) + resource_mappings[res_id] = child_resource + last_resource = child_resource + else: + # we have an existing Resource, pull it out. + res_id = self._resource_id(last_resource, path_seg) + last_resource = resource_mappings[res_id] + + # now attach the actual resource + last_path_seg = full_path.split('/')[-1] + last_resource.putChild(last_path_seg, resource) + res_id = self._resource_id(last_resource, last_path_seg) + resource_mappings[res_id] = resource + + return self.root_resource + + def _resource_id(self, resource, path_seg): + """Construct an arbitrary resource ID so you can retrieve the mapping + later. + + If you want to represent resource A putChild resource B with path C, + the mapping should looks like _resource_id(A,C) = B. + + Args: + resource (Resource): The *parent* Resource + path_seg (str): The name of the child Resource to be attached. + Returns: + str: A unique string which can be a key to the child Resource. + """ + return "%s-%s" % (resource, path_seg) + + def start_listening(self, port): + reactor.listenTCP(port, Site(self.root_resource)) + def setup_logging(verbosity=0, filename=None, config_path=None): """ Sets up logging with verbosity levels. @@ -150,7 +222,8 @@ def setup(): hs.register_servlets() - hs.get_http_server().start_listening(args.port) + hs.create_resource_tree() + hs.start_listening(args.port) hs.build_db_pool() diff --git a/synapse/http/server.py b/synapse/http/server.py index d7f4b691bc..32dfd836cd 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -185,3 +185,8 @@ def respond_with_json_bytes(request, code, json_bytes, send_cors=False): request.write(json_bytes) request.finish() return NOT_DONE_YET + + +# FIXME: Temp, just so the new name can be used without breaking the world. +class JsonResource(TwistedHttpServer): + pass \ No newline at end of file diff --git a/synapse/rest/__init__.py b/synapse/rest/__init__.py index 74a372e2ff..82c09f4c4a 100644 --- a/synapse/rest/__init__.py +++ b/synapse/rest/__init__.py @@ -32,19 +32,15 @@ class RestServletFactory(object): """ def __init__(self, hs): - http_server = hs.get_http_server() + client_resource = hs.get_resource_for_client() # TODO(erikj): There *must* be a better way of doing this. - room.register_servlets(hs, http_server) - events.register_servlets(hs, http_server) - register.register_servlets(hs, http_server) - login.register_servlets(hs, http_server) - profile.register_servlets(hs, http_server) - public.register_servlets(hs, http_server) - presence.register_servlets(hs, http_server) - im.register_servlets(hs, http_server) - directory.register_servlets(hs, http_server) - - def register_web_client(self, hs): - http_server = hs.get_http_server() - webclient.register_servlets(hs, http_server) + room.register_servlets(hs, client_resource) + events.register_servlets(hs, client_resource) + register.register_servlets(hs, client_resource) + login.register_servlets(hs, client_resource) + profile.register_servlets(hs, client_resource) + public.register_servlets(hs, client_resource) + presence.register_servlets(hs, client_resource) + im.register_servlets(hs, client_resource) + directory.register_servlets(hs, client_resource) diff --git a/synapse/rest/base.py b/synapse/rest/base.py index 65d417f757..124b3a2e0c 100644 --- a/synapse/rest/base.py +++ b/synapse/rest/base.py @@ -16,6 +16,8 @@ """ This module contains base REST classes for constructing REST servlets. """ import re +CLIENT_PREFIX = "/matrix/client/api/v1" + def client_path_pattern(path_regex): """Creates a regex compiled client path with the correct client path @@ -27,7 +29,7 @@ def client_path_pattern(path_regex): Returns: SRE_Pattern """ - return re.compile("^/matrix/client/api/v1" + path_regex) + return re.compile("^" + CLIENT_PREFIX + path_regex) class RestServlet(object): diff --git a/synapse/server.py b/synapse/server.py index 96830a88b1..537e431375 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -70,6 +70,9 @@ class BaseHomeServer(object): 'room_lock_manager', 'notifier', 'distributor', + 'resource_for_client', + 'resource_for_federation', + 'resource_for_web_client', ] def __init__(self, hostname, **kwargs): @@ -178,9 +181,6 @@ class HomeServer(BaseHomeServer): def register_servlets(self): """ Register all servlets associated with this HomeServer. - - Args: - host_web_client (bool): True to host the web client as well. """ # Simply building the ServletFactory is sufficient to have it register - factory = self.get_rest_servlet_factory() + self.get_rest_servlet_factory() diff --git a/tests/rest/test_presence.py b/tests/rest/test_presence.py index f013abbee4..98b308dcdb 100644 --- a/tests/rest/test_presence.py +++ b/tests/rest/test_presence.py @@ -51,6 +51,7 @@ class PresenceStateTestCase(unittest.TestCase): hs = HomeServer("test", db_pool=None, http_client=None, + resource_for_client=self.mock_server, http_server=self.mock_server, ) @@ -108,6 +109,7 @@ class PresenceListTestCase(unittest.TestCase): hs = HomeServer("test", db_pool=None, http_client=None, + resource_for_client=self.mock_server, http_server=self.mock_server, ) @@ -184,6 +186,7 @@ class PresenceEventStreamTestCase(unittest.TestCase): db_pool=None, http_client=None, http_server=self.mock_server, + resource_for_client=self.mock_server, datastore=Mock(spec=[ "set_presence_state", "get_presence_list", diff --git a/tests/rest/test_profile.py b/tests/rest/test_profile.py index 46e6137775..975d32f64d 100644 --- a/tests/rest/test_profile.py +++ b/tests/rest/test_profile.py @@ -44,6 +44,7 @@ class ProfileTestCase(unittest.TestCase): db_pool=None, http_client=None, http_server=self.mock_server, + resource_for_client=self.mock_server, federation=Mock(), replication_layer=Mock(), ) From 29aa13f0d4424729813bea99b84d34a3b3d4e68c Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 14 Aug 2014 10:05:06 +0100 Subject: [PATCH 2/4] Make federation use resource_for_federation as well. --- synapse/app/homeserver.py | 4 ++- synapse/federation/__init__.py | 2 +- synapse/rest/__init__.py | 3 +-- synapse/rest/webclient.py | 45 ---------------------------------- 4 files changed, 5 insertions(+), 49 deletions(-) delete mode 100644 synapse/rest/webclient.py diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 1acc87e99c..d9494cb054 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -27,6 +27,7 @@ from twisted.web.server import Site from synapse.http.server import TwistedHttpServer, JsonResource from synapse.http.client import TwistedHttpClient from synapse.rest.base import CLIENT_PREFIX +from synapse.federation.transport import PREFIX from daemonize import Daemonize @@ -94,7 +95,8 @@ class SynapseHomeServer(HomeServer): """ desired_tree = ( # list of tuples containing (path_str, Resource) ("/matrix/client", self.get_resource_for_web_client()), - (CLIENT_PREFIX, self.get_resource_for_client()) + (CLIENT_PREFIX, self.get_resource_for_client()), + (PREFIX, self.get_resource_for_federation()) ) self.root_resource = Resource() diff --git a/synapse/federation/__init__.py b/synapse/federation/__init__.py index ac0c10dc33..b15e7cf941 100644 --- a/synapse/federation/__init__.py +++ b/synapse/federation/__init__.py @@ -23,7 +23,7 @@ from .transport import TransportLayer def initialize_http_replication(homeserver): transport = TransportLayer( homeserver.hostname, - server=homeserver.get_http_server(), + server=homeserver.get_resource_for_federation(), client=homeserver.get_http_client() ) diff --git a/synapse/rest/__init__.py b/synapse/rest/__init__.py index 82c09f4c4a..da18933b63 100644 --- a/synapse/rest/__init__.py +++ b/synapse/rest/__init__.py @@ -15,8 +15,7 @@ from . import ( - room, events, register, login, profile, public, presence, im, directory, - webclient + room, events, register, login, profile, public, presence, im, directory ) diff --git a/synapse/rest/webclient.py b/synapse/rest/webclient.py deleted file mode 100644 index 75a425c14c..0000000000 --- a/synapse/rest/webclient.py +++ /dev/null @@ -1,45 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2014 matrix.org -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from synapse.rest.base import RestServlet - -import logging -import re - -logger = logging.getLogger(__name__) - - -class WebClientRestServlet(RestServlet): - # No PATTERN; we have custom dispatch rules here - - def register(self, http_server): - http_server.register_path("GET", - re.compile("^/$"), - self.on_GET_redirect) - http_server.register_path("GET", - re.compile("^/matrix/client$"), - self.on_GET) - - def on_GET(self, request): - return (200, "not implemented") - - def on_GET_redirect(self, request): - request.setHeader("Location", request.uri + "matrix/client") - return (302, None) - - -def register_servlets(hs, http_server): - logger.info("Registering web client.") - WebClientRestServlet(hs).register(http_server) \ No newline at end of file From 9a1638ed21ea716b999853c8d63c30073e677c46 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 14 Aug 2014 10:18:54 +0100 Subject: [PATCH 3/4] Removed http_server from HomeServer. Updated unit tests to use either resource_for_federation or resource_for_client depending on what is being tested. --- synapse/app/homeserver.py | 4 +--- synapse/http/server.py | 12 +++--------- synapse/server.py | 5 +++-- tests/federation/test_federation.py | 2 +- tests/handlers/test_directory.py | 2 +- tests/handlers/test_federation.py | 2 +- tests/handlers/test_presence.py | 8 ++++---- tests/handlers/test_presencelike.py | 2 +- tests/handlers/test_profile.py | 4 ++-- tests/handlers/test_room.py | 3 +-- tests/rest/test_presence.py | 6 +++--- tests/rest/test_profile.py | 1 - 12 files changed, 21 insertions(+), 30 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index d9494cb054..ea6f0985ef 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -24,7 +24,7 @@ from twisted.python.log import PythonLoggingObserver from twisted.web.resource import Resource from twisted.web.static import File from twisted.web.server import Site -from synapse.http.server import TwistedHttpServer, JsonResource +from synapse.http.server import JsonResource from synapse.http.client import TwistedHttpClient from synapse.rest.base import CLIENT_PREFIX from synapse.federation.transport import PREFIX @@ -40,8 +40,6 @@ logger = logging.getLogger(__name__) class SynapseHomeServer(HomeServer): - def build_http_server(self): - return TwistedHttpServer() def build_http_client(self): return TwistedHttpClient() diff --git a/synapse/http/server.py b/synapse/http/server.py index 32dfd836cd..87b4fc8a5f 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -52,10 +52,9 @@ class HttpServer(object): pass -# The actual HTTP server impl, using twisted http server -class TwistedHttpServer(HttpServer, resource.Resource): - """ This wraps the twisted HTTP server, and triggers the correct callbacks - on the transport_layer. +class JsonResource(HttpServer, resource.Resource): + """ This implements the HttpServer interface and provides JSON support for + Resources. Register callbacks via register_path() """ @@ -185,8 +184,3 @@ def respond_with_json_bytes(request, code, json_bytes, send_cors=False): request.write(json_bytes) request.finish() return NOT_DONE_YET - - -# FIXME: Temp, just so the new name can be used without breaking the world. -class JsonResource(TwistedHttpServer): - pass \ No newline at end of file diff --git a/synapse/server.py b/synapse/server.py index 537e431375..0f7ac352ae 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -55,7 +55,6 @@ class BaseHomeServer(object): DEPENDENCIES = [ 'clock', - 'http_server', 'http_client', 'db_pool', 'persistence_service', @@ -138,7 +137,9 @@ class HomeServer(BaseHomeServer): required. It still requires the following to be specified by the caller: - http_server + resource_for_client + resource_for_web_client + resource_for_federation http_client db_pool """ diff --git a/tests/federation/test_federation.py b/tests/federation/test_federation.py index ec39c7ee33..478ddd879e 100644 --- a/tests/federation/test_federation.py +++ b/tests/federation/test_federation.py @@ -70,7 +70,7 @@ class FederationTestCase(unittest.TestCase): ) self.clock = MockClock() hs = HomeServer("test", - http_server=self.mock_http_server, + resource_for_federation=self.mock_http_server, http_client=self.mock_http_client, db_pool=None, datastore=self.mock_persistence, diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py index 0ace2d0c9a..88ac8933f8 100644 --- a/tests/handlers/test_directory.py +++ b/tests/handlers/test_directory.py @@ -51,7 +51,7 @@ class DirectoryTestCase(unittest.TestCase): "get_association_from_room_alias", ]), http_client=None, - http_server=Mock(), + resource_for_federation=Mock(), replication_layer=self.mock_federation, ) hs.handlers = DirectoryHandlers(hs) diff --git a/tests/handlers/test_federation.py b/tests/handlers/test_federation.py index bdee7cfad4..ab9c242579 100644 --- a/tests/handlers/test_federation.py +++ b/tests/handlers/test_federation.py @@ -42,7 +42,7 @@ class FederationTestCase(unittest.TestCase): "persist_event", "store_room", ]), - http_server=NonCallableMock(), + resource_for_federation=NonCallableMock(), http_client=NonCallableMock(spec_set=[]), notifier=NonCallableMock(spec_set=["on_new_room_event"]), handlers=NonCallableMock(spec_set=[ diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index b365741d99..61c2547af4 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -66,7 +66,7 @@ class PresenceStateTestCase(unittest.TestCase): "set_presence_list_accepted", ]), handlers=None, - http_server=Mock(), + resource_for_federation=Mock(), http_client=None, ) hs.handlers = JustPresenceHandlers(hs) @@ -188,7 +188,7 @@ class PresenceInvitesTestCase(unittest.TestCase): "del_presence_list", ]), handlers=None, - http_server=Mock(), + resource_for_client=Mock(), http_client=None, replication_layer=self.replication ) @@ -402,7 +402,7 @@ class PresencePushTestCase(unittest.TestCase): "set_presence_state", ]), handlers=None, - http_server=Mock(), + resource_for_client=Mock(), http_client=None, replication_layer=self.replication, ) @@ -727,7 +727,7 @@ class PresencePollingTestCase(unittest.TestCase): db_pool=None, datastore=Mock(spec=[]), handlers=None, - http_server=Mock(), + resource_for_client=Mock(), http_client=None, replication_layer=self.replication, ) diff --git a/tests/handlers/test_presencelike.py b/tests/handlers/test_presencelike.py index 6eeb1bb522..bba5dd4e53 100644 --- a/tests/handlers/test_presencelike.py +++ b/tests/handlers/test_presencelike.py @@ -71,7 +71,7 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): "set_profile_displayname", ]), handlers=None, - http_server=Mock(), + resource_for_federation=Mock(), http_client=None, replication_layer=MockReplication(), ) diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index eb1df2a4cf..87a8139920 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -56,7 +56,7 @@ class ProfileTestCase(unittest.TestCase): "set_profile_avatar_url", ]), handlers=None, - http_server=Mock(), + resource_for_federation=Mock(), replication_layer=self.mock_federation, ) hs.handlers = ProfileHandlers(hs) @@ -139,7 +139,7 @@ class ProfileTestCase(unittest.TestCase): mocked_set = self.datastore.set_profile_avatar_url mocked_set.return_value = defer.succeed(()) - yield self.handler.set_avatar_url(self.frank, self.frank, + yield self.handler.set_avatar_url(self.frank, self.frank, "http://my.server/pic.gif") mocked_set.assert_called_with("1234ABCD", "http://my.server/pic.gif") diff --git a/tests/handlers/test_room.py b/tests/handlers/test_room.py index 99067da6a5..fd2d66db38 100644 --- a/tests/handlers/test_room.py +++ b/tests/handlers/test_room.py @@ -46,7 +46,7 @@ class RoomMemberHandlerTestCase(unittest.TestCase): "get_room", "store_room", ]), - http_server=NonCallableMock(), + resource_for_federation=NonCallableMock(), http_client=NonCallableMock(spec_set=[]), notifier=NonCallableMock(spec_set=["on_new_room_event"]), handlers=NonCallableMock(spec_set=[ @@ -317,7 +317,6 @@ class RoomCreationTest(unittest.TestCase): datastore=NonCallableMock(spec_set=[ "store_room", ]), - http_server=NonCallableMock(), http_client=NonCallableMock(spec_set=[]), notifier=NonCallableMock(spec_set=["on_new_room_event"]), handlers=NonCallableMock(spec_set=[ diff --git a/tests/rest/test_presence.py b/tests/rest/test_presence.py index 98b308dcdb..91d4d1ff6c 100644 --- a/tests/rest/test_presence.py +++ b/tests/rest/test_presence.py @@ -52,7 +52,7 @@ class PresenceStateTestCase(unittest.TestCase): db_pool=None, http_client=None, resource_for_client=self.mock_server, - http_server=self.mock_server, + resource_for_federation=self.mock_server, ) def _get_user_by_token(token=None): @@ -110,7 +110,7 @@ class PresenceListTestCase(unittest.TestCase): db_pool=None, http_client=None, resource_for_client=self.mock_server, - http_server=self.mock_server, + resource_for_federation=self.mock_server ) def _get_user_by_token(token=None): @@ -185,8 +185,8 @@ class PresenceEventStreamTestCase(unittest.TestCase): hs = HomeServer("test", db_pool=None, http_client=None, - http_server=self.mock_server, resource_for_client=self.mock_server, + resource_for_federation=self.mock_server, datastore=Mock(spec=[ "set_presence_state", "get_presence_list", diff --git a/tests/rest/test_profile.py b/tests/rest/test_profile.py index 975d32f64d..ff1e92805e 100644 --- a/tests/rest/test_profile.py +++ b/tests/rest/test_profile.py @@ -43,7 +43,6 @@ class ProfileTestCase(unittest.TestCase): hs = HomeServer("test", db_pool=None, http_client=None, - http_server=self.mock_server, resource_for_client=self.mock_server, federation=Mock(), replication_layer=Mock(), From de65c34fcf996b83febada7f786f9863c67c675d Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 14 Aug 2014 10:24:17 +0100 Subject: [PATCH 4/4] Honour the -w flag to enable the web client at /matrix/client --- synapse/app/homeserver.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index ea6f0985ef..07d38b5035 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -85,17 +85,20 @@ class SynapseHomeServer(HomeServer): return pool - def create_resource_tree(self): + def create_resource_tree(self, web_client): """Create the resource tree for this Home Server. This in unduly complicated because Twisted does not support putting child resources more than 1 level deep at a time. """ - desired_tree = ( # list of tuples containing (path_str, Resource) - ("/matrix/client", self.get_resource_for_web_client()), + desired_tree = [ # list containing (path_str, Resource) (CLIENT_PREFIX, self.get_resource_for_client()), (PREFIX, self.get_resource_for_federation()) - ) + ] + if web_client: + logger.info("Adding the web client.") + desired_tree.append(("/matrix/client", # TODO constant please + self.get_resource_for_web_client())) self.root_resource = Resource() # ideally we'd just use getChild and putChild but getChild doesn't work @@ -222,7 +225,7 @@ def setup(): hs.register_servlets() - hs.create_resource_tree() + hs.create_resource_tree(web_client=args.webclient) hs.start_listening(args.port) hs.build_db_pool()