vbatts/quay
Archived
1
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

Merge pull request #1672 from coreos-inc/off-by-one

Browse source 
Fix off-by-one error in repo tags pagination
This commit is contained in:
josephschorr 2016-08-03 15:00:23 -04:00 committed by GitHub
commit 8bc0080aeb
3 changed files with 50 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
data/model/tag.py
View file

@ -203,12 +203,14 @@ def list_repository_tag_history(repo_obj, page=1, size=100, specific_tag=None):
.where(RepositoryTag.repository == repo_obj)
.where(RepositoryTag.hidden == False)
.order_by(RepositoryTag.lifetime_start_ts.desc(), RepositoryTag.name)
.paginate(page, size))
.limit(size + 1)
.offset(size * (page - 1)))
if specific_tag:
query = query.where(RepositoryTag.name == specific_tag)
return query
tags = list(query)
return tags[0:size], len(tags) > size
def revert_tag(repo_obj, tag_name, docker_image_id):

12
endpoints/api/tag.py
View file

@ -47,17 +47,13 @@ class ListRepositoryTags(RepositoryParamResource):
page = max(1, parsed_args.get('page', 1))
limit = min(100, max(1, parsed_args.get('limit', 50)))
tags, has_additional = model.tag.list_repository_tag_history(repo, page=page, size=limit,
specific_tag=specific_tag)
# Note: We ask for limit+1 here, so we can check to see if there are
# additional pages of results.
tags = model.tag.list_repository_tag_history(repo, page=page, size=limit+1,
specific_tag=specific_tag)
tags = list(tags)
return {
'tags': [tag_view(tag) for tag in tags[0:limit]],
'tags': [tag_view(tag) for tag in tags],
'page': page,
'has_additional': len(tags) >= limit
'has_additional': has_additional,
}

48
test/test_api_usage.py
View file

@ -2411,15 +2411,51 @@ class TestListAndDeleteTag(ApiTestCase):
latest_image = model.tag.get_tag_image(ADMIN_ACCESS_USER, "complex", "prod")
for i in xrange(1, 100):
model.tag.create_or_update_tag(ADMIN_ACCESS_USER, "complex", "tag" + str(i), latest_image.docker_image_id)
# Create 10 tags in an empty repo.
user = model.user.get_user_or_org(ADMIN_ACCESS_USER)
repo = model.repository.create_repository(ADMIN_ACCESS_USER, "empty", user)
image = model.image.find_create_or_link_image(latest_image.docker_image_id, repo,
ADMIN_ACCESS_USER, {}, ['local_us'])
remaining_tags = set()
for i in xrange(1, 11):
tag_name = "tag" + str(i)
remaining_tags.add(tag_name)
model.tag.create_or_update_tag(ADMIN_ACCESS_USER, "empty", tag_name,
image.docker_image_id)
# Make sure we can iterate over all of them.
json = self.getJsonResponse(ListRepositoryTags,
params=dict(repository=ADMIN_ACCESS_USER + '/empty', page=1,
limit=5))
self.assertEquals(1, json['page'])
self.assertEquals(5, len(json['tags']))
self.assertTrue(json['has_additional'])
names = {tag['name'] for tag in json['tags']}
remaining_tags = remaining_tags - names
self.assertEquals(5, len(remaining_tags))
json = self.getJsonResponse(ListRepositoryTags,
params=dict(repository=ADMIN_ACCESS_USER + '/complex', page=2))
params=dict(repository=ADMIN_ACCESS_USER + '/empty', page=2,
limit=5))
self.assertEquals(2, json['page'])
self.assertEquals(5, len(json['tags']))
self.assertFalse(json['has_additional'])
names = {tag['name'] for tag in json['tags']}
remaining_tags = remaining_tags - names
self.assertEquals(0, len(remaining_tags))
json = self.getJsonResponse(ListRepositoryTags,
params=dict(repository=ADMIN_ACCESS_USER + '/empty', page=3,
limit=5))
self.assertEquals(3, json['page'])
self.assertEquals(0, len(json['tags']))
self.assertFalse(json['has_additional'])
# Make sure that we're able to see the second page of results.
assert json['page'] == 2
assert len(json['tags']) == 50
class TestRepoPermissions(ApiTestCase):
def listUserPermissions(self, namespace=ADMIN_ACCESS_USER, repo='simple'):