From 09ea0eac513d4bcaa26c5e9d60ab82c385a01bb0 Mon Sep 17 00:00:00 2001 From: Michael Krotscheck Date: Tue, 13 Jan 2015 10:54:01 -0800 Subject: [PATCH] Created Subscriber Utility A utility which automatically returns a list of all users who are subscribed to a given resource. Relevant use cases have been updated. Change-Id: I55cc011639af6d67ee6c809166dd070c82b59eef --- storyboard/db/api/subscriptions.py | 85 +++++++++++++++++++ .../notifications/subscriptions_handler.py | 72 ++-------------- storyboard/tests/db/test_subscriptions.py | 69 +++++++++++++++ storyboard/tests/mock_data.py | 2 +- storyboard/worker/task/subscription.py | 26 +++--- 5 files changed, 174 insertions(+), 80 deletions(-) create mode 100644 storyboard/tests/db/test_subscriptions.py diff --git a/storyboard/db/api/subscriptions.py b/storyboard/db/api/subscriptions.py index 443925a4..24f99a43 100644 --- a/storyboard/db/api/subscriptions.py +++ b/storyboard/db/api/subscriptions.py @@ -13,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +from sqlalchemy import distinct + from storyboard.db.api import base as api_base from storyboard.db import models @@ -59,3 +61,86 @@ def subscription_delete(subscription_id): if subscription: api_base.entity_hard_delete(models.Subscription, subscription_id) + + +def subscription_get_all_subscriber_ids(resource, resource_id): + '''Test subscription discovery. The tested algorithm is as follows: + + If you're subscribed to a project_group, you will be notified about + project_group, project, story, and task changes. + + If you are subscribed to a project, you will be notified about project, + story, and task changes. + + If you are subscribed to a task, you will be notified about changes to + that task. + + If you are subscribed to a story, you will be notified about changes to + that story and its tasks. + + :param resource: The name of the resource. + :param resource_id: The ID of the resource. + :return: A list of user id's. + ''' + affected = { + 'project_group': set(), + 'project': set(), + 'story': set(), + 'task': set() + } + + # Sanity check exit. + if resource not in affected.keys(): + return set() + + # Make sure the requested resource is going to be handled. + affected[resource].add(resource_id) + + # Resolve either from story->task or from task->story, so the root + # resource id remains pristine. + if resource == 'story': + # Get this story's tasks + query = api_base.model_query(models.Task.id) \ + .filter(models.Task.story_id.in_(affected['story'])) + + affected['task'] = affected['task'] \ + .union(r for (r, ) in query.all()) + elif resource == 'task': + # Get this tasks's story + query = api_base.model_query(models.Task.story_id) \ + .filter(models.Task.id == resource_id) + + affected['story'].add(query.first().story_id) + + # If there are tasks, there will also be projects. + if affected['task']: + # Get all the tasks's projects + query = api_base.model_query(distinct(models.Task.project_id)) \ + .filter(models.Task.id.in_(affected['task'])) + + affected['project'] = affected['project'] \ + .union(r for (r, ) in query.all()) + + # If there are projects, there will also be project groups. + if affected['project']: + # Get all the projects' groups. + query = api_base.model_query( + distinct(models.project_group_mapping.c.project_group_id)) \ + .filter(models.project_group_mapping.c.project_id + .in_(affected['project'])) + + affected['project_group'] = affected['project_group'] \ + .union(r for (r, ) in query.all()) + + # Load all subscribers. + subscribers = set() + for affected_type in affected: + query = api_base.model_query(distinct( + models.Subscription.user_id)) \ + .filter(models.Subscription.target_type == affected_type) \ + .filter(models.Subscription.target_id.in_(affected[affected_type])) + + results = query.all() + subscribers = subscribers.union(r for (r, ) in results) + + return subscribers diff --git a/storyboard/notifications/subscriptions_handler.py b/storyboard/notifications/subscriptions_handler.py index c8e59e04..8329cb70 100644 --- a/storyboard/notifications/subscriptions_handler.py +++ b/storyboard/notifications/subscriptions_handler.py @@ -16,57 +16,15 @@ import json from storyboard.db.api import comments as comments_api -from storyboard.db.api import project_groups as project_groups_api from storyboard.db.api import stories as story_api from storyboard.db.api import subscription_events as sub_events_api from storyboard.db.api import subscriptions as subscriptions_api -from storyboard.db.api import tasks as tasks_api from storyboard.db.api import timeline_events as timeline_api -def handle_timeline_events(event, author_id): +def handle_timeline_events(event, author_id, subscribers): - target_subs = [] - user_ids = set() - - story_id = event.story_id - if event.event_info: - event_info = json.loads(event.event_info) - task_id = event_info.get("task_id") - - # Handling tasks targeted. - target_sub = subscriptions_api.subscription_get_all_by_target( - ['task'], task_id) - target_subs.extend(target_sub) - - # Handling stories targeted. - target_sub = subscriptions_api.subscription_get_all_by_target( - ['story'], story_id) - target_subs.extend(target_sub) - - # Handling projects, project groups targeted for stories without tasks. - - tasks = tasks_api.task_get_all(story_id=story_id) - - for task in tasks: - project_id = task.project_id - - # Handling projects targeted. - target_sub = subscriptions_api.subscription_get_all_by_target( - ['project'], project_id) - target_subs.extend(target_sub) - - # Handling project groups targeted. - pgs = project_groups_api.project_group_get_all(project_id=project_id) - for pg in pgs: - target_sub = subscriptions_api.subscription_get_all_by_target( - ['project_group'], pg.id) - target_subs.extend(target_sub) - - for sub in target_subs: - user_ids.add(sub.user_id) - - for user_id in user_ids: + for user_id in subscribers: if event.event_type == 'user_comment': event_info = resolve_comments(event) @@ -81,22 +39,12 @@ def handle_timeline_events(event, author_id): }) -def handle_resources(method, resource_id, sub_resource_id, author_id): - - target_subs = [] - user_ids = set() +def handle_resources(method, resource_id, sub_resource_id, author_id, + subscribers): if sub_resource_id: - # Handling project addition/deletion to/from project_group. - target_sub = subscriptions_api.subscription_get_all_by_target( - ['project'], sub_resource_id) - target_subs.extend(target_sub) - - for sub in target_subs: - user_ids.add(sub.user_id) - - for user_id in user_ids: + for user_id in subscribers: if method == 'DELETE': event_type = 'project removed from project_group' @@ -118,14 +66,7 @@ def handle_resources(method, resource_id, sub_resource_id, author_id): else: if method == 'DELETE': # Handling project_group targeted. - target_sub = subscriptions_api.subscription_get_all_by_target( - ['project_group'], resource_id) - target_subs.extend(target_sub) - - for sub in target_subs: - user_ids.add(sub.user_id) - - for user_id in user_ids: + for user_id in subscribers: sub_events_api.subscription_events_create({ "author_id": author_id, "subscriber_id": user_id, @@ -135,7 +76,6 @@ def handle_resources(method, resource_id, sub_resource_id, author_id): def handle_deletions(resource_name, resource_id): - target_subs = [] sub_ids = set() resource_name = resource_name[:-1] diff --git a/storyboard/tests/db/test_subscriptions.py b/storyboard/tests/db/test_subscriptions.py new file mode 100644 index 00000000..a6398e32 --- /dev/null +++ b/storyboard/tests/db/test_subscriptions.py @@ -0,0 +1,69 @@ +# Copyright (c) 2014 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. + +from storyboard.db.api.subscriptions import subscription_get_all_subscriber_ids +from storyboard.tests import base + + +class TestSubscription(base.FunctionalTest): + '''Tests for the subscription db interface. + ''' + + def test_get_subscribers(self): + '''Test subscription discovery. The tested algorithm is as follows: + If you're subscribed to a project_group, you will be notified about + project_group, project, story, and task changes. If you are + subscribed to a project, you will be notified about project, story, and + task changes. If you are subscribed to a task, you will be notified + about changes to that task. If you are subscribed to a story, + you will be notified about changes to that story and its tasks. + + ''' + + subscribers = subscription_get_all_subscriber_ids('invalid', 1) + self.assertSetEqual(set(), subscribers) + + subscribers = subscription_get_all_subscriber_ids('story', 1) + self.assertSetEqual({1, 3}, subscribers) + subscribers = subscription_get_all_subscriber_ids('story', 2) + self.assertSetEqual(set(), subscribers) + subscribers = subscription_get_all_subscriber_ids('story', 3) + self.assertSetEqual(set(), subscribers) + subscribers = subscription_get_all_subscriber_ids('story', 4) + self.assertSetEqual(set(), subscribers) + subscribers = subscription_get_all_subscriber_ids('story', 5) + self.assertSetEqual(set(), subscribers) + + subscribers = subscription_get_all_subscriber_ids('task', 1) + self.assertSetEqual({1, 3}, subscribers) + subscribers = subscription_get_all_subscriber_ids('task', 2) + self.assertSetEqual({3}, subscribers) + subscribers = subscription_get_all_subscriber_ids('task', 3) + self.assertSetEqual({1, 3}, subscribers) + subscribers = subscription_get_all_subscriber_ids('task', 4) + self.assertSetEqual(set(), subscribers) + + subscribers = subscription_get_all_subscriber_ids('project', 1) + self.assertSetEqual({1}, subscribers) + subscribers = subscription_get_all_subscriber_ids('project', 2) + self.assertSetEqual(set(), subscribers) + subscribers = subscription_get_all_subscriber_ids('project', 3) + self.assertSetEqual({1}, subscribers) + + subscribers = subscription_get_all_subscriber_ids('project_group', 1) + self.assertSetEqual({1}, subscribers) + subscribers = subscription_get_all_subscriber_ids('project_group', 2) + self.assertSetEqual(set(), subscribers) + subscribers = subscription_get_all_subscriber_ids('project_group', 3) + self.assertSetEqual(set(), subscribers) diff --git a/storyboard/tests/mock_data.py b/storyboard/tests/mock_data.py index 114dc85e..e3dd995f 100644 --- a/storyboard/tests/mock_data.py +++ b/storyboard/tests/mock_data.py @@ -196,7 +196,7 @@ def load(): Subscription( id=1, user_id=1, - target_type='project', + target_type='project_group', target_id=1 ), Subscription( diff --git a/storyboard/worker/task/subscription.py b/storyboard/worker/task/subscription.py index 56583344..962efd39 100644 --- a/storyboard/worker/task/subscription.py +++ b/storyboard/worker/task/subscription.py @@ -14,6 +14,7 @@ import json +from storyboard.db.api import subscriptions from storyboard.db.api import timeline_events from storyboard.notifications.subscriptions_handler import handle_deletions from storyboard.notifications.subscriptions_handler import handle_resources @@ -31,23 +32,22 @@ class Subscription(WorkerTaskBase): :return: """ body_dict = json.loads(body) + subscribers = subscriptions.subscription_get_all_subscriber_ids( + body_dict['resource'], body_dict['resource_id'] + ) + if 'event_id' in body_dict: event_id = body_dict['event_id'] event = timeline_events.event_get(event_id) - handle_timeline_events(event, body_dict['author_id']) + handle_timeline_events(event, body_dict['author_id'], subscribers) - else: - if body_dict['resource'] == 'project_groups': - if 'sub_resource_id' in body_dict: - handle_resources(method=body_dict['method'], - resource_id=body_dict['resource_id'], - sub_resource_id=body_dict[ - 'sub_resource_id'], - author_id=body_dict['author_id']) - else: - handle_resources(method=body_dict['method'], - resource_id=body_dict['resource_id'], - author_id=body_dict['author_id']) + elif body_dict['resource'] == 'project_groups': + handle_resources(method=body_dict['method'], + resource_id=body_dict['resource_id'], + sub_resource_id=getattr(body_dict, + 'sub_resource_id', None), + author_id=body_dict['author_id'], + subscribers=subscribers) if body_dict['method'] == 'DELETE': resource_name = body_dict['resource']