diff --git a/data/model/oci/tag.py b/data/model/oci/tag.py index 0f3b7ff30..df694344e 100644 --- a/data/model/oci/tag.py +++ b/data/model/oci/tag.py @@ -203,6 +203,7 @@ def get_most_recent_tag_lifetime_start(repository_ids): return {} assert len(repository_ids) > 0 and None not in repository_ids + assert len(repository_ids) <= 100 query = ( Tag.select(Tag.repository, fn.Max(Tag.lifetime_start_ms)) diff --git a/data/model/repository.py b/data/model/repository.py index 70d01723e..e20517905 100644 --- a/data/model/repository.py +++ b/data/model/repository.py @@ -351,8 +351,10 @@ def get_filtered_matching_repositories( if filter_user is None: return [] + # NOTE: We add the offset to the limit here to ensure we have enough results + # for the take's we conduct below. iterator = _filter_repositories_visible_to_user( - unfiltered_query, filter_user.id, limit, repo_kind + unfiltered_query, filter_user.id, offset + limit, repo_kind ) if offset > 0: take(offset, iterator) @@ -429,7 +431,7 @@ def _get_sorted_matching_repositories( Repository.select(*select_fields) .join(RepositorySearchScore) .where(Repository.state != RepositoryState.MARKED_FOR_DELETION) - .order_by(RepositorySearchScore.score.desc()) + .order_by(RepositorySearchScore.score.desc(), RepositorySearchScore.id) ) else: if search_fields is None: @@ -454,7 +456,7 @@ def _get_sorted_matching_repositories( .join(RepositorySearchScore) .where(clause) .where(Repository.state != RepositoryState.MARKED_FOR_DELETION) - .order_by(SQL("score").desc()) + .order_by(SQL("score").desc(), RepositorySearchScore.id) ) if repo_kind is not None: diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py index 6cd8a4c51..26c5b1519 100644 --- a/data/registry_model/interface.py +++ b/data/registry_model/interface.py @@ -198,8 +198,9 @@ class RegistryDataInterface(object): @abstractmethod def get_most_recent_tag_lifetime_start(self, repository_refs): """ - Returns a map from repository ID to the last modified time ( seconds from epoch, UTC) for - each repository in the given repository reference list. + Returns a map from repository ID to the last modified time (seconds from epoch, UTC) for + each repository in the given repository reference list. There can be a maximum of 100 + repositories specified, as this is a VERY heavy operation. """ @abstractmethod diff --git a/endpoints/api/repository.py b/endpoints/api/repository.py index 5cf762509..074ba344b 100644 --- a/endpoints/api/repository.py +++ b/endpoints/api/repository.py @@ -56,7 +56,6 @@ from util.parsing import truthy_bool logger = logging.getLogger(__name__) -REPOS_PER_PAGE = 100 MAX_DAYS_IN_3_MONTHS = 92 diff --git a/endpoints/api/repository_models_pre_oci.py b/endpoints/api/repository_models_pre_oci.py index 5c324f598..45773c051 100644 --- a/endpoints/api/repository_models_pre_oci.py +++ b/endpoints/api/repository_models_pre_oci.py @@ -91,6 +91,7 @@ class PreOCIModel(RepositoryDataInterface): popularity, ): next_page_token = None + # Lookup the requested repositories (either starred or non-starred.) if starred: # Return the full list of repos starred by the current user that are still visible to them. @@ -103,17 +104,6 @@ class PreOCIModel(RepositoryDataInterface): user, kind_filter=repo_kind ) repos = [repo for repo in unfiltered_repos if can_view_repo(repo)] - elif namespace: - # Repositories filtered by namespace do not need pagination (their results are fairly small), - # so we just do the lookup directly. - repos = list( - model.repository.get_visible_repositories( - username=username, - include_public=public, - namespace=namespace, - kind_filter=repo_kind, - ) - ) else: # Determine the starting offset for pagination. Note that we don't use the normal # model.modelutil.paginate method here, as that does not operate over UNION queries, which @@ -128,13 +118,17 @@ class PreOCIModel(RepositoryDataInterface): start_id=start_id, limit=REPOS_PER_PAGE + 1, kind_filter=repo_kind, + namespace=namespace, ) repos, next_page_token = model.modelutil.paginate_query( repo_query, limit=REPOS_PER_PAGE, sort_field_name="rid" ) - # Collect the IDs of the repositories found for subequent lookup of popularity + repos = list(repos) + assert len(repos) <= REPOS_PER_PAGE + + # Collect the IDs of the repositories found for subsequent lookup of popularity # and/or last modified. last_modified_map = {} action_sum_map = {} diff --git a/endpoints/api/search.py b/endpoints/api/search.py index 18f924c2e..966589dba 100644 --- a/endpoints/api/search.py +++ b/endpoints/api/search.py @@ -408,6 +408,9 @@ class ConductRepositorySearch(ApiResource): @parse_args() @query_param("query", "The search query.", type=str, default="") @query_param("page", "The page.", type=int, default=1) + @query_param( + "includeUsage", "Whether to include usage metadata", type=truthy_bool, default=False + ) @nickname("conductRepoSearch") def get(self, parsed_args): """ @@ -416,7 +419,7 @@ class ConductRepositorySearch(ApiResource): query = parsed_args["query"] page = min(max(1, parsed_args["page"]), MAX_RESULT_PAGE_COUNT) offset = (page - 1) * MAX_PER_PAGE - limit = offset + MAX_PER_PAGE + 1 + limit = MAX_PER_PAGE + 1 username = get_authenticated_user().username if get_authenticated_user() else None @@ -427,27 +430,35 @@ class ConductRepositorySearch(ApiResource): ) ) + assert len(matching_repos) <= limit + has_additional = len(matching_repos) > MAX_PER_PAGE + matching_repos = matching_repos[0:MAX_PER_PAGE] + # Load secondary information such as last modified time, star count and action count. - repository_ids = [repo.id for repo in matching_repos] - last_modified_map = registry_model.get_most_recent_tag_lifetime_start(matching_repos) - star_map = model.repository.get_stars(repository_ids) - action_sum_map = model.log.get_repositories_action_sums(repository_ids) + last_modified_map = None + star_map = None + action_sum_map = None + if parsed_args["includeUsage"]: + repository_ids = [repo.id for repo in matching_repos] + last_modified_map = registry_model.get_most_recent_tag_lifetime_start(matching_repos) + star_map = model.repository.get_stars(repository_ids) + action_sum_map = model.log.get_repositories_action_sums(repository_ids) # Build the results list. results = [ repo_result_view( repo, username, - last_modified_map.get(repo.id), - star_map.get(repo.id, 0), - float(action_sum_map.get(repo.id, 0)), + last_modified_map.get(repo.id) if last_modified_map is not None else None, + star_map.get(repo.id, 0) if star_map is not None else None, + float(action_sum_map.get(repo.id, 0)) if action_sum_map is not None else None, ) for repo in matching_repos ] return { - "results": results[0:MAX_PER_PAGE], - "has_additional": len(results) > MAX_PER_PAGE, + "results": results, + "has_additional": has_additional, "page": page, "page_size": MAX_PER_PAGE, "start_index": offset, diff --git a/endpoints/api/test/test_search.py b/endpoints/api/test/test_search.py index 580c8b8d8..4f3851934 100644 --- a/endpoints/api/test/test_search.py +++ b/endpoints/api/test/test_search.py @@ -3,7 +3,7 @@ import pytest from playhouse.test_utils import assert_query_count from data import model, database -from endpoints.api.search import ConductRepositorySearch, ConductSearch +from endpoints.api.search import ConductRepositorySearch, ConductSearch, MAX_PER_PAGE from endpoints.api.test.shared import conduct_api_call from endpoints.test.shared import client_with_identity from test.fixtures import * @@ -17,7 +17,7 @@ def test_repository_search(query, client): with client_with_identity("devtable", client) as cl: params = {"query": query} - with assert_query_count(7): + with assert_query_count(4): result = conduct_api_call(cl, ConductRepositorySearch, "GET", params, None, 200).json assert result["start_index"] == 0 assert result["page"] == 1 @@ -31,3 +31,37 @@ def test_search_query_count(query, client): with assert_query_count(10): result = conduct_api_call(cl, ConductSearch, "GET", params, None, 200).json assert len(result["results"]) + + +@pytest.mark.skipif( + os.environ.get("TEST_DATABASE_URI", "").find("mysql") >= 0, + reason="MySQL FULLTEXT indexes don't update synchronously", +) +@pytest.mark.parametrize("page_count", [1, 2, 4, 6,]) +def test_repository_search_pagination(page_count, client): + # Create at least a few pages of results. + all_repositories = set() + user = model.user.get_user("devtable") + for index in range(0, MAX_PER_PAGE * page_count): + repo_name = "somerepo%s" % index + all_repositories.add(repo_name) + model.repository.create_repository("devtable", repo_name, user) + + with client_with_identity("devtable", client) as cl: + for page_index in range(0, page_count): + params = {"query": "somerepo", "page": page_index + 1} + + repo_results = conduct_api_call( + cl, ConductRepositorySearch, "GET", params, None, 200 + ).json + assert len(repo_results["results"]) <= MAX_PER_PAGE + for repo in repo_results["results"]: + all_repositories.remove(repo["name"]) + + if page_index < page_count - 1: + assert len(repo_results["results"]) == MAX_PER_PAGE + assert repo_results["has_additional"] + else: + assert not repo_results["has_additional"] + + assert not all_repositories diff --git a/static/directives/repo-list-view.html b/static/directives/repo-list-view.html index a55a19d3d..3a8e5a5cd 100644 --- a/static/directives/repo-list-view.html +++ b/static/directives/repo-list-view.html @@ -1,41 +1,8 @@
- -
- - -
-
- -
-
-
-
- - -
- -
-
- - -
-
-
-
+
diff --git a/static/js/directives/ui/repo-list-table.js b/static/js/directives/ui/repo-list-table.js index 69da2c047..63d5307b3 100644 --- a/static/js/directives/ui/repo-list-table.js +++ b/static/js/directives/ui/repo-list-table.js @@ -17,7 +17,7 @@ angular.module('quay').directive('repoListTable', function () { $scope.inReadOnlyMode = StateService.inReadOnlyMode(); $scope.repositories = null; $scope.orderedRepositories = []; - $scope.reposPerPage = 50; + $scope.reposPerPage = 25; $scope.maxPopularity = 0; $scope.options = { diff --git a/static/js/directives/ui/repo-list-view.js b/static/js/directives/ui/repo-list-view.js index 6ed3cc532..4e0ab0bf2 100644 --- a/static/js/directives/ui/repo-list-view.js +++ b/static/js/directives/ui/repo-list-view.js @@ -18,7 +18,6 @@ angular.module('quay').directive('repoListView', function () { $scope.inReadOnlyMode = StateService.inReadOnlyMode(); $scope.resources = []; $scope.loading = true; - $scope.showAsList = CookieService.get('quay.repoview') == 'list'; $scope.optionAllowed = true; $scope.$watch('namespaces', function(namespaces) { @@ -34,17 +33,7 @@ angular.module('quay').directive('repoListView', function () { } } }); - - $scope.optionAllowed = $scope.resources.length <= 250; - if (!$scope.optionAllowed) { - $scope.showAsList = true; - } }, true); - - $scope.setShowAsList = function(value) { - $scope.showAsList = value; - CookieService.putPermanent('quay.repoview', value ? 'list' : 'grid'); - }; } }; return directiveDefinitionObject; diff --git a/static/js/pages/org-view.js b/static/js/pages/org-view.js index c3ca81e01..163cc9923 100644 --- a/static/js/pages/org-view.js +++ b/static/js/pages/org-view.js @@ -44,7 +44,7 @@ 'popularity': true }; - $scope.organization.repositories = ApiService.listReposAsResource().withOptions(options).get(function(resp) { + $scope.organization.repositories = ApiService.listReposAsResource().withPagination('repositories').withOptions(options).get(function(resp) { return resp.repositories; }); }; diff --git a/static/js/pages/repo-list.js b/static/js/pages/repo-list.js index 36d8d6e83..4514dfc07 100644 --- a/static/js/pages/repo-list.js +++ b/static/js/pages/repo-list.js @@ -106,7 +106,7 @@ 'public': namespace.public }; - namespace.repositories = ApiService.listReposAsResource().withOptions(options).get(function(resp) { + namespace.repositories = ApiService.listReposAsResource().withPagination('repositories').withOptions(options).get(function(resp) { return resp.repositories.map(findDuplicateRepo); }); diff --git a/static/js/pages/search.js b/static/js/pages/search.js index 6529ea447..b8505f58a 100644 --- a/static/js/pages/search.js +++ b/static/js/pages/search.js @@ -14,7 +14,8 @@ var params = { 'query': $routeParams['q'], - 'page': $scope.currentPage + 'page': $scope.currentPage, + 'includeUsage': true }; var MAX_PAGE_RESULTS = Config['SEARCH_MAX_RESULT_PAGE_COUNT']; diff --git a/static/js/pages/user-view.js b/static/js/pages/user-view.js index d76eaa761..de953e1ec 100644 --- a/static/js/pages/user-view.js +++ b/static/js/pages/user-view.js @@ -59,7 +59,7 @@ 'popularity': true }; - $scope.context.viewuser.repositories = ApiService.listReposAsResource().withOptions(options).get(function(resp) { + $scope.context.viewuser.repositories = ApiService.listReposAsResource().withPagination('repositories').withOptions(options).get(function(resp) { return resp.repositories; }); }; diff --git a/static/js/services/api-service.js b/static/js/services/api-service.js index 581239d75..e70f89621 100644 --- a/static/js/services/api-service.js +++ b/static/js/services/api-service.js @@ -10,11 +10,18 @@ angular.module('quay').factory('ApiService', ['Restangular', '$q', 'UtilService' var getResource = function(getMethod, operation, opt_parameters, opt_background) { var resource = {}; + var paginationKey = null; + resource.withOptions = function(options) { this.options = options; return this; }; + resource.withPagination = function(key) { + paginationKey = key; + return this; + }; + resource.get = function(processor, opt_errorHandler) { var options = this.options; var result = { @@ -23,17 +30,44 @@ angular.module('quay').factory('ApiService', ['Restangular', '$q', 'UtilService' 'hasError': false }; - getMethod(options, opt_parameters, opt_background, true).then(function(resp) { - result.value = processor(resp); - result.loading = false; - }, function(resp) { - result.hasError = true; - result.loading = false; - if (opt_errorHandler) { - opt_errorHandler(resp); - } - }); + var paginatedResults = []; + var performGet = function(opt_nextPageToken) { + if (opt_nextPageToken) { + opt_parameters = opt_parameters || {}; + opt_parameters['next_page'] = opt_nextPageToken; + } + + getMethod(options, opt_parameters, opt_background, true).then(function(resp) { + if (paginationKey) { + if (resp && resp[paginationKey]) { + Array.prototype.push.apply(paginatedResults, resp[paginationKey]); + + var fullResp = {}; + fullResp[paginationKey] = paginatedResults; + result.value = processor(fullResp); + result.loading = resp['next_page'] != null; + + if (result.loading) { + performGet(resp['next_page']); + } + + return; + } + } + + result.value = processor(resp); + result.loading = false; + }, function(resp) { + result.hasError = true; + result.loading = false; + if (opt_errorHandler) { + opt_errorHandler(resp); + } + }); + }; + + performGet(); return result; }; diff --git a/test/test_api_usage.py b/test/test_api_usage.py index 3c49ae14b..b18147aca 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -117,12 +117,10 @@ from endpoints.api.organization import ( OrganizationApplicationResetClientSecret, Organization, ) -from endpoints.api.repository import ( - RepositoryList, - RepositoryVisibility, - Repository, - REPOS_PER_PAGE, -) +from endpoints.api.repository import RepositoryList, RepositoryVisibility, Repository + +from endpoints.api.repository_models_pre_oci import REPOS_PER_PAGE + from endpoints.api.permission import ( RepositoryUserPermission, RepositoryTeamPermission,