Refactor, enhance, and adjust unified_course_view flag.

This includes several general enhancement in addition
to the fixes for unified_course_view:
1. Add support for default when no waffle flag defined.
2. Add support for table_blacklist to assertNumQueries.
3. Rename flag to 'course_experience.course_outline_page'.
4. Change flag default to True when it is not defined.
This commit is contained in:
Robert Raposa
2017-06-26 17:30:00 -04:00
parent d311a83474
commit 2e80c1e66b
26 changed files with 364 additions and 178 deletions

View File

@@ -10,8 +10,10 @@ namespace. For example:
WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='course_experience')
HIDE_SEARCH_FLAG = WaffleFlag(WAFFLE_FLAG_NAMESPACE, 'hide_search')
# Use CourseWaffleFlag when you are in the context of a course.
UNIFIED_COURSE_TAB_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'unified_course_tab')
# Use WaffleFlag when outside the context of a course.
HIDE_SEARCH_FLAG = WaffleFlag(WAFFLE_FLAG_NAMESPACE, 'hide_search')
You can check these flags in code using the following:
@@ -43,14 +45,14 @@ To test WaffleSwitchNamespace, use the provided context managers. For example:
...
"""
import logging
from abc import ABCMeta
from contextlib import contextmanager
import logging
from waffle.testutils import override_switch as waffle_override_switch
from waffle import flag_is_active, switch_is_active
from opaque_keys.edx.keys import CourseKey
from request_cache import get_request, get_cache as get_request_cache
from request_cache import get_cache as get_request_cache, get_request
from waffle import flag_is_active, switch_is_active
from waffle.models import Flag
from waffle.testutils import override_switch as waffle_override_switch
from .models import WaffleFlagCourseOverrideModel
@@ -64,7 +66,6 @@ class WaffleNamespace(object):
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.
"""
__metaclass__ = ABCMeta
@@ -92,7 +93,6 @@ class WaffleNamespace(object):
Arguments:
setting_name (String): The name of the flag or switch.
"""
return u'{}.{}'.format(self.name, setting_name)
@@ -110,7 +110,6 @@ class WaffleSwitchNamespace(WaffleNamespace):
All namespaced switch values are stored in a single request cache containing
all switches for all namespaces.
"""
def is_enabled(self, switch_name):
"""
@@ -174,7 +173,6 @@ class WaffleFlagNamespace(WaffleNamespace):
All namespaced flag values are stored in a single request cache containing
all flags for all namespaces.
"""
__metaclass__ = ABCMeta
@@ -185,7 +183,7 @@ class WaffleFlagNamespace(WaffleNamespace):
"""
return self._get_request_cache().setdefault('flags', {})
def is_flag_active(self, flag_name, check_before_waffle_callback=None):
def is_flag_active(self, flag_name, check_before_waffle_callback=None, flag_undefined_default=None):
"""
Returns and caches whether the provided flag is active.
@@ -202,7 +200,8 @@ class WaffleFlagNamespace(WaffleNamespace):
check_before_waffle_callback(namespaced_flag_name) returns True
or False, it is cached and returned. If it returns None, then
waffle is used.
flag_undefined_default (Boolean): A default value to be returned if
the waffle flag is to be checked, but doesn't exist.
"""
# validate arguments
namespaced_flag_name = self._namespaced_name(flag_name)
@@ -213,7 +212,16 @@ class WaffleFlagNamespace(WaffleNamespace):
value = check_before_waffle_callback(namespaced_flag_name)
if value is None:
value = flag_is_active(get_request(), namespaced_flag_name)
if flag_undefined_default is not None:
# determine if the flag is undefined in waffle
try:
Flag.objects.get(name=namespaced_flag_name)
except Flag.DoesNotExist:
value = flag_undefined_default
if value is None:
value = flag_is_active(get_request(), namespaced_flag_name)
self._cached_flags[namespaced_flag_name] = value
return value
@@ -224,7 +232,7 @@ class WaffleFlag(object):
Represents a single waffle flag, using a cached waffle namespace.
"""
def __init__(self, waffle_namespace, flag_name):
def __init__(self, waffle_namespace, flag_name, flag_undefined_default=None):
"""
Initializes the waffle flag instance.
@@ -232,16 +240,21 @@ class WaffleFlag(object):
waffle_namespace (WaffleFlagNamespace): Provides a cached namespace
for this flag.
flag_name (String): The name of the flag (without namespacing).
flag_undefined_default (Boolean): A default value to be returned if
the waffle flag is to be checked, but doesn't exist.
"""
self.waffle_namespace = waffle_namespace
self.flag_name = flag_name
self.flag_undefined_default = flag_undefined_default
def is_enabled(self):
"""
Returns whether or not the flag is enabled.
"""
return self.waffle_namespace.is_flag_active(self.flag_name)
return self.waffle_namespace.is_flag_active(
self.flag_name,
flag_undefined_default=self.flag_undefined_default
)
class CourseWaffleFlag(WaffleFlag):
@@ -249,7 +262,6 @@ class CourseWaffleFlag(WaffleFlag):
Represents a single waffle flag that can be forced on/off for a course.
Uses a cached waffle namespace.
"""
def _get_course_override_callback(self, course_id):
@@ -259,7 +271,6 @@ class CourseWaffleFlag(WaffleFlag):
Arguments:
course_id (CourseKey): The course to check for override before
checking waffle.
"""
def course_override_callback(namespaced_flag_name):
"""
@@ -269,7 +280,6 @@ class CourseWaffleFlag(WaffleFlag):
Arguments:
namespaced_flag_name (String): A namespaced version of the flag
to check.
"""
force_override = WaffleFlagCourseOverrideModel.override_value(namespaced_flag_name, course_id)
@@ -287,12 +297,12 @@ class CourseWaffleFlag(WaffleFlag):
Arguments:
course_id (CourseKey): The course to check for override before
checking waffle.
"""
# validate arguments
assert issubclass(type(course_id), CourseKey), "The course_id '{}' must be a CourseKey.".format(str(course_id))
return self.waffle_namespace.is_flag_active(
self.flag_name,
check_before_waffle_callback=self._get_course_override_callback(course_id)
check_before_waffle_callback=self._get_course_override_callback(course_id),
flag_undefined_default=self.flag_undefined_default
)

View File

@@ -5,9 +5,8 @@ import ddt
from django.test import TestCase
from mock import patch
from opaque_keys.edx.keys import CourseKey
from waffle.testutils import override_flag
from request_cache.middleware import RequestCache
from waffle.testutils import override_flag
from .. import CourseWaffleFlag, WaffleFlagNamespace
from ..models import WaffleFlagCourseOverrideModel
@@ -50,3 +49,34 @@ class TestCourseWaffleFlag(TestCase):
self.NAMESPACED_FLAG_NAME,
self.TEST_COURSE_KEY
)
@ddt.data(
{'flag_undefined_default': None, 'result': False},
{'flag_undefined_default': False, 'result': False},
{'flag_undefined_default': True, 'result': True},
)
def test_undefined_waffle_flag(self, data):
"""
Test flag with various defaults provided for undefined waffle flags.
"""
RequestCache.clear_request_cache()
test_course_flag = CourseWaffleFlag(
self.TEST_NAMESPACE,
self.FLAG_NAME,
flag_undefined_default=data['flag_undefined_default']
)
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), data['result'])
self.assertEqual(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
)

View File

@@ -6,6 +6,12 @@ from functools import wraps
from waffle.testutils import override_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']
def override_waffle_flag(flag, active):
"""

View File

@@ -9,6 +9,7 @@ Utility classes for testing django applications.
"""
import copy
import re
from unittest import skipUnless
import crum
@@ -17,9 +18,10 @@ from django.conf import settings
from django.contrib import sites
from django.contrib.auth.models import AnonymousUser
from django.core.cache import caches
from django.db import DEFAULT_DB_ALIAS, connections
from django.test import RequestFactory, TestCase, override_settings
from django.test.utils import CaptureQueriesContext
from nose.plugins import Plugin
from request_cache.middleware import RequestCache
@@ -145,6 +147,82 @@ class CacheIsolationTestCase(CacheIsolationMixin, TestCase):
self.addCleanup(self.clear_caches)
class _AssertNumQueriesContext(CaptureQueriesContext):
"""
This is a copy of Django's internal class of the same name, with the
addition of being able to provide a table_blacklist used to filter queries
before comparing the count.
"""
def __init__(self, test_case, num, connection, table_blacklist=None):
"""
Same as Django's _AssertNumQueriesContext __init__, with the addition of
the following argument:
table_blacklist (List): A list of table names to filter out of the
set of queries that get counted.
"""
self.test_case = test_case
self.num = num
self.table_blacklist = table_blacklist
super(_AssertNumQueriesContext, self).__init__(connection)
def __exit__(self, exc_type, exc_value, traceback):
def is_unfiltered_query(query):
"""
Returns True if the query does not contain a blacklisted table, and
False otherwise.
Note: This is a simple naive implementation that makes no attempt
to parse the query.
"""
if self.table_blacklist:
for table in self.table_blacklist:
# SQL contains the following format for columns:
# "table_name"."column_name". The regex ensures there is no
# "." before the name to avoid matching columns.
if re.search(r'[^.]"{}"'.format(table), query['sql']):
return False
return True
super(_AssertNumQueriesContext, self).__exit__(exc_type, exc_value, traceback)
if exc_type is not None:
return
filtered_queries = [query for query in self.captured_queries if is_unfiltered_query(query)]
executed = len(filtered_queries)
self.test_case.assertEqual(
executed, self.num,
"%d queries executed, %d expected\nCaptured queries were:\n%s" % (
executed, self.num,
'\n'.join(
query['sql'] for query in filtered_queries
)
)
)
class FilteredQueryCountMixin(object):
"""
Mixin to add to any subclass of Django's TestCase that replaces
assertNumQueries with one that accepts a blacklist of tables to filter out
of the count.
"""
def assertNumQueries(self, num, func=None, table_blacklist=None, *args, **kwargs):
"""
Used to replace Django's assertNumQueries with the same capability, with
the addition of the following argument:
table_blacklist (List): A list of table names to filter out of the
set of queries that get counted.
"""
using = kwargs.pop("using", DEFAULT_DB_ALIAS)
conn = connections[using]
context = _AssertNumQueriesContext(self, num, conn, table_blacklist=table_blacklist)
if func is None:
return context
with context:
func(*args, **kwargs)
class NoseDatabaseIsolation(Plugin):
"""
nosetest plugin that resets django databases before any tests begin.