Merge pull request #2753 from charltonaustin/api_tag_v22

refactor(endpoints/api/tag): refactor code for v22

data/model/log.py

@@ -6,6 +6,7 @@ from peewee import JOIN_LEFT_OUTER, fn, PeeweeException
 from datetime import datetime, timedelta
 from cachetools import lru_cache
 
+import data
 from data.database import LogEntry, LogEntryKind, User, RepositoryActionCount, db
 from data.model import config, user, DataModelException
 
@@ -13,11 +14,10 @@ logger = logging.getLogger(__name__)
 
 ACTIONS_ALLOWED_WITHOUT_AUDIT_LOGGING = ['pull_repo']
 
+
 def _logs_query(selections, start_time, end_time, performer=None, repository=None, namespace=None,
                 ignore=None):
-  joined = (LogEntry
+  joined = (LogEntry.select(*selections).switch(LogEntry)
-            .select(*selections)
-            .switch(LogEntry)
             .where(LogEntry.datetime >= start_time, LogEntry.datetime < end_time))
 
   if repository:
@@ -74,14 +74,12 @@ def get_logs_query(start_time, end_time, performer=None, repository=None, namesp
     selections.append(Account)
 
   query = _logs_query(selections, start_time, end_time, performer, repository, namespace, ignore)
-  query = (query.switch(LogEntry)
+  query = (query.switch(LogEntry).join(Performer, JOIN_LEFT_OUTER,
-                .join(Performer, JOIN_LEFT_OUTER,
+                                       on=(LogEntry.performer == Performer.id).alias('performer')))
-                  on=(LogEntry.performer == Performer.id).alias('performer')))
 
   if namespace is None and repository is None:
-    query = (query.switch(LogEntry)
+    query = (query.switch(LogEntry).join(Account, JOIN_LEFT_OUTER,
-                  .join(Account, JOIN_LEFT_OUTER,
+                                         on=(LogEntry.account == Account.id).alias('account')))
-                    on=(LogEntry.account == Account.id).alias('account')))
 
   return query
 
@@ -93,8 +91,8 @@ def _json_serialize(obj):
   return obj
 
 
-def log_action(kind_name, user_or_organization_name, performer=None, repository=None,
+def log_action(kind_name, user_or_organization_name, performer=None, repository=None, ip=None,
-               ip=None, metadata={}, timestamp=None):
+               metadata={}, timestamp=None):
   if not timestamp:
     timestamp = datetime.today()
 
@@ -134,15 +132,10 @@ def log_action(kind_name, user_or_organization_name, performer=None, repository=
       raise
 
 
-
 def get_stale_logs_start_id():
   """ Gets the oldest log entry. """
   try:
-    return (LogEntry
+    return (LogEntry.select(LogEntry.id).order_by(LogEntry.id).limit(1).tuples())[0][0]
-            .select(LogEntry.id)
-            .order_by(LogEntry.id)
-            .limit(1)
-            .tuples())[0][0]
   except IndexError:
     return None
 
@@ -150,9 +143,7 @@ def get_stale_logs_start_id():
 def get_stale_logs_cutoff_id(cutoff_date):
   """ Gets the most recent ID created before the cutoff_date. """
   try:
-    return (LogEntry
+    return (LogEntry.select(fn.Max(LogEntry.id)).where(LogEntry.datetime <= cutoff_date)
-            .select(fn.Max(LogEntry.id))
-            .where(LogEntry.datetime <= cutoff_date)
             .tuples())[0][0]
   except IndexError:
     return None
@@ -179,12 +170,11 @@ def get_repositories_action_sums(repository_ids):
 
   # Filter the join to recent entries only.
   last_week = datetime.now() - timedelta(weeks=1)
-  tuples = (RepositoryActionCount
+  tuples = (RepositoryActionCount.select(RepositoryActionCount.repository,
-            .select(RepositoryActionCount.repository, fn.Sum(RepositoryActionCount.count))
+                                         fn.Sum(RepositoryActionCount.count))
             .where(RepositoryActionCount.repository << repository_ids)
             .where(RepositoryActionCount.date >= last_week)
-            .group_by(RepositoryActionCount.repository)
+            .group_by(RepositoryActionCount.repository).tuples())
-            .tuples())
 
   action_count_map = {}
   for record in tuples:

endpoints/api/__init__.py

@@ -353,7 +353,7 @@ def request_error(exception=None, **kwargs):
   raise InvalidRequest(message, data)
 
 
-def log_action(kind, user_or_orgname, metadata=None, repo=None):
+def log_action(kind, user_or_orgname, metadata=None, repo=None, repo_name=None):
   if not metadata:
     metadata = {}
 
@@ -364,8 +364,14 @@ def log_action(kind, user_or_orgname, metadata=None, repo=None):
     metadata['oauth_token_application'] = oauth_token.application.name
 
   performer = get_authenticated_user()
+
+  if repo_name:
+    repository = model.repository.get_repository(user_or_orgname, repo_name)
+  else:
+    repository = repo
+
   model.log.log_action(kind, user_or_orgname, performer=performer, ip=request.remote_addr,
-                       metadata=metadata, repository=repo)
+                       metadata=metadata, repository=repository)
 
 
 def define_json_response(schema_name):

endpoints/api/image.py

@@ -47,6 +47,7 @@ def historical_image_view(image, image_map):
 @path_param('repository', 'The full path of the repository. e.g. namespace/name')
 class RepositoryImageList(RepositoryParamResource):
   """ Resource for listing repository images. """
+
   @require_repo_read
   @nickname('listRepositoryImages')
   @disallow_for_app_repositories
@@ -78,9 +79,7 @@ class RepositoryImageList(RepositoryParamResource):
       image_json['tags'] = tags_by_docker_id[image_json['id']]
       return image_json
 
-    return {
+    return {'images': [add_tags(image_view(image, image_map)) for image in filtered_images]}
-      'images': [add_tags(image_view(image, image_map)) for image in filtered_images]
-    }
 
 
 @resource('/v1/repository/<apirepopath:repository>/image/<image_id>')
@@ -88,6 +87,7 @@ class RepositoryImageList(RepositoryParamResource):
 @path_param('image_id', 'The Docker image ID')
 class RepositoryImage(RepositoryParamResource):
   """ Resource for handling repository images. """
+
   @require_repo_read
   @nickname('getImage')
   @disallow_for_app_repositories
@@ -103,4 +103,3 @@ class RepositoryImage(RepositoryParamResource):
       image_map[current_image.id] = current_image
 
     return historical_image_view(image, image_map)
-

endpoints/api/tag.py

@@ -3,36 +3,17 @@
 from flask import request, abort
 
 from auth.auth_context import get_authenticated_user
-from data import model
+from data.model import DataModelException
 from endpoints.api import (resource, nickname, require_repo_read, require_repo_write,
                            RepositoryParamResource, log_action, validate_json_request, path_param,
                            parse_args, query_param, truthy_bool, disallow_for_app_repositories)
-from endpoints.api.image import image_view
+from endpoints.api.tag_models_interface import Repository
-from endpoints.api.tag_models_pre_oci import pre_oci_model
+from endpoints.api.tag_models_pre_oci import pre_oci_model as model
 from endpoints.exception import NotFound
 from endpoints.v2.manifest import _generate_and_store_manifest
 from util.names import TAG_ERROR, TAG_REGEX
 
 
-def tag_view(tag):
-  tag_info = {
-    'name': tag.name,
-    'docker_image_id': tag.docker_image_id,
-    'reversion': tag.reversion,
-  }
-
-  if tag.lifetime_start_ts > 0:
-    tag_info['start_ts'] = tag.lifetime_start_ts
-
-  if tag.lifetime_end_ts > 0:
-    tag_info['end_ts'] = tag.lifetime_end_ts
-
-  if tag.manifest_list:
-    tag_info['manifest_digest'] = tag.manifest_list
-
-  return tag_info
-
-
 @resource('/v1/repository/<apirepopath:repository>/tag/')
 @path_param('repository', 'The full path of the repository. e.g. namespace/name')
 class ListRepositoryTags(RepositoryParamResource):
@@ -51,15 +32,15 @@ class ListRepositoryTags(RepositoryParamResource):
     page = max(1, parsed_args.get('page', 1))
     limit = min(100, max(1, parsed_args.get('limit', 50)))
 
-    tag_history = pre_oci_model.list_repository_tag_history(namespace_name=namespace,
+    tag_history = model.list_repository_tag_history(namespace_name=namespace,
-                                                            repository_name=repository, page=page,
+                                                    repository_name=repository, page=page,
-                                                            size=limit, specific_tag=specific_tag)
+                                                    size=limit, specific_tag=specific_tag)
 
     if not tag_history:
       raise NotFound()
 
     return {
-      'tags': [tag_view(tag) for tag in tag_history.tags],
+      'tags': [tag.to_dict() for tag in tag_history.tags],
       'page': page,
       'has_additional': tag_history.more,
     }
@@ -95,20 +76,12 @@ class RepositoryTag(RepositoryParamResource):
       abort(400, TAG_ERROR)
 
     image_id = request.get_json()['image']
-    image = model.image.get_repo_image(namespace, repository, image_id)
+    repo = model.get_repo(namespace, repository, image_id)
-    if not image:
+    if not repo:
       raise NotFound()
 
-    original_image_id = None
+    original_image_id = model.get_repo_tag_image(repo, tag)
-    try:
+    model.create_or_update_tag(namespace, repository, tag, image_id)
-      original_tag_image = model.tag.get_repo_tag_image(image.repository, tag)
-      if original_tag_image:
-        original_image_id = original_tag_image.docker_image_id
-    except model.DataModelException:
-      # This is a new tag.
-      pass
-
-    model.tag.create_or_update_tag(namespace, repository, tag, image_id)
 
     username = get_authenticated_user().username
     log_action('move_tag' if original_image_id else 'create_tag', namespace, {
@@ -118,7 +91,7 @@ class RepositoryTag(RepositoryParamResource):
       'namespace': namespace,
       'image': image_id,
       'original_image': original_image_id
-    }, repo=model.repository.get_repository(namespace, repository))
+    }, repo_name=repository)
 
     _generate_and_store_manifest(namespace, repository, tag)
 
@@ -129,14 +102,14 @@ class RepositoryTag(RepositoryParamResource):
   @nickname('deleteFullTag')
   def delete(self, namespace, repository, tag):
     """ Delete the specified repository tag. """
-    model.tag.delete_tag(namespace, repository, tag)
+    model.delete_tag(namespace, repository, tag)
 
     username = get_authenticated_user().username
     log_action('delete_tag', namespace,
                {'username': username,
                 'repo': repository,
                 'namespace': namespace,
-                'tag': tag}, repo=model.repository.get_repository(namespace, repository))
+                'tag': tag}, repo_name=repository)
 
     return '', 204
 
@@ -156,17 +129,19 @@ class RepositoryTagImages(RepositoryParamResource):
   def get(self, namespace, repository, tag, parsed_args):
     """ List the images for the specified repository tag. """
     try:
-      tag_image = model.tag.get_tag_image(namespace, repository, tag)
+      tag_image = model.get_repo_tag_image(
-    except model.DataModelException:
+        Repository(namespace_name=namespace, repository_name=repository), tag)
+    except DataModelException:
       raise NotFound()
 
-    parent_images = model.image.get_parent_images(namespace, repository, tag_image)
+    if tag_image is None:
-    image_map = {}
+      raise NotFound()
 
-    image_map[str(tag_image.id)] = tag_image
+    parent_images = model.get_parent_images(namespace, repository, tag_image.docker_image_id)
+    image_map = {str(tag_image.docker_image_id): tag_image}
 
     for image in parent_images:
-      image_map[str(image.id)] = image
+      image_map[str(image.docker_image_id)] = image
 
     image_map_all = dict(image_map)
     all_images = [tag_image] + list(parent_images)
@@ -174,13 +149,13 @@ class RepositoryTagImages(RepositoryParamResource):
     # Filter the images returned to those not found in the ancestry of any of the other tags in
     # the repository.
     if parsed_args['owned']:
-      all_tags = model.tag.list_repository_tags(namespace, repository)
+      all_tags = model.list_repository_tags(namespace, repository)
       for current_tag in all_tags:
         if current_tag.name == tag:
           continue
 
         # Remove the tag's image ID.
-        tag_image_id = str(current_tag.image_id)
+        tag_image_id = str(current_tag.docker_image_id)
         image_map.pop(tag_image_id, None)
 
         # Remove any ancestors:
@@ -189,8 +164,8 @@ class RepositoryTagImages(RepositoryParamResource):
 
     return {
       'images': [
-        image_view(image, image_map_all) for image in all_images
+        image.to_dict(image_map_all) for image in all_images
-        if not parsed_args['owned'] or (str(image.id) in image_map)
+        if not parsed_args['owned'] or (str(image.docker_image_id) in image_map)
       ]
     }
 
@@ -224,7 +199,6 @@ class RestoreTag(RepositoryParamResource):
   @validate_json_request('RestoreTag')
   def post(self, namespace, repository, tag):
     """ Restores a repository tag back to a previous image in the repository. """
-    repo = model.repository.get_repository(namespace, repository)
 
     # Restore the tag back to the previous image.
     image_id = request.get_json()['image']
@@ -238,18 +212,17 @@ class RestoreTag(RepositoryParamResource):
       'tag': tag,
       'image': image_id,
     }
-
+    repo = Repository(namespace, repository)
     if manifest_digest is not None:
-      existing_image = model.tag.restore_tag_to_manifest(repo, tag, manifest_digest)
+      existing_image = model.restore_tag_to_manifest(repo, tag, manifest_digest)
     else:
-      existing_image = model.tag.restore_tag_to_image(repo, tag, image_id)
+      existing_image = model.restore_tag_to_image(repo, tag, image_id)
       _generate_and_store_manifest(namespace, repository, tag)
 
     if existing_image is not None:
       log_data['original_image'] = existing_image.docker_image_id
 
-    log_action('revert_tag', namespace, log_data, repo=model.repository.get_repository(namespace,
+    log_action('revert_tag', namespace, log_data, repo_name=repository)
-                                                                                       repository))
 
     return {
       'image_id': image_id,

endpoints/api/tag_models_interface.py

@@ -1,14 +1,17 @@
+import json
 from abc import ABCMeta, abstractmethod
 from collections import namedtuple
 
 from six import add_metaclass
 
+from endpoints.api import format_date
+
 
 class Tag(
-    namedtuple('Tag', [
+  namedtuple('Tag', [
-      'name', 'image', 'reversion', 'lifetime_start_ts', 'lifetime_end_ts', 'manifest_list',
+    'name', 'image', 'reversion', 'lifetime_start_ts', 'lifetime_end_ts', 'manifest_list',
-      'docker_image_id'
+    'docker_image_id'
-    ])):
+  ])):
   """
   Tag represents a name to an image.
   :type name: string
@@ -20,6 +23,24 @@ class Tag(
   :type docker_image_id: string
   """
 
+  def to_dict(self):
+    tag_info = {
+      'name': self.name,
+      'docker_image_id': self.docker_image_id,
+      'reversion': self.reversion,
+    }
+
+    if self.lifetime_start_ts > 0:
+      tag_info['start_ts'] = self.lifetime_start_ts
+
+    if self.lifetime_end_ts > 0:
+      tag_info['end_ts'] = self.lifetime_end_ts
+
+    if self.manifest_list:
+      tag_info['manifest_digest'] = self.manifest_list
+
+    return tag_info
+
 
 class RepositoryTagHistory(namedtuple('RepositoryTagHistory', ['tags', 'more'])):
   """
@@ -29,6 +50,58 @@ class RepositoryTagHistory(namedtuple('RepositoryTagHistory', ['tags', 'more']))
   """
 
 
+class Repository(namedtuple('Repository', ['namespace_name', 'repository_name'])):
+  """
+  Repository a single quay repository
+  :type namespace_name: string
+  :type repository_name: string
+  """
+
+
+class Image(
+  namedtuple('Image', [
+    'docker_image_id', 'created', 'comment', 'command', 'storage_image_size',
+    'storage_uploading', 'ancestor_length', 'ancestor_id_list'
+  ])):
+  """
+  Image
+  :type docker_image_id: string
+  :type created: datetime
+  :type comment: string
+  :type command: string
+  :type storage_image_size: int
+  :type storage_uploading: boolean
+  :type ancestor_length: int
+  :type ancestor_id_list: [int]
+  """
+
+  def to_dict(self, image_map, include_ancestors=True):
+    command = self.command
+
+    def docker_id(aid):
+      if aid not in image_map:
+        return ''
+
+      return image_map[aid].docker_image_id
+
+    image_data = {
+      'id': self.docker_image_id,
+      'created': format_date(self.created),
+      'comment': self.comment,
+      'command': json.loads(command) if command else None,
+      'size': self.storage_image_size,
+      'uploading': self.storage_uploading,
+      'sort_index': self.ancestor_length,
+    }
+
+    if include_ancestors:
+      # Calculate the ancestors string, with the DBID's replaced with the docker IDs.
+      ancestors = [docker_id(a) for a in self.ancestor_id_list]
+      image_data['ancestors'] = '/{0}/'.format('/'.join(ancestors))
+
+    return image_data
+
+
 @add_metaclass(ABCMeta)
 class TagDataInterface(object):
   """
@@ -41,3 +114,59 @@ class TagDataInterface(object):
     """
     Returns a RepositoryTagHistory with a list of historic tags and whether there are more tags then returned.
     """
+
+  @abstractmethod
+  def get_repo(self, namespace_name, repository_name, docker_image_id):
+    """
+    Returns a repository associated with the given namespace, repository, and docker_image_id
+    """
+
+  @abstractmethod
+  def get_repo_tag_image(self, repository, tag_name):
+    """
+    Returns an image associated with the repository and tag_name
+    """
+
+  @abstractmethod
+  def create_or_update_tag(self, namespace_name, repository_name, tag_name, docker_image_id):
+    """
+    Returns the repository tag if it is created.
+    """
+
+  @abstractmethod
+  def delete_tag(self, namespace_name, repository_name, tag_name):
+    """
+    Returns the tag for the given namespace and repository if it was created
+    """
+
+  @abstractmethod
+  def get_parent_images(self, namespace, repository, tag_name):
+    """
+    Returns a list of the parent images for the namespace, repository and tag specified.
+    """
+
+  @abstractmethod
+  def list_repository_tags(self, namespace_name, repository_name):
+    """
+    Returns a list of all tags associated with namespace_nam and repository_name
+    """
+
+  @abstractmethod
+  def get_repository(self, namespace_name, repository_name):
+    """
+    Returns the repository associated with the namespace_name and repository_name
+    """
+
+  @abstractmethod
+  def restore_tag_to_manifest(self, repository_name, tag_name, manifest_digest):
+    """
+    Returns the existing repo tag image if it exists or else returns None.
+    Side effects include adding the tag with associated name to the manifest_digest in the named repo.
+    """
+
+  @abstractmethod
+  def restore_tag_to_image(self, repository_name, tag_name, image_id):
+    """
+    Returns the existing repo tag image if it exists or else returns None
+    Side effects include adding the tag with associated name to the image with the associated id in the named repo.
+    """

endpoints/api/tag_models_pre_oci.py

@@ -1,5 +1,6 @@
 from data import model
-from endpoints.api.tag_models_interface import TagDataInterface, Tag, RepositoryTagHistory
+from data.model import DataModelException, InvalidImageException
+from endpoints.api.tag_models_interface import TagDataInterface, Tag, RepositoryTagHistory, Repository, Image
 
 
 class PreOCIModel(TagDataInterface):
@@ -13,6 +14,7 @@ class PreOCIModel(TagDataInterface):
     repository = model.repository.get_repository(namespace_name, repository_name)
     if repository is None:
       return None
+
     tags, manifest_map, more = model.tag.list_repository_tag_history(repository, page, size,
                                                                      specific_tag)
     repository_tag_history = []
@@ -20,11 +22,100 @@ class PreOCIModel(TagDataInterface):
       manifest_list = None
       if tag.id in manifest_map:
         manifest_list = manifest_map[tag.id]
-      repository_tag_history.append(
+
-        Tag(name=tag.name, image=tag.image, reversion=tag.reversion,
+      repository_tag_history.append(convert_tag(tag, manifest_list))
-            lifetime_start_ts=tag.lifetime_start_ts, lifetime_end_ts=tag.lifetime_end_ts,
+
-            manifest_list=manifest_list, docker_image_id=tag.image.docker_image_id))
     return RepositoryTagHistory(tags=repository_tag_history, more=more)
 
+  def get_repo(self, namespace_name, repository_name, docker_image_id):
+    image = model.image.get_repo_image(namespace_name, repository_name, docker_image_id)
+    if image is None:
+      return None
+
+    return Repository(image.repository.namespace_user, image.repository.name)
+
+  def get_repo_tag_image(self, repository, tag_name):
+    repo = model.repository.get_repository(str(repository.namespace_name), str(repository.repository_name))
+    if repo is None:
+      return None
+
+    try:
+      image = model.tag.get_repo_tag_image(repo, tag_name)
+    except DataModelException:
+      return None
+
+    return convert_image(image)
+
+  def create_or_update_tag(self, namespace_name, repository_name, tag_name, docker_image_id):
+    return model.tag.create_or_update_tag(namespace_name, repository_name, tag_name,
+                                          docker_image_id)
+
+  def delete_tag(self, namespace_name, repository_name, tag_name):
+    return model.tag.delete_tag(namespace_name, repository_name, tag_name)
+
+  def get_parent_images(self, namespace_name, repository_name, docker_image_id):
+    try:
+      image = model.image.get_image_by_id(namespace_name, repository_name, docker_image_id)
+    except InvalidImageException:
+      return []
+
+    parent_tags = model.image.get_parent_images(namespace_name, repository_name, image)
+    return_tags = []
+    for image in parent_tags:
+      return_tags.append(convert_image(image))
+
+    return return_tags
+
+  def list_repository_tags(self, namespace_name, repository_name):
+    tags = model.tag.list_repository_tags(namespace_name, repository_name)
+    new_tags = []
+    for tag in tags:
+      new_tags.append(convert_tag(tag))
+    return new_tags
+
+  def get_repository(self, namespace_name, repository_name):
+    repo = model.repository.get_repository(namespace_name, repository_name)
+    if repo is None:
+      return None
+
+    return Repository(namespace_name=namespace_name, repository_name=repository_name)
+
+  def restore_tag_to_manifest(self, repository, tag_name, manifest_digest):
+    repo = model.repository.get_repository(repository.namespace_name, repository.repository_name)
+    if repo is None:
+      return None
+
+    image = model.tag.restore_tag_to_manifest(repo, tag_name, manifest_digest)
+    if image is None:
+      return None
+
+    return convert_image(image)
+
+  def restore_tag_to_image(self, repository, tag_name, image_id):
+    repo = model.repository.get_repository(repository.namespace_name, repository.repository_name)
+    if repo is None:
+      return None
+
+    image = model.tag.restore_tag_to_image(repo, tag_name, image_id)
+    if image is None:
+      return None
+
+    return convert_image(image)
+
+
+def convert_image(database_image):
+  return Image(docker_image_id=database_image.docker_image_id, created=database_image.created,
+               comment=database_image.comment, command=database_image.command,
+               storage_image_size=database_image.storage.image_size,
+               storage_uploading=database_image.storage.uploading,
+               ancestor_length=len(database_image.ancestors),
+               ancestor_id_list=database_image.ancestor_id_list())
+
+
+def convert_tag(tag, manifest_list=None):
+  return Tag(name=tag.name, image=tag.image, reversion=tag.reversion,
+             lifetime_start_ts=tag.lifetime_start_ts, lifetime_end_ts=tag.lifetime_end_ts,
+             manifest_list=manifest_list, docker_image_id=tag.image.docker_image_id)
+
 
 pre_oci_model = PreOCIModel()

endpoints/api/test/test_models_pre_oci.py

@@ -1,9 +1,12 @@
 import pytest
-from endpoints.api.tag_models_interface import RepositoryTagHistory, Tag
+
-from mock import Mock
+from data.model import DataModelException, InvalidImageException
+from endpoints.api.tag_models_interface import RepositoryTagHistory, Tag, Repository
+from mock import Mock, call
 
 from data import model
 from endpoints.api.tag_models_pre_oci import pre_oci_model
+from util.morecollections import AttrDict
 
 EMPTY_REPOSITORY = 'empty_repository'
 EMPTY_NAMESPACE = 'empty_namespace'
@@ -101,3 +104,241 @@ def test_list_repository_tag_history(expected, namespace_name, repository_name,
                                        size, specific_tag)
   assert pre_oci_model.list_repository_tag_history(namespace_name, repository_name, page, size,
                                                    specific_tag) == expected
+
+
+def get_repo_image_mock(monkeypatch, return_value):
+  def return_return_value(namespace_name, repository_name, image_id):
+    return return_value
+
+  monkeypatch.setattr(model.image, 'get_repo_image', return_return_value)
+
+
+def test_get_repo_not_exists(get_monkeypatch):
+  namespace_name = 'namespace_name'
+  repository_name = 'repository_name'
+  image_id = 'image_id'
+  get_repo_image_mock(get_monkeypatch, None)
+
+  repo = pre_oci_model.get_repo(namespace_name, repository_name, image_id)
+
+  assert repo is None
+
+
+def test_get_repo_exists(get_monkeypatch):
+  namespace_name = 'namespace_name'
+  repository_name = 'repository_name'
+  image_id = 'image_id'
+  mock = Mock()
+  mock.namespace_user = namespace_name
+  mock.name = repository_name
+  mock.repository = mock
+  get_repo_image_mock(get_monkeypatch, mock)
+
+  repo = pre_oci_model.get_repo(namespace_name, repository_name, image_id)
+
+  assert repo is not None
+  assert repo.repository_name == repository_name
+  assert repo.namespace_name == namespace_name
+
+
+def get_repository_mock(monkeypatch, return_value):
+  def return_return_value(namespace_name, repository_name, kind_filter=None):
+    return return_value
+
+  monkeypatch.setattr(model.repository, 'get_repository', return_return_value)
+
+
+def get_repo_tag_image_mock(monkeypatch, return_value):
+  def return_return_value(repo, tag_name, include_storage=False):
+    return return_value
+
+  monkeypatch.setattr(model.tag, 'get_repo_tag_image', return_return_value)
+
+
+def test_get_repo_tag_image_with_repo_and_repo_tag(get_monkeypatch):
+  mock_storage = Mock()
+  mock_image = Mock()
+  mock_image.docker_image_id = 'some docker image id'
+  mock_image.created = 1235
+  mock_image.comment = 'some comment'
+  mock_image.command = 'some command'
+  mock_image.storage = mock_storage
+  mock_image.ancestors = []
+
+  get_repository_mock(get_monkeypatch, mock_image)
+  get_repo_tag_image_mock(get_monkeypatch, mock_image)
+
+  image = pre_oci_model.get_repo_tag_image(
+    Repository('namespace_name', 'repository_name'), 'tag_name')
+
+  assert image is not None
+  assert image.docker_image_id == 'some docker image id'
+
+
+def test_get_repo_tag_image_without_repo(get_monkeypatch):
+  get_repository_mock(get_monkeypatch, None)
+
+  image = pre_oci_model.get_repo_tag_image(
+    Repository('namespace_name', 'repository_name'), 'tag_name')
+
+  assert image is None
+
+
+def test_get_repo_tag_image_without_repo_tag_image(get_monkeypatch):
+  mock = Mock()
+  mock.docker_image_id = 'some docker image id'
+  get_repository_mock(get_monkeypatch, mock)
+
+  def raise_exception(repo, tag_name, include_storage=False):
+    raise DataModelException()
+
+  get_monkeypatch.setattr(model.tag, 'get_repo_tag_image', raise_exception)
+
+  image = pre_oci_model.get_repo_tag_image(
+    Repository('namespace_name', 'repository_name'), 'tag_name')
+
+  assert image is None
+
+
+def test_create_or_update_tag(get_monkeypatch):
+  mock = Mock()
+  get_monkeypatch.setattr(model.tag, 'create_or_update_tag', mock)
+
+  pre_oci_model.create_or_update_tag('namespace_name', 'repository_name', 'tag_name',
+                                     'docker_image_id')
+
+  assert mock.call_count == 1
+  assert mock.call_args == call('namespace_name', 'repository_name', 'tag_name', 'docker_image_id')
+
+
+def test_delete_tag(get_monkeypatch):
+  mock = Mock()
+  get_monkeypatch.setattr(model.tag, 'delete_tag', mock)
+
+  pre_oci_model.delete_tag('namespace_name', 'repository_name', 'tag_name')
+
+  assert mock.call_count == 1
+  assert mock.call_args == call('namespace_name', 'repository_name', 'tag_name')
+
+
+def test_get_parent_images_with_exception(get_monkeypatch):
+  mock = Mock(side_effect=InvalidImageException)
+
+  get_monkeypatch.setattr(model.image, 'get_image_by_id', mock)
+
+  images = pre_oci_model.get_parent_images('namespace_name', 'repository_name', 'tag_name')
+  assert images == []
+
+
+def test_get_parent_images_empty_parent_images(get_monkeypatch):
+  get_image_by_id_mock = Mock()
+  get_monkeypatch.setattr(model.image, 'get_image_by_id', get_image_by_id_mock)
+
+  get_parent_images_mock = Mock(return_value=[])
+  get_monkeypatch.setattr(model.image, 'get_parent_images', get_parent_images_mock)
+
+  images = pre_oci_model.get_parent_images('namespace_name', 'repository_name', 'tag_name')
+  assert images == []
+
+
+def compare_images(parent_image, attr_dict):
+  for field in parent_image._fields:
+    assert getattr(parent_image, field) == getattr(attr_dict, field)
+
+
+def test_get_parent_images(get_monkeypatch):
+  get_image_by_id_mock = Mock()
+  get_monkeypatch.setattr(model.image, 'get_image_by_id', get_image_by_id_mock)
+
+  def fake_list():
+    return []
+
+  image_one = AttrDict({
+    'docker_image_id': 'docker_image_id',
+    'created': 'created_one',
+    'comment': 'comment_one',
+    'command': 'command_one',
+    'storage': AttrDict({
+      'image_size': 'image_size_one',
+      'uploading': 'uploading_one'
+    }),
+    'ancestors': 'one/two/three',
+    'ancestor_id_list': fake_list
+  })
+  image_two = AttrDict({
+    'docker_image_id': 'docker_image_id_two',
+    'created': 'created',
+    'comment': 'comment',
+    'command': 'command',
+    'storage': AttrDict({
+      'image_size': 'image_size',
+      'uploading': 'uploading'
+    }),
+    'ancestors': 'four/five/six',
+    'ancestor_id_list': fake_list
+  })
+
+  get_parent_images_mock = Mock(return_value=[image_one, image_two])
+  get_monkeypatch.setattr(model.image, 'get_parent_images', get_parent_images_mock)
+  get_monkeypatch.setattr(model.image, 'get_image_by_id', Mock())
+
+  images = pre_oci_model.get_parent_images('namespace_name', 'repository_name', 'tag_name')
+  image_one.ancestor_id_list = []
+  image_one.storage_image_size = 'image_size_one'
+  image_one.storage_uploading = 'uploading_one'
+  image_one.ancestor_length = 13
+  image_two.ancestor_id_list = []
+  image_two.storage_uploading = 'uploading'
+  image_two.storage_image_size = 'image_size'
+  image_two.ancestor_length = 13
+  compare_images(images[0], image_one)
+  compare_images(images[1], image_two)
+
+
+def test_list_repository_tags(get_monkeypatch):
+  mock = Mock(return_value=[])
+
+  get_monkeypatch.setattr(model.tag, 'list_repository_tags', mock)
+
+  pre_oci_model.list_repository_tags('namespace_name', 'repository_name')
+
+  mock.assert_called_once_with('namespace_name', 'repository_name')
+
+
+def test_get_repository(get_monkeypatch):
+  mock = Mock()
+
+  get_monkeypatch.setattr(model.repository, 'get_repository', mock)
+
+  pre_oci_model.get_repository('namespace_name', 'repository_name')
+
+  mock.assert_called_once_with('namespace_name', 'repository_name')
+
+
+def test_tag_to_manifest(get_monkeypatch):
+  repo_mock = Mock()
+  restore_tag_mock = Mock(return_value=None)
+  get_repository_mock = Mock(return_value=repo_mock)
+
+  get_monkeypatch.setattr(model.tag, 'restore_tag_to_manifest', restore_tag_mock)
+  get_monkeypatch.setattr(model.repository, 'get_repository', get_repository_mock)
+
+  pre_oci_model.restore_tag_to_manifest(
+    Repository('namespace', 'repository'), 'tag_name', 'manifest_digest')
+
+  get_repository_mock.assert_called_once_with('namespace', 'repository')
+  restore_tag_mock.assert_called_once_with(repo_mock, 'tag_name', 'manifest_digest')
+
+
+def test__tag_to_image(get_monkeypatch):
+  repo_mock = Mock()
+  restore_tag_mock = Mock(return_value=None)
+  get_repository_mock = Mock(return_value=repo_mock)
+
+  get_monkeypatch.setattr(model.tag, 'restore_tag_to_image', restore_tag_mock)
+  get_monkeypatch.setattr(model.repository, 'get_repository', get_repository_mock)
+
+  pre_oci_model.restore_tag_to_image(Repository('namespace', 'repository'), 'tag_name', 'image_id')
+
+  get_repository_mock.assert_called_once_with('namespace', 'repository')
+  restore_tag_mock.assert_called_once_with(repo_mock, 'tag_name', 'image_id')

endpoints/api/test/test_tag.py

@@ -4,7 +4,7 @@ import pytest
 
 from mock import patch, Mock, MagicMock, call
 
-
+from data.model import DataModelException
 from endpoints.api.tag_models_interface import RepositoryTagHistory, Tag
 from endpoints.api.test.shared import conduct_api_call
 from endpoints.test.shared import client_with_identity
@@ -18,26 +18,39 @@ from test.fixtures import *
 @pytest.fixture()
 def get_repo_image():
   def mock_callable(namespace, repository, image_id):
-    img = Mock(repository='fetched_repository') if image_id == 'image1' else None
+    mock = Mock(namespace_user='devtable')
+    mock.name = 'simple'
+    img = Mock(repository=mock, docker_image_id=12) if image_id == 'image1' else None
     return img
 
-  with patch('endpoints.api.tag.model.image.get_repo_image', side_effect=mock_callable) as mk:
+  with patch('endpoints.api.tag_models_pre_oci.model.image.get_repo_image',
+             side_effect=mock_callable) as mk:
     yield mk
 
 
 @pytest.fixture()
 def get_repository():
-  with patch('endpoints.api.tag.model.image.get_repo_image', return_value='mock_repo') as mk:
+  with patch('endpoints.api.tag_models_pre_oci.model.image.get_repo_image',
+             return_value='mock_repo') as mk:
     yield mk
 
 
 @pytest.fixture()
 def get_repo_tag_image():
   def mock_get_repo_tag_image(repository, tag):
-    tag_img = Mock(docker_image_id='mock_docker_image_id') if tag == 'existing-tag' else None
+    storage_mock = Mock(image_size=1234, uploading='uploading')
-    return tag_img
 
-  with patch('endpoints.api.tag.model.tag.get_repo_tag_image',
+    def fake_ancestor_id_list():
+      return []
+
+    if tag == 'existing-tag':
+      return Mock(docker_image_id='mock_docker_image_id', created=12345, comment='comment',
+                  command='command', storage=storage_mock, ancestors=[],
+                  ancestor_id_list=fake_ancestor_id_list)
+    else:
+      raise DataModelException('Unable to find image for tag.')
+
+  with patch('endpoints.api.tag_models_pre_oci.model.tag.get_repo_tag_image',
              side_effect=mock_get_repo_tag_image):
     yield
 
@@ -48,7 +61,7 @@ def restore_tag_to_manifest():
     tag_img = Mock(docker_image_id='mock_docker_image_id') if tag == 'existing-tag' else None
     return tag_img
 
-  with patch('endpoints.api.tag.model.tag.restore_tag_to_manifest',
+  with patch('endpoints.api.tag_models_pre_oci.model.tag.restore_tag_to_manifest',
              side_effect=mock_restore_tag_to_manifest):
     yield
 
@@ -59,14 +72,14 @@ def restore_tag_to_image():
     tag_img = Mock(docker_image_id='mock_docker_image_id') if tag == 'existing-tag' else None
     return tag_img
 
-  with patch('endpoints.api.tag.model.tag.restore_tag_to_image',
+  with patch('endpoints.api.tag_models_pre_oci.model.tag.restore_tag_to_image',
              side_effect=mock_restore_tag_to_image):
     yield
 
 
 @pytest.fixture()
 def create_or_update_tag():
-  with patch('endpoints.api.tag.model.tag.create_or_update_tag') as mk:
+  with patch('endpoints.api.tag_models_pre_oci.model.tag.create_or_update_tag') as mk:
     yield mk
 
 
@@ -90,12 +103,14 @@ def authd_client(client):
 def list_repository_tag_history():
   def list_repository_tag_history(namespace_name, repository_name, page, size, specific_tag):
     return RepositoryTagHistory(tags=[
-      Tag(name='First Tag', image='image', reversion=False, lifetime_start_ts=0, lifetime_end_ts=0, manifest_list=[],
+      Tag(name='First Tag', image='image', reversion=False, lifetime_start_ts=0, lifetime_end_ts=0,
-          docker_image_id='first docker image id'),
+          manifest_list=[], docker_image_id='first docker image id'),
-      Tag(name='Second Tag', image='second image', reversion=True, lifetime_start_ts=10, lifetime_end_ts=100,
+      Tag(name='Second Tag', image='second image', reversion=True, lifetime_start_ts=10,
-          manifest_list=[], docker_image_id='second docker image id')], more=False)
+          lifetime_end_ts=100, manifest_list=[], docker_image_id='second docker image id')
+    ], more=False)
 
-  with patch('endpoints.api.tag.pre_oci_model.list_repository_tag_history', side_effect=list_repository_tag_history):
+  with patch('endpoints.api.tag.model.list_repository_tag_history',
+             side_effect=list_repository_tag_history):
     yield
 
 
@@ -104,7 +119,8 @@ def find_no_repo_tag_history():
   def list_repository_tag_history(namespace_name, repository_name, page, size, specific_tag):
     return None
 
-  with patch('endpoints.api.tag.pre_oci_model.list_repository_tag_history', side_effect=list_repository_tag_history):
+  with patch('endpoints.api.tag.model.list_repository_tag_history',
+             side_effect=list_repository_tag_history):
     yield
 
 
@@ -130,23 +146,39 @@ def test_move_tag(test_image, test_tag, expected_status, get_repo_image, get_rep
     conduct_api_call(authd_client, RepositoryTag, 'put', params, request_body, expected_status)
 
 
-@pytest.mark.parametrize('namespace, repository, specific_tag, page, limit, expected_response_code, expected', [
+@pytest.mark.parametrize(
-  ('devtable', 'simple', None, 1, 10, 200, {'has_additional': False}),
+  'namespace, repository, specific_tag, page, limit, expected_response_code, expected', [
-  ('devtable', 'simple', None, 1, 10, 200, {'page': 1}),
+    ('devtable', 'simple', None, 1, 10, 200, {
-  ('devtable', 'simple', None, 1, 10, 200, {'tags': [{'docker_image_id': 'first docker image id',
+      'has_additional': False
-                                                      'name': 'First Tag',
+    }),
-                                                      'reversion': False},
+    ('devtable', 'simple', None, 1, 10, 200, {
-                                                     {'docker_image_id': 'second docker image id',
+      'page': 1
-                                                      'end_ts': 100,
+    }),
-                                                      'name': 'Second Tag',
+    ('devtable', 'simple', None, 1, 10, 200, {
-                                                      'reversion': True,
+      'tags': [{
-                                                      'start_ts': 10}]}),
+        'docker_image_id': 'first docker image id',
-])
+        'name': 'First Tag',
+        'reversion': False
+      }, {
+        'docker_image_id': 'second docker image id',
+        'end_ts': 100,
+        'name': 'Second Tag',
+        'reversion': True,
+        'start_ts': 10
+      }]
+    }),
+  ])
 def test_list_repository_tags_view_is_correct(namespace, repository, specific_tag, page, limit,
-                                              list_repository_tag_history, expected_response_code, expected,
+                                              list_repository_tag_history, expected_response_code,
-                                              authd_client):
+                                              expected, authd_client):
-  params = {'repository': namespace + '/' + repository, 'specificTag': specific_tag, 'page': page, 'limit': limit}
+  params = {
-  response = conduct_api_call(authd_client, ListRepositoryTags, 'get', params, expected_code=expected_response_code)
+    'repository': namespace + '/' + repository,
+    'specificTag': specific_tag,
+    'page': page,
+    'limit': limit
+  }
+  response = conduct_api_call(authd_client, ListRepositoryTags, 'get', params,
+                              expected_code=expected_response_code)
   compare_list_history_tags_response(expected, response.json)
 
 
@@ -173,17 +205,23 @@ def test_no_repo_tag_history(find_no_repo_tag_history, authd_client):
     ('specific_tag', -1, 101, 'specific_tag', 1, 100),
     ('specific_tag', 0, 0, 'specific_tag', 1, 1),
   ])
-def test_repo_tag_history_param_parse(specific_tag, page, limit, expected_specific_tag, expected_page, expected_limit,
+def test_repo_tag_history_param_parse(specific_tag, page, limit, expected_specific_tag,
-                                      authd_client):
+                                      expected_page, expected_limit, authd_client):
   mock = MagicMock()
   mock.return_value = RepositoryTagHistory(tags=[], more=False)
 
-  with patch('endpoints.api.tag.pre_oci_model.list_repository_tag_history', side_effect=mock):
+  with patch('endpoints.api.tag.model.list_repository_tag_history', side_effect=mock):
-    params = {'repository': 'devtable/simple', 'specificTag': specific_tag, 'page': page, 'limit': limit}
+    params = {
+      'repository': 'devtable/simple',
+      'specificTag': specific_tag,
+      'page': page,
+      'limit': limit
+    }
     conduct_api_call(authd_client, ListRepositoryTags, 'get', params)
 
   assert mock.call_args == call(namespace_name='devtable', repository_name='simple',
-                                page=expected_page, size=expected_limit, specific_tag=expected_specific_tag)
+                                page=expected_page, size=expected_limit,
+                                specific_tag=expected_specific_tag)
 
 
 @pytest.mark.parametrize('test_manifest,test_tag,manifest_generated,expected_status', [