Merge pull request #1787 from He3lixxx/results-caching

Allow running refresh_results_cache during normal operation, with each cache update happening atomically

evap/evaluation/management/commands/refresh_results_cache.py

@@ -1,4 +1,3 @@
-from django.core.cache import caches
 from django.core.management.base import BaseCommand
 from django.core.serializers.base import ProgressBar
 
@@ -9,7 +8,7 @@
     STATES_WITH_RESULTS_CACHING,
     cache_results,
 )
-from evap.results.views import warm_up_template_cache
+from evap.results.views import update_template_cache
 
 
 class Command(BaseCommand):
@@ -18,9 +17,6 @@ class Command(BaseCommand):
     requires_migrations_checks = True
 
     def handle(self, *args, **options):
-        self.stdout.write("Clearing results cache...")
-        caches["results"].clear()
-
         self.stdout.write("Calculating results for all evaluations...")
 
         self.stdout.ending = None
@@ -34,7 +30,6 @@ def handle(self, *args, **options):
             cache_results(evaluation, refetch_related_objects=False)
 
         self.stdout.write("Prerendering result index page...\n")
-
-        warm_up_template_cache(Evaluation.objects.filter(state__in=STATES_WITH_RESULT_TEMPLATE_CACHING))
+        update_template_cache(Evaluation.objects.filter(state__in=STATES_WITH_RESULT_TEMPLATE_CACHING))
 
         self.stdout.write("Results cache has been refreshed.\n")

evap/results/templates/results_index_course.html

@@ -1,38 +1,8 @@
 {% load cache %}
 {% load i18n %}
-
-{% load evaluation_filters %}
 {% load results_templatetags %}
-
 {% get_current_language as LANGUAGE_CODE %}
 
 {% cache None course_result_template_fragment course.id LANGUAGE_CODE using="results" %}
-
-<div class="results-grid-row heading-row course-row">
-    <div data-col="name" data-order="{{ course.name|lower }}">
-        <div>
-            {% for degree in course.degrees.all %}
-                <span class="badge bg-primary badge-degree">{{ degree }}</span>
-            {% endfor %}
-            <span class="badge bg-secondary badge-course-type">{{ course.type }}</span>
-        </div>
-        <span class="evaluation-name">{{ course.name }}</span>
-    </div>
-    <div data-col="semester" data-order="{{ course.semester.id }}" class="text-center semester-short-name">
-        {{ course.semester.short_name }}
-    </div>
-    <div data-col="responsible" data-order="{{ course.responsibles.first.last_name|lower }}">
-        {{ course.responsibles_names }}
-    </div>
-    <div data-col="result" data-order="{{ course.avg_grade|default:7 }}">
-        {% if course.not_all_evaluations_are_published %}
-            <div class="d-flex" data-bs-toggle="tooltip" data-bs-placement="left" title="{% trans 'Course result is not yet available.' %}">
-                {% include 'distribution_with_grade_disabled.html' with icon="fas fa-hourglass" %}
-            </div>
-        {% else %}
-            {% include 'evaluation_result_widget.html' with course_or_evaluation=course %}
-        {% endif %}
-    </div>
-</div>
-
+{% include 'results_index_course_impl.html' %}
 {% endcache %}

evap/results/templates/results_index_course_impl.html

@@ -0,0 +1,29 @@
+{% load evaluation_filters %}
+{% load results_templatetags %}
+
+<div class="results-grid-row heading-row course-row">
+    <div data-col="name" data-order="{{ course.name|lower }}">
+        <div>
+            {% for degree in course.degrees.all %}
+                <span class="badge bg-primary badge-degree">{{ degree }}</span>
+            {% endfor %}
+            <span class="badge bg-secondary badge-course-type">{{ course.type }}</span>
+        </div>
+        <span class="evaluation-name">{{ course.name }}</span>
+    </div>
+    <div data-col="semester" data-order="{{ course.semester.id }}" class="text-center semester-short-name">
+        {{ course.semester.short_name }}
+    </div>
+    <div data-col="responsible" data-order="{{ course.responsibles.first.last_name|lower }}">
+        {{ course.responsibles_names }}
+    </div>
+    <div data-col="result" data-order="{{ course.avg_grade|default:7 }}">
+        {% if course.not_all_evaluations_are_published %}
+            <div class="d-flex" data-bs-toggle="tooltip" data-bs-placement="left" title="{% trans 'Course result is not yet available.' %}">
+                {% include 'distribution_with_grade_disabled.html' with icon="fas fa-hourglass" %}
+            </div>
+        {% else %}
+            {% include 'evaluation_result_widget.html' with course_or_evaluation=course %}
+        {% endif %}
+    </div>
+</div>

evap/results/templates/results_index_evaluation.html

@@ -1,78 +1,8 @@
 {% load cache %}
 {% load i18n %}
-
-{% load evaluation_filters %}
 {% load results_templatetags %}
-
 {% get_current_language as LANGUAGE_CODE %}
 
 {% cache evaluation|evaluation_results_cache_timeout evaluation_result_template_fragment evaluation.id LANGUAGE_CODE links_to_results_page using="results" %}
-
-<{% if links_to_results_page %}a href="{% url 'results:evaluation_detail' evaluation.course.semester.id evaluation.id %}"{% else %}div{% endif %}
-        class="results-grid-row {% if not is_subentry %}heading-row{% else %}evaluation-row{% endif %}
-            {% if links_to_results_page %} hover-row{% endif %}
-            {% if evaluation.State.IN_EVALUATION <= evaluation.state and evaluation.state < evaluation.State.PUBLISHED %}
-                preview-row
-            {% endif %}">
-    <div data-col="name"{% if not is_subentry %} data-order="{{ evaluation.course.name|lower }}"{% endif %}>
-        <div>
-            {% if is_subentry %}
-                {% include 'evaluation_badges.html' with mode='subentry' %}
-            {% else %}
-                {% include 'evaluation_badges.html' %}
-            {% endif %}
-        </div>
-        <span class="evaluation-name">
-            {% if is_subentry %}
-                {% if evaluation.name %}
-                    {{ evaluation.name }}
-                {% else %}
-                    {{ evaluation.course.name }}
-                {% endif %}
-            {% else %}
-                {{ evaluation.full_name }}
-            {% endif %}
-            {% if evaluation.is_single_result %} ({{ evaluation.vote_start_datetime|date }}){% endif %}
-            {% if evaluation.state == evaluation.State.IN_EVALUATION %}
-                <span data-bs-toggle="tooltip" data-bs-placement="top" class="fas fa-play icon-gray" title="{% trans 'This evaluation is still running' %}"></span>
-            {% elif evaluation.state == evaluation.State.EVALUATED %}
-                <span data-bs-toggle="tooltip" data-bs-placement="top" class="fas fa-chart-simple icon-yellow" title="{% trans 'Results not yet published' %}"></span>
-            {% elif evaluation.state == evaluation.State.REVIEWED %}
-                {% if evaluation.is_single_result or evaluation.grading_process_is_finished %}
-                    <span data-bs-toggle="tooltip" data-bs-placement="top" class="fas fa-chart-simple icon-red" title="{% trans 'Results not yet published although grading process is finished' %}"></span>
-                {% else %}
-                    <span data-bs-toggle="tooltip" data-bs-placement="top" class="fas fa-chart-simple icon-yellow" title="{% trans 'Results not yet published' %}"></span>
-                {% endif %}
-            {% endif %}
-        </span>
-    </div>
-    {% if not is_subentry %}
-        <div data-col="semester" data-order="{{ evaluation.course.semester.id }}" class="text-center semester-short-name">
-            {{ evaluation.course.semester.short_name }}
-        </div>
-    {% endif %}
-    {% if not is_subentry %}
-        <div data-col="responsible" data-order="{{ evaluation.course.responsibles.first.last_name|lower }}">
-            {{ evaluation.course.responsibles_names }}
-        </div>
-    {% endif %}
-    {% if evaluation.is_single_result %}
-        <div data-col="participants" class="text-center"><span class="fas fa-user"></span>&nbsp;{{ evaluation.single_result_rating_result.count_sum }}</div>
-    {% else %}
-        {% with num_participants=evaluation.num_participants num_voters=evaluation.num_voters %}
-            <div data-col="participants">{% include 'progress_bar.html' with done=num_voters total=num_participants %}</div>
-        {% endwith %}
-    {% endif %}
-
-    <div data-col="result"{% if not is_subentry %} data-order="{% if evaluation.is_single_result %}{{ evaluation.single_result_rating_result.average }}{% else %}{{ evaluation.avg_grade|default:7 }}{% endif %}"{% endif %}>
-        {% if not evaluation.can_staff_see_average_grade %}
-            <div class="d-flex" data-bs-toggle="tooltip" data-bs-placement="left" title="{% trans 'Evaluation result is not yet available.' %}">
-                {% include "distribution_with_grade_disabled.html" with icon="fas fa-hourglass" %}
-            </div>
-        {% else %}
-            {% include 'evaluation_result_widget.html' with course_or_evaluation=evaluation %}
-        {% endif %}
-    </div>
-</{% if links_to_results_page %}a{% else %}div{% endif %}>
-
+{% include 'results_index_evaluation_impl.html' %}
 {% endcache %}

evap/results/templates/results_index_evaluation_impl.html

@@ -0,0 +1,69 @@
+{% load evaluation_filters %}
+{% load results_templatetags %}
+
+<{% if links_to_results_page %}a href="{% url 'results:evaluation_detail' evaluation.course.semester.id evaluation.id %}"{% else %}div{% endif %}
+        class="results-grid-row {% if not is_subentry %}heading-row{% else %}evaluation-row{% endif %}
+            {% if links_to_results_page %} hover-row{% endif %}
+            {% if evaluation.State.IN_EVALUATION <= evaluation.state and evaluation.state < evaluation.State.PUBLISHED %}
+                preview-row
+            {% endif %}">
+    <div data-col="name"{% if not is_subentry %} data-order="{{ evaluation.course.name|lower }}"{% endif %}>
+        <div>
+            {% if is_subentry %}
+                {% include 'evaluation_badges.html' with mode='subentry' %}
+            {% else %}
+                {% include 'evaluation_badges.html' %}
+            {% endif %}
+        </div>
+        <span class="evaluation-name">
+            {% if is_subentry %}
+                {% if evaluation.name %}
+                    {{ evaluation.name }}
+                {% else %}
+                    {{ evaluation.course.name }}
+                {% endif %}
+            {% else %}
+                {{ evaluation.full_name }}
+            {% endif %}
+            {% if evaluation.is_single_result %} ({{ evaluation.vote_start_datetime|date }}){% endif %}
+            {% if evaluation.state == evaluation.State.IN_EVALUATION %}
+                <span data-bs-toggle="tooltip" data-bs-placement="top" class="fas fa-play icon-gray" title="{% trans 'This evaluation is still running' %}"></span>
+            {% elif evaluation.state == evaluation.State.EVALUATED %}
+                <span data-bs-toggle="tooltip" data-bs-placement="top" class="fas fa-chart-simple icon-yellow" title="{% trans 'Results not yet published' %}"></span>
+            {% elif evaluation.state == evaluation.State.REVIEWED %}
+                {% if evaluation.is_single_result or evaluation.grading_process_is_finished %}
+                    <span data-bs-toggle="tooltip" data-bs-placement="top" class="fas fa-chart-simple icon-red" title="{% trans 'Results not yet published although grading process is finished' %}"></span>
+                {% else %}
+                    <span data-bs-toggle="tooltip" data-bs-placement="top" class="fas fa-chart-simple icon-yellow" title="{% trans 'Results not yet published' %}"></span>
+                {% endif %}
+            {% endif %}
+        </span>
+    </div>
+    {% if not is_subentry %}
+        <div data-col="semester" data-order="{{ evaluation.course.semester.id }}" class="text-center semester-short-name">
+            {{ evaluation.course.semester.short_name }}
+        </div>
+    {% endif %}
+    {% if not is_subentry %}
+        <div data-col="responsible" data-order="{{ evaluation.course.responsibles.first.last_name|lower }}">
+            {{ evaluation.course.responsibles_names }}
+        </div>
+    {% endif %}
+    {% if evaluation.is_single_result %}
+        <div data-col="participants" class="text-center"><span class="fas fa-user"></span>&nbsp;{{ evaluation.single_result_rating_result.count_sum }}</div>
+    {% else %}
+        {% with num_participants=evaluation.num_participants num_voters=evaluation.num_voters %}
+            <div data-col="participants">{% include 'progress_bar.html' with done=num_voters total=num_participants %}</div>
+        {% endwith %}
+    {% endif %}
+
+    <div data-col="result"{% if not is_subentry %} data-order="{% if evaluation.is_single_result %}{{ evaluation.single_result_rating_result.average }}{% else %}{{ evaluation.avg_grade|default:7 }}{% endif %}"{% endif %}>
+        {% if not evaluation.can_staff_see_average_grade %}
+            <div class="d-flex" data-bs-toggle="tooltip" data-bs-placement="left" title="{% trans 'Evaluation result is not yet available.' %}">
+                {% include "distribution_with_grade_disabled.html" with icon="fas fa-hourglass" %}
+            </div>
+        {% else %}
+            {% include 'evaluation_result_widget.html' with course_or_evaluation=evaluation %}
+        {% endif %}
+    </div>
+</{% if links_to_results_page %}a{% else %}div{% endif %}>

evap/results/tests/test_views.py

@@ -32,7 +32,7 @@
 )
 from evap.results.exporters import TextAnswerExporter
 from evap.results.tools import cache_results
-from evap.results.views import get_evaluations_with_prefetched_data, warm_up_template_cache
+from evap.results.views import get_evaluations_with_prefetched_data, update_template_cache
 from evap.staff.tests.utils import WebTestStaffMode, helper_exit_staff_mode, run_in_staff_mode
 
 
@@ -283,7 +283,7 @@ def test_evaluation_weight_sums(self):
 
         contributions = [e.general_contribution for e in published]
         baker.make(RatingAnswerCounter, contribution=iter(contributions), answer=2, count=2, _quantity=len(published))
-        warm_up_template_cache(published)
+        update_template_cache(published)
 
         page = self.app.get(self.url, user=student)
         decoded = page.body.decode()

evap/results/views.py

@@ -59,67 +59,50 @@ def _delete_course_template_cache_impl(course):
     caches["results"].delete(get_course_result_template_fragment_cache_key(course.id, "de"))
 
 
-def warm_up_template_cache(evaluations):
+def update_template_cache(evaluations):
+    assert all(evaluation.state in STATES_WITH_RESULT_TEMPLATE_CACHING for evaluation in evaluations)
     evaluations = get_evaluations_with_course_result_attributes(get_evaluations_with_prefetched_data(evaluations))
     courses_to_render = {evaluation.course for evaluation in evaluations if evaluation.course.evaluation_count > 1}
 
     current_language = translation.get_language()
 
-    results_index_course_template = get_template("results_index_course.html", using="CachedEngine")
-    results_index_evaluation_template = get_template("results_index_evaluation.html", using="CachedEngine")
+    results_index_course_template = get_template("results_index_course_impl.html", using="CachedEngine")
+    results_index_evaluation_template = get_template("results_index_evaluation_impl.html", using="CachedEngine")
 
     try:
-        for course in courses_to_render:
-            translation.activate("en")
-            results_index_course_template.render(dict(course=course))
-
-            translation.activate("de")
-            results_index_course_template.render(dict(course=course))
-
-            assert get_course_result_template_fragment_cache_key(course.id, "en") in caches["results"]
-            assert get_course_result_template_fragment_cache_key(course.id, "de") in caches["results"]
-
-        for evaluation in evaluations:
-            assert evaluation.state in STATES_WITH_RESULT_TEMPLATE_CACHING
-            is_subentry = evaluation.course.evaluation_count > 1
-
-            translation.activate("en")
-            results_index_evaluation_template.render(
-                dict(evaluation=evaluation, links_to_results_page=True, is_subentry=is_subentry)
-            )
-            results_index_evaluation_template.render(
-                dict(evaluation=evaluation, links_to_results_page=False, is_subentry=is_subentry)
-            )
-
-            translation.activate("de")
-            results_index_evaluation_template.render(
-                dict(evaluation=evaluation, links_to_results_page=True, is_subentry=is_subentry)
-            )
-            results_index_evaluation_template.render(
-                dict(evaluation=evaluation, links_to_results_page=False, is_subentry=is_subentry)
-            )
+        for lang in ["en", "de"]:
+            translation.activate(lang)
+
+            for course in courses_to_render:
+                caches["results"].set(
+                    get_course_result_template_fragment_cache_key(course.id, lang),
+                    results_index_course_template.render(dict(course=course)),
+                )
+
+            for evaluation in evaluations:
+                assert evaluation.state in STATES_WITH_RESULT_TEMPLATE_CACHING
+                is_subentry = evaluation.course.evaluation_count > 1
+                base_args = dict(evaluation=evaluation, is_subentry=is_subentry)
+
+                caches["results"].set(
+                    get_evaluation_result_template_fragment_cache_key(evaluation.id, lang, True),
+                    results_index_evaluation_template.render(dict(**base_args, links_to_results_page=True)),
+                )
+                caches["results"].set(
+                    get_evaluation_result_template_fragment_cache_key(evaluation.id, lang, False),
+                    results_index_evaluation_template.render(dict(**base_args, links_to_results_page=False)),
+                )
 
-            assert get_evaluation_result_template_fragment_cache_key(evaluation.id, "en", True) in caches["results"]
-            assert get_evaluation_result_template_fragment_cache_key(evaluation.id, "en", False) in caches["results"]
-            assert get_evaluation_result_template_fragment_cache_key(evaluation.id, "de", True) in caches["results"]
-            assert get_evaluation_result_template_fragment_cache_key(evaluation.id, "de", False) in caches["results"]
     finally:
         translation.activate(current_language)  # reset to previously set language to prevent unwanted side effects
 
 
-def update_template_cache(evaluations):
-    for evaluation in evaluations:
-        assert evaluation.state in STATES_WITH_RESULT_TEMPLATE_CACHING
-        _delete_template_cache_impl(evaluation)
-        warm_up_template_cache([evaluation])
-
-
 def update_template_cache_of_published_evaluations_in_course(course):
-    course_evaluations = course.evaluations.filter(state__in=STATES_WITH_RESULT_TEMPLATE_CACHING)
-    for course_evaluation in course_evaluations:
-        _delete_evaluation_template_cache_impl(course_evaluation)
+    # Delete template caches for evaluations that no longer need to be cached (e.g. after unpublishing)
     _delete_course_template_cache_impl(course)
-    warm_up_template_cache(course_evaluations)
+
+    course_evaluations = course.evaluations.filter(state__in=STATES_WITH_RESULT_TEMPLATE_CACHING)
+    update_template_cache(course_evaluations)
 
 
 def get_evaluations_with_prefetched_data(evaluations):

evap/staff/tests/test_forms.py

@@ -955,13 +955,13 @@ def test_course_change_updates_cache(self):
         form = EvaluationForm(form_data, instance=evaluation, semester=semester)
         self.assertTrue(form.is_valid())
         with patch("evap.results.views._delete_course_template_cache_impl") as delete_call, patch(
-            "evap.results.views.warm_up_template_cache"
-        ) as warmup_call:
+            "evap.results.views.update_template_cache"
+        ) as update_call:
             # save without changes
             form.save()
             self.assertEqual(Evaluation.objects.get(pk=evaluation.pk).course, course1)
             self.assertEqual(delete_call.call_count, 0)
-            self.assertEqual(warmup_call.call_count, 0)
+            self.assertEqual(update_call.call_count, 0)
 
             # change course and save
             form_data = get_form_data_from_instance(EvaluationForm, evaluation)
@@ -971,7 +971,7 @@ def test_course_change_updates_cache(self):
             form.save()
             self.assertEqual(Evaluation.objects.get(pk=evaluation.pk).course, course2)
             self.assertEqual(delete_call.call_count, 2)
-            self.assertEqual(warmup_call.call_count, 2)
+            self.assertEqual(update_call.call_count, 2)
 
     def test_locked_questionnaire(self):
         """