Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Track notification counts per thread (implement MSC3773) (redo) #13776

Merged
merged 26 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9cb167c
Update filtering to include the thread notifications flag.
clokep Sep 8, 2022
8ac2f32
Ensure that the thread_id column is non-null and then require it to b…
clokep Sep 9, 2022
111fe57
Add infrastructure to pass notifications per thread.
clokep Sep 8, 2022
62aa85b
Calculate thread specific notification counts.
clokep Sep 8, 2022
cb679e2
Clarify comment.
clokep Sep 16, 2022
ba00c5f
Simplify handling of summaries with neither notifications or unread c…
clokep Sep 16, 2022
eb56567
Delete old push summaries.
clokep Sep 16, 2022
e6f97ec
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 16, 2022
4f4711a
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 19, 2022
8b63c5b
Fix postgres compatibility.
clokep Sep 19, 2022
c3783df
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 20, 2022
c4f2d50
Create a constant for "main".
clokep Sep 20, 2022
6927e59
Reduce duplicated code.
clokep Sep 20, 2022
1d05975
Lint
clokep Sep 20, 2022
28b5a1f
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 22, 2022
55d15a3
Threads must already be summarized between the stream orderings that …
clokep Sep 22, 2022
56c21e4
Don't delete empty push summaries.
clokep Sep 22, 2022
241b40c
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 23, 2022
a04258f
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 26, 2022
ddbb644
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 27, 2022
79452e9
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 28, 2022
b0d9008
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 28, 2022
f279a15
Merge remote-tracking branch 'origin/develop' into clokep/threads-not…
clokep Sep 29, 2022
f416be9
Merge branch 'develop' into clokep/threads-notif-2
clokep Oct 4, 2022
0b1b432
Update for changes in develop.
clokep Oct 4, 2022
bd6c80c
Update background index numbers.
clokep Oct 4, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add infrastructure to pass notifications per thread.
  • Loading branch information
clokep committed Sep 15, 2022
commit 111fe5799f5644217410ca79080a0a71867fe255
40 changes: 35 additions & 5 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
from synapse.logging.context import current_context
from synapse.logging.opentracing import SynapseTags, log_kv, set_tag, start_active_span
from synapse.push.clientformat import format_push_rules_for_user
from synapse.storage.databases.main.event_push_actions import NotifCounts
from synapse.storage.databases.main.event_push_actions import RoomNotifCounts
from synapse.storage.roommember import MemberSummary
from synapse.storage.state import StateFilter
from synapse.types import (
Expand Down Expand Up @@ -128,6 +128,7 @@ class JoinedSyncResult:
ephemeral: List[JsonDict]
account_data: List[JsonDict]
unread_notifications: JsonDict
unread_thread_notifications: JsonDict
summary: Optional[JsonDict]
unread_count: int

Expand Down Expand Up @@ -278,6 +279,8 @@ def __init__(self, hs: "HomeServer"):

self.rooms_to_exclude = hs.config.server.rooms_to_exclude_from_sync

self._msc3773_enabled = hs.config.experimental.msc3773_enabled

async def wait_for_sync_for_user(
self,
requester: Requester,
Expand Down Expand Up @@ -1272,7 +1275,7 @@ async def _find_missing_partial_state_memberships(

async def unread_notifs_for_room_id(
self, room_id: str, sync_config: SyncConfig
) -> NotifCounts:
) -> RoomNotifCounts:
with Measure(self.clock, "unread_notifs_for_room_id"):

return await self.store.get_unread_event_push_actions_by_room_for_user(
Expand Down Expand Up @@ -2343,17 +2346,44 @@ async def _generate_room_entry(
ephemeral=ephemeral,
account_data=account_data_events,
unread_notifications=unread_notifications,
unread_thread_notifications={},
summary=summary,
unread_count=0,
)

if room_sync or always_include:
notifs = await self.unread_notifs_for_room_id(room_id, sync_config)

unread_notifications["notification_count"] = notifs.notify_count
unread_notifications["highlight_count"] = notifs.highlight_count
# Notifications for the main timeline.
notify_count = notifs.main_timeline.notify_count
highlight_count = notifs.main_timeline.highlight_count
unread_count = notifs.main_timeline.unread_count

room_sync.unread_count = notifs.unread_count
# Check the sync configuration.
if (
self._msc3773_enabled
and sync_config.filter_collection.unread_thread_notifications()
):
# And add info for each thread.
room_sync.unread_thread_notifications = {
thread_id: {
"notification_count": thread_notifs.notify_count,
"highlight_count": thread_notifs.highlight_count,
}
for thread_id, thread_notifs in notifs.threads.items()
if thread_id is not None
}

else:
# Combine the unread counts for all threads and main timeline.
for thread_notifs in notifs.threads.values():
notify_count += thread_notifs.notify_count
highlight_count += thread_notifs.highlight_count
unread_count += thread_notifs.unread_count

unread_notifications["notification_count"] = notify_count
unread_notifications["highlight_count"] = highlight_count
room_sync.unread_count = unread_count

sync_result_builder.joined.append(room_sync)

Expand Down
9 changes: 7 additions & 2 deletions synapse/push/push_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,20 @@ async def get_room_unread_count(room_id: str) -> None:
await concurrently_execute(get_room_unread_count, joins, 10)

for notifs in room_notifs:
if notifs.notify_count == 0:
# Combine the counts from all the threads.
notify_count = notifs.main_timeline.notify_count + sum(
n.notify_count for n in notifs.threads.values()
)

if notify_count == 0:
continue

if group_by_room:
# return one badge count per conversation
badge += 1
else:
# increment the badge count by the number of unread messages in the room
badge += notifs.notify_count
badge += notify_count
return badge


Expand Down
4 changes: 4 additions & 0 deletions synapse/rest/client/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,10 @@ async def encode_room(
ephemeral_events = room.ephemeral
result["ephemeral"] = {"events": ephemeral_events}
result["unread_notifications"] = room.unread_notifications
if room.unread_thread_notifications:
result[
"org.matrix.msc3773.unread_thread_notifications"
] = room.unread_thread_notifications
result["summary"] = room.summary
if self._msc2654_enabled:
result["org.matrix.msc2654.unread_count"] = room.unread_count
Expand Down
39 changes: 28 additions & 11 deletions synapse/storage/databases/main/event_push_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,29 @@ class UserPushAction(EmailPushAction):
@attr.s(slots=True, auto_attribs=True)
class NotifCounts:
"""
The per-user, per-room count of notifications. Used by sync and push.
The per-user, per-room, per-thread count of notifications. Used by sync and push.
"""

notify_count: int = 0
unread_count: int = 0
highlight_count: int = 0


@attr.s(slots=True, auto_attribs=True)
class RoomNotifCounts:
"""
The per-user, per-room count of notifications. Used by sync and push.
"""

main_timeline: NotifCounts
# Map of thread ID to the notification counts.
threads: Dict[str, NotifCounts]

def __len__(self) -> int:
# To properly account for the amount of space in any caches.
return len(self.threads) + 1


def _serialize_action(
actions: Collection[Union[Mapping, str]], is_highlight: bool
) -> str:
Expand Down Expand Up @@ -331,12 +346,12 @@ def add_thread_id_txn(

return result

@cached(tree=True, max_entries=5000)
@cached(tree=True, max_entries=5000, iterable=True)
async def get_unread_event_push_actions_by_room_for_user(
self,
room_id: str,
user_id: str,
) -> NotifCounts:
) -> RoomNotifCounts:
"""Get the notification count, the highlight count and the unread message count
for a given user in a given room after their latest read receipt.

Expand All @@ -349,8 +364,9 @@ async def get_unread_event_push_actions_by_room_for_user(
user_id: The user to retrieve the counts for.

Returns
A NotifCounts object containing the notification count, the highlight count
and the unread message count.
A RoomNotifCounts object containing the notification count, the
highlight count and the unread message count for both the main timeline
and threads.
"""
return await self.db_pool.runInteraction(
"get_unread_event_push_actions_by_room",
Expand All @@ -364,7 +380,7 @@ def _get_unread_counts_by_receipt_txn(
txn: LoggingTransaction,
room_id: str,
user_id: str,
) -> NotifCounts:
) -> RoomNotifCounts:
# Get the stream ordering of the user's latest receipt in the room.
result = self.get_last_receipt_for_user_txn(
txn,
Expand Down Expand Up @@ -402,7 +418,7 @@ def _get_unread_counts_by_pos_txn(
room_id: str,
user_id: str,
receipt_stream_ordering: int,
) -> NotifCounts:
) -> RoomNotifCounts:
"""Get the number of unread messages for a user/room that have happened
since the given stream ordering.

Expand All @@ -414,9 +430,10 @@ def _get_unread_counts_by_pos_txn(
receipt in the room. If there are no receipts, the stream ordering
of the user's join event.

Returns
A NotifCounts object containing the notification count, the highlight count
and the unread message count.
Returns:
A RoomNotifCounts object containing the notification count, the
highlight count and the unread message count for both the main timeline
and threads.
"""

counts = NotifCounts()
Expand Down Expand Up @@ -481,7 +498,7 @@ def _get_unread_counts_by_pos_txn(
counts.notify_count += notify_count
counts.unread_count += unread_count

return counts
return RoomNotifCounts(counts, {})

def _get_notif_unread_count_for_user_room(
self,
Expand Down
17 changes: 13 additions & 4 deletions tests/replication/slave/storage/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
from synapse.events import FrozenEvent, _EventInternalMetadata, make_event_from_dict
from synapse.handlers.room import RoomEventSource
from synapse.replication.slave.storage.events import SlavedEventStore
from synapse.storage.databases.main.event_push_actions import NotifCounts
from synapse.storage.databases.main.event_push_actions import (
NotifCounts,
RoomNotifCounts,
)
from synapse.storage.roommember import GetRoomsForUserWithStreamOrdering, RoomsForUser
from synapse.types import PersistedEventPosition

Expand Down Expand Up @@ -178,7 +181,9 @@ def test_push_actions_for_user(self, send_receipt: bool):
self.check(
"get_unread_event_push_actions_by_room_for_user",
[ROOM_ID, USER_ID_2],
NotifCounts(highlight_count=0, unread_count=0, notify_count=0),
RoomNotifCounts(
NotifCounts(highlight_count=0, unread_count=0, notify_count=0), {}
),
)

self.persist(
Expand All @@ -191,7 +196,9 @@ def test_push_actions_for_user(self, send_receipt: bool):
self.check(
"get_unread_event_push_actions_by_room_for_user",
[ROOM_ID, USER_ID_2],
NotifCounts(highlight_count=0, unread_count=0, notify_count=1),
RoomNotifCounts(
NotifCounts(highlight_count=0, unread_count=0, notify_count=1), {}
),
)

self.persist(
Expand All @@ -206,7 +213,9 @@ def test_push_actions_for_user(self, send_receipt: bool):
self.check(
"get_unread_event_push_actions_by_room_for_user",
[ROOM_ID, USER_ID_2],
NotifCounts(highlight_count=1, unread_count=0, notify_count=2),
RoomNotifCounts(
NotifCounts(highlight_count=1, unread_count=0, notify_count=2), {}
),
)

def test_get_rooms_for_user_with_stream_ordering(self):
Expand Down
3 changes: 2 additions & 1 deletion tests/storage/test_event_push_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,14 @@ def _assert_counts(noitf_count: int, highlight_count: int) -> None:
)
)
self.assertEqual(
counts,
counts.main_timeline,
NotifCounts(
notify_count=noitf_count,
unread_count=0,
highlight_count=highlight_count,
),
)
self.assertEqual(counts.threads, {})

def _create_event(highlight: bool = False) -> str:
result = self.helper.send_event(
Expand Down