Search code examples
djangodjango-rest-frameworkorm

Is there a way to optimize the query for this piece of code؟


I want that when the user gets the messages of a page, the field seen of all unread messages should be set to True Is there a way to improve and optimize this code below?

I have 4 table (user, room, message, seen)

One user can have several rooms and one room can have several users A user can send several messages and a message can only be related to a specific user And one message can be seen by several users (seen table)

models.py file:

class User(models.Model):
      username = models.CharField(max_length=255, unique=True)
      is_active = models.BooleanField(default=False)
      date_join = models.DateTimeField(auto_now_add=True)
      is_admin = models.BooleanField(default=False)

      def __str__(self):
          return f"{self.username}|{self.is_admin}"

class Room(models.Model):
     name = models.CharField(max_length=255)
     users = models.ManyToManyField(User, related_name="rooms")


     def __str__(self):
         return f"{self.name}"

class Message(models.Model):
      content = models.TextField()
      seen = models.BooleanField(default=False)
      created_at = models.DateTimeField(auto_now_add=True)
      user = models.ForeignKey(User, on_delete=models.CASCADE, 
      related_name="messages")
      room = models.ForeignKey(Room, on_delete=models.CASCADE, 
      related_name="room_messages")
      replied_to = models.ForeignKey('self', null=True, blank=True, 
      on_delete=models.SET_NULL)


      def __str__(self):
          return f"{self.user.id}: {self.content} [{self.created_at}]"

class Seen(models.Model):
      user = models.ForeignKey(User, on_delete=models.CASCADE)
      message = models.ForeignKey(Message, on_delete=models.CASCADE, 
      related_name="seens")
      date_seen = models.DateTimeField(auto_now_add=True)

views.py file:

class MessageViewSet(GenericViewSet, ListModelMixin, RetrieveModelMixin, DestroyModelMixin):

    def list(self, request, *args, **kwargs):

        queryset = self.get_queryset()
        if self.kwargs.get("room_pk"):
            queryset = queryset.filter(room_id=self.kwargs.get("room_pk"))

        filter_queryset = self.filter_queryset(queryset)
        page = self.paginate_queryset(filter_queryset)

        if page is not None:
            serializer = self.get_serializer(page, many=True)
            response = self.get_paginated_response(serializer.data)
            for message in page:
                if message.user.id != self.request.user.id:
                    if not message.seens.select_related("message").select_related("user").filter(user_id=self.request.user.id).exists():

                        message.seen = True
                        seen_obj = Seen()
                        seen_obj.user = self.request.user
                        seen_obj.message = message
                        seen_obj.save()
                        message.seens.add(seen_obj)
                        message.save()
                else:
                    continue
            return response

Solution

  • You don't need to add the item to the seens, this is just the reverse relation of the ForeignKey, so it will be added "implicitly". Another problem is that you each time make a query to check if the message has been seen, you can annotate that once with an Exists subquery [Django-doc], furthermore you can boost efficiency significantly by constructing the Seen objects in bulk:

    from django.db.models import Exists, OuterRef
    
    
    class MessageViewSet(
        ListModelMixin, RetrieveModelMixin, DestroyModelMixin, GenericViewSet
    ):
        def list(self, request, *args, **kwargs):
            queryset = self.get_queryset().annotate(
                is_seen=Exists(
                    Seen.objects.filter(
                        message_id=OuterRef('pk'), user_id=request.user.pk
                    )
                )
            )
            if 'room_pk' in self.kwargs:
                queryset = queryset.filter(room_id=self.kwargs['room_pk'])
    
            filter_queryset = self.filter_queryset(queryset)
            page = self.paginate_queryset(filter_queryset)
    
            if page is not None:
                serializer = self.get_serializer(page, many=True)
                response = self.get_paginated_response(serializer.data)
                Seen.objects.bulk_create(
                    [
                        Seen(user=self.request.user, message=message)
                        for message in page
                        if message.user_id != self.request.user.id
                        and not message.is_seen
                    ]
                )
                return response

    This will inspect and create Seen objects all in only one extra query.


    Note: It is normally better to make use of the settings.AUTH_USER_MODEL [Django-doc] to refer to the user model, than to use the User model [Django-doc] directly. For more information you can see the referencing the User model section of the documentation.