Skip to content

Commit

Permalink
Resolved bug cloning models with UniqueConstraint. (#423)
Browse files Browse the repository at this point in the history
* Resolved bug cloning models with UniqueConstraint.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fixed test.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fixed test.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fixed test.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fixed test.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Updated test.

* Updated test.

* Updated test.

* Update utils.py

* Update test_clone_mixin.py

* Update test_clone_mixin.py

* Update test_clone_mixin.py

* Update test_clone_mixin.py

* Update utils.py

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
jackton1 and pre-commit-ci[bot] authored Jul 18, 2021
1 parent 72b5b15 commit 45530f6
Show file tree
Hide file tree
Showing 7 changed files with 268 additions and 15 deletions.
2 changes: 1 addition & 1 deletion django_clone/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"sample_driver",
]

if sys.version_info >= (3, 6):
if sys.version_info >= (3, 6) and "--fix" in sys.argv:
INSTALLED_APPS += ["migration_fixer"]


Expand Down
22 changes: 20 additions & 2 deletions model_clone/mixins/clone.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
clean_value,
context_mutable_attribute,
get_fields_and_unique_fields_from_cls,
get_unique_default,
get_unique_value,
transaction_autocommit,
)
Expand Down Expand Up @@ -341,9 +342,26 @@ def _create_copy_of_instance(instance, using=None, force=False, sub_clone=False)
and not f.choices
):
value = clean_value(value, unique_duplicate_suffix)
if use_unique_duplicate_suffix:

if f.has_default():
value = f.get_default()

if not callable(f.default):
value = get_unique_default(
model=cls,
fname=f.attname,
value=value,
transform=(
slugify if isinstance(f, SlugField) else str
),
suffix=unique_duplicate_suffix,
max_length=f.max_length,
max_attempts=max_unique_duplicate_query_attempts,
)

elif use_unique_duplicate_suffix:
value = get_unique_value(
obj=instance,
model=cls,
fname=f.attname,
value=value,
transform=(slugify if isinstance(f, SlugField) else str),
Expand Down
74 changes: 68 additions & 6 deletions model_clone/tests/test_clone_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,34 @@ def test_cloning_without_explicit_clone_m2m_fields(self):
list(book_clone.authors.values_list("first_name", "last_name")),
)

def test_cloning_with_unique_constraint_is_valid(self):
sale_tag = SaleTag.objects.create(name="test-sale-tag")
clone_sale_tag_1 = sale_tag.make_clone()

self.assertNotEqual(sale_tag.pk, clone_sale_tag_1.pk)
self.assertRegexpMatches(
clone_sale_tag_1.name,
r"{}\s[\d]".format(SaleTag.UNIQUE_DUPLICATE_SUFFIX),
)

clone_sale_tag_2 = clone_sale_tag_1.make_clone()

self.assertNotEqual(clone_sale_tag_1.pk, clone_sale_tag_2.pk)
self.assertRegexpMatches(
clone_sale_tag_2.name,
r"{}\s[\d]".format(SaleTag.UNIQUE_DUPLICATE_SUFFIX),
)

def test_cloning_with_unique_constraint_uses_field_default(self):
tag = Tag.objects.create(name="test-tag")
clone_tag = tag.make_clone()

self.assertNotEqual(tag.pk, clone_tag.pk)
self.assertRegexpMatches(
clone_tag.name,
r"\s[\d]",
)

@patch("sample.models.Book._clone_m2m_fields", new_callable=PropertyMock)
def test_cloning_with_explicit_clone_m2m_fields(
self,
Expand Down Expand Up @@ -416,17 +444,51 @@ def test_cloning_unique_together_fields_with_enum_field(self):
created_by=self.user1,
)

author_clone = author.make_clone()
author_clone_1 = author.make_clone()

self.assertNotEqual(author.pk, author_clone.pk)
self.assertEqual(author.sex, author_clone.sex)
self.assertNotEqual(author.pk, author_clone_1.pk)
self.assertEqual(author.sex, author_clone_1.sex)
self.assertEqual(
author_clone.first_name,
author_clone_1.first_name,
"{} {} {}".format(first_name, Author.UNIQUE_DUPLICATE_SUFFIX, 1),
)
self.assertEqual(
author_clone.last_name,
"{} {} {}".format(last_name, Author.UNIQUE_DUPLICATE_SUFFIX, 1),
author_clone_1.last_name,
Author._meta.get_field("last_name").get_default(),
)

author_clone_2 = author.make_clone()

self.assertNotEqual(author.pk, author_clone_2.pk)
self.assertEqual(author.sex, author_clone_2.sex)
self.assertEqual(
author_clone_2.first_name,
"{} {} {}".format(first_name, Author.UNIQUE_DUPLICATE_SUFFIX, 2),
)
self.assertEqual(
author_clone_2.last_name,
"{} {} {}".format(
Author._meta.get_field("last_name").get_default(),
Author.UNIQUE_DUPLICATE_SUFFIX,
1,
),
)

author_clone_3 = author.make_clone()

self.assertNotEqual(author.pk, author_clone_3.pk)
self.assertEqual(author.sex, author_clone_3.sex)
self.assertEqual(
author_clone_3.first_name,
"{} {} {}".format(first_name, Author.UNIQUE_DUPLICATE_SUFFIX, 3),
)
self.assertEqual(
author_clone_3.last_name,
"{} {} {}".format(
Author._meta.get_field("last_name").get_default(),
Author.UNIQUE_DUPLICATE_SUFFIX,
2,
),
)

def test_cloning_unique_slug_field(self):
Expand Down
79 changes: 75 additions & 4 deletions model_clone/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,25 @@ def create_copy_of_instance(
return new_obj


def unpack_unique_constraints(opts, only_fields=()):
"""
Unpack unique constraint fields.
:param opts: Model options
:type opts: `django.db.models.options.Options`
:param only_fields: Fields that should be considered.
:type only_fields: `collections.Iterable`
:return: Flat list of fields.
"""
fields = []
constraints = getattr(
opts, "total_unique_constraints", getattr(opts, "constraints", [])
)
for constraint in constraints:
fields.extend([f for f in constraint.fields if f in only_fields])
return fields


def unpack_unique_together(opts, only_fields=()):
"""
Unpack unique together fields.
Expand Down Expand Up @@ -163,7 +182,7 @@ def get_value(value, suffix, transform, max_length, index):
Append a suffix to a string value and apply a pass directly to a
transformation function.
"""
duplicate_suffix = " {} {}".format(suffix, index)
duplicate_suffix = " " + "{} {}".format(suffix, index).strip()
total_length = len(value + duplicate_suffix)

if max_length is not None and total_length > max_length:
Expand All @@ -189,12 +208,20 @@ def generate_value(value, suffix, transform, max_length, max_attempts):
)


def get_unique_value(obj, fname, value, transform, suffix, max_length, max_attempts):
def get_unique_value(
model,
fname,
value="",
transform=lambda v: v,
suffix="",
max_length=None,
max_attempts=100,
):
"""
Generate a unique value using current value and query the model
for existing objects with the new value.
"""
qs = obj.__class__._default_manager.all()
qs = model._default_manager.all()
it = generate_value(value, suffix, transform, max_length, max_attempts)

new = six.next(it)
Expand Down Expand Up @@ -251,10 +278,54 @@ def get_fields_and_unique_fields_from_cls(
only_fields=[f.attname for f in fields],
)

unique_constraint_field_names = unpack_unique_constraints(
opts=cls._meta,
only_fields=[f.attname for f in fields],
)

unique_fields = [
f.name
for f in fields
if not f.auto_created and (f.unique or f.name in unique_field_names)
if not f.auto_created
and (
f.unique
or f.name in unique_field_names
or f.name in unique_constraint_field_names
)
]

return fields, unique_fields


def get_unique_default(
model,
fname,
value,
transform=lambda v: v,
suffix="",
max_length=None,
max_attempts=100,
):
"""Get a unique value using the value and adding a suffix if needed."""

qs = model._default_manager.all()

if not qs.filter(**{fname: value}).exists():
return value

it = generate_value(
value,
suffix,
transform,
max_length,
max_attempts,
)

new = six.next(it)
kwargs = {fname: new}

while qs.filter(**kwargs).exists():
new = six.next(it)
kwargs[fname] = new

return new
30 changes: 30 additions & 0 deletions sample/migrations/0019_saletag_sale_tag_unique_name.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Generated by Django 3.2.5 on 2021-07-17 20:26

import django
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("sample", "0018_auto_20210628_2301"),
]

operations = (
[
migrations.AddConstraint(
model_name="saletag",
constraint=models.UniqueConstraint(
fields=("name",), name="sale_tag_unique_name"
),
),
]
if django.VERSION >= (2, 2)
else [
migrations.AlterField(
model_name="saletag",
name="name",
field=models.CharField(max_length=255, unique=True),
),
]
)
42 changes: 42 additions & 0 deletions sample/migrations/0020_auto_20210717_2230.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Generated by Django 3.2.5 on 2021-07-17 22:30
import django
from django.db import migrations, models

import sample.models


class Migration(migrations.Migration):

dependencies = [
("sample", "0019_saletag_sale_tag_unique_name"),
]

operations = (
[
migrations.AlterField(
model_name="tag",
name="name",
field=models.CharField(
default=sample.models.get_unique_tag_name, max_length=255
),
),
migrations.AddConstraint(
model_name="tag",
constraint=models.UniqueConstraint(
fields=("name",), name="tag_unique_name"
),
),
]
if django.VERSION >= (2, 2)
else [
migrations.AlterField(
model_name="tag",
name="name",
field=models.CharField(
default=sample.models.get_unique_tag_name,
max_length=255,
unique=True,
),
),
]
)
34 changes: 32 additions & 2 deletions sample/models.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
from uuid import uuid4

import django
from django.conf import settings
from django.db import models
from django.utils import timezone
from django.utils.translation import gettext as _

from model_clone.utils import get_unique_default

if django.VERSION >= (2, 2):
from django.db.models import UniqueConstraint

from model_clone import CloneMixin
from model_clone.models import CloneModel

Expand Down Expand Up @@ -38,15 +44,39 @@ class Meta:
unique_together = (("first_name", "last_name", "sex"),)


def get_unique_tag_name():
return get_unique_default(
model=Tag,
fname="name",
value="test-tag",
)


class Tag(CloneModel):
name = models.CharField(max_length=255)
name = models.CharField(
max_length=255, default=get_unique_tag_name, unique=django.VERSION < (2, 2)
)

if django.VERSION >= (2, 2):

class Meta:
constraints = [
UniqueConstraint(fields=["name"], name="tag_unique_name"),
]

def __str__(self):
return _(self.name)


class SaleTag(CloneModel):
name = models.CharField(max_length=255)
name = models.CharField(max_length=255, unique=django.VERSION < (2, 2))

if django.VERSION >= (2, 2):

class Meta:
constraints = [
UniqueConstraint(fields=["name"], name="sale_tag_unique_name"),
]

def __str__(self):
return _(self.name)
Expand Down

0 comments on commit 45530f6

Please sign in to comment.