diff --git a/data/model/repository.py b/data/model/repository.py index 368dfab24..90857f93c 100644 --- a/data/model/repository.py +++ b/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, - limit=None, namespace=None, namespace_only=False): +def get_visible_repositories(username, namespace=None, page=None, limit=None, include_public=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, - limit=limit, namespace=namespace, + query = _visible_repository_query(username=username, page=page, + 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, diff --git a/endpoints/api/repository.py b/endpoints/api/repository.py index 80b0ce82f..344633e2e 100644 --- a/endpoints/api/repository.py +++ b/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('public', 'Whether to include repositories not explicitly visible by the user.', - type=truthy_bool, default=True) - @query_param('private', 'Whether to include private repositories.', type=truthy_bool, - default=True) - @query_param('namespace_only', 'Whether to limit only to the given namespace.', + @query_param('namespace', 'Filters the repositories returned to this namespace', type=str) + @query_param('starred', 'Filters the repositories returned to those starred by the user', + type=truthy_bool, default=False) + @query_param('public', 'Adds any repositories visible to the user by virtue of being public', 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.""" - 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]) + """ Fetch the list of repositories visible to the current user under a variety of situations. + """ - if args['private']: - username = get_authenticated_user().username - - 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']) + repositories, star_lookup = self._load_repositories(args['namespace'], args['public'], + args['starred'], args['limit'], + args['page']) # 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), - 'name': repo_obj.get(RepositoryTable.name), - 'description': repo_obj.get(RepositoryTable.description), - 'is_public': repo_obj.get(Visibility.name) == 'public' + 'namespace': repo_obj.namespace_user.username, + 'name': repo_obj.name, + 'description': repo_obj.description, + '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 response + return { + 'repositories': [repo_view(repo) for repo in repositories] + } @resource('/v1/repository/') diff --git a/initdb.py b/initdb.py index bb9a3c141..3889a8f52 100644 --- a/initdb.py +++ b/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, - [], (4, [], ['latest', 'prod'])) + simple_repo = __generate_repository(new_user_1, 'simple', 'Simple repository.', False, + [], (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 diff --git a/static/js/pages/org-view.js b/static/js/pages/org-view.js index 019116cec..a8aac0b9a 100644 --- a/static/js/pages/org-view.js +++ b/static/js/pages/org-view.js @@ -30,7 +30,6 @@ var loadRepositories = function() { var options = { - 'namespace_only': true, 'namespace': orgname, }; diff --git a/static/js/pages/repo-list.js b/static/js/pages/repo-list.js index cdc6d676f..2009042e6 100644 --- a/static/js/pages/repo-list.js +++ b/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 diff --git a/static/js/pages/user-view.js b/static/js/pages/user-view.js index f86e505ca..325322036 100644 --- a/static/js/pages/user-view.js +++ b/static/js/pages/user-view.js @@ -24,7 +24,6 @@ var loadRepositories = function() { var options = { 'sort': true, - 'namespace_only': true, 'namespace': username, }; diff --git a/test/test_api_usage.py b/test/test_api_usage.py index dfadf75d5..c4f9dd1f6 100644 --- a/test/test_api_usage.py +++ b/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)) - - 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) + json = self.getJsonResponse(RepositoryList, params=dict(limit=1)) + self.assertEquals(len(json['repositories']), 1) def test_listrepos_allparams(self): self.login(ADMIN_ACCESS_USER) - json = self.getJsonResponse(RepositoryList, - params=dict(namespace=ORGANIZATION, - public=False, - last_modified=True)) + + # Queries: Base + the list query + the popularity and last modified queries + with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 3): + 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,