Revert "feat: store split modulestore's course indexes in Django/MySQL"
This reverts commit 96e5ff8dce.
This commit is contained in:
David Ormsbee
@@ -13,14 +13,11 @@ from contextlib import contextmanager
|
||||
from time import time
|
||||
|
||||
from django.core.cache import caches, InvalidCacheBackendError
|
||||
from django.db.transaction import TransactionManagementError
|
||||
import pymongo
|
||||
import pytz
|
||||
from mongodb_proxy import autoretry_read
|
||||
# Import this just to export it
|
||||
from pymongo.errors import DuplicateKeyError # pylint: disable=unused-import
|
||||
|
||||
from common.djangoapps.split_modulestore_django.models import SplitModulestoreCourseIndex
|
||||
from xmodule.exceptions import HeartbeatFailure
|
||||
from xmodule.modulestore import BlockData
|
||||
from xmodule.modulestore.split_mongo import BlockKey
|
||||
@@ -246,7 +243,7 @@ class CourseStructureCache:
|
||||
self.cache.set(key, compressed_pickled_data, None)
|
||||
|
||||
|
||||
class MongoPersistenceBackend:
|
||||
class MongoConnection:
|
||||
"""
|
||||
Segregation of pymongo functions from the data modeling mechanisms for split modulestore.
|
||||
"""
|
||||
@@ -432,18 +429,15 @@ class MongoPersistenceBackend:
|
||||
|
||||
return courses_queries
|
||||
|
||||
def insert_course_index(self, course_index, course_context=None, last_update_already_set=False):
|
||||
def insert_course_index(self, course_index, course_context=None):
|
||||
"""
|
||||
Create the course_index in the db
|
||||
"""
|
||||
with TIMER.timer("insert_course_index", course_context):
|
||||
# Set last_update which is used to avoid collisions, unless a subclass already set it before calling super()
|
||||
if not last_update_already_set:
|
||||
course_index['last_update'] = datetime.datetime.now(pytz.utc)
|
||||
# Insert the new index:
|
||||
course_index['last_update'] = datetime.datetime.now(pytz.utc)
|
||||
self.course_index.insert_one(course_index)
|
||||
|
||||
def update_course_index(self, course_index, from_index=None, course_context=None, last_update_already_set=False):
|
||||
def update_course_index(self, course_index, from_index=None, course_context=None):
|
||||
"""
|
||||
Update the db record for course_index.
|
||||
|
||||
@@ -463,10 +457,7 @@ class MongoPersistenceBackend:
|
||||
'course': course_index['course'],
|
||||
'run': course_index['run'],
|
||||
}
|
||||
# Set last_update which is used to avoid collisions, unless a subclass already set it before calling super()
|
||||
if not last_update_already_set:
|
||||
course_index['last_update'] = datetime.datetime.now(pytz.utc)
|
||||
# Update the course index:
|
||||
course_index['last_update'] = datetime.datetime.now(pytz.utc)
|
||||
self.course_index.replace_one(query, course_index, upsert=False,)
|
||||
|
||||
def delete_course_index(self, course_key):
|
||||
@@ -560,167 +551,3 @@ class MongoPersistenceBackend:
|
||||
|
||||
if connections:
|
||||
connection.close()
|
||||
|
||||
|
||||
class DjangoFlexPersistenceBackend(MongoPersistenceBackend):
|
||||
"""
|
||||
Backend for split mongo that can read/write from MySQL and/or S3 instead of Mongo,
|
||||
either partially replacing MongoDB or fully replacing it.
|
||||
"""
|
||||
|
||||
# Structures and definitions are only supported in MongoDB for now.
|
||||
# Course indexes are read from MySQL and written to both MongoDB and MySQL
|
||||
|
||||
def get_course_index(self, key, ignore_case=False):
|
||||
"""
|
||||
Get the course_index from the persistence mechanism whose id is the given key
|
||||
"""
|
||||
if key.version_guid and not key.org:
|
||||
# I don't think it was intentional, but with the MongoPersistenceBackend, using a key with only a version
|
||||
# guid and no org/course/run value would not raise an error, but would always return None. So we need to be
|
||||
# compatible with that.
|
||||
# e.g. test_split_modulestore.py:SplitModuleCourseTests.test_get_course -> get_course(key with only version)
|
||||
# > _load_items > cache_items > begin bulk operations > get_course_index > results in this situation.
|
||||
log.warning("DjangoFlexPersistenceBackend: get_course_index without org/course/run will always return None")
|
||||
return None
|
||||
# We never include the branch or the version in the course key in the SplitModulestoreCourseIndex table:
|
||||
key = key.for_branch(None).version_agnostic()
|
||||
if not ignore_case:
|
||||
query = {"course_id": key}
|
||||
else:
|
||||
# Case insensitive search is important when creating courses to reject course IDs that differ only by
|
||||
# capitalization.
|
||||
query = {"course_id__iexact": key}
|
||||
try:
|
||||
return SplitModulestoreCourseIndex.objects.get(**query).as_v1_schema()
|
||||
except SplitModulestoreCourseIndex.DoesNotExist:
|
||||
return None
|
||||
|
||||
def find_matching_course_indexes( # pylint: disable=arguments-differ
|
||||
self,
|
||||
branch=None,
|
||||
search_targets=None,
|
||||
org_target=None,
|
||||
course_context=None,
|
||||
course_keys=None,
|
||||
force_mongo=False,
|
||||
):
|
||||
"""
|
||||
Find the course_index matching particular conditions.
|
||||
|
||||
Arguments:
|
||||
branch: If specified, this branch must exist in the returned courses
|
||||
search_targets: If specified, this must be a dictionary specifying field values
|
||||
that must exist in the search_targets of the returned courses
|
||||
org_target: If specified, this is an ORG filter so that only course_indexs are
|
||||
returned for the specified ORG
|
||||
"""
|
||||
if force_mongo:
|
||||
# For data migration purposes, this argument will read from MongoDB instead of MySQL
|
||||
return super().find_matching_course_indexes(
|
||||
branch=branch, search_targets=search_targets, org_target=org_target,
|
||||
course_context=course_context, course_keys=course_keys,
|
||||
)
|
||||
queryset = SplitModulestoreCourseIndex.objects.all()
|
||||
if course_keys:
|
||||
queryset = queryset.filter(course_id__in=course_keys)
|
||||
if search_targets:
|
||||
if "wiki_slug" in search_targets:
|
||||
queryset = queryset.filter(wiki_slug=search_targets.pop("wiki_slug"))
|
||||
if search_targets: # If there are any search targets besides wiki_slug (which we've handled by this point):
|
||||
raise ValueError(f"Unsupported search_targets: {', '.join(search_targets.keys())}")
|
||||
if org_target:
|
||||
queryset = queryset.filter(org=org_target)
|
||||
if branch is not None:
|
||||
branch_field = SplitModulestoreCourseIndex.field_name_for_branch(branch)
|
||||
queryset = queryset.exclude(**{branch_field: ""})
|
||||
|
||||
return (course_index.as_v1_schema() for course_index in queryset)
|
||||
|
||||
def insert_course_index(self, course_index, course_context=None): # pylint: disable=arguments-differ
|
||||
"""
|
||||
Create the course_index in the db
|
||||
"""
|
||||
course_index['last_update'] = datetime.datetime.now(pytz.utc)
|
||||
new_index = SplitModulestoreCourseIndex(**SplitModulestoreCourseIndex.fields_from_v1_schema(course_index))
|
||||
new_index.save()
|
||||
# TEMP: Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well:
|
||||
super().insert_course_index(course_index, course_context, last_update_already_set=True)
|
||||
|
||||
def update_course_index(self, course_index, from_index=None, course_context=None): # pylint: disable=arguments-differ
|
||||
"""
|
||||
Update the db record for course_index.
|
||||
|
||||
Arguments:
|
||||
from_index: If set, only update an index if it matches the one specified in `from_index`.
|
||||
|
||||
Exceptions:
|
||||
SplitModulestoreCourseIndex.DoesNotExist: If the given object_id is not valid
|
||||
"""
|
||||
# "last_update not only tells us when this course was last updated but also helps prevent collisions"
|
||||
# This code is just copying the behavior of the existing MongoPersistenceBackend
|
||||
# See https://github.com/edx/edx-platform/pull/5200 for context
|
||||
course_index['last_update'] = datetime.datetime.now(pytz.utc)
|
||||
# Find the SplitModulestoreCourseIndex entry that we'll be updating:
|
||||
index_obj = SplitModulestoreCourseIndex.objects.get(objectid=course_index["_id"])
|
||||
|
||||
# Check for collisions:
|
||||
if from_index and index_obj.last_update != from_index["last_update"]:
|
||||
# "last_update not only tells us when this course was last updated but also helps prevent collisions"
|
||||
log.warning(
|
||||
"Collision in Split Mongo when applying course index. This can happen in dev if django debug toolbar "
|
||||
"is enabled, as it slows down parallel queries. New index was: %s",
|
||||
course_index,
|
||||
)
|
||||
return # Collision; skip this update
|
||||
|
||||
# Apply updates to the index entry. While doing so, track which branch versions were changed (if any).
|
||||
changed_branches = []
|
||||
for attr, value in SplitModulestoreCourseIndex.fields_from_v1_schema(course_index).items():
|
||||
if attr in ("objectid", "course_id"):
|
||||
# Enforce these attributes as immutable.
|
||||
if getattr(index_obj, attr) != value:
|
||||
raise ValueError(
|
||||
f"Attempted to change the {attr} key of a course index entry ({index_obj.course_id})"
|
||||
)
|
||||
else:
|
||||
if attr.endswith("_version"):
|
||||
# Model fields ending in _version are branches. If the branch version has changed, convert the field
|
||||
# name to a branch name and report it in the history below.
|
||||
if getattr(index_obj, attr) != value:
|
||||
changed_branches.append(attr[:-8])
|
||||
setattr(index_obj, attr, value)
|
||||
if changed_branches:
|
||||
# For the django simple history, indicate what was changed. Unfortunately at this point we only really know
|
||||
# which branch(es) were changed, not anything more useful than that.
|
||||
index_obj._change_reason = f'Updated {" and ".join(changed_branches)} branch' # pylint: disable=protected-access
|
||||
|
||||
# Save the course index entry and create a historical record:
|
||||
index_obj.save()
|
||||
# TEMP: Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well:
|
||||
super().update_course_index(course_index, from_index, course_context, last_update_already_set=True)
|
||||
|
||||
def delete_course_index(self, course_key):
|
||||
"""
|
||||
Delete the course_index from the persistence mechanism whose id is the given course_index
|
||||
"""
|
||||
SplitModulestoreCourseIndex.objects.filter(course_id=course_key).delete()
|
||||
# TEMP: Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well:
|
||||
super().delete_course_index(course_key)
|
||||
|
||||
def _drop_database(self, database=True, collections=True, connections=True):
|
||||
"""
|
||||
Reset data for testing.
|
||||
"""
|
||||
try:
|
||||
SplitModulestoreCourseIndex.objects.all().delete()
|
||||
except TransactionManagementError as err:
|
||||
# If the test doesn't use 'with self.allow_transaction_exception():', then this error can occur and it may
|
||||
# be non-obvious why, so give a very clear explanation of how to fix it. See the docstring of
|
||||
# allow_transaction_exception() for more details.
|
||||
raise RuntimeError(
|
||||
"post-test cleanup failed with TransactionManagementError. "
|
||||
"Use 'with self.allow_transaction_exception():' from ModuleStoreTestCase/...IsolationMixin to fix it."
|
||||
) from err
|
||||
# TEMP: Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well:
|
||||
super()._drop_database(database, collections, connections)
|
||||
|
||||
@@ -101,7 +101,7 @@ from xmodule.modulestore.exceptions import (
|
||||
VersionConflictError
|
||||
)
|
||||
from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope
|
||||
from xmodule.modulestore.split_mongo.mongo_connection import DuplicateKeyError, DjangoFlexPersistenceBackend
|
||||
from xmodule.modulestore.split_mongo.mongo_connection import DuplicateKeyError, MongoConnection
|
||||
from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES
|
||||
from xmodule.partitions.partitions_service import PartitionService
|
||||
|
||||
@@ -651,7 +651,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
|
||||
super().__init__(contentstore, **kwargs)
|
||||
|
||||
self.db_connection = DjangoFlexPersistenceBackend(**doc_store_config)
|
||||
self.db_connection = MongoConnection(**doc_store_config)
|
||||
|
||||
if default_class is not None:
|
||||
module_path, __, class_name = default_class.rpartition('.')
|
||||
|
||||
@@ -12,7 +12,7 @@ from unittest.mock import patch
|
||||
|
||||
from django.conf import settings
|
||||
from django.contrib.auth.models import AnonymousUser
|
||||
from django.db import connections, transaction
|
||||
from django.db import connections
|
||||
from django.test import TestCase
|
||||
from django.test.utils import override_settings
|
||||
|
||||
@@ -331,30 +331,6 @@ class ModuleStoreIsolationMixin(CacheIsolationMixin, SignalIsolationMixin):
|
||||
cls.end_cache_isolation()
|
||||
cls.enable_all_signals()
|
||||
|
||||
@staticmethod
|
||||
def allow_transaction_exception():
|
||||
"""
|
||||
Context manager to wrap modulestore-using test code that may throw an exception.
|
||||
|
||||
(Use this if a modulestore test is failing with TransactionManagementError during cleanup.)
|
||||
|
||||
Details:
|
||||
Some test cases that purposely throw an exception may normally cause the end_modulestore_isolation() cleanup
|
||||
step to fail with
|
||||
TransactionManagementError:
|
||||
An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.
|
||||
This happens because the test is wrapped in an implicit transaction and when the exception occurs, django won't
|
||||
allow any subsequent database queries in the same transaction - in particular, the queries needed to clean up
|
||||
split modulestore's SplitModulestoreCourseIndex table after the test.
|
||||
|
||||
By wrapping the inner part of the test in this atomic() call, we create a savepoint so that if an exception is
|
||||
thrown, Django merely rolls back to the savepoint and the overall transaction continues, including the eventual
|
||||
cleanup step.
|
||||
|
||||
This method mostly exists to provide this docstring/explanation; the code itself is trivial.
|
||||
"""
|
||||
return transaction.atomic()
|
||||
|
||||
|
||||
class ModuleStoreTestUsersMixin():
|
||||
"""
|
||||
|
||||
@@ -23,21 +23,17 @@ class TestLibraries(MixedSplitTestCase):
|
||||
|
||||
def test_create_library(self):
|
||||
"""
|
||||
Test that we can create a library, and see how many database calls it uses to do so.
|
||||
Test that we can create a library, and see how many mongo calls it uses to do so.
|
||||
|
||||
Expected mongo calls, in order:
|
||||
-> insert(definition: {'block_type': 'library', 'fields': {}})
|
||||
-> insert_structure(bulk)
|
||||
-> insert_course_index(bulk)
|
||||
find_one({'org': '...', 'run': 'library', 'course': '...'})
|
||||
insert(definition: {'block_type': 'library', 'fields': {}})
|
||||
|
||||
Expected MySQL calls in order:
|
||||
-> SELECT from SplitModulestoreCourseIndex case insensitive search for existing libraries
|
||||
-> SELECT from SplitModulestoreCourseIndex lookup library with that exact ID
|
||||
-> SELECT from XBlockConfiguration (?)
|
||||
-> INSERT into SplitModulestoreCourseIndex to save the new library
|
||||
-> INSERT a historical record of the SplitModulestoreCourseIndex
|
||||
insert_structure(bulk)
|
||||
insert_course_index(bulk)
|
||||
get_course_index(bulk)
|
||||
"""
|
||||
with check_mongo_calls(0, 3), self.assertNumQueries(5):
|
||||
with check_mongo_calls(2, 3):
|
||||
LibraryFactory.create(modulestore=self.store)
|
||||
|
||||
def test_duplicate_library(self):
|
||||
|
||||
@@ -366,9 +366,8 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# Draft:
|
||||
# problem: One lookup to locate an item that exists
|
||||
# fake: one w/ wildcard version
|
||||
# split: has one lookup for the course and then one for the course items
|
||||
# but the active_versions check is done in MySQL
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, [1, 1], 0), (ModuleStoreEnum.Type.split, [1, 1], 0))
|
||||
# split has one lookup for the course and then one for the course items
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, [1, 1], 0), (ModuleStoreEnum.Type.split, [2, 2], 0))
|
||||
@ddt.unpack
|
||||
def test_has_item(self, default_ms, max_find, max_send):
|
||||
self.initdb(default_ms)
|
||||
@@ -391,17 +390,17 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# split:
|
||||
# problem: active_versions, structure
|
||||
# non-existent problem: ditto
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, [3, 2], 0), (ModuleStoreEnum.Type.split, 1, [1, 1], 0))
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, [3, 2], 0), (ModuleStoreEnum.Type.split, [2, 2], 0))
|
||||
@ddt.unpack
|
||||
def test_get_item(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_get_item(self, default_ms, max_find, max_send):
|
||||
self.initdb(default_ms)
|
||||
self._create_block_hierarchy()
|
||||
|
||||
with check_mongo_calls(max_find.pop(0), max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find.pop(0), max_send):
|
||||
assert self.store.get_item(self.problem_x1a_1) is not None # lint-amnesty, pylint: disable=no-member
|
||||
|
||||
# try negative cases
|
||||
with check_mongo_calls(max_find.pop(0), max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find.pop(0), max_send):
|
||||
with pytest.raises(ItemNotFoundError):
|
||||
self.store.get_item(self.fake_location)
|
||||
|
||||
@@ -412,16 +411,15 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# Draft:
|
||||
# wildcard query, 6! load pertinent items for inheritance calls, load parents, course root fetch (why)
|
||||
# Split:
|
||||
# mysql: fetch course's active version from SplitModulestoreCourseIndex, spurious refetch x2
|
||||
# find: get structure
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 14, 0), (ModuleStoreEnum.Type.split, 3, 1, 0))
|
||||
# active_versions (with regex), structure, and spurious active_versions refetch
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 14, 0), (ModuleStoreEnum.Type.split, 4, 0))
|
||||
@ddt.unpack
|
||||
def test_get_items(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_get_items(self, default_ms, max_find, max_send):
|
||||
self.initdb(default_ms)
|
||||
self._create_block_hierarchy()
|
||||
|
||||
course_locn = self.course_locations[self.MONGO_COURSEID]
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
modules = self.store.get_items(course_locn.course_key, qualifiers={'category': 'problem'})
|
||||
assert len(modules) == 6
|
||||
|
||||
@@ -515,16 +513,13 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
assert (orphan in [item.location for item in items_in_tree]) == orphan_in_items
|
||||
assert len(items_in_tree) == expected_items_in_tree
|
||||
|
||||
# draft:
|
||||
# find: get draft, get ancestors up to course (2-6), compute inheritance
|
||||
# draft: get draft, get ancestors up to course (2-6), compute inheritance
|
||||
# sends: update problem and then each ancestor up to course (edit info)
|
||||
# split:
|
||||
# mysql: SplitModulestoreCourseIndex - select 2x (by course_id, by objectid), update, update historical record
|
||||
# find: definitions (calculator field), structures
|
||||
# sends: 2 sends to update index & structure (note, it would also be definition if a content field changed)
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 7, 5), (ModuleStoreEnum.Type.split, 4, 2, 2))
|
||||
# split: active_versions, definitions (calculator field), structures
|
||||
# 2 sends to update index & structure (note, it would also be definition if a content field changed)
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 7, 5), (ModuleStoreEnum.Type.split, 3, 2))
|
||||
@ddt.unpack
|
||||
def test_update_item(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_update_item(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
Update should succeed for r/w dbs
|
||||
"""
|
||||
@@ -534,7 +529,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# if following raised, then the test is really a noop, change it
|
||||
assert problem.max_attempts != 2, 'Default changed making test meaningless'
|
||||
problem.max_attempts = 2
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
problem = self.store.update_item(problem, self.user_id)
|
||||
|
||||
assert problem.max_attempts == 2, "Update didn't persist"
|
||||
@@ -914,12 +909,11 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# get item (to delete subtree), get inheritance again.
|
||||
# Sends: delete item, update parent
|
||||
# Split
|
||||
# mysql: SplitModulestoreCourseIndex - select 2x (by course_id, by objectid), update, update historical record
|
||||
# Find: active_versions, 2 structures (published & draft), definition (unnecessary)
|
||||
# Sends: updated draft and published structures and active_versions
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 7, 2), (ModuleStoreEnum.Type.split, 4, 2, 3))
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 7, 2), (ModuleStoreEnum.Type.split, 3, 3))
|
||||
@ddt.unpack
|
||||
def test_delete_item(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_delete_item(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
Delete should reject on r/o db and work on r/w one
|
||||
"""
|
||||
@@ -928,7 +922,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
max_find += 1
|
||||
|
||||
with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.writable_chapter_location.course_key): # lint-amnesty, pylint: disable=line-too-long
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
self.store.delete_item(self.writable_chapter_location, self.user_id)
|
||||
|
||||
# verify it's gone
|
||||
@@ -939,16 +933,15 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
self.store.get_item(self.writable_chapter_location, revision=ModuleStoreEnum.RevisionOption.published_only)
|
||||
|
||||
# Draft:
|
||||
# find: find parent (definition.children), count versions of item, get parent, count grandparents,
|
||||
# inheritance items, draft item, draft child, inheritance
|
||||
# queries: find parent (definition.children), count versions of item, get parent, count grandparents,
|
||||
# inheritance items, draft item, draft child, inheritance
|
||||
# sends: delete draft vertical and update parent
|
||||
# Split:
|
||||
# mysql: SplitModulestoreCourseIndex - select 2x (by course_id, by objectid), update, update historical record
|
||||
# find: draft and published structures, definition (unnecessary)
|
||||
# queries: active_versions, draft and published structures, definition (unnecessary)
|
||||
# sends: update published (why?), draft, and active_versions
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 9, 2), (ModuleStoreEnum.Type.split, 4, 3, 3))
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 9, 2), (ModuleStoreEnum.Type.split, 4, 3))
|
||||
@ddt.unpack
|
||||
def test_delete_private_vertical(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_delete_private_vertical(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
Because old mongo treated verticals as the first layer which could be draft, it has some interesting
|
||||
behavioral properties which this deletion test gets at.
|
||||
@@ -979,7 +972,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
assert vert_loc in course.children
|
||||
|
||||
# delete the vertical and ensure the course no longer points to it
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
self.store.delete_item(vert_loc, self.user_id)
|
||||
course = self.store.get_course(self.course_locations[self.MONGO_COURSEID].course_key, 0)
|
||||
if hasattr(private_vert.location, 'version_guid'):
|
||||
@@ -997,12 +990,11 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# find: find parent (definition.children) 2x, find draft item, get inheritance items
|
||||
# send: one delete query for specific item
|
||||
# Split:
|
||||
# mysql: SplitModulestoreCourseIndex - select 2x (by course_id, by objectid), update, update historical record
|
||||
# find: structure (cached)
|
||||
# find: active_version & structure (cached)
|
||||
# send: update structure and active_versions
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 4, 1), (ModuleStoreEnum.Type.split, 4, 1, 2))
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 4, 1), (ModuleStoreEnum.Type.split, 2, 2))
|
||||
@ddt.unpack
|
||||
def test_delete_draft_vertical(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_delete_draft_vertical(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
Test deleting a draft vertical which has a published version.
|
||||
"""
|
||||
@@ -1032,23 +1024,23 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# test succeeds if delete succeeds w/o error
|
||||
if default_ms == ModuleStoreEnum.Type.mongo and mongo_uses_error_check(self.store):
|
||||
max_find += 1
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
self.store.delete_item(private_leaf.location, self.user_id)
|
||||
|
||||
# Draft:
|
||||
# mysql: 1 select on SplitModulestoreCourseIndex since this searches both modulestores
|
||||
# find: 1 find all courses (wildcard), 1 find to get each course 1 at a time (1 course)
|
||||
# 1) find all courses (wildcard),
|
||||
# 2) get each course 1 at a time (1 course),
|
||||
# 3) wildcard split if it has any (1) but it doesn't
|
||||
# Split:
|
||||
# mysql: 3 selects on SplitModulestoreCourseIndex - 1 to get all courses, 2 to get specific course (this query is
|
||||
# executed twice, possibly unnecessarily)
|
||||
# find: 2 reads of structure, definition (s/b lazy; so, unnecessary),
|
||||
# plus 1 wildcard find in draft mongo which has none
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 1, 2, 0), (ModuleStoreEnum.Type.split, 3, 3, 0))
|
||||
# 1) wildcard split search,
|
||||
# 2-4) active_versions, structure, definition (s/b lazy; so, unnecessary)
|
||||
# 5) wildcard draft mongo which has none
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 3, 0), (ModuleStoreEnum.Type.split, 6, 0))
|
||||
@ddt.unpack
|
||||
def test_get_courses(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_get_courses(self, default_ms, max_find, max_send):
|
||||
self.initdb(default_ms)
|
||||
# we should have one course across all stores
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
courses = self.store.get_courses()
|
||||
course_ids = [course.location for course in courses]
|
||||
assert len(courses) == 1, f'Not one course: {course_ids}'
|
||||
@@ -1082,16 +1074,16 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
assert len(mongo_course.children) == 1
|
||||
|
||||
# draft is 2: find out which ms owns course, get item
|
||||
# split: active_versions (mysql), structure, definition (to load course wiki string)
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 2, 0), (ModuleStoreEnum.Type.split, 1, 2, 0))
|
||||
# split: active_versions, structure, definition (to load course wiki string)
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 2, 0), (ModuleStoreEnum.Type.split, 3, 0))
|
||||
@ddt.unpack
|
||||
def test_get_course(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_get_course(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
This test is here for the performance comparison not functionality. It tests the performance
|
||||
of getting an item whose scope.content fields are looked at.
|
||||
"""
|
||||
self.initdb(default_ms)
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
course = self.store.get_item(self.course_locations[self.MONGO_COURSEID])
|
||||
assert course.id == self.course_locations[self.MONGO_COURSEID].course_key
|
||||
|
||||
@@ -1120,16 +1112,16 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# still only 2)
|
||||
# Draft: get_parent
|
||||
# Split: active_versions, structure
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 1, 0), (ModuleStoreEnum.Type.split, 1, 1, 0))
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0))
|
||||
@ddt.unpack
|
||||
def test_get_parent_locations(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_get_parent_locations(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
Test a simple get parent for a direct only category (i.e, always published)
|
||||
"""
|
||||
self.initdb(default_ms)
|
||||
self._create_block_hierarchy()
|
||||
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
parent = self.store.get_parent_location(self.problem_x1a_1) # lint-amnesty, pylint: disable=no-member
|
||||
assert parent == self.vertical_x1a # lint-amnesty, pylint: disable=no-member
|
||||
|
||||
@@ -1637,10 +1629,10 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# 7-8. get sequential, compute inheritance
|
||||
# 8-9. get vertical, compute inheritance
|
||||
# 10-11. get other vertical_x1b (why?) and compute inheritance
|
||||
# Split: loading structure from mongo (also loads active version from MySQL, not tracked here)
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, [12, 3], 0), (ModuleStoreEnum.Type.split, 1, [2, 1], 0))
|
||||
# Split: active_versions & structure
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, [12, 3], 0), (ModuleStoreEnum.Type.split, [3, 2], 0))
|
||||
@ddt.unpack
|
||||
def test_path_to_location(self, default_ms, num_mysql, num_finds, num_sends):
|
||||
def test_path_to_location(self, default_ms, num_finds, num_sends):
|
||||
"""
|
||||
Make sure that path_to_location works
|
||||
"""
|
||||
@@ -1659,7 +1651,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
|
||||
for location, expected in should_work:
|
||||
# each iteration has different find count, pop this iter's find count
|
||||
with check_mongo_calls(num_finds.pop(0), num_sends), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(num_finds.pop(0), num_sends):
|
||||
path = path_to_location(self.store, location)
|
||||
assert path == expected
|
||||
|
||||
@@ -1876,10 +1868,10 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
assert False, "SplitMongoModuleStore was not found in MixedModuleStore"
|
||||
|
||||
# Draft: get all items which can be or should have parents
|
||||
# Split: active_versions (mysql), structure (mongo)
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 1, 0), (ModuleStoreEnum.Type.split, 1, 1, 0))
|
||||
# Split: active_versions, structure
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0))
|
||||
@ddt.unpack
|
||||
def test_get_orphans(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_get_orphans(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
Test finding orphans.
|
||||
"""
|
||||
@@ -1911,7 +1903,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
block_id=location.block_id
|
||||
)
|
||||
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
found_orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key)
|
||||
self.assertCountEqual(found_orphans, orphan_locations)
|
||||
|
||||
@@ -2012,17 +2004,17 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
assert self.user_id == block.subtree_edited_by
|
||||
assert datetime.datetime.now(UTC) > block.subtree_edited_on
|
||||
|
||||
# Draft: wildcard search of draft (find) and split (mysql)
|
||||
# Split: wildcard search of draft (find) and split (mysql)
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 1, 1, 0), (ModuleStoreEnum.Type.split, 1, 1, 0))
|
||||
# Draft: wildcard search of draft and split
|
||||
# Split: wildcard search of draft and split
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 2, 0), (ModuleStoreEnum.Type.split, 2, 0))
|
||||
@ddt.unpack
|
||||
def test_get_courses_for_wiki(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_get_courses_for_wiki(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
Test the get_courses_for_wiki method
|
||||
"""
|
||||
self.initdb(default_ms)
|
||||
# Test Mongo wiki
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
wiki_courses = self.store.get_courses_for_wiki('999')
|
||||
assert len(wiki_courses) == 1
|
||||
assert self.course_locations[self.MONGO_COURSEID].course_key.replace(branch=None) in wiki_courses
|
||||
@@ -2036,18 +2028,13 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# 1. delete all of the published nodes in subtree
|
||||
# 2. insert vertical as published (deleted in step 1) w/ the deleted problems as children
|
||||
# 3-6. insert the 3 problems and 1 html as published
|
||||
# Split:
|
||||
# MySQL SplitModulestoreCourseIndex:
|
||||
# 1. Select by course ID
|
||||
# 2. Select by objectid
|
||||
# 3-4. Update index version, update historical record
|
||||
# Find: 2 structures (pre & post published?)
|
||||
# Sends:
|
||||
# 1. insert structure
|
||||
# 2. write index entry
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 2, 6), (ModuleStoreEnum.Type.split, 4, 2, 2))
|
||||
# Split: active_versions, 2 structures (pre & post published?)
|
||||
# Sends:
|
||||
# - insert structure
|
||||
# - write index entry
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 2, 6), (ModuleStoreEnum.Type.split, 3, 2))
|
||||
@ddt.unpack
|
||||
def test_unpublish(self, default_ms, num_mysql, max_find, max_send):
|
||||
def test_unpublish(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
Test calling unpublish
|
||||
"""
|
||||
@@ -2065,7 +2052,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
assert published_xblock is not None
|
||||
|
||||
# unpublish
|
||||
with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
self.store.unpublish(self.vertical_x1a, self.user_id) # lint-amnesty, pylint: disable=no-member
|
||||
|
||||
with pytest.raises(ItemNotFoundError):
|
||||
@@ -2082,10 +2069,10 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
assert draft_xblock is not None
|
||||
|
||||
# Draft: specific query for revision None
|
||||
# Split: active_versions from MySQL, structure from mongo
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 0, 1, 0), (ModuleStoreEnum.Type.split, 1, 1, 0))
|
||||
# Split: active_versions, structure
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0))
|
||||
@ddt.unpack
|
||||
def test_has_published_version(self, default_ms, mysql_queries, max_find, max_send):
|
||||
def test_has_published_version(self, default_ms, max_find, max_send):
|
||||
"""
|
||||
Test the has_published_version method
|
||||
"""
|
||||
@@ -2095,7 +2082,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
# start off as Private
|
||||
item = self.store.create_child(self.user_id, self.writable_chapter_location, 'problem', 'test_compute_publish_state') # lint-amnesty, pylint: disable=line-too-long
|
||||
item_location = item.location
|
||||
with self.assertNumQueries(mysql_queries), check_mongo_calls(max_find, max_send):
|
||||
with check_mongo_calls(max_find, max_send):
|
||||
assert not self.store.has_published_version(item)
|
||||
|
||||
# Private -> Public
|
||||
@@ -3784,7 +3771,7 @@ class TestAsidesWithMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
assert asides2[0].field11 == 'aside1_default_value1'
|
||||
assert asides2[0].field12 == 'aside1_default_value2'
|
||||
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 1, 0))
|
||||
@ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0))
|
||||
@XBlockAside.register_temp_plugin(AsideFoo, 'test_aside1')
|
||||
@patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types',
|
||||
lambda self, block: ['test_aside1'])
|
||||
|
||||
@@ -154,14 +154,14 @@ class CountMongoCallsCourseTraversal(TestCase):
|
||||
(MIXED_OLD_MONGO_MODULESTORE_BUILDER, 0, True, False, 359),
|
||||
# The line below shows the way this traversal *should* be done
|
||||
# (if you'll eventually access all the fields and load all the definitions anyway).
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, None, False, True, 2),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, None, True, True, 37),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, True, 37),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, True, 37),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, None, False, False, 2),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, None, True, False, 2),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, False, 2),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, False, 2),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, None, False, True, 3),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, None, True, True, 38),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, True, 38),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, True, 38),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, None, False, False, 3),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, None, True, False, 3),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, False, 3),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, False, 3)
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_number_mongo_calls(self, store_builder, depth, lazy, access_all_block_fields, num_mongo_calls):
|
||||
@@ -178,7 +178,7 @@ class CountMongoCallsCourseTraversal(TestCase):
|
||||
|
||||
@ddt.data(
|
||||
(MIXED_OLD_MONGO_MODULESTORE_BUILDER, 176),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 3),
|
||||
(MIXED_SPLIT_MODULESTORE_BUILDER, 4),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_lazy_when_course_previously_cached(self, store_builder, num_mongo_calls):
|
||||
|
||||
@@ -163,7 +163,6 @@ class TestPublish(SplitWMongoCourseBootstrapper):
|
||||
assert self.draft_mongo.has_item(other_child_loc), 'Oops, lost moved item'
|
||||
|
||||
|
||||
@pytest.mark.django_db # required if using split modulestore
|
||||
class DraftPublishedOpTestCourseSetup(unittest.TestCase):
|
||||
"""
|
||||
This class exists to test XML import and export between different modulestore
|
||||
|
||||
@@ -53,7 +53,6 @@ TEST_ASSISTANT_USER_ID = ModuleStoreEnum.UserID.test - 12
|
||||
|
||||
|
||||
@attr('mongo')
|
||||
@pytest.mark.django_db
|
||||
class SplitModuleTest(unittest.TestCase):
|
||||
'''
|
||||
The base set of tests manually populates a db w/ courses which have
|
||||
|
||||
@@ -12,7 +12,7 @@ import ddt
|
||||
from bson.objectid import ObjectId
|
||||
from opaque_keys.edx.locator import CourseLocator
|
||||
|
||||
from xmodule.modulestore.split_mongo.mongo_connection import MongoPersistenceBackend
|
||||
from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection
|
||||
from xmodule.modulestore.split_mongo.split import SplitBulkWriteMixin
|
||||
|
||||
VERSION_GUID_DICT = {
|
||||
@@ -30,7 +30,7 @@ class TestBulkWriteMixin(unittest.TestCase): # lint-amnesty, pylint: disable=mi
|
||||
self.bulk = SplitBulkWriteMixin()
|
||||
self.bulk.SCHEMA_VERSION = 1
|
||||
self.clear_cache = self.bulk._clear_cache = Mock(name='_clear_cache')
|
||||
self.conn = self.bulk.db_connection = MagicMock(name='db_connection', spec=MongoPersistenceBackend)
|
||||
self.conn = self.bulk.db_connection = MagicMock(name='db_connection', spec=MongoConnection)
|
||||
self.conn.get_course_index.return_value = {'initial': 'index'}
|
||||
|
||||
self.course_key = CourseLocator('org', 'course', 'run-a', branch='test')
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
""" Test the behavior of split_mongo/MongoPersistenceBackend """
|
||||
""" Test the behavior of split_mongo/MongoConnection """
|
||||
|
||||
|
||||
import unittest
|
||||
@@ -8,7 +8,7 @@ import pytest
|
||||
from pymongo.errors import ConnectionFailure
|
||||
|
||||
from xmodule.exceptions import HeartbeatFailure
|
||||
from xmodule.modulestore.split_mongo.mongo_connection import MongoPersistenceBackend
|
||||
from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection
|
||||
|
||||
|
||||
class TestHeartbeatFailureException(unittest.TestCase):
|
||||
@@ -20,7 +20,7 @@ class TestHeartbeatFailureException(unittest.TestCase):
|
||||
# pylint: disable=W0613
|
||||
with patch('mongodb_proxy.MongoProxy') as mock_proxy:
|
||||
mock_proxy.return_value.admin.command.side_effect = ConnectionFailure('Test')
|
||||
useless_conn = MongoPersistenceBackend('useless', 'useless', 'useless')
|
||||
useless_conn = MongoConnection('useless', 'useless', 'useless')
|
||||
|
||||
with pytest.raises(HeartbeatFailure):
|
||||
useless_conn.heartbeat()
|
||||
|
||||
@@ -19,7 +19,6 @@ from xmodule.x_module import XModuleMixin
|
||||
|
||||
|
||||
@pytest.mark.mongo
|
||||
@pytest.mark.django_db
|
||||
class SplitWMongoCourseBootstrapper(unittest.TestCase):
|
||||
"""
|
||||
Helper for tests which need to construct split mongo & old mongo based courses to get interesting internal structure. # lint-amnesty, pylint: disable=line-too-long
|
||||
|
||||
@@ -34,7 +34,6 @@ _LAST_WEEK = _TODAY - timedelta(days=7)
|
||||
_NEXT_WEEK = _TODAY + timedelta(days=7)
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
class CourseMetadataUtilsTestCase(TestCase):
|
||||
"""
|
||||
Tests for course_metadata_utils.
|
||||
|
||||
Reference in New Issue
Block a user