Merge pull request #2815 from coreos-inc/joseph.schorr/QUAY-650/image-api-data-interface

Change Image API to use a data interface

endpoints/api/image.py

@@ -1,46 +1,9 @@
 """ List and lookup repository images. """
 
-import json
-
-from collections import defaultdict
 from endpoints.api import (resource, nickname, require_repo_read, RepositoryParamResource,
-                           format_date, path_param, disallow_for_app_repositories)
+                           path_param, disallow_for_app_repositories)
+from endpoints.api.image_models_pre_oci import pre_oci_model as model
 from endpoints.exception import NotFound
-from data import model
-
-
-def image_view(image, image_map, include_ancestors=True):
-  command = image.command
-
-  def docker_id(aid):
-    if aid not in image_map:
-      return ''
-
-    return image_map[aid].docker_image_id
-
-  image_data = {
-    'id': image.docker_image_id,
-    'created': format_date(image.created),
-    'comment': image.comment,
-    'command': json.loads(command) if command else None,
-    'size': image.storage.image_size,
-    'uploading': image.storage.uploading,
-    'sort_index': len(image.ancestors),
-  }
-
-  if include_ancestors:
-    # Calculate the ancestors string, with the DBID's replaced with the docker IDs.
-    ancestors = [docker_id(a) for a in image.ancestor_id_list()]
-    image_data['ancestors'] = '/{0}/'.format('/'.join(ancestors))
-
-  return image_data
-
-
-def historical_image_view(image, image_map):
-  ancestors = [image_map[a] for a in image.ancestor_id_list()]
-  normal_view = image_view(image, image_map)
-  normal_view['history'] = [image_view(parent, image_map, False) for parent in ancestors]
-  return normal_view
 
 
 @resource('/v1/repository/<apirepopath:repository>/image/')
@@ -53,33 +16,11 @@ class RepositoryImageList(RepositoryParamResource):
   @disallow_for_app_repositories
   def get(self, namespace, repository):
     """ List the images for the specified repository. """
-    repo = model.repository.get_repository(namespace, repository)
-    if not repo:
+    images = model.get_repository_images(namespace, repository)
+    if images is None:
       raise NotFound()
 
-    all_images = model.image.get_repository_images_without_placements(repo)
-    all_tags = model.tag.list_repository_tags(namespace, repository)
-
-    tags_by_docker_id = defaultdict(list)
-    found_image_ids = set()
-
-    for tag in all_tags:
-      tags_by_docker_id[tag.image.docker_image_id].append(tag.name)
-      found_image_ids.add(tag.image.id)
-      found_image_ids.update(tag.image.ancestor_id_list())
-
-    image_map = {}
-    filtered_images = []
-    for image in all_images:
-      if image.id in found_image_ids:
-        image_map[image.id] = image
-        filtered_images.append(image)
-
-    def add_tags(image_json):
-      image_json['tags'] = tags_by_docker_id[image_json['id']]
-      return image_json
-
-    return {'images': [add_tags(image_view(image, image_map)) for image in filtered_images]}
+    return {'images': [image.to_dict() for image in images]}
 
 
 @resource('/v1/repository/<apirepopath:repository>/image/<image_id>')
@@ -93,13 +34,8 @@ class RepositoryImage(RepositoryParamResource):
   @disallow_for_app_repositories
   def get(self, namespace, repository, image_id):
     """ Get the information available for the specified image. """
-    image = model.image.get_repo_image_extended(namespace, repository, image_id)
-    if not image:
+    image = model.get_repository_image(namespace, repository, image_id)
+    if image is None:
       raise NotFound()
 
-    # Lookup all the ancestor images for the image.
-    image_map = {}
-    for current_image in model.image.get_parent_images(namespace, repository, image):
-      image_map[current_image.id] = current_image
-
-    return historical_image_view(image, image_map)
+    return image.to_dict()

endpoints/api/image_models_interface.py

@@ -0,0 +1,77 @@
+import json
+
+from endpoints.api import format_date
+
+from abc import ABCMeta, abstractmethod
+from collections import namedtuple
+from six import add_metaclass
+
+class Image(namedtuple('Image', ['docker_image_id', 'created', 'comment', 'command', 'image_size',
+                                 'uploading', 'parents'])):
+  """
+  Image represents an image.
+  :type name: string
+  """
+
+  def to_dict(self):
+    image_data = {
+      'id': self.docker_image_id,
+      'created': format_date(self.created),
+      'comment': self.comment,
+      'command': json.loads(self.command) if self.command else None,
+      'size': self.image_size,
+      'uploading': self.uploading,
+      'sort_index': len(self.parents),
+    }
+
+    # Calculate the ancestors string, with the DBID's replaced with the docker IDs.
+    parent_docker_ids = [parent_image.docker_image_id for parent_image in self.parents]
+    image_data['ancestors'] = '/{0}/'.format('/'.join(parent_docker_ids))
+
+    return image_data
+
+class ImageWithTags(namedtuple('ImageWithTags', ['image', 'tag_names'])):
+  """
+  ImageWithTags represents an image, along with the tags that point to it.
+  :type image: Image
+  :type tag_names: list of string
+  """
+
+  def to_dict(self):
+    image_dict = self.image.to_dict()
+    image_dict['tags'] = self.tag_names
+    return image_dict
+
+
+class ImageWithHistory(namedtuple('ImageWithHistory', ['image'])):
+  """
+  ImageWithHistory represents an image, along with its full parent image dictionaries.
+  :type image: Image
+  :type history: list of Image parents (name is old and must be kept for compat)
+  """
+
+  def to_dict(self):
+    image_dict = self.image.to_dict()
+    image_dict['history'] = [parent_image.to_dict() for parent_image in self.image.parents]
+    return image_dict
+
+
+@add_metaclass(ABCMeta)
+class ImageInterface(object):
+  """
+  Interface that represents all data store interactions required by the image API endpoint.
+  """
+
+  @abstractmethod
+  def get_repository_images(self, namespace_name, repo_name):
+    """
+    Returns an iterator of all the ImageWithTag's defined in the matching repository. If the
+    repository doesn't exist, returns None.
+    """
+
+  @abstractmethod
+  def get_repository_image(self, namespace_name, repo_name, docker_image_id):
+    """
+    Returns the matching ImageWithHistory under the matching repository, if any. If none,
+    returns None.
+    """

endpoints/api/image_models_pre_oci.py

@@ -0,0 +1,52 @@
+from collections import defaultdict
+from data import model
+from endpoints.api.image_models_interface import (ImageInterface, ImageWithHistory, ImageWithTags,
+                                                  Image)
+
+def _image(namespace_name, repo_name, image, all_images):
+  parent_images = [all_images[ancestor_id] for ancestor_id in image.ancestor_id_list()
+                   if all_images.get(ancestor_id)]
+  parent_image_tuples = [_image(namespace_name, repo_name, parent_image, all_images)
+                         for parent_image in parent_images]
+  return Image(image.docker_image_id, image.created, image.comment, image.command,
+               image.storage.image_size, image.storage.uploading, parent_image_tuples)
+
+def _tag_names(image, tags_by_docker_id):
+  return [tag.name for tag in tags_by_docker_id.get(image.docker_image_id, [])]
+
+
+class PreOCIModel(ImageInterface):
+  """
+  PreOCIModel implements the data model for the Image API using a database schema
+  before it was changed to support the OCI specification.
+  """
+  def get_repository_images(self, namespace_name, repo_name):
+    repo = model.repository.get_repository(namespace_name, repo_name)
+    if not repo:
+      return None
+
+    all_images = model.image.get_repository_images_without_placements(repo)
+    all_images_map = {image.id: image for image in all_images}
+
+    all_tags = model.tag.list_repository_tags(namespace_name, repo_name)
+    tags_by_docker_id = defaultdict(list)
+    for tag in all_tags:
+      tags_by_docker_id[tag.image.docker_image_id].append(tag)
+
+    def _build_image(image):
+      image_itself = _image(namespace_name, repo_name, image, all_images_map)
+      return ImageWithTags(image_itself, tag_names=_tag_names(image, tags_by_docker_id))
+
+    return [_build_image(image) for image in all_images]
+
+  def get_repository_image(self, namespace_name, repo_name, docker_image_id):
+    image = model.image.get_repo_image_extended(namespace_name, repo_name, docker_image_id)
+    if not image:
+      return None
+
+    parent_images = model.image.get_parent_images(namespace_name, repo_name, image)
+    all_images_map = {image.id: image for image in parent_images}
+    return ImageWithHistory(_image(namespace_name, repo_name, image, all_images_map))
+
+
+pre_oci_model = PreOCIModel()