Merge pull request #973 from skulonen/database-optimization-inheritan…

…ce-2

Performance optimization (model inheritance)

* Change the default manager `objects` in ModelWithInheritance
  so that it efficiently returns instances of the subclasses.
* Add DefaultForeignKey and DefaultOneToOneField model fields
  that use the default manager and thus return instances of the subclasses.

apps/app_renderers.py

@@ -76,7 +76,6 @@ def build_plugin_renderers(plugins,
 
         renderers = []
         for p in plugins:
-            p = p.as_leaf_class()
             if hasattr(p, "get_renderer_class"):
                 renderers.append(p.get_renderer_class()(p, view_name, context))
             else:

apps/views.py

@@ -14,7 +14,7 @@ def get_resource_objects(self):
         self.tab_object = get_object_or_404(
             BaseTab,
             id=self._get_kwarg(self.tab_kw),
-        ).as_leaf_class()
+        )
         self.container = self.tab_object.container
 
         if isinstance(self.container, CourseInstance):

deviations/migrations/0005_auto_20220211_1540.py

@@ -0,0 +1,26 @@
+# Generated by Django 3.2.4 on 2022-02-11 13:40
+
+from django.db import migrations
+import django.db.models.deletion
+import lib.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('exercise', '0045_auto_20220211_1540'),
+        ('deviations', '0004_auto_20211207_1459'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='deadlineruledeviation',
+            name='exercise',
+            field=lib.fields.DefaultForeignKey(on_delete=django.db.models.deletion.CASCADE, to='exercise.baseexercise', verbose_name='LABEL_EXERCISE'),
+        ),
+        migrations.AlterField(
+            model_name='maxsubmissionsruledeviation',
+            name='exercise',
+            field=lib.fields.DefaultForeignKey(on_delete=django.db.models.deletion.CASCADE, to='exercise.baseexercise', verbose_name='LABEL_EXERCISE'),
+        ),
+    ]

deviations/models.py

@@ -8,6 +8,7 @@
 from exercise.exercise_models import BaseExercise
 from exercise.submission_models import Submission
 from userprofile.models import UserProfile
+from lib.fields import DefaultForeignKey
 from lib.models import UrlMixin
 
 
@@ -78,7 +79,7 @@ class SubmissionRuleDeviation(UrlMixin, models.Model):
     default bounds, all of the submitters must have an allowing instance of
     SubmissionRuleDeviation subclass in order for the submission to be allowed.
     """
-    exercise = models.ForeignKey(BaseExercise,
+    exercise = DefaultForeignKey(BaseExercise,
         verbose_name=_('LABEL_EXERCISE'),
         on_delete=models.CASCADE,
     )

edit_course/managers.py

@@ -62,9 +62,6 @@ class ExerciseManager(ModelManager):
     instance_field = "course_module__course_instance"
     name = _('LEARNING_OBJECT')
 
-    def get_object(self, instance, object_id):
-        obj = super().get_object(instance, object_id)
-        return obj.as_leaf_class()
 
     def new_object(self, instance, parent_id, type):
         CLASSES = {

edit_course/operations/clone.py

@@ -19,8 +19,8 @@ def clone_learning_objects(
     """
     Clones learning objects recursively.
     """
-    for lobject in list(a.as_leaf_class() for a in objects):
-        children = list(lobject.children.all())
+    for lobject in objects:
+        children = list(lobject.children.defer(None))
 
         # The user can choose to import just chapters or just exercises. If
         # this learning object is not of a requested type, skip it and reparent
@@ -153,7 +153,7 @@ def clone(
 
     if clone_modules:
         for module in modules:
-            objects = list(module.learning_objects.filter(parent__isnull=True))
+            objects = list(module.learning_objects.filter(parent__isnull=True).defer(None))
 
             # Save as new module.
             module.id = None

edit_course/operations/configure.py

@@ -138,8 +138,6 @@ def configure_learning_objects(
             course_module=module,
             url=str(o["key"])
         ).defer(None).first()
-        if not lobject is None:
-            lobject = lobject.as_leaf_class()
 
         # Select exercise class.
         lobject_cls = (
@@ -643,14 +641,13 @@ def configure_content(instance: CourseInstance, url: str) -> Tuple[bool, List[st
         for module in instance.course_modules.all():
             # cache invalidation uses the parent when learning object is saved:
             # prefetch parent so that it wont be fetched after the it was deleted
-            for lobject in module.learning_objects.all().prefetch_related('parent'):
+            for lobject in module.learning_objects.all():
                 if lobject.id not in seen_objects:
-                    exercise = lobject.as_leaf_class()
                     if (
-                        not isinstance(exercise, BaseExercise)
-                        or exercise.submissions.count() == 0
+                        not isinstance(lobject, BaseExercise)
+                        or lobject.submissions.count() == 0
                     ):
-                        exercise.delete()
+                        lobject.delete()
                     else:
                         lobject.status = LearningObject.STATUS.HIDDEN
                         lobject.order = 9999

edit_course/tests.py

@@ -83,8 +83,8 @@ def test_course_clone(self):
                 self._as_class(new_modules[i].learning_objects.all())
             )
 
-        old_exercise = old_modules[1].learning_objects.first().as_leaf_class()
-        new_exercise = new_modules[1].learning_objects.first().as_leaf_class()
+        old_exercise = old_modules[1].learning_objects.first()
+        new_exercise = new_modules[1].learning_objects.first()
         self.assertTrue(old_exercise.submissions.count() > 0)
         self.assertEqual(new_exercise.submissions.count(), 0)
 
@@ -109,7 +109,7 @@ def _as_names(self, items):
         return [a.name for a in items]
 
     def _as_class(self, items):
-        return [a.as_leaf_class().__class__ for a in items]
+        return [a.__class__ for a in items]
 
     @override_settings(SIS_PLUGIN_MODULE = 'course.sis_test')
     @override_settings(SIS_PLUGIN_CLASS = 'SisTest')

edit_course/views.py

@@ -75,12 +75,10 @@ class EditContentView(EditInstanceView):
 
     def get_common_objects(self) -> None:
         self.modules = list(
-            self.instance.course_modules.prefetch_related(
-                models.Prefetch('learning_objects', LearningObject.objects.all()),
-            ),
+            self.instance.course_modules.prefetch_related('learning_objects'),
         )
         for module in self.modules:
-            learning_objects = {lobject.id: lobject.as_leaf_class() for lobject in module.learning_objects.all()}
+            learning_objects = {lobject.id: lobject for lobject in module.learning_objects.all()}
             module.flat_objects = []
             try:
                 for entry in self.content.flat_module(module, enclosed=False):

exercise/admin.py

@@ -21,7 +21,7 @@ def real_class(obj):
     """
     Returns the leaf class name of an exercise.
     """
-    return obj.as_leaf_class().__class__.__name__
+    return obj.content_type.model_class().__name__
 
 
 def course_wrapper(obj):

exercise/api/csv/views.py

@@ -101,7 +101,7 @@ def list(
         queryset = Submission.objects.filter(
             exercise_id__in=ids,
             submitters__in=profiles
-        ).prefetch_related(Prefetch('exercise', BaseExercise.objects.all()), 'notifications', 'files')
+        ).prefetch_related('exercise', 'notifications', 'files')
         return self.serialize_submissions(request, queryset, revealed_ids, best=search_args['best'])
 
     def retrieve(
@@ -118,7 +118,7 @@ def retrieve(
         revealed_ids = get_revealed_exercise_ids(search_args, points)
         queryset = Submission.objects.filter(
             id__in=ids
-        ).prefetch_related(Prefetch('exercise', BaseExercise.objects.all()), 'notifications', 'files')
+        ).prefetch_related('exercise', 'notifications', 'files')
         return self.serialize_submissions(request, queryset, revealed_ids)
 
     def serialize_submissions(

exercise/api/mixins.py

@@ -27,7 +27,7 @@ def get_exercise_object(self):
         exercise = self.get_object_or_none(self.exercise_kw, LearningObject)
         if not exercise:
             raise Http404("Learning object not found")
-        return exercise.as_leaf_class()
+        return exercise
 
     def get_course_module_object(self):
         return self.exercise.course_module

exercise/cache/content.py

@@ -106,7 +106,7 @@ def recursion(
             'requirements__threshold__passed_exercises',
             'requirements__threshold__passed_exercises__parent',
             'requirements__threshold__points',
-            Prefetch('learning_objects', LearningObject.objects.all()),
+            'learning_objects',
         ):
             entry = {
                 'type': 'module',

exercise/exercise_models.py

@@ -26,12 +26,12 @@
 from course.models import Enrollment, StudentGroup, CourseInstance, CourseModule, LearningObjectCategory
 from external_services.lti import CustomStudentInfoLTIRequest
 from external_services.models import LTIService
-from inheritance.models import ModelWithInheritance
+from inheritance.models import ModelWithInheritance, ModelWithInheritanceManager
 from lib.api.authentication import (
     get_graderauth_submission_params,
     get_graderauth_exercise_params,
 )
-from lib.fields import JSONField
+from lib.fields import DefaultForeignKey, DefaultOneToOneField, JSONField
 from lib.helpers import (
     Enum,
     update_url_params,
@@ -56,14 +56,19 @@
     from .submission_models import Submission, SubmissionDraft
 
 
-
-class LearningObjectManager(models.Manager):
-
+class LearningObjectManager(ModelWithInheritanceManager):
     def get_queryset(self):
-        return super().get_queryset()\
-            .defer('description')\
-            .select_related('course_module', 'course_module__course_instance',
-                'course_module__course_instance__course', 'category')
+        return (
+            super().get_queryset()
+            .defer('description')
+            .select_related(
+                'course_module',
+                'course_module__course_instance',
+                'course_module__course_instance__course',
+                'category',
+            )
+            .prefetch_related('parent')
+        )
 
     def find_enrollment_exercise(self, course_instance, profile):
         exercise = None
@@ -116,7 +121,7 @@ class LearningObject(UrlMixin, ModelWithInheritance):
         on_delete=models.CASCADE,
         related_name="learning_objects",
     )
-    parent = models.ForeignKey('self',
+    parent = DefaultForeignKey('self',
         verbose_name=_('LABEL_PARENT'),
         on_delete=models.SET_NULL,
         blank=True, null=True,
@@ -405,7 +410,7 @@ class LearningObjectDisplay(models.Model):
     """
     Records views of learning objects.
     """
-    learning_object = models.ForeignKey(LearningObject,
+    learning_object = DefaultForeignKey(LearningObject,
         verbose_name=_('LABEL_LEARNING_OBJECT'),
         on_delete=models.CASCADE,
     )
@@ -432,8 +437,6 @@ class CourseChapter(LearningObject):
         default=False,
     )
 
-    objects = models.Manager()
-
     class Meta:
         verbose_name = _('MODEL_NAME_COURSE_CHAPTER')
         verbose_name_plural = _('MODEL_NAME_COURSE_CHAPTER_PLURAL')
@@ -442,15 +445,8 @@ def _is_empty(self):
         return not self.generate_table_of_contents
 
 
-class BaseExerciseManager(JWTAccessible["BaseExercise"], models.Manager['BaseExercise']):
-
-    def get_queryset(self):
-        return super().get_queryset().select_related(
-            'category',
-            'course_module',
-            'course_module__course_instance',
-            'course_module__course_instance__course',
-        )
+class BaseExerciseManager(JWTAccessible["BaseExercise"], LearningObjectManager):
+    pass
 
 
 @register_jwt_accessible_class("exercise")
@@ -515,14 +511,14 @@ class BaseExercise(LearningObject):
         max_length=32,
         blank=True,
     )
-    submission_feedback_reveal_rule = models.OneToOneField(RevealRule,
+    submission_feedback_reveal_rule = DefaultOneToOneField(RevealRule,
         verbose_name=_('LABEL_SUBMISSION_FEEDBACK_REVEAL_RULE'),
         on_delete=models.SET_NULL,
         related_name='+',
         blank=True,
         null=True,
     )
-    model_solutions_reveal_rule = models.OneToOneField(RevealRule,
+    model_solutions_reveal_rule = DefaultOneToOneField(RevealRule,
         verbose_name=_('LABEL_MODEL_SOLUTIONS_REVEAL_RULE'),
         on_delete=models.SET_NULL,
         related_name='+',
@@ -1067,7 +1063,7 @@ class LTIExercise(BaseExercise):
     """
     Exercise launched by LTI or optionally amending A+ protocol with LTI data.
     """
-    lti_service = models.ForeignKey(LTIService,
+    lti_service = DefaultForeignKey(LTIService,
         verbose_name=_('LABEL_LTI_SERVICE'),
         on_delete=models.CASCADE,
     )
@@ -1100,8 +1096,6 @@ class LTIExercise(BaseExercise):
         help_text=_('LTI_EXERCISE_OPEN_IN_IFRAME_HELPTEXT'),
     )
 
-    objects = models.Manager()
-
     class Meta:
         verbose_name = _('MODEL_NAME_LTI_EXERCISE')
         verbose_name_plural = _('MODEL_NAME_LTI_EXERCISE_PLURAL')
@@ -1226,8 +1220,6 @@ class StaticExercise(BaseExercise):
         verbose_name=_('LABEL_SUBMISSION_PAGE_CONTENT'),
     )
 
-    objects = models.Manager()
-
     class Meta:
         verbose_name = _('MODEL_NAME_STATIC_EXERCISE')
         verbose_name_plural = _('MODEL_NAME_STATICE_EXERCISE_PLURAL')
@@ -1295,8 +1287,6 @@ class ExerciseWithAttachment(BaseExercise):
         upload_to=build_upload_dir,
     )
 
-    objects = models.Manager()
-
     class Meta:
         verbose_name = _('MODEL_NAME_EXERCISE_WITH_ATTACHMENT')
         verbose_name_plural = _('MODEL_NAME_EXERCISE_WITH_ATTACHMENT_PLURAL')
@@ -1369,7 +1359,7 @@ class ExerciseTask(models.Model):
     TASK_TYPE = Enum([
         ('REGRADE', 'regrade', _('REGRADE')),
     ])
-    exercise = models.ForeignKey('BaseExercise',
+    exercise = DefaultForeignKey('BaseExercise',
         verbose_name=_('LABEL_EXERCISE'),
         on_delete=models.CASCADE,
     )

exercise/management/commands/list_exercises_services.py

@@ -67,8 +67,7 @@ def handle(self, *args, **options):
 
             service_urls = parse_localized(lo.service_url)
             if content_types[cid] == 'LTIExercise':
-                ltie = lo.as_leaf_class()
-                urls = [ltie.get_service_url(lang) for lang, url in service_urls]
+                urls = [lo.get_service_url(lang) for lang, url in service_urls]
             else:
                 urls = [url for lang, url in service_urls]
             domains = set(urlsplit(url).netloc for url in urls)