Disable calculating unread counts unless the config flag is enabled. (#13694)

This avoids doing work that will never be used (since the
resulting unread counts will never be sent in a /sync
response).

The negative of doing this is that unread counts will be
incorrect when the feature is initially enabled.
This commit is contained in:
Patrick Cloke 2022-09-01 12:52:03 -04:00 committed by GitHub
parent f48f4dd59e
commit 390b7ce946
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 30 additions and 23 deletions

1
changelog.d/13694.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a bug introduced in Synapse v1.20.0 that would cause the unstable unread counts from [MSC2654](https://github.com/matrix-org/matrix-spec-proposals/pull/2654) to be calculated even if the feature is disabled.

View File

@ -71,6 +71,9 @@ class ExperimentalConfig(Config):
self.msc3720_enabled: bool = experimental.get("msc3720_enabled", False) self.msc3720_enabled: bool = experimental.get("msc3720_enabled", False)
# MSC2654: Unread counts # MSC2654: Unread counts
#
# Note that enabling this will result in an incorrect unread count for
# previously calculated push actions.
self.msc2654_enabled: bool = experimental.get("msc2654_enabled", False) self.msc2654_enabled: bool = experimental.get("msc2654_enabled", False)
# MSC2815 (allow room moderators to view redacted event content) # MSC2815 (allow room moderators to view redacted event content)

View File

@ -262,7 +262,12 @@ class BulkPushRuleEvaluator:
# This can happen due to out of band memberships # This can happen due to out of band memberships
return return
count_as_unread = _should_count_as_unread(event, context) # Disable counting as unread unless the experimental configuration is
# enabled, as it can cause additional (unwanted) rows to be added to the
# event_push_actions table.
count_as_unread = False
if self.hs.config.experimental.msc2654_enabled:
count_as_unread = _should_count_as_unread(event, context)
rules_by_user = await self._get_rules_for_event(event) rules_by_user = await self._get_rules_for_event(event)
actions_by_user: Dict[str, Collection[Union[Mapping, str]]] = {} actions_by_user: Dict[str, Collection[Union[Mapping, str]]] = {}

View File

@ -67,9 +67,7 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
last_event_id: str last_event_id: str
def _assert_counts( def _assert_counts(noitf_count: int, highlight_count: int) -> None:
noitf_count: int, unread_count: int, highlight_count: int
) -> None:
counts = self.get_success( counts = self.get_success(
self.store.db_pool.runInteraction( self.store.db_pool.runInteraction(
"get-unread-counts", "get-unread-counts",
@ -82,7 +80,7 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
counts, counts,
NotifCounts( NotifCounts(
notify_count=noitf_count, notify_count=noitf_count,
unread_count=unread_count, unread_count=0,
highlight_count=highlight_count, highlight_count=highlight_count,
), ),
) )
@ -112,27 +110,27 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
) )
) )
_assert_counts(0, 0, 0) _assert_counts(0, 0)
_create_event() _create_event()
_assert_counts(1, 1, 0) _assert_counts(1, 0)
_rotate() _rotate()
_assert_counts(1, 1, 0) _assert_counts(1, 0)
event_id = _create_event() event_id = _create_event()
_assert_counts(2, 2, 0) _assert_counts(2, 0)
_rotate() _rotate()
_assert_counts(2, 2, 0) _assert_counts(2, 0)
_create_event() _create_event()
_mark_read(event_id) _mark_read(event_id)
_assert_counts(1, 1, 0) _assert_counts(1, 0)
_mark_read(last_event_id) _mark_read(last_event_id)
_assert_counts(0, 0, 0) _assert_counts(0, 0)
_create_event() _create_event()
_rotate() _rotate()
_assert_counts(1, 1, 0) _assert_counts(1, 0)
# Delete old event push actions, this should not affect the (summarised) count. # Delete old event push actions, this should not affect the (summarised) count.
# #
@ -151,35 +149,35 @@ class EventPushActionsStoreTestCase(HomeserverTestCase):
) )
) )
self.assertEqual(result, []) self.assertEqual(result, [])
_assert_counts(1, 1, 0) _assert_counts(1, 0)
_mark_read(last_event_id) _mark_read(last_event_id)
_assert_counts(0, 0, 0) _assert_counts(0, 0)
event_id = _create_event(True) event_id = _create_event(True)
_assert_counts(1, 1, 1) _assert_counts(1, 1)
_rotate() _rotate()
_assert_counts(1, 1, 1) _assert_counts(1, 1)
# Check that adding another notification and rotating after highlight # Check that adding another notification and rotating after highlight
# works. # works.
_create_event() _create_event()
_rotate() _rotate()
_assert_counts(2, 2, 1) _assert_counts(2, 1)
# Check that sending read receipts at different points results in the # Check that sending read receipts at different points results in the
# right counts. # right counts.
_mark_read(event_id) _mark_read(event_id)
_assert_counts(1, 1, 0) _assert_counts(1, 0)
_mark_read(last_event_id) _mark_read(last_event_id)
_assert_counts(0, 0, 0) _assert_counts(0, 0)
_create_event(True) _create_event(True)
_assert_counts(1, 1, 1) _assert_counts(1, 1)
_mark_read(last_event_id) _mark_read(last_event_id)
_assert_counts(0, 0, 0) _assert_counts(0, 0)
_rotate() _rotate()
_assert_counts(0, 0, 0) _assert_counts(0, 0)
def test_find_first_stream_ordering_after_ts(self) -> None: def test_find_first_stream_ordering_after_ts(self) -> None:
def add_event(so: int, ts: int) -> None: def add_event(so: int, ts: int) -> None: