diff --git a/openedx/core/djangoapps/config_model_utils/models.py b/openedx/core/djangoapps/config_model_utils/models.py index 2d29ac3605..909099c328 100644 --- a/openedx/core/djangoapps/config_model_utils/models.py +++ b/openedx/core/djangoapps/config_model_utils/models.py @@ -43,6 +43,11 @@ class StackedConfigurationModel(ConfigurationModel): """ class Meta(object): abstract = True + indexes = [ + # This index optimizes the .object.current_set() query + # by preventing a filesort + models.Index(fields=['site', 'org', 'course']) + ] KEY_FIELDS = ('site', 'org', 'course') STACKABLE_FIELDS = ('enabled',) @@ -134,23 +139,18 @@ class StackedConfigurationModel(ConfigurationModel): site_override_q | org_override_q | course_override_q - ).order_by( - # Sort nulls first, and in reverse specificity order - # so that the overrides are in the order of general to specific. - # - # Site | Org | Course - # -------------------- - # Null | Null | Null - # site | Null | Null - # Null | org | Null - # Null | Null | Course - F('course').desc(nulls_first=True), - F('org').desc(nulls_first=True), - F('site').desc(nulls_first=True), ) provenances = defaultdict(lambda: Provenance.default) - for override in overrides: + # We are sorting in python to avoid doing a filesort in the database for + # what will only be 4 rows at maximum + for override in sorted( + overrides, + # This particular sort order sorts None before not-None (because False < True) + # It sorts global first (because all entries are None), then site entries + # (because course_id and org are None), then org and course (by the same logic) + key=lambda o: (o.course_id is not None, o.org is not None, o.site_id is not None) + ): for field in stackable_fields: value = field.value_from_object(override) if value != field_defaults[field.name]: diff --git a/openedx/features/content_type_gating/migrations/0006_auto_20190308_1447.py b/openedx/features/content_type_gating/migrations/0006_auto_20190308_1447.py new file mode 100644 index 0000000000..6e3792e462 --- /dev/null +++ b/openedx/features/content_type_gating/migrations/0006_auto_20190308_1447.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-03-08 14:47 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('content_type_gating', '0005_auto_20190306_1547'), + ] + + operations = [ + migrations.AddIndex( + model_name='contenttypegatingconfig', + index=models.Index(fields=['site', 'org', 'course'], name='content_typ_site_id_e91576_idx'), + ), + ] diff --git a/openedx/features/course_duration_limits/migrations/0006_auto_20190308_1447.py b/openedx/features/course_duration_limits/migrations/0006_auto_20190308_1447.py new file mode 100644 index 0000000000..f2ae47015f --- /dev/null +++ b/openedx/features/course_duration_limits/migrations/0006_auto_20190308_1447.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-03-08 14:47 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('course_duration_limits', '0005_auto_20190306_1546'), + ] + + operations = [ + migrations.AddIndex( + model_name='coursedurationlimitconfig', + index=models.Index(fields=['site', 'org', 'course'], name='course_dura_site_id_424016_idx'), + ), + ] diff --git a/openedx/features/course_duration_limits/tests/test_models.py b/openedx/features/course_duration_limits/tests/test_models.py index 584a4b210b..82e98d7f33 100644 --- a/openedx/features/course_duration_limits/tests/test_models.py +++ b/openedx/features/course_duration_limits/tests/test_models.py @@ -157,10 +157,10 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): @ddt.data( # Generate all combinations of setting each configuration level to True/False/None - *itertools.product(*[(True, False, None)] * 4) + *itertools.product(*([(True, False, None)] * 4 + [(True, False)])) ) @ddt.unpack - def test_config_overrides(self, global_setting, site_setting, org_setting, course_setting): + def test_config_overrides(self, global_setting, site_setting, org_setting, course_setting, reverse_order): """ Test that the stacked configuration overrides happen in the correct order and priority. @@ -189,10 +189,16 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): test_course = CourseOverviewFactory.create(org='test-org') test_site_cfg = SiteConfigurationFactory.create(values={'course_org_filter': test_course.org}) - CourseDurationLimitConfig.objects.create(enabled=global_setting) - CourseDurationLimitConfig.objects.create(course=test_course, enabled=course_setting) - CourseDurationLimitConfig.objects.create(org=test_course.org, enabled=org_setting) - CourseDurationLimitConfig.objects.create(site=test_site_cfg.site, enabled=site_setting) + if reverse_order: + CourseDurationLimitConfig.objects.create(site=test_site_cfg.site, enabled=site_setting) + CourseDurationLimitConfig.objects.create(org=test_course.org, enabled=org_setting) + CourseDurationLimitConfig.objects.create(course=test_course, enabled=course_setting) + CourseDurationLimitConfig.objects.create(enabled=global_setting) + else: + CourseDurationLimitConfig.objects.create(enabled=global_setting) + CourseDurationLimitConfig.objects.create(course=test_course, enabled=course_setting) + CourseDurationLimitConfig.objects.create(org=test_course.org, enabled=org_setting) + CourseDurationLimitConfig.objects.create(site=test_site_cfg.site, enabled=site_setting) expected_global_setting = self._resolve_settings([global_setting]) expected_site_setting = self._resolve_settings([global_setting, site_setting])