From d0c8952f0372879136093551828be7a69811536b Mon Sep 17 00:00:00 2001 From: Adam Coldrick Date: Thu, 29 Oct 2015 22:31:20 +0000 Subject: [PATCH] Single event email worker. This worker generates emails to users who are subscribed to story resources, and have selected 'individual' emails in their preferences. Further email templates will be provided in separate templates. Most of this work was originally carried out by Michael Krotscheck . Change-Id: I9785ed16f589dcd40cc29f2af09c2cd2552ff4c1 --- setup.cfg | 1 + storyboard/db/models.py | 9 +- .../plugin/email/templates/story/PUT.txt | 12 + .../email/templates/story/PUT_subject.txt | 1 + storyboard/plugin/email/workers.py | 319 ++++++++++++++++++ storyboard/tests/mock_data.py | 30 ++ storyboard/tests/mock_smtp.py | 9 +- storyboard/tests/plugin/email/test_workers.py | 201 +++++++++++ 8 files changed, 578 insertions(+), 4 deletions(-) create mode 100644 storyboard/plugin/email/templates/story/PUT.txt create mode 100644 storyboard/plugin/email/templates/story/PUT_subject.txt create mode 100644 storyboard/plugin/email/workers.py create mode 100644 storyboard/tests/plugin/email/test_workers.py diff --git a/setup.cfg b/setup.cfg index 5985c8e8..ceef51f9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,6 +36,7 @@ console_scripts = storyboard-cron = storyboard.plugin.cron:main storyboard.plugin.worker = subscription = storyboard.plugin.subscription.base:Subscription + subscription-email = storyboard.plugin.email.workers:SubscriptionEmailWorker storyboard.plugin.user_preferences = email = storyboard.plugin.email.preferences:EmailPreferences storyboard.plugin.scheduler = diff --git a/storyboard/db/models.py b/storyboard/db/models.py index f6ed3429..4a087ed9 100644 --- a/storyboard/db/models.py +++ b/storyboard/db/models.py @@ -31,6 +31,7 @@ from sqlalchemy import Enum from sqlalchemy.ext import declarative from sqlalchemy import ForeignKey from sqlalchemy import Integer +from sqlalchemy.orm.collections import attribute_mapped_collection from sqlalchemy.orm import relationship from sqlalchemy import schema from sqlalchemy import select @@ -153,7 +154,11 @@ class User(FullText, ModelBuilder, Base): permissions = relationship("Permission", secondary="user_permissions") enable_login = Column(Boolean, default=True) - preferences = relationship("UserPreference") + preferences = relationship("UserPreference", + collection_class=attribute_mapped_collection( + 'key' + ), + cascade="all, delete-orphan") _public_fields = ["id", "openid", "full_name", "last_login", "enable_login"] @@ -175,7 +180,7 @@ class UserPreference(ModelBuilder, Base): cast_func = { 'float': lambda x: float(x), 'int': lambda x: int(x), - 'bool': lambda x: bool(x), + 'bool': lambda x: bool(x == 'True'), 'string': lambda x: six.text_type(x) }[self.type] diff --git a/storyboard/plugin/email/templates/story/PUT.txt b/storyboard/plugin/email/templates/story/PUT.txt new file mode 100644 index 00000000..f8996ea6 --- /dev/null +++ b/storyboard/plugin/email/templates/story/PUT.txt @@ -0,0 +1,12 @@ +Story "{{resource.title}}" was updated by {{author.full_name}}: + +{% for key, value in before.items() %} +Property: {{key}} +From: +{{value}} + +To: +{{after[key]}} + + +{% endfor %} diff --git a/storyboard/plugin/email/templates/story/PUT_subject.txt b/storyboard/plugin/email/templates/story/PUT_subject.txt new file mode 100644 index 00000000..9188862e --- /dev/null +++ b/storyboard/plugin/email/templates/story/PUT_subject.txt @@ -0,0 +1 @@ +Story "{{resource.title}}" was updated. diff --git a/storyboard/plugin/email/workers.py b/storyboard/plugin/email/workers.py new file mode 100644 index 00000000..17760168 --- /dev/null +++ b/storyboard/plugin/email/workers.py @@ -0,0 +1,319 @@ +# Copyright (c) 2015 Hewlett-Packard Development Company, L.P. +# +# 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. + + +import abc +import copy +import os +import six +import smtplib + +from jinja2.exceptions import TemplateNotFound +from oslo_config import cfg +from oslo_log import log + +import storyboard.db.api.base as db_base +from storyboard.db.api.subscriptions import subscription_get_all_subscriber_ids +import storyboard.db.models as models +from storyboard.plugin.email.base import EmailPluginBase +from storyboard.plugin.email.factory import EmailFactory +from storyboard.plugin.email import smtp_client as smtp +from storyboard.plugin.event_worker import WorkerTaskBase + + +LOG = log.getLogger(__name__) + +CONF = cfg.CONF + + +@six.add_metaclass(abc.ABCMeta) +class EmailWorkerBase(EmailPluginBase, WorkerTaskBase): + """An abstract email construction worker. + + Abstract class that encapsulates common functionality needed in + building emails off of our event queue. + """ + + __metaclass__ = abc.ABCMeta + + def handle(self, session, author, method, path, status, resource, + resource_id, sub_resource=None, sub_resource_id=None, + resource_before=None, resource_after=None): + """Handle an event. + + :param session: An event-specific SQLAlchemy session. + :param author: The author's user record. + :param method: The HTTP Method. + :param path: The full HTTP Path requested. + :param status: The returned HTTP Status of the response. + :param resource: The resource type. + :param resource_id: The ID of the resource. + :param sub_resource: The subresource type. + :param sub_resource_id: The ID of the subresource. + :param resource_before: The resource state before this event occurred. + :param resource_after: The resource state after this event occurred. + """ + + # We only care about a subset of resource types. + if resource not in ['task', 'project_group', 'project', 'story', + 'branch', 'milestone', 'tag']: + return + + # We only care about PUT, POST, and DELETE requests that do not + # result in errors or redirects. + if method == 'GET' or status >= 300: + return + + # We only care if the current resource has subscribers. + subscribers = self.get_subscribers(session, resource, resource_id) + if not subscribers: + return + + # Pass our values on to the handler. + self.handle_email(session=session, + author=author, + subscribers=subscribers, + method=method, + status=status, + path=path, + resource=resource, + resource_id=resource_id, + sub_resource=sub_resource, + sub_resource_id=sub_resource_id, + resource_before=resource_before, + resource_after=resource_after) + + @abc.abstractmethod + def handle_email(self, session, author, subscribers, method, path, status, + resource, resource_id, sub_resource=None, + sub_resource_id=None, resource_before=None, + resource_after=None): + """Handle an email notification for the given subscribers. + + :param session: An event-specific SQLAlchemy session. + :param author: The author's user record. + :param subscribers: A list of subscribers that should receive an email. + :param method: The HTTP Method. + :param path: The full HTTP Path requested. + :param status: The returned HTTP Status of the response. + :param resource: The resource type. + :param resource_id: The ID of the resource. + :param sub_resource: The subresource type. + :param sub_resource_id: The ID of the subresource. + :param resource_before: The resource state before this event occurred. + :param resource_after: The resource state after this event occurred. + """ + + def get_subscribers(self, session, resource, resource_id): + """Get a list of users who are subscribed to the resource, + whose email address is valid, and whose email preferences indicate + that they'd like to receive non-digest email. + """ + subscribers = [] + + # Resolve all the subscriber ID's. + subscriber_ids = subscription_get_all_subscriber_ids(resource, + resource_id, + session=session) + users = db_base.model_query(models.User, session) \ + .filter(models.User.id.in_(subscriber_ids)).all() + + for user in users: + if not self.get_preference('plugin_email_enable', user) == 'true': + continue + subscribers.append(user) + + return subscribers + + def get_preference(self, name, user): + if name not in user.preferences: + return None + return user.preferences[name].cast_value + + def get_changed_properties(self, original, new): + """Shallow comparison diff. + + This method creates a shallow comparison between two dicts, + and returns two dicts containing only the changed properties from + before and after. It intentionally excludes created_at and updated_at, + as those aren't true 'values' so to say. + """ + + # Clone our value arrays, since we might return them. + before = copy.copy(original) or None + after = copy.copy(new) or None + + # Strip out protected parameters + for protected in ['created_at', 'updated_at']: + if before and protected in before: + del before[protected] + if after and protected in after: + del after[protected] + + # Sanity check, null values. + if not before or not after: + return before, after + + # Collect all the keys + before_keys = set(before.keys()) + after_keys = set(after.keys()) + keys = before_keys | after_keys + + # Run the comparison. + for key in keys: + if key not in before: + before[key] = None + if key not in after: + after[key] = None + + if after[key] == before[key]: + del after[key] + del before[key] + + return before, after + + +class SubscriptionEmailWorker(EmailWorkerBase): + """This worker plugin generates individual event emails for users who + have indicated that they wish to receive emails, but don't want digests. + """ + + def handle_email(self, session, author, subscribers, method, path, status, + resource, resource_id, sub_resource=None, + sub_resource_id=None, resource_before=None, + resource_after=None): + """Send an email for a specific event. + + We assume that filtering logic has already occurred when this method + is invoked. + + :param session: An event-specific SQLAlchemy session. + :param author: The author's user record. + :param subscribers: A list of subscribers that should receive an email. + :param method: The HTTP Method. + :param path: The full HTTP Path requested. + :param status: The returned HTTP Status of the response. + :param resource: The resource type. + :param resource_id: The ID of the resource. + :param sub_resource: The subresource type. + :param sub_resource_id: The ID of the subresource. + :param resource_before: The resource state before this event occurred. + :param resource_after: The resource state after this event occurred. + """ + + email_config = CONF.plugin_email + + # Retrieve the template names. + (subject_template, text_template, html_template) = \ + self.get_templates(method=method, + resource_name=resource, + sub_resource_name=sub_resource) + + # Build our factory. If an HTML template exists, add it. If it can't + # find the template, skip. + try: + factory = EmailFactory(sender=email_config.sender, + subject=subject_template, + text_template=text_template) + except TemplateNotFound: + LOG.error("Templates not found [%s, %s]" % (subject_template, + text_template)) + return + + # Try to add an HTML template + try: + factory.add_text_template(html_template, 'html') + except TemplateNotFound: + LOG.debug('Template %s not found' % (html_template,)) + + # If there's a reply-to in our config, add that. + if email_config.reply_to: + factory.add_header('Reply-To', email_config.reply_to) + + # Resolve the resource instance + resource_instance = self.resolve_resource_by_name(session, resource, + resource_id) + sub_resource_instance = self.resolve_resource_by_name(session, + sub_resource, + sub_resource_id) + + # Figure out the diff between old and new. + before, after = self.get_changed_properties(resource_before, + resource_after) + + # For each subscriber, create the email and send it. + with smtp.get_smtp_client() as smtp_client: + for subscriber in subscribers: + + # Make sure this subscriber's preferences indicate they want + # email and they're not receiving digests. + if not self.get_preference('plugin_email_enable', subscriber) \ + or self.get_preference('plugin_email_digest', + subscriber): + continue + + try: + # Build an email. + message = factory.build(recipient=subscriber.email, + author=author, + resource=resource_instance, + sub_resource=sub_resource_instance, + before=before, + after=after) + # Send the email. + from_addr = message.get('From') + to_addrs = message.get('To') + + try: + smtp_client.sendmail(from_addr=from_addr, + to_addrs=to_addrs, + msg=message.as_string()) + except smtplib.SMTPException as e: + LOG.error('Cannot send email, discarding: %s' % (e,)) + except Exception as e: + # Skip, keep going. + LOG.error("Cannot schedule email: %s" % (e.message,)) + + def get_templates(self, method, resource_name, sub_resource_name=None): + """Return the email templates for the given resource. + + This method builds the names of templates for a provided resource + action. The template folder structure is as follows: + + /{{resource_name}}/{{method}}_subject.txt + /{{resource_name}}/{{method}}.txt + /{{resource_name}}/{{method}}.html (optional) + + For subresources, it is as follows: + /{{resource_name}}/{{subresource_name}}/{{method}}_subject.txt + /{{resource_name}}/{{subresource_name}}/{{method}}.txt + /{{resource_name}}/{{subresource_name}}/{{method}}.html (optional) + """ + ## TODO(krotscheck): Templates can also resolve by user language. + + if sub_resource_name: + base_template = os.path.join(resource_name, sub_resource_name) + else: + base_template = resource_name + + base_file = '%s' % (method,) + + subject_template = os.path.join(base_template, + '%s_subject.txt' % (base_file,)) + text_template = os.path.join(base_template, + '%s.txt' % (base_file,)) + html_template = os.path.join(base_template, + '%s.html' % (base_file,)) + + return subject_template, text_template, html_template diff --git a/storyboard/tests/mock_data.py b/storyboard/tests/mock_data.py index 00952e55..f84f70f9 100644 --- a/storyboard/tests/mock_data.py +++ b/storyboard/tests/mock_data.py @@ -29,6 +29,7 @@ from storyboard.db.models import Task from storyboard.db.models import Team from storyboard.db.models import TimeLineEvent from storyboard.db.models import User +from storyboard.db.models import UserPreference def load(): @@ -58,6 +59,35 @@ def load(): is_superuser=False) ]) + # Load some preferences for the above users. + load_data([ + UserPreference(id=1, + user_id=1, + key='foo', + value='bar', + type='string'), + UserPreference(id=2, + user_id=1, + key='plugin_email_enable', + value='true', + type='string'), + UserPreference(id=3, + user_id=1, + key='plugin_email_digest', + value='True', + type='bool'), + UserPreference(id=4, + user_id=3, + key='plugin_email_enable', + value='true', + type='string'), + UserPreference(id=5, + user_id=3, + key='plugin_email_digest', + value='False', + type='bool'), + ]) + # Load a variety of sensibly named access tokens. load_data([ AccessToken( diff --git a/storyboard/tests/mock_smtp.py b/storyboard/tests/mock_smtp.py index 5993dc53..78be2c3e 100644 --- a/storyboard/tests/mock_smtp.py +++ b/storyboard/tests/mock_smtp.py @@ -27,7 +27,7 @@ class DummySMTP(OLD_SMTP): def __init__(self, host='', port=0, local_hostname=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT): OLD_SMTP.__init__(self, host, port, local_hostname, timeout) - + self.sendmail_invoked = 0 if hasattr(self, 'exception'): raise self.exception() @@ -44,7 +44,12 @@ class DummySMTP(OLD_SMTP): def sendmail(self, from_addr, to_addrs, msg, mail_options=[], rcpt_options=[]): - pass + self.sendmail_invoked += 1 + self.from_addr = from_addr + self.to_addr = to_addrs + self.msg = msg + self.mail_options = mail_options + self.rcpt_options = rcpt_options def quit(self): self.has_quit = True diff --git a/storyboard/tests/plugin/email/test_workers.py b/storyboard/tests/plugin/email/test_workers.py new file mode 100644 index 00000000..f8e6fbcc --- /dev/null +++ b/storyboard/tests/plugin/email/test_workers.py @@ -0,0 +1,201 @@ +# Copyright (c) 2015 Hewlett-Packard Development Company, L.P. +# +# 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. + +import copy +import mock +import six +import smtplib + +from oslo_config import cfg + +import storyboard.db.api.base as db_api_base +import storyboard.db.models as models +from storyboard.plugin.email.workers import EmailWorkerBase +from storyboard.plugin.email.workers import SubscriptionEmailWorker +from storyboard.tests import base + + +CONF = cfg.CONF + + +class TestEmailWorkerBase(base.FunctionalTest): + + def test_handle(self): + """Assert that the handle method passes the correct values on.""" + worker_base = MockEmailWorkerBase({}) + + with base.HybridSessionManager(): + session = db_api_base.get_session() + user_1 = db_api_base.entity_get(models.User, 1, session=session) + + worker_base.handle(session=session, + author=user_1, + method='POST', + path='/test', + status=201, + resource='story', + resource_id=1) + + self.assertIsInstance(worker_base.handled_values['author'], + models.User) + self.assertEqual(1, worker_base.handled_values['author'].id) + + self.assertEqual(2, len(worker_base.handled_values['subscribers'])) + self.assertEqual('POST', worker_base.handled_values['method']) + self.assertEqual(201, worker_base.handled_values['status']) + self.assertEqual('/test', worker_base.handled_values['path']) + self.assertEqual('story', worker_base.handled_values['resource']) + self.assertEqual(1, worker_base.handled_values['resource_id']) + + def test_get_subscribers(self): + """Assert that the get_subscribers method functions as expected.""" + worker_base = MockEmailWorkerBase({}) + + with base.HybridSessionManager(): + session = db_api_base.get_session() + + # Users 1 and 3 are subscribed to this story, user 1 as digest + # and user 3 as individual emails. + subscribers = worker_base.get_subscribers(session, 'story', 1) + self.assertEqual(2, len(subscribers)) + self.assertEqual(1, subscribers[0].id) + self.assertEqual(3, subscribers[1].id) + + def test_get_preference(self): + """Assert that the get_preference method functions as expected.""" + worker_base = MockEmailWorkerBase({}) + + with base.HybridSessionManager(): + session = db_api_base.get_session() + user_1 = db_api_base.entity_get(models.User, 1, session=session) + + foo_value = worker_base.get_preference('foo', user_1) + self.assertEqual('bar', foo_value) + + no_value = worker_base.get_preference('no_value', user_1) + self.assertIsNone(no_value) + + def test_get_changed_properties(self): + """Assert that changed properties are correctly detected.""" + worker_base = MockEmailWorkerBase({}) + + # Null checks + before, after = worker_base.get_changed_properties(None, {}) + self.assertIsNone(before) + self.assertIsNone(after) + before, after = worker_base.get_changed_properties(None, None) + self.assertIsNone(before) + self.assertIsNone(after) + before, after = worker_base.get_changed_properties({}, None) + self.assertIsNone(before) + self.assertIsNone(after) + + # Comparison check + before, after = worker_base.get_changed_properties({ + 'foo': 'bar', + 'lol': 'cats', + 'created_at': 'some_date', + 'before_only': 'value' + }, { + 'foo': 'bar', + 'lol': 'dogs', + 'created_at': 'some_other_date', + 'after_only': 'value' + }) + self.assertIsNotNone(before) + self.assertIsNotNone(after) + self.assertEqual(3, len(before.keys())) + self.assertIn('before_only', before.keys()) + self.assertIn('after_only', before.keys()) + self.assertIn('lol', before.keys()) + self.assertIn('before_only', after.keys()) + self.assertIn('after_only', after.keys()) + self.assertIn('lol', after.keys()) + + self.assertEqual('cats', before['lol']) + self.assertEqual('dogs', after['lol']) + self.assertEqual('value', after['after_only']) + self.assertEqual(None, before['after_only']) + self.assertEqual('value', before['before_only']) + self.assertEqual(None, after['before_only']) + + +class TestSubscriptionEmailWorker(base.FunctionalTest): + @mock.patch('storyboard.plugin.email.smtp_client.get_smtp_client') + def test_handle_email(self, get_smtp_client): + """Make sure that events from the queue are sent as emails.""" + dummy_smtp = mock.Mock(smtplib.SMTP) + worker_base = SubscriptionEmailWorker({}) + get_smtp_client.return_value.__enter__ = dummy_smtp + + with base.HybridSessionManager(): + session = db_api_base.get_session() + author = db_api_base.entity_get(models.User, 2, session=session) + story = db_api_base.entity_get(models.Story, 1, session=session) + story_dict = story.as_dict() + story_after_dict = copy.copy(story_dict) + story_after_dict['title'] = 'New Test Title' + + subscribers = worker_base.get_subscribers(session, 'story', 1) + self.assertEqual(2, len(subscribers)) + + worker_base.handle_email(session=session, + author=author, + subscribers=subscribers, + method='PUT', + path='/stories/1', + status=200, + resource='story', + resource_id=1, + resource_before=story_dict, + resource_after=story_after_dict) + # There should be two subscribers, but only one should get an + # email since the other is a digest receiver. + + subscribed_user = db_api_base.entity_get(models.User, 3, + session=session) + self.assertEqual(dummy_smtp.return_value.sendmail.call_count, 1) + self.assertEqual( + dummy_smtp.return_value.sendmail.call_args[1]['to_addrs'], + subscribed_user.email) + + def test_get_templates(self): + """Make sure the get_templates method behaves as expected.""" + worker_base = SubscriptionEmailWorker({}) + + # Basic template test. + subject, txt, html = worker_base.get_templates(method='POST', + resource_name='story', + sub_resource_name=None) + self.assertEqual('story/POST_subject.txt', subject) + self.assertEqual('story/POST.txt', txt) + self.assertEqual('story/POST.html', html) + + # Subresource template test. + subject, txt, html = worker_base.get_templates(method='POST', + resource_name='story', + sub_resource_name='f') + self.assertEqual('story/f/POST_subject.txt', subject) + self.assertEqual('story/f/POST.txt', txt) + self.assertEqual('story/f/POST.html', html) + + +class MockEmailWorkerBase(EmailWorkerBase): + """Mock instantiation of the abstract base class.""" + + def handle_email(self, **kwargs): + self.handled_values = {} + + for key, value in six.iteritems(kwargs): + self.handled_values[key] = value