From 989bdc9e569e6ca414369e90e7b88e2f1d99c753 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 23 May 2016 19:24:11 +0100 Subject: Tune email notifs to make them quieter: * After initial 10 minute window, only alert every 24h for room notifs * Reset room state after 6h of idleness * Synchronise throttles for messages sent in the same notif, so the 24 hourly notifs 'line up' * Fix the email subjects to say what triggered the notification * Order the rooms in reverse activity order in the email, so the 'reason' room should always come first --- synapse/push/emailpusher.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) (limited to 'synapse/push/emailpusher.py') diff --git a/synapse/push/emailpusher.py b/synapse/push/emailpusher.py index b4b728adc5..a72cba8306 100644 --- a/synapse/push/emailpusher.py +++ b/synapse/push/emailpusher.py @@ -32,12 +32,19 @@ DELAY_BEFORE_MAIL_MS = 10 * 60 * 1000 # Each room maintains its own throttle counter, but each new mail notification # sends the pending notifications for all rooms. THROTTLE_START_MS = 10 * 60 * 1000 -THROTTLE_MAX_MS = 24 * 60 * 60 * 1000 # (2 * 60 * 1000) * (2 ** 11) # ~3 days -THROTTLE_MULTIPLIER = 6 # 10 mins, 1 hour, 6 hours, 24 hours +THROTTLE_MAX_MS = 24 * 60 * 60 * 1000 # 24h +# THROTTLE_MULTIPLIER = 6 # 10 mins, 1 hour, 6 hours, 24 hours +THROTTLE_MULTIPLIER = 144 # 10 mins, 24 hours - i.e. jump straight to 1 day # If no event triggers a notification for this long after the previous, # the throttle is released. -THROTTLE_RESET_AFTER_MS = (2 * 60 * 1000) * (2 ** 11) # ~3 days +# 12 hours - a gap of 12 hours in conversation is surely enough to merit a new +# notification when things get going again... +THROTTLE_RESET_AFTER_MS = (12 * 60 * 60 * 1000) + +# does each email include all unread notifs, or just the ones which have happened +# since the last mail? +INCLUDE_ALL_UNREAD_NOTIFS = True class EmailPusher(object): @@ -126,8 +133,9 @@ class EmailPusher(object): up logging, measures and guards against multiple instances of it being run. """ + start = 0 if INCLUDE_ALL_UNREAD_NOTIFS else self.last_stream_ordering unprocessed = yield self.store.get_unread_push_actions_for_user_in_range( - self.user_id, self.last_stream_ordering, self.max_stream_ordering + self.user_id, start, self.max_stream_ordering ) soonest_due_at = None @@ -150,7 +158,6 @@ class EmailPusher(object): # we then consider all previously outstanding notifications # to be delivered. - # debugging: reason = { 'room_id': push_action['room_id'], 'now': self.clock.time_msec(), @@ -165,9 +172,12 @@ class EmailPusher(object): yield self.save_last_stream_ordering_and_success(max([ ea['stream_ordering'] for ea in unprocessed ])) - yield self.sent_notif_update_throttle( - push_action['room_id'], push_action - ) + + # we update the throttle on all the possible unprocessed push actions + for ea in unprocessed: + yield self.sent_notif_update_throttle( + ea['room_id'], ea + ) break else: if soonest_due_at is None or should_notify_at < soonest_due_at: -- cgit 1.5.1 From a15ad608496fd29fb8bf289152c23adca822beca Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 2 Jun 2016 11:44:15 +0100 Subject: Email unsubscribing that may in theory, work Were it not for that fact that you can't use the base handler in the pusher because it pulls in the world. Comitting while I fix that on a different branch. --- synapse/handlers/auth.py | 5 +++++ synapse/push/emailpusher.py | 2 +- synapse/push/mailer.py | 21 ++++++++++++++++----- 3 files changed, 22 insertions(+), 6 deletions(-) (limited to 'synapse/push/emailpusher.py') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 26c865e171..200793b5ed 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -529,6 +529,11 @@ class AuthHandler(BaseHandler): macaroon.add_first_party_caveat("time < %d" % (expiry,)) return macaroon.serialize() + def generate_delete_pusher_token(self, user_id): + macaroon = self._generate_base_macaroon(user_id) + macaroon.add_first_party_caveat("type = delete_pusher") + return macaroon.serialize() + def validate_short_term_login_token_and_get_user_id(self, login_token): try: macaroon = pymacaroons.Macaroon.deserialize(login_token) diff --git a/synapse/push/emailpusher.py b/synapse/push/emailpusher.py index a72cba8306..46d7c0434b 100644 --- a/synapse/push/emailpusher.py +++ b/synapse/push/emailpusher.py @@ -273,5 +273,5 @@ class EmailPusher(object): logger.info("Sending notif email for user %r", self.user_id) yield self.mailer.send_notification_mail( - self.user_id, self.email, push_actions, reason + self.app_id, self.user_id, self.email, push_actions, reason ) diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index 3ae92d1574..95250bad7d 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -81,6 +81,7 @@ class Mailer(object): def __init__(self, hs): self.hs = hs self.store = self.hs.get_datastore() + self.handlers = self.hs.get_handlers() self.state_handler = self.hs.get_state_handler() loader = jinja2.FileSystemLoader(self.hs.config.email_template_dir) self.app_name = self.hs.config.email_app_name @@ -95,7 +96,8 @@ class Mailer(object): ) @defer.inlineCallbacks - def send_notification_mail(self, user_id, email_address, push_actions, reason): + def send_notification_mail(self, app_id, user_id, email_address, + push_actions, reason): raw_from = email.utils.parseaddr(self.hs.config.email_notif_from)[1] raw_to = email.utils.parseaddr(email_address)[1] @@ -157,7 +159,7 @@ class Mailer(object): template_vars = { "user_display_name": user_display_name, - "unsubscribe_link": self.make_unsubscribe_link(), + "unsubscribe_link": self.make_unsubscribe_link(app_id, email_address), "summary_text": summary_text, "app_name": self.app_name, "rooms": rooms, @@ -423,9 +425,18 @@ class Mailer(object): notif['room_id'], notif['event_id'] ) - def make_unsubscribe_link(self): - # XXX: matrix.to - return "https://vector.im/#/settings" + def make_unsubscribe_link(self, app_id, email_address): + params = { + "access_token": self.handlers.auth.generate_delete_pusher_token(), + "app_id": app_id, + "pushkey": email_address, + } + + # XXX: make r0 once API is stable + return "%s_matrix/client/unstable/pushers/remove?%s" % ( + self.hs.config.public_baseurl, + urllib.urlencode(params), + ) def mxc_to_http_filter(self, value, width, height, resize_method="crop"): if value[0:6] != "mxc://": -- cgit 1.5.1 From f84b89f0c6b2e67897fec8639b79bf1d45c8f2b6 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 2 Jun 2016 13:29:48 +0100 Subject: if an email pusher specifies a brand param, use it --- synapse/push/emailpusher.py | 7 ++++++- synapse/push/mailer.py | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'synapse/push/emailpusher.py') diff --git a/synapse/push/emailpusher.py b/synapse/push/emailpusher.py index a72cba8306..e38ed02006 100644 --- a/synapse/push/emailpusher.py +++ b/synapse/push/emailpusher.py @@ -72,7 +72,12 @@ class EmailPusher(object): self.processing = False if self.hs.config.email_enable_notifs: - self.mailer = Mailer(self.hs) + if 'data' in pusherdict and 'brand' in pusherdict['data']: + app_name = pusherdict['data']['brand'] + else: + app_name = self.hs.config.email_app_name + + self.mailer = Mailer(self.hs, app_name) else: self.mailer = None diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index fe5d67a03c..0e9d8ccb53 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -78,12 +78,12 @@ ALLOWED_ATTRS = { class Mailer(object): - def __init__(self, hs): + def __init__(self, hs, app_name): self.hs = hs self.store = self.hs.get_datastore() self.state_handler = self.hs.get_state_handler() loader = jinja2.FileSystemLoader(self.hs.config.email_template_dir) - self.app_name = self.hs.config.email_app_name + self.app_name = app_name env = jinja2.Environment(loader=loader) env.filters["format_ts"] = format_ts_filter env.filters["mxc_to_http"] = self.mxc_to_http_filter -- cgit 1.5.1 From 356f13c0696526032c211c103dad2f57d18473fa Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 2 Jun 2016 14:07:38 +0100 Subject: Disable INCLUDE_ALL_UNREAD_NOTIFS --- synapse/push/emailpusher.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'synapse/push/emailpusher.py') diff --git a/synapse/push/emailpusher.py b/synapse/push/emailpusher.py index e38ed02006..2c21ed3088 100644 --- a/synapse/push/emailpusher.py +++ b/synapse/push/emailpusher.py @@ -44,7 +44,8 @@ THROTTLE_RESET_AFTER_MS = (12 * 60 * 60 * 1000) # does each email include all unread notifs, or just the ones which have happened # since the last mail? -INCLUDE_ALL_UNREAD_NOTIFS = True +# XXX: this is currently broken as it includes ones from parted rooms(!) +INCLUDE_ALL_UNREAD_NOTIFS = False class EmailPusher(object): -- cgit 1.5.1