Merge lp:~leonardr/launchpad/toggle-representation-cache into lp:launchpad

[Leonard Richardson]

This branch implements the enable_server_side_representation_cache lazr.restful configuration option using a Launchpad configuration option. That option is defined in https://code.edge.launchpad.net/~leonardr/lazr.restful/disable-cache/+merge/26713 and is still in review.

[Jelmer Vernooij (jelmer)]

In lib/lp/services/memcache/doc/restful-cache.txt you're missing an asterisk on the first line.

It'd be nice to have docstrings for the methods in MemcachedStormRepresentationCache, even if they just say "See `BaseRepositoryCache`."

review: Approve (code)

[Leonard Richardson (leonardr)]

I need another review of the last couple revisions. While going through stub's review of my earlier branch, I remembered that we planned to set a default expiration time for items in the cache, so that when we tested on staging the cache wouldn't contain bad data indefinitely. I've implemented that code and a test, as well as responding to Jelmer's feedback and some more of Stuart's.

(Stuart's review is here: https://code.edge.launchpad.net/~leonardr/launchpad/test-representation-cache/+merge/26513)

I couldn't think of any way to test the expiration except from actually sleeping for 2 seconds, since I don't think we have a mocked-up memcached for use in testing. Suggestions are welcome.

[Paul Hummer (rockstar)]

review: Approve (code)

[Leonard Richardson (leonardr)]

I need one more tiny review. I got a lots of test failures because memcached thinks space is a control character. So after creating the key, I replace any spaces with periods.

[Graham Binns (gmb)]

review: Approve (code)

[Leonard Richardson (leonardr)]

I was still getting test failures, so I made a number of changes (which required a new version of lazr.restful: see https://code.edge.launchpad.net/~leonardr/lazr.restful/cache-unauthorized/+merge/27523). Here's a paste with the contents of the two commits I've made that weren't merges against trunk:

http://pastebin.ubuntu.com/449786/

[Aaron Bentley (abentley)]

As discussed in IRC
- Please catch storm.exceptions.ClassInfoError rather than Exception
- If possible, narrow the scope of the related try/except block.
- Please be more specific in applying defaults, so that cached values that evaluate to False are handled correctly.

review: Approve

=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf	2010-05-27 07:05:20 +0000
+++ configs/development/launchpad-lazr.conf	2010-06-15 13:39:43 +0000
@@ -275,6 +275,9 @@
 [vhost.api]
 hostname: api.launchpad.dev
 rooturl: https://api.launchpad.dev/
+# Turn this on once we've solved cache invalidation problems and are
+# ready to test.
+# enable_server_side_representation_cache: True
 
 [vhost.blueprints]
 hostname: blueprints.launchpad.dev

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf	2010-06-08 15:13:20 +0000
+++ lib/canonical/config/schema-lazr.conf	2010-06-15 13:39:43 +0000
@@ -1950,6 +1950,9 @@
 [vhost.api]
 # This key should be removed once the production configs have been updated.
 beta_test_team: disabled
+enable_server_side_representation_cache: False
+# By default, cache representations for 4 hours.
+representation_cache_expiration_time: 14400
 
 [vhost.blueprints]
 

=== modified file 'lib/canonical/database/sqlbase.py'
--- lib/canonical/database/sqlbase.py	2010-03-24 17:37:26 +0000
+++ lib/canonical/database/sqlbase.py	2010-06-15 13:39:43 +0000
@@ -28,6 +28,8 @@
 from zope.interface import implements
 from zope.security.proxy import removeSecurityProxy
 
+from lazr.restful.interfaces import IRepresentationCache
+
 from canonical.config import config, dbconfig
 from canonical.database.interfaces import ISQLBase
 
@@ -246,6 +248,11 @@
         """Inverse of __eq__."""
         return not (self == other)
 
+    def __storm_flushed__(self):
+        """Invalidate the web service cache."""
+        cache = getUtility(IRepresentationCache)
+        cache.delete(self)
+
 alreadyInstalledMsg = ("A ZopelessTransactionManager with these settings is "
 "already installed.  This is probably caused by calling initZopeless twice.")
 

=== added file 'lib/canonical/launchpad/pagetests/webservice/cache.txt'
--- lib/canonical/launchpad/pagetests/webservice/cache.txt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/pagetests/webservice/cache.txt	2010-06-15 13:39:43 +0000
@@ -0,0 +1,77 @@
+************************
+The representation cache
+************************
+
+Launchpad stores JSON representations of objects in a memcached
+cache. The full cache functionality is tested in lazr.restful and in
+lib/lp/services/memcache/doc/restful-cache.txt. This is just a simple
+integration test.
+
+By default, the cache is disabled, even in the testrunner
+environment. (This will change once we improve the Launchpad cache's
+cache invalidation.) Let's enable the cache just for this test.
+
+    >>> from canonical.config import config
+    >>> config.vhost.api.enable_server_side_representation_cache
+    False
+    >>> config.push('enable cache', """\
+    ... [vhost.api]
+    ... enable_server_side_representation_cache: True
+    ... """)
+
+Now we need to get a reference to the cache object, so we can look
+inside.
+
+    >>> from zope.component import getUtility
+    >>> from lazr.restful.interfaces import IRepresentationCache
+    >>> cache = getUtility(IRepresentationCache)
+
+Since the cache is keyed by the underlying database object, we also
+need one of those objects.
+
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> login(ANONYMOUS)
+    >>> person = getUtility(IPersonSet).getByName('salgado')
+    >>> key = cache.key_for(person, 'application/json', 'devel')
+    >>> logout()
+
+The cache starts out empty.
+
+    >>> print cache.get_by_key(key)
+    None
+
+Retrieving a representation of an object populates the cache.
+
+    >>> ignore = webservice.get("/~salgado", api_version="devel").jsonBody()
+
+    >>> cache.get_by_key(key)
+    '{...}'
+
+Once the cache is populated with a representation, the cached
+representation is used in preference to generating a new
+representation of that object. We can verify this by putting a fake
+value into the cache and retrieving a representation of the
+corresponding object.
+
+    >>> import simplejson
+    >>> cache.set_by_key(key, simplejson.dumps("Fake representation"))
+
+    >>> print webservice.get("/~salgado", api_version="devel").jsonBody()
+    Fake representation
+
+If there's a problem with the cache or the invalidation code, we can
+disable the cache by setting a configuration variable.
+
+Cleanup: re-disable the cache.
+
+    >>> ignore = config.pop('enable cache')
+
+Note that documents are never served from a disabled cache, even if the
+cache is populated.
+
+    >>> print webservice.get("/~salgado", api_version="devel").jsonBody()
+    {...}
+
+Cleanup: clear the cache.
+
+    >>> cache.delete(person)

=== modified file 'lib/canonical/launchpad/rest/configuration.py'
--- lib/canonical/launchpad/rest/configuration.py	2010-05-17 20:03:02 +0000
+++ lib/canonical/launchpad/rest/configuration.py	2010-06-15 13:39:43 +0000
@@ -63,6 +63,10 @@
         return request
 
     @property
+    def enable_server_side_representation_cache(self):
+        return config.vhost.api.enable_server_side_representation_cache
+
+    @property
     def default_batch_size(self):
         return config.launchpad.default_batch_size
 

=== modified file 'lib/canonical/launchpad/testing/pages.py'
--- lib/canonical/launchpad/testing/pages.py	2010-03-30 09:40:48 +0000
+++ lib/canonical/launchpad/testing/pages.py	2010-06-15 13:39:43 +0000
@@ -40,6 +40,7 @@
 from canonical.launchpad.webapp.interfaces import OAuthPermission
 from canonical.launchpad.webapp.url import urlsplit
 from canonical.testing import PageTestLayer
+from lazr.restful.interfaces import IRepresentationCache
 from lazr.restful.testing.webservice import WebServiceCaller
 from lp.testing import (
     ANONYMOUS, launchpadlib_for, login, login_person, logout)
@@ -672,6 +673,16 @@
     return LaunchpadWebServiceCaller(consumer_key, access_token.key)
 
 
+def ws_uncache(obj):
+    """Manually remove an object from the web service representation cache.
+
+    Directly modifying a data model object during a test may leave
+    invalid data in the representation cache.
+    """
+    cache = getUtility(IRepresentationCache)
+    cache.delete(obj)
+
+
 def setupDTCBrowser():
     """Testbrowser configured for Distribution Translations Coordinators.
 
@@ -776,6 +787,7 @@
     test.globs['print_tag_with_id'] = print_tag_with_id
     test.globs['PageTestLayer'] = PageTestLayer
     test.globs['stop'] = stop
+    test.globs['ws_uncache'] = ws_uncache
 
 
 class PageStoryTestCase(unittest.TestCase):

=== modified file 'lib/canonical/launchpad/zcml/webservice.zcml'
--- lib/canonical/launchpad/zcml/webservice.zcml	2010-03-26 19:11:50 +0000
+++ lib/canonical/launchpad/zcml/webservice.zcml	2010-06-15 13:39:43 +0000
@@ -16,6 +16,11 @@
         provides="lazr.restful.interfaces.IWebServiceConfiguration">
     </utility>
 
+    <utility
+        factory="lp.services.memcache.restful.MemcachedStormRepresentationCache"
+        provides="lazr.restful.interfaces.IRepresentationCache">
+    </utility>
+
     <securedutility
         class="canonical.launchpad.systemhomes.WebServiceApplication"
         provides="canonical.launchpad.interfaces.IWebServiceApplication">

=== modified file 'lib/lp/registry/stories/webservice/xx-project-registry.txt'
--- lib/lp/registry/stories/webservice/xx-project-registry.txt	2010-06-11 18:05:59 +0000
+++ lib/lp/registry/stories/webservice/xx-project-registry.txt	2010-06-15 13:39:43 +0000
@@ -1250,6 +1250,8 @@
 
     >>> logout()
 
+    >>> from lazr.restful.interfaces import IRepresentationCache
+    >>> ws_uncache(mmm)
     >>> mmm = webservice.get("/mega-money-maker").jsonBody()
     >>> print mmm['display_name']
     Mega Money Maker

=== added file 'lib/lp/services/memcache/doc/restful-cache.txt'
--- lib/lp/services/memcache/doc/restful-cache.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/services/memcache/doc/restful-cache.txt	2010-06-15 13:39:43 +0000
@@ -0,0 +1,143 @@
+****************************************
+The Storm/memcached representation cache
+****************************************
+
+The web service library lazr.restful will store the representations it
+generates in a cache, if a suitable cache implementation is
+provided. We implement a cache that stores representations of Storm
+objects in memcached.
+
+    >>> login('[e-mail]')
+
+    >>> from lp.services.memcache.restful import (
+    ...     MemcachedStormRepresentationCache)
+    >>> cache = MemcachedStormRepresentationCache()
+
+An object's cache key is derived from its Storm metadata: its database
+table name and its primary key.
+
+    >>> from zope.component import getUtility
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> person = getUtility(IPersonSet).getByName('salgado')
+
+    >>> cache_key = cache.key_for(
+    ...     person, 'media/type', 'web-service-version')
+    >>> print person.id, cache_key
+    29 Person(29,),testrunner,media/type,web-service-version
+
+    >>> from operator import attrgetter
+    >>> languages = sorted(person.languages, key=attrgetter('englishname'))
+    >>> for language in languages:
+    ...     cache_key = cache.key_for(
+    ...         language, 'media/type', 'web-service-version')
+    ...     print language.id, cache_key
+    119 Language(119,),testrunner,media/type,web-service-version
+    521 Language(521,),testrunner,media/type,web-service-version
+
+The cache starts out empty.
+
+    >>> json_type = 'application/json'
+
+    >>> print cache.get(person, json_type, "v1", default="missing")
+    missing
+
+Add a representation to the cache, and you can retrieve it later.
+
+    >>> cache.set(person, json_type, "beta",
+    ...           "This is a representation for version beta.")
+
+    >>> print cache.get(person, json_type, "beta")
+    This is a representation for version beta.
+
+If an object has no Storm metadata, it is currently not cached at all.
+
+    >>> from lp.hardwaredb.interfaces.hwdb import IHWDBApplication
+    >>> hwdb_app = getUtility(IHWDBApplication)
+    >>> cache.set(hwdb_app, 'media/type', 'web-service-version', 'data')
+    >>> print cache.get(hwdb_app, 'media/type', 'web-service-version')
+    None
+
+A single object can cache different representations for different
+web service versions.
+
+    >>> cache.set(person, json_type, '1.0',
+    ...           'This is a different representation for version 1.0.')
+
+    >>> print cache.get(person, json_type, "1.0")
+    This is a different representation for version 1.0.
+
+The web service version doesn't have to actually be defined in the
+configuration. (But you shouldn't use this--see below!)
+
+    >>> cache.set(person, json_type, 'no-such-version',
+    ...           'This is a representation for a nonexistent version.')
+
+    >>> print cache.get(person, json_type, "no-such-version")
+    This is a representation for a nonexistent version.
+
+A single object can also cache different representations for different
+media types, not just application/json. (But you shouldn't use
+this--see below!)
+
+    >>> cache.set(person, 'media/type', '1.0',
+    ...           'This is a representation for a strange media type.')
+
+    >>> print cache.get(person, "media/type", "1.0")
+    This is a representation for a strange media type.
+
+When a Launchpad object is modified, its JSON representations for
+recognized web service versions are automatically removed from the
+cache.
+
+    >>> person.addressline1 = "New address"
+    >>> from canonical.launchpad.ftests import syncUpdate
+    >>> syncUpdate(person)
+
+    >>> print cache.get(person, json_type, "beta", default="missing")
+    missing
+
+    >>> print cache.get(person, json_type, "1.0", default="missing")
+    missing
+
+But non-JSON representations, and representations for unrecognized web
+service versions, are _not_ removed from the cache. (This is why you
+shouldn't put such representations into the cache.)
+
+    >>> print cache.get(person, json_type, "no-such-version")
+    This is a representation for a nonexistent version.
+
+    >>> print cache.get(person, "media/type", "1.0")
+    This is a representation for a strange media type.
+
+Expiration time
+===============
+
+Objects will automatically be purged from the cache after a
+configurable time. The default is 4 hours (14400 seconds)
+
+    >>> from canonical.config import config
+    >>> print config.vhost.api.representation_cache_expiration_time
+    14400
+
+Let's change that.
+
+    >>> from canonical.config import config
+    >>> config.push('short expiration time', """\
+    ... [vhost.api]
+    ... representation_cache_expiration_time: 1
+    ... """)
+
+    >>> cache.set(person, "some/type", "version", "Some representation")
+    >>> print cache.get(person, "some/type", "version")
+    Some representation
+
+Now objects are purged from the cache after one second.
+
+    >>> import time
+    >>> time.sleep(2)
+    >>> print cache.get(person, "some/type", "version")
+    None
+
+Cleanup.
+
+    >>> ignore = config.pop("short expiration time")

=== added file 'lib/lp/services/memcache/restful.py'
--- lib/lp/services/memcache/restful.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/memcache/restful.py	2010-06-15 13:39:43 +0000
@@ -0,0 +1,63 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Storm/memcached implementation of lazr.restful's representation cache."""
+
+import storm
+
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+from zope.traversing.browser import absoluteURL
+
+from canonical.config import config
+from lp.services.memcache.interfaces import IMemcacheClient
+from lazr.restful.simple import BaseRepresentationCache
+from lazr.restful.utils import get_current_web_service_request
+
+__metaclass__ = type
+__all__ = [
+    'MemcachedStormRepresentationCache',
+]
+
+
+class MemcachedStormRepresentationCache(BaseRepresentationCache):
+    """Caches lazr.restful representations of Storm objects in memcached."""
+
+    def __init__(self):
+        """Initialize with the memcached client."""
+        self.client = getUtility(IMemcacheClient)
+
+    def key_for(self, obj, media_type, version):
+        """See `BaseRepresentationCache`."""
+        obj = removeSecurityProxy(obj)
+        try:
+            storm_info = storm.info.get_obj_info(obj)
+        except storm.exceptions.ClassInfoError, e:
+            # There's no Storm data for this object. Don't cache it,
+            # since we don't know how to invalidate the cache.
+            return self.DO_NOT_CACHE
+        table_name = storm_info.cls_info.table
+        primary_key = tuple(var.get() for var in storm_info.primary_vars)
+        identifier = table_name + repr(primary_key)
+
+        key = (identifier
+               + ',' + config._instance_name
+               + ',' + media_type + ',' + str(version)).replace(' ', '.')
+        return key
+
+    def get_by_key(self, key, default=None):
+        """See `BaseRepresentationCache`."""
+        value = self.client.get(key)
+        if value is None:
+            value = default
+        return value
+
+    def set_by_key(self, key, value):
+        """See `BaseRepresentationCache`."""
+        self.client.set(
+            key, value,
+            time=config.vhost.api.representation_cache_expiration_time)
+
+    def delete_by_key(self, key):
+        """See `BaseRepresentationCache`."""
+        self.client.delete(key)

=== modified file 'lib/lp/services/memcache/tests/test_doc.py'
--- lib/lp/services/memcache/tests/test_doc.py	2010-03-03 11:00:42 +0000
+++ lib/lp/services/memcache/tests/test_doc.py	2010-06-15 13:39:43 +0000
@@ -7,9 +7,7 @@
 
 import os.path
 from textwrap import dedent
-import unittest
 
-from zope.component import getUtility
 import zope.pagetemplate.engine
 from zope.pagetemplate.pagetemplate import PageTemplate
 from zope.publisher.browser import TestRequest
@@ -17,9 +15,7 @@
 from canonical.launchpad.testing.systemdocs import (
     LayeredDocFileSuite, setUp, tearDown)
 from canonical.testing.layers import LaunchpadFunctionalLayer, MemcachedLayer
-from lp.services.memcache.interfaces import IMemcacheClient
 from lp.services.testing import build_test_suite
-from lp.testing import TestCase
 
 
 here = os.path.dirname(os.path.realpath(__file__))
@@ -59,11 +55,15 @@
     test.globs['MemcachedLayer'] = MemcachedLayer
 
 
+def suite_for_doctest(filename):
+    return LayeredDocFileSuite(
+        '../doc/%s' % filename,
+        setUp=memcacheSetUp, tearDown=tearDown,
+        layer=LaunchpadFunctionalLayer)
+
 special = {
-    'tales-cache.txt': LayeredDocFileSuite(
-        '../doc/tales-cache.txt',
-        setUp=memcacheSetUp, tearDown=tearDown,
-        layer=LaunchpadFunctionalLayer),
+    'tales-cache.txt': suite_for_doctest('tales-cache.txt'),
+    'restful-cache.txt': suite_for_doctest('restful-cache.txt'),
     }
 
 

=== modified file 'lib/lp/soyuz/stories/webservice/xx-builds.txt'
--- lib/lp/soyuz/stories/webservice/xx-builds.txt	2010-05-17 11:30:31 +0000
+++ lib/lp/soyuz/stories/webservice/xx-builds.txt	2010-06-15 13:39:43 +0000
@@ -93,6 +93,7 @@
 
     >>> build = getUtility(IBinaryPackageBuildSet).getByBuildID(26)
     >>> build.upload_log = build.log
+    >>> ws_uncache(build)
 
     >>> logout()
 

=== modified file 'lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt'
--- lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt	2010-02-26 14:49:00 +0000
+++ lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt	2010-06-15 13:39:43 +0000
@@ -152,6 +152,8 @@
     ...     pub.sourcepackagerelease.dscsigningkey = None
     >>> logout()
 
+    >>> ws_uncache(pub)
+
 Query the source again:
 
     >>> pubs = webservice.named_get(

=== modified file 'versions.cfg'
--- versions.cfg	2010-06-09 19:45:18 +0000
+++ versions.cfg	2010-06-15 13:39:43 +0000
@@ -28,7 +28,7 @@
 lazr.delegates = 1.1.0
 lazr.enum = 1.1.2
 lazr.lifecycle = 1.1
-lazr.restful = 0.9.26
+lazr.restful = 0.9.29
 lazr.restfulclient = 0.9.14
 lazr.smtptest = 1.1
 lazr.testing = 0.1.1