Incremental diff: * facade loads everything at construction time * remove write facade methods === modified file 'lib/lp/services/features/flags.py' --- lib/lp/services/features/flags.py 2010-07-22 10:48:14 +0000 +++ lib/lp/services/features/flags.py 2010-07-22 12:28:32 +0000 @@ -6,21 +6,16 @@ __metaclass__ = type -from lp.services.features import model +from lp.services.features.model import ( + getFeatureStore, + FeatureFlag, + ) from storm.locals import ( Desc, ) -# Intended performance: when this object is first constructed, it will read -# the whole current feature flags from the database. This will take a few ms. -# The controller is then supposed to be held in a thread-local for the -# duration of the request. The scopes can be changed over the lifetime of the -# controller, because we might not know enough to determine all the active -# scopes when the object's first created. This isn't validated to work yet. - - class FeatureController(object): """A FeatureController tells application code what features are active. @@ -30,18 +25,21 @@ of the current web request, or the user for whom a job is being run, or something similar. - This presents a higher-level facade that is independent of how the flags - are stored. At the moment they are stored in the database but callers - shouldn't need to know about that: they could go into memcachedb or - elsewhere in future. + FeatureController presents a high level interface for application code to + query flag values, without it needing to know that they are stored in the database. At this level flag names and scope names are presented as strings for easier use in Python code, though the values remain unicode. They should always be ascii like Python identifiers. - One instance of this should be constructed for the lifetime of code that - has consistent configuration values. For instance there will be one per web - app request. + One instance of FeatureController should be constructed for the lifetime + of code that has consistent configuration values. For instance there will + be one per web app request. + + Intended performance: when this object is first constructed, it will read + the whole current feature flags from the database. This will take a few ms. + The controller is then supposed to be held in a thread-local for the + duration of the request. See """ @@ -49,34 +47,33 @@ def __init__(self, scopes): """Construct a new view of the features for a set of scopes. """ - self._store = model.getFeatureStore() - self.scopes = self._preenScopes(scopes) + self._store = getFeatureStore() + self._scopes = self._preenScopes(scopes) + self._cached_flags = self._queryAllFlags() - def setScopes(self, scopes): - self.scopes = self._preenScopes(scopes) + def getScopes(self): + return frozenset(self._scopes) def getFlag(self, flag_name): - rs = (self._store - .find(model.FeatureFlag, - model.FeatureFlag.scope.is_in(self.scopes), - model.FeatureFlag.flag == unicode(flag_name)) - .order_by(Desc(model.FeatureFlag.priority))) - row = rs.first() - if row is None: - return None - else: - return row.value + return self._cached_flags.get(flag_name) def getAllFlags(self): """Get the feature flags active for the current scopes. :returns: dict from flag_name (unicode) to value (unicode). """ + return dict(self._cached_flags) + + def _queryAllFlags(self): + d = {} rs = (self._store - .find(model.FeatureFlag, - model.FeatureFlag.scope.is_in(self.scopes)) - .order_by(model.FeatureFlag.priority)) - return dict((str(f.flag), f.value) for f in rs) + .find(FeatureFlag, + FeatureFlag.scope.is_in(self._scopes)) + .order_by(FeatureFlag.priority) + .values(FeatureFlag.flag, FeatureFlag.value)) + for flag, value in rs: + d[str(flag)] = value + return d def _preenScopes(self, scopes): # for convenience turn strings to unicode @@ -89,16 +86,3 @@ else: raise TypeError("invalid scope: %r" % s) return us - - def addSetting(self, scope, flag, value, priority): - """Add a setting for a flag. - - Note that flag settings are global in the database: they affect all - FeatureControllers connected to this database, and they will persist - if the database transaction is committed. - """ - flag_obj = model.FeatureFlag(scope=unicode(scope), - flag=unicode(flag), - value=value, - priority=priority) - self._store.add(flag_obj) === modified file 'lib/lp/services/features/model.py' --- lib/lp/services/features/model.py 2010-07-22 10:48:14 +0000 +++ lib/lp/services/features/model.py 2010-07-22 10:58:57 +0000 @@ -1,7 +1,9 @@ # Copyright 2010 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). -__all__ = ['FeatureFlag' +__all__ = [ + 'FeatureFlag', + 'getFeatureStore', ] __metaclass__ = type === modified file 'lib/lp/services/features/tests/__init__.py' --- lib/lp/services/features/tests/__init__.py 2010-07-21 11:05:14 +0000 +++ lib/lp/services/features/tests/__init__.py 2010-07-22 11:51:22 +0000 @@ -1,4 +1,2 @@ # Copyright 2010 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). - -pass === modified file 'lib/lp/services/features/tests/test_db_settings.py' --- lib/lp/services/features/tests/test_db_settings.py 2010-07-22 10:48:14 +0000 +++ lib/lp/services/features/tests/test_db_settings.py 2010-07-22 11:56:18 +0000 @@ -7,16 +7,15 @@ from __future__ import with_statement __metaclass__ = type -import testtools - from canonical.testing import layers +from lp.testing import TestCase from lp.services.features.model import ( getFeatureStore, FeatureFlag, ) -class TestFeatureModel(testtools.TestCase): +class TestFeatureModel(TestCase): layer = layers.DatabaseFunctionalLayer === modified file 'lib/lp/services/features/tests/test_flags.py' --- lib/lp/services/features/tests/test_flags.py 2010-07-22 10:48:14 +0000 +++ lib/lp/services/features/tests/test_flags.py 2010-07-22 12:27:38 +0000 @@ -1,20 +1,21 @@ # Copyright 2010 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). -"""Tests for feature flags. - -""" +"""Tests for feature flags.""" from __future__ import with_statement __metaclass__ = type import os -import testtools from canonical.testing import layers +from lp.testing import TestCase from lp.services.features import flags, model +from lp.services.features.flags import ( + FeatureController, + ) notification_name = 'notification.global.text' @@ -22,7 +23,14 @@ example_scope = 'beta_user' -class TestFeatureFlags(testtools.TestCase): +testdata = [ + (example_scope, notification_name, notification_value, 100), + ('default', 'ui.icing', u'3.0', 100), + ('beta_user', 'ui.icing', u'4.0', 300), + ] + + +class TestFeatureFlags(TestCase): layer = layers.DatabaseFunctionalLayer @@ -32,35 +40,32 @@ from storm.tracer import debug debug(True) + def populateStore(self): + store = model.getFeatureStore() + for (scope, flag, value, priority) in testdata: + store.add(model.FeatureFlag( + scope=unicode(scope), + flag=unicode(flag), + value=value, + priority=priority)) + def test_defaultFlags(self): # the sample db has no flags set - control = flags.FeatureController([]) + control = FeatureController([]) self.assertEqual({}, control.getAllFlags()) - def test_simpleFlags(self): - # with some flags set in the db, you can query them through the - # FeatureController - flag = model.FeatureFlag( - scope=unicode(example_scope), - flag=unicode(notification_name), - value=notification_value, - priority=100) - model.getFeatureStore().add(flag) - control = flags.FeatureController(['beta_user']) - self.assertEqual(notification_value, - control.getFlag(notification_name)) - - def test_setFlags(self): - # you can also set flags through a facade - control = self.makePopulatedController() - self.assertEqual(notification_value, - control.getFlag(notification_name)) + def test_getFlag(self): + self.populateStore() + control = FeatureController(['default']) + self.assertEqual(u'3.0', + control.getFlag('ui.icing')) def test_getAllFlags(self): # can fetch all the active flags, and it gives back only the # highest-priority settings - control = self.makeControllerWithOverrides() + self.populateStore() + control = FeatureController(['default', 'beta_user']) self.assertEqual( {'ui.icing': '4.0', notification_name: notification_value}, @@ -69,43 +74,22 @@ def test_overrideFlag(self): # if there are multiple settings for a flag, and they match multiple # scopes, the priorities determine which is matched - control = self.makeControllerWithOverrides() - control.setScopes(['default']) + self.populateStore() + default_control = FeatureController(['default']) self.assertEqual( u'3.0', - control.getFlag('ui.icing')) - control.setScopes(['default', 'beta_user']) + default_control.getFlag('ui.icing')) + beta_control = FeatureController(['default', 'beta_user']) self.assertEqual( u'4.0', - control.getFlag('ui.icing')) + beta_control.getFlag('ui.icing')) def test_undefinedFlag(self): # if the flag is not defined, we get None - control = self.makeControllerWithOverrides() + self.populateStore() + control = FeatureController(['default', 'beta_user']) self.assertIs(None, control.getFlag('unknown_flag')) - control.setScopes([]) + no_scope_flags = FeatureController([]) self.assertIs(None, - control.getFlag('ui.icing')) - - def makePopulatedController(self): - # make a controller with some test flags - control = flags.FeatureController(['beta_user']) - control.addSetting( - scope=example_scope, flag=notification_name, - value=notification_value, priority=100) - return control - - def makeControllerWithOverrides(self): - control = self.makePopulatedController() - control.addSetting( - scope='default', - flag='ui.icing', - value=u'3.0', - priority=100) - control.addSetting( - scope='beta_user', - flag='ui.icing', - value=u'4.0', - priority=300) - return control + no_scope_flags.getFlag('ui.icing'))