From 31689659ce5c9dd1d99e417e584078096d1f95f0 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 6 Mar 2015 14:57:55 -0500 Subject: [PATCH] Add query count tests to DjangoKeyValueStore/FieldDataCache --- .../courseware/tests/test_model_data.py | 109 +++++++++++++----- 1 file changed, 80 insertions(+), 29 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_model_data.py b/lms/djangoapps/courseware/tests/test_model_data.py index 80b4c04512..770391b142 100644 --- a/lms/djangoapps/courseware/tests/test_model_data.py +++ b/lms/djangoapps/courseware/tests/test_model_data.py @@ -106,48 +106,68 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase): student_module = StudentModuleFactory(state=json.dumps({'a_field': 'a_value', 'b_field': 'b_value'})) self.user = student_module.student self.assertEqual(self.user.id, 1) # check our assumption hard-coded in the key functions above. - self.field_data_cache = FieldDataCache([mock_descriptor([mock_field(Scope.user_state, 'a_field')])], course_id, self.user) + + # There should be only one query to load a single descriptor with a single user_state field + with self.assertNumQueries(1): + self.field_data_cache = FieldDataCache([mock_descriptor([mock_field(Scope.user_state, 'a_field')])], course_id, self.user) + self.kvs = DjangoKeyValueStore(self.field_data_cache) def test_get_existing_field(self): "Test that getting an existing field in an existing StudentModule works" - self.assertEquals('a_value', self.kvs.get(user_state_key('a_field'))) + # This should only read from the cache, not the database + with self.assertNumQueries(0): + self.assertEquals('a_value', self.kvs.get(user_state_key('a_field'))) def test_get_missing_field(self): "Test that getting a missing field from an existing StudentModule raises a KeyError" - self.assertRaises(KeyError, self.kvs.get, user_state_key('not_a_field')) + # This should only read from the cache, not the database + with self.assertNumQueries(0): + self.assertRaises(KeyError, self.kvs.get, user_state_key('not_a_field')) def test_set_existing_field(self): "Test that setting an existing user_state field changes the value" - self.kvs.set(user_state_key('a_field'), 'new_value') + # We are updating a problem, so we write to courseware_studentmodulehistory + # as well as courseware_studentmodule + with self.assertNumQueries(3): + self.kvs.set(user_state_key('a_field'), 'new_value') self.assertEquals(1, StudentModule.objects.all().count()) self.assertEquals({'b_field': 'b_value', 'a_field': 'new_value'}, json.loads(StudentModule.objects.all()[0].state)) def test_set_missing_field(self): "Test that setting a new user_state field changes the value" - self.kvs.set(user_state_key('not_a_field'), 'new_value') + # We are updating a problem, so we write to courseware_studentmodulehistory + # as well as courseware_studentmodule + with self.assertNumQueries(3): + self.kvs.set(user_state_key('not_a_field'), 'new_value') self.assertEquals(1, StudentModule.objects.all().count()) self.assertEquals({'b_field': 'b_value', 'a_field': 'a_value', 'not_a_field': 'new_value'}, json.loads(StudentModule.objects.all()[0].state)) def test_delete_existing_field(self): "Test that deleting an existing field removes it from the StudentModule" - self.kvs.delete(user_state_key('a_field')) + # We are updating a problem, so we write to courseware_studentmodulehistory + # as well as courseware_studentmodule + with self.assertNumQueries(3): + self.kvs.delete(user_state_key('a_field')) self.assertEquals(1, StudentModule.objects.all().count()) self.assertRaises(KeyError, self.kvs.get, user_state_key('not_a_field')) def test_delete_missing_field(self): "Test that deleting a missing field from an existing StudentModule raises a KeyError" - self.assertRaises(KeyError, self.kvs.delete, user_state_key('not_a_field')) + with self.assertNumQueries(0): + self.assertRaises(KeyError, self.kvs.delete, user_state_key('not_a_field')) self.assertEquals(1, StudentModule.objects.all().count()) self.assertEquals({'b_field': 'b_value', 'a_field': 'a_value'}, json.loads(StudentModule.objects.all()[0].state)) def test_has_existing_field(self): "Test that `has` returns True for existing fields in StudentModules" - self.assertTrue(self.kvs.has(user_state_key('a_field'))) + with self.assertNumQueries(0): + self.assertTrue(self.kvs.has(user_state_key('a_field'))) def test_has_missing_field(self): "Test that `has` returns False for missing fields in StudentModule" - self.assertFalse(self.kvs.has(user_state_key('not_a_field'))) + with self.assertNumQueries(0): + self.assertFalse(self.kvs.has(user_state_key('not_a_field'))) def construct_kv_dict(self): """Construct a kv_dict that can be passed to set_many""" @@ -160,7 +180,12 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase): def test_set_many(self): "Test setting many fields that are scoped to Scope.user_state" kv_dict = self.construct_kv_dict() - self.kvs.set_many(kv_dict) + + # Scope.user_state is stored in a single row in the database, so we only + # need to send a single update to that table. + # We also are updating a problem, so we write to courseware student module history + with self.assertNumQueries(3): + self.kvs.set_many(kv_dict) for key in kv_dict: self.assertEquals(self.kvs.get(key), kv_dict[key]) @@ -185,19 +210,26 @@ class TestMissingStudentModule(TestCase): self.user = UserFactory.create(username='user') self.assertEqual(self.user.id, 1) # check our assumption hard-coded in the key functions above. - self.field_data_cache = FieldDataCache([mock_descriptor()], course_id, self.user) + + # The descriptor has no fields, so FDC shouldn't send any queries + with self.assertNumQueries(0): + self.field_data_cache = FieldDataCache([mock_descriptor()], course_id, self.user) self.kvs = DjangoKeyValueStore(self.field_data_cache) def test_get_field_from_missing_student_module(self): "Test that getting a field from a missing StudentModule raises a KeyError" - self.assertRaises(KeyError, self.kvs.get, user_state_key('a_field')) + with self.assertNumQueries(0): + self.assertRaises(KeyError, self.kvs.get, user_state_key('a_field')) def test_set_field_in_missing_student_module(self): "Test that setting a field in a missing StudentModule creates the student module" self.assertEquals(0, len(self.field_data_cache.cache)) self.assertEquals(0, StudentModule.objects.all().count()) - self.kvs.set(user_state_key('a_field'), 'a_value') + # We are updating a problem, so we write to courseware_studentmodulehistory + # as well as courseware_studentmodule + with self.assertNumQueries(6): + self.kvs.set(user_state_key('a_field'), 'a_value') self.assertEquals(1, len(self.field_data_cache.cache)) self.assertEquals(1, StudentModule.objects.all().count()) @@ -210,11 +242,13 @@ class TestMissingStudentModule(TestCase): def test_delete_field_from_missing_student_module(self): "Test that deleting a field from a missing StudentModule raises a KeyError" - self.assertRaises(KeyError, self.kvs.delete, user_state_key('a_field')) + with self.assertNumQueries(0): + self.assertRaises(KeyError, self.kvs.delete, user_state_key('a_field')) def test_has_field_for_missing_student_module(self): "Test that `has` returns False for missing StudentModules" - self.assertFalse(self.kvs.has(user_state_key('a_field'))) + with self.assertNumQueries(0): + self.assertFalse(self.kvs.has(user_state_key('a_field'))) class StorageTestBase(object): @@ -240,51 +274,64 @@ class StorageTestBase(object): self.mock_descriptor = mock_descriptor([ mock_field(self.scope, 'existing_field'), mock_field(self.scope, 'other_existing_field')]) - self.field_data_cache = FieldDataCache([self.mock_descriptor], course_id, self.user) + # Each field is stored as a separate row in the table, + # but we can query them in a single query + with self.assertNumQueries(1): + self.field_data_cache = FieldDataCache([self.mock_descriptor], course_id, self.user) self.kvs = DjangoKeyValueStore(self.field_data_cache) def test_set_and_get_existing_field(self): - self.kvs.set(self.key_factory('existing_field'), 'test_value') - self.assertEquals('test_value', self.kvs.get(self.key_factory('existing_field'))) + with self.assertNumQueries(2): + self.kvs.set(self.key_factory('existing_field'), 'test_value') + with self.assertNumQueries(0): + self.assertEquals('test_value', self.kvs.get(self.key_factory('existing_field'))) def test_get_existing_field(self): "Test that getting an existing field in an existing Storage Field works" - self.assertEquals('old_value', self.kvs.get(self.key_factory('existing_field'))) + with self.assertNumQueries(0): + self.assertEquals('old_value', self.kvs.get(self.key_factory('existing_field'))) def test_get_missing_field(self): "Test that getting a missing field from an existing Storage Field raises a KeyError" - self.assertRaises(KeyError, self.kvs.get, self.key_factory('missing_field')) + with self.assertNumQueries(0): + self.assertRaises(KeyError, self.kvs.get, self.key_factory('missing_field')) def test_set_existing_field(self): "Test that setting an existing field changes the value" - self.kvs.set(self.key_factory('existing_field'), 'new_value') + with self.assertNumQueries(2): + self.kvs.set(self.key_factory('existing_field'), 'new_value') self.assertEquals(1, self.storage_class.objects.all().count()) self.assertEquals('new_value', json.loads(self.storage_class.objects.all()[0].value)) def test_set_missing_field(self): "Test that setting a new field changes the value" - self.kvs.set(self.key_factory('missing_field'), 'new_value') + with self.assertNumQueries(4): + self.kvs.set(self.key_factory('missing_field'), 'new_value') self.assertEquals(2, self.storage_class.objects.all().count()) self.assertEquals('old_value', json.loads(self.storage_class.objects.get(field_name='existing_field').value)) self.assertEquals('new_value', json.loads(self.storage_class.objects.get(field_name='missing_field').value)) def test_delete_existing_field(self): "Test that deleting an existing field removes it" - self.kvs.delete(self.key_factory('existing_field')) + with self.assertNumQueries(1): + self.kvs.delete(self.key_factory('existing_field')) self.assertEquals(0, self.storage_class.objects.all().count()) def test_delete_missing_field(self): "Test that deleting a missing field from an existing Storage Field raises a KeyError" - self.assertRaises(KeyError, self.kvs.delete, self.key_factory('missing_field')) + with self.assertNumQueries(0): + self.assertRaises(KeyError, self.kvs.delete, self.key_factory('missing_field')) self.assertEquals(1, self.storage_class.objects.all().count()) def test_has_existing_field(self): "Test that `has` returns True for an existing Storage Field" - self.assertTrue(self.kvs.has(self.key_factory('existing_field'))) + with self.assertNumQueries(0): + self.assertTrue(self.kvs.has(self.key_factory('existing_field'))) def test_has_missing_field(self): "Test that `has` return False for an existing Storage Field" - self.assertFalse(self.kvs.has(self.key_factory('missing_field'))) + with self.assertNumQueries(0): + self.assertFalse(self.kvs.has(self.key_factory('missing_field'))) def construct_kv_dict(self): """Construct a kv_dict that can be passed to set_many""" @@ -298,15 +345,19 @@ class StorageTestBase(object): """Test that setting many regular fields at the same time works""" kv_dict = self.construct_kv_dict() - self.kvs.set_many(kv_dict) + # Each field is a separate row in the database, hence + # a separate query + with self.assertNumQueries(len(kv_dict)*3): + self.kvs.set_many(kv_dict) for key in kv_dict: self.assertEquals(self.kvs.get(key), kv_dict[key]) def test_set_many_failure(self): """Test that setting many regular fields with a DB error """ kv_dict = self.construct_kv_dict() - for key in kv_dict: - self.kvs.set(key, 'test value') + with self.assertNumQueries(6): + for key in kv_dict: + self.kvs.set(key, 'test value') with patch('django.db.models.Model.save', side_effect=[None, DatabaseError]): with self.assertRaises(KeyValueMultiSaveError) as exception_context: