Merge pull request #7258 from cpennington/lms-field-data-query-counts

Reduce query counts in FieldDataCache/DjangoKeyValueStore

lms/djangoapps/courseware/model_data.py

@@ -266,7 +266,7 @@ class FieldDataCache(object):
 
     def find_or_create(self, key):
         '''
-        Find a model data object in this cache, or create it if it doesn't
+        Find a model data object in this cache, or create a new one if it doesn't
         exist
         '''
         field_object = self.find(key)
@@ -275,28 +275,26 @@ class FieldDataCache(object):
             return field_object
 
         if key.scope == Scope.user_state:
-            field_object, __ = StudentModule.objects.get_or_create(
+            field_object = StudentModule(
                 course_id=self.course_id,
                 student_id=key.user_id,
                 module_state_key=key.block_scope_id,
-                defaults={
-                    'state': json.dumps({}),
-                    'module_type': key.block_scope_id.block_type,
-                },
+                state=json.dumps({}),
+                module_type=key.block_scope_id.block_type,
             )
         elif key.scope == Scope.user_state_summary:
-            field_object, __ = XModuleUserStateSummaryField.objects.get_or_create(
+            field_object = XModuleUserStateSummaryField(
                 field_name=key.field_name,
                 usage_id=key.block_scope_id
             )
         elif key.scope == Scope.preferences:
-            field_object, __ = XModuleStudentPrefsField.objects.get_or_create(
+            field_object = XModuleStudentPrefsField(
                 field_name=key.field_name,
                 module_type=BlockTypeKeyV1(key.block_family, key.block_scope_id),
                 student_id=key.user_id,
             )
         elif key.scope == Scope.user_info:
-            field_object, __ = XModuleStudentInfoField.objects.get_or_create(
+            field_object = XModuleStudentInfoField(
                 field_name=key.field_name,
                 student_id=key.user_id,
             )
@@ -362,39 +360,39 @@ class DjangoKeyValueStore(KeyValueStore):
 
         """
         saved_fields = []
-        # field_objects maps a field_object to a list of associated fields
-        field_objects = dict()
-        for field in kv_dict:
-            # Check field for validity
-            if field.scope not in self._allowed_scopes:
-                raise InvalidScopeError(field)
-
-            # If the field is valid and isn't already in the dictionary, add it.
-            field_object = self._field_data_cache.find_or_create(field)
-            if field_object not in field_objects.keys():
-                field_objects[field_object] = []
-            # Update the list of associated fields
-            field_objects[field_object].append(field)
+        # field_objects maps id(field_object) to a the object and a list of associated fields.
+        # We use id() because FieldDataCache might return django models with no primary key
+        # set, but will return the same django model each time the same key is passed in.
+        dirty_field_objects = defaultdict(lambda: (None, []))
+        for key in kv_dict:
+            # Check key for validity
+            if key.scope not in self._allowed_scopes:
+                raise InvalidScopeError(key)
+
+            field_object = self._field_data_cache.find_or_create(key)
+            # Update the list dirtied field_objects
+            _, dirty_names = dirty_field_objects.setdefault(id(field_object), (field_object, []))
+            dirty_names.append(key.field_name)
 
             # Special case when scope is for the user state, because this scope saves fields in a single row
-            if field.scope == Scope.user_state:
+            if key.scope == Scope.user_state:
                 state = json.loads(field_object.state)
-                state[field.field_name] = kv_dict[field]
+                state[key.field_name] = kv_dict[key]
                 field_object.state = json.dumps(state)
             else:
                 # The remaining scopes save fields on different rows, so
                 # we don't have to worry about conflicts
-                field_object.value = json.dumps(kv_dict[field])
+                field_object.value = json.dumps(kv_dict[key])
 
-        for field_object in field_objects:
+        for field_object, names in dirty_field_objects.values():
             try:
                 # Save the field object that we made above
-                field_object.save()
+                field_object.save(force_update=field_object.pk is not None)
                 # If save is successful on this scope, add the saved fields to
                 # the list of successful saves
-                saved_fields.extend([field.field_name for field in field_objects[field_object]])
+                saved_fields.extend(names)
             except DatabaseError:
-                log.exception('Error saving fields %r', field_objects[field_object])
+                log.exception('Error saving fields %r', names)
                 raise KeyValueMultiSaveError(saved_fields)
 
     def delete(self, key):
@@ -409,7 +407,7 @@ class DjangoKeyValueStore(KeyValueStore):
             state = json.loads(field_object.state)
             del state[key.field_name]
             field_object.state = json.dumps(state)
-            field_object.save()
+            field_object.save(force_update=field_object.pk is not None)
         else:
             field_object.delete()