Opened 11 hours ago

Closed 10 hours ago

Last modified 10 hours ago

#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 Simon Charette, 10 hours ago

Resolution: invalid
Status: newclosed

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 to Model.save(update_fields) and not the the set of fields that were locally changed from their last persisted value.

update_fields

The set of fields to update as passed to Model.save(), or None if update_fields wasn’t passed to save().

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 by update_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.

Version 0, edited 10 hours ago by Simon Charette (next)
Note: See TracTickets for help on using tickets.
Back to Top