Implement minor fixes from code review

setup.py

@@ -44,7 +44,6 @@
     install_requires=[
         'psycopg2>=2.6.2',
         'Django>=1.11.0',
-        'typing>=3.5.2,<4.0.0;python_version<"3.5"'
     ],
     setup_requires=SETUP_DEPENDENCIES,
     test_suite='tests.runtests.run_tests',

temporal_django/clock.py

@@ -5,15 +5,12 @@
 EntityClock and FieldHistory models, and attaches the ClockedOption.
 """
 import hashlib
-import sys
 import typing
 import uuid
 
 from django.contrib.postgres.fields.ranges import DateTimeRangeField, IntegerRangeField
 from django.db import models
 from django.db.models.signals import post_init
-from django.utils import timezone
-import psycopg2.extras as psql_extras
 
 from .db_extensions import GistIndex, GistExclusionConstraint
 
@@ -89,6 +86,8 @@ def _build_entity_clock_model(
         timestamp=models.DateTimeField(auto_now_add=True),
         activity=models.ForeignKey(activity_model) if activity_model else None,
         Meta=type('Meta', (), {
+            'app_label': cls._meta.app_label,
+            'ordering': ['tick'],  # Sort by tick so that first_tick and latest_tick work correctly
             'db_table': clock_table_name,
             'unique_together': unique_constraints,
         }),
@@ -97,7 +96,6 @@ def _build_entity_clock_model(
 
     clock_class_name = '%sClock' % cls.__name__
     clock_model = type(clock_class_name, (EntityClock,), attrs)
-    setattr(sys.modules[cls.__module__], clock_class_name, clock_model)
 
     return clock_model
 
@@ -124,10 +122,14 @@ def _build_field_history_model(cls: typing.Type[Clocked], field: str, schema: st
 
     attrs = dict(
         id=models.UUIDField(primary_key=True, default=uuid.uuid4),
-        entity=models.ForeignKey(cls),
-        effective=DateTimeRangeField(default=psql_extras.DateTimeRange(timezone.now(), None)),
+        entity=models.ForeignKey(
+            cls,
+            related_name='%s_history' % field,
+        ),
+        effective=DateTimeRangeField(),
         vclock=IntegerRangeField(),
         Meta=type('Meta', (), {
+            'app_label': cls._meta.app_label,
             'db_table': table_name,
             'indexes': [
                 GistIndex(
@@ -150,7 +152,6 @@ def _build_field_history_model(cls: typing.Type[Clocked], field: str, schema: st
     attrs[field] = next(f for f in cls._meta.fields if f.name == field)
 
     model = type(class_name, (FieldHistory,), attrs)
-    setattr(sys.modules[cls.__module__], class_name, model)
     return model
 
 
@@ -165,7 +166,9 @@ def _save_initial_state_post_init(sender: typing.Type[Clocked], instance: Clocke
         instance (Clocked)
     """
     fields = sender.temporal_options.temporal_fields
-    instance._state.previous = {f: instance._meta.get_field(f).value_from_object(instance) for f in fields}
+    instance._state._django_temporal_previous = {
+        f: instance._meta.get_field(f).value_from_object(instance) for f in fields
+    }
 
 
 def _disable_bulk_create(cls: typing.Type[Clocked]):

temporal_django/clocked_option.py

@@ -32,11 +32,13 @@ def pre_save_receiver(self, sender, instance=None, **kwargs):
         """receiver for pre_save signal on a Clocked subclass"""
         # We'll check this in the post_save receiver to see if we need to set
         # the initial clock/history
-        instance._state.temporal_add = instance._state.adding
+        if instance:
+            instance._state._django_temporal_add = instance._state.adding
 
     def post_save_receiver(self, sender, instance=None, **kwargs):
         """receiver for post_save signal on a Clocked subclass"""
-        self._record_history(instance)
+        if instance:
+            self._record_history(instance)
 
     def pre_delete_receiver(self, sender, **kwargs):
         """receiver for pre_delete signal on a Clocked subclass"""
@@ -67,7 +69,8 @@ def _record_history(self, clocked: Clocked):
         changed_fields = {}
         for field, history_model in self.history_models.items():
             new_val = clocked._meta.get_field(field).value_from_object(clocked)
-            if clocked._state.temporal_add or (new_val != clocked._state.previous[field]):
+            prev_val = clocked._state._django_temporal_previous[field]
+            if new_val != prev_val or clocked._state._django_temporal_add:
                 changed_fields[(field, history_model)] = new_val
 
         if not changed_fields:
@@ -95,16 +98,22 @@ def _record_history(self, clocked: Clocked):
         #
         for (field, history_model), new_val in changed_fields.items():
             if new_tick > 1:
-                # This is an update. Record the upper bounds of the previous tick
-                with connection.cursor() as cursor:
-                    # TODO: Try to rewrite with the ORM
-                    query = """
-                        update {}
-                        set vclock=int4range(lower(vclock), %s),
-                            effective=tstzrange(lower(effective), %s)
-                        where entity_id=%s and lower(vclock)=%s
-                        """.format(connection.ops.quote_name(history_model._meta.db_table))
-                    cursor.execute(query, [new_tick, timestamp, clocked.pk, new_tick - 1])
+                # This is an update, not a create, so update the upper bounds of the previous tick.
+                #
+                # This cannot be done with F expressions because of the range updates, unless we write our
+                # own implementations of int4range/tstzrange.
+                #
+                # Instead use .raw and force execution by wrapping with list(). This approach ensures that
+                # the update happens in the correct connection/transaction.
+                list(history_model.objects.raw(
+                    """ UPDATE {table_name}
+                        SET vclock = int4range(lower(vclock), %s),
+                            effective = tstzrange(lower(effective), %s)
+                        WHERE entity_id = %s AND upper(vclock) IS NULL
+                        RETURNING id;
+                    """.format(table_name=connection.ops.quote_name(history_model._meta.db_table)),
+                    [new_tick, timestamp, clocked.pk]
+                ))
 
             hist = history_model(**{field: new_val},
                                  entity=clocked,
@@ -113,7 +122,7 @@ def _record_history(self, clocked: Clocked):
             hist.save()
 
             # Update the stored state for this field to detect future changes
-            clocked._state.previous[field] = new_val
+            clocked._state._django_temporal_previous[field] = new_val
 
         # Reset the activity so it can't be accidentally reused easily
         clocked.activity = None

temporal_django/models.py

@@ -2,22 +2,23 @@
 import typing  # noqa
 
 from django.db import models
+from django.contrib.postgres.fields import DateTimeRangeField, IntegerRangeField
 
 
 class EntityClock(models.Model):
     """Model for a clock table"""
-    class Meta:
-        abstract = True
-
     tick = models.IntegerField()
     timestamp = models.DateTimeField()
 
+    class Meta:
+        abstract = True
+
 
 class FieldHistory(models.Model):
     """Model for a column/field history table"""
     entity = None  # type: models.ForeignKey
-    effective = None  # type: psql_extras.DateTimeRange
-    vclock = None  # type: psql_extras.NumericRange
+    effective = DateTimeRangeField()
+    vclock = IntegerRangeField()
 
     class Meta:
         abstract = True
@@ -29,8 +30,6 @@ class Clocked(models.Model):
 
     Use with add_clock to handle temporalizing your model:
     """
-    class Meta:
-        abstract = True
 
     vclock = models.IntegerField(default=0)
     """The current clock tick for this object"""
@@ -44,25 +43,26 @@ class Meta:
     activity = None  # type: models.Model
     """Use this to set the activity for the next save"""
 
-    @property
+    class Meta:
+        abstract = True
+
     def first_tick(self) -> EntityClock:
         """The clock object for the earliest tick of this object's history"""
         return self.clock.first()
 
-    @property
     def latest_tick(self) -> EntityClock:
         """The clock object for the most recent tick of this object's history"""
         return self.clock.last()
 
-    @property
     def date_created(self) -> datetime.datetime:
-        first_tick = self.first_tick
+        """Returns the date and time this object was created"""
+        first_tick = self.first_tick()
         if first_tick:
             return first_tick.timestamp
 
-    @property
     def date_modified(self) -> datetime.datetime:
-        latest_tick = self.latest_tick
+        """Returns the date and time this object was most recently modified"""
+        latest_tick = self.latest_tick()
         if latest_tick:
             return latest_tick.timestamp
 

tests/test_activity.py

@@ -80,8 +80,8 @@ def test_activity_with_multiple_models(self):
         obj1.save()
         obj2.save()
 
-        self.assertEqual(obj1.first_tick.activity, act)
-        self.assertEqual(obj2.first_tick.activity, act)
+        self.assertEqual(obj1.first_tick().activity, act)
+        self.assertEqual(obj2.first_tick().activity, act)
 
         # Make sure it didn't do something silly like duplicate the activity.
         self.assertEquals(TestModelActivity.objects.count(), 1)
@@ -104,8 +104,8 @@ def test_activity_with_multiple_objects(self):
         obj1.save()
         obj2.save()
 
-        self.assertEqual(obj1.first_tick.activity, act)
-        self.assertEqual(obj2.first_tick.activity, act)
+        self.assertEqual(obj1.first_tick().activity, act)
+        self.assertEqual(obj2.first_tick().activity, act)
 
         # Make sure it didn't do something silly like duplicate the activity.
         self.assertEquals(TestModelActivity.objects.count(), 1)

tests/test_clocked.py

@@ -15,16 +15,16 @@ def test_date_created_and_modified(self):
 
             obj = TestModel(title='Test', num=1)
 
-            self.assertIsNone(obj.date_created)
-            self.assertIsNone(obj.date_modified)
+            self.assertIsNone(obj.date_created())
+            self.assertIsNone(obj.date_modified())
 
             obj.activity = act
             obj.save()
 
             created_obj = TestModel.objects.first()
             self.assertEqual(created_obj.title, 'Test')
-            self.assertEqual(created_obj.date_created, datetime.datetime(2017, 10, 31))
-            self.assertEqual(created_obj.date_created, created_obj.date_modified)
+            self.assertEqual(created_obj.date_created(), datetime.datetime(2017, 10, 31))
+            self.assertEqual(created_obj.date_created(), created_obj.date_modified())
 
         with freeze_time('2017-11-01'):
             act = TestModelActivity(desc='Edit the object')
@@ -37,8 +37,8 @@ def test_date_created_and_modified(self):
             edited_obj = TestModel.objects.first()
 
             self.assertEqual(edited_obj.title, 'Test 2')
-            self.assertEqual(edited_obj.date_created, datetime.datetime(2017, 10, 31))
-            self.assertEqual(edited_obj.date_modified, datetime.datetime(2017, 11, 1))
+            self.assertEqual(edited_obj.date_created(), datetime.datetime(2017, 10, 31))
+            self.assertEqual(edited_obj.date_modified(), datetime.datetime(2017, 11, 1))
 
     def test_first_and_latest_tick(self):
         """The first_tick and latest_tick models should also work"""
@@ -50,8 +50,8 @@ def test_first_and_latest_tick(self):
         obj.save()
 
         created_obj = TestModel.objects.first()
-        self.assertEqual(created_obj.first_tick.tick, 1)
-        self.assertEqual(created_obj.first_tick.tick, created_obj.vclock)
+        self.assertEqual(created_obj.first_tick().tick, 1)
+        self.assertEqual(created_obj.first_tick().tick, created_obj.vclock)
 
         act = TestModelActivity(desc='Edit the object')
         act.save()
@@ -62,9 +62,9 @@ def test_first_and_latest_tick(self):
 
         edited_obj = TestModel.objects.first()
 
-        self.assertEqual(created_obj.first_tick.tick, 1)
-        self.assertEqual(created_obj.latest_tick.tick, 2)
-        self.assertEqual(created_obj.latest_tick.tick, edited_obj.vclock)
+        self.assertEqual(created_obj.first_tick().tick, 1)
+        self.assertEqual(created_obj.latest_tick().tick, 2)
+        self.assertEqual(created_obj.latest_tick().tick, edited_obj.vclock)
 
     def test_no_changes_no_tick(self):
         """Verify that if you don't change anything, but do a save, that no tick is created"""
@@ -76,8 +76,8 @@ def test_no_changes_no_tick(self):
         obj.save()
 
         created_obj = TestModel.objects.first()
-        self.assertEqual(created_obj.first_tick.tick, 1)
-        self.assertEqual(created_obj.first_tick.tick, created_obj.vclock)
+        self.assertEqual(created_obj.first_tick().tick, 1)
+        self.assertEqual(created_obj.first_tick().tick, created_obj.vclock)
 
         act = TestModelActivity(desc='Edit the object')
         act.save()
@@ -86,6 +86,6 @@ def test_no_changes_no_tick(self):
 
         edited_obj = TestModel.objects.first()
 
-        self.assertEqual(created_obj.first_tick.tick, 1)
-        self.assertEqual(created_obj.latest_tick.tick, 1)
-        self.assertEqual(created_obj.latest_tick.tick, edited_obj.vclock)
+        self.assertEqual(created_obj.first_tick().tick, 1)
+        self.assertEqual(created_obj.latest_tick().tick, 1)
+        self.assertEqual(created_obj.latest_tick().tick, edited_obj.vclock)