Launchpad itself

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

  1. registry-cache-4
  2. Merge into devel
Proposed by Curtis Hovey
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 11314
Proposed branch: lp:~sinzui/launchpad/registry-cache-4
Merge into: lp:launchpad
Diff against target: 92 lines (+27/-9)
2 files modified
lib/lp/registry/browser/tests/test_milestone.py (+24/-6)
lib/lp/registry/templates/milestone-index.pt (+3/-3)
To merge this branch: bzr merge lp:~sinzui/launchpad/registry-cache-4
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+31998@code.launchpad.net

Description of the change

This is my branch to remove caching from milestone pages.

    lp:~sinzui/launchpad/registry-cache-3
    Diff size: 93
    Launchpad bug:
          https://bugs.launchpad.net/bugs/id
    Test command: ./bin/test -vv -t test_milestone
    Pre-implementation: bjorn, jelmer, Danilos
    Target release: 10.08

Remove caching from milestone pages
-----------------------------------

On Fri, 2010-08-06 at 10:07 +0300, Bjorn Tillenius wrote:
> I'd be interested in knowing this as well. I've always wondered
> whether
> the caching on the milestone list actually helps, and how it helps.
> Considering it's cache:private, it means that each user has its own
> cache, right? So it doesn't help when multiple people look at the page
> the first time. It only helps when the same person reloads the page
> multiple times. Now, which use cases exist for that, except for
> checking
> if something changed, usually after having modified bugs himself, or
> someone on IRC told him that they changed bugs? For the use case when
> the user knows something has changed, he will try to reload until he
> actually sees the changes, and is a really bad user experience.

This is a well thought argument. I think anonymous users are the only
ones who benefit from the cache. The page spends most of its time in
python formatting objects and anonymous users/bots do benefit from a
fast loading page.

Rules
-----

Change the cache rules in the milestone template to anonymous. Only anonymous
users see a cached page.

QA
--

Using two browsers, one logged in, the other not.

    * View a milestone in both browsers.
    * Visit a bug listed in the page in the logged browser
    * Change the status or assignee of the bug in the logged browser
    * Visit the milestone gain in the logged browser
    * Verify you see the change.
    * Reload the anonymous browser
    * Verify it has not changed, it does not show the bug has changed.

Lint
----

Linting changed files:
  lib/lp/registry/browser/tests/test_milestone.py
  lib/lp/registry/templates/milestone-index.pt

Test
----

    * lib/lp/registry/browser/tests/test_milestone.py
      * Renamed an existing test to show it tests the anonymous condition
      * Added a new test for the logged in condition

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

    * lib/lp/registry/templates/milestone-index.pt
      * Changed the cache instructions to anonymous.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py 2010-06-30 19:37:09 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py 2010-08-06 20:17:49 +0000
@@ -7,7 +7,7 @@
77
8import unittest8import unittest
99
10from lp.testing import login_person10from lp.testing import ANONYMOUS, login_person, login
11from lp.testing.views import create_initialized_view11from lp.testing.views import create_initialized_view
12from lp.testing.memcache import MemcacheTestCase12from lp.testing.memcache import MemcacheTestCase
1313
@@ -26,25 +26,43 @@
26 bugtask.milestone = self.milestone26 bugtask.milestone = self.milestone
27 self.observer = self.factory.makePerson()27 self.observer = self.factory.makePerson()
2828
29 def test_milestone_index_memcache(self):29 def test_milestone_index_memcache_anonymous(self):
30 # Miss the cache on first render.30 # Miss the cache on first render.
31 login(ANONYMOUS)
31 view = create_initialized_view(32 view = create_initialized_view(
32 self.milestone, name='+index', principal=self.observer)33 self.milestone, name='+index', principal=None)
33 content = view.render()34 content = view.render()
34 self.assertCacheMiss('<dt>Assigned to you:</dt>', content)35 self.assertCacheMiss('<dt>Assigned to you:</dt>', content)
35 self.assertCacheMiss('id="milestone_bugtasks"', content)36 self.assertCacheMiss('id="milestone_bugtasks"', content)
36 # Hit the cache on the second render.37 # Hit the cache on the second render.
37 view = create_initialized_view(38 view = create_initialized_view(
38 self.milestone, name='+index', principal=self.observer)39 self.milestone, name='+index', principal=None)
39 self.assertTrue(view.milestone.active)40 self.assertTrue(view.milestone.active)
40 self.assertEqual(10, view.expire_cache_minutes)41 self.assertEqual(10, view.expire_cache_minutes)
41 content = view.render()42 content = view.render()
42 self.assertCacheHit(43 self.assertCacheHit(
43 '<dt>Assigned to you:</dt>',44 '<dt>Assigned to you:</dt>',
44 'private, view/expire_cache_minutes minute', content)45 'anonymous, view/expire_cache_minutes minute', content)
45 self.assertCacheHit(46 self.assertCacheHit(
46 'id="milestone_bugtasks"',47 'id="milestone_bugtasks"',
47 'private, view/expire_cache_minutes minute', content)48 'anonymous, view/expire_cache_minutes minute', content)
49
50 def test_milestone_index_memcache_no_cache_logged_in(self):
51 login_person(self.observer)
52 # Miss the cache on first render.
53 view = create_initialized_view(
54 self.milestone, name='+index', principal=self.observer)
55 content = view.render()
56 self.assertCacheMiss('<dt>Assigned to you:</dt>', content)
57 self.assertCacheMiss('id="milestone_bugtasks"', content)
58 # Miss the cache again on the second render.
59 view = create_initialized_view(
60 self.milestone, name='+index', principal=self.observer)
61 self.assertTrue(view.milestone.active)
62 self.assertEqual(10, view.expire_cache_minutes)
63 content = view.render()
64 self.assertCacheMiss('<dt>Assigned to you:</dt>', content)
65 self.assertCacheMiss('id="milestone_bugtasks"', content)
4866
49 def test_milestone_index_active_cache_time(self):67 def test_milestone_index_active_cache_time(self):
50 # Verify the active milestone cache time.68 # Verify the active milestone cache time.
5169
=== modified file 'lib/lp/registry/templates/milestone-index.pt'
--- lib/lp/registry/templates/milestone-index.pt 2010-06-29 14:52:51 +0000
+++ lib/lp/registry/templates/milestone-index.pt 2010-08-06 20:17:49 +0000
@@ -129,7 +129,7 @@
129129
130 <div class="yui-u">130 <div class="yui-u">
131 <div id="milestone-activities" class="portlet"131 <div id="milestone-activities" class="portlet"
132 tal:content="cache:private, view/expire_cache_minutes minute">132 tal:content="cache:anonymous, view/expire_cache_minutes minute">
133 <h2>Activities</h2>133 <h2>Activities</h2>
134134
135 <dl>135 <dl>
@@ -208,7 +208,7 @@
208 </h2>208 </h2>
209209
210 <tal:has_specs condition="specs"210 <tal:has_specs condition="specs"
211 tal:content="cache:public, view/expire_cache_minutes minute">211 tal:content="cache:anonymous, view/expire_cache_minutes minute">
212 <table class="listing sortable" id="milestone_specs"212 <table class="listing sortable" id="milestone_specs"
213 style="margin-bottom: 2em;">213 style="margin-bottom: 2em;">
214 <thead>214 <thead>
@@ -266,7 +266,7 @@
266 </tal:has_specs>266 </tal:has_specs>
267267
268 <tal:has_bugtasks condition="bugtasks">268 <tal:has_bugtasks condition="bugtasks">
269 <tal:cache content="cache:private, view/expire_cache_minutes minute">269 <tal:cache content="cache:anonymous, view/expire_cache_minutes minute">
270 <tal:milestone-bugtasks270 <tal:milestone-bugtasks
271 metal:use-macro="context/@@+milestone-macros/milestone_bugtasks" />271 metal:use-macro="context/@@+milestone-macros/milestone_bugtasks" />
272 </tal:cache>272 </tal:cache>