From f4af87979a80f8cf721c38b7380133133bf0dc22 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 21 Feb 2020 13:53:54 +0000 Subject: [PATCH 1/2] Upsert room version when we join over federation This is intended as a precursor to storing room versions when we receive an invite over federation, but has the happy side-effect of fixing #3374 at last. In short: change the store_room with try/except to a proper upsert which updates the right columns. --- changelog.d/6968.bugfix | 1 + synapse/handlers/federation.py | 22 ++++++++++++---------- synapse/storage/data_stores/main/room.py | 16 ++++++++++++++++ 3 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 changelog.d/6968.bugfix diff --git a/changelog.d/6968.bugfix b/changelog.d/6968.bugfix new file mode 100644 index 000000000000..9965bfc0c307 --- /dev/null +++ b/changelog.d/6968.bugfix @@ -0,0 +1 @@ +Fix `duplicate key` error which was logged when rejoining a room over federation. diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index eb20ef4aece9..35db02655f94 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1323,16 +1323,18 @@ async def do_invite_join( logger.debug("do_invite_join event: %s", event) - try: - await self.store.store_room( - room_id=room_id, - room_creator_user_id="", - is_public=False, - room_version=room_version_obj, - ) - except Exception: - # FIXME - pass + # if this is the first time we've joined this room, it's time to add + # a row to `rooms` with the correct room version. If there's already a + # row there, we should override it, since it may have been populated + # based on an invite request which lied about the room version. + # + # federation_client.send_join has already checked that the room + # version in the received create event is the same as room_version_obj, + # so we can rely on it now. + # + await self.store.upsert_room_on_join( + room_id=room_id, room_version=room_version_obj, + ) await self._persist_auth_tree( origin, auth_chain, state, event, room_version_obj diff --git a/synapse/storage/data_stores/main/room.py b/synapse/storage/data_stores/main/room.py index 9a17e336ba16..f71f4c0fcad9 100644 --- a/synapse/storage/data_stores/main/room.py +++ b/synapse/storage/data_stores/main/room.py @@ -954,6 +954,22 @@ def __init__(self, database: Database, db_conn, hs): self.config = hs.config + async def upsert_room_on_join(self, room_id: str, room_version: RoomVersion): + """Ensure that the room is stored in the table + + Called when we join a room over federation, and overwrites any room version + currently in the table. + """ + await self.db.simple_upsert( + table="rooms", + keyvalues={"room_id": room_id}, + values={"room_version": room_version.identifier}, + insertion_values={"is_public": False, "creator": ""}, + # rooms has a unique constraint on room_id, so no need to lock when doing an + # emulated upsert. + lock=False, + ) + @defer.inlineCallbacks def store_room( self, From c2a423b2f1d541bf122855b5726281d51999288c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Feb 2020 12:41:54 +0000 Subject: [PATCH 2/2] add a desc --- synapse/storage/data_stores/main/room.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/storage/data_stores/main/room.py b/synapse/storage/data_stores/main/room.py index f71f4c0fcad9..70137dfbe4af 100644 --- a/synapse/storage/data_stores/main/room.py +++ b/synapse/storage/data_stores/main/room.py @@ -961,6 +961,7 @@ async def upsert_room_on_join(self, room_id: str, room_version: RoomVersion): currently in the table. """ await self.db.simple_upsert( + desc="upsert_room_on_join", table="rooms", keyvalues={"room_id": room_id}, values={"room_version": room_version.identifier},