Fix content__in filter

CHANGES/3634.bugfix

@@ -0,0 +1 @@
+Fixed Publication & RepositoryVersion's `content__in` filter to properly accept an array of strings.

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."""
 

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

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,47 +179,15 @@ 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
     # /?number__range=4,6
     # /?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."""

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"

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