#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
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_fieldsand 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_fieldsis not specified which is denoted byupdate_fields=Nonein signal receivers.If you want to keep track of which field changed you should write your own tracker of old-field values and flush it on
save()or use a third party application that does it for you.