Merge pull request #2542 from coreos-inc/better-tag-query

Switch get repo API to use a single list tags query

data/model/tag.py

@@ -83,13 +83,30 @@ def filter_tags_have_repository_event(query, event):
           .order_by(RepositoryTag.lifetime_start_ts.desc()))
 
 
-def get_tag_manifests(tags):
-  """ Returns a map from tag ID to its associated manifest, if any. """
+def get_tag_manifest_digests(tags):
+  """ Returns a map from tag ID to its associated manifest digest, if any. """
   if not tags:
     return dict()
 
-  manifests = TagManifest.select().where(TagManifest.tag << [t.id for t in tags])
-  return {manifest.tag_id:manifest for manifest in manifests}
+  manifests = (TagManifest
+               .select(TagManifest.tag, TagManifest.digest)
+               .where(TagManifest.tag << [t.id for t in tags]))
+
+  return {manifest.tag_id: manifest.digest for manifest in manifests}
+
+
+def list_active_repo_tags(repo):
+  """ Returns all of the active, non-hidden tags in a repository, joined to they images
+      and (if present), their manifest.
+  """
+  query = _tag_alive(RepositoryTag
+                     .select(RepositoryTag, Image, TagManifest.digest)
+                     .join(Image)
+                     .where(RepositoryTag.repository == repo)
+                     .switch(RepositoryTag)
+                     .join(TagManifest, JOIN_LEFT_OUTER))
+
+  return query
 
 
 def list_repository_tags(namespace_name, repository_name, include_hidden=False,
@@ -320,7 +337,7 @@ def list_repository_tag_history(repo_obj, page=1, size=100, specific_tag=None):
   if not tags:
     return [], {}, False
 
-  manifest_map = get_tag_manifests(tags)
+  manifest_map = get_tag_manifest_digests(tags)
   return tags[0:size], manifest_map, len(tags) > size
 
 

endpoints/api/repository.py

@@ -322,7 +322,7 @@ class Repository(RepositoryParamResource):
       return repo_data
 
     # Older image-only repo code.
-    def tag_view(tag, manifest):
+    def tag_view(tag):
       tag_info = {
         'name': tag.name,
         'image_id': tag.image.docker_image_id,
@@ -333,16 +333,14 @@ class Repository(RepositoryParamResource):
         last_modified = format_date(datetime.fromtimestamp(tag.lifetime_start_ts))
         tag_info['last_modified'] = last_modified
 
-      if manifest is not None:
-        tag_info['manifest_digest'] = manifest.digest
+      if tag.tagmanifest is not None:
+        tag_info['manifest_digest'] = tag.tagmanifest.digest
 
       return tag_info
 
     stats = None
-    tags = model.tag.list_repository_tags(namespace, repository, include_storage=True)
-    manifests = model.tag.get_tag_manifests(tags)
-
-    tag_dict = {tag.name: tag_view(tag, manifests.get(tag.id)) for tag in tags}
+    tags = model.tag.list_active_repo_tags(repo)
+    tag_dict = {tag.name: tag_view(tag) for tag in tags}
     if parsed_args['includeStats']:
       stats = []
       found_dates = {}
@@ -419,7 +417,7 @@ class Repository(RepositoryParamResource):
 
     # Remove any builds from the queue.
     dockerfile_build_queue.delete_namespaced_items(namespace, repository)
-    
+
     log_action('delete_repo', namespace,
                {'repo': repository, 'namespace': namespace})
     return '', 204

endpoints/api/tag.py

@@ -44,7 +44,7 @@ class ListRepositoryTags(RepositoryParamResource):
         tag_info['end_ts'] = tag.lifetime_end_ts
 
       if tag.id in manifest_map:
-        tag_info['manifest_digest'] = manifest_map[tag.id].digest
+        tag_info['manifest_digest'] = manifest_map[tag.id]
 
       return tag_info
 

test/test_api_usage.py

@@ -2425,6 +2425,24 @@ class TestDeleteRepository(ApiTestCase):
 class TestGetRepository(ApiTestCase):
   PUBLIC_REPO = PUBLIC_USER + '/publicrepo'
 
+  def test_get_largerepo(self):
+    self.login(ADMIN_ACCESS_USER)
+
+    # base + repo + is_starred + tags
+    with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 4):
+      self.getJsonResponse(Repository,
+                           params=dict(repository=ADMIN_ACCESS_USER + '/simple'))
+
+    # base + repo + is_starred + tags
+    with assert_query_count(BASE_LOGGEDIN_QUERY_COUNT + 4):
+      json = self.getJsonResponse(Repository,
+                                  params=dict(repository=ADMIN_ACCESS_USER + '/gargantuan'))
+
+    self.assertEquals(ADMIN_ACCESS_USER, json['namespace'])
+    self.assertEquals('gargantuan', json['name'])
+
+    self.assertEquals(False, json['is_public'])
+
   def test_getrepo_badnames(self):
     self.login(ADMIN_ACCESS_USER)
 
@@ -4540,7 +4558,7 @@ class TestRepositoryImageSecurity(ApiTestCase):
                          expected_code=200)
 
 
-      
+
 
 class TestSuperUserCustomCertificates(ApiTestCase):
   def test_custom_certificates(self):