I am using Django 3.2 in a project. I have a method that compares the value of a field to an integer (see code below).

Django throws an error (which makes sense because of the different types).

My question is - how else am I to implement this check (whilst using F expression to avoid race conditions)?

Code

def bookmark(self, actor, note=''):
    if self.can_bookmark(actor):
        ct, object_id = self.get_content_info()
        found_objects = self.bookmarks.filter(content_type=ct, object_id=object_id, actor=actor)        
            
        if found_objects:
            # We are unbookmarking
            found_objects.delete()
            self.bookmarks_count = F('bookmarks_count') - 1

            # Sanity check (-ve value may occur if Db has been manipulated manually for instance)
            if self.bookmarks_count < 0:  # <- Barfs here
                self.bookmarks_count = 0

            if self.bookmarks_count == 0: # <- Barfs here too
                self.last_bookmarked = None    

            self.save()            
            return True

        else:
            # Create bookmark object and save it
            if self.create_and_increment(self.bookmarks, actor, note=note):
                self.bookmarks_count = F('bookmarks_count')   1
                self.last_bookmarked = timezone.now()

                actionable_object = self.actionable_object
                self.save()

                self.refresh_from_db()

                item_bookmarked.send(sender= actionable_object.__class__, instance=actionable_object, event_type = BOOKMARK, actor=actor)  
                return True    

            else:
                return False      
    else:
        # do nothing
        return False

[[Edit]]

Here is the full traceback:

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/path/to/myproj/models.py", line 455, in bookmark
    if self.bookmarks_count < 0:
TypeError: '<' not supported between instances of 'CombinedExpression' and 'int'

CodePudding user response：

You should perform the sanity check first, or work with Greatest ^[Django-doc]:

from django.db.models import Value
from django.db.models.functions import Greatest

self.bookmarks_count = Greatest(F('bookmarks_count') - 1, Value(0))
self.save()
self.refresh_from_db(fields=('bookmarks_count',))
if self.bookmarks_count == 0:
    self.last_bookmarked = None
self.save()

Here the .refresh_from_db(…) ^[Django-doc] will ensure that we get the updated bookmarks_count.

Note: You do not have to store the number of items of a ManyToManyField in another field. You can use .annotate(…) ^[Django-doc] when you need to determine this by the database. Storing this explicitly in a field is a form of data duplication, and it turns out that keeping these in sync is harder than what one might expect.