fix(form): evaluate calc question in efficient order

This sorts the graph that is formed by calculated questions and
their dependents, and then initializes them in the correct order.

caluma/caluma_form/domain_logic.py

@@ -1,3 +1,5 @@
+import itertools
+from graphlib import TopologicalSorter
 from typing import Optional
 
 from django.db import transaction
@@ -6,12 +8,13 @@
 
 from caluma.caluma_core.models import BaseModel
 from caluma.caluma_core.relay import extract_global_id
-from caluma.caluma_form import models, validators, structure, utils
-from caluma.caluma_form.utils import recalculate_answers_from_document, update_or_create_calc_answer
+from caluma.caluma_form import models, structure, utils, validators
+from caluma.caluma_form.utils import (
+    recalculate_answers_from_document,
+    update_or_create_calc_answer,
+)
 from caluma.caluma_user.models import BaseUser
 from caluma.utils import update_model
-import itertools
-
 
 
 class BaseLogic:
@@ -172,11 +175,19 @@ def create(
 
         print("creating answer", flush=True)
         if answer.question.calc_dependents:
-            print("creating answer for question", answer.question, answer.question.calc_dependents)
+            print(
+                "creating answer for question",
+                answer.question,
+                answer.question.calc_dependents,
+            )
             root_doc = answer.document.family
-            root_doc = models.Document.objects.filter(pk=answer.document.family_id).prefetch_related(
-                *utils.build_document_prefetch_statements(prefetch_options=True)
-            ).first()
+            root_doc = (
+                models.Document.objects.filter(pk=answer.document.family_id)
+                .prefetch_related(
+                    *utils.build_document_prefetch_statements(prefetch_options=True)
+                )
+                .first()
+            )
             print("init structure top level")
             struc = structure.FieldSet(root_doc, root_doc.form)
 
@@ -205,9 +216,13 @@ def update(cls, answer, validated_data, user: Optional[BaseUser] = None):
 
         if answer.question.calc_dependents:
             root_doc = answer.document.family
-            root_doc = models.Document.objects.filter(pk=answer.document.family_id).prefetch_related(
-                *utils.build_document_prefetch_statements(prefetch_options=True)
-            ).first()
+            root_doc = (
+                models.Document.objects.filter(pk=answer.document.family_id)
+                .prefetch_related(
+                    *utils.build_document_prefetch_statements(prefetch_options=True)
+                )
+                .first()
+            )
             print("init structure top level")
             struc = structure.FieldSet(root_doc, root_doc.form)
 
@@ -308,30 +323,43 @@ def create(
         document.meta.pop("_defer_calculation", None)
         document.save()
 
-        # TODO do we need really this? If yes, can we make it more efficient?
         print("domain logic: update calc answers after document has been created")
-        #for question in models.Form.get_all_questions(
-        #    [(document.family or document).form_id]
-        #).filter(type=models.Question.TYPE_CALCULATED_FLOAT):
-        #    update_or_create_calc_answer(question, document, None)
-
         root_doc = document.family
-        root_doc = models.Document.objects.filter(pk=document.family_id).prefetch_related(
-            *utils.build_document_prefetch_statements(prefetch_options=True)
-        ).first()
+        root_doc = (
+            models.Document.objects.filter(pk=document.family_id)
+            .prefetch_related(
+                *utils.build_document_prefetch_statements(prefetch_options=True)
+            )
+            .first()
+        )
         print("init structure top level")
         struc = structure.FieldSet(root_doc, root_doc.form)
 
-        dependents = document.form.questions.exclude(
-            calc_dependents=[]
-        ).values_list("calc_dependents", flat=True)
-
-        dependent_questions = list(itertools.chain(*dependents))
-        print(f"document {document.form.pk} created, update {dependent_questions}")
-
-        for question in models.Question.objects.filter(pk__in=dependent_questions):
-            print("question", question)
-            update_or_create_calc_answer(question, document, struc)
+        # Initialize all calculated questions in the form.
+        # In order to do this efficiently, we get all calculated questions with their dependents,
+        # sort them topoligically, and then update their answer.
+        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
 

caluma/caluma_form/signals.py

@@ -133,18 +133,18 @@ def update_calc_from_form_question(sender, instance, created, **kwargs):
 #     # Also skip non-referenced answers.
 #     if instance.document.family.meta.get("_defer_calculation"):
 #         return
-# 
+#
 #     if instance.question.type == models.Question.TYPE_TABLE:
 #         print("skipping update calc of table questions in event layer, because we don't have access to the question slug here")
 #         return
-# 
+#
 #     print(f"saved answer to {instance.question.pk}, recalculate dependents:")
 #     document = models.Document.objects.filter(pk=instance.document_id).prefetch_related(
 #         *build_document_prefetch_statements(
 #             "family", prefetch_options=True
 #         ),
 #     ).first()
-# 
+#
 #     for question in models.Question.objects.filter(
 #         pk__in=instance.question.calc_dependents
 #     ):

caluma/caluma_form/utils.py

@@ -4,6 +4,7 @@
 from time import time
 import inspect
 
+
 def build_document_prefetch_statements(prefix="", prefetch_options=False):
     """Build needed prefetch statements to performantly fetch a document.
 
@@ -74,7 +75,6 @@ def build_document_prefetch_statements(prefix="", prefetch_options=False):
     ]
 
 
-
 def update_calc_dependents(slug, old_expr, new_expr):
     jexl = QuestionJexl()
     old_q = set(
@@ -101,16 +101,17 @@ def update_calc_dependents(slug, old_expr, new_expr):
         question.save()
 
 
-def update_or_create_calc_answer(question, document, struc):
-    print("callsite", inspect.stack()[1][3], flush=True)
+def update_or_create_calc_answer(question, document, struc, update_dependents=True):
+    # print("callsite", inspect.stack()[1][3], flush=True)
 
     root_doc = document.family
 
     if not struc:
         print("init structure")
         struc = structure.FieldSet(root_doc, root_doc.form)
     else:
-        print("reusing struc")
+        # print("reusing struc")
+        pass
     start = time()
     field = struc.get_field(question.slug)
     # print(f"get_field: ", time() - start)
@@ -134,12 +135,15 @@ def update_or_create_calc_answer(question, document, struc):
         question=question, document=field.document, defaults={"value": value}
     )
 
-    for _question in models.Question.objects.filter(
-        pk__in=field.question.calc_dependents
-    ):
-        print(f"{question.pk} -> {_question.pk}")
-        update_or_create_calc_answer(_question, document, struc)
-
+    if update_dependents:
+        print(
+            f"{question.pk}: updating {len(field.question.calc_dependents)} calc dependents)"
+        )
+        for _question in models.Question.objects.filter(
+            pk__in=field.question.calc_dependents
+        ):
+            # print(f"{question.pk} -> {_question.pk}")
+            update_or_create_calc_answer(_question, document, struc)
 
 
 def recalculate_answers_from_document(instance):
@@ -152,5 +156,3 @@ def recalculate_answers_from_document(instance):
         [(instance.family or instance).form_id]
     ).filter(type=models.Question.TYPE_CALCULATED_FLOAT):
         update_or_create_calc_answer(question, instance)
-
-