diff --git a/Makefile b/Makefile index 49e63dca32..32b078d402 100644 --- a/Makefile +++ b/Makefile @@ -63,8 +63,10 @@ detect_changed_source_translations: ## check if translation files are up-to-date pull: ## update the Docker image used by "make shell" docker pull edxops/edxapp:latest -requirements: ## install development environment requirements +pre-requirements: ## install Python requirements for running pip-tools pip install -qr requirements/edx/pip-tools.txt + +requirements: pre-requirements ## install development environment requirements pip-sync -q requirements/edx/development.txt requirements/edx/private.* shell: ## launch a bash shell in a Docker container with all edx-platform dependencies installed @@ -87,15 +89,14 @@ REQ_FILES = \ requirements/edx/development \ scripts/xblock/requirements -upgrade: export CUSTOM_COMPILE_COMMAND=make upgrade -upgrade: ## update the pip requirements files to use the latest releases satisfying our constraints - pip install -qr requirements/edx/pip-tools.txt +compile-requirements: export CUSTOM_COMPILE_COMMAND=make upgrade +compile-requirements: ## Re-compile *.in requirements to *.txt @ export REBUILD='--rebuild'; \ for f in $(REQ_FILES); do \ echo ; \ echo "== $$f ===============================" ; \ - echo "pip-compile -v --no-emit-trusted-host --no-index $$REBUILD --upgrade -o $$f.txt $$f.in"; \ - pip-compile -v --no-emit-trusted-host --no-index $$REBUILD --upgrade -o $$f.txt $$f.in || exit 1; \ + echo "pip-compile -v --no-emit-trusted-host --no-index $$REBUILD ${COMPILE_OPTS} -o $$f.txt $$f.in"; \ + pip-compile -v --no-emit-trusted-host --no-index $$REBUILD ${COMPILE_OPTS} -o $$f.txt $$f.in || exit 1; \ export REBUILD=''; \ done # Post process all of the files generated above to work around open pip-tools issues @@ -105,6 +106,9 @@ upgrade: ## update the pip requirements files to use the latest releases satisfy sed '/^[dD]jango==/d' requirements/edx/testing.txt > requirements/edx/testing.tmp mv requirements/edx/testing.tmp requirements/edx/testing.txt +upgrade: pre-requirements ## update the pip requirements files to use the latest releases satisfying our constraints + $(MAKE) compile-requirements COMPILE_OPTS="--upgrade" + # These make targets currently only build LMS images. docker_build: docker build . -f Dockerfile --target lms -t openedx/edx-platform diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index 8bbcbb263a..086d97e287 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -1,139 +1,32 @@ """ -Utilities for waffle. - -Includes namespacing, caching, and course overrides for waffle flags. - -Usage: - -For Waffle Flags, first set up the namespace, and then create flags using the -namespace. For example:: - - WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='my_namespace') - - # Use CourseWaffleFlag when you are in the context of a course. - SOME_COURSE_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'some_course_feature', __name__) - # Use WaffleFlag when outside the context of a course. - SOME_FLAG = WaffleFlag(WAFFLE_FLAG_NAMESPACE, 'some_feature', __name__) - -You can check these flags in code using the following:: - - SOME_FLAG.is_enabled() - SOME_COURSE_FLAG.is_enabled(course_key) - -To test these WaffleFlags, see testutils.py. - -In the above examples, you will use Django Admin "waffle" section to configure -for a flag named: my_namespace.some_course_feature - -You could also use the Django Admin "waffle_utils" section to configure a course -override for this same flag (e.g. my_namespace.some_course_feature). - -For Waffle Switches, first set up the namespace, and then create the flag name. -For example:: - - WAFFLE_SWITCHES = WaffleSwitchNamespace(name=WAFFLE_NAMESPACE) - - ESTIMATE_FIRST_ATTEMPTED = 'estimate_first_attempted' - -You can then use the switch as follows:: - - WAFFLE_SWITCHES.is_enabled(waffle.ESTIMATE_FIRST_ATTEMPTED) - -To test WaffleSwitchNamespace, use the provided context managers. For example: - - with WAFFLE_SWITCHES.override(waffle.ESTIMATE_FIRST_ATTEMPTED, active=True): - ... - -For long-lived flags, you may want to change the default for devstack, sandboxes, -or new Open edX releases. For help with this, see: -openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst - -Also see ``WAFFLE_FLAG_CUSTOM_ATTRIBUTES`` and docstring for _set_waffle_flag_attribute -for temporarily instrumenting/monitoring waffle flag usage. - +Extra utilities for waffle: most classes are defined in edx_toggles.toggles (https://edx-toggles.readthedocs.io/), but +we keep here some extra classes for usage within edx-platform. These classes cover course override use cases. """ - import logging -import re -import traceback -from abc import ABCMeta from contextlib import contextmanager -from weakref import WeakSet -import crum -import six -from django.conf import settings -from edx_django_utils.monitoring import set_custom_attribute from opaque_keys.edx.keys import CourseKey -from waffle import flag_is_active, switch_is_active -from openedx.core.lib.cache_utils import get_cache as get_request_cache +from edx_toggles.toggles import WaffleFlag as BaseWaffleFlag +from edx_toggles.toggles import WaffleFlagNamespace +from edx_toggles.toggles import WaffleSwitch as BaseWaffleSwitch +from edx_toggles.toggles import WaffleSwitchNamespace as BaseWaffleSwitchNamespace log = logging.getLogger(__name__) -class WaffleNamespace(six.with_metaclass(ABCMeta, object)): +class WaffleSwitchNamespace(BaseWaffleSwitchNamespace): """ - A base class for a request cached namespace for waffle flags/switches. + Waffle switch namespace that implements custom overriding methods. We should eventually get rid of this class. - An instance of this class represents a single namespace - (e.g. "course_experience"), and can be used to work with a set of - flags or switches that will all share this namespace. + To test WaffleSwitchNamespace, use the provided context managers. For example: + + with WAFFLE_SWITCHES.override(waffle.ESTIMATE_FIRST_ATTEMPTED, active=True): + ... + + Note: this should eventually be deprecated in favour of a dedicated `override_waffle_switch` context manager. """ - def __init__(self, name, log_prefix=None): - """ - Initializes the waffle namespace instance. - - Arguments: - name (String): Namespace string appended to start of all waffle - flags and switches (e.g. "grades") - log_prefix (String): Optional string to be appended to log messages - (e.g. "Grades: "). Defaults to ''. - - """ - assert name, "The name is required." - self.name = name - self.log_prefix = log_prefix if log_prefix else '' - - def _namespaced_name(self, setting_name): - """ - Returns the namespaced name of the waffle switch/flag. - - For example, the namespaced name of a waffle switch/flag would be: - my_namespace.my_setting_name - - Arguments: - setting_name (String): The name of the flag or switch. - """ - return u'{}.{}'.format(self.name, setting_name) - - @staticmethod - def _get_request_cache(): - """ - Returns a request cache shared by all instances of this class. - """ - return get_request_cache('WaffleNamespace') - - -class WaffleSwitchNamespace(WaffleNamespace): - """ - Provides a single namespace for a set of waffle switches. - - All namespaced switch values are stored in a single request cache containing - all switches for all namespaces. - """ - def is_enabled(self, switch_name): - """ - Returns and caches whether the given waffle switch is enabled. - """ - namespaced_switch_name = self._namespaced_name(switch_name) - value = self._cached_switches.get(namespaced_switch_name) - if value is None: - value = switch_is_active(namespaced_switch_name) - self._cached_switches[namespaced_switch_name] = value - return value - @contextmanager def override(self, switch_name, active=True): """ @@ -157,7 +50,12 @@ class WaffleSwitchNamespace(WaffleNamespace): """ namespaced_switch_name = self._namespaced_name(switch_name) self._cached_switches[namespaced_switch_name] = active - log.info(u"%sSwitch '%s' set to %s for request.", self.log_prefix, namespaced_switch_name, active) + log.info( + "%sSwitch '%s' set to %s for request.", + self.log_prefix, + namespaced_switch_name, + active, + ) @contextmanager def override_in_model(self, switch_name, active=True): @@ -165,70 +63,30 @@ class WaffleSwitchNamespace(WaffleNamespace): Overrides the active value for the given switch for the duration of this contextmanager. Note: The value is overridden in the model, not the request cache. + Note: This should probably be moved to a test class. """ # Import is placed here to avoid model import at project startup. + # pylint: disable=import-outside-toplevel from waffle.testutils import override_switch as waffle_override_switch + namespaced_switch_name = self._namespaced_name(switch_name) with waffle_override_switch(namespaced_switch_name, active): - log.info(u"%sSwitch '%s' set to %s in model.", self.log_prefix, namespaced_switch_name, active) + log.info( + "%sSwitch '%s' set to %s in model.", + self.log_prefix, + namespaced_switch_name, + active, + ) yield - @property - def _cached_switches(self): - """ - Returns a dictionary of all namespaced switches in the request cache. - """ - return self._get_request_cache().setdefault('switches', {}) - -class WaffleSwitch(object): +class WaffleSwitch(BaseWaffleSwitch): """ - Represents a single waffle switch, using a cached namespace. + This class should be removed in favour of edx_toggles.toggles.WaffleSwitch once we get rid of the + WaffleSwitchNamespace class. """ - # use a WeakSet so these instances can be garbage collected if need be - _class_instances = WeakSet() - def __init__(self, waffle_namespace, switch_name, module_name=None): - """ - Arguments: - waffle_namespace (WaffleSwitchNamespace | String): Namespace for this switch. - switch_name (String): The name of the switch (without namespacing). - module_name (String): The name of the module where the flag is created. This should be ``__name__`` in most - cases. - """ - if isinstance(waffle_namespace, six.string_types): - waffle_namespace = WaffleSwitchNamespace(name=waffle_namespace) - - self.waffle_namespace = waffle_namespace - self.switch_name = switch_name - self._module_name = module_name or "" - self._class_instances.add(self) - - @classmethod - def get_instances(cls): - """ Returns a WeakSet of the instantiated instances of WaffleFlag. """ - return cls._class_instances - - @property - def module_name(self): - """ - Returns the module name. This is cached to work with the WeakSet instances. - """ - return self._module_name - - @module_name.setter - def module_name(self, value): - self._module_name = value - - @property - def namespaced_switch_name(self): - """ - Returns the fully namespaced switch name. - """ - return self.waffle_namespace._namespaced_name(self.switch_name) # pylint: disable=protected-access - - def is_enabled(self): - return self.waffle_namespace.is_enabled(self.switch_name) + NAMESPACE_CLASS = WaffleSwitchNamespace @contextmanager def override(self, active=True): @@ -236,268 +94,76 @@ class WaffleSwitch(object): yield -class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)): +class WaffleFlag(BaseWaffleFlag): """ - Provides a single namespace for a set of waffle flags. + Waffle flag class that implements custom override method. - All namespaced flag values are stored in a single request cache containing - all flags for all namespaces. + This class should be removed in favour of edx_toggles.toggles.WaffleFlag once we get rid of the WaffleFlagNamespace + class and the `override` method. """ - @property - def _cached_flags(self): - """ - Returns a dictionary of all namespaced flags in the request cache. - """ - return self._get_request_cache().setdefault('flags', {}) - - def is_flag_active(self, flag_name, check_before_waffle_callback=None): - """ - Returns and caches whether the provided flag is active. - - If the flag value is already cached in the request, it is returned. - If check_before_waffle_callback is supplied, it is called before - checking waffle. - If check_before_waffle_callback returns None, or if it is not supplied, - then waffle is used to check the flag. - - Important: Caching for the check_before_waffle_callback must be handled - by the callback itself. - - Note: A waffle flag's default is False if not defined. If you think you - need the default to be True, see the module docstring for - alternatives. - - Arguments: - flag_name (String): The name of the flag to check. - check_before_waffle_callback (function): (Optional) A function that - will be checked before continuing on to waffle. If - check_before_waffle_callback(namespaced_flag_name) returns True - or False, it is returned. If it returns None, then waffle is - used. - - """ - # validate arguments - namespaced_flag_name = self._namespaced_name(flag_name) - - if check_before_waffle_callback: - value = check_before_waffle_callback(namespaced_flag_name) - if value is not None: - # Do not cache value for the callback, because the key might be different. - # The callback needs to handle its own caching if it wants it. - self._set_waffle_flag_attribute(namespaced_flag_name, value) - return value - - value = self._cached_flags.get(namespaced_flag_name) - if value is not None: - self._set_waffle_flag_attribute(namespaced_flag_name, value) - return value - - request = crum.get_current_request() - if not request: - log.warning(u"%sFlag '%s' accessed without a request", self.log_prefix, namespaced_flag_name) - # Return the Flag's Everyone value if not in a request context. - # Note: this skips the cache as the value might be different - # in a normal request context. This case seems to occur when - # a page redirects to a 404, or for celery workers. - value = self._is_flag_active_for_everyone(namespaced_flag_name) - self._set_waffle_flag_attribute(namespaced_flag_name, value) - set_custom_attribute('warn_flag_no_request_return_value', value) - return value - - value = flag_is_active(request, namespaced_flag_name) - self._cached_flags[namespaced_flag_name] = value - - self._set_waffle_flag_attribute(namespaced_flag_name, value) - return value - - def _is_flag_active_for_everyone(self, namespaced_flag_name): - """ - Returns True if the waffle flag is configured as active for Everyone, - False otherwise. - """ - # Import is placed here to avoid model import at project startup. - from waffle.models import Flag - - try: - waffle_flag = Flag.objects.get(name=namespaced_flag_name) - return (waffle_flag.everyone is True) - except Flag.DoesNotExist: - return False - - def _set_waffle_flag_attribute(self, name, value): - """ - For any flag name in _WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET, add name/value - to cached values and set custom attribute if the value changed. - - The name of the custom attribute will have the prefix ``flag_`` and the - suffix will match the name of the flag. - The value of the custom attribute could be False, True, or Both. - - The value Both would mean that the flag had both a True and False - value at different times during the transaction. This is most - likely due to having a check_before_waffle_callback, as is the - case with CourseWaffleFlag. - - An example NewRelic query to see the values of a flag in different - environments, if your waffle flag was named ``my.waffle.flag`` might - look like:: - - SELECT count(*) FROM Transaction - WHERE flag_my.waffle.flag IS NOT NULL - FACET appName, flag_my.waffle.flag - - Important: Remember to configure ``WAFFLE_FLAG_CUSTOM_ATTRIBUTES`` for - LMS, Studio and Workers in order to see waffle flag usage in all - edx-platform environments. - - """ - if name not in _WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET: - return - - flag_attribute_data = self._get_request_cache().setdefault('flag_attribute', {}) - is_value_change = False - if name not in flag_attribute_data: - flag_attribute_data[name] = str(value) - is_value_change = True - else: - if flag_attribute_data[name] != str(value): - flag_attribute_data[name] = 'Both' - is_value_change = True - - if is_value_change: - attribute_name = 'flag_{}'.format(name) - set_custom_attribute(attribute_name, flag_attribute_data[name]) - - -def _get_waffle_flag_custom_attributes_set(): - """ - Returns a set based on the Django setting WAFFLE_FLAG_CUSTOM_ATTRIBUTES (list). - """ - waffle_flag_custom_attributes = getattr(settings, _WAFFLE_FLAG_CUSTOM_ATTRIBUTES, None) - waffle_flag_custom_attributes = waffle_flag_custom_attributes if waffle_flag_custom_attributes else [] - return set(waffle_flag_custom_attributes) - -# .. toggle_name: WAFFLE_FLAG_CUSTOM_ATTRIBUTES -# .. toggle_implementation: DjangoSetting -# .. toggle_default: False -# .. toggle_description: A list of waffle flags to track with custom attributes having -# values of (True, False, or Both). -# .. toggle_use_cases: opt_in -# .. toggle_creation_date: 2020-06-17 -# .. toggle_warnings: Intent is for temporary research (1 day - several weeks) of a flag's usage. -_WAFFLE_FLAG_CUSTOM_ATTRIBUTES = 'WAFFLE_FLAG_CUSTOM_ATTRIBUTES' - -# set of waffle flags that should be instrumented with custom attributes -_WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET = _get_waffle_flag_custom_attributes_set() - - -class WaffleFlag(object): - """ - Represents a single waffle flag, using a cached waffle namespace. - """ - # use a WeakSet so these instances can be garbage collected if need be - _class_instances = WeakSet() - - def __init__(self, waffle_namespace, flag_name, module_name=None): - """ - Initializes the waffle flag instance. - - Arguments: - waffle_namespace (WaffleFlagNamespace | String): Namespace for this flag. - flag_name (String): The name of the flag (without namespacing). - module_name (String): The name of the module where the flag is created. This should be ``__name__`` in most - cases. - """ - if isinstance(waffle_namespace, six.string_types): - waffle_namespace = WaffleFlagNamespace(name=waffle_namespace) - - self.waffle_namespace = waffle_namespace - self.waffle_namespace = waffle_namespace - self.flag_name = flag_name - self._module_name = module_name or "" - self._class_instances.add(self) - - @classmethod - def get_instances(cls): - """ Returns a WeakSet of the instantiated instances of WaffleFlag. """ - return cls._class_instances - - @property - def module_name(self): - """ - Returns the module name. This is cached to work with the WeakSet instances. - """ - return self._module_name - - @module_name.setter - def module_name(self, value): - self._module_name = value - - @property - def namespaced_flag_name(self): - """ - Returns the fully namespaced flag name. - """ - return self.waffle_namespace._namespaced_name(self.flag_name) - - def is_enabled(self): - """ - Returns whether or not the flag is enabled. - """ - return self.waffle_namespace.is_flag_active(self.flag_name) - @contextmanager def override(self, active=True): + """ + Shortcut method for `override_waffle_flag`. + """ # TODO We can move this import to the top of the file once this code is # not all contained within the __init__ module. from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag + with override_waffle_flag(self, active): yield class CourseWaffleFlag(WaffleFlag): """ - Represents a single waffle flag that can be forced on/off for a course. + Represents a single waffle flag that can be forced on/off for a course. This class should be used instead of + WaffleFlag when in the context of a course. Uses a cached waffle namespace. + + Usage: + + WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='my_namespace') + SOME_COURSE_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'some_course_feature', __name__) + + And then we can check this flag in code with:: + + SOME_COURSE_FLAG.is_enabled(course_key) + + The Django Admin "waffle_utils" section can be used to configure a course override for this same flag (e.g. + my_namespace.some_course_feature). """ - def _get_course_override_callback(self, course_key): + def _get_course_override_value(self, course_key): """ - Returns a function to use as the check_before_waffle_callback. + Returns True/False if the flag was forced on or off for the provided course. Returns None if the flag was not + overridden. + + Note: Has side effect of caching the override value. Arguments: - course_key (CourseKey): The course to check for override before - checking waffle. + course_key (CourseKey): The course to check for override before checking waffle. """ - def course_override_callback(namespaced_flag_name): - """ - Returns True/False if the flag was forced on or off for the provided - course. Returns None if the flag was not overridden. + # Import is placed here to avoid model import at project startup. + from .models import WaffleFlagCourseOverrideModel - Note: Has side effect of caching the override value. + cache_key = "{}.{}".format(self.namespaced_flag_name, str(course_key)) + # pylint: disable=protected-access + course_override = self.waffle_namespace._cached_flags.get(cache_key) - Arguments: - namespaced_flag_name (String): A namespaced version of the flag - to check. - """ - # Import is placed here to avoid model import at project startup. - from .models import WaffleFlagCourseOverrideModel - cache_key = u'{}.{}'.format(namespaced_flag_name, six.text_type(course_key)) - force_override = self.waffle_namespace._cached_flags.get(cache_key) + if course_override is None: + course_override = WaffleFlagCourseOverrideModel.override_value( + self.namespaced_flag_name, course_key + ) + # pylint: disable=protected-access + self.waffle_namespace._cached_flags[cache_key] = course_override - if force_override is None: - force_override = WaffleFlagCourseOverrideModel.override_value(namespaced_flag_name, course_key) - self.waffle_namespace._cached_flags[cache_key] = force_override - - if force_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.on: - return True - if force_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.off: - return False - return None - - return course_override_callback + if course_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.on: + return True + if course_override == WaffleFlagCourseOverrideModel.ALL_CHOICES.off: + return False + return None def is_enabled(self, course_key=None): # pylint: disable=arguments-differ """ @@ -509,10 +175,16 @@ class CourseWaffleFlag(WaffleFlag): outside the context of any course. """ if course_key: - assert isinstance(course_key, CourseKey), ( - "Provided course_key '{}' is not instance of CourseKey.".format(course_key) + assert isinstance( + course_key, CourseKey + ), "Provided course_key '{}' is not instance of CourseKey.".format( + course_key ) - return self.waffle_namespace.is_flag_active( - self.flag_name, - check_before_waffle_callback=self._get_course_override_callback(course_key), - ) + is_enabled_for_course = self._get_course_override_value(course_key) + if is_enabled_for_course is not None: + # pylint: disable=protected-access + self.NAMESPACE_CLASS._monitor_value( + self.namespaced_flag_name, is_enabled_for_course + ) + return is_enabled_for_course + return super().is_enabled() diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_init.py b/openedx/core/djangoapps/waffle_utils/tests/test_init.py index 3135e0d864..e39359dc4f 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_init.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py @@ -7,17 +7,16 @@ import ddt from django.test import TestCase from django.test.client import RequestFactory from django.test.utils import override_settings +# Note that we really shouldn't import from edx_toggles' internal API +import edx_toggles.toggles.internal.waffle from edx_django_utils.cache import RequestCache from mock import call, patch from opaque_keys.edx.keys import CourseKey from waffle.testutils import override_flag from .. import ( - _get_waffle_flag_custom_attributes_set, CourseWaffleFlag, WaffleFlagNamespace, - WaffleSwitchNamespace, - WaffleSwitch, ) from ..models import WaffleFlagCourseOverrideModel @@ -47,7 +46,7 @@ class TestCourseWaffleFlag(TestCase): RequestCache.clear_all_namespaces() @override_settings(WAFFLE_FLAG_CUSTOM_ATTRIBUTES=[NAMESPACED_FLAG_NAME]) - @patch('openedx.core.djangoapps.waffle_utils.set_custom_attribute') + @patch.object(edx_toggles.toggles.internal.waffle, 'set_custom_attribute') @ddt.data( {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.on, 'waffle_enabled': False, 'result': True}, {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.off, 'waffle_enabled': True, 'result': False}, @@ -59,41 +58,38 @@ class TestCourseWaffleFlag(TestCase): Tests various combinations of a flag being set in waffle and overridden for a course. """ - with patch( - 'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET', - _get_waffle_flag_custom_attributes_set(), - ): - with patch.object(WaffleFlagCourseOverrideModel, 'override_value', return_value=data['course_override']): - with override_flag(self.NAMESPACED_FLAG_NAME, active=data['waffle_enabled']): - # check twice to test that the result is properly cached - self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result']) - self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result']) - # result is cached, so override check should happen once - WaffleFlagCourseOverrideModel.override_value.assert_called_once_with( - self.NAMESPACED_FLAG_NAME, - self.TEST_COURSE_KEY - ) + with patch.object(WaffleFlagCourseOverrideModel, 'override_value', return_value=data['course_override']): + with override_flag(self.NAMESPACED_FLAG_NAME, active=data['waffle_enabled']): + # check twice to test that the result is properly cached + self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result']) + self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result']) + # result is cached, so override check should happen once + # pylint: disable=no-member + WaffleFlagCourseOverrideModel.override_value.assert_called_once_with( + self.NAMESPACED_FLAG_NAME, + self.TEST_COURSE_KEY + ) - self._assert_waffle_flag_attribute(mock_set_custom_attribute, expected_flag_value=str(data['result'])) - mock_set_custom_attribute.reset_mock() + self._assert_waffle_flag_attribute(mock_set_custom_attribute, expected_flag_value=str(data['result'])) + mock_set_custom_attribute.reset_mock() - # check flag for a second course - if data['course_override'] == WaffleFlagCourseOverrideModel.ALL_CHOICES.unset: - # When course override wasn't set for the first course, the second course will get the same - # cached value from waffle. - second_value = data['waffle_enabled'] - self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), second_value) - else: - # When course override was set for the first course, it should not apply to the second - # course which should get the default value of False. - second_value = False - self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), second_value) + # check flag for a second course + if data['course_override'] == WaffleFlagCourseOverrideModel.ALL_CHOICES.unset: + # When course override wasn't set for the first course, the second course will get the same + # cached value from waffle. + second_value = data['waffle_enabled'] + self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), second_value) + else: + # When course override was set for the first course, it should not apply to the second + # course which should get the default value of False. + second_value = False + self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), second_value) expected_flag_value = None if second_value == data['result'] else 'Both' self._assert_waffle_flag_attribute(mock_set_custom_attribute, expected_flag_value=expected_flag_value) @override_settings(WAFFLE_FLAG_CUSTOM_ATTRIBUTES=[NAMESPACED_FLAG_NAME]) - @patch('openedx.core.djangoapps.waffle_utils.set_custom_attribute') + @patch.object(edx_toggles.toggles.internal.waffle, 'set_custom_attribute') def test_undefined_waffle_flag(self, mock_set_custom_attribute): """ Test flag with undefined waffle flag. @@ -104,23 +100,20 @@ class TestCourseWaffleFlag(TestCase): __name__, ) - with patch( - 'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET', - _get_waffle_flag_custom_attributes_set(), + with patch.object( + WaffleFlagCourseOverrideModel, + 'override_value', + return_value=WaffleFlagCourseOverrideModel.ALL_CHOICES.unset ): - with patch.object( - WaffleFlagCourseOverrideModel, - 'override_value', - return_value=WaffleFlagCourseOverrideModel.ALL_CHOICES.unset - ): - # check twice to test that the result is properly cached - self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), False) - self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), False) - # result is cached, so override check should happen once - WaffleFlagCourseOverrideModel.override_value.assert_called_once_with( - self.NAMESPACED_FLAG_NAME, - self.TEST_COURSE_KEY - ) + # check twice to test that the result is properly cached + self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), False) + self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), False) + # result is cached, so override check should happen once + # pylint: disable=no-member + WaffleFlagCourseOverrideModel.override_value.assert_called_once_with( + self.NAMESPACED_FLAG_NAME, + self.TEST_COURSE_KEY + ) self._assert_waffle_flag_attribute( mock_set_custom_attribute, @@ -157,20 +150,16 @@ class TestCourseWaffleFlag(TestCase): {'expected_count': 1, 'waffle_flag_attribute_setting': [NAMESPACED_FLAG_NAME]}, {'expected_count': 2, 'waffle_flag_attribute_setting': [NAMESPACED_FLAG_NAME, NAMESPACED_FLAG_2_NAME]}, ) - @patch('openedx.core.djangoapps.waffle_utils.set_custom_attribute') + @patch.object(edx_toggles.toggles.internal.waffle, 'set_custom_attribute') def test_waffle_flag_attribute_for_various_settings(self, data, mock_set_custom_attribute): """ Test that custom attributes are recorded when waffle flag accessed. """ with override_settings(WAFFLE_FLAG_CUSTOM_ATTRIBUTES=data['waffle_flag_attribute_setting']): - with patch( - 'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET', - _get_waffle_flag_custom_attributes_set(), - ): - test_course_flag = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_NAME, __name__) - test_course_flag.is_enabled(self.TEST_COURSE_KEY) - test_course_flag_2 = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_2_NAME, __name__) - test_course_flag_2.is_enabled(self.TEST_COURSE_KEY) + test_course_flag = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_NAME, __name__) + test_course_flag.is_enabled(self.TEST_COURSE_KEY) + test_course_flag_2 = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_2_NAME, __name__) + test_course_flag_2.is_enabled(self.TEST_COURSE_KEY) self.assertEqual(mock_set_custom_attribute.call_count, data['expected_count']) @@ -185,22 +174,3 @@ class TestCourseWaffleFlag(TestCase): self.assertEqual(mock_set_custom_attribute.call_count, 1) else: self.assertEqual(mock_set_custom_attribute.call_count, 0) - - -class TestWaffleSwitch(TestCase): - """ - Tests the WaffleSwitch. - """ - - NAMESPACE_NAME = "test_namespace" - WAFFLE_SWITCH_NAME = "test_switch_name" - TEST_NAMESPACE = WaffleSwitchNamespace(NAMESPACE_NAME) - WAFFLE_SWITCH = WaffleSwitch(TEST_NAMESPACE, WAFFLE_SWITCH_NAME, __name__) - - def test_namespaced_switch_name(self): - """ - Verify namespaced_switch_name returns the correct namespace switch name - """ - expected = self.NAMESPACE_NAME + "." + self.WAFFLE_SWITCH_NAME - actual = self.WAFFLE_SWITCH.namespaced_switch_name - self.assertEqual(actual, expected) diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_testutils.py b/openedx/core/djangoapps/waffle_utils/tests/test_testutils.py deleted file mode 100644 index a6bd73b4f6..0000000000 --- a/openedx/core/djangoapps/waffle_utils/tests/test_testutils.py +++ /dev/null @@ -1,68 +0,0 @@ -""" -Tests for waffle utils test utilities. -""" - - -import crum -from django.test import TestCase -from django.test.client import RequestFactory -from edx_django_utils.cache import RequestCache -from opaque_keys.edx.keys import CourseKey - -from .. import CourseWaffleFlag, WaffleFlagNamespace -from ..testutils import override_waffle_flag - - -class OverrideWaffleFlagTests(TestCase): - """ - Tests for the override_waffle_flag decorator/context manager. - """ - - NAMESPACE_NAME = "test_namespace" - FLAG_NAME = "test_flag" - NAMESPACED_FLAG_NAME = NAMESPACE_NAME + "." + FLAG_NAME - - TEST_COURSE_KEY = CourseKey.from_string("edX/DemoX/Demo_Course") - TEST_NAMESPACE = WaffleFlagNamespace(NAMESPACE_NAME) - TEST_COURSE_FLAG = CourseWaffleFlag(TEST_NAMESPACE, FLAG_NAME, __name__) - - def setUp(self): - super(OverrideWaffleFlagTests, self).setUp() - request = RequestFactory().request() - self.addCleanup(crum.set_current_request, None) - crum.set_current_request(request) - RequestCache.clear_all_namespaces() - - @override_waffle_flag(TEST_COURSE_FLAG, True) - def assert_decorator_activates_flag(self): - assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) - - def test_override_waffle_flag_pre_cached(self): - # checks and caches the is_enabled value - assert not self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) - flag_cache = self.TEST_COURSE_FLAG.waffle_namespace._cached_flags - assert self.NAMESPACED_FLAG_NAME in flag_cache - - self.assert_decorator_activates_flag() - - # test cached flag is restored - assert self.NAMESPACED_FLAG_NAME in flag_cache - assert not self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) - - def test_override_waffle_flag_not_pre_cached(self): - # check that the flag is not yet cached - flag_cache = self.TEST_COURSE_FLAG.waffle_namespace._cached_flags - assert self.NAMESPACED_FLAG_NAME not in flag_cache - - self.assert_decorator_activates_flag() - - # test cache is removed when no longer using decorator/context manager - assert self.NAMESPACED_FLAG_NAME not in flag_cache - - def test_override_waffle_flag_as_context_manager(self): - assert not self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) - - with override_waffle_flag(self.TEST_COURSE_FLAG, True): - assert self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) - - assert not self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY) diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_views.py b/openedx/core/djangoapps/waffle_utils/tests/test_views.py index f6396a95d2..de47e17d02 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_views.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_views.py @@ -2,14 +2,16 @@ Tests for waffle utils views. """ from django.test import TestCase +from edx_django_utils.monitoring.code_owner import utils as code_owner_utils +from mock import patch from rest_framework.test import APIRequestFactory from waffle.testutils import override_switch from student.tests.factories import UserFactory from .. import WaffleFlag, WaffleFlagNamespace, WaffleSwitch, WaffleSwitchNamespace -from ..views import ToggleStateView from ..testutils import override_waffle_flag +from ..views import ToggleStateView TEST_WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace('test') TEST_WAFFLE_FLAG = WaffleFlag(TEST_WAFFLE_FLAG_NAMESPACE, 'flag', __name__) @@ -45,6 +47,17 @@ class ToggleStateViewTests(TestCase): # This is no longer the first switch #self.assertEqual(response.data['waffle_switches'][0]['name'], 'test.switch') + def test_code_owners_without_module_information(self): + # Create a waffle flag without any associated module_name + waffle_flag = WaffleFlag(TEST_WAFFLE_FLAG_NAMESPACE, "flag2", module_name=None) + with patch.object(code_owner_utils, "get_code_owner_mappings", return_value={}): + response = self._get_toggle_state_response(is_staff=True) + + result = [ + flag for flag in response.data["waffle_flags"] if flag["name"] == waffle_flag.name + ][0] + self.assertNotIn("code_owner", result) + def _get_toggle_state_response(self, is_staff=True): request = APIRequestFactory().get('/api/toggles/state/') user = UserFactory() diff --git a/openedx/core/djangoapps/waffle_utils/testutils.py b/openedx/core/djangoapps/waffle_utils/testutils.py index af7b6a614b..03c44ce3a5 100644 --- a/openedx/core/djangoapps/waffle_utils/testutils.py +++ b/openedx/core/djangoapps/waffle_utils/testutils.py @@ -2,66 +2,18 @@ Test utilities for waffle utilities. """ - -from waffle.testutils import override_flag +# Import from edx-toggles to preserve import paths +# TODO: Deprecate and remove +# pylint: disable=unused-import +from edx_toggles.toggles.testutils import override_waffle_flag # Can be used with FilteredQueryCountMixin.assertNumQueries() to blacklist # waffle tables. For example: # QUERY_COUNT_TABLE_BLACKLIST = WAFFLE_TABLES # with self.assertNumQueries(6, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): -WAFFLE_TABLES = ['waffle_utils_waffleflagcourseoverridemodel', 'waffle_flag', 'waffle_switch', 'waffle_sample'] - - -class override_waffle_flag(override_flag): - """ - override_waffle_flag is a contextmanager for easier testing of flags. - - It accepts two parameters, the flag itself and its intended state. Example - usage:: - - with override_waffle_flag(SOME_COURSE_FLAG, active=True): - ... - - If the flag already exists, its value will be changed inside the context - block, then restored to the original value. If the flag does not exist - before entering the context, it is created, then removed at the end of the - block. - - It can also act as a decorator:: - - @override_waffle_flag(SOME_COURSE_FLAG, active=True) - def test_happy_mode_enabled(): - ... - """ - _cached_value = None - - def __init__(self, flag, active): - """ - - Args: - flag (WaffleFlag): The namespaced cached waffle flag. - active (Boolean): The value to which the flag will be set. - """ - self.flag = flag - waffle_namespace = flag.waffle_namespace - name = waffle_namespace._namespaced_name(flag.flag_name) # pylint: disable=protected-access - super(override_waffle_flag, self).__init__(name, active) - - def __enter__(self): - super(override_waffle_flag, self).__enter__() - - # pylint: disable=protected-access - # Store values that have been cached on the flag - self._cached_value = self.flag.waffle_namespace._cached_flags.get(self.name) - self.flag.waffle_namespace._cached_flags[self.name] = self.active - - def __exit__(self, exc_type, exc_val, exc_tb): - super(override_waffle_flag, self).__exit__(exc_type, exc_val, exc_tb) - - # pylint: disable=protected-access - # Restore the cached values - waffle_namespace = self.flag.waffle_namespace - waffle_namespace._cached_flags.pop(self.name, None) - - if self._cached_value is not None: - waffle_namespace._cached_flags[self.name] = self._cached_value +WAFFLE_TABLES = [ + "waffle_utils_waffleflagcourseoverridemodel", + "waffle_flag", + "waffle_switch", + "waffle_sample", +] diff --git a/openedx/core/djangoapps/waffle_utils/views.py b/openedx/core/djangoapps/waffle_utils/views.py index 64ea31fcb4..2ce629a62e 100644 --- a/openedx/core/djangoapps/waffle_utils/views.py +++ b/openedx/core/djangoapps/waffle_utils/views.py @@ -2,13 +2,13 @@ Views that we will use to view toggle state in edx-platform. """ from collections import OrderedDict -from django.conf import settings -from edx_django_utils.monitoring.code_owner.utils import get_code_owner_from_module, is_code_owner_mappings_configured +from django.conf import settings +from edx_django_utils.monitoring import get_code_owner_from_module from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.permissions import IsStaff -from rest_framework.authentication import SessionAuthentication from rest_framework import permissions, views +from rest_framework.authentication import SessionAuthentication from rest_framework.response import Response from waffle.models import Flag, Switch @@ -107,7 +107,7 @@ class ToggleStateView(views.APIView): """ toggle['class'] = toggle_instance.__class__.__name__ toggle['module'] = toggle_instance.module_name - if is_code_owner_mappings_configured(): + if toggle_instance.module_name: code_owner = get_code_owner_from_module(toggle_instance.module_name) if code_owner: toggle['code_owner'] = code_owner diff --git a/openedx/features/course_experience/__init__.py b/openedx/features/course_experience/__init__.py index 5ae92ba88c..40be566219 100644 --- a/openedx/features/course_experience/__init__.py +++ b/openedx/features/course_experience/__init__.py @@ -8,7 +8,8 @@ from waffle import flag_is_active from lms.djangoapps.experiments.flags import ExperimentWaffleFlag from openedx.core.djangoapps.util.user_messages import UserMessageCollection -from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlag, WaffleFlagNamespace +from openedx.core.djangoapps.waffle_utils import WaffleFlag, WaffleFlagNamespace +from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag # Namespace for course experience waffle flags. WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='course_experience') diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 98e41abf49..2390752fb1 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -40,6 +40,9 @@ edx-enterprise==3.9.11 # v2 requires the ES7 upgrade work to be complete edx-search<2.0.0 +# We expect v2.0.0 to introduce large breaking changes in the feature toggle API +edx-toggles<2.0.0 + # Upgrading to 2.12.0 breaks several test classes due to API changes, need to update our code accordingly factory-boy==2.8.1