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
1=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
2--- lib/lp/registry/browser/tests/test_milestone.py 2010-06-30 19:37:09 +0000
3+++ lib/lp/registry/browser/tests/test_milestone.py 2010-08-06 20:17:49 +0000
4@@ -7,7 +7,7 @@
5
6 import unittest
7
8-from lp.testing import login_person
9+from lp.testing import ANONYMOUS, login_person, login
10 from lp.testing.views import create_initialized_view
11 from lp.testing.memcache import MemcacheTestCase
12
13@@ -26,25 +26,43 @@
14 bugtask.milestone = self.milestone
15 self.observer = self.factory.makePerson()
16
17- def test_milestone_index_memcache(self):
18+ def test_milestone_index_memcache_anonymous(self):
19 # Miss the cache on first render.
20+ login(ANONYMOUS)
21 view = create_initialized_view(
22- self.milestone, name='+index', principal=self.observer)
23+ self.milestone, name='+index', principal=None)
24 content = view.render()
25 self.assertCacheMiss('<dt>Assigned to you:</dt>', content)
26 self.assertCacheMiss('id="milestone_bugtasks"', content)
27 # Hit the cache on the second render.
28 view = create_initialized_view(
29- self.milestone, name='+index', principal=self.observer)
30+ self.milestone, name='+index', principal=None)
31 self.assertTrue(view.milestone.active)
32 self.assertEqual(10, view.expire_cache_minutes)
33 content = view.render()
34 self.assertCacheHit(
35 '<dt>Assigned to you:</dt>',
36- 'private, view/expire_cache_minutes minute', content)
37+ 'anonymous, view/expire_cache_minutes minute', content)
38 self.assertCacheHit(
39 'id="milestone_bugtasks"',
40- 'private, view/expire_cache_minutes minute', content)
41+ 'anonymous, view/expire_cache_minutes minute', content)
42+
43+ def test_milestone_index_memcache_no_cache_logged_in(self):
44+ login_person(self.observer)
45+ # Miss the cache on first render.
46+ view = create_initialized_view(
47+ self.milestone, name='+index', principal=self.observer)
48+ content = view.render()
49+ self.assertCacheMiss('<dt>Assigned to you:</dt>', content)
50+ self.assertCacheMiss('id="milestone_bugtasks"', content)
51+ # Miss the cache again on the second render.
52+ view = create_initialized_view(
53+ self.milestone, name='+index', principal=self.observer)
54+ self.assertTrue(view.milestone.active)
55+ self.assertEqual(10, view.expire_cache_minutes)
56+ content = view.render()
57+ self.assertCacheMiss('<dt>Assigned to you:</dt>', content)
58+ self.assertCacheMiss('id="milestone_bugtasks"', content)
59
60 def test_milestone_index_active_cache_time(self):
61 # Verify the active milestone cache time.
62
63=== modified file 'lib/lp/registry/templates/milestone-index.pt'
64--- lib/lp/registry/templates/milestone-index.pt 2010-06-29 14:52:51 +0000
65+++ lib/lp/registry/templates/milestone-index.pt 2010-08-06 20:17:49 +0000
66@@ -129,7 +129,7 @@
67
68 <div class="yui-u">
69 <div id="milestone-activities" class="portlet"
70- tal:content="cache:private, view/expire_cache_minutes minute">
71+ tal:content="cache:anonymous, view/expire_cache_minutes minute">
72 <h2>Activities</h2>
73
74 <dl>
75@@ -208,7 +208,7 @@
76 </h2>
77
78 <tal:has_specs condition="specs"
79- tal:content="cache:public, view/expire_cache_minutes minute">
80+ tal:content="cache:anonymous, view/expire_cache_minutes minute">
81 <table class="listing sortable" id="milestone_specs"
82 style="margin-bottom: 2em;">
83 <thead>
84@@ -266,7 +266,7 @@
85 </tal:has_specs>
86
87 <tal:has_bugtasks condition="bugtasks">
88- <tal:cache content="cache:private, view/expire_cache_minutes minute">
89+ <tal:cache content="cache:anonymous, view/expire_cache_minutes minute">
90 <tal:milestone-bugtasks
91 metal:use-macro="context/@@+milestone-macros/milestone_bugtasks" />
92 </tal:cache>