From b1346fc7476268eeb29d9d139d5dc1cccba542e7 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Mon, 22 Jun 2015 23:14:07 -0400 Subject: [PATCH] add tests for structure cache use default cache for tests add test for when cache isn't configured add test for dummy cache noop if no cache found --- .../xmodule/xmodule/modulestore/__init__.py | 26 +++++++ .../split_mongo/mongo_connection.py | 14 +++- .../tests/test_mixed_modulestore.py | 2 +- .../tests/test_split_modulestore.py | 75 +++++++++++++++++++ 4 files changed, 113 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index c167c25422..f47c901b65 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -369,6 +369,19 @@ class EditInfo(object): source_version="UNSET" if self.source_version is None else self.source_version, ) # pylint: disable=bad-continuation + def __eq__(self, edit_info): + """ + Two EditInfo instances are equal iff their storable representations + are equal. + """ + return self.to_storable() == edit_info.to_storable() + + def __neq__(self, edit_info): + """ + Two EditInfo instances are not equal if they're not equal. + """ + return not self == edit_info + class BlockData(object): """ @@ -426,6 +439,19 @@ class BlockData(object): classname=self.__class__.__name__, ) # pylint: disable=bad-continuation + def __eq__(self, block_data): + """ + Two BlockData objects are equal iff all their attributes are equal. + """ + attrs = ['fields', 'block_type', 'definition', 'defaults', 'edit_info'] + return all(getattr(self, attr) == getattr(block_data, attr) for attr in attrs) + + def __neq__(self, block_data): + """ + Just define this as not self.__eq__(block_data) + """ + return not self == block_data + new_contract('BlockData', BlockData) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py index bcee23b7f0..fc179c81ba 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -4,9 +4,7 @@ Segregation of pymongo functions from the data modeling mechanisms for split mod import datetime import cPickle as pickle import math -import re import zlib -from mongodb_proxy import autoretry_read, MongoProxy import pymongo import pytz import re @@ -213,15 +211,22 @@ class CourseStructureCache(object): """ Wrapper around django cache object to cache course structure objects. The course structures are pickled and compressed when cached. + + If the 'course_structure_cache' doesn't exist, then don't do anything for + for set and get. """ def __init__(self): + self.no_cache_found = False try: self.cache = get_cache('course_structure_cache') except InvalidCacheBackendError: - self.cache = get_cache('default') + self.no_cache_found = True def get(self, key): """Pull the compressed, pickled struct data from cache and deserialize.""" + if self.no_cache_found: + return None + compressed_pickled_data = self.cache.get(key) if compressed_pickled_data is None: return None @@ -229,6 +234,9 @@ class CourseStructureCache(object): def set(self, key, structure): """Given a structure, will pickle, compress, and write to cache.""" + if self.no_cache_found: + return None + pickled_data = pickle.dumps(structure, pickle.HIGHEST_PROTOCOL) # 1 = Fastest (slightly larger results) compressed_pickled_data = zlib.compress(pickled_data, 1) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index e83c9be516..b41bf268d6 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -794,7 +794,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # find: find parent (definition.children) 2x, find draft item, get inheritance items # send: one delete query for specific item # Split: - # find: active_version & structure (cached) + # find: active_version & structure # send: update structure and active_versions @ddt.data(('draft', 4, 1), ('split', 2, 2)) @ddt.unpack diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index a47655f1a7..06b5067223 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -12,6 +12,7 @@ import uuid from contracts import contract from nose.plugins.attrib import attr +from django.core.cache import get_cache, InvalidCacheBackendError from openedx.core.lib import tempdir from xblock.fields import Reference, ReferenceList, ReferenceValueDict @@ -29,6 +30,7 @@ from xmodule.fields import Date, Timedelta from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore from xmodule.modulestore.tests.test_modulestore import check_has_course_method from xmodule.modulestore.split_mongo import BlockKey +from xmodule.modulestore.tests.factories import check_mongo_calls from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.modulestore.tests.utils import mock_tab_from_json from xmodule.modulestore.edit_info import EditInfoMixin @@ -771,6 +773,79 @@ class SplitModuleCourseTests(SplitModuleTest): self.assertEqual(result.children[0].children[0].locator.version_guid, versions[0]) +class TestCourseStructureCache(SplitModuleTest): + """Tests for the CourseStructureCache""" + + def setUp(self): + # use the default cache, since the `course_structure_cache` + # is a dummy cache during testing + self.cache = get_cache('default') + + # make sure we clear the cache before every test... + self.cache.clear() + # ... and after + self.addCleanup(self.cache.clear) + + # make a new course: + self.user = random.getrandbits(32) + self.new_course = modulestore().create_course( + 'org', 'course', 'test_run', self.user, BRANCH_NAME_DRAFT, + ) + + super(TestCourseStructureCache, self).setUp() + + @patch('xmodule.modulestore.split_mongo.mongo_connection.get_cache') + def test_course_structure_cache(self, mock_get_cache): + # force get_cache to return the default cache so we can test + # its caching behavior + mock_get_cache.return_value = self.cache + + with check_mongo_calls(1): + not_cached_structure = self._get_structure(self.new_course) + + # when cache is warmed, we should have one fewer mongo call + with check_mongo_calls(0): + cached_structure = self._get_structure(self.new_course) + + # now make sure that you get the same structure + self.assertEqual(cached_structure, not_cached_structure) + + @patch('xmodule.modulestore.split_mongo.mongo_connection.get_cache') + def test_course_structure_cache_no_cache_configured(self, mock_get_cache): + mock_get_cache.side_effect = InvalidCacheBackendError + + with check_mongo_calls(1): + not_cached_structure = self._get_structure(self.new_course) + + # if the cache isn't configured, we expect to have to make + # another mongo call here if we want the same course structure + with check_mongo_calls(1): + cached_structure = self._get_structure(self.new_course) + + # now make sure that you get the same structure + self.assertEqual(cached_structure, not_cached_structure) + + def test_dummy_cache(self): + with check_mongo_calls(1): + not_cached_structure = self._get_structure(self.new_course) + + # Since the test is using the dummy cache, it's not actually caching + # anything + with check_mongo_calls(1): + cached_structure = self._get_structure(self.new_course) + + # now make sure that you get the same structure + self.assertEqual(cached_structure, not_cached_structure) + + def _get_structure(self, course): + """ + Helper function to get a structure from a course. + """ + return modulestore().db_connection.get_structure( + course.location.as_object_id(course.location.version_guid) + ) + + class SplitModuleItemTests(SplitModuleTest): ''' Item read tests including inheritance