From cff22855c26b52537d1364caf7f8cbf9ddc948c5 Mon Sep 17 00:00:00 2001 From: Swen Kooij Date: Thu, 25 May 2017 17:23:39 +0300 Subject: [PATCH] Revert "LocalizedUniqueSlugField refactored" This reverts commit 03df76d6d795f4453629bf526cc04a86c6393adc. --- README.rst | 3 +- .../fields/localized_uniqueslug_field.py | 120 ++++++------------ localized_fields/mixins.py | 47 +++++-- tests/fake_model.py | 3 +- 4 files changed, 75 insertions(+), 98 deletions(-) diff --git a/README.rst b/README.rst index bf92714..d70d996 100644 --- a/README.rst +++ b/README.rst @@ -193,10 +193,11 @@ Besides ``LocalizedField``, there's also: .. code-block:: python from localized_fields import (LocalizedModel, + AtomicSlugRetryMixin, LocalizedField, LocalizedUniqueSlugField) - class MyModel(LocalizedModel): + class MyModel(AtomicSlugRetryMixin, LocalizedModel): title = LocalizedField() slug = LocalizedUniqueSlugField(populate_from='title') diff --git a/localized_fields/fields/localized_uniqueslug_field.py b/localized_fields/fields/localized_uniqueslug_field.py index 120089a..a5ac993 100644 --- a/localized_fields/fields/localized_uniqueslug_field.py +++ b/localized_fields/fields/localized_uniqueslug_field.py @@ -1,19 +1,18 @@ from datetime import datetime from django.conf import settings -from django import forms from django.utils.text import slugify -from django.db import transaction -from django.db.utils import IntegrityError - +from django.core.exceptions import ImproperlyConfigured from ..util import get_language_codes +from ..mixins import AtomicSlugRetryMixin from ..localized_value import LocalizedValue -from .localized_field import LocalizedField +from .localized_autoslug_field import LocalizedAutoSlugField -class LocalizedUniqueSlugField(LocalizedField): - """Automatically provides slugs for a localized field upon saving." +class LocalizedUniqueSlugField(LocalizedAutoSlugField): + """Automatically provides slugs for a localized + field upon saving." An improved version of :see:LocalizedAutoSlugField, which adds: @@ -22,6 +21,8 @@ class LocalizedUniqueSlugField(LocalizedField): - Improved performance When in doubt, use this over :see:LocalizedAutoSlugField. + Inherit from :see:AtomicSlugRetryMixin in your model to + make this field work properly. """ def __init__(self, *args, **kwargs): @@ -29,11 +30,14 @@ class LocalizedUniqueSlugField(LocalizedField): kwargs['uniqueness'] = kwargs.pop('uniqueness', get_language_codes()) + super(LocalizedUniqueSlugField, self).__init__( + *args, + **kwargs + ) + self.populate_from = kwargs.pop('populate_from') self.include_time = kwargs.pop('include_time', False) - super().__init__(*args, **kwargs) - def deconstruct(self): """Deconstructs the field into something the database can store.""" @@ -45,88 +49,36 @@ class LocalizedUniqueSlugField(LocalizedField): kwargs['include_time'] = self.include_time return name, path, args, kwargs - def formfield(self, **kwargs): - """Gets the form field associated with this field. - - Because this is a slug field which is automatically - populated, it should be hidden from the form. - """ - - defaults = { - 'form_class': forms.CharField, - 'required': False - } - - defaults.update(kwargs) - - form_field = super().formfield(**defaults) - form_field.widget = forms.HiddenInput() - - return form_field - - def contribute_to_class(self, cls, name, *args, **kwargs): - """Hook that allow us to operate with model class. We overwrite save() - method to run retry logic. - - Arguments: - cls: - Model class. - - name: - Name of field in model. - """ - # apparently in inheritance cases, contribute_to_class is called more - # than once, so we have to be careful not to overwrite the original - # save method. - if not hasattr(cls, '_orig_save'): - cls._orig_save = cls.save - max_retries = getattr( - settings, - 'LOCALIZED_FIELDS_MAX_RETRIES', - 100 - ) - - def _new_save(instance, *args_, **kwargs_): - retries = 0 - while True: - with transaction.atomic(): - try: - slugs = self.populate_slugs(instance, retries) - setattr(instance, name, slugs) - instance._orig_save(*args_, **kwargs_) - break - except IntegrityError as e: - if retries >= max_retries: - raise e - # check to be sure a slug fight caused - # the IntegrityError - s_e = str(e) - if name in s_e and 'unique' in s_e: - retries += 1 - else: - raise e - - cls.save = _new_save - super().contribute_to_class(cls, name, *args, **kwargs) - - def populate_slugs(self, instance, retries=0): - """Built the slug from populate_from field. + def pre_save(self, instance, add: bool): + """Ran just before the model is saved, allows us to built + the slug. Arguments: instance: The model that is being saved. - retries: - The value of the current attempt. + add: + Indicates whether this is a new entry + to the database or an update. Returns: The localized slug that was generated. """ - slugs = LocalizedValue() - populates_slugs = getattr(instance, self.populate_from, {}) - for lang_code, _ in settings.LANGUAGES: - value = populates_slugs.get(lang_code) + if not isinstance(instance, AtomicSlugRetryMixin): + raise ImproperlyConfigured(( + 'Model \'%s\' does not inherit from AtomicSlugRetryMixin. ' + 'Without this, the LocalizedUniqueSlugField will not work.' + ) % type(instance).__name__) + + slugs = LocalizedValue() + + for lang_code, _ in settings.LANGUAGES: + value = self._get_populate_from_value( + instance, + self.populate_from, + lang_code + ) if not value: continue @@ -146,11 +98,13 @@ class LocalizedUniqueSlugField(LocalizedField): if self.include_time: slug += '-%d' % datetime.now().microsecond - if retries > 0: + if instance.retries > 0: # do not add another - if we already added time if not self.include_time: slug += '-' - slug += '%d' % retries + slug += '%d' % instance.retries slugs.set(lang_code, slug) + + setattr(instance, self.name, slugs) return slugs diff --git a/localized_fields/mixins.py b/localized_fields/mixins.py index dc3b2b2..1402715 100644 --- a/localized_fields/mixins.py +++ b/localized_fields/mixins.py @@ -1,17 +1,38 @@ -from django.core.checks import Warning +from django.db import transaction +from django.conf import settings +from django.db.utils import IntegrityError + class AtomicSlugRetryMixin: - """A Mixin keeped for backwards compatibility""" + """Makes :see:LocalizedUniqueSlugField work by retrying upon + violation of the UNIQUE constraint.""" - @classmethod - def check(cls, **kwargs): - errors = super().check(**kwargs) - errors.append( - Warning( - 'localized_fields.AtomicSlugRetryMixin is deprecated', - hint='There is no need to use ' - 'localized_fields.AtomicSlugRetryMixin', - obj=cls - ) + def save(self, *args, **kwargs): + """Saves this model instance to the database.""" + + max_retries = getattr( + settings, + 'LOCALIZED_FIELDS_MAX_RETRIES', + 100 ) - return errors + + if not hasattr(self, 'retries'): + self.retries = 0 + + with transaction.atomic(): + try: + return super().save(*args, **kwargs) + except IntegrityError as ex: + # this is as retarded as it looks, there's no + # way we can put the retry logic inside the slug + # field class... we can also not only catch exceptions + # that apply to slug fields... so yea.. this is as + # retarded as it gets... i am sorry :( + if 'slug' not in str(ex): + raise ex + + if self.retries >= max_retries: + raise ex + + self.retries += 1 + return self.save() diff --git a/tests/fake_model.py b/tests/fake_model.py index dc8cf8e..04e59d8 100644 --- a/tests/fake_model.py +++ b/tests/fake_model.py @@ -3,6 +3,7 @@ from django.db.migrations.executor import MigrationExecutor from django.contrib.postgres.operations import HStoreExtension from localized_fields.models import LocalizedModel +from localized_fields.mixins import AtomicSlugRetryMixin def define_fake_model(name='TestModel', fields=None): @@ -15,7 +16,7 @@ def define_fake_model(name='TestModel', fields=None): if fields: attributes.update(fields) - model = type(name, (LocalizedModel,), attributes) + model = type(name, (AtomicSlugRetryMixin,LocalizedModel,), attributes) return model