From 6f9b810f9a3ca0039d0882c5c10179f4e4ab3e49 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Sun, 26 Feb 2017 21:08:36 -0500 Subject: [PATCH] Storage-backed versioned Block Structures: Configuration --- common/djangoapps/request_cache/middleware.py | 22 +++++---- .../content/block_structure/admin.py | 23 +++++++++ .../content/block_structure/config.py | 28 ----------- .../block_structure/config/__init__.py | 47 +++++++++++++++++++ .../content/block_structure/config/models.py | 26 ++++++++++ .../block_structure/migrations/0001_config.py | 30 ++++++++++++ .../block_structure/migrations/__init__.py | 0 .../content/block_structure/tests/helpers.py | 15 ++++++ .../block_structure/tests/test_signals.py | 7 ++- .../djangolib/testing/tests/test_utils.py | 28 +---------- openedx/core/djangolib/testing/utils.py | 22 --------- .../core/djangolib/testing/waffle_utils.py | 27 +++++++++++ openedx/core/djangolib/waffle_utils.py | 15 ++++++ 13 files changed, 202 insertions(+), 88 deletions(-) create mode 100644 openedx/core/djangoapps/content/block_structure/admin.py delete mode 100644 openedx/core/djangoapps/content/block_structure/config.py create mode 100644 openedx/core/djangoapps/content/block_structure/config/__init__.py create mode 100644 openedx/core/djangoapps/content/block_structure/config/models.py create mode 100644 openedx/core/djangoapps/content/block_structure/migrations/0001_config.py create mode 100644 openedx/core/djangoapps/content/block_structure/migrations/__init__.py create mode 100644 openedx/core/djangolib/testing/waffle_utils.py create mode 100644 openedx/core/djangolib/waffle_utils.py diff --git a/common/djangoapps/request_cache/middleware.py b/common/djangoapps/request_cache/middleware.py index 05456a22f9..7aa3c0b048 100644 --- a/common/djangoapps/request_cache/middleware.py +++ b/common/djangoapps/request_cache/middleware.py @@ -85,17 +85,10 @@ def request_cached(f): """ Wrapper function to decorate with. """ - - # Build our cache key based on the module the function belongs to, the functions name, and a stringified - # list of arguments and a query string-style stringified list of keyword arguments. - converted_args = map(str, args) - converted_kwargs = map(str, reduce(list.__add__, map(list, sorted(kwargs.iteritems())), [])) - cache_keys = [f.__module__, f.func_name] + converted_args + converted_kwargs - cache_key = '.'.join(cache_keys) - # Check to see if we have a result in cache. If not, invoke our wrapped # function. Cache and return the result to the caller. rcache = RequestCache.get_request_cache() + cache_key = func_call_cache_key(f, *args, **kwargs) if cache_key in rcache.data: return rcache.data.get(cache_key) @@ -105,4 +98,17 @@ def request_cached(f): return result + wrapper.request_cached_contained_func = f return wrapper + + +def func_call_cache_key(func, *args, **kwargs): + """ + Returns a cache key based on the function's module + the function's name, and a stringified list of arguments + and a query string-style stringified list of keyword arguments. + """ + converted_args = map(str, args) + converted_kwargs = map(str, reduce(list.__add__, map(list, sorted(kwargs.iteritems())), [])) + cache_keys = [func.__module__, func.func_name] + converted_args + converted_kwargs + return '.'.join(cache_keys) diff --git a/openedx/core/djangoapps/content/block_structure/admin.py b/openedx/core/djangoapps/content/block_structure/admin.py new file mode 100644 index 0000000000..95ef3e140b --- /dev/null +++ b/openedx/core/djangoapps/content/block_structure/admin.py @@ -0,0 +1,23 @@ +""" +Django Admin for Block Structures. +""" +from django.contrib import admin + +from config_models.admin import ConfigurationModelAdmin +from .config.models import BlockStructureConfiguration + + +class BlockStructureAdmin(ConfigurationModelAdmin): + """ + Configuration Admin for BlockStructureConfiguration. + """ + def get_displayable_field_names(self): + """ + Excludes unused 'enabled field from super's list. + """ + displayable_field_names = super(BlockStructureAdmin, self).get_displayable_field_names() + displayable_field_names.remove('enabled') + return displayable_field_names + + +admin.site.register(BlockStructureConfiguration, BlockStructureAdmin) diff --git a/openedx/core/djangoapps/content/block_structure/config.py b/openedx/core/djangoapps/content/block_structure/config.py deleted file mode 100644 index 45d994f4b5..0000000000 --- a/openedx/core/djangoapps/content/block_structure/config.py +++ /dev/null @@ -1,28 +0,0 @@ -""" -This module contains various configuration settings via -waffle switches for the Block Structure framework. -""" - -from waffle import switch_is_active - - -INVALIDATE_CACHE_ON_PUBLISH = u'invalidate_cache_on_publish' -STORAGE_BACKING_FOR_CACHE = u'storage_backing_for_cache' -RAISE_ERROR_WHEN_NOT_FOUND = u'raise_error_when_not_found' - - -def is_enabled(setting_name): - """ - Returns whether the given setting is enabled. - """ - return switch_is_active( - waffle_switch_name(setting_name) - ) - - -def waffle_switch_name(setting_name): - """ - Returns the name of the waffle switch for the - given name of the setting. - """ - return u'block_structure.{}'.format(setting_name) diff --git a/openedx/core/djangoapps/content/block_structure/config/__init__.py b/openedx/core/djangoapps/content/block_structure/config/__init__.py new file mode 100644 index 0000000000..c5feba2990 --- /dev/null +++ b/openedx/core/djangoapps/content/block_structure/config/__init__.py @@ -0,0 +1,47 @@ +""" +This module contains various configuration settings via +waffle switches for the Block Structure framework. +""" +from openedx.core.djangolib.waffle_utils import is_switch_enabled +from request_cache.middleware import request_cached + +from .models import BlockStructureConfiguration + + +INVALIDATE_CACHE_ON_PUBLISH = u'invalidate_cache_on_publish' +STORAGE_BACKING_FOR_CACHE = u'storage_backing_for_cache' +RAISE_ERROR_WHEN_NOT_FOUND = u'raise_error_when_not_found' +PRUNE_OLD_VERSIONS = u'prune_old_versions' + + +def is_enabled(setting_name): + """ + Returns whether the given block_structure setting + is enabled. + """ + bs_waffle_name = _bs_waffle_switch_name(setting_name) + return is_switch_enabled(bs_waffle_name) + + +@request_cached +def num_versions_to_keep(): + """ + Returns and caches the current setting for num_versions_to_keep. + """ + return BlockStructureConfiguration.current().num_versions_to_keep + + +@request_cached +def cache_timeout_in_seconds(): + """ + Returns and caches the current setting for cache_timeout_in_seconds. + """ + return BlockStructureConfiguration.current().cache_timeout_in_seconds + + +def _bs_waffle_switch_name(setting_name): + """ + Returns the name of the waffle switch for the + given block structure setting. + """ + return u'block_structure.{}'.format(setting_name) diff --git a/openedx/core/djangoapps/content/block_structure/config/models.py b/openedx/core/djangoapps/content/block_structure/config/models.py new file mode 100644 index 0000000000..88d2005a5b --- /dev/null +++ b/openedx/core/djangoapps/content/block_structure/config/models.py @@ -0,0 +1,26 @@ +""" +Models for configuration of Block Structures. +""" +from django.db.models import IntegerField +from config_models.models import ConfigurationModel + + +class BlockStructureConfiguration(ConfigurationModel): + """ + Configuration model for Block Structures. + """ + DEFAULT_PRUNE_KEEP_COUNT = 5 + DEFAULT_CACHE_TIMEOUT_IN_SECONDS = 60 * 60 * 24 # 24 hours + + class Meta(object): + app_label = 'block_structure' + db_table = 'block_structure_config' + + num_versions_to_keep = IntegerField(blank=True, null=True, default=DEFAULT_PRUNE_KEEP_COUNT) + cache_timeout_in_seconds = IntegerField(blank=True, null=True, default=DEFAULT_CACHE_TIMEOUT_IN_SECONDS) + + def __unicode__(self): + return u"BlockStructureConfiguration: num_versions_to_keep: {}, cache_timeout_in_seconds: {}".format( + self.num_versions_to_keep, + self.cache_timeout_in_seconds, + ) diff --git a/openedx/core/djangoapps/content/block_structure/migrations/0001_config.py b/openedx/core/djangoapps/content/block_structure/migrations/0001_config.py new file mode 100644 index 0000000000..24a675dcfb --- /dev/null +++ b/openedx/core/djangoapps/content/block_structure/migrations/0001_config.py @@ -0,0 +1,30 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion +from django.conf import settings + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='BlockStructureConfiguration', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('num_versions_to_keep', models.IntegerField(default=5, null=True, blank=True)), + ('cache_timeout_in_seconds', models.IntegerField(default=86400, null=True, blank=True)), + ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')), + ], + options={ + 'db_table': 'block_structure_config', + }, + ), + ] diff --git a/openedx/core/djangoapps/content/block_structure/migrations/__init__.py b/openedx/core/djangoapps/content/block_structure/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/content/block_structure/tests/helpers.py b/openedx/core/djangoapps/content/block_structure/tests/helpers.py index 6ec3137028..bf9d7da80e 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/helpers.py +++ b/openedx/core/djangoapps/content/block_structure/tests/helpers.py @@ -3,7 +3,10 @@ Helpers for Course Blocks tests. """ from openedx.core.lib.block_structure.cache import BlockStructureCache +from openedx.core.djangolib.testing.waffle_utils import override_switch + from ..api import get_cache +from ..config import _bs_waffle_switch_name def is_course_in_block_structure_cache(course_key, store): @@ -12,3 +15,15 @@ def is_course_in_block_structure_cache(course_key, store): """ course_usage_key = store.make_course_usage_key(course_key) return BlockStructureCache(get_cache()).get(course_usage_key) is not None + + +class override_config_setting(override_switch): # pylint:disable=invalid-name + """ + Subclasses override_switch to use the block structure + name-spaced switch names. + """ + def __init__(self, name, active): + super(override_config_setting, self).__init__( + _bs_waffle_switch_name(name), + active + ) diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_signals.py b/openedx/core/djangoapps/content/block_structure/tests/test_signals.py index 358db01b48..1dbcd516fa 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_signals.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_signals.py @@ -3,7 +3,6 @@ Unit tests for the Course Blocks signals """ import ddt from mock import patch -from waffle.testutils import override_switch from opaque_keys.edx.locator import LibraryLocator, CourseLocator from xmodule.modulestore.exceptions import ItemNotFoundError @@ -11,9 +10,9 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from ..api import get_block_structure_manager +from ..config import INVALIDATE_CACHE_ON_PUBLISH from ..signals import _listen_for_course_publish -from ..config import INVALIDATE_CACHE_ON_PUBLISH, waffle_switch_name -from .helpers import is_course_in_block_structure_cache +from .helpers import is_course_in_block_structure_cache, override_config_setting @ddt.ddt @@ -55,7 +54,7 @@ class CourseBlocksSignalTest(ModuleStoreTestCase): def test_cache_invalidation(self, invalidate_cache_enabled, mock_bs_manager_clear): test_display_name = "Jedi 101" - with override_switch(waffle_switch_name(INVALIDATE_CACHE_ON_PUBLISH), active=invalidate_cache_enabled): + with override_config_setting(INVALIDATE_CACHE_ON_PUBLISH, active=invalidate_cache_enabled): self.course.display_name = test_display_name self.store.update_item(self.course, self.user.id) diff --git a/openedx/core/djangolib/testing/tests/test_utils.py b/openedx/core/djangolib/testing/tests/test_utils.py index 3009c29190..cb5e123c1e 100644 --- a/openedx/core/djangolib/testing/tests/test_utils.py +++ b/openedx/core/djangolib/testing/tests/test_utils.py @@ -5,9 +5,9 @@ from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser from django.http.request import HttpRequest from django.test import TestCase -from waffle.models import Switch -from ..utils import get_mock_request, toggle_switch +from ..utils import get_mock_request + USER_MODEL = get_user_model() @@ -28,27 +28,3 @@ class TestGetMockRequest(TestCase): def test_mock_request_without_user(self): request = get_mock_request() self.assertIsInstance(request.user, AnonymousUser) - - -class TestToggleSwitch(TestCase): - """ - Verify that the toggle_switch utility can be used to turn Waffle Switches - on and off. - """ - def test_toggle_switch(self): - """Verify that a new switch can be turned on and off.""" - name = 'foo' - - switch = toggle_switch(name) - - # Verify that the switch was saved. - self.assertEqual(switch, Switch.objects.get()) - - # Verify that the switch has the right name and is active. - self.assertEqual(switch.name, name) - self.assertTrue(switch.active) - - switch = toggle_switch(name) - - # Verify that the switch has been turned off. - self.assertFalse(switch.active) diff --git a/openedx/core/djangolib/testing/utils.py b/openedx/core/djangolib/testing/utils.py index 07f0ba0c18..3aca971047 100644 --- a/openedx/core/djangolib/testing/utils.py +++ b/openedx/core/djangolib/testing/utils.py @@ -190,25 +190,3 @@ def skip_unless_lms(func): Only run the decorated test in the LMS test suite """ return skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS')(func) - - -def toggle_switch(name, active=True): - """ - Activate or deactivate a Waffle switch. The switch is created if it does not exist. - - Arguments: - name (str): Name of the switch to be toggled. - - Keyword Arguments: - active (bool): Whether a newly created switch should be on or off. - - Returns: - Switch - """ - switch, created = Switch.objects.get_or_create(name=name, defaults={'active': active}) - - if not created: - switch.active = not switch.active - switch.save() - - return switch diff --git a/openedx/core/djangolib/testing/waffle_utils.py b/openedx/core/djangolib/testing/waffle_utils.py new file mode 100644 index 0000000000..cc4d45e2f7 --- /dev/null +++ b/openedx/core/djangolib/testing/waffle_utils.py @@ -0,0 +1,27 @@ +""" +Test utilities when using waffle. +""" +from waffle.testutils import override_switch as waffle_override_switch +from request_cache.middleware import RequestCache, func_call_cache_key +from ..waffle_utils import is_switch_enabled + + +class override_switch(waffle_override_switch): # pylint:disable=invalid-name + """ + Subclasses waffle's override_switch in order clear the cache + used on the is_switch_enabled function. + """ + def _clear_cache(self): + """ + Clears the requestcached values on the is_switch_enabled function. + """ + cache_key = func_call_cache_key(is_switch_enabled.request_cached_contained_func, self.name) + RequestCache.get_request_cache().data.pop(cache_key, None) + + def __enter__(self): + self._clear_cache() + super(override_switch, self).__enter__() + + def __exit__(self, *args, **kwargs): + self._clear_cache() + super(override_switch, self).__exit__(*args, **kwargs) diff --git a/openedx/core/djangolib/waffle_utils.py b/openedx/core/djangolib/waffle_utils.py new file mode 100644 index 0000000000..c27e7870ea --- /dev/null +++ b/openedx/core/djangolib/waffle_utils.py @@ -0,0 +1,15 @@ +""" +Utilities for waffle usage. +""" +from request_cache.middleware import request_cached +from waffle import switch_is_active + + +@request_cached +def is_switch_enabled(waffle_name): + """ + Returns and caches whether the given waffle switch is enabled. + See testing.waffle_utils.override_config_setting for a + helper to override and clear the cache during tests. + """ + return switch_is_active(waffle_name)