Prevent expired events from being filtered out when retention is disabled

changelog.d/12611.bugfix

@@ -0,0 +1 @@
+Fix a long-standing bug introduced in Synapse 1.7.0 that would prevent events from being sent to clients if there's a retention policy in the room and the support for retention policies is disabled.

synapse/storage/databases/main/room.py

@@ -699,7 +699,9 @@ def delete_ratelimit_txn(txn: LoggingTransaction) -> None:
         await self.db_pool.runInteraction("delete_ratelimit", delete_ratelimit_txn)
 
     @cached()
-    async def get_retention_policy_for_room(self, room_id: str) -> Dict[str, int]:
+    async def get_retention_policy_for_room(
+        self, room_id: str
+    ) -> Dict[str, Optional[int]]:
         """Get the retention policy for a given room.
 
         If no retention policy has been found for this room, returns a policy defined
@@ -713,6 +715,11 @@ async def get_retention_policy_for_room(self, room_id: str) -> Dict[str, int]:
         Returns:
             A dict containing "min_lifetime" and "max_lifetime" for this room.
         """
+        # If the room retention feature is disabled, return a policy with no minimum nor
+        # maximum, in order not to filter out events we should filter out when sending to
+        # the client.
+        if not self.config.retention.retention_enabled:
+            return {"min_lifetime": None, "max_lifetime": None}
 
         def get_retention_policy_for_room_txn(
             txn: LoggingTransaction,

tests/rest/client/test_relations.py

@@ -982,7 +982,7 @@ def assert_annotations(bundled_aggregations: JsonDict) -> None:
                 bundled_aggregations,
             )
 
-        self._test_bundled_aggregations(RelationTypes.ANNOTATION, assert_annotations, 7)
+        self._test_bundled_aggregations(RelationTypes.ANNOTATION, assert_annotations, 6)
 
     def test_reference(self) -> None:
         """
@@ -1000,7 +1000,7 @@ def assert_annotations(bundled_aggregations: JsonDict) -> None:
                 bundled_aggregations,
             )
 
-        self._test_bundled_aggregations(RelationTypes.REFERENCE, assert_annotations, 7)
+        self._test_bundled_aggregations(RelationTypes.REFERENCE, assert_annotations, 6)
 
     def test_thread(self) -> None:
         """
@@ -1029,7 +1029,7 @@ def assert_thread(bundled_aggregations: JsonDict) -> None:
                 bundled_aggregations.get("latest_event"),
             )
 
-        self._test_bundled_aggregations(RelationTypes.THREAD, assert_thread, 9)
+        self._test_bundled_aggregations(RelationTypes.THREAD, assert_thread, 8)
 
     def test_thread_edit_latest_event(self) -> None:
         """Test that editing the latest event in a thread works."""

tests/rest/client/test_retention.py

@@ -254,10 +254,14 @@ class RetentionNoDefaultPolicyTestCase(unittest.HomeserverTestCase):
 
     def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
         config = self.default_config()
-        config["retention"] = {
+
+        retention_config = {
             "enabled": True,
         }
 
+        retention_config.update(config.get("retention", {}))
+        config["retention"] = retention_config
+
         mock_federation_client = Mock(spec=["backfill"])
 
         self.hs = self.setup_test_homeserver(
@@ -295,6 +299,23 @@ def test_state_policy(self) -> None:
 
         self._test_retention(room_id, expected_code_for_first_event=404)
 
+    @unittest.override_config({"retention": {"enabled": False}})
+    def test_visibility_when_disabled(self) -> None:
+        room_id = self.helper.create_room_as(self.user_id, tok=self.token)
+
+        self.helper.send_state(
+            room_id=room_id,
+            event_type=EventTypes.Retention,
+            body={"max_lifetime": one_day_ms},
+            tok=self.token,
+        )
+
+        resp = self.helper.send(room_id=room_id, body="test", tok=self.token)
+
+        self.reactor.advance(one_day_ms * 2 / 1000)
+
+        self.get_event(room_id, resp["event_id"])
+
     def _test_retention(
         self, room_id: str, expected_code_for_first_event: int = 200
     ) -> None: