Sliding Sync: Support filtering by 'tags' / 'not_tags' in SSS (#17662)

This appears to be enough to make Element Web work (or at least move it
on to the next hurdle)

---------

Co-authored-by: Eric Eastwood <eric.eastwood@beta.gouv.fr>

changelog.d/17662.feature

@@ -0,0 +1 @@
+Add support for the `tags` and `not_tags` filters for simplified sliding sync.

synapse/handlers/sliding_sync/room_lists.py

@@ -1524,6 +1524,8 @@ class SlidingSyncRoomLists:
             A filtered dictionary of room IDs along with membership information in the
             room at the time of `to_token`.
         """
+        user_id = user.to_string()
+
         room_id_to_stripped_state_map: Dict[
             str, Optional[StateMap[StrippedStateEvent]]
         ] = {}
@@ -1657,9 +1659,36 @@ class SlidingSyncRoomLists:
                 # )
                 raise NotImplementedError()
 
+        # Filter by room tags according to the users account data
         if filters.tags is not None or filters.not_tags is not None:
             with start_active_span("filters.tags"):
-                raise NotImplementedError()
+                # Fetch the user tags for their rooms
+                room_tags = await self.store.get_tags_for_user(user_id)
+                room_id_to_tag_name_set: Dict[str, Set[str]] = {
+                    room_id: set(tags.keys()) for room_id, tags in room_tags.items()
+                }
+
+                if filters.tags is not None:
+                    tags_set = set(filters.tags)
+                    filtered_room_id_set = {
+                        room_id
+                        for room_id in filtered_room_id_set
+                        # Remove rooms that don't have one of the tags in the filter
+                        if room_id_to_tag_name_set.get(room_id, set()).intersection(
+                            tags_set
+                        )
+                    }
+
+                if filters.not_tags is not None:
+                    not_tags_set = set(filters.not_tags)
+                    filtered_room_id_set = {
+                        room_id
+                        for room_id in filtered_room_id_set
+                        # Remove rooms if they have any of the tags in the filter
+                        if not room_id_to_tag_name_set.get(room_id, set()).intersection(
+                            not_tags_set
+                        )
+                    }
 
         # Assemble a new sync room map but only with the `filtered_room_id_set`
         return {room_id: sync_room_map[room_id] for room_id in filtered_room_id_set}
@@ -1683,6 +1712,7 @@ class SlidingSyncRoomLists:
             filters: Filters to apply
             to_token: We filter based on the state of the room at this token
             dm_room_ids: Set of room IDs which are DMs
+            room_tags: Mapping of room ID to tags
 
         Returns:
             A filtered dictionary of room IDs along with membership information in the
@@ -1778,9 +1808,36 @@ class SlidingSyncRoomLists:
                 # )
                 raise NotImplementedError()
 
+        # Filter by room tags according to the users account data
         if filters.tags is not None or filters.not_tags is not None:
             with start_active_span("filters.tags"):
-                raise NotImplementedError()
+                # Fetch the user tags for their rooms
+                room_tags = await self.store.get_tags_for_user(user_id)
+                room_id_to_tag_name_set: Dict[str, Set[str]] = {
+                    room_id: set(tags.keys()) for room_id, tags in room_tags.items()
+                }
+
+                if filters.tags is not None:
+                    tags_set = set(filters.tags)
+                    filtered_room_id_set = {
+                        room_id
+                        for room_id in filtered_room_id_set
+                        # Remove rooms that don't have one of the tags in the filter
+                        if room_id_to_tag_name_set.get(room_id, set()).intersection(
+                            tags_set
+                        )
+                    }
+
+                if filters.not_tags is not None:
+                    not_tags_set = set(filters.not_tags)
+                    filtered_room_id_set = {
+                        room_id
+                        for room_id in filtered_room_id_set
+                        # Remove rooms if they have any of the tags in the filter
+                        if not room_id_to_tag_name_set.get(room_id, set()).intersection(
+                            not_tags_set
+                        )
+                    }
 
         # Assemble a new sync room map but only with the `filtered_room_id_set`
         return {room_id: sync_room_map[room_id] for room_id in filtered_room_id_set}

tests/rest/client/sliding_sync/test_lists_filters.py

@@ -25,7 +25,7 @@ from synapse.api.constants import (
 )
 from synapse.api.room_versions import RoomVersions
 from synapse.events import StrippedStateEvent
-from synapse.rest.client import login, room, sync
+from synapse.rest.client import login, room, sync, tags
 from synapse.server import HomeServer
 from synapse.types import JsonDict
 from synapse.util import Clock
@@ -60,6 +60,7 @@ class SlidingSyncFiltersTestCase(SlidingSyncBase):
         login.register_servlets,
         room.register_servlets,
         sync.register_servlets,
+        tags.register_servlets,
     ]
 
     def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
@@ -1148,6 +1149,27 @@ class SlidingSyncFiltersTestCase(SlidingSyncBase):
             exact=True,
         )
 
+        # Just make sure we know what happens when you specify an empty list of room_types
+        # (we should find nothing)
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "room_types": [],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            set(),
+            exact=True,
+        )
+
     def test_filters_not_room_types(self) -> None:
         """
         Test `filters.not_room_types` for different room types
@@ -1283,6 +1305,27 @@ class SlidingSyncFiltersTestCase(SlidingSyncBase):
             exact=True,
         )
 
+        # Just make sure we know what happens when you specify an empty list of not_room_types
+        # (we should find all of the rooms)
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "not_room_types": [],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            {room_id, foo_room_id, space_room_id},
+            exact=True,
+        )
+
     def test_filters_room_types_server_left_room(self) -> None:
         """
         Test that we can apply a `filters.room_types` against a room that everyone has left.
@@ -1679,3 +1722,268 @@ class SlidingSyncFiltersTestCase(SlidingSyncBase):
             {space_room_id},
             exact=True,
         )
+
+    def _add_tag_to_room(
+        self, *, room_id: str, user_id: str, access_token: str, tag_name: str
+    ) -> None:
+        channel = self.make_request(
+            method="PUT",
+            path=f"/user/{user_id}/rooms/{room_id}/tags/{tag_name}",
+            content={},
+            access_token=access_token,
+        )
+        self.assertEqual(channel.code, 200, channel.json_body)
+
+    def test_filters_tags(self) -> None:
+        """
+        Test `filters.tags` for rooms with given tags
+        """
+        user1_id = self.register_user("user1", "pass")
+        user1_tok = self.login(user1_id, "pass")
+
+        # Create a room with no tags
+        self.helper.create_room_as(user1_id, tok=user1_tok)
+
+        # Create some rooms with tags
+        foo_room_id = self.helper.create_room_as(user1_id, tok=user1_tok)
+        bar_room_id = self.helper.create_room_as(user1_id, tok=user1_tok)
+        # Create a room without multiple tags
+        foobar_room_id = self.helper.create_room_as(user1_id, tok=user1_tok)
+
+        # Add the "foo" tag to the foo room
+        self._add_tag_to_room(
+            room_id=foo_room_id,
+            user_id=user1_id,
+            access_token=user1_tok,
+            tag_name="foo",
+        )
+        # Add the "bar" tag to the bar room
+        self._add_tag_to_room(
+            room_id=bar_room_id,
+            user_id=user1_id,
+            access_token=user1_tok,
+            tag_name="bar",
+        )
+        # Add both "foo" and "bar" tags to the foobar room
+        self._add_tag_to_room(
+            room_id=foobar_room_id,
+            user_id=user1_id,
+            access_token=user1_tok,
+            tag_name="foo",
+        )
+        self._add_tag_to_room(
+            room_id=foobar_room_id,
+            user_id=user1_id,
+            access_token=user1_tok,
+            tag_name="bar",
+        )
+
+        # Try finding rooms with the "foo" tag
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "tags": ["foo"],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            {foo_room_id, foobar_room_id},
+            exact=True,
+        )
+
+        # Try finding rooms with either "foo" or "bar" tags
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "tags": ["foo", "bar"],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            {foo_room_id, bar_room_id, foobar_room_id},
+            exact=True,
+        )
+
+        # Try with a random tag we didn't add
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "tags": ["flomp"],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        # No rooms should match
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            set(),
+            exact=True,
+        )
+
+        # Just make sure we know what happens when you specify an empty list of tags
+        # (we should find nothing)
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "tags": [],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            set(),
+            exact=True,
+        )
+
+    def test_filters_not_tags(self) -> None:
+        """
+        Test `filters.not_tags` for excluding rooms with given tags
+        """
+        user1_id = self.register_user("user1", "pass")
+        user1_tok = self.login(user1_id, "pass")
+
+        # Create a room with no tags
+        untagged_room_id = self.helper.create_room_as(user1_id, tok=user1_tok)
+
+        # Create some rooms with tags
+        foo_room_id = self.helper.create_room_as(user1_id, tok=user1_tok)
+        bar_room_id = self.helper.create_room_as(user1_id, tok=user1_tok)
+        # Create a room without multiple tags
+        foobar_room_id = self.helper.create_room_as(user1_id, tok=user1_tok)
+
+        # Add the "foo" tag to the foo room
+        self._add_tag_to_room(
+            room_id=foo_room_id,
+            user_id=user1_id,
+            access_token=user1_tok,
+            tag_name="foo",
+        )
+        # Add the "bar" tag to the bar room
+        self._add_tag_to_room(
+            room_id=bar_room_id,
+            user_id=user1_id,
+            access_token=user1_tok,
+            tag_name="bar",
+        )
+        # Add both "foo" and "bar" tags to the foobar room
+        self._add_tag_to_room(
+            room_id=foobar_room_id,
+            user_id=user1_id,
+            access_token=user1_tok,
+            tag_name="foo",
+        )
+        self._add_tag_to_room(
+            room_id=foobar_room_id,
+            user_id=user1_id,
+            access_token=user1_tok,
+            tag_name="bar",
+        )
+
+        # Try finding rooms without the "foo" tag
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "not_tags": ["foo"],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            {untagged_room_id, bar_room_id},
+            exact=True,
+        )
+
+        # Try finding rooms without either "foo" or "bar" tags
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "not_tags": ["foo", "bar"],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            {untagged_room_id},
+            exact=True,
+        )
+
+        # Test how it behaves when we have both `tags` and `not_tags`.
+        # `not_tags` should win.
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "tags": ["foo"],
+                        "not_tags": ["foo"],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        # Nothing matches because nothing is both tagged with "foo" and not tagged with "foo"
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            set(),
+            exact=True,
+        )
+
+        # Just make sure we know what happens when you specify an empty list of not_tags
+        # (we should find all of the rooms)
+        sync_body = {
+            "lists": {
+                "foo-list": {
+                    "ranges": [[0, 99]],
+                    "required_state": [],
+                    "timeline_limit": 0,
+                    "filters": {
+                        "not_tags": [],
+                    },
+                },
+            }
+        }
+        response_body, _ = self.do_sync(sync_body, tok=user1_tok)
+        self.assertIncludes(
+            set(response_body["lists"]["foo-list"]["ops"][0]["room_ids"]),
+            {untagged_room_id, foo_room_id, bar_room_id, foobar_room_id},
+            exact=True,
+        )