#36108 closed Bug (invalid)
The `update_field` when using `django.db.models.signals` does not work with a custom Model Manager
Reported by: | NicoJJohnson | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 4.2 |
Severity: | Normal | Keywords: | signal, model, manager, filter |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I have a Model that has some sensitive objects labeled with is_private=True
.
Furthermore, all private objects should be filtered out by default, unless there is a User, in which we can check if they own the object.
class SensitiveObjectManager(models.Manager): def __init__(self): super().__init__() self.user = None def for_user(self, user): """Create a new manager instance with the user context""" manager = SensitiveObjectManager() manager.model = self.model manager.user = user if hasattr(self, "core_filters"): manager.core_filters = self.core_filters return manager def get_queryset(self): qs = super().get_queryset() if hasattr(self, "core_filters"): qs = qs.filter(**self.core_filters) if self.user is None: return qs.filter(is_private=False) return qs.filter(Q(is_private=False) | Q(owner=self.user.id)) class SensitiveObject(models.Model): objects = SensitiveObjectManager() all_objects = models.Manager() id = models.UUIDField(primary_key=True, default=uuid.uuid4) is_private = models.BooleanField(default=True) owner = models.ForeignKey(User) is_leaked = models.BooleanField(default=False)
This is designed because SensitiveObject.objects.all()
is commonly used throughout our code, but with this Manager, we can always filter out the private objects. To include objects that the user owns, we can use SensitiveObject.objects.for_user(User).all()
.
Everything works fine with it so far except for a very odd bug using django.db.models.signals.post_save
. We want to catch when is_leaked
is updated so that we can update a few other Models. So we have
#apps.py def sensitive_object_updated(sender, instance, created, update_fields, **kwargs): # ( Would perform additional ORM logic, but for testing, the print statements are all that's needed) print("Instance:") print(instance) print("Instance.is_private:") print(instance.is_private) print("Instance.is_leaked:") print(instance.is_leaked) print("update_fields:") print(update_fields) from django.apps import AppConfig class SensitiveObjectConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" name = "sensitive_object" def ready(self): from django.db.models.signals import post_save from sensitive_object.models import SensitiveObject post_save.connect(sensitive_object_updated, sender=SensitiveObject)
If the SensitiveObject.is_private=True
, then the update_fields
will NOT be included in sensitive_object_updated
which is very annoying (update_fields
will equal None
). But when SensitiveObject.is_private=False
, the update_fields
work fine. I tested this, and I know it is because of the SensitiveObjectManager
. Furthermore, if you test it, you will see the instance
and instance.is_leaked
have the correct values inside sensitive_object_updated
always, (whether if is_private
is True or False). I’m pretty confident that this is a bug with Django.
For more information about trying to find a patch, this is also posted on the django forum under the name "Signal does not pick up update_field if the model’s Manager filters objects"
Example Test Case:
# tests.py from django.test import TestCase def SensitiveObjectTest(TestCase): def example_test(self): private_object = SensitiveObject.objects.create( owner=self.user, is_private=True, is_leaked=False ) self.assertTrue(SensitiveObject.all_objects.filter(id=private_dataset.id).exists()) # Thanks to the SensitiveObjectManager, private datasets are filtered out of objects by default. self.assertFalse(SensitiveObject.objects.filter(id=private_dataset.id).exists()) self.assertTrue(SensitiveObject.objects.for_user(self.user).filter(id=private_dataset.id).exists()) print("Observe print logs from sensitive_object_updated for private object") private_object.is_leaked=True private_object.save() # Should see the print statements from `senstive_object_updated`. # update_fields will be None public_object = SensitiveObject.objects.create( owner=self.user, is_private=False, is_leaked=False ) self.assertTrue(SensitiveObject.all_objects.filter(id=public_dataset.id).exists()) self.assertTrue(SensitiveObject.objects.filter(id=public_dataset.id).exists()) print("Observe print logs from sensitive_object_updated for public object") public_object.is_leaked=True public_object.save() # Should see the print statements from `senstive_object_updated`. # update_fields will be correct
Change History (1)
comment:1 by , 17 hours ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
This the forum thread referred to in the report which I assume could not be included due to spam filter configuration.
There's something that doesn't add up in this report with regards to
update_fields
and I believe could be due to a misunderstanding of the feature; the value provided to the save signal receiver is nothing more than what's passed toModel.save(update_fields)
and not the the set of fields that were locally changed from their last persisted value.In other words Django doesn't keep track of model instance fields that were assigned a different value since their last retrieval / persistence to the database and will default to saving all fields if
update_fields
is not specified which is denoted byupdate_fields=None
in signal receivers.If you want to keep track of which field changed and you should use a third party application for that.