Merge pull request #690 from bmihelac/refactor-transaction-handling

Refactor transaction handling
This commit is contained in:
Bojan Mihelac 2017-12-13 08:15:07 +01:00 committed by GitHub
commit 9a32c6e08b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 119 additions and 22 deletions

View File

@ -12,7 +12,7 @@ from django import VERSION
from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
from django.core.management.color import no_style
from django.db import connections, transaction, DEFAULT_DB_ALIAS
from django.db import connections, DEFAULT_DB_ALIAS
from django.db.models.fields import FieldDoesNotExist
from django.db.models.query import QuerySet
from django.db.transaction import TransactionManagementError
@ -23,6 +23,7 @@ from . import widgets
from .fields import Field
from .instance_loaders import ModelInstanceLoader
from .results import Error, Result, RowResult
from .utils import atomic_if_using_transaction
try:
from django.db.transaction import atomic, savepoint, savepoint_rollback, savepoint_commit # noqa
@ -459,8 +460,7 @@ class Resource(six.with_metaclass(DeclarativeMetaclass)):
if self.skip_row(instance, original):
row_result.import_type = RowResult.IMPORT_TYPE_SKIP
else:
with transaction.atomic():
self.save_instance(instance, using_transactions, dry_run)
self.save_instance(instance, using_transactions, dry_run)
self.save_m2m(instance, row, using_transactions, dry_run)
diff.compare_with(self, instance, dry_run)
row_result.diff = diff.as_html()
@ -511,10 +511,8 @@ class Resource(six.with_metaclass(DeclarativeMetaclass)):
using_transactions = (use_transactions or dry_run) and supports_transactions
if using_transactions:
with transaction.atomic():
return self.import_data_inner(dataset, dry_run, raise_errors, using_transactions, collect_failed_rows, **kwargs)
return self.import_data_inner(dataset, dry_run, raise_errors, using_transactions, collect_failed_rows, **kwargs)
with atomic_if_using_transaction(using_transactions):
return self.import_data_inner(dataset, dry_run, raise_errors, using_transactions, collect_failed_rows, **kwargs)
def import_data_inner(self, dataset, dry_run, raise_errors, using_transactions, collect_failed_rows, **kwargs):
result = self.get_result_class()()
@ -527,15 +525,12 @@ class Resource(six.with_metaclass(DeclarativeMetaclass)):
sp1 = savepoint()
try:
self.before_import(dataset, using_transactions, dry_run, **kwargs)
with atomic_if_using_transaction(using_transactions):
self.before_import(dataset, using_transactions, dry_run, **kwargs)
except Exception as e:
logging.exception(e)
tb_info = traceback.format_exc()
result.append_base_error(self.get_error_result_class()(e, tb_info))
if raise_errors:
if using_transactions:
savepoint_rollback(sp1)
raise
instance_loader = self._meta.instance_loader_class(self, dataset)
@ -546,30 +541,32 @@ class Resource(six.with_metaclass(DeclarativeMetaclass)):
result.add_dataset_headers(dataset.headers)
for row in dataset.dict:
row_result = self.import_row(row, instance_loader,
using_transactions=using_transactions, dry_run=dry_run,
**kwargs)
with atomic_if_using_transaction(using_transactions):
row_result = self.import_row(
row,
instance_loader,
using_transactions=using_transactions,
dry_run=dry_run,
**kwargs
)
result.increment_row_result_total(row_result)
if row_result.errors:
if collect_failed_rows:
result.append_failed_row(row, row_result.errors[0])
if raise_errors:
if using_transactions:
savepoint_rollback(sp1)
raise row_result.errors[-1].error
if (row_result.import_type != RowResult.IMPORT_TYPE_SKIP or
self._meta.report_skipped):
result.append_row_result(row_result)
try:
self.after_import(dataset, result, using_transactions, dry_run, **kwargs)
with atomic_if_using_transaction(using_transactions):
self.after_import(dataset, result, using_transactions, dry_run, **kwargs)
except Exception as e:
logging.exception(e)
tb_info = traceback.format_exc()
result.append_base_error(self.get_error_result_class()(e, tb_info))
if raise_errors:
if using_transactions:
savepoint_rollback(sp1)
raise
if using_transactions:

27
import_export/utils.py Normal file
View File

@ -0,0 +1,27 @@
from __future__ import unicode_literals
from django.db import transaction
class atomic_if_using_transaction(object):
"""Context manager wraps `atomic` if `using_transactions`.
Replaces code::
if using_transactions:
with transaction.atomic():
return somethng()
return something()
"""
def __init__(self, using_transactions):
self.using_transactions = using_transactions
if using_transactions:
self.context_manager = transaction.atomic()
def __enter__(self):
if self.using_transactions:
self.context_manager.__enter__()
def __exit__(self, *args):
if self.using_transactions:
self.context_manager.__exit__(*args)

View File

@ -0,0 +1,20 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.7 on 2017-11-30 01:47
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('core', '0005_addparentchild'),
]
operations = [
migrations.AlterField(
model_name='category',
name='name',
field=models.CharField(max_length=100, unique=True),
),
]

View File

@ -17,7 +17,10 @@ class Author(models.Model):
@python_2_unicode_compatible
class Category(models.Model):
name = models.CharField(max_length=100)
name = models.CharField(
max_length=100,
unique=True,
)
def __str__(self):
return self.name

View File

@ -9,7 +9,7 @@ from unittest import skip, skipUnless
from django import VERSION
from django.conf import settings
from django.contrib.auth.models import User
from django.db import IntegrityError
from django.db import IntegrityError, DatabaseError
from django.db.models import Count
from django.db.models.fields import FieldDoesNotExist
from django.test import TestCase, TransactionTestCase, skipUnlessDBFeature
@ -118,6 +118,12 @@ class BookResource(resources.ModelResource):
exclude = ('imported', )
class CategoryResource(resources.ModelResource):
class Meta:
model = Category
class ProfileResource(resources.ModelResource):
class Meta:
model = Profile
@ -761,6 +767,50 @@ class ModelResourceTransactionTest(TransactionTestCase):
# Ensure the rollback has worked properly.
self.assertEqual(Profile.objects.count(), 0)
@skipUnlessDBFeature('supports_transactions')
def test_integrity_error_rollback_on_savem2m(self):
# savepoint_rollback() after an IntegrityError gives
# TransactionManagementError (#399)
class CategoryResourceRaisesIntegrityError(CategoryResource):
def save_m2m(self, instance, *args, **kwargs):
# force raising IntegrityError
Category.objects.create(name=instance.name)
resource = CategoryResourceRaisesIntegrityError()
headers = ['id', 'name']
rows = [
[None, 'foo'],
]
dataset = tablib.Dataset(*rows, headers=headers)
result = resource.import_data(
dataset,
use_transactions=True,
)
self.assertTrue(result.has_errors())
@skipUnlessDBFeature('supports_transactions')
def test_multiple_database_errors(self):
class CategoryResourceDbErrorsResource(CategoryResource):
def before_import(self, *args, **kwargs):
raise DatabaseError()
def save_instance(self):
raise DatabaseError()
resource = CategoryResourceDbErrorsResource()
headers = ['id', 'name']
rows = [
[None, 'foo'],
]
dataset = tablib.Dataset(*rows, headers=headers)
result = resource.import_data(
dataset,
use_transactions=True,
)
self.assertTrue(result.has_errors())
class ModelResourceFactoryTest(TestCase):

Binary file not shown.