From c4af3c0d41a0e4d179a6118218ce3a3ec9dde2ea Mon Sep 17 00:00:00 2001 From: jarekwg Date: Wed, 30 Dec 2015 11:59:37 +1100 Subject: [PATCH 1/3] Let Resource rollback if import throws exception --- import_export/resources.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/import_export/resources.py b/import_export/resources.py index f65fcbba3..f9c2621a3 100644 --- a/import_export/resources.py +++ b/import_export/resources.py @@ -1,27 +1,26 @@ from __future__ import unicode_literals import functools -from copy import deepcopy import sys +import tablib import traceback +from copy import deepcopy -import tablib from diff_match_patch import diff_match_patch from django import VERSION -from django.utils.safestring import mark_safe -from django.utils import six +from django.conf import settings +from django.db import transaction from django.db.models.fields import FieldDoesNotExist from django.db.models.query import QuerySet from django.db.transaction import TransactionManagementError -from django.conf import settings +from django.utils import six +from django.utils.safestring import mark_safe -from .results import Error, Result, RowResult +from . import widgets from .fields import Field -from import_export import widgets -from .instance_loaders import ( - ModelInstanceLoader, -) +from .instance_loaders import ModelInstanceLoader +from .results import Error, Result, RowResult try: from django.db.transaction import atomic, savepoint, savepoint_rollback, savepoint_commit # noqa @@ -47,7 +46,7 @@ from django.utils.datastructures import SortedDict as OrderedDict # Set default logging handler to avoid "No handler found" warnings. -import logging +import logging # isort:skip try: # Python 2.7+ from logging import NullHandler except ImportError: @@ -375,7 +374,8 @@ def import_data(self, dataset, dry_run=False, raise_errors=False, if self.skip_row(instance, original): row_result.import_type = RowResult.IMPORT_TYPE_SKIP else: - self.save_instance(instance, real_dry_run) + with transaction.atomic(): + self.save_instance(instance, real_dry_run) self.save_m2m(instance, row, real_dry_run) # Add object info to RowResult for LogEntry row_result.object_repr = force_text(instance) From f6696dbd9e76205ff0decbace827258ae96c7699 Mon Sep 17 00:00:00 2001 From: Jarek Glowacki Date: Tue, 12 Jan 2016 00:46:52 +1100 Subject: [PATCH 2/3] Add failing test Also includes: -Some pep8 cleanup -Testing against python3.5 -No longer testing against django1.5 --- .travis.yml | 1 + tests/core/tests/resources_tests.py | 84 ++++++++++++++++------------- tox.ini | 47 ++++++++++------ 3 files changed, 78 insertions(+), 54 deletions(-) diff --git a/.travis.yml b/.travis.yml index bb54f66a4..e31190c4e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ python: - "2.7" - "3.3" - "3.4" + - "3.5" env: - DJANGO="Django>=1.5,<1.6" - DJANGO="Django>=1.6,<1.7" diff --git a/tests/core/tests/resources_tests.py b/tests/core/tests/resources_tests.py index 64dae1b8f..628620b40 100644 --- a/tests/core/tests/resources_tests.py +++ b/tests/core/tests/resources_tests.py @@ -1,32 +1,21 @@ from __future__ import unicode_literals -from decimal import Decimal -from datetime import date +import tablib from copy import deepcopy -from unittest import ( - skip, -) +from datetime import date +from decimal import Decimal +from unittest import skip -from django.db import models +from django.contrib.auth.models import User from django.db.models import Count from django.db.models.fields import FieldDoesNotExist -from django.test import ( - skipUnlessDBFeature, - TestCase, - TransactionTestCase, - ) +from django.test import TestCase, TransactionTestCase, skipUnlessDBFeature from django.utils.html import strip_tags -from django.contrib.auth.models import User - -import tablib -from import_export import resources -from import_export import fields -from import_export import widgets -from import_export import results +from import_export import fields, resources, results, widgets from import_export.instance_loaders import ModelInstanceLoader -from core.models import Book, Author, Category, Entry, Profile, WithDefault, WithDynamicDefault +from ..models import Author, Book, Category, Entry, Profile, WithDefault, WithDynamicDefault try: from django.utils.encoding import force_text @@ -122,6 +111,12 @@ class Meta: exclude = ('imported', ) +class ProfileResource(resources.ModelResource): + class Meta: + model = Profile + exclude = ('user', ) + + class WithDefaultResource(resources.ModelResource): class Meta: model = WithDefault @@ -199,8 +194,7 @@ def test_get_instance_with_missing_field_data(self): dataset = tablib.Dataset(headers=['name', 'author_email', 'price']) dataset.append(['Some book', 'test@example.com', "10.25"]) with self.assertRaises(KeyError) as cm: - instance = self.resource.get_instance(instance_loader, - dataset.dict[0]) + self.resource.get_instance(instance_loader, dataset.dict[0]) self.assertEqual(u"Column 'id' not found in dataset. Available columns " "are: %s" % [u'name', u'author_email', u'price'], cm.exception.args[0]) @@ -359,7 +353,7 @@ class Meta: fields = ('published',) widgets = { 'published': {'format': '%d.%m.%Y'}, - } + } resource = B() self.book.published = date(2012, 8, 13) @@ -503,7 +497,7 @@ def before_import(self, dataset, dry_run, **kwargs): def test_link_to_nonexistent_field(self): with self.assertRaises(FieldDoesNotExist) as cm: - class BrokenBook(resources.ModelResource): + class BrokenBook1(resources.ModelResource): class Meta: model = Book fields = ('nonexistent__invalid',) @@ -511,7 +505,7 @@ class Meta: cm.exception.args[0]) with self.assertRaises(FieldDoesNotExist) as cm: - class BrokenBook(resources.ModelResource): + class BrokenBook2(resources.ModelResource): class Meta: model = Book fields = ('author__nonexistent',) @@ -520,7 +514,7 @@ class Meta: def test_link_to_nonrelation_field(self): with self.assertRaises(KeyError) as cm: - class BrokenBook(resources.ModelResource): + class BrokenBook1(resources.ModelResource): class Meta: model = Book fields = ('published__invalid',) @@ -528,7 +522,7 @@ class Meta: cm.exception.args[0]) with self.assertRaises(KeyError) as cm: - class BrokenBook(resources.ModelResource): + class BrokenBook2(resources.ModelResource): class Meta: model = Book fields = ('author__name__invalid',) @@ -547,7 +541,7 @@ def field_from_django_field(self, field_name, django_field, if field_name == 'published': return {'sound': 'quack'} - resource = B() + B() self.assertEqual({'sound': 'quack'}, B.fields['published']) def test_readonly_annotated_field_import_and_export(self): @@ -612,7 +606,7 @@ class Meta: self.assertTrue(callable(DynamicDefaultResource.fields['name'].default)) resource = DynamicDefaultResource() - dataset = tablib.Dataset(headers=['id', 'name',]) + dataset = tablib.Dataset(headers=['id', 'name', ]) dataset.append([1, None]) dataset.append([2, None]) resource.import_data(dataset, raise_errors=False) @@ -621,36 +615,52 @@ class Meta: class ModelResourceTransactionTest(TransactionTestCase): - - def setUp(self): - self.resource = BookResource() - @skipUnlessDBFeature('supports_transactions') def test_m2m_import_with_transactions(self): + resource = BookResource() cat1 = Category.objects.create(name='Cat 1') headers = ['id', 'name', 'categories'] row = [None, 'FooBook', "%s" % cat1.pk] dataset = tablib.Dataset(row, headers=headers) - result = self.resource.import_data(dataset, dry_run=True, - use_transactions=True) + result = resource.import_data( + dataset, dry_run=True, use_transactions=True + ) row_diff = result.rows[0].diff - fields = self.resource.get_fields() + fields = resource.get_fields() - id_field = self.resource.fields['id'] + id_field = resource.fields['id'] id_diff = row_diff[fields.index(id_field)] # id diff should exists because in rollbacked transaction # FooBook has been saved self.assertTrue(id_diff) - category_field = self.resource.fields['categories'] + category_field = resource.fields['categories'] categories_diff = row_diff[fields.index(category_field)] self.assertEqual(strip_tags(categories_diff), force_text(cat1.pk)) # check that it is really rollbacked self.assertFalse(Book.objects.filter(name='FooBook')) + @skipUnlessDBFeature('supports_transactions') + def test_m2m_import_with_transactions_error(self): + resource = ProfileResource() + headers = ['id', 'user'] + # 'user' is a required field, the database will raise an error. + row = [None, None] + dataset = tablib.Dataset(row, headers=headers) + + result = resource.import_data( + dataset, dry_run=True, use_transactions=True + ) + + # Ensure the error raised by the database has been saved. + self.assertTrue(result.has_errors()) + + # Ensure the rollback has worked properly. + self.assertEqual(Profile.objects.count(), 0) + class ModelResourceFactoryTest(TestCase): diff --git a/tox.ini b/tox.ini index 697301a3f..c7c6ca318 100644 --- a/tox.ini +++ b/tox.ini @@ -1,78 +1,84 @@ [tox] -envlist = py27-1.5, py27-1.6, py33-1.6, py27-1.7, py33-1.7, py34-1.7, py27-1.8, py33-1.8, py34-1.8, py27-1.9, py34-1.9, py27-dev, py34-dev +envlist = + py{27,33}-1.6, + py{27,33,34}-1.7, + py{27,33,34}-1.8, + py{27,34,35}-1.9, + py{27,34,35}-dev [testenv] commands=python {toxinidir}/tests/manage.py test core deps = openpyxl -[testenv:py27-1.5] -basepython = python2.7 -deps = - django==1.5.3 - {[testenv]deps} - [testenv:py27-1.6] basepython = python2.7 -deps = +deps = django==1.6 {[testenv]deps} [testenv:py33-1.6] basepython = python3.3 -deps = +deps = django==1.6.1 -egit+https://github.com/kennethreitz/tablib.git#egg=tablib {[testenv]deps} [testenv:py27-1.7] basepython = python2.7 -deps = +deps = django==1.7.0 {[testenv]deps} [testenv:py33-1.7] basepython = python3.3 -deps = +deps = django==1.7.0 -egit+https://github.com/kennethreitz/tablib.git#egg=tablib {[testenv]deps} [testenv:py34-1.7] basepython = python3.4 -deps = +deps = django==1.7.0 -egit+https://github.com/kennethreitz/tablib.git#egg=tablib {[testenv]deps} [testenv:py27-1.8] basepython = python2.7 -deps = +deps = django==1.8 {[testenv]deps} [testenv:py33-1.8] basepython = python3.3 -deps = +deps = django==1.8 -egit+https://github.com/kennethreitz/tablib.git#egg=tablib {[testenv]deps} [testenv:py34-1.8] basepython = python3.4 -deps = +deps = django==1.8 -egit+https://github.com/kennethreitz/tablib.git#egg=tablib {[testenv]deps} [testenv:py27-1.9] basepython = python2.7 -deps = +deps = django==1.9 {[testenv]deps} [testenv:py34-1.9] basepython = python3.4 -deps = +deps = + django==1.9 + -egit+https://github.com/kennethreitz/tablib.git#egg=tablib + {[testenv]deps} + +[testenv:py35-1.9] +basepython = python3.5 +deps = django==1.9 -egit+https://github.com/kennethreitz/tablib.git#egg=tablib {[testenv]deps} @@ -89,3 +95,10 @@ deps = https://github.com/django/django/archive/master.tar.gz -egit+https://github.com/kennethreitz/tablib.git#egg=tablib {[testenv]deps} + +[testenv:py35-dev] +basepython = python3.4 +deps = + https://github.com/django/django/archive/master.tar.gz + -egit+https://github.com/kennethreitz/tablib.git#egg=tablib + {[testenv]deps} From 43d3dae43331a8961e3497168063d779c5dbefe4 Mon Sep 17 00:00:00 2001 From: Jarek Glowacki Date: Tue, 12 Jan 2016 01:03:46 +1100 Subject: [PATCH 3/3] Typo --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index c7c6ca318..34b1f0b12 100644 --- a/tox.ini +++ b/tox.ini @@ -97,7 +97,7 @@ deps = {[testenv]deps} [testenv:py35-dev] -basepython = python3.4 +basepython = python3.5 deps = https://github.com/django/django/archive/master.tar.gz -egit+https://github.com/kennethreitz/tablib.git#egg=tablib