Merge a4a2801 into 0ba3582

docs/changelog.rst

@@ -10,33 +10,29 @@ Breaking changes
 This release makes some minor changes to the public API.  If you have overridden any methods from the `resources` or `widgets` modules, you may need to update your implementation to accommodate these changes.
 
 - Check value of `ManyToManyField` in skip_row() (#1271)
-
     - This fixes an issue where ManyToMany fields are not checked correctly in `skip_row()`.  This means that `skip_row()` now takes `row` as a mandatory arg.  If you have overridden `skip_row()` in your own implementation, you will need to add `row` as an arg.
 
 - Bug fix: validation errors were being ignored when `skip_unchanged` is set (#1378)
-
     - If you have overridden `skip_row()` you can choose whether or not to skip rows if validation errors are present.  The default behavior is to not to skip rows if there are validation errors during import.
 
 - Use 'create' flag instead of instance.pk (#1362)
-
     - `import_export.resources.save_instance()` now takes an additional mandatory argument: `is_create`.  If you have overridden `save_instance()` in your own code, you will need to add this new argument.
 
 - `widgets`: Unused `*args` params have been removed from method definitions. (#1413)
+    - If you have overridden `clean()` then you should update your method definition to reflect this change.
+    - `widgets.ForeignKeyWidget` / `widgets.ManyToManyWidget`: The unused `*args` param has been removed from `__init__()`.  If you have overridden `ForeignKeyWidget` or `ManyToManyWidget` you may need to update your implementation to reflect this change.
 
-  - If you have overridden `clean()` then you will need to update your method definition to reflect this change.
-
-  - `widgets.ForeignKeyWidget` / `widgets.ManyToManyWidget`: The unused `*args` param has been removed from `__init__()`.  If you have overridden `ForeignKeyWidget` or `ManyToManyWidget` you may need to update your implementation to reflect this change.
+- Admin interface: Modified handling of import errors (#1306)
+    - Exceptions raised during the import process are now presented as form errors, instead of being wrapped in a \<H1\> tag in the response.  If you have any custom logic which uses the error written directly into the response, then this may need to be changed.
 
 Deprecations
 ############
 
 This release adds some deprecations which will be removed in the 3.1 release.
 
 - Add support for multiple resources in ModelAdmin. (#1223)
-
-  - The `*Mixin.resource_class` accepting single resource has been deprecated and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added.
-
-  - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods.
+    - The `*Mixin.resource_class` accepting single resource has been deprecated and the new `*Mixin.resource_classes` accepting subscriptable type (list, tuple, ...) has been added.
+    - Same applies to all of the `get_resource_class`, `get_import_resource_class` and `get_export_resource_class` methods.
 
 - Deprecated `exceptions.py` (#1372)
 
@@ -53,6 +49,11 @@ Development
 
 - Increased test coverage, refactored CI build to use tox (#1372)
 
+Documentation
+#############
+
+- Clarified issues around the usage of temporary storage (#1306)
+
 2.8.0 (2022-03-31)
 ------------------
 

docs/getting_started.rst

@@ -392,10 +392,14 @@ mixins (:class:`~import_export.admin.ImportMixin`,
 
     admin.site.register(Book, BookAdmin)
 
+.. _change-screen-figure:
+
 .. figure:: _static/images/django-import-export-change.png
 
    A screenshot of the change view with Import and Export buttons.
 
+.. _confirm-import-figure:
+
 .. figure:: _static/images/django-import-export-import.png
 
    A screenshot of the import view.
@@ -530,6 +534,45 @@ as well as importing customizations.
     :doc:`/api_admin`
         available mixins and options.
 
+Import confirmation
+~~~~~~~~~~~~~~~~~~~
+
+Importing in the Admin site is a two step process.
+
+#. Choose the file to import (:ref:`screenshot<change-screen-figure>`).
+#. Review changes and confirm import (:ref:`screenshot<confirm-import-figure>`).
+
+To support this, uploaded data is written to temporary storage after step 1, and read
+back for final import after step 2.
+
+There are three mechanisms for temporary storage.
+
+#. Temporary file storage on the host server (default).  This is suitable for development only.
+   Use of temporary filesystem storage is not recommended for production sites.
+
+#. The `Django cache <https://docs.djangoproject.com/en/dev/topics/cache/>`_.
+
+#. `Django storage <https://docs.djangoproject.com/en/dev/ref/files/storage/>`_.
+
+To modify which storage mechanism is used, please refer to the setting :ref:`IMPORT_EXPORT_TMP_STORAGE_CLASS`.
+
+Temporary resources are removed when data is successfully imported after the confirmation step.
+
+Your choice of temporary storage will be influenced by the following factors:
+
+* Sensitivity of the data being imported.
+* Volume and frequency of uploads.
+* File upload size.
+* Use of containers or load-balanced servers.
+
+.. warning::
+
+    If users do not complete the confirmation step of the workflow,
+    or if there are errors during import, then temporary resources may not be deleted.
+    This will need to be understood and managed in production settings.
+    For example, using a cache expiration policy or cron job to clear stale resources.
+
+
 Using multiple resources
 ------------------------
 

docs/installation.rst

@@ -65,6 +65,8 @@ of losing an audit trail.
 Can be overridden on a ``ModelAdmin`` class inheriting from ``ImportMixin`` by
 setting the ``skip_admin_log`` class attribute.
 
+.. _IMPORT_EXPORT_TMP_STORAGE_CLASS:
+
 ``IMPORT_EXPORT_TMP_STORAGE_CLASS``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

import_export/admin.py

@@ -10,7 +10,6 @@
 from django.template.response import TemplateResponse
 from django.urls import path, reverse
 from django.utils.decorators import method_decorator
-from django.utils.encoding import force_str
 from django.utils.module_loading import import_string
 from django.utils.translation import gettext_lazy as _
 from django.views.decorators.http import require_POST
@@ -19,7 +18,7 @@
 from .mixins import BaseExportMixin, BaseImportMixin
 from .results import RowResult
 from .signals import post_export, post_import
-from .tmp_storages import TempFolderStorage
+from .tmp_storages import MediaStorage, TempFolderStorage
 
 
 class ImportExportMixinBase:
@@ -41,7 +40,7 @@ class ImportMixin(BaseImportMixin, ImportExportMixinBase):
     #: template for import view
     import_template_name = 'admin/import_export/import.html'
     #: import data encoding
-    from_encoding = "utf-8"
+    from_encoding = "utf-8-sig"
     skip_admin_log = None
     # storage class for saving temporary files
     tmp_storage_class = None
@@ -103,13 +102,17 @@ def process_import(self, request, *args, **kwargs):
             import_formats = self.get_import_formats()
             input_format = import_formats[
                 int(confirm_form.cleaned_data['input_format'])
-            ]()
-            tmp_storage = self.get_tmp_storage_class()(name=confirm_form.cleaned_data['import_file_name'])
-            data = tmp_storage.read(input_format.get_read_mode())
-            if not input_format.is_binary() and self.from_encoding:
-                data = force_str(data, self.from_encoding)
-            dataset = input_format.create_dataset(data)
+            ](encoding=self.from_encoding)
+            encoding = None if input_format.is_binary() else self.from_encoding
+            tmp_storage_cls = self.get_tmp_storage_class()
+            tmp_storage = tmp_storage_cls(
+                name=confirm_form.cleaned_data['import_file_name'],
+                encoding=encoding,
+                read_mode=input_format.get_read_mode()
+            )
 
+            data = tmp_storage.read()
+            dataset = input_format.create_dataset(data)
             result = self.process_dataset(dataset, confirm_form, request, *args, **kwargs)
 
             tmp_storage.remove()
@@ -214,18 +217,26 @@ def get_import_data_kwargs(self, request, *args, **kwargs):
         return {}
 
     def write_to_tmp_storage(self, import_file, input_format):
-        tmp_storage = self.get_tmp_storage_class()()
+        encoding = None
+        if not input_format.is_binary():
+            encoding = self.from_encoding
+
+        tmp_storage_cls = self.get_tmp_storage_class()
+        tmp_storage = tmp_storage_cls(encoding=encoding, read_mode=input_format.get_read_mode())
         data = bytes()
         for chunk in import_file.chunks():
             data += chunk
 
-        tmp_storage.save(data, input_format.get_read_mode())
+        if tmp_storage_cls == MediaStorage and not input_format.is_binary():
+            data = data.decode(self.from_encoding)
+
+        tmp_storage.save(data)
         return tmp_storage
 
     def import_action(self, request, *args, **kwargs):
         """
         Perform a dry_run of the import to make sure the import will not
-        result in errors.  If there where no error, save the user
+        result in errors.  If there are no errors, save the user
         uploaded file to a local temp file that will be used by
         'process_import' for the actual import.
         """
@@ -243,52 +254,56 @@ def import_action(self, request, *args, **kwargs):
                          request.FILES or None,
                          **form_kwargs)
 
+        resources = list()
         if request.POST and form.is_valid():
             input_format = import_formats[
                 int(form.cleaned_data['input_format'])
             ]()
+            if not input_format.is_binary():
+                input_format.encoding = self.from_encoding
+
             import_file = form.cleaned_data['import_file']
             # first always write the uploaded file to disk as it may be a
             # memory file or else based on settings upload handlers
             tmp_storage = self.write_to_tmp_storage(import_file, input_format)
 
-            # then read the file, using the proper format-specific mode
-            # warning, big files may exceed memory
             try:
-                data = tmp_storage.read(input_format.get_read_mode())
-                if not input_format.is_binary() and self.from_encoding:
-                    data = force_str(data, self.from_encoding)
+                # then read the file, using the proper format-specific mode
+                # warning, big files may exceed memory
+                data = tmp_storage.read()
                 dataset = input_format.create_dataset(data)
-            except UnicodeDecodeError as e:
-                return HttpResponse(_(u"<h1>Imported file has a wrong encoding: %s</h1>" % e))
             except Exception as e:
-                return HttpResponse(_(u"<h1>%s encountered while trying to read file: %s</h1>" % (type(e).__name__, import_file.name)))
-
-            # prepare kwargs for import data, if needed
-            res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs)
-            resource = self.choose_import_resource_class(form)(**res_kwargs)
-            resources = [resource]
-
-            # prepare additional kwargs for import_data, if needed
-            imp_kwargs = self.get_import_data_kwargs(request, form=form, *args, **kwargs)
-            result = resource.import_data(dataset, dry_run=True,
-                                          raise_errors=False,
-                                          file_name=import_file.name,
-                                          user=request.user,
-                                          **imp_kwargs)
-
-            context['result'] = result
-
-            if not result.has_errors() and not result.has_validation_errors():
-                initial = {
-                    'import_file_name': tmp_storage.name,
-                    'original_file_name': import_file.name,
-                    'input_format': form.cleaned_data['input_format'],
-                    'resource': request.POST.get('resource', ''),
-                }
-                confirm_form = self.get_confirm_import_form()
-                initial = self.get_form_kwargs(form=form, **initial)
-                context['confirm_form'] = confirm_form(initial=initial)
+                form.add_error('import_file',
+                               _(f"'{type(e).__name__}' encountered while trying to read file. "
+                                 "Ensure you have chosen the correct format for the file. "
+                                 f"{str(e)}"))
+
+            if not form.errors:
+                # prepare kwargs for import data, if needed
+                res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs)
+                resource = self.choose_import_resource_class(form)(**res_kwargs)
+                resources = [resource]
+
+                # prepare additional kwargs for import_data, if needed
+                imp_kwargs = self.get_import_data_kwargs(request, form=form, *args, **kwargs)
+                result = resource.import_data(dataset, dry_run=True,
+                                              raise_errors=False,
+                                              file_name=import_file.name,
+                                              user=request.user,
+                                              **imp_kwargs)
+
+                context['result'] = result
+
+                if not result.has_errors() and not result.has_validation_errors():
+                    initial = {
+                        'import_file_name': tmp_storage.name,
+                        'original_file_name': import_file.name,
+                        'input_format': form.cleaned_data['input_format'],
+                        'resource': request.POST.get('resource', ''),
+                    }
+                    confirm_form = self.get_confirm_import_form()
+                    initial = self.get_form_kwargs(form=form, **initial)
+                    context['confirm_form'] = confirm_form(initial=initial)
         else:
             res_kwargs = self.get_import_resource_kwargs(request, form=form, *args, **kwargs)
             resource_classes = self.get_import_resource_classes()

import_export/formats/base_formats.py

@@ -56,6 +56,9 @@ class TablibFormat(Format):
     TABLIB_MODULE = None
     CONTENT_TYPE = 'application/octet-stream'
 
+    def __init__(self, encoding=None):
+        self.encoding = encoding
+
     def get_format(self):
         """
         Import and returns tablib module.
@@ -96,6 +99,12 @@ def can_export(self):
 
 
 class TextFormat(TablibFormat):
+
+    def create_dataset(self, in_stream, **kwargs):
+        if isinstance(in_stream, bytes) and self.encoding:
+            in_stream = in_stream.decode(self.encoding)
+        return super().create_dataset(in_stream, **kwargs)
+
     def get_read_mode(self):
         return 'r'
 
@@ -107,9 +116,6 @@ class CSV(TextFormat):
     TABLIB_MODULE = 'tablib.formats._csv'
     CONTENT_TYPE = 'text/csv'
 
-    def create_dataset(self, in_stream, **kwargs):
-        return super().create_dataset(in_stream, **kwargs)
-
 
 class JSON(TextFormat):
     TABLIB_MODULE = 'tablib.formats._json'