From 97a39b0652477581671d4bdfd51ca5bd3296714e Mon Sep 17 00:00:00 2001 From: Skia Date: Sat, 20 May 2017 12:47:48 +0200 Subject: [PATCH] Make **a lot** of Forum improvements in reducing the number of queries per page --- forum/admin.py | 1 + forum/migrations/0004_auto_20170520_1235.py | 57 +++++++++ forum/models.py | 128 ++++++++++++++------ forum/urls.py | 1 + forum/views.py | 21 +++- 5 files changed, 170 insertions(+), 38 deletions(-) create mode 100644 forum/migrations/0004_auto_20170520_1235.py diff --git a/forum/admin.py b/forum/admin.py index 5db34b10..148e47ce 100644 --- a/forum/admin.py +++ b/forum/admin.py @@ -29,3 +29,4 @@ from forum.models import * admin.site.register(Forum) admin.site.register(ForumTopic) admin.site.register(ForumMessage) +admin.site.register(ForumUserInfo) diff --git a/forum/migrations/0004_auto_20170520_1235.py b/forum/migrations/0004_auto_20170520_1235.py new file mode 100644 index 00000000..9dcb408d --- /dev/null +++ b/forum/migrations/0004_auto_20170520_1235.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('forum', '0003_auto_20170510_1754'), + ] + + operations = [ + migrations.AlterModelOptions( + name='forummessage', + options={'ordering': ['-date']}, + ), + migrations.AlterModelOptions( + name='forumtopic', + options={'ordering': ['-_last_message__date']}, + ), + migrations.AddField( + model_name='forum', + name='_last_message', + field=models.ForeignKey(related_name='forums_where_its_last', to='forum.ForumMessage', null=True, verbose_name='the last message'), + ), + migrations.AddField( + model_name='forum', + name='_topic_number', + field=models.IntegerField(default=0, verbose_name='number of topics'), + ), + migrations.AddField( + model_name='forummessage', + name='_deleted', + field=models.BooleanField(default=False, verbose_name='is deleted'), + ), + migrations.AddField( + model_name='forumtopic', + name='_last_message', + field=models.ForeignKey(related_name='+', to='forum.ForumMessage', null=True, verbose_name='the last message'), + ), + migrations.AddField( + model_name='forumtopic', + name='_title', + field=models.CharField(max_length=64, blank=True, verbose_name='title'), + ), + migrations.AlterField( + model_name='forum', + name='description', + field=models.CharField(max_length=512, default='', verbose_name='description'), + ), + migrations.AlterField( + model_name='forum', + name='id', + field=models.AutoField(primary_key=True, serialize=False, db_index=True), + ), + ] diff --git a/forum/models.py b/forum/models.py index 234b477d..dad33469 100644 --- a/forum/models.py +++ b/forum/models.py @@ -46,8 +46,9 @@ class Forum(models.Model): edit_groups allows to put any group as a forum admin view_groups allows some groups to view a forum """ + id = models.AutoField(primary_key=True, db_index=True) name = models.CharField(_('name'), max_length=64) - description = models.CharField(_('description'), max_length=256, default="") + description = models.CharField(_('description'), max_length=512, default="") is_category = models.BooleanField(_('is a category'), default=False) parent = models.ForeignKey('Forum', related_name='children', null=True, blank=True) owner_club = models.ForeignKey(Club, related_name="owned_forums", verbose_name=_("owner club"), @@ -57,6 +58,8 @@ class Forum(models.Model): view_groups = models.ManyToManyField(Group, related_name="viewable_forums", blank=True, default=[settings.SITH_GROUP_PUBLIC_ID]) number = models.IntegerField(_("number to choose a specific forum ordering"), default=1) + _last_message = models.ForeignKey('ForumMessage', related_name="forums_where_its_last", verbose_name=_("the last message"), null=True) + _topic_number = models.IntegerField(_("number of topics"), default=0) class Meta: ordering = ['number'] @@ -72,6 +75,28 @@ class Forum(models.Model): if copy_rights: self.copy_rights() + def set_topic_number(self): + self._topic_number = self.get_topic_number() + self.save() + if self.parent: + self.parent.set_topic_number() + + def set_last_message(self): + topic = ForumTopic.objects.filter(forum__id=self.id).exclude(_last_message=None).order_by('-_last_message__id').first() + forum = Forum.objects.filter(parent__id=self.id).exclude(_last_message=None).order_by('-_last_message__id').first() + if topic and forum: + if topic._last_message_id < forum._last_message_id: + self._last_message_id = forum._last_message_id + else: + self._last_message_id = topic._last_message_id + elif topic: + self._last_message_id = topic._last_message_id + elif forum: + self._last_message_id = forum._last_message_id + self.save() + if self.parent: + self.parent.set_last_message() + def apply_rights_recursively(self): children = self.children.all() for c in children: @@ -86,10 +111,19 @@ class Forum(models.Model): self.view_groups = self.parent.view_groups.all() self.save() + _club_memberships = {} # This cache is particularly efficient: + # divided by 3 the number of requests on the main forum page + # after the first load def is_owned_by(self, user): if user.is_in_group(settings.SITH_GROUP_FORUM_ADMIN_ID): return True - m = self.owner_club.get_membership_for(user) + try: m = Forum._club_memberships[self.id][user.id] + except: + m = self.owner_club.get_membership_for(user) + try: Forum._club_memberships[self.id][user.id] = m + except: + Forum._club_memberships[self.id] = {} + Forum._club_memberships[self.id][user.id] = m if m: return m.role > settings.SITH_MAXIMUM_FREE_ROLE return False @@ -122,9 +156,9 @@ class Forum(models.Model): p = p.parent return l - @cached_property + @property def topic_number(self): - return self.get_topic_number() + return self._topic_number def get_topic_number(self): number = self.topics.all().count() @@ -134,31 +168,29 @@ class Forum(models.Model): @cached_property def last_message(self): - return self.get_last_message() + return self._last_message - forum_list = {} # Class variable used for cache purpose - def get_last_message(self): - last_msg = None - for m in ForumMessage.objects.select_related('topic__forum').order_by('-id'): - if m.topic.forum.id in Forum.forum_list.keys(): # The object is already in Python's memory, - # so there's no need to query it again - forum = Forum.forum_list[m.topic.forum.id] - else: # Query the forum object and store it in the class variable for further use. - # Keeping the same object allows the @cached_property to work properly. -# This trick divided by 4 the number of DB queries in the main forum page, and about the same on many other forum pages. -# This also divided by 4 the amount of CPU usage for thoses pages, according to Django Debug Toolbar. - forum = m.topic.forum - Forum.forum_list[forum.id] = forum - if self in (forum.parent_list + [forum]) and not m.deleted: - return m + def get_children_list(self): + l = [self.id] + for c in self.children.all(): + l.append(c.id) + l += c.get_children_list() + return l class ForumTopic(models.Model): forum = models.ForeignKey(Forum, related_name='topics') author = models.ForeignKey(User, related_name='forum_topics') description = models.CharField(_('description'), max_length=256, default="") + _last_message = models.ForeignKey('ForumMessage', related_name="+", verbose_name=_("the last message"), null=True) + _title = models.CharField(_('title'), max_length=64, blank=True) class Meta: - ordering = ['-id'] # TODO: add date message ordering + ordering = ['-_last_message__date'] + + def save(self, *args, **kwargs): + super(ForumTopic, self).save(*args, **kwargs) + self.forum.set_topic_number() # Recompute the cached value + self.forum.set_last_message() def is_owned_by(self, user): return self.forum.is_owned_by(user) @@ -184,13 +216,16 @@ class ForumTopic(models.Model): @cached_property def last_message(self): - for msg in self.messages.order_by('id').select_related('author').order_by('-id').all(): - if not msg.deleted: - return msg + return self._last_message - @property + @cached_property def title(self): - return self.messages.order_by('date').first().title + if self._title: + return self._title + else: + self._title = self.messages.order_by('id').first().title + self.save() + return self._title class ForumMessage(models.Model): """ @@ -202,12 +237,29 @@ class ForumMessage(models.Model): message = models.TextField(_("message"), default="") date = models.DateTimeField(_('date'), default=timezone.now) readers = models.ManyToManyField(User, related_name="read_messages", verbose_name=_("readers")) + _deleted = models.BooleanField(_('is deleted'), default=False) class Meta: - ordering = ['id'] + ordering = ['-date'] def __str__(self): - return "%s - %s" % (self.id, self.title) + return "%s (%s) - %s" % (self.id, self.author, self.title) + + def save(self, *args, **kwargs): + self._deleted = self.is_deleted() # Recompute the cached value + super(ForumMessage, self).save(*args, **kwargs) + if self.is_last_in_topic(): + self.topic._last_message_id = self.id + self.topic.save() + if self.is_first_in_topic(): + self.topic._title = self.title + self.topic.save() + + def is_first_in_topic(self): + return bool(self.id == self.topic.messages.order_by('date').first().id) + + def is_last_in_topic(self): + return bool(self.id == self.topic.messages.order_by('date').last().id) def is_owned_by(self, user): # Anyone can create a topic: it's better to # check the rights at the forum level, since it's more controlled @@ -217,12 +269,15 @@ class ForumMessage(models.Model): return user.can_edit(self.topic.forum) def can_be_viewed_by(self, user): - return (not self.deleted and user.can_view(self.topic)) + return not self._deleted # No need to check the real rights since it's already done by the Topic view def can_be_moderated_by(self, user): return self.topic.forum.is_owned_by(user) or user.id == self.author.id def get_absolute_url(self): + return reverse('forum:view_message', kwargs={'message_id': self.id}) + + def get_url(self): return self.topic.get_absolute_url() + "?page=" + str(self.get_page()) + "#msg_" + str(self.id) def get_page(self): @@ -236,10 +291,6 @@ class ForumMessage(models.Model): def is_read(self, user): return (self.date < user.forum_infos.last_read_date) or (user in self.readers.all()) - @cached_property - def deleted(self): - return self.is_deleted() - def is_deleted(self): meta = self.metas.exclude(action="EDIT").order_by('-date').first() if meta: @@ -258,13 +309,22 @@ class ForumMessageMeta(models.Model): date = models.DateTimeField(_('date'), default=timezone.now) action = models.CharField(_("action"), choices=MESSAGE_META_ACTIONS, max_length=16) + def save(self, *args, **kwargs): + super(ForumMessageMeta, self).save(*args, **kwargs) + self.message._deleted = self.message.is_deleted() + self.message.save() + + class ForumUserInfo(models.Model): """ This currently stores only the last date a user clicked "Mark all as read". However, this can be extended with lot of user preferences dedicated to a user, such as the favourite topics, the signature, and so on... """ - user = models.OneToOneField(User, related_name="_forum_infos") # TODO: see to move that to the User class in order to reduce the number of db queries + user = models.OneToOneField(User, related_name="_forum_infos") last_read_date = models.DateTimeField(_('last read date'), default=datetime(year=settings.SITH_SCHOOL_START_YEAR, month=1, day=1, tzinfo=pytz.UTC)) + def __str__(self): + return str(self.user) + diff --git a/forum/urls.py b/forum/urls.py index 7fabc688..9096e3cb 100644 --- a/forum/urls.py +++ b/forum/urls.py @@ -38,6 +38,7 @@ urlpatterns = [ url(r'^topic/(?P[0-9]+)$', ForumTopicDetailView.as_view(), name='view_topic'), url(r'^topic/(?P[0-9]+)/edit$', ForumTopicEditView.as_view(), name='edit_topic'), url(r'^topic/(?P[0-9]+)/new_message$', ForumMessageCreateView.as_view(), name='new_message'), + url(r'^message/(?P[0-9]+)$', ForumMessageView.as_view(), name='view_message'), url(r'^message/(?P[0-9]+)/edit$', ForumMessageEditView.as_view(), name='edit_message'), url(r'^message/(?P[0-9]+)/delete$', ForumMessageDeleteView.as_view(), name='delete_message'), url(r'^message/(?P[0-9]+)/undelete$', ForumMessageUndeleteView.as_view(), name='undelete_message'), diff --git a/forum/views.py b/forum/views.py index 87005b87..157ab624 100644 --- a/forum/views.py +++ b/forum/views.py @@ -41,7 +41,7 @@ from core.views import CanViewMixin, CanEditMixin, CanEditPropMixin, CanCreateMi from forum.models import Forum, ForumMessage, ForumTopic, ForumMessageMeta class ForumMainView(ListView): - queryset = Forum.objects.filter(parent=None) + queryset = Forum.objects.filter(parent=None).select_related("_last_message__author", "_last_message__topic___title") template_name = "forum/main.jinja" class ForumMarkAllAsRead(RedirectView): @@ -53,6 +53,8 @@ class ForumMarkAllAsRead(RedirectView): fi = request.user.forum_infos fi.last_read_date = timezone.now() fi.save() + for m in request.user.read_messages.filter(date__lt=fi.last_read_date): + m.readers.remove(request.user) # Clean up to keep table low in data except: pass return super(ForumMarkAllAsRead, self).get(request, *args, **kwargs) @@ -62,9 +64,11 @@ class ForumLastUnread(ListView): paginate_by = settings.SITH_FORUM_PAGE_LENGTH / 2 def get_queryset(self): - l = ForumMessage.objects.exclude(readers=self.request.user).filter( - date__gt=self.request.user.forum_infos.last_read_date).values_list('topic') # TODO try to do better - return self.model.objects.filter(id__in=l).annotate(models.Max('messages__date')).order_by('-messages__date__max').select_related('author') + topic_list = self.model.objects.filter(_last_message__date__gt=self.request.user.forum_infos.last_read_date)\ + .exclude(_last_message__readers=self.request.user)\ + .order_by('-_last_message__date')\ + .select_related('_last_message__author__avatar_pict') + return topic_list class ForumForm(forms.ModelForm): class Meta: @@ -185,6 +189,15 @@ class ForumTopicDetailView(CanViewMixin, DetailView): kwargs["msgs"] = paginator.page(paginator.num_pages) return kwargs +class ForumMessageView(SingleObjectMixin, RedirectView): + model = ForumMessage + pk_url_kwarg = "message_id" + permanent = False + + def get_redirect_url(self, *args, **kwargs): + self.object = self.get_object() + return self.object.get_url() + class ForumMessageEditView(CanEditMixin, UpdateView): model = ForumMessage fields = ['title', 'message']