Merge pull request #1333 from edx/dhm/separate_pymongo

Pull all db (mongo) functions into another file to enable easier replacement

common/lib/xmodule/xmodule/modulestore/split_migrator.py

@@ -88,7 +88,7 @@ class SplitMigrator(object):
         index_info = self.split_modulestore.get_course_index_info(course_version_locator)
         versions = index_info['versions']
         versions['draft'] = versions['published']
-        self.split_modulestore.update_course_index(course_version_locator, {'versions': versions}, update_versions=True)
+        self.split_modulestore.update_course_index(index_info)
 
         # clean up orphans in published version: in old mongo, parents pointed to the union of their published and draft
         # children which meant some pointers were to non-existent locations in 'direct'

common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py

@@ -22,5 +22,4 @@ class DefinitionLazyLoader(object):
         Fetch the definition. Note, the caller should replace this lazy
         loader pointer with the result so as not to fetch more than once
         """
-        return self.modulestore.definitions.find_one(
-            {'_id': self.definition_locator.definition_id})
+        return self.modulestore.db_connection.get_definition(self.definition_locator.definition_id)

common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py

+"""
+Segregation of pymongo functions from the data modeling mechanisms for split modulestore.
+"""
+import pymongo
+
+class MongoConnection(object):
+    """
+    Segregation of pymongo functions from the data modeling mechanisms for split modulestore.
+    """
+    def __init__(
+        self, db, collection, host, port=27017, tz_aware=True, user=None, password=None, **kwargs
+    ):
+        """
+        Create & open the connection, authenticate, and provide pointers to the collections
+        """
+        self.database = pymongo.database.Database(
+            pymongo.MongoClient(
+                host=host,
+                port=port,
+                tz_aware=tz_aware,
+                **kwargs
+            ),
+            db
+        )
+
+        if user is not None and password is not None:
+            self.database.authenticate(user, password)
+
+        self.course_index = self.database[collection + '.active_versions']
+        self.structures = self.database[collection + '.structures']
+        self.definitions = self.database[collection + '.definitions']
+
+        # every app has write access to the db (v having a flag to indicate r/o v write)
+        # Force mongo to report errors, at the expense of performance
+        # pymongo docs suck but explanation:
+        # http://api.mongodb.org/java/2.10.1/com/mongodb/WriteConcern.html
+        self.course_index.write_concern = {'w': 1}
+        self.structures.write_concern = {'w': 1}
+        self.definitions.write_concern = {'w': 1}
+
+    def get_structure(self, key):
+        """
+        Get the structure from the persistence mechanism whose id is the given key
+        """
+        return self.structures.find_one({'_id': key})
+
+    def find_matching_structures(self, query):
+        """
+        Find the structure matching the query. Right now the query must be a legal mongo query
+        :param query: a mongo-style query of {key: [value|{$in ..}|..], ..}
+        """
+        return self.structures.find(query)
+
+    def insert_structure(self, structure):
+        """
+        Create the structure in the db
+        """
+        self.structures.insert(structure)
+
+    def update_structure(self, structure):
+        """
+        Update the db record for structure
+        """
+        self.structures.update({'_id': structure['_id']}, structure)
+
+    def get_course_index(self, key):
+        """
+        Get the course_index from the persistence mechanism whose id is the given key
+        """
+        return self.course_index.find_one({'_id': key})
+
+    def find_matching_course_indexes(self, query):
+        """
+        Find the course_index matching the query. Right now the query must be a legal mongo query
+        :param query: a mongo-style query of {key: [value|{$in ..}|..], ..}
+        """
+        return self.course_index.find(query)
+
+    def insert_course_index(self, course_index):
+        """
+        Create the course_index in the db
+        """
+        self.course_index.insert(course_index)
+
+    def update_course_index(self, course_index):
+        """
+        Update the db record for course_index
+        """
+        self.course_index.update({'_id': course_index['_id']}, course_index)
+
+    def delete_course_index(self, key):
+        """
+        Delete the course_index from the persistence mechanism whose id is the given key
+        """
+        return self.course_index.remove({'_id': key})
+
+    def get_definition(self, key):
+        """
+        Get the definition from the persistence mechanism whose id is the given key
+        """
+        return self.definitions.find_one({'_id': key})
+
+    def find_matching_definitions(self, query):
+        """
+        Find the definitions matching the query. Right now the query must be a legal mongo query
+        :param query: a mongo-style query of {key: [value|{$in ..}|..], ..}
+        """
+        return self.definitions.find(query)
+
+    def insert_definition(self, definition):
+        """
+        Create the definition in the db
+        """
+        self.definitions.insert(definition)
+
+

common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py

@@ -59,9 +59,9 @@ class TestMigration(unittest.TestCase):
         dbref = self.loc_mapper.db
         dbref.drop_collection(self.loc_mapper.location_map)
         split_db = self.split_mongo.db
-        split_db.drop_collection(split_db.course_index)
-        split_db.drop_collection(split_db.structures)
-        split_db.drop_collection(split_db.definitions)
+        split_db.drop_collection(self.split_mongo.db_connection.course_index)
+        split_db.drop_collection(self.split_mongo.db_connection.structures)
+        split_db.drop_collection(self.split_mongo.db_connection.definitions)
         # old_mongo doesn't give a db attr, but all of the dbs are the same
         dbref.drop_collection(self.old_mongo.collection)
 

common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py

@@ -1018,41 +1018,29 @@ class TestCourseCreation(SplitModuleTest):
         Test changing the org, pretty id, etc of a course. Test that it doesn't allow changing the id, etc.
         """
         locator = CourseLocator(course_id="GreekHero", branch='draft')
-        modulestore().update_course_index(locator, {'org': 'funkyU'})
+        course_info = modulestore().get_course_index_info(locator)
+        course_info['org'] = 'funkyU'
+        modulestore().update_course_index(course_info)
         course_info = modulestore().get_course_index_info(locator)
         self.assertEqual(course_info['org'], 'funkyU')
 
-        modulestore().update_course_index(locator, {'org': 'moreFunky', 'prettyid': 'Ancient Greek Demagods'})
+        course_info['org'] = 'moreFunky'
+        course_info['prettyid'] = 'Ancient Greek Demagods'
+        modulestore().update_course_index(course_info)
         course_info = modulestore().get_course_index_info(locator)
         self.assertEqual(course_info['org'], 'moreFunky')
         self.assertEqual(course_info['prettyid'], 'Ancient Greek Demagods')
 
-        self.assertRaises(ValueError, modulestore().update_course_index, locator, {'_id': 'funkygreeks'})
-
-        with self.assertRaises(ValueError):
-            modulestore().update_course_index(
-                locator,
-                {'edited_on': datetime.datetime.now(UTC)}
-            )
-        with self.assertRaises(ValueError):
-            modulestore().update_course_index(
-                locator,
-                {'edited_by': 'sneak'}
-            )
-
-        self.assertRaises(ValueError, modulestore().update_course_index, locator,
-                          {'versions': {'draft': self.GUID_D1}})
-
         # an allowed but not necessarily recommended way to revert the draft version
         versions = course_info['versions']
         versions['draft'] = self.GUID_D1
-        modulestore().update_course_index(locator, {'versions': versions}, update_versions=True)
+        modulestore().update_course_index(course_info)
         course = modulestore().get_course(locator)
         self.assertEqual(str(course.location.version_guid), self.GUID_D1)
 
         # an allowed but not recommended way to publish a course
         versions['published'] = self.GUID_D1
-        modulestore().update_course_index(locator, {'versions': versions}, update_versions=True)
+        modulestore().update_course_index(course_info)
         course = modulestore().get_course(CourseLocator(course_id=locator.course_id, branch="published"))
         self.assertEqual(str(course.location.version_guid), self.GUID_D1)
 
@@ -1068,9 +1056,9 @@ class TestCourseCreation(SplitModuleTest):
         self.assertEqual(new_course.location.usage_id, 'top')
         self.assertEqual(new_course.category, 'chapter')
         # look at db to verify
-        db_structure = modulestore().structures.find_one({
-            '_id': new_course.location.as_object_id(new_course.location.version_guid)
-        })
+        db_structure = modulestore().db_connection.get_structure(
+            new_course.location.as_object_id(new_course.location.version_guid)
+        )
         self.assertIsNotNone(db_structure, "Didn't find course")
         self.assertNotIn('course', db_structure['blocks'])
         self.assertIn('top', db_structure['blocks'])