From fa48029db8213993a9bd1e61a2b7ca9a4414aee2 Mon Sep 17 00:00:00 2001 From: Ben Lopatin Date: Sun, 3 Apr 2016 19:04:05 -0400 Subject: [PATCH] Remove swappable base model for TimeStampedModel and fix slug field The base timestamping model is declared locally using fields taken from django-model-utils and the configurable slug field is added via a pass-through class. These features were carried over from the pre-Django migrations era, and South permitted this as it made changes based on expected database changes. Django's migration library looks at the fields as classes, and records changes to *anything* on the field class. This includes keyword arguments and the class path. The upshot is that it is fields can only be swapped if they have the same deconstructed values (from `deconstruct`). The result of leaving these in is spurious migrations due to different import paths and keyword arguments - even when the database column is unchanged. For the slug field, the two supported libraries do, import path aside. So if the import path can be replaced, by ensuring that a local class is alwasy used, simply inheriting from the desired base class, then these can be safely used. The base model is more troublesome since the underlying field classes in the model need to be updated. In the case of the two base classes, between django-extensions and django-model-utils, these have differing keyword arguments as well. Closes gh-96 --- conftest.py | 4 +- manage.py | 3 +- organizations/fields.py | 85 ++++++++++++++++++++++++ organizations/migrations/0001_initial.py | 24 +++---- organizations/models.py | 38 +++++------ requirements-test.txt | 1 + 6 files changed, 116 insertions(+), 39 deletions(-) create mode 100644 organizations/fields.py diff --git a/conftest.py b/conftest.py index e77c1d31..eabc0730 100644 --- a/conftest.py +++ b/conftest.py @@ -29,10 +29,10 @@ def pytest_configure(): "organizations", "test_custom", ], - MIDDLEWARE_CLASSES=(), # Silence Django 1.7 warnings + MIDDLEWARE_CLASSES=[], SITE_ID=1, FIXTURE_DIRS=['tests/fixtures'], - ORGS_TIMESTAMPED_MODEL='django_extensions.db.models.TimeStampedModel', + ORGS_SLUGFIELD='autoslug.AutoSlugField', ROOT_URLCONF="tests.urls", ) django.setup() diff --git a/manage.py b/manage.py index 5b17d47e..1bc135e1 100755 --- a/manage.py +++ b/manage.py @@ -34,7 +34,8 @@ MIDDLEWARE_CLASSES=(), # Silence Django 1.7 warnings SITE_ID=1, FIXTURE_DIRS=['tests/fixtures'], - ORGS_TIMESTAMPED_MODEL='django_extensions.db.models.TimeStampedModel', + ORGS_SLUGFIELD='autoslug.AutoSlugField', + # ORGS_SLUGFIELD='django_extensions.db.fields.AutoSlugField', INSTALLED_APPS=INSTALLED_APPS, ROOT_URLCONF="tests.urls", ) diff --git a/organizations/fields.py b/organizations/fields.py new file mode 100644 index 00000000..bcc03095 --- /dev/null +++ b/organizations/fields.py @@ -0,0 +1,85 @@ +# -*- coding: utf-8 -*- + +""" +Most of this code extracted and borrowed from django-model-utils + +Copyright (c) 2009-2015, Carl Meyer and contributors +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above + copyright notice, this list of conditions and the following + disclaimer in the documentation and/or other materials provided + with the distribution. + * Neither the name of the author nor the names of other + contributors may be used to endorse or promote products derived + from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +""" + +from __future__ import unicode_literals + +from importlib import import_module + +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured +from django.db import models +from django.utils.timezone import now + + +class AutoCreatedField(models.DateTimeField): + """ + A DateTimeField that automatically populates itself at + object creation. + + By default, sets editable=False, default=datetime.now. + + """ + def __init__(self, *args, **kwargs): + kwargs.setdefault('editable', False) + kwargs.setdefault('default', now) + super(AutoCreatedField, self).__init__(*args, **kwargs) + + +class AutoLastModifiedField(AutoCreatedField): + """ + A DateTimeField that updates itself on each save() of the model. + + By default, sets editable=False and default=datetime.now. + + """ + def pre_save(self, model_instance, add): + value = now() + setattr(model_instance, self.attname, value) + return value + + +ORGS_SLUGFIELD = getattr(settings, 'ORGS_SLUGFIELD', + 'django_extensions.db.fields.AutoSlugField') + +try: + module, klass = ORGS_SLUGFIELD.rsplit('.', 1) + BaseSlugField = getattr(import_module(module), klass) +except (ImportError, ValueError): + raise ImproperlyConfigured("Your SlugField class, '{0}', is improperly defined. " + "See the documentation and install an auto slug field".format(ORGS_SLUGFIELD)) + + +class SlugField(BaseSlugField): + """Class redefinition for migrations""" diff --git a/organizations/migrations/0001_initial.py b/organizations/migrations/0001_initial.py index 2a7537f1..83e4ba48 100644 --- a/organizations/migrations/0001_initial.py +++ b/organizations/migrations/0001_initial.py @@ -2,14 +2,10 @@ from __future__ import unicode_literals from django.db import migrations, models -from django.conf import settings +import organizations.fields import organizations.base - -# Workaround to prevent migrations from using *developer installed* fields -# rather than locally configured fields. -from ..models import SlugField, TimeStampedModel -CreationDateTimeField = TimeStampedModel._meta.get_field('created').__class__ -ModificationDateTimeField = TimeStampedModel._meta.get_field('modified').__class__ +import django.utils.timezone +from django.conf import settings class Migration(migrations.Migration): @@ -25,9 +21,9 @@ class Migration(migrations.Migration): ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), ('name', models.CharField(help_text='The name of the organization', max_length=200)), ('is_active', models.BooleanField(default=True)), - ('created', CreationDateTimeField(auto_now_add=True, verbose_name='created')), - ('modified', ModificationDateTimeField(auto_now=True, verbose_name='modified')), - ('slug', SlugField(populate_from=b'name', editable=False, max_length=200, blank=True, help_text='The name in all lowercase, suitable for URL identification', unique=True)), + ('created', organizations.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False)), + ('modified', organizations.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False)), + ('slug', organizations.fields.SlugField(populate_from=b'name', editable=True, max_length=200, help_text='The name in all lowercase, suitable for URL identification', unique=True)), ], options={ 'ordering': ['name'], @@ -41,8 +37,8 @@ class Migration(migrations.Migration): name='OrganizationOwner', fields=[ ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), - ('created', CreationDateTimeField(auto_now_add=True, verbose_name='created')), - ('modified', ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('created', organizations.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False)), + ('modified', organizations.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False)), ('organization', models.OneToOneField(related_name='owner', to='organizations.Organization')), ], options={ @@ -55,8 +51,8 @@ class Migration(migrations.Migration): name='OrganizationUser', fields=[ ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), - ('created', CreationDateTimeField(auto_now_add=True, verbose_name='created')), - ('modified', ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('created', organizations.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False)), + ('modified', organizations.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False)), ('is_admin', models.BooleanField(default=False)), ('organization', models.ForeignKey(related_name='organization_users', to='organizations.Organization')), ('user', models.ForeignKey(related_name='organizations_organizationuser', to=settings.AUTH_USER_MODEL)), diff --git a/organizations/models.py b/organizations/models.py index 63d1dd03..699f939b 100644 --- a/organizations/models.py +++ b/organizations/models.py @@ -23,7 +23,8 @@ # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from importlib import import_module +import warnings + from django.apps import apps from django.conf import settings from django.core.exceptions import ImproperlyConfigured @@ -32,30 +33,23 @@ from django.utils.translation import ugettext_lazy as _ from .base import OrganizationBase, OrganizationUserBase, OrganizationOwnerBase +from .fields import SlugField, AutoCreatedField, AutoLastModifiedField from .signals import user_added, user_removed, owner_changed USER_MODEL = getattr(settings, 'AUTH_USER_MODEL', 'auth.User') -ORGS_SLUGFIELD = getattr(settings, 'ORGS_SLUGFIELD', - 'django_extensions.db.fields.AutoSlugField') -ORGS_TIMESTAMPED_MODEL = getattr(settings, 'ORGS_TIMESTAMPED_MODEL', - 'django_extensions.db.models.TimeStampedModel') - -ERR_MSG = """You may need to install django-extensions or similar library. See -the documentation.""" - -try: - module, klass = ORGS_SLUGFIELD.rsplit('.', 1) - SlugField = getattr(import_module(module), klass) -except: - raise ImproperlyConfigured("Your SlugField class, '{0}'," - " is improperly defined. {1}".format(ORGS_SLUGFIELD, ERR_MSG)) - -try: - module, klass = ORGS_TIMESTAMPED_MODEL.rsplit('.', 1) - TimeStampedModel = getattr(import_module(module), klass) -except: - raise ImproperlyConfigured("Your TimeStampedBaseModel class, '{0}'," - " is improperly defined. {1}".format(ORGS_TIMESTAMPED_MODEL, ERR_MSG)) +ORGS_TIMESTAMPED_MODEL = getattr(settings, 'ORGS_TIMESTAMPED_MODEL', None) + +if ORGS_TIMESTAMPED_MODEL: + warnings.warn("Configured TimestampModel has been replaced and is now ignored.", + DeprecationWarning) + + +class TimeStampedModel(models.Model): + created = AutoCreatedField() + modified = AutoLastModifiedField() + + class Meta: + abstract = True class Organization(OrganizationBase, TimeStampedModel): diff --git a/requirements-test.txt b/requirements-test.txt index 5b5a4b47..9d350635 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -7,6 +7,7 @@ flake8>=2.4.1 # Required to test default models django-extensions>=1.6.0 +django-autoslug>=1.9.0 # Required for mocking signals mock-django==0.6.9