Merge pull request #33 from jackton1/feature/fix-bug-with-duplicate-i…

…nstance-with-unique-fields

Fix bug with cloning unique fields and added support for bulk_clone.

.gitignore

@@ -16,3 +16,4 @@ node_modules/
 coverage.*
 .envrc
 htmlcov/
+test-reports/

Makefile

@@ -85,4 +85,11 @@ clean-test-all: clean-build  ## Clean build and test assets.
 	@rm -rf .tox/
 	@rm -rf test-results
 	@rm -rf .pytest_cache/
-	@rm test.db
+	@rm -f test.db
+
+
+# -----------------------------------------------------------
+# --------- Run autopep8 ------------------------------------
+# -----------------------------------------------------------
+run-autopep8:  ## Run autopep8 with inplace for model_clone package. 
+	@autopep8 -ri model_clone

model_clone/mixins/clone.py

@@ -1,14 +1,20 @@
 import abc
+from itertools import repeat
 
+from django.core.checks import Error
 from django.core.exceptions import ValidationError
-from django.db import transaction, models
+from django.db import transaction, models, connections
 from django.db.models import SlugField
 from django.db.models.base import ModelBase
 from django.utils import six
 from django.utils.text import slugify
-from django.core.checks import Error
+from conditional import conditional
 
 from model_clone.apps import ModelCloneConfig
+from model_clone.utils import (
+    clean_value, transaction_autocommit,
+    get_unique_value, context_mutable_attribute
+)
 
 
 class CloneMetaClass(abc.ABCMeta, ModelBase):
@@ -85,6 +91,7 @@ class TestModel(CloneMixin, models.Model):
     # 1 for space, 4 for copy, 1 for space, 2 for count  == ' copy 33'
     UNIQUE_DUPLICATE_LENGTH = 8
     USE_UNIQUE_DUPLICATE_SUFFIX = True
+    MAX_UNIQUE_DUPLICATE_QUERY_ATTEMPTS = 100
 
     @classmethod
     def _create_copy_of_instance(cls, instance):
@@ -124,17 +131,16 @@ def _create_copy_of_instance(cls, instance):
             ]):
                 value = getattr(instance, f.attname, f.get_default())
                 if f.attname in unique_fields and isinstance(f, models.CharField):
-                    count = (
-                        instance.__class__._default_manager
-                        .filter(**{'{}__startswith'.format(f.attname): value})
-                        .count()
-                    )
+                    value = clean_value(value, cls.UNIQUE_DUPLICATE_SUFFIX)
                     if cls.USE_UNIQUE_DUPLICATE_SUFFIX:
-                        if len(value) + cls.UNIQUE_DUPLICATE_LENGTH > f.max_length:
-                            value = value[: f.max_length - cls.UNIQUE_DUPLICATE_LENGTH]
-                        if not str(value).isdigit():
-                            value += ' {} {}'.format(
-                                cls.UNIQUE_DUPLICATE_SUFFIX, count)
+                        value = get_unique_value(
+                            instance,
+                            f.attname,
+                            value,
+                            cls.UNIQUE_DUPLICATE_SUFFIX,
+                            f.max_length,
+                            cls.MAX_UNIQUE_DUPLICATE_QUERY_ATTEMPTS
+                        )
                     if isinstance(f, SlugField):
                         value = slugify(value)
                 defaults[f.attname] = value
@@ -349,3 +355,38 @@ def unpack_unique_together(opts, only_fields=()):
             else:
                 fields.extend(list([f for f in field if f in only_fields]))
         return fields
+
+    @classmethod
+    def bulk_clone_multi(cls, objs, attrs=None, batch_size=None):
+        # type: (List[models.Model], Optional[List[Dict]], Optional[int]) -> List[models.Model]
+        # TODO: Support bulk clones split by the batch_szie
+        pass
+
+    def bulk_clone(self, count, attrs=None, batch_size=None, auto_commit=False):
+        ops = connections[self.__class__._default_manager.db].ops
+        objs = range(count)
+        clones = []
+        batch_size = (batch_size or max(
+            ops.bulk_batch_size([], list(objs)), 1))
+
+        with conditional(
+            auto_commit,
+            transaction_autocommit(using=self.__class__._default_manager.db),
+        ):
+            # If count exceeds the MAX_UNIQUE_DUPLICATE_QUERY_ATTEMPTS
+            with conditional(
+                self.MAX_UNIQUE_DUPLICATE_QUERY_ATTEMPTS < count,
+                context_mutable_attribute(
+                    self,
+                    'MAX_UNIQUE_DUPLICATE_QUERY_ATTEMPTS',
+                    count,
+                ),
+            ):
+                if not self.MAX_UNIQUE_DUPLICATE_QUERY_ATTEMPTS >= count:
+                    raise AssertionError(
+                        'An Unknown error has occured: Expected ({}) >= ({})'
+                        .format(self.MAX_UNIQUE_DUPLICATE_QUERY_ATTEMPTS, count),
+                    )
+                clones = list(repeat(self.make_clone(attrs=attrs), batch_size))
+
+        return clones

model_clone/tests.py

@@ -1,6 +1,7 @@
 from django.contrib.auth import get_user_model
 from django.db import IntegrityError
-from django.test import TestCase
+from django.db.transaction import TransactionManagementError
+from django.test import TestCase, TransactionTestCase
 from mock import patch, PropertyMock
 
 from sample.models import Library, Book, Author
@@ -206,10 +207,117 @@ def test_cloning_unique_fields_max_length(self):
 
         author_clone = author.make_clone()
 
-        # clone slices 8 chars of but count uses only 1 digit.
-        self.assertEqual(len(author_clone.first_name), 199)
+        self.assertEqual(
+            len(author_clone.first_name),
+            Author._meta.get_field('first_name').max_length,
+        )
         self.assertNotEqual(author.pk, author_clone.pk)
         self.assertEqual(
             author_clone.first_name,
-            '{} {} {}'.format(first_name[:192], Author.UNIQUE_DUPLICATE_SUFFIX, 1),
+            '{} {} {}'.format(first_name[:193],
+                              Author.UNIQUE_DUPLICATE_SUFFIX, 1),
+        )
+
+    def test_cloning_instances_in_an_atomic_transaction_with_auto_commit_on_raises_errors(
+        self,
+    ):
+        first_name = (
+            'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, '
+            'sed diam nonumy eirmod tempor invidunt ut labore et dolore '
+            'magna aliquyam erat, sed diam voluptua. At vero eos et accusam '
+            'et justo duo dolores '
+        )
+        author = Author.objects.create(
+            first_name=first_name,
+            last_name='Jack',
+            age=26,
+            sex='F',
+            created_by=self.user
+        )
+
+        with self.assertRaises(TransactionManagementError):
+            author.bulk_clone(1000, auto_commit=True)
+
+    def test_cloning_instances_in_an_atomic_transaction_with_auto_commit_off_is_valid(
+        self,
+    ):
+        first_name = (
+            'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, '
+            'sed diam nonumy eirmod tempor invidunt ut labore et dolore '
+            'magna aliquyam erat, sed diam voluptua. At vero eos et accusam '
+            'et justo duo dolores '
+        )
+        author = Author.objects.create(
+            first_name=first_name,
+            last_name='Jack',
+            age=26,
+            sex='F',
+            created_by=self.user
         )
+
+        clones = author.bulk_clone(1000)
+
+        self.assertEqual(len(clones), 1000)
+
+        for clone in clones:
+            self.assertNotEqual(author.pk, clone.pk)
+            self.assertRegexpMatches(
+                clone.first_name,
+                r'{}\s[\d]'.format(Author.UNIQUE_DUPLICATE_SUFFIX),
+            )
+
+
+class CloneMixinTransactionTestCase(TransactionTestCase):
+    def test_cloning_multiple_instances_doesnt_exceed_the_max_length(self):
+        user = User.objects.create()
+        first_name = (
+            'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, '
+            'sed diam nonumy eirmod tempor invidunt ut labore et dolore '
+            'magna aliquyam erat, sed diam voluptua. At vero eos et accusam '
+            'et justo duo dolores '
+        )
+        author = Author.objects.create(
+            first_name=first_name,
+            last_name='Jack',
+            age=26,
+            sex='F',
+            created_by=user
+        )
+
+        clones = author.bulk_clone(1000)
+
+        self.assertEqual(len(clones), 1000)
+
+        for clone in clones:
+            self.assertNotEqual(author.pk, clone.pk)
+            self.assertRegexpMatches(
+                clone.first_name,
+                r'{}\s[\d]'.format(Author.UNIQUE_DUPLICATE_SUFFIX),
+            )
+
+    def test_cloning_multiple_instances_with_autocommit_is_valid(self):
+        user = User.objects.create()
+        first_name = (
+            'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, '
+            'sed diam nonumy eirmod tempor invidunt ut labore et dolore '
+            'magna aliquyam erat, sed diam voluptua. At vero eos et accusam '
+            'et justo duo dolores 2'
+        )
+        author = Author.objects.create(
+            first_name=first_name,
+            last_name='Jack',
+            age=26,
+            sex='F',
+            created_by=user
+        )
+
+        clones = author.bulk_clone(1000, auto_commit=True)
+
+        self.assertEqual(len(clones), 1000)
+
+        for clone in clones:
+            self.assertNotEqual(author.pk, clone.pk)
+            self.assertRegexpMatches(
+                clone.first_name,
+                r'{}\s[\d]'.format(Author.UNIQUE_DUPLICATE_SUFFIX),
+            )

model_clone/utils.py

@@ -1,15 +1,21 @@
+import contextlib
+import re
+
 from django.core.exceptions import ValidationError
-from django.db import models
+from django.db import models, transaction
+from django.db.transaction import TransactionManagementError
+from django.utils import six
 
 
-def create_copy_of_instance(instance, exclude=(), save_new=True, attrs=()):
+def create_copy_of_instance(instance, exclude=(), save_new=True, attrs=None):
     """
     Clone an instance of `django.db.models.Model`.
 
     Args:
         instance(django.db.models.Model): The model instance to clone.
         exclude(list|set): List or set of fields to exclude from unique validation.
-        **attrs: Kwargs of field and value to set on the duplicated instance.
+        save_new(bool): Save the model instance after duplication calling .save().
+        attrs(dict): Kwargs of field and value to set on the duplicated instance.
 
     Returns:
         (django.db.models.Model): The new duplicated instance.
@@ -23,6 +29,7 @@ def create_copy_of_instance(instance, exclude=(), save_new=True, attrs=()):
         1
         >>> instance.name
         "The Beautiful Life"
+        >>> duplicate = instance.make_clone(attrs={'name': 'Duplicate Book 2'})
         >>> duplicate.pk
         2
         >>> duplicate.name
@@ -76,3 +83,70 @@ def create_copy_of_instance(instance, exclude=(), save_new=True, attrs=()):
         new_obj.save()
 
     return new_obj
+
+
+def clean_value(value, suffix):
+    # type: (str, str) -> str
+    return re.sub(r'{}\s[\d]$'.format(suffix), '', value, flags=re.I)
+
+
+@contextlib.contextmanager
+def transaction_autocommit(using=None):
+    try:
+        transaction.set_autocommit(True, using=using)
+        yield
+    except TransactionManagementError:
+        raise
+
+
+@contextlib.contextmanager
+def context_mutable_attribute(obj, key, value):
+    default = None
+    is_set = hasattr(obj, key)
+    if is_set:
+        default = getattr(obj, key)
+    try:
+        setattr(obj, key, value)
+        yield
+    finally:
+        if not is_set and hasattr(obj, key):
+            del obj[key]
+        else:
+            setattr(obj, key, default)
+
+
+def get_value(value, suffix, max_length, index):
+    duplicate_suffix = ' {} {}'.format(suffix, index)
+    total_length = len(value + duplicate_suffix)
+
+    if total_length > max_length:
+        # Truncate the value to max_length - suffix length.
+        value = value[:max_length - len(duplicate_suffix)]
+
+    return '{}{}'.format(value, duplicate_suffix)
+
+
+def generate_value(value, suffix, max_length, max_attempts):
+    yield get_value(value, suffix, max_length, 1)
+
+    for i in range(1, max_attempts):
+        yield get_value(value, suffix, max_length, i)
+
+    raise StopIteration(
+        'CloneError: max unique attempts for {} exceeded ({})'
+        .format(value, max_attempts)
+    )
+
+
+def get_unique_value(obj, fname, value, suffix, max_length, max_attempts):
+    qs = obj.__class__._default_manager.exclude(pk=obj.pk)
+    it = generate_value(value, suffix, max_length, max_attempts)
+
+    new = six.next(it)
+    kwargs = {fname: new}
+
+    while not new or qs.filter(**kwargs):
+        new = six.next(it)
+        kwargs[fname] = new
+
+    return new

requirements.txt

@@ -2,6 +2,7 @@
 # This file is autogenerated by pip-compile
 # To update, run:
 #
-#    pip-compile --output-file requirements.txt setup.py
+#    pip-compile
 #
+conditional==1.3
 future==0.17.1

setup.py

@@ -3,7 +3,10 @@
 
 from setuptools import find_packages, setup
 
-install_requires = ['future==0.17.1']
+install_requires = [
+    'future>=0.17.1',
+    'conditional>=1.3',
+]
 
 test_requires = [
     'tox==3.8.6',