Rework Assessment serialization to take advantage of caching.

apps/openassessment/assessment/models.py

@@ -106,11 +106,16 @@ class Rubric(models.Model):
             InvalidOptionSelection: the selected options do not match the rubric.
 
         """
-        # Create a dict of dicts that maps:
-        # criterion names --> option names --> option ids
+        # Cache based on the content_hash, not the id. It's slighlty safer, and
+        # we don't have to worry about invalidation of the cache while running
+        # tests.
         rubric_criteria_dict_cache_key = (
             "assessment.rubric_criteria_dict.{}".format(self.content_hash)
         )
+
+        # Create a dict of dicts that maps:
+        # criterion names --> option names --> option ids
+        #
         # If we've already generated one of these for this rubric, grab it from
         # the cache instead of hitting the database again.
         rubric_criteria_dict = cache.get(rubric_criteria_dict_cache_key)
@@ -345,7 +350,7 @@ class Assessment(models.Model):
         """
         scores = defaultdict(list)
         for assessment in assessments:
-            for part in assessment.parts.all():
+            for part in assessment.parts.all().select_related("option__criterion"):
                 criterion_name = part.option.criterion.name
                 scores[criterion_name].append(part.option.points)
         return scores

apps/openassessment/assessment/peer_api.py

@@ -18,8 +18,9 @@ from openassessment.assessment.models import (
     InvalidOptionSelection, PeerWorkflow, PeerWorkflowItem,
 )
 from openassessment.assessment.serializers import (
-    AssessmentSerializer, rubric_from_dict, AssessmentFeedbackSerializer,
-    full_assessment_dict)
+    AssessmentSerializer, AssessmentFeedbackSerializer,
+    rubric_from_dict, serialize_assessments
+)
 from submissions import api as sub_api
 from submissions.api import get_submission_and_student
 from submissions.models import Submission, StudentItem
@@ -259,12 +260,17 @@ def get_rubric_max_scores(submission_uuid):
             the submission, or its associated rubric.
     """
     try:
-        assessments = Assessment.objects.filter(submission_uuid=submission_uuid).order_by( "-scored_at", "-id")
-        if assessments:
-            return {
-                criterion.name: criterion.points_possible
-                for criterion in assessments[0].rubric.criteria.all()
-            }
+        serialized_assessments = serialize_assessments(
+            Assessment.objects.filter(submission_uuid=submission_uuid).order_by( "-scored_at", "-id")[:1]
+        )
+        if not serialized_assessments:
+            return None
+
+        assessment = serialized_assessments[0]
+        return {
+            criterion["name"]: criterion["points_possible"]
+            for criterion in assessment["rubric"]["criteria"]
+        }
     except Submission.DoesNotExist:
         return None
     except DatabaseError:
@@ -407,13 +413,13 @@ def get_assessments(submission_uuid, scored_only=True, limit=None):
         if scored_only:
             assessments = PeerWorkflowItem.get_scored_assessments(
                 submission_uuid
-            )
+            )[:limit]
         else:
             assessments = Assessment.objects.filter(
                 submission_uuid=submission_uuid,
                 score_type=PEER_TYPE
-            )
-        return [full_assessment_dict(assessment) for assessment in assessments[:limit]]
+            )[:limit]
+        return serialize_assessments(assessments)
     except DatabaseError:
         error_message = _(
             u"Error getting assessments for submission {}".format(submission_uuid)

apps/openassessment/assessment/self_api.py

 """
 Public interface for self-assessment.
 """
+from django.core.cache import cache
 from django.utils.translation import ugettext as _
 from submissions.api import (
     get_submission_and_student, get_submission,
     SubmissionNotFoundError, SubmissionRequestError
 )
 from openassessment.assessment.serializers import (
-    rubric_from_dict, AssessmentSerializer, full_assessment_dict, InvalidRubric
+    AssessmentSerializer, InvalidRubric, RubricSerializer,
+    rubric_from_dict, serialize_assessments
 )
 from openassessment.assessment.models import (
     Assessment, AssessmentPart, InvalidOptionSelection
@@ -121,14 +123,11 @@ def get_assessment(submission_uuid):
     # but not at the database level.  Someone could take advantage of the race condition
     # between checking the number of self-assessments and creating a new self-assessment.
     # To be safe, we retrieve just the most recent submission.
-    assessments = Assessment.objects.filter(
+    serialized_assessments = serialize_assessments(Assessment.objects.filter(
         score_type=SELF_TYPE, submission_uuid=submission_uuid
-    ).order_by('-scored_at')
+    ).order_by('-scored_at')[:1])
 
-    if assessments.exists():
-        assessment_dict = full_assessment_dict(assessments[0])
-        return assessment_dict
-    return None
+    return serialized_assessments[0] if serialized_assessments else None
 
 
 def is_complete(submission_uuid):

apps/openassessment/assessment/serializers.py

@@ -4,7 +4,9 @@ Serializers are created to ensure models do not have to be accessed outside the
 scope of the Tim APIs.
 """
 from copy import deepcopy
+import logging
 
+from django.core.cache import cache
 from django.utils.translation import ugettext as _
 from rest_framework import serializers
 from openassessment.assessment.models import (
@@ -12,6 +14,9 @@ from openassessment.assessment.models import (
     PeerWorkflowItem, PeerWorkflow)
 
 
+logger = logging.getLogger(__name__)
+
+
 class InvalidRubric(Exception):
     """This can be raised during the deserialization process."""
     def __init__(self, errors):
@@ -66,10 +71,11 @@ class CriterionOptionSerializer(NestedModelSerializer):
 class CriterionSerializer(NestedModelSerializer):
     """Serializer for :class:`Criterion`"""
     options = CriterionOptionSerializer(required=True, many=True)
+    points_possible = serializers.Field(source='points_possible')
 
     class Meta:
         model = Criterion
-        fields = ('order_num', 'name', 'prompt', 'options')
+        fields = ('order_num', 'name', 'prompt', 'options', 'points_possible')
 
     def validate_options(self, attrs, source):
         """Make sure we have at least one CriterionOption in a Criterion."""
@@ -97,6 +103,33 @@ class RubricSerializer(NestedModelSerializer):
             raise serializers.ValidationError("Must have at least one criterion")
         return attrs
 
+    @classmethod
+    def serialized_from_cache(cls, rubric, local_cache=None):
+        # Optional local cache you can send in (for when you're calling this
+        # in a loop).
+        local_cache = local_cache or {}
+
+        # Check our in-memory cache...
+        if rubric.content_hash in local_cache:
+            return local_cache[rubric.content_hash]
+
+        # Check the external cache (e.g. memcached)
+        rubric_dict_cache_key = (
+            "RubricSerializer.serialized_from_cache.{}"
+            .format(rubric.content_hash)
+        )
+        rubric_dict = cache.get(rubric_dict_cache_key)
+        if rubric_dict:
+            local_cache[rubric.content_hash] = rubric_dict
+            return rubric_dict
+
+        # Grab it from the database
+        rubric_dict = RubricSerializer(rubric).data
+        cache.set(rubric_dict_cache_key, rubric_dict)
+        local_cache[rubric.content_hash] = rubric_dict
+
+        return rubric_dict
+
 
 class AssessmentPartSerializer(serializers.ModelSerializer):
     """Serializer for :class:`AssessmentPart`."""
@@ -109,9 +142,9 @@ class AssessmentPartSerializer(serializers.ModelSerializer):
 class AssessmentSerializer(serializers.ModelSerializer):
     """Serializer for :class:`Assessment`."""
 
-    parts = AssessmentPartSerializer(many=True, read_only=True)
-    points_earned = serializers.Field(source='points_earned')
-    points_possible = serializers.Field(source='points_possible')
+    # parts = AssessmentPartSerializer(many=True, read_only=True)
+    # points_earned = serializers.Field(source='points_earned')
+    # points_possible = serializers.Field(source='points_possible')
 
     class Meta:
         model = Assessment
@@ -124,18 +157,38 @@ class AssessmentSerializer(serializers.ModelSerializer):
             'feedback',
 
             # Foreign Key
-            'parts',
+            # 'parts',
 
             # Computed, not part of the model
-            'points_earned',
-            'points_possible',
+            #'points_earned',
+            #'points_possible',
+        )
+
+
+def serialize_assessments(assessments_qset):
+    assessments = list(assessments_qset.select_related("rubric"))
+    rubric_cache = {}
+
+    return [
+        full_assessment_dict(
+            assessment,
+            RubricSerializer.serialized_from_cache(
+                assessment.rubric, rubric_cache
+            )
         )
+        for assessment in assessments
+    ]
+
 
 
-def full_assessment_dict(assessment):
+def full_assessment_dict(assessment, rubric_dict=None):
     """
-    Return a dict representation of the Assessment model,
-    including nested assessment parts.
+    Return a dict representation of the Assessment model, including nested
+    assessment parts. We do some of the serialization ourselves here instead
+    of relying on the Django REST Framework serializers. This is for performance
+    reasons -- we have a cached rubric easily available, and we don't want to
+    follow all the DB relations from assessment -> assessment part -> option ->
+    criterion.
 
     Args:
         assessment (Assessment): The Assessment model to serialize
@@ -144,17 +197,34 @@ def full_assessment_dict(assessment):
         dict with keys 'rubric' (serialized Rubric model) and 'parts' (serialized assessment parts)
     """
     assessment_dict = AssessmentSerializer(assessment).data
-    rubric_dict = RubricSerializer(assessment.rubric).data
+    if not rubric_dict:
+        rubric_dict = RubricSerializer(assessment.rubric).data
+
     assessment_dict["rubric"] = rubric_dict
+
     parts = []
-    for part in assessment.parts.all():
-        part_dict = AssessmentPartSerializer(part).data
-        options_dict = CriterionOptionSerializer(part.option).data
-        criterion_dict = CriterionSerializer(part.option.criterion).data
+    for part in assessment.parts.all().select_related("option__criterion"):
+        criterion_dict = next(
+            crit
+            for crit in rubric_dict["criteria"]
+            if crit["name"] == part.option.criterion.name
+        )
+        options_dict = next(
+            option
+            for option in criterion_dict["options"]
+            if option["name"] == part.option.name
+        )
         options_dict["criterion"] = criterion_dict
-        part_dict["option"] = options_dict
-        parts.append(part_dict)
+        parts.append({
+            "option": options_dict
+        })
+
     assessment_dict["parts"] = parts
+    assessment_dict["points_earned"] = sum(
+        part_dict["option"]["points"] for part_dict in parts
+    )
+    assessment_dict["points_possible"] = rubric_dict["points_possible"]
+
     return assessment_dict