Merge pull request #4370 from edx/nimisha/split-drop-database-2952

Implement close_connections and drop_database on modulestores.

cms/djangoapps/contentstore/management/commands/tests/test_migrate_to_split.py

@@ -56,7 +56,7 @@ class TestMigrateToSplit(ModuleStoreTestCase):
         password = 'foo'
         self.user = User.objects.create_user(uname, email, password)
         self.course = CourseFactory()
-        self.addCleanup(ModuleStoreTestCase.drop_mongo_collections, ModuleStoreEnum.Type.split)
+        self.addCleanup(ModuleStoreTestCase.drop_mongo_collections)
         self.addCleanup(clear_existing_modulestores)
 
     def test_user_email(self):

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -22,7 +22,7 @@ from django.test.utils import override_settings
 from contentstore.tests.utils import parse_json, AjaxEnabledTestClient, CourseTestCase
 from contentstore.views.component import ADVANCED_COMPONENT_TYPES
 
-from xmodule.contentstore.django import contentstore, _CONTENTSTORE
+from xmodule.contentstore.django import contentstore
 from xmodule.contentstore.utils import restore_asset_from_trashcan, empty_asset_trashcan
 from xmodule.exceptions import NotFoundError, InvalidVersionError
 from xmodule.modulestore import ModuleStoreEnum
@@ -74,10 +74,6 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
     Tests that rely on the toy courses.
     TODO: refactor using CourseFactory so they do not.
     """
-    def tearDown(self):
-        contentstore().drop_database()
-        _CONTENTSTORE.clear()
-
     def check_components_on_page(self, component_types, expected_types):
         """
         Ensure that the right types end up on the page.
@@ -947,10 +943,6 @@ class ContentStoreTest(ContentStoreTestCase):
             'run': '2013_Spring'
         }
 
-    def tearDown(self):
-        contentstore().drop_database()
-        _CONTENTSTORE.clear()
-
     def assert_created_course(self, number_suffix=None):
         """
         Checks that the course was created properly.

cms/djangoapps/contentstore/tests/test_crud.py

@@ -20,7 +20,7 @@ class TemplateTests(unittest.TestCase):
 
     def setUp(self):
         clear_existing_modulestores()  # redundant w/ cleanup but someone was getting errors
-        self.addCleanup(ModuleStoreTestCase.drop_mongo_collections, ModuleStoreEnum.Type.split)
+        self.addCleanup(ModuleStoreTestCase.drop_mongo_collections)
         self.addCleanup(clear_existing_modulestores)
         self.split_store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.split)
 

cms/djangoapps/contentstore/tests/test_export_git.py

@@ -13,7 +13,6 @@ from django.test.utils import override_settings
 
 from .utils import CourseTestCase
 import contentstore.git_export_utils as git_export_utils
-from xmodule.contentstore.django import _CONTENTSTORE
 from xmodule.modulestore.django import modulestore
 from contentstore.utils import reverse_course_url
 
@@ -35,10 +34,6 @@ class TestExportGit(CourseTestCase):
         self.course_module = modulestore().get_course(self.course.id)
         self.test_url = reverse_course_url('export_git', self.course.id)
 
-    def tearDown(self):
-        modulestore().contentstore.drop_database()
-        _CONTENTSTORE.clear()
-
     def test_giturl_missing(self):
         """
         Test to make sure an appropriate error is displayed

cms/djangoapps/contentstore/tests/test_import.py

@@ -9,16 +9,11 @@ from django.test.utils import override_settings
 from django.conf import settings
 import copy
 
-from django.contrib.auth.models import User
-
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
 from xmodule.modulestore.xml_importer import import_from_xml
-from xmodule.contentstore.django import _CONTENTSTORE
-
 from xmodule.exceptions import NotFoundError
 from uuid import uuid4
 
@@ -38,10 +33,6 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         self.client = Client()
         self.client.login(username=self.user.username, password=password)
 
-    def tearDown(self):
-        contentstore().drop_database()
-        _CONTENTSTORE.clear()
-
     def load_test_import_course(self):
         '''
         Load the standard course used to test imports

cms/djangoapps/contentstore/tests/test_transcripts_utils.py

@@ -18,7 +18,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.contentstore.content import StaticContent
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.exceptions import NotFoundError
-from xmodule.contentstore.django import contentstore, _CONTENTSTORE
+from xmodule.contentstore.django import contentstore
 from xmodule.video_module import transcripts_utils
 
 TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
@@ -151,8 +151,6 @@ class TestSaveSubsToStore(ModuleStoreTestCase):
 
     def tearDown(self):
         self.clear_subs_content()
-        contentstore().drop_database()
-        _CONTENTSTORE.clear()
 
 
 @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE)
@@ -189,10 +187,6 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase):
         self.course = CourseFactory.create(
             org=self.org, number=self.number, display_name=self.display_name)
 
-    def tearDown(self):
-        contentstore().drop_database()
-        _CONTENTSTORE.clear()
-
     def test_success_downloading_subs(self):
 
         response = textwrap.dedent("""<?xml version="1.0" encoding="utf-8" ?>

cms/djangoapps/contentstore/views/tests/test_import_export.py

@@ -9,20 +9,17 @@ import shutil
 import tarfile
 import tempfile
 from path import path
-from pymongo import MongoClient
 from uuid import uuid4
 
 from django.test.utils import override_settings
 from django.conf import settings
 from contentstore.utils import reverse_course_url
 
-from xmodule.contentstore.django import _CONTENTSTORE
 from xmodule.modulestore.tests.factories import ItemFactory
 
 from contentstore.tests.utils import CourseTestCase
 from student import auth
 from student.roles import CourseInstructorRole, CourseStaffRole
-from xmodule.modulestore.django import modulestore
 
 TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
 TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex
@@ -70,8 +67,6 @@ class ImportTestCase(CourseTestCase):
 
     def tearDown(self):
         shutil.rmtree(self.content_dir)
-        modulestore().contentstore.drop_database()
-        _CONTENTSTORE.clear()
 
     def test_no_coursexml(self):
         """

cms/djangoapps/contentstore/views/tests/test_transcripts.py

@@ -15,7 +15,7 @@ from django.conf import settings
 from contentstore.tests.utils import CourseTestCase
 from cache_toolbox.core import del_cached_content
 from xmodule.modulestore.django import modulestore
-from xmodule.contentstore.django import contentstore, _CONTENTSTORE
+from xmodule.contentstore.django import contentstore
 from xmodule.contentstore.content import StaticContent
 from xmodule.exceptions import NotFoundError
 from opaque_keys.edx.keys import UsageKey
@@ -80,10 +80,6 @@ class Basetranscripts(CourseTestCase):
             1.5: item.youtube_id_1_5
         }
 
-    def tearDown(self):
-        contentstore().drop_database()
-        _CONTENTSTORE.clear()
-
 
 class TestUploadtranscripts(Basetranscripts):
     """Tests for '/transcripts/upload' url."""

common/djangoapps/contentserver/tests/test.py

@@ -11,7 +11,7 @@ from django.test.utils import override_settings
 
 from student.models import CourseEnrollment
 
-from xmodule.contentstore.django import contentstore, _CONTENTSTORE
+from xmodule.contentstore.django import contentstore
 from xmodule.modulestore.django import modulestore
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -55,11 +55,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         self.contentstore.set_attr(self.locked_asset, 'locked', True)
 
-    def tearDown(self):
-
-        contentstore().drop_database()
-        _CONTENTSTORE.clear()
-
     def test_unlocked_asset(self):
         """
         Test that unlocked assets are being served.

common/djangoapps/terrain/browser.py

@@ -16,10 +16,9 @@ import requests
 from base64 import encodestring
 from json import dumps
 
-from pymongo import MongoClient
 import xmodule.modulestore.django
-from xmodule.contentstore.django import _CONTENTSTORE
 from xmodule.modulestore import ModuleStoreEnum
+from xmodule.contentstore.django import _CONTENTSTORE
 
 # There is an import issue when using django-staticfiles with lettuce
 # Lettuce assumes that we are using django.contrib.staticfiles,
@@ -186,12 +185,9 @@ def reset_databases(scenario):
     whereas modulestore data is in unique collection names.  This data is created implicitly during the scenarios.
     If no data is created during the test, these lines equivilently do nothing.
     '''
-    modulestore = xmodule.modulestore.django.modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo)
-    modulestore.contentstore.drop_database()
-    _CONTENTSTORE.clear()
-
-    modulestore.collection.drop()
+    xmodule.modulestore.django.modulestore()._drop_database()  # pylint: disable=protected-access
     xmodule.modulestore.django.clear_existing_modulestores()
+    _CONTENTSTORE.clear()
 
 
 @world.absorb

common/djangoapps/terrain/course_helpers.py

@@ -5,9 +5,8 @@ import urllib
 from lettuce import world
 from django.contrib.auth.models import User, Group
 from student.models import CourseEnrollment
-from xmodule.modulestore.django import modulestore
-from xmodule.modulestore import ModuleStoreEnum
-from xmodule.contentstore.django import contentstore
+from xmodule.modulestore.django import modulestore, clear_existing_modulestores
+from xmodule.contentstore.django import _CONTENTSTORE
 
 
 @world.absorb
@@ -72,6 +71,6 @@ def clear_courses():
     # (though it shouldn't), do this manually
     # from the bash shell to drop it:
     # $ mongo test_xmodule --eval "db.dropDatabase()"
-    store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo)
-    store.collection.drop()
-    contentstore().fs_files.drop()
+    modulestore()._drop_database()  # pylint: disable=protected-access
+    _CONTENTSTORE.clear()
+    clear_existing_modulestores()

common/lib/xmodule/xmodule/contentstore/mongo.py

@@ -42,11 +42,18 @@ class MongoContentStore(ContentStore):
 
         self.fs_files = _db[bucket + ".files"]  # the underlying collection GridFS uses
 
-    def drop_database(self):
+    def close_connections(self):
         """
-        Only for use by test code. Removes the database!
+        Closes any open connections to the underlying databases
         """
         self.fs_files.database.connection.close()
+
+    def _drop_database(self):
+        """
+        A destructive operation to drop the underlying database and close all connections.
+        Intended to be used by test code for cleanup.
+        """
+        self.close_connections()
         self.fs_files.database.connection.drop_database(self.fs_files.database)
 
     def save(self, content):

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -298,6 +298,14 @@ class ModuleStoreRead(object):
         """
         pass
 
+    @abstractmethod
+    def close_connections(self):
+        """
+        Closes any open connections to the underlying databases
+        """
+        pass
+
+
 class ModuleStoreWrite(ModuleStoreRead):
     """
     An abstract interface for a database backend that stores XModuleDescriptor
@@ -387,6 +395,14 @@ class ModuleStoreWrite(ModuleStoreRead):
         """
         pass
 
+    @abstractmethod
+    def _drop_database(self):
+        """
+        A destructive operation to drop the underlying database and close all connections.
+        Intended to be used by test code for cleanup.
+        """
+        pass
+
 
 class ModuleStoreReadBase(ModuleStoreRead):
     '''
@@ -396,6 +412,7 @@ class ModuleStoreReadBase(ModuleStoreRead):
 
     def __init__(
         self,
+        contentstore=None,
         doc_store_config=None,  # ignore if passed up
         metadata_inheritance_cache_subsystem=None, request_cache=None,
         xblock_mixins=(), xblock_select=None,
@@ -412,6 +429,7 @@ class ModuleStoreReadBase(ModuleStoreRead):
         self.request_cache = request_cache
         self.xblock_mixins = xblock_mixins
         self.xblock_select = xblock_select
+        self.contentstore = contentstore
 
     def get_course_errors(self, course_key):
         """
@@ -484,6 +502,14 @@ class ModuleStoreReadBase(ModuleStoreRead):
         # default is to say yes by not raising an exception
         return {'default_impl': True}
 
+    def close_connections(self):
+        """
+        Closes any open connections to the underlying databases
+        """
+        if self.contentstore:
+            self.contentstore.close_connections()
+        super(ModuleStoreReadBase, self).close_connections()
+
     @contextmanager
     def default_store(self, store_type):
         """
@@ -511,9 +537,8 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
     Implement interface functionality that can be shared.
     '''
     def __init__(self, contentstore, **kwargs):
-        super(ModuleStoreWriteBase, self).__init__(**kwargs)
+        super(ModuleStoreWriteBase, self).__init__(contentstore=contentstore, **kwargs)
 
-        self.contentstore = contentstore
         # TODO: Don't have a runtime just to generate the appropriate mixin classes (cpennington)
         # This is only used by partition_fields_by_scope, which is only needed because
         # the split mongo store is used for item creation as well as item persistence
@@ -585,8 +610,9 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
         content.
         """
         # copy the assets
-        self.contentstore.copy_all_course_assets(source_course_id, dest_course_id)
-        super(ModuleStoreWriteBase, self).clone_course(source_course_id, dest_course_id, user_id)
+        if self.contentstore:
+            self.contentstore.copy_all_course_assets(source_course_id, dest_course_id)
+            super(ModuleStoreWriteBase, self).clone_course(source_course_id, dest_course_id, user_id)
         return dest_course_id
 
     def delete_course(self, course_key, user_id):
@@ -595,9 +621,19 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
         content.
         """
         # delete the assets
-        self.contentstore.delete_all_course_assets(course_key)
+        if self.contentstore:
+            self.contentstore.delete_all_course_assets(course_key)
         super(ModuleStoreWriteBase, self).delete_course(course_key, user_id)
 
+    def _drop_database(self):
+        """
+        A destructive operation to drop the underlying database and close all connections.
+        Intended to be used by test code for cleanup.
+        """
+        if self.contentstore:
+            self.contentstore._drop_database()  # pylint: disable=protected-access
+        super(ModuleStoreWriteBase, self)._drop_database()  # pylint: disable=protected-access
+
     @contextmanager
     def bulk_write_operations(self, course_id):
         """

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -393,11 +393,18 @@ class MixedModuleStore(ModuleStoreWriteBase):
         """
         Close all db connections
         """
-        for mstore in self.modulestores:
-            if hasattr(mstore, 'database'):
-                mstore.database.connection.close()
-            elif hasattr(mstore, 'db'):
-                mstore.db.connection.close()
+        for modulestore in self.modulestores:
+            modulestore.close_connections()
+
+    def _drop_database(self):
+        """
+        A destructive operation to drop all databases and close all db connections.
+        Intended to be used by test code for cleanup.
+        """
+        for modulestore in self.modulestores:
+            # drop database if the store supports it (read-only stores do not)
+            if hasattr(modulestore, '_drop_database'):
+                modulestore._drop_database()  # pylint: disable=protected-access
 
     def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}):
         """

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -394,6 +394,24 @@ class MongoModuleStore(ModuleStoreWriteBase):
         self.ignore_write_events_on_courses = set()
         self._course_run_cache = {}
 
+    def close_connections(self):
+        """
+        Closes any open connections to the underlying database
+        """
+        self.collection.database.connection.close()
+
+    def _drop_database(self):
+        """
+        A destructive operation to drop the underlying database and close all connections.
+        Intended to be used by test code for cleanup.
+        """
+        # drop the assets
+        super(MongoModuleStore, self)._drop_database()
+
+        connection = self.collection.database.connection
+        connection.drop_database(self.collection.database)
+        connection.close()
+
     def _begin_bulk_write_operation(self, course_id):
         """
         Prevent updating the meta-data inheritance cache for the given course

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

@@ -135,6 +135,24 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         self.render_template = render_template
         self.i18n_service = i18n_service
 
+    def close_connections(self):
+        """
+        Closes any open connections to the underlying databases
+        """
+        self.db.connection.close()
+
+    def _drop_database(self):
+        """
+        A destructive operation to drop the underlying database and close all connections.
+        Intended to be used by test code for cleanup.
+        """
+        # drop the assets
+        super(SplitMongoModuleStore, self)._drop_database()
+
+        connection = self.db.connection
+        connection.drop_database(self.db.name)
+        connection.close()
+
     def cache_items(self, system, base_block_ids, course_key, depth=0, lazy=True):
         '''
         Handles caching of items once inheritance and any other one time

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

@@ -8,6 +8,7 @@ from django.contrib.auth.models import User
 from xmodule.modulestore.django import (
     modulestore, clear_existing_modulestores, loc_mapper
 )
+from xmodule.contentstore.django import _CONTENTSTORE
 from xmodule.modulestore import ModuleStoreEnum
 
 
@@ -187,27 +188,14 @@ class ModuleStoreTestCase(TestCase):
         return updated_course
 
     @staticmethod
-    def drop_mongo_collections(modulestore_type=ModuleStoreEnum.Type.mongo):
+    def drop_mongo_collections():
         """
         If using a Mongo-backed modulestore & contentstore, drop the collections.
         """
-        store = modulestore()
-        if hasattr(store, '_get_modulestore_by_type'):
-            store = store._get_modulestore_by_type(modulestore_type)  # pylint: disable=W0212
-        if hasattr(store, 'collection'):
-            connection = store.collection.database.connection
-            store.collection.drop()
-            connection.drop_database(store.collection.database.name)
-            connection.close()
-        elif hasattr(store, 'close_all_connections'):
-            store.close_all_connections()
-        elif hasattr(store, 'db'):
-            connection = store.db.connection
-            connection.drop_database(store.db.name)
-            connection.close()
-
-        if hasattr(store, 'contentstore'):
-            store.contentstore.drop_database()
+        module_store = modulestore()
+        if hasattr(module_store, '_drop_database'):
+            module_store._drop_database()  # pylint: disable=protected-access
+        _CONTENTSTORE.clear()
 
         location_mapper = loc_mapper()
         if location_mapper.db:

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

@@ -57,7 +57,7 @@ class TestContentstore(unittest.TestCase):
         # since MongoModuleStore and MongoContentStore are basically assumed to be together, create this class
         # as well
         self.contentstore = MongoContentStore(HOST, DB, port=PORT)
-        self.addCleanup(self.contentstore.drop_database)
+        self.addCleanup(self.contentstore._drop_database)  # pylint: disable=protected-access
 
         setattr(AssetLocation, 'deprecated', deprecated)
         setattr(SlashSeparatedCourseKey, 'deprecated', deprecated)

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

@@ -74,7 +74,6 @@ class TestMongoModuleStore(unittest.TestCase):
             tz_aware=True,
             document_class=dict,
         )
-        cls.connection.drop_database(DB)
 
         # NOTE: Creating a single db for all the tests to save time.  This
         # is ok only as long as none of the tests modify the db.

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

@@ -50,10 +50,8 @@ class ModuleStoreNoSettings(unittest.TestCase):
         """
         cleanup
         """
-        if modulestore:
-            connection = self.modulestore.database.connection
-            connection.drop_database(self.modulestore.database)
-            connection.close()
+        if self.modulestore:
+            self.modulestore._drop_database()  # pylint: disable=protected-access
 
     def setUp(self):
         """