Fix ordering of tags in the OCI model

Before this change, we were neglecting to sort the tags by ID, which meant that pagination was broken

data/model/oci/tag.py

@@ -44,7 +44,7 @@ def get_tag(repository_id, tag_name):
     return None
 
 
-def list_alive_tags(repository_id, start_pagination_id=None, limit=None):
+def list_alive_tags(repository_id, start_pagination_id=None, limit=None, sort_tags=False):
   """ Returns a list of all the tags alive in the specified repository, with optional limits.
       Tag's returned are joined with their manifest.
   """
@@ -54,11 +54,15 @@ def list_alive_tags(repository_id, start_pagination_id=None, limit=None):
            .where(Tag.repository == repository_id))
 
   if start_pagination_id is not None:
+    assert sort_tags
     query = query.where(Tag.id >= start_pagination_id)
 
   if limit is not None:
     query = query.limit(limit)
 
+  if sort_tags:
+    query = query.order_by(Tag.id)
+
   return filter_to_visible_tags(filter_to_alive_tags(query))
 
 

data/registry_model/interface.py

@@ -114,7 +114,8 @@ class RegistryDataInterface(object):
   @abstractmethod
   def list_repository_tags(self, repository_ref, include_legacy_images=False,
                            start_pagination_id=None,
-                           limit=None):
+                           limit=None,
+                           sort_tags=False):
     """
     Returns a list of all the active tags in the repository. Note that this can be a *heavy*
     operation on repositories with a lot of tags, and should be avoided for more targetted

data/registry_model/registry_oci_model.py

@@ -201,13 +201,18 @@ class OCIModel(SharedModel, RegistryDataInterface):
 
   def list_repository_tags(self, repository_ref, include_legacy_images=False,
                            start_pagination_id=None,
-                           limit=None):
+                           limit=None,
+                           sort_tags=False):
     """
     Returns a list of all the active tags in the repository. Note that this can be a *heavy*
     operation on repositories with a lot of tags, and should be avoided for more targetted
     operations wherever possible.
     """
-    tags = list(oci.tag.list_alive_tags(repository_ref._db_id, start_pagination_id, limit))
+    if start_pagination_id:
+      assert sort_tags
+
+    tags = list(oci.tag.list_alive_tags(repository_ref._db_id, start_pagination_id, limit,
+                                        sort_tags=sort_tags))
     legacy_images_map = {}
     if include_legacy_images:
       legacy_images_map = oci.tag.get_legacy_images_for_tags(tags)

data/registry_model/registry_pre_oci_model.py

@@ -265,7 +265,8 @@ class PreOCIModel(SharedModel, RegistryDataInterface):
 
   def list_repository_tags(self, repository_ref, include_legacy_images=False,
                            start_pagination_id=None,
-                           limit=None):
+                           limit=None,
+                           sort_tags=False):
     """
     Returns a list of all the active tags in the repository. Note that this can be a *heavy*
     operation on repositories with a lot of tags, and should be avoided for more targetted

data/registry_model/test/test_interface.py

@@ -936,3 +936,37 @@ def test_unicode_emoji(registry_model):
   assert found.digest == manifest.digest
   assert found.internal_manifest_bytes.as_encoded_str() == manifest.bytes.as_encoded_str()
   assert found.get_parsed_manifest().digest == manifest.digest
+
+
+def test_list_repository_tags(oci_model):
+  repository_ref = oci_model.lookup_repository('devtable', 'simple')
+  latest_tag = oci_model.get_repo_tag(repository_ref, 'latest')
+  manifest = oci_model.get_manifest_for_tag(latest_tag)
+
+  tag_count = 500
+
+  # Create a bunch of tags.
+  tags_expected = set()
+  for index in range(0, tag_count):
+    tags_expected.add('somenewtag%s' % index)
+    oci_model.retarget_tag(repository_ref, 'somenewtag%s' % index, manifest, storage)
+
+  # List the tags.
+  tags_found = set()
+  tag_id = None
+  while True:
+    tags = oci_model.list_repository_tags(repository_ref, start_pagination_id=tag_id,
+                                          limit=11, sort_tags=True)
+    assert len(tags) <= 11
+    for tag in tags[0:10]:
+      assert tag.name not in tags_found
+      if tag.name in tags_expected:
+        tags_expected.remove(tag.name)
+
+    if len(tags) < 11:
+      break
+
+    tag_id = tags[10].id
+
+  # Make sure we've found all the tags.
+  assert not tags_expected

endpoints/v2/tag.py

@@ -21,7 +21,7 @@ def list_all_tags(namespace_name, repo_name, start_id, limit, pagination_callbac
   # NOTE: We add 1 to the limit because that's how pagination_callback knows if there are
   # additional tags.
   tags = registry_model.list_repository_tags(repository_ref, start_pagination_id=start_id,
-                                             limit=limit + 1)
+                                             limit=limit + 1, sort_tags=True)
   response = jsonify({
     'name': '{0}/{1}'.format(namespace_name, repo_name),
     'tags': [tag.name for tag in tags][0:limit],