Merge pull request #258 from coreos-inc/repolistapi

Clean up the repository list API and loads stars with it

data/model/repository.py

@@ -147,6 +147,9 @@ def repository_is_starred(user, repository):
 
 
 def get_when_last_modified(repository_ids):
+  if not repository_ids:
+    return {}
+
   tuples = (RepositoryTag
             .select(RepositoryTag.repository, fn.Max(RepositoryTag.lifetime_start_ts))
             .where(RepositoryTag.repository << repository_ids)
@@ -161,6 +164,9 @@ def get_when_last_modified(repository_ids):
 
 
 def get_action_counts(repository_ids):
+  if not repository_ids:
+    return {}
+
   # Filter the join to recent entries only.
   last_week = datetime.now() - timedelta(weeks=1)
   tuples = (RepositoryActionCount
@@ -177,25 +183,26 @@ def get_action_counts(repository_ids):
   return action_count_map
 
 
-def get_visible_repositories(username=None, include_public=True, page=None,
+def get_visible_repositories(username, namespace=None, page=None, limit=None, include_public=False):
-                             limit=None, namespace=None, namespace_only=False):
+  """ Returns the repositories visible to the given user (if any).
+  """
   if not include_public and not username:
     return []
 
   fields = [Repository.name, Repository.id, Repository.description, Visibility.name,
             Namespace.username]
 
-  query = _visible_repository_query(username=username, include_public=include_public, page=page,
+  query = _visible_repository_query(username=username, page=page,
-                                    limit=limit, namespace=namespace,
+                                    limit=limit, namespace=namespace, include_public=include_public,
                                     select_models=fields)
 
   if limit:
     query = query.limit(limit)
 
-  if namespace and namespace_only:
+  if namespace:
     query = query.where(Namespace.username == namespace)
 
-  return TupleSelector(query, fields)
+  return query
 
 
 def _visible_repository_query(username=None, include_public=True, limit=None,

endpoints/api/repository.py

@@ -102,45 +102,69 @@ class RepositoryList(ApiResource):
 
     raise Unauthorized()
 
+  def _load_repositories(self, namespace=None, public=False, starred=False, limit=None, page=None):
+    """ Loads the filtered list of repositories and returns it and the star lookup set. """
+    # Load the starred repositories and username (if applicable)
+    username = get_authenticated_user().username if get_authenticated_user() else None
+
+    # If starred (and only starred) repositories were requested, then load them directly.
+    if starred and namespace is None and not public:
+      if not username:
+        return []
+
+      repositories = model.repository.get_user_starred_repositories(get_authenticated_user(),
+                                                                    limit=limit,
+                                                                    page=page)
+
+      return repositories, set([repo.id for repo in repositories])
+
+    # Otherwise, conduct a full filtered lookup and (optionally) filter by the starred repositories.
+    starred_repos = []
+    star_lookup = set()
+
+    if username:
+      starred_repos = model.repository.get_user_starred_repositories(get_authenticated_user())
+      star_lookup = set([repo.id for repo in starred_repos])
+
+    # Find the matching repositories.
+    repositories = model.repository.get_visible_repositories(username=username,
+                                                             limit=limit,
+                                                             page=page,
+                                                             include_public=public,
+                                                             namespace=namespace)
+
+    # Filter down to just the starred repositories, if asked.
+    if starred:
+      return [repo for repo in repositories if repo.id in star_lookup], star_lookup
+
+    return repositories, star_lookup
+
+
   @require_scope(scopes.READ_REPO)
   @nickname('listRepos')
   @parse_args
   @query_param('page', 'Offset page number. (int)', type=int)
   @query_param('limit', 'Limit on the number of results (int)', type=int)
-  @query_param('namespace', 'Namespace to use when querying for org repositories.', type=str)
+  @query_param('namespace', 'Filters the repositories returned to this namespace', type=str)
-  @query_param('public', 'Whether to include repositories not explicitly visible by the user.',
+  @query_param('starred', 'Filters the repositories returned to those starred by the user',
-                         type=truthy_bool, default=True)
+               type=truthy_bool, default=False)
-  @query_param('private', 'Whether to include private repositories.', type=truthy_bool,
+  @query_param('public', 'Adds any repositories visible to the user by virtue of being public',
-               default=True)
-  @query_param('namespace_only', 'Whether to limit only to the given namespace.',
                type=truthy_bool, default=False)
   @query_param('last_modified', 'Whether to include when the repository was last modified.',
                type=truthy_bool, default=False)
   @query_param('popularity', 'Whether to include the repository\'s popularity metric.',
                type=truthy_bool, default=False)
   def get(self, args):
-    """Fetch the list of repositories under a variety of situations."""
+    """ Fetch the list of repositories visible to the current user under a variety of situations.
-    username = None
+    """
-    if get_authenticated_user():
-      starred_repos = model.repository.get_user_starred_repositories(get_authenticated_user())
-      star_lookup = set([repo.id for repo in starred_repos])
 
-      if args['private']:
+    repositories, star_lookup = self._load_repositories(args['namespace'], args['public'],
-        username = get_authenticated_user().username
+                                                        args['starred'], args['limit'],
-
+                                                        args['page'])
-    response = {}
-
-    # Find the matching repositories.
-    repo_query = model.repository.get_visible_repositories(username,
-                                                           limit=args['limit'],
-                                                           page=args['page'],
-                                                           include_public=args['public'],
-                                                           namespace=args['namespace'],
-                                                           namespace_only=args['namespace_only'])
 
     # Collect the IDs of the repositories found for subequent lookup of popularity
     # and/or last modified.
-    repository_ids = [repo.get(RepositoryTable.id) for repo in repo_query]
+    repository_ids = [repo.id for repo in repositories]
 
     if args['last_modified']:
       last_modified_map = model.repository.get_when_last_modified(repository_ids)
@@ -150,13 +174,13 @@ class RepositoryList(ApiResource):
 
     def repo_view(repo_obj):
       repo = {
-        'namespace': repo_obj.get(Namespace.username),
+        'namespace': repo_obj.namespace_user.username,
-        'name': repo_obj.get(RepositoryTable.name),
+        'name': repo_obj.name,
-        'description': repo_obj.get(RepositoryTable.description),
+        'description': repo_obj.description,
-        'is_public': repo_obj.get(Visibility.name) == 'public'
+        'is_public': repo_obj.visibility.name == 'public'
       }
 
-      repo_id = repo_obj.get(RepositoryTable.id)
+      repo_id = repo_obj.id
 
       if args['last_modified']:
         repo['last_modified'] = last_modified_map.get(repo_id)
@@ -169,8 +193,9 @@ class RepositoryList(ApiResource):
 
       return repo
 
-    response['repositories'] = [repo_view(repo) for repo in repo_query]
+    return {
-    return response
+      'repositories': [repo_view(repo) for repo in repositories]
+    }
 
 
 @resource('/v1/repository/<repopath:repository>')

initdb.py

@@ -385,8 +385,8 @@ def populate_database():
   __generate_repository(new_user_4, 'randomrepo', 'Random repo repository.', False,
                         [], (4, [], ['latest', 'prod']))
 
-  __generate_repository(new_user_1, 'simple', 'Simple repository.', False,
+  simple_repo = __generate_repository(new_user_1, 'simple', 'Simple repository.', False,
-                        [], (4, [], ['latest', 'prod']))
+                                      [], (4, [], ['latest', 'prod']))
 
   __generate_repository(new_user_1, 'sharedtags',
                         'Shared tags repository',
@@ -457,6 +457,8 @@ def populate_database():
     }
   }
 
+  model.repository.star_repository(new_user_1, simple_repo)
+
   record = model.repository.create_email_authorization_for_repo(new_user_1.username, 'simple',
                                                                 'jschorr@devtable.com')
   record.confirmed = True

static/js/pages/org-view.js

@@ -30,7 +30,6 @@
 
     var loadRepositories = function() {
       var options = {
-        'namespace_only': true,
         'namespace': orgname,
       };
 

static/js/pages/repo-list.js

@@ -80,7 +80,13 @@
         return;
       }
 
-      $scope.starred_repositories = ApiService.listStarredReposAsResource().get(function(resp) {
+      var options = {
+        'starred': true,
+        'last_modified': true,
+        'popularity': true
+      };
+
+      $scope.starred_repositories = ApiService.listReposAsResource().withOptions(options).get(function(resp) {
         return resp.repositories.map(function(repo) {
           repo = findDuplicateRepo(repo);
           repo.is_starred = true;
@@ -96,8 +102,6 @@
 
       $scope.namespaces.map(function(namespace) {
         var options = {
-          'public': false,
-          'sort': true,
           'namespace': namespace.name,
           'last_modified': true,
           'popularity': true

static/js/pages/user-view.js

@@ -24,7 +24,6 @@
     var loadRepositories = function() {
       var options = {
         'sort': true,
-        'namespace_only': true,
         'namespace': username,
       };
 

test/test_api_usage.py

@@ -1281,30 +1281,45 @@ class TestListRepos(ApiTestCase):
                                 params=dict(namespace=ORGANIZATION,
                                             public=False))
 
+    self.assertTrue(len(json['repositories']) > 0)
+
     for repo in json['repositories']:
       self.assertEquals(ORGANIZATION, repo['namespace'])
 
   def test_listrepos_limit(self):
     self.login(READ_ACCESS_USER)
-    json = self.getJsonResponse(RepositoryList, params=dict(limit=2))
+    json = self.getJsonResponse(RepositoryList, params=dict(limit=1))
-
+    self.assertEquals(len(json['repositories']), 1)
-    self.assertEquals(len(json['repositories']), 2)
-
-  def test_action_last_modified(self):
-    self.login(READ_ACCESS_USER)
-    json = self.getJsonResponse(RepositoryList, params=dict(last_modified=True, popularity=True))
-    self.assertTrue(len(json['repositories']) > 2)
 
   def test_listrepos_allparams(self):
     self.login(ADMIN_ACCESS_USER)
-    json = self.getJsonResponse(RepositoryList,
+
-                                params=dict(namespace=ORGANIZATION,
+    # Queries: Base + the list query + the popularity and last modified queries
-                                            public=False,
+    with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 3):
-                                            last_modified=True))
+      json = self.getJsonResponse(RepositoryList,
+                                  params=dict(namespace=ORGANIZATION,
+                                              public=False,
+                                              last_modified=True,
+                                              popularity=True))
+
+      self.assertTrue(len(json['repositories']) > 0)
 
     for repo in json['repositories']:
       self.assertEquals(ORGANIZATION, repo['namespace'])
 
+  def test_listrepos_starred(self):
+    self.login(ADMIN_ACCESS_USER)
+
+    json = self.getJsonResponse(RepositoryList,
+                                params=dict(last_modified=True,
+                                            popularity=True,
+                                            starred=True))
+
+    self.assertTrue(len(json['repositories']) > 0)
+
+    for repo in json['repositories']:
+      self.assertTrue(repo['is_starred'])
+
   def test_listrepos_asguest_allparams(self):
     json = self.getJsonResponse(RepositoryList,
                                 params=dict(namespace=ORGANIZATION,