blindly incorporate PR review - needs testing & fixing

2 files changed, 31 insertions, 46 deletions

diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py

index 8efca11a8c..c11417c415 100644
--- a/synapse/storage/e2e_room_keys.py
+++ b/synapse/storage/e2e_room_keys.py
@@ -44,30 +44,21 @@ class EndToEndRoomKeyStore(SQLBaseStore):
 
     def set_e2e_room_key(self, user_id, version, room_id, session_id, room_key):
 
-        def _set_e2e_room_key_txn(txn):
-
-            self._simple_upsert_txn(
-                txn,
-                table="e2e_room_keys",
-                keyvalues={
-                    "user_id": user_id,
-                    "room_id": room_id,
-                    "session_id": session_id,
-                },
-                values={
-                    "version": version,
-                    "first_message_index": room_key['first_message_index'],
-                    "forwarded_count": room_key['forwarded_count'],
-                    "is_verified": room_key['is_verified'],
-                    "session_data": room_key['session_data'],
-                },
-                lock=False,
-            )
-
-            return True
-
-        return self.runInteraction(
-            "set_e2e_room_key", _set_e2e_room_key_txn
+        yield self._simple_upsert(
+            table="e2e_room_keys",
+            keyvalues={
+                "user_id": user_id,
+                "room_id": room_id,
+                "session_id": session_id,
+            },
+            values={
+                "version": version,
+                "first_message_index": room_key['first_message_index'],
+                "forwarded_count": room_key['forwarded_count'],
+                "is_verified": room_key['is_verified'],
+                "session_data": room_key['session_data'],
+            },
+            lock=False,
         )
 
     # XXX: this isn't currently used and isn't tested anywhere
@@ -107,7 +98,9 @@ class EndToEndRoomKeyStore(SQLBaseStore):
         )
 
     @defer.inlineCallbacks
-    def get_e2e_room_keys(self, user_id, version, room_id, session_id):
+    def get_e2e_room_keys(
+        self, user_id, version, room_id=room_id, session_id=session_id
+    ):
 
         keyvalues = {
             "user_id": user_id,
@@ -115,8 +108,8 @@ class EndToEndRoomKeyStore(SQLBaseStore):
         }
         if room_id:
             keyvalues['room_id'] = room_id
-        if session_id:
-            keyvalues['session_id'] = session_id
+            if session_id:
+                keyvalues['session_id'] = session_id
 
         rows = yield self._simple_select_list(
             table="e2e_room_keys",
@@ -133,18 +126,10 @@ class EndToEndRoomKeyStore(SQLBaseStore):
             desc="get_e2e_room_keys",
         )
 
-        # perlesque autovivification from https://stackoverflow.com/a/19829714/6764493
-        class AutoVivification(dict):
-            def __getitem__(self, item):
-                try:
-                    return dict.__getitem__(self, item)
-                except KeyError:
-                    value = self[item] = type(self)()
-                    return value
-
-        sessions = AutoVivification()
+        sessions = {}
         for row in rows:
-            sessions['rooms'][row['room_id']]['sessions'][row['session_id']] = {
+            room_entry = sessions['rooms'].setdefault(row['room_id'], {"sessions": {}})
+            room_entry['sessions'][row['session_id']] = {
                 "first_message_index": row["first_message_index"],
                 "forwarded_count": row["forwarded_count"],
                 "is_verified": row["is_verified"],
@@ -154,7 +139,9 @@ class EndToEndRoomKeyStore(SQLBaseStore):
         defer.returnValue(sessions)
 
     @defer.inlineCallbacks
-    def delete_e2e_room_keys(self, user_id, version, room_id, session_id):
+    def delete_e2e_room_keys(
+        self, user_id, version, room_id=room_id, session_id=session_id
+    ):
 
         keyvalues = {
             "user_id": user_id,
@@ -162,8 +149,8 @@ class EndToEndRoomKeyStore(SQLBaseStore):
         }
         if room_id:
             keyvalues['room_id'] = room_id
-        if session_id:
-            keyvalues['session_id'] = session_id
+            if session_id:
+                keyvalues['session_id'] = session_id
 
         yield self._simple_delete(
             table="e2e_room_keys",
diff --git a/synapse/storage/schema/delta/46/e2e_room_keys.sql b/synapse/storage/schema/delta/46/e2e_room_keys.sql

index 0d2a85fbe6..16499ac34c 100644
--- a/synapse/storage/schema/delta/46/e2e_room_keys.sql
+++ b/synapse/storage/schema/delta/46/e2e_room_keys.sql
@@ -25,16 +25,14 @@ CREATE TABLE e2e_room_keys (
     session_data TEXT NOT NULL
 );
 
-CREATE UNIQUE INDEX e2e_room_keys_user_idx ON e2e_room_keys(user_id);
-CREATE UNIQUE INDEX e2e_room_keys_room_idx ON e2e_room_keys(room_id);
-CREATE UNIQUE INDEX e2e_room_keys_session_idx ON e2e_room_keys(session_id);
+CREATE UNIQUE INDEX e2e_room_keys_idx ON e2e_room_keys(user_id, room_id, session_id);
 
 -- the metadata for each generation of encrypted e2e session backups
-CREATE TABLE e2e_room_key_versions (
+CREATE TABLE e2e_room_keys_versions (
     user_id TEXT NOT NULL,
     version TEXT NOT NULL,
     algorithm TEXT NOT NULL,
     auth_data TEXT NOT NULL
 );
 
-CREATE UNIQUE INDEX e2e_room_key_user_idx ON e2e_room_keys(user_id);
+CREATE UNIQUE INDEX e2e_room_keys_versions_user_idx ON e2e_room_keys_versions(user_id);