Incorporate review

7 files changed, 21 insertions, 15 deletions

diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index 066ce18704..49c4b85054 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -144,4 +144,4 @@ class EventContentFields(object):
     """Fields found in events' content, regardless of type."""
 
     # Labels for the event, cf https://github.com/matrix-org/matrix-doc/pull/2326
-    Labels = "org.matrix.labels"
+    LABELS = "org.matrix.labels"
diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py
index 30a7ee0a7a..bec13f08d8 100644
--- a/synapse/api/filtering.py
+++ b/synapse/api/filtering.py
@@ -309,7 +309,7 @@ class Filter(object):
             content = event.get("content", {})
             # check if there is a string url field in the content for filtering purposes
             contains_url = isinstance(content.get("url"), text_type)
-            labels = content.get(EventContentFields.Labels, [])
+            labels = content.get(EventContentFields.LABELS, [])
 
         return self.check_fields(room_id, sender, ev_type, labels, contains_url)
 
diff --git a/synapse/storage/data_stores/main/events.py b/synapse/storage/data_stores/main/events.py
index 577e79bcf9..1045c7fa2e 100644
--- a/synapse/storage/data_stores/main/events.py
+++ b/synapse/storage/data_stores/main/events.py
@@ -1491,7 +1491,7 @@ class EventsStore(
             self._handle_event_relations(txn, event)
 
             # Store the labels for this event.
-            labels = event.content.get(EventContentFields.Labels)
+            labels = event.content.get(EventContentFields.LABELS)
             if labels:
                 self.insert_labels_for_event_txn(
                     txn, event.event_id, labels, event.room_id, event.depth
diff --git a/synapse/storage/data_stores/main/schema/delta/56/event_labels.sql b/synapse/storage/data_stores/main/schema/delta/56/event_labels.sql
index 2acd8e1be5..5e29c1da19 100644
--- a/synapse/storage/data_stores/main/schema/delta/56/event_labels.sql
+++ b/synapse/storage/data_stores/main/schema/delta/56/event_labels.sql
@@ -13,6 +13,8 @@
  * limitations under the License.
  */
 
+-- room_id and topoligical_ordering are denormalised from the events table in order to
+-- make the index work.
 CREATE TABLE IF NOT EXISTS event_labels (
     event_id TEXT,
     label TEXT,
@@ -21,4 +23,8 @@ CREATE TABLE IF NOT EXISTS event_labels (
     PRIMARY KEY(event_id, label)
 );
 
+
+-- This index enables an event pagination looking for a particular label to index the
+-- event_labels table first, which is much quicker than scanning the events table and then
+-- filtering by label, if the label is rarely used relative to the size of the room.
 CREATE INDEX event_labels_room_id_label_idx ON event_labels(room_id, label, topological_ordering);
diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py
index 8ec48c4154..2dc5052249 100644
--- a/tests/api/test_filtering.py
+++ b/tests/api/test_filtering.py
@@ -329,7 +329,7 @@ class FilteringTestCase(unittest.TestCase):
             sender="@foo:bar",
             type="m.room.message",
             room_id="!secretbase:unknown",
-            content={EventContentFields.Labels: ["#fun"]},
+            content={EventContentFields.LABELS: ["#fun"]},
         )
 
         self.assertTrue(Filter(definition).check(event))
@@ -338,7 +338,7 @@ class FilteringTestCase(unittest.TestCase):
             sender="@foo:bar",
             type="m.room.message",
             room_id="!secretbase:unknown",
-            content={EventContentFields.Labels: ["#notfun"]},
+            content={EventContentFields.LABELS: ["#notfun"]},
         )
 
         self.assertFalse(Filter(definition).check(event))
@@ -349,7 +349,7 @@ class FilteringTestCase(unittest.TestCase):
             sender="@foo:bar",
             type="m.room.message",
             room_id="!secretbase:unknown",
-            content={EventContentFields.Labels: ["#fun"]},
+            content={EventContentFields.LABELS: ["#fun"]},
         )
 
         self.assertFalse(Filter(definition).check(event))
@@ -358,7 +358,7 @@ class FilteringTestCase(unittest.TestCase):
             sender="@foo:bar",
             type="m.room.message",
             room_id="!secretbase:unknown",
-            content={EventContentFields.Labels: ["#notfun"]},
+            content={EventContentFields.LABELS: ["#notfun"]},
         )
 
         self.assertTrue(Filter(definition).check(event))
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 0dc0faa0e5..5e38fd6ced 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -860,7 +860,7 @@ class RoomMessageListTestCase(RoomBase):
             content={
                 "msgtype": "m.text",
                 "body": "with right label",
-                EventContentFields.Labels: ["#fun"],
+                EventContentFields.LABELS: ["#fun"],
             },
         )
 
@@ -876,7 +876,7 @@ class RoomMessageListTestCase(RoomBase):
             content={
                 "msgtype": "m.text",
                 "body": "with wrong label",
-                EventContentFields.Labels: ["#work"],
+                EventContentFields.LABELS: ["#work"],
             },
         )
 
@@ -886,7 +886,7 @@ class RoomMessageListTestCase(RoomBase):
             content={
                 "msgtype": "m.text",
                 "body": "with two wrong labels",
-                EventContentFields.Labels: ["#work", "#notfun"],
+                EventContentFields.LABELS: ["#work", "#notfun"],
             },
         )
 
@@ -896,7 +896,7 @@ class RoomMessageListTestCase(RoomBase):
             content={
                 "msgtype": "m.text",
                 "body": "with right label",
-                EventContentFields.Labels: ["#fun"],
+                EventContentFields.LABELS: ["#fun"],
             },
         )
 
diff --git a/tests/rest/client/v2_alpha/test_sync.py b/tests/rest/client/v2_alpha/test_sync.py
index c3c6f75ced..3283c0e47b 100644
--- a/tests/rest/client/v2_alpha/test_sync.py
+++ b/tests/rest/client/v2_alpha/test_sync.py
@@ -157,7 +157,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             content={
                 "msgtype": "m.text",
                 "body": "with right label",
-                EventContentFields.Labels: ["#fun"],
+                EventContentFields.LABELS: ["#fun"],
             },
             tok=tok,
         )
@@ -175,7 +175,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             content={
                 "msgtype": "m.text",
                 "body": "with wrong label",
-                EventContentFields.Labels: ["#work"],
+                EventContentFields.LABELS: ["#work"],
             },
             tok=tok,
         )
@@ -186,7 +186,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             content={
                 "msgtype": "m.text",
                 "body": "with two wrong labels",
-                EventContentFields.Labels: ["#work", "#notfun"],
+                EventContentFields.LABELS: ["#work", "#notfun"],
             },
             tok=tok,
         )
@@ -197,7 +197,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             content={
                 "msgtype": "m.text",
                 "body": "with right label",
-                EventContentFields.Labels: ["#fun"],
+                EventContentFields.LABELS: ["#fun"],
             },
             tok=tok,
         )