diff --git a/data/database.py b/data/database.py index ac25421ab..a673c9a71 100644 --- a/data/database.py +++ b/data/database.py @@ -398,6 +398,16 @@ class QuayUserField(ForeignKeyField): super(QuayUserField, self).__init__(*args, **kwargs) +@lru_cache(maxsize=16) +def _get_enum_field_values(enum_field): + values = [] + for row in enum_field.rel_model.select(): + key = getattr(row, enum_field.enum_key_field) + value = getattr(row, 'id') + values.append((key, value)) + return Enum(enum_field.rel_model.__name__, values) + + class EnumField(ForeignKeyField): """ Create a cached python Enum from an EnumTable """ def __init__(self, model, enum_key_field='name', *args, **kwargs): @@ -409,15 +419,9 @@ class EnumField(ForeignKeyField): super(EnumField, self).__init__(model, *args, **kwargs) @property - @lru_cache(maxsize=1) def enum(self): """ Returns a python enun.Enum generated from the associated EnumTable """ - values = [] - for row in self.rel_model.select(): - key = getattr(row, self.enum_key_field) - value = getattr(row, 'id') - values.append((key, value)) - return Enum(self.rel_model.__name__, values) + return _get_enum_field_values(self) def get_id(self, name): """ Returns the ForeignKeyId from the name field diff --git a/data/registry_model/datatypes.py b/data/registry_model/datatypes.py index bb64da471..60002c7bf 100644 --- a/data/registry_model/datatypes.py +++ b/data/registry_model/datatypes.py @@ -6,6 +6,7 @@ from enum import Enum, unique from cachetools import lru_cache from data import model +from data.database import Manifest as ManifestTable from data.registry_model.datatype import datatype, requiresinput, optionalinput from image.docker import ManifestException from image.docker.schemas import parse_manifest_from_bytes @@ -197,7 +198,7 @@ class Manifest(datatype('Manifest', ['digest', 'media_type', 'internal_manifest_ return Manifest(db_id=manifest.id, digest=manifest.digest, internal_manifest_bytes=Bytes.for_string_or_unicode(manifest.manifest_bytes), - media_type=manifest.media_type.name, + media_type=ManifestTable.media_type.get_name(manifest.media_type_id), inputs=dict(legacy_image=legacy_image, tag_manifest=False)) @property diff --git a/data/registry_model/test/test_interface.py b/data/registry_model/test/test_interface.py index 5eb0e2d38..c20bb28ff 100644 --- a/data/registry_model/test/test_interface.py +++ b/data/registry_model/test/test_interface.py @@ -256,17 +256,31 @@ def test_repository_tags(repo_namespace, repo_name, registry_model): assert tags_map[tag.name] == found_image.docker_image_id -def test_repository_tag_history(registry_model): - repository_ref = registry_model.lookup_repository('devtable', 'history') +@pytest.mark.parametrize('namespace, name, expected_tag_count, has_expired', [ + ('devtable', 'simple', 2, False), + ('devtable', 'history', 2, True), + ('devtable', 'gargantuan', 8, False), + ('public', 'publicrepo', 1, False), +]) +def test_repository_tag_history(namespace, name, expected_tag_count, has_expired, registry_model): + # Pre-cache media type loads to ensure consistent query count. + Manifest.media_type.get_name(1) + + repository_ref = registry_model.lookup_repository(namespace, name) with assert_query_count(4 if isinstance(registry_model, SplitModel) else 2): history, has_more = registry_model.list_repository_tag_history(repository_ref) assert not has_more - assert len(history) == 2 + assert len(history) == expected_tag_count - # Ensure the latest tag is marked expired, since there is an expired one. - with assert_query_count(1): - assert registry_model.has_expired_tag(repository_ref, 'latest') + for tag in history: + # Retrieve the manifest to ensure it doesn't issue extra queries. + tag.manifest + + if has_expired: + # Ensure the latest tag is marked expired, since there is an expired one. + with assert_query_count(1): + assert registry_model.has_expired_tag(repository_ref, 'latest') @pytest.mark.parametrize('repo_namespace, repo_name', [ diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index ed08b104c..bdc631254 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -37,6 +37,7 @@ def _tag_dict(tag): # TODO(jschorr): Remove this check once fully on OCI data model. if tag.manifest_digest: tag_info['manifest_digest'] = tag.manifest_digest + if tag.manifest: try: tag_info['manifest'] = json.loads(tag.manifest.internal_manifest_bytes.as_unicode()) diff --git a/endpoints/api/test/test_tag.py b/endpoints/api/test/test_tag.py index f3da18972..c97164282 100644 --- a/endpoints/api/test/test_tag.py +++ b/endpoints/api/test/test_tag.py @@ -1,6 +1,9 @@ import pytest +from playhouse.test_utils import assert_query_count + from data.registry_model import registry_model +from data.database import Manifest from endpoints.api.test.shared import conduct_api_call from endpoints.test.shared import client_with_identity @@ -75,16 +78,23 @@ def test_move_tag(image_exists, test_tag, expected_status, client, app): conduct_api_call(cl, RepositoryTag, 'put', params, request_body, expected_status) -@pytest.mark.parametrize('repo_namespace, repo_name', [ - ('devtable', 'simple'), - ('devtable', 'history'), - ('devtable', 'complex'), - ('buynlarge', 'orgrepo'), +@pytest.mark.parametrize('repo_namespace, repo_name, query_count', [ + ('devtable', 'simple', 7), + ('devtable', 'history', 7), + ('devtable', 'complex', 7), + ('devtable', 'gargantuan', 7), + ('buynlarge', 'orgrepo', 9), # +2 for permissions checks. + ('buynlarge', 'anotherorgrepo', 9), # +2 for permissions checks. ]) -def test_list_repo_tags(repo_namespace, repo_name, client, app): +def test_list_repo_tags(repo_namespace, repo_name, client, query_count, app): + # Pre-cache media type loads to ensure consistent query count. + Manifest.media_type.get_name(1) + params = {'repository': repo_namespace + '/' + repo_name} with client_with_identity('devtable', client) as cl: - tags = conduct_api_call(cl, ListRepositoryTags, 'get', params).json['tags'] + with assert_query_count(query_count): + tags = conduct_api_call(cl, ListRepositoryTags, 'get', params).json['tags'] + repo_ref = registry_model.lookup_repository(repo_namespace, repo_name) history, _ = registry_model.list_repository_tag_history(repo_ref) assert len(tags) == len(history)