Merge lp:~sinzui/launchpad/registry-cache-3 into lp:launchpad

[Curtis Hovey]

This is my branch to fix the +packaging and +needs-packaging cache rule.

    lp:~sinzui/launchpad/registry-cache-3
    Diff size: 284
    Launchpad bug:
          https://bugs.launchpad.net/bugs/599818
    Test command: ./bin/test -vv \
          -t packaging-views \
          -t xx-sourcepackage-packaging -t xx-show-distroseries-packaging
    Pre-implementation: no one.
    Target release: 10.06

Fix the +packaging and +needs-packaging cache rule
--------------------------------------------------

The pages still timeout on edge and staging even though the caching was added.
The cache rule was applied to the repeat statement, which means the view
always calls the expensive query, then uses the cache for the item.

Rules
-----

    * Move the cache rule before the call to `batch` to ensure the entire
      listing is cached. The batch will be called again when the cache
      expires.
    * ADDENDUM: Add a memcache test helper so that it is easy to verify
      that a fragment is in the cache chunk.

QA
--

    * Visit https://edge.launchpad.net/ubuntu/maverick/+packaging
    * Load the page (the test does not start until it is fully loaded)
    * Reload the page 10 times and verify there are no timeouts.
    * Visit https://edge.launchpad.net/ubuntu/maverick/+needs-packaging
    * Load the page (the test does not start until it is fully loaded)
    * Reload the page 10 times and verify there are no timeouts.

Lint
----

Linting changed files:
  lib/lp/registry/browser/tests/milestone-views.txt
  lib/lp/registry/browser/tests/test_milestone.py
  lib/lp/registry/browser/tests/test_packaging.py
  lib/lp/registry/templates/distroseries-needs-packaging.pt
  lib/lp/registry/templates/distroseries-packaging.pt
  lib/lp/testing/memcache.py

Test
----

    * lib/lp/testing/memcache.py
      * Added a test fixture for memcache testing.
    * lib/lp/registry/browser/tests/test_packaging.py
      * Added tests to verify the whole table is in the cache chunk for
        +packaging and +needs-packaging.
    * lib/lp/registry/browser/tests/milestone-views.txt
      * Moved cache tests to test_milestone.py
    * lib/lp/registry/browser/tests/test_milestone.py
      * Updated tests to use the new memcache unit test fixture.

Implementation
--------------

    * lib/lp/registry/templates/distroseries-needs-packaging.pt
      * Move cache rule to the whole table to ensure batch is not called
        if there is a cache hit.
    * lib/lp/registry/templates/distroseries-packaging.pt
      * Move cache rule to the whole table to ensure batch is not called
        if there is a cache hit.

[Edwin Grubbs (edwin-grubbs)]

Hi Curtis,

This branch looks good.

-Edwin

review: Approve

=== modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
--- lib/lp/registry/browser/tests/milestone-views.txt	2010-06-29 14:52:51 +0000
+++ lib/lp/registry/browser/tests/milestone-views.txt	2010-06-30 20:05:55 +0000
@@ -248,7 +248,7 @@
 the product release data for a milestone.
 
     >>> from canonical.launchpad.testing.pages import (
-    ...     extract_text, find_main_content, find_tag_by_id)
+    ...     extract_text, find_tag_by_id)
 
     >>> view = create_view(
     ...     milestone, '+productrelease-data', principal=person)
@@ -301,55 +301,6 @@
     >>> login_person(person)
 
 
-Milestone caching
------------------
-
-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)
-    >>> view.expire_cache_minutes
-    10
-
-    >>> 'Cache hit' in view.render()
-    False
-
-    >>> content = find_main_content(create_view(
-    ...     cached_milestone, '+index', principal=person).render())
-    >>> print find_tag_by_id(content, 'milestone-activities')
-    <div id="milestone-activities" ...<!--
-      Cache hit: memcache expression
-      (private, view/expire_cache_minutes minute) [600 seconds] ...
-    <dt>Assigned to you:</dt> ... No blueprints or bugs assigned to you. ...
-
-    >>> print find_tag_by_id(content, 'milestone-specs-bugs')
-    <div id="milestone-specs-bugs" ...
-    <!-- Cache hit: memcache expression
-      (private, view/expire_cache_minutes minute) [600 seconds] -->
-    <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)
-    >>> view.expire_cache_minutes
-    360
-    >>> 'Cache hit' in view.render()
-    False
-    >>> print create_view(
-    ...     cached_milestone, '+index', principal=person).render()
-    <...<!-- Cache hit: memcache expression
-      (private, view/expire_cache_minutes minute) [21600 seconds] -->
-    <table class="listing sortable" id="milestone_bugtasks">...
-
-
 ProjectGroup milestones
 -----------------------
 

=== added file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py	2010-06-30 20:05:55 +0000
@@ -0,0 +1,64 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test milestone views."""
+
+__metaclass__ = type
+
+import unittest
+
+from lp.testing import login_person
+from lp.testing.views import create_initialized_view
+from lp.testing.memcache import MemcacheTestCase
+
+
+class TestMilestoneMemcache(MemcacheTestCase):
+
+    def setUp(self):
+        super(TestMilestoneMemcache, self).setUp()
+        product = self.factory.makeProduct()
+        login_person(product.owner)
+        series = self.factory.makeProductSeries(product=product)
+        self.milestone = self.factory.makeMilestone(
+            productseries=series, name="1.1")
+        bugtask = self.factory.makeBugTask(target=product)
+        bugtask.transitionToAssignee(product.owner)
+        bugtask.milestone = self.milestone
+        self.observer = self.factory.makePerson()
+
+    def test_milestone_index_memcache(self):
+        # Miss the cache on first render.
+        view = create_initialized_view(
+            self.milestone, name='+index', principal=self.observer)
+        content = view.render()
+        self.assertCacheMiss('<dt>Assigned to you:</dt>', content)
+        self.assertCacheMiss('id="milestone_bugtasks"', content)
+        # Hit the cache on the second render.
+        view = create_initialized_view(
+            self.milestone, name='+index', principal=self.observer)
+        self.assertTrue(view.milestone.active)
+        self.assertEqual(10, view.expire_cache_minutes)
+        content = view.render()
+        self.assertCacheHit(
+            '<dt>Assigned to you:</dt>',
+            'private, view/expire_cache_minutes minute', content)
+        self.assertCacheHit(
+            'id="milestone_bugtasks"',
+            'private, view/expire_cache_minutes minute', content)
+
+    def test_milestone_index_active_cache_time(self):
+        # Verify the active milestone cache time.
+        view = create_initialized_view(self.milestone, name='+index')
+        self.assertTrue(view.milestone.active)
+        self.assertEqual(10, view.expire_cache_minutes)
+
+    def test_milestone_index_inactive_cache_time(self):
+        # Verify the inactive milestone cache time.
+        self.milestone.active = False
+        view = create_initialized_view(self.milestone, name='+index')
+        self.assertFalse(view.milestone.active)
+        self.assertEqual(360, view.expire_cache_minutes)
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/registry/browser/tests/test_packaging.py'
--- lib/lp/registry/browser/tests/test_packaging.py	2010-04-16 19:28:24 +0000
+++ lib/lp/registry/browser/tests/test_packaging.py	2010-06-30 20:05:55 +0000
@@ -15,6 +15,7 @@
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.testing import TestCaseWithFactory
 from lp.testing.views import create_initialized_view
+from lp.testing.memcache import MemcacheTestCase
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.ftests import login, logout
 from canonical.launchpad.testing.pages import setupBrowser
@@ -165,5 +166,39 @@
             distroseries=distroseries))
 
 
+class TestDistroseriesPackagingMemcache(MemcacheTestCase):
+    """Tests distroseries packaging cache rules."""
+
+    def setUp(self):
+        super(TestDistroseriesPackagingMemcache, self).setUp()
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        self.hoary = ubuntu.getSeries('hoary')
+        self.observer = self.factory.makePerson()
+
+    def test_needs_packaging_memcache(self):
+        # Verify that the packages table is cached.
+        # Miss the cache on first render.
+        view = create_initialized_view(
+            self.hoary, name='+needs-packaging', principal=self.observer)
+        self.assertCacheMiss('<table id="packages"', view.render())
+        # Hit the cache on the second render.
+        view = create_initialized_view(
+            self.hoary, name='+needs-packaging', principal=self.observer)
+        self.assertCacheHit(
+            '<table id="packages"', 'public, 30 minute', view.render())
+
+    def test_packaging_memcache(self):
+        # Verify that the packagings table is cached.
+        # Miss the cache on first render.
+        view = create_initialized_view(
+            self.hoary, name='+packaging', principal=self.observer)
+        self.assertCacheMiss('<table id="packagings"', view.render())
+        # Hit the cache on the second render.
+        view = create_initialized_view(
+            self.hoary, name='+packaging', principal=self.observer)
+        self.assertCacheHit(
+            '<table id="packagings"', 'public, 30 minute', view.render())
+
+
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/registry/templates/distroseries-needs-packaging.pt'
--- lib/lp/registry/templates/distroseries-needs-packaging.pt	2010-06-29 15:03:03 +0000
+++ lib/lp/registry/templates/distroseries-needs-packaging.pt	2010-06-30 20:05:55 +0000
@@ -31,6 +31,7 @@
         </ul>
       </div>
 
+      <tal:cache content="cache:public, 30 minute">
       <div class="narrow"
         tal:define="summaries view/cached_unlinked_packages;"
         tal:condition="summaries/batch">
@@ -47,8 +48,7 @@
           </thead>
 
           <tbody>
-            <tr tal:repeat="summary summaries/batch"
-              tal:content="cache:public, 30 minute">
+            <tr tal:repeat="summary summaries/batch">
               <td>
                 <a class="sprite package-source"
                   tal:attributes="href summary/package/fmt:url"
@@ -87,6 +87,7 @@
         <div class="results"
           tal:content="structure summaries/@@+navigation-links-lower" />
       </div>
+      </tal:cache>
     </div>
   </body>
 </html>

=== modified file 'lib/lp/registry/templates/distroseries-packaging.pt'
--- lib/lp/registry/templates/distroseries-packaging.pt	2010-06-29 15:03:03 +0000
+++ lib/lp/registry/templates/distroseries-packaging.pt	2010-06-30 20:05:55 +0000
@@ -31,8 +31,8 @@
         </ul>
       </div>
 
-      <div tal:condition="packagings/batch"
-        tal:content="cache:public, 30 minute">
+      <tal:cache content="cache:public, 30 minute">
+      <div tal:condition="packagings/batch">
         <h2>All upstream links</h2>
 
         <div class="results"
@@ -82,6 +82,7 @@
         <div class="results"
           tal:content="structure packagings/@@+navigation-links-lower" />
       </div>
+      </tal:cache>
     </div>
   </body>
 </html>

=== added file 'lib/lp/testing/memcache.py'
--- lib/lp/testing/memcache.py	1970-01-01 00:00:00 +0000
+++ lib/lp/testing/memcache.py	2010-06-30 20:05:55 +0000
@@ -0,0 +1,36 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from canonical.testing import LaunchpadFunctionalLayer
+
+from lp.testing import TestCaseWithFactory
+
+
+class MemcacheTestCase(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def assertCacheMiss(self, fragment, content):
+        # Verify that fragment is not cached in the content.
+        self.assertTrue(fragment in content)
+        before, after = content.split(fragment, 1)
+        cache_start = '<!-- Cache hit: memcache expression'
+        if cache_start in before:
+            # Verify that the preceding cache is not for this fragment
+            cache_end = '<!-- End cache hit'
+            self.assertTrue(cache_end in before)
+            ignore, start = before.rsplit(cache_end, 1)
+            self.assertTrue(cache_start not in start)
+
+    def assertCacheHit(self, fragment, expression, content):
+        # Verify that fragment is cached by the specific expression in
+        # the content.
+        self.assertTrue(fragment in content)
+        before, after = content.split(fragment, 1)
+        cache_start = (
+            '<!-- Cache hit: memcache expression (%s) ' % expression)
+        self.assertTrue(cache_start in before)
+        ignore, start = before.rsplit(cache_start, 1)
+        self.assertTrue('<!-- End cache hit' not in start)