From af27a1b6dc50adb358593f44d07218014d6cb995 Mon Sep 17 00:00:00 2001
From: Evan Cordell <cordell.evan@gmail.com>
Date: Thu, 20 Jul 2017 14:24:43 -0400
Subject: [PATCH 1/2] Add a data interface for manifest labels API

---
 endpoints/api/manifest.py                  | 92 +++++++++-------------
 endpoints/api/manifest_models_interface.py | 42 ++++++++++
 endpoints/api/manifest_models_pre_oci.py   | 50 ++++++++++++
 3 files changed, 128 insertions(+), 56 deletions(-)
 create mode 100644 endpoints/api/manifest_models_interface.py
 create mode 100644 endpoints/api/manifest_models_pre_oci.py

diff --git a/endpoints/api/manifest.py b/endpoints/api/manifest.py
index 66716da3f..7eaad827b 100644
--- a/endpoints/api/manifest.py
+++ b/endpoints/api/manifest.py
@@ -4,10 +4,11 @@ from app import label_validator
 from flask import request
 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, abort, api,
+                           path_param, parse_args, query_param, abort, api,
                            disallow_for_app_repositories)
 from endpoints.exception import NotFound
-from data import model
+from manifest_models_pre_oci import pre_oci_model as model
+from data.model import InvalidLabelKeyException, InvalidMediaTypeException
 
 from digest import digest_tools
 from util.validation import VALID_LABEL_KEY_REGEX
@@ -16,16 +17,6 @@ BASE_MANIFEST_ROUTE = '/v1/repository/<apirepopath:repository>/manifest/<regex("
 MANIFEST_DIGEST_ROUTE = BASE_MANIFEST_ROUTE.format(digest_tools.DIGEST_PATTERN)
 ALLOWED_LABEL_MEDIA_TYPES = ['text/plain', 'application/json']
 
-def label_view(label):
-  view = {
-    'id': label.uuid,
-    'key': label.key,
-    'value': label.value,
-    'source_type': label.source_type.name,
-    'media_type': label.media_type.name,
-  }
-
-  return view
 
 @resource(MANIFEST_DIGEST_ROUTE + '/labels')
 @path_param('repository', 'The full path of the repository. e.g. namespace/name')
@@ -65,28 +56,20 @@ class RepositoryManifestLabels(RepositoryParamResource):
   @parse_args()
   @query_param('filter', 'If specified, only labels matching the given prefix will be returned',
                type=str, default=None)
-  def get(self, namespace, repository, manifestref, parsed_args):
-    try:
-      tag_manifest = model.tag.load_manifest_by_digest(namespace, repository, manifestref)
-    except model.DataModelException:
+  def get(self, namespace_name, repository_name, manifestref, parsed_args):
+    labels = model.get_manifest_labels(namespace_name, repository_name, manifestref, filter=parsed_args['filter'])
+    if labels is None:
       raise NotFound()
-
-    labels = model.label.list_manifest_labels(tag_manifest, prefix_filter=parsed_args['filter'])
     return {
-      'labels': [label_view(label) for label in labels]
+      'labels': [label.to_dict() for label in labels]
     }
 
   @require_repo_write
   @nickname('addManifestLabel')
   @disallow_for_app_repositories
   @validate_json_request('AddLabel')
-  def post(self, namespace, repository, manifestref):
+  def post(self, namespace_name, repository_name, manifestref):
     """ Adds a new label into the tag manifest. """
-    try:
-      tag_manifest = model.tag.load_manifest_by_digest(namespace, repository, manifestref)
-    except model.DataModelException:
-      raise NotFound()
-
     label_data = request.get_json()
 
     # Check for any reserved prefixes.
@@ -95,29 +78,36 @@ class RepositoryManifestLabels(RepositoryParamResource):
 
     label = None
     try:
-      label = model.label.create_manifest_label(tag_manifest, label_data['key'],
-                                                label_data['value'], 'api',
-                                                media_type_name=label_data['media_type'])
-    except model.InvalidLabelKeyException:
+      label = model.create_manifest_label(namespace_name, 
+                                          repository_name, 
+                                          manifestref, 
+                                          label_data['key'], 
+                                          label_data['value'], 
+                                          'api', 
+                                          label_data['media_type'])
+    except InvalidLabelKeyException:
       abort(400, message='Label is of an invalid format or missing please use %s format for labels'.format(
             VALID_LABEL_KEY_REGEX))
-    except model.InvalidMediaTypeException:
+    except InvalidMediaTypeException:
       abort(400, message='Media type is invalid please use a valid media type of text/plain or application/json')
 
+    if label is None:
+      raise NotFound()
+    
     metadata = {
       'id': label.uuid,
-      'key': label_data['key'],
-      'value': label_data['value'],
+      'key': label.key,
+      'value': label.value,
       'manifest_digest': manifestref,
-      'media_type': label_data['media_type'],
-      'namespace': namespace,
-      'repo': repository,
+      'media_type': label.media_type_name,
+      'namespace': namespace_name,
+      'repo': repository_name,
     }
 
-    log_action('manifest_label_add', namespace, metadata, repo=tag_manifest.tag.repository)
+    log_action('manifest_label_add', namespace_name, metadata, repo_name=repository_name)
 
-    resp = {'label': label_view(label)}
-    repo_string = '%s/%s' % (namespace, repository)
+    resp = {'label': label.to_dict()}
+    repo_string = '%s/%s' % (namespace_name, repository_name)
     headers = {
       'Location': api.url_for(ManageRepositoryManifestLabel, repository=repo_string,
                               manifestref=manifestref, labelid=label.uuid),
@@ -134,31 +124,21 @@ class ManageRepositoryManifestLabel(RepositoryParamResource):
   @require_repo_read
   @nickname('getManifestLabel')
   @disallow_for_app_repositories
-  def get(self, namespace, repository, manifestref, labelid):
+  def get(self, namespace_name, repository_name, manifestref, labelid):
     """ Retrieves the label with the specific ID under the manifest. """
-    try:
-      tag_manifest = model.tag.load_manifest_by_digest(namespace, repository, manifestref)
-    except model.DataModelException:
-      raise NotFound()
-
-    label = model.label.get_manifest_label(labelid, tag_manifest)
+    label = model.get_manifest_label(namespace_name, repository_name, manifestref, labelid)
     if label is None:
       raise NotFound()
 
-    return label_view(label)
+    return label.to_dict()
 
 
   @require_repo_write
   @nickname('deleteManifestLabel')
   @disallow_for_app_repositories
-  def delete(self, namespace, repository, manifestref, labelid):
+  def delete(self, namespace_name, repository_name, manifestref, labelid):
     """ Deletes an existing label from a manifest. """
-    try:
-      tag_manifest = model.tag.load_manifest_by_digest(namespace, repository, manifestref)
-    except model.DataModelException:
-      raise NotFound()
-
-    deleted = model.label.delete_manifest_label(labelid, tag_manifest)
+    deleted = model.delete_manifest_label(namespace_name, repository_name, manifestref, labelid)
     if deleted is None:
       raise NotFound()
 
@@ -167,10 +147,10 @@ class ManageRepositoryManifestLabel(RepositoryParamResource):
       'key': deleted.key,
       'value': deleted.value,
       'manifest_digest': manifestref,
-      'namespace': namespace,
-      'repo': repository,
+      'namespace': namespace_name,
+      'repo': repository_name,
     }
 
-    log_action('manifest_label_delete', namespace, metadata, repo=tag_manifest.tag.repository)
+    log_action('manifest_label_delete', namespace_name, metadata, repo_name=repository_name)
     return '', 204
 
diff --git a/endpoints/api/manifest_models_interface.py b/endpoints/api/manifest_models_interface.py
new file mode 100644
index 000000000..5157fb0da
--- /dev/null
+++ b/endpoints/api/manifest_models_interface.py
@@ -0,0 +1,42 @@
+from abc import ABCMeta, abstractmethod
+from collections import namedtuple
+
+from six import add_metaclass
+
+
+class ManifestLabel(
+  namedtuple('ManifestLabel', [
+    'uuid',
+    'key',
+    'value',
+    'source_type_name',
+    'media_type_name',
+    ])):
+  
+  def to_dict(self):
+    return {
+      'id': self.uuid,
+      'key': self.key,
+      'value': self.value,
+      'source_type': self.source_type_name,
+      'media_type': self.media_type_name,
+    }
+  
+  
+@add_metaclass(ABCMeta)
+class ManifestLabelInterface(object):
+  @abstractmethod
+  def get_manifest_labels(self, namespace_name, repository_name, manifestref, filter=None):
+    pass
+
+  @abstractmethod
+  def create_manifest_label(self, namespace_name, repository_name, manifestref, key, value, source_type_name, media_type_name):
+    pass
+  
+  @abstractmethod
+  def get_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
+    pass
+  
+  @abstractmethod
+  def delete_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
+    pass
diff --git a/endpoints/api/manifest_models_pre_oci.py b/endpoints/api/manifest_models_pre_oci.py
new file mode 100644
index 000000000..5d0c7cbdd
--- /dev/null
+++ b/endpoints/api/manifest_models_pre_oci.py
@@ -0,0 +1,50 @@
+from manifest_models_interface import ManifestLabel, ManifestLabelInterface
+from data import model
+
+
+class ManifestLabelPreOCI(ManifestLabelInterface):
+  def get_manifest_labels(self, namespace_name, repository_name, manifestref, filter=None):
+    try:
+      tag_manifest = model.tag.load_manifest_by_digest(namespace_name, repository_name, manifestref)
+    except model.DataModelException:
+      return None
+
+    labels = model.label.list_manifest_labels(tag_manifest, prefix_filter=filter)
+    return [self._label(l) for l in labels]
+
+  def create_manifest_label(self, namespace_name, repository_name, manifestref, key, value, source_type_name, media_type_name):
+    try:
+      tag_manifest = model.tag.load_manifest_by_digest(namespace_name, repository_name, manifestref)
+    except model.DataModelException:
+      return None
+
+    return self._label(model.label.create_manifest_label(tag_manifest, key, value, source_type_name, media_type_name))
+  
+  def get_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
+    try:
+      tag_manifest = model.tag.load_manifest_by_digest(namespace_name, repository_name, manifestref)
+    except model.DataModelException:
+      return None
+
+    return self._label(model.label.get_manifest_label(label_uuid, tag_manifest))
+
+  def delete_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
+    try:
+      tag_manifest = model.tag.load_manifest_by_digest(namespace_name, repository_name, manifestref)
+    except model.DataModelException:
+      return None
+
+    return self._label(model.label.delete_manifest_label(label_uuid, tag_manifest))
+  
+  def _label(self, label_obj):
+    if not label_obj:
+      return None
+    return ManifestLabel(
+      uuid=label_obj.uuid,
+      key=label_obj.key,
+      value=label_obj.value,
+      source_type_name=label_obj.source_type.name,
+      media_type_name=label_obj.media_type.name,
+    )
+
+pre_oci_model = ManifestLabelPreOCI()
\ No newline at end of file

From c92b56642775602f80c452d26565225aa045f2f4 Mon Sep 17 00:00:00 2001
From: Evan Cordell <cordell.evan@gmail.com>
Date: Wed, 26 Jul 2017 14:21:51 -0400
Subject: [PATCH 2/2] Add doc comments to data interface for manifest labels

---
 endpoints/api/manifest_models_interface.py | 65 ++++++++++++++++++++--
 endpoints/api/manifest_models_pre_oci.py   |  2 +-
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/endpoints/api/manifest_models_interface.py b/endpoints/api/manifest_models_interface.py
index 5157fb0da..618e434b7 100644
--- a/endpoints/api/manifest_models_interface.py
+++ b/endpoints/api/manifest_models_interface.py
@@ -12,7 +12,14 @@ class ManifestLabel(
     'source_type_name',
     'media_type_name',
     ])):
-  
+  """
+  ManifestLabel represents a label on a manifest
+  :type uuid: string
+  :type key: string 
+  :type value: string 
+  :type source_type_name: string  
+  :type media_type_name: string 
+  """ 
   def to_dict(self):
     return {
       'id': self.uuid,
@@ -25,18 +32,66 @@ class ManifestLabel(
   
 @add_metaclass(ABCMeta)
 class ManifestLabelInterface(object):
+  """
+  Data interface that the manifest labels API uses
+  """
+  
   @abstractmethod
   def get_manifest_labels(self, namespace_name, repository_name, manifestref, filter=None):
-    pass
+    """
+    
+    Args:
+      namespace_name: string
+      repository_name: string
+      manifestref: string
+      filter: string
+
+    Returns:
+      list(ManifestLabel) or None
+
+    """   
 
   @abstractmethod
   def create_manifest_label(self, namespace_name, repository_name, manifestref, key, value, source_type_name, media_type_name):
-    pass
+    """
+    
+    Args:
+      namespace_name: string
+      repository_name: string
+      manifestref: string
+      key: string
+      value: string
+      source_type_name: string 
+      media_type_name: string
+
+    Returns:
+      ManifestLabel or None 
+    """
   
   @abstractmethod
   def get_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
-    pass
+    """
+    
+    Args:
+      namespace_name: string
+      repository_name: string
+      manifestref: string
+      label_uuid: string
+
+    Returns:
+      ManifestLabel or None
+    """
   
   @abstractmethod
   def delete_manifest_label(self, namespace_name, repository_name, manifestref, label_uuid):
-    pass
+    """
+    
+    Args:
+      namespace_name: string
+      repository_name: string
+      manifestref: string
+      label_uuid: string
+
+    Returns:
+      ManifestLabel or None
+    """
diff --git a/endpoints/api/manifest_models_pre_oci.py b/endpoints/api/manifest_models_pre_oci.py
index 5d0c7cbdd..9cc44ce93 100644
--- a/endpoints/api/manifest_models_pre_oci.py
+++ b/endpoints/api/manifest_models_pre_oci.py
@@ -47,4 +47,4 @@ class ManifestLabelPreOCI(ManifestLabelInterface):
       media_type_name=label_obj.media_type.name,
     )
 
-pre_oci_model = ManifestLabelPreOCI()
\ No newline at end of file
+pre_oci_model = ManifestLabelPreOCI()