From 8714ff6d515ab0fb937518d4ba7d1d3b1593617d Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 10 May 2019 11:09:53 +0100 Subject: [PATCH 01/49] Add a DUMMY stage to captcha-only registration flow This allows the client to complete the email last which is more natual for the user. Without this stage, if the client would complete the recaptcha (and terms, if enabled) stages and then the registration request would complete because you've now completed a flow, even if you were intending to complete the flow that's the same except has email auth at the end. Adding a dummy auth stage to the recaptcha-only flow means it's always unambiguous which flow the client was trying to complete. Longer term we should think about changing the protocol so the client explicitly says which flow it's trying to complete. vector-im/riot-web#9586 --- synapse/rest/client/v2_alpha/register.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index dc3e265bcd1d..95578b76aea6 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -345,7 +345,7 @@ def on_POST(self, request): if self.hs.config.enable_registration_captcha: # only support 3PIDless registration if no 3PIDs are required if not require_email and not require_msisdn: - flows.extend([[LoginType.RECAPTCHA]]) + flows.extend([[LoginType.RECAPTCHA, LoginType.DUMMY]]) # only support the email-only flow if we don't require MSISDN 3PIDs if not require_msisdn: flows.extend([[LoginType.EMAIL_IDENTITY, LoginType.RECAPTCHA]]) From a18f93279eb7b8505db63af062695e8c7fe263c4 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 10 May 2019 11:11:59 +0100 Subject: [PATCH 02/49] Add changelog entry --- changelog.d/5174.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5174.bugfix diff --git a/changelog.d/5174.bugfix b/changelog.d/5174.bugfix new file mode 100644 index 000000000000..f4d3192bd9f5 --- /dev/null +++ b/changelog.d/5174.bugfix @@ -0,0 +1 @@ +Allow clients to complete email auth first when registering From 9c61dce3c81283f9c7bb47b634512dd848eb116e Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 10 May 2019 11:14:55 +0100 Subject: [PATCH 03/49] Comment --- synapse/rest/client/v2_alpha/register.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 95578b76aea6..5c4c8dca2826 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -345,6 +345,10 @@ def on_POST(self, request): if self.hs.config.enable_registration_captcha: # only support 3PIDless registration if no 3PIDs are required if not require_email and not require_msisdn: + # Also add a dummy flow here, otherwise if a client completes + # recaptcha first we'll assume they were going for this flow + # and complete the request, when they could have been trying to + # complete one of the flows with email/msisdn auth. flows.extend([[LoginType.RECAPTCHA, LoginType.DUMMY]]) # only support the email-only flow if we don't require MSISDN 3PIDs if not require_msisdn: From 7a3eb8657d66bf0ae479b5a0db9529df5d0226c1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 10 May 2019 11:18:35 +0100 Subject: [PATCH 04/49] Thanks, automated grammar pedantry. --- changelog.d/5174.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/5174.bugfix b/changelog.d/5174.bugfix index f4d3192bd9f5..2d7c71739b8c 100644 --- a/changelog.d/5174.bugfix +++ b/changelog.d/5174.bugfix @@ -1 +1 @@ -Allow clients to complete email auth first when registering +Allow clients to complete email auth first when registering. From 04299132af8dee4047a28f9d92ccd7ba2d1c10a0 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 10 May 2019 13:58:03 +0100 Subject: [PATCH 05/49] Re-order flows so that email auth is done last It's more natural for the user if the bit that takes them away from the registration flow comes last. Adding the dummy stage allows us to do the stages in this order without the ambiguity. --- synapse/rest/client/v2_alpha/register.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 5c4c8dca2826..c51241fa0153 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -352,15 +352,15 @@ def on_POST(self, request): flows.extend([[LoginType.RECAPTCHA, LoginType.DUMMY]]) # only support the email-only flow if we don't require MSISDN 3PIDs if not require_msisdn: - flows.extend([[LoginType.EMAIL_IDENTITY, LoginType.RECAPTCHA]]) + flows.extend([[LoginType.RECAPTCHA, LoginType.EMAIL_IDENTITY]]) if show_msisdn: # only support the MSISDN-only flow if we don't require email 3PIDs if not require_email: - flows.extend([[LoginType.MSISDN, LoginType.RECAPTCHA]]) + flows.extend([[LoginType.RECAPTCHA, LoginType.MSISDN]]) # always let users provide both MSISDN & email flows.extend([ - [LoginType.MSISDN, LoginType.EMAIL_IDENTITY, LoginType.RECAPTCHA], + [LoginType.RECAPTCHA, LoginType.MSISDN, LoginType.EMAIL_IDENTITY], ]) else: # only support 3PIDless registration if no 3PIDs are required @@ -383,7 +383,15 @@ def on_POST(self, request): if self.hs.config.user_consent_at_registration: new_flows = [] for flow in flows: - flow.append(LoginType.TERMS) + inserted = False + # m.login.terms should go near the end but before msisdn or email auth + for i, stage in enumerate(flow): + if stage == LoginType.EMAIL_IDENTITY or stage == LoginType.MSISDN: + flow.insert(i, LoginType.TERMS) + inserted = True + break + if not inserted: + flow.append(LoginType.TERMS) flows.extend(new_flows) auth_result, params, session_id = yield self.auth_handler.check_auth( From c9f811c5d45ac52d4b55ed74d2919b0d41790983 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 10 May 2019 14:01:19 +0100 Subject: [PATCH 06/49] Update changelog --- changelog.d/5174.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/5174.bugfix b/changelog.d/5174.bugfix index 2d7c71739b8c..0f26d46b2c61 100644 --- a/changelog.d/5174.bugfix +++ b/changelog.d/5174.bugfix @@ -1 +1 @@ -Allow clients to complete email auth first when registering. +Re-order stages in registration flows such that msisdn and email verification are done last. From 8782bfb783ef4da3882951e56d8911a1d81eada5 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 13 May 2019 15:34:11 +0100 Subject: [PATCH 07/49] And now I realise why the test is failing... --- tests/rest/client/v2_alpha/test_auth.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/v2_alpha/test_auth.py b/tests/rest/client/v2_alpha/test_auth.py index 0ca3c4657b15..fa833ba5a298 100644 --- a/tests/rest/client/v2_alpha/test_auth.py +++ b/tests/rest/client/v2_alpha/test_auth.py @@ -92,7 +92,14 @@ def test_fallback_captcha(self): self.assertEqual(len(self.recaptcha_attempts), 1) self.assertEqual(self.recaptcha_attempts[0][0]["response"], "a") - # Now we have fufilled the recaptcha fallback step, we can then send a + # also complete the dummy auth + request, channel = self.make_request( + "POST", "register", {"auth": {"session": session, "type": "m.login.dummy"}} + ) + self.render(request) + + # Now we should have fufilled a complete auth flow, including + # the recaptcha fallback step, we can then send a # request to the register API with the session in the authdict. request, channel = self.make_request( "POST", "register", {"auth": {"session": session}} From 822072b1bb8c68006dffff196885202d80099361 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 13 May 2019 16:10:26 +0100 Subject: [PATCH 08/49] Terms might not be the last stage --- tests/test_terms_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_terms_auth.py b/tests/test_terms_auth.py index f412985d2c8c..52739fbabc3a 100644 --- a/tests/test_terms_auth.py +++ b/tests/test_terms_auth.py @@ -59,7 +59,7 @@ def test_ui_auth(self): for flow in channel.json_body["flows"]: self.assertIsInstance(flow["stages"], list) self.assertTrue(len(flow["stages"]) > 0) - self.assertEquals(flow["stages"][-1], "m.login.terms") + self.assertTrue("m.login.terms" in flow["stages"]) expected_params = { "m.login.terms": { From 6ca88c469373a7ef345d05a4b69afe810e240183 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 14 May 2019 19:04:59 +0100 Subject: [PATCH 09/49] Only check 3pids not in use when registering We checked that 3pids were not already in use before we checked if we were going to return the account previously registered in the same UI auth session, in which case the 3pids will definitely be in use. https://github.com/vector-im/riot-web/issues/9586 --- synapse/rest/client/v2_alpha/register.py | 40 +++++++++++++----------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index dc3e265bcd1d..ecec610859cf 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -391,13 +391,6 @@ def on_POST(self, request): # the user-facing checks will probably already have happened in # /register/email/requestToken when we requested a 3pid, but that's not # guaranteed. - # - # Also check that we're not trying to register a 3pid that's already - # been registered. - # - # This has probably happened in /register/email/requestToken as well, - # but if a user hits this endpoint twice then clicks on each link from - # the two activation emails, they would register the same 3pid twice. if auth_result: for login_type in [LoginType.EMAIL_IDENTITY, LoginType.MSISDN]: @@ -413,17 +406,6 @@ def on_POST(self, request): Codes.THREEPID_DENIED, ) - existingUid = yield self.store.get_user_id_by_threepid( - medium, address, - ) - - if existingUid is not None: - raise SynapseError( - 400, - "%s is already in use" % medium, - Codes.THREEPID_IN_USE, - ) - if registered_user_id is not None: logger.info( "Already registered user ID %r for this session", @@ -446,6 +428,28 @@ def on_POST(self, request): if auth_result: threepid = auth_result.get(LoginType.EMAIL_IDENTITY) + # Also check that we're not trying to register a 3pid that's already + # been registered. + # + # This has probably happened in /register/email/requestToken as well, + # but if a user hits this endpoint twice then clicks on each link from + # the two activation emails, they would register the same 3pid twice. + for login_type in [LoginType.EMAIL_IDENTITY, LoginType.MSISDN]: + if login_type in auth_result: + medium = auth_result[login_type]['medium'] + address = auth_result[login_type]['address'] + + existingUid = yield self.store.get_user_id_by_threepid( + medium, address, + ) + + if existingUid is not None: + raise SynapseError( + 400, + "%s is already in use" % medium, + Codes.THREEPID_IN_USE, + ) + (registered_user_id, _) = yield self.registration_handler.register( localpart=desired_username, password=new_password, From efefb5bda2364de6c205a8e3fa2e8be95b2ae89e Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 14 May 2019 19:18:42 +0100 Subject: [PATCH 10/49] Have I got newsfile for you --- changelog.d/5187.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5187.bugfix diff --git a/changelog.d/5187.bugfix b/changelog.d/5187.bugfix new file mode 100644 index 000000000000..03b985265fb0 --- /dev/null +++ b/changelog.d/5187.bugfix @@ -0,0 +1 @@ +Fix register endpoint returning a previously registered account. From 54d77107c1fde88333ecccedfe30f26fb7645847 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 15 May 2019 11:27:50 +0100 Subject: [PATCH 11/49] Make generating SQL bounds for pagination generic This will allow us to reuse the same structure when we paginate e.g. relations --- synapse/storage/stream.py | 179 +++++++++++++++++++++++++------------- 1 file changed, 118 insertions(+), 61 deletions(-) diff --git a/synapse/storage/stream.py b/synapse/storage/stream.py index d105b6b17d95..163363c0c254 100644 --- a/synapse/storage/stream.py +++ b/synapse/storage/stream.py @@ -64,59 +64,120 @@ ) -def lower_bound(token, engine, inclusive=False): - inclusive = "=" if inclusive else "" - if token.topological is None: - return "(%d <%s %s)" % (token.stream, inclusive, "stream_ordering") - else: - if isinstance(engine, PostgresEngine): - # Postgres doesn't optimise ``(x < a) OR (x=a AND y= (topological_ordering, stream_ordering) + AND (5, 3) < (topological_ordering, stream_ordering) + + would be generated for dir=b, from_token=(6, 7) and to_token=(5, 3). + + Args: + direction (str): Whether we're paginating backwards("b") or + forwards ("f"). + column_names (tuple[str, str]): The column names to bound. Must *not* + be user defined as these get inserted directly into the SQL + statement without escapes. + from_token (tuple[int, int]|None) + to_token (tuple[int, int]|None) + engine: The database engine to generate the clauses for + + Returns: + str: The sql expression + """ + assert direction in ("b", "f") + + where_clause = [] + if from_token: + where_clause.append( + _make_generic_sql_bound( + bound=">=" if direction == "b" else "<", + column_names=column_names, + values=from_token, + engine=engine, ) - return "(%d < %s OR (%d = %s AND %d <%s %s))" % ( - token.topological, - "topological_ordering", - token.topological, - "topological_ordering", - token.stream, - inclusive, - "stream_ordering", - ) - - -def upper_bound(token, engine, inclusive=True): - inclusive = "=" if inclusive else "" - if token.topological is None: - return "(%d >%s %s)" % (token.stream, inclusive, "stream_ordering") - else: - if isinstance(engine, PostgresEngine): - # Postgres doesn't optimise ``(x > a) OR (x=a AND y>b)`` as well - # as it optimises ``(x,y) > (a,b)`` on multicolumn indexes. So we - # use the later form when running against postgres. - return "((%d,%d) >%s (%s,%s))" % ( - token.topological, - token.stream, - inclusive, - "topological_ordering", - "stream_ordering", + ) + + if to_token: + where_clause.append( + _make_generic_sql_bound( + bound="<" if direction == "b" else ">=", + column_names=column_names, + values=to_token, + engine=engine, ) - return "(%d > %s OR (%d = %s AND %d >%s %s))" % ( - token.topological, - "topological_ordering", - token.topological, - "topological_ordering", - token.stream, - inclusive, - "stream_ordering", ) + return " AND ".join(where_clause) + + +def _make_generic_sql_bound(bound, column_names, values, engine): + """Create an SQL expression that bounds the given column names by the + values, e.g. create the equivalent of `(1, 2) < (col1, col2)`. + + Only works with two columns. + + Older versions of SQLite don't support that syntax so we have to expand it + out manually. + + Args: + bound (str): The comparison operator to use. One of ">", "<", ">=", + "<=", where the values are on the left and columns on the right. + names (tuple[str, str]): The column names. Must *not* be user defined + as these get inserted directly into the SQL statement without + escapes. + values (tuple[int, int]): The values to bound the columns by. + engine: The database engine to generate the SQL for + + Returns: + str + """ + + assert(bound in (">", "<", ">=", "<=")) + + name1, name2 = column_names + val1, val2 = values + + if val1 is None: + val2 = int(val2) + return "(%d %s %s)" % (val2, bound, name2) + + val1 = int(val1) + val2 = int(val2) + + if isinstance(engine, PostgresEngine): + # Postgres doesn't optimise ``(x < a) OR (x=a AND y Date: Wed, 15 May 2019 11:33:22 +0100 Subject: [PATCH 12/49] Newsfile --- changelog.d/5191.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5191.misc diff --git a/changelog.d/5191.misc b/changelog.d/5191.misc new file mode 100644 index 000000000000..e0615fec9cf9 --- /dev/null +++ b/changelog.d/5191.misc @@ -0,0 +1 @@ +Make generating SQL bounds for pagination generic. From efe3c7977a3dc9c4308388f71e82b837df7d09b4 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 14 May 2019 16:59:21 +0100 Subject: [PATCH 13/49] Add simple send_relation API and track in DB --- synapse/api/constants.py | 8 ++ synapse/rest/__init__.py | 2 + synapse/rest/client/v2_alpha/relations.py | 110 ++++++++++++++++++ synapse/storage/__init__.py | 2 + synapse/storage/events.py | 2 + synapse/storage/relations.py | 62 ++++++++++ synapse/storage/schema/delta/54/relations.sql | 27 +++++ tests/rest/client/v2_alpha/test_relations.py | 98 ++++++++++++++++ 8 files changed, 311 insertions(+) create mode 100644 synapse/rest/client/v2_alpha/relations.py create mode 100644 synapse/storage/relations.py create mode 100644 synapse/storage/schema/delta/54/relations.sql create mode 100644 tests/rest/client/v2_alpha/test_relations.py diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 8547a635350a..30bebd749f62 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -116,3 +116,11 @@ class UserTypes(object): """ SUPPORT = "support" ALL_USER_TYPES = (SUPPORT,) + + +class RelationTypes(object): + """The types of relations known to this server. + """ + ANNOTATION = "m.annotation" + REPLACES = "m.replaces" + REFERENCES = "m.references" diff --git a/synapse/rest/__init__.py b/synapse/rest/__init__.py index 3a24d31d1ba7..e6110ad9b1aa 100644 --- a/synapse/rest/__init__.py +++ b/synapse/rest/__init__.py @@ -44,6 +44,7 @@ read_marker, receipts, register, + relations, report_event, room_keys, room_upgrade_rest_servlet, @@ -115,6 +116,7 @@ def register_servlets(client_resource, hs): room_upgrade_rest_servlet.register_servlets(hs, client_resource) capabilities.register_servlets(hs, client_resource) account_validity.register_servlets(hs, client_resource) + relations.register_servlets(hs, client_resource) # moving to /_synapse/admin synapse.rest.admin.register_servlets_for_client_rest_resource( diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py new file mode 100644 index 000000000000..b504b4a8be28 --- /dev/null +++ b/synapse/rest/client/v2_alpha/relations.py @@ -0,0 +1,110 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 New Vector Ltd +# +# 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. + +"""This class implements the proposed relation APIs from MSC 1849. + +Since the MSC has not been approved all APIs here are unstable and may change at +any time to reflect changes in the MSC. +""" + +import logging + +from twisted.internet import defer + +from synapse.api.constants import EventTypes +from synapse.api.errors import SynapseError +from synapse.http.servlet import ( + RestServlet, + parse_json_object_from_request, + parse_string, +) + +from ._base import client_v2_patterns + +logger = logging.getLogger(__name__) + + +class RelationSendServlet(RestServlet): + """Helper API for sending events that have relation data. + + Example API shape to send a 👍 reaction to a room: + + POST /rooms/!foo/send_relation/$bar/m.annotation/m.reaction?key=%F0%9F%91%8D + {} + + { + "event_id": "$foobar" + } + """ + + PATTERN = ( + "/rooms/(?P[^/]*)/send_relation" + "/(?P[^/]*)/(?P[^/]*)/(?P[^/]*)" + ) + + def __init__(self, hs): + super(RelationSendServlet, self).__init__() + self.auth = hs.get_auth() + self.event_creation_handler = hs.get_event_creation_handler() + + def register(self, http_server): + http_server.register_paths( + "POST", + client_v2_patterns(self.PATTERN + "$", releases=()), + self.on_PUT_or_POST, + ) + http_server.register_paths( + "PUT", + client_v2_patterns(self.PATTERN + "/(?P[^/]*)$", releases=()), + self.on_PUT_or_POST, + ) + + @defer.inlineCallbacks + def on_PUT_or_POST( + self, request, room_id, parent_id, relation_type, event_type, txn_id=None + ): + requester = yield self.auth.get_user_by_req(request, allow_guest=True) + + if event_type == EventTypes.Member: + # Add relations to a membership is meaningless, so we just deny it + # at the CS API rather than trying to handle it correctly. + raise SynapseError(400, "Cannot send member events with relations") + + content = parse_json_object_from_request(request) + + aggregation_key = parse_string(request, "key", encoding="utf-8") + + content["m.relates_to"] = { + "event_id": parent_id, + "key": aggregation_key, + "rel_type": relation_type, + } + + event_dict = { + "type": event_type, + "content": content, + "room_id": room_id, + "sender": requester.user.to_string(), + } + + event = yield self.event_creation_handler.create_and_send_nonmember_event( + requester, event_dict=event_dict, txn_id=txn_id + ) + + defer.returnValue((200, {"event_id": event.event_id})) + + +def register_servlets(hs, http_server): + RelationSendServlet(hs).register(http_server) diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index c432041b4ee1..7522d3fd5731 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -49,6 +49,7 @@ from .receipts import ReceiptsStore from .registration import RegistrationStore from .rejections import RejectionsStore +from .relations import RelationsStore from .room import RoomStore from .roommember import RoomMemberStore from .search import SearchStore @@ -99,6 +100,7 @@ class DataStore( GroupServerStore, UserErasureStore, MonthlyActiveUsersStore, + RelationsStore, ): def __init__(self, db_conn, hs): self.hs = hs diff --git a/synapse/storage/events.py b/synapse/storage/events.py index 7a7f841c6ceb..6802bf42ce4e 100644 --- a/synapse/storage/events.py +++ b/synapse/storage/events.py @@ -1351,6 +1351,8 @@ def _update_metadata_tables_txn( # Insert into the event_search table. self._store_guest_access_txn(txn, event) + self._handle_event_relations(txn, event) + # Insert into the room_memberships table. self._store_room_members_txn( txn, diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py new file mode 100644 index 000000000000..a4905162e0ab --- /dev/null +++ b/synapse/storage/relations.py @@ -0,0 +1,62 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 New Vector Ltd +# +# 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. + +import logging + +from synapse.api.constants import RelationTypes +from synapse.storage._base import SQLBaseStore + +logger = logging.getLogger(__name__) + + +class RelationsStore(SQLBaseStore): + def _handle_event_relations(self, txn, event): + """Handles inserting relation data during peristence of events + + Args: + txn + event (EventBase) + """ + relation = event.content.get("m.relates_to") + if not relation: + # No relations + return + + rel_type = relation.get("rel_type") + if rel_type not in ( + RelationTypes.ANNOTATION, + RelationTypes.REFERENCES, + RelationTypes.REPLACES, + ): + # Unknown relation type + return + + parent_id = relation.get("event_id") + if not parent_id: + # Invalid relation + return + + aggregation_key = relation.get("key") + + self._simple_insert_txn( + txn, + table="event_relations", + values={ + "event_id": event.event_id, + "relates_to_id": parent_id, + "relation_type": rel_type, + "aggregation_key": aggregation_key, + }, + ) diff --git a/synapse/storage/schema/delta/54/relations.sql b/synapse/storage/schema/delta/54/relations.sql new file mode 100644 index 000000000000..134862b8702c --- /dev/null +++ b/synapse/storage/schema/delta/54/relations.sql @@ -0,0 +1,27 @@ +/* Copyright 2019 New Vector Ltd + * + * 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. + */ + +-- Tracks related events, like reactions, replies, edits, etc. Note that things +-- in this table are not necessarily "valid", e.g. it may contain edits from +-- people who don't have power to edit other peoples events. +CREATE TABLE IF NOT EXISTS event_relations ( + event_id TEXT NOT NULL, + relates_to_id TEXT NOT NULL, + relation_type TEXT NOT NULL, + aggregation_key TEXT +); + +CREATE UNIQUE INDEX event_relations_id ON event_relations(event_id); +CREATE INDEX event_relations_relates ON event_relations(relates_to_id, relation_type, aggregation_key); diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py new file mode 100644 index 000000000000..61163d5b268d --- /dev/null +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -0,0 +1,98 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 New Vector Ltd +# +# 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. + +import six + +from synapse.api.constants import EventTypes, RelationTypes +from synapse.rest.client.v1 import login, room +from synapse.rest.client.v2_alpha import relations + +from tests import unittest + + +class RelationsTestCase(unittest.HomeserverTestCase): + user_id = "@alice:test" + servlets = [ + relations.register_servlets, + room.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor, clock, hs): + self.room = self.helper.create_room_as(self.user_id) + res = self.helper.send(self.room, body="Hi!") + self.parent_id = res["event_id"] + + def test_send_relation(self): + """Tests that sending a relation using the new /send_relation works + creates the right shape of event. + """ + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key="👍") + self.assertEquals(200, channel.code, channel.json_body) + + event_id = channel.json_body["event_id"] + + request, channel = self.make_request( + "GET", "/rooms/%s/event/%s" % (self.room, event_id) + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.assert_dict( + { + "type": "m.reaction", + "sender": self.user_id, + "content": { + "m.relates_to": { + "event_id": self.parent_id, + "key": u"👍", + "rel_type": RelationTypes.ANNOTATION, + } + }, + }, + channel.json_body, + ) + + def test_deny_membership(self): + """Test that we deny relations on membership events + """ + channel = self._send_relation(RelationTypes.ANNOTATION, EventTypes.Member) + self.assertEquals(400, channel.code, channel.json_body) + + def _send_relation(self, relation_type, event_type, key=None): + """Helper function to send a relation pointing at `self.parent_id` + + Args: + relation_type (str): One of `RelationTypes` + event_type (str): The type of the event to create + key (str|None): The aggregation key used for m.annotation relation + type. + + Returns: + FakeChannel + """ + query = "" + if key: + query = "?key=" + six.moves.urllib.parse.quote_plus(key) + + request, channel = self.make_request( + "POST", + "/_matrix/client/unstable/rooms/%s/send_relation/%s/%s/%s%s" + % (self.room, self.parent_id, relation_type, event_type, query), + b"{}", + ) + self.render(request) + return channel From b50641e357f6139b0807df3714440a8ccc21d628 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 14 May 2019 16:59:21 +0100 Subject: [PATCH 14/49] Add simple pagination API --- synapse/rest/client/v2_alpha/relations.py | 50 ++++++++++++ synapse/storage/relations.py | 80 ++++++++++++++++++++ tests/rest/client/v2_alpha/test_relations.py | 30 ++++++++ 3 files changed, 160 insertions(+) diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py index b504b4a8be28..bac9e85c21c0 100644 --- a/synapse/rest/client/v2_alpha/relations.py +++ b/synapse/rest/client/v2_alpha/relations.py @@ -27,6 +27,7 @@ from synapse.api.errors import SynapseError from synapse.http.servlet import ( RestServlet, + parse_integer, parse_json_object_from_request, parse_string, ) @@ -106,5 +107,54 @@ def on_PUT_or_POST( defer.returnValue((200, {"event_id": event.event_id})) +class RelationPaginationServlet(RestServlet): + """API to paginate relations on an event by topological ordering, optionally + filtered by relation type and event type. + """ + + PATTERNS = client_v2_patterns( + "/rooms/(?P[^/]*)/relations/(?P[^/]*)" + "(/(?P[^/]*)(/(?P[^/]*))?)?$", + releases=(), + ) + + def __init__(self, hs): + super(RelationPaginationServlet, self).__init__() + self.auth = hs.get_auth() + self.store = hs.get_datastore() + self.clock = hs.get_clock() + self._event_serializer = hs.get_event_client_serializer() + + @defer.inlineCallbacks + def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=None): + requester = yield self.auth.get_user_by_req(request, allow_guest=True) + + yield self.auth.check_in_room_or_world_readable( + room_id, requester.user.to_string() + ) + + limit = parse_integer(request, "limit", default=5) + + result = yield self.store.get_relations_for_event( + event_id=parent_id, + relation_type=relation_type, + event_type=event_type, + limit=limit, + ) + + events = yield self.store.get_events_as_list( + [c["event_id"] for c in result.chunk] + ) + + now = self.clock.time_msec() + events = yield self._event_serializer.serialize_events(events, now) + + return_value = result.to_dict() + return_value["chunk"] = events + + defer.returnValue((200, return_value)) + + def register_servlets(hs, http_server): RelationSendServlet(hs).register(http_server) + RelationPaginationServlet(hs).register(http_server) diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index a4905162e0ab..f304b8a9a48a 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -15,13 +15,93 @@ import logging +import attr + from synapse.api.constants import RelationTypes from synapse.storage._base import SQLBaseStore logger = logging.getLogger(__name__) +@attr.s +class PaginationChunk(object): + """Returned by relation pagination APIs. + + Attributes: + chunk (list): The rows returned by pagination + """ + + chunk = attr.ib() + + def to_dict(self): + d = {"chunk": self.chunk} + + return d + + class RelationsStore(SQLBaseStore): + def get_relations_for_event( + self, event_id, relation_type=None, event_type=None, limit=5, direction="b" + ): + """Get a list of relations for an event, ordered by topological ordering. + + Args: + event_id (str): Fetch events that relate to this event ID. + relation_type (str|None): Only fetch events with this relation + type, if given. + event_type (str|None): Only fetch events with this event type, if + given. + limit (int): Only fetch the most recent `limit` events. + direction (str): Whether to fetch the most recent first (`"b"`) or + the oldest first (`"f"`). + + Returns: + Deferred[PaginationChunk]: List of event IDs that match relations + requested. The rows are of the form `{"event_id": "..."}`. + """ + + # TODO: Pagination tokens + + where_clause = ["relates_to_id = ?"] + where_args = [event_id] + + if relation_type: + where_clause.append("relation_type = ?") + where_args.append(relation_type) + + if event_type: + where_clause.append("type = ?") + where_args.append(event_type) + + order = "ASC" + if direction == "b": + order = "DESC" + + sql = """ + SELECT event_id FROM event_relations + INNER JOIN events USING (event_id) + WHERE %s + ORDER BY topological_ordering %s, stream_ordering %s + LIMIT ? + """ % ( + " AND ".join(where_clause), + order, + order, + ) + + def _get_recent_references_for_event_txn(txn): + txn.execute(sql, where_args + [limit + 1]) + + events = [{"event_id": row[0]} for row in txn] + + return PaginationChunk( + chunk=list(events[:limit]), + ) + + return self.runInteraction( + "get_recent_references_for_event", _get_recent_references_for_event_txn + ) + def _handle_event_relations(self, txn, event): """Handles inserting relation data during peristence of events diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index 61163d5b268d..bcc1c1bb8579 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -72,6 +72,36 @@ def test_deny_membership(self): channel = self._send_relation(RelationTypes.ANNOTATION, EventTypes.Member) self.assertEquals(400, channel.code, channel.json_body) + def test_paginate(self): + """Tests that calling pagination API corectly the latest relations. + """ + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction") + self.assertEquals(200, channel.code, channel.json_body) + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction") + self.assertEquals(200, channel.code, channel.json_body) + annotation_id = channel.json_body["event_id"] + + request, channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/relations/%s?limit=1" + % (self.room, self.parent_id), + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + # We expect to get back a single pagination result, which is the full + # relation event we sent above. + self.assertEquals(len(channel.json_body["chunk"]), 1, channel.json_body) + self.assert_dict( + { + "event_id": annotation_id, + "sender": self.user_id, + "type": "m.reaction", + }, + channel.json_body["chunk"][0], + ) + def _send_relation(self, relation_type, event_type, key=None): """Helper function to send a relation pointing at `self.parent_id` From 5fb72e6888c2dbd54a7b7d76657ea36c0faccedc Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 14 May 2019 17:05:45 +0100 Subject: [PATCH 15/49] Newsfile --- changelog.d/5198.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5198.feature diff --git a/changelog.d/5198.feature b/changelog.d/5198.feature new file mode 100644 index 000000000000..c8f624d17231 --- /dev/null +++ b/changelog.d/5198.feature @@ -0,0 +1 @@ +Add experimental support for reactions. From e6459c26b45866558056bee991024e093e0de059 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 15 May 2019 17:28:33 +0100 Subject: [PATCH 16/49] Actually implement idempotency --- synapse/rest/client/v2_alpha/relations.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py index bac9e85c21c0..c3ac73b8c79e 100644 --- a/synapse/rest/client/v2_alpha/relations.py +++ b/synapse/rest/client/v2_alpha/relations.py @@ -31,6 +31,7 @@ parse_json_object_from_request, parse_string, ) +from synapse.rest.client.transactions import HttpTransactionCache from ._base import client_v2_patterns @@ -59,6 +60,7 @@ def __init__(self, hs): super(RelationSendServlet, self).__init__() self.auth = hs.get_auth() self.event_creation_handler = hs.get_event_creation_handler() + self.txns = HttpTransactionCache(hs) def register(self, http_server): http_server.register_paths( @@ -69,7 +71,12 @@ def register(self, http_server): http_server.register_paths( "PUT", client_v2_patterns(self.PATTERN + "/(?P[^/]*)$", releases=()), - self.on_PUT_or_POST, + self.on_PUT, + ) + + def on_PUT(self, request, *args, **kwargs): + return self.txns.fetch_or_execute_request( + request, self.on_PUT_or_POST, request, *args, **kwargs ) @defer.inlineCallbacks From 5be34fc3e3045b3ba5a97b73ba0f25887874e622 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 15 May 2019 17:30:23 +0100 Subject: [PATCH 17/49] Actually check for None rather falsey --- synapse/storage/relations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index f304b8a9a48a..31ef6679af6d 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -65,11 +65,11 @@ def get_relations_for_event( where_clause = ["relates_to_id = ?"] where_args = [event_id] - if relation_type: + if relation_type is not None: where_clause.append("relation_type = ?") where_args.append(relation_type) - if event_type: + if event_type is not None: where_clause.append("type = ?") where_args.append(event_type) From cd0faba7cd8d469533342a35882648051b393744 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 15 May 2019 20:53:48 +0100 Subject: [PATCH 18/49] Make newsfile clearer --- changelog.d/5187.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/5187.bugfix b/changelog.d/5187.bugfix index 03b985265fb0..df176cf5b20b 100644 --- a/changelog.d/5187.bugfix +++ b/changelog.d/5187.bugfix @@ -1 +1 @@ -Fix register endpoint returning a previously registered account. +Fix a bug where the register endpoint would fail with M_THREEPID_IN_USE instead of returning an account previously registered in the same session. From a0603523d2e210cf59f887bd75e1a755720cb7a8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 14 May 2019 16:59:21 +0100 Subject: [PATCH 19/49] Add aggregations API --- synapse/config/server.py | 5 + synapse/events/utils.py | 34 ++- synapse/rest/client/v2_alpha/relations.py | 141 ++++++++++- synapse/storage/relations.py | 225 ++++++++++++++++- tests/rest/client/v2_alpha/test_relations.py | 251 ++++++++++++++++++- 5 files changed, 643 insertions(+), 13 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 7874cd9da71c..f403477b5419 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -100,6 +100,11 @@ def read_config(self, config): "block_non_admin_invites", False, ) + # Whether to enable experimental MSC1849 (aka relations) support + self.experimental_msc1849_support_enabled = config.get( + "experimental_msc1849_support_enabled", False, + ) + # Options to control access by tracking MAU self.limit_usage_by_mau = config.get("limit_usage_by_mau", False) self.max_mau_value = 0 diff --git a/synapse/events/utils.py b/synapse/events/utils.py index a5454556ccae..16d0c643723e 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -21,7 +21,7 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, RelationTypes from synapse.util.async_helpers import yieldable_gather_results from . import EventBase @@ -324,8 +324,12 @@ class EventClientSerializer(object): """ def __init__(self, hs): - pass + self.store = hs.get_datastore() + self.experimental_msc1849_support_enabled = ( + hs.config.experimental_msc1849_support_enabled + ) + @defer.inlineCallbacks def serialize_event(self, event, time_now, **kwargs): """Serializes a single event. @@ -337,8 +341,32 @@ def serialize_event(self, event, time_now, **kwargs): Returns: Deferred[dict]: The serialized event """ + # To handle the case of presence events and the like + if not isinstance(event, EventBase): + defer.returnValue(event) + + event_id = event.event_id event = serialize_event(event, time_now, **kwargs) - return defer.succeed(event) + + # If MSC1849 is enabled then we need to look if thre are any relations + # we need to bundle in with the event + if self.experimental_msc1849_support_enabled: + annotations = yield self.store.get_aggregation_groups_for_event( + event_id, + ) + references = yield self.store.get_relations_for_event( + event_id, RelationTypes.REFERENCES, direction="f", + ) + + if annotations.chunk: + r = event["unsigned"].setdefault("m.relations", {}) + r[RelationTypes.ANNOTATION] = annotations.to_dict() + + if references.chunk: + r = event["unsigned"].setdefault("m.relations", {}) + r[RelationTypes.REFERENCES] = references.to_dict() + + defer.returnValue(event) def serialize_events(self, events, time_now, **kwargs): """Serializes multiple events. diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py index c3ac73b8c79e..f468409b6823 100644 --- a/synapse/rest/client/v2_alpha/relations.py +++ b/synapse/rest/client/v2_alpha/relations.py @@ -23,7 +23,7 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, RelationTypes from synapse.api.errors import SynapseError from synapse.http.servlet import ( RestServlet, @@ -141,12 +141,16 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non ) limit = parse_integer(request, "limit", default=5) + from_token = parse_string(request, "from") + to_token = parse_string(request, "to") result = yield self.store.get_relations_for_event( event_id=parent_id, relation_type=relation_type, event_type=event_type, limit=limit, + from_token=from_token, + to_token=to_token, ) events = yield self.store.get_events_as_list( @@ -162,6 +166,141 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non defer.returnValue((200, return_value)) +class RelationAggregationPaginationServlet(RestServlet): + """API to paginate aggregation groups of relations, e.g. paginate the + types and counts of the reactions on the events. + + Example request and response: + + GET /rooms/{room_id}/aggregations/{parent_id} + + { + chunk: [ + { + "type": "m.reaction", + "key": "👍", + "count": 3 + } + ] + } + """ + + PATTERNS = client_v2_patterns( + "/rooms/(?P[^/]*)/aggregations/(?P[^/]*)" + "(/(?P[^/]*)(/(?P[^/]*))?)?$", + releases=(), + ) + + def __init__(self, hs): + super(RelationAggregationPaginationServlet, self).__init__() + self.auth = hs.get_auth() + self.store = hs.get_datastore() + + @defer.inlineCallbacks + def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=None): + requester = yield self.auth.get_user_by_req(request, allow_guest=True) + + yield self.auth.check_in_room_or_world_readable( + room_id, requester.user.to_string() + ) + + if relation_type not in (RelationTypes.ANNOTATION, None): + raise SynapseError(400, "Relation type must be 'annotation'") + + limit = parse_integer(request, "limit", default=5) + from_token = parse_string(request, "from") + to_token = parse_string(request, "to") + + res = yield self.store.get_aggregation_groups_for_event( + event_id=parent_id, + event_type=event_type, + limit=limit, + from_token=from_token, + to_token=to_token, + ) + + defer.returnValue((200, res.to_dict())) + + +class RelationAggregationGroupPaginationServlet(RestServlet): + """API to paginate within an aggregation group of relations, e.g. paginate + all the 👍 reactions on an event. + + Example request and response: + + GET /rooms/{room_id}/aggregations/{parent_id}/m.annotation/m.reaction/👍 + + { + chunk: [ + { + "type": "m.reaction", + "content": { + "m.relates_to": { + "rel_type": "m.annotation", + "key": "👍" + } + } + }, + ... + ] + } + """ + + PATTERNS = client_v2_patterns( + "/rooms/(?P[^/]*)/aggregations/(?P[^/]*)" + "/(?P[^/]*)/(?P[^/]*)/(?P[^/]*)$", + releases=(), + ) + + def __init__(self, hs): + super(RelationAggregationGroupPaginationServlet, self).__init__() + self.auth = hs.get_auth() + self.store = hs.get_datastore() + self.clock = hs.get_clock() + self._event_serializer = hs.get_event_client_serializer() + + @defer.inlineCallbacks + def on_GET(self, request, room_id, parent_id, relation_type, event_type, key): + requester = yield self.auth.get_user_by_req(request, allow_guest=True) + + yield self.auth.check_in_room_or_world_readable( + room_id, requester.user.to_string() + ) + + if relation_type != RelationTypes.ANNOTATION: + raise SynapseError(400, "Relation type must be 'annotation'") + + limit = parse_integer(request, "limit", default=5) + from_token = parse_string(request, "from") + to_token = parse_string(request, "to") + + result = yield self.store.get_relations_for_event( + event_id=parent_id, + relation_type=relation_type, + event_type=event_type, + aggregation_key=key, + limit=limit, + from_token=from_token, + to_token=to_token, + ) + + events = yield self.store.get_events_as_list( + [c["event_id"] for c in result.chunk] + ) + + now = self.clock.time_msec() + events = yield self._event_serializer.serialize_events(events, now) + + return_value = result.to_dict() + return_value["chunk"] = events + + defer.returnValue((200, return_value)) + + defer.returnValue((200, return_value)) + + def register_servlets(hs, http_server): RelationSendServlet(hs).register(http_server) RelationPaginationServlet(hs).register(http_server) + RelationAggregationPaginationServlet(hs).register(http_server) + RelationAggregationGroupPaginationServlet(hs).register(http_server) diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index 31ef6679af6d..db4b842c97ec 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -18,7 +18,9 @@ import attr from synapse.api.constants import RelationTypes +from synapse.api.errors import SynapseError from synapse.storage._base import SQLBaseStore +from synapse.storage.stream import generate_pagination_where_clause logger = logging.getLogger(__name__) @@ -29,19 +31,94 @@ class PaginationChunk(object): Attributes: chunk (list): The rows returned by pagination + next_batch (Any|None): Token to fetch next set of results with, if + None then there are no more results. + prev_batch (Any|None): Token to fetch previous set of results with, if + None then there are no previous results. """ chunk = attr.ib() + next_batch = attr.ib(default=None) + prev_batch = attr.ib(default=None) def to_dict(self): d = {"chunk": self.chunk} + if self.next_batch: + d["next_batch"] = self.next_batch.to_string() + + if self.prev_batch: + d["prev_batch"] = self.prev_batch.to_string() + return d +@attr.s +class RelationPaginationToken(object): + """Pagination token for relation pagination API. + + As the results are order by topological ordering, we can use the + `topological_ordering` and `stream_ordering` fields of the events at the + boundaries of the chunk as pagination tokens. + + Attributes: + topological (int): The topological ordering of the boundary event + stream (int): The stream ordering of the boundary event. + """ + + topological = attr.ib() + stream = attr.ib() + + @staticmethod + def from_string(string): + try: + t, s = string.split("-") + return RelationPaginationToken(int(t), int(s)) + except ValueError: + raise SynapseError(400, "Invalid token") + + def to_string(self): + return "%d-%d" % (self.topological, self.stream) + + +@attr.s +class AggregationPaginationToken(object): + """Pagination token for relation aggregation pagination API. + + As the results are order by count and then MAX(stream_ordering) of the + aggregation groups, we can just use them as our pagination token. + + Attributes: + count (int): The count of relations in the boundar group. + stream (int): The MAX stream ordering in the boundary group. + """ + + count = attr.ib() + stream = attr.ib() + + @staticmethod + def from_string(string): + try: + c, s = string.split("-") + return AggregationPaginationToken(int(c), int(s)) + except ValueError: + raise SynapseError(400, "Invalid token") + + def to_string(self): + return "%d-%d" % (self.count, self.stream) + + class RelationsStore(SQLBaseStore): def get_relations_for_event( - self, event_id, relation_type=None, event_type=None, limit=5, direction="b" + self, + event_id, + relation_type=None, + event_type=None, + aggregation_key=None, + limit=5, + direction="b", + from_token=None, + to_token=None, ): """Get a list of relations for an event, ordered by topological ordering. @@ -51,16 +128,26 @@ def get_relations_for_event( type, if given. event_type (str|None): Only fetch events with this event type, if given. + aggregation_key (str|None): Only fetch events with this aggregation + key, if given. limit (int): Only fetch the most recent `limit` events. direction (str): Whether to fetch the most recent first (`"b"`) or the oldest first (`"f"`). + from_token (RelationPaginationToken|None): Fetch rows from the given + token, or from the start if None. + to_token (RelationPaginationToken|None): Fetch rows up to the given + token, or up to the end if None. Returns: Deferred[PaginationChunk]: List of event IDs that match relations requested. The rows are of the form `{"event_id": "..."}`. """ - # TODO: Pagination tokens + if from_token: + from_token = RelationPaginationToken.from_string(from_token) + + if to_token: + to_token = RelationPaginationToken.from_string(to_token) where_clause = ["relates_to_id = ?"] where_args = [event_id] @@ -73,12 +160,29 @@ def get_relations_for_event( where_clause.append("type = ?") where_args.append(event_type) - order = "ASC" + if aggregation_key: + where_clause.append("aggregation_key = ?") + where_args.append(aggregation_key) + + pagination_clause = generate_pagination_where_clause( + direction=direction, + column_names=("topological_ordering", "stream_ordering"), + from_token=attr.astuple(from_token) if from_token else None, + to_token=attr.astuple(to_token) if to_token else None, + engine=self.database_engine, + ) + + if pagination_clause: + where_clause.append(pagination_clause) + if direction == "b": order = "DESC" + else: + order = "ASC" sql = """ - SELECT event_id FROM event_relations + SELECT event_id, topological_ordering, stream_ordering + FROM event_relations INNER JOIN events USING (event_id) WHERE %s ORDER BY topological_ordering %s, stream_ordering %s @@ -92,16 +196,125 @@ def get_relations_for_event( def _get_recent_references_for_event_txn(txn): txn.execute(sql, where_args + [limit + 1]) - events = [{"event_id": row[0]} for row in txn] + last_topo_id = None + last_stream_id = None + events = [] + for row in txn: + events.append({"event_id": row[0]}) + last_topo_id = row[1] + last_stream_id = row[2] + + next_batch = None + if len(events) > limit and last_topo_id and last_stream_id: + next_batch = RelationPaginationToken(last_topo_id, last_stream_id) return PaginationChunk( - chunk=list(events[:limit]), + chunk=list(events[:limit]), next_batch=next_batch, prev_batch=from_token ) return self.runInteraction( "get_recent_references_for_event", _get_recent_references_for_event_txn ) + def get_aggregation_groups_for_event( + self, + event_id, + event_type=None, + limit=5, + direction="b", + from_token=None, + to_token=None, + ): + """Get a list of annotations on the event, grouped by event type and + aggregation key, sorted by count. + + This is used e.g. to get the what and how many reactions have happend + on an event. + + Args: + event_id (str): Fetch events that relate to this event ID. + event_type (str|None): Only fetch events with this event type, if + given. + limit (int): Only fetch the `limit` groups. + direction (str): Whether to fetch the highest count first (`"b"`) or + the lowest count first (`"f"`). + from_token (AggregationPaginationToken|None): Fetch rows from the + given token, or from the start if None. + to_token (AggregationPaginationToken|None): Fetch rows up to the + given token, or up to the end if None. + + + Returns: + Deferred[PaginationChunk]: List of groups of annotations that + match. Each row is a dict with `type`, `key` and `count` fields. + """ + + if from_token: + from_token = AggregationPaginationToken.from_string(from_token) + + if to_token: + to_token = AggregationPaginationToken.from_string(to_token) + + where_clause = ["relates_to_id = ?", "relation_type = ?"] + where_args = [event_id, RelationTypes.ANNOTATION] + + if event_type: + where_clause.append("type = ?") + where_args.append(event_type) + + having_clause = generate_pagination_where_clause( + direction=direction, + column_names=("COUNT(*)", "MAX(stream_ordering)"), + from_token=attr.astuple(from_token) if from_token else None, + to_token=attr.astuple(to_token) if to_token else None, + engine=self.database_engine, + ) + + if direction == "b": + order = "DESC" + else: + order = "ASC" + + if having_clause: + having_clause = "HAVING " + having_clause + else: + having_clause = "" + + sql = """ + SELECT type, aggregation_key, COUNT(*), MAX(stream_ordering) + FROM event_relations + INNER JOIN events USING (event_id) + WHERE {where_clause} + GROUP BY relation_type, type, aggregation_key + {having_clause} + ORDER BY COUNT(*) {order}, MAX(stream_ordering) {order} + LIMIT ? + """.format( + where_clause=" AND ".join(where_clause), + order=order, + having_clause=having_clause, + ) + + def _get_aggregation_groups_for_event_txn(txn): + txn.execute(sql, where_args + [limit + 1]) + + next_batch = None + events = [] + for row in txn: + events.append({"type": row[0], "key": row[1], "count": row[2]}) + next_batch = AggregationPaginationToken(row[2], row[3]) + + if len(events) <= limit: + next_batch = None + + return PaginationChunk( + chunk=list(events[:limit]), next_batch=next_batch, prev_batch=from_token + ) + + return self.runInteraction( + "get_aggregation_groups_for_event", _get_aggregation_groups_for_event_txn + ) + def _handle_event_relations(self, txn, event): """Handles inserting relation data during peristence of events diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index bcc1c1bb8579..661dd4575577 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -13,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import itertools + import six from synapse.api.constants import EventTypes, RelationTypes @@ -30,6 +32,12 @@ class RelationsTestCase(unittest.HomeserverTestCase): login.register_servlets, ] + def make_homeserver(self, reactor, clock): + # We need to enable msc1849 support for aggregations + config = self.default_config() + config["experimental_msc1849_support_enabled"] = True + return self.setup_test_homeserver(config=config) + def prepare(self, reactor, clock, hs): self.room = self.helper.create_room_as(self.user_id) res = self.helper.send(self.room, body="Hi!") @@ -40,7 +48,7 @@ def test_send_relation(self): creates the right shape of event. """ - channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key="👍") + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key=u"👍") self.assertEquals(200, channel.code, channel.json_body) event_id = channel.json_body["event_id"] @@ -72,7 +80,7 @@ def test_deny_membership(self): channel = self._send_relation(RelationTypes.ANNOTATION, EventTypes.Member) self.assertEquals(400, channel.code, channel.json_body) - def test_paginate(self): + def test_basic_paginate_relations(self): """Tests that calling pagination API corectly the latest relations. """ channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction") @@ -102,6 +110,243 @@ def test_paginate(self): channel.json_body["chunk"][0], ) + # Make sure next_batch has something in it that looks like it could be a + # valid token. + self.assertIsInstance( + channel.json_body.get("next_batch"), six.string_types, channel.json_body + ) + + def test_repeated_paginate_relations(self): + """Test that if we paginate using a limit and tokens then we get the + expected events. + """ + + expected_event_ids = [] + for _ in range(10): + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction") + self.assertEquals(200, channel.code, channel.json_body) + expected_event_ids.append(channel.json_body["event_id"]) + + prev_token = None + found_event_ids = [] + for _ in range(20): + from_token = "" + if prev_token: + from_token = "&from=" + prev_token + + request, channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/relations/%s?limit=1%s" + % (self.room, self.parent_id, from_token), + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + found_event_ids.extend(e["event_id"] for e in channel.json_body["chunk"]) + next_batch = channel.json_body.get("next_batch") + + self.assertNotEquals(prev_token, next_batch) + prev_token = next_batch + + if not prev_token: + break + + # We paginated backwards, so reverse + found_event_ids.reverse() + self.assertEquals(found_event_ids, expected_event_ids) + + def test_aggregation_pagination_groups(self): + """Test that we can paginate annotation groups correctly. + """ + + sent_groups = {u"👍": 10, u"a": 7, u"b": 5, u"c": 3, u"d": 2, u"e": 1} + for key in itertools.chain.from_iterable( + itertools.repeat(key, num) for key, num in sent_groups.items() + ): + channel = self._send_relation( + RelationTypes.ANNOTATION, "m.reaction", key=key + ) + self.assertEquals(200, channel.code, channel.json_body) + + prev_token = None + found_groups = {} + for _ in range(20): + from_token = "" + if prev_token: + from_token = "&from=" + prev_token + + request, channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/aggregations/%s?limit=1%s" + % (self.room, self.parent_id, from_token), + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.assertEqual(len(channel.json_body["chunk"]), 1, channel.json_body) + + for groups in channel.json_body["chunk"]: + # We only expect reactions + self.assertEqual(groups["type"], "m.reaction", channel.json_body) + + # We should only see each key once + self.assertNotIn(groups["key"], found_groups, channel.json_body) + + found_groups[groups["key"]] = groups["count"] + + next_batch = channel.json_body.get("next_batch") + + self.assertNotEquals(prev_token, next_batch) + prev_token = next_batch + + if not prev_token: + break + + self.assertEquals(sent_groups, found_groups) + + def test_aggregation_pagination_within_group(self): + """Test that we can paginate within an annotation group. + """ + + expected_event_ids = [] + for _ in range(10): + channel = self._send_relation( + RelationTypes.ANNOTATION, "m.reaction", key=u"👍" + ) + self.assertEquals(200, channel.code, channel.json_body) + expected_event_ids.append(channel.json_body["event_id"]) + + # Also send a different type of reaction so that we test we don't see it + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key="a") + self.assertEquals(200, channel.code, channel.json_body) + + prev_token = None + found_event_ids = [] + encoded_key = six.moves.urllib.parse.quote_plus(u"👍".encode("utf-8")) + for _ in range(20): + from_token = "" + if prev_token: + from_token = "&from=" + prev_token + + request, channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s" + "/aggregations/%s/%s/m.reaction/%s?limit=1%s" + % ( + self.room, + self.parent_id, + RelationTypes.ANNOTATION, + encoded_key, + from_token, + ), + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.assertEqual(len(channel.json_body["chunk"]), 1, channel.json_body) + + found_event_ids.extend(e["event_id"] for e in channel.json_body["chunk"]) + + next_batch = channel.json_body.get("next_batch") + + self.assertNotEquals(prev_token, next_batch) + prev_token = next_batch + + if not prev_token: + break + + # We paginated backwards, so reverse + found_event_ids.reverse() + self.assertEquals(found_event_ids, expected_event_ids) + + def test_aggregation(self): + """Test that annotations get correctly aggregated. + """ + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + self.assertEquals(200, channel.code, channel.json_body) + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + self.assertEquals(200, channel.code, channel.json_body) + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "b") + self.assertEquals(200, channel.code, channel.json_body) + + request, channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/aggregations/%s" + % (self.room, self.parent_id), + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.assertEquals( + channel.json_body, + { + "chunk": [ + {"type": "m.reaction", "key": "a", "count": 2}, + {"type": "m.reaction", "key": "b", "count": 1}, + ] + }, + ) + + def test_aggregation_must_be_annotation(self): + """Test that aggregations must be annotations. + """ + + request, channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/aggregations/m.replaces/%s?limit=1" + % (self.room, self.parent_id), + ) + self.render(request) + self.assertEquals(400, channel.code, channel.json_body) + + def test_aggregation_get_event(self): + """Test that annotations and references get correctly bundled when + getting the parent event. + """ + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + self.assertEquals(200, channel.code, channel.json_body) + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + self.assertEquals(200, channel.code, channel.json_body) + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "b") + self.assertEquals(200, channel.code, channel.json_body) + + channel = self._send_relation(RelationTypes.REFERENCES, "m.room.test") + self.assertEquals(200, channel.code, channel.json_body) + reply_1 = channel.json_body["event_id"] + + channel = self._send_relation(RelationTypes.REFERENCES, "m.room.test") + self.assertEquals(200, channel.code, channel.json_body) + reply_2 = channel.json_body["event_id"] + + request, channel = self.make_request( + "GET", "/rooms/%s/event/%s" % (self.room, self.parent_id) + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.maxDiff = None + + self.assertEquals( + channel.json_body["unsigned"].get("m.relations"), + { + RelationTypes.ANNOTATION: { + "chunk": [ + {"type": "m.reaction", "key": "a", "count": 2}, + {"type": "m.reaction", "key": "b", "count": 1}, + ] + }, + RelationTypes.REFERENCES: { + "chunk": [{"event_id": reply_1}, {"event_id": reply_2}] + }, + }, + ) + def _send_relation(self, relation_type, event_type, key=None): """Helper function to send a relation pointing at `self.parent_id` @@ -116,7 +361,7 @@ def _send_relation(self, relation_type, event_type, key=None): """ query = "" if key: - query = "?key=" + six.moves.urllib.parse.quote_plus(key) + query = "?key=" + six.moves.urllib.parse.quote_plus(key.encode("utf-8")) request, channel = self.make_request( "POST", From 33453419b0cbbe29d3f4b376e9eafab10d5d012a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 16 May 2019 09:56:12 +0100 Subject: [PATCH 20/49] Add cache to relations --- synapse/storage/relations.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index db4b842c97ec..1fd3d4fafc2c 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -21,6 +21,7 @@ from synapse.api.errors import SynapseError from synapse.storage._base import SQLBaseStore from synapse.storage.stream import generate_pagination_where_clause +from synapse.util.caches.descriptors import cached logger = logging.getLogger(__name__) @@ -109,6 +110,7 @@ def to_string(self): class RelationsStore(SQLBaseStore): + @cached(tree=True) def get_relations_for_event( self, event_id, @@ -216,6 +218,7 @@ def _get_recent_references_for_event_txn(txn): "get_recent_references_for_event", _get_recent_references_for_event_txn ) + @cached(tree=True) def get_aggregation_groups_for_event( self, event_id, @@ -353,3 +356,8 @@ def _handle_event_relations(self, txn, event): "aggregation_key": aggregation_key, }, ) + + txn.call_after(self.get_relations_for_event.invalidate_many, (parent_id,)) + txn.call_after( + self.get_aggregation_groups_for_event.invalidate_many, (parent_id,) + ) From b5c62c6b2643a138956af0b04521540c270a3e94 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 16 May 2019 10:18:53 +0100 Subject: [PATCH 21/49] Fix relations in worker mode --- synapse/replication/slave/storage/events.py | 13 ++++++++++--- synapse/replication/tcp/streams/_base.py | 1 + synapse/replication/tcp/streams/events.py | 11 ++++++----- synapse/storage/events.py | 12 ++++++++---- synapse/storage/relations.py | 4 +++- 5 files changed, 28 insertions(+), 13 deletions(-) diff --git a/synapse/replication/slave/storage/events.py b/synapse/replication/slave/storage/events.py index b457c5563fa0..797450bc6610 100644 --- a/synapse/replication/slave/storage/events.py +++ b/synapse/replication/slave/storage/events.py @@ -23,6 +23,7 @@ from synapse.storage.event_federation import EventFederationWorkerStore from synapse.storage.event_push_actions import EventPushActionsWorkerStore from synapse.storage.events_worker import EventsWorkerStore +from synapse.storage.relations import RelationsWorkerStore from synapse.storage.roommember import RoomMemberWorkerStore from synapse.storage.signatures import SignatureWorkerStore from synapse.storage.state import StateGroupWorkerStore @@ -52,6 +53,7 @@ class SlavedEventStore(EventFederationWorkerStore, EventsWorkerStore, SignatureWorkerStore, UserErasureWorkerStore, + RelationsWorkerStore, BaseSlavedStore): def __init__(self, db_conn, hs): @@ -89,7 +91,7 @@ def process_replication_rows(self, stream_name, token, rows): for row in rows: self.invalidate_caches_for_event( -token, row.event_id, row.room_id, row.type, row.state_key, - row.redacts, + row.redacts, row.relates_to, backfilled=True, ) return super(SlavedEventStore, self).process_replication_rows( @@ -102,7 +104,7 @@ def _process_event_stream_row(self, token, row): if row.type == EventsStreamEventRow.TypeId: self.invalidate_caches_for_event( token, data.event_id, data.room_id, data.type, data.state_key, - data.redacts, + data.redacts, data.relates_to, backfilled=False, ) elif row.type == EventsStreamCurrentStateRow.TypeId: @@ -114,7 +116,8 @@ def _process_event_stream_row(self, token, row): raise Exception("Unknown events stream row type %s" % (row.type, )) def invalidate_caches_for_event(self, stream_ordering, event_id, room_id, - etype, state_key, redacts, backfilled): + etype, state_key, redacts, relates_to, + backfilled): self._invalidate_get_event_cache(event_id) self.get_latest_event_ids_in_room.invalidate((room_id,)) @@ -136,3 +139,7 @@ def invalidate_caches_for_event(self, stream_ordering, event_id, room_id, state_key, stream_ordering ) self.get_invited_rooms_for_user.invalidate((state_key,)) + + if relates_to: + self.get_relations_for_event.invalidate_many((relates_to,)) + self.get_aggregation_groups_for_event.invalidate_many((relates_to,)) diff --git a/synapse/replication/tcp/streams/_base.py b/synapse/replication/tcp/streams/_base.py index 8971a6a22e83..b6ce7a7beee7 100644 --- a/synapse/replication/tcp/streams/_base.py +++ b/synapse/replication/tcp/streams/_base.py @@ -32,6 +32,7 @@ "type", # str "state_key", # str, optional "redacts", # str, optional + "relates_to", # str, optional )) PresenceStreamRow = namedtuple("PresenceStreamRow", ( "user_id", # str diff --git a/synapse/replication/tcp/streams/events.py b/synapse/replication/tcp/streams/events.py index e0f6e2924815..f1290d022a21 100644 --- a/synapse/replication/tcp/streams/events.py +++ b/synapse/replication/tcp/streams/events.py @@ -80,11 +80,12 @@ def from_data(cls, data): class EventsStreamEventRow(BaseEventsStreamRow): TypeId = "ev" - event_id = attr.ib() # str - room_id = attr.ib() # str - type = attr.ib() # str - state_key = attr.ib() # str, optional - redacts = attr.ib() # str, optional + event_id = attr.ib() # str + room_id = attr.ib() # str + type = attr.ib() # str + state_key = attr.ib() # str, optional + redacts = attr.ib() # str, optional + relates_to = attr.ib() # str, optional @attr.s(slots=True, frozen=True) diff --git a/synapse/storage/events.py b/synapse/storage/events.py index 6802bf42ce4e..b025ebc926b3 100644 --- a/synapse/storage/events.py +++ b/synapse/storage/events.py @@ -1657,10 +1657,11 @@ def get_all_new_forward_event_rows(self, last_id, current_id, limit): def get_all_new_forward_event_rows(txn): sql = ( "SELECT e.stream_ordering, e.event_id, e.room_id, e.type," - " state_key, redacts" + " state_key, redacts, relates_to_id" " FROM events AS e" " LEFT JOIN redactions USING (event_id)" " LEFT JOIN state_events USING (event_id)" + " LEFT JOIN event_relations USING (event_id)" " WHERE ? < stream_ordering AND stream_ordering <= ?" " ORDER BY stream_ordering ASC" " LIMIT ?" @@ -1675,11 +1676,12 @@ def get_all_new_forward_event_rows(txn): sql = ( "SELECT event_stream_ordering, e.event_id, e.room_id, e.type," - " state_key, redacts" + " state_key, redacts, relates_to_id" " FROM events AS e" " INNER JOIN ex_outlier_stream USING (event_id)" " LEFT JOIN redactions USING (event_id)" " LEFT JOIN state_events USING (event_id)" + " LEFT JOIN event_relations USING (event_id)" " WHERE ? < event_stream_ordering" " AND event_stream_ordering <= ?" " ORDER BY event_stream_ordering DESC" @@ -1700,10 +1702,11 @@ def get_all_new_backfill_event_rows(self, last_id, current_id, limit): def get_all_new_backfill_event_rows(txn): sql = ( "SELECT -e.stream_ordering, e.event_id, e.room_id, e.type," - " state_key, redacts" + " state_key, redacts, relates_to_id" " FROM events AS e" " LEFT JOIN redactions USING (event_id)" " LEFT JOIN state_events USING (event_id)" + " LEFT JOIN event_relations USING (event_id)" " WHERE ? > stream_ordering AND stream_ordering >= ?" " ORDER BY stream_ordering ASC" " LIMIT ?" @@ -1718,11 +1721,12 @@ def get_all_new_backfill_event_rows(txn): sql = ( "SELECT -event_stream_ordering, e.event_id, e.room_id, e.type," - " state_key, redacts" + " state_key, redacts, relates_to_id" " FROM events AS e" " INNER JOIN ex_outlier_stream USING (event_id)" " LEFT JOIN redactions USING (event_id)" " LEFT JOIN state_events USING (event_id)" + " LEFT JOIN event_relations USING (event_id)" " WHERE ? > event_stream_ordering" " AND event_stream_ordering >= ?" " ORDER BY event_stream_ordering DESC" diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index 1fd3d4fafc2c..732418ec6580 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -109,7 +109,7 @@ def to_string(self): return "%d-%d" % (self.count, self.stream) -class RelationsStore(SQLBaseStore): +class RelationsWorkerStore(SQLBaseStore): @cached(tree=True) def get_relations_for_event( self, @@ -318,6 +318,8 @@ def _get_aggregation_groups_for_event_txn(txn): "get_aggregation_groups_for_event", _get_aggregation_groups_for_event_txn ) + +class RelationsStore(RelationsWorkerStore): def _handle_event_relations(self, txn, event): """Handles inserting relation data during peristence of events From 4a6d5de98c7b697d2368521a0bb22b9d721dbb38 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 16 May 2019 13:23:43 +0100 Subject: [PATCH 22/49] Make /sync attempt to return device updates for both joined and invited users (#3484) --- changelog.d/3484.misc | 1 + synapse/handlers/sync.py | 44 +++++++++++++++++++++++----------------- 2 files changed, 26 insertions(+), 19 deletions(-) create mode 100644 changelog.d/3484.misc diff --git a/changelog.d/3484.misc b/changelog.d/3484.misc new file mode 100644 index 000000000000..36458498440d --- /dev/null +++ b/changelog.d/3484.misc @@ -0,0 +1 @@ +Make /sync attempt to return device updates for both joined and invited users. Note that this doesn't currently work correctly due to other bugs. diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 153312e39fff..1ee9a6e313f0 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -934,7 +934,7 @@ def generate_sync_result(self, sync_config, since_token=None, full_state=False): res = yield self._generate_sync_entry_for_rooms( sync_result_builder, account_data_by_room ) - newly_joined_rooms, newly_joined_users, _, _ = res + newly_joined_rooms, newly_joined_or_invited_users, _, _ = res _, _, newly_left_rooms, newly_left_users = res block_all_presence_data = ( @@ -943,7 +943,7 @@ def generate_sync_result(self, sync_config, since_token=None, full_state=False): ) if self.hs_config.use_presence and not block_all_presence_data: yield self._generate_sync_entry_for_presence( - sync_result_builder, newly_joined_rooms, newly_joined_users + sync_result_builder, newly_joined_rooms, newly_joined_or_invited_users ) yield self._generate_sync_entry_for_to_device(sync_result_builder) @@ -951,7 +951,7 @@ def generate_sync_result(self, sync_config, since_token=None, full_state=False): device_lists = yield self._generate_sync_entry_for_device_list( sync_result_builder, newly_joined_rooms=newly_joined_rooms, - newly_joined_users=newly_joined_users, + newly_joined_or_invited_users=newly_joined_or_invited_users, newly_left_rooms=newly_left_rooms, newly_left_users=newly_left_users, ) @@ -1036,7 +1036,8 @@ def _generate_sync_entry_for_groups(self, sync_result_builder): @measure_func("_generate_sync_entry_for_device_list") @defer.inlineCallbacks def _generate_sync_entry_for_device_list(self, sync_result_builder, - newly_joined_rooms, newly_joined_users, + newly_joined_rooms, + newly_joined_or_invited_users, newly_left_rooms, newly_left_users): user_id = sync_result_builder.sync_config.user.to_string() since_token = sync_result_builder.since_token @@ -1050,7 +1051,7 @@ def _generate_sync_entry_for_device_list(self, sync_result_builder, # share a room with? for room_id in newly_joined_rooms: joined_users = yield self.state.get_current_users_in_room(room_id) - newly_joined_users.update(joined_users) + newly_joined_or_invited_users.update(joined_users) for room_id in newly_left_rooms: left_users = yield self.state.get_current_users_in_room(room_id) @@ -1058,7 +1059,7 @@ def _generate_sync_entry_for_device_list(self, sync_result_builder, # TODO: Check that these users are actually new, i.e. either they # weren't in the previous sync *or* they left and rejoined. - changed.update(newly_joined_users) + changed.update(newly_joined_or_invited_users) if not changed and not newly_left_users: defer.returnValue(DeviceLists( @@ -1176,7 +1177,7 @@ def _generate_sync_entry_for_account_data(self, sync_result_builder): @defer.inlineCallbacks def _generate_sync_entry_for_presence(self, sync_result_builder, newly_joined_rooms, - newly_joined_users): + newly_joined_or_invited_users): """Generates the presence portion of the sync response. Populates the `sync_result_builder` with the result. @@ -1184,8 +1185,9 @@ def _generate_sync_entry_for_presence(self, sync_result_builder, newly_joined_ro sync_result_builder(SyncResultBuilder) newly_joined_rooms(list): List of rooms that the user has joined since the last sync (or empty if an initial sync) - newly_joined_users(list): List of users that have joined rooms - since the last sync (or empty if an initial sync) + newly_joined_or_invited_users(list): List of users that have joined + or been invited to rooms since the last sync (or empty if an initial + sync) """ now_token = sync_result_builder.now_token sync_config = sync_result_builder.sync_config @@ -1211,7 +1213,7 @@ def _generate_sync_entry_for_presence(self, sync_result_builder, newly_joined_ro "presence_key", presence_key ) - extra_users_ids = set(newly_joined_users) + extra_users_ids = set(newly_joined_or_invited_users) for room_id in newly_joined_rooms: users = yield self.state.get_current_users_in_room(room_id) extra_users_ids.update(users) @@ -1243,7 +1245,8 @@ def _generate_sync_entry_for_rooms(self, sync_result_builder, account_data_by_ro Returns: Deferred(tuple): Returns a 4-tuple of - `(newly_joined_rooms, newly_joined_users, newly_left_rooms, newly_left_users)` + `(newly_joined_rooms, newly_joined_or_invited_users, + newly_left_rooms, newly_left_users)` """ user_id = sync_result_builder.sync_config.user.to_string() block_all_room_ephemeral = ( @@ -1314,8 +1317,8 @@ def handle_room_entries(room_entry): sync_result_builder.invited.extend(invited) - # Now we want to get any newly joined users - newly_joined_users = set() + # Now we want to get any newly joined or invited users + newly_joined_or_invited_users = set() newly_left_users = set() if since_token: for joined_sync in sync_result_builder.joined: @@ -1324,19 +1327,22 @@ def handle_room_entries(room_entry): ) for event in it: if event.type == EventTypes.Member: - if event.membership == Membership.JOIN: - newly_joined_users.add(event.state_key) + if ( + event.membership == Membership.JOIN or + event.membership == Membership.INVITE + ): + newly_joined_or_invited_users.add(event.state_key) else: prev_content = event.unsigned.get("prev_content", {}) prev_membership = prev_content.get("membership", None) if prev_membership == Membership.JOIN: newly_left_users.add(event.state_key) - newly_left_users -= newly_joined_users + newly_left_users -= newly_joined_or_invited_users defer.returnValue(( newly_joined_rooms, - newly_joined_users, + newly_joined_or_invited_users, newly_left_rooms, newly_left_users, )) @@ -1381,7 +1387,7 @@ def _get_rooms_changed(self, sync_result_builder, ignored_users): where: room_entries is a list [RoomSyncResultBuilder] invited_rooms is a list [InvitedSyncResult] - newly_joined rooms is a list[str] of room ids + newly_joined_rooms is a list[str] of room ids newly_left_rooms is a list[str] of room ids """ user_id = sync_result_builder.sync_config.user.to_string() @@ -1422,7 +1428,7 @@ def _get_rooms_changed(self, sync_result_builder, ignored_users): if room_id in sync_result_builder.joined_room_ids and non_joins: # Always include if the user (re)joined the room, especially # important so that device list changes are calculated correctly. - # If there are non join member events, but we are still in the room, + # If there are non-join member events, but we are still in the room, # then the user must have left and joined newly_joined_rooms.append(room_id) From 95f3fcda3c398bc82825694bde143a98940194c6 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 16 May 2019 14:19:06 +0100 Subject: [PATCH 23/49] Check that event is visible in new APIs --- synapse/rest/client/v2_alpha/relations.py | 17 +++++++++++++++-- tests/rest/client/v2_alpha/test_relations.py | 2 +- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py index f468409b6823..1b53e638ebbf 100644 --- a/synapse/rest/client/v2_alpha/relations.py +++ b/synapse/rest/client/v2_alpha/relations.py @@ -131,6 +131,7 @@ def __init__(self, hs): self.store = hs.get_datastore() self.clock = hs.get_clock() self._event_serializer = hs.get_event_client_serializer() + self.event_handler = hs.get_event_handler() @defer.inlineCallbacks def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=None): @@ -140,6 +141,10 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non room_id, requester.user.to_string() ) + # This checks that a) the event exists and b) the user is allowed to + # view it. + yield self.event_handler.get_event(requester.user, room_id, parent_id) + limit = parse_integer(request, "limit", default=5) from_token = parse_string(request, "from") to_token = parse_string(request, "to") @@ -195,6 +200,7 @@ def __init__(self, hs): super(RelationAggregationPaginationServlet, self).__init__() self.auth = hs.get_auth() self.store = hs.get_datastore() + self.event_handler = hs.get_event_handler() @defer.inlineCallbacks def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=None): @@ -204,6 +210,10 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non room_id, requester.user.to_string() ) + # This checks that a) the event exists and b) the user is allowed to + # view it. + yield self.event_handler.get_event(requester.user, room_id, parent_id) + if relation_type not in (RelationTypes.ANNOTATION, None): raise SynapseError(400, "Relation type must be 'annotation'") @@ -258,6 +268,7 @@ def __init__(self, hs): self.store = hs.get_datastore() self.clock = hs.get_clock() self._event_serializer = hs.get_event_client_serializer() + self.event_handler = hs.get_event_handler() @defer.inlineCallbacks def on_GET(self, request, room_id, parent_id, relation_type, event_type, key): @@ -267,6 +278,10 @@ def on_GET(self, request, room_id, parent_id, relation_type, event_type, key): room_id, requester.user.to_string() ) + # This checks that a) the event exists and b) the user is allowed to + # view it. + yield self.event_handler.get_event(requester.user, room_id, parent_id) + if relation_type != RelationTypes.ANNOTATION: raise SynapseError(400, "Relation type must be 'annotation'") @@ -296,8 +311,6 @@ def on_GET(self, request, room_id, parent_id, relation_type, event_type, key): defer.returnValue((200, return_value)) - defer.returnValue((200, return_value)) - def register_servlets(hs, http_server): RelationSendServlet(hs).register(http_server) diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index 661dd4575577..775622bd2b9c 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -296,7 +296,7 @@ def test_aggregation_must_be_annotation(self): request, channel = self.make_request( "GET", - "/_matrix/client/unstable/rooms/%s/aggregations/m.replaces/%s?limit=1" + "/_matrix/client/unstable/rooms/%s/aggregations/%s/m.replace?limit=1" % (self.room, self.parent_id), ) self.render(request) From 2c662ddde4e1277a0dc17295748aa5f0c41fa163 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 16 May 2019 14:21:39 +0100 Subject: [PATCH 24/49] Indirect tuple conversion --- synapse/storage/relations.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index 732418ec6580..996cb6903aa8 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -81,6 +81,9 @@ def from_string(string): def to_string(self): return "%d-%d" % (self.topological, self.stream) + def as_tuple(self): + return attr.astuple(self) + @attr.s class AggregationPaginationToken(object): @@ -108,6 +111,9 @@ def from_string(string): def to_string(self): return "%d-%d" % (self.count, self.stream) + def as_tuple(self): + return attr.astuple(self) + class RelationsWorkerStore(SQLBaseStore): @cached(tree=True) From 7a7eba8302d6566c044ca82c8ac0da65aa85e36b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 16 May 2019 14:24:58 +0100 Subject: [PATCH 25/49] Move parsing of tokens out of storage layer --- synapse/rest/client/v2_alpha/relations.py | 19 +++++++++++++++++++ synapse/storage/relations.py | 16 ++-------------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py index 1b53e638ebbf..41e0a4493667 100644 --- a/synapse/rest/client/v2_alpha/relations.py +++ b/synapse/rest/client/v2_alpha/relations.py @@ -32,6 +32,7 @@ parse_string, ) from synapse.rest.client.transactions import HttpTransactionCache +from synapse.storage.relations import AggregationPaginationToken, RelationPaginationToken from ._base import client_v2_patterns @@ -149,6 +150,12 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non from_token = parse_string(request, "from") to_token = parse_string(request, "to") + if from_token: + from_token = RelationPaginationToken.from_string(from_token) + + if to_token: + to_token = RelationPaginationToken.from_string(to_token) + result = yield self.store.get_relations_for_event( event_id=parent_id, relation_type=relation_type, @@ -221,6 +228,12 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non from_token = parse_string(request, "from") to_token = parse_string(request, "to") + if from_token: + from_token = AggregationPaginationToken.from_string(from_token) + + if to_token: + to_token = AggregationPaginationToken.from_string(to_token) + res = yield self.store.get_aggregation_groups_for_event( event_id=parent_id, event_type=event_type, @@ -289,6 +302,12 @@ def on_GET(self, request, room_id, parent_id, relation_type, event_type, key): from_token = parse_string(request, "from") to_token = parse_string(request, "to") + if from_token: + from_token = RelationPaginationToken.from_string(from_token) + + if to_token: + to_token = RelationPaginationToken.from_string(to_token) + result = yield self.store.get_relations_for_event( event_id=parent_id, relation_type=relation_type, diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index 996cb6903aa8..de67e305a14a 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -54,7 +54,7 @@ def to_dict(self): return d -@attr.s +@attr.s(frozen=True, slots=True) class RelationPaginationToken(object): """Pagination token for relation pagination API. @@ -85,7 +85,7 @@ def as_tuple(self): return attr.astuple(self) -@attr.s +@attr.s(frozen=True, slots=True) class AggregationPaginationToken(object): """Pagination token for relation aggregation pagination API. @@ -151,12 +151,6 @@ def get_relations_for_event( requested. The rows are of the form `{"event_id": "..."}`. """ - if from_token: - from_token = RelationPaginationToken.from_string(from_token) - - if to_token: - to_token = RelationPaginationToken.from_string(to_token) - where_clause = ["relates_to_id = ?"] where_args = [event_id] @@ -258,12 +252,6 @@ def get_aggregation_groups_for_event( match. Each row is a dict with `type`, `key` and `count` fields. """ - if from_token: - from_token = AggregationPaginationToken.from_string(from_token) - - if to_token: - to_token = AggregationPaginationToken.from_string(to_token) - where_clause = ["relates_to_id = ?", "relation_type = ?"] where_args = [event_id, RelationTypes.ANNOTATION] From cd32375846397ed15f27a4f6602bf20999d2b8b3 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 16 May 2019 14:26:41 +0100 Subject: [PATCH 26/49] Add option to disable per-room profiles --- synapse/config/server.py | 11 +++++++++++ synapse/handlers/room_member.py | 9 +++++++++ 2 files changed, 20 insertions(+) diff --git a/synapse/config/server.py b/synapse/config/server.py index 7874cd9da71c..1b8968608eea 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2014-2016 OpenMarket Ltd # Copyright 2017-2018 New Vector Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -173,6 +174,10 @@ def read_config(self, config): "require_membership_for_aliases", True, ) + # Whether to allow per-room membership profiles through the send of membership + # events with profile information that differ from the target's global profile. + self.allow_per_room_profiles = config.get("allow_per_room_profiles", True) + self.listeners = [] for listener in config.get("listeners", []): if not isinstance(listener.get("port", None), int): @@ -566,6 +571,12 @@ def default_config(self, server_name, data_dir_path, **kwargs): # Defaults to 'true'. # #require_membership_for_aliases: false + + # Whether to allow per-room membership profiles through the send of membership + # events with profile information that differ from the target's global profile. + # Defaults to 'true'. + # + #allow_per_room_profiles: false """ % locals() def read_arguments(self, args): diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 3e86b9c690ff..ffc588d4546c 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2016 OpenMarket Ltd # Copyright 2018 New Vector Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -73,6 +74,7 @@ def __init__(self, hs): self.spam_checker = hs.get_spam_checker() self._server_notices_mxid = self.config.server_notices_mxid self._enable_lookup = hs.config.enable_3pid_lookup + self.allow_per_room_profiles = self.config.allow_per_room_profiles # This is only used to get at ratelimit function, and # maybe_kick_guest_users. It's fine there are multiple of these as @@ -357,6 +359,13 @@ def _update_membership( # later on. content = dict(content) + if not self.allow_per_room_profiles: + # Strip profile data, knowing that new profile data will be added to the + # event's content in event_creation_handler.create_event() using the target's + # global profile. + content.pop("displayname", None) + content.pop("avatar_url", None) + effective_membership_state = action if action in ["kick", "unban"]: effective_membership_state = "leave" From 54a582ed44372a8ecd0d20bf474e5ac81e140220 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 16 May 2019 15:09:16 +0100 Subject: [PATCH 27/49] Add test case --- tests/rest/client/v1/test_rooms.py | 68 +++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index 6220172cde0f..7639648752fd 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -24,7 +24,7 @@ import synapse.rest.admin from synapse.api.constants import Membership -from synapse.rest.client.v1 import login, room +from synapse.rest.client.v1 import login, profile, room from tests import unittest @@ -936,3 +936,69 @@ def test_restricted_auth(self): request, channel = self.make_request("GET", self.url, access_token=tok) self.render(request) self.assertEqual(channel.code, 200, channel.result) + + +class PerRoomProfilesForbiddenTestCase(unittest.HomeserverTestCase): + + servlets = [ + synapse.rest.admin.register_servlets_for_client_rest_resource, + room.register_servlets, + login.register_servlets, + profile.register_servlets, + ] + + def make_homeserver(self, reactor, clock): + config = self.default_config() + config["allow_per_room_profiles"] = False + self.hs = self.setup_test_homeserver(config=config) + + return self.hs + + def prepare(self, reactor, clock, homeserver): + self.user_id = self.register_user("test", "test") + self.tok = self.login("test", "test") + + # Set a profile for the test user + self.displayname = "test user" + data = { + "displayname": self.displayname, + } + request_data = json.dumps(data) + request, channel = self.make_request( + "PUT", + "/_matrix/client/r0/profile/%s/displayname" % (self.user_id,), + request_data, + access_token=self.tok, + ) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + + self.room_id = self.helper.create_room_as(self.user_id, tok=self.tok) + + def test_per_room_profile_forbidden(self): + data = { + "membership": "join", + "displayname": "other test user" + } + request_data = json.dumps(data) + request, channel = self.make_request( + "PUT", + "/_matrix/client/r0/rooms/%s/state/m.room.member/%s" % (self.room_id, self.user_id), + request_data, + access_token=self.tok, + ) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + event_id = channel.json_body["event_id"] + + request, channel = self.make_request( + "GET", + "/_matrix/client/r0/rooms/%s/event/%s" % (self.room_id, event_id), + access_token=self.tok, + ) + self.render(request) + self.assertEqual(channel.code, 200, channel.result) + + res_displayname = channel.json_body["content"]["displayname"] + self.assertEqual(res_displayname, self.displayname, channel.result) + From efdc55db7500df778ffa3a63c4864ae0241315f0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 16 May 2019 15:10:24 +0100 Subject: [PATCH 28/49] Forgot copyright --- tests/rest/client/v1/test_rooms.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index 7639648752fd..ffd28772b04b 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2014-2016 OpenMarket Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From a5fe16c5a7cbff5106524791f2d7ab621ca13bb1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 16 May 2019 15:11:37 +0100 Subject: [PATCH 29/49] Changelog + sample config --- changelog.d/5196.feature | 1 + docs/sample_config.yaml | 6 ++++++ 2 files changed, 7 insertions(+) create mode 100644 changelog.d/5196.feature diff --git a/changelog.d/5196.feature b/changelog.d/5196.feature new file mode 100644 index 000000000000..1ffb928f62bb --- /dev/null +++ b/changelog.d/5196.feature @@ -0,0 +1 @@ +Add an option to disable per-room profiles. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 09ee0e8984d8..d218aefee58d 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -276,6 +276,12 @@ listeners: # #require_membership_for_aliases: false +# Whether to allow per-room membership profiles through the send of membership +# events with profile information that differ from the target's global profile. +# Defaults to 'true'. +# +#allow_per_room_profiles: false + ## TLS ## From cc8c139a39c6c5e095873372f6c35420d5bd1f99 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 16 May 2019 15:20:59 +0100 Subject: [PATCH 30/49] Lint --- tests/rest/client/v1/test_rooms.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index ffd28772b04b..8df0a07824f3 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -984,7 +984,9 @@ def test_per_room_profile_forbidden(self): request_data = json.dumps(data) request, channel = self.make_request( "PUT", - "/_matrix/client/r0/rooms/%s/state/m.room.member/%s" % (self.room_id, self.user_id), + "/_matrix/client/r0/rooms/%s/state/m.room.member/%s" % ( + self.room_id, self.user_id, + ), request_data, access_token=self.tok, ) From 8f9ce1a8a22a6299b62eb0d6ab73cb95a4e01d20 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 16 May 2019 15:25:54 +0100 Subject: [PATCH 31/49] Lint --- tests/rest/client/v1/test_rooms.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index 8df0a07824f3..5f75ad757952 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -1004,4 +1004,3 @@ def test_per_room_profile_forbidden(self): res_displayname = channel.json_body["content"]["displayname"] self.assertEqual(res_displayname, self.displayname, channel.result) - From 895179a4dcc993af9702e3ab36e5d2157fdd0c26 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 16 May 2019 16:41:05 +0100 Subject: [PATCH 32/49] Update docstring --- synapse/storage/stream.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/synapse/storage/stream.py b/synapse/storage/stream.py index 163363c0c254..824e77c89e86 100644 --- a/synapse/storage/stream.py +++ b/synapse/storage/stream.py @@ -77,6 +77,15 @@ def generate_pagination_where_clause( would be generated for dir=b, from_token=(6, 7) and to_token=(5, 3). + Note that tokens are considered to be after the row they are in, e.g. if + a row A has a token T, then we consider A to be before T. This covention + is important when figuring out inequalities for the generated SQL, and + produces the following result: + - If paginatiting forwards then we exclude any rows matching the from + token, but include those that match the to token. + - If paginatiting backwards then we include any rows matching the from + token, but include those that match the to token. + Args: direction (str): Whether we're paginating backwards("b") or forwards ("f"). @@ -131,7 +140,9 @@ def _make_generic_sql_bound(bound, column_names, values, engine): names (tuple[str, str]): The column names. Must *not* be user defined as these get inserted directly into the SQL statement without escapes. - values (tuple[int, int]): The values to bound the columns by. + values (tuple[int|None, int]): The values to bound the columns by. If + the first value is None then only creates a bound on the second + column. engine: The database engine to generate the SQL for Returns: From d46aab3fa8bc02d8db9616490e849b9443fed8e7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 14 May 2019 16:59:21 +0100 Subject: [PATCH 33/49] Add basic editing support --- synapse/events/utils.py | 30 ++++++- synapse/replication/slave/storage/events.py | 1 + synapse/storage/relations.py | 60 ++++++++++++- tests/rest/client/v2_alpha/test_relations.py | 91 ++++++++++++++++++-- 4 files changed, 167 insertions(+), 15 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 16d0c643723e..2019ce9b1cf0 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -346,7 +346,7 @@ def serialize_event(self, event, time_now, **kwargs): defer.returnValue(event) event_id = event.event_id - event = serialize_event(event, time_now, **kwargs) + serialized_event = serialize_event(event, time_now, **kwargs) # If MSC1849 is enabled then we need to look if thre are any relations # we need to bundle in with the event @@ -359,14 +359,36 @@ def serialize_event(self, event, time_now, **kwargs): ) if annotations.chunk: - r = event["unsigned"].setdefault("m.relations", {}) + r = serialized_event["unsigned"].setdefault("m.relations", {}) r[RelationTypes.ANNOTATION] = annotations.to_dict() if references.chunk: - r = event["unsigned"].setdefault("m.relations", {}) + r = serialized_event["unsigned"].setdefault("m.relations", {}) r[RelationTypes.REFERENCES] = references.to_dict() - defer.returnValue(event) + edit = None + if event.type == EventTypes.Message: + edit = yield self.store.get_applicable_edit( + event.event_id, event.type, event.sender, + ) + + if edit: + # If there is an edit replace the content, preserving existing + # relations. + + relations = event.content.get("m.relates_to") + serialized_event["content"] = edit.content.get("m.new_content", {}) + if relations: + serialized_event["content"]["m.relates_to"] = relations + else: + serialized_event["content"].pop("m.relates_to", None) + + r = serialized_event["unsigned"].setdefault("m.relations", {}) + r[RelationTypes.REPLACES] = { + "event_id": edit.event_id, + } + + defer.returnValue(serialized_event) def serialize_events(self, events, time_now, **kwargs): """Serializes multiple events. diff --git a/synapse/replication/slave/storage/events.py b/synapse/replication/slave/storage/events.py index 797450bc6610..857128b3118b 100644 --- a/synapse/replication/slave/storage/events.py +++ b/synapse/replication/slave/storage/events.py @@ -143,3 +143,4 @@ def invalidate_caches_for_event(self, stream_ordering, event_id, room_id, if relates_to: self.get_relations_for_event.invalidate_many((relates_to,)) self.get_aggregation_groups_for_event.invalidate_many((relates_to,)) + self.get_applicable_edit.invalidate_many((relates_to,)) diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index de67e305a14a..aa91e5273339 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -17,11 +17,13 @@ import attr -from synapse.api.constants import RelationTypes +from twisted.internet import defer + +from synapse.api.constants import EventTypes, RelationTypes from synapse.api.errors import SynapseError from synapse.storage._base import SQLBaseStore from synapse.storage.stream import generate_pagination_where_clause -from synapse.util.caches.descriptors import cached +from synapse.util.caches.descriptors import cached, cachedInlineCallbacks logger = logging.getLogger(__name__) @@ -312,6 +314,59 @@ def _get_aggregation_groups_for_event_txn(txn): "get_aggregation_groups_for_event", _get_aggregation_groups_for_event_txn ) + @cachedInlineCallbacks(tree=True) + def get_applicable_edit(self, event_id, event_type, sender): + """Get the most recent edit (if any) that has happened for the given + event. + + Correctly handles checking whether edits were allowed to happen. + + Args: + event_id (str): The original event ID + event_type (str): The original event type + sender (str): The original event sender + + Returns: + Deferred[EventBase|None]: Returns the most recent edit, if any. + """ + + # We only allow edits for `m.room.message` events that have the same sender + # and event type. We can't assert these things during regular event auth so + # we have to do the post hoc. + + if event_type != EventTypes.Message: + return + + sql = """ + SELECT event_id, origin_server_ts FROM events + INNER JOIN event_relations USING (event_id) + WHERE + relates_to_id = ? + AND relation_type = ? + AND type = ? + AND sender = ? + ORDER by origin_server_ts DESC, event_id DESC + LIMIT 1 + """ + + def _get_applicable_edit_txn(txn): + txn.execute( + sql, (event_id, RelationTypes.REPLACES, event_type, sender) + ) + row = txn.fetchone() + if row: + return row[0] + + edit_id = yield self.runInteraction( + "get_applicable_edit", _get_applicable_edit_txn + ) + + if not edit_id: + return + + edit_event = yield self.get_event(edit_id, allow_none=True) + defer.returnValue(edit_event) + class RelationsStore(RelationsWorkerStore): def _handle_event_relations(self, txn, event): @@ -357,3 +412,4 @@ def _handle_event_relations(self, txn, event): txn.call_after( self.get_aggregation_groups_for_event.invalidate_many, (parent_id,) ) + txn.call_after(self.get_applicable_edit.invalidate_many, (parent_id,)) diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index 775622bd2b9c..b0e4c47ae3c8 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -14,6 +14,7 @@ # limitations under the License. import itertools +import json import six @@ -102,11 +103,7 @@ def test_basic_paginate_relations(self): # relation event we sent above. self.assertEquals(len(channel.json_body["chunk"]), 1, channel.json_body) self.assert_dict( - { - "event_id": annotation_id, - "sender": self.user_id, - "type": "m.reaction", - }, + {"event_id": annotation_id, "sender": self.user_id, "type": "m.reaction"}, channel.json_body["chunk"][0], ) @@ -330,8 +327,6 @@ def test_aggregation_get_event(self): self.render(request) self.assertEquals(200, channel.code, channel.json_body) - self.maxDiff = None - self.assertEquals( channel.json_body["unsigned"].get("m.relations"), { @@ -347,7 +342,84 @@ def test_aggregation_get_event(self): }, ) - def _send_relation(self, relation_type, event_type, key=None): + def test_edit(self): + """Test that a simple edit works. + """ + + new_body = {"msgtype": "m.text", "body": "I've been edited!"} + channel = self._send_relation( + RelationTypes.REPLACES, + "m.room.message", + content={"msgtype": "m.text", "body": "foo", "m.new_content": new_body}, + ) + self.assertEquals(200, channel.code, channel.json_body) + + edit_event_id = channel.json_body["event_id"] + + request, channel = self.make_request( + "GET", "/rooms/%s/event/%s" % (self.room, self.parent_id) + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.assertEquals(channel.json_body["content"], new_body) + + self.assertEquals( + channel.json_body["unsigned"].get("m.relations"), + {RelationTypes.REPLACES: {"event_id": edit_event_id}}, + ) + + def test_multi_edit(self): + """Test that multiple edits, including attempts by people who + shouldn't be allowed, are correctly handled. + """ + + channel = self._send_relation( + RelationTypes.REPLACES, + "m.room.message", + content={ + "msgtype": "m.text", + "body": "Wibble", + "m.new_content": {"msgtype": "m.text", "body": "First edit"}, + }, + ) + self.assertEquals(200, channel.code, channel.json_body) + + new_body = {"msgtype": "m.text", "body": "I've been edited!"} + channel = self._send_relation( + RelationTypes.REPLACES, + "m.room.message", + content={"msgtype": "m.text", "body": "foo", "m.new_content": new_body}, + ) + self.assertEquals(200, channel.code, channel.json_body) + + edit_event_id = channel.json_body["event_id"] + + channel = self._send_relation( + RelationTypes.REPLACES, + "m.room.message.WRONG_TYPE", + content={ + "msgtype": "m.text", + "body": "Wibble", + "m.new_content": {"msgtype": "m.text", "body": "Edit, but wrong type"}, + }, + ) + self.assertEquals(200, channel.code, channel.json_body) + + request, channel = self.make_request( + "GET", "/rooms/%s/event/%s" % (self.room, self.parent_id) + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.assertEquals(channel.json_body["content"], new_body) + + self.assertEquals( + channel.json_body["unsigned"].get("m.relations"), + {RelationTypes.REPLACES: {"event_id": edit_event_id}}, + ) + + def _send_relation(self, relation_type, event_type, key=None, content={}): """Helper function to send a relation pointing at `self.parent_id` Args: @@ -355,6 +427,7 @@ def _send_relation(self, relation_type, event_type, key=None): event_type (str): The type of the event to create key (str|None): The aggregation key used for m.annotation relation type. + content(dict|None): The content of the created event. Returns: FakeChannel @@ -367,7 +440,7 @@ def _send_relation(self, relation_type, event_type, key=None): "POST", "/_matrix/client/unstable/rooms/%s/send_relation/%s/%s/%s%s" % (self.room, self.parent_id, relation_type, event_type, query), - b"{}", + json.dumps(content).encode("utf-8"), ) self.render(request) return channel From f89f688a55e1ddffbfa3613ac333f43e81d4b995 Mon Sep 17 00:00:00 2001 From: PauRE Date: Thu, 16 May 2019 20:04:26 +0200 Subject: [PATCH 34/49] Fix image orientation when generating thumbnail (#5039) --- changelog.d/5039.bugfix | 1 + synapse/python_dependencies.py | 2 +- synapse/rest/media/v1/media_repository.py | 9 ++++++ synapse/rest/media/v1/thumbnailer.py | 35 +++++++++++++++++++++++ 4 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5039.bugfix diff --git a/changelog.d/5039.bugfix b/changelog.d/5039.bugfix new file mode 100644 index 000000000000..212cff7ae87b --- /dev/null +++ b/changelog.d/5039.bugfix @@ -0,0 +1 @@ +Fix image orientation when generating thumbnails (needs pillow>=4.3.0). Contributed by Pau Rodriguez-Estivill. diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 2708f5e820f2..fdcfb90a7e92 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -53,7 +53,7 @@ "pyasn1-modules>=0.0.7", "daemonize>=2.3.1", "bcrypt>=3.1.0", - "pillow>=3.1.2", + "pillow>=4.3.0", "sortedcontainers>=1.4.4", "psutil>=2.0.0", "pymacaroons>=0.13.0", diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index bdffa9780558..856967735537 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -444,6 +444,9 @@ def _generate_thumbnail(self, thumbnailer, t_width, t_height, ) return + if thumbnailer.transpose_method is not None: + m_width, m_height = thumbnailer.transpose() + if t_method == "crop": t_byte_source = thumbnailer.crop(t_width, t_height, t_type) elif t_method == "scale": @@ -578,6 +581,12 @@ def _generate_thumbnails(self, server_name, media_id, file_id, media_type, ) return + if thumbnailer.transpose_method is not None: + m_width, m_height = yield logcontext.defer_to_thread( + self.hs.get_reactor(), + thumbnailer.transpose + ) + # We deduplicate the thumbnail sizes by ignoring the cropped versions if # they have the same dimensions of a scaled one. thumbnails = {} diff --git a/synapse/rest/media/v1/thumbnailer.py b/synapse/rest/media/v1/thumbnailer.py index a4b26c25877c..3efd0d80fc16 100644 --- a/synapse/rest/media/v1/thumbnailer.py +++ b/synapse/rest/media/v1/thumbnailer.py @@ -20,6 +20,17 @@ logger = logging.getLogger(__name__) +EXIF_ORIENTATION_TAG = 0x0112 +EXIF_TRANSPOSE_MAPPINGS = { + 2: Image.FLIP_LEFT_RIGHT, + 3: Image.ROTATE_180, + 4: Image.FLIP_TOP_BOTTOM, + 5: Image.TRANSPOSE, + 6: Image.ROTATE_270, + 7: Image.TRANSVERSE, + 8: Image.ROTATE_90 +} + class Thumbnailer(object): @@ -31,6 +42,30 @@ class Thumbnailer(object): def __init__(self, input_path): self.image = Image.open(input_path) self.width, self.height = self.image.size + self.transpose_method = None + try: + # We don't use ImageOps.exif_transpose since it crashes with big EXIF + image_exif = self.image._getexif() + if image_exif is not None: + image_orientation = image_exif.get(EXIF_ORIENTATION_TAG) + self.transpose_method = EXIF_TRANSPOSE_MAPPINGS.get(image_orientation) + except Exception as e: + # A lot of parsing errors can happen when parsing EXIF + logger.info("Error parsing image EXIF information: %s", e) + + def transpose(self): + """Transpose the image using its EXIF Orientation tag + + Returns: + Tuple[int, int]: (width, height) containing the new image size in pixels. + """ + if self.transpose_method is not None: + self.image = self.image.transpose(self.transpose_method) + self.width, self.height = self.image.size + self.transpose_method = None + # We don't need EXIF any more + self.image.info["exif"] = None + return self.image.size def aspect(self, max_width, max_height): """Calculate the largest size that preserves aspect ratio which From 7ce1f97a1353e7bd9232c22a20835e40fa5662e0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 17 May 2019 12:38:03 +0100 Subject: [PATCH 35/49] Stop telling people to install the optional dependencies. (#5197) * Stop telling people to install the optional dependencies. They're optional. Also update the postgres docs a bit for clarity(?) --- INSTALL.md | 4 ++-- changelog.d/5197.misc | 1 + docs/postgres.rst | 45 +++++++++++++++++++++---------------------- 3 files changed, 25 insertions(+), 25 deletions(-) create mode 100644 changelog.d/5197.misc diff --git a/INSTALL.md b/INSTALL.md index b88d826f6c53..1934593148c8 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -35,7 +35,7 @@ virtualenv -p python3 ~/synapse/env source ~/synapse/env/bin/activate pip install --upgrade pip pip install --upgrade setuptools -pip install matrix-synapse[all] +pip install matrix-synapse ``` This will download Synapse from [PyPI](https://pypi.org/project/matrix-synapse) @@ -48,7 +48,7 @@ update flag: ``` source ~/synapse/env/bin/activate -pip install -U matrix-synapse[all] +pip install -U matrix-synapse ``` Before you can start Synapse, you will need to generate a configuration diff --git a/changelog.d/5197.misc b/changelog.d/5197.misc new file mode 100644 index 000000000000..fca1d86b2e7c --- /dev/null +++ b/changelog.d/5197.misc @@ -0,0 +1 @@ +Stop telling people to install the optional dependencies by default. diff --git a/docs/postgres.rst b/docs/postgres.rst index f7ebbed0c33a..e81e10403f57 100644 --- a/docs/postgres.rst +++ b/docs/postgres.rst @@ -3,6 +3,28 @@ Using Postgres Postgres version 9.4 or later is known to work. +Install postgres client libraries +================================= + +Synapse will require the python postgres client library in order to connect to +a postgres database. + +* If you are using the `matrix.org debian/ubuntu + packages <../INSTALL.md#matrixorg-packages>`_, + the necessary libraries will already be installed. + +* For other pre-built packages, please consult the documentation from the + relevant package. + +* If you installed synapse `in a virtualenv + <../INSTALL.md#installing-from-source>`_, you can install the library with:: + + ~/synapse/env/bin/pip install matrix-synapse[postgres] + + (substituting the path to your virtualenv for ``~/synapse/env``, if you used a + different path). You will require the postgres development files. These are in + the ``libpq-dev`` package on Debian-derived distributions. + Set up database =============== @@ -26,29 +48,6 @@ encoding use, e.g.:: This would create an appropriate database named ``synapse`` owned by the ``synapse_user`` user (which must already exist). -Set up client in Debian/Ubuntu -=========================== - -Postgres support depends on the postgres python connector ``psycopg2``. In the -virtual env:: - - sudo apt-get install libpq-dev - pip install psycopg2 - -Set up client in RHEL/CentOs 7 -============================== - -Make sure you have the appropriate version of postgres-devel installed. For a -postgres 9.4, use the postgres 9.4 packages from -[here](https://wiki.postgresql.org/wiki/YUM_Installation). - -As with Debian/Ubuntu, postgres support depends on the postgres python connector -``psycopg2``. In the virtual env:: - - sudo yum install postgresql-devel libpqxx-devel.x86_64 - export PATH=/usr/pgsql-9.4/bin/:$PATH - pip install psycopg2 - Tuning Postgres =============== From afb463fb7a1878cca5e96feec4e883ae500421ab Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 17 May 2019 12:56:46 +0100 Subject: [PATCH 36/49] Some vagrant hackery for testing the debs --- debian/test/.gitignore | 2 ++ debian/test/provision.sh | 23 +++++++++++++++++++++++ debian/test/stretch/Vagrantfile | 13 +++++++++++++ debian/test/xenial/Vagrantfile | 10 ++++++++++ 4 files changed, 48 insertions(+) create mode 100644 debian/test/.gitignore create mode 100644 debian/test/provision.sh create mode 100644 debian/test/stretch/Vagrantfile create mode 100644 debian/test/xenial/Vagrantfile diff --git a/debian/test/.gitignore b/debian/test/.gitignore new file mode 100644 index 000000000000..95eda73fcc30 --- /dev/null +++ b/debian/test/.gitignore @@ -0,0 +1,2 @@ +.vagrant +*.log diff --git a/debian/test/provision.sh b/debian/test/provision.sh new file mode 100644 index 000000000000..a5c7f59712a1 --- /dev/null +++ b/debian/test/provision.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# +# provisioning script for vagrant boxes for testing the matrix-synapse debs. +# +# Will install the most recent matrix-synapse-py3 deb for this platform from +# the /debs directory. + +set -e + +apt-get update +apt-get install -y lsb-release + +deb=`ls /debs/matrix-synapse-py3_*+$(lsb_release -cs)*.deb | sort | tail -n1` + +debconf-set-selections < Date: Fri, 17 May 2019 13:27:19 +0100 Subject: [PATCH 37/49] expose SlavedProfileStore to ClientReaderSlavedStore (#5200) * expose SlavedProfileStore to ClientReaderSlavedStore --- changelog.d/5200.bugfix | 1 + synapse/app/client_reader.py | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 changelog.d/5200.bugfix diff --git a/changelog.d/5200.bugfix b/changelog.d/5200.bugfix new file mode 100644 index 000000000000..f346c7b0cc8d --- /dev/null +++ b/changelog.d/5200.bugfix @@ -0,0 +1 @@ +Fix worker registration bug caused by ClientReaderSlavedStore being unable to see get_profileinfo. diff --git a/synapse/app/client_reader.py b/synapse/app/client_reader.py index 864f1eac4826..707f7c2f078a 100644 --- a/synapse/app/client_reader.py +++ b/synapse/app/client_reader.py @@ -29,6 +29,7 @@ from synapse.http.site import SynapseSite from synapse.metrics import RegistryProxy from synapse.metrics.resource import METRICS_PREFIX, MetricsResource +from synapse.replication.slave.storage import SlavedProfileStore from synapse.replication.slave.storage._base import BaseSlavedStore from synapse.replication.slave.storage.account_data import SlavedAccountDataStore from synapse.replication.slave.storage.appservice import SlavedApplicationServiceStore @@ -83,6 +84,7 @@ class ClientReaderSlavedStore( SlavedTransactionStore, SlavedClientIpStore, BaseSlavedStore, + SlavedProfileStore, ): pass From 5dbff345094327e61fa9c1425ba0f955c9530ba7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 17 May 2019 15:48:04 +0100 Subject: [PATCH 38/49] Fixup bsaed on review comments --- synapse/events/utils.py | 4 +-- synapse/replication/slave/storage/events.py | 2 +- synapse/storage/relations.py | 32 +++++++++++---------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 2019ce9b1cf0..bf3c8f8dc1be 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -368,9 +368,7 @@ def serialize_event(self, event, time_now, **kwargs): edit = None if event.type == EventTypes.Message: - edit = yield self.store.get_applicable_edit( - event.event_id, event.type, event.sender, - ) + edit = yield self.store.get_applicable_edit(event_id) if edit: # If there is an edit replace the content, preserving existing diff --git a/synapse/replication/slave/storage/events.py b/synapse/replication/slave/storage/events.py index 857128b3118b..a3952506c13e 100644 --- a/synapse/replication/slave/storage/events.py +++ b/synapse/replication/slave/storage/events.py @@ -143,4 +143,4 @@ def invalidate_caches_for_event(self, stream_ordering, event_id, room_id, if relates_to: self.get_relations_for_event.invalidate_many((relates_to,)) self.get_aggregation_groups_for_event.invalidate_many((relates_to,)) - self.get_applicable_edit.invalidate_many((relates_to,)) + self.get_applicable_edit.invalidate((relates_to,)) diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index aa91e5273339..6e216066ab65 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -19,7 +19,7 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes, RelationTypes +from synapse.api.constants import RelationTypes from synapse.api.errors import SynapseError from synapse.storage._base import SQLBaseStore from synapse.storage.stream import generate_pagination_where_clause @@ -314,8 +314,8 @@ def _get_aggregation_groups_for_event_txn(txn): "get_aggregation_groups_for_event", _get_aggregation_groups_for_event_txn ) - @cachedInlineCallbacks(tree=True) - def get_applicable_edit(self, event_id, event_type, sender): + @cachedInlineCallbacks() + def get_applicable_edit(self, event_id): """Get the most recent edit (if any) that has happened for the given event. @@ -323,8 +323,6 @@ def get_applicable_edit(self, event_id, event_type, sender): Args: event_id (str): The original event ID - event_type (str): The original event type - sender (str): The original event sender Returns: Deferred[EventBase|None]: Returns the most recent edit, if any. @@ -332,26 +330,28 @@ def get_applicable_edit(self, event_id, event_type, sender): # We only allow edits for `m.room.message` events that have the same sender # and event type. We can't assert these things during regular event auth so - # we have to do the post hoc. - - if event_type != EventTypes.Message: - return + # we have to do the checks post hoc. + # Fetches latest edit that has the same type and sender as the + # original, and is an `m.room.message`. sql = """ - SELECT event_id, origin_server_ts FROM events + SELECT edit.event_id FROM events AS edit INNER JOIN event_relations USING (event_id) + INNER JOIN events AS original ON + original.event_id = relates_to_id + AND edit.type = original.type + AND edit.sender = original.sender WHERE relates_to_id = ? AND relation_type = ? - AND type = ? - AND sender = ? - ORDER by origin_server_ts DESC, event_id DESC + AND edit.type = 'm.room.message' + ORDER by edit.origin_server_ts DESC, edit.event_id DESC LIMIT 1 """ def _get_applicable_edit_txn(txn): txn.execute( - sql, (event_id, RelationTypes.REPLACES, event_type, sender) + sql, (event_id, RelationTypes.REPLACES,) ) row = txn.fetchone() if row: @@ -412,4 +412,6 @@ def _handle_event_relations(self, txn, event): txn.call_after( self.get_aggregation_groups_for_event.invalidate_many, (parent_id,) ) - txn.call_after(self.get_applicable_edit.invalidate_many, (parent_id,)) + + if rel_type == RelationTypes.REPLACES: + txn.call_after(self.get_applicable_edit.invalidate, (parent_id,)) From 8dd9cca8ead9fc0cf0789edf9fcfce04814c5eda Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 17 May 2019 16:40:51 +0100 Subject: [PATCH 39/49] Spelling and clarifications --- synapse/storage/stream.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/synapse/storage/stream.py b/synapse/storage/stream.py index 824e77c89e86..529ad4ea79f3 100644 --- a/synapse/storage/stream.py +++ b/synapse/storage/stream.py @@ -78,12 +78,12 @@ def generate_pagination_where_clause( would be generated for dir=b, from_token=(6, 7) and to_token=(5, 3). Note that tokens are considered to be after the row they are in, e.g. if - a row A has a token T, then we consider A to be before T. This covention + a row A has a token T, then we consider A to be before T. This convention is important when figuring out inequalities for the generated SQL, and produces the following result: - - If paginatiting forwards then we exclude any rows matching the from + - If paginating forwards then we exclude any rows matching the from token, but include those that match the to token. - - If paginatiting backwards then we include any rows matching the from + - If paginating backwards then we include any rows matching the from token, but include those that match the to token. Args: @@ -92,8 +92,12 @@ def generate_pagination_where_clause( column_names (tuple[str, str]): The column names to bound. Must *not* be user defined as these get inserted directly into the SQL statement without escapes. - from_token (tuple[int, int]|None) - to_token (tuple[int, int]|None) + from_token (tuple[int, int]|None): The start point for the pagination. + This is an exclusive minimum bound if direction is "f", and an + inclusive maximum bound if direction is "b". + to_token (tuple[int, int]|None): The endpoint point for the pagination. + This is an inclusive maximum bound if direction is "f", and an + exclusive minimum bound if direction is "b". engine: The database engine to generate the clauses for Returns: From 291e1eea5eab04df79ad607ca23fb421f11a63ff Mon Sep 17 00:00:00 2001 From: bytepoets-blo Date: Fri, 17 May 2019 18:27:14 +0200 Subject: [PATCH 40/49] fix mapping of return values for get_or_register_3pid_guest (#5177) * fix mapping of return values for get_or_register_3pid_guest --- changelog.d/5177.bugfix | 1 + synapse/handlers/room_member.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5177.bugfix diff --git a/changelog.d/5177.bugfix b/changelog.d/5177.bugfix new file mode 100644 index 000000000000..c2f1644ae5fa --- /dev/null +++ b/changelog.d/5177.bugfix @@ -0,0 +1 @@ +Fix 3pid guest invites. diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index ffc588d4546c..93ac986c86ee 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -944,7 +944,7 @@ def _ask_id_server_for_third_party_invite( } if self.config.invite_3pid_guest: - guest_access_token, guest_user_id = yield self.get_or_register_3pid_guest( + guest_user_id, guest_access_token = yield self.get_or_register_3pid_guest( requester=requester, medium=medium, address=address, From d4ca533d702e9518244b8ca6e1861a7c8f64adf1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 17 May 2019 17:45:51 +0100 Subject: [PATCH 41/49] Make tests use different user for each reaction it sends As users aren't allowed to react with the same emoji more than once. --- tests/rest/client/v2_alpha/test_relations.py | 80 +++++++++++++++++--- 1 file changed, 68 insertions(+), 12 deletions(-) diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index b0e4c47ae3c8..cd965167f8c1 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -19,19 +19,22 @@ import six from synapse.api.constants import EventTypes, RelationTypes +from synapse.rest import admin from synapse.rest.client.v1 import login, room -from synapse.rest.client.v2_alpha import relations +from synapse.rest.client.v2_alpha import register, relations from tests import unittest class RelationsTestCase(unittest.HomeserverTestCase): - user_id = "@alice:test" servlets = [ relations.register_servlets, room.register_servlets, login.register_servlets, + register.register_servlets, + admin.register_servlets_for_client_rest_resource, ] + hijack_auth = False def make_homeserver(self, reactor, clock): # We need to enable msc1849 support for aggregations @@ -40,8 +43,12 @@ def make_homeserver(self, reactor, clock): return self.setup_test_homeserver(config=config) def prepare(self, reactor, clock, hs): - self.room = self.helper.create_room_as(self.user_id) - res = self.helper.send(self.room, body="Hi!") + self.user_id, self.user_token = self._create_user("alice") + self.user2_id, self.user2_token = self._create_user("bob") + + self.room = self.helper.create_room_as(self.user_id, tok=self.user_token) + self.helper.join(self.room, user=self.user2_id, tok=self.user2_token) + res = self.helper.send(self.room, body="Hi!", tok=self.user_token) self.parent_id = res["event_id"] def test_send_relation(self): @@ -55,7 +62,9 @@ def test_send_relation(self): event_id = channel.json_body["event_id"] request, channel = self.make_request( - "GET", "/rooms/%s/event/%s" % (self.room, event_id) + "GET", + "/rooms/%s/event/%s" % (self.room, event_id), + access_token=self.user_token, ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) @@ -95,6 +104,7 @@ def test_basic_paginate_relations(self): "GET", "/_matrix/client/unstable/rooms/%s/relations/%s?limit=1" % (self.room, self.parent_id), + access_token=self.user_token, ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) @@ -135,6 +145,7 @@ def test_repeated_paginate_relations(self): "GET", "/_matrix/client/unstable/rooms/%s/relations/%s?limit=1%s" % (self.room, self.parent_id, from_token), + access_token=self.user_token, ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) @@ -156,15 +167,32 @@ def test_aggregation_pagination_groups(self): """Test that we can paginate annotation groups correctly. """ + # We need to create ten separate users to send each reaction. + access_tokens = [self.user_token, self.user2_token] + idx = 0 + while len(access_tokens) < 10: + user_id, token = self._create_user("test" + str(idx)) + idx += 1 + + self.helper.join(self.room, user=user_id, tok=token) + access_tokens.append(token) + + idx = 0 sent_groups = {u"👍": 10, u"a": 7, u"b": 5, u"c": 3, u"d": 2, u"e": 1} for key in itertools.chain.from_iterable( itertools.repeat(key, num) for key, num in sent_groups.items() ): channel = self._send_relation( - RelationTypes.ANNOTATION, "m.reaction", key=key + RelationTypes.ANNOTATION, + "m.reaction", + key=key, + access_token=access_tokens[idx], ) self.assertEquals(200, channel.code, channel.json_body) + idx += 1 + idx %= len(access_tokens) + prev_token = None found_groups = {} for _ in range(20): @@ -176,6 +204,7 @@ def test_aggregation_pagination_groups(self): "GET", "/_matrix/client/unstable/rooms/%s/aggregations/%s?limit=1%s" % (self.room, self.parent_id, from_token), + access_token=self.user_token, ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) @@ -236,6 +265,7 @@ def test_aggregation_pagination_within_group(self): encoded_key, from_token, ), + access_token=self.user_token, ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) @@ -263,7 +293,9 @@ def test_aggregation(self): channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") self.assertEquals(200, channel.code, channel.json_body) - channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + channel = self._send_relation( + RelationTypes.ANNOTATION, "m.reaction", "a", access_token=self.user2_token + ) self.assertEquals(200, channel.code, channel.json_body) channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "b") @@ -273,6 +305,7 @@ def test_aggregation(self): "GET", "/_matrix/client/unstable/rooms/%s/aggregations/%s" % (self.room, self.parent_id), + access_token=self.user_token, ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) @@ -295,6 +328,7 @@ def test_aggregation_must_be_annotation(self): "GET", "/_matrix/client/unstable/rooms/%s/aggregations/%s/m.replace?limit=1" % (self.room, self.parent_id), + access_token=self.user_token, ) self.render(request) self.assertEquals(400, channel.code, channel.json_body) @@ -307,7 +341,9 @@ def test_aggregation_get_event(self): channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") self.assertEquals(200, channel.code, channel.json_body) - channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + channel = self._send_relation( + RelationTypes.ANNOTATION, "m.reaction", "a", access_token=self.user2_token + ) self.assertEquals(200, channel.code, channel.json_body) channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "b") @@ -322,7 +358,9 @@ def test_aggregation_get_event(self): reply_2 = channel.json_body["event_id"] request, channel = self.make_request( - "GET", "/rooms/%s/event/%s" % (self.room, self.parent_id) + "GET", + "/rooms/%s/event/%s" % (self.room, self.parent_id), + access_token=self.user_token, ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) @@ -357,7 +395,9 @@ def test_edit(self): edit_event_id = channel.json_body["event_id"] request, channel = self.make_request( - "GET", "/rooms/%s/event/%s" % (self.room, self.parent_id) + "GET", + "/rooms/%s/event/%s" % (self.room, self.parent_id), + access_token=self.user_token, ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) @@ -407,7 +447,9 @@ def test_multi_edit(self): self.assertEquals(200, channel.code, channel.json_body) request, channel = self.make_request( - "GET", "/rooms/%s/event/%s" % (self.room, self.parent_id) + "GET", + "/rooms/%s/event/%s" % (self.room, self.parent_id), + access_token=self.user_token, ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) @@ -419,7 +461,9 @@ def test_multi_edit(self): {RelationTypes.REPLACES: {"event_id": edit_event_id}}, ) - def _send_relation(self, relation_type, event_type, key=None, content={}): + def _send_relation( + self, relation_type, event_type, key=None, content={}, access_token=None + ): """Helper function to send a relation pointing at `self.parent_id` Args: @@ -428,10 +472,15 @@ def _send_relation(self, relation_type, event_type, key=None, content={}): key (str|None): The aggregation key used for m.annotation relation type. content(dict|None): The content of the created event. + access_token (str|None): The access token used to send the relation, + defaults to `self.user_token` Returns: FakeChannel """ + if not access_token: + access_token = self.user_token + query = "" if key: query = "?key=" + six.moves.urllib.parse.quote_plus(key.encode("utf-8")) @@ -441,6 +490,13 @@ def _send_relation(self, relation_type, event_type, key=None, content={}): "/_matrix/client/unstable/rooms/%s/send_relation/%s/%s/%s%s" % (self.room, self.parent_id, relation_type, event_type, query), json.dumps(content).encode("utf-8"), + access_token=access_token, ) self.render(request) return channel + + def _create_user(self, localpart): + user_id = self.register_user(localpart, "abc123") + access_token = self.login(localpart, "abc123") + + return user_id, access_token From 3787133c9e3fcf0e9b85700418bf03c48ec86ab3 Mon Sep 17 00:00:00 2001 From: ReidAnderson Date: Mon, 20 May 2019 05:20:08 -0500 Subject: [PATCH 42/49] Limit UserIds to a length that fits in a state key (#5198) --- changelog.d/5198.bugfix | 1 + synapse/api/constants.py | 3 +++ synapse/handlers/register.py | 11 ++++++++++- tests/handlers/test_register.py | 7 +++++++ 4 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5198.bugfix diff --git a/changelog.d/5198.bugfix b/changelog.d/5198.bugfix new file mode 100644 index 000000000000..c6b156f17dff --- /dev/null +++ b/changelog.d/5198.bugfix @@ -0,0 +1 @@ +Prevent registration for user ids that are to long to fit into a state key. Contributed by Reid Anderson. \ No newline at end of file diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 8547a635350a..c7bf95b42671 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -23,6 +23,9 @@ # the maximum length for a room alias is 255 characters MAX_ALIAS_LENGTH = 255 +# the maximum length for a user id is 255 characters +MAX_USERID_LENGTH = 255 + class Membership(object): diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index a51d11a257f4..e83ee24f103b 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -19,7 +19,7 @@ from twisted.internet import defer from synapse import types -from synapse.api.constants import LoginType +from synapse.api.constants import MAX_USERID_LENGTH, LoginType from synapse.api.errors import ( AuthError, Codes, @@ -123,6 +123,15 @@ def check_username(self, localpart, guest_access_token=None, self.check_user_id_not_appservice_exclusive(user_id) + if len(user_id) > MAX_USERID_LENGTH: + raise SynapseError( + 400, + "User ID may not be longer than %s characters" % ( + MAX_USERID_LENGTH, + ), + Codes.INVALID_USERNAME + ) + users = yield self.store.get_users_by_id_case_insensitive(user_id) if users: if not guest_access_token: diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index 1c253d0579c5..5ffba2ca7aa6 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -228,3 +228,10 @@ def test_register_support_user(self): def test_register_not_support_user(self): res = self.get_success(self.handler.register(localpart='user')) self.assertFalse(self.store.is_support_user(res[0])) + + def test_invalid_user_id_length(self): + invalid_user_id = "x" * 256 + self.get_failure( + self.handler.register(localpart=invalid_user_id), + SynapseError + ) From 935af0da380f39ba284b78054270331bdbad7712 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 20 May 2019 10:13:05 +0100 Subject: [PATCH 43/49] Correctly update aggregation counts after redaction --- synapse/storage/events.py | 3 ++ synapse/storage/relations.py | 17 +++++++++ tests/rest/client/v2_alpha/test_relations.py | 37 ++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/synapse/storage/events.py b/synapse/storage/events.py index b025ebc926b3..881d6d012609 100644 --- a/synapse/storage/events.py +++ b/synapse/storage/events.py @@ -1325,6 +1325,9 @@ def _update_metadata_tables_txn( txn, event.room_id, event.redacts ) + # Remove from relations table. + self._handle_redaction(txn, event.redacts) + # Update the event_forward_extremities, event_backward_extremities and # event_edges tables. self._handle_mult_prev_events( diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index 6e216066ab65..63e6185ee350 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -415,3 +415,20 @@ def _handle_event_relations(self, txn, event): if rel_type == RelationTypes.REPLACES: txn.call_after(self.get_applicable_edit.invalidate, (parent_id,)) + + def _handle_redaction(self, txn, redacted_event_id): + """Handles receiving a redaction and checking whether we need to remove + any redacted relations from the database. + + Args: + txn + redacted_event_id (str): The event that was redacted. + """ + + self._simple_delete_txn( + txn, + table="event_relations", + keyvalues={ + "event_id": redacted_event_id, + } + ) diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index cd965167f8c1..3737cdc396df 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -320,6 +320,43 @@ def test_aggregation(self): }, ) + def test_aggregation_redactions(self): + """Test that annotations get correctly aggregated after a redactions. + """ + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + self.assertEquals(200, channel.code, channel.json_body) + to_redact_event_id = channel.json_body["event_id"] + + channel = self._send_relation( + RelationTypes.ANNOTATION, "m.reaction", "a", access_token=self.user2_token + ) + self.assertEquals(200, channel.code, channel.json_body) + + # Now lets redact the 'a' reaction + request, channel = self.make_request( + "POST", + "/_matrix/client/r0/rooms/%s/redact/%s" % (self.room, to_redact_event_id), + access_token=self.user_token, + content={}, + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + request, channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/aggregations/%s" + % (self.room, self.parent_id), + access_token=self.user_token, + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.assertEquals( + channel.json_body, + {"chunk": [{"type": "m.reaction", "key": "a", "count": 1}]}, + ) + def test_aggregation_must_be_annotation(self): """Test that aggregations must be annotations. """ From 2ac9c965ddfb049251d0406f93c1104b227009e3 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 20 May 2019 12:32:26 +0100 Subject: [PATCH 44/49] Fixup comments --- tests/rest/client/v2_alpha/test_relations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index 3737cdc396df..b3dc4bd5bcda 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -321,7 +321,7 @@ def test_aggregation(self): ) def test_aggregation_redactions(self): - """Test that annotations get correctly aggregated after a redactions. + """Test that annotations get correctly aggregated after a redaction. """ channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") @@ -333,7 +333,7 @@ def test_aggregation_redactions(self): ) self.assertEquals(200, channel.code, channel.json_body) - # Now lets redact the 'a' reaction + # Now lets redact one of the 'a' reactions request, channel = self.make_request( "POST", "/_matrix/client/r0/rooms/%s/redact/%s" % (self.room, to_redact_event_id), From 06671057b626042ca4645a370cd62edcf4d6211b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 20 May 2019 12:38:20 +0100 Subject: [PATCH 45/49] Newsfile --- changelog.d/5198.feature | 1 - changelog.d/5209.feature | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 changelog.d/5198.feature create mode 100644 changelog.d/5209.feature diff --git a/changelog.d/5198.feature b/changelog.d/5198.feature deleted file mode 100644 index c8f624d17231..000000000000 --- a/changelog.d/5198.feature +++ /dev/null @@ -1 +0,0 @@ -Add experimental support for reactions. diff --git a/changelog.d/5209.feature b/changelog.d/5209.feature new file mode 100644 index 000000000000..747098c16624 --- /dev/null +++ b/changelog.d/5209.feature @@ -0,0 +1 @@ +Add experimental support for relations (aka reactions and edits). From 1dff859d6aed58561a2b5913e5c9b897bbd3599c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 20 May 2019 14:31:19 +0100 Subject: [PATCH 46/49] Rename relation types to match MSC --- synapse/api/constants.py | 4 ++-- synapse/events/utils.py | 6 +++--- synapse/storage/relations.py | 8 +++---- tests/rest/client/v2_alpha/test_relations.py | 22 ++++++++++---------- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 28862a1d254b..6b347b17491f 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -125,5 +125,5 @@ class RelationTypes(object): """The types of relations known to this server. """ ANNOTATION = "m.annotation" - REPLACES = "m.replaces" - REFERENCES = "m.references" + REPLACE = "m.replace" + REFERENCE = "m.reference" diff --git a/synapse/events/utils.py b/synapse/events/utils.py index bf3c8f8dc1be..27a2a9ef986e 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -355,7 +355,7 @@ def serialize_event(self, event, time_now, **kwargs): event_id, ) references = yield self.store.get_relations_for_event( - event_id, RelationTypes.REFERENCES, direction="f", + event_id, RelationTypes.REFERENCE, direction="f", ) if annotations.chunk: @@ -364,7 +364,7 @@ def serialize_event(self, event, time_now, **kwargs): if references.chunk: r = serialized_event["unsigned"].setdefault("m.relations", {}) - r[RelationTypes.REFERENCES] = references.to_dict() + r[RelationTypes.REFERENCE] = references.to_dict() edit = None if event.type == EventTypes.Message: @@ -382,7 +382,7 @@ def serialize_event(self, event, time_now, **kwargs): serialized_event["content"].pop("m.relates_to", None) r = serialized_event["unsigned"].setdefault("m.relations", {}) - r[RelationTypes.REPLACES] = { + r[RelationTypes.REPLACE] = { "event_id": edit.event_id, } diff --git a/synapse/storage/relations.py b/synapse/storage/relations.py index 63e6185ee350..493abe405ecc 100644 --- a/synapse/storage/relations.py +++ b/synapse/storage/relations.py @@ -351,7 +351,7 @@ def get_applicable_edit(self, event_id): def _get_applicable_edit_txn(txn): txn.execute( - sql, (event_id, RelationTypes.REPLACES,) + sql, (event_id, RelationTypes.REPLACE,) ) row = txn.fetchone() if row: @@ -384,8 +384,8 @@ def _handle_event_relations(self, txn, event): rel_type = relation.get("rel_type") if rel_type not in ( RelationTypes.ANNOTATION, - RelationTypes.REFERENCES, - RelationTypes.REPLACES, + RelationTypes.REFERENCE, + RelationTypes.REPLACE, ): # Unknown relation type return @@ -413,7 +413,7 @@ def _handle_event_relations(self, txn, event): self.get_aggregation_groups_for_event.invalidate_many, (parent_id,) ) - if rel_type == RelationTypes.REPLACES: + if rel_type == RelationTypes.REPLACE: txn.call_after(self.get_applicable_edit.invalidate, (parent_id,)) def _handle_redaction(self, txn, redacted_event_id): diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index b3dc4bd5bcda..3d040cf1188d 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -363,8 +363,8 @@ def test_aggregation_must_be_annotation(self): request, channel = self.make_request( "GET", - "/_matrix/client/unstable/rooms/%s/aggregations/%s/m.replace?limit=1" - % (self.room, self.parent_id), + "/_matrix/client/unstable/rooms/%s/aggregations/%s/%s?limit=1" + % (self.room, self.parent_id, RelationTypes.REPLACE), access_token=self.user_token, ) self.render(request) @@ -386,11 +386,11 @@ def test_aggregation_get_event(self): channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "b") self.assertEquals(200, channel.code, channel.json_body) - channel = self._send_relation(RelationTypes.REFERENCES, "m.room.test") + channel = self._send_relation(RelationTypes.REFERENCE, "m.room.test") self.assertEquals(200, channel.code, channel.json_body) reply_1 = channel.json_body["event_id"] - channel = self._send_relation(RelationTypes.REFERENCES, "m.room.test") + channel = self._send_relation(RelationTypes.REFERENCE, "m.room.test") self.assertEquals(200, channel.code, channel.json_body) reply_2 = channel.json_body["event_id"] @@ -411,7 +411,7 @@ def test_aggregation_get_event(self): {"type": "m.reaction", "key": "b", "count": 1}, ] }, - RelationTypes.REFERENCES: { + RelationTypes.REFERENCE: { "chunk": [{"event_id": reply_1}, {"event_id": reply_2}] }, }, @@ -423,7 +423,7 @@ def test_edit(self): new_body = {"msgtype": "m.text", "body": "I've been edited!"} channel = self._send_relation( - RelationTypes.REPLACES, + RelationTypes.REPLACE, "m.room.message", content={"msgtype": "m.text", "body": "foo", "m.new_content": new_body}, ) @@ -443,7 +443,7 @@ def test_edit(self): self.assertEquals( channel.json_body["unsigned"].get("m.relations"), - {RelationTypes.REPLACES: {"event_id": edit_event_id}}, + {RelationTypes.REPLACE: {"event_id": edit_event_id}}, ) def test_multi_edit(self): @@ -452,7 +452,7 @@ def test_multi_edit(self): """ channel = self._send_relation( - RelationTypes.REPLACES, + RelationTypes.REPLACE, "m.room.message", content={ "msgtype": "m.text", @@ -464,7 +464,7 @@ def test_multi_edit(self): new_body = {"msgtype": "m.text", "body": "I've been edited!"} channel = self._send_relation( - RelationTypes.REPLACES, + RelationTypes.REPLACE, "m.room.message", content={"msgtype": "m.text", "body": "foo", "m.new_content": new_body}, ) @@ -473,7 +473,7 @@ def test_multi_edit(self): edit_event_id = channel.json_body["event_id"] channel = self._send_relation( - RelationTypes.REPLACES, + RelationTypes.REPLACE, "m.room.message.WRONG_TYPE", content={ "msgtype": "m.text", @@ -495,7 +495,7 @@ def test_multi_edit(self): self.assertEquals( channel.json_body["unsigned"].get("m.relations"), - {RelationTypes.REPLACES: {"event_id": edit_event_id}}, + {RelationTypes.REPLACE: {"event_id": edit_event_id}}, ) def _send_relation( From d642178654a025664fd4ff984c1259a1b1271845 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 20 May 2019 14:32:16 +0100 Subject: [PATCH 47/49] Newsfile --- changelog.d/5211.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5211.feature diff --git a/changelog.d/5211.feature b/changelog.d/5211.feature new file mode 100644 index 000000000000..747098c16624 --- /dev/null +++ b/changelog.d/5211.feature @@ -0,0 +1 @@ +Add experimental support for relations (aka reactions and edits). From 5206648a4a2c94543d46e5c22da6fd595b120eeb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 20 May 2019 15:54:42 +0100 Subject: [PATCH 48/49] Add a test room version which updates event ID format (#5210) Implements MSC1884 --- changelog.d/5210.feature | 1 + synapse/api/room_versions.py | 13 +++++++++++-- synapse/events/__init__.py | 23 ++++++++++++++++++++++- 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 changelog.d/5210.feature diff --git a/changelog.d/5210.feature b/changelog.d/5210.feature new file mode 100644 index 000000000000..a7476bf9b94a --- /dev/null +++ b/changelog.d/5210.feature @@ -0,0 +1 @@ +Add a new room version which uses a new event ID format. diff --git a/synapse/api/room_versions.py b/synapse/api/room_versions.py index e77abe10407d..485b3d0237b9 100644 --- a/synapse/api/room_versions.py +++ b/synapse/api/room_versions.py @@ -19,13 +19,15 @@ class EventFormatVersions(object): """This is an internal enum for tracking the version of the event format, independently from the room version. """ - V1 = 1 # $id:server format - V2 = 2 # MSC1659-style $hash format: introduced for room v3 + V1 = 1 # $id:server event id format + V2 = 2 # MSC1659-style $hash event id format: introduced for room v3 + V3 = 3 # MSC1884-style $hash format: introduced for room v4 KNOWN_EVENT_FORMAT_VERSIONS = { EventFormatVersions.V1, EventFormatVersions.V2, + EventFormatVersions.V3, } @@ -75,6 +77,12 @@ class RoomVersions(object): EventFormatVersions.V2, StateResolutionVersions.V2, ) + EVENTID_NOSLASH_TEST = RoomVersion( + "eventid-noslash-test", + RoomDisposition.UNSTABLE, + EventFormatVersions.V3, + StateResolutionVersions.V2, + ) # the version we will give rooms which are created on this server @@ -87,5 +95,6 @@ class RoomVersions(object): RoomVersions.V2, RoomVersions.V3, RoomVersions.STATE_V2_TEST, + RoomVersions.EVENTID_NOSLASH_TEST, ) } # type: dict[str, RoomVersion] diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 12056d5be28d..badeb903fc5d 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -335,13 +335,32 @@ def __str__(self): return self.__repr__() def __repr__(self): - return "" % ( + return "<%s event_id='%s', type='%s', state_key='%s'>" % ( + self.__class__.__name__, self.event_id, self.get("type", None), self.get("state_key", None), ) +class FrozenEventV3(FrozenEventV2): + """FrozenEventV3, which differs from FrozenEventV2 only in the event_id format""" + format_version = EventFormatVersions.V3 # All events of this type are V3 + + @property + def event_id(self): + # We have to import this here as otherwise we get an import loop which + # is hard to break. + from synapse.crypto.event_signing import compute_event_reference_hash + + if self._event_id: + return self._event_id + self._event_id = "$" + encode_base64( + compute_event_reference_hash(self)[1], urlsafe=True + ) + return self._event_id + + def room_version_to_event_format(room_version): """Converts a room version string to the event format @@ -376,6 +395,8 @@ def event_type_from_format_version(format_version): return FrozenEvent elif format_version == EventFormatVersions.V2: return FrozenEventV2 + elif format_version == EventFormatVersions.V3: + return FrozenEventV3 else: raise Exception( "No event format %r" % (format_version,) From 24b93b9c76e0623c231abf37b84e0b841879b890 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 20 May 2019 16:21:34 +0100 Subject: [PATCH 49/49] Revert "expose SlavedProfileStore to ClientReaderSlavedStore (#5200)" This reverts commit ce5bcefc609db40740c692bd53a1ef84ab675e8c. This caused: ``` Traceback (most recent call last): File "/usr/local/lib/python3.7/runpy.py", line 193, in _run_module_as_main "__main__", mod_spec) File "/usr/local/lib/python3.7/runpy.py", line 85, in _run_code exec(code, run_globals) File "/home/synapse/src/synapse/app/client_reader.py", line 32, in from synapse.replication.slave.storage import SlavedProfileStore ImportError: cannot import name 'SlavedProfileStore' from 'synapse.replication.slave.storage' (/home/synapse/src/synapse/replication/slave/storage/__init__.py) error starting synapse.app.client_reader('/home/synapse/config/workers/client_reader.yaml') (exit code: 1); see above for logs ``` --- changelog.d/5200.bugfix | 1 - synapse/app/client_reader.py | 2 -- 2 files changed, 3 deletions(-) delete mode 100644 changelog.d/5200.bugfix diff --git a/changelog.d/5200.bugfix b/changelog.d/5200.bugfix deleted file mode 100644 index f346c7b0cc8d..000000000000 --- a/changelog.d/5200.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix worker registration bug caused by ClientReaderSlavedStore being unable to see get_profileinfo. diff --git a/synapse/app/client_reader.py b/synapse/app/client_reader.py index 707f7c2f078a..864f1eac4826 100644 --- a/synapse/app/client_reader.py +++ b/synapse/app/client_reader.py @@ -29,7 +29,6 @@ from synapse.http.site import SynapseSite from synapse.metrics import RegistryProxy from synapse.metrics.resource import METRICS_PREFIX, MetricsResource -from synapse.replication.slave.storage import SlavedProfileStore from synapse.replication.slave.storage._base import BaseSlavedStore from synapse.replication.slave.storage.account_data import SlavedAccountDataStore from synapse.replication.slave.storage.appservice import SlavedApplicationServiceStore @@ -84,7 +83,6 @@ class ClientReaderSlavedStore( SlavedTransactionStore, SlavedClientIpStore, BaseSlavedStore, - SlavedProfileStore, ): pass