Merge pull request #969 from skulonen/database-optimization

Database performance optimizations
(mainly by using select_related and prefetch_related)

course/api/full_serializers.py

@@ -46,10 +46,13 @@ class Meta(AplusModelSerializer.Meta):
             'exercises',
         )
 
-    def get_exercises(self, obj):
+    def get_exercises(self, obj: CourseModule) -> OrderedDict:
         # List only exercises derived from BaseExercise, thus no Chapter texts (/exercise/<id> returns 404 for those)
-        exercises = BaseExercise.objects.filter(course_module=obj).all()
-        # FIXME: populating learning_object.parent_list() creates subqueries -> get exercise list and data from cache
+        # Check if the exercises were prefetched in the view
+        if hasattr(obj, 'exercises'):
+            exercises = obj.exercises
+        else:
+            exercises = BaseExercise.objects.filter(course_module=obj).all()
         serializer = ExerciseBriefSerializer(instance=exercises, many=True, context=self.context)
         return serializer.data
 

course/api/views.py

@@ -6,9 +6,10 @@
 from rest_framework.reverse import reverse
 from rest_framework_extensions.mixins import NestedViewSetMixin
 from rest_framework.permissions import IsAdminUser
-from django.db.models import Q
+from django.db.models import Prefetch, Q, QuerySet
 from django.utils.text import format_lazy
 from django.utils.translation import gettext_lazy as _
+from exercise.exercise_models import BaseExercise
 from lib.email_messages import email_course_instance
 
 from lib.viewbase import BaseMixin
@@ -195,10 +196,15 @@ class CourseExercisesViewSet(NestedViewSetMixin,
     parent_lookup_map = {'course_id': 'course_instance.id'}
     serializer_class = CourseModuleSerializer
 
-    def get_queryset(self):
-        return ( CourseModule.objects
-                 .get_visible(self.request.user)
-                 .all() )
+    def get_queryset(self) -> QuerySet[CourseModule]:
+        return (
+            CourseModule.objects
+            .get_visible(self.request.user)
+            .prefetch_related(
+                Prefetch('learning_objects', BaseExercise.objects.all(), 'exercises')
+            )
+            .all()
+        )
 
     def get_object(self):
         return self.get_member_object('module', 'Exercise module')
@@ -423,13 +429,17 @@ class CourseUsertaggingsViewSet(NestedViewSetMixin,
     lookup_field = 'id'
     lookup_url_kwarg = 'usertag_id'
     serializer_class = CourseUsertaggingsSerializer
-    queryset = ( UserTagging.objects
-                 .select_related('tag', 'user', 'user__user')
-                 .only('tag__id', 'tag__course_instance', 'tag__name', 'tag__slug',
-                       'user__user__id', 'user__user__email', 'user__user__username', 'user__student_id',
-                       'course_instance__id')
-                 .order_by('user__user__id')
-                 .all() )
+    queryset = (
+        UserTagging.objects
+        .select_related('tag', 'user', 'user__user')
+        .only('tag__id', 'tag__course_instance', 'tag__name', 'tag__slug',
+            'user__user__id', 'user__user__email', 'user__user__username', 'user__student_id',
+            'user__user__first_name', 'user__user__last_name', 'user__organization',
+            'course_instance__id'
+        )
+        .order_by('user__user__id')
+        .all()
+    )
     parent_lookup_map = {'course_id': 'course_instance_id'}
 
     def get_serializer_context(self):
@@ -494,10 +504,13 @@ class CourseOwnStudentGroupsViewSet(NestedViewSetMixin,
     serializer_class = CourseStudentGroupBriefSerializer
     parent_lookup_map = {'course_id': 'course_instance.id'}
 
-    def get_queryset(self):
-        return StudentGroup.objects.filter(
-            course_instance=self.instance,
-            members=self.request.user.userprofile,
+    def get_queryset(self) -> QuerySet[StudentGroup]:
+        return (
+            StudentGroup.objects.filter(
+                course_instance=self.instance,
+                members=self.request.user.userprofile,
+            )
+            .select_related('course_instance')
         )
 
 
@@ -521,7 +534,10 @@ class CourseStudentGroupsViewSet(NestedViewSetMixin,
         OnlyCourseTeacherPermission,
     ]
     serializer_class = CourseStudentGroupSerializer
-    queryset = StudentGroup.objects.all()
+    queryset = (
+        StudentGroup.objects
+        .select_related('course_instance')
+    )
     parent_lookup_map = {'course_id': 'course_instance.id'}
 
 class CourseNewsViewSet(NestedViewSetMixin,
@@ -545,10 +561,10 @@ class CourseNewsViewSet(NestedViewSetMixin,
     serializer_class = CourseNewsSerializer
     parent_lookup_map = {'course_id': 'course_instance.id'}
 
-    def get_queryset(self):
+    def get_queryset(self) -> QuerySet[News]:
         user = self.request.user
         AUDIENCE = CourseInstance.ENROLLMENT_AUDIENCE
-        queryset = News.objects.all()
+        queryset = News.objects.select_related('course_instance')
         if not user.is_superuser and not self.instance.is_course_staff(user):
             if user.userprofile.is_external:
                 return queryset.filter(

course/models.py

@@ -2,7 +2,7 @@
 import json
 import logging
 import string
-from typing import Any, Dict
+from typing import Any, Dict, List
 import urllib.request, urllib.parse
 from random import randint, choice
 
@@ -103,6 +103,11 @@ def get_url_kwargs(self):
         return dict(course_slug=self.url)
 
 
+class StudentGroupManager(models.Manager):
+    def get_queryset(self) -> models.QuerySet['StudentGroup']:
+        return super().get_queryset().prefetch_related('members')
+
+
 class StudentGroup(models.Model):
     """
     Stores a user group for a course instance.
@@ -121,6 +126,8 @@ class StudentGroup(models.Model):
         auto_now_add=True,
     )
 
+    objects = StudentGroupManager()
+
     class Meta:
         verbose_name = _('MODEL_NAME_STUDENT_GROUP')
         verbose_name_plural = _('MODEL_NAME_STUDENT_GROUP_PLURAL')
@@ -354,12 +361,20 @@ def tags_for_instance(self, course_instance):
             .select_related('tag')
         return [t.tag for t in ts]
 
-    def get_all(self, profile, course_instance):
-        qs = (self.filter(user=profile,
-                          course_instance=course_instance)
-              .select_related('tag'))
+    def get_all(self, profile: UserProfile, course_instance: 'CourseInstance') -> List[UserTag]:
+        # Check if taggings for the course instance were prefetched
+        if hasattr(profile, 'instance_taggings'):
+            taggings = profile.instance_taggings
+        else:
+            taggings = (
+                self.filter(
+                    user=profile,
+                    course_instance=course_instance
+                )
+                .select_related('tag')
+            )
         tags = [USERTAG_EXTERNAL if profile.is_external else USERTAG_INTERNAL]
-        tags.extend(t.tag for t in qs.all())
+        tags.extend(t.tag for t in taggings)
         return tags
 
     def set(self, profile, tag):

course/staff_views.py

@@ -1,5 +1,8 @@
 import json
+from typing import Any, Dict, List
+
 from django.contrib import messages
+from django.db import models
 from django.shortcuts import get_object_or_404
 from django.urls import reverse
 from django.utils.text import format_lazy
@@ -9,6 +12,7 @@
 from authorization.permissions import ACCESS
 from lib.helpers import settings_text
 from lib.viewbase import BaseFormView, BaseTemplateView, BaseRedirectMixin
+from userprofile.models import UserProfile
 from .cache.students import CachedStudent
 from .forms import EnrollStudentsForm, GroupEditForm
 from .models import (
@@ -43,7 +47,7 @@ def get_common_objects(self):
             'internal_user_label', 'external_user_label',
         )
 
-    def _get_students_with_tags(self):
+    def _get_students_with_tags(self) -> List[Dict[str, Any]]:
         ci = self.instance
         fake = '10' * 32
         link = (reverse('user-results',
@@ -56,11 +60,11 @@ def _get_students_with_tags(self):
             for k in Enrollment.ENROLLMENT_STATUS.keys()
         }
 
-        participants = ci.all_students.all()
+        participants = ci.all_students.prefetch_tags(ci)
         data = []
         for participant in participants:
             user_id = participant.user.id
-            user_tags = CachedStudent(ci, user_id).data
+            user_tags = CachedStudent(ci, participant.user).data
             user_tags_html = ' '.join(tags[slug].html_label for slug in user_tags['tag_slugs'] if slug in tags)
             data.append({
                 'id': participant.student_id or '',
@@ -82,9 +86,13 @@ class GroupsView(CourseInstanceBaseView):
     access_mode = ACCESS.ASSISTANT
     template_name = "course/staff/groups.html"
 
-    def get_common_objects(self):
+    def get_common_objects(self) -> None:
         super().get_common_objects()
-        self.groups = list(self.instance.groups.all())
+        self.groups = list(
+            self.instance.groups.prefetch_related(None).prefetch_related(
+                models.Prefetch('members', UserProfile.objects.prefetch_tags(self.instance)),
+            )
+        )
         self.note('groups')
 
 
@@ -128,9 +136,12 @@ class GroupsDeleteView(CourseInstanceMixin, BaseRedirectMixin, BaseTemplateView)
     group_kw = "group_id"
     template_name = "course/staff/group_delete.html"
 
-    def get_resource_objects(self):
+    def get_resource_objects(self) -> None:
         super().get_resource_objects()
-        self.group = get_object_or_404(StudentGroup,
+        self.group = get_object_or_404(
+            StudentGroup.objects.prefetch_related(None).prefetch_related(
+                models.Prefetch('members', UserProfile.objects.prefetch_tags(self.instance))
+            ),
             course_instance=self.instance,
             id=self._get_kwarg(self.group_kw),
         )

course/templates/course/_course_menu.html

@@ -118,19 +118,19 @@ <h4>{{ group.label|parse_localization }}</h4>
 {% endfor %}
 {% endfor %}
 
-{% if instance.tabs.exists %}
+{% for tab in instance.tabs.all %}
+{% if forloop.first %}
 <li class="header">
 	<h4>{% translate "APPS" %}</h4>
 </li>
-{% for tab in instance.tabs.all %}
+{% endif %}
 <li class="menu-tab{{ tab.id }}">
 	<a href="{% url 'apps-tab' course_slug=course.url instance_slug=instance.url tab_id=tab.id %}">
 		<span class="glyphicon glyphicon-th-large" aria-hidden="true"></span>
 		{{ tab.label }}
 	</a>
 </li>
 {% endfor %}
-{% endif %}
 
 {% if is_course_staff %}
 <li class="header">

course/templatetags/course.py

@@ -1,6 +1,9 @@
 from datetime import timedelta
+from typing import Any, Dict, List, Union
+
 from django import template
 from django.conf import settings
+from django.db import models
 from django.utils import timezone
 from django.utils.safestring import mark_safe
 from django.utils.translation import get_language
@@ -112,9 +115,16 @@ def avatars(profiles):
 
 
 @register.inclusion_tag("course/_profiles.html")
-def profiles(profiles, instance, is_teacher):
+def profiles(
+        profiles: Union[UserProfile, List[UserProfile], models.QuerySet[UserProfile]],
+        instance: CourseInstance,
+        is_teacher: bool
+        ) -> Dict[str, Any]:
     if isinstance(profiles, UserProfile):
         profiles = [profiles]
+    elif isinstance(profiles, models.QuerySet):
+        # Avoid re-fetching the queryset
+        profiles = list(profiles)
     return {
         'instance': instance,
         'profiles': profiles,

course/viewbase.py

@@ -140,9 +140,9 @@ def handle_exception(self, exc):
         return super().handle_exception(exc)
 
 class CourseInstanceMixin(CourseInstanceBaseMixin, UserProfileMixin):
-    def get_course_instance_object(self):
+    def get_course_instance_object(self) -> CourseInstance:
         return get_object_or_404(
-            CourseInstance,
+            CourseInstance.objects.prefetch_related('tabs'),
             url=self.kwargs[self.instance_kw],
             course__url=self.kwargs[self.course_kw],
         )

deviations/viewbase.py

@@ -238,7 +238,11 @@ def get_deviation_groups(
 
     ordered_deviations = (
         all_deviations
-        .select_related()
+        .select_related(
+            'submitter', 'submitter__user',
+            'granter', 'granter__user',
+            'exercise', 'exercise__course_module',
+        )
         # parent is prefetched because there may be multiple ancestors, and
         # they are needed for building the deviation's URL.
         .prefetch_related('exercise__parent')

edit_course/templates/edit_course/edit_content.html

@@ -38,7 +38,7 @@ <h3 class="panel-title">{% translate "EXERCISE_CATEGORIES" %}</h3>
     </div>
     <table class="table table-striped table-bordered table-condensed">
         <tbody>
-            {% for category in instance.categories.all %}
+            {% for category in categories %}
             <tr>
                 <td>
                     {{ category.name|parse_localization }}
@@ -48,7 +48,7 @@ <h3 class="panel-title">{% translate "EXERCISE_CATEGORIES" %}</h3>
                         <span class="glyphicon glyphicon-edit" aria-hidden="true"></span>
                         {% translate "EDIT_CATEGORY" %}
                     </a>
-                    {% if category.learning_objects.count == 0 %}
+                    {% if category.count_lobjects == 0 %}
                     <a class="aplus-button--secondary aplus-button--xs" href="{{ category|removeurl:'category' }}" role="button">
                     	<span class="glyphicon glyphicon-remove" aria-hidden="true"></span> {% translate "REMOVE" %}
                     </a>

edit_course/views.py

@@ -3,7 +3,7 @@
 from django.contrib import messages
 from django.contrib.auth import login as auth_login
 from django.contrib.auth.models import User
-from django.db import IntegrityError
+from django.db import models, IntegrityError
 from django.http.response import Http404, HttpResponse
 from django.urls import reverse
 from django.utils.text import format_lazy, capfirst
@@ -73,21 +73,27 @@ class EditContentView(EditInstanceView):
     template_name = "edit_course/edit_content.html"
     form_class = CourseContentForm
 
-    def get_common_objects(self):
-        self.modules = list(self.instance.course_modules.all())
+    def get_common_objects(self) -> None:
+        self.modules = list(
+            self.instance.course_modules.prefetch_related(
+                models.Prefetch('learning_objects', LearningObject.objects.all()),
+            ),
+        )
         for module in self.modules:
+            learning_objects = {lobject.id: lobject.as_leaf_class() for lobject in module.learning_objects.all()}
             module.flat_objects = []
             try:
                 for entry in self.content.flat_module(module, enclosed=False):
                     if entry['type'] != 'level':
-                        try:
-                            module.flat_objects.append(
-                                LearningObject.objects.get(id=entry['id']).as_leaf_class())
-                        except LearningObject.DoesNotExist:
-                            continue
+                        learning_object = learning_objects.get(entry['id'])
+                        if learning_object:
+                            module.flat_objects.append(learning_object)
             except NoSuchContent:
                 continue
-        self.note('modules')
+        self.categories = self.instance.categories.annotate(
+            count_lobjects=models.Count('learning_objects')
+        )
+        self.note('modules', 'categories')
 
     def get_success_url(self):
         return self.instance.get_url('course-edit')