feat(full-stack): disable notifications after 3 failures

This stops notifications from firing over and over again if they are repeatedly failing.

[TESTING -> locally with docker compose, DATABASE MIGRATION -> there is a single migration]

Issue: https://www.pivotaltracker.com/story/show/b144646649n

- [ ] It works!
- [ ] Comments provide sufficient explanations for the next contributor
- [ ] Tests cover changes and corner cases
- [ ] Follows Quay syntax patterns and format

data/database.py

@@ -1006,6 +1006,7 @@ class RepositoryNotification(BaseModel):
   title = CharField(null=True)
   config_json = TextField()
   event_config_json = TextField(default='{}')
+  number_of_failures = IntegerField(default=0)
 
 
 class RepositoryAuthorizedEmail(BaseModel):

data/migrations/versions/dc4af11a5f90_add_notification_number_of_failures_.py

@@ -0,0 +1,29 @@
+"""add notification number of failures column
+
+Revision ID: dc4af11a5f90
+Revises: 53e2ac668296
+Create Date: 2017-05-16 17:24:02.630365
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'dc4af11a5f90'
+down_revision = '53e2ac668296'
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade(tables):
+  op.add_column('repositorynotification', sa.Column('number_of_failures', sa.Integer(), nullable=False))
+  op.bulk_insert(tables.logentrykind, [
+    {'name': 'reset_repo_notification'},
+  ])
+
+
+def downgrade(tables):
+  op.drop_column('repositorynotification', 'number_of_failures')
+  op.execute(tables
+             .logentrykind
+             .delete()
+             .where(tables.logentrykind.c.name == op.inline_literal('reset_repo_notification')))

data/model/notification.py

@@ -1,9 +1,9 @@
 import json
 
-from data.model import InvalidNotificationException, db_transaction
 from data.database import (Notification, NotificationKind, User, Team, TeamMember, TeamRole,
                            RepositoryNotification, ExternalNotificationEvent, Repository,
-                           ExternalNotificationMethod, Namespace)
+                           ExternalNotificationMethod, Namespace, db_for_update)
+from data.model import InvalidNotificationException, db_transaction
 
 
 def create_notification(kind_name, target, metadata={}, lookup_path=None):
@@ -125,6 +125,29 @@ def delete_matching_notifications(target, kind_name, **kwargs):
     notification.delete_instance()
 
 
+def increment_notification_failure_count(notification_id):
+  """ This increments the number of failures by one """
+  RepositoryNotification.update(number_of_failures=RepositoryNotification.number_of_failures + 1).where(
+    RepositoryNotification.id == notification_id).execute()
+
+
+def reset_notification_number_of_failures(namespace_name, repository_name, uuid):
+  """ This resets the number of failures for a repo notification to 0 """
+  try:
+    notification = RepositoryNotification.select().where(RepositoryNotification.uuid == uuid).get()
+    if (notification.repository.namespace_user.username != namespace_name or
+            notification.repository.name != repository_name):
+      raise InvalidNotificationException('No repository notification found with uuid: %s' % uuid)
+    reset_number_of_failures_to_zero(notification.id)
+  except RepositoryNotification.DoesNotExist:
+    pass
+
+
+def reset_number_of_failures_to_zero(notification_id):
+  """ This resets the number of failures for a repo notification to 0 """
+  RepositoryNotification.update(number_of_failures=0).where(RepositoryNotification.id == notification_id).execute()
+
+
 def create_repo_notification(repo, event_name, method_name, method_config, event_config, title=None):
   event = ExternalNotificationEvent.get(ExternalNotificationEvent.name == event_name)
   method = ExternalNotificationMethod.get(ExternalNotificationMethod.name == method_name)
@@ -134,23 +157,34 @@ def create_repo_notification(repo, event_name, method_name, method_config, event
                                        event_config_json=json.dumps(event_config))
 
 
-def get_repo_notification(uuid):
-  try:
+def _base_get_notification(uuid):
+  """ This is a base query for get statements """
   return (RepositoryNotification
           .select(RepositoryNotification, Repository, Namespace)
           .join(Repository)
           .join(Namespace, on=(Repository.namespace_user == Namespace.id))
-            .where(RepositoryNotification.uuid == uuid)
-            .get())
+          .where(RepositoryNotification.uuid == uuid))
+
+
+def get_enabled_notification(uuid):
+  """ This returns a notification with less than 3 failures """
+  try:
+    return _base_get_notification(uuid).where(RepositoryNotification.number_of_failures < 3).get()
   except RepositoryNotification.DoesNotExist:
-    raise InvalidNotificationException('No repository notification found with id: %s' % uuid)
+    raise InvalidNotificationException('No repository notification found with uuid: %s' % uuid)
+
+
+def get_repo_notification(uuid):
+  try:
+    return _base_get_notification(uuid).get()
+  except RepositoryNotification.DoesNotExist:
+    raise InvalidNotificationException('No repository notification found with uuid: %s' % uuid)
 
 
 def delete_repo_notification(namespace_name, repository_name, uuid):
   found = get_repo_notification(uuid)
-  if (found.repository.namespace_user.username != namespace_name or
-      found.repository.name != repository_name):
-    raise InvalidNotificationException('No repository notifiation found with id: %s' % uuid)
+  if found.repository.namespace_user.username != namespace_name or found.repository.name != repository_name:
+    raise InvalidNotificationException('No repository notifiation found with uuid: %s' % uuid)
   found.delete_instance()
   return found
 

endpoints/api/repositorynotification.py

@@ -2,6 +2,7 @@
 
 import json
 
+import logging
 from flask import request
 
 from app import notification_queue
@@ -14,7 +15,7 @@ from endpoints.notificationmethod import (NotificationMethod,
                                           CannotValidateNotificationMethodException)
 from endpoints.notificationhelper import build_notification_data
 from data import model
-
+logger = logging.getLogger(__name__)
 
 def notification_view(note):
   config = {}
@@ -36,6 +37,7 @@ def notification_view(note):
     'config': config,
     'title': note.title,
     'event_config': event_config,
+    'number_of_failures': note.number_of_failures,
   }
 
 
@@ -154,6 +156,18 @@ class RepositoryNotification(RepositoryParamResource):
 
     return 'No Content', 204
 
+  @require_repo_admin
+  @nickname('resetRepositoryNotificationFailures')
+  @disallow_for_app_repositories
+  def post(self, namespace, repository, uuid):
+    """ Resets repository notification to 0 failures. """
+    model.notification.reset_notification_number_of_failures(namespace, repository, uuid)
+    log_action('reset_repo_notification', namespace,
+               {'repo': repository, 'namespace': namespace, 'notification_id': uuid},
+               repo=model.repository.get_repository(namespace, repository))
+
+    return 'No Content', 204
+
 
 @resource('/v1/repository/<apirepopath:repository>/notification/<uuid>/test')
 @path_param('repository', 'The full path of the repository. e.g. namespace/name')

endpoints/api/test/test_disallow_for_apps.py

@@ -45,6 +45,7 @@ FIELD_ARGS = {'trigger_uuid': '1234', 'field_name': 'foobar'}
   (RepositoryNotificationList, 'post', None),
   (RepositoryNotification, 'get', NOTIFICATION_ARGS),
   (RepositoryNotification, 'delete', NOTIFICATION_ARGS),
+  (RepositoryNotification, 'post', NOTIFICATION_ARGS),
   (TestRepositoryNotification, 'post', NOTIFICATION_ARGS),
   (RepositoryImageSecurity, 'get', IMAGE_ARGS),
   (RepositoryManifestSecurity, 'get', MANIFEST_ARGS),

endpoints/api/test/test_security.py

@@ -2,6 +2,7 @@ import pytest
 from flask_principal import AnonymousIdentity
 
 from endpoints.api import api
+from endpoints.api.repositorynotification import RepositoryNotification
 from endpoints.api.team import OrganizationTeamSyncing
 from endpoints.api.test.shared import client_with_identity, conduct_api_call
 from endpoints.api.repository import RepositoryTrust
@@ -16,6 +17,8 @@ TEAM_PARAMS = {'orgname': 'buynlarge', 'teamname': 'owners'}
 BUILD_PARAMS = {'build_uuid': 'test-1234'}
 REPO_PARAMS = {'repository': 'devtable/someapp'}
 SEARCH_PARAMS = {'query': ''}
+NOTIFICATION_PARAMS = {'namespace': 'devtable', 'repository': 'devtable/simple', 'uuid': 'some uuid'}
+
 
 @pytest.mark.parametrize('resource,method,params,body,identity,expected', [
   (OrganizationTeamSyncing, 'POST', TEAM_PARAMS, {}, None, 403),
@@ -52,6 +55,11 @@ SEARCH_PARAMS = {'query': ''}
   (RepositorySignatures, 'GET', REPO_PARAMS, {}, 'reader', 403),
   (RepositorySignatures, 'GET', REPO_PARAMS, {}, 'devtable', 404),
 
+  (RepositoryNotification, 'POST', NOTIFICATION_PARAMS, {}, None, 403),
+  (RepositoryNotification, 'POST', NOTIFICATION_PARAMS, {}, 'freshuser', 403),
+  (RepositoryNotification, 'POST', NOTIFICATION_PARAMS, {}, 'reader', 403),
+  (RepositoryNotification, 'POST', NOTIFICATION_PARAMS, {}, 'devtable', 204),
+
   (RepositoryTrust, 'POST', REPO_PARAMS, {'trust_enabled': True}, None, 403),
   (RepositoryTrust, 'POST', REPO_PARAMS, {'trust_enabled': True}, 'freshuser', 403),
   (RepositoryTrust, 'POST', REPO_PARAMS, {'trust_enabled': True}, 'reader', 403),

endpoints/notificationmethod.py

@@ -51,6 +51,7 @@ class NotificationMethod(object):
     """
     raise NotImplementedError
 
+
   def perform(self, notification_obj, event_handler, notification_data):
     """
     Performs the notification method.
@@ -114,7 +115,6 @@ class QuayNotificationMethod(NotificationMethod):
       # Lookup the team's members
       return (True, None, model.organization.get_organization_team_members(org_team.id))
 
-
   def perform(self, notification_obj, event_handler, notification_data):
     repository = notification_obj.repository
     if not repository:

initdb.py

@@ -332,6 +332,7 @@ def initialize_database():
 
   LogEntryKind.create(name='add_repo_notification')
   LogEntryKind.create(name='delete_repo_notification')
+  LogEntryKind.create(name='reset_repo_notification')
 
   LogEntryKind.create(name='regenerate_robot_token')
 

static/directives/repository-events-table.html

@@ -29,6 +29,7 @@
                 <td>Title</td>
                 <td>Event</td>
                 <td>Notification</td>
+                <td>Enabled</td>
                 <td class="options-col"></td>
               </tr>
             </thead>
@@ -70,6 +71,11 @@
                   </span>
                 </td>
 
+                <td>
+                  <span ng-if="notification.number_of_failures >= 3">Disabled due to 3 failed attempts in a row</span>
+                  <span ng-if="notification.number_of_failures < 3">Enabled</span>
+                </td>
+
                 <td>
                   <span class="cor-options-menu">
                     <span class="cor-option" option-click="testNotification(notification)">
@@ -93,6 +99,9 @@
                     <span class="cor-option" option-click="deleteNotification(notification)">
                       <i class="fa fa-times"></i> Delete Notification
                     </span>
+                    <span ng-if="notification.number_of_failures >= 3" class="cor-option" option-click="reenableNotification(notification)">
+                      <i class="fa fa-adjust"></i> Re-enable Notification
+                    </span>
                   </span>
                 </td>
               </tr>

static/js/directives/ui/logs-view.js

@@ -225,6 +225,11 @@ angular.module('quay').directive('logsView', function () {
             return 'Delete notification of event "' + eventData['title'] + '" for repository {namespace}/{repo}';
           },
 
+          'reset_repo_notification': function(metadata) {
+            var eventData = ExternalNotificationData.getEventInfo(metadata.event);
+            return 'Re-enable notification of event "' + eventData['title'] + '" for repository {namespace}/{repo}';
+          },
+
           'regenerate_robot_token': 'Regenerated token for robot {robot}',
 
           'service_key_create': function(metadata) {
@@ -302,6 +307,7 @@ angular.module('quay').directive('logsView', function () {
         'reset_application_client_secret': 'Reset Client Secret',
         'add_repo_notification': 'Add repository notification',
         'delete_repo_notification': 'Delete repository notification',
+        'reset_repo_notification': 'Re-enable repository notification',
         'regenerate_robot_token': 'Regenerate Robot Token',
         'service_key_create': 'Create Service Key',
         'service_key_approve': 'Approve Service Key',

static/js/directives/ui/repository-events-table.js

@@ -93,6 +93,20 @@ angular.module('quay').directive('repositoryEventsTable', function () {
         }, ApiService.errorDisplay('Cannot delete notification'));
       };
 
+      $scope.reenableNotification = function(notification) {
+        var params = {
+          'repository': $scope.repository.namespace + '/' + $scope.repository.name,
+          'uuid': notification.uuid
+        };
+
+        ApiService.resetRepositoryNotificationFailures(null, params).then(function() {
+          var index = $.inArray(notification, $scope.notifications);
+          if (index < 0) { return; }
+          $scope.notifications[index].number_of_failures = 0
+        }, ApiService.errorDisplay('Cannot re-enable notification'));
+      };
+
+
       $scope.showNotifyInfo = function(notification, field) {
         var dom = document.createElement('input');
         dom.setAttribute('type', 'text');

workers/notificationworker.py

@@ -1,6 +1,5 @@
-import logging
-
 from app import notification_queue
+from data.model.notification import increment_notification_failure_count, reset_number_of_failures_to_zero
 
 from endpoints.notificationmethod import NotificationMethod, InvalidNotificationMethodException
 from endpoints.notificationevent import NotificationEvent, InvalidNotificationEventException
@@ -9,7 +8,7 @@ from workers.queueworker import QueueWorker, JobException
 from data import model
 from data.model import InvalidNotificationException
 
-logger = logging.getLogger(__name__)
+
 
 
 class NotificationWorker(QueueWorker):
@@ -17,9 +16,8 @@ class NotificationWorker(QueueWorker):
     notification_uuid = job_details['notification_uuid']
 
     try:
-      notification = model.notification.get_repo_notification(notification_uuid)
+      notification = model.notification.get_enabled_notification(notification_uuid)
     except InvalidNotificationException:
-      # Probably deleted.
       return
 
     event_name = notification.event.name
@@ -36,7 +34,12 @@ class NotificationWorker(QueueWorker):
       raise JobException('Cannot find notification event: %s' % ex.message)
 
     if event_handler.should_perform(job_details['event_data'], notification):
+      try:
         method_handler.perform(notification, event_handler, job_details)
+        reset_number_of_failures_to_zero(notification.id)
+      except (NotificationMethod, KeyError) as exc:
+        increment_notification_failure_count(notification.id)
+        raise exc
 
 
 if __name__ == "__main__":