Merge pull request #33 from saritasa-nest/feature/fix-pickling-issue

Fix pickling issue

docker-compose.yaml

@@ -1,5 +1,3 @@
-version: '3.7'
-
 services:
   redis:
     image: redis:5

import_export_extensions/migrations/0007_alter_exportjob_result_alter_importjob_result.py

@@ -0,0 +1,35 @@
+# Generated by Django 4.2.7 on 2024-05-16 09:21
+
+from django.db import migrations
+import import_export.results
+import picklefield.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("import_export_extensions", "0006_importjob_input_errors_file"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="exportjob",
+            name="result",
+            field=picklefield.fields.PickledObjectField(
+                default=import_export.results.Result,
+                editable=False,
+                help_text="Internal job result object that contain info about job statistics. Pickled Python object",
+                verbose_name="Job result",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="importjob",
+            name="result",
+            field=picklefield.fields.PickledObjectField(
+                default=import_export.results.Result,
+                editable=False,
+                help_text="Internal job result object that contain info about job statistics. Pickled Python object",
+                verbose_name="Job result",
+            ),
+        ),
+    ]

import_export_extensions/models/core.py

@@ -5,6 +5,7 @@
 from django.utils import module_loading
 from django.utils.translation import gettext_lazy as _
 
+from import_export.results import Result
 from picklefield.fields import PickledObjectField
 
 
@@ -97,7 +98,7 @@ class BaseJob(TimeStampedModel):
         help_text=_("User which started job"),
     )
     result = PickledObjectField(
-        default=str,
+        default=Result,
         verbose_name=_("Job result"),
         help_text=_(
             "Internal job result object that contain "

import_export_extensions/results.py

@@ -1,3 +1,6 @@
+import collections
+import typing
+
 from django.core.exceptions import ValidationError
 
 from import_export import results
@@ -23,10 +26,57 @@ def __reduce__(self):
 
 class RowResult(results.RowResult):
     """Custom row result class with ability to store skipped errors in row."""
-    def __init__(self):
-        self.non_field_skipped_errors: list[Error] = []
+
+    def __init__(self) -> None:
+        """Copy of base init except creating validation error field."""
+        self.non_field_skipped_errors: list[results.Error] = []
         self.field_skipped_errors: dict[str, list[ValidationError]] = dict()
-        super().__init__()
+        self.errors: list[Error] = []
+        self.diff: list[str] | None = None
+        self.import_type = None
+        self.row_values: dict[str, typing.Any] = {}
+        self.object_id = None
+        self.object_repr = None
+        self.instance = None
+        self.original = None
+        self.new_record = None
+        # variable to store modified value of ValidationError
+        self._validation_error: ValidationError | None = None
+
+    @property
+    def validation_error(self) -> ValidationError | None:
+        """Return modified validation error."""
+        return self._validation_error
+
+    @validation_error.setter
+    def validation_error(self, value: ValidationError) -> None:
+        """Modify passed error to reduce count of nested exception classes.
+
+        Result class is saved in `ImportJob` model as pickled field
+        and python's `pickle` can't handle
+        `ValidationError` with high level of nested errors,
+        therefore we reduce nesting by using string value of nested errors.
+
+        If `ValidationError` has no `message_dict` attr,
+        then it means that there're no nested exceptions
+        and we can safely save it.
+
+        Otherwise, we will go through all nested validation errors
+        and build new `ValidationError` with only one level of
+        nested `ValidationError` instances.
+
+        """
+        if not hasattr(value, "message_dict"):
+            self._validation_error = value
+            return
+        result = collections.defaultdict(list)
+        for field, error_messages in value.message_dict.items():
+            validation_errors = [
+                ValidationError(message=message, code="invalid")
+                for message in error_messages
+            ]
+            result[field].extend(validation_errors)
+        self._validation_error = ValidationError(result)
 
     @property
     def has_skipped_errors(self) -> bool:

tests/fake_app/migrations/0001_initial.py

@@ -23,7 +23,7 @@ class Migration(migrations.Migration):
                         verbose_name="ID",
                     ),
                 ),
-                ("name", models.CharField(max_length=100)),
+                ("name", models.CharField(max_length=100, unique=True)),
             ],
             options={
                 'verbose_name': 'Artist',

tests/fake_app/models.py

@@ -17,7 +17,7 @@ def __str__(self) -> str:
 
 class Artist(models.Model):
     """Model representing artist."""
-    name = models.CharField(max_length=100)
+    name = models.CharField(max_length=100, unique=True)
     bands = models.ManyToManyField("Band", through="Membership")
 
     instrument = models.ForeignKey(

tests/fake_app/resources.py

@@ -12,6 +12,7 @@ class SimpleArtistResource(CeleryModelResource):
 
     class Meta:
         model = Artist
+        clean_model_instances = True
         fields = [
             "id",
             "name",
@@ -34,5 +35,5 @@ class ArtistResourceWithM2M(CeleryModelResource):
 
     class Meta:
         model = Artist
-        fields = ["id", "name", "bands", "instrument"]
         clean_model_instances = True
+        fields = ["id", "name", "bands", "instrument"]

tests/test_models/test_import/test_import_data.py

@@ -2,7 +2,7 @@
 
 from import_export_extensions.models import ImportJob
 
-from ...fake_app.factories import ArtistImportJobFactory
+from ...fake_app.factories import ArtistFactory, ArtistImportJobFactory
 from ...fake_app.models import Artist
 
 
@@ -143,3 +143,16 @@ def test_force_import_create_correct_rows(
     import_job.refresh_from_db()
     assert import_job.import_status == import_job.ImportStatus.IMPORTED
     assert Artist.objects.filter(name=new_artist.name).exists()
+
+
+def test_import_data_with_validation_error(existing_artist: Artist):
+    """Test import handles `ValidationError` instances correctly.
+
+    If validation error occur, then job should end with `INPUT_ERROR` status.
+
+    """
+    wrong_artist = ArtistFactory.build(name=existing_artist.name)
+    job = ArtistImportJobFactory(artists=[wrong_artist])
+    job.parse_data()
+    job.refresh_from_db()
+    assert job.import_status == ImportJob.ImportStatus.INPUT_ERROR