Merge lp:~gary/launchpad/bug586466 into lp:launchpad

[Gary Poster]

Provide tal memcached testing convenience

The basic change here was already approved by sinzui on IRC. However, I had to change the test for the tal feature. I'd like a re-review to make sure the change I made to underwear couldn't be done better.

[Curtis Hovey (sinzui)]

This is a nice fix. Thanks for doing this. Really!

review: Approve

=== modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
--- lib/lp/registry/browser/tests/milestone-views.txt	2010-05-27 04:10:17 +0000
+++ lib/lp/registry/browser/tests/milestone-views.txt	2010-06-09 16:13:28 +0000
@@ -306,22 +306,31 @@
 
 Active milestones cache bugtasks for 10 minutes.
 
+    >>> from canonical.testing.layers import MemcachedLayer
+    >>> MemcachedLayer.purge()
     >>> cached_milestone = factory.makeMilestone(
     ...     'cache-test', productseries=series)
     >>> cached_bugtask = factory.makeBugTask(target=cached_milestone.product)
     >>> cached_bugtask.milestone = cached_milestone
     >>> view = create_view(cached_milestone, '+index', principal=person)
-    >>> content = view.render()
-    >>> 'milestone_bugtasks cache:private 10 minute' in content
-    True
+    >>> 'Cache hit' in view.render()
+    False
+    >>> print create_view(
+    ...     cached_milestone, '+index', principal=person).render()
+    <...<!-- Cache hit: memcache expression (private, 10 minute) -->
+    <table class="listing sortable" id="milestone_bugtasks">...
 
 Inactive milestones cache bugtasks for 3 hours.
 
+    >>> MemcachedLayer.purge()
     >>> cached_milestone.active = False
     >>> view = create_view(cached_milestone, '+index', principal=person)
-    >>> content = view.render()
-    >>> 'milestone_bugtasks cache:private 3 hour' in content
-    True
+    >>> 'Cache hit' in view.render()
+    False
+    >>> print create_view(
+    ...     cached_milestone, '+index', principal=person).render()
+    <...<!-- Cache hit: memcache expression (private, 3 hour) -->
+    <table class="listing sortable" id="milestone_bugtasks">...
 
 
 ProjectGroup milestones

=== modified file 'lib/lp/registry/templates/milestone-index.pt'
--- lib/lp/registry/templates/milestone-index.pt	2010-05-27 04:10:17 +0000
+++ lib/lp/registry/templates/milestone-index.pt	2010-06-09 16:13:28 +0000
@@ -268,13 +268,11 @@
         <tal:has_bugtasks condition="bugtasks">
           <tal:active content="cache:private, 10 minute"
             condition="view/milestone/active">
-            <!-- milestone_bugtasks cache:private 10 minute -->
             <tal:milestone-bugtasks
               metal:use-macro="context/@@+milestone-macros/milestone_bugtasks" />
           </tal:active>
           <tal:inactive content="cache:private, 3 hour"
             condition="not: view/milestone/active">
-            <!-- milestone_bugtasks cache:private 3 hour -->
             <tal:milestone-bugtasks
               metal:use-macro="context/@@+milestone-macros/milestone_bugtasks" />
           </tal:inactive>

=== modified file 'lib/lp/services/memcache/doc/tales-cache.txt'
--- lib/lp/services/memcache/doc/tales-cache.txt	2010-03-31 06:39:03 +0000
+++ lib/lp/services/memcache/doc/tales-cache.txt	2010-06-09 16:13:28 +0000
@@ -26,10 +26,14 @@
 cause this template to render differently.
 
     >>> print template(param='second')
-    <div>
+    <div><!-- Cache hit: memcache expression (public) -->
         <span>first</span>
-    </div>
+    <!-- End cache hit: memcache expression (public) --></div>
 
+We also see some comments showing that we had a cache hit, and what the
+configuration was for it ("public").  These comments are only present
+when Launchpad's config is not configured as edge or production (``is_edge``
+or ``is_lpnet``, respectively).
 
 If we clear the cache, it will be rendered as expected.
 
@@ -141,7 +145,9 @@
 
     >>> login('[e-mail]')
     >>> print template(username='Foo Bar')
+    <!-- Cache hit: memcache expression (public) -->
     Public:        Anonymous
+    <!-- End cache hit: memcache expression (public) -->
     Private:       Foo Bar
     Anonymous:     Foo Bar
     Authenticated: Foo Bar
@@ -151,10 +157,14 @@
 
     >>> login('[e-mail]')
     >>> print template(username='Test')
+    <!-- Cache hit: memcache expression (public) -->
     Public:        Anonymous
+    <!-- End cache hit: memcache expression (public) -->
     Private:       Test
     Anonymous:     Test
+    <!-- Cache hit: memcache expression (authenticated) -->
     Authenticated: Foo Bar
+    <!-- End cache hit: memcache expression (authenticated) -->
 
 
 Nesting & Loops
@@ -163,6 +173,14 @@
 Cached chunks can contain other cached chunks, useful for specifying
 different timeouts of different visibilities.
 
+We will disable the cache hit comments by claiming to be in production.
+
+    >>> from canonical.config import config
+    >>> original_instance_name = config.instance_name
+    >>> config.push('is_production', """
+    ...     [launchpad]
+    ...     is_lpnet: True
+    ... """)
     >>> template = TestPageTemplate(dedent("""\
     ...     <body tal:content="cache:private,25 seconds" tal:omit-tag="">
     ...         This bit is private to <span tal:replace="username" />
@@ -291,3 +309,5 @@
     Mutated comment
     </body>
 
+    >>> ignore = config.pop('is_production')
+

=== modified file 'lib/lp/services/memcache/tales.py'
--- lib/lp/services/memcache/tales.py	2010-03-31 06:39:03 +0000
+++ lib/lp/services/memcache/tales.py	2010-06-09 16:13:28 +0000
@@ -225,7 +225,7 @@
 
         if cached_chunk is None:
             logging.debug("Memcache miss for %s", key)
-            return MemcacheMiss(key, self.max_age)
+            return MemcacheMiss(key, self)
         else:
             logging.debug("Memcache hit for %s", key)
             return MemcacheHit(cached_chunk)
@@ -244,13 +244,18 @@
     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):
+    def __init__(self, key, memcache_expr):
         self._key = key
-        self._max_age = max_age
+        self._memcache_expr = memcache_expr
 
     def __call__(self, value):
+        if not config.launchpad.is_lpnet and not config.launchpad.is_edge:
+            # For debugging and testing purposes, prepend a description of
+            # the memcache expression used to the stored value.
+            value = "<!-- Cache hit: %s -->%s<!-- End cache hit: %s -->" % (
+                self._memcache_expr, value, self._memcache_expr)
         if getUtility(IMemcacheClient).set(
-            self._key, value, self._max_age):
+            self._key, value, self._memcache_expr.max_age):
             logging.debug("Memcache set succeeded for %s", self._key)
         else:
             logging.warn("Memcache set failed for %s", self._key)