Clean up the repository list API and loads stars with it

We load stars with the same list API now so that we get the extra metadata needed in the repo list (popularity and last modified)

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,