From eae86c15d1c4908887bc37c920cada0a05ab11fa Mon Sep 17 00:00:00 2001 From: Michael Krotscheck Date: Thu, 8 Jan 2015 10:07:41 -0800 Subject: [PATCH] Explicit Event API The event handler's message parameters have been made explicit, to provide a third-party engineer with a self-describing method signature to implement. This is intended to improve the readability of the codebase. Change-Id: I61b294d9e9505c7dcd7095f4f2a92b307ebb038b --- storyboard/db/api/timeline_events.py | 17 +++-- storyboard/notifications/notification_hook.py | 65 ++++++++----------- storyboard/notifications/publisher.py | 34 ++++++++-- storyboard/notifications/subscriber.py | 11 +++- storyboard/worker/task/base.py | 15 ++++- storyboard/worker/task/subscription.py | 44 ++++++------- 6 files changed, 109 insertions(+), 77 deletions(-) diff --git a/storyboard/db/api/timeline_events.py b/storyboard/db/api/timeline_events.py index 833d21d8..76736f52 100644 --- a/storyboard/db/api/timeline_events.py +++ b/storyboard/db/api/timeline_events.py @@ -16,6 +16,7 @@ import json from oslo.config import cfg from pecan import request +from pecan import response from storyboard.common import event_types from storyboard.db.api import base as api_base @@ -48,13 +49,15 @@ def event_create(values): new_event = api_base.entity_create(models.TimeLineEvent, values) if CONF.enable_notifications: - payload = { - "author_id": request.current_user_id, - "method": "POST", - "resource": "timeline_events", - "event_id": new_event.id - } - publish("timeline_events", payload) + # Build the payload. Use of None is included to ensure that we don't + # accidentally blow up the API call, but we don't anticipate it + # happening. + publish(author_id=request.current_user_id or None, + method="POST", + path=request.path or None, + status=response.status_code or None, + resource="timeline_events", + resource_id=new_event.id or None) return new_event diff --git a/storyboard/notifications/notification_hook.py b/storyboard/notifications/notification_hook.py index 9ec486f8..15ebc192 100644 --- a/storyboard/notifications/notification_hook.py +++ b/storyboard/notifications/notification_hook.py @@ -31,52 +31,41 @@ class NotificationHook(hooks.PecanHook): return request = state.request - req_method = request.method - req_author_id = request.current_user_id - req_path = request.path - req_resource_grp = self._parse(req_path) + response = state.response - if not req_resource_grp: + # Attempt to determine the type of the payload. This checks for + # nested paths. + (resource, resource_id, subresource, subresource_id) \ + = self._parse(request.path) + if not resource: return - resource = req_resource_grp[0] - - if req_resource_grp[1]: - resource_id = req_resource_grp[1] - else: + if state.request.method == 'POST': # When a resource is created.. - response_str = state.response.body - response = json.loads(response_str) - if response: - resource_id = response.get('id') + response_body = json.loads(response.body) + if response_body: + resource_id = response_body.get('id') else: resource_id = None - # when adding/removing projects to project_groups.. - if req_resource_grp[3]: - sub_resource_id = req_resource_grp[3] - payload = { - "author_id": req_author_id, - "method": req_method, - "resource": resource, - "resource_id": resource_id, - "sub_resource_id": sub_resource_id - } - - else: - payload = { - "author_id": req_author_id, - "method": req_method, - "resource": resource, - "resource_id": resource_id - } - - publish(resource, payload) + # Build the payload. Use of None is included to ensure that we don't + # accidentally blow up the API call, but we don't anticipate it + # happening. + publish(author_id=request.current_user_id, + method=request.method, + path=request.path, + status=response.status_code, + resource=resource, + resource_id=resource_id, + sub_resource=subresource, + sub_resource_id=subresource_id) def _parse(self, s): url_pattern = re.match("^\/v1\/([a-z_]+)\/?([0-9]+)?" "\/?([a-z]+)?\/?([0-9]+)?$", s) - if url_pattern and url_pattern.groups()[0] != "openid": - return url_pattern.groups() - else: - return + if not url_pattern or url_pattern.groups()[0] == "openid": + return None, None, None, None + + groups = url_pattern.groups() + + return groups[0], groups[1], groups[2], groups[3] diff --git a/storyboard/notifications/publisher.py b/storyboard/notifications/publisher.py index 37420270..be0214c7 100644 --- a/storyboard/notifications/publisher.py +++ b/storyboard/notifications/publisher.py @@ -131,13 +131,19 @@ class Payload(object): self.payload = payload -def publish(topic, payload): - """Send a message with a given topic and payload to the storyboard - exchange. The message will be automatically JSON encoded. +def publish(resource, author_id=None, method=None, path=None, status=None, + resource_id=None, sub_resource=None, sub_resource_id=None): + """Send a message for an API event to the storyboard exchange. The message + will be automatically JSON encoded. - :param topic: The RabbitMQ topic. - :param payload: The JSON-serializable payload. - :return: + :param resource: The extrapolated resource type (project, story, etc). + :param author_id: The ID of the author who performed this action. + :param method: The HTTP Method used. + :param path: The HTTP Path used. + :param status: The HTTP Status code of the response. + :param resource_id: The ID of the resource. + :param sub_resource: The extracted subresource (user_token, etc) + :param sub_resource_id: THe ID of the subresource. """ global PUBLISHER @@ -146,4 +152,18 @@ def publish(topic, payload): PUBLISHER = Publisher(CONF.notifications) PUBLISHER.start() - PUBLISHER.publish_message(topic, payload) + payload = { + "author_id": author_id, + "method": method, + "path": path, + "status": status, + "resource": resource, + "resource_id": resource_id, + "sub_resource": sub_resource, + "sub_resource_id": sub_resource_id + } + + if resource: + PUBLISHER.publish_message(resource, payload) + else: + LOG.warning("Attempted to send payload with no destination resource.") diff --git a/storyboard/notifications/subscriber.py b/storyboard/notifications/subscriber.py index cbf646bd..c97c304c 100644 --- a/storyboard/notifications/subscriber.py +++ b/storyboard/notifications/subscriber.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import time from oslo.config import cfg @@ -65,7 +66,15 @@ def handle_event(ext, body): :param body: The body of the event. :return: The result of the handler. """ - return ext.obj.handle(body) + payload = json.loads(body) + return ext.obj.handle(author_id=payload['author_id'] or None, + method=payload['method'] or None, + path=payload['path'] or None, + status=payload['status'] or None, + resource=payload['resource'] or None, + resource_id=payload['resource_id'] or None, + sub_resource=payload['sub_resource'] or None, + sub_resource_id=payload['sub_resource_id'] or None) def check_enabled(ext): diff --git a/storyboard/worker/task/base.py b/storyboard/worker/task/base.py index a5cd8162..76303444 100644 --- a/storyboard/worker/task/base.py +++ b/storyboard/worker/task/base.py @@ -33,5 +33,16 @@ class WorkerTaskBase(object): """ @abc.abstractmethod - def handle(self, body): - """Handle an event.""" + def handle(self, author_id, method, path, status, resource, resource_id, + sub_resource=None, sub_resource_id=None): + """Handle an event. + + :param author_id: ID of 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. + """ diff --git a/storyboard/worker/task/subscription.py b/storyboard/worker/task/subscription.py index 962efd39..f66b7392 100644 --- a/storyboard/worker/task/subscription.py +++ b/storyboard/worker/task/subscription.py @@ -12,8 +12,6 @@ # implied. See the License for the specific language governing permissions and # limitations under the License. -import json - from storyboard.db.api import subscriptions from storyboard.db.api import timeline_events from storyboard.notifications.subscriptions_handler import handle_deletions @@ -24,36 +22,38 @@ from storyboard.worker.task.base import WorkerTaskBase class Subscription(WorkerTaskBase): - def handle(self, body): + def handle(self, author_id, method, path, status, resource, resource_id, + sub_resource=None, sub_resource_id=None): """This worker handles API events and attempts to determine whether they correspond to user subscriptions. - :param body: The event message body. - :return: + :param author_id: ID of 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. """ - body_dict = json.loads(body) + subscribers = subscriptions.subscription_get_all_subscriber_ids( - body_dict['resource'], body_dict['resource_id'] + resource, 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'], subscribers) + if resource == 'timeline_events': + event = timeline_events.event_get(resource_id) + handle_timeline_events(event, author_id, subscribers) - 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'], + elif resource == 'project_groups': + handle_resources(method=method, + resource_id=resource_id, + sub_resource_id=sub_resource_id, + author_id=author_id, subscribers=subscribers) - if body_dict['method'] == 'DELETE': - resource_name = body_dict['resource'] - resource_id = body_dict['resource_id'] - if 'sub_resource_id' not in body_dict: - handle_deletions(resource_name, resource_id) + if method == 'DELETE' and not sub_resource_id: + handle_deletions(resource, resource_id) def enabled(self): return True