Launchpad itself

Code review comment for lp:~allenap/launchpad/bugwatch-bugtasks-as-a-list

  1. bugwatch-bugtasks-as-a-list
  2. Merge into devel
Revision history for this message
Gavin Panella (allenap) wrote :

> Review: Approve release-critical
> Looks good, just use the shortlist() instead of list() to prevent abuse and DoS vector.

Thanks Danilo. Diff attached.

post-danilo.diff
1=== modified file 'lib/lp/bugs/model/bugwatch.py'
2--- lib/lp/bugs/model/bugwatch.py 2009-12-15 13:47:52 +0000
3+++ lib/lp/bugs/model/bugwatch.py 2009-12-15 17:28:51 +0000
4@@ -29,6 +29,7 @@
5 from canonical.database.enumcol import EnumCol
6 from canonical.database.sqlbase import SQLBase
7 from canonical.launchpad.database.message import Message
8+from canonical.launchpad.helpers import shortlist
9 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
10 from canonical.launchpad.validators.email import valid_email
11 from canonical.launchpad.webapp import urlappend, urlsplit
12@@ -80,7 +81,8 @@
13 @property
14 def bugtasks(self):
15 tasks = Store.of(self).find(BugTask, BugTask.bugwatch == self.id)
16- return list(tasks.order_by(Desc(BugTask.datecreated)))
17+ tasks = tasks.order_by(Desc(BugTask.datecreated))
18+ return shortlist(tasks, 10, 100)
19
20 @property
21 def title(self):
22
23=== modified file 'lib/lp/bugs/tests/test_bugwatch.py'
24--- lib/lp/bugs/tests/test_bugwatch.py 2009-06-25 00:40:31 +0000
25+++ lib/lp/bugs/tests/test_bugwatch.py 2009-12-15 17:28:51 +0000
26@@ -11,13 +11,15 @@
27
28 from zope.component import getUtility
29
30-from canonical.launchpad.ftests import login, ANONYMOUS
31+from canonical.launchpad.webapp import urlsplit
32+from canonical.testing import (
33+ DatabaseFunctionalLayer, LaunchpadFunctionalLayer)
34+
35 from lp.bugs.interfaces.bugtracker import BugTrackerType, IBugTrackerSet
36 from lp.bugs.interfaces.bugwatch import (
37 IBugWatchSet, NoBugTrackerFound, UnrecognizedBugTrackerURL)
38 from lp.registry.interfaces.person import IPersonSet
39-from canonical.launchpad.webapp import urlsplit
40-from canonical.testing import LaunchpadFunctionalLayer
41+from lp.testing import ANONYMOUS, TestCaseWithFactory, login
42
43
44 class ExtractBugTrackerAndBugTestBase(unittest.TestCase):
45@@ -309,6 +311,20 @@
46 bug_id = '12345'
47
48
49+class TestBugWatchBugTasks(TestCaseWithFactory):
50+
51+ layer = DatabaseFunctionalLayer
52+
53+ def setUp(self):
54+ super(TestBugWatchBugTasks, self).setUp('test@canonical.com')
55+ self.bug_watch = self.factory.makeBugWatch()
56+
57+ def test_bugtasks(self):
58+ # BugWatch.bugtasks is always a list.
59+ self.assertIsInstance(
60+ self.bug_watch.bugtasks, list)
61+
62+
63 def test_suite():
64 suite = unittest.TestSuite()
65 suite.addTest(unittest.makeSuite(BugzillaExtractBugTrackerAndBugTest))
66@@ -331,6 +347,7 @@
67 PHPProjectBugTrackerExtractBugTrackerAndBugTest))
68 suite.addTest(unittest.makeSuite(
69 GoogleCodeBugTrackerExtractBugTrackerAndBugTest))
70+ suite.addTest(unittest.makeSuite(TestBugWatchBugTasks))
71 return suite
72
73

« Back to merge proposal