From 47f767269c51773a1cb24e65f958a891e00c7c06 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Tue, 29 Oct 2019 16:56:22 +0000
Subject: [PATCH 01/16] Add database table for keeping track of labels on
 events

---
 .../main/schema/delta/56/event_labels.sql     | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 synapse/storage/data_stores/main/schema/delta/56/event_labels.sql

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
new file mode 100644
index 0000000000..323d797419
--- /dev/null
+++ b/synapse/storage/data_stores/main/schema/delta/56/event_labels.sql
@@ -0,0 +1,20 @@
+/* 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.
+ */
+
+CREATE TABLE IF NOT EXISTS event_labels (
+    event_id TEXT,
+    label TEXT,
+    PRIMARY KEY(event_id, label)
+);
\ No newline at end of file

From fa0dcbc8fa396fa78aabc524728e08fd84b70a0c Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Tue, 29 Oct 2019 18:35:49 +0000
Subject: [PATCH 02/16] Store labels for new events

---
 synapse/api/constants.py                   |  3 +++
 synapse/storage/data_stores/main/events.py | 20 +++++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index 312196675e..999ec02fd9 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -138,3 +138,6 @@ class LimitBlockingTypes(object):
 
     MONTHLY_ACTIVE_USER = "monthly_active_user"
     HS_DISABLED = "hs_disabled"
+
+
+LabelsField = "org.matrix.labels"
diff --git a/synapse/storage/data_stores/main/events.py b/synapse/storage/data_stores/main/events.py
index 03b5111c5d..f80b5f1a3f 100644
--- a/synapse/storage/data_stores/main/events.py
+++ b/synapse/storage/data_stores/main/events.py
@@ -29,7 +29,7 @@ from prometheus_client import Counter, Histogram
 from twisted.internet import defer
 
 import synapse.metrics
-from synapse.api.constants import EventTypes
+from synapse.api.constants import EventTypes, LabelsField
 from synapse.api.errors import SynapseError
 from synapse.events import EventBase  # noqa: F401
 from synapse.events.snapshot import EventContext  # noqa: F401
@@ -1490,6 +1490,11 @@ class EventsStore(
 
             self._handle_event_relations(txn, event)
 
+            # Store the labels for this event.
+            labels = event.content.get(LabelsField)
+            if labels:
+                self.insert_labels_for_event_txn(txn, event.event_id, labels)
+
         # Insert into the room_memberships table.
         self._store_room_members_txn(
             txn,
@@ -2477,6 +2482,19 @@ class EventsStore(
             get_all_updated_current_state_deltas_txn,
         )
 
+    def insert_labels_for_event_txn(self, txn, event_id, labels):
+        return self._simple_insert_many_txn(
+            txn=txn,
+            table="event_labels",
+            values=[
+                {
+                    "event_id": event_id,
+                    "label": label,
+                }
+                for label in labels
+            ],
+        )
+
 
 AllNewEventsResult = namedtuple(
     "AllNewEventsResult",

From acd16ad86a8f61ef261fa82960ee3634864db9ed Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 30 Oct 2019 15:56:33 +0000
Subject: [PATCH 03/16] Implement filtering

---
 synapse/api/filtering.py                   | 13 +++++++++++--
 synapse/storage/data_stores/main/stream.py |  9 +++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py
index 9f06556bd2..a27029c678 100644
--- a/synapse/api/filtering.py
+++ b/synapse/api/filtering.py
@@ -20,6 +20,7 @@ from jsonschema import FormatChecker
 
 from twisted.internet import defer
 
+from synapse.api.constants import LabelsField
 from synapse.api.errors import SynapseError
 from synapse.storage.presence import UserPresenceState
 from synapse.types import RoomID, UserID
@@ -66,6 +67,8 @@ ROOM_EVENT_FILTER_SCHEMA = {
         "contains_url": {"type": "boolean"},
         "lazy_load_members": {"type": "boolean"},
         "include_redundant_members": {"type": "boolean"},
+        "org.matrix.labels": {"type": "array", "items": {"type": "string"}},
+        "org.matrix.not_labels": {"type": "array", "items": {"type": "string"}},
     },
 }
 
@@ -259,6 +262,9 @@ class Filter(object):
 
         self.contains_url = self.filter_json.get("contains_url", None)
 
+        self.labels = self.filter_json.get("org.matrix.labels", None)
+        self.not_labels = self.filter_json.get("org.matrix.not_labels", [])
+
     def filters_all_types(self):
         return "*" in self.not_types
 
@@ -282,6 +288,7 @@ class Filter(object):
             room_id = None
             ev_type = "m.presence"
             contains_url = False
+            labels = []
         else:
             sender = event.get("sender", None)
             if not sender:
@@ -300,10 +307,11 @@ 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(LabelsField)
 
-        return self.check_fields(room_id, sender, ev_type, contains_url)
+        return self.check_fields(room_id, sender, ev_type, labels, contains_url)
 
-    def check_fields(self, room_id, sender, event_type, contains_url):
+    def check_fields(self, room_id, sender, event_type, labels, contains_url):
         """Checks whether the filter matches the given event fields.
 
         Returns:
@@ -313,6 +321,7 @@ class Filter(object):
             "rooms": lambda v: room_id == v,
             "senders": lambda v: sender == v,
             "types": lambda v: _matches_wildcard(event_type, v),
+            "labels": lambda v: v in labels,
         }
 
         for name, match_func in literal_keys.items():
diff --git a/synapse/storage/data_stores/main/stream.py b/synapse/storage/data_stores/main/stream.py
index 263999dfca..907d7f20ba 100644
--- a/synapse/storage/data_stores/main/stream.py
+++ b/synapse/storage/data_stores/main/stream.py
@@ -229,6 +229,14 @@ def filter_to_clause(event_filter):
         clauses.append("contains_url = ?")
         args.append(event_filter.contains_url)
 
+    # We're only applying the "labels" filter on the database query, because applying the
+    # "not_labels" filter via a SQL query is non-trivial. Instead, we let
+    # event_filter.check_fields apply it, which is not as efficient but makes the
+    # implementation simpler.
+    if event_filter.labels:
+        clauses.append("(%s)" % " OR ".join("label = ?" for _ in event_filter.labels))
+        args.extend(event_filter.labels)
+
     return " AND ".join(clauses), args
 
 
@@ -866,6 +874,7 @@ class StreamWorkerStore(EventsWorkerStore, SQLBaseStore):
         sql = (
             "SELECT event_id, topological_ordering, stream_ordering"
             " FROM events"
+            " LEFT JOIN event_labels USING (event_id)"
             " WHERE outlier = ? AND room_id = ? AND %(bounds)s"
             " ORDER BY topological_ordering %(order)s,"
             " stream_ordering %(order)s LIMIT ?"

From 233b14ebe1d96bd7cc3fcca09663794490186ad2 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 30 Oct 2019 15:58:05 +0000
Subject: [PATCH 04/16] Add index on label

---
 .../storage/data_stores/main/schema/delta/56/event_labels.sql | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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 323d797419..9550b0adaa 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
@@ -17,4 +17,6 @@ CREATE TABLE IF NOT EXISTS event_labels (
     event_id TEXT,
     label TEXT,
     PRIMARY KEY(event_id, label)
-);
\ No newline at end of file
+);
+
+CREATE INDEX event_labels_label_idx ON event_labels(label);

From e7943f660add8b602ea5225060bd0d74e6440017 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 30 Oct 2019 16:15:04 +0000
Subject: [PATCH 05/16] Add unit tests

---
 synapse/api/filtering.py    |  2 +-
 tests/api/test_filtering.py | 51 +++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py
index a27029c678..bd91b9f018 100644
--- a/synapse/api/filtering.py
+++ b/synapse/api/filtering.py
@@ -307,7 +307,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(LabelsField)
+            labels = content.get(LabelsField, [])
 
         return self.check_fields(room_id, sender, ev_type, labels, contains_url)
 
diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py
index 6ba623de13..66b3c828db 100644
--- a/tests/api/test_filtering.py
+++ b/tests/api/test_filtering.py
@@ -19,6 +19,7 @@ import jsonschema
 
 from twisted.internet import defer
 
+from synapse.api.constants import LabelsField
 from synapse.api.errors import SynapseError
 from synapse.api.filtering import Filter
 from synapse.events import FrozenEvent
@@ -95,6 +96,8 @@ class FilteringTestCase(unittest.TestCase):
                         "types": ["m.room.message"],
                         "not_rooms": ["!726s6s6q:example.com"],
                         "not_senders": ["@spam:example.com"],
+                        "org.matrix.labels": ["#fun"],
+                        "org.matrix.not_labels": ["#work"],
                     },
                     "ephemeral": {
                         "types": ["m.receipt", "m.typing"],
@@ -320,6 +323,54 @@ class FilteringTestCase(unittest.TestCase):
         )
         self.assertFalse(Filter(definition).check(event))
 
+    def test_filter_labels(self):
+        definition = {"org.matrix.labels": ["#fun"]}
+        event = MockEvent(
+            sender="@foo:bar",
+            type="m.room.message",
+            room_id="!secretbase:unknown",
+            content={
+                LabelsField: ["#fun"]
+            },
+        )
+
+        self.assertTrue(Filter(definition).check(event))
+
+        event = MockEvent(
+            sender="@foo:bar",
+            type="m.room.message",
+            room_id="!secretbase:unknown",
+            content={
+                LabelsField: ["#notfun"]
+            },
+        )
+
+        self.assertFalse(Filter(definition).check(event))
+
+    def test_filter_not_labels(self):
+        definition = {"org.matrix.not_labels": ["#fun"]}
+        event = MockEvent(
+            sender="@foo:bar",
+            type="m.room.message",
+            room_id="!secretbase:unknown",
+            content={
+                LabelsField: ["#fun"]
+            },
+        )
+
+        self.assertFalse(Filter(definition).check(event))
+
+        event = MockEvent(
+            sender="@foo:bar",
+            type="m.room.message",
+            room_id="!secretbase:unknown",
+            content={
+                LabelsField: ["#notfun"]
+            },
+        )
+
+        self.assertTrue(Filter(definition).check(event))
+
     @defer.inlineCallbacks
     def test_filter_presence_match(self):
         user_filter_json = {"presence": {"types": ["m.*"]}}

From 395683add1d569c0fdfd83d279551a3ba926f4d5 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 30 Oct 2019 16:47:37 +0000
Subject: [PATCH 06/16] Add integration tests for sync

---
 tests/rest/client/v1/utils.py           |  15 +++-
 tests/rest/client/v2_alpha/test_sync.py | 112 +++++++++++++++++++++++-
 2 files changed, 122 insertions(+), 5 deletions(-)

diff --git a/tests/rest/client/v1/utils.py b/tests/rest/client/v1/utils.py
index cdded88b7f..8ea0cb05ea 100644
--- a/tests/rest/client/v1/utils.py
+++ b/tests/rest/client/v1/utils.py
@@ -106,13 +106,22 @@ class RestHelper(object):
         self.auth_user_id = temp_id
 
     def send(self, room_id, body=None, txn_id=None, tok=None, expect_code=200):
-        if txn_id is None:
-            txn_id = "m%s" % (str(time.time()))
         if body is None:
             body = "body_text_here"
 
-        path = "/_matrix/client/r0/rooms/%s/send/m.room.message/%s" % (room_id, txn_id)
         content = {"msgtype": "m.text", "body": body}
+
+        return self.send_event(
+            room_id, "m.room.message", content, txn_id, tok, expect_code
+        )
+
+    def send_event(
+        self, room_id, type, content={}, txn_id=None, tok=None, expect_code=200
+    ):
+        if txn_id is None:
+            txn_id = "m%s" % (str(time.time()))
+
+        path = "/_matrix/client/r0/rooms/%s/send/%s/%s" % (room_id, type, txn_id)
         if tok:
             path = path + "?access_token=%s" % tok
 
diff --git a/tests/rest/client/v2_alpha/test_sync.py b/tests/rest/client/v2_alpha/test_sync.py
index 71895094bd..0263be010f 100644
--- a/tests/rest/client/v2_alpha/test_sync.py
+++ b/tests/rest/client/v2_alpha/test_sync.py
@@ -12,9 +12,10 @@
 # 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.
-
+import json
 from mock import Mock
 
+from synapse.api.constants import EventTypes, LabelsField
 import synapse.rest.admin
 from synapse.rest.client.v1 import login, room
 from synapse.rest.client.v2_alpha import sync
@@ -26,7 +27,12 @@ from tests.server import TimedOutException
 class FilterTestCase(unittest.HomeserverTestCase):
 
     user_id = "@apple:test"
-    servlets = [sync.register_servlets]
+    servlets = [
+        synapse.rest.admin.register_servlets_for_client_rest_resource,
+        room.register_servlets,
+        login.register_servlets,
+        sync.register_servlets,
+    ]
 
     def make_homeserver(self, reactor, clock):
 
@@ -70,6 +76,108 @@ class FilterTestCase(unittest.HomeserverTestCase):
         )
 
 
+class SyncFilterTestCase(unittest.HomeserverTestCase):
+    servlets = [
+        synapse.rest.admin.register_servlets_for_client_rest_resource,
+        room.register_servlets,
+        login.register_servlets,
+        sync.register_servlets,
+    ]
+
+    def test_sync_filter_labels(self):
+        sync_filter = json.dumps(
+            {
+                "room": {
+                    "timeline": {
+                        "types": [EventTypes.Message],
+                        "org.matrix.labels": ["#fun"],
+                    }
+                }
+            }
+        )
+
+        events = self._test_sync_filter_labels(sync_filter)
+
+        self.assertEqual(len(events), 2, events)
+        self.assertEqual(events[0]["content"]["body"], "with label", events[0])
+        self.assertEqual(events[1]["content"]["body"], "with label", events[1])
+
+    def test_sync_filter_not_labels(self):
+        sync_filter = json.dumps(
+            {
+                "room": {
+                    "timeline": {
+                        "types": [EventTypes.Message],
+                        "org.matrix.not_labels": ["#fun"],
+                    }
+                }
+            }
+        )
+
+        events = self._test_sync_filter_labels(sync_filter)
+
+        self.assertEqual(len(events), 2, events)
+        self.assertEqual(events[0]["content"]["body"], "without label", events[0])
+        self.assertEqual(events[1]["content"]["body"], "with wrong label", events[1])
+
+    def _test_sync_filter_labels(self, sync_filter):
+        user_id = self.register_user("kermit", "test")
+        tok = self.login("kermit", "test")
+
+        room_id = self.helper.create_room_as(user_id, tok=tok)
+
+        self.helper.send_event(
+            room_id=room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "with label",
+                LabelsField: ["#fun"],
+            },
+            tok=tok,
+        )
+
+        self.helper.send_event(
+            room_id=room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "without label",
+            },
+            tok=tok,
+        )
+
+        self.helper.send_event(
+            room_id=room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "with wrong label",
+                LabelsField: ["#work"],
+            },
+            tok=tok,
+        )
+
+        self.helper.send_event(
+            room_id=room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "with label",
+                LabelsField: ["#fun"],
+            },
+            tok=tok,
+        )
+
+        request, channel = self.make_request(
+            "GET", "/sync?filter=%s" % sync_filter, access_token=tok
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200, channel.result)
+
+        return channel.json_body["rooms"]["join"][room_id]["timeline"]["events"]
+
+
 class SyncTypingTests(unittest.HomeserverTestCase):
 
     servlets = [

From fe51d6cacf6e1a2da5fc3589d0bc4118342b33dd Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 30 Oct 2019 17:28:41 +0000
Subject: [PATCH 07/16] Add more integration testing

---
 synapse/storage/data_stores/main/stream.py |  2 +-
 tests/rest/client/v2_alpha/test_sync.py    | 45 +++++++++++++++++++---
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/synapse/storage/data_stores/main/stream.py b/synapse/storage/data_stores/main/stream.py
index 907d7f20ba..cfa34ba1e7 100644
--- a/synapse/storage/data_stores/main/stream.py
+++ b/synapse/storage/data_stores/main/stream.py
@@ -872,7 +872,7 @@ class StreamWorkerStore(EventsWorkerStore, SQLBaseStore):
         args.append(int(limit))
 
         sql = (
-            "SELECT event_id, topological_ordering, stream_ordering"
+            "SELECT DISTINCT event_id, topological_ordering, stream_ordering"
             " FROM events"
             " LEFT JOIN event_labels USING (event_id)"
             " WHERE outlier = ? AND room_id = ? AND %(bounds)s"
diff --git a/tests/rest/client/v2_alpha/test_sync.py b/tests/rest/client/v2_alpha/test_sync.py
index 0263be010f..a1aa7d87bd 100644
--- a/tests/rest/client/v2_alpha/test_sync.py
+++ b/tests/rest/client/v2_alpha/test_sync.py
@@ -85,6 +85,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
     ]
 
     def test_sync_filter_labels(self):
+        """Test that we can filter by a label."""
         sync_filter = json.dumps(
             {
                 "room": {
@@ -98,11 +99,12 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
 
         events = self._test_sync_filter_labels(sync_filter)
 
-        self.assertEqual(len(events), 2, events)
-        self.assertEqual(events[0]["content"]["body"], "with label", events[0])
-        self.assertEqual(events[1]["content"]["body"], "with label", events[1])
+        self.assertEqual(len(events), 2, [event["content"] for event in events])
+        self.assertEqual(events[0]["content"]["body"], "with right label", events[0])
+        self.assertEqual(events[1]["content"]["body"], "with right label", events[1])
 
     def test_sync_filter_not_labels(self):
+        """Test that we can filter by the absence of a label."""
         sync_filter = json.dumps(
             {
                 "room": {
@@ -116,9 +118,29 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
 
         events = self._test_sync_filter_labels(sync_filter)
 
-        self.assertEqual(len(events), 2, events)
+        self.assertEqual(len(events), 3, [event["content"] for event in events])
         self.assertEqual(events[0]["content"]["body"], "without label", events[0])
         self.assertEqual(events[1]["content"]["body"], "with wrong label", events[1])
+        self.assertEqual(events[2]["content"]["body"], "with two wrong labels", events[2])
+
+    def test_sync_filter_labels_not_labels(self):
+        """Test that we can filter by both a label and the absence of another label."""
+        sync_filter = json.dumps(
+            {
+                "room": {
+                    "timeline": {
+                        "types": [EventTypes.Message],
+                        "org.matrix.labels": ["#work"],
+                        "org.matrix.not_labels": ["#notfun"],
+                    }
+                }
+            }
+        )
+
+        events = self._test_sync_filter_labels(sync_filter)
+
+        self.assertEqual(len(events), 1, [event["content"] for event in events])
+        self.assertEqual(events[0]["content"]["body"], "with wrong label", events[0])
 
     def _test_sync_filter_labels(self, sync_filter):
         user_id = self.register_user("kermit", "test")
@@ -131,7 +153,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             type=EventTypes.Message,
             content={
                 "msgtype": "m.text",
-                "body": "with label",
+                "body": "with right label",
                 LabelsField: ["#fun"],
             },
             tok=tok,
@@ -163,7 +185,18 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             type=EventTypes.Message,
             content={
                 "msgtype": "m.text",
-                "body": "with label",
+                "body": "with two wrong labels",
+                LabelsField: ["#work", "#notfun"],
+            },
+            tok=tok,
+        )
+
+        self.helper.send_event(
+            room_id=room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "with right label",
                 LabelsField: ["#fun"],
             },
             tok=tok,

From d8c9109aeee58950f0fd4d9865836b82aa7aafb6 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 30 Oct 2019 17:48:22 +0000
Subject: [PATCH 08/16] Add integration tests for /messages

---
 tests/rest/client/v1/test_rooms.py | 102 ++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 1 deletion(-)

diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 2f2ca74611..ba2008497e 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -24,7 +24,7 @@ from six.moves.urllib import parse as urlparse
 from twisted.internet import defer
 
 import synapse.rest.admin
-from synapse.api.constants import Membership
+from synapse.api.constants import EventTypes, LabelsField, Membership
 from synapse.rest.client.v1 import login, profile, room
 
 from tests import unittest
@@ -811,6 +811,106 @@ class RoomMessageListTestCase(RoomBase):
         self.assertTrue("chunk" in channel.json_body)
         self.assertTrue("end" in channel.json_body)
 
+    def test_filter_labels(self):
+        """Test that we can filter by a label."""
+        message_filter = json.dumps({
+            "types": [EventTypes.Message],
+            "org.matrix.labels": ["#fun"],
+        })
+
+        events = self._test_filter_labels(message_filter)
+
+        self.assertEqual(len(events), 2, [event["content"] for event in events])
+        self.assertEqual(events[0]["content"]["body"], "with right label", events[0])
+        self.assertEqual(events[1]["content"]["body"], "with right label", events[1])
+
+    def test_filter_not_labels(self):
+        """Test that we can filter by the absence of a label."""
+        message_filter = json.dumps({
+            "types": [EventTypes.Message],
+            "org.matrix.not_labels": ["#fun"],
+        })
+
+        events = self._test_filter_labels(message_filter)
+
+        self.assertEqual(len(events), 3, [event["content"] for event in events])
+        self.assertEqual(events[0]["content"]["body"], "without label", events[0])
+        self.assertEqual(events[1]["content"]["body"], "with wrong label", events[1])
+        self.assertEqual(events[2]["content"]["body"], "with two wrong labels", events[2])
+
+    def test_filter_labels_not_labels(self):
+        """Test that we can filter by both a label and the absence of another label."""
+        sync_filter = json.dumps({
+            "types": [EventTypes.Message],
+            "org.matrix.labels": ["#work"],
+            "org.matrix.not_labels": ["#notfun"],
+        })
+
+        events = self._test_filter_labels(sync_filter)
+
+        self.assertEqual(len(events), 1, [event["content"] for event in events])
+        self.assertEqual(events[0]["content"]["body"], "with wrong label", events[0])
+
+    def _test_filter_labels(self, message_filter):
+        self.helper.send_event(
+            room_id=self.room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "with right label",
+                LabelsField: ["#fun"],
+            }
+        )
+
+        self.helper.send_event(
+            room_id=self.room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "without label",
+            }
+        )
+
+        self.helper.send_event(
+            room_id=self.room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "with wrong label",
+                LabelsField: ["#work"],
+            }
+        )
+
+        self.helper.send_event(
+            room_id=self.room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "with two wrong labels",
+                LabelsField: ["#work", "#notfun"],
+            }
+        )
+
+        self.helper.send_event(
+            room_id=self.room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "with right label",
+                LabelsField: ["#fun"],
+            }
+        )
+
+        token = "s0_0_0_0_0_0_0_0_0"
+        request, channel = self.make_request(
+            "GET", "/rooms/%s/messages?access_token=x&from=%s&filter=%s" % (
+                self.room_id, token, message_filter
+            )
+        )
+        self.render(request)
+
+        return channel.json_body["chunk"]
+
 
 class RoomSearchTestCase(unittest.HomeserverTestCase):
     servlets = [

From 62588eae4a1a0a894f66709a38403a153d78687d Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 30 Oct 2019 17:54:40 +0000
Subject: [PATCH 09/16] Changelog

---
 changelog.d/6301.feature | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/6301.feature

diff --git a/changelog.d/6301.feature b/changelog.d/6301.feature
new file mode 100644
index 0000000000..b7ff3fad3b
--- /dev/null
+++ b/changelog.d/6301.feature
@@ -0,0 +1 @@
+Implement label-based filtering.

From dcc069a2e2540862c233a20037e3e59591a42431 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 30 Oct 2019 18:01:56 +0000
Subject: [PATCH 10/16] Lint

---
 synapse/storage/data_stores/main/events.py |  8 +---
 tests/api/test_filtering.py                | 16 ++-----
 tests/rest/client/v1/test_rooms.py         | 49 +++++++++++-----------
 tests/rest/client/v2_alpha/test_sync.py    | 12 +++---
 4 files changed, 35 insertions(+), 50 deletions(-)

diff --git a/synapse/storage/data_stores/main/events.py b/synapse/storage/data_stores/main/events.py
index f80b5f1a3f..2b900f1ce1 100644
--- a/synapse/storage/data_stores/main/events.py
+++ b/synapse/storage/data_stores/main/events.py
@@ -2486,13 +2486,7 @@ class EventsStore(
         return self._simple_insert_many_txn(
             txn=txn,
             table="event_labels",
-            values=[
-                {
-                    "event_id": event_id,
-                    "label": label,
-                }
-                for label in labels
-            ],
+            values=[{"event_id": event_id, "label": label} for label in labels],
         )
 
 
diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py
index 66b3c828db..e004ab1ee5 100644
--- a/tests/api/test_filtering.py
+++ b/tests/api/test_filtering.py
@@ -329,9 +329,7 @@ class FilteringTestCase(unittest.TestCase):
             sender="@foo:bar",
             type="m.room.message",
             room_id="!secretbase:unknown",
-            content={
-                LabelsField: ["#fun"]
-            },
+            content={LabelsField: ["#fun"]},
         )
 
         self.assertTrue(Filter(definition).check(event))
@@ -340,9 +338,7 @@ class FilteringTestCase(unittest.TestCase):
             sender="@foo:bar",
             type="m.room.message",
             room_id="!secretbase:unknown",
-            content={
-                LabelsField: ["#notfun"]
-            },
+            content={LabelsField: ["#notfun"]},
         )
 
         self.assertFalse(Filter(definition).check(event))
@@ -353,9 +349,7 @@ class FilteringTestCase(unittest.TestCase):
             sender="@foo:bar",
             type="m.room.message",
             room_id="!secretbase:unknown",
-            content={
-                LabelsField: ["#fun"]
-            },
+            content={LabelsField: ["#fun"]},
         )
 
         self.assertFalse(Filter(definition).check(event))
@@ -364,9 +358,7 @@ class FilteringTestCase(unittest.TestCase):
             sender="@foo:bar",
             type="m.room.message",
             room_id="!secretbase:unknown",
-            content={
-                LabelsField: ["#notfun"]
-            },
+            content={LabelsField: ["#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 ba2008497e..188f47bd7d 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -813,10 +813,9 @@ class RoomMessageListTestCase(RoomBase):
 
     def test_filter_labels(self):
         """Test that we can filter by a label."""
-        message_filter = json.dumps({
-            "types": [EventTypes.Message],
-            "org.matrix.labels": ["#fun"],
-        })
+        message_filter = json.dumps(
+            {"types": [EventTypes.Message], "org.matrix.labels": ["#fun"]}
+        )
 
         events = self._test_filter_labels(message_filter)
 
@@ -826,25 +825,28 @@ class RoomMessageListTestCase(RoomBase):
 
     def test_filter_not_labels(self):
         """Test that we can filter by the absence of a label."""
-        message_filter = json.dumps({
-            "types": [EventTypes.Message],
-            "org.matrix.not_labels": ["#fun"],
-        })
+        message_filter = json.dumps(
+            {"types": [EventTypes.Message], "org.matrix.not_labels": ["#fun"]}
+        )
 
         events = self._test_filter_labels(message_filter)
 
         self.assertEqual(len(events), 3, [event["content"] for event in events])
         self.assertEqual(events[0]["content"]["body"], "without label", events[0])
         self.assertEqual(events[1]["content"]["body"], "with wrong label", events[1])
-        self.assertEqual(events[2]["content"]["body"], "with two wrong labels", events[2])
+        self.assertEqual(
+            events[2]["content"]["body"], "with two wrong labels", events[2]
+        )
 
     def test_filter_labels_not_labels(self):
         """Test that we can filter by both a label and the absence of another label."""
-        sync_filter = json.dumps({
-            "types": [EventTypes.Message],
-            "org.matrix.labels": ["#work"],
-            "org.matrix.not_labels": ["#notfun"],
-        })
+        sync_filter = json.dumps(
+            {
+                "types": [EventTypes.Message],
+                "org.matrix.labels": ["#work"],
+                "org.matrix.not_labels": ["#notfun"],
+            }
+        )
 
         events = self._test_filter_labels(sync_filter)
 
@@ -859,16 +861,13 @@ class RoomMessageListTestCase(RoomBase):
                 "msgtype": "m.text",
                 "body": "with right label",
                 LabelsField: ["#fun"],
-            }
+            },
         )
 
         self.helper.send_event(
             room_id=self.room_id,
             type=EventTypes.Message,
-            content={
-                "msgtype": "m.text",
-                "body": "without label",
-            }
+            content={"msgtype": "m.text", "body": "without label"},
         )
 
         self.helper.send_event(
@@ -878,7 +877,7 @@ class RoomMessageListTestCase(RoomBase):
                 "msgtype": "m.text",
                 "body": "with wrong label",
                 LabelsField: ["#work"],
-            }
+            },
         )
 
         self.helper.send_event(
@@ -888,7 +887,7 @@ class RoomMessageListTestCase(RoomBase):
                 "msgtype": "m.text",
                 "body": "with two wrong labels",
                 LabelsField: ["#work", "#notfun"],
-            }
+            },
         )
 
         self.helper.send_event(
@@ -898,14 +897,14 @@ class RoomMessageListTestCase(RoomBase):
                 "msgtype": "m.text",
                 "body": "with right label",
                 LabelsField: ["#fun"],
-            }
+            },
         )
 
         token = "s0_0_0_0_0_0_0_0_0"
         request, channel = self.make_request(
-            "GET", "/rooms/%s/messages?access_token=x&from=%s&filter=%s" % (
-                self.room_id, token, message_filter
-            )
+            "GET",
+            "/rooms/%s/messages?access_token=x&from=%s&filter=%s"
+            % (self.room_id, token, message_filter),
         )
         self.render(request)
 
diff --git a/tests/rest/client/v2_alpha/test_sync.py b/tests/rest/client/v2_alpha/test_sync.py
index a1aa7d87bd..c5c199d412 100644
--- a/tests/rest/client/v2_alpha/test_sync.py
+++ b/tests/rest/client/v2_alpha/test_sync.py
@@ -13,10 +13,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import json
+
 from mock import Mock
 
-from synapse.api.constants import EventTypes, LabelsField
 import synapse.rest.admin
+from synapse.api.constants import EventTypes, LabelsField
 from synapse.rest.client.v1 import login, room
 from synapse.rest.client.v2_alpha import sync
 
@@ -121,7 +122,9 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
         self.assertEqual(len(events), 3, [event["content"] for event in events])
         self.assertEqual(events[0]["content"]["body"], "without label", events[0])
         self.assertEqual(events[1]["content"]["body"], "with wrong label", events[1])
-        self.assertEqual(events[2]["content"]["body"], "with two wrong labels", events[2])
+        self.assertEqual(
+            events[2]["content"]["body"], "with two wrong labels", events[2]
+        )
 
     def test_sync_filter_labels_not_labels(self):
         """Test that we can filter by both a label and the absence of another label."""
@@ -162,10 +165,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
         self.helper.send_event(
             room_id=room_id,
             type=EventTypes.Message,
-            content={
-                "msgtype": "m.text",
-                "body": "without label",
-            },
+            content={"msgtype": "m.text", "body": "without label"},
             tok=tok,
         )
 

From c6dbca2422bf77ccbf0b52d9245d28c258dac4f3 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Fri, 1 Nov 2019 10:30:51 +0000
Subject: [PATCH 11/16] Incorporate review

---
 changelog.d/6301.feature                   |  2 +-
 synapse/api/constants.py                   |  5 ++++-
 synapse/api/filtering.py                   |  6 ++++--
 synapse/storage/data_stores/main/events.py | 12 ++++++++++--
 tests/api/test_filtering.py                | 10 +++++-----
 tests/rest/client/v1/test_rooms.py         | 10 +++++-----
 tests/rest/client/v2_alpha/test_sync.py    | 10 +++++-----
 7 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/changelog.d/6301.feature b/changelog.d/6301.feature
index b7ff3fad3b..78a187a1dc 100644
--- a/changelog.d/6301.feature
+++ b/changelog.d/6301.feature
@@ -1 +1 @@
-Implement label-based filtering.
+Implement label-based filtering on `/sync` and `/messages` ([MSC2326](https://github.com/matrix-org/matrix-doc/pull/2326)).
diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index 999ec02fd9..cf4ce5f5a2 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -140,4 +140,7 @@ class LimitBlockingTypes(object):
     HS_DISABLED = "hs_disabled"
 
 
-LabelsField = "org.matrix.labels"
+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"
diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py
index bd91b9f018..30a7ee0a7a 100644
--- a/synapse/api/filtering.py
+++ b/synapse/api/filtering.py
@@ -20,7 +20,7 @@ from jsonschema import FormatChecker
 
 from twisted.internet import defer
 
-from synapse.api.constants import LabelsField
+from synapse.api.constants import EventContentFields
 from synapse.api.errors import SynapseError
 from synapse.storage.presence import UserPresenceState
 from synapse.types import RoomID, UserID
@@ -67,6 +67,8 @@ ROOM_EVENT_FILTER_SCHEMA = {
         "contains_url": {"type": "boolean"},
         "lazy_load_members": {"type": "boolean"},
         "include_redundant_members": {"type": "boolean"},
+        # Include or exclude events with the provided labels.
+        # cf https://github.com/matrix-org/matrix-doc/pull/2326
         "org.matrix.labels": {"type": "array", "items": {"type": "string"}},
         "org.matrix.not_labels": {"type": "array", "items": {"type": "string"}},
     },
@@ -307,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(LabelsField, [])
+            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 2b900f1ce1..42ffa9066a 100644
--- a/synapse/storage/data_stores/main/events.py
+++ b/synapse/storage/data_stores/main/events.py
@@ -29,7 +29,7 @@ from prometheus_client import Counter, Histogram
 from twisted.internet import defer
 
 import synapse.metrics
-from synapse.api.constants import EventTypes, LabelsField
+from synapse.api.constants import EventTypes, EventContentFields
 from synapse.api.errors import SynapseError
 from synapse.events import EventBase  # noqa: F401
 from synapse.events.snapshot import EventContext  # noqa: F401
@@ -1491,7 +1491,7 @@ class EventsStore(
             self._handle_event_relations(txn, event)
 
             # Store the labels for this event.
-            labels = event.content.get(LabelsField)
+            labels = event.content.get(EventContentFields.Labels)
             if labels:
                 self.insert_labels_for_event_txn(txn, event.event_id, labels)
 
@@ -2483,6 +2483,14 @@ class EventsStore(
         )
 
     def insert_labels_for_event_txn(self, txn, event_id, labels):
+        """Store the mapping between an event's ID and its labels, with one row per
+        (event_id, label) tuple.
+
+        Args:
+            txn (LoggingTransaction): The transaction to execute.
+            event_id (str): The event's ID.
+            labels (list[str]): A list of text labels.
+        """
         return self._simple_insert_many_txn(
             txn=txn,
             table="event_labels",
diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py
index e004ab1ee5..8ec48c4154 100644
--- a/tests/api/test_filtering.py
+++ b/tests/api/test_filtering.py
@@ -19,7 +19,7 @@ import jsonschema
 
 from twisted.internet import defer
 
-from synapse.api.constants import LabelsField
+from synapse.api.constants import EventContentFields
 from synapse.api.errors import SynapseError
 from synapse.api.filtering import Filter
 from synapse.events import FrozenEvent
@@ -329,7 +329,7 @@ class FilteringTestCase(unittest.TestCase):
             sender="@foo:bar",
             type="m.room.message",
             room_id="!secretbase:unknown",
-            content={LabelsField: ["#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={LabelsField: ["#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={LabelsField: ["#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={LabelsField: ["#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 188f47bd7d..0dc0faa0e5 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -24,7 +24,7 @@ from six.moves.urllib import parse as urlparse
 from twisted.internet import defer
 
 import synapse.rest.admin
-from synapse.api.constants import EventTypes, LabelsField, Membership
+from synapse.api.constants import EventContentFields, EventTypes, Membership
 from synapse.rest.client.v1 import login, profile, room
 
 from tests import unittest
@@ -860,7 +860,7 @@ class RoomMessageListTestCase(RoomBase):
             content={
                 "msgtype": "m.text",
                 "body": "with right label",
-                LabelsField: ["#fun"],
+                EventContentFields.Labels: ["#fun"],
             },
         )
 
@@ -876,7 +876,7 @@ class RoomMessageListTestCase(RoomBase):
             content={
                 "msgtype": "m.text",
                 "body": "with wrong label",
-                LabelsField: ["#work"],
+                EventContentFields.Labels: ["#work"],
             },
         )
 
@@ -886,7 +886,7 @@ class RoomMessageListTestCase(RoomBase):
             content={
                 "msgtype": "m.text",
                 "body": "with two wrong labels",
-                LabelsField: ["#work", "#notfun"],
+                EventContentFields.Labels: ["#work", "#notfun"],
             },
         )
 
@@ -896,7 +896,7 @@ class RoomMessageListTestCase(RoomBase):
             content={
                 "msgtype": "m.text",
                 "body": "with right label",
-                LabelsField: ["#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 c5c199d412..c3c6f75ced 100644
--- a/tests/rest/client/v2_alpha/test_sync.py
+++ b/tests/rest/client/v2_alpha/test_sync.py
@@ -17,7 +17,7 @@ import json
 from mock import Mock
 
 import synapse.rest.admin
-from synapse.api.constants import EventTypes, LabelsField
+from synapse.api.constants import EventContentFields, EventTypes
 from synapse.rest.client.v1 import login, room
 from synapse.rest.client.v2_alpha import sync
 
@@ -157,7 +157,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             content={
                 "msgtype": "m.text",
                 "body": "with right label",
-                LabelsField: ["#fun"],
+                EventContentFields.Labels: ["#fun"],
             },
             tok=tok,
         )
@@ -175,7 +175,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             content={
                 "msgtype": "m.text",
                 "body": "with wrong label",
-                LabelsField: ["#work"],
+                EventContentFields.Labels: ["#work"],
             },
             tok=tok,
         )
@@ -186,7 +186,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             content={
                 "msgtype": "m.text",
                 "body": "with two wrong labels",
-                LabelsField: ["#work", "#notfun"],
+                EventContentFields.Labels: ["#work", "#notfun"],
             },
             tok=tok,
         )
@@ -197,7 +197,7 @@ class SyncFilterTestCase(unittest.HomeserverTestCase):
             content={
                 "msgtype": "m.text",
                 "body": "with right label",
-                LabelsField: ["#fun"],
+                EventContentFields.Labels: ["#fun"],
             },
             tok=tok,
         )

From 57cdb046e48c6837fc9b41ade9b06d3ff68ec91b Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Fri, 1 Nov 2019 10:39:14 +0000
Subject: [PATCH 12/16] Lint

---
 synapse/api/constants.py                   | 1 +
 synapse/storage/data_stores/main/events.py | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index cf4ce5f5a2..066ce18704 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -142,5 +142,6 @@ class LimitBlockingTypes(object):
 
 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"
diff --git a/synapse/storage/data_stores/main/events.py b/synapse/storage/data_stores/main/events.py
index 42ffa9066a..0480161056 100644
--- a/synapse/storage/data_stores/main/events.py
+++ b/synapse/storage/data_stores/main/events.py
@@ -29,7 +29,7 @@ from prometheus_client import Counter, Histogram
 from twisted.internet import defer
 
 import synapse.metrics
-from synapse.api.constants import EventTypes, EventContentFields
+from synapse.api.constants import EventContentFields, EventTypes
 from synapse.api.errors import SynapseError
 from synapse.events import EventBase  # noqa: F401
 from synapse.events.snapshot import EventContext  # noqa: F401

From e3689ac6f75681c75f9931f79f248269811704c5 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Fri, 1 Nov 2019 10:41:23 +0000
Subject: [PATCH 13/16] Add unstable feature flag

---
 synapse/rest/client/versions.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py
index 1044ae7b4e..bb30ce3f34 100644
--- a/synapse/rest/client/versions.py
+++ b/synapse/rest/client/versions.py
@@ -65,6 +65,9 @@ class VersionsRestServlet(RestServlet):
                     "m.require_identity_server": False,
                     # as per MSC2290
                     "m.separate_add_and_bind": True,
+                    # Implements support for label-based filtering as described in
+                    # MSC2326.
+                    "org.matrix.label_based_filtering": True,
                 },
             },
         )

From a2c63c619ad1116cad07564c055aa6af8943d899 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Fri, 1 Nov 2019 11:47:28 +0000
Subject: [PATCH 14/16] Add more data to the event_labels table and fix the
 indexes

---
 synapse/storage/data_stores/main/events.py    | 20 ++++++++++++++++---
 .../main/schema/delta/56/event_labels.sql     |  4 +++-
 synapse/storage/data_stores/main/stream.py    |  2 +-
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/synapse/storage/data_stores/main/events.py b/synapse/storage/data_stores/main/events.py
index 0480161056..577e79bcf9 100644
--- a/synapse/storage/data_stores/main/events.py
+++ b/synapse/storage/data_stores/main/events.py
@@ -1493,7 +1493,9 @@ class EventsStore(
             # 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)
+                self.insert_labels_for_event_txn(
+                    txn, event.event_id, labels, event.room_id, event.depth
+                )
 
         # Insert into the room_memberships table.
         self._store_room_members_txn(
@@ -2482,7 +2484,9 @@ class EventsStore(
             get_all_updated_current_state_deltas_txn,
         )
 
-    def insert_labels_for_event_txn(self, txn, event_id, labels):
+    def insert_labels_for_event_txn(
+        self, txn, event_id, labels, room_id, topological_ordering
+    ):
         """Store the mapping between an event's ID and its labels, with one row per
         (event_id, label) tuple.
 
@@ -2490,11 +2494,21 @@ class EventsStore(
             txn (LoggingTransaction): The transaction to execute.
             event_id (str): The event's ID.
             labels (list[str]): A list of text labels.
+            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} for label in labels],
+            values=[
+                {
+                    "event_id": event_id,
+                    "label": label,
+                    "room_id": room_id,
+                    "topological_ordering": topological_ordering,
+                }
+                for label in labels
+            ],
         )
 
 
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 9550b0adaa..765124d131 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
@@ -16,7 +16,9 @@
 CREATE TABLE IF NOT EXISTS event_labels (
     event_id TEXT,
     label TEXT,
+    room_id TEXT NOT NULL,
+    topological_ordering bigint NOT NULL,
     PRIMARY KEY(event_id, label)
 );
 
-CREATE INDEX event_labels_label_idx ON event_labels(label);
+CREATE INDEX event_labels_room_id_label_idx ON event_labels(room_id, label, topological_ordering);
diff --git a/synapse/storage/data_stores/main/stream.py b/synapse/storage/data_stores/main/stream.py
index cfa34ba1e7..616ef91d4e 100644
--- a/synapse/storage/data_stores/main/stream.py
+++ b/synapse/storage/data_stores/main/stream.py
@@ -874,7 +874,7 @@ class StreamWorkerStore(EventsWorkerStore, SQLBaseStore):
         sql = (
             "SELECT DISTINCT event_id, topological_ordering, stream_ordering"
             " FROM events"
-            " LEFT JOIN event_labels USING (event_id)"
+            " LEFT JOIN event_labels USING (event_id, room_id, topological_ordering)"
             " WHERE outlier = ? AND room_id = ? AND %(bounds)s"
             " ORDER BY topological_ordering %(order)s,"
             " stream_ordering %(order)s LIMIT ?"

From 559844565515334d1168746f96fcc0db4fce3f6e Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Fri, 1 Nov 2019 16:18:34 +0000
Subject: [PATCH 15/16] Update
 synapse/storage/data_stores/main/schema/delta/56/event_labels.sql

Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 .../storage/data_stores/main/schema/delta/56/event_labels.sql   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 765124d131..2acd8e1be5 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
@@ -17,7 +17,7 @@ CREATE TABLE IF NOT EXISTS event_labels (
     event_id TEXT,
     label TEXT,
     room_id TEXT NOT NULL,
-    topological_ordering bigint NOT NULL,
+    topological_ordering BIGINT NOT NULL,
     PRIMARY KEY(event_id, label)
 );
 

From 988d8d6507a0e8b34f2c352c77b5742197762190 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Fri, 1 Nov 2019 16:22:44 +0000
Subject: [PATCH 16/16] Incorporate review

---
 synapse/api/constants.py                                  | 2 +-
 synapse/api/filtering.py                                  | 2 +-
 synapse/storage/data_stores/main/events.py                | 2 +-
 .../data_stores/main/schema/delta/56/event_labels.sql     | 6 ++++++
 tests/api/test_filtering.py                               | 8 ++++----
 tests/rest/client/v1/test_rooms.py                        | 8 ++++----
 tests/rest/client/v2_alpha/test_sync.py                   | 8 ++++----
 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,
         )