Merge pull request #1329 from coreos-inc/fasternotification

Make notification lookup faster and fix repo pagination on Postgres

data/model/notification.py

@@ -1,7 +1,5 @@
 import json
 
-from peewee import JOIN_LEFT_OUTER
-
 from data.model import InvalidNotificationException, db_transaction
 from data.database import (Notification, NotificationKind, User, Team, TeamMember, TeamRole,
                            RepositoryNotification, ExternalNotificationEvent, Repository,
@@ -17,7 +15,7 @@ def create_notification(kind_name, target, metadata={}):
 
 def create_unique_notification(kind_name, target, metadata={}):
   with db_transaction():
-    if list_notifications(target, kind_name, limit=1).count() == 0:
+    if list_notifications(target, kind_name).count() == 0:
       create_notification(kind_name, target, metadata)
 
 
@@ -31,45 +29,44 @@ def lookup_notification(user, uuid):
 
 def list_notifications(user, kind_name=None, id_filter=None, include_dismissed=False,
                        page=None, limit=None):
+
+  base_query = Notification.select().join(NotificationKind)
+
+  if kind_name is not None:
+    base_query = base_query.where(NotificationKind.name == kind_name)
+
+  if id_filter is not None:
+    base_query = base_query.where(Notification.uuid == id_filter)
+
+  if not include_dismissed:
+    base_query = base_query.where(Notification.dismissed == False)
+
+  # Lookup directly for the user.
+  user_direct = base_query.clone().where(Notification.target == user)
+
+  # Lookup via organizations admined by the user.
   Org = User.alias()
   AdminTeam = Team.alias()
   AdminTeamMember = TeamMember.alias()
   AdminUser = User.alias()
 
-  query = (Notification.select()
-           .join(User)
-           .switch(Notification)
-           .join(Org, JOIN_LEFT_OUTER, on=(Org.id == Notification.target))
-           .join(AdminTeam, JOIN_LEFT_OUTER, on=(Org.id == AdminTeam.organization))
-           .join(TeamRole, JOIN_LEFT_OUTER, on=(AdminTeam.role == TeamRole.id))
-           .switch(AdminTeam)
-           .join(AdminTeamMember, JOIN_LEFT_OUTER, on=(AdminTeam.id == AdminTeamMember.team))
-           .join(AdminUser, JOIN_LEFT_OUTER, on=(AdminTeamMember.user == AdminUser.id))
-           .where((Notification.target == user) |
-                  ((AdminUser.id == user) & (TeamRole.name == 'admin')))
-           .order_by(Notification.created)
-           .desc())
+  via_orgs = (base_query.clone()
+              .join(Org, on=(Org.id == Notification.target))
+              .join(AdminTeam, on=(Org.id == AdminTeam.organization))
+              .join(TeamRole, on=(AdminTeam.role == TeamRole.id))
+              .switch(AdminTeam)
+              .join(AdminTeamMember, on=(AdminTeam.id == AdminTeamMember.team))
+              .join(AdminUser, on=(AdminTeamMember.user == AdminUser.id))
+              .where((AdminUser.id == user) & (TeamRole.name == 'admin')))
 
-  if not include_dismissed:
-    query = query.switch(Notification).where(Notification.dismissed == False)
-
-  if kind_name:
-    query = (query
-             .switch(Notification)
-             .join(NotificationKind)
-             .where(NotificationKind.name == kind_name))
-
-  if id_filter:
-    query = (query
-             .switch(Notification)
-             .where(Notification.uuid == id_filter))
+  query = user_direct | via_orgs
 
   if page:
     query = query.paginate(page, limit)
   elif limit:
     query = query.limit(limit)
 
-  return query
+  return query.order_by(base_query.c.created.desc())
 
 
 def delete_all_notifications_by_kind(kind_name):

data/model/repository.py

@@ -254,7 +254,7 @@ def get_visible_repositories(username, namespace=None, include_public=False):
     return []
 
   query = (Repository
-           .select(Repository.name, Repository.id, Repository.description, Namespace.username,
+           .select(Repository.name, Repository.id.alias('id'), Repository.description, Namespace.username,
                    Repository.visibility)
            .distinct()
            .switch(Repository)

endpoints/api/repository.py

@@ -164,7 +164,7 @@ class RepositoryList(ApiResource):
     # Note: We only limit repositories when there isn't a namespace or starred filter, as they
     # result in far smaller queries.
     if not parsed_args['namespace'] and not parsed_args['starred']:
-      repos, next_page_token = model.modelutil.paginate(repo_query, RepositoryTable,
+      repos, next_page_token = model.modelutil.paginate(repo_query, repo_query.c,
                                                         page_token=page_token, limit=REPOS_PER_PAGE)
     else:
       repos = list(repo_query)

test/test_api_usage.py

@@ -314,11 +314,30 @@ class TestUserNotification(ApiTestCase):
     assert json['notifications']
     assert not json['notifications'][0]['dismissed']
 
+    notification = json['notifications'][0]
     pjson = self.putJsonResponse(UserNotification, params=dict(uuid=notification['id']),
                                  data=dict(dismissed=True))
 
     self.assertEquals(True, pjson['dismissed'])
 
+  def test_org_notifications(self):
+    # Create a notification on the organization.
+    org = model.user.get_user_or_org(ORGANIZATION)
+    model.notification.create_notification('test_notification', org, {'org': 'notification'})
+
+    # Ensure it is visible to the org admin.
+    self.login(ADMIN_ACCESS_USER)
+    json = self.getJsonResponse(UserNotificationList)
+    notification = json['notifications'][0]
+
+    self.assertEquals(notification['kind'], 'test_notification')
+    self.assertEquals(notification['metadata'], {'org': 'notification'})
+
+    # Ensure it is not visible to an org member.
+    self.login(READ_ACCESS_USER)
+    json = self.getJsonResponse(UserNotificationList)
+    self.assertEquals(0, len(json['notifications']))
+
 
 class TestGetUserPrivateAllowed(ApiTestCase):
   def test_nonallowed(self):