Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimise certaines requêtes SQL #6490

Merged
merged 10 commits into from
Aug 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions zds/forum/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def get_all_topics_of_a_forum(self, forum_pk, is_sticky=False):
return (
self.filter(forum__pk=forum_pk, is_sticky=is_sticky)
.order_by("-last_message__pubdate")
.select_related("author__profile")
.select_related("author__profile", "solved_by")
.prefetch_related("last_message", "tags")
.all()
)
Expand All @@ -105,7 +105,9 @@ def get_all_topics_of_a_user(self, current, target):
return queryset.order_by("-pubdate").all()

def get_all_topics_of_a_tag(self, tag, user):
queryset = self.filter(tags__in=[tag]).prefetch_related("author", "last_message", "tags")
queryset = (
self.filter(tags__in=[tag]).select_related("solved_by").prefetch_related("author", "last_message", "tags")
)
queryset = queryset.filter(self.visibility_check_query(user)).distinct()
return queryset.order_by("-last_message__pubdate")

Expand Down Expand Up @@ -144,7 +146,11 @@ def get_all_messages_of_a_user(self, current, target):
if not current.has_perm("forum.change_post"):
queryset = queryset.filter(is_visible=True)

queryset = queryset.filter(self.visibility_check_query(current)).prefetch_related("author").order_by("-pubdate")
queryset = (
queryset.filter(self.visibility_check_query(current))
.prefetch_related("author", "topic")
.order_by("-pubdate")
)

return queryset

Expand All @@ -161,11 +167,11 @@ def is_topic_last_message_read(self, topic, user=None, check_auth=True):
:param check_auth: if True will shortcut to ``False`` if user is not authenticated
:return: ``True`` if topic has been read by user
"""
if not hasattr(topic, "_is_read"):
setattr(topic, "_is_read", {})
if not hasattr(topic, "_user_has_read"):
setattr(topic, "_user_has_read", {})
if user is None or (check_auth and not user.is_authenticated):
return False
cache_is_read = getattr(topic, "_is_read")
cache_is_read = getattr(topic, "_user_has_read")
if user.username not in cache_is_read:
cache_is_read[user.username] = self.filter(post=topic.last_message, topic=topic, user=user).exists()
return cache_is_read[user.username]
Expand Down
23 changes: 19 additions & 4 deletions zds/forum/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,13 @@ class Meta:
tags = models.ManyToManyField(Tag, verbose_name="Tags du forum", blank=True, db_index=True)

objects = TopicManager()
_first_post = None

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._last_user_post = dict()
self._last_post = None
self._first_post = None
self._is_read = None

def __str__(self):
return self.title
Expand Down Expand Up @@ -262,7 +268,10 @@ def get_last_post(self):
"""
:return: the last post in the thread.
"""
return self.last_message
if self._last_post is None:
self._last_post = Post.objects.filter(pk=self.last_message).select_related("author").first()

return self._last_post

def get_last_answer(self):
"""
Expand Down Expand Up @@ -400,7 +409,11 @@ def antispam(self, user=None):
if user is None:
user = get_current_user()

last_user_post = Post.objects.filter(topic=self).filter(author=user.pk).order_by("position").last()
if user not in self._last_user_post:
self._last_user_post[user] = (
Post.objects.filter(topic=self).filter(author=user.pk).order_by("position").last()
)
last_user_post = self._last_user_post[user]

if last_user_post and last_user_post == self.get_last_post():
duration = datetime.now() - last_user_post.pubdate
Expand All @@ -427,7 +440,9 @@ def old_post_warning(self):

@property
def is_read(self):
return self.is_read_by_user(get_current_user())
if self._is_read is None:
self._is_read = self.is_read_by_user(get_current_user())
return self._is_read

def is_read_by_user(self, user=None, check_auth=True):
return TopicRead.objects.is_topic_last_message_read(self, user, check_auth)
Expand Down
5 changes: 4 additions & 1 deletion zds/forum/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,10 @@ def get_object(self, queryset=None):
if queryset is None:
queryset = Topic.objects
result = (
queryset.filter(pk=self.kwargs.get("topic_pk")).select_related("solved_by").select_related("author").first()
queryset.filter(pk=self.kwargs.get("topic_pk"))
.select_related("solved_by", "author")
.prefetch_related("tags")
.first()
)
if result is None:
raise Http404(f"Pas de forum avec l'identifiant {self.kwargs.get('topic_pk')}")
Expand Down
16 changes: 9 additions & 7 deletions zds/member/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class Meta:
last_visit = models.DateTimeField("Date de dernière visite", null=True, blank=True)
_permissions = {}
_groups = None
_hats = None
_cached_city = None

objects = ProfileManager()
Expand Down Expand Up @@ -409,15 +410,16 @@ def get_hats(self):
"""
Return all hats the user is allowed to use.
"""
profile_hats = list(self.hats.all())
groups_hats = list(Hat.objects.filter(group__in=self.user.groups.all()))
hats = profile_hats + groups_hats
if self._hats is None:
profile_hats = list(self.hats.all())
groups_hats = list(Hat.objects.filter(group__in=self.user.groups.all()))
self._hats = profile_hats + groups_hats

# We sort internal hats before the others, and we slugify for sorting to sort correctly
# with diatrics.
hats.sort(key=lambda hat: f'{"a" if hat.is_staff else "b"}-{old_slugify(hat.name)}')
# We sort internal hats before the others, and we slugify for sorting to sort correctly
# with diatrics.
self._hats.sort(key=lambda hat: f'{"a" if hat.is_staff else "b"}-{old_slugify(hat.name)}')

return hats
return self._hats

def get_requested_hats(self):
"""
Expand Down
3 changes: 2 additions & 1 deletion zds/mp/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ def get_private_topics_of_user(self, user_id):
super()
.get_queryset()
.filter(Q(participants__in=[user_id]) | Q(author=user_id))
.select_related("author")
.select_related("author__profile")
.prefetch_related("participants")
.distinct()
.order_by("-last_message__pubdate")
.all()
Expand Down
17 changes: 14 additions & 3 deletions zds/mp/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ class Meta:
pubdate = models.DateTimeField("Date de création", auto_now_add=True, db_index=True)
objects = PrivateTopicManager()

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._cache_is_unread = dict()
self._cache_first_post = None

@staticmethod
def create(title, subtitle, author, recipients):
limit = PrivateTopic._meta.get_field("title").max_length
Expand Down Expand Up @@ -140,7 +145,10 @@ def first_post(self):
:return: PrivateTopic object first answer (PrivatePost)
:rtype: PrivatePost object or None
"""
return PrivatePost.objects.filter(privatetopic=self).order_by("position_in_topic").first()
if self._cache_first_post is None:
self._cache_first_post = PrivatePost.objects.filter(privatetopic=self).order_by("position_in_topic").first()

return self._cache_first_post

def last_read_post(self, user=None):
"""
Expand Down Expand Up @@ -212,7 +220,7 @@ def resolve_last_read_post_absolute_url(self, user=None):
return self.first_unread_post().get_absolute_url()

def resolve_last_post_pk_and_pos_read_by_user(self, user):
"""Determine the primary ey of position of the last post read by a user.
"""Determine the primary key of position of the last post read by a user.

:param user: the current (authenticated) user. Please do not try with unauthenticated user, il would lead to a \
useless request.
Expand Down Expand Up @@ -252,7 +260,10 @@ def is_unread(self, user=None):
if user is None:
user = get_current_user()

return is_privatetopic_unread(self, user)
if user not in self._cache_is_unread:
self._cache_is_unread[user] = is_privatetopic_unread(self, user)

return self._cache_is_unread[user]

def is_author(self, user):
"""
Expand Down
20 changes: 17 additions & 3 deletions zds/mp/tests/tests_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@

from zds.member.tests.factories import ProfileFactory
from zds.mp.tests.factories import PrivateTopicFactory, PrivatePostFactory
from zds.mp.models import mark_read, is_privatetopic_unread, is_reachable, NotParticipatingError, NotReachableError
from zds.mp.models import (
is_privatetopic_unread,
is_reachable,
mark_read,
NotParticipatingError,
NotReachableError,
PrivateTopic,
)

# by moment, i wrote the scenario to be simpler

Expand Down Expand Up @@ -105,14 +112,17 @@ def test_is_unread(self):
# post1 - user1 - read
# post2 - user2 - read
mark_read(self.topic1, self.user1)
self.assertTrue(self.topic1.is_unread(self.user1)) # cache is working
self.topic1 = PrivateTopic.objects.get(pk=self.topic1.pk) # get a new object to have an empty cache
self.assertFalse(self.topic1.is_unread(self.user1))

# scenario - topic1 :
# post1 - user1 - read
# post2 - user2 - read
# post3 - user2 - unread
PrivatePostFactory(privatetopic=self.topic1, author=self.user2, position_in_topic=3)

self.assertFalse(self.topic1.is_unread(self.user1)) # cache is working
self.topic1 = PrivateTopic.objects.get(pk=self.topic1.pk) # get a new object to have an empty cache
self.assertTrue(self.topic1.is_unread(self.user1))

def test_topic_never_read_get_last_read(self):
Expand Down Expand Up @@ -270,12 +280,16 @@ def test_mark_read(self, topic_read):
# post1 - user1 - read
# post2 - user2 - read
mark_read(self.topic1, self.profile1.user)
self.assertFalse(self.topic1.is_unread(self.profile1.user))
self.assertEqual(topic_read.send.call_count, 1)
self.assertTrue(self.topic1.is_unread(self.profile1.user)) # cache is working
self.topic1 = PrivateTopic.objects.get(pk=self.topic1.pk) # get a new object to have an empty cache
self.assertFalse(self.topic1.is_unread(self.profile1.user))

# scenario - topic1 :
# post1 - user1 - read
# post2 - user2 - read
# post3 - user2 - unread
PrivatePostFactory(privatetopic=self.topic1, author=self.profile2.user, position_in_topic=3)
self.assertFalse(self.topic1.is_unread(self.profile1.user)) # cache is working
self.topic1 = PrivateTopic.objects.get(pk=self.topic1.pk) # get a new object to have an empty cache
self.assertTrue(self.topic1.is_unread(self.profile1.user))
6 changes: 5 additions & 1 deletion zds/notification/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,11 @@ def get_objects_followed_by(self, user):
user=user, is_active=True, content_type=ContentType.objects.get_for_model(Topic)
).values_list("object_id", flat=True)

return Topic.objects.filter(id__in=topic_list).order_by("-last_message__pubdate")
return (
Topic.objects.filter(id__in=topic_list)
.select_related("solved_by", "last_message")
.order_by("-last_message__pubdate")
)

def unfollow_and_mark_read_everybody_at(self, topic):
"""
Expand Down
4 changes: 3 additions & 1 deletion zds/pages/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ class ContactView(ListView):
"""

model = GroupContact
queryset = GroupContact.objects.order_by("position").prefetch_related("group")
queryset = GroupContact.objects.prefetch_related("persons_in_charge__profile", "group__user_set__profile").order_by(
"position"
)
template_name = "pages/contact.html"
context_object_name = "groups"

Expand Down
2 changes: 1 addition & 1 deletion zds/tutorialv2/views/goals.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class ContentsByGoalMixin:
def get_queryset(self):
self.current_filter_pk = None

self.base_queryset = PublishableContent.objects.exclude(public_version=None)
self.base_queryset = PublishableContent.objects.exclude(public_version=None).prefetch_related("goals")
self.num_all = self.base_queryset.count()

queryset_not_classified = self.base_queryset.filter(goals=None)
Expand Down
1 change: 1 addition & 0 deletions zds/tutorialv2/views/lists.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def get_queryset(self):
queryset.prefetch_related("content")
.prefetch_related("content__subcategory")
.prefetch_related("content__authors")
.prefetch_related("content__tags")
.select_related("content__licence")
.select_related("content__image")
.select_related("content__last_note")
Expand Down
3 changes: 3 additions & 0 deletions zds/utils/templatetags/profile.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from functools import lru_cache

from django import template
from django.contrib.auth.models import User
from django.core.cache import cache
Expand All @@ -9,6 +11,7 @@


@register.filter("profile")
@lru_cache()
def profile(current_user):
# we currently expect to receive a User in most cases, but as we move
# toward using Profiles instead, we have to handle them as well.
Expand Down