launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01454
[Merge] lp:~stub/launchpad/memcache into lp:launchpad/devel
Stuart Bishop has proposed merging lp:~stub/launchpad/memcache into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#586461 tal cache: directive could support an age variable
https://bugs.launchpad.net/bugs/586461
#634326 memcache cache keys interact poorly with query parameters
https://bugs.launchpad.net/bugs/634326
#634646 MemcachedKeyCharacterError: Control characters not allowed
https://bugs.launchpad.net/bugs/634646
Add a 'noparam' option to allow cache to be shared when URLs differ only by the query string, resolving Bug #634326.
Also fix a bug where invalid memcache keys could be generated when the loop key contained a long string, per Bug #634646.
Some minor delinting.
--
https://code.launchpad.net/~stub/launchpad/memcache/+merge/37963
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/memcache into lp:launchpad/devel.
=== modified file 'lib/lp/services/memcache/doc/tales-cache.txt'
--- lib/lp/services/memcache/doc/tales-cache.txt 2010-09-12 05:10:19 +0000
+++ lib/lp/services/memcache/doc/tales-cache.txt 2010-10-08 12:48:49 +0000
@@ -199,6 +199,42 @@
<!-- End cache hit: memcache expression (authenticated) [0 seconds] -->
+Visibility Modifiers
+--------------------
+
+A optional modifier can also be applied to the visibility. Only 'param'
+and 'noparam' are supported. 'param' is the default, and pages that
+differ by just the query string do not share cache.
+
+ >>> param_template = TestPageTemplate(dedent("""\
+ ... <div tal:omit-tag=""><tal:cache content="cache:public param">
+ ... Arg: <tal:x content="arg" />
+ ... </tal:cache></div>"""))
+ >>> login(ANONYMOUS)
+ >>> request1 = LaunchpadTestRequest(QUERY_STRING='a=1')
+ >>> request2 = LaunchpadTestRequest(QUERY_STRING='a=2')
+ >>> print param_template(arg="First", request=request1)
+ Arg: First
+ >>> print param_template(arg="Second", request=request2)
+ Arg: Second
+
+If 'noparam' is used, query parameters are ignored and the cache is shared.
+
+ >>> noparam_template = TestPageTemplate(dedent("""\
+ ... <div tal:omit-tag=""><tal:cache content="cache:public noparam">
+ ... Arg: <tal:x content="arg" />
+ ... </tal:cache></div>"""))
+ >>> login(ANONYMOUS)
+ >>> request3 = LaunchpadTestRequest(QUERY_STRING='a=3')
+ >>> request4 = LaunchpadTestRequest(QUERY_STRING='a=4')
+ >>> print noparam_template(arg="Third", request=request3)
+ Arg: Third
+ >>> print noparam_template(arg="Fourth", request=request4)
+ <!-- Cache hit: memcache expression (public noparam) [0 seconds] -->
+ Arg: Third
+ <!-- End cache hit: memcache expression (public noparam) [0 seconds] -->
+
+
Nesting & Loops
---------------
@@ -300,21 +336,21 @@
... <body>
... <div tal:omit-tag="" tal:repeat="comment comments">
... <div tal:replace="cache:
- ... public,1 hour,comment/comment_num">
+ ... public,1 hour,comment/comment_id">
... <span tal:replace="comment/comment" />
... </div>
... </div>
... </body>"""))
>>> comments = [
- ... {'comment_num': 42, 'comment': "Today's comment"},
- ... {'comment_num': 12, 'comment': "Yesterday's comment"}]
+ ... {'comment_id': 42, 'comment': "Today's comment"},
+ ... {'comment_id': 'long!'*40, 'comment': "Yesterday's comment"}]
>>> print template(comments=comments)
<body>
Today's comment
Yesterday's comment
</body>
- >>> comments.insert(0, {'comment_num': 'foo', 'comment': 'New comment'})
+ >>> comments.insert(0, {'comment_id': 'foo', 'comment': 'New comment'})
>>> print template(comments=comments)
<body>
New comment
@@ -349,7 +385,6 @@
Memcache in templates can be disabled entirely by setting the memcache flag to
'disabled'.
- >>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
>>> from lp.services.features.model import FeatureFlag, getFeatureStore
>>> from lp.services.features.webapp import ScopesFromRequest
>>> from lp.services.features.flags import FeatureController
=== modified file 'lib/lp/services/memcache/tales.py'
--- lib/lp/services/memcache/tales.py 2010-09-12 05:52:41 +0000
+++ lib/lp/services/memcache/tales.py 2010-10-08 12:48:49 +0000
@@ -103,6 +103,18 @@
else:
raise SyntaxError("Too many arguments in cache: expression")
+ try:
+ self.visibility, modifier = self.visibility.split()
+ if modifier == 'param':
+ self.include_params = True
+ elif modifier == 'noparam':
+ self.include_params = False
+ else:
+ raise SyntaxError(
+ 'visibility modifier must be param or noparam')
+ except ValueError:
+ self.include_params = True
+
if self.visibility not in (
'anonymous', 'public', 'private', 'authenticated'):
raise SyntaxError(
@@ -170,7 +182,9 @@
# We use a sanitized version in the human readable chunk of
# the key.
request = econtext.getValue('request')
- url = str(request.URL) + '?' + str(request.get('QUERY_STRING', ''))
+ url = str(request.URL)
+ if self.include_params:
+ url += '?' + str(request.get('QUERY_STRING', ''))
url = url.encode('utf8') # Ensure it is a byte string.
sanitized_url = url.translate(self._key_translate_map)
@@ -198,9 +212,9 @@
# The extra_key is used to differentiate items inside loops.
if self.extra_key is not None:
- # Encode it to base64 to make it memcached key safe.
- extra_key = unicode(
- self.extra_key(econtext)).encode('base64').rstrip()
+ # Encode it to to a memcached key safe string. base64
+ # isn't suitable for this because it can contain whitespace.
+ extra_key = unicode(self.extra_key(econtext)).encode('hex')
else:
# If no extra_key was specified, we include a counter in the
# key that is reset at the start of the request. This
@@ -268,6 +282,7 @@
tag contents and invokes this callback, which will store the
result in memcache against the key calculated by the MemcacheExpr.
"""
+
def __init__(self, key, max_age, memcache_expr):
self._key = key
self._max_age = max_age
@@ -292,6 +307,7 @@
We use a special object so the TALInterpreter knows that this
information should not be quoted.
"""
+
def __init__(self, value):
self.value = value
@@ -335,9 +351,13 @@
TALInterpreter.bytecode_handlers_tal["insertText"] = do_insertText_tal
-# Just like the original, except MemcacheHit and MemcacheMiss
-# instances are also passed through unharmed.
def evaluateText(self, expr):
+ """Replacement for zope.pagetemplate.engine.ZopeContextBase.evaluateText.
+
+ Just like the original, except MemcacheHit and MemcacheMiss
+ instances are also passed through unharmed.
+ """
+
text = self.evaluate(expr)
if (text is None
or isinstance(text, (basestring, MemcacheHit, MemcacheMiss))
@@ -346,4 +366,3 @@
return unicode(text)
import zope.pagetemplate.engine
zope.pagetemplate.engine.ZopeContextBase.evaluateText = evaluateText
-
=== modified file 'lib/lp/services/memcache/tests/test_doc.py'
--- lib/lp/services/memcache/tests/test_doc.py 2010-08-20 20:31:18 +0000
+++ lib/lp/services/memcache/tests/test_doc.py 2010-10-08 12:48:49 +0000
@@ -17,6 +17,7 @@
setUp,
tearDown,
)
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
from canonical.testing.layers import (
LaunchpadFunctionalLayer,
MemcachedLayer,
@@ -49,8 +50,8 @@
def pt_getContext(self, args=(), options={}):
# Build a minimal context. The cache: expression requires
# a request.
- context = {'request': TestRequest()}
- context.update(options)
+ context = dict(options)
+ context.setdefault('request', TestRequest())
return context
@@ -59,6 +60,7 @@
test.globs['TestPageTemplate'] = TestPageTemplate
test.globs['dedent'] = dedent
test.globs['MemcachedLayer'] = MemcachedLayer
+ test.globs['LaunchpadTestRequest'] = LaunchpadTestRequest
def suite_for_doctest(filename):
