Skip to content

Commit

Permalink
missed-emails-sending: Move email sending to separate queue worker.
Browse files Browse the repository at this point in the history
- Add new 'missedmessage_email_senders' queue for sending missed messages emails.
- Add the new worker to process 'missedmessage_email_senders' queue.
- Split aggregation missed messages and sending missed messages email
  to separate queue workers.
- Adapt tests for sending missed emails to the new logic.

Fixes zulip#2607
  • Loading branch information
kkanahin authored and timabbott committed Mar 8, 2017
1 parent ea4b9cb commit 6a801db
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 15 deletions.
1 change: 1 addition & 0 deletions puppet/zulip/manifests/base.pp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
'error_reports',
'feedback_messages',
'invites',
'missedmessage_email_senders',
'missedmessage_emails',
'missedmessage_mobile_notifications',
'signups',
Expand Down
9 changes: 9 additions & 0 deletions puppet/zulip_ops/files/nagios3/conf.d/services.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,15 @@ define service {
contact_groups admins
}

define service {
use generic-service
service_description Check missedmessage_email_senders queue processor
check_command check_remote_arg_string!manage.py process_queue --queue_name=missedmessage_email_senders!1:1!1:1
max_check_attempts 3
hostgroup_name frontends
contact_groups admins
}

define service {
use generic-service
service_description Check slow_queries queue processor
Expand Down
1 change: 1 addition & 0 deletions scripts/nagios/check-rabbitmq-consumers
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ queues = {
'invites',
'message_sender',
'missedmessage_emails',
'missedmessage_email_senders',
'missedmessage_mobile_notifications',
'notify_tornado',
'signups',
Expand Down
1 change: 1 addition & 0 deletions tools/test-queue-worker-reload
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ successful_worker_launches = [
'launching queue worker thread test',
'launching queue worker thread message_sender',
'launching queue worker thread missedmessage_emails',
'launching queue worker thread missedmessage_email_senders',
'launching queue worker thread email_mirror',
'launching queue worker thread user_activity_interval',
'launching queue worker thread invites',
Expand Down
34 changes: 26 additions & 8 deletions zerver/lib/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.template import loader
from django.utils import timezone
from zerver.decorator import statsd_increment, uses_mandrill
from zerver.lib.queue import queue_json_publish
from zerver.models import (
Recipient,
ScheduledJob,
Expand Down Expand Up @@ -281,15 +282,32 @@ def do_send_missedmessage_events_reply_in_zulip(user_profile, missed_messages, m

text_content = loader.render_to_string('zerver/missed_message_email.txt', template_payload)
html_content = loader.render_to_string('zerver/missed_message_email.html', template_payload)

msg = EmailMultiAlternatives(subject, text_content, from_email, [user_profile.email],
headers = headers)
msg.attach_alternative(html_content, "text/html")
msg.send()
email_content = {
'subject': subject,
'text_content': text_content,
'html_content': html_content,
'from_email': from_email,
'to': [user_profile.email],
'headers': headers
}
queue_json_publish("missedmessage_email_senders", email_content, send_missedmessage_email)

user_profile.last_reminder = timezone.now()
user_profile.save(update_fields=['last_reminder'])


def send_missedmessage_email(data):
# type: (Mapping[str, Any]) -> None
msg = EmailMultiAlternatives(
data.get('subject'),
data.get('text_content'),
data.get('from_email'),
data.get('to'),
headers=data.get('headers'))
msg.attach_alternative(data.get('html_content'), "text/html")
msg.send()


def handle_missedmessage_emails(user_profile_id, missed_email_events):
# type: (int, Iterable[Dict[str, Any]]) -> None
message_ids = [event.get('message_id') for event in missed_email_events]
Expand All @@ -298,9 +316,9 @@ def handle_missedmessage_emails(user_profile_id, missed_email_events):
if not receives_offline_notifications(user_profile):
return

messages = [um.message for um in UserMessage.objects.filter(user_profile=user_profile,
message__id__in=message_ids,
flags=~UserMessage.flags.read)]
messages = Message.objects.filter(usermessage__user_profile_id=user_profile,
id__in=message_ids,
usermessage__flags=~UserMessage.flags.read)
if not messages:
return

Expand Down
2 changes: 1 addition & 1 deletion zerver/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,7 @@ def pre_save_message(sender, **kwargs):
message.update_calculated_fields()

def get_context_for_message(message):
# type: (Message) -> Sequence[Message]
# type: (Message) -> QuerySet[Message]
# TODO: Change return type to QuerySet[Message]
return Message.objects.filter(
recipient_id=message.recipient_id,
Expand Down
11 changes: 6 additions & 5 deletions zerver/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
from __future__ import absolute_import
from __future__ import print_function

from typing import Any, Callable, Dict, Iterable, List, Mapping, Optional, Tuple, TypeVar, Text
from typing import (Any, Callable, Dict, Iterable, List, Mapping,
Optional, Tuple, TypeVar, Text, Union)
from mock import patch, MagicMock

from django.http import HttpResponse
Expand All @@ -23,7 +24,7 @@
from zerver.models import UserProfile, Recipient, \
Realm, RealmAlias, UserActivity, \
get_user_profile_by_email, get_realm, get_client, get_stream, \
Message, get_unique_open_realm, completely_open
Message, get_unique_open_realm, completely_open, get_context_for_message

from zerver.lib.avatar import avatar_url
from zerver.lib.initial_password import initial_password
Expand All @@ -33,7 +34,8 @@
do_change_is_admin, extract_recipients, \
do_set_realm_name, do_deactivate_realm, \
do_change_stream_invite_only
from zerver.lib.notifications import handle_missedmessage_emails
from zerver.lib.notifications import handle_missedmessage_emails, \
send_missedmessage_email
from zerver.lib.session_user import get_session_dict_user
from zerver.middleware import is_slow_query
from zerver.lib.utils import split_by
Expand Down Expand Up @@ -2330,9 +2332,8 @@ def _test_cases(self, tokens, msg_id, body, send_as_user):
othello = get_user_profile_by_email('[email protected]')
hamlet = get_user_profile_by_email('[email protected]')
handle_missedmessage_emails(hamlet.id, [{'message_id': msg_id}])

msg = mail.outbox[0]
reply_to_addresses = [settings.EMAIL_GATEWAY_PATTERN % (u'mm' + t) for t in tokens]
msg = mail.outbox[0]
sender = 'Zulip <{}>'.format(settings.NOREPLY_EMAIL_ADDRESS)
from_email = sender
self.assertEqual(len(mail.outbox), 1)
Expand Down
9 changes: 8 additions & 1 deletion zerver/worker/queue_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
from zerver.lib.queue import SimpleQueueClient, queue_json_publish
from zerver.lib.timestamp import timestamp_to_datetime
from zerver.lib.notifications import handle_missedmessage_emails, enqueue_welcome_emails, \
clear_followup_emails_queue, send_local_email_template_with_delay
clear_followup_emails_queue, send_local_email_template_with_delay, \
send_missedmessage_email
from zerver.lib.push_notifications import handle_push_notification
from zerver.lib.actions import do_send_confirmation_email, \
do_update_user_activity, do_update_user_activity_interval, do_update_user_presence, \
Expand Down Expand Up @@ -218,6 +219,12 @@ def start(self):
# of messages
time.sleep(2 * 60)

@assign_queue('missedmessage_email_senders')
class MissedMessageSendingWorker(QueueProcessingWorker):
def consume(self, data):
# type: (Mapping[str, Any]) -> None
send_missedmessage_email(data)

@assign_queue('missedmessage_mobile_notifications')
class PushNotificationsWorker(QueueProcessingWorker):
def consume(self, data):
Expand Down

0 comments on commit 6a801db

Please sign in to comment.