Skip to content

Commit

Permalink
Adds meta flag 'skip_diff' to enable skipping of diff operations (#1045)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewhegarty committed Apr 23, 2020
1 parent 7c58899 commit 24c99ef
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 8 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,4 @@ The following is a list of much appreciated contributors:
* ababic (Andy Babic)
* BramManuel (Bram Janssen)
* kjpc-tech (Kyle)
* Matthew Hegarty
39 changes: 31 additions & 8 deletions import_export/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,17 @@ class ResourceOptions:
The default value is False.
"""

skip_diff = False
"""
Controls whether or not an instance should be diffed following import.
By default, an instance is copied prior to insert, update or delete.
After each row is processed, the instance's copy is diffed against the original, and the value
stored in each ``RowResult``.
If diffing is not required, then disabling the diff operation by setting this value to ``True``
improves performance, because the copy and comparison operations are skipped for each row.
The default value is False.
"""


class DeclarativeMetaclass(type):

Expand Down Expand Up @@ -422,7 +433,12 @@ def skip_row(self, instance, original):
"""
Returns ``True`` if ``row`` importing should be skipped.
Default implementation returns ``False`` unless skip_unchanged == True.
Default implementation returns ``False`` unless skip_unchanged == True,
or skip_diff == True.
If skip_diff is True, then no comparisons can be made because ``original``
will be None.
Override this method to handle skipping rows meeting certain
conditions.
Expand All @@ -434,7 +450,7 @@ def skip_row(self, instance, original):
return super(YourResource, self).skip_row(instance, original)
"""
if not self._meta.skip_unchanged:
if not self._meta.skip_unchanged or self._meta.skip_diff:
return False
for field in self.get_import_fields():
try:
Expand Down Expand Up @@ -498,7 +514,9 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals
:param dry_run: If ``dry_run`` is set, or error occurs, transaction
will be rolled back.
"""
skip_diff = self._meta.skip_diff
row_result = self.get_row_result_class()()
original = None
try:
self.before_import_row(row, **kwargs)
instance, new = self.get_or_init_instance(instance_loader, row)
Expand All @@ -508,16 +526,19 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals
else:
row_result.import_type = RowResult.IMPORT_TYPE_UPDATE
row_result.new_record = new
original = deepcopy(instance)
diff = self.get_diff_class()(self, original, new)
if not skip_diff:
original = deepcopy(instance)
diff = self.get_diff_class()(self, original, new)
if self.for_delete(row, instance):
if new:
row_result.import_type = RowResult.IMPORT_TYPE_SKIP
diff.compare_with(self, None, dry_run)
if not skip_diff:
diff.compare_with(self, None, dry_run)
else:
row_result.import_type = RowResult.IMPORT_TYPE_DELETE
self.delete_instance(instance, using_transactions, dry_run)
diff.compare_with(self, None, dry_run)
if not skip_diff:
diff.compare_with(self, None, dry_run)
else:
import_validation_errors = {}
try:
Expand All @@ -536,9 +557,11 @@ def import_row(self, row, instance_loader, using_transactions=True, dry_run=Fals
# Add object info to RowResult for LogEntry
row_result.object_id = instance.pk
row_result.object_repr = force_str(instance)
diff.compare_with(self, instance, dry_run)
if not skip_diff:
diff.compare_with(self, instance, dry_run)

row_result.diff = diff.as_html()
if not skip_diff:
row_result.diff = diff.as_html()
self.after_import_row(row, row_result, **kwargs)

except ValidationError as e:
Expand Down
79 changes: 79 additions & 0 deletions tests/core/tests/test_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -1205,3 +1205,82 @@ def check_value(self, result, export_headers, expected_value):
diff = result.rows[0].diff
self.assertEqual(diff[export_headers.index("categories")],
expected_value)


@mock.patch("import_export.resources.Diff", spec=True)
class SkipDiffTest(TestCase):
"""
Tests that the meta attribute 'skip_diff' means that no diff operations are called.
'copy.deepcopy' cannot be patched at class level because it causes interferes with
``resources.Resource.__init__()``.
"""
def setUp(self):
class _BookResource(resources.ModelResource):

class Meta:
model = Book
skip_diff = True

self.resource = _BookResource()
self.dataset = tablib.Dataset(headers=['id', 'name', 'birthday'])
self.dataset.append(['', 'A.A.Milne', '1882test-01-18'])

def test_skip_diff(self, mock_diff):
with mock.patch("copy.deepcopy") as mock_deep_copy:
self.resource.import_data(self.dataset)
mock_diff.return_value.compare_with.assert_not_called()
mock_diff.return_value.as_html.assert_not_called()
mock_deep_copy.assert_not_called()

def test_skip_diff_for_delete_new_resource(self, mock_diff):
class BookResource(resources.ModelResource):

class Meta:
model = Book
skip_diff = True

def for_delete(self, row, instance):
return True

resource = BookResource()
with mock.patch("copy.deepcopy") as mock_deep_copy:
resource.import_data(self.dataset)
mock_diff.return_value.compare_with.assert_not_called()
mock_diff.return_value.as_html.assert_not_called()
mock_deep_copy.assert_not_called()

def test_skip_diff_for_delete_existing_resource(self, mock_diff):
book = Book.objects.create()
class BookResource(resources.ModelResource):

class Meta:
model = Book
skip_diff = True

def get_or_init_instance(self, instance_loader, row):
return book, False

def for_delete(self, row, instance):
return True

resource = BookResource()

with mock.patch("copy.deepcopy") as mock_deep_copy:
resource.import_data(self.dataset, dry_run=True)
mock_diff.return_value.compare_with.assert_not_called()
mock_diff.return_value.as_html.assert_not_called()
mock_deep_copy.assert_not_called()

def test_skip_row_returns_false_when_skip_diff_is_true(self, mock_diff):
class BookResource(resources.ModelResource):

class Meta:
model = Book
skip_unchanged = True
skip_diff = True

resource = BookResource()

with mock.patch('import_export.resources.Resource.get_import_fields') as mock_get_import_fields:
resource.import_data(self.dataset, dry_run=True)
self.assertEqual(2, mock_get_import_fields.call_count)

0 comments on commit 24c99ef

Please sign in to comment.