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
@@ -934,13 +934,7 @@ def _update_metadata_tables_txn(
                 self._store_retention_policy_for_room_txn(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)
 
             if self._ephemeral_messages_enabled:
                 # If there's an expiry timestamp on the event, store it.
@@ -1947,32 +1941,124 @@ 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 database cursor to use.
+            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=txn,
+            event_id=event.event_id,
+            sender=event.sender,
+            replaces=replaces,
+            labels=labels,
+            room_id=event.room_id,
+            topological_ordering=event.depth,
+        )
+
     def insert_labels_for_event_txn(
-        self, txn, event_id, labels, room_id, topological_ordering
+        self, txn, event_id, replaces, sender, 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.
+
+        If the event is an edit and the provided sender doesn't match with the sender
+        of the original event, log and return.
 
         Args:
-            txn (LoggingTransaction): The transaction to execute.
+            txn (LoggingTransaction): The database cursor to use.
             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.
+            sender (str): The sender of the event.
+            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:
+            # Make sure that the sender of the edit match the original event's sender,
+            # otherwise ignore this edit. We do this here because _store_labels_txn and
+            # the background update converge here.
+            original_sender = self.simple_select_one_onecol_txn(
+                txn=txn,
+                table="events",
+                keyvalues={"event_id": replaces},
+                retcol="sender",
+            )
+
+            if sender != original_sender:
+                logger.error(
+                    "The edit's sender of edit %s doesn't match the sender of the "
+                    "original event %s, not updating labels.",
+                    sender,
+                    original_sender,
+                )
+                return
+
+            # 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)
+                ],
+            )
 
     def _insert_event_expiry_txn(self, txn, event_id, expiry_ts):
         """Save the expiry timestamp associated with a given event ID.

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
 
@@ -533,21 +533,25 @@ def _event_store_labels_txn(txn):
                 try:
                     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,
+                        sender=event_json["sender"],
+                        labels=labels,
+                        room_id=event_json["room_id"],
+                        topological_ordering=event_json["depth"],
                     )
                 except Exception as e:
                     logger.warning(

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);