From a90a0f5c8a38ff7f99a12dd436b75680d3fee747 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 21 Apr 2017 11:32:48 +0100 Subject: [PATCH 01/23] Propagate errors sensibly from proxied IS requests When we're proxying Matrix endpoints, parse out Matrix error responses and turn them into SynapseErrors so they can be propagated sensibly upstream. --- synapse/handlers/identity.py | 10 +++++----- synapse/http/client.py | 30 ++++++++++++++++++++++++++++++ synapse/server.py | 8 +++++++- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 6a53c5eb47..41f978d990 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -35,7 +35,7 @@ class IdentityHandler(BaseHandler): def __init__(self, hs): super(IdentityHandler, self).__init__(hs) - self.http_client = hs.get_simple_http_client() + self.proxy_client = hs.get_matrix_proxy_client() self.trusted_id_servers = set(hs.config.trusted_third_party_id_servers) self.trust_any_id_server_just_for_testing_do_not_use = ( @@ -83,7 +83,7 @@ class IdentityHandler(BaseHandler): data = {} try: - data = yield self.http_client.get_json( + data = yield self.proxy_client.get_json( "https://%s%s" % ( id_server, "/_matrix/identity/api/v1/3pid/getValidated3pid" @@ -118,7 +118,7 @@ class IdentityHandler(BaseHandler): raise SynapseError(400, "No client_secret in creds") try: - data = yield self.http_client.post_urlencoded_get_json( + data = yield self.proxy_client.post_urlencoded_get_json( "https://%s%s" % ( id_server, "/_matrix/identity/api/v1/3pid/bind" ), @@ -151,7 +151,7 @@ class IdentityHandler(BaseHandler): params.update(kwargs) try: - data = yield self.http_client.post_json_get_json( + data = yield self.proxy_client.post_json_get_json( "https://%s%s" % ( id_server, "/_matrix/identity/api/v1/validate/email/requestToken" @@ -185,7 +185,7 @@ class IdentityHandler(BaseHandler): params.update(kwargs) try: - data = yield self.http_client.post_json_get_json( + data = yield self.proxy_client.post_json_get_json( "https://%s%s" % ( id_server, "/_matrix/identity/api/v1/validate/msisdn/requestToken" diff --git a/synapse/http/client.py b/synapse/http/client.py index ca2f770f5d..c8b76b2191 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -145,6 +145,9 @@ class SimpleHttpClient(object): body = yield preserve_context_over_fn(readBody, response) + if response.code / 100 != 2: + raise CodeMessageException(response.code, body) + defer.returnValue(json.loads(body)) @defer.inlineCallbacks @@ -306,6 +309,33 @@ class SimpleHttpClient(object): defer.returnValue((length, headers, response.request.absoluteURI, response.code)) +class MatrixProxyClient(object): + """ + An HTTP client that proxies other Matrix endpoints, ie. if the remote endpoint + returns Matrix-style error response, this will raise the appropriate SynapseError + """ + def __init__(self, hs): + self.simpleHttpClient = SimpleHttpClient(hs) + + @defer.inlineCallbacks + def post_json_get_json(self, uri, post_json): + try: + result = yield self.simpleHttpClient.post_json_get_json(uri, post_json) + defer.returnValue(result) + except CodeMessageException as cme: + ex = None + try: + errbody = json.loads(cme.msg) + errcode = errbody['errcode'] + errtext = errbody['error'] + ex = SynapseError(cme.code, errtext, errcode) + except: + pass + if ex is not None: + raise ex + raise cme + + # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. # The two should be factored out. diff --git a/synapse/server.py b/synapse/server.py index 6310152560..f73aef19c8 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -48,7 +48,9 @@ from synapse.handlers.typing import TypingHandler from synapse.handlers.events import EventHandler, EventStreamHandler from synapse.handlers.initial_sync import InitialSyncHandler from synapse.handlers.receipts import ReceiptsHandler -from synapse.http.client import SimpleHttpClient, InsecureInterceptableContextFactory +from synapse.http.client import ( + SimpleHttpClient, InsecureInterceptableContextFactory, MatrixProxyClient +) from synapse.http.matrixfederationclient import MatrixFederationHttpClient from synapse.notifier import Notifier from synapse.push.pusherpool import PusherPool @@ -127,6 +129,7 @@ class HomeServer(object): 'filtering', 'http_client_context_factory', 'simple_http_client', + 'matrix_proxy_client', 'media_repository', 'federation_transport_client', 'federation_sender', @@ -188,6 +191,9 @@ class HomeServer(object): def build_simple_http_client(self): return SimpleHttpClient(self) + def build_matrix_proxy_client(self): + return MatrixProxyClient(self) + def build_v1auth(self): orf = Auth(self) # Matrix spec makes no reference to what HTTP status code is returned, From a1595cec787d4bfa4f4a2ede7ebafe54919c1f06 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 21 Apr 2017 11:51:17 +0100 Subject: [PATCH 02/23] Don't error for 3xx responses --- synapse/http/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index c8b76b2191..483f37fe03 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -145,7 +145,7 @@ class SimpleHttpClient(object): body = yield preserve_context_over_fn(readBody, response) - if response.code / 100 != 2: + if response.code / 100 >= 4: raise CodeMessageException(response.code, body) defer.returnValue(json.loads(body)) From 70caf499142a3bc5e5a6e6228471983326d6e147 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 21 Apr 2017 16:09:03 +0100 Subject: [PATCH 03/23] Do the same for get_json --- synapse/http/client.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index 483f37fe03..4458855cec 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -323,18 +323,31 @@ class MatrixProxyClient(object): result = yield self.simpleHttpClient.post_json_get_json(uri, post_json) defer.returnValue(result) except CodeMessageException as cme: - ex = None - try: - errbody = json.loads(cme.msg) - errcode = errbody['errcode'] - errtext = errbody['error'] - ex = SynapseError(cme.code, errtext, errcode) - except: - pass + ex = self._tryGetMatrixError(cme.msg) if ex is not None: raise ex raise cme + @defer.inlineCallbacks + def get_json(self, uri, args={}): + try: + result = yield self.simpleHttpClient.get_json(uri, args) + defer.returnValue(result) + except CodeMessageException as cme: + ex = self._tryGetMatrixError(cme.msg) + if ex is not None: + raise ex + raise cme + + def _tryGetMatrixError(self, body): + try: + errbody = json.loads(body) + errcode = errbody['errcode'] + errtext = errbody['error'] + return SynapseError(cme.code, errtext, errcode) + except: + return None + # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. # The two should be factored out. From a46982cee9308294da0fcd99e06a5f1b7a7c439a Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 21 Apr 2017 16:20:12 +0100 Subject: [PATCH 04/23] Need the HTTP status code --- synapse/http/client.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index 4458855cec..df8c3e3c2c 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -323,7 +323,7 @@ class MatrixProxyClient(object): result = yield self.simpleHttpClient.post_json_get_json(uri, post_json) defer.returnValue(result) except CodeMessageException as cme: - ex = self._tryGetMatrixError(cme.msg) + ex = self._tryGetMatrixError(cme) if ex is not None: raise ex raise cme @@ -334,17 +334,17 @@ class MatrixProxyClient(object): result = yield self.simpleHttpClient.get_json(uri, args) defer.returnValue(result) except CodeMessageException as cme: - ex = self._tryGetMatrixError(cme.msg) + ex = self._tryGetMatrixError(cme) if ex is not None: raise ex raise cme - def _tryGetMatrixError(self, body): + def _tryGetMatrixError(self, codeMessageException): try: - errbody = json.loads(body) + errbody = json.loads(codeMessageException.msg) errcode = errbody['errcode'] errtext = errbody['error'] - return SynapseError(cme.code, errtext, errcode) + return SynapseError(codeMessageException.code, errtext, errcode) except: return None From 1a9255c12eb73245bdbb626a1a0cad2fbe967caa Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 25 Apr 2017 19:30:55 +0100 Subject: [PATCH 05/23] Use CodeMessageException subclass instead Parse json errors from get_json client methods and throw special errors. --- synapse/api/errors.py | 11 +++++++ synapse/handlers/identity.py | 29 ++++++++++------ synapse/http/client.py | 64 +++++++++++------------------------- synapse/server.py | 8 +---- 4 files changed, 51 insertions(+), 61 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 6fbd5d6876..d0dfa959dc 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -66,6 +66,17 @@ class CodeMessageException(RuntimeError): return cs_error(self.msg) +class MatrixCodeMessageException(CodeMessageException): + """An error from a general matrix endpoint, eg. from a proxied Matrix API call. + + Attributes: + errcode (str): Matrix error code e.g 'M_FORBIDDEN' + """ + def __init__(self, code, msg, errcode=Codes.UNKNOWN): + super(MatrixCodeMessageException, self).__init__(code, msg) + self.errcode = errcode + + class SynapseError(CodeMessageException): """A base exception type for matrix errors which have an errcode and error message (as well as an HTTP status code). diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 41f978d990..8b407a307c 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -18,7 +18,7 @@ from twisted.internet import defer from synapse.api.errors import ( - CodeMessageException + MatrixCodeMessageException, CodeMessageException ) from ._base import BaseHandler from synapse.util.async import run_on_reactor @@ -35,7 +35,7 @@ class IdentityHandler(BaseHandler): def __init__(self, hs): super(IdentityHandler, self).__init__(hs) - self.proxy_client = hs.get_matrix_proxy_client() + self.http_client = hs.get_simple_http_client() self.trusted_id_servers = set(hs.config.trusted_third_party_id_servers) self.trust_any_id_server_just_for_testing_do_not_use = ( @@ -83,13 +83,16 @@ class IdentityHandler(BaseHandler): data = {} try: - data = yield self.proxy_client.get_json( - "https://%s%s" % ( + data = yield self.http_client.get_json( + "http://%s%s" % ( id_server, "/_matrix/identity/api/v1/3pid/getValidated3pid" ), {'sid': creds['sid'], 'client_secret': client_secret} ) + except MatrixCodeMessageException as e: + logger.info("getValidated3pid failed with Matrix error: %r", e) + raise SynapseError(e.code, e.msg, e.errcode) except CodeMessageException as e: data = json.loads(e.msg) @@ -118,8 +121,8 @@ class IdentityHandler(BaseHandler): raise SynapseError(400, "No client_secret in creds") try: - data = yield self.proxy_client.post_urlencoded_get_json( - "https://%s%s" % ( + data = yield self.http_client.post_urlencoded_get_json( + "http://%s%s" % ( id_server, "/_matrix/identity/api/v1/3pid/bind" ), { @@ -151,14 +154,17 @@ class IdentityHandler(BaseHandler): params.update(kwargs) try: - data = yield self.proxy_client.post_json_get_json( - "https://%s%s" % ( + data = yield self.http_client.post_json_get_json( + "http://%s%s" % ( id_server, "/_matrix/identity/api/v1/validate/email/requestToken" ), params ) defer.returnValue(data) + except MatrixCodeMessageException as e: + logger.info("Proxied requestToken failed with Matrix error: %r", e) + raise SynapseError(e.code, e.msg, e.errcode) except CodeMessageException as e: logger.info("Proxied requestToken failed: %r", e) raise e @@ -185,14 +191,17 @@ class IdentityHandler(BaseHandler): params.update(kwargs) try: - data = yield self.proxy_client.post_json_get_json( - "https://%s%s" % ( + data = yield self.http_client.post_json_get_json( + "http://%s%s" % ( id_server, "/_matrix/identity/api/v1/validate/msisdn/requestToken" ), params ) defer.returnValue(data) + except MatrixCodeMessageException as e: + logger.info("Proxied requestToken failed with Matrix error: %r", e) + raise SynapseError(e.code, e.msg, e.errcode) except CodeMessageException as e: logger.info("Proxied requestToken failed: %r", e) raise e diff --git a/synapse/http/client.py b/synapse/http/client.py index df8c3e3c2c..57a49b2827 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -16,7 +16,7 @@ from OpenSSL import SSL from OpenSSL.SSL import VERIFY_NONE from synapse.api.errors import ( - CodeMessageException, SynapseError, Codes, + CodeMessageException, MatrixCodeMessageException, SynapseError, Codes, ) from synapse.util.logcontext import preserve_context_over_fn import synapse.metrics @@ -145,8 +145,10 @@ class SimpleHttpClient(object): body = yield preserve_context_over_fn(readBody, response) - if response.code / 100 >= 4: - raise CodeMessageException(response.code, body) + if 200 <= response.code < 300: + defer.returnValue(json.loads(body)) + else: + raise self._exceptionFromFailedRequest(response, body) defer.returnValue(json.loads(body)) @@ -168,7 +170,11 @@ class SimpleHttpClient(object): error message. """ body = yield self.get_raw(uri, args) - defer.returnValue(json.loads(body)) + + if 200 <= response.code < 300: + defer.returnValue(json.loads(body)) + else: + raise self._exceptionFromFailedRequest(response, body) @defer.inlineCallbacks def put_json(self, uri, json_body, args={}): @@ -249,6 +255,16 @@ class SimpleHttpClient(object): else: raise CodeMessageException(response.code, body) + def _exceptionFromFailedRequest(self, response, body): + try: + jsonBody = json.loads(body) + errcode = jsonBody['errcode'] + error = jsonBody['error'] + return MatrixCodeMessageException(response.code, error, errcode) + except e: + print e + return CodeMessageException(response.code, body) + # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. # The two should be factored out. @@ -309,46 +325,6 @@ class SimpleHttpClient(object): defer.returnValue((length, headers, response.request.absoluteURI, response.code)) -class MatrixProxyClient(object): - """ - An HTTP client that proxies other Matrix endpoints, ie. if the remote endpoint - returns Matrix-style error response, this will raise the appropriate SynapseError - """ - def __init__(self, hs): - self.simpleHttpClient = SimpleHttpClient(hs) - - @defer.inlineCallbacks - def post_json_get_json(self, uri, post_json): - try: - result = yield self.simpleHttpClient.post_json_get_json(uri, post_json) - defer.returnValue(result) - except CodeMessageException as cme: - ex = self._tryGetMatrixError(cme) - if ex is not None: - raise ex - raise cme - - @defer.inlineCallbacks - def get_json(self, uri, args={}): - try: - result = yield self.simpleHttpClient.get_json(uri, args) - defer.returnValue(result) - except CodeMessageException as cme: - ex = self._tryGetMatrixError(cme) - if ex is not None: - raise ex - raise cme - - def _tryGetMatrixError(self, codeMessageException): - try: - errbody = json.loads(codeMessageException.msg) - errcode = errbody['errcode'] - errtext = errbody['error'] - return SynapseError(codeMessageException.code, errtext, errcode) - except: - return None - - # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. # The two should be factored out. diff --git a/synapse/server.py b/synapse/server.py index 8325d77a7a..12754c89ae 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -49,9 +49,7 @@ from synapse.handlers.events import EventHandler, EventStreamHandler from synapse.handlers.initial_sync import InitialSyncHandler from synapse.handlers.receipts import ReceiptsHandler from synapse.handlers.read_marker import ReadMarkerHandler -from synapse.http.client import ( - SimpleHttpClient, InsecureInterceptableContextFactory, MatrixProxyClient -) +from synapse.http.client import SimpleHttpClient, InsecureInterceptableContextFactory from synapse.http.matrixfederationclient import MatrixFederationHttpClient from synapse.notifier import Notifier from synapse.push.pusherpool import PusherPool @@ -130,7 +128,6 @@ class HomeServer(object): 'filtering', 'http_client_context_factory', 'simple_http_client', - 'matrix_proxy_client', 'media_repository', 'federation_transport_client', 'federation_sender', @@ -193,9 +190,6 @@ class HomeServer(object): def build_simple_http_client(self): return SimpleHttpClient(self) - def build_matrix_proxy_client(self): - return MatrixProxyClient(self) - def build_v1auth(self): orf = Auth(self) # Matrix spec makes no reference to what HTTP status code is returned, From c3662760566a7cdf48442d399fbffb9163590dc5 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 26 Apr 2017 10:07:01 +0100 Subject: [PATCH 06/23] Fix get_json --- synapse/http/client.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index 57a49b2827..6e0ddfa0f6 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -169,12 +169,11 @@ class SimpleHttpClient(object): On a non-2xx HTTP response. The response body will be used as the error message. """ - body = yield self.get_raw(uri, args) - - if 200 <= response.code < 300: + try: + body = yield self.get_raw(uri, args) defer.returnValue(json.loads(body)) - else: - raise self._exceptionFromFailedRequest(response, body) + except CodeMessageException as e: + raise self._exceptionFromFailedRequest(e.code, e.msg) @defer.inlineCallbacks def put_json(self, uri, json_body, args={}): From 82ae0238f9ff15a5a5445cb681ab0cb507d865f3 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 26 Apr 2017 11:43:16 +0100 Subject: [PATCH 07/23] Revert accidental commit --- synapse/handlers/identity.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 8b407a307c..9efcdff1d6 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -84,7 +84,7 @@ class IdentityHandler(BaseHandler): data = {} try: data = yield self.http_client.get_json( - "http://%s%s" % ( + "https://%s%s" % ( id_server, "/_matrix/identity/api/v1/3pid/getValidated3pid" ), @@ -122,7 +122,7 @@ class IdentityHandler(BaseHandler): try: data = yield self.http_client.post_urlencoded_get_json( - "http://%s%s" % ( + "https://%s%s" % ( id_server, "/_matrix/identity/api/v1/3pid/bind" ), { @@ -155,7 +155,7 @@ class IdentityHandler(BaseHandler): try: data = yield self.http_client.post_json_get_json( - "http://%s%s" % ( + "https://%s%s" % ( id_server, "/_matrix/identity/api/v1/validate/email/requestToken" ), @@ -192,7 +192,7 @@ class IdentityHandler(BaseHandler): try: data = yield self.http_client.post_json_get_json( - "http://%s%s" % ( + "https://%s%s" % ( id_server, "/_matrix/identity/api/v1/validate/msisdn/requestToken" ), From 5fd12dce013ad46f171dde10245d78a225b12387 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 26 Apr 2017 12:36:26 +0100 Subject: [PATCH 08/23] Remove debugging --- synapse/http/client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index 6e0ddfa0f6..379ddcb540 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -260,8 +260,7 @@ class SimpleHttpClient(object): errcode = jsonBody['errcode'] error = jsonBody['error'] return MatrixCodeMessageException(response.code, error, errcode) - except e: - print e + except: return CodeMessageException(response.code, body) # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. From 7166854f4169999fee0cd40a5ed389cc684b6dc8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 May 2017 10:36:35 +0100 Subject: [PATCH 09/23] Add cache for get_current_hosts_in_room --- synapse/federation/transaction_queue.py | 6 +--- synapse/state.py | 11 +++++++ synapse/storage/roommember.py | 38 +++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/synapse/federation/transaction_queue.py b/synapse/federation/transaction_queue.py index dee387eb7f..695f1a7375 100644 --- a/synapse/federation/transaction_queue.py +++ b/synapse/federation/transaction_queue.py @@ -24,7 +24,6 @@ from synapse.util.async import run_on_reactor from synapse.util.logcontext import preserve_context_over_fn, preserve_fn from synapse.util.retryutils import NotRetryingDestination, get_retry_limiter from synapse.util.metrics import measure_func -from synapse.types import get_domain_from_id from synapse.handlers.presence import format_user_presence_state, get_interested_remotes import synapse.metrics @@ -183,15 +182,12 @@ class TransactionQueue(object): # Otherwise if the last member on a server in a room is # banned then it won't receive the event because it won't # be in the room after the ban. - users_in_room = yield self.state.get_current_user_in_room( + destinations = yield self.state.get_current_hosts_in_room( event.room_id, latest_event_ids=[ prev_id for prev_id, _ in event.prev_events ], ) - destinations = set( - get_domain_from_id(user_id) for user_id in users_in_room - ) if send_on_behalf_of is not None: # If we are sending the event on behalf of another server # then it already has the event and there is no reason to diff --git a/synapse/state.py b/synapse/state.py index f6b83d888a..f8b18a4a2d 100644 --- a/synapse/state.py +++ b/synapse/state.py @@ -175,6 +175,17 @@ class StateHandler(object): ) defer.returnValue(joined_users) + @defer.inlineCallbacks + def get_current_hosts_in_room(self, room_id, latest_event_ids=None): + if not latest_event_ids: + latest_event_ids = yield self.store.get_latest_event_ids_in_room(room_id) + logger.debug("calling resolve_state_groups from get_current_user_in_room") + entry = yield self.resolve_state_groups(room_id, latest_event_ids) + joined_hosts = yield self.store.get_joined_hosts( + room_id, entry.state_id, entry.state + ) + defer.returnValue(joined_hosts) + @defer.inlineCallbacks def compute_event_context(self, event, old_state=None): """Build an EventContext structure for the event. diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 7ad2198d96..1c0fa8a680 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -482,6 +482,44 @@ class RoomMemberStore(SQLBaseStore): defer.returnValue(False) + def get_joined_hosts(self, room_id, state_group, state_ids): + if not state_group: + # If state_group is None it means it has yet to be assigned a + # state group, i.e. we need to make sure that calls with a state_group + # of None don't hit previous cached calls with a None state_group. + # To do this we set the state_group to a new object as object() != object() + state_group = object() + + return self._get_joined_hosts( + room_id, state_group, state_ids + ) + + @cachedInlineCallbacks(num_args=3) + def _get_joined_hosts(self, room_id, state_group, current_state_ids): + # We don't use `state_group`, its there so that we can cache based + # on it. However, its important that its never None, since two current_state's + # with a state_group of None are likely to be different. + # See bulk_get_push_rules_for_room for how we work around this. + assert state_group is not None + + joined_hosts = set() + for (etype, state_key), event_id in current_state_ids.items(): + if etype == EventTypes.Member: + try: + host = get_domain_from_id(state_key) + except: + logger.warn("state_key not user_id: %s", state_key) + continue + + if host in joined_hosts: + continue + + event = yield self.get_event(event_id, allow_none=True) + if event and event.content["membership"] == Membership.JOIN: + joined_hosts.add(host) + + defer.returnValue(joined_hosts) + @defer.inlineCallbacks def _background_add_membership_profile(self, progress, batch_size): target_min_stream_id = progress.get( From c0380402bc0c736a88db35a48ad554cbc2770fa6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 3 May 2017 10:56:22 +0100 Subject: [PATCH 10/23] List caught expection types --- synapse/http/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index 379ddcb540..f8dff37d24 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -260,7 +260,7 @@ class SimpleHttpClient(object): errcode = jsonBody['errcode'] error = jsonBody['error'] return MatrixCodeMessageException(response.code, error, errcode) - except: + except (ValueError, KeyError) as e: return CodeMessageException(response.code, body) # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. From 482a2ad12239cfcc4886e33a0b3ac5e16196e03a Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 3 May 2017 11:02:59 +0100 Subject: [PATCH 11/23] No need for the exception variable --- synapse/http/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index f8dff37d24..9cf797043a 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -260,7 +260,7 @@ class SimpleHttpClient(object): errcode = jsonBody['errcode'] error = jsonBody['error'] return MatrixCodeMessageException(response.code, error, errcode) - except (ValueError, KeyError) as e: + except (ValueError, KeyError): return CodeMessageException(response.code, body) # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. From 34ed4f4206b4ff6830c381beabbaa7739b1a63f8 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 3 May 2017 11:55:44 +0100 Subject: [PATCH 12/23] Implement username availability checker Outlined here: https://github.com/vector-im/riot-web/issues/3605#issuecomment-298679388 ```HTTP GET /_matrix/.../register/available { "username": "desiredlocalpart123" } ``` If available, the response looks like ```HTTP HTTP/1.1 200 OK { "available": true } ``` Otherwise, ```HTTP HTTP/1.1 429 { "errcode": "M_LIMIT_EXCEEDED", "error": "Too Many Requests", "retry_after_ms": 2000 } ``` or ```HTTP HTTP/1.1 400 { "errcode": "M_USER_IN_USE", "error": "User ID already taken." } ``` or ```HTTP HTTP/1.1 400 { "errcode": "M_INVALID_USERNAME", "error": "Some reason for username being invalid" } ``` --- synapse/rest/client/v2_alpha/register.py | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 3acf4eacdd..49fee04ec8 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -31,6 +31,7 @@ import logging import hmac from hashlib import sha1 from synapse.util.async import run_on_reactor +from synapse.util.ratelimitutils import FederationRateLimiter # We ought to be using hmac.compare_digest() but on older pythons it doesn't @@ -115,6 +116,40 @@ class MsisdnRegisterRequestTokenRestServlet(RestServlet): defer.returnValue((200, ret)) +class UsernameAvailabilityRestServlet(RestServlet): + PATTERNS = client_v2_patterns("/register/available") + + def __init__(self, hs): + """ + Args: + hs (synapse.server.HomeServer): server + """ + super(UsernameAvailabilityRestServlet, self).__init__() + self.hs = hs + self.registration_handler = hs.get_handlers().registration_handler + self.ratelimiter = FederationRateLimiter( + hs.get_clock(), + window_size=2000, # Time window of 2s + sleep_limit=1, # Artificially delay requests if rate > sleep_limit/window_size + sleep_msec=1000, # Amount of artificial delay to apply + reject_limit=1, # Error with 429 if more than reject_limit requests are queued + concurrent_requests=1, # Allow 1 request at a time + ) + + @defer.inlineCallbacks + def on_GET(self, request): + ip = self.hs.get_ip_from_request(request) + with self.ratelimiter.ratelimit(ip) as wait_deferred: + yield wait_deferred + + body = parse_json_object_from_request(request) + assert_params_in_request(body, ['username']) + + yield self.registration_handler.check_username(body['username']) + + defer.returnValue((200, {"available": True})) + + class RegisterRestServlet(RestServlet): PATTERNS = client_v2_patterns("/register$") @@ -555,4 +590,5 @@ class RegisterRestServlet(RestServlet): def register_servlets(hs, http_server): EmailRegisterRequestTokenRestServlet(hs).register(http_server) MsisdnRegisterRequestTokenRestServlet(hs).register(http_server) + UsernameAvailabilityRestServlet(hs).register(http_server) RegisterRestServlet(hs).register(http_server) From 7ebf518c028088d1932262649c1042fb76f9d013 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 May 2017 10:43:34 +0100 Subject: [PATCH 13/23] Make get_joined_users faster --- synapse/storage/roommember.py | 54 ++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 7ad2198d96..46efdfb3e2 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -417,25 +417,47 @@ class RoomMemberStore(SQLBaseStore): if key[0] == EventTypes.Member ] - rows = yield self._simple_select_many_batch( - table="room_memberships", - column="event_id", - iterable=member_event_ids, - retcols=['user_id', 'display_name', 'avatar_url'], - keyvalues={ - "membership": Membership.JOIN, - }, - batch_size=500, - desc="_get_joined_users_from_context", + event_map = self._get_events_from_cache( + member_event_ids, + allow_rejected=False, ) - users_in_room = { - to_ascii(row["user_id"]): ProfileInfo( - avatar_url=to_ascii(row["avatar_url"]), - display_name=to_ascii(row["display_name"]), + missing_member_event_ids = [] + users_in_room = {} + for event_id, ev_entry in event_map.iteritems(): + if event_id: + if ev_entry.event.membership == Membership.JOIN: + users_in_room[to_ascii(ev_entry.event.state_key)] = ProfileInfo( + display_name=to_ascii( + ev_entry.event.content.get("displayname", None) + ), + avatar_url=to_ascii( + ev_entry.event.content.get("avatar_url", None) + ), + ) + else: + missing_member_event_ids.append(event_id) + + if missing_member_event_ids: + rows = yield self._simple_select_many_batch( + table="room_memberships", + column="event_id", + iterable=member_event_ids, + retcols=('user_id', 'display_name', 'avatar_url',), + keyvalues={ + "membership": Membership.JOIN, + }, + batch_size=500, + desc="_get_joined_users_from_context", ) - for row in rows - } + + users_in_room.update({ + to_ascii(row["user_id"]): ProfileInfo( + avatar_url=to_ascii(row["avatar_url"]), + display_name=to_ascii(row["display_name"]), + ) + for row in rows + }) if event is not None and event.type == EventTypes.Member: if event.membership == Membership.JOIN: From 3669065466ea35d5337631a7a089c03a65e9ddb8 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 3 May 2017 18:05:49 +0100 Subject: [PATCH 14/23] Appease the flake8 gods --- synapse/rest/client/v2_alpha/register.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 49fee04ec8..38a739f2f8 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -129,11 +129,16 @@ class UsernameAvailabilityRestServlet(RestServlet): self.registration_handler = hs.get_handlers().registration_handler self.ratelimiter = FederationRateLimiter( hs.get_clock(), - window_size=2000, # Time window of 2s - sleep_limit=1, # Artificially delay requests if rate > sleep_limit/window_size - sleep_msec=1000, # Amount of artificial delay to apply - reject_limit=1, # Error with 429 if more than reject_limit requests are queued - concurrent_requests=1, # Allow 1 request at a time + # Time window of 2s + window_size=2000, + # Artificially delay requests if rate > sleep_limit/window_size + sleep_limit=1, + # Amount of artificial delay to apply + sleep_msec=1000, + # Error with 429 if more than reject_limit requests are queued + reject_limit=1, + # Allow 1 request at a time + concurrent_requests=1, ) @defer.inlineCallbacks From 5d8290429ce0ae37a6ef86dd5b96ca82d090ae39 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 May 2017 13:43:19 +0100 Subject: [PATCH 15/23] Reduce size of get_users_in_room --- synapse/storage/roommember.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 7ad2198d96..f2630787ba 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -147,7 +147,7 @@ class RoomMemberStore(SQLBaseStore): hosts = frozenset(get_domain_from_id(user_id) for user_id in user_ids) defer.returnValue(hosts) - @cached(max_entries=500000, iterable=True) + @cached(max_entries=100000, iterable=True) def get_users_in_room(self, room_id): def f(txn): sql = ( @@ -160,7 +160,7 @@ class RoomMemberStore(SQLBaseStore): ) txn.execute(sql, (room_id, Membership.JOIN,)) - return [r[0] for r in txn] + return [to_ascii(r[0]) for r in txn] return self.runInteraction("get_users_in_room", f) @cached() From d2d8ed48848fd61b34b4327117bdf0d0e84ea285 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 May 2017 14:18:46 +0100 Subject: [PATCH 16/23] Optimise caches with single key --- synapse/util/caches/descriptors.py | 42 +++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/synapse/util/caches/descriptors.py b/synapse/util/caches/descriptors.py index 807e147657..aa182eeac7 100644 --- a/synapse/util/caches/descriptors.py +++ b/synapse/util/caches/descriptors.py @@ -18,6 +18,7 @@ from synapse.util.async import ObservableDeferred from synapse.util import unwrapFirstError, logcontext from synapse.util.caches.lrucache import LruCache from synapse.util.caches.treecache import TreeCache, iterate_tree_cache_entry +from synapse.util.stringutils import to_ascii from . import register_cache @@ -163,10 +164,6 @@ class Cache(object): def invalidate(self, key): self.check_thread() - if not isinstance(key, tuple): - raise TypeError( - "The cache key must be a tuple not %r" % (type(key),) - ) # Increment the sequence number so that any SELECT statements that # raced with the INSERT don't update the cache (SYN-369) @@ -312,7 +309,7 @@ class CacheDescriptor(_CacheDescriptorBase): iterable=self.iterable, ) - def get_cache_key(args, kwargs): + def get_cache_key_gen(args, kwargs): """Given some args/kwargs return a generator that resolves into the cache_key. @@ -330,13 +327,29 @@ class CacheDescriptor(_CacheDescriptorBase): else: yield self.arg_defaults[nm] + # By default our cache key is a tuple, but if there is only one item + # then don't bother wrapping in a tuple. This is to save memory. + if self.num_args == 1: + nm = self.arg_names[0] + + def get_cache_key(args, kwargs): + if nm in kwargs: + return kwargs[nm] + elif len(args): + return args[0] + else: + return self.arg_defaults[nm] + else: + def get_cache_key(args, kwargs): + return tuple(get_cache_key_gen(args, kwargs)) + @functools.wraps(self.orig) def wrapped(*args, **kwargs): # If we're passed a cache_context then we'll want to call its invalidate() # whenever we are invalidated invalidate_callback = kwargs.pop("on_invalidate", None) - cache_key = tuple(get_cache_key(args, kwargs)) + cache_key = get_cache_key(args, kwargs) # Add our own `cache_context` to argument list if the wrapped function # has asked for one @@ -363,6 +376,11 @@ class CacheDescriptor(_CacheDescriptorBase): ret.addErrback(onErr) + # If our cache_key is a string, try to convert to ascii to save + # a bit of space in large caches + if isinstance(cache_key, basestring): + cache_key = to_ascii(cache_key) + result_d = ObservableDeferred(ret, consumeErrors=True) cache.set(cache_key, result_d, callback=invalidate_callback) observer = result_d.observe() @@ -372,10 +390,16 @@ class CacheDescriptor(_CacheDescriptorBase): else: return observer - wrapped.invalidate = cache.invalidate + if self.num_args == 1: + wrapped.invalidate = lambda key: cache.invalidate(key[0]) + wrapped.prefill = lambda key, val: cache.prefill(key[0], val) + else: + wrapped.invalidate = cache.invalidate + wrapped.invalidate_all = cache.invalidate_all + wrapped.invalidate_many = cache.invalidate_many + wrapped.prefill = cache.prefill + wrapped.invalidate_all = cache.invalidate_all - wrapped.invalidate_many = cache.invalidate_many - wrapped.prefill = cache.prefill wrapped.cache = cache obj.__dict__[self.orig.__name__] = wrapped From 9ac263ed1b6f5dee46e85fe42ddfe7f9239e9690 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 May 2017 14:29:03 +0100 Subject: [PATCH 17/23] Add new storage functions to slave store --- synapse/replication/slave/storage/events.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/replication/slave/storage/events.py b/synapse/replication/slave/storage/events.py index ab48ff925e..fcaf58b93b 100644 --- a/synapse/replication/slave/storage/events.py +++ b/synapse/replication/slave/storage/events.py @@ -144,6 +144,9 @@ class SlavedEventStore(BaseSlavedStore): RoomMemberStore.__dict__["_get_joined_users_from_context"] ) + get_joined_hosts = DataStore.get_joined_hosts.__func__ + _get_joined_hosts = RoomMemberStore.__dict__["_get_joined_hosts"] + get_recent_events_for_room = DataStore.get_recent_events_for_room.__func__ get_room_events_stream_for_rooms = ( DataStore.get_room_events_stream_for_rooms.__func__ From dfaa58f72d3455affcac58c8b604d21935183e88 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 May 2017 14:50:24 +0100 Subject: [PATCH 18/23] Fix comment and num args --- synapse/state.py | 2 +- synapse/storage/roommember.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/state.py b/synapse/state.py index f8b18a4a2d..02fee47f39 100644 --- a/synapse/state.py +++ b/synapse/state.py @@ -179,7 +179,7 @@ class StateHandler(object): def get_current_hosts_in_room(self, room_id, latest_event_ids=None): if not latest_event_ids: latest_event_ids = yield self.store.get_latest_event_ids_in_room(room_id) - logger.debug("calling resolve_state_groups from get_current_user_in_room") + logger.debug("calling resolve_state_groups from get_current_hosts_in_room") entry = yield self.resolve_state_groups(room_id, latest_event_ids) joined_hosts = yield self.store.get_joined_hosts( room_id, entry.state_id, entry.state diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 1c0fa8a680..c571da2ce4 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -494,7 +494,7 @@ class RoomMemberStore(SQLBaseStore): room_id, state_group, state_ids ) - @cachedInlineCallbacks(num_args=3) + @cachedInlineCallbacks(num_args=2) def _get_joined_hosts(self, room_id, state_group, current_state_ids): # We don't use `state_group`, its there so that we can cache based # on it. However, its important that its never None, since two current_state's From 07a07588a01a644837ccea57f2307d0450cd28d9 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 May 2017 14:52:28 +0100 Subject: [PATCH 19/23] Make caches bigger --- synapse/storage/roommember.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index c571da2ce4..1963b95724 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -494,7 +494,7 @@ class RoomMemberStore(SQLBaseStore): room_id, state_group, state_ids ) - @cachedInlineCallbacks(num_args=2) + @cachedInlineCallbacks(num_args=2, max_entries=10000, iterable=True) def _get_joined_hosts(self, room_id, state_group, current_state_ids): # We don't use `state_group`, its there so that we can cache based # on it. However, its important that its never None, since two current_state's From 537dbadea05dfcc7fa855d06ae0f81405ed4e81f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 May 2017 14:55:28 +0100 Subject: [PATCH 20/23] Intern host strings --- synapse/storage/roommember.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 1963b95724..6ec8a6345d 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -18,6 +18,7 @@ from twisted.internet import defer from collections import namedtuple from ._base import SQLBaseStore +from synapse.util.caches import intern_string from synapse.util.caches.descriptors import cached, cachedInlineCallbacks from synapse.util.stringutils import to_ascii @@ -516,7 +517,7 @@ class RoomMemberStore(SQLBaseStore): event = yield self.get_event(event_id, allow_none=True) if event and event.content["membership"] == Membership.JOIN: - joined_hosts.add(host) + joined_hosts.add(intern_string(host)) defer.returnValue(joined_hosts) From aa93cb9f442019dde1704a0c12b47e5b664e192a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 May 2017 14:59:28 +0100 Subject: [PATCH 21/23] Add comment --- synapse/storage/roommember.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 46efdfb3e2..dcb95b924a 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -417,6 +417,9 @@ class RoomMemberStore(SQLBaseStore): if key[0] == EventTypes.Member ] + # We check if we have any of the member event ids in the event cache + # before we ask the DB + event_map = self._get_events_from_cache( member_event_ids, allow_rejected=False, From 587f07543fcf604da58903b22a7448e631d797ea Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 May 2017 15:07:27 +0100 Subject: [PATCH 22/23] Revert "Prefill state caches" --- synapse/storage/_base.py | 8 ++++---- synapse/storage/events.py | 16 +++++----------- synapse/storage/state.py | 12 ------------ 3 files changed, 9 insertions(+), 27 deletions(-) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 58b73af7d2..c659004e8d 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -60,12 +60,12 @@ class LoggingTransaction(object): object.__setattr__(self, "database_engine", database_engine) object.__setattr__(self, "after_callbacks", after_callbacks) - def call_after(self, callback, *args, **kwargs): + def call_after(self, callback, *args): """Call the given callback on the main twisted thread after the transaction has finished. Used to invalidate the caches on the correct thread. """ - self.after_callbacks.append((callback, args, kwargs)) + self.after_callbacks.append((callback, args)) def __getattr__(self, name): return getattr(self.txn, name) @@ -319,8 +319,8 @@ class SQLBaseStore(object): inner_func, *args, **kwargs ) finally: - for after_callback, after_args, after_kwargs in after_callbacks: - after_callback(*after_args, **after_kwargs) + for after_callback, after_args in after_callbacks: + after_callback(*after_args) defer.returnValue(result) @defer.inlineCallbacks diff --git a/synapse/storage/events.py b/synapse/storage/events.py index d946024c9b..98707d40ee 100644 --- a/synapse/storage/events.py +++ b/synapse/storage/events.py @@ -374,12 +374,6 @@ class EventsStore(SQLBaseStore): new_forward_extremeties=new_forward_extremeties, ) persist_event_counter.inc_by(len(chunk)) - - for room_id, (_, _, new_state) in current_state_for_room.iteritems(): - self.get_current_state_ids.prefill( - (room_id, ), new_state - ) - for event, context in chunk: if context.app_service: origin_type = "local" @@ -441,10 +435,10 @@ class EventsStore(SQLBaseStore): Assumes that we are only persisting events for one room at a time. Returns: - 3-tuple (to_delete, to_insert, new_state) where both are state dicts, - i.e. (type, state_key) -> event_id. `to_delete` are the entries to + 2-tuple (to_delete, to_insert) where both are state dicts, i.e. + (type, state_key) -> event_id. `to_delete` are the entries to first be deleted from current_state_events, `to_insert` are entries - to insert. `new_state` is the full set of state. + to insert. May return None if there are no changes to be applied. """ # Now we need to work out the different state sets for @@ -551,7 +545,7 @@ class EventsStore(SQLBaseStore): if ev_id in events_to_insert } - defer.returnValue((to_delete, to_insert, current_state)) + defer.returnValue((to_delete, to_insert)) @defer.inlineCallbacks def get_event(self, event_id, check_redacted=True, @@ -704,7 +698,7 @@ class EventsStore(SQLBaseStore): def _update_current_state_txn(self, txn, state_delta_by_room): for room_id, current_state_tuple in state_delta_by_room.iteritems(): - to_delete, to_insert, _ = current_state_tuple + to_delete, to_insert = current_state_tuple txn.executemany( "DELETE FROM current_state_events WHERE event_id = ?", [(ev_id,) for ev_id in to_delete.itervalues()], diff --git a/synapse/storage/state.py b/synapse/storage/state.py index 03981f5d2b..a16afa8df5 100644 --- a/synapse/storage/state.py +++ b/synapse/storage/state.py @@ -227,18 +227,6 @@ class StateStore(SQLBaseStore): ], ) - # Prefill the state group cache with this group. - # It's fine to use the sequence like this as the state group map - # is immutable. (If the map wasn't immutable then this prefill could - # race with another update) - txn.call_after( - self._state_group_cache.update, - self._state_group_cache.sequence, - key=context.state_group, - value=context.current_state_ids, - full=True, - ) - self._simple_insert_many_txn( txn, table="event_to_state_groups", From cf589f2c1e68b71db80cbc2cf56f88118faf545b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 May 2017 10:17:56 +0100 Subject: [PATCH 23/23] Fixes --- synapse/storage/roommember.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index dcb95b924a..c63c0622dd 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -427,8 +427,9 @@ class RoomMemberStore(SQLBaseStore): missing_member_event_ids = [] users_in_room = {} - for event_id, ev_entry in event_map.iteritems(): - if event_id: + for event_id in member_event_ids: + ev_entry = event_map.get(event_id) + if ev_entry: if ev_entry.event.membership == Membership.JOIN: users_in_room[to_ascii(ev_entry.event.state_key)] = ProfileInfo( display_name=to_ascii( @@ -445,7 +446,7 @@ class RoomMemberStore(SQLBaseStore): rows = yield self._simple_select_many_batch( table="room_memberships", column="event_id", - iterable=member_event_ids, + iterable=missing_member_event_ids, retcols=('user_id', 'display_name', 'avatar_url',), keyvalues={ "membership": Membership.JOIN,