Update for merge and make additional interface improvements

endpoints/api/repositorynotification.py

@@ -5,9 +5,8 @@ from flask import request
 
 from endpoints.api import (RepositoryParamResource, nickname, resource, require_repo_admin,
                            log_action, validate_json_request, request_error,
-                           path_param, disallow_for_app_repositories)
+                           path_param, disallow_for_app_repositories, InvalidRequest)
 from endpoints.exception import NotFound
-from notifications import build_notification_data
 from notifications.models_interface import Repository
 from notifications.notificationevent import NotificationEvent
 from notifications.notificationmethod import (NotificationMethod,
@@ -153,9 +152,6 @@ class TestRepositoryNotification(RepositoryParamResource):
   def post(self, namespace_name, repository_name, uuid):
     """ Queues a test notification for this repository. """
     test_note = model.queue_test_notification(uuid)
-    event_info = NotificationEvent.get_event(test_note.event_name)
-    sample_data = event_info.get_sample_data(Repository(namespace, repository), event_config)
-
     if not test_note:
       raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid))
 

endpoints/api/repositorynotification_models_interface.py

@@ -19,12 +19,12 @@ class RepositoryNotification(
   """
   RepositoryNotification represents a notification for a repository.
   :type uuid: string
-  :type event: string 
+  :type event: string
-  :type method: string 
+  :type method: string
-  :type config: string  
+  :type config: string
-  :type title: string 
+  :type title: string
   :type event_config: string
-  :type number_of_failures: int 
+  :type number_of_failures: int
   """
   def to_dict(self):
     try:
@@ -53,11 +53,11 @@ class RepoNotificationInterface(object):
   """
   Interface that represents all data store interactions required by the RepositoryNotification API
   """
-  
+
   @abstractmethod
   def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, method_config, event_config, title=None):
     """
-    
+
     Args:
       namespace_name: namespace of repository
       repository_name: name of repository
@@ -72,11 +72,11 @@ class RepoNotificationInterface(object):
 
     """
     pass
-  
+
   @abstractmethod
   def list_repo_notifications(self, namespace_name, repository_name, event_name=None):
     """
-    
+
     Args:
       namespace_name: namespace of repository
       repository_name: name of repository
@@ -86,11 +86,11 @@ class RepoNotificationInterface(object):
       list(RepositoryNotification)
     """
     pass
-  
+
   @abstractmethod
   def get_repo_notification(self, uuid):
     """
-    
+
     Args:
       uuid: uuid of notification
 
@@ -98,47 +98,47 @@ class RepoNotificationInterface(object):
       RepositoryNotification or None
 
     """
-    pass  
+    pass
-  
+
   @abstractmethod
   def delete_repo_notification(self, namespace_name, repository_name, uuid):
     """
-    
+
     Args:
       namespace_name: namespace of repository
       repository_name: name of repository
       uuid: uuid of notification
-    
+
     Returns:
       RepositoryNotification or None
-    
+
     """
     pass
 
   @abstractmethod
   def reset_notification_number_of_failures(self, namespace_name, repository_name, uuid):
     """
-    
+
     Args:
       namespace_name: namespace of repository
       repository_name: name of repository
       uuid: uuid of notification
-    
+
     Returns:
       RepositoryNotification
-    
+
     """
     pass
 
   @abstractmethod
   def queue_test_notification(self, uuid):
     """
-    
+
     Args:
       uuid: uuid of notification
-    
+
     Returns:
       RepositoryNotification or None
-    
+
     """
     pass

endpoints/api/repositorynotification_models_pre_oci.py

@@ -4,23 +4,23 @@ from app import notification_queue
 from data import model
 from data.model import InvalidNotificationException
 from endpoints.api.repositorynotification_models_interface import RepoNotificationInterface, RepositoryNotification
-from endpoints.notificationevent import NotificationEvent
+from notifications import build_notification_data
-from endpoints.notificationhelper import build_notification_data
+from notifications.notificationevent import NotificationEvent
 
 
 class RepoNotificationPreOCIModel(RepoNotificationInterface):
 
   def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, method_config, event_config, title=None):
     repository = model.repository.get_repository(namespace_name, repository_name)
-    return self._notification(model.notification.create_repo_notification(repository, 
+    return self._notification(model.notification.create_repo_notification(repository,
-                                                                          event_name, 
+                                                                          event_name,
-                                                                          method_name, 
+                                                                          method_name,
-                                                                          method_config, 
+                                                                          method_config,
-                                                                          event_config, 
+                                                                          event_config,
                                                                           title))
 
   def list_repo_notifications(self, namespace_name, repository_name, event_name=None):
-    return [self._notification(n) 
+    return [self._notification(n)
             for n in model.notification.list_repo_notifications(namespace_name, repository_name, event_name)]
 
   def get_repo_notification(self, uuid):
@@ -40,15 +40,17 @@ class RepoNotificationPreOCIModel(RepoNotificationInterface):
   def reset_notification_number_of_failures(self, namespace_name, repository_name, uuid):
     return self._notification(
       model.notification.reset_notification_number_of_failures(namespace_name, repository_name, uuid))
-  
+
   def queue_test_notification(self, uuid):
     try:
       notification = model.notification.get_repo_notification(uuid)
     except InvalidNotificationException:
       return None
-    
+
+    event_config = json.loads(notification.event_config_json or '{}')
     event_info = NotificationEvent.get_event(notification.event.name)
-    sample_data = event_info.get_sample_data(notification)
+    sample_data = event_info.get_sample_data(notification.repository.namespace_user.username,
+                                             notification.repository.name, event_config)
     notification_data = build_notification_data(notification, sample_data)
     notification_queue.put([notification.repository.namespace_user.username, notification.uuid,
                             notification.event.name], json.dumps(notification_data))
@@ -58,6 +60,7 @@ class RepoNotificationPreOCIModel(RepoNotificationInterface):
   def _notification(self, notification):
     if not notification:
       return None
+
     return RepositoryNotification(uuid=notification.uuid,
                                   title=notification.title,
                                   event_name=notification.event.name,
@@ -65,7 +68,7 @@ class RepoNotificationPreOCIModel(RepoNotificationInterface):
                                   config_json=notification.config_json,
                                   event_config_json=notification.event_config_json,
                                   number_of_failures=notification.number_of_failures)
-  
 
-    
+
+
 pre_oci_model = RepoNotificationPreOCIModel()

notifications/__init__.py

@@ -10,8 +10,12 @@ from auth.auth_context import get_authenticated_user, get_validated_oauth_token
 DEFAULT_BATCH_SIZE = 1000
 
 
-def build_event_data(repo, extra_data=None, subpage=None):
+def build_repository_event_data(namespace_name, repo_name, extra_data=None, subpage=None):
-  repo_string = '%s/%s' % (repo.namespace_name, repo.name)
+  """ Builds the basic repository data for an event, including the repository's name, Docker URL
+      and homepage. If extra_data is specified, it is appended to the dictionary before it is
+      returned.
+  """
+  repo_string = '%s/%s' % (namespace_name, repo_name)
   homepage = '%s://%s/repository/%s' % (app.config['PREFERRED_URL_SCHEME'],
                                         app.config['SERVER_HOSTNAME'],
                                         repo_string)
@@ -24,8 +28,8 @@ def build_event_data(repo, extra_data=None, subpage=None):
 
   event_data = {
     'repository': repo_string,
-    'namespace': repo.namespace_name,
+    'namespace': namespace_name,
-    'name': repo.name,
+    'name': repo_name,
     'docker_url': '%s/%s' % (app.config['SERVER_HOSTNAME'], repo_string),
     'homepage': homepage,
   }
@@ -65,7 +69,8 @@ def notification_batch(batch_size=DEFAULT_BATCH_SIZE):
   with notification_queue.batch_insert(batch_size) as queue_put:
     def spawn_notification_batch(repo, event_name, extra_data=None, subpage=None, pathargs=None,
                                  performer_data=None):
-      event_data = build_event_data(repo, extra_data=extra_data, subpage=subpage)
+      event_data = build_repository_event_data(repo.namespace_name, repo.name,
+                                               extra_data=extra_data, subpage=subpage)
 
       notifications = model.notification.list_repo_notifications(repo.namespace_name,
                                                                  repo.name,

notifications/notificationevent.py

@@ -3,7 +3,7 @@ import time
 import re
 
 from datetime import datetime
-from notifications import build_event_data
+from notifications import build_repository_event_data
 from util.jinjautil import get_template_env
 from util.secscan import PRIORITY_LEVELS, get_priority_for_index
 
@@ -40,7 +40,7 @@ class NotificationEvent(object):
       'notification_data': notification_data
     })
 
-  def get_sample_data(self, repository, event_config):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     """
     Returns sample data for testing the raising of this notification, with an example notification.
     """
@@ -98,8 +98,8 @@ class RepoPushEvent(NotificationEvent):
   def get_summary(self, event_data, notification_data):
     return 'Repository %s updated' % (event_data['repository'])
 
-  def get_sample_data(self, repository, event_config):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
-    return build_event_data(repository, {
+    return build_repository_event_data(namespace_name, repo_name, {
       'updated_tags': {'latest': 'someimageid', 'foo': 'anotherimage'},
       'pruned_image_count': 3
     })
@@ -132,9 +132,9 @@ class VulnerabilityFoundEvent(NotificationEvent):
 
     return 'info'
 
-  def get_sample_data(self, repository, event_config):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     level = event_config.get(VulnerabilityFoundEvent.CONFIG_LEVEL, 'Critical')
-    return build_event_data(repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'tags': ['latest', 'prod', 'foo', 'bar', 'baz'],
       'image': 'some-image-id',
       'vulnerability': {
@@ -219,9 +219,9 @@ class BuildQueueEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'info'
 
-  def get_sample_data(self, repository, event_config):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'is_manual': False,
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
@@ -257,9 +257,9 @@ class BuildStartEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'info'
 
-  def get_sample_data(self, repository, event_config):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
       'docker_tags': ['latest', 'foo', 'bar'],
@@ -284,9 +284,9 @@ class BuildSuccessEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'success'
 
-  def get_sample_data(self, repository, event_config):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
       'docker_tags': ['latest', 'foo', 'bar'],
@@ -312,9 +312,9 @@ class BuildFailureEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'error'
 
-  def get_sample_data(self, repository, event_config):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
       'docker_tags': ['latest', 'foo', 'bar'],
@@ -351,9 +351,9 @@ class BuildCancelledEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'info'
 
-  def get_sample_data(self, repository, event_config):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
       'docker_tags': ['latest', 'foo', 'bar'],

notifications/notificationhelper.py

@@ -1,85 +0,0 @@
-import json
-
-from contextlib import contextmanager
-
-from app import app, notification_queue
-from data import model
-from auth.auth_context import get_authenticated_user, get_validated_oauth_token
-from endpoints.notificationmethod import _get_namespace_name_from
-
-
-DEFAULT_BATCH_SIZE = 1000
-
-
-def build_event_data(repo, extra_data=None, subpage=None):
-  repo_string = '%s/%s' % (_get_namespace_name_from(repo), repo.name)
-  homepage = '%s://%s/repository/%s' % (app.config['PREFERRED_URL_SCHEME'],
-                                        app.config['SERVER_HOSTNAME'],
-                                        repo_string)
-
-  if subpage:
-    if not subpage.startswith('/'):
-      subpage = '/' + subpage
-
-    homepage = homepage + subpage
-
-  event_data = {
-    'repository': repo_string,
-    'namespace': _get_namespace_name_from(repo),
-    'name': repo.name,
-    'docker_url': '%s/%s' % (app.config['SERVER_HOSTNAME'], repo_string),
-    'homepage': homepage,
-  }
-
-  event_data.update(extra_data or {})
-  return event_data
-
-def build_notification_data(notification, event_data, performer_data=None):
-  if not performer_data:
-    performer_data = {}
-
-    oauth_token = get_validated_oauth_token()
-    if oauth_token:
-      performer_data['oauth_token_id'] = oauth_token.id
-      performer_data['oauth_token_application_id'] = oauth_token.application.client_id
-      performer_data['oauth_token_application'] = oauth_token.application.name
-
-    performer_user = get_authenticated_user()
-    if performer_user:
-      performer_data['entity_id'] = performer_user.id
-      performer_data['entity_name'] = performer_user.username
-
-  return {
-    'notification_uuid': notification.uuid,
-    'event_data': event_data,
-    'performer_data': performer_data,
-  }
-
-
-@contextmanager
-def notification_batch(batch_size=DEFAULT_BATCH_SIZE):
-  """
-  Context manager implementation which returns a target callable with the same signature
-  as spawn_notification. When the the context block exits the notifications generated by
-  the callable will be bulk inserted into the queue with the specified batch size.
-  """
-  with notification_queue.batch_insert(batch_size) as queue_put:
-    def spawn_notification_batch(repo, event_name, extra_data=None, subpage=None, pathargs=None,
-                                 performer_data=None):
-      event_data = build_event_data(repo, extra_data=extra_data, subpage=subpage)
-
-      notifications = model.notification.list_repo_notifications(repo.namespace_name,
-                                                                 repo.name,
-                                                                 event_name=event_name)
-      path = [repo.namespace_name, repo.name, event_name] + (pathargs or [])
-      for notification in list(notifications):
-        notification_data = build_notification_data(notification, event_data, performer_data)
-        queue_put(path, json.dumps(notification_data))
-
-    yield spawn_notification_batch
-
-
-def spawn_notification(repo, event_name, extra_data=None, subpage=None, pathargs=None,
-                       performer_data=None):
-  with notification_batch(1) as batch_spawn:
-    batch_spawn(repo, event_name, extra_data, subpage, pathargs, performer_data)

notifications/notificationmethod.py

@@ -45,7 +45,7 @@ class NotificationMethod(object):
     """
     raise NotImplementedError
 
-  def validate(self, repository, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
     """
     Validates that the notification can be created with the given data. Throws
     a CannotValidateNotificationMethodException on failure.
@@ -77,12 +77,12 @@ class QuayNotificationMethod(NotificationMethod):
   def method_name(cls):
     return 'quay_notification'
 
-  def validate(self, repository, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
-    _, err_message, _ = self.find_targets(repository, config_data)
+    _, err_message, _ = self.find_targets(namespace_name, config_data)
     if err_message:
       raise CannotValidateNotificationMethodException(err_message)
 
-  def find_targets(self, repository, config_data):
+  def find_targets(self, namespace_name, config_data):
     target_info = config_data.get('target', None)
     if not target_info or not target_info.get('kind'):
       return (True, 'Missing target', [])
@@ -101,7 +101,7 @@ class QuayNotificationMethod(NotificationMethod):
         return (True, 'Unknown organization %s' % target_info['name'], None)
 
       # Only repositories under the organization can cause notifications to that org.
-      if target_info['name'] != repository.namespace_name:
+      if target_info['name'] != namespace_name:
         return (False, 'Organization name must match repository namespace')
 
       return (True, None, [target])
@@ -109,7 +109,7 @@ class QuayNotificationMethod(NotificationMethod):
       # Lookup the team.
       org_team = None
       try:
-        org_team = model.team.get_organization_team(repository.namespace_name, target_info['name'])
+        org_team = model.team.get_organization_team(namespace_name, target_info['name'])
       except model.InvalidTeamException:
         # Probably deleted.
         return (True, 'Unknown team %s' % target_info['name'], None)
@@ -125,7 +125,7 @@ class QuayNotificationMethod(NotificationMethod):
 
     # Lookup the target user or team to which we'll send the notification.
     config_data = notification_obj.method_config_dict
-    status, err_message, target_users = self.find_targets(repository, config_data)
+    status, err_message, target_users = self.find_targets(repository.namespace_name, config_data)
     if not status:
       raise NotificationMethodPerformException(err_message)
 
@@ -140,13 +140,12 @@ class EmailMethod(NotificationMethod):
   def method_name(cls):
     return 'email'
 
-  def validate(self, repository, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
     email = config_data.get('email', '')
     if not email:
       raise CannotValidateNotificationMethodException('Missing e-mail address')
 
-    record = model.repository.get_email_authorized_for_repo(repository.namespace_name,
+    record = model.repository.get_email_authorized_for_repo(namespace_name, repo_name, email)
-                                                            repository.name, email)
     if not record or not record.confirmed:
       raise CannotValidateNotificationMethodException('The specified e-mail address '
                                                       'is not authorized to receive '
@@ -175,7 +174,7 @@ class WebhookMethod(NotificationMethod):
   def method_name(cls):
     return 'webhook'
 
-  def validate(self, repository, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
     url = config_data.get('url', '')
     if not url:
       raise CannotValidateNotificationMethodException('Missing webhook URL')
@@ -212,7 +211,7 @@ class FlowdockMethod(NotificationMethod):
   def method_name(cls):
     return 'flowdock'
 
-  def validate(self, repository, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
     token = config_data.get('flow_api_token', '')
     if not token:
       raise CannotValidateNotificationMethodException('Missing Flowdock API Token')
@@ -264,7 +263,7 @@ class HipchatMethod(NotificationMethod):
   def method_name(cls):
     return 'hipchat'
 
-  def validate(self, repository, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
     if not config_data.get('notification_token', ''):
       raise CannotValidateNotificationMethodException('Missing Hipchat Room Notification Token')
 
@@ -375,7 +374,7 @@ class SlackMethod(NotificationMethod):
   def method_name(cls):
     return 'slack'
 
-  def validate(self, repository, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
     if not config_data.get('url', ''):
       raise CannotValidateNotificationMethodException('Missing Slack Callback URL')
 

notifications/test/test_notificationevent.py

@@ -1,6 +1,5 @@
 import pytest
 
-from notifications.models_interface import Repository
 from notifications.notificationevent import (BuildSuccessEvent, NotificationEvent,
                                              VulnerabilityFoundEvent)
 from util.morecollections import AttrDict
@@ -17,7 +16,7 @@ def test_create_notifications(event_kind):
 def test_build_notification(event_name, initialized_db):
   # Create the notification event.
   found = NotificationEvent.get_event(event_name)
-  sample_data = found.get_sample_data(Repository('foo', 'bar'), {'level': 'low'})
+  sample_data = found.get_sample_data('foo', 'bar', {'level': 'low'})
 
   # Make sure all calls succeed.
   notification_data = {

notifications/test/test_notificationmethod.py

@@ -14,10 +14,10 @@ from test.fixtures import *
 
 def assert_validated(method, method_config, error_message, namespace_name, repo_name):
   if error_message is None:
-    method.validate(Repository(namespace_name, repo_name), method_config)
+    method.validate(namespace_name, repo_name, method_config)
   else:
     with pytest.raises(CannotValidateNotificationMethodException) as ipe:
-      method.validate(Repository(namespace_name, repo_name), method_config)
+      method.validate(namespace_name, repo_name, method_config)
     assert ipe.value.message == error_message
 
 
@@ -98,7 +98,7 @@ def test_perform_quay_notification(target, expected_users, initialized_db):
 
   event_handler = NotificationEvent.get_event('repo_push')
 
-  sample_data = event_handler.get_sample_data(repository, {})
+  sample_data = event_handler.get_sample_data(repository.namespace_name, repository.name, {})
 
   method = QuayNotificationMethod()
   method.perform(notification, event_handler, {'event_data': sample_data})
@@ -116,8 +116,7 @@ def test_perform_email(initialized_db):
                               repository=repository)
 
   event_handler = NotificationEvent.get_event('repo_push')
-
+  sample_data = event_handler.get_sample_data(repository.namespace_name, repository.name, {})
-  sample_data = event_handler.get_sample_data(repository, {})
 
   mock = Mock()
   def get_mock(*args, **kwargs):
@@ -143,8 +142,7 @@ def test_perform_http_call(method, method_config, netloc, initialized_db):
                               repository=repository)
 
   event_handler = NotificationEvent.get_event('repo_push')
-
+  sample_data = event_handler.get_sample_data(repository.namespace_name, repository.name, {})
-  sample_data = event_handler.get_sample_data(repository, {})
 
   url_hit = [False]
   @urlmatch(netloc=netloc)

workers/notificationworker/test/test_notificationworker.py

@@ -58,7 +58,7 @@ def test_notifications(method, method_config, netloc, initialized_db):
       notification_uuid = model.create_notification_for_testing('public',
                                                                 method_name=method.method_name(),
                                                                 method_config=method_config)
-      event_data = RepoPushEvent().get_sample_data(Repository('devtable', 'simple'), {})
+      event_data = RepoPushEvent().get_sample_data('devtable', 'simple', {})
 
       # Fire off the queue processing.
       worker = NotificationWorker(None)