From 96e5ff8dce2817453e028bb035266bb2ab6ea83f Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 29 Sep 2021 15:55:52 -0700 Subject: [PATCH] feat: store split modulestore's course indexes in Django/MySQL Split modulestore persists data in three MongoDB "collections": course_index (list of courses and the current version of each), structure (outline of the courses, and some XBlock fields), and definition (other XBlock fields). While "structure" and "definition" data can get very large, which is one of the reasons MongoDB was chosen for modulestore, the course index data is very small. By moving course index data to MySQL / a django model, we get these advantages: * Full history of changes to the course index data is now preserved * Includes a django admin view to inspect the list of courses and libraries * It's much easier to "reset" a corrupted course to a known working state, by using the simple-history revert tools from the django admin. * The remaining MongoDB collections (structure and definition) are essentially just used as key-value stores of large JSON data structures. This paves the way for future changes that allow migrating courses one at a time from MongoDB to S3, and thus eliminating any use of MongoDB by split modulestore, simplifying the stack. --- .../contentstore/tests/test_clone_course.py | 2 +- .../contentstore/tests/test_course_listing.py | 10 +- .../contentstore/tests/test_orphan.py | 2 +- .../views/tests/test_course_index.py | 12 +- .../contentstore/views/tests/test_item.py | 2 +- cms/envs/common.py | 1 + .../split_modulestore_django/__init__.py | 0 .../split_modulestore_django/admin.py | 18 ++ .../split_modulestore_django/apps.py | 12 ++ .../migrations/0001_initial.py | 65 +++++++ .../migrations/0002_data_migration.py | 41 ++++ .../migrations/__init__.py | 0 .../split_modulestore_django/models.py | 164 ++++++++++++++++ .../split_mongo/mongo_connection.py | 183 +++++++++++++++++- .../xmodule/modulestore/split_mongo/split.py | 4 +- .../xmodule/modulestore/tests/django_utils.py | 26 ++- .../modulestore/tests/test_libraries.py | 18 +- .../tests/test_mixed_modulestore.py | 153 ++++++++------- .../tests/test_mongo_call_count.py | 18 +- .../xmodule/modulestore/tests/test_publish.py | 1 + .../tests/test_split_modulestore.py | 1 + .../test_split_modulestore_bulk_operations.py | 4 +- .../test_split_mongo_mongo_connection.py | 6 +- .../tests/test_split_w_old_mongo.py | 1 + .../tests/test_course_metadata_utils.py | 1 + .../backends/tests/test_badgr_backend.py | 3 +- lms/djangoapps/badges/tests/test_models.py | 4 +- .../bulk_enroll/tests/test_views.py | 3 +- .../tests/test_field_override_performance.py | 32 +-- lms/djangoapps/ccx/tests/test_models.py | 6 +- .../course_api/blocks/tests/test_api.py | 16 +- .../courseware/tests/test_module_render.py | 12 +- lms/djangoapps/courseware/tests/test_views.py | 2 +- lms/djangoapps/courseware/testutils.py | 6 +- .../django_comment_client/base/tests.py | 4 +- .../discussion/rest_api/tests/test_views.py | 6 +- lms/djangoapps/discussion/tests/test_views.py | 8 +- lms/djangoapps/grades/tests/test_tasks.py | 10 +- .../grades/tests/test_transformer.py | 2 +- lms/djangoapps/instructor/tests/test_tools.py | 4 +- .../tests/views/test_instructor_dashboard.py | 2 +- .../tests/test_tasks_helper.py | 7 +- lms/envs/common.py | 1 + .../djangoapps/bookmarks/tests/test_models.py | 20 +- .../djangoapps/bookmarks/tests/test_tasks.py | 8 +- .../tests/test_course_overviews.py | 2 +- .../course_groups/tests/test_cohorts.py | 2 +- .../courseware_api/tests/test_views.py | 10 +- .../djangoapps/user_api/tests/test_models.py | 3 +- .../tests/views/test_course_home.py | 4 +- .../tests/views/test_course_updates.py | 4 +- .../completion_integration/test_models.py | 8 +- 52 files changed, 729 insertions(+), 205 deletions(-) create mode 100644 common/djangoapps/split_modulestore_django/__init__.py create mode 100644 common/djangoapps/split_modulestore_django/admin.py create mode 100644 common/djangoapps/split_modulestore_django/apps.py create mode 100644 common/djangoapps/split_modulestore_django/migrations/0001_initial.py create mode 100644 common/djangoapps/split_modulestore_django/migrations/0002_data_migration.py create mode 100644 common/djangoapps/split_modulestore_django/migrations/__init__.py create mode 100644 common/djangoapps/split_modulestore_django/models.py diff --git a/cms/djangoapps/contentstore/tests/test_clone_course.py b/cms/djangoapps/contentstore/tests/test_clone_course.py index 34b3ae7053..f6ef12fddb 100644 --- a/cms/djangoapps/contentstore/tests/test_clone_course.py +++ b/cms/djangoapps/contentstore/tests/test_clone_course.py @@ -125,7 +125,7 @@ class CloneCourseTest(CourseTestCase): ) # try to hit the generic exception catch - with patch('xmodule.modulestore.split_mongo.mongo_connection.MongoConnection.insert_course_index', Mock(side_effect=Exception)): # lint-amnesty, pylint: disable=line-too-long + with patch('xmodule.modulestore.split_mongo.mongo_connection.MongoPersistenceBackend.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 acd2fbfd82..dcffbd7954 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -157,8 +157,8 @@ class TestCourseListing(ModuleStoreTestCase): self.assertEqual(len(list(courses_iter)), 0) @ddt.data( - (ModuleStoreEnum.Type.split, 3), - (ModuleStoreEnum.Type.mongo, 2) + (ModuleStoreEnum.Type.split, 2), + (ModuleStoreEnum.Type.mongo, 1) ) @ddt.unpack def test_staff_course_listing(self, default_store, mongo_calls): @@ -239,8 +239,8 @@ class TestCourseListing(ModuleStoreTestCase): ) @ddt.data( - (ModuleStoreEnum.Type.split, 3, 3), - (ModuleStoreEnum.Type.mongo, 2, 2) + (ModuleStoreEnum.Type.split, 2, 2), + (ModuleStoreEnum.Type.mongo, 1, 1) ) @ddt.unpack def test_course_listing_performance(self, store, courses_list_from_group_calls, courses_list_calls): @@ -289,7 +289,7 @@ class TestCourseListing(ModuleStoreTestCase): # Calls: # 1) query old mongo # 2) get_more on old mongo - # 3) query split (but no courses so no fetching of data) + # 3) query split (handled with MySQL only) @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 48745753f4..1e19bc21fc 100644 --- a/cms/djangoapps/contentstore/tests/test_orphan.py +++ b/cms/djangoapps/contentstore/tests/test_orphan.py @@ -103,7 +103,7 @@ class TestOrphan(TestOrphanBase): self.assertIn(str(location), orphans) @ddt.data( - (ModuleStoreEnum.Type.split, 9, 5), + (ModuleStoreEnum.Type.split, 5, 3), (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 b2182b9ec9..dff64735d5 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 @@ class TestCourseIndexArchived(CourseTestCase): @ddt.data( # Staff user has course staff access - (True, 'staff', None, 3, 18), - (False, 'staff', None, 3, 18), + (True, 'staff', None, 1, 20), + (False, 'staff', None, 1, 20), # Base user has global staff access - (True, 'user', ORG, 3, 18), - (False, 'user', ORG, 3, 18), - (True, 'user', None, 3, 18), - (False, 'user', None, 3, 18), + (True, 'user', ORG, 1, 20), + (False, 'user', ORG, 1, 20), + (True, 'user', None, 1, 20), + (False, 'user', None, 1, 20), ) @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 0d93d8b454..a5367abc24 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -2528,7 +2528,7 @@ class TestXBlockInfo(ItemTest): self.validate_course_xblock_info(json_response, course_outline=True) @ddt.data( - (ModuleStoreEnum.Type.split, 4, 4), + (ModuleStoreEnum.Type.split, 3, 3), (ModuleStoreEnum.Type.mongo, 5, 7), ) @ddt.unpack diff --git a/cms/envs/common.py b/cms/envs/common.py index 5036425d25..1b01d6c4f1 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1456,6 +1456,7 @@ INSTALLED_APPS = [ # 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 new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/split_modulestore_django/admin.py b/common/djangoapps/split_modulestore_django/admin.py new file mode 100644 index 0000000000..4dbddce9fa --- /dev/null +++ b/common/djangoapps/split_modulestore_django/admin.py @@ -0,0 +1,18 @@ +""" +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 new file mode 100644 index 0000000000..022533938d --- /dev/null +++ b/common/djangoapps/split_modulestore_django/apps.py @@ -0,0 +1,12 @@ +""" +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 new file mode 100644 index 0000000000..598090f8b0 --- /dev/null +++ b/common/djangoapps/split_modulestore_django/migrations/0001_initial.py @@ -0,0 +1,65 @@ +# 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 new file mode 100644 index 0000000000..148e1b6b75 --- /dev/null +++ b/common/djangoapps/split_modulestore_django/migrations/0002_data_migration.py @@ -0,0 +1,41 @@ +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 new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/split_modulestore_django/models.py b/common/djangoapps/split_modulestore_django/models.py new file mode 100644 index 0000000000..b3f8b6d145 --- /dev/null +++ b/common/djangoapps/split_modulestore_django/models.py @@ -0,0 +1,164 @@ +""" +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 c868ee89f2..e51dc9e0df 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -13,11 +13,14 @@ 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 @@ -243,7 +246,7 @@ class CourseStructureCache: self.cache.set(key, compressed_pickled_data, None) -class MongoConnection: +class MongoPersistenceBackend: """ Segregation of pymongo functions from the data modeling mechanisms for split modulestore. """ @@ -429,15 +432,18 @@ class MongoConnection: return courses_queries - def insert_course_index(self, course_index, course_context=None): + def insert_course_index(self, course_index, course_context=None, last_update_already_set=False): """ Create the course_index in the db """ with TIMER.timer("insert_course_index", course_context): - course_index['last_update'] = datetime.datetime.now(pytz.utc) + # 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: self.course_index.insert_one(course_index) - def update_course_index(self, course_index, from_index=None, course_context=None): + def update_course_index(self, course_index, from_index=None, course_context=None, last_update_already_set=False): """ Update the db record for course_index. @@ -457,7 +463,10 @@ class MongoConnection: 'course': course_index['course'], 'run': course_index['run'], } - course_index['last_update'] = datetime.datetime.now(pytz.utc) + # 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: self.course_index.replace_one(query, course_index, upsert=False,) def delete_course_index(self, course_key): @@ -551,3 +560,167 @@ class MongoConnection: 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 ac50ace8a7..7878b85d07 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 @@ from xmodule.modulestore.exceptions import ( VersionConflictError ) from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope -from xmodule.modulestore.split_mongo.mongo_connection import DuplicateKeyError, MongoConnection +from xmodule.modulestore.split_mongo.mongo_connection import DuplicateKeyError, DjangoFlexPersistenceBackend 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 = MongoConnection(**doc_store_config) + self.db_connection = DjangoFlexPersistenceBackend(**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 908ad97baa..bd94c642d8 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 unittest.mock import patch from django.conf import settings from django.contrib.auth.models import AnonymousUser -from django.db import connections +from django.db import connections, transaction from django.test import TestCase from django.test.utils import override_settings @@ -331,6 +331,30 @@ 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(): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py index 6cb17268a7..d607c0e7e9 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py @@ -23,17 +23,21 @@ class TestLibraries(MixedSplitTestCase): def test_create_library(self): """ - Test that we can create a library, and see how many mongo calls it uses to do so. + Test that we can create a library, and see how many database calls it uses to do so. Expected mongo calls, in order: - find_one({'org': '...', 'run': 'library', 'course': '...'}) - insert(definition: {'block_type': 'library', 'fields': {}}) + -> insert(definition: {'block_type': 'library', 'fields': {}}) + -> insert_structure(bulk) + -> insert_course_index(bulk) - insert_structure(bulk) - insert_course_index(bulk) - get_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(2, 3): + with check_mongo_calls(0, 3), self.assertNumQueries(5): 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 27b014a3a1..f847630403 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -366,8 +366,9 @@ 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 - @ddt.data((ModuleStoreEnum.Type.mongo, [1, 1], 0), (ModuleStoreEnum.Type.split, [2, 2], 0)) + # 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)) @ddt.unpack def test_has_item(self, default_ms, max_find, max_send): self.initdb(default_ms) @@ -390,17 +391,17 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # split: # problem: active_versions, structure # non-existent problem: ditto - @ddt.data((ModuleStoreEnum.Type.mongo, [3, 2], 0), (ModuleStoreEnum.Type.split, [2, 2], 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 0, [3, 2], 0), (ModuleStoreEnum.Type.split, 1, [1, 1], 0)) @ddt.unpack - def test_get_item(self, default_ms, max_find, max_send): + def test_get_item(self, default_ms, num_mysql, max_find, max_send): self.initdb(default_ms) self._create_block_hierarchy() - with check_mongo_calls(max_find.pop(0), max_send): + with check_mongo_calls(max_find.pop(0), max_send), self.assertNumQueries(num_mysql): 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): + with check_mongo_calls(max_find.pop(0), max_send), self.assertNumQueries(num_mysql): with pytest.raises(ItemNotFoundError): self.store.get_item(self.fake_location) @@ -411,15 +412,16 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Draft: # wildcard query, 6! load pertinent items for inheritance calls, load parents, course root fetch (why) # Split: - # active_versions (with regex), structure, and spurious active_versions refetch - @ddt.data((ModuleStoreEnum.Type.mongo, 14, 0), (ModuleStoreEnum.Type.split, 4, 0)) + # 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)) @ddt.unpack - def test_get_items(self, default_ms, max_find, max_send): + def test_get_items(self, default_ms, num_mysql, 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): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): modules = self.store.get_items(course_locn.course_key, qualifiers={'category': 'problem'}) assert len(modules) == 6 @@ -513,13 +515,16 @@ 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: get draft, get ancestors up to course (2-6), compute inheritance + # draft: + # find: get draft, get ancestors up to course (2-6), compute inheritance # sends: update problem and then each ancestor up to course (edit info) - # 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)) + # 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)) @ddt.unpack - def test_update_item(self, default_ms, max_find, max_send): + def test_update_item(self, default_ms, num_mysql, max_find, max_send): """ Update should succeed for r/w dbs """ @@ -529,7 +534,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): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): problem = self.store.update_item(problem, self.user_id) assert problem.max_attempts == 2, "Update didn't persist" @@ -909,11 +914,12 @@ 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, 7, 2), (ModuleStoreEnum.Type.split, 3, 3)) + @ddt.data((ModuleStoreEnum.Type.mongo, 0, 7, 2), (ModuleStoreEnum.Type.split, 4, 2, 3)) @ddt.unpack - def test_delete_item(self, default_ms, max_find, max_send): + def test_delete_item(self, default_ms, num_mysql, max_find, max_send): """ Delete should reject on r/o db and work on r/w one """ @@ -922,7 +928,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): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): self.store.delete_item(self.writable_chapter_location, self.user_id) # verify it's gone @@ -933,15 +939,16 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.store.get_item(self.writable_chapter_location, revision=ModuleStoreEnum.RevisionOption.published_only) # Draft: - # queries: find parent (definition.children), count versions of item, get parent, count grandparents, - # inheritance items, draft item, draft child, inheritance + # find: 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: - # queries: active_versions, draft and published structures, definition (unnecessary) + # mysql: SplitModulestoreCourseIndex - select 2x (by course_id, by objectid), update, update historical record + # find: draft and published structures, definition (unnecessary) # sends: update published (why?), draft, and active_versions - @ddt.data((ModuleStoreEnum.Type.mongo, 9, 2), (ModuleStoreEnum.Type.split, 4, 3)) + @ddt.data((ModuleStoreEnum.Type.mongo, 0, 9, 2), (ModuleStoreEnum.Type.split, 4, 3, 3)) @ddt.unpack - def test_delete_private_vertical(self, default_ms, max_find, max_send): + def test_delete_private_vertical(self, default_ms, num_mysql, 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. @@ -972,7 +979,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): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): 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'): @@ -990,11 +997,12 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # find: find parent (definition.children) 2x, find draft item, get inheritance items # send: one delete query for specific item # Split: - # find: active_version & structure (cached) + # mysql: SplitModulestoreCourseIndex - select 2x (by course_id, by objectid), update, update historical record + # find: structure (cached) # send: update structure and active_versions - @ddt.data((ModuleStoreEnum.Type.mongo, 4, 1), (ModuleStoreEnum.Type.split, 2, 2)) + @ddt.data((ModuleStoreEnum.Type.mongo, 0, 4, 1), (ModuleStoreEnum.Type.split, 4, 1, 2)) @ddt.unpack - def test_delete_draft_vertical(self, default_ms, max_find, max_send): + def test_delete_draft_vertical(self, default_ms, num_mysql, max_find, max_send): """ Test deleting a draft vertical which has a published version. """ @@ -1024,23 +1032,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): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): self.store.delete_item(private_leaf.location, self.user_id) # Draft: - # 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 + # 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) # Split: - # 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)) + # 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)) @ddt.unpack - def test_get_courses(self, default_ms, max_find, max_send): + def test_get_courses(self, default_ms, num_mysql, max_find, max_send): self.initdb(default_ms) # we should have one course across all stores - with check_mongo_calls(max_find, max_send): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): courses = self.store.get_courses() course_ids = [course.location for course in courses] assert len(courses) == 1, f'Not one course: {course_ids}' @@ -1074,16 +1082,16 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): assert len(mongo_course.children) == 1 # draft is 2: find out which ms owns course, get item - # split: active_versions, structure, definition (to load course wiki string) - @ddt.data((ModuleStoreEnum.Type.mongo, 2, 0), (ModuleStoreEnum.Type.split, 3, 0)) + # 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)) @ddt.unpack - def test_get_course(self, default_ms, max_find, max_send): + def test_get_course(self, default_ms, num_mysql, 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): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): course = self.store.get_item(self.course_locations[self.MONGO_COURSEID]) assert course.id == self.course_locations[self.MONGO_COURSEID].course_key @@ -1112,16 +1120,16 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # still only 2) # Draft: get_parent # Split: active_versions, structure - @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 0, 1, 0), (ModuleStoreEnum.Type.split, 1, 1, 0)) @ddt.unpack - def test_get_parent_locations(self, default_ms, max_find, max_send): + def test_get_parent_locations(self, default_ms, num_mysql, 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): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): 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 @@ -1629,10 +1637,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: active_versions & structure - @ddt.data((ModuleStoreEnum.Type.mongo, [12, 3], 0), (ModuleStoreEnum.Type.split, [3, 2], 0)) + # 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)) @ddt.unpack - def test_path_to_location(self, default_ms, num_finds, num_sends): + def test_path_to_location(self, default_ms, num_mysql, num_finds, num_sends): """ Make sure that path_to_location works """ @@ -1651,7 +1659,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): + with check_mongo_calls(num_finds.pop(0), num_sends), self.assertNumQueries(num_mysql): path = path_to_location(self.store, location) assert path == expected @@ -1868,10 +1876,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, structure - @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0)) + # Split: active_versions (mysql), structure (mongo) + @ddt.data((ModuleStoreEnum.Type.mongo, 0, 1, 0), (ModuleStoreEnum.Type.split, 1, 1, 0)) @ddt.unpack - def test_get_orphans(self, default_ms, max_find, max_send): + def test_get_orphans(self, default_ms, num_mysql, max_find, max_send): """ Test finding orphans. """ @@ -1903,7 +1911,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): block_id=location.block_id ) - with check_mongo_calls(max_find, max_send): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): found_orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key) self.assertCountEqual(found_orphans, orphan_locations) @@ -2004,17 +2012,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 and split - # Split: wildcard search of draft and split - @ddt.data((ModuleStoreEnum.Type.mongo, 2, 0), (ModuleStoreEnum.Type.split, 2, 0)) + # 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)) @ddt.unpack - def test_get_courses_for_wiki(self, default_ms, max_find, max_send): + def test_get_courses_for_wiki(self, default_ms, num_mysql, 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): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): 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 @@ -2028,13 +2036,18 @@ 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: 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)) + # 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)) @ddt.unpack - def test_unpublish(self, default_ms, max_find, max_send): + def test_unpublish(self, default_ms, num_mysql, max_find, max_send): """ Test calling unpublish """ @@ -2052,7 +2065,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): assert published_xblock is not None # unpublish - with check_mongo_calls(max_find, max_send): + with check_mongo_calls(max_find, max_send), self.assertNumQueries(num_mysql): self.store.unpublish(self.vertical_x1a, self.user_id) # lint-amnesty, pylint: disable=no-member with pytest.raises(ItemNotFoundError): @@ -2069,10 +2082,10 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): assert draft_xblock is not None # Draft: specific query for revision None - # Split: active_versions, structure - @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0)) + # Split: active_versions from MySQL, structure from mongo + @ddt.data((ModuleStoreEnum.Type.mongo, 0, 1, 0), (ModuleStoreEnum.Type.split, 1, 1, 0)) @ddt.unpack - def test_has_published_version(self, default_ms, max_find, max_send): + def test_has_published_version(self, default_ms, mysql_queries, max_find, max_send): """ Test the has_published_version method """ @@ -2082,7 +2095,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 check_mongo_calls(max_find, max_send): + with self.assertNumQueries(mysql_queries), check_mongo_calls(max_find, max_send): assert not self.store.has_published_version(item) # Private -> Public @@ -3771,7 +3784,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, 2, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 1, 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 8402d2be88..ba618da8f3 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 @@ 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, 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) + (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), ) @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, 4), + (MIXED_SPLIT_MODULESTORE_BUILDER, 3), ) @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 efcf5ef68c..c2a4b16848 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py @@ -163,6 +163,7 @@ 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 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 1739f48761..3550ef69ea 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -53,6 +53,7 @@ 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 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 da0a6d0d7a..6690770a86 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 @@ import ddt from bson.objectid import ObjectId from opaque_keys.edx.locator import CourseLocator -from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection +from xmodule.modulestore.split_mongo.mongo_connection import MongoPersistenceBackend 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=MongoConnection) + self.conn = self.bulk.db_connection = MagicMock(name='db_connection', spec=MongoPersistenceBackend) 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 3460470186..008c1cc2a6 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/MongoConnection """ +""" Test the behavior of split_mongo/MongoPersistenceBackend """ 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 MongoConnection +from xmodule.modulestore.split_mongo.mongo_connection import MongoPersistenceBackend 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 = MongoConnection('useless', 'useless', 'useless') + useless_conn = MongoPersistenceBackend('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 2798bcc9fe..fa55146808 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,6 +19,7 @@ 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 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 dc3ad0813f..f1bd4b6f8d 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py +++ b/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py @@ -34,6 +34,7 @@ _LAST_WEEK = _TODAY - timedelta(days=7) _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 3a500c117b..1817fa92a8 100644 --- a/lms/djangoapps/badges/backends/tests/test_badgr_backend.py +++ b/lms/djangoapps/badges/backends/tests/test_badgr_backend.py @@ -114,7 +114,8 @@ class BadgrBackendTestCase(ModuleStoreTestCase, EventTrackingTestCase): Verify badge spec creation works. """ self.handler._get_access_token = Mock(return_value='12345') - self.handler._create_badge(self.badge_class) + with self.allow_transaction_exception(): + 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 154dd74704..41a49c6629 100644 --- a/lms/djangoapps/badges/tests/test_models.py +++ b/lms/djangoapps/badges/tests/test_models.py @@ -196,8 +196,8 @@ class BadgeClassTest(ModuleStoreTestCase): Verify handing incomplete data for required fields when making a badge class raises an Integrity error. """ image = get_image('good') - pytest.raises(IntegrityError, BadgeClass.get_badge_class, slug='new_slug', issuing_component='new_component', - image_file_handle=image) + with pytest.raises(IntegrityError), self.allow_transaction_exception(): + 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 1ce5b1775b..70e37b15c7 100644 --- a/lms/djangoapps/bulk_enroll/tests/test_views.py +++ b/lms/djangoapps/bulk_enroll/tests/test_views.py @@ -107,7 +107,8 @@ class BulkEnrollmentTest(ModuleStoreTestCase, LoginEnrollmentTestCase, APITestCa """ Test that non global staff users are forbidden from API use. """ self.staff.is_staff = False self.staff.save() - response = self.request_bulk_enroll() + with self.allow_transaction_exception(): + 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 8cfca804d3..0970ab8c99 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 = 33 + QUERY_COUNT = 34 TEST_DATA = { - ('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), + ('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), } diff --git a/lms/djangoapps/ccx/tests/test_models.py b/lms/djangoapps/ccx/tests/test_models.py index 006ba60de5..97c8761b59 100644 --- a/lms/djangoapps/ccx/tests/test_models.py +++ b/lms/djangoapps/ccx/tests/test_models.py @@ -46,7 +46,7 @@ class TestCCX(ModuleStoreTestCase): def test_ccx_course_caching(self): """verify that caching the propery works to limit queries""" - with check_mongo_calls(3): + with check_mongo_calls(2): # 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 @@ class TestCCX(ModuleStoreTestCase): """verify that caching the start property works to limit queries""" now = datetime.now(utc) self.set_ccx_override('start', now) - with check_mongo_calls(3): + with check_mongo_calls(2): # 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 @@ class TestCCX(ModuleStoreTestCase): """verify that caching the due property works to limit queries""" expected = datetime.now(utc) self.set_ccx_override('due', expected) - with check_mongo_calls(3): + with check_mongo_calls(2): # 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 e7b3fbdd9a..5297f7f9f2 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -224,23 +224,17 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): ) @ddt.data( - *product( - ((ModuleStoreEnum.Type.mongo, 5), (ModuleStoreEnum.Type.split, 3)), - (True, False), - ) + (ModuleStoreEnum.Type.mongo, 5, True, 23), + (ModuleStoreEnum.Type.mongo, 5, False, 13), + (ModuleStoreEnum.Type.split, 2, True, 24), + (ModuleStoreEnum.Type.split, 2, False, 14), ) @ddt.unpack - def test_query_counts_uncached(self, store_type_tuple, with_storage_backing): - store_type, expected_mongo_queries = store_type_tuple + def test_query_counts_uncached(self, store_type, expected_mongo_queries, with_storage_backing, num_sql_queries): 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 4cc29a5e11..e6acde7d22 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1014,9 +1014,9 @@ class TestTOC(ModuleStoreTestCase): # Split makes 2 queries to load the course to depth 2: # - 1 for the structure # - 1 for 5 definitions - # 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)) + # 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)) @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 @@ class TestTOC(ModuleStoreTestCase): # Split makes 2 queries to load the course to depth 2: # - 1 for the structure # - 1 for 5 definitions - # 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)) + # 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)) @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 ddafdde656..350bf95eda 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, 4, 160), + (ModuleStoreEnum.Type.split, 3, 161), ) @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 6c4caa6be1..e3fb0132ed 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -162,9 +162,9 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta): @ddt.data( ('vertical_block', ModuleStoreEnum.Type.mongo, 13), - ('vertical_block', ModuleStoreEnum.Type.split, 6), + ('vertical_block', ModuleStoreEnum.Type.split, 4), ('html_block', ModuleStoreEnum.Type.mongo, 14), - ('html_block', ModuleStoreEnum.Type.split, 6), + ('html_block', ModuleStoreEnum.Type.split, 4), ) @ddt.unpack def test_courseware_html(self, block_name, default_store, mongo_calls): @@ -192,7 +192,7 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta): @ddt.data( (ModuleStoreEnum.Type.mongo, 5), - (ModuleStoreEnum.Type.split, 5), + (ModuleStoreEnum.Type.split, 4), ) @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 efb0189809..efc96a325d 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 @@ class ViewsQueryCountTestCase( @ddt.data( (ModuleStoreEnum.Type.mongo, 3, 4, 39), - (ModuleStoreEnum.Type.split, 3, 13, 39), + (ModuleStoreEnum.Type.split, 3, 8, 44), ) @ddt.unpack @count_queries @@ -411,7 +411,7 @@ class ViewsQueryCountTestCase( @ddt.data( (ModuleStoreEnum.Type.mongo, 3, 3, 35), - (ModuleStoreEnum.Type.split, 3, 10, 35), + (ModuleStoreEnum.Type.split, 3, 6, 39), ) @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 61103af01c..ffcffbef6c 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 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): (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, 3, {"Test Topic 1": {"id": "test_topic_1"}}), - (2, ModuleStoreEnum.Type.split, 3, + (2, ModuleStoreEnum.Type.split, 2, {"Test Topic 1": {"id": "test_topic_1"}}), + (2, ModuleStoreEnum.Type.split, 2, {"Test Topic 1": {"id": "test_topic_1"}, "Test Topic 2": {"id": "test_topic_2"}}), - (10, ModuleStoreEnum.Type.split, 3, {"Test Topic 1": {"id": "test_topic_1"}}), + (10, ModuleStoreEnum.Type.split, 2, {"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 0cf438e231..fb1da7cb68 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, 3, 3, 21, 8), - (ModuleStoreEnum.Type.split, False, 50, 3, 3, 21, 8), + (ModuleStoreEnum.Type.split, False, 1, 2, 2, 22, 8), + (ModuleStoreEnum.Type.split, False, 50, 2, 2, 22, 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, 3, 3, 21, 8), - (ModuleStoreEnum.Type.split, True, 50, 3, 3, 21, 8), + (ModuleStoreEnum.Type.split, True, 1, 2, 2, 22, 8), + (ModuleStoreEnum.Type.split, True, 50, 2, 2, 22, 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 c68b17f37b..cfee479e46 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -164,8 +164,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest @ddt.data( (ModuleStoreEnum.Type.mongo, 1, 39, True), (ModuleStoreEnum.Type.mongo, 1, 39, False), - (ModuleStoreEnum.Type.split, 3, 39, True), - (ModuleStoreEnum.Type.split, 3, 39, False), + (ModuleStoreEnum.Type.split, 2, 40, True), + (ModuleStoreEnum.Type.split, 2, 40, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -178,7 +178,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest @ddt.data( (ModuleStoreEnum.Type.mongo, 1, 39), - (ModuleStoreEnum.Type.split, 3, 39), + (ModuleStoreEnum.Type.split, 2, 40), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -224,7 +224,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest @ddt.data( (ModuleStoreEnum.Type.mongo, 1, 22), - (ModuleStoreEnum.Type.split, 3, 22), + (ModuleStoreEnum.Type.split, 2, 23), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -239,7 +239,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest @ddt.data( (ModuleStoreEnum.Type.mongo, 1, 40), - (ModuleStoreEnum.Type.split, 3, 40), + (ModuleStoreEnum.Type.split, 2, 41), ) @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 f2bf8741b8..98685ea2d5 100644 --- a/lms/djangoapps/grades/tests/test_transformer.py +++ b/lms/djangoapps/grades/tests/test_transformer.py @@ -430,7 +430,7 @@ class MultiProblemModulestoreAccessTestCase(CourseStructureTestCase, SharedModul self.client.login(username=self.student.username, password=password) @ddt.data( - (ModuleStoreEnum.Type.split, 3), + (ModuleStoreEnum.Type.split, 2), (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 0713bd1f56..0840f74265 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -252,12 +252,12 @@ class TestSetDueDateExtension(ModuleStoreTestCase): def test_set_due_date_extension_invalid_date(self): extended = datetime.datetime(2009, 1, 1, 0, 0, tzinfo=UTC) - with pytest.raises(tools.DashboardError): + with pytest.raises(tools.DashboardError), self.allow_transaction_exception(): 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): + with pytest.raises(tools.DashboardError), self.allow_transaction_exception(): 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 90a35f1d6b..6be624e9fd 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 @@ class TestInstructorDashboardPerformance(ModuleStoreTestCase, LoginEnrollmentTes # check MongoDB calls count url = reverse('spoc_gradebook', kwargs={'course_id': self.course.id}) - with check_mongo_calls(9): + with check_mongo_calls(6): 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 7420dfa9d5..c3dd3eb9af 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 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): self._verify_cell_data_for_user(verified_user.username, course.id, 'Certificate Eligible', 'Y', num_rows=2) @ddt.data( - (ModuleStoreEnum.Type.mongo, 4), - (ModuleStoreEnum.Type.split, 3), + (ModuleStoreEnum.Type.mongo, 4, 46), + (ModuleStoreEnum.Type.split, 2, 47), ) @ddt.unpack - def test_query_counts(self, store_type, mongo_count): + def test_query_counts(self, store_type, mongo_count, expected_query_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,7 +401,6 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): 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 fdf5867418..b3d43bb36c 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2910,6 +2910,7 @@ INSTALLED_APPS = [ '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 46e5b8ed70..df5d89c5f5 100644 --- a/openedx/core/djangoapps/bookmarks/tests/test_models.py +++ b/openedx/core/djangoapps/bookmarks/tests/test_models.py @@ -276,11 +276,11 @@ class BookmarkModelTests(BookmarksTestsBase): (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', [], 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), + (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), ) @ddt.unpack def test_path_and_queries_on_create(self, store_type, block_to_bookmark, ancestors_attrs, expected_mongo_calls): @@ -376,11 +376,11 @@ class BookmarkModelTests(BookmarksTestsBase): # (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, 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), + (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), ) @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 f033e6bc1e..615d1b5d8a 100644 --- a/openedx/core/djangoapps/bookmarks/tests/test_tasks.py +++ b/openedx/core/djangoapps/bookmarks/tests/test_tasks.py @@ -108,10 +108,10 @@ class XBlockCacheTaskTests(BookmarksTestsBase): (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, 3), - (ModuleStoreEnum.Type.split, 4, 2, 3), - (ModuleStoreEnum.Type.split, 2, 3, 3), - (ModuleStoreEnum.Type.split, 2, 4, 3), + (ModuleStoreEnum.Type.split, 2, 2, 2), + (ModuleStoreEnum.Type.split, 4, 2, 2), + (ModuleStoreEnum.Type.split, 2, 3, 2), + (ModuleStoreEnum.Type.split, 2, 4, 2), ) @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 425f2c89a1..39b30bd417 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 @@ class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase, Cache 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, 3, 3)) + @ddt.data((ModuleStoreEnum.Type.mongo, 4, 4), (ModuleStoreEnum.Type.split, 2, 2)) @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 f3760cb987..8b082bef0b 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 @@ class TestCohortsAndPartitionGroups(ModuleStoreTestCase): self.partition_id, self.group1_id, ) - with pytest.raises(IntegrityError): + with pytest.raises(IntegrityError), self.allow_transaction_exception(): 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 2c66336bce..4e00ac0b23 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -396,7 +396,15 @@ class SequenceApiTestViews(MasqueradeMixin, BaseCoursewareTests): 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=self.chapter, category='sequential', hide_after_due=True, due=due) + 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, + ) 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 6a5d64bd24..5634aa00b9 100644 --- a/openedx/core/djangoapps/user_api/tests/test_models.py +++ b/openedx/core/djangoapps/user_api/tests/test_models.py @@ -23,7 +23,8 @@ class UserPreferenceModelTest(ModuleStoreTestCase): def test_duplicate_user_key(self): user = UserFactory.create() UserPreferenceFactory.create(user=user, key="testkey", value="first") - pytest.raises(IntegrityError, UserPreferenceFactory.create) + with self.allow_transaction_exception(): + 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 36dd785966..e8499de9fb 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 @@ class TestCourseHomePage(CourseHomePageTestCase): # lint-amnesty, pylint: disab # Fetch the view and verify the query counts # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(72, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): - with check_mongo_calls(4): + with self.assertNumQueries(73, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with check_mongo_calls(3): 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 a3abea4b3d..a246486e2d 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 @@ class TestCourseUpdatesPage(BaseCourseUpdatesTestCase): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(50, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): - with check_mongo_calls(4): + with self.assertNumQueries(51, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with check_mongo_calls(3): 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 8871272b3c..74c61de254 100644 --- a/openedx/tests/completion_integration/test_models.py +++ b/openedx/tests/completion_integration/test_models.py @@ -62,8 +62,8 @@ class SubmitCompletionTestCase(CompletionSetUpMixin, TestCase): self.set_up_completion() def test_changed_value(self): - with self.assertNumQueries(SELECT + UPDATE + 2 * SAVEPOINT + 2 * OTHER): - # OTHER = user exists, completion exists + with self.assertNumQueries(SELECT + UPDATE + 2 * SAVEPOINT + 4 * OTHER): + # OTHER = user exists, completion exists, 2x look up course in splitmodulestorecourseindex completion, isnew = models.BlockCompletion.objects.submit_completion( user=self.user, block_key=self.block_key, @@ -88,7 +88,7 @@ class SubmitCompletionTestCase(CompletionSetUpMixin, TestCase): def test_new_user(self): newuser = UserFactory() - with self.assertNumQueries(SELECT + UPDATE + 4 * SAVEPOINT): + with self.assertNumQueries(SELECT + UPDATE + 4 * SAVEPOINT + 2 * OTHER): _, isnew = models.BlockCompletion.objects.submit_completion( user=newuser, block_key=self.block_key, @@ -99,7 +99,7 @@ class SubmitCompletionTestCase(CompletionSetUpMixin, TestCase): 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): + with self.assertNumQueries(SELECT + UPDATE + 4 * SAVEPOINT + 2 * OTHER): _, isnew = models.BlockCompletion.objects.submit_completion( user=self.user, block_key=newblock,