Matrix-Mirrors/synapse-product
1
0
Fork 0
mirror of https://git.anonymousland.org/anonymousland/synapse-product.git synced 2025-01-06 18:57:55 -05:00
Code Issues Packages Projects Releases Wiki Activity

Back out MSC2625 implementation (#7761)

Browse Source
This commit is contained in:
Brendan Abolivier 2020-07-01 11:08:25 +01:00 committed by GitHub
parent 71cccf1593
commit 74d3e177f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 53 additions and 189 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/7673.feature
View File

@ -1 +0,0 @@
Add a per-room counter for unread messages in responses to `/sync` requests. Implements [MSC2625](https://github.com/matrix-org/matrix-doc/pull/2625).

1
changelog.d/7716.feature
View File

@ -1 +0,0 @@
Add a per-room counter for unread messages in responses to `/sync` requests. Implements [MSC2625](https://github.com/matrix-org/matrix-doc/pull/2625).

1
changelog.d/7761.feature Normal file
View File

@ -0,0 +1 @@
Add unread messages count to sync responses.

3
synapse/handlers/sync.py
View File

@ -1893,9 +1893,6 @@ class SyncHandler(object):
if notifs is not None: if notifs is not None:
unread_notifications["notification_count"] = notifs["notify_count"] unread_notifications["notification_count"] = notifs["notify_count"]
unread_notifications["highlight_count"] = notifs["highlight_count"] unread_notifications["highlight_count"] = notifs["highlight_count"]
unread_notifications["org.matrix.msc2625.unread_count"] = notifs[
"unread_count"
]
sync_result_builder.joined.append(room_sync) sync_result_builder.joined.append(room_sync)

7
synapse/push/bulk_push_rule_evaluator.py
View File

@ -189,11 +189,8 @@ class BulkPushRuleEvaluator(object):
) )
if matches: if matches:
actions = [x for x in rule["actions"] if x != "dont_notify"] actions = [x for x in rule["actions"] if x != "dont_notify"]
if ( if actions and "notify" in actions:
"notify" in actions # Push rules say we should notify the user of this event
or "org.matrix.msc2625.mark_unread" in actions
):
# Push rules say we should act on this event.
actions_by_user[uid] = actions actions_by_user[uid] = actions
break break

5
synapse/push/push_tools.py
View File

@ -39,10 +39,7 @@ def get_badge_count(store, user_id):
) )
# return one badge count per conversation, as count per # return one badge count per conversation, as count per
# message is so noisy as to be almost useless # message is so noisy as to be almost useless
# We're populating this badge using the unread_count (instead of the badge += 1 if notifs["notify_count"] else 0
# notify_count) as this badge is the number of missed messages, not the
# number of missed notifications.
badge += 1 if notifs.get("unread_count") else 0
return badge return badge

4
synapse/rest/client/v1/push_rule.py
View File

@ -1,5 +1,5 @@
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
# Copyright 2014-2020 The Matrix.org Foundation C.I.C. # Copyright 2014-2016 OpenMarket Ltd
# #
# Licensed under the Apache License, Version 2.0 (the "License"); # Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License. # you may not use this file except in compliance with the License.
@ -267,7 +267,7 @@ def _check_actions(actions):
raise InvalidRuleException("No actions found") raise InvalidRuleException("No actions found")
for a in actions: for a in actions:
if a in ["notify", "dont_notify", "coalesce", "org.matrix.msc2625.mark_unread"]: if a in ["notify", "dont_notify", "coalesce"]:
pass pass
elif isinstance(a, dict) and "set_tweak" in a: elif isinstance(a, dict) and "set_tweak" in a:
pass pass

133
synapse/storage/data_stores/main/event_push_actions.py
View File

@ -1,5 +1,6 @@
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
# Copyright 2015-2020 The Matrix.org Foundation C.I.C. # Copyright 2015 OpenMarket Ltd
# Copyright 2018 New Vector Ltd
# #
# Licensed under the Apache License, Version 2.0 (the "License"); # Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License. # you may not use this file except in compliance with the License.
@ -14,9 +15,7 @@
# limitations under the License. # limitations under the License.
import logging import logging
from typing import Dict, Tuple
import attr
from canonicaljson import json from canonicaljson import json
from twisted.internet import defer from twisted.internet import defer
@ -37,16 +36,6 @@ DEFAULT_HIGHLIGHT_ACTION = [
] ]
@attr.s
class EventPushSummary:
"""Summary of pending event push actions for a given user in a given room."""
unread_count = attr.ib(type=int)
stream_ordering = attr.ib(type=int)
old_user_id = attr.ib(type=str)
notif_count = attr.ib(type=int)
def _serialize_action(actions, is_highlight): def _serialize_action(actions, is_highlight):
"""Custom serializer for actions. This allows us to "compress" common actions. """Custom serializer for actions. This allows us to "compress" common actions.
@ -123,7 +112,7 @@ class EventPushActionsWorkerStore(SQLBaseStore):
txn.execute(sql, (room_id, last_read_event_id)) txn.execute(sql, (room_id, last_read_event_id))
results = txn.fetchall() results = txn.fetchall()
if len(results) == 0: if len(results) == 0:
return {"notify_count": 0, "highlight_count": 0, "unread_count": 0} return {"notify_count": 0, "highlight_count": 0}
stream_ordering = results[0][0] stream_ordering = results[0][0]
@ -133,42 +122,25 @@ class EventPushActionsWorkerStore(SQLBaseStore):
def _get_unread_counts_by_pos_txn(self, txn, room_id, user_id, stream_ordering): def _get_unread_counts_by_pos_txn(self, txn, room_id, user_id, stream_ordering):
# First get number of actions, grouped on whether the action notifies. # First get number of notifications.
# We don't need to put a notif=1 clause as all rows always have
# notif=1
sql = ( sql = (
"SELECT count(*), notif" "SELECT count(*)"
" FROM event_push_actions ea" " FROM event_push_actions ea"
" WHERE" " WHERE"
" user_id = ?" " user_id = ?"
" AND room_id = ?" " AND room_id = ?"
" AND stream_ordering > ?" " AND stream_ordering > ?"
" GROUP BY notif"
) )
txn.execute(sql, (user_id, room_id, stream_ordering))
rows = txn.fetchall()
# We should get a maximum number of two rows: one for notif = 0, which is the txn.execute(sql, (user_id, room_id, stream_ordering))
# number of actions that contribute to the unread_count but not to the row = txn.fetchone()
# notify_count, and one for notif = 1, which is the number of actions that notify_count = row[0] if row else 0
# contribute to both counters. If one or both rows don't appear, then the
# value for the matching counter should be 0.
unread_count = 0
notify_count = 0
for row in rows:
# We always increment unread_count because actions that notify also
# contribute to it.
unread_count += row[0]
if row[1] == 1:
notify_count = row[0]
elif row[1] != 0:
logger.warning(
"Unexpected value %d for column 'notif' in table"
" 'event_push_actions'",
row[1],
)
txn.execute( txn.execute(
""" """
SELECT notif_count, unread_count FROM event_push_summary SELECT notif_count FROM event_push_summary
WHERE room_id = ? AND user_id = ? AND stream_ordering > ? WHERE room_id = ? AND user_id = ? AND stream_ordering > ?
""", """,
(room_id, user_id, stream_ordering), (room_id, user_id, stream_ordering),
@ -176,7 +148,6 @@ class EventPushActionsWorkerStore(SQLBaseStore):
rows = txn.fetchall() rows = txn.fetchall()
if rows: if rows:
notify_count += rows[0][0] notify_count += rows[0][0]
unread_count += rows[0][1]
# Now get the number of highlights # Now get the number of highlights
sql = ( sql = (
@ -193,11 +164,7 @@ class EventPushActionsWorkerStore(SQLBaseStore):
row = txn.fetchone() row = txn.fetchone()
highlight_count = row[0] if row else 0 highlight_count = row[0] if row else 0
return { return {"notify_count": notify_count, "highlight_count": highlight_count}
"unread_count": unread_count,
"notify_count": notify_count,
"highlight_count": highlight_count,
}
@defer.inlineCallbacks @defer.inlineCallbacks
def get_push_action_users_in_range(self, min_stream_ordering, max_stream_ordering): def get_push_action_users_in_range(self, min_stream_ordering, max_stream_ordering):
@ -255,7 +222,6 @@ class EventPushActionsWorkerStore(SQLBaseStore):
" AND ep.user_id = ?" " AND ep.user_id = ?"
" AND ep.stream_ordering > ?" " AND ep.stream_ordering > ?"
" AND ep.stream_ordering <= ?" " AND ep.stream_ordering <= ?"
" AND ep.notif = 1"
" ORDER BY ep.stream_ordering ASC LIMIT ?" " ORDER BY ep.stream_ordering ASC LIMIT ?"
) )
args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit] args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit]
@ -284,7 +250,6 @@ class EventPushActionsWorkerStore(SQLBaseStore):
" AND ep.user_id = ?" " AND ep.user_id = ?"
" AND ep.stream_ordering > ?" " AND ep.stream_ordering > ?"
" AND ep.stream_ordering <= ?" " AND ep.stream_ordering <= ?"
" AND ep.notif = 1"
" ORDER BY ep.stream_ordering ASC LIMIT ?" " ORDER BY ep.stream_ordering ASC LIMIT ?"
) )
args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit] args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit]
@ -357,7 +322,6 @@ class EventPushActionsWorkerStore(SQLBaseStore):
" AND ep.user_id = ?" " AND ep.user_id = ?"
" AND ep.stream_ordering > ?" " AND ep.stream_ordering > ?"
" AND ep.stream_ordering <= ?" " AND ep.stream_ordering <= ?"
" AND ep.notif = 1"
" ORDER BY ep.stream_ordering DESC LIMIT ?" " ORDER BY ep.stream_ordering DESC LIMIT ?"
) )
args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit] args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit]
@ -386,7 +350,6 @@ class EventPushActionsWorkerStore(SQLBaseStore):
" AND ep.user_id = ?" " AND ep.user_id = ?"
" AND ep.stream_ordering > ?" " AND ep.stream_ordering > ?"
" AND ep.stream_ordering <= ?" " AND ep.stream_ordering <= ?"
" AND ep.notif = 1"
" ORDER BY ep.stream_ordering DESC LIMIT ?" " ORDER BY ep.stream_ordering DESC LIMIT ?"
) )
args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit] args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit]
@ -436,7 +399,7 @@ class EventPushActionsWorkerStore(SQLBaseStore):
def _get_if_maybe_push_in_range_for_user_txn(txn): def _get_if_maybe_push_in_range_for_user_txn(txn):
sql = """ sql = """
SELECT 1 FROM event_push_actions SELECT 1 FROM event_push_actions
WHERE user_id = ? AND stream_ordering > ? AND notif = 1 WHERE user_id = ? AND stream_ordering > ?
LIMIT 1 LIMIT 1
""" """
@ -465,15 +428,14 @@ class EventPushActionsWorkerStore(SQLBaseStore):
return return
# This is a helper function for generating the necessary tuple that # This is a helper function for generating the necessary tuple that
# can be used to insert into the `event_push_actions_staging` table. # can be used to inert into the `event_push_actions_staging` table.
def _gen_entry(user_id, actions): def _gen_entry(user_id, actions):
is_highlight = 1 if _action_has_highlight(actions) else 0 is_highlight = 1 if _action_has_highlight(actions) else 0
notif = 0 if "org.matrix.msc2625.mark_unread" in actions else 1
return ( return (
event_id, # event_id column event_id, # event_id column
user_id, # user_id column user_id, # user_id column
_serialize_action(actions, is_highlight), # actions column _serialize_action(actions, is_highlight), # actions column
notif, # notif column 1, # notif column
is_highlight, # highlight column is_highlight, # highlight column
) )
@ -855,51 +817,24 @@ class EventPushActionsStore(EventPushActionsWorkerStore):
# Calculate the new counts that should be upserted into event_push_summary # Calculate the new counts that should be upserted into event_push_summary
sql = """ sql = """
SELECT user_id, room_id, SELECT user_id, room_id,
coalesce(old.%s, 0) + upd.cnt, coalesce(old.notif_count, 0) + upd.notif_count,
upd.stream_ordering, upd.stream_ordering,
old.user_id old.user_id
FROM ( FROM (
SELECT user_id, room_id, count(*) as cnt, SELECT user_id, room_id, count(*) as notif_count,
max(stream_ordering) as stream_ordering max(stream_ordering) as stream_ordering
FROM event_push_actions FROM event_push_actions
WHERE ? <= stream_ordering AND stream_ordering < ? WHERE ? <= stream_ordering AND stream_ordering < ?
AND highlight = 0 AND highlight = 0
%s
GROUP BY user_id, room_id GROUP BY user_id, room_id
) AS upd ) AS upd
LEFT JOIN event_push_summary AS old USING (user_id, room_id) LEFT JOIN event_push_summary AS old USING (user_id, room_id)
""" """
# First get the count of unread messages. txn.execute(sql, (old_rotate_stream_ordering, rotate_to_stream_ordering))
txn.execute( rows = txn.fetchall()
sql % ("unread_count", ""),
(old_rotate_stream_ordering, rotate_to_stream_ordering),
)
# We need to merge both lists into a single object because we might not have the logger.info("Rotating notifications, handling %d rows", len(rows))
# same amount of rows in each of them. In this case we use a dict indexed on the
# user ID and room ID to make it easier to populate.
summaries = {} # type: Dict[Tuple[str, str], EventPushSummary]
for row in txn:
summaries[(row[0], row[1])] = EventPushSummary(
unread_count=row[2],
stream_ordering=row[3],
old_user_id=row[4],
notif_count=0,
)
# Then get the count of notifications.
txn.execute(
sql % ("notif_count", "AND notif = 1"),
(old_rotate_stream_ordering, rotate_to_stream_ordering),
)
# notif_rows is populated based on a subset of the query used to populate
# unread_rows, so we can be sure that there will be no KeyError here.
for row in txn:
summaries[(row[0], row[1])].notif_count = row[2]
logger.info("Rotating notifications, handling %d rows", len(summaries))
# If the `old.user_id` above is NULL then we know there isn't already an # If the `old.user_id` above is NULL then we know there isn't already an
# entry in the table, so we simply insert it. Otherwise we update the # entry in the table, so we simply insert it. Otherwise we update the
@ -909,34 +844,22 @@ class EventPushActionsStore(EventPushActionsWorkerStore):
table="event_push_summary", table="event_push_summary",
values=[ values=[
{ {
"user_id": user_id, "user_id": row[0],
"room_id": room_id, "room_id": row[1],
"notif_count": summary.notif_count, "notif_count": row[2],
"unread_count": summary.unread_count, "stream_ordering": row[3],
"stream_ordering": summary.stream_ordering,
} }
for ((user_id, room_id), summary) in summaries.items() for row in rows
if summary.old_user_id is None if row[4] is None
], ],
) )
txn.executemany( txn.executemany(
""" """
UPDATE event_push_summary UPDATE event_push_summary SET notif_count = ?, stream_ordering = ?
SET notif_count = ?, unread_count = ?, stream_ordering = ?
WHERE user_id = ? AND room_id = ? WHERE user_id = ? AND room_id = ?
""", """,
( ((row[2], row[3], row[0], row[1]) for row in rows if row[4] is not None),
(
summary.notif_count,
summary.unread_count,
summary.stream_ordering,
user_id,
room_id,
)
for ((user_id, room_id), summary) in summaries.items()
if summary.old_user_id is not None
),
) )
txn.execute( txn.execute(

23
synapse/storage/data_stores/main/schema/delta/58/07push_summary_unread_count.sql
View File

@ -1,23 +0,0 @@
/* Copyright 2020 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.
*/
-- Store the number of unread messages, i.e. messages that triggered either a notify
-- action or a mark_unread one.
ALTER TABLE event_push_summary ADD COLUMN unread_count BIGINT NOT NULL DEFAULT 0;
-- Pre-populate the new column with the count of pending notifications.
-- We expect event_push_summary to be relatively small, so we can do this update
-- synchronously without impacting Synapse's startup time too much.
UPDATE event_push_summary SET unread_count = notif_count;

19
tests/replication/slave/storage/test_events.py
View File

@ -160,7 +160,7 @@ class SlavedEventStoreTestCase(BaseSlavedStoreTestCase):
self.check( self.check(
"get_unread_event_push_actions_by_room_for_user", "get_unread_event_push_actions_by_room_for_user",
[ROOM_ID, USER_ID_2, event1.event_id], [ROOM_ID, USER_ID_2, event1.event_id],
{"highlight_count": 0, "notify_count": 0, "unread_count": 0}, {"highlight_count": 0, "notify_count": 0},
) )
self.persist( self.persist(
@ -173,7 +173,7 @@ class SlavedEventStoreTestCase(BaseSlavedStoreTestCase):
self.check( self.check(
"get_unread_event_push_actions_by_room_for_user", "get_unread_event_push_actions_by_room_for_user",
[ROOM_ID, USER_ID_2, event1.event_id], [ROOM_ID, USER_ID_2, event1.event_id],
{"highlight_count": 0, "notify_count": 1, "unread_count": 1}, {"highlight_count": 0, "notify_count": 1},
) )
self.persist( self.persist(
@ -188,20 +188,7 @@ class SlavedEventStoreTestCase(BaseSlavedStoreTestCase):
self.check( self.check(
"get_unread_event_push_actions_by_room_for_user", "get_unread_event_push_actions_by_room_for_user",
[ROOM_ID, USER_ID_2, event1.event_id], [ROOM_ID, USER_ID_2, event1.event_id],
{"highlight_count": 1, "notify_count": 2, "unread_count": 2}, {"highlight_count": 1, "notify_count": 2},
)
self.persist(
type="m.room.message",
msgtype="m.text",
body="world",
push_actions=[(USER_ID_2, ["org.matrix.msc2625.mark_unread"])],
)
self.replicate()
self.check(
"get_unread_event_push_actions_by_room_for_user",
[ROOM_ID, USER_ID_2, event1.event_id],
{"highlight_count": 1, "notify_count": 2, "unread_count": 3},
) )
def test_get_rooms_for_user_with_stream_ordering(self): def test_get_rooms_for_user_with_stream_ordering(self):

45
tests/storage/test_event_push_actions.py
View File

@ -22,10 +22,6 @@ import tests.utils
USER_ID = "@user:example.com" USER_ID = "@user:example.com"
MARK_UNREAD = [
"org.matrix.msc2625.mark_unread",
{"set_tweak": "highlight", "value": False},
]
PlAIN_NOTIF = ["notify", {"set_tweak": "highlight", "value": False}] PlAIN_NOTIF = ["notify", {"set_tweak": "highlight", "value": False}]
HIGHLIGHT = [ HIGHLIGHT = [
"notify", "notify",
@ -59,17 +55,13 @@ class EventPushActionsStoreTestCase(tests.unittest.TestCase):
user_id = "@user1235:example.com" user_id = "@user1235:example.com"
@defer.inlineCallbacks @defer.inlineCallbacks
def _assert_counts(unread_count, notif_count, highlight_count): def _assert_counts(noitf_count, highlight_count):
counts = yield self.store.db.runInteraction( counts = yield self.store.db.runInteraction(
"", self.store._get_unread_counts_by_pos_txn, room_id, user_id, 0 "", self.store._get_unread_counts_by_pos_txn, room_id, user_id, 0
) )
self.assertEquals( self.assertEquals(
counts, counts,
{ {"notify_count": noitf_count, "highlight_count": highlight_count},
"unread_count": unread_count,
"notify_count": notif_count,
"highlight_count": highlight_count,
},
) )
@defer.inlineCallbacks @defer.inlineCallbacks
@ -104,23 +96,23 @@ class EventPushActionsStoreTestCase(tests.unittest.TestCase):
stream, stream,
) )
yield _assert_counts(0, 0, 0) yield _assert_counts(0, 0)
yield _inject_actions(1, PlAIN_NOTIF) yield _inject_actions(1, PlAIN_NOTIF)
yield _assert_counts(1, 1, 0) yield _assert_counts(1, 0)
yield _rotate(2) yield _rotate(2)
yield _assert_counts(1, 1, 0) yield _assert_counts(1, 0)
yield _inject_actions(3, PlAIN_NOTIF) yield _inject_actions(3, PlAIN_NOTIF)
yield _assert_counts(2, 2, 0) yield _assert_counts(2, 0)
yield _rotate(4) yield _rotate(4)
yield _assert_counts(2, 2, 0) yield _assert_counts(2, 0)
yield _inject_actions(5, PlAIN_NOTIF) yield _inject_actions(5, PlAIN_NOTIF)
yield _mark_read(3, 3) yield _mark_read(3, 3)
yield _assert_counts(1, 1, 0) yield _assert_counts(1, 0)
yield _mark_read(5, 5) yield _mark_read(5, 5)
yield _assert_counts(0, 0, 0) yield _assert_counts(0, 0)
yield _inject_actions(6, PlAIN_NOTIF) yield _inject_actions(6, PlAIN_NOTIF)
yield _rotate(7) yield _rotate(7)
@ -129,22 +121,17 @@ class EventPushActionsStoreTestCase(tests.unittest.TestCase):
table="event_push_actions", keyvalues={"1": 1}, desc="" table="event_push_actions", keyvalues={"1": 1}, desc=""
) )
yield _assert_counts(1, 1, 0) yield _assert_counts(1, 0)
yield _mark_read(7, 7) yield _mark_read(7, 7)
yield _assert_counts(0, 0, 0) yield _assert_counts(0, 0)
yield _inject_actions(8, MARK_UNREAD) yield _inject_actions(8, HIGHLIGHT)
yield _assert_counts(1, 0, 0) yield _assert_counts(1, 1)
yield _rotate(9) yield _rotate(9)
yield _assert_counts(1, 0, 0) yield _assert_counts(1, 1)
yield _rotate(10)
yield _inject_actions(10, HIGHLIGHT) yield _assert_counts(1, 1)
yield _assert_counts(2, 1, 1)
yield _rotate(11)
yield _assert_counts(2, 1, 1)
yield _rotate(12)
yield _assert_counts(2, 1, 1)
@defer.inlineCallbacks @defer.inlineCallbacks
def test_find_first_stream_ordering_after_ts(self): def test_find_first_stream_ordering_after_ts(self):