Merge lp:~mbp/launchpad/flags-webapp into lp:launchpad

[Martin Pool]

add flags webapp infrastructure

Since <https://code.edge.launchpad.net/~mbp/launchpad/flags-webapp/+merge/31122> was accepted into db-devel, I'd like to also land it into devel, since the db dependencies are now there.

[Robert Collins (lifeless)]

naturally, please do so.

review: Approve

=== modified file 'lib/canonical/configure.zcml'
--- lib/canonical/configure.zcml	2010-07-24 00:07:54 +0000
+++ lib/canonical/configure.zcml	2010-08-17 02:31:17 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009, 2010 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -21,6 +21,7 @@
     <include package="lp.services.job" />
     <include package="lp.services.memcache" />
     <include package="lp.services.profile" />
+    <include package="lp.services.features" />
     <include package="lp.services.scripts" />
     <include package="lp.services.worlddata" />
     <include package="lp.services.salesforce" />

=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py	2010-08-02 02:23:26 +0000
+++ lib/canonical/launchpad/webapp/servers.py	2010-08-17 02:31:17 +0000
@@ -79,6 +79,10 @@
 
 from canonical.lazr.timeout import set_default_timeout_function
 
+from lp.services.features.flags import (
+    NullFeatureController,
+    )
+
 
 class StepsToGo:
     """
@@ -838,6 +842,9 @@
         self.needs_datetimepicker_iframe = False
         self.needs_json = False
         self.needs_gmap2 = False
+        # stub out the FeatureController that would normally be provided by
+        # the publication mechanism
+        self.features = NullFeatureController()
 
     @property
     def uuid(self):

=== modified file 'lib/lp/app/browser/tests/base-layout.txt'
--- lib/lp/app/browser/tests/base-layout.txt	2010-08-04 13:38:22 +0000
+++ lib/lp/app/browser/tests/base-layout.txt	2010-08-17 02:31:17 +0000
@@ -135,10 +135,11 @@
       Has application tabs: False
       Has side portlets: False
       At least ... queries issued in ... seconds
+      Features: {}
+      in scopes {}
       r...
     -->
 
-
 Page Headings
 -------------
 

=== modified file 'lib/lp/app/templates/base-layout.pt'
--- lib/lp/app/templates/base-layout.pt	2010-08-06 15:40:39 +0000
+++ lib/lp/app/templates/base-layout.pt	2010-08-17 02:31:17 +0000
@@ -14,6 +14,8 @@
     site_message modules/canonical.config/config/launchpad/site_message;
     icingroot string:${rooturl}+icing/rev${revno};
     icingroot_contrib string:${rooturl}+icing-contrib/rev${revno};
+    features request/features;
+    feature_scopes request/features/scopes;
     CONTEXTS python:{'template':template, 'context': context, 'view':view};
   "
 ><metal:doctype define-slot="doctype"><tal:doctype tal:replace="structure string:&lt;!DOCTYPE html PUBLIC &quot;-//W3C//DTD XHTML 1.0 Transitional//EN&quot; &quot;http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd&quot;&gt;" /></metal:doctype>
@@ -171,7 +173,7 @@
 
 <tal:template>
   <tal:comment
-    define="log modules/canonical.launchpad.webapp.adapter/summarize_requests"
+    define="log modules/canonical.launchpad.webapp.adapter/summarize_requests;"
     replace="structure string:&lt;!--
     Facet name: ${view/menu:selectedfacetname}
     Page type: ${view/macro:pagetype}
@@ -181,6 +183,9 @@
 
     At least ${log}
 
+    Features: ${request/features/usedFlags} 
+    in scopes ${request/features/usedScopes} 
+
     r${revno}
   --&gt;" />
 </tal:template>

=== modified file 'lib/lp/services/features/__init__.py'
--- lib/lp/services/features/__init__.py	2010-07-21 11:05:14 +0000
+++ lib/lp/services/features/__init__.py	2010-08-17 02:31:17 +0000
@@ -5,6 +5,25 @@
 
 These can be turned on and off by admins, and can affect particular
 defined scopes such as "beta users" or "production servers."
-
-See <https://dev.launchpad.net/LEP/FeatureFlags>
-"""
+"""
+
+import threading
+
+
+__all__ = [
+    'getFeatureFlag',
+    'per_thread',
+    ]
+
+
+per_thread = threading.local()
+"""Holds the default per-thread feature controller in its .features attribute.
+
+Framework code is responsible for setting this in the appropriate context, eg
+when starting a web request.
+"""
+
+
+def getFeatureFlag(flag):
+    """Get the value of a flag for this thread's scopes."""
+    return per_thread.features.getFlag(flag)

=== added file 'lib/lp/services/features/configure.zcml'
--- lib/lp/services/features/configure.zcml	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/configure.zcml	2010-08-17 02:31:17 +0000
@@ -0,0 +1,19 @@
+<!-- Copyright 2010 Canonical Ltd.  This software is licensed under the
+     GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<configure
+    xmlns="http://namespaces.zope.org/zope"
+    xmlns:browser="http://namespaces.zope.org/browser">
+
+    <subscriber
+        for="canonical.launchpad.webapp.interfaces.IStartRequestEvent"
+        handler="lp.services.features.webapp.start_request"
+        />
+
+    <subscriber
+        for="zope.app.publication.interfaces.IEndRequestEvent"
+        handler="lp.services.features.webapp.end_request"
+        />
+
+</configure>

=== added directory 'lib/lp/services/features/doc'
=== added file 'lib/lp/services/features/doc/features.txt'
--- lib/lp/services/features/doc/features.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/doc/features.txt	2010-08-17 02:31:17 +0000
@@ -0,0 +1,127 @@
+****************************
+Feature Flag Developer Guide
+****************************
+
+Introduction
+************
+
+The point of feature flags is to let us turn some features of Launchpad on
+and off without changing the code or restarting the application, and to
+expose different features to different subsets of users.
+
+See <https://dev.launchpad.net/LEP/FeatureFlags> for more discussion and
+rationale.
+
+The typical use for feature flags is within web page requests but they can
+also be used in asynchronous jobs or apis or other parts of Launchpad.
+
+Internal model for feature flags
+********************************
+
+A feature flag maps from a *name* to a *value*.  The specific value used
+for a particular request is determined by a set of zero or more *scopes*
+that apply to that request, by finding the *rule* with the highest
+*priority*.
+
+Flags are defined by a *name* that typically looks like a Python
+identifier, for example ``notification.global.text``.  A definition is
+given for a particular *scope*, which also looks like a dotted identifier,
+for example ``user.beta`` or ``server.edge``.  This is just a naming
+convention, and they do not need to correspond to Python modules.
+
+The value is stored in the database as just a Unicode string, and it might
+be interpreted as a boolean, number, human-readable string or whatever.
+
+The default for flags is to be None if they're not set in the database, so
+that should be a sensible baseline default state.
+
+Performance model
+*****************
+
+Flags are supposed to be cheap enough that you can introduce them without
+causing a performance concern.
+
+If the page does not check any flags, no extra work will be done.  The
+first time a page checks a flag, all the rules will be read from the
+database and held in memory for the duration of the request.
+
+Scopes may be expensive in some cases, such as checking group membership.
+Whether a scope is active or not is looked up the first time it's needed
+within a particular request.
+
+The standard page footer identifies the flags and scopes that were
+actually used by the page.
+
+Naming conventions
+******************
+
+We have naming conventions for feature flags and scopes, so that people can
+understand the likely impact of a particular flag and so they can find all
+the flags likely to affect a feature.
+
+So for any flag we want to say:
+
+* What application area does this affect? (malone, survey, questions,
+  code, etc)
+
+* What specific feature does it change?
+
+* What affect does it have on this feature?  The most common is "enabled"
+  but for some other we want to specify a specific value as well such as
+  "date" or "size".
+
+These are concatenated with dots so the overall feature name looks a bit
+like a Python module name.
+
+A similar approach is used for scopes.
+
+Checking flags in page templates
+********************************
+
+You can conditionally show some text like this::
+
+  <tal:survey condition="features/user_survey.enabled">
+    &nbsp;&bull;&nbsp;
+    <a href="http://survey.example.com/">Take our survey!</a>
+  </tal:survey>
+
+You can use the built-in TAL feature of prepending ``not:`` to the
+condition, and for flags that have a value you could use them in
+``tal:replace`` or ``tal:attributes``.
+
+If you just want to simply insert some text taken from a feature, say
+something like::
+
+  Message of the day: ${motd.text}
+
+Templates can also check whether the request is in a particular scope, but
+before using this consider whether the code will always be bound to that
+scope or whether it would be more correct to define a new feature::
+
+  <p tal:condition="feature_scopes/server.staging">
+    Staging server: all data will be discarded daily!</p>
+
+Checking flags in code
+**********************
+
+The Zope traversal code establishes a `FeatureController` for the duration
+of a request.  The object can be obtained through either
+`request.features` or `lp.services.features.per_thread.features`.  This
+provides various useful methods including `getFlag` to look up one feature
+(memoized), and `isInScope` to check one scope (also memoized).
+
+As a convenience, `lp.services.features.getFeatureFlag` looks up a single
+flag in the thread default controller.
+
+Debugging feature usage
+***********************
+
+The flags active during a page request, and the scopes that were looked
+up are visible in the comment at the bottom of every standard Launchpad
+page.
+
+Defining scopes
+***************
+
+
+.. vim: ft=rst

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2010-07-22 12:45:51 +0000
+++ lib/lp/services/features/flags.py	2010-08-17 02:31:17 +0000
@@ -1,17 +1,47 @@
 # Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-__all__ = ['FeatureController']
+__all__ = [
+    'FeatureController',
+    'NullFeatureController',
+    ]
+
 
 __metaclass__ = type
 
 
+from storm.locals import Desc
+
 from lp.services.features.model import (
+    FeatureFlag,
     getFeatureStore,
-    FeatureFlag,
     )
 
 
+class Memoize(object):
+
+    def __init__(self, calc):
+        self._known = {}
+        self._calc = calc
+
+    def lookup(self, key):
+        if key in self._known:
+            return self._known[key]
+        v = self._calc(key)
+        self._known[key] = v
+        return v
+
+
+class ScopeDict(object):
+    """Allow scopes to be looked up by getitem"""
+
+    def __init__(self, features):
+        self.features = features
+
+    def __getitem__(self, scope_name):
+        return self.features.isInScope(scope_name)
+
+
 class FeatureController(object):
     """A FeatureController tells application code what features are active.
 
@@ -34,52 +64,101 @@
     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.
+    the whole feature flag table from the database.  It is expected to be
+    reasonably small.  The scopes may be expensive to compute (eg checking
+    team membership) so they are checked at most once when they are first
+    needed.
+
+    The controller is then supposed to be held in a thread-local and reused
+    for the duration of the request.
 
     See <https://dev.launchpad.net/LEP/FeatureFlags>
     """
 
-    def __init__(self, scopes):
+    def __init__(self, scope_check_callback):
         """Construct a new view of the features for a set of scopes.
-        """
-        self._store = getFeatureStore()
-        self._scopes = self._preenScopes(scopes)
-        self._cached_flags = self._queryAllFlags()
-
-    def getScopes(self):
-        return frozenset(self._scopes)
-
-    def getFlag(self, flag_name):
-        return self._cached_flags.get(flag_name)
+
+        :param scope_check_callback: Given a scope name, says whether
+            it's active or not.
+        """
+        self._known_scopes = Memoize(scope_check_callback)
+        self._known_flags = Memoize(self._checkFlag)
+        # rules are read from the database the first time they're needed
+        self._rules = None
+        self.scopes = ScopeDict(self)
+
+    def getFlag(self, flag):
+        """Get the value of a specific flag.
+        
+        :param flag: A name to lookup. e.g. 'recipes.enabled'
+        :return: The value of the flag determined by the highest priority rule
+            that matched.
+        """
+        return self._known_flags.lookup(flag)
+
+    def _checkFlag(self, flag):
+        self._needRules()
+        if flag in self._rules:
+            for scope, value in self._rules[flag]:
+                if self._known_scopes.lookup(scope):
+                    return value
+
+    def isInScope(self, scope):
+        return self._known_scopes.lookup(scope)
+
+    def __getitem__(self, flag_name):
+        """FeatureController can be indexed.
+
+        This is to support easy zope traversal through eg
+        "request/features/a.b.c".  We don't support other collection
+        protocols.
+
+        Note that calling this the first time for any key may cause
+        arbitrarily large amounts of work to be done to determine if the
+        controller is in any scopes relevant to this flag.
+        """
+        return self.getFlag(flag_name)
 
     def getAllFlags(self):
-        """Get the feature flags active for the current scopes.
+        """Return a dict of all active flags.
 
-        :returns: dict from flag_name (unicode) to value (unicode).
+        This may be expensive because of evaluating many scopes, so it
+        shouldn't normally be used by code that only wants to know about one
+        or a few flags.
         """
-        return dict(self._cached_flags)
+        self._needRules()
+        return dict((f, self.getFlag(f)) for f in self._rules)
 
-    def _queryAllFlags(self):
+    def _loadRules(self):
+        store = getFeatureStore()
         d = {}
-        rs = (self._store
-                .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
+        rs = (store
+                .find(FeatureFlag)
+                .order_by(Desc(FeatureFlag.priority))
+                .values(FeatureFlag.flag, FeatureFlag.scope,
+                    FeatureFlag.value))
+        for flag, scope, value in rs:
+            d.setdefault(str(flag), []).append((str(scope), value))
         return d
 
-    def _preenScopes(self, scopes):
-        # for convenience turn strings to unicode
-        us = []
-        for s in scopes:
-            if isinstance(s, unicode):
-                us.append(s)
-            elif isinstance(s, str):
-                us.append(unicode(s))
-            else:
-                raise TypeError("invalid scope: %r" % s)
-        return us
+    def _needRules(self):
+        if self._rules is None:
+            self._rules = self._loadRules()
+
+    def usedFlags(self):
+        """Return dict of flags used in this controller so far."""
+        return dict(self._known_flags._known)
+
+    def usedScopes(self):
+        """Return {scope: active} for scopes that have been used so far."""
+        return dict(self._known_scopes._known)
+
+
+class NullFeatureController(FeatureController):
+    """For use in testing: everything is turned off"""
+
+    def __init__(self):
+        FeatureController.__init__(self, lambda scope: None)
+
+    def _loadRules(self):
+        return []

=== modified file 'lib/lp/services/features/tests/test_flags.py'
--- lib/lp/services/features/tests/test_flags.py	2010-07-22 12:45:51 +0000
+++ lib/lp/services/features/tests/test_flags.py	2010-08-17 02:31:17 +0000
@@ -12,7 +12,11 @@
 from canonical.testing import layers
 from lp.testing import TestCase
 
-from lp.services.features import model
+from lp.services.features import (
+    getFeatureFlag,
+    model,
+    per_thread,
+    )
 from lp.services.features.flags import (
     FeatureController,
     )
@@ -20,11 +24,10 @@
 
 notification_name = 'notification.global.text'
 notification_value = u'\N{SNOWMAN} stormy Launchpad weather ahead'
-example_scope = 'beta_user'
 
 
 testdata = [
-    (example_scope, notification_name, notification_value, 100),
+    ('beta_user', notification_name, notification_value, 100),
     ('default', 'ui.icing', u'3.0', 100),
     ('beta_user', 'ui.icing', u'4.0', 300),
     ]
@@ -40,6 +43,14 @@
             from storm.tracer import debug
             debug(True)
 
+    def makeControllerInScopes(self, scopes):
+        """Make a controller that will report it's in the given scopes."""
+        call_log = []
+        def scope_cb(scope):
+            call_log.append(scope)
+            return scope in scopes
+        return FeatureController(scope_cb), call_log
+
     def populateStore(self):
         store = model.getFeatureStore()
         for (scope, flag, value, priority) in testdata:
@@ -49,37 +60,51 @@
                 value=value,
                 priority=priority))
 
-    def test_defaultFlags(self):
-        # the sample db has no flags set
-        control = FeatureController([])
-        self.assertEqual({},
-            control.getAllFlags())
-
     def test_getFlag(self):
         self.populateStore()
-        control = FeatureController(['default'])
+        control, call_log = self.makeControllerInScopes(['default'])
         self.assertEqual(u'3.0',
             control.getFlag('ui.icing'))
+        self.assertEqual(['beta_user', 'default'], call_log)
+
+    def test_getItem(self):
+        # for use in page templates, the flags can be treated as a dict
+        self.populateStore()
+        control, call_log = self.makeControllerInScopes(['default'])
+        self.assertEqual(u'3.0',
+            control['ui.icing'])
+        self.assertEqual(['beta_user', 'default'], call_log)
+        # after looking this up the value is known and the scopes are
+        # positively and negatively cached
+        self.assertEqual({'ui.icing': u'3.0'}, control.usedFlags())
+        self.assertEqual(dict(beta_user=False, default=True),
+            control.usedScopes())
 
     def test_getAllFlags(self):
         # can fetch all the active flags, and it gives back only the
-        # highest-priority settings
+        # highest-priority settings.  this may be expensive and shouldn't
+        # normally be used.
         self.populateStore()
-        control = FeatureController(['default', 'beta_user'])
+        control, call_log = self.makeControllerInScopes(
+            ['beta_user', 'default'])
         self.assertEqual(
             {'ui.icing': '4.0',
              notification_name: notification_value},
             control.getAllFlags())
+        # evaluates all necessary flags; in this test data beta_user shadows
+        # default settings
+        self.assertEqual(['beta_user'], call_log)
 
     def test_overrideFlag(self):
         # if there are multiple settings for a flag, and they match multiple
         # scopes, the priorities determine which is matched
         self.populateStore()
-        default_control = FeatureController(['default'])
+        default_control, call_log = self.makeControllerInScopes(['default'])
         self.assertEqual(
             u'3.0',
             default_control.getFlag('ui.icing'))
-        beta_control = FeatureController(['default', 'beta_user'])
+        beta_control, call_log = self.makeControllerInScopes(
+            ['beta_user', 'default'])
         self.assertEqual(
             u'4.0',
             beta_control.getFlag('ui.icing'))
@@ -87,9 +112,53 @@
     def test_undefinedFlag(self):
         # if the flag is not defined, we get None
         self.populateStore()
-        control = FeatureController(['default', 'beta_user'])
+        control, call_log = self.makeControllerInScopes(
+            ['beta_user', 'default'])
         self.assertIs(None,
             control.getFlag('unknown_flag'))
-        no_scope_flags = FeatureController([])
+        no_scope_flags, call_log = self.makeControllerInScopes([])
         self.assertIs(None,
             no_scope_flags.getFlag('ui.icing'))
+
+    def test_threadGetFlag(self):
+        self.populateStore()
+        # the start-of-request handler will do something like this:
+        per_thread.features, call_log = self.makeControllerInScopes(
+            ['default', 'beta_user'])
+        try:
+            # then application code can simply ask without needing a context
+            # object
+            self.assertEqual(u'4.0', getFeatureFlag('ui.icing'))
+        finally:
+            per_thread.features = None
+
+    def testLazyScopeLookup(self):
+        # feature scopes may be a bit expensive to look up, so we do it only
+        # when it will make a difference to the result.
+        self.populateStore()
+        f, call_log = self.makeControllerInScopes(['beta_user'])
+        self.assertEqual(u'4.0', f.getFlag('ui.icing'))
+        # to calculate this it should only have had to check we're in the
+        # beta_users scope; nothing else makes a difference
+        self.assertEqual(dict(beta_user=True), f._known_scopes._known)
+
+    def testUnknownFeature(self):
+        # looking up an unknown feature gives you None
+        self.populateStore()
+        f, call_log = self.makeControllerInScopes([])
+        self.assertEqual(None, f.getFlag('unknown'))
+        # no scopes need to be checked because it's just not in the database
+        # and there's no point checking
+        self.assertEqual({}, f._known_scopes._known)
+        self.assertEquals([], call_log)
+        # however, this we have now negative-cached the flag
+        self.assertEqual(dict(unknown=None), f.usedFlags())
+        self.assertEqual(dict(), f.usedScopes())
+
+    def testScopeDict(self):
+        # can get scopes as a dict, for use by "feature_scopes/server.demo"
+        f, call_log = self.makeControllerInScopes(['beta_user'])
+        self.assertEqual(True, f.scopes['beta_user'])
+        self.assertEqual(False, f.scopes['alpha_user'])
+        self.assertEqual(True, f.scopes['beta_user'])
+        self.assertEqual(['beta_user', 'alpha_user'], call_log)

=== added file 'lib/lp/services/features/webapp.py'
--- lib/lp/services/features/webapp.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/webapp.py	2010-08-17 02:31:17 +0000
@@ -0,0 +1,41 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Connect Feature flags into webapp requests."""
+
+__all__ = []
+
+__metaclass__ = type
+
+import canonical.config
+
+from lp.services.features import (
+    per_thread,
+    )
+from lp.services.features.flags import (
+    FeatureController,
+    )
+
+
+class ScopesFromRequest(object):
+    """Identify feature scopes based on request state."""
+
+    def __init__(self, request):
+        self._request = request
+
+    def lookup(self, scope_name):
+        parts = scope_name.split('.')
+        if len(parts) == 2:
+            if parts[0] == 'server':
+                return canonical.config.config['launchpad']['is_' + parts[1]]
+
+
+def start_request(event):
+    """Register FeatureController."""
+    event.request.features = per_thread.features = FeatureController(
+        ScopesFromRequest(event.request).lookup)
+
+
+def end_request(event):
+    """Done with this FeatureController."""
+    event.request.features = per_thread.features = None