fix(form): optimize calculated question performance

caluma/caluma_form/domain_logic.py

@@ -1,3 +1,4 @@
+from graphlib import TopologicalSorter
 from typing import Optional
 
 from django.db import transaction
@@ -6,8 +7,8 @@
 
 from caluma.caluma_core.models import BaseModel
 from caluma.caluma_core.relay import extract_global_id
-from caluma.caluma_form import models, validators
-from caluma.caluma_form.utils import recalculate_answers_from_document
+from caluma.caluma_form import models, structure, utils, validators
+from caluma.caluma_form.utils import update_or_create_calc_answer
 from caluma.caluma_user.models import BaseUser
 from caluma.utils import update_model
 
@@ -151,6 +152,19 @@ def post_save(answer: models.Answer) -> models.Answer:
         # TODO emit events
         return answer
 
+    @staticmethod
+    def update_calc_dependents(answer):
+        if not answer.question.calc_dependents:
+            return
+
+        root_doc = utils.prefetch_document(answer.document.family_id)
+        struc = structure.FieldSet(root_doc, root_doc.form)
+
+        for question in models.Question.objects.filter(
+            pk__in=answer.question.calc_dependents
+        ):
+            update_or_create_calc_answer(question, root_doc, struc)
+
     @classmethod
     @transaction.atomic
     def create(
@@ -168,6 +182,8 @@ def create(
         if answer.question.type == models.Question.TYPE_TABLE:
             answer.create_answer_documents(documents)
 
+        cls.update_calc_dependents(answer)
+
         return answer
 
     @classmethod
@@ -185,6 +201,8 @@ def update(cls, answer, validated_data, user: Optional[BaseUser] = None):
         if answer.question.type == models.Question.TYPE_TABLE:
             answer.create_answer_documents(documents)
 
+        cls.update_calc_dependents(answer)
+
         answer.refresh_from_db()
         return answer
 
@@ -276,7 +294,44 @@ def create(
         document.meta.pop("_defer_calculation", None)
         document.save()
 
-        recalculate_answers_from_document(document)
+        SaveDocumentLogic._initialize_calculated_answers(document)
+
+        return document
+
+    @staticmethod
+    def _initialize_calculated_answers(document):
+        """
+        Initialize all calculated questions in the document.
+
+        In order to do this efficiently, we get all calculated questions with their dependents,
+        sort them topoligically, and then update their answer.
+        """
+        root_doc = utils.prefetch_document(document.family_id)
+        struc = structure.FieldSet(root_doc, root_doc.form)
+
+        calculated_questions = (
+            models.Form.get_all_questions([(document.family or document).form_id])
+            .filter(type=models.Question.TYPE_CALCULATED_FLOAT)
+            .values("slug", "calc_dependents")
+        )
+        adjacency_list = {
+            dep["slug"]: dep["calc_dependents"] for dep in calculated_questions
+        }
+        ts = TopologicalSorter(adjacency_list)
+        # TopologicalSorter expects the adjacency_list the "other way around", i.e.
+        # for every node the incoming nodes should be given. To account for this, we
+        # just reverse the resulting order.
+        sorted_question_slugs = list(reversed(list(ts.static_order())))
+
+        # fetch all related questions in one query, but iterate according
+        # to pre-established sorting
+        _questions = models.Question.objects.in_bulk(sorted_question_slugs)
+        for slug in sorted_question_slugs:
+            print("question", slug)
+            update_or_create_calc_answer(
+                _questions[slug], document, struc, update_dependents=False
+            )
+
         return document
 
     @staticmethod

caluma/caluma_form/models.py

@@ -396,9 +396,9 @@ def set_family(self, root_doc):
 
     def copy(self, family=None, user=None):
         """Create a copy including all its answers."""
-        from caluma.caluma_form.utils import recalculate_answers_from_document
 
-        # defer calculated questions, as many unecessary recomputations will happen otherwise
+        # no need to update calculated questions, since calculated answers
+        # are copied as well
         meta = dict(self.meta)
         meta["_defer_calculation"] = True
 
@@ -423,7 +423,6 @@ def copy(self, family=None, user=None):
 
         new_document.meta.pop("_defer_calculation", None)
         new_document.save()
-        recalculate_answers_from_document(new_document)
 
         return new_document
 

caluma/caluma_form/signals.py

@@ -1,7 +1,4 @@
-import itertools
-
 from django.db.models.signals import (
-    m2m_changed,
     post_delete,
     post_init,
     post_save,
@@ -15,11 +12,7 @@
 from caluma.utils import disable_raw
 
 from . import models
-from .utils import (
-    recalculate_answers_from_document,
-    update_calc_dependents,
-    update_or_create_calc_answer,
-)
+from .utils import update_calc_dependents, update_or_create_calc_answer
 
 
 @receiver(pre_create_historical_record, sender=models.HistoricalAnswer)
@@ -78,13 +71,17 @@ def save_calc_dependents(sender, instance, **kwargs):
         update_calc_dependents(
             instance.slug, old_expr="false", new_expr=instance.calc_expression
         )
+        instance.calc_expression_changed = True
 
     elif original.calc_expression != instance.calc_expression:
         update_calc_dependents(
             instance.slug,
             old_expr=original.calc_expression,
             new_expr=instance.calc_expression,
         )
+        instance.calc_expression_changed = True
+    else:
+        instance.calc_expression_changed = False
 
 
 @receiver(pre_delete, sender=models.Question)
@@ -104,10 +101,8 @@ def remove_calc_dependents(sender, instance, **kwargs):
 @receiver(post_save, sender=models.Question)
 @disable_raw
 @filter_events(lambda instance: instance.type == models.Question.TYPE_CALCULATED_FLOAT)
+@filter_events(lambda instance: getattr(instance, "calc_expression_changed", False))
 def update_calc_from_question(sender, instance, created, update_fields, **kwargs):
-    # TODO optimize: only update answers if calc_expression is updated
-    # needs to happen during save() or __init__()
-
     for document in models.Document.objects.filter(form__questions=instance):
         update_or_create_calc_answer(instance, document)
 
@@ -120,40 +115,3 @@ def update_calc_from_question(sender, instance, created, update_fields, **kwargs
 def update_calc_from_form_question(sender, instance, created, **kwargs):
     for document in instance.form.documents.all():
         update_or_create_calc_answer(instance.question, document)
-
-
-@receiver(post_save, sender=models.Answer)
-@disable_raw
-@filter_events(lambda instance: instance.document and instance.question.calc_dependents)
-def update_calc_from_answer(sender, instance, **kwargs):
-    # If there is no document on the answer it means that it's a default
-    # answer. They shouldn't trigger a recalculation of a calculated field
-    # even when they are technically listed as a dependency.
-    # Also skip non-referenced answers.
-    if instance.document.family.meta.get("_defer_calculation"):
-        return
-
-    for question in models.Question.objects.filter(
-        pk__in=instance.question.calc_dependents
-    ):
-        update_or_create_calc_answer(question, instance.document)
-
-
-@receiver(post_save, sender=models.Document)
-@disable_raw
-# We're only interested in table row forms
-@filter_events(lambda instance, created: instance.pk != instance.family_id or created)
-def update_calc_from_document(sender, instance, created, **kwargs):
-    recalculate_answers_from_document(instance)
-
-
-@receiver(m2m_changed, sender=models.AnswerDocument)
-def update_calc_from_answerdocument(sender, instance, **kwargs):
-    dependents = instance.document.form.questions.exclude(
-        calc_dependents=[]
-    ).values_list("calc_dependents", flat=True)
-
-    dependent_questions = list(itertools.chain(*dependents))
-
-    for question in models.Question.objects.filter(pk__in=dependent_questions):
-        update_or_create_calc_answer(question, instance.document)

caluma/caluma_form/structure.py

@@ -254,6 +254,11 @@ def children(self):
             for question in self.form.questions.all()
         ]
 
+    def set_answer(self, question_slug, answer):
+        field = self.get_field(question_slug)
+        if field:
+            field.answer = answer
+
     def __repr__(self):
         q_slug = self.question.slug if self.question else None
         if q_slug: