diff --git a/cms/djangoapps/contentstore/tests/test_clone_course.py b/cms/djangoapps/contentstore/tests/test_clone_course.py index f6ef12fddba0..34b3ae705358 100644 --- a/cms/djangoapps/contentstore/tests/test_clone_course.py +++ b/cms/djangoapps/contentstore/tests/test_clone_course.py @@ -125,7 +125,7 @@ def test_rerun_course(self): ) # try to hit the generic exception catch - with patch('xmodule.modulestore.split_mongo.mongo_connection.MongoPersistenceBackend.insert_course_index', Mock(side_effect=Exception)): # lint-amnesty, pylint: disable=line-too-long + with patch('xmodule.modulestore.split_mongo.mongo_connection.MongoConnection.insert_course_index', Mock(side_effect=Exception)): # lint-amnesty, pylint: disable=line-too-long split_course4_id = CourseLocator(org="edx3", course="split3", run="rerun_fail") fields = {'display_name': 'total failure'} CourseRerunState.objects.initiated(split_course3_id, split_course4_id, self.user, fields['display_name']) diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index dcffbd795497..acd2fbfd82fd 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -157,8 +157,8 @@ def test_courses_list_with_ccx_courses(self): self.assertEqual(len(list(courses_iter)), 0) @ddt.data( - (ModuleStoreEnum.Type.split, 2), - (ModuleStoreEnum.Type.mongo, 1) + (ModuleStoreEnum.Type.split, 3), + (ModuleStoreEnum.Type.mongo, 2) ) @ddt.unpack def test_staff_course_listing(self, default_store, mongo_calls): @@ -239,8 +239,8 @@ def test_get_course_list_with_invalid_course_location(self, store): ) @ddt.data( - (ModuleStoreEnum.Type.split, 2, 2), - (ModuleStoreEnum.Type.mongo, 1, 1) + (ModuleStoreEnum.Type.split, 3, 3), + (ModuleStoreEnum.Type.mongo, 2, 2) ) @ddt.unpack def test_course_listing_performance(self, store, courses_list_from_group_calls, courses_list_calls): @@ -289,7 +289,7 @@ def test_course_listing_performance(self, store, courses_list_from_group_calls, # Calls: # 1) query old mongo # 2) get_more on old mongo - # 3) query split (handled with MySQL only) + # 3) query split (but no courses so no fetching of data) @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) def test_course_listing_errored_deleted_courses(self, store): diff --git a/cms/djangoapps/contentstore/tests/test_orphan.py b/cms/djangoapps/contentstore/tests/test_orphan.py index 1e19bc21fcde..48745753f496 100644 --- a/cms/djangoapps/contentstore/tests/test_orphan.py +++ b/cms/djangoapps/contentstore/tests/test_orphan.py @@ -103,7 +103,7 @@ def test_get_orphans(self, default_store): self.assertIn(str(location), orphans) @ddt.data( - (ModuleStoreEnum.Type.split, 5, 3), + (ModuleStoreEnum.Type.split, 9, 5), (ModuleStoreEnum.Type.mongo, 34, 12), ) @ddt.unpack diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index dff64735d53a..b2182b9ec913 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -391,13 +391,13 @@ def check_index_page(self, separate_archived_courses, org): @ddt.data( # Staff user has course staff access - (True, 'staff', None, 1, 20), - (False, 'staff', None, 1, 20), + (True, 'staff', None, 3, 18), + (False, 'staff', None, 3, 18), # Base user has global staff access - (True, 'user', ORG, 1, 20), - (False, 'user', ORG, 1, 20), - (True, 'user', None, 1, 20), - (False, 'user', None, 1, 20), + (True, 'user', ORG, 3, 18), + (False, 'user', ORG, 3, 18), + (True, 'user', None, 3, 18), + (False, 'user', None, 3, 18), ) @ddt.unpack def test_separate_archived_courses(self, separate_archived_courses, username, org, mongo_queries, sql_queries): diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index a5367abc245f..0d93d8b4545d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -2528,7 +2528,7 @@ def test_json_responses(self): self.validate_course_xblock_info(json_response, course_outline=True) @ddt.data( - (ModuleStoreEnum.Type.split, 3, 3), + (ModuleStoreEnum.Type.split, 4, 4), (ModuleStoreEnum.Type.mongo, 5, 7), ) @ddt.unpack diff --git a/cms/envs/common.py b/cms/envs/common.py index 1b01d6c4f15a..5036425d25f6 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1456,7 +1456,6 @@ # For CMS 'cms.djangoapps.contentstore.apps.ContentstoreConfig', - 'common.djangoapps.split_modulestore_django.apps.SplitModulestoreDjangoBackendAppConfig', 'openedx.core.djangoapps.contentserver', 'cms.djangoapps.course_creators', diff --git a/common/djangoapps/split_modulestore_django/__init__.py b/common/djangoapps/split_modulestore_django/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/common/djangoapps/split_modulestore_django/admin.py b/common/djangoapps/split_modulestore_django/admin.py deleted file mode 100644 index 4dbddce9fa08..000000000000 --- a/common/djangoapps/split_modulestore_django/admin.py +++ /dev/null @@ -1,18 +0,0 @@ -""" -Admin registration for Split Modulestore Django Backend -""" -from django.contrib import admin -from simple_history.admin import SimpleHistoryAdmin - -from .models import SplitModulestoreCourseIndex - - -@admin.register(SplitModulestoreCourseIndex) -class SplitModulestoreCourseIndexAdmin(SimpleHistoryAdmin): - """ - Admin config for course indexes - """ - list_display = ('course_id', 'draft_version', 'published_version', 'library_version', 'wiki_slug', 'last_update') - search_fields = ('course_id', 'wiki_slug') - ordering = ('course_id', ) - readonly_fields = ('id', 'objectid', 'course_id', 'org', ) diff --git a/common/djangoapps/split_modulestore_django/apps.py b/common/djangoapps/split_modulestore_django/apps.py deleted file mode 100644 index 022533938d01..000000000000 --- a/common/djangoapps/split_modulestore_django/apps.py +++ /dev/null @@ -1,12 +0,0 @@ -""" -Define this module as a Django app -""" -from django.apps import AppConfig - - -class SplitModulestoreDjangoBackendAppConfig(AppConfig): - """ - Django app that provides a backend for Split Modulestore instead of MongoDB. - """ - name = 'common.djangoapps.split_modulestore_django' - verbose_name = "Split Modulestore Django Backend" diff --git a/common/djangoapps/split_modulestore_django/migrations/0001_initial.py b/common/djangoapps/split_modulestore_django/migrations/0001_initial.py deleted file mode 100644 index 598090f8b014..000000000000 --- a/common/djangoapps/split_modulestore_django/migrations/0001_initial.py +++ /dev/null @@ -1,65 +0,0 @@ -# Generated by Django 2.2.20 on 2021-05-07 18:29 - -from django.conf import settings -from django.db import migrations, models -import django.db.models.deletion -import opaque_keys.edx.django.models -import simple_history.models - - -class Migration(migrations.Migration): - - initial = True - - dependencies = [ - migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ] - - operations = [ - migrations.CreateModel( - name='SplitModulestoreCourseIndex', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('objectid', models.CharField(max_length=24, unique=True)), - ('course_id', opaque_keys.edx.django.models.LearningContextKeyField(db_index=True, max_length=255, unique=True)), - ('org', models.CharField(db_index=True, max_length=255)), - ('draft_version', models.CharField(blank=True, max_length=24)), - ('published_version', models.CharField(blank=True, max_length=24)), - ('library_version', models.CharField(blank=True, max_length=24)), - ('wiki_slug', models.CharField(db_index=True, blank=True, max_length=255)), - ('base_store', models.CharField(choices=[('mongodb', 'MongoDB'), ('django', 'Django - not implemented yet')], max_length=20)), - ('edited_on', models.DateTimeField()), - ('last_update', models.DateTimeField()), - ('edited_by_id', models.IntegerField(null=True)), - ], - options={'ordering': ['course_id'], 'verbose_name_plural': 'Split modulestore course indexes'}, - ), - migrations.CreateModel( - name='HistoricalSplitModulestoreCourseIndex', - fields=[ - ('id', models.IntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), - ('objectid', models.CharField(db_index=True, max_length=24)), - ('course_id', opaque_keys.edx.django.models.LearningContextKeyField(db_index=True, max_length=255)), - ('org', models.CharField(db_index=True, max_length=255)), - ('draft_version', models.CharField(blank=True, max_length=24)), - ('published_version', models.CharField(blank=True, max_length=24)), - ('library_version', models.CharField(blank=True, max_length=24)), - ('wiki_slug', models.CharField(db_index=True, blank=True, max_length=255)), - ('base_store', models.CharField(choices=[('mongodb', 'MongoDB'), ('django', 'Django - not implemented yet')], max_length=20)), - ('edited_on', models.DateTimeField()), - ('last_update', models.DateTimeField()), - ('history_id', models.AutoField(primary_key=True, serialize=False)), - ('history_date', models.DateTimeField()), - ('history_change_reason', models.CharField(max_length=100, null=True)), - ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), - ('edited_by_id', models.IntegerField(null=True)), - ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), - ], - options={ - 'verbose_name': 'historical split modulestore course index', - 'ordering': ('-history_date', '-history_id'), - 'get_latest_by': 'history_date', - }, - bases=(simple_history.models.HistoricalChanges, models.Model), - ), - ] diff --git a/common/djangoapps/split_modulestore_django/migrations/0002_data_migration.py b/common/djangoapps/split_modulestore_django/migrations/0002_data_migration.py deleted file mode 100644 index 148e1b6b751a..000000000000 --- a/common/djangoapps/split_modulestore_django/migrations/0002_data_migration.py +++ /dev/null @@ -1,41 +0,0 @@ -from django.db import migrations, models -from django.db.utils import IntegrityError - - -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.django import modulestore - -from ..models import SplitModulestoreCourseIndex as SplitModulestoreCourseIndex_Real - - -def forwards_func(apps, schema_editor): - """ - Copy all course index data from MongoDB to MySQL. - """ - db_alias = schema_editor.connection.alias - SplitModulestoreCourseIndex = apps.get_model("split_modulestore_django", "SplitModulestoreCourseIndex") - split_modulestore = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.split) - - for course_index in split_modulestore.db_connection.find_matching_course_indexes(force_mongo=True): - data = SplitModulestoreCourseIndex_Real.fields_from_v1_schema(course_index) - - SplitModulestoreCourseIndex(**data).save(using=db_alias) - -def reverse_func(apps, schema_editor): - """ - Reverse the data migration, deleting all entries in this table. - """ - db_alias = schema_editor.connection.alias - SplitModulestoreCourseIndex = apps.get_model("split_modulestore_django", "SplitModulestoreCourseIndex") - SplitModulestoreCourseIndex.objects.using(db_alias).all().delete() - - -class Migration(migrations.Migration): - - dependencies = [ - ('split_modulestore_django', '0001_initial'), - ] - - operations = [ - migrations.RunPython(forwards_func, reverse_func), - ] diff --git a/common/djangoapps/split_modulestore_django/migrations/__init__.py b/common/djangoapps/split_modulestore_django/migrations/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/common/djangoapps/split_modulestore_django/models.py b/common/djangoapps/split_modulestore_django/models.py deleted file mode 100644 index b3f8b6d1457e..000000000000 --- a/common/djangoapps/split_modulestore_django/models.py +++ /dev/null @@ -1,164 +0,0 @@ -""" -Django model to store the "course index" data -""" -from bson.objectid import ObjectId -from django.contrib.auth import get_user_model -from django.db import models -from opaque_keys.edx.locator import CourseLocator, LibraryLocator -from opaque_keys.edx.django.models import LearningContextKeyField -from simple_history.models import HistoricalRecords - -from xmodule.modulestore import ModuleStoreEnum - -User = get_user_model() - - -class SplitModulestoreCourseIndex(models.Model): - """ - A "course index" for a course in "split modulestore." - - This model/table mostly stores the current version of each course. - (Well, twice for each course - "draft" and "published" branch versions are - tracked separately.) - - This MySQL table / django model is designed to replace the "active_versions" - MongoDB collection. They contain the same information. - - It also stores the "wiki_slug" to facilitate looking up a course - by it's wiki slug, which is required due to the nuances of the - django-wiki integration. - - .. no_pii: - """ - # For compatibility with MongoDB, each course index must have an ObjectId. We still have an integer primary key too. - objectid = models.CharField(max_length=24, null=False, blank=False, unique=True) - - # The ID of this course (or library). Must start with "course-v1:" or "library-v1:" - course_id = LearningContextKeyField(max_length=255, db_index=True, unique=True, null=False) - # Extract the "org" value from the course_id key so that we can search by org. - # This gets set automatically by clean() - org = models.CharField(max_length=255, db_index=True) - - # Version fields: The ObjectId of the current entry in the "structures" collection, for this course. - # The version is stored separately for each "branch". - # Note that there are only three branch names allowed. Draft/published are used for courses, while "library" is used - # for content libraries. - - # ModuleStoreEnum.BranchName.draft = 'draft-branch' - draft_version = models.CharField(max_length=24, null=False, blank=True) - # ModuleStoreEnum.BranchName.published = 'published-branch' - published_version = models.CharField(max_length=24, null=False, blank=True) - # ModuleStoreEnum.BranchName.library = 'library' - library_version = models.CharField(max_length=24, null=False, blank=True) - - # Wiki slug for this course - wiki_slug = models.CharField(max_length=255, db_index=True, blank=True) - - # Base store - whether the "structures" and "definitions" data are in MongoDB or object storage (S3) - BASE_STORE_MONGO = "mongodb" - BASE_STORE_DJANGO = "django" - BASE_STORE_CHOICES = [ - (BASE_STORE_MONGO, "MongoDB"), # For now, MongoDB is the only implemented option - (BASE_STORE_DJANGO, "Django - not implemented yet"), - ] - base_store = models.CharField(max_length=20, blank=False, choices=BASE_STORE_CHOICES) - - # Edit history: - # ID of the user that made the latest edit. This is not a ForeignKey because some values (like - # ModuleStoreEnum.UserID.*) are not real user IDs. - edited_by_id = models.IntegerField(null=True) - edited_on = models.DateTimeField() - # last_update is different from edited_on, and is used only to prevent collisions? - last_update = models.DateTimeField() - - # Keep track of the history of this table: - history = HistoricalRecords() - - def __str__(self): - return f"Course Index ({self.course_id})" - - class Meta: - ordering = ["course_id"] - verbose_name_plural = "Split modulestore course indexes" - - def as_v1_schema(self): - """ Return in the same format as was stored in MongoDB """ - versions = {} - for branch in ("draft", "published", "library"): - # The current version of this branch, a hex-encoded ObjectID - or an empty string: - version_str = getattr(self, f"{branch}_version") - if version_str: - versions[getattr(ModuleStoreEnum.BranchName, branch)] = ObjectId(version_str) - return { - "_id": ObjectId(self.objectid), - "org": self.course_id.org, - "course": self.course_id.course, - "run": self.course_id.run, # pylint: disable=no-member - "edited_by": self.edited_by_id, - "edited_on": self.edited_on, - "last_update": self.last_update, - "versions": versions, - "schema_version": 1, # This matches schema version 1, see SplitMongoModuleStore.SCHEMA_VERSION - "search_targets": {"wiki_slug": self.wiki_slug}, - } - - @staticmethod - def fields_from_v1_schema(values): - """ Convert the MongoDB-style dict shape to a dict of fields that match this model """ - if values["run"] == LibraryLocator.RUN and ModuleStoreEnum.BranchName.library in values["versions"]: - # This is a content library: - locator = LibraryLocator(org=values["org"], library=values["course"]) - else: - # This is a course: - locator = CourseLocator(org=values["org"], course=values["course"], run=values["run"]) - result = { - "course_id": locator, - "org": values["org"], - "edited_by_id": values["edited_by"], - "edited_on": values["edited_on"], - "base_store": SplitModulestoreCourseIndex.BASE_STORE_MONGO, - } - if "_id" in values: - result["objectid"] = str(values["_id"]) # Convert ObjectId to its hex representation - if "last_update" in values: - result["last_update"] = values["last_update"] - if "search_targets" in values and "wiki_slug" in values["search_targets"]: - result["wiki_slug"] = values["search_targets"]["wiki_slug"] - for branch in ("draft", "published", "library"): - version = values["versions"].get(getattr(ModuleStoreEnum.BranchName, branch)) - if version: - result[f"{branch}_version"] = str(version) # Convert version from ObjectId to hex string - return result - - @staticmethod - def field_name_for_branch(branch_name): - """ Given a full branch name, get the name of the field in this table that stores that branch's version """ - if branch_name == ModuleStoreEnum.BranchName.draft: - return "draft_version" - if branch_name == ModuleStoreEnum.BranchName.published: - return "published_version" - if branch_name == ModuleStoreEnum.BranchName.library: - return "library_version" - raise ValueError(f"Unknown branch name: {branch_name}") - - def clean(self): - """ - Validation for this model - """ - super().clean() - # Check that course_id is a supported type: - course_id_str = str(self.course_id) - if not course_id_str.startswith("course-v1:") and not course_id_str.startswith("library-v1:"): - raise ValueError( - f"Split modulestore cannot store course[like] object with key {course_id_str}" - " - only course-v1/library-v1 prefixed keys are supported." - ) - # Set the "org" field automatically - ensure it always matches the "org" in the course_id - self.org = self.course_id.org - - def save(self, *args, **kwargs): - """ Save this model """ - # Override to ensure that full_clean()/clean() is always called, so that the checks in clean() above are run. - # But don't validate_unique(), it just runs extra queries and the database enforces it anyways. - self.full_clean(validate_unique=False) - return super().save(*args, **kwargs) 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 e51dc9e0dfb0..c868ee89f255 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -13,14 +13,11 @@ 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 @@ def set(self, key, structure, course_context=None): 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 @@ def _generate_query_from_course_keys(self, branch, course_keys): 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 @@ def update_course_index(self, course_index, from_index=None, course_context=None '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 @@ def _drop_database(self, database=True, collections=True, connections=True): 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) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 7878b85d076f..ac50ace8a734 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -101,7 +101,7 @@ 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 @@ def __init__(self, contentstore, doc_store_config, fs_root, render_template, 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('.') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index bd94c642d851..908ad97baa61 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -12,7 +12,7 @@ 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 @@ def end_modulestore_isolation(cls): 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(): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py index d607c0e7e903..6cb17268a740 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py @@ -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) - - 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 - """ - with check_mongo_calls(0, 3), self.assertNumQueries(5): + find_one({'org': '...', 'run': 'library', 'course': '...'}) + insert(definition: {'block_type': 'library', 'fields': {}}) + + insert_structure(bulk) + insert_course_index(bulk) + get_course_index(bulk) + """ + with check_mongo_calls(2, 3): LibraryFactory.create(modulestore=self.store) def test_duplicate_library(self): 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 f847630403c7..27b014a3a163 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -366,9 +366,8 @@ def test_duplicate_course_error_with_different_case_ids(self, default_store): # 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 @@ def test_has_item(self, default_ms, max_find, max_send): # 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 @@ def test_get_item(self, default_ms, num_mysql, max_find, max_send): # 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 @@ def test_get_items_include_orphans(self, default_ms, expected_items_in_tree, orp 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 @@ def test_update_item(self, default_ms, num_mysql, max_find, max_send): # 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 @@ def test_has_changes_missing_child(self, default_ms, default_branch): # 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 @@ def test_delete_item(self, default_ms, num_mysql, max_find, max_send): 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 @@ def test_delete_item(self, default_ms, num_mysql, max_find, max_send): 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 @@ def test_delete_private_vertical(self, default_ms, num_mysql, max_find, max_send 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 @@ def test_delete_private_vertical(self, default_ms, num_mysql, max_find, max_send # 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 @@ def test_delete_draft_vertical(self, default_ms, num_mysql, max_find, max_send): # 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 @@ def test_create_child_detached_tabs(self, default_ms): 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 @@ def test_get_library(self, default_ms): # 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 @@ def test_get_parent_location_draft(self, default_ms): # 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 @@ def test_path_to_location(self, default_ms, num_mysql, num_finds, num_sends): 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 @@ def _get_split_modulestore(self): 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 @@ def test_get_orphans(self, default_ms, num_mysql, max_find, max_send): 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 @@ def test_create_item_populates_subtree_edited_info(self, default_ms): 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 @@ def test_get_courses_for_wiki(self, default_ms, num_mysql, max_find, max_send): # 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 @@ def test_unpublish(self, default_ms, num_mysql, max_find, max_send): 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 @@ def test_unpublish(self, default_ms, num_mysql, max_find, max_send): 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 @@ def test_has_published_version(self, default_ms, mysql_queries, max_find, max_se # 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 @@ def test_delete_item_with_asides(self, default_store): 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']) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py index ba618da8f329..8402d2be882a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py @@ -154,14 +154,14 @@ def _import_course(self, content_store, modulestore): (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 @@ def test_number_mongo_calls(self, store_builder, depth, lazy, access_all_block_f @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): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py index c2a4b168487e..efcf5ef68ca6 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py @@ -163,7 +163,6 @@ def test_publish_draft_delete(self): 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 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 3550ef69ea3c..1739f48761c0 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -53,7 +53,6 @@ @attr('mongo') -@pytest.mark.django_db class SplitModuleTest(unittest.TestCase): ''' The base set of tests manually populates a db w/ courses which have diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py index 6690770a86ee..da0a6d0d7a87 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py @@ -12,7 +12,7 @@ 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 @@ def setUp(self): 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') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py index 008c1cc2a654..3460470186b3 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py @@ -1,4 +1,4 @@ -""" Test the behavior of split_mongo/MongoPersistenceBackend """ +""" Test the behavior of split_mongo/MongoConnection """ import unittest @@ -8,7 +8,7 @@ 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 @@ def test_heartbeat_raises_exception_when_connection_alive_is_false(self, *calls) # 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() diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py index fa55146808f4..2798bcc9fe90 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py @@ -19,7 +19,6 @@ @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 diff --git a/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py b/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py index f1bd4b6f8df8..dc3ad0813f20 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py +++ b/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py @@ -34,7 +34,6 @@ _NEXT_WEEK = _TODAY + timedelta(days=7) -@pytest.mark.django_db class CourseMetadataUtilsTestCase(TestCase): """ Tests for course_metadata_utils. diff --git a/lms/djangoapps/badges/backends/tests/test_badgr_backend.py b/lms/djangoapps/badges/backends/tests/test_badgr_backend.py index 1817fa92a8fe..3a500c117bd2 100644 --- a/lms/djangoapps/badges/backends/tests/test_badgr_backend.py +++ b/lms/djangoapps/badges/backends/tests/test_badgr_backend.py @@ -114,8 +114,7 @@ def test_create_badge(self, post): Verify badge spec creation works. """ self.handler._get_access_token = Mock(return_value='12345') - with self.allow_transaction_exception(): - self.handler._create_badge(self.badge_class) + self.handler._create_badge(self.badge_class) args, kwargs = post.call_args assert args[0] == 'https://example.com/v2/issuers/test-issuer/badgeclasses' assert kwargs['files']['image'][0] == self.badge_class.image.name diff --git a/lms/djangoapps/badges/tests/test_models.py b/lms/djangoapps/badges/tests/test_models.py index 41a49c6629ac..154dd747045f 100644 --- a/lms/djangoapps/badges/tests/test_models.py +++ b/lms/djangoapps/badges/tests/test_models.py @@ -196,8 +196,8 @@ def test_get_badge_class_data_validate(self): Verify handing incomplete data for required fields when making a badge class raises an Integrity error. """ image = get_image('good') - with pytest.raises(IntegrityError), self.allow_transaction_exception(): - BadgeClass.get_badge_class(slug='new_slug', issuing_component='new_component', image_file_handle=image) + pytest.raises(IntegrityError, BadgeClass.get_badge_class, slug='new_slug', issuing_component='new_component', + image_file_handle=image) def test_get_for_user(self): """ diff --git a/lms/djangoapps/bulk_enroll/tests/test_views.py b/lms/djangoapps/bulk_enroll/tests/test_views.py index 70e37b15c73e..1ce5b1775ba1 100644 --- a/lms/djangoapps/bulk_enroll/tests/test_views.py +++ b/lms/djangoapps/bulk_enroll/tests/test_views.py @@ -107,8 +107,7 @@ def test_non_staff(self): """ Test that non global staff users are forbidden from API use. """ self.staff.is_staff = False self.staff.save() - with self.allow_transaction_exception(): - response = self.request_bulk_enroll() + response = self.request_bulk_enroll() assert response.status_code == 403 def test_missing_params(self): diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 0970ab8c990f..8cfca804d33a 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -271,22 +271,22 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True # TODO: decrease query count as part of REVO-28 - QUERY_COUNT = 34 + QUERY_COUNT = 33 TEST_DATA = { - ('no_overrides', 1, True, False): (QUERY_COUNT, 2), - ('no_overrides', 2, True, False): (QUERY_COUNT, 2), - ('no_overrides', 3, True, False): (QUERY_COUNT, 2), - ('ccx', 1, True, False): (QUERY_COUNT, 2), - ('ccx', 2, True, False): (QUERY_COUNT, 2), - ('ccx', 3, True, False): (QUERY_COUNT, 2), - ('ccx', 1, True, True): (QUERY_COUNT + 3, 2), - ('ccx', 2, True, True): (QUERY_COUNT + 3, 2), - ('ccx', 3, True, True): (QUERY_COUNT + 3, 2), - ('no_overrides', 1, False, False): (QUERY_COUNT, 2), - ('no_overrides', 2, False, False): (QUERY_COUNT, 2), - ('no_overrides', 3, False, False): (QUERY_COUNT, 2), - ('ccx', 1, False, False): (QUERY_COUNT, 2), - ('ccx', 2, False, False): (QUERY_COUNT, 2), - ('ccx', 3, False, False): (QUERY_COUNT, 2), + ('no_overrides', 1, True, False): (QUERY_COUNT, 3), + ('no_overrides', 2, True, False): (QUERY_COUNT, 3), + ('no_overrides', 3, True, False): (QUERY_COUNT, 3), + ('ccx', 1, True, False): (QUERY_COUNT, 3), + ('ccx', 2, True, False): (QUERY_COUNT, 3), + ('ccx', 3, True, False): (QUERY_COUNT, 3), + ('ccx', 1, True, True): (QUERY_COUNT + 2, 3), + ('ccx', 2, True, True): (QUERY_COUNT + 2, 3), + ('ccx', 3, True, True): (QUERY_COUNT + 2, 3), + ('no_overrides', 1, False, False): (QUERY_COUNT, 3), + ('no_overrides', 2, False, False): (QUERY_COUNT, 3), + ('no_overrides', 3, False, False): (QUERY_COUNT, 3), + ('ccx', 1, False, False): (QUERY_COUNT, 3), + ('ccx', 2, False, False): (QUERY_COUNT, 3), + ('ccx', 3, False, False): (QUERY_COUNT, 3), } diff --git a/lms/djangoapps/ccx/tests/test_models.py b/lms/djangoapps/ccx/tests/test_models.py index 97c8761b5946..006ba60de568 100644 --- a/lms/djangoapps/ccx/tests/test_models.py +++ b/lms/djangoapps/ccx/tests/test_models.py @@ -46,7 +46,7 @@ def test_ccx_course_is_correct_course(self): def test_ccx_course_caching(self): """verify that caching the propery works to limit queries""" - with check_mongo_calls(2): + with check_mongo_calls(3): # these statements are used entirely to demonstrate the # instance-level caching of these values on CCX objects. The # check_mongo_calls context is the point here. @@ -72,7 +72,7 @@ def test_ccx_start_caching(self): """verify that caching the start property works to limit queries""" now = datetime.now(utc) self.set_ccx_override('start', now) - with check_mongo_calls(2): + with check_mongo_calls(3): # these statements are used entirely to demonstrate the # instance-level caching of these values on CCX objects. The # check_mongo_calls context is the point here. @@ -97,7 +97,7 @@ def test_ccx_due_caching(self): """verify that caching the due property works to limit queries""" expected = datetime.now(utc) self.set_ccx_override('due', expected) - with check_mongo_calls(2): + with check_mongo_calls(3): # these statements are used entirely to demonstrate the # instance-level caching of these values on CCX objects. The # check_mongo_calls context is the point here. diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index 5297f7f9f2aa..e7b3fbdd9a98 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -224,17 +224,23 @@ def test_query_counts_cached(self, store_type, with_storage_backing): ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 5, True, 23), - (ModuleStoreEnum.Type.mongo, 5, False, 13), - (ModuleStoreEnum.Type.split, 2, True, 24), - (ModuleStoreEnum.Type.split, 2, False, 14), + *product( + ((ModuleStoreEnum.Type.mongo, 5), (ModuleStoreEnum.Type.split, 3)), + (True, False), + ) ) @ddt.unpack - def test_query_counts_uncached(self, store_type, expected_mongo_queries, with_storage_backing, num_sql_queries): + def test_query_counts_uncached(self, store_type_tuple, with_storage_backing): + store_type, expected_mongo_queries = store_type_tuple with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): course = self._create_course(store_type) clear_course_from_cache(course.id) + if with_storage_backing: + num_sql_queries = 23 + else: + num_sql_queries = 13 + self._get_blocks( course, expected_mongo_queries, diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index e6acde7d225d..4cc29a5e117f 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1014,9 +1014,9 @@ def setup_request_and_course(self, num_finds, num_sends): # Split makes 2 queries to load the course to depth 2: # - 1 for the structure # - 1 for 5 definitions - # Split makes 1 MySQL query to render the toc: - # - 1 MySQL for the active version at the start of the bulk operation (no mongo calls) - @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0, 0), (ModuleStoreEnum.Type.split, 2, 0, 0)) + # Split makes 1 query to render the toc: + # - 1 for the active version at the start of the bulk operation + @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0, 0), (ModuleStoreEnum.Type.split, 2, 0, 1)) @ddt.unpack def test_toc_toy_from_chapter(self, default_ms, setup_finds, setup_sends, toc_finds): with self.store.default_store(default_ms): @@ -1054,9 +1054,9 @@ def test_toc_toy_from_chapter(self, default_ms, setup_finds, setup_sends, toc_fi # Split makes 2 queries to load the course to depth 2: # - 1 for the structure # - 1 for 5 definitions - # Split makes 1 MySQL query to render the toc: - # - 1 MySQL for the active version at the start of the bulk operation (no mongo calls) - @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0, 0), (ModuleStoreEnum.Type.split, 2, 0, 0)) + # Split makes 1 query to render the toc: + # - 1 for the active version at the start of the bulk operation + @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0, 0), (ModuleStoreEnum.Type.split, 2, 0, 1)) @ddt.unpack def test_toc_toy_from_section(self, default_ms, setup_finds, setup_sends, toc_finds): with self.store.default_store(default_ms): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 350bf95eda7c..ddafdde6563e 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -389,7 +389,7 @@ class IndexQueryTestCase(ModuleStoreTestCase): @ddt.data( (ModuleStoreEnum.Type.mongo, 10, 164), - (ModuleStoreEnum.Type.split, 3, 161), + (ModuleStoreEnum.Type.split, 4, 160), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index e3fb0132ed82..6c4caa6be16e 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -162,9 +162,9 @@ def verify_response(self, expected_response_code=200, url_params=None): @ddt.data( ('vertical_block', ModuleStoreEnum.Type.mongo, 13), - ('vertical_block', ModuleStoreEnum.Type.split, 4), + ('vertical_block', ModuleStoreEnum.Type.split, 6), ('html_block', ModuleStoreEnum.Type.mongo, 14), - ('html_block', ModuleStoreEnum.Type.split, 4), + ('html_block', ModuleStoreEnum.Type.split, 6), ) @ddt.unpack def test_courseware_html(self, block_name, default_store, mongo_calls): @@ -192,7 +192,7 @@ def test_courseware_html(self, block_name, default_store, mongo_calls): @ddt.data( (ModuleStoreEnum.Type.mongo, 5), - (ModuleStoreEnum.Type.split, 4), + (ModuleStoreEnum.Type.split, 5), ) @ddt.unpack def test_success_enrolled_staff(self, default_store, mongo_calls): diff --git a/lms/djangoapps/discussion/django_comment_client/base/tests.py b/lms/djangoapps/discussion/django_comment_client/base/tests.py index efc96a325dc9..efb018980908 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/tests.py +++ b/lms/djangoapps/discussion/django_comment_client/base/tests.py @@ -402,7 +402,7 @@ def inner(self, default_store, module_count, mongo_calls, sql_queries, *args, ** @ddt.data( (ModuleStoreEnum.Type.mongo, 3, 4, 39), - (ModuleStoreEnum.Type.split, 3, 8, 44), + (ModuleStoreEnum.Type.split, 3, 13, 39), ) @ddt.unpack @count_queries @@ -411,7 +411,7 @@ def test_create_thread(self, mock_request): @ddt.data( (ModuleStoreEnum.Type.mongo, 3, 3, 35), - (ModuleStoreEnum.Type.split, 3, 6, 39), + (ModuleStoreEnum.Type.split, 3, 10, 35), ) @ddt.unpack @count_queries diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index ffcffbef6cc5..61103af01c01 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -413,10 +413,10 @@ def test_basic(self): (2, ModuleStoreEnum.Type.mongo, 2, {"Test Topic 1": {"id": "test_topic_1"}}), (2, ModuleStoreEnum.Type.mongo, 2, {"Test Topic 1": {"id": "test_topic_1"}, "Test Topic 2": {"id": "test_topic_2"}}), - (2, ModuleStoreEnum.Type.split, 2, {"Test Topic 1": {"id": "test_topic_1"}}), - (2, ModuleStoreEnum.Type.split, 2, + (2, ModuleStoreEnum.Type.split, 3, {"Test Topic 1": {"id": "test_topic_1"}}), + (2, ModuleStoreEnum.Type.split, 3, {"Test Topic 1": {"id": "test_topic_1"}, "Test Topic 2": {"id": "test_topic_2"}}), - (10, ModuleStoreEnum.Type.split, 2, {"Test Topic 1": {"id": "test_topic_1"}}), + (10, ModuleStoreEnum.Type.split, 3, {"Test Topic 1": {"id": "test_topic_1"}}), ) @ddt.unpack def test_bulk_response(self, modules_count, module_store, mongo_calls, topics): diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index fb1da7cb6822..0cf438e2319a 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -483,15 +483,15 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase): (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 21, 7), (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 21, 7), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, False, 1, 2, 2, 22, 8), - (ModuleStoreEnum.Type.split, False, 50, 2, 2, 22, 8), + (ModuleStoreEnum.Type.split, False, 1, 3, 3, 21, 8), + (ModuleStoreEnum.Type.split, False, 50, 3, 3, 21, 8), # Enabling Enterprise integration should have no effect on the number of mongo queries made. (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 21, 7), (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 21, 7), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, True, 1, 2, 2, 22, 8), - (ModuleStoreEnum.Type.split, True, 50, 2, 2, 22, 8), + (ModuleStoreEnum.Type.split, True, 1, 3, 3, 21, 8), + (ModuleStoreEnum.Type.split, True, 50, 3, 3, 21, 8), ) @ddt.unpack def test_number_of_mongo_queries( diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index cfee479e46ae..c68b17f37bc5 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -164,8 +164,8 @@ def test_block_structure_created_only_once(self): @ddt.data( (ModuleStoreEnum.Type.mongo, 1, 39, True), (ModuleStoreEnum.Type.mongo, 1, 39, False), - (ModuleStoreEnum.Type.split, 2, 40, True), - (ModuleStoreEnum.Type.split, 2, 40, False), + (ModuleStoreEnum.Type.split, 3, 39, True), + (ModuleStoreEnum.Type.split, 3, 39, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -178,7 +178,7 @@ def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, creat @ddt.data( (ModuleStoreEnum.Type.mongo, 1, 39), - (ModuleStoreEnum.Type.split, 2, 40), + (ModuleStoreEnum.Type.split, 3, 39), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -224,7 +224,7 @@ def test_other_inaccessible_subsection(self, mock_subsection_signal): @ddt.data( (ModuleStoreEnum.Type.mongo, 1, 22), - (ModuleStoreEnum.Type.split, 2, 23), + (ModuleStoreEnum.Type.split, 3, 22), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -239,7 +239,7 @@ def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_ @ddt.data( (ModuleStoreEnum.Type.mongo, 1, 40), - (ModuleStoreEnum.Type.split, 2, 41), + (ModuleStoreEnum.Type.split, 3, 40), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/grades/tests/test_transformer.py b/lms/djangoapps/grades/tests/test_transformer.py index 98685ea2d534..f2bf8741b8ac 100644 --- a/lms/djangoapps/grades/tests/test_transformer.py +++ b/lms/djangoapps/grades/tests/test_transformer.py @@ -430,7 +430,7 @@ def setUp(self): self.client.login(username=self.student.username, password=password) @ddt.data( - (ModuleStoreEnum.Type.split, 2), + (ModuleStoreEnum.Type.split, 3), (ModuleStoreEnum.Type.mongo, 2), ) @ddt.unpack diff --git a/lms/djangoapps/instructor/tests/test_tools.py b/lms/djangoapps/instructor/tests/test_tools.py index 0840f7426570..0713bd1f5616 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -252,12 +252,12 @@ def test_set_due_date_extension(self): def test_set_due_date_extension_invalid_date(self): extended = datetime.datetime(2009, 1, 1, 0, 0, tzinfo=UTC) - with pytest.raises(tools.DashboardError), self.allow_transaction_exception(): + with pytest.raises(tools.DashboardError): tools.set_due_date_extension(self.course, self.week1, self.user, extended) def test_set_due_date_extension_no_date(self): extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=UTC) - with pytest.raises(tools.DashboardError), self.allow_transaction_exception(): + with pytest.raises(tools.DashboardError): tools.set_due_date_extension(self.course, self.week3, self.user, extended) def test_reset_due_date_extension(self): diff --git a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py index 6be624e9fd9c..90a35f1d6b0b 100644 --- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py +++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py @@ -673,6 +673,6 @@ def test_spoc_gradebook_mongo_calls(self): # check MongoDB calls count url = reverse('spoc_gradebook', kwargs={'course_id': self.course.id}) - with check_mongo_calls(6): + with check_mongo_calls(9): response = self.client.get(url) assert response.status_code == 200 diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index c3dd3eb9afb2..7420dfa9d5fa 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -371,11 +371,11 @@ def test_certificate_eligibility(self): self._verify_cell_data_for_user(verified_user.username, course.id, 'Certificate Eligible', 'Y', num_rows=2) @ddt.data( - (ModuleStoreEnum.Type.mongo, 4, 46), - (ModuleStoreEnum.Type.split, 2, 47), + (ModuleStoreEnum.Type.mongo, 4), + (ModuleStoreEnum.Type.split, 3), ) @ddt.unpack - def test_query_counts(self, store_type, mongo_count, expected_query_count): + def test_query_counts(self, store_type, mongo_count): with self.store.default_store(store_type): experiment_group_a = Group(2, 'Expériment Group A') experiment_group_b = Group(3, 'Expériment Group B') @@ -401,6 +401,7 @@ def test_query_counts(self, store_type, mongo_count, expected_query_count): RequestCache.clear_all_namespaces() + expected_query_count = 46 with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): with check_mongo_calls(mongo_count): with self.assertNumQueries(expected_query_count): diff --git a/lms/envs/common.py b/lms/envs/common.py index b3d43bb36c5b..fdf5867418be 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2910,7 +2910,6 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring 'lms.djangoapps.courseware', 'lms.djangoapps.coursewarehistoryextended', 'common.djangoapps.student.apps.StudentConfig', - 'common.djangoapps.split_modulestore_django.apps.SplitModulestoreDjangoBackendAppConfig', 'lms.djangoapps.static_template_view', 'lms.djangoapps.staticbook', diff --git a/openedx/core/djangoapps/bookmarks/tests/test_models.py b/openedx/core/djangoapps/bookmarks/tests/test_models.py index df5d89c5f5fc..46e5b8ed7095 100644 --- a/openedx/core/djangoapps/bookmarks/tests/test_models.py +++ b/openedx/core/djangoapps/bookmarks/tests/test_models.py @@ -276,11 +276,11 @@ def get_bookmark_data(self, block, user=None): (ModuleStoreEnum.Type.mongo, 'sequential_1', ['chapter_1'], 4), (ModuleStoreEnum.Type.mongo, 'vertical_1', ['chapter_1', 'sequential_1'], 6), (ModuleStoreEnum.Type.mongo, 'html_1', ['chapter_1', 'sequential_2', 'vertical_2'], 7), - (ModuleStoreEnum.Type.split, 'course', [], 2), - (ModuleStoreEnum.Type.split, 'chapter_1', [], 1), - (ModuleStoreEnum.Type.split, 'sequential_1', ['chapter_1'], 1), - (ModuleStoreEnum.Type.split, 'vertical_1', ['chapter_1', 'sequential_1'], 1), - (ModuleStoreEnum.Type.split, 'html_1', ['chapter_1', 'sequential_2', 'vertical_2'], 1), + (ModuleStoreEnum.Type.split, 'course', [], 3), + (ModuleStoreEnum.Type.split, 'chapter_1', [], 2), + (ModuleStoreEnum.Type.split, 'sequential_1', ['chapter_1'], 2), + (ModuleStoreEnum.Type.split, 'vertical_1', ['chapter_1', 'sequential_1'], 2), + (ModuleStoreEnum.Type.split, 'html_1', ['chapter_1', 'sequential_2', 'vertical_2'], 2), ) @ddt.unpack def test_path_and_queries_on_create(self, store_type, block_to_bookmark, ancestors_attrs, expected_mongo_calls): @@ -376,11 +376,11 @@ def test_path(self, seconds_delta, paths, get_path_call_count, mock_get_path): # (ModuleStoreEnum.Type.mongo, 6, 3, 3), Too slow. (ModuleStoreEnum.Type.mongo, 2, 4, 4), # (ModuleStoreEnum.Type.mongo, 4, 4, 4), - (ModuleStoreEnum.Type.split, 2, 2, 1), - (ModuleStoreEnum.Type.split, 4, 2, 1), - (ModuleStoreEnum.Type.split, 2, 3, 1), - # (ModuleStoreEnum.Type.split, 4, 3, 1), - (ModuleStoreEnum.Type.split, 2, 4, 1), + (ModuleStoreEnum.Type.split, 2, 2, 2), + (ModuleStoreEnum.Type.split, 4, 2, 2), + (ModuleStoreEnum.Type.split, 2, 3, 2), + # (ModuleStoreEnum.Type.split, 4, 3, 2), + (ModuleStoreEnum.Type.split, 2, 4, 2), ) @ddt.unpack def test_get_path_queries(self, store_type, children_per_block, depth, expected_mongo_calls): diff --git a/openedx/core/djangoapps/bookmarks/tests/test_tasks.py b/openedx/core/djangoapps/bookmarks/tests/test_tasks.py index 615d1b5d8a4e..f033e6bc1e49 100644 --- a/openedx/core/djangoapps/bookmarks/tests/test_tasks.py +++ b/openedx/core/djangoapps/bookmarks/tests/test_tasks.py @@ -108,10 +108,10 @@ def setUp(self): (ModuleStoreEnum.Type.mongo, 4, 3, 5), (ModuleStoreEnum.Type.mongo, 2, 4, 6), # (ModuleStoreEnum.Type.mongo, 4, 4, 6), Too slow. - (ModuleStoreEnum.Type.split, 2, 2, 2), - (ModuleStoreEnum.Type.split, 4, 2, 2), - (ModuleStoreEnum.Type.split, 2, 3, 2), - (ModuleStoreEnum.Type.split, 2, 4, 2), + (ModuleStoreEnum.Type.split, 2, 2, 3), + (ModuleStoreEnum.Type.split, 4, 2, 3), + (ModuleStoreEnum.Type.split, 2, 3, 3), + (ModuleStoreEnum.Type.split, 2, 4, 3), ) @ddt.unpack def test_calculate_course_xblocks_data_queries(self, store_type, children_per_block, depth, expected_mongo_calls): diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py index 39b30bd4174f..425f2c89a154 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py @@ -378,7 +378,7 @@ def test_malformed_grading_policy(self): course_overview = CourseOverview._create_or_update(course) # pylint: disable=protected-access assert course_overview.lowest_passing_grade is None - @ddt.data((ModuleStoreEnum.Type.mongo, 4, 4), (ModuleStoreEnum.Type.split, 2, 2)) + @ddt.data((ModuleStoreEnum.Type.mongo, 4, 4), (ModuleStoreEnum.Type.split, 3, 3)) @ddt.unpack def test_versioning(self, modulestore_type, min_mongo_calls, max_mongo_calls): """ diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index 8b082bef0b81..f3760cb9879b 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -743,7 +743,7 @@ def test_multiple_partition_groups(self): self.partition_id, self.group1_id, ) - with pytest.raises(IntegrityError), self.allow_transaction_exception(): + with pytest.raises(IntegrityError): self._link_cohort_partition_group( self.first_cohort, self.partition_id, diff --git a/openedx/core/djangoapps/courseware_api/tests/test_views.py b/openedx/core/djangoapps/courseware_api/tests/test_views.py index 4e00ac0b23db..2c66336bce5a 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -396,15 +396,7 @@ def test_sequence_metadata(self): def test_hidden_after_due(self, is_past_due, masquerade_config, expected_hidden, expected_banner): """Validate the metadata when hide-after-due is set for a sequence""" due = datetime.now() + timedelta(days=-1 if is_past_due else 1) - sequence = ItemFactory( - parent_location=self.chapter.location, - # ^ It is very important that we use parent_location=self.chapter.location (and not parent=self.chapter), as - # chapter is a class attribute and passing it by value will update its .children=[] which will then leak - # into other tests and cause errors if the children no longer exist. - category='sequential', - hide_after_due=True, - due=due, - ) + sequence = ItemFactory(parent=self.chapter, category='sequential', hide_after_due=True, due=due) CourseEnrollment.enroll(self.user, self.course.id) diff --git a/openedx/core/djangoapps/user_api/tests/test_models.py b/openedx/core/djangoapps/user_api/tests/test_models.py index 5634aa00b9ec..6a5d64bd2456 100644 --- a/openedx/core/djangoapps/user_api/tests/test_models.py +++ b/openedx/core/djangoapps/user_api/tests/test_models.py @@ -23,8 +23,7 @@ class UserPreferenceModelTest(ModuleStoreTestCase): def test_duplicate_user_key(self): user = UserFactory.create() UserPreferenceFactory.create(user=user, key="testkey", value="first") - with self.allow_transaction_exception(): - pytest.raises(IntegrityError, UserPreferenceFactory.create) + pytest.raises(IntegrityError, UserPreferenceFactory.create) def test_arbitrary_values(self): user = UserFactory.create() diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index e8499de9fb76..36dd785966e2 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -205,8 +205,8 @@ def test_queries(self): # Fetch the view and verify the query counts # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(73, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): - with check_mongo_calls(3): + with self.assertNumQueries(72, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url) diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index a246486e2db8..a3abea4b3db8 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -49,7 +49,7 @@ def test_queries(self): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(51, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): - with check_mongo_calls(3): + with self.assertNumQueries(50, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url) diff --git a/openedx/tests/completion_integration/test_models.py b/openedx/tests/completion_integration/test_models.py index 74c61de25461..8871272b3cf0 100644 --- a/openedx/tests/completion_integration/test_models.py +++ b/openedx/tests/completion_integration/test_models.py @@ -62,8 +62,8 @@ def setUp(self): self.set_up_completion() def test_changed_value(self): - with self.assertNumQueries(SELECT + UPDATE + 2 * SAVEPOINT + 4 * OTHER): - # OTHER = user exists, completion exists, 2x look up course in splitmodulestorecourseindex + with self.assertNumQueries(SELECT + UPDATE + 2 * SAVEPOINT + 2 * OTHER): + # OTHER = user exists, completion exists completion, isnew = models.BlockCompletion.objects.submit_completion( user=self.user, block_key=self.block_key, @@ -88,7 +88,7 @@ def test_unchanged_value(self): def test_new_user(self): newuser = UserFactory() - with self.assertNumQueries(SELECT + UPDATE + 4 * SAVEPOINT + 2 * OTHER): + with self.assertNumQueries(SELECT + UPDATE + 4 * SAVEPOINT): _, isnew = models.BlockCompletion.objects.submit_completion( user=newuser, block_key=self.block_key, @@ -99,7 +99,7 @@ def test_new_user(self): def test_new_block(self): newblock = UsageKey.from_string('block-v1:edx+test+run+type@video+block@puppers') - with self.assertNumQueries(SELECT + UPDATE + 4 * SAVEPOINT + 2 * OTHER): + with self.assertNumQueries(SELECT + UPDATE + 4 * SAVEPOINT): _, isnew = models.BlockCompletion.objects.submit_completion( user=self.user, block_key=newblock,