From fd666c7f40655c43453fcfe57fb1cefa9c4fd42a Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Tue, 17 Sep 2024 10:12:33 -0400 Subject: [PATCH] Add PRN support fixes: #5766 --- CHANGES/5766.feature | 4 + docs/dev/learn/subclassing/serializers.md | 17 ++- pulpcore/app/serializers/__init__.py | 1 + pulpcore/app/serializers/base.py | 38 ++++-- pulpcore/app/serializers/task.py | 1 + pulpcore/app/serializers/user.py | 12 +- pulpcore/app/util.py | 63 ++++++++-- pulpcore/app/viewsets/__init__.py | 2 +- pulpcore/app/viewsets/base.py | 40 +++--- pulpcore/app/viewsets/custom_filters.py | 40 +++--- pulpcore/app/viewsets/publication.py | 2 +- pulpcore/app/viewsets/repository.py | 34 +++-- pulpcore/filters.py | 48 ++++--- pulpcore/plugin/serializers/__init__.py | 1 + pulpcore/plugin/util.py | 2 + pulpcore/pytest_plugin.py | 15 +++ .../functional/api/test_openapi_schema.py | 16 --- .../functional/api/using_plugin/test_prn.py | 119 ++++++++++++++++++ .../functional/api/using_plugin/test_tasks.py | 2 +- .../tests/unit/serializers/test_domain.py | 1 + .../tests/unit/serializers/test_repository.py | 1 + 21 files changed, 357 insertions(+), 102 deletions(-) create mode 100644 CHANGES/5766.feature create mode 100644 pulpcore/tests/functional/api/using_plugin/test_prn.py diff --git a/CHANGES/5766.feature b/CHANGES/5766.feature new file mode 100644 index 0000000000..9bc4c291f9 --- /dev/null +++ b/CHANGES/5766.feature @@ -0,0 +1,4 @@ +Introduced new immutable resource identifier: Pulp Resource Name (PRN). All objects within Pulp +will now show their PRN alongside their pulp_href. The PRN can be used in lieu of the pulp_href +in API calls when creating or filtering objects. The PRN of any object has the form of +`prn:app_label.model_label:pulp_id`. diff --git a/docs/dev/learn/subclassing/serializers.md b/docs/dev/learn/subclassing/serializers.md index d7809d2fc6..0ade5be97a 100644 --- a/docs/dev/learn/subclassing/serializers.md +++ b/docs/dev/learn/subclassing/serializers.md @@ -11,8 +11,10 @@ Most plugins will implement: : - serializer(s) for plugin specific content type(s), should be subclassed from one of NoArtifactContentSerializer, SingleArtifactContentSerializer, or MultipleArtifactContentSerializer, depending on the properties of the content type(s) + - serializer(s) for plugin specific repository(s), should be subclassed from RepositorySerializer - serializer(s) for plugin specific remote(s), should be subclassed from RemoteSerializer - - serializer(s) for plugin specific publisher(s), should be subclassed from PublisherSerializer + - serializer(s) for plugin specific publication(s), should be subclassed from PublicationSerializer + - serializer(s) for plugin specific distribution(s), should be subclassed from DistributionSerializer ## Adding Fields @@ -34,6 +36,19 @@ class Meta: model = FileContent ``` +!!! note + + When inheriting from `pulpcore.plugin.serializers.ModelSerializer`, or one of its many subclasses, + you should include the fields of that serializer in your own. Important fields it provides include + `pulp_created`, `pulp_last_updated`, and `prn`. + + See `pulpcore.plugin.serializers.ContentGuardSerializer` for an example: + ```python + class Meta: + model = models.ContentGuard + fields = ModelSerializer.Meta.fields + ("name", "description") + ``` + ### Help Text The REST APIs of Pulp Core and each plugin are automatically documented using swagger. Each field's diff --git a/pulpcore/app/serializers/__init__.py b/pulpcore/app/serializers/__init__.py index ccdc5327bc..f05416fcbc 100644 --- a/pulpcore/app/serializers/__init__.py +++ b/pulpcore/app/serializers/__init__.py @@ -13,6 +13,7 @@ ModelSerializer, NestedIdentityField, NestedRelatedField, + PRNField, RelatedField, RelatedResourceField, SetLabelSerializer, diff --git a/pulpcore/app/serializers/base.py b/pulpcore/app/serializers/base.py index 8a174de3ca..3b00a8a3e7 100644 --- a/pulpcore/app/serializers/base.py +++ b/pulpcore/app/serializers/base.py @@ -35,6 +35,8 @@ get_request_without_query_params, get_domain, reverse, + get_prn, + resolve_prn, ) @@ -61,14 +63,22 @@ def _patched_reverse(viewname, request=None, args=None, kwargs=None, **extra): return reverse -class HrefFieldMixin: - """A mixin to configure related fields to generate relative hrefs.""" +class HrefPrnFieldMixin: + """A mixin to configure related fields to generate relative hrefs and accept PRNs.""" def get_url(self, obj, view_name, request, *args, **kwargs): # Use the Pulp reverse method to display relative hrefs. self.reverse = _reverse(obj) return super().get_url(obj, view_name, request, *args, **kwargs) + def to_internal_value(self, data): + # Properly also handle PRNs as values by converting them to URLs first + if data.startswith("prn:"): + model, pk = resolve_prn(data) + obj = model(pk=pk) if self.use_pk_only_optimization() else model.objects.get(pk=pk) + data = self.get_url(obj, self.view_name, self.context.get("request"), None) + return super().to_internal_value(data) + class _MatchingRegexViewName(object): """This is a helper class to help defining object matching rules for master-detail. @@ -90,7 +100,7 @@ def __eq__(self, other): return re.fullmatch(self.pattern, other) is not None -class _DetailFieldMixin(HrefFieldMixin): +class _DetailFieldMixin(HrefPrnFieldMixin): """Mixin class containing code common to DetailIdentityField and DetailRelatedField""" def __init__(self, view_name=None, view_name_pattern=None, **kwargs): @@ -132,7 +142,7 @@ def get_url(self, obj, view_name, request, *args, **kwargs): class IdentityField( - HrefFieldMixin, + HrefPrnFieldMixin, serializers.HyperlinkedIdentityField, ): """IdentityField for use in the pulp_href field of non-Master/Detail Serializers. @@ -142,7 +152,7 @@ class IdentityField( class RelatedField( - HrefFieldMixin, + HrefPrnFieldMixin, serializers.HyperlinkedRelatedField, ): """RelatedField when relating to non-Master/Detail models @@ -151,6 +161,17 @@ class RelatedField( """ +class PRNField(serializers.StringRelatedField): + """A special IdentityField that shows any object's PRN.""" + + def __init__(self, **kwargs): + kwargs["source"] = "*" + super().__init__(**kwargs) + + def to_representation(self, value): + return get_prn(instance=value) + + PKObject = namedtuple("PKObject", ["pk"]) PKDomainObject = namedtuple("PKDomainObject", ["pk", "pulp_domain"]) @@ -255,7 +276,7 @@ class to get the relevant `view_name`. return False -class NestedIdentityField(HrefFieldMixin, NestedHyperlinkedIdentityField): +class NestedIdentityField(HrefPrnFieldMixin, NestedHyperlinkedIdentityField): """NestedIdentityField for use with nested resources. When using this field in a serializer, it serializes the resource as a relative URL. @@ -263,7 +284,7 @@ class NestedIdentityField(HrefFieldMixin, NestedHyperlinkedIdentityField): class NestedRelatedField( - HrefFieldMixin, + HrefPrnFieldMixin, NestedHyperlinkedRelatedField, ): """NestedRelatedField for use when relating to nested resources. @@ -417,8 +438,9 @@ class ModelSerializer( exclude_arg_name = "exclude_fields" class Meta: - fields = ("pulp_href", "pulp_created", "pulp_last_updated") + fields = ("pulp_href", "prn", "pulp_created", "pulp_last_updated") + prn = PRNField(help_text=_("The Pulp Resource Name (PRN).")) pulp_created = serializers.DateTimeField(help_text=_("Timestamp of creation."), read_only=True) pulp_last_updated = serializers.DateTimeField( help_text=_( diff --git a/pulpcore/app/serializers/task.py b/pulpcore/app/serializers/task.py index afb6c57d48..ae7bf0f968 100755 --- a/pulpcore/app/serializers/task.py +++ b/pulpcore/app/serializers/task.py @@ -166,6 +166,7 @@ class Meta: model = models.TaskGroup fields = ( "pulp_href", + "prn", "description", "all_tasks_dispatched", "waiting", diff --git a/pulpcore/app/serializers/user.py b/pulpcore/app/serializers/user.py index ff4b50d9ee..0a7627c4de 100644 --- a/pulpcore/app/serializers/user.py +++ b/pulpcore/app/serializers/user.py @@ -20,6 +20,7 @@ ModelSerializer, HiddenFieldsMixin, RelatedField, + PRNField, ) from pulpcore.app.util import ( get_viewset_for_model, @@ -86,16 +87,18 @@ class UserGroupSerializer(serializers.ModelSerializer): name = serializers.CharField(help_text=_("Name."), max_length=150) pulp_href = IdentityField(view_name="groups-detail") + prn = PRNField() class Meta: model = Group - fields = ("name", "pulp_href") + fields = ("name", "pulp_href", "prn") class UserSerializer(serializers.ModelSerializer, HiddenFieldsMixin): """Serializer for User.""" pulp_href = IdentityField(view_name="users-detail") + prn = PRNField() id = serializers.IntegerField(read_only=True) username = serializers.CharField( help_text=_("Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only."), @@ -138,6 +141,7 @@ class Meta: model = User fields = ( "pulp_href", + "prn", "id", "username", "password", @@ -160,10 +164,11 @@ class GroupUserSerializer(ValidateFieldsMixin, serializers.ModelSerializer): max_length=150, ) pulp_href = IdentityField(view_name="users-detail") + prn = PRNField() class Meta: model = User - fields = ("username", "pulp_href") + fields = ("username", "pulp_href", "prn") class GroupSerializer(ValidateFieldsMixin, serializers.ModelSerializer): @@ -176,10 +181,11 @@ class GroupSerializer(ValidateFieldsMixin, serializers.ModelSerializer): max_length=150, validators=[UniqueValidator(queryset=Group.objects.all())], ) + prn = PRNField() class Meta: model = Group - fields = ("name", "pulp_href", "id") + fields = ("name", "pulp_href", "prn", "id") class RoleSerializer(ModelSerializer): diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index 4d8da2614e..dacaedce90 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -10,7 +10,9 @@ from contextlib import ExitStack from contextvars import ContextVar from datetime import timedelta +from uuid import UUID +from django.apps import apps from django.conf import settings from django.db import connection from django.db.models import Model, Sum @@ -120,12 +122,51 @@ def get_prn(instance=None, uri=None): return f"prn:{instance._meta.label_lower}:{instance.pk}" -def extract_pk(uri): +def resolve_prn(prn): """ - Resolve a resource URI to a simple PK value. + Resolve a PRN to its model and pk. - Provides a means to resolve an href passed in a POST body to a primary key. - Doesn't assume anything about whether the resource corresponding to the URI + Args: + prn (str): The PRN to resolve. + + Returns: + model_pk_tuple (tuple): A tuple of the model class and pk of the PRN + + Raises: + rest_framework.exceptions.ValidationError: on invalid PRN. + """ + if not prn.startswith("prn:"): + raise ValidationError(_("PRN must start with 'prn:': {}").format(prn)) + split = prn.split(":") + if len(split) != 3: + raise ValidationError( + _("PRN must be of the form 'prn:app_label.model_label:pk': {}").format(prn) + ) + p, full_model_label, pk = split + try: + model = apps.get_model(full_model_label) + except LookupError: + raise ValidationError(_("Model {} does not exist").format(full_model_label)) + except ValueError: + raise ValidationError( + _("The model must be in the form of 'app_label.model_label': {}").format( + full_model_label + ) + ) + try: + UUID(pk, version=4) + except ValueError: + raise ValidationError(_("PK invalid: {}").format(pk)) + + return model, pk + + +def extract_pk(uri, only_prn=False): + """ + Resolve a resource URI or PRN to a simple PK value. + + Provides a means to resolve an href/prn passed in a POST body to a primary key. + Doesn't assume anything about whether the resource corresponding to the URI/PRN passed in actually exists. Note: @@ -134,14 +175,22 @@ def extract_pk(uri): RepositoryVersion PK is present within the URI. Args: - uri (str): A resource URI. + uri (str): A resource URI/PRN. + only_prn (bool): Ensure passed in value is only a valid PRN Returns: - primary_key (uuid.uuid4): The primary key of the resource extracted from the URI. + primary_key (uuid.uuid4): The primary key of the resource extracted from the URI/PRN. Raises: - rest_framework.exceptions.ValidationError: on invalid URI. + rest_framework.exceptions.ValidationError: on invalid URI/PRN. """ + if uri.startswith("prn:"): + prn = uri.split(":") + if len(prn) != 3: + raise ValidationError(_("PRN not valid: {p}").format(p=prn)) + return prn[2] + elif only_prn: + raise ValidationError(_("Not a valid PRN: {p}, must start with 'prn:'").format(p=uri)) try: match = resolve(urlparse(uri).path) except Resolver404: diff --git a/pulpcore/app/viewsets/__init__.py b/pulpcore/app/viewsets/__init__.py index 4455bdf14e..3d01156594 100644 --- a/pulpcore/app/viewsets/__init__.py +++ b/pulpcore/app/viewsets/__init__.py @@ -23,7 +23,7 @@ SigningServiceViewSet, ) from .custom_filters import ( - RepoVersionHrefFilter, + RepoVersionHrefPrnFilter, RepositoryVersionFilter, ) from .domain import DomainViewSet diff --git a/pulpcore/app/viewsets/base.py b/pulpcore/app/viewsets/base.py index 757651302d..eaeb2fcc03 100644 --- a/pulpcore/app/viewsets/base.py +++ b/pulpcore/app/viewsets/base.py @@ -27,7 +27,7 @@ SetLabelSerializer, UnsetLabelSerializer, ) -from pulpcore.app.util import get_viewset_for_model +from pulpcore.app.util import get_viewset_for_model, resolve_prn from pulpcore.tasking.tasks import dispatch # These should be used to prevent duplication and keep things consistent @@ -158,35 +158,43 @@ def get_resource_model(uri): @staticmethod def get_resource(uri, model=None): """ - Resolve a resource URI to an instance of the resource. + Resolve a resource URI/PRN to an instance of the resource. - Provides a means to resolve an href passed in a POST body to an + Provides a means to resolve an href/prn passed in a POST body to an instance of the resource. Args: - uri (str): A resource URI. + uri (str): A resource URI/PRN. model (django.models.Model): A model class. If not provided, the method automatically - determines the used model from the resource URI. + determines the used model from the resource URI/PRN. Returns: django.models.Model: The resource fetched from the DB. Raises: - rest_framework.exceptions.ValidationError: on invalid URI or resource not found. + rest_framework.exceptions.ValidationError: on invalid URI/PRN or resource not found. """ - try: - match = resolve(urlparse(uri).path) - except Resolver404: - raise DRFValidationError(detail=_("URI not valid: {u}").format(u=uri)) - - if model is None: - model = match.func.cls.queryset.model + found_kwargs = {} + if uri.startswith("prn:"): + m, pk = resolve_prn(uri) + if model is None: + model = m + found_kwargs["pk"] = pk + else: + try: + match = resolve(urlparse(uri).path) + except Resolver404: + raise DRFValidationError(detail=_("URI not valid: {u}").format(u=uri)) + else: + if model is None: + model = match.func.cls.queryset.model + found_kwargs = match.kwargs - if "pk" in match.kwargs: - kwargs = {"pk": match.kwargs["pk"]} + if "pk" in found_kwargs: + kwargs = {"pk": found_kwargs["pk"]} else: kwargs = {} - for key, value in match.kwargs.items(): + for key, value in found_kwargs.items(): if key.endswith("_pk"): kwargs["{}__pk".format(key[:-3])] = value elif key == "pulp_domain": diff --git a/pulpcore/app/viewsets/custom_filters.py b/pulpcore/app/viewsets/custom_filters.py index 42ee56f474..f1835c4fb2 100644 --- a/pulpcore/app/viewsets/custom_filters.py +++ b/pulpcore/app/viewsets/custom_filters.py @@ -9,7 +9,6 @@ from gettext import gettext as _ from django.conf import settings -from django.urls import resolve from django.db.models import ObjectDoesNotExist from django_filters import BaseInFilter, CharFilter, Filter from rest_framework import serializers @@ -110,28 +109,27 @@ def filter(self, qs, value): if value is None: return qs - match = resolve(value) - resource = NamedModelViewSet.get_resource(value, match.func.cls.queryset.model) + resource = NamedModelViewSet.get_resource(value) return qs.filter(created_resources__object_id=resource.pk) -class RepoVersionHrefFilter(Filter): +class RepoVersionHrefPrnFilter(Filter): """ Filter Content by a Repository Version. """ def __init__(self, *args, **kwargs): - kwargs.setdefault("help_text", _("Repository Version referenced by HREF")) + kwargs.setdefault("help_text", _("Repository Version referenced by HREF/PRN")) super().__init__(*args, **kwargs) @staticmethod def get_repository_version(value): """ - Get the repository version from the HREF value provided by the user. + Get the repository version from the HREF/PRN value provided by the user. Args: - value (string): The RepositoryVersion href to filter by + value (string): The RepositoryVersion href/prn to filter by """ if not value: raise serializers.ValidationError( @@ -144,21 +142,21 @@ def filter(self, qs, value): """ Args: qs (django.db.models.query.QuerySet): The Content Queryset - value (string): The RepositoryVersion href to filter by + value (string): The RepositoryVersion href/prn to filter by """ raise NotImplementedError() -class RepositoryVersionFilter(RepoVersionHrefFilter): +class RepositoryVersionFilter(RepoVersionHrefPrnFilter): """ - Filter by RepositoryVersion href. + Filter by RepositoryVersion href/prn. """ def filter(self, qs, value): """ Args: qs (django.db.models.query.QuerySet): The Queryset to filter - value (string): The RepositoryVersion href to filter by + value (string): The RepositoryVersion href/prn to filter by Returns: Queryset filtered by given repository version on field_name @@ -172,7 +170,7 @@ def filter(self, qs, value): return qs.filter(**{key: repo_version.pk}) -class ArtifactRepositoryVersionFilter(RepoVersionHrefFilter): +class ArtifactRepositoryVersionFilter(RepoVersionHrefPrnFilter): """ Filter used to get the artifacts in a repository version. """ @@ -181,7 +179,7 @@ def filter(self, qs, value): """ Args: qs (django.db.models.query.QuerySet): The Artifact Queryset - value (string): The RepositoryVersion href to filter by + value (string): The RepositoryVersion href/prn to filter by Returns: Queryset of the artifacts contained within the specified repository version @@ -196,7 +194,7 @@ def filter(self, qs, value): return qs.filter(pk__in=artifact_pks) -class ContentRepositoryVersionFilter(RepoVersionHrefFilter): +class ContentRepositoryVersionFilter(RepoVersionHrefPrnFilter): """ Filter used to get the content of this type found in a repository version. """ @@ -205,7 +203,7 @@ def filter(self, qs, value): """ Args: qs (django.db.models.query.QuerySet): The Content Queryset - value (string): The RepositoryVersion href to filter by + value (string): The RepositoryVersion href/prn to filter by Returns: Queryset of the content contained within the specified repository version @@ -218,7 +216,7 @@ def filter(self, qs, value): return qs.filter(pk__in=repo_version.content) -class ContentAddedRepositoryVersionFilter(RepoVersionHrefFilter): +class ContentAddedRepositoryVersionFilter(RepoVersionHrefPrnFilter): """ Filter used to get the content of this type found in a repository version. """ @@ -227,7 +225,7 @@ def filter(self, qs, value): """ Args: qs (django.db.models.query.QuerySet): The Content Queryset - value (string): The RepositoryVersion href to filter by + value (string): The RepositoryVersion href/prn to filter by Returns: Queryset of the content added by the specified repository version @@ -240,7 +238,7 @@ def filter(self, qs, value): return qs.filter(pk__in=repo_version.added()) -class ContentRemovedRepositoryVersionFilter(RepoVersionHrefFilter): +class ContentRemovedRepositoryVersionFilter(RepoVersionHrefPrnFilter): """ Filter used to get the content of this type found in a repository version. """ @@ -249,7 +247,7 @@ def filter(self, qs, value): """ Args: qs (django.db.models.query.QuerySet): The Content Queryset - value (string): The RepositoryVersion href to filter by + value (string): The RepositoryVersion href/prn to filter by Returns: Queryset of the content removed by the specified repository version @@ -329,14 +327,14 @@ 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")) + kwargs.setdefault("help_text", _("Content Unit referenced by HREF/PRN")) 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 + value (string or list[string]): of content href/prn(s) to filter Returns: Queryset of the Publication/RepositoryVersion containing the specified content diff --git a/pulpcore/app/viewsets/publication.py b/pulpcore/app/viewsets/publication.py index 31bd544745..ee17776d54 100644 --- a/pulpcore/app/viewsets/publication.py +++ b/pulpcore/app/viewsets/publication.py @@ -61,7 +61,7 @@ def filter(self, qs, value): class PublicationFilter(BaseFilterSet): - repository = RepositoryThroughVersionFilter(help_text=_("Repository referenced by HREF")) + repository = RepositoryThroughVersionFilter(help_text=_("Repository referenced by HREF/PRN")) repository_version = RepositoryVersionFilter() content = WithContentFilter() content__in = WithContentInFilter() diff --git a/pulpcore/app/viewsets/repository.py b/pulpcore/app/viewsets/repository.py index c794cb51e9..9b6524807e 100644 --- a/pulpcore/app/viewsets/repository.py +++ b/pulpcore/app/viewsets/repository.py @@ -42,6 +42,7 @@ from pulpcore.app.viewsets.custom_filters import LabelFilter, WithContentFilter, WithContentInFilter from pulpcore.tasking.tasks import dispatch from pulpcore.filters import HyperlinkRelatedFilter +from pulpcore.app.util import resolve_prn class RepositoryContentFilter(Filter): @@ -50,7 +51,7 @@ class RepositoryContentFilter(Filter): """ def __init__(self, *args, **kwargs): - kwargs.setdefault("help_text", _("Content Unit referenced by HREF")) + kwargs.setdefault("help_text", _("Content Unit referenced by HREF/PRN")) self.latest = kwargs.pop("latest", False) super().__init__(*args, **kwargs) @@ -58,7 +59,7 @@ def filter(self, qs, value): """ Args: qs (django.db.models.query.QuerySet): The Repository Queryset - value (string): of content href to filter + value (string): of content href/prn to filter Returns: Queryset of the Repository containing the specified content @@ -192,18 +193,29 @@ class RepositoryVersionFilter(BaseFilterSet): def filter_pulp_href(self, queryset, name, value): """Special handling for RepositoryVersion HREF filtering.""" repo_versions = defaultdict(list) + repo_version_pks = [] for uri in value: - try: - href_match = resolve(urlparse(uri).path).kwargs - except Resolver404: - href_match = {} - if "repository_pk" not in href_match or "number" not in href_match: - raise serializers.ValidationError( - _("Invalid RepositoryVersion HREF: {}".format(uri)) - ) - repo_versions[href_match["repository_pk"]].append(int(href_match["number"])) + if uri.startswith("prn:"): + model, pk = resolve_prn(uri) + if model != RepositoryVersion: + raise serializers.ValidationError( + _("Invalid RepositoryVersion PRN: {}").format(uri) + ) + repo_version_pks.append(pk) + else: + try: + href_match = resolve(urlparse(uri).path).kwargs + except Resolver404: + href_match = {} + if "repository_pk" not in href_match or "number" not in href_match: + raise serializers.ValidationError( + _("Invalid RepositoryVersion HREF: {}".format(uri)) + ) + repo_versions[href_match["repository_pk"]].append(int(href_match["number"])) filter_Q = Q() + if repo_version_pks: + filter_Q |= Q(pk__in=repo_version_pks) for repo_pk, numbers in repo_versions.items(): filter_Q |= Q(repository__pk=repo_pk, number__in=numbers) return queryset.filter(filter_Q) diff --git a/pulpcore/filters.py b/pulpcore/filters.py index c281d1ff71..be327759a9 100644 --- a/pulpcore/filters.py +++ b/pulpcore/filters.py @@ -5,6 +5,7 @@ from functools import lru_cache, partial from urllib.parse import urlparse from uuid import UUID +from collections import namedtuple from django import forms from django.db import models from django.forms.utils import ErrorList @@ -18,9 +19,10 @@ from drf_spectacular.plumbing import build_basic_type from drf_spectacular.contrib.django_filters import DjangoFilterExtension -from pulpcore.app.util import extract_pk +from pulpcore.app.util import extract_pk, resolve_prn, get_domain_pk EMPTY_VALUES = (*EMPTY_VALUES, "null") +UPMatch = namedtuple("UPMatch", ("model", "pk")) class StableOrderingFilter(filters.OrderingFilter): @@ -41,10 +43,11 @@ def filter(self, qs, value): class HyperlinkRelatedFilter(filters.Filter): """ - Enables a user to filter by a foreign key using that FK's href. + Enables a user to filter by a foreign key using that FK's href/prn. Foreign key filter can be specified to an object type by specifying the base URI of that type. e.g. Filter by file remotes: ?remote=/pulp/api/v3/remotes/file/file/ + Filtering by object type is not possible using PRNs. Can also filter for foreign key to be unset by setting ``allow_null`` to True. Query parameter will then accept "null" or "" for filtering. @@ -57,19 +60,24 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) def _resolve_uri(self, uri): - try: - return resolve(urlparse(uri).path) - except Resolver404: - raise serializers.ValidationError( - detail=_("URI couldn't be resolved: {uri}".format(uri=uri)) - ) + if uri.startswith("prn:"): + match = UPMatch(*resolve_prn(uri)) + else: + try: + match = resolve(urlparse(uri).path) + except Resolver404: + raise serializers.ValidationError( + detail=_("URI couldn't be resolved: {uri}".format(uri=uri)) + ) + match = UPMatch(match.func.cls.queryset.model, match.kwargs.get("pk")) + return match def _check_subclass(self, qs, uri, match): fields_model = getattr(qs.model, self.field_name).get_queryset().model - lookups_model = match.func.cls.queryset.model + lookups_model = match.model if not issubclass(lookups_model, fields_model): raise serializers.ValidationError( - detail=_("URI is not a valid href for {field_name} model: {uri}").format( + detail=_("URI/PRN is not a valid href for {field_name} model: {uri}").format( field_name=self.field_name, uri=uri ) ) @@ -83,14 +91,14 @@ def _check_valid_uuid(self, uuid): raise serializers.ValidationError(detail=_("UUID invalid: {uuid}").format(uuid=uuid)) def _validations(self, *args, **kwargs): - self._check_valid_uuid(kwargs["match"].kwargs.get("pk")) + self._check_valid_uuid(kwargs["match"].pk) self._check_subclass(*args, **kwargs) def filter(self, qs, value): """ Args: qs (django.db.models.query.QuerySet): The Queryset to filter - value (string or list of strings): href containing pk for the foreign key instance + value (string or list of strings): href/prn containing pk for the foreign key instance Returns: django.db.models.query.QuerySet: Queryset filtered by the foreign key pk @@ -111,14 +119,15 @@ def filter(self, qs, value): if self.lookup_expr == "in": matches = {uri: self._resolve_uri(uri) for uri in value} [self._validations(qs, uri=uri, match=matches[uri]) for uri in matches] - value = [pk if (pk := matches[match].kwargs.get("pk")) else match for match in matches] + value = [pk if (pk := matches[match].pk) else match for match in matches] else: match = self._resolve_uri(value) self._validations(qs, uri=value, match=match) - if pk := match.kwargs.get("pk"): + if pk := match.pk: value = pk else: - return qs.filter(**{f"{self.field_name}__in": match.func.cls.queryset}) + field_in_qs = match.model.objects.filter(pulp_domain=get_domain_pk()) + return qs.filter(**{f"{self.field_name}__in": field_in_qs}) return super().filter(qs, value) @@ -131,14 +140,20 @@ def filter(self, qs, value): class HREFInFilter(BaseInFilter, filters.CharFilter): + ONLY_PRN = False + def filter(self, qs, value): if value is None: return qs - pks = [extract_pk(href) for href in value] + pks = [extract_pk(href, self.ONLY_PRN) for href in value] return qs.filter(pk__in=pks) +class PRNInFilter(HREFInFilter): + ONLY_PRN = True + + class PulpTypeFilter(filters.ChoiceFilter): """Special pulp_type filter only added to generic list endpoints.""" @@ -283,6 +298,7 @@ class BaseFilterSet(filterset.FilterSet): help_text = {} pulp_id__in = IdInFilter(field_name="pk") pulp_href__in = HREFInFilter(field_name="pk") + prn__in = PRNInFilter(field_name="pk") q = ExpressionFilter() FILTER_DEFAULTS = { diff --git a/pulpcore/plugin/serializers/__init__.py b/pulpcore/plugin/serializers/__init__.py index 980606a3f3..cc0de73709 100644 --- a/pulpcore/plugin/serializers/__init__.py +++ b/pulpcore/plugin/serializers/__init__.py @@ -22,6 +22,7 @@ NestedRelatedField, NoArtifactContentSerializer, ProgressReportSerializer, + PRNField, PublicationSerializer, RelatedField, RemoteSerializer, diff --git a/pulpcore/plugin/util.py b/pulpcore/plugin/util.py index da755b7a4a..8d35dceb6d 100644 --- a/pulpcore/plugin/util.py +++ b/pulpcore/plugin/util.py @@ -16,6 +16,7 @@ batch_qs, extract_pk, get_artifact_url, + get_prn, get_url, gpg_verify, raise_for_unknown_content_units, @@ -27,4 +28,5 @@ get_current_authenticated_user, reverse, set_current_user, + resolve_prn, ) diff --git a/pulpcore/pytest_plugin.py b/pulpcore/pytest_plugin.py index 2401d8be98..16b07822ee 100644 --- a/pulpcore/pytest_plugin.py +++ b/pulpcore/pytest_plugin.py @@ -841,6 +841,21 @@ def pulp_content_url(pulp_settings, pulp_domain_enabled): return url +@pytest.fixture(scope="session") +def pulp_openapi_schema_url(pulp_api_v3_url): + return f"{pulp_api_v3_url}docs/api.json" + + +@pytest.fixture(scope="session") +def pulp_openapi_schema(pulp_openapi_schema_url): + return requests.get(pulp_openapi_schema_url).json() + + +@pytest.fixture(scope="session") +def pulp_openapi_schema_pk_path_set(pulp_openapi_schema_url): + return requests.get(f"{pulp_openapi_schema_url}?pk_path=1").json() + + # Pulp status information fixtures diff --git a/pulpcore/tests/functional/api/test_openapi_schema.py b/pulpcore/tests/functional/api/test_openapi_schema.py index 3f1025bbd8..812eac1ad7 100644 --- a/pulpcore/tests/functional/api/test_openapi_schema.py +++ b/pulpcore/tests/functional/api/test_openapi_schema.py @@ -4,7 +4,6 @@ import json import os -import requests import pytest import jsonschema @@ -35,21 +34,6 @@ def openapi3_schema_with_modified_safe_chars(openapi3_schema_spec): return openapi3_schema_spec_copy -@pytest.fixture(scope="session") -def pulp_openapi_schema_url(pulp_api_v3_url): - return f"{pulp_api_v3_url}docs/api.json" - - -@pytest.fixture(scope="session") -def pulp_openapi_schema(pulp_openapi_schema_url): - return requests.get(pulp_openapi_schema_url).json() - - -@pytest.fixture(scope="session") -def pulp_openapi_schema_pk_path_set(pulp_openapi_schema_url): - return requests.get(f"{pulp_openapi_schema_url}?pk_path=1").json() - - @pytest.mark.parallel @pytest.mark.from_pulpcore_for_all_plugins def test_valid_with_pk_path_set(pulp_openapi_schema_pk_path_set, openapi3_schema_spec): diff --git a/pulpcore/tests/functional/api/using_plugin/test_prn.py b/pulpcore/tests/functional/api/using_plugin/test_prn.py new file mode 100644 index 0000000000..c2fcb46f97 --- /dev/null +++ b/pulpcore/tests/functional/api/using_plugin/test_prn.py @@ -0,0 +1,119 @@ +import pytest + +from random import sample, choice + + +@pytest.mark.parallel +def test_prn_schema(pulp_openapi_schema): + """Test that PRN is a part of every serializer with a pulp_href.""" + failed = [] + for name, schema in pulp_openapi_schema["components"]["schemas"].items(): + if name.endswith("Response"): + if "pulp_href" in schema["properties"]: + if "prn" in schema["properties"]: + prn_schema = schema["properties"]["prn"] + if prn_schema["type"] == "string" and prn_schema["readOnly"]: + continue + failed.append(name) + + assert len(failed) == 0 + + +@pytest.mark.parallel +def test_read_prn(file_repo): + """Test that PRN is of the form 'prn:app_label.model_label:model_pk'.""" + href = file_repo.pulp_href + prn = f"prn:file.filerepository:{href.split('/')[-2]}" + assert file_repo.prn == prn + + +@pytest.mark.parallel +def test_prn_in_filter(file_repository_factory, file_bindings): + """Test the prn__in filter.""" + repos = [file_repository_factory() for _ in range(10)] + prns = [r.prn for r in repos] + selections = sample(prns, k=4) + response = file_bindings.RepositoriesFileApi.list(prn__in=selections) + assert response.count == 4 + assert {r.prn for r in response.results}.issubset(set(prns)) + + +@pytest.mark.parallel +def test_create_and_filter_with_prn( + file_bindings, + file_repo, + file_remote_factory, + basic_manifest_path, + file_publication_factory, + file_distribution_factory, + monitor_task, +): + """Test that we can use PRNs to refer to any object""" + # Creation tests + remote = file_remote_factory(basic_manifest_path) + task = file_bindings.RepositoriesFileApi.sync(file_repo.pulp_href, {"remote": remote.prn}).task + task = monitor_task(task) + assert len(task.created_resources) == 1 + + repo_ver = file_bindings.RepositoriesFileVersionsApi.read(task.created_resources[0]) + pub1 = file_publication_factory(repository=file_repo.prn) + pub2 = file_publication_factory(repository_version=repo_ver.prn) + assert pub1.repository_version == pub2.repository_version + + dis1 = file_distribution_factory(repository=file_repo.prn) + dis2 = file_distribution_factory(publication=pub1.prn) + dis3 = file_distribution_factory(publication=pub2.prn) + assert dis1.repository == file_repo.pulp_href + assert dis2.publication == pub1.pulp_href + assert dis3.publication == pub2.pulp_href + + # Filtering tests + response = file_bindings.ContentFilesApi.list(repository_version=repo_ver.pulp_href) + response2 = file_bindings.ContentFilesApi.list(repository_version=repo_ver.prn) + assert response.count == 3 + assert response == response2 + + response = file_bindings.ContentFilesApi.list(repository_version_added=repo_ver.prn) + assert response == response2 + response = file_bindings.ContentFilesApi.list(repository_version_removed=repo_ver.prn) + assert response.count == 0 + + content_hrefs = [c.pulp_href for c in response2.results] + content_prns = [c.prn for c in response2.results] + cindex = choice(range(len(content_prns))) + response = file_bindings.DistributionsFileApi.list(with_content=content_hrefs[cindex]) + response2 = file_bindings.DistributionsFileApi.list(with_content=content_prns[cindex]) + assert response.count == 3 + assert response == response2 + + response = file_bindings.DistributionsFileApi.list(repository=file_repo.pulp_href) + response2 = file_bindings.DistributionsFileApi.list(repository=file_repo.prn) + assert response.count == 1 + assert response == response2 + + response = file_bindings.PublicationsFileApi.list(content__in=content_hrefs) + response2 = file_bindings.PublicationsFileApi.list(content__in=content_prns) + assert response.count == 2 + assert response == response2 + + response = file_bindings.PublicationsFileApi.list(repository=file_repo.pulp_href) + response2 = file_bindings.PublicationsFileApi.list(repository=file_repo.prn) + assert response.count == 2 + assert response == response2 + + cindex = choice(range(len(content_prns))) + response = file_bindings.RepositoriesFileApi.list(with_content=content_hrefs[cindex]) + response2 = file_bindings.RepositoriesFileApi.list(with_content=content_prns[cindex]) + assert response.count == 1 + assert response == response2 + response = file_bindings.RepositoriesFileApi.list(remote=remote.prn) + assert response.count == 0 + + response = file_bindings.RepositoriesFileVersionsApi.list( + file_repo.pulp_href, content__in=content_hrefs + ) + response2 = file_bindings.RepositoriesFileVersionsApi.list( + file_repo.pulp_href, content__in=content_prns + ) + assert response.count == 1 + assert response == response2 diff --git a/pulpcore/tests/functional/api/using_plugin/test_tasks.py b/pulpcore/tests/functional/api/using_plugin/test_tasks.py index c650e6267c..494ac8cd75 100644 --- a/pulpcore/tests/functional/api/using_plugin/test_tasks.py +++ b/pulpcore/tests/functional/api/using_plugin/test_tasks.py @@ -99,7 +99,7 @@ def test_filter_tasks_by_reserved_resources(setup_filter_fixture, pulpcore_bindi with pytest.raises(ApiException) as ctx: pulpcore_bindings.TasksApi.list(created_resources=created_resources) - assert ctx.value.status == 404 + assert ctx.value.status == 400 def get_prn(uri): diff --git a/pulpcore/tests/unit/serializers/test_domain.py b/pulpcore/tests/unit/serializers/test_domain.py index cd07f3917f..40f76529f1 100644 --- a/pulpcore/tests/unit/serializers/test_domain.py +++ b/pulpcore/tests/unit/serializers/test_domain.py @@ -158,6 +158,7 @@ def test_hidden_settings(storage_class, serializer_class, all_settings): ) serializer = DomainSerializer(domain) serializer.fields.pop("pulp_href") + serializer.fields.pop("prn") assert "hidden_fields" in serializer.data["storage_settings"] hidden_fields = serializer.data["storage_settings"]["hidden_fields"] diff --git a/pulpcore/tests/unit/serializers/test_repository.py b/pulpcore/tests/unit/serializers/test_repository.py index 8199e24c82..ce5184f0e3 100644 --- a/pulpcore/tests/unit/serializers/test_repository.py +++ b/pulpcore/tests/unit/serializers/test_repository.py @@ -52,6 +52,7 @@ def _gen_remote_serializer(): serializer = RemoteSerializer(remote) # The pulp_href field needs too much things we are not interested in here. serializer.fields.pop("pulp_href") + serializer.fields.pop("prn") return serializer