Add upload to NoArtifactContentUploadSerializer

fixes #4348

CHANGES.rst

@@ -568,8 +568,7 @@ Plugin API
 Features
 ~~~~~~~~
 
-- Added ``retrieve`` logic to ``MultipleArtifactContentSerializer``. Allow to use uploads with
-  ``NoArtifactContentSerializer``.
+- Added ``retrieve`` logic to ``MultipleArtifactContentSerializer``.
   :github:`3951`
 
 

CHANGES/4348.feature

@@ -0,0 +1 @@
+Added upload field to no artifact content upload apis.

pulpcore/app/tasks/base.py

@@ -1,8 +1,7 @@
 from django.db import transaction
 
 from pulpcore.app.apps import get_plugin_config
-from pulpcore.app.models import CreatedResource, PulpTemporaryFile
-from pulpcore.app.files import PulpTemporaryUploadedFile
+from pulpcore.app.models import CreatedResource
 from pulpcore.plugin.models import MasterModel
 
 
@@ -13,13 +12,11 @@ def general_create_from_temp_file(app_label, serializer_name, temp_file_pk, *arg
     A task which executes this function takes the ownership of a temporary file and deletes it
     afterwards. This function calls the function general_create() to create a model instance.
     """
-    temp_file = PulpTemporaryFile.objects.get(pk=temp_file_pk)
-
     data = kwargs.pop("data", {})
-    data["file"] = PulpTemporaryUploadedFile.from_file(temp_file.file)
+    context = kwargs.pop("context", {})
+    context["pulp_temp_file_pk"] = temp_file_pk
 
-    general_create(app_label, serializer_name, data=data, *args, **kwargs)
-    temp_file.delete()
+    general_create(app_label, serializer_name, data=data, context=context, *args, **kwargs)
 
 
 def general_create(app_label, serializer_name, *args, **kwargs):

pulpcore/plugin/serializers/content.py

@@ -1,6 +1,5 @@
 from gettext import gettext as _
 
-from logging import getLogger
 from tempfile import NamedTemporaryFile
 
 from django.db import DatabaseError
@@ -10,29 +9,94 @@
     ValidationError,
 )
 from pulpcore.app.files import PulpTemporaryUploadedFile
-from pulpcore.app.models import Artifact, Upload, UploadChunk
+from pulpcore.app.models import Artifact, PulpTemporaryFile, Upload, UploadChunk
 from pulpcore.app.serializers import (
     RelatedField,
     ArtifactSerializer,
     NoArtifactContentSerializer,
     SingleArtifactContentSerializer,
 )
-
-
-log = getLogger(__name__)
+from pulpcore.app.util import get_domain_pk
 
 
 class UploadSerializerFieldsMixin(Serializer):
     """A mixin class that contains fields and methods common to content upload serializers."""
 
     file = FileField(
-        help_text=_("An uploaded file that may be turned into the artifact of the content unit."),
+        help_text=_("An uploaded file that may be turned into the content unit."),
         required=False,
         write_only=True,
     )
+    upload = RelatedField(
+        help_text=_("An uncommitted upload that may be turned into the content unit."),
+        required=False,
+        write_only=True,
+        view_name=r"uploads-detail",
+        queryset=Upload.objects.all(),
+    )
+
+    def validate(self, data):
+        """Validate that we have an Artifact/File or can create one."""
+
+        data = super().validate(data)
+
+        if "request" in self.context:
+            upload_fields = {
+                field for field in self.Meta.fields if field in {"file", "upload", "artifact"}
+            }
+            if len(upload_fields.intersection(data.keys())) != 1:
+                raise ValidationError(
+                    _("Exactly one of {} must be specified.").format(", ".join(upload_fields))
+                )
+        else:
+            if "file" in data:
+                raise RuntimeError(
+                    "The file field must be resolved into an artifact by the viewset before "
+                    "dispatching the create task."
+                )
+            data = self.deferred_validate(data)
+
+        return data
+
+    def deferred_validate(self, data):
+        """
+        Validate the content unit by deeply analyzing the specified Artifact.
+
+        This is only called when validating without a request context to prevent stalling
+        an ongoing http request.
+        It should be overwritten by plugins to extract metadata from the actual content in
+        much the same way as `validate`.
+        When overwriting, plugins must super-call this method to handle uploads before analyzing
+        the artifact.
+        """
+        if upload := data.pop("upload", None):
+            self.context["upload"] = upload
+            chunks = UploadChunk.objects.filter(upload=upload).order_by("offset")
+            with NamedTemporaryFile(mode="ab", dir=".", delete=False) as temp_file:
+                for chunk in chunks:
+                    temp_file.write(chunk.file.read())
+                    chunk.file.close()
+                temp_file.flush()
+            data["file"] = PulpTemporaryUploadedFile.from_file(open(temp_file.name, "rb"))
+        elif pulp_temp_file_pk := self.context.get("pulp_temp_file_pk"):
+            pulp_temp_file = PulpTemporaryFile.objects.get(pk=pulp_temp_file_pk)
+            data["file"] = PulpTemporaryUploadedFile.from_file(pulp_temp_file.file)
+        return data
+
+    def create(self, validated_data):
+        result = super().create(validated_data)
+        if upload := self.context.get("upload"):
+            upload.delete()
+        if pulp_temp_file_pk := self.context.get("pulp_temp_file_pk"):
+            pulp_temp_file = PulpTemporaryFile.objects.get(pk=pulp_temp_file_pk)
+            pulp_temp_file.delete()
+        return result
 
     class Meta:
-        fields = ("file",)
+        fields = (
+            "file",
+            "upload",
+        )
 
 
 class NoArtifactContentUploadSerializer(UploadSerializerFieldsMixin, NoArtifactContentSerializer):
@@ -61,42 +125,12 @@ class SingleArtifactContentUploadSerializer(
     touches the Artifact and locks the Upload and Repository when specified.
     """
 
-    upload = RelatedField(
-        help_text=_(
-            "An uncommitted upload that may be turned into the artifact of the content unit."
-        ),
-        required=False,
-        write_only=True,
-        view_name=r"uploads-detail",
-        queryset=Upload.objects.all(),
-    )
-
     def __init__(self, *args, **kwargs):
         """Initializer for SingleArtifactContentUploadSerializer."""
         super().__init__(*args, **kwargs)
-        if self.fields.get("artifact"):
+        if "artifact" in self.fields:
             self.fields["artifact"].required = False
 
-    def validate(self, data):
-        """Validate that we have an Artifact or can create one."""
-
-        data = super().validate(data)
-
-        if len({"file", "upload", "artifact"}.intersection(data.keys())) != 1:
-            raise ValidationError(
-                _("Exactly one of 'file', 'artifact' or 'upload' must be specified.")
-            )
-
-        if "request" not in self.context:
-            if "file" in data:
-                raise RuntimeError(
-                    "The file field must be resolved into an artifact by the viewset before "
-                    "dispatching the create task."
-                )
-            data = self.deferred_validate(data)
-
-        return data
-
     def deferred_validate(self, data):
         """
         Validate the content unit by deeply analyzing the specified Artifact.
@@ -108,40 +142,24 @@ def deferred_validate(self, data):
         When overwriting, plugins must super-call this method to handle uploads before analyzing
         the artifact.
         """
-        if "upload" in data:
-            upload = data.pop("upload")
-            self.context["upload"] = upload
-            chunks = UploadChunk.objects.filter(upload=upload).order_by("offset")
-            with NamedTemporaryFile(mode="ab", dir=".", delete=False) as temp_file:
-                for chunk in chunks:
-                    temp_file.write(chunk.file.read())
-                    chunk.file.close()
-                temp_file.flush()
-            with open(temp_file.name, "rb") as artifact_file:
-                file = PulpTemporaryUploadedFile.from_file(artifact_file)
-                # if artifact already exists, let's use it
-                try:
-                    artifact = Artifact.objects.get(
-                        sha256=file.hashers["sha256"].hexdigest(), pulp_domain=upload.pulp_domain
-                    )
-                    artifact.touch()
-                except (Artifact.DoesNotExist, DatabaseError):
-                    artifact_data = {"file": file}
-                    serializer = ArtifactSerializer(data=artifact_data)
-                    serializer.is_valid(raise_exception=True)
-                    artifact = serializer.save()
+        data = super().deferred_validate(data)
+        if "file" in data:
+            file = data.pop("file")
+            # if artifact already exists, let's use it
+            try:
+                artifact = Artifact.objects.get(
+                    sha256=file.hashers["sha256"].hexdigest(), pulp_domain=get_domain_pk()
+                )
+                artifact.touch()
+            except (Artifact.DoesNotExist, DatabaseError):
+                artifact_data = {"file": file}
+                serializer = ArtifactSerializer(data=artifact_data)
+                serializer.is_valid(raise_exception=True)
+                artifact = serializer.save()
             data["artifact"] = artifact
         return data
 
-    def create(self, validated_data):
-        result = super().create(validated_data)
-        if upload := self.context.get("upload"):
-            upload.delete()
-        return result
-
     class Meta(SingleArtifactContentSerializer.Meta):
         fields = (
-            SingleArtifactContentSerializer.Meta.fields
-            + UploadSerializerFieldsMixin.Meta.fields
-            + ("upload",)
+            SingleArtifactContentSerializer.Meta.fields + UploadSerializerFieldsMixin.Meta.fields
         )

pulpcore/plugin/viewsets/content.py

@@ -53,11 +53,13 @@ def create(self, request):
         ]
 
         app_label = self.queryset.model._meta.app_label
+        context = self.get_deferred_context(request)
+        context["pulp_temp_file_pk"] = str(temp_file.pk)
         task = dispatch(
-            tasks.base.general_create_from_temp_file,
+            tasks.base.general_create,
             exclusive_resources=exclusive_resources,
-            args=(app_label, serializer.__class__.__name__, str(temp_file.pk)),
-            kwargs={"data": task_payload, "context": self.get_deferred_context(request)},
+            args=(app_label, serializer.__class__.__name__),
+            kwargs={"data": task_payload, "context": context},
         )
         return OperationPostponedResponse(task, request)