Merge pull request #101 from brki/use-django-uuid-field

Use django uuid field

docs/doc/historization_with_cleanerversion.rst

@@ -814,6 +814,34 @@ end date, and the version start date show in the change view. These fields are `
 Out of the box, VersionedAdmin allows for filtering the change view by the ``as_of`` queryset filter, and whether the
 object is current.
 
+Upgrade notes
+=============
+CleanerVersion 1.6.0 / Django 1.8.3
+-----------------------------------
+Starting with CleanerVersion 1.6.0, Django's ``UUIDField`` will be used for the ``id``, ``identity``,
+and ``VersionedForeignKey`` columns if the Django version is 1.8.3 or greater.
+
+If you are upgrading from lower versions of CleanerVersion or Django, you have two choices:
+
+1. Add a setting to your project so that CleanerVersion will continue to use ``CharField`` for ``Versionable``'s
+   UUID fields. Add this to your project's settings::
+
+       VERSIONS_USE_UUIDFIELD = False
+
+This value defaults to ``True`` if not explicitly set when using Django >= 1.8.3.
+
+2. Convert all of the relevant database fields to the type and size that Django uses for UUID fields for the
+   database that you are using.  This may be possible using Django's migrations, or could be done manually by
+   altering the column type as necessary for your database type for all the  ``id``, ``identity``, and
+   foreign key columns of your ``Versionable`` models (don't forget the auto-generated many-to-many tables).
+   This is not a trivial undertaking; it will involve for example dropping and recreating constraints.
+   An example of column altering syntax for PostgreSQL::
+
+       ALTER TABLE blog_author ALTER COLUMN id type uuid USING id:uuid;
+       ALTER TABLE blog_author ALTER COLUMN identity type uuid USING identity:uuid;
+
+You must choose one or the other solution; not doing so will result in your application no longer working.
+
 Known Issues
 ============
 

versions/models.py

@@ -21,7 +21,7 @@
 from django.db.models.sql.datastructures import Join
 from django.apps.registry import apps
 from django.core.exceptions import SuspiciousOperation, ObjectDoesNotExist
-from django.db import transaction
+from django.db import models, router, transaction
 from django.db.models.base import Model
 from django.db.models import Q
 from django.db.models.constants import LOOKUP_SEP
@@ -38,16 +38,21 @@
 from django.utils.timezone import utc
 from django.utils import six
 
-from django.db import models, router
-
-from versions import settings as versions_settings
+from versions.settings import get_versioned_delete_collector_class
+from versions.settings import settings as versions_settings
 from versions.exceptions import DeletionOfNonCurrentVersionError
 
 
 def get_utc_now():
     return datetime.datetime.utcnow().replace(tzinfo=utc)
 
 
+def validate_uuid(uuid_obj):
+    """
+    Check that the UUID object is in fact a valid version 4 uuid.
+    """
+    return isinstance(uuid_obj, uuid.UUID) and uuid_obj.version == 4
+
 QueryTime = namedtuple('QueryTime', 'time active')
 
 
@@ -61,11 +66,6 @@ class VersionManager(models.Manager):
     """
     use_for_related_fields = True
 
-    # Based on http://en.wikipedia.org/wiki/Universally_unique_identifier#Version_4_.28random.29
-    # Matches a valid hyphen-separated version 4 UUID string.
-    uuid_valid_form_regex = re.compile(
-        '^[A-Fa-f0-9]{8}-[A-Fa-f0-9]{4}-4[A-Fa-f0-9]{3}-[89aAbB][A-Fa-f0-9]{3}-[A-Fa-f0-9]{12}$')
-
     def get_queryset(self):
         """
         Returns a VersionedQuerySet capable of handling version time restrictions.
@@ -245,24 +245,14 @@ def _create_at(self, timestamp=None, id=None, forced_identity=None, **kwargs):
 
         Create a Versionable having a version_start_date and version_birth_date set to some pre-defined timestamp
         :param timestamp: point in time at which the instance has to be created
-        :param id: version 4 UUID unicode string.  Usually this is not specified, it will be automatically created.
-        :param forced_identity: version 4 UUID unicode string.  For internal use only.
+        :param id: version 4 UUID unicode object.  Usually this is not specified, it will be automatically created.
+        :param forced_identity: version 4 UUID unicode object.  For internal use only.
         :param kwargs: arguments needed for initializing the instance
         :return: an instance of the class
         """
-        if id:
-            if not self.validate_uuid(id):
-                raise ValueError("id, if provided, must be a valid UUID version 4 string")
-            # Ensure that it's a unicode string:
-            id = six.text_type(id)
-
-        else:
-            id = Versionable.uuid()
-
+        id = Versionable.uuid(id)
         if forced_identity:
-            if not self.validate_uuid(forced_identity):
-                raise ValueError("forced_identity, if provided, must be a valid UUID version 4 string")
-            ident = six.text_type(forced_identity)
+            ident = Versionable.uuid(forced_identity)
         else:
             ident = id
 
@@ -274,12 +264,6 @@ def _create_at(self, timestamp=None, id=None, forced_identity=None, **kwargs):
         kwargs['version_birth_date'] = timestamp
         return super(VersionManager, self).create(**kwargs)
 
-    def validate_uuid(self, uuid_string):
-        """
-        Check that the UUID string is in fact a valid uuid.
-        """
-        return self.uuid_valid_form_regex.match(uuid_string) is not None
-
 
 class VersionedWhereNode(WhereNode):
     def as_sql(self, qn, connection):
@@ -596,7 +580,7 @@ def delete(self):
         del_query.query.select_related = False
         del_query.query.clear_ordering(force_empty=True)
 
-        collector_class = versions_settings.get_versioned_delete_collector_class()
+        collector_class = get_versioned_delete_collector_class()
         collector = collector_class(using=del_query.db)
         collector.collect(del_query)
         collector.delete(get_utc_now())
@@ -1167,11 +1151,17 @@ class Versionable(models.Model):
     VERSIONABLE_FIELDS = [VERSION_IDENTIFIER_FIELD, OBJECT_IDENTIFIER_FIELD, 'version_start_date',
                           'version_end_date', 'version_birth_date']
 
-    id = models.CharField(max_length=36, primary_key=True)
-    """id stands for ID and is the primary key; sometimes also referenced as the surrogate key"""
+    if versions_settings.VERSIONS_USE_UUIDFIELD:
+        id = models.UUIDField(primary_key=True)
+        """id stands for ID and is the primary key; sometimes also referenced as the surrogate key"""
+    else:
+        id = models.CharField(max_length=36, primary_key=True)
 
-    identity = models.CharField(max_length=36)
-    """identity is used as the identifier of an object, ignoring its versions; sometimes also referenced as the natural key"""
+    if versions_settings.VERSIONS_USE_UUIDFIELD:
+        identity = models.UUIDField()
+        """identity is used as the identifier of an object, ignoring its versions; sometimes also referenced as the natural key"""
+    else:
+        identity = models.CharField(max_length=36)
 
     version_start_date = models.DateTimeField()
     """version_start_date points the moment in time, when a version was created (ie. an versionable was cloned).
@@ -1207,7 +1197,7 @@ def delete(self, using=None):
             "{} object can't be deleted because its {} attribute is set to None.".format(
                 self._meta.object_name, self._meta.pk.attname)
 
-        collector_class = versions_settings.get_versioned_delete_collector_class()
+        collector_class = get_versioned_delete_collector_class()
         collector = collector_class(using=using)
         collector.collect([self])
         collector.delete(get_utc_now())
@@ -1265,13 +1255,22 @@ def as_of(self, time):
         self._querytime = QueryTime(time=time, active=True)
 
     @staticmethod
-    def uuid():
+    def uuid(uuid_value=None):
         """
-        Gets a new uuid string that is valid to use for id and identity fields.
+        Returns a uuid value that is valid to use for id and identity fields.
 
-        :return: unicode uuid string
+        :return: unicode uuid object if using UUIDFields, uuid unicode string otherwise.
         """
-        return six.u(str(uuid.uuid4()))
+        if uuid_value:
+            if not validate_uuid(uuid_value):
+                raise ValueError("uuid_value must be a valid UUID version 4 object")
+        else:
+            uuid_value = uuid.uuid4()
+
+        if versions_settings.VERSIONS_USE_UUIDFIELD:
+            return uuid_value
+        else:
+            return six.u(str(uuid_value))
 
     def _clone_at(self, timestamp):
         """

versions/settings.py

@@ -1,5 +1,37 @@
-from django.conf import settings
+from django.conf import settings as django_settings
 import importlib
+from django import VERSION
+
+
+_cache = {}
+
+
+class VersionsSettings(object):
+    """
+    Gets a setting from django.conf.settings if set, otherwise from the defaults
+    defined in this class.
+
+    A magic accessor is used instead of just defining module-level variables because
+    Django doesn't like attributes of the django.conf.settings object to be accessed in
+    module scope.
+    """
+
+    defaults = {
+        'VERSIONED_DELETE_COLLECTOR': 'versions.deletion.VersionedCollector',
+        'VERSIONS_USE_UUIDFIELD': VERSION[:3] >= (1, 8, 3),
+    }
+
+    def __getattr__(self, name):
+        try:
+            return getattr(django_settings, name)
+        except AttributeError:
+            try:
+                return self.defaults[name]
+            except KeyError:
+                raise AttributeError("{} object has no attribute {}".format(self.__class__, name))
+
+
+settings = VersionsSettings()
 
 
 def import_from_string(val, setting_name):
@@ -16,10 +48,6 @@ def import_from_string(val, setting_name):
         raise ImportError("Could not import '{}' for CleanerVersion setting '{}'. {}: {}.".format(
             (val, setting_name, e.__class__.__name__, e)))
 
-_cache = {}
-_defaults = {
-    'VERSIONED_DELETE_COLLECTOR': 'versions.deletion.VersionedCollector'
-}
 
 def get_versioned_delete_collector_class():
     """
@@ -31,25 +59,7 @@ def get_versioned_delete_collector_class():
     try:
         cls = _cache[key]
     except KeyError:
-        collector_class_string = get_setting(key)
+        collector_class_string = getattr(settings, key)
         cls = import_from_string(collector_class_string, key)
         _cache[key] = cls
     return cls
-
-def get_setting(setting_name):
-    """
-    Gets a setting from django.conf.settings if set, otherwise from the defaults
-    defined in this module.
-
-    A function is used for this instead of just defining a module-level variable because
-    Django doesn't like attributes of the django.conf.settings object to be accessed in
-    module scope.
-
-    :param string setting_name: setting to take from django.conf.setting
-    :return: class
-    """
-    try:
-        return getattr(settings, setting_name)
-    except AttributeError:
-        return _defaults[setting_name]
-

versions_tests/tests/test_models.py

@@ -2131,9 +2131,12 @@ def test_prefetch_related_via_foreignkey(self):
             self.assertEqual(self.city1, team.city)
 
     def test_prefetch_related_via_many_to_many(self):
+        # award1 - award10
         awards = [Award.objects.create(name='award' + str(i)) for i in range(1, 11)]
+        # city0 - city2
         cities = [City.objects.create(name='city-' + str(i)) for i in range(3)]
         teams = []
+        # team-0-0 with city0 - team-2-1 with city1
         for i in range(3):
             for j in range(2):
                 teams.append(Team.objects.create(
@@ -2149,6 +2152,12 @@ def test_prefetch_related_via_many_to_many(self):
 
         t2 = get_utc_now()
 
+        # players is player-0-0 with team-0-0 through player-5-5 with team-2-1
+        # players with awards:
+        # player-[012345]-1, [012345]-3, [012345]-5,
+        # the -1s have awards: 1,2
+        # the -3s have awards: 3,4
+        # the -5s have awards: 5,6
         with self.assertNumQueries(6):
             players_t2 = list(
                 Player.objects.as_of(t2).prefetch_related('team', 'awards').filter(
@@ -2476,16 +2485,20 @@ def test_filter_on_fk_relation(self):
 class SpecifiedUUIDTest(TestCase):
 
     @staticmethod
-    def uuid4():
-        return six.text_type(str(uuid.uuid4()))
+    def uuid4(uuid_value=None):
+        if not uuid_value:
+            return uuid.uuid4()
+        if isinstance(uuid_value, uuid.UUID):
+            return uuid_value
+        return uuid.UUID(uuid_value)
 
     def test_create_with_uuid(self):
         p_id = self.uuid4()
         p = Person.objects.create(id=p_id, name="Alice")
-        self.assertEqual(p_id, p.id)
-        self.assertEqual(p_id, p.identity)
+        self.assertEqual(str(p_id), str(p.id))
+        self.assertEqual(str(p_id), str(p.identity))
 
-        p_id = six.text_type(str(uuid.uuid5(uuid.NAMESPACE_OID, 'bar')))
+        p_id = uuid.uuid5(uuid.NAMESPACE_OID, 'bar')
         with self.assertRaises(ValueError):
             Person.objects.create(id=p_id, name="Alexis")
 
@@ -2501,12 +2514,14 @@ def test_create_with_forced_identity(self):
         if connection.vendor == 'postgresql' and get_version() >= '1.7':
             with self.assertRaises(IntegrityError):
                 with transaction.atomic():
-                    Person.objects.create(forced_identity=p.identity, name="Alexis")
+                    ident = self.uuid4(p.identity)
+                    Person.objects.create(forced_identity=ident, name="Alexis")
 
         p.delete()
         sleep(0.1)  # The start date of p2 does not necessarily have to equal the end date of p.
 
-        p2 = Person.objects.create(forced_identity=p.identity, name="Alexis")
+        ident = self.uuid4(p.identity)
+        p2 = Person.objects.create(forced_identity=ident, name="Alexis")
         p2.version_birth_date = p.version_birth_date
         p2.save()
         self.assertEqual(p.identity, p2.identity)