From 1dc5539c7a4c3ff18472d2483924d328c0c2f0ef Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Fri, 20 Sep 2024 14:52:23 -0400 Subject: [PATCH] Fix content__in filter fixes: #3634 --- CHANGES/3634.bugfix | 1 + pulpcore/app/viewsets/custom_filters.py | 45 +++++++++++++++- pulpcore/app/viewsets/publication.py | 26 ++-------- pulpcore/app/viewsets/repository.py | 38 ++------------ .../functional/api/test_openapi_schema.py | 17 ++++++ .../api/using_plugin/test_repo_versions.py | 52 ++++++++++++++++--- 6 files changed, 112 insertions(+), 67 deletions(-) create mode 100644 CHANGES/3634.bugfix diff --git a/CHANGES/3634.bugfix b/CHANGES/3634.bugfix new file mode 100644 index 0000000000..1ecb92ff36 --- /dev/null +++ b/CHANGES/3634.bugfix @@ -0,0 +1 @@ +Fixed Publication & RepositoryVersion's `content__in` filter to properly accept an array of strings. diff --git a/pulpcore/app/viewsets/custom_filters.py b/pulpcore/app/viewsets/custom_filters.py index 7baded35bd..42ee56f474 100644 --- a/pulpcore/app/viewsets/custom_filters.py +++ b/pulpcore/app/viewsets/custom_filters.py @@ -15,9 +15,9 @@ from rest_framework import serializers from rest_framework.serializers import ValidationError as DRFValidationError -from pulpcore.app.models import ContentArtifact, RepositoryVersion, Publication +from pulpcore.app.models import Content, ContentArtifact, RepositoryVersion, Publication from pulpcore.app.viewsets import NamedModelViewSet -from pulpcore.app.util import get_prn, get_domain_pk +from pulpcore.app.util import get_prn, get_domain_pk, extract_pk, raise_for_unknown_content_units # Lookup conversion table from old resource hrefs to new PDRN resource names @@ -325,6 +325,47 @@ def filter(self, qs, value): return qs +class WithContentFilter(Filter): + """Filter class for filtering by content in Publications and RepositoryVersions.""" + + def __init__(self, *args, **kwargs): + kwargs.setdefault("help_text", _("Content Unit referenced by HREF")) + super().__init__(*args, **kwargs) + + def filter(self, qs, value): + """ + Args: + qs (django.db.models.query.QuerySet): The Publication/RepositoryVersion Queryset + value (string or list[string]): of content href(s) to filter + + Returns: + Queryset of the Publication/RepositoryVersion containing the specified content + """ + if value is None: + # user didn't supply a value + return qs + + if not value: + raise serializers.ValidationError(detail=_("No value supplied for content filter")) + + if isinstance(value, str): + value = [value] + + content_units = {} + for url in value: + content_units[extract_pk(url)] = url + + content_units_pks = set(content_units.keys()) + existing_content_units = Content.objects.filter(pk__in=content_units_pks) + raise_for_unknown_content_units(existing_content_units, content_units) + + return qs.with_content(content_units_pks) + + +class WithContentInFilter(BaseInFilter, WithContentFilter): + """The multi-content variant of WithContentFilter.""" + + class DistributionWithContentFilter(Filter): """A Filter class enabling filtering by content units served by distributions.""" diff --git a/pulpcore/app/viewsets/publication.py b/pulpcore/app/viewsets/publication.py index 320613951d..31bd544745 100644 --- a/pulpcore/app/viewsets/publication.py +++ b/pulpcore/app/viewsets/publication.py @@ -15,7 +15,6 @@ Distribution, Publication, Repository, - Content, ArtifactDistribution, ) from pulpcore.app.serializers import ( @@ -40,29 +39,12 @@ DistributionWithContentFilter, LabelFilter, RepositoryVersionFilter, + WithContentFilter, + WithContentInFilter, ) from pulpcore.app.util import get_domain -class PublicationContentFilter(Filter): - def __init__(self, *args, **kwargs): - kwargs.setdefault("help_text", _("Content Unit referenced by HREF")) - super().__init__(*args, **kwargs) - - def filter(self, qs, value): - if value is None: - # user didn't supply a value - return qs - - if not value: - raise serializers.ValidationError(detail=_("No value supplied for content filter")) - - # Get the content object from the content_href - content = NamedModelViewSet.get_resource(value, Content) - - return qs.with_content([content.pk]) - - class RepositoryThroughVersionFilter(Filter): def filter(self, qs, value): if value is None: @@ -81,8 +63,8 @@ def filter(self, qs, value): class PublicationFilter(BaseFilterSet): repository = RepositoryThroughVersionFilter(help_text=_("Repository referenced by HREF")) repository_version = RepositoryVersionFilter() - content = PublicationContentFilter() - content__in = PublicationContentFilter(field_name="content", lookup_expr="in") + content = WithContentFilter() + content__in = WithContentInFilter() class Meta: model = Publication diff --git a/pulpcore/app/viewsets/repository.py b/pulpcore/app/viewsets/repository.py index ac121bb20d..c794cb51e9 100644 --- a/pulpcore/app/viewsets/repository.py +++ b/pulpcore/app/viewsets/repository.py @@ -39,7 +39,7 @@ NAME_FILTER_OPTIONS, NULLABLE_NUMERIC_FILTER_OPTIONS, ) -from pulpcore.app.viewsets.custom_filters import LabelFilter +from pulpcore.app.viewsets.custom_filters import LabelFilter, WithContentFilter, WithContentInFilter from pulpcore.tasking.tasks import dispatch from pulpcore.filters import HyperlinkRelatedFilter @@ -179,38 +179,6 @@ class RepositoryViewSet(ImmutableRepositoryViewSet, AsyncUpdateMixin, LabelsMixi """ -class RepositoryVersionContentFilter(Filter): - """ - Filter used to get the repository versions where some given content can be found. - """ - - def __init__(self, *args, **kwargs): - kwargs.setdefault("help_text", _("Content Unit referenced by HREF")) - super().__init__(*args, **kwargs) - - def filter(self, qs, value): - """ - Args: - qs (django.db.models.query.QuerySet): The RepositoryVersion Queryset - value (string): of content href to filter - - Returns: - Queryset of the RepositoryVersions containing the specified content - """ - - if value is None: - # user didn't supply a value - return qs - - if not value: - raise serializers.ValidationError(detail=_("No value supplied for content filter")) - - # Get the content object from the content_href - content = NamedModelViewSet.get_resource(value, Content) - - return qs.with_content([content.pk]) - - class RepositoryVersionFilter(BaseFilterSet): # e.g. # /?number=4 @@ -218,8 +186,8 @@ class RepositoryVersionFilter(BaseFilterSet): # /?pulp_created__gte=2018-04-12T19:45 # /?pulp_created__range=2018-04-12T19:45,2018-04-13T20:00 # /?content=/pulp/api/v3/content/file/fb8ad2d0-03a8-4e36-a209-77763d4ed16c/ - content = RepositoryVersionContentFilter() - content__in = RepositoryVersionContentFilter(field_name="content", lookup_expr="in") + content = WithContentFilter() + content__in = WithContentInFilter() def filter_pulp_href(self, queryset, name, value): """Special handling for RepositoryVersion HREF filtering.""" diff --git a/pulpcore/tests/functional/api/test_openapi_schema.py b/pulpcore/tests/functional/api/test_openapi_schema.py index 38412797aa..3f1025bbd8 100644 --- a/pulpcore/tests/functional/api/test_openapi_schema.py +++ b/pulpcore/tests/functional/api/test_openapi_schema.py @@ -101,3 +101,20 @@ def test_external_auth_on_security_scheme(pulp_settings, pulp_openapi_schema): security_scheme = security_schemes["json_header_remote_authentication"] assert pulp_settings.AUTHENTICATION_JSON_HEADER_OPENAPI_SECURITY_SCHEME == security_scheme + + +@pytest.mark.parallel +def test_content_in_filter_is_array(pulp_openapi_schema): + tested = [] + for name, path in pulp_openapi_schema["paths"].items(): + if name.endswith("repository_versions/") or name.endswith("publications/"): + tested.append(name) + for parameter in path["get"]["parameters"]: + if parameter["name"] == "content__in": + schema = parameter["schema"] + assert schema["type"] == "array" + assert schema["items"]["type"] == "string" + break + else: + assert False, "Couldn't find the content__in filter!" + assert len(tested) == 2, "Couldn't test both the Publication and RepositoryVersion endpoints" diff --git a/pulpcore/tests/functional/api/using_plugin/test_repo_versions.py b/pulpcore/tests/functional/api/using_plugin/test_repo_versions.py index 30b42a5465..115dcba355 100644 --- a/pulpcore/tests/functional/api/using_plugin/test_repo_versions.py +++ b/pulpcore/tests/functional/api/using_plugin/test_repo_versions.py @@ -934,7 +934,7 @@ def test_content_in_repository_version_view( pulpcore_bindings, file_bindings, file_repository_factory, - file_random_content_unit, + file_content_unit_with_name_factory, monitor_task, ): """Sync two repositories and check view filter.""" @@ -950,17 +950,17 @@ def test_content_in_repository_version_view( repo = file_repository_factory() repo2 = file_repository_factory() + all_content = [file_content_unit_with_name_factory(str(uuid4())) for i in range(3)] + content = choice(all_content) # Add content to first repo and assert repo-ver list w/ content is correct - body = {"add_content_units": [file_random_content_unit.pulp_href]} + body = {"add_content_units": [content.pulp_href]} task = file_bindings.RepositoriesFileApi.modify(repo.pulp_href, body).task monitor_task(task) repo = file_bindings.RepositoriesFileApi.read(repo.pulp_href) assert repo.latest_version_href[-2] == "1" - repo_vers = pulpcore_bindings.RepositoryVersionsApi.list( - content=file_random_content_unit.pulp_href - ) + repo_vers = pulpcore_bindings.RepositoryVersionsApi.list(content=content.pulp_href) assert repo_vers.count == 1 assert repo_vers.results[0].pulp_href == repo.latest_version_href @@ -970,11 +970,47 @@ def test_content_in_repository_version_view( repo2 = file_bindings.RepositoriesFileApi.read(repo2.pulp_href) assert repo2.latest_version_href[-2] == "1" - repo_vers = pulpcore_bindings.RepositoryVersionsApi.list( - content=file_random_content_unit.pulp_href - ) + repo_vers = pulpcore_bindings.RepositoryVersionsApi.list(content=content.pulp_href) + assert repo_vers.count == 2 + assert {r.pulp_href for r in repo_vers.results} == { + repo.latest_version_href, + repo2.latest_version_href, + } + + # Test that content__in w/ one unit gives identical results to content filter + repo_vers = pulpcore_bindings.RepositoryVersionsApi.list(content__in=[content.pulp_href]) assert repo_vers.count == 2 assert {r.pulp_href for r in repo_vers.results} == { repo.latest_version_href, repo2.latest_version_href, } + + # Add all content to first repo and assert content__in w/ more content returns 1 repo-ver + all_content_href = [c.pulp_href for c in all_content] + body = {"add_content_units": all_content_href} + task = file_bindings.RepositoriesFileApi.modify(repo.pulp_href, body).task + monitor_task(task) + repo = file_bindings.RepositoriesFileApi.read(repo.pulp_href) + assert repo.latest_version_href[-2] == "2" + + contents = list(set(all_content_href) - {content.pulp_href}) + repo_vers = pulpcore_bindings.RepositoryVersionsApi.list(content__in=contents) + assert repo_vers.count == 1 + assert repo_vers.results[0].pulp_href == repo.latest_version_href + + # Add back the first content and assert that all 3 repo-vers are returned + contents.append(content.pulp_href) + repo_vers = pulpcore_bindings.RepositoryVersionsApi.list(content__in=contents) + assert repo_vers.count == 3 + assert {r.pulp_href for r in repo_vers.results} == { + f"{repo.versions_href}1/", + repo.latest_version_href, + repo2.latest_version_href, + } + + # Test content__in validates the hrefs passed in + contents.append(non_existant_content_href) + with pytest.raises(pulpcore_bindings.ApiException) as e: + pulpcore_bindings.RepositoryVersionsApi.list(content__in=contents) + + assert e.value.status == 400