From b5ed40148b484f914dc4cf2744ee3c849a85bdc2 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg <mdellweg@redhat.com> Date: Thu, 22 Jun 2023 14:36:04 +0200 Subject: [PATCH] Bridge gaps between content serializer features Refactor the content serializers to reuse more code and share more features. * Allow to implement 'retrieve' on all content serializers for multiple upload. * Allow adding all content to a repository on creation. * Validate that the content type matches the provided repository. fixes #3951 --- CHANGES/plugin_api/3951.feature | 2 + pulpcore/app/serializers/content.py | 151 ++++++++++++++----------- pulpcore/plugin/serializers/content.py | 31 +---- pulpcore/plugin/viewsets/content.py | 13 +-- 4 files changed, 97 insertions(+), 100 deletions(-) create mode 100644 CHANGES/plugin_api/3951.feature diff --git a/CHANGES/plugin_api/3951.feature b/CHANGES/plugin_api/3951.feature new file mode 100644 index 0000000000..94273092ec --- /dev/null +++ b/CHANGES/plugin_api/3951.feature @@ -0,0 +1,2 @@ +Added ``retrieve`` logic to ``MultipleArtifactContentSerializer``. Allow to use uploads with +``NoArtifactContentSerializer``. diff --git a/pulpcore/app/serializers/content.py b/pulpcore/app/serializers/content.py index 37172cbd80..fcb62c80a8 100644 --- a/pulpcore/app/serializers/content.py +++ b/pulpcore/app/serializers/content.py @@ -5,91 +5,127 @@ from rest_framework.validators import UniqueValidator from pulpcore.app import models -from pulpcore.app.serializers import base, fields +from pulpcore.app.serializers import base, fields, DetailRelatedField from pulpcore.app.util import get_domain -class BaseContentSerializer(base.ModelSerializer): +class NoArtifactContentSerializer(base.ModelSerializer): pulp_href = base.DetailIdentityField(view_name_pattern=r"contents(-.*/.*)-detail") - - class Meta: - model = models.Content - fields = base.ModelSerializer.Meta.fields - - -class NoArtifactContentSerializer(BaseContentSerializer): - class Meta: - model = models.Content - fields = BaseContentSerializer.Meta.fields - - -class SingleArtifactContentSerializer(BaseContentSerializer): - artifact = fields.SingleContentArtifactField( - help_text=_("Artifact file representing the physical content"), - ) - - relative_path = serializers.CharField( - help_text=_("Path where the artifact is located relative to distributions base_path"), - validators=[fields.relative_path_validator], + repository = DetailRelatedField( + help_text=_("A URI of a repository the new content unit should be associated with."), + required=False, write_only=True, + view_name_pattern=r"repositories(-.*/.*)-detail", + queryset=models.Repository.objects.all(), ) - def __init__(self, *args, **kwargs): + def get_artifacts(self, validated_data): """ - Initializer for SingleArtifactContentSerializer + Extract artifacts from validated_data. + + This function is supposed to extract the information about content artifacts from + validated_data and return a dictionary with artifacts and relative paths as keys. """ - super().__init__(*args, **kwargs) + return {} - # If the content model has its own database field 'relative_path', - # we should not mark the field write_only - if hasattr(self.Meta.model, "relative_path") and "relative_path" in self.fields: - self.fields["relative_path"].write_only = False + def retrieve(self, validated_data): + """ + Retrieve existing content unit if it exists, else return None. + + This method is plugin-specific and implementing it for a specific content type + allows for uploading already existing content units of that type. + """ + return None + + def validate(self, data): + """Validate that we have an Artifact or can create one.""" + + data = super().validate(data) + if repository := data.get("repository"): + if ( + self.Meta.model + not in repository.get_model_for_pulp_type(repository.pulp_type).CONTENT_TYPES + ): + raise serializers.ValidationError("Content is not supported by this repository.") + return data def create(self, validated_data): """ - Create the content and associate it with its Artifact, or retrieve the existing content. + Create the content and associate it with its Artifacts, or retrieve the existing content. Args: validated_data (dict): Data to save to the database """ - content = self.retrieve(validated_data) + repository = validated_data.pop("repository", None) + artifacts = self.get_artifacts(validated_data) + content = self.retrieve(validated_data) if content is not None: content.touch() else: - artifact = validated_data.pop("artifact") - if "relative_path" not in self.fields or self.fields["relative_path"].write_only: - relative_path = validated_data.pop("relative_path") - else: - relative_path = validated_data.get("relative_path") try: with transaction.atomic(): content = self.Meta.model.objects.create(**validated_data) - models.ContentArtifact.objects.create( - artifact=artifact, content=content, relative_path=relative_path - ) + for relative_path, artifact in artifacts.items(): + models.ContentArtifact.objects.create( + artifact=artifact, content=content, relative_path=relative_path + ) except IntegrityError: content = self.retrieve(validated_data) if content is None: raise + if repository: + repository.cast() + content_to_add = self.Meta.model.objects.filter(pk=content.pk) + + # create new repo version with uploaded package + with repository.new_version() as new_version: + new_version.add_content(content_to_add) + return content - def retrieve(self, validated_data): - """ - Retrieve existing content unit if it exists, else return None. + class Meta: + model = models.Content + fields = base.ModelSerializer.Meta.fields + ("repository",) - This method is plugin-specific and implementing it for a specific content type - allows for uploading already existing content units of that type. + +class SingleArtifactContentSerializer(NoArtifactContentSerializer): + artifact = fields.SingleContentArtifactField( + help_text=_("Artifact file representing the physical content"), + ) + + relative_path = serializers.CharField( + help_text=_("Path where the artifact is located relative to distributions base_path"), + validators=[fields.relative_path_validator], + write_only=True, + ) + + def __init__(self, *args, **kwargs): """ - return None + Initializer for SingleArtifactContentSerializer + """ + super().__init__(*args, **kwargs) + + # If the content model has its own database field 'relative_path', + # we should not mark the field write_only + if hasattr(self.Meta.model, "relative_path") and "relative_path" in self.fields: + self.fields["relative_path"].write_only = False + + def get_artifacts(self, validated_data): + artifact = validated_data.pop("artifact") + if "relative_path" not in self.fields or self.fields["relative_path"].write_only: + relative_path = validated_data.pop("relative_path") + else: + relative_path = validated_data.get("relative_path") + return {relative_path: artifact} class Meta: model = models.Content - fields = BaseContentSerializer.Meta.fields + ("artifact", "relative_path") + fields = NoArtifactContentSerializer.Meta.fields + ("artifact", "relative_path") -class MultipleArtifactContentSerializer(BaseContentSerializer): +class MultipleArtifactContentSerializer(NoArtifactContentSerializer): artifacts = fields.ContentArtifactsField( help_text=_( "A dict mapping relative paths inside the Content to the corresponding" @@ -98,25 +134,12 @@ class MultipleArtifactContentSerializer(BaseContentSerializer): ), ) - @transaction.atomic - def create(self, validated_data): - """ - Create the content and associate it with all its Artifacts. - - Args: - validated_data (dict): Data to save to the database - """ - artifacts = validated_data.pop("artifacts") - content = self.Meta.model.objects.create(**validated_data) - for relative_path, artifact in artifacts.items(): - models.ContentArtifact.objects.create( - artifact=artifact, content=content, relative_path=relative_path - ) - return content + def get_artifacts(self, validated_data): + return validated_data.pop("artifacts") class Meta: model = models.Content - fields = BaseContentSerializer.Meta.fields + ("artifacts",) + fields = NoArtifactContentSerializer.Meta.fields + ("artifacts",) class ContentChecksumSerializer(serializers.Serializer): @@ -290,7 +313,7 @@ class SigningServiceSerializer(base.ModelSerializer): class Meta: model = models.SigningService - fields = BaseContentSerializer.Meta.fields + ( + fields = base.ModelSerializer.Meta.fields + ( "name", "public_key", "pubkey_fingerprint", diff --git a/pulpcore/plugin/serializers/content.py b/pulpcore/plugin/serializers/content.py index a0afd6dc41..3ec096c1cc 100644 --- a/pulpcore/plugin/serializers/content.py +++ b/pulpcore/plugin/serializers/content.py @@ -10,9 +10,8 @@ ValidationError, ) from pulpcore.app.files import PulpTemporaryUploadedFile -from pulpcore.app.models import Artifact, Repository, Upload, UploadChunk +from pulpcore.app.models import Artifact, Upload, UploadChunk from pulpcore.app.serializers import ( - DetailRelatedField, RelatedField, ArtifactSerializer, NoArtifactContentSerializer, @@ -31,35 +30,9 @@ class UploadSerializerFieldsMixin(Serializer): required=False, write_only=True, ) - repository = DetailRelatedField( - help_text=_("A URI of a repository the new content unit should be associated with."), - required=False, - write_only=True, - view_name_pattern=r"repositories(-.*/.*)-detail", - queryset=Repository.objects.all(), - ) - - def create(self, validated_data): - """ - Save a GenericContent unit. - - This must be used inside a task that locks on the Artifact and if given, the repository. - """ - - repository = validated_data.pop("repository", None) - content = super().create(validated_data) - - if repository: - repository.cast() - content_to_add = self.Meta.model.objects.filter(pk=content.pk) - - # create new repo version with uploaded package - with repository.new_version() as new_version: - new_version.add_content(content_to_add) - return content class Meta: - fields = ("file", "repository") + fields = ("file",) class NoArtifactContentUploadSerializer(UploadSerializerFieldsMixin, NoArtifactContentSerializer): diff --git a/pulpcore/plugin/viewsets/content.py b/pulpcore/plugin/viewsets/content.py index 0f61bb0655..3772a64bca 100644 --- a/pulpcore/plugin/viewsets/content.py +++ b/pulpcore/plugin/viewsets/content.py @@ -43,20 +43,19 @@ def create(self, request): serializer.is_valid(raise_exception=True) task_payload = {k: v for k, v in request.data.items()} - file_content = task_payload.pop("file", None) + file_content = task_payload.pop("file", None) temp_file = PulpTemporaryFile.init_and_validate(file_content) temp_file.save() - resources = [] - repository = serializer.validated_data.get("repository") - if repository: - resources.append(repository) + exclusive_resources = [ + item for item in (serializer.validated_data.get(key) for key in ("repository",)) if item + ] app_label = self.queryset.model._meta.app_label task = dispatch( tasks.base.general_create_from_temp_file, - exclusive_resources=resources, + exclusive_resources=exclusive_resources, args=(app_label, serializer.__class__.__name__, str(temp_file.pk)), kwargs={"data": task_payload, "context": self.get_deferred_context(request)}, ) @@ -87,8 +86,8 @@ def create(self, request): app_label = self.queryset.model._meta.app_label task = dispatch( tasks.base.general_create, - args=(app_label, serializer.__class__.__name__), exclusive_resources=exclusive_resources, + args=(app_label, serializer.__class__.__name__), kwargs={ "data": task_payload, "context": self.get_deferred_context(request),