Better pre and post save logic

chamber/models/__init__.py

@@ -355,14 +355,29 @@ def _persistence_clean(self, *args, **kwargs):
     def _get_save_extra_kwargs(self):
         return {}
 
-    def _pre_save(self, *args, **kwargs):
+    def _pre_save(self, changed, changed_fields, *args, **kwargs):
+        """
+        :param change: True if model instance was changed, False if was created
+        :param changed_fields: fields that was changed before _pre_save was called (changes in the method do not
+                affect it)
+        """
         pass
 
-    def _call_pre_save(self, *args, **kwargs):
-        self._pre_save(*args, **kwargs)
+    def _call_pre_save(self, changed, changed_fields, *args, **kwargs):
+        self._pre_save(changed, changed_fields, *args, **kwargs)
 
     def _save(self, update_only_changed_fields=False, is_cleaned_pre_save=None, is_cleaned_post_save=None,
               force_insert=False, force_update=False, using=None, update_fields=None, *args, **kwargs):
+        """
+        Save of SmartModel has the following sequence:
+        * pre-save methods are called
+        * pre-save validation is invoked (it can be turned off)
+        * pre-save signals are called
+        * model is saved, model changed fields are reset, is_adding is not False and is_changing has True value
+        * post-save methods are called
+        * post-save validation is invoked (it is turned off by default)
+        * post-save signals are invoked
+        """
         is_cleaned_pre_save = (
             self._smart_meta.is_cleaned_pre_save if is_cleaned_pre_save is None else is_cleaned_pre_save
         )
@@ -374,46 +389,63 @@ def _save(self, update_only_changed_fields=False, is_cleaned_pre_save=None, is_c
 
         kwargs.update(self._get_save_extra_kwargs())
 
-        self._call_pre_save(self.is_changing, self.changed_fields, *args, **kwargs)
+        self._call_pre_save(
+            changed=self.is_changing, changed_fields=self.changed_fields.get_static_changes(), *args, **kwargs
+        )
         if is_cleaned_pre_save:
             self._clean_pre_save(*args, **kwargs)
-        dispatcher_pre_save.send(sender=origin, instance=self, change=self.is_changing,
-                                 changed_fields=self.changed_fields.get_static_changes(),
-                                 *args, **kwargs)
+        dispatcher_pre_save.send(
+            sender=origin, instance=self, changed=self.is_changing,
+            changed_fields=self.changed_fields.get_static_changes(),
+            *args, **kwargs
+        )
+
         if not update_fields and update_only_changed_fields:
             update_fields = list(self.changed_fields.keys()) + ['changed_at']
             # remove primary key from updating fields
             if self._meta.pk.name in update_fields:
                 update_fields.remove(self._meta.pk.name)
-        super().save(force_insert=force_insert, force_update=force_update, using=using,
-                                     update_fields=update_fields)
 
-        self._call_post_save(self.is_changing, self.changed_fields, *args, **kwargs)
+        # Changed fields must be cached before save, for post_save and signal purposes
+        post_save_changed_fields = self.changed_fields.get_static_changes()
+        post_save_is_changing = self.is_changing
+
+        self.save_simple(force_insert=force_insert, force_update=force_update, using=using, update_fields=update_fields)
+
+        self._call_post_save(
+            changed=post_save_is_changing, changed_fields=post_save_changed_fields, *args, **kwargs
+        )
         if is_cleaned_post_save:
             self._clean_post_save(*args, **kwargs)
-        dispatcher_post_save.send(sender=origin, instance=self, change=self.is_changing,
-                                  changed_fields=self.changed_fields.get_static_changes(),
-                                  *args, **kwargs)
+        dispatcher_post_save.send(
+            sender=origin, instance=self, changed=post_save_is_changing, changed_fields=post_save_changed_fields,
+            *args, **kwargs
+        )
         self.post_save.send()
 
-    def _post_save(self, *args, **kwargs):
+    def _post_save(self, changed, changed_fields, *args, **kwargs):
+        """
+        :param change: True if model instance was changed, False if was created
+        :param changed_fields: fields that was changed before _post_save was called (changes in the method do not
+               affect it)
+         """
         pass
 
-    def _call_post_save(self, *args, **kwargs):
-        self._post_save(*args, **kwargs)
+    def _call_post_save(self, changed, changed_fields, *args, **kwargs):
+        self._post_save(changed, changed_fields, *args, **kwargs)
 
     def save_simple(self, *args, **kwargs):
         super().save(*args, **kwargs)
+        self.is_adding = False
+        self.is_changing = True
+        self.changed_fields = DynamicChangedFields(self)
 
     def save(self, update_only_changed_fields=False, *args, **kwargs):
         if self._smart_meta.is_save_atomic:
             with transaction.atomic():
                 self._save(update_only_changed_fields=update_only_changed_fields, *args, **kwargs)
         else:
             self._save(update_only_changed_fields=update_only_changed_fields, *args, **kwargs)
-        self.is_adding = False
-        self.is_changing = True
-        self.changed_fields = DynamicChangedFields(self)
 
     def _pre_delete(self, *args, **kwargs):
         pass

chamber/models/dispatchers.py

@@ -65,8 +65,8 @@ class CreatedDispatcher(BaseDispatcher):
     Calls registered handler if and only if an instance of the model is being created.
     """
 
-    def _can_dispatch(self, instance, change, **kwargs):
-        return not change
+    def _can_dispatch(self, instance, changed, **kwargs):
+        return not changed
 
 
 class StateDispatcher(BaseDispatcher):
@@ -85,7 +85,7 @@ def __init__(self, handler, enum, field, field_value, signal=None):
         self.field_value = field_value
         super().__init__(handler, signal=signal)
 
-    def _can_dispatch(self, instance, change, changed_fields, *args, **kwargs):
+    def _can_dispatch(self, instance, changed, changed_fields, *args, **kwargs):
         return (
             self.field.get_attname() in changed_fields and
             getattr(instance, self.field.get_attname()) == self.field_value

docs/models.rst

@@ -119,13 +119,15 @@ SmartModel
 
         Like a django form field you can use your own method named by field name for cleaning input value. You can too raise ``ValidationError`` if input value is invalid
 
-    .. method:: _pre_save()
+    .. method:: _pre_save(change, changed_fields, *args, **kwargs)
 
-        Method that is called before saving instance. You can here change instance structure or call some operations before saving object
+        Method that is called before saving instance. You can here change instance structure or call some operations before saving object. Parameter ``change`` has True value if model instance existed before save was called, otherwise False.
+        Parameter ``changed_fields`` contains name of changed fields and its original and current values. Changes in the method body have no effect on ``changed_fields`` property.
 
-    .. method:: _post_save()
+    .. method:: _post_save(change, changed_fields, *args, **kwargs)
 
-        Method that is called after saving instance. You can here change instance structure or call some operations after saving object
+        Method that is called after saving instance. You can here change instance structure or call some operations after saving object. Parameter ``change`` has True value if model instance existed before save was called, otherwise False.
+        Parameter ``changed_fields`` contains name of changed fields and its original and current values. Changes in the method body have no effect on ``changed_fields`` property.
 
     .. method:: _pre_delete()
 

example/dj/apps/test_chamber/handlers.py

@@ -27,8 +27,8 @@ def create_csv_record_handler(instance, **kwargs):
 
 class OneTimeStateChangedHandler(InstanceOneTimeOnSuccessHandler):
 
-    def can_handle(self, instance, **kwargs):
-        return 'state' in instance.changed_fields
+    def can_handle(self, instance, changed, changed_fields, **kwargs):
+        return 'state' in changed_fields
 
     def handle(self, instance, **kwargs):
         from .models import TestOnDispatchModel

example/dj/apps/test_chamber/tests/shortcuts.py

@@ -5,8 +5,10 @@
 from django.test import TestCase
 from django.utils import timezone
 
-from chamber.shortcuts import (bulk_change, bulk_change_and_save, bulk_save, change, change_and_save, distinct_field,
-                               exclude_by_date, filter_by_date, get_object_or_404, get_object_or_none)
+from chamber.shortcuts import (
+    bulk_change, bulk_change_and_save, bulk_save, change, change_and_save, distinct_field,
+    exclude_by_date, filter_by_date, get_object_or_404, get_object_or_none
+)
 
 from germanium.tools import assert_equal, assert_is_none, assert_raises  # pylint: disable=E0401