Make label-based filtering support edits

changelog.d/6374.feature

@@ -0,0 +1 @@
+Make the event labelling feature support edits.

synapse/storage/data_stores/main/events.py

@@ -29,7 +29,7 @@
 from twisted.internet import defer
 
 import synapse.metrics
-from synapse.api.constants import EventContentFields, EventTypes
+from synapse.api.constants import EventContentFields, EventTypes, RelationTypes
 from synapse.api.errors import SynapseError
 from synapse.events import EventBase  # noqa: F401
 from synapse.events.snapshot import EventContext  # noqa: F401
@@ -931,13 +931,7 @@ def _update_metadata_tables_txn(
                 self._store_redaction(txn, event)
 
             self._handle_event_relations(txn, event)
-
-            # Store the labels for this event.
-            labels = event.content.get(EventContentFields.LABELS)
-            if labels:
-                self.insert_labels_for_event_txn(
-                    txn, event.event_id, labels, event.room_id, event.depth
-                )
+            self._store_labels_txn(txn, event)
 
         # Insert into the room_memberships table.
         self._store_room_members_txn(
@@ -1929,32 +1923,95 @@ def get_all_updated_current_state_deltas_txn(txn):
             get_all_updated_current_state_deltas_txn,
         )
 
+    def _store_labels_txn(self, txn, event):
+        """Extract labels from an event and store them.
+
+        Args:
+            txn (LoggingTransaction): The transaction to execute.
+            event (EventBase): The event to process.
+        """
+        # Check if the event replaces another one (e.g. it's an edit)
+        relation = event.content.get("m.relates_to", {})
+        replaces = None
+        if relation.get("rel_type") == RelationTypes.REPLACE:
+            replaces = relation.get("event_id")
+
+        # Retrieve the labels on this event.
+        labels = event.content.get(EventContentFields.LABELS)
+
+        # Insert the labels, if any.
+        self.insert_labels_for_event_txn(
+            txn, event.event_id, replaces, labels, event.room_id, event.depth
+        )
+
     def insert_labels_for_event_txn(
-        self, txn, event_id, labels, room_id, topological_ordering
+        self, txn, event_id, replaces, labels, room_id, topological_ordering
     ):
         """Store the mapping between an event's ID and its labels, with one row per
-        (event_id, label) tuple.
+        (event_id, label) tuple, after deleting labels associated with the event it
+        replaces (or a prior replacement of it), if applicable.
+
+        Can be called with labels being None, in which case it will only delete labels
+        associated with the replaced event.
 
         Args:
             txn (LoggingTransaction): The transaction to execute.
             event_id (str): The event's ID.
-            labels (list[str]): A list of text labels.
+            replaces (str|None): The ID of the event this event replaces, if any.
+            labels (list[str]|None): A list of text labels, if any.
             room_id (str): The ID of the room the event was sent to.
             topological_ordering (int): The position of the event in the room's topology.
         """
-        return self._simple_insert_many_txn(
-            txn=txn,
-            table="event_labels",
-            values=[
-                {
-                    "event_id": event_id,
-                    "label": label,
-                    "room_id": room_id,
-                    "topological_ordering": topological_ordering,
-                }
-                for label in labels
-            ],
-        )
+        if replaces:
+            # Check that processing that edit won't overwrite the processing of a more
+            # recent edit of the same event.
+            txn.execute(
+                """
+                SELECT topological_ordering FROM event_labels
+                WHERE event_id = ? OR replaces = ?
+                """,
+                (replaces, replaces),
+            )
+
+            txn.execute("SELECT 1")
+            # We don't care about which row we're using because they'll all have the same
+            # topological ordering, since they're all about the same event.
+            # If the topological ordering (depth) of the current event is lower than the
+            # one of the event which labels are already stored (which means the current
+            # event is less recent), then stop here because we already have a more recent
+            # list of labels.
+            # FIXME: This isn't true if the most recent edit removes every label from the
+            #  event, what should we do in that case?
+            results = tuple(txn)
+            if not results or topological_ordering <= results[0][0]:
+                return
+
+            # If the event is replacing another one (e.g. it's an edit), delete all
+            # labels associated with the event it replaces or past replacements of it.
+            sql = """
+                DELETE FROM event_labels
+                WHERE event_id = ? OR replaces = ?
+            """
+
+            txn.execute(sql, (replaces, replaces))
+
+        # If the event doesn't have any label, don't insert anything.
+        if labels:
+            self._simple_insert_many_txn(
+                txn=txn,
+                table="event_labels",
+                values=[
+                    {
+                        "event_id": event_id,
+                        "label": label,
+                        "room_id": room_id,
+                        "topological_ordering": topological_ordering,
+                        "replaces": replaces,
+                    }
+                    for label in labels
+                    if isinstance(label, str)
+                ],
+            )
 
 
 AllNewEventsResult = namedtuple(

synapse/storage/data_stores/main/events_bg_updates.py

@@ -21,7 +21,7 @@
 
 from twisted.internet import defer
 
-from synapse.api.constants import EventContentFields
+from synapse.api.constants import EventContentFields, RelationTypes
 from synapse.storage._base import make_in_list_sql_clause
 from synapse.storage.background_updates import BackgroundUpdateStore
 
@@ -532,21 +532,24 @@ def _event_store_labels_txn(txn):
             for (event_id, event_json_raw) in results:
                 event_json = json.loads(event_json_raw)
 
-                self._simple_insert_many_txn(
+                # Check if the event replaces another one (e.g. it's an edit)
+                relation = event_json["content"].get("m.relates_to", {})
+                replaces = None
+                if relation.get("rel_type") == RelationTypes.REPLACE:
+                    replaces = relation.get("event_id")
+
+                # Extract the labels from the event's JSON
+                labels = event_json["content"].get(EventContentFields.LABELS, [])
+
+                # Inserts the labels in the database table. This function will take care
+                # of processing the edit (if it's one) correctly.
+                self.insert_labels_for_event_txn(
                     txn=txn,
-                    table="event_labels",
-                    values=[
-                        {
-                            "event_id": event_id,
-                            "label": label,
-                            "room_id": event_json["room_id"],
-                            "topological_ordering": event_json["depth"],
-                        }
-                        for label in event_json["content"].get(
-                            EventContentFields.LABELS, []
-                        )
-                        if isinstance(label, str)
-                    ],
+                    event_id=event_id,
+                    replaces=replaces,
+                    labels=labels,
+                    room_id=event_json["room_id"],
+                    topological_ordering=event_json["depth"],
                 )
 
                 nbrows += 1

synapse/storage/data_stores/main/schema/delta/56/event_labels_edit.sql

@@ -0,0 +1,23 @@
+/* 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.
+ * 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.
+ */
+
+-- Track the ID of the event this event replaces (with a "m.replace" relation). This
+-- exists so we can keep track of the changes in the list of labels associated with a
+-- message.
+ALTER TABLE event_labels ADD COLUMN replaces TEXT;
+
+-- We need this index because we'll be querying labels which are either for a specific
+-- event or for events that replace it.
+CREATE INDEX event_labels_replaces_idx ON event_labels(replaces);