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

@@ -0,0 +1,2 @@
+Added ``retrieve`` logic to ``MultipleArtifactContentSerializer``. Allow to use uploads with
+``NoArtifactContentSerializer``.

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

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):

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),