From 66525d77fd011a9544f082c1a3a798ea378f2a68 Mon Sep 17 00:00:00 2001 From: tumb1er Date: Sun, 29 Aug 2021 19:30:10 +0300 Subject: [PATCH 01/12] Add tracker context manager and decorator --- model_utils/tracker.py | 50 ++++++++++++++++++++ tests/test_fields/test_field_tracker.py | 63 +++++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/model_utils/tracker.py b/model_utils/tracker.py index dd1315f0..ad62a3c0 100644 --- a/model_utils/tracker.py +++ b/model_utils/tracker.py @@ -1,3 +1,4 @@ +from collections import Counter from copy import deepcopy from functools import wraps @@ -86,11 +87,46 @@ def __delete__(self, obj): self.descriptor.__delete__(obj) +class FieldsContext: + def __init__(self, tracker, *fields, state=None): + if state is None: + state = {} + self.tracker = tracker + self.fields = fields + self.state = state + + def __enter__(self): + for f in self.fields: + self.state.setdefault(f, 0) + self.state[f] += 1 + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + reset_fields = [] + for f in self.fields: + self.state[f] -= 1 + if self.state[f] == 0: + reset_fields.append(f) + del self.state[f] + if reset_fields: + self.tracker.set_saved_fields(fields=reset_fields) + + class FieldInstanceTracker: def __init__(self, instance, fields, field_map): self.instance = instance self.fields = fields self.field_map = field_map + self.context = FieldsContext(self, *self.fields) + + def __enter__(self): + return self.context.__enter__() + + def __exit__(self, exc_type, exc_val, exc_tb): + return self.context.__exit__(exc_type, exc_val, exc_tb) + + def __call__(self, *fields): + return FieldsContext(self, *fields, state=self.context.state) @property def deferred_fields(self): @@ -195,6 +231,20 @@ class FieldTracker: def __init__(self, fields=None): self.fields = fields + def __call__(self, func=None, fields=None): + def decorator(f): + @wraps(f) + def inner(obj, *args, **kwargs): + tracker = getattr(obj, self.attname) + field_list = tracker.fields if fields is None else fields + with tracker(*field_list): + return f(obj, *args, **kwargs) + + return inner + if func is None: + return decorator + return decorator(func) + def get_field_map(self, cls): """Returns dict mapping fields names to model attribute names""" field_map = {field: field for field in self.fields} diff --git a/tests/test_fields/test_field_tracker.py b/tests/test_fields/test_field_tracker.py index 164b9b7d..9399ffb3 100644 --- a/tests/test_fields/test_field_tracker.py +++ b/tests/test_fields/test_field_tracker.py @@ -831,3 +831,66 @@ def test_child_fields_not_tracked(self): class AbstractModelTrackerTests(ModelTrackerTests): tracked_class = TrackedAbstract + + +class TrackerContextDecoratorTests(TestCase): + + def setUp(self): + self.instance = Tracked.objects.create(number=1) + self.tracker = self.instance.tracker + + def assertChanged(self, *fields): + for f in fields: + self.assertTrue(self.tracker.has_changed(f)) + + def assertNotChanged(self, *fields): + for f in fields: + self.assertFalse(self.tracker.has_changed(f)) + + def test_context_manager(self): + with self.tracker: + with self.tracker: + self.instance.name = 'new' + + self.assertChanged('name') + + self.assertChanged('name') + + self.assertNotChanged('name') + + def test_context_manager_fields(self): + with self.tracker('number'): + with self.tracker('number', 'name'): + self.instance.name = 'new' + self.instance.number += 1 + + self.assertChanged('name', 'number') + + self.assertChanged('number') + self.assertNotChanged('name') + + self.assertNotChanged('number', 'name') + + def test_tracker_decorator(self): + + @Tracked.tracker + def tracked_method(obj): + obj.name = 'new' + self.assertChanged('name') + + tracked_method(self.instance) + + self.assertNotChanged('name') + + def test_tracker_decorator_fields(self): + + @Tracked.tracker(fields=['name']) + def tracked_method(obj): + obj.name = 'new' + obj.number += 1 + self.assertChanged('name', 'number') + + tracked_method(self.instance) + + self.assertChanged('number') + self.assertNotChanged('name') From a661ac9cae94e8bf4dc07e03f351ba5d1d91208d Mon Sep 17 00:00:00 2001 From: tumb1er Date: Sun, 29 Aug 2021 19:30:17 +0300 Subject: [PATCH 02/12] Handle auto-field warning in tests --- tests/settings.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/settings.py b/tests/settings.py index 6fc2bdb2..ac1a80f7 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -21,3 +21,5 @@ 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', } } + +DEFAULT_AUTO_FIELD = 'django.db.models.AutoField' From 9c215ab4ad4c98530767a4d53fb6b0118c262af0 Mon Sep 17 00:00:00 2001 From: tumb1er Date: Sun, 29 Aug 2021 19:37:43 +0300 Subject: [PATCH 03/12] Use tracker context in monkey-patched methods --- model_utils/tracker.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/model_utils/tracker.py b/model_utils/tracker.py index ad62a3c0..cbf1daa7 100644 --- a/model_utils/tracker.py +++ b/model_utils/tracker.py @@ -290,21 +290,17 @@ def _patch(self, model, method, fields_kwarg): @wraps(original) def inner(instance, *args, **kwargs): - ret = original(instance, *args, **kwargs) update_fields = kwargs.get(fields_kwarg) - if not update_fields and update_fields is not None: # () or [] - fields = update_fields - elif update_fields is None: - fields = None + if update_fields is None: + fields = self.fields else: fields = ( field for field in update_fields if field in self.fields ) - getattr(instance, self.attname).set_saved_fields( - fields=fields - ) - return ret + tracker = getattr(instance, self.attname) + with tracker(*fields): + return original(instance, *args, **kwargs) setattr(model, method, inner) From 7ce84d32675beb4518599eb3318fbf531677d143 Mon Sep 17 00:00:00 2001 From: tumb1er Date: Sun, 29 Aug 2021 19:42:54 +0300 Subject: [PATCH 04/12] Test delaying set_saved_fields call with context manager --- tests/test_fields/test_field_tracker.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_fields/test_field_tracker.py b/tests/test_fields/test_field_tracker.py index 9399ffb3..cd9b4858 100644 --- a/tests/test_fields/test_field_tracker.py +++ b/tests/test_fields/test_field_tracker.py @@ -894,3 +894,13 @@ def tracked_method(obj): self.assertChanged('number') self.assertNotChanged('name') + + def test_tracker_context_with_save(self): + + with self.tracker: + self.instance.name = 'new' + self.instance.save() + + self.assertChanged('name') + + self.assertNotChanged('name') From 36bdb3872722dfe9a040f8af85f51b8820efab82 Mon Sep 17 00:00:00 2001 From: tumb1er Date: Sun, 29 Aug 2021 21:22:02 +0300 Subject: [PATCH 05/12] Docs for tracker context --- docs/utilities.rst | 72 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/docs/utilities.rst b/docs/utilities.rst index cc154beb..12184cf6 100644 --- a/docs/utilities.rst +++ b/docs/utilities.rst @@ -346,3 +346,75 @@ This is how ``FieldTracker`` tracks field changes on ``instance.save`` call. 8. ``instance.refresh_from_db()`` call causes initial state reset like for ``save_base()``. +When FieldTracker resets fields state +------------------------------------- + +By the definition: + +.. NOTE:: + * Field value *is changed* if it differs from current database value. + * Field value *was changed* if value has changed in database and field state didn't reset. + +.. code-block:: python + + instance = Tracked.objects.get(pk=1) + # name not changed + instance.name += '_changed' + # name is changed + instance.save() + # name is not changed again + +Current implementation resets fields state after ``post_save`` signals emitting. This is convenient for "outer" code +like in example above, but does not help when model ``save`` method is overridden. + +.. code-block:: python + + class MyModel(models.Model) + name = models.CharField(max_length=64) + tracker = FieldsTracker() + + def save(self): # erroneous implementation + self.name = self.name.replace(' ', '_') + name_changed = self.tracker.has_changed('name') + super().save() + # changed state has been reset here, so we need to store previous state somewhere else + if name_changed: + do_something_about_it() + +``FieldTracker`` provides a context manager interface to postpone fields state reset in complicate situations. + +* Fields state resets after exiting from outer-most context +* By default, all fields are reset, but field list can be provided +* Fields are counted separately depending on field list passed to context managers +* Tracker can be used as decorator +* Different instances have it's own context state +* Different trackers in same instance have separate context state + +.. code-block:: python + + class MyModel(models.Model) + name = models.CharField(max_length=64) + tracker = FieldTracker() + + def save(self): # correct implementation + self.name = self.name.replace(' ', '_') + + with self.tracker: + super().save() + # changed state reset is postponed + if self.tracker.has_changed('name'): + do_something_about_it() + + # Decorator example + @tracker + def save(self): ... + + # Restrict a set of fields to reset here + @tracker(fields=('name')) + def save(self): ... + + # Context manager with field list + def save(self): + with self.tracker('name'): + ... + From 920db5e743079e3d819be1d17b2eca9162e3093b Mon Sep 17 00:00:00 2001 From: tumb1er Date: Sun, 29 Aug 2021 21:29:47 +0300 Subject: [PATCH 06/12] Describe FieldsContext context manager in changes --- CHANGES.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.rst b/CHANGES.rst index 06e005b9..62adb77f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,6 +7,7 @@ Unreleased - Drop support for `Django 3.0` - Added urlsafe token field. +- Introduce context manager for FieldTracker state reset (GH-#491) 4.1.1 (2020-12-01) ------------------ From 0eb5545f4bd9040ba339ba65674f8cf0c6388371 Mon Sep 17 00:00:00 2001 From: tumb1er Date: Sun, 29 Aug 2021 21:38:56 +0300 Subject: [PATCH 07/12] Fix unused import --- model_utils/tracker.py | 1 - 1 file changed, 1 deletion(-) diff --git a/model_utils/tracker.py b/model_utils/tracker.py index cbf1daa7..57434dd8 100644 --- a/model_utils/tracker.py +++ b/model_utils/tracker.py @@ -1,4 +1,3 @@ -from collections import Counter from copy import deepcopy from functools import wraps From d7df35c665d2eca437a551b39f7a4e2ff9726c25 Mon Sep 17 00:00:00 2001 From: tumb1er Date: Mon, 13 Sep 2021 09:15:39 +0300 Subject: [PATCH 08/12] #494 add breaking changes note on 4.1.1 release for #404 --- CHANGES.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 62adb77f..8a375a21 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -11,6 +11,17 @@ Unreleased 4.1.1 (2020-12-01) ------------------ + +**Breaking changes:** +- `FieldTracker` now marks fields as not changed after `refresh_from_db` + respecting `fields` argument (GH-#404) +- `FieldTracker` now respects `update_fields` changed in overridden `save()` + method (GH-#404) +- `FieldTracker` now resets states after `pre_save()` and not anymore `save()` + signals, possibly altering the behaviour of overridden `save()` + methods (GH-#404) + +**Other changes:** - Applied `isort` to codebase (Refs GH-#402) - Fix `TypeError` in save when model inherits from both TimeStampModel and StatusModel. (Fixes GH-465) From 7d95317b3efff81ea4461ddccd571d30dc13716e Mon Sep 17 00:00:00 2001 From: tumb1er Date: Mon, 13 Sep 2021 09:16:14 +0300 Subject: [PATCH 09/12] #494 fix typo --- docs/utilities.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/utilities.rst b/docs/utilities.rst index 12184cf6..b1a618d3 100644 --- a/docs/utilities.rst +++ b/docs/utilities.rst @@ -387,7 +387,7 @@ like in example above, but does not help when model ``save`` method is overridde * By default, all fields are reset, but field list can be provided * Fields are counted separately depending on field list passed to context managers * Tracker can be used as decorator -* Different instances have it's own context state +* Different instances have their own context state * Different trackers in same instance have separate context state .. code-block:: python From 59ab645a1ca83d7efeeb63a6ebe47689db4b21d9 Mon Sep 17 00:00:00 2001 From: tumb1er Date: Mon, 13 Sep 2021 09:27:44 +0300 Subject: [PATCH 10/12] #494 add docstring for FieldsContext --- model_utils/tracker.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/model_utils/tracker.py b/model_utils/tracker.py index 57434dd8..9d14c5c9 100644 --- a/model_utils/tracker.py +++ b/model_utils/tracker.py @@ -87,6 +87,25 @@ def __delete__(self, obj): class FieldsContext: + """ + A context manager for tracking nested reset fields contexts. + + If tracked fields is mentioned in more than one FieldsContext, it's state + is being reset on exiting last context that mentions that field. + + >>> with fields_context(obj.tracker, 'f1', state=state): + ... with fields_context(obj.tracker, 'f1', 'f2', state=state): + ... obj.do_something_useful() + ... # f2 is reset after inner context exit + ... obj.do_something_else() + ... # f1 is reset after outer context exit + >>> + + * Note that fields are countedbe passing same state dict + * FieldsContext is instantiated using FieldInstanceTracker (`obj.tracker`) + * Different objects has own state stack + + """ def __init__(self, tracker, *fields, state=None): if state is None: state = {} From eb8df825eb089cdc94960b11220beb15ebc4b00f Mon Sep 17 00:00:00 2001 From: tumb1er Date: Mon, 13 Sep 2021 09:30:13 +0300 Subject: [PATCH 11/12] #494 move breaking changes from 4.1.1 to 4.1.0 --- CHANGES.rst | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 8a375a21..89ffe1a5 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,6 +12,13 @@ Unreleased 4.1.1 (2020-12-01) ------------------ +- Applied `isort` to codebase (Refs GH-#402) +- Fix `TypeError` in save when model inherits from both TimeStampModel + and StatusModel. (Fixes GH-465) + +4.1.0 (2020-11-29) +------------------ + **Breaking changes:** - `FieldTracker` now marks fields as not changed after `refresh_from_db` respecting `fields` argument (GH-#404) @@ -22,18 +29,9 @@ Unreleased methods (GH-#404) **Other changes:** -- Applied `isort` to codebase (Refs GH-#402) -- Fix `TypeError` in save when model inherits from both TimeStampModel - and StatusModel. (Fixes GH-465) - -4.1.0 (2020-11-29) ------------------- - Update InheritanceQuerySetMixin to avoid querying too much tables - TimeStampedModel now automatically adds 'modified' field as an update_fields parameter even if it is forgotten while using save() -- `FieldTracker` now marks fields as not changed after `refresh_from_db` -- `FieldTracker` now respects `update_fields` changed in overridden `save()` - method - Replace ugettext_lazy with gettext_lazy to satisfy Django deprecation warning - Add available_objects manager to SoftDeletableModel and add deprecation warning to objects manager. From 3249a5c826dbc323cdd16126ab4802ec3ff94ea6 Mon Sep 17 00:00:00 2001 From: tumb1er Date: Fri, 8 Oct 2021 11:31:13 +0300 Subject: [PATCH 12/12] #494 fix typo and add some more docstrings --- model_utils/tracker.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/model_utils/tracker.py b/model_utils/tracker.py index 9d14c5c9..d19c8c42 100644 --- a/model_utils/tracker.py +++ b/model_utils/tracker.py @@ -101,12 +101,24 @@ class FieldsContext: ... # f1 is reset after outer context exit >>> - * Note that fields are countedbe passing same state dict + * Note that fields are counted by passing same state dict * FieldsContext is instantiated using FieldInstanceTracker (`obj.tracker`) * Different objects has own state stack """ + def __init__(self, tracker, *fields, state=None): + """ + :param tracker: FieldInstanceTracker instance to be reset after + context exit + :param fields: a list of field names to be tracked in current context + :param state: shared state dict used to count number of field + occurrences in context stack. + + On context enter each field mentioned in `fields` has +1 in shared + state, and on exit it receives -1. Fields that have zero after context + exit are reset in tracker instance. + """ if state is None: state = {} self.tracker = tracker @@ -114,12 +126,21 @@ def __init__(self, tracker, *fields, state=None): self.state = state def __enter__(self): + """ + Increments tracked fields occurrences count in shared state. + """ for f in self.fields: self.state.setdefault(f, 0) self.state[f] += 1 return self def __exit__(self, exc_type, exc_val, exc_tb): + """ + Decrements tracked fields occurrences count in shared state. + + If any field has no more occurrences in shared state, this field is + being reset by tracker. + """ reset_fields = [] for f in self.fields: self.state[f] -= 1