Merge pull request #2793 from coreos-inc/notifications-refactor

Refactor the notifications system into its own package and add much better testing

buildman/jobutil/buildjob.py

@@ -2,7 +2,7 @@ import json
 import logging
 
 from cachetools import lru_cache
-from endpoints.notificationhelper import spawn_notification
+from notifications import spawn_notification
 from data import model
 from util.imagetree import ImageTree
 from util.morecollections import AttrDict

endpoints/api/repositorynotification.py

@@ -3,14 +3,14 @@
 import logging
 from flask import request
 
-from endpoints.api import (RepositoryParamResource, nickname, resource, require_repo_admin,
+from endpoints.api import (
-                           log_action, validate_json_request, request_error,
+  RepositoryParamResource, nickname, resource, require_repo_admin, log_action,
-                           path_param, disallow_for_app_repositories)
+  validate_json_request, request_error, path_param, disallow_for_app_repositories, InvalidRequest)
-from endpoints.exception import NotFound, InvalidRequest
+from endpoints.exception import NotFound
-from endpoints.notificationmethod import (NotificationMethod,
+from notifications.models_interface import Repository
-                                          CannotValidateNotificationMethodException)
+from notifications.notificationevent import NotificationEvent
-from endpoints.notificationhelper import build_notification_data
+from notifications.notificationmethod import (
-from workers.notificationworker.models_pre_oci import notification
+  NotificationMethod, CannotValidateNotificationMethodException)
 from endpoints.api.repositorynotification_models_pre_oci import pre_oci_model as model
 
 logger = logging.getLogger(__name__)
@@ -69,17 +69,16 @@ class RepositoryNotificationList(RepositoryParamResource):
       raise request_error(message=ex.message)
 
     new_notification = model.create_repo_notification(namespace_name, repository_name,
-                                                      parsed['event'], 
+                                                      parsed['event'], parsed['method'],
-                                                      parsed['method'], 
+                                                      parsed['config'], parsed['eventConfig'],
-                                                      parsed['config'], 
-                                                      parsed['eventConfig'], 
                                                       parsed.get('title'))
 
-    log_action('add_repo_notification', namespace_name,
+    log_action('add_repo_notification', namespace_name, {
-               {'repo': repository_name, 'namespace': namespace_name,
+      'repo': repository_name,
-                'notification_id': new_notification.uuid,
+      'namespace': namespace_name,
-                'event': new_notification.event_name, 'method': new_notification.method_name},
+      'notification_id': new_notification.uuid,
-               repo_name=repository_name)
+      'event': new_notification.event_name,
+      'method': new_notification.method_name}, repo_name=repository_name)
     return new_notification.to_dict(), 201
 
   @require_repo_admin
@@ -88,9 +87,7 @@ class RepositoryNotificationList(RepositoryParamResource):
   def get(self, namespace_name, repository_name):
     """ List the notifications for the specified repository. """
     notifications = model.list_repo_notifications(namespace_name, repository_name)
-    return {
+    return {'notifications': [n.to_dict() for n in notifications]}
-      'notifications': [n.to_dict() for n in notifications]
-    }
 
 
 @resource('/v1/repository/<apirepopath:repository>/notification/<uuid>')
@@ -98,6 +95,7 @@ class RepositoryNotificationList(RepositoryParamResource):
 @path_param('uuid', 'The UUID of the notification')
 class RepositoryNotification(RepositoryParamResource):
   """ Resource for dealing with specific notifications. """
+
   @require_repo_admin
   @nickname('getRepoNotification')
   @disallow_for_app_repositories
@@ -115,12 +113,15 @@ class RepositoryNotification(RepositoryParamResource):
     """ Deletes the specified notification. """
     deleted = model.delete_repo_notification(namespace_name, repository_name, uuid)
     if not deleted:
-      raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid))
+      raise InvalidRequest("No repository notification found for: %s, %s, %s" %
+                           (namespace_name, repository_name, uuid))
 
-    log_action('delete_repo_notification', namespace_name,
+    log_action('delete_repo_notification', namespace_name, {
-               {'repo': repository_name, 'namespace': namespace_name, 'notification_id': uuid,
+      'repo': repository_name,
-                'event': deleted.event_name, 'method': deleted.method_name},
+      'namespace': namespace_name,
-               repo_name=repository_name)
+      'notification_id': uuid,
+      'event': deleted.event_name,
+      'method': deleted.method_name}, repo_name=repository_name)
 
     return 'No Content', 204
 
@@ -131,12 +132,15 @@ class RepositoryNotification(RepositoryParamResource):
     """ Resets repository notification to 0 failures. """
     reset = model.reset_notification_number_of_failures(namespace_name, repository_name, uuid)
     if not reset:
-      raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid))
+      raise InvalidRequest("No repository notification found for: %s, %s, %s" %
+                           (namespace_name, repository_name, uuid))
 
-    log_action('reset_repo_notification', namespace_name,
+    log_action('reset_repo_notification', namespace_name, {
-               {'repo': repository_name, 'namespace': namespace_name, 'notification_id': uuid,
+      'repo': repository_name,
-                'event': reset.event_name, 'method': reset.method_name},
+      'namespace': namespace_name,
-               repo_name=repository_name)
+      'notification_id': uuid,
+      'event': reset.event_name,
+      'method': reset.method_name}, repo_name=repository_name)
 
     return 'No Content', 204
 
@@ -146,14 +150,15 @@ class RepositoryNotification(RepositoryParamResource):
 @path_param('uuid', 'The UUID of the notification')
 class TestRepositoryNotification(RepositoryParamResource):
   """ Resource for queuing a test of a notification. """
+
   @require_repo_admin
   @nickname('testRepoNotification')
   @disallow_for_app_repositories
   def post(self, namespace_name, repository_name, uuid):
     """ Queues a test notification for this repository. """
     test_note = model.queue_test_notification(uuid)
-
     if not test_note:
-      raise InvalidRequest("No repository notification found for: %s, %s, %s" % (namespace_name, repository_name, uuid))
+      raise InvalidRequest("No repository notification found for: %s, %s, %s" %
+                           (namespace_name, repository_name, uuid))
 
     return {}, 200

endpoints/api/repositorynotification_models_interface.py

@@ -26,6 +26,7 @@ class RepositoryNotification(
   :type event_config: string
   :type number_of_failures: int
   """
+
   def to_dict(self):
     try:
       config = json.loads(self.config_json)
@@ -55,7 +56,8 @@ class RepoNotificationInterface(object):
   """
 
   @abstractmethod
-  def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, method_config, event_config, title=None):
+  def create_repo_notification(self, namespace_name, repository_name, event_name, method_name,
+                               method_config, event_config, title=None):
     """
 
     Args:

endpoints/api/repositorynotification_models_pre_oci.py

@@ -3,25 +3,25 @@ import json
 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.api.repositorynotification_models_interface import (RepoNotificationInterface,
-from endpoints.notificationevent import NotificationEvent
+                                                                   RepositoryNotification)
-from endpoints.notificationhelper import build_notification_data
+from notifications 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,
-  def create_repo_notification(self, namespace_name, repository_name, event_name, method_name, method_config, event_config, title=None):
+                               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(
-                                                                          event_name, 
+      model.notification.create_repo_notification(repository, event_name, method_name,
-                                                                          method_name, 
+                                                  method_config, event_config, title))
-                                                                          method_config, 
-                                                                          event_config, 
-                                                                          title))
 
   def list_repo_notifications(self, namespace_name, repository_name, event_name=None):
-    return [self._notification(n) 
+    return [
-            for n in model.notification.list_repo_notifications(namespace_name, repository_name, event_name)]
+      self._notification(n)
+      for n in model.notification.list_repo_notifications(namespace_name, repository_name,
+                                                          event_name)]
 
   def get_repo_notification(self, uuid):
     try:
@@ -39,7 +39,8 @@ 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))
+      model.notification.reset_notification_number_of_failures(namespace_name, repository_name,
+                                                               uuid))
 
   def queue_test_notification(self, uuid):
     try:
@@ -47,25 +48,25 @@ class RepoNotificationPreOCIModel(RepoNotificationInterface):
     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_queue.put([
-                            notification.event.name], json.dumps(notification_data))
+      notification.repository.namespace_user.username, notification.uuid, notification.event.name],
+                           json.dumps(notification_data))
     return self._notification(notification)
 
-
   def _notification(self, notification):
     if not notification:
       return None
-    return RepositoryNotification(uuid=notification.uuid,
-                                  title=notification.title,
-                                  event_name=notification.event.name,
-                                  method_name=notification.method.name,
-                                  config_json=notification.config_json,
-                                  event_config_json=notification.event_config_json,
-                                  number_of_failures=notification.number_of_failures)
 
+    return RepositoryNotification(
+      uuid=notification.uuid, title=notification.title, event_name=notification.event.name,
+      method_name=notification.method.name, config_json=notification.config_json,
+      event_config_json=notification.event_config_json,
+      number_of_failures=notification.number_of_failures)
 
 
 pre_oci_model = RepoNotificationPreOCIModel()

endpoints/building.py

@@ -9,7 +9,7 @@ from app import app, dockerfile_build_queue, metric_queue
 from data import model
 from data.database import db
 from auth.auth_context import get_authenticated_user
-from endpoints.notificationhelper import spawn_notification
+from notifications import spawn_notification
 from util.names import escape_tag
 from util.morecollections import AttrDict
 

endpoints/test/test_notificationevent.py

@@ -1,33 +0,0 @@
-import json
-
-from endpoints.notificationevent import NotificationEvent
-from util.morecollections import AttrDict
-
-from test.fixtures import *
-
-def test_all_notifications(app):
-  # Create a test notification.
-  test_notification = AttrDict({
-    'repository': AttrDict({
-      'namespace_name': AttrDict(dict(username='foo')),
-      'name': 'bar',
-    }),
-    'event_config_dict': {
-      'level': 'low',
-    },
-  })
-
-  for subc in NotificationEvent.__subclasses__():
-    if subc.event_name() is not None:
-      # Create the notification event.
-      found = NotificationEvent.get_event(subc.event_name())
-      sample_data = found.get_sample_data(test_notification)
-
-      # Make sure all calls succeed.
-      notification_data = {
-        'performer_data': {},
-      }
-
-      found.get_level(sample_data, notification_data)
-      found.get_summary(sample_data, notification_data)
-      found.get_message(sample_data, notification_data)

endpoints/v1/index.py

@@ -14,9 +14,9 @@ from auth.permissions import (
   CreateRepositoryPermission, repository_read_grant, repository_write_grant)
 from auth.signedgrant import generate_signed_token
 from endpoints.decorators import anon_protect, anon_allowed, parse_repository_name
-from endpoints.notificationhelper import spawn_notification
 from endpoints.v1 import v1_bp
 from endpoints.v1.models_pre_oci import pre_oci_model as model
+from notifications import spawn_notification
 from util.audit import track_and_log
 from util.http import abort
 from util.names import REPOSITORY_NAME_REGEX

endpoints/v2/manifest.py

@@ -10,7 +10,6 @@ from app import docker_v2_signing_key, app, metric_queue
 from auth.registry_jwt_auth import process_registry_jwt_auth
 from digest import digest_tools
 from endpoints.decorators import anon_protect, parse_repository_name
-from endpoints.notificationhelper import spawn_notification
 from endpoints.v2 import v2_bp, require_repo_read, require_repo_write
 from endpoints.v2.models_interface import Label
 from endpoints.v2.models_pre_oci import data_model as model
@@ -20,6 +19,7 @@ from endpoints.v2.labelhandlers import handle_label
 from image.docker import ManifestException
 from image.docker.schema1 import DockerSchema1Manifest, DockerSchema1ManifestBuilder
 from image.docker.schema2 import DOCKER_SCHEMA2_CONTENT_TYPES
+from notifications import spawn_notification
 from util.audit import track_and_log
 from util.names import VALID_TAG_PATTERN
 from util.registry.replication import queue_replication_batch

events/build_cancelled.html

@@ -0,0 +1,2 @@
+{% extends "build_event.html" %}
+{% block eventkind %}canceled{% endblock %}

notifications/__init__.py

@@ -5,17 +5,18 @@ 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):
+def build_repository_event_data(namespace_name, repo_name, extra_data=None, subpage=None):
-  repo_string = '%s/%s' % (_get_namespace_name_from(repo), 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'],
+                                        app.config['SERVER_HOSTNAME'], repo_string)
-                                        repo_string)
 
   if subpage:
     if not subpage.startswith('/'):
@@ -25,8 +26,8 @@ def build_event_data(repo, extra_data=None, subpage=None):
 
   event_data = {
     'repository': repo_string,
-    'namespace': _get_namespace_name_from(repo),
+    'namespace': namespace_name,
-    'name': repo.name,
+    'name': repo_name,
     'docker_url': '%s/%s' % (app.config['SERVER_HOSTNAME'], repo_string),
     'homepage': homepage,
   }
@@ -34,6 +35,7 @@ def build_event_data(repo, extra_data=None, subpage=None):
   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 = {}
@@ -64,12 +66,13 @@ def notification_batch(batch_size=DEFAULT_BATCH_SIZE):
   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)
+      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,
+      notifications = model.notification.list_repo_notifications(repo.namespace_name, repo.name,
-                                                                 repo.name,
                                                                  event_name=event_name)
       path = [repo.namespace_name, repo.name, event_name] + (pathargs or [])
       for notification in list(notifications):

notifications/models_interface.py

@@ -0,0 +1,16 @@
+from collections import namedtuple
+
+
+class Repository(namedtuple('Repository', ['namespace_name', 'name'])):
+  """
+  Repository represents a repository.
+  """
+
+
+class Notification(
+    namedtuple('Notification', [
+      'uuid', 'event_name', 'method_name', 'event_config_dict', 'method_config_dict',
+      'repository'])):
+  """
+  Notification represents a registered notification of some kind.
+  """

notifications/notificationevent.py

@@ -1,17 +1,16 @@
 import logging
 import time
-import json
 import re
 
 from datetime import datetime
-from endpoints.notificationhelper import build_event_data
+from notifications import build_repository_event_data
 from util.jinjautil import get_template_env
-from util.morecollections import AttrDict
 from util.secscan import PRIORITY_LEVELS, get_priority_for_index
 
-template_env = get_template_env("events")
 logger = logging.getLogger(__name__)
 
+TEMPLATE_ENV = get_template_env("events")
+
 class InvalidNotificationEventException(Exception):
   pass
 
@@ -36,12 +35,12 @@ class NotificationEvent(object):
     """
     Returns a human readable HTML message for the given notification data.
     """
-    return template_env.get_template(self.event_name() + '.html').render({
+    return TEMPLATE_ENV.get_template(self.event_name() + '.html').render({
       'event_data': event_data,
       'notification_data': notification_data
     })
 
-  def get_sample_data(self, notification):
+  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.
     """
@@ -68,6 +67,14 @@ class NotificationEvent(object):
 
     raise InvalidNotificationEventException('Unable to find event: %s' % eventname)
 
+  @classmethod
+  def event_names(cls):
+    for subc in cls.__subclasses__():
+      if subc.event_name() is None:
+        for subsubc in subc.__subclasses__():
+          yield subsubc.event_name()
+      else:
+        yield subc.event_name()
 
   @staticmethod
   def _get_event(cls, eventname):
@@ -91,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, notification):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
-    return build_event_data(notification.repository, {
+    return build_repository_event_data(namespace_name, repo_name, {
       'updated_tags': {'latest': 'someimageid', 'foo': 'anotherimage'},
       'pruned_image_count': 3
     })
@@ -125,10 +132,9 @@ class VulnerabilityFoundEvent(NotificationEvent):
 
     return 'info'
 
-  def get_sample_data(self, notification):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
-    event_config = notification.event_config_dict
     level = event_config.get(VulnerabilityFoundEvent.CONFIG_LEVEL, 'Critical')
-    return build_event_data(notification.repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'tags': ['latest', 'prod', 'foo', 'bar', 'baz'],
       'image': 'some-image-id',
       'vulnerability': {
@@ -213,9 +219,9 @@ class BuildQueueEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'info'
 
-  def get_sample_data(self, notification):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(notification.repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'is_manual': False,
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
@@ -251,9 +257,9 @@ class BuildStartEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'info'
 
-  def get_sample_data(self, notification):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(notification.repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
       'docker_tags': ['latest', 'foo', 'bar'],
@@ -278,9 +284,9 @@ class BuildSuccessEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'success'
 
-  def get_sample_data(self, notification):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(notification.repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
       'docker_tags': ['latest', 'foo', 'bar'],
@@ -306,9 +312,9 @@ class BuildFailureEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'error'
 
-  def get_sample_data(self, notification):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(notification.repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
       'docker_tags': ['latest', 'foo', 'bar'],
@@ -345,9 +351,9 @@ class BuildCancelledEvent(BaseBuildEvent):
   def get_level(self, event_data, notification_data):
     return 'info'
 
-  def get_sample_data(self, notification):
+  def get_sample_data(self, namespace_name, repo_name, event_config):
     build_uuid = 'fake-build-id'
-    return build_event_data(notification.repository, {
+    return build_repository_event_data(namespace_name, repo_name,  {
       'build_id': build_uuid,
       'build_name': 'some-fake-build',
       'docker_tags': ['latest', 'foo', 'bar'],
@@ -363,4 +369,3 @@ class BuildCancelledEvent(BaseBuildEvent):
 
   def get_summary(self, event_data, notification_data):
     return 'Build cancelled ' + _build_summary(event_data)
-

notifications/notificationmethod.py

@@ -1,6 +1,6 @@
-import json
 import logging
 import re
+import json
 
 import requests
 from flask_mail import Message
@@ -27,22 +27,11 @@ class NotificationMethodPerformException(JobException):
   pass
 
 
-def _get_namespace_name_from(repository):
+def _ssl_cert():
-  # TODO Charlie 2017-07-14: This is hack for a bug in production
+  if app.config['PREFERRED_URL_SCHEME'] == 'https':
-  # because in some places have started calling this method with
+    return [OVERRIDE_CONFIG_DIRECTORY + f for f in SSL_FILENAMES]
-  # pre oci models and in some we have started calling with non pre oci models. We should
-  # remove this when we have switched over to database interfaces.
-  if hasattr(repository, 'namespace_name'):
-    namespace_name = repository.namespace_name
-  else:
-    namespace_name = repository.namespace_user.username
-  return namespace_name
 
-
+  return None
-SSLClientCert = None
-if app.config['PREFERRED_URL_SCHEME'] == 'https':
-  # TODO(jschorr): move this into the config provider library
-  SSLClientCert = [OVERRIDE_CONFIG_DIRECTORY + f for f in SSL_FILENAMES]
 
 
 class NotificationMethod(object):
@@ -56,7 +45,7 @@ class NotificationMethod(object):
     """
     raise NotImplementedError
 
-  def validate(self, namespace_name, repository_name, 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.
@@ -68,7 +57,7 @@ class NotificationMethod(object):
     """
     Performs the notification method.
 
-    notification_obj: The noticication namedtuple.
+    notification_obj: The notification namedtuple.
     event_handler: The NotificationEvent handler.
     notification_data: The dict of notification data placed in the queue.
     """
@@ -88,13 +77,15 @@ class QuayNotificationMethod(NotificationMethod):
   def method_name(cls):
     return 'quay_notification'
 
-  def validate(self, namespace_name, repository_name, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
-    status, err_message, target_users = self.find_targets(namespace_name, repository_name, config_data)
+    _, err_message, _ = self.find_targets(namespace_name, config_data)
     if err_message:
       raise CannotValidateNotificationMethodException(err_message)
 
-  def find_targets(self, namespace_name, repository_name, config_data):
+  def find_targets(self, namespace_name, config_data):
-    target_info = config_data['target']
+    target_info = config_data.get('target', None)
+    if not target_info or not target_info.get('kind'):
+      return (True, 'Missing target', [])
 
     if target_info['kind'] == 'user':
       target = model.user.get_nonrobot_user(target_info['name'])
@@ -104,13 +95,13 @@ class QuayNotificationMethod(NotificationMethod):
 
       return (True, None, [target])
     elif target_info['kind'] == 'org':
-      target = model.organization.get_organization(target_info['name'])
+      try:
-      if not target:
+        target = model.organization.get_organization(target_info['name'])
-        # Just to be safe.
+      except model.organization.InvalidOrganizationException:
         return (True, 'Unknown organization %s' % target_info['name'], None)
 
       # Only repositories under the organization can cause notifications to that org.
-      if target_info['name'] != _get_namespace_name_from(repository):
+      if target_info['name'] != namespace_name:
         return (False, 'Organization name must match repository namespace')
 
       return (True, None, [target])
@@ -118,7 +109,7 @@ class QuayNotificationMethod(NotificationMethod):
       # Lookup the team.
       org_team = None
       try:
-        org_team = model.team.get_organization_team(_get_namespace_name_from(repository), 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)
@@ -134,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(_get_namespace_name_from(repository), repository.name, config_data)
+    status, err_message, target_users = self.find_targets(repository.namespace_name, config_data)
     if not status:
       raise NotificationMethodPerformException(err_message)
 
@@ -149,13 +140,12 @@ class EmailMethod(NotificationMethod):
   def method_name(cls):
     return 'email'
 
-  def validate(self, namespace_name, repository_name, 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(_get_namespace_name_from(repository),
+    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 +165,7 @@ class EmailMethod(NotificationMethod):
       try:
         mail.send(msg)
       except Exception as ex:
-        logger.exception('Email was unable to be sent: %s' % ex.message)
+        logger.exception('Email was unable to be sent')
         raise NotificationMethodPerformException(ex.message)
 
 
@@ -184,7 +174,7 @@ class WebhookMethod(NotificationMethod):
   def method_name(cls):
     return 'webhook'
 
-  def validate(self, namespace_name, repository_name, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
     url = config_data.get('url', '')
     if not url:
       raise CannotValidateNotificationMethodException('Missing webhook URL')
@@ -199,7 +189,7 @@ class WebhookMethod(NotificationMethod):
     headers = {'Content-type': 'application/json'}
 
     try:
-      resp = requests.post(url, data=json.dumps(payload), headers=headers, cert=SSLClientCert,
+      resp = requests.post(url, data=json.dumps(payload), headers=headers, cert=_ssl_cert(),
                            timeout=METHOD_TIMEOUT)
       if resp.status_code / 100 != 2:
         error_message = '%s response for webhook to url: %s' % (resp.status_code, url)
@@ -208,7 +198,7 @@ class WebhookMethod(NotificationMethod):
         raise NotificationMethodPerformException(error_message)
 
     except requests.exceptions.RequestException as ex:
-      logger.exception('Webhook was unable to be sent: %s' % ex.message)
+      logger.exception('Webhook was unable to be sent')
       raise NotificationMethodPerformException(ex.message)
 
 
@@ -221,7 +211,7 @@ class FlowdockMethod(NotificationMethod):
   def method_name(cls):
     return 'flowdock'
 
-  def validate(self, namespace_name, repository_name, 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')
@@ -232,7 +222,7 @@ class FlowdockMethod(NotificationMethod):
     if not token:
       return
 
-    owner = model.user.get_user_or_org(_get_namespace_name_from(notification_obj.repository))
+    owner = model.user.get_user_or_org(notification_obj.repository.namespace_name)
     if not owner:
       # Something went wrong.
       return
@@ -245,7 +235,7 @@ class FlowdockMethod(NotificationMethod):
       'subject': event_handler.get_summary(notification_data['event_data'], notification_data),
       'content': event_handler.get_message(notification_data['event_data'], notification_data),
       'from_name': owner.username,
-      'project': (_get_namespace_name_from(notification_obj.repository)+ ' ' +
+      'project': (notification_obj.repository.namespace_name + ' ' +
                   notification_obj.repository.name),
       'tags': ['#' + event_handler.event_name()],
       'link': notification_data['event_data']['homepage']
@@ -260,7 +250,7 @@ class FlowdockMethod(NotificationMethod):
         raise NotificationMethodPerformException(error_message)
 
     except requests.exceptions.RequestException as ex:
-      logger.exception('Flowdock method was unable to be sent: %s' % ex.message)
+      logger.exception('Flowdock method was unable to be sent')
       raise NotificationMethodPerformException(ex.message)
 
 
@@ -273,7 +263,7 @@ class HipchatMethod(NotificationMethod):
   def method_name(cls):
     return 'hipchat'
 
-  def validate(self, namespace_name, repository_name, 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')
 
@@ -288,7 +278,7 @@ class HipchatMethod(NotificationMethod):
     if not token or not room_id:
       return
 
-    owner = model.user.get_user_or_org(_get_namespace_name_from(notification_obj.repository))
+    owner = model.user.get_user_or_org(notification_obj.repository.namespace_name)
     if not owner:
       # Something went wrong.
       return
@@ -321,7 +311,7 @@ class HipchatMethod(NotificationMethod):
         raise NotificationMethodPerformException(error_message)
 
     except requests.exceptions.RequestException as ex:
-      logger.exception('Hipchat method was unable to be sent: %s' % ex.message)
+      logger.exception('Hipchat method was unable to be sent')
       raise NotificationMethodPerformException(ex.message)
 
 
@@ -384,7 +374,7 @@ class SlackMethod(NotificationMethod):
   def method_name(cls):
     return 'slack'
 
-  def validate(self, namespace_name, repository_name, config_data):
+  def validate(self, namespace_name, repo_name, config_data):
     if not config_data.get('url', ''):
       raise CannotValidateNotificationMethodException('Missing Slack Callback URL')
 
@@ -400,7 +390,7 @@ class SlackMethod(NotificationMethod):
     if not url:
       return
 
-    owner = model.user.get_user_or_org(_get_namespace_name_from(notification_obj.repository))
+    owner = model.user.get_user_or_org(notification_obj.repository.namespace_name)
     if not owner:
       # Something went wrong.
       return

notifications/test/test_notificationevent.py

@@ -0,0 +1,189 @@
+import pytest
+
+from notifications.notificationevent import (BuildSuccessEvent, NotificationEvent,
+                                             VulnerabilityFoundEvent)
+from util.morecollections import AttrDict
+
+from test.fixtures import *
+
+
+@pytest.mark.parametrize('event_kind', NotificationEvent.event_names())
+def test_create_notifications(event_kind):
+  assert NotificationEvent.get_event(event_kind) is not None
+
+
+@pytest.mark.parametrize('event_name', NotificationEvent.event_names())
+def test_build_notification(event_name, initialized_db):
+  # Create the notification event.
+  found = NotificationEvent.get_event(event_name)
+  sample_data = found.get_sample_data('foo', 'bar', {'level': 'low'})
+
+  # Make sure all calls succeed.
+  notification_data = {
+    'performer_data': {},
+  }
+
+  found.get_level(sample_data, notification_data)
+  found.get_summary(sample_data, notification_data)
+  found.get_message(sample_data, notification_data)
+
+
+def test_build_emptyjson():
+  notification_data = AttrDict({
+    'event_config_dict': None,
+  })
+
+  # No build data at all.
+  assert BuildSuccessEvent().should_perform({}, notification_data)
+
+def test_build_nofilter():
+  notification_data = AttrDict({
+    'event_config_dict': {},
+  })
+
+  # No build data at all.
+  assert BuildSuccessEvent().should_perform({}, notification_data)
+
+  # With trigger metadata but no ref.
+  assert BuildSuccessEvent().should_perform({
+    'trigger_metadata': {},
+  }, notification_data)
+
+  # With trigger metadata and a ref.
+  assert BuildSuccessEvent().should_perform({
+    'trigger_metadata': {
+      'ref': 'refs/heads/somebranch',
+    },
+  }, notification_data)
+
+
+def test_build_emptyfilter():
+  notification_data = AttrDict({
+    'event_config_dict': {"ref-regex": ""},
+  })
+
+  # No build data at all.
+  assert BuildSuccessEvent().should_perform({}, notification_data)
+
+  # With trigger metadata but no ref.
+  assert BuildSuccessEvent().should_perform({
+    'trigger_metadata': {},
+  }, notification_data)
+
+  # With trigger metadata and a ref.
+  assert BuildSuccessEvent().should_perform({
+    'trigger_metadata': {
+      'ref': 'refs/heads/somebranch',
+    },
+  }, notification_data)
+
+
+def test_build_invalidfilter():
+  notification_data = AttrDict({
+    'event_config_dict': {"ref-regex": "]["},
+  })
+
+  # No build data at all.
+  assert not BuildSuccessEvent().should_perform({}, notification_data)
+
+  # With trigger metadata but no ref.
+  assert not BuildSuccessEvent().should_perform({
+    'trigger_metadata': {},
+  }, notification_data)
+
+  # With trigger metadata and a ref.
+  assert not BuildSuccessEvent().should_perform({
+    'trigger_metadata': {
+      'ref': 'refs/heads/somebranch',
+    },
+  }, notification_data)
+
+
+def test_build_withfilter():
+  notification_data = AttrDict({
+    'event_config_dict': {"ref-regex": "refs/heads/master"},
+  })
+
+  # No build data at all.
+  assert not BuildSuccessEvent().should_perform({}, notification_data)
+
+  # With trigger metadata but no ref.
+  assert not BuildSuccessEvent().should_perform({
+    'trigger_metadata': {},
+  }, notification_data)
+
+  # With trigger metadata and a not-matching ref.
+  assert not BuildSuccessEvent().should_perform({
+    'trigger_metadata': {
+      'ref': 'refs/heads/somebranch',
+    },
+  }, notification_data)
+
+  # With trigger metadata and a matching ref.
+  assert BuildSuccessEvent().should_perform({
+    'trigger_metadata': {
+      'ref': 'refs/heads/master',
+    },
+  }, notification_data)
+
+
+def test_build_withwildcardfilter():
+  notification_data = AttrDict({
+    'event_config_dict': {"ref-regex": "refs/heads/.+"},
+  })
+
+  # No build data at all.
+  assert not BuildSuccessEvent().should_perform({}, notification_data)
+
+  # With trigger metadata but no ref.
+  assert not BuildSuccessEvent().should_perform({
+    'trigger_metadata': {},
+  }, notification_data)
+
+  # With trigger metadata and a not-matching ref.
+  assert not BuildSuccessEvent().should_perform({
+    'trigger_metadata': {
+      'ref': 'refs/tags/sometag',
+    },
+  }, notification_data)
+
+  # With trigger metadata and a matching ref.
+  assert BuildSuccessEvent().should_perform({
+    'trigger_metadata': {
+      'ref': 'refs/heads/master',
+    },
+  }, notification_data)
+
+  # With trigger metadata and another matching ref.
+  assert BuildSuccessEvent().should_perform({
+    'trigger_metadata': {
+      'ref': 'refs/heads/somebranch',
+    },
+  }, notification_data)
+
+
+def test_vulnerability_notification_nolevel():
+  notification_data = AttrDict({
+    'event_config_dict': {},
+  })
+
+  # No level specified.
+  assert VulnerabilityFoundEvent().should_perform({}, notification_data)
+
+
+def test_vulnerability_notification_nopvulninfo():
+  notification_data = AttrDict({
+    'event_config_dict': {"level": 3},
+  })
+
+  # No vuln info.
+  assert not VulnerabilityFoundEvent().should_perform({}, notification_data)
+
+
+def test_vulnerability_notification_normal():
+  notification_data = AttrDict({
+    'event_config_dict': {"level": 3},
+  })
+
+  info = {"vulnerability": {"priority": "Critical"}}
+  assert VulnerabilityFoundEvent().should_perform(info, notification_data)

notifications/test/test_notificationmethod.py

@@ -0,0 +1,156 @@
+import pytest
+
+from mock import patch, Mock
+from httmock import urlmatch, HTTMock
+
+from data import model
+from notifications.notificationmethod import (QuayNotificationMethod, EmailMethod, WebhookMethod,
+                                              FlowdockMethod, HipchatMethod, SlackMethod,
+                                              CannotValidateNotificationMethodException)
+from notifications.notificationevent import NotificationEvent
+from notifications.models_interface import Repository, Notification
+
+from test.fixtures import *
+
+def assert_validated(method, method_config, error_message, namespace_name, repo_name):
+  if error_message is None:
+    method.validate(namespace_name, repo_name, method_config)
+  else:
+    with pytest.raises(CannotValidateNotificationMethodException) as ipe:
+      method.validate(namespace_name, repo_name, method_config)
+    assert ipe.value.message == error_message
+
+
+@pytest.mark.parametrize('method_config,error_message', [
+  ({}, 'Missing target'),
+  ({'target': {'name': 'invaliduser', 'kind': 'user'}}, 'Unknown user invaliduser'),
+  ({'target': {'name': 'invalidorg', 'kind': 'org'}}, 'Unknown organization invalidorg'),
+  ({'target': {'name': 'invalidteam', 'kind': 'team'}}, 'Unknown team invalidteam'),
+
+  ({'target': {'name': 'devtable', 'kind': 'user'}}, None),
+  ({'target': {'name': 'buynlarge', 'kind': 'org'}}, None),
+  ({'target': {'name': 'owners', 'kind': 'team'}}, None),
+])
+def test_validate_quay_notification(method_config, error_message, initialized_db):
+  method = QuayNotificationMethod()
+  assert_validated(method, method_config, error_message, 'buynlarge', 'orgrepo')
+
+
+@pytest.mark.parametrize('method_config,error_message', [
+  ({}, 'Missing e-mail address'),
+  ({'email': 'a@b.com'}, 'The specified e-mail address is not authorized to receive '
+                         'notifications for this repository'),
+
+  ({'email': 'jschorr@devtable.com'}, None),
+])
+def test_validate_email(method_config, error_message, initialized_db):
+  method = EmailMethod()
+  assert_validated(method, method_config, error_message, 'devtable', 'simple')
+
+
+@pytest.mark.parametrize('method_config,error_message', [
+  ({}, 'Missing webhook URL'),
+  ({'url': 'http://example.com'}, None),
+])
+def test_validate_webhook(method_config, error_message, initialized_db):
+  method = WebhookMethod()
+  assert_validated(method, method_config, error_message, 'devtable', 'simple')
+
+
+@pytest.mark.parametrize('method_config,error_message', [
+  ({}, 'Missing Flowdock API Token'),
+  ({'flow_api_token': 'sometoken'}, None),
+])
+def test_validate_flowdock(method_config, error_message, initialized_db):
+  method = FlowdockMethod()
+  assert_validated(method, method_config, error_message, 'devtable', 'simple')
+
+
+@pytest.mark.parametrize('method_config,error_message', [
+  ({}, 'Missing Hipchat Room Notification Token'),
+  ({'notification_token': 'sometoken'}, 'Missing Hipchat Room ID'),
+  ({'notification_token': 'sometoken', 'room_id': 'foo'}, None),
+])
+def test_validate_hipchat(method_config, error_message, initialized_db):
+  method = HipchatMethod()
+  assert_validated(method, method_config, error_message, 'devtable', 'simple')
+
+
+@pytest.mark.parametrize('method_config,error_message', [
+  ({}, 'Missing Slack Callback URL'),
+  ({'url': 'http://example.com'}, None),
+])
+def test_validate_slack(method_config, error_message, initialized_db):
+  method = SlackMethod()
+  assert_validated(method, method_config, error_message, 'devtable', 'simple')
+
+
+@pytest.mark.parametrize('target,expected_users', [
+  ({'name': 'devtable', 'kind': 'user'}, ['devtable']),
+  ({'name': 'buynlarge', 'kind': 'org'}, ['buynlarge']),
+  ({'name': 'creators', 'kind': 'team'}, ['creator']),
+])
+def test_perform_quay_notification(target, expected_users, initialized_db):
+  repository = Repository('buynlarge', 'orgrepo')
+  notification = Notification(uuid='fake', event_name='repo_push', method_name='quay',
+                              event_config_dict={}, method_config_dict={'target': target},
+                              repository=repository)
+
+  event_handler = NotificationEvent.get_event('repo_push')
+
+  sample_data = event_handler.get_sample_data(repository.namespace_name, repository.name, {})
+
+  method = QuayNotificationMethod()
+  method.perform(notification, event_handler, {'event_data': sample_data})
+
+  # Ensure that the notification was written for all the expected users.
+  if target['kind'] != 'team':
+    user = model.user.get_namespace_user(target['name'])
+    assert len(model.notification.list_notifications(user, kind_name='repo_push')) > 0
+
+
+def test_perform_email(initialized_db):
+  repository = Repository('buynlarge', 'orgrepo')
+  notification = Notification(uuid='fake', event_name='repo_push', method_name='email',
+                              event_config_dict={}, method_config_dict={'email': 'test@example.com'},
+                              repository=repository)
+
+  event_handler = NotificationEvent.get_event('repo_push')
+  sample_data = event_handler.get_sample_data(repository.namespace_name, repository.name, {})
+
+  mock = Mock()
+  def get_mock(*args, **kwargs):
+    return mock
+
+  with patch('notifications.notificationmethod.Message', get_mock):
+    method = EmailMethod()
+    method.perform(notification, event_handler, {'event_data': sample_data, 'performer_data': {}})
+
+  mock.send.assert_called_once()
+
+
+@pytest.mark.parametrize('method, method_config, netloc', [
+  (WebhookMethod, {'url': 'http://testurl'}, 'testurl'),
+  (FlowdockMethod, {'flow_api_token': 'token'}, 'api.flowdock.com'),
+  (HipchatMethod, {'notification_token': 'token', 'room_id': 'foo'}, 'api.hipchat.com'),
+  (SlackMethod, {'url': 'http://example.com'}, 'example.com'),
+])
+def test_perform_http_call(method, method_config, netloc, initialized_db):
+  repository = Repository('buynlarge', 'orgrepo')
+  notification = Notification(uuid='fake', event_name='repo_push', method_name=method.method_name(),
+                              event_config_dict={}, method_config_dict=method_config,
+                              repository=repository)
+
+  event_handler = NotificationEvent.get_event('repo_push')
+  sample_data = event_handler.get_sample_data(repository.namespace_name, repository.name, {})
+
+  url_hit = [False]
+  @urlmatch(netloc=netloc)
+  def url_handler(_, __):
+    url_hit[0] = True
+    return ''
+
+  with HTTMock(url_handler):
+    method().perform(notification, event_handler, {'event_data': sample_data, 'performer_data': {}})
+
+  assert url_hit[0]

test/test_notifications.py

@@ -1,184 +0,0 @@
-import unittest
-
-from endpoints.notificationevent import (BuildSuccessEvent, NotificationEvent,
-                                         VulnerabilityFoundEvent)
-from util.morecollections import AttrDict
-
-class TestCreate(unittest.TestCase):
-  def test_create_notifications(self):
-    self.assertIsNotNone(NotificationEvent.get_event('repo_push'))
-    self.assertIsNotNone(NotificationEvent.get_event('build_queued'))
-    self.assertIsNotNone(NotificationEvent.get_event('build_success'))
-    self.assertIsNotNone(NotificationEvent.get_event('build_failure'))
-    self.assertIsNotNone(NotificationEvent.get_event('build_start'))
-    self.assertIsNotNone(NotificationEvent.get_event('build_cancelled'))
-    self.assertIsNotNone(NotificationEvent.get_event('vulnerability_found'))
-
-
-class TestShouldPerform(unittest.TestCase):
-  def test_build_emptyjson(self):
-    notification_data = AttrDict({
-      'event_config_dict': None,
-    })
-
-    # No build data at all.
-    self.assertTrue(BuildSuccessEvent().should_perform({}, notification_data))
-
-  def test_build_nofilter(self):
-    notification_data = AttrDict({
-      'event_config_dict': {},
-    })
-
-    # No build data at all.
-    self.assertTrue(BuildSuccessEvent().should_perform({}, notification_data))
-
-    # With trigger metadata but no ref.
-    self.assertTrue(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {},
-    }, notification_data))
-
-    # With trigger metadata and a ref.
-    self.assertTrue(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {
-        'ref': 'refs/heads/somebranch',
-      },
-    }, notification_data))
-
-
-  def test_build_emptyfilter(self):
-    notification_data = AttrDict({
-      'event_config_dict': {"ref-regex": ""},
-    })
-
-    # No build data at all.
-    self.assertTrue(BuildSuccessEvent().should_perform({}, notification_data))
-
-    # With trigger metadata but no ref.
-    self.assertTrue(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {},
-    }, notification_data))
-
-    # With trigger metadata and a ref.
-    self.assertTrue(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {
-        'ref': 'refs/heads/somebranch',
-      },
-    }, notification_data))
-
-
-  def test_build_invalidfilter(self):
-    notification_data = AttrDict({
-      'event_config_dict': {"ref-regex": "]["},
-    })
-
-    # No build data at all.
-    self.assertFalse(BuildSuccessEvent().should_perform({}, notification_data))
-
-    # With trigger metadata but no ref.
-    self.assertFalse(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {},
-    }, notification_data))
-
-    # With trigger metadata and a ref.
-    self.assertFalse(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {
-        'ref': 'refs/heads/somebranch',
-      },
-    }, notification_data))
-
-
-  def test_build_withfilter(self):
-    notification_data = AttrDict({
-      'event_config_dict': {"ref-regex": "refs/heads/master"},
-    })
-
-    # No build data at all.
-    self.assertFalse(BuildSuccessEvent().should_perform({}, notification_data))
-
-    # With trigger metadata but no ref.
-    self.assertFalse(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {},
-    }, notification_data))
-
-    # With trigger metadata and a not-matching ref.
-    self.assertFalse(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {
-        'ref': 'refs/heads/somebranch',
-      },
-    }, notification_data))
-
-    # With trigger metadata and a matching ref.
-    self.assertTrue(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {
-        'ref': 'refs/heads/master',
-      },
-    }, notification_data))
-
-
-  def test_build_withwildcardfilter(self):
-    notification_data = AttrDict({
-      'event_config_dict': {"ref-regex": "refs/heads/.+"},
-    })
-
-    # No build data at all.
-    self.assertFalse(BuildSuccessEvent().should_perform({}, notification_data))
-
-    # With trigger metadata but no ref.
-    self.assertFalse(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {},
-    }, notification_data))
-
-    # With trigger metadata and a not-matching ref.
-    self.assertFalse(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {
-        'ref': 'refs/tags/sometag',
-      },
-    }, notification_data))
-
-    # With trigger metadata and a matching ref.
-    self.assertTrue(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {
-        'ref': 'refs/heads/master',
-      },
-    }, notification_data))
-
-    # With trigger metadata and another matching ref.
-    self.assertTrue(BuildSuccessEvent().should_perform({
-      'trigger_metadata': {
-        'ref': 'refs/heads/somebranch',
-      },
-    }, notification_data))
-
-
-  def test_vulnerability_notification_nolevel(self):
-    notification_data = AttrDict({
-      'event_config_dict': {},
-    })
-
-    # No level specified.
-    self.assertTrue(VulnerabilityFoundEvent().should_perform({}, notification_data))
-
-
-  def test_vulnerability_notification_nopvulninfo(self):
-    notification_data = AttrDict({
-      'event_config_dict': {"level": 3},
-    })
-
-    # No vuln info.
-    self.assertFalse(VulnerabilityFoundEvent().should_perform({}, notification_data))
-
-
-  def test_vulnerability_notification_normal(self):
-    notification_data = AttrDict({
-      'event_config_dict': {"level": 3},
-    })
-
-    info = {"vulnerability": {"priority": "Critical"}}
-    self.assertTrue(VulnerabilityFoundEvent().should_perform(info, notification_data))
-
-
-
-
-if __name__ == '__main__':
-  unittest.main()
-

test/test_secscan.py

@@ -5,9 +5,9 @@ import unittest
 from app import app, storage, notification_queue
 from data import model
 from data.database import Image, IMAGE_NOT_SCANNED_ENGINE_VERSION
-from endpoints.notificationevent import VulnerabilityFoundEvent
 from endpoints.v2 import v2_bp
 from initdb import setup_database_for_testing, finished_database_for_testing
+from notifications.notificationevent import VulnerabilityFoundEvent
 from util.morecollections import AttrDict
 from util.secscan.api import SecurityScannerAPI, APIRequestFailure
 from util.secscan.analyzer import LayerAnalyzer

util/secscan/analyzer.py

@@ -5,10 +5,10 @@ from collections import defaultdict
 
 import features
 
-from endpoints.notificationhelper import spawn_notification
 from data.database import ExternalNotificationEvent, IMAGE_NOT_SCANNED_ENGINE_VERSION, Image
 from data.model.tag import filter_tags_have_repository_event, get_tags_for_image
 from data.model.image import set_secscan_status, get_image_with_storage_and_parent_base
+from notifications import spawn_notification
 from util.secscan import PRIORITY_LEVELS
 from util.secscan.api import (APIRequestFailure, AnalyzeLayerException, MissingParentLayerException,
                               InvalidLayerException, AnalyzeLayerRetryException)

util/secscan/notifier.py

@@ -10,7 +10,7 @@ from data.model.tag import (filter_has_repository_event, filter_tags_have_reposi
 
 from data.database import (Image, ImageStorage, ExternalNotificationEvent, Repository,
                            RepositoryTag)
-from endpoints.notificationhelper import notification_batch
+from notifications import notification_batch
 from util.secscan import PRIORITY_LEVELS
 from util.secscan.api import APIRequestFailure
 from util.morecollections import AttrDict, StreamingDiffTracker, IndexedStreamingDiffTracker

workers/notificationworker/models_interface.py

@@ -40,7 +40,7 @@ class NotificationWorkerDataInterface(object):
     pass
 
   @abstractmethod
-  def create_notification_for_testing(self, target_username):
+  def create_notification_for_testing(self, target_username, method_name=None, method_config=None):
     """ Creates a notification for testing. """
     pass
 

workers/notificationworker/models_pre_oci.py

@@ -29,16 +29,17 @@ class PreOCIModel(NotificationWorkerDataInterface):
   def increment_notification_failure_count(self, notification):
     model.notification.increment_notification_failure_count(notification.uuid)
 
-  def create_notification_for_testing(self, target_username):
+  def create_notification_for_testing(self, target_username, method_name='quay_notification',
+                                      method_config=None):
     repo = model.repository.get_repository('devtable', 'simple')
-    method_data = {
+    method_data = method_config or {
       'target': {
         'kind': 'user',
         'name': target_username,
       }
     }
-    notification = model.notification.create_repo_notification(repo, 'build_success',
+    notification = model.notification.create_repo_notification(repo, 'repo_push',
-                                                               'quay_notification', method_data, {})
+                                                               method_name, method_data, {})
     return notification.uuid
 
   def user_has_local_notifications(self, target_username):

workers/notificationworker/notificationworker.py

@@ -1,8 +1,8 @@
 import logging
 
 from app import notification_queue
-from endpoints.notificationmethod import NotificationMethod, InvalidNotificationMethodException
+from notifications.notificationmethod import NotificationMethod, InvalidNotificationMethodException
-from endpoints.notificationevent import NotificationEvent, InvalidNotificationEventException
+from notifications.notificationevent import NotificationEvent, InvalidNotificationEventException
 from workers.notificationworker.models_pre_oci import pre_oci_model as model
 from workers.queueworker import QueueWorker, JobException
 

workers/notificationworker/test/test_notificationworker.py

@@ -1,10 +1,20 @@
+import pytest
+
+from mock import patch, Mock
+from httmock import urlmatch, HTTMock
+
+from notifications.notificationmethod import (QuayNotificationMethod, EmailMethod, WebhookMethod,
+                                              FlowdockMethod, HipchatMethod, SlackMethod,
+                                              CannotValidateNotificationMethodException)
+from notifications.notificationevent import RepoPushEvent
+from notifications.models_interface import Repository
 from workers.notificationworker.notificationworker import NotificationWorker
 
 from test.fixtures import *
 
 from workers.notificationworker.models_pre_oci import pre_oci_model as model
 
-def test_basic_notification(initialized_db):
+def test_basic_notification_endtoend(initialized_db):
   # Ensure the public user doesn't have any notifications.
   assert not model.user_has_local_notifications('public')
 
@@ -21,3 +31,42 @@ def test_basic_notification(initialized_db):
 
   # Ensure the notification was handled.
   assert model.user_has_local_notifications('public')
+
+
+@pytest.mark.parametrize('method,method_config,netloc', [
+  (QuayNotificationMethod, {'target': {'name': 'devtable', 'kind': 'user'}}, None),
+  (EmailMethod, {'email': 'jschorr@devtable.com'}, None),
+  (WebhookMethod, {'url': 'http://example.com'}, 'example.com'),
+  (FlowdockMethod, {'flow_api_token': 'sometoken'}, 'api.flowdock.com'),
+  (HipchatMethod, {'notification_token': 'token', 'room_id': 'foo'}, 'api.hipchat.com'),
+  (SlackMethod, {'url': 'http://example.com'}, 'example.com'),
+])
+def test_notifications(method, method_config, netloc, initialized_db):
+  url_hit = [False]
+  @urlmatch(netloc=netloc)
+  def url_handler(_, __):
+    url_hit[0] = True
+    return ''
+
+  mock = Mock()
+  def get_mock(*args, **kwargs):
+    return mock
+
+  with patch('notifications.notificationmethod.Message', get_mock):
+    with HTTMock(url_handler):
+      # Add a basic build notification.
+      notification_uuid = model.create_notification_for_testing('public',
+                                                                method_name=method.method_name(),
+                                                                method_config=method_config)
+      event_data = RepoPushEvent().get_sample_data('devtable', 'simple', {})
+
+      # Fire off the queue processing.
+      worker = NotificationWorker(None)
+      worker.process_queue_item({
+        'notification_uuid': notification_uuid,
+        'event_data': event_data,
+        'performer_data': {},
+      })
+
+  if netloc is not None:
+    assert url_hit[0]