Add a config option to change whether unread push notification counts are per-message or per-room (#8820)

This PR adds a new config option to the `push` section of the homeserver config, `group_unread_count_by_room`. By default Synapse will group push notifications by room (so if you have 1000 unread messages, if they lie in 55 rooms, you'll see an unread count on your phone of 55).

However, it is also useful to be able to send out the true count of unread messages if desired. If `group_unread_count_by_room` is set to `false`, then with the above example, one would see an unread count of 1000 (email anyone?).
This commit is contained in:
Andrew Morgan 2020-11-30 18:43:54 +00:00 committed by GitHub
parent ca60822b34
commit 17fa58bdd1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 207 additions and 9 deletions

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

@ -0,0 +1 @@
Add a config option, `push.group_by_unread_count`, which controls whether unread message counts in push notifications are defined as "the number of rooms with unread messages" or "total unread messages".

View File

@ -2271,6 +2271,16 @@ push:
# #
#include_content: false #include_content: false
# When a push notification is received, an unread count is also sent.
# This number can either be calculated as the number of unread messages
# for the user, or the number of *rooms* the user has unread messages in.
#
# The default value is "true", meaning push clients will see the number of
# rooms with unread messages in them. Uncomment to instead send the number
# of unread messages.
#
#group_unread_count_by_room: false
# Spam checkers are third-party modules that can block specific actions # Spam checkers are third-party modules that can block specific actions
# of local users, such as creating rooms and registering undesirable # of local users, such as creating rooms and registering undesirable

View File

@ -23,6 +23,9 @@ class PushConfig(Config):
def read_config(self, config, **kwargs): def read_config(self, config, **kwargs):
push_config = config.get("push") or {} push_config = config.get("push") or {}
self.push_include_content = push_config.get("include_content", True) self.push_include_content = push_config.get("include_content", True)
self.push_group_unread_count_by_room = push_config.get(
"group_unread_count_by_room", True
)
pusher_instances = config.get("pusher_instances") or [] pusher_instances = config.get("pusher_instances") or []
self.pusher_shard_config = ShardedWorkerHandlingConfig(pusher_instances) self.pusher_shard_config = ShardedWorkerHandlingConfig(pusher_instances)
@ -68,4 +71,14 @@ class PushConfig(Config):
# include the event ID and room ID in push notification payloads. # include the event ID and room ID in push notification payloads.
# #
#include_content: false #include_content: false
# When a push notification is received, an unread count is also sent.
# This number can either be calculated as the number of unread messages
# for the user, or the number of *rooms* the user has unread messages in.
#
# The default value is "true", meaning push clients will see the number of
# rooms with unread messages in them. Uncomment to instead send the number
# of unread messages.
#
#group_unread_count_by_room: false
""" """

View File

@ -75,6 +75,7 @@ class HttpPusher:
self.failing_since = pusherdict["failing_since"] self.failing_since = pusherdict["failing_since"]
self.timed_call = None self.timed_call = None
self._is_processing = False self._is_processing = False
self._group_unread_count_by_room = hs.config.push_group_unread_count_by_room
# This is the highest stream ordering we know it's safe to process. # This is the highest stream ordering we know it's safe to process.
# When new events arrive, we'll be given a window of new events: we # When new events arrive, we'll be given a window of new events: we
@ -136,7 +137,11 @@ class HttpPusher:
async def _update_badge(self): async def _update_badge(self):
# XXX as per https://github.com/matrix-org/matrix-doc/issues/2627, this seems # XXX as per https://github.com/matrix-org/matrix-doc/issues/2627, this seems
# to be largely redundant. perhaps we can remove it. # to be largely redundant. perhaps we can remove it.
badge = await push_tools.get_badge_count(self.hs.get_datastore(), self.user_id) badge = await push_tools.get_badge_count(
self.hs.get_datastore(),
self.user_id,
group_by_room=self._group_unread_count_by_room,
)
await self._send_badge(badge) await self._send_badge(badge)
def on_timer(self): def on_timer(self):
@ -283,7 +288,11 @@ class HttpPusher:
return True return True
tweaks = push_rule_evaluator.tweaks_for_actions(push_action["actions"]) tweaks = push_rule_evaluator.tweaks_for_actions(push_action["actions"])
badge = await push_tools.get_badge_count(self.hs.get_datastore(), self.user_id) badge = await push_tools.get_badge_count(
self.hs.get_datastore(),
self.user_id,
group_by_room=self._group_unread_count_by_room,
)
event = await self.store.get_event(push_action["event_id"], allow_none=True) event = await self.store.get_event(push_action["event_id"], allow_none=True)
if event is None: if event is None:

View File

@ -12,12 +12,12 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from synapse.push.presentable_names import calculate_room_name, name_from_member_event from synapse.push.presentable_names import calculate_room_name, name_from_member_event
from synapse.storage import Storage from synapse.storage import Storage
from synapse.storage.databases.main import DataStore
async def get_badge_count(store, user_id): async def get_badge_count(store: DataStore, user_id: str, group_by_room: bool) -> int:
invites = await store.get_invited_rooms_for_local_user(user_id) invites = await store.get_invited_rooms_for_local_user(user_id)
joins = await store.get_rooms_for_user(user_id) joins = await store.get_rooms_for_user(user_id)
@ -34,9 +34,15 @@ async def get_badge_count(store, user_id):
room_id, user_id, last_unread_event_id room_id, user_id, last_unread_event_id
) )
) )
# return one badge count per conversation, as count per if notifs["notify_count"] == 0:
# message is so noisy as to be almost useless continue
badge += 1 if notifs["notify_count"] else 0
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"]
return badge return badge

View File

@ -12,7 +12,6 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from mock import Mock from mock import Mock
from twisted.internet.defer import Deferred from twisted.internet.defer import Deferred
@ -20,8 +19,9 @@ from twisted.internet.defer import Deferred
import synapse.rest.admin import synapse.rest.admin
from synapse.logging.context import make_deferred_yieldable from synapse.logging.context import make_deferred_yieldable
from synapse.rest.client.v1 import login, room from synapse.rest.client.v1 import login, room
from synapse.rest.client.v2_alpha import receipts
from tests.unittest import HomeserverTestCase from tests.unittest import HomeserverTestCase, override_config
class HTTPPusherTests(HomeserverTestCase): class HTTPPusherTests(HomeserverTestCase):
@ -29,6 +29,7 @@ class HTTPPusherTests(HomeserverTestCase):
synapse.rest.admin.register_servlets_for_client_rest_resource, synapse.rest.admin.register_servlets_for_client_rest_resource,
room.register_servlets, room.register_servlets,
login.register_servlets, login.register_servlets,
receipts.register_servlets,
] ]
user_id = True user_id = True
hijack_auth = False hijack_auth = False
@ -499,3 +500,161 @@ class HTTPPusherTests(HomeserverTestCase):
# check that this is low-priority # check that this is low-priority
self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low") self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low")
def test_push_unread_count_group_by_room(self):
"""
The HTTP pusher will group unread count by number of unread rooms.
"""
# Carry out common push count tests and setup
self._test_push_unread_count()
# Carry out our option-value specific test
#
# This push should still only contain an unread count of 1 (for 1 unread room)
self.assertEqual(
self.push_attempts[5][2]["notification"]["counts"]["unread"], 1
)
@override_config({"push": {"group_unread_count_by_room": False}})
def test_push_unread_count_message_count(self):
"""
The HTTP pusher will send the total unread message count.
"""
# Carry out common push count tests and setup
self._test_push_unread_count()
# Carry out our option-value specific test
#
# We're counting every unread message, so there should now be 4 since the
# last read receipt
self.assertEqual(
self.push_attempts[5][2]["notification"]["counts"]["unread"], 4
)
def _test_push_unread_count(self):
"""
Tests that the correct unread count appears in sent push notifications
Note that:
* Sending messages will cause push notifications to go out to relevant users
* Sending a read receipt will cause a "badge update" notification to go out to
the user that sent the receipt
"""
# Register the user who gets notified
user_id = self.register_user("user", "pass")
access_token = self.login("user", "pass")
# Register the user who sends the message
other_user_id = self.register_user("other_user", "pass")
other_access_token = self.login("other_user", "pass")
# Create a room (as other_user)
room_id = self.helper.create_room_as(other_user_id, tok=other_access_token)
# The user to get notified joins
self.helper.join(room=room_id, user=user_id, tok=access_token)
# Register the pusher
user_tuple = self.get_success(
self.hs.get_datastore().get_user_by_access_token(access_token)
)
token_id = user_tuple.token_id
self.get_success(
self.hs.get_pusherpool().add_pusher(
user_id=user_id,
access_token=token_id,
kind="http",
app_id="m.http",
app_display_name="HTTP Push Notifications",
device_display_name="pushy push",
pushkey="a@example.com",
lang=None,
data={"url": "example.com"},
)
)
# Send a message
response = self.helper.send(
room_id, body="Hello there!", tok=other_access_token
)
# To get an unread count, the user who is getting notified has to have a read
# position in the room. We'll set the read position to this event in a moment
first_message_event_id = response["event_id"]
# Advance time a bit (so the pusher will register something has happened) and
# make the push succeed
self.push_attempts[0][0].callback({})
self.pump()
# Check our push made it
self.assertEqual(len(self.push_attempts), 1)
self.assertEqual(self.push_attempts[0][1], "example.com")
# Check that the unread count for the room is 0
#
# The unread count is zero as the user has no read receipt in the room yet
self.assertEqual(
self.push_attempts[0][2]["notification"]["counts"]["unread"], 0
)
# Now set the user's read receipt position to the first event
#
# This will actually trigger a new notification to be sent out so that
# even if the user does not receive another message, their unread
# count goes down
request, channel = self.make_request(
"POST",
"/rooms/%s/receipt/m.read/%s" % (room_id, first_message_event_id),
{},
access_token=access_token,
)
self.assertEqual(channel.code, 200, channel.json_body)
# Advance time and make the push succeed
self.push_attempts[1][0].callback({})
self.pump()
# Unread count is still zero as we've read the only message in the room
self.assertEqual(len(self.push_attempts), 2)
self.assertEqual(
self.push_attempts[1][2]["notification"]["counts"]["unread"], 0
)
# Send another message
self.helper.send(
room_id, body="How's the weather today?", tok=other_access_token
)
# Advance time and make the push succeed
self.push_attempts[2][0].callback({})
self.pump()
# This push should contain an unread count of 1 as there's now been one
# message since our last read receipt
self.assertEqual(len(self.push_attempts), 3)
self.assertEqual(
self.push_attempts[2][2]["notification"]["counts"]["unread"], 1
)
# Since we're grouping by room, sending more messages shouldn't increase the
# unread count, as they're all being sent in the same room
self.helper.send(room_id, body="Hello?", tok=other_access_token)
# Advance time and make the push succeed
self.pump()
self.push_attempts[3][0].callback({})
self.helper.send(room_id, body="Hello??", tok=other_access_token)
# Advance time and make the push succeed
self.pump()
self.push_attempts[4][0].callback({})
self.helper.send(room_id, body="HELLO???", tok=other_access_token)
# Advance time and make the push succeed
self.pump()
self.push_attempts[5][0].callback({})
self.assertEqual(len(self.push_attempts), 6)