Merge lp:~adeuring/launchpad/bug-675595 into lp:launchpad
- bug-675595
- Merge into devel
Proposed by
Abel Deuring
Status: | Merged |
---|---|
Approved by: | Graham Binns |
Approved revision: | no longer in the source branch. |
Merged at revision: | 11973 |
Proposed branch: | lp:~adeuring/launchpad/bug-675595 |
Merge into: | lp:launchpad |
Diff against target: |
434 lines (+183/-22) 7 files modified
lib/lp/bugs/interfaces/bugtarget.py (+6/-3) lib/lp/bugs/interfaces/bugtask.py (+11/-7) lib/lp/bugs/model/bugtarget.py (+4/-4) lib/lp/bugs/model/bugtask.py (+10/-2) lib/lp/bugs/tests/test_bugtarget.py (+108/-1) lib/lp/bugs/tests/test_bugtask_search.py (+37/-2) lib/lp/registry/model/person.py (+7/-3) |
To merge this branch: | bzr merge lp:~adeuring/launchpad/bug-675595 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Graham Binns (community) | code | Approve | |
Review via email: mp+41595@code.launchpad.net |
Commit message
Description of the change
This branch is a first step to fix bug 675595:
BugTaskSet.
directly and retrieve the milestones with one SQL query instead of two.
It adds an optional parameter "prejoins" to BugTaskSet.search()
and to HasBugs.
BugTaskSet.search() already prejoined a number of tables in order to
reduce the total query count; in certain cases it makes sense to
prejoin other tables too.
tests:
./bin/test -vvt test_bugtarget -t test_bugtask_search
no lint
To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) : | # |
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/bugs/interfaces/bugtarget.py' | |||
2 | --- lib/lp/bugs/interfaces/bugtarget.py 2010-10-21 16:40:10 +0000 | |||
3 | +++ lib/lp/bugs/interfaces/bugtarget.py 2010-11-23 13:40:38 +0000 | |||
4 | @@ -82,7 +82,8 @@ | |||
5 | 82 | "omit_duplicates": copy_field(IBugTaskSearch['omit_dupes']), | 82 | "omit_duplicates": copy_field(IBugTaskSearch['omit_dupes']), |
6 | 83 | "omit_targeted": copy_field(IBugTaskSearch['omit_targeted']), | 83 | "omit_targeted": copy_field(IBugTaskSearch['omit_targeted']), |
7 | 84 | "status_upstream": copy_field(IBugTaskSearch['status_upstream']), | 84 | "status_upstream": copy_field(IBugTaskSearch['status_upstream']), |
9 | 85 | "milestone_assignment": copy_field(IBugTaskSearch['milestone_assignment']), | 85 | "milestone_assignment": copy_field( |
10 | 86 | IBugTaskSearch['milestone_assignment']), | ||
11 | 86 | "milestone": copy_field(IBugTaskSearch['milestone']), | 87 | "milestone": copy_field(IBugTaskSearch['milestone']), |
12 | 87 | "component": copy_field(IBugTaskSearch['component']), | 88 | "component": copy_field(IBugTaskSearch['component']), |
13 | 88 | "nominated_for": Reference(schema=Interface), | 89 | "nominated_for": Reference(schema=Interface), |
14 | @@ -232,7 +233,7 @@ | |||
15 | 232 | hardware_owner_is_subscribed_to_bug=False, | 233 | hardware_owner_is_subscribed_to_bug=False, |
16 | 233 | hardware_is_linked_to_bug=False, linked_branches=None, | 234 | hardware_is_linked_to_bug=False, linked_branches=None, |
17 | 234 | structural_subscriber=None, modified_since=None, | 235 | structural_subscriber=None, modified_since=None, |
19 | 235 | created_since=None): | 236 | created_since=None, prejoins=[]): |
20 | 236 | """Search the IBugTasks reported on this entity. | 237 | """Search the IBugTasks reported on this entity. |
21 | 237 | 238 | ||
22 | 238 | :search_params: a BugTaskSearchParams object | 239 | :search_params: a BugTaskSearchParams object |
23 | @@ -337,8 +338,10 @@ | |||
24 | 337 | :istargeted: Is there a fix targeted to this series? | 338 | :istargeted: Is there a fix targeted to this series? |
25 | 338 | :sourcepackage: The sourcepackage to which the fix would be targeted. | 339 | :sourcepackage: The sourcepackage to which the fix would be targeted. |
26 | 339 | :assignee: An IPerson, or None if no assignee. | 340 | :assignee: An IPerson, or None if no assignee. |
28 | 340 | :status: A BugTaskStatus dbschema item, or None, if series is not targeted. | 341 | :status: A BugTaskStatus dbschema item, or None, if series is not |
29 | 342 | targeted. | ||
30 | 341 | """ | 343 | """ |
31 | 344 | |||
32 | 342 | def __init__(self, series, istargeted=False, sourcepackage=None, | 345 | def __init__(self, series, istargeted=False, sourcepackage=None, |
33 | 343 | assignee=None, status=None): | 346 | assignee=None, status=None): |
34 | 344 | self.series = series | 347 | self.series = series |
35 | 345 | 348 | ||
36 | === modified file 'lib/lp/bugs/interfaces/bugtask.py' | |||
37 | --- lib/lp/bugs/interfaces/bugtask.py 2010-11-03 16:50:05 +0000 | |||
38 | +++ lib/lp/bugs/interfaces/bugtask.py 2010-11-23 13:40:38 +0000 | |||
39 | @@ -667,10 +667,11 @@ | |||
40 | 667 | underlying bug for this bugtask. | 667 | underlying bug for this bugtask. |
41 | 668 | 668 | ||
42 | 669 | This method was documented as being required here so that | 669 | This method was documented as being required here so that |
47 | 670 | MentorshipOffers could happen on IBugTask. If that was the sole reason | 670 | MentorshipOffers could happen on IBugTask. If that was the sole |
48 | 671 | then this method should be deletable. When we move to context-less bug | 671 | reason then this method should be deletable. When we move to |
49 | 672 | presentation (where the bug is at /bugs/n?task=ubuntu) then we can | 672 | context-less bug presentation (where the bug is at |
50 | 673 | eliminate this if it is no longer useful. | 673 | /bugs/n?task=ubuntu) then we can eliminate this if it is no |
51 | 674 | longer useful. | ||
52 | 674 | """ | 675 | """ |
53 | 675 | 676 | ||
54 | 676 | @mutator_for(milestone) | 677 | @mutator_for(milestone) |
55 | @@ -1387,15 +1388,18 @@ | |||
56 | 1387 | Only BugTasks that the user has access to will be returned. | 1388 | Only BugTasks that the user has access to will be returned. |
57 | 1388 | """ | 1389 | """ |
58 | 1389 | 1390 | ||
60 | 1390 | def search(params, *args): | 1391 | def search(params, *args, **kwargs): |
61 | 1391 | """Search IBugTasks with the given search parameters. | 1392 | """Search IBugTasks with the given search parameters. |
62 | 1392 | 1393 | ||
63 | 1393 | Note: only use this method of BugTaskSet if you want to query | 1394 | Note: only use this method of BugTaskSet if you want to query |
64 | 1394 | tasks across multiple IBugTargets; otherwise, use the | 1395 | tasks across multiple IBugTargets; otherwise, use the |
65 | 1395 | IBugTarget's searchTasks() method. | 1396 | IBugTarget's searchTasks() method. |
66 | 1396 | 1397 | ||
69 | 1397 | :search_params: a BugTaskSearchParams object | 1398 | :param search_params: a BugTaskSearchParams object |
70 | 1398 | :args: any number of BugTaskSearchParams objects | 1399 | :param args: any number of BugTaskSearchParams objects |
71 | 1400 | :param prejoins: (keyword) A sequence of tuples | ||
72 | 1401 | (table, table_join) which should be pre-joined in addition | ||
73 | 1402 | to the default prejoins. | ||
74 | 1399 | 1403 | ||
75 | 1400 | If more than one BugTaskSearchParams is given, return the union of | 1404 | If more than one BugTaskSearchParams is given, return the union of |
76 | 1401 | IBugTasks which match any of them, with the results ordered by the | 1405 | IBugTasks which match any of them, with the results ordered by the |
77 | 1402 | 1406 | ||
78 | === modified file 'lib/lp/bugs/model/bugtarget.py' | |||
79 | --- lib/lp/bugs/model/bugtarget.py 2010-10-21 16:40:10 +0000 | |||
80 | +++ lib/lp/bugs/model/bugtarget.py 2010-11-23 13:40:38 +0000 | |||
81 | @@ -72,6 +72,7 @@ | |||
82 | 72 | All `IHasBugs` implementations should inherit from this class | 72 | All `IHasBugs` implementations should inherit from this class |
83 | 73 | or from `BugTargetBase`. | 73 | or from `BugTargetBase`. |
84 | 74 | """ | 74 | """ |
85 | 75 | |||
86 | 75 | def searchTasks(self, search_params, user=None, | 76 | def searchTasks(self, search_params, user=None, |
87 | 76 | order_by=None, search_text=None, | 77 | order_by=None, search_text=None, |
88 | 77 | status=None, | 78 | status=None, |
89 | @@ -93,7 +94,7 @@ | |||
90 | 93 | hardware_owner_is_affected_by_bug=False, | 94 | hardware_owner_is_affected_by_bug=False, |
91 | 94 | hardware_owner_is_subscribed_to_bug=False, | 95 | hardware_owner_is_subscribed_to_bug=False, |
92 | 95 | hardware_is_linked_to_bug=False, linked_branches=None, | 96 | hardware_is_linked_to_bug=False, linked_branches=None, |
94 | 96 | modified_since=None, created_since=None): | 97 | modified_since=None, created_since=None, prejoins=[]): |
95 | 97 | """See `IHasBugs`.""" | 98 | """See `IHasBugs`.""" |
96 | 98 | if status is None: | 99 | if status is None: |
97 | 99 | # If no statuses are supplied, default to the | 100 | # If no statuses are supplied, default to the |
98 | @@ -109,9 +110,10 @@ | |||
99 | 109 | del kwargs['self'] | 110 | del kwargs['self'] |
100 | 110 | del kwargs['user'] | 111 | del kwargs['user'] |
101 | 111 | del kwargs['search_params'] | 112 | del kwargs['search_params'] |
102 | 113 | del kwargs['prejoins'] | ||
103 | 112 | search_params = BugTaskSearchParams.fromSearchForm(user, **kwargs) | 114 | search_params = BugTaskSearchParams.fromSearchForm(user, **kwargs) |
104 | 113 | self._customizeSearchParams(search_params) | 115 | self._customizeSearchParams(search_params) |
106 | 114 | return BugTaskSet().search(search_params) | 116 | return BugTaskSet().search(search_params, prejoins=prejoins) |
107 | 115 | 117 | ||
108 | 116 | def _customizeSearchParams(self, search_params): | 118 | def _customizeSearchParams(self, search_params): |
109 | 117 | """Customize `search_params` for a specific target.""" | 119 | """Customize `search_params` for a specific target.""" |
110 | @@ -328,7 +330,6 @@ | |||
111 | 328 | self.project.recalculateBugHeatCache() | 330 | self.project.recalculateBugHeatCache() |
112 | 329 | 331 | ||
113 | 330 | 332 | ||
114 | 331 | |||
115 | 332 | class OfficialBugTagTargetMixin: | 333 | class OfficialBugTagTargetMixin: |
116 | 333 | """See `IOfficialBugTagTarget`. | 334 | """See `IOfficialBugTagTarget`. |
117 | 334 | 335 | ||
118 | @@ -441,4 +442,3 @@ | |||
119 | 441 | 'IDistribution instance or an IProduct instance.') | 442 | 'IDistribution instance or an IProduct instance.') |
120 | 442 | 443 | ||
121 | 443 | target = property(target, _settarget, doc=target.__doc__) | 444 | target = property(target, _settarget, doc=target.__doc__) |
122 | 444 | |||
123 | 445 | 445 | ||
124 | === modified file 'lib/lp/bugs/model/bugtask.py' | |||
125 | --- lib/lp/bugs/model/bugtask.py 2010-11-18 13:09:22 +0000 | |||
126 | +++ lib/lp/bugs/model/bugtask.py 2010-11-23 13:40:38 +0000 | |||
127 | @@ -2277,6 +2277,9 @@ | |||
128 | 2277 | :param _noprejoins: Private internal parameter to BugTaskSet which | 2277 | :param _noprejoins: Private internal parameter to BugTaskSet which |
129 | 2278 | disables all use of prejoins : consolidated from code paths that | 2278 | disables all use of prejoins : consolidated from code paths that |
130 | 2279 | claim they were inefficient and unwanted. | 2279 | claim they were inefficient and unwanted. |
131 | 2280 | :param prejoins: A sequence of tuples (table, table_join) which | ||
132 | 2281 | which should be pre-joined in addition to the default prejoins. | ||
133 | 2282 | This parameter has no effect if _noprejoins is True. | ||
134 | 2280 | """ | 2283 | """ |
135 | 2281 | # Prevent circular import problems. | 2284 | # Prevent circular import problems. |
136 | 2282 | from lp.registry.model.product import Product | 2285 | from lp.registry.model.product import Product |
137 | @@ -2286,6 +2289,7 @@ | |||
138 | 2286 | prejoins = [] | 2289 | prejoins = [] |
139 | 2287 | resultrow = BugTask | 2290 | resultrow = BugTask |
140 | 2288 | else: | 2291 | else: |
141 | 2292 | requested_joins = kwargs.get('prejoins', []) | ||
142 | 2289 | prejoins = [ | 2293 | prejoins = [ |
143 | 2290 | (Bug, LeftJoin(Bug, BugTask.bug == Bug.id)), | 2294 | (Bug, LeftJoin(Bug, BugTask.bug == Bug.id)), |
144 | 2291 | (Product, LeftJoin(Product, BugTask.product == Product.id)), | 2295 | (Product, LeftJoin(Product, BugTask.product == Product.id)), |
145 | @@ -2293,8 +2297,12 @@ | |||
146 | 2293 | LeftJoin( | 2297 | LeftJoin( |
147 | 2294 | SourcePackageName, | 2298 | SourcePackageName, |
148 | 2295 | BugTask.sourcepackagename == SourcePackageName.id)), | 2299 | BugTask.sourcepackagename == SourcePackageName.id)), |
151 | 2296 | ] | 2300 | ] + requested_joins |
152 | 2297 | resultrow = (BugTask, Bug, Product, SourcePackageName, ) | 2301 | resultrow = (BugTask, Bug, Product, SourcePackageName) |
153 | 2302 | additional_result_objects = [ | ||
154 | 2303 | table for table, join in requested_joins | ||
155 | 2304 | if table not in resultrow] | ||
156 | 2305 | resultrow = resultrow + tuple(additional_result_objects) | ||
157 | 2298 | return self._search(resultrow, prejoins, params, *args) | 2306 | return self._search(resultrow, prejoins, params, *args) |
158 | 2299 | 2307 | ||
159 | 2300 | def searchBugIds(self, params): | 2308 | def searchBugIds(self, params): |
160 | 2301 | 2309 | ||
161 | === modified file 'lib/lp/bugs/tests/test_bugtarget.py' | |||
162 | --- lib/lp/bugs/tests/test_bugtarget.py 2010-10-04 19:50:45 +0000 | |||
163 | +++ lib/lp/bugs/tests/test_bugtarget.py 2010-11-23 13:40:38 +0000 | |||
164 | @@ -15,8 +15,11 @@ | |||
165 | 15 | __all__ = [] | 15 | __all__ = [] |
166 | 16 | 16 | ||
167 | 17 | import random | 17 | import random |
168 | 18 | from testtools.matchers import Equals | ||
169 | 18 | import unittest | 19 | import unittest |
170 | 19 | 20 | ||
171 | 21 | from storm.expr import LeftJoin | ||
172 | 22 | from storm.store import Store | ||
173 | 20 | from zope.component import getUtility | 23 | from zope.component import getUtility |
174 | 21 | 24 | ||
175 | 22 | from canonical.launchpad.testing.systemdocs import ( | 25 | from canonical.launchpad.testing.systemdocs import ( |
176 | @@ -28,15 +31,24 @@ | |||
177 | 28 | from canonical.testing.layers import LaunchpadFunctionalLayer | 31 | from canonical.testing.layers import LaunchpadFunctionalLayer |
178 | 29 | from lp.bugs.interfaces.bug import CreateBugParams | 32 | from lp.bugs.interfaces.bug import CreateBugParams |
179 | 30 | from lp.bugs.interfaces.bugtask import ( | 33 | from lp.bugs.interfaces.bugtask import ( |
180 | 34 | BugTaskSearchParams, | ||
181 | 31 | BugTaskStatus, | 35 | BugTaskStatus, |
182 | 32 | IBugTaskSet, | 36 | IBugTaskSet, |
183 | 33 | ) | 37 | ) |
184 | 38 | from lp.bugs.model.bugtask import BugTask | ||
185 | 34 | from lp.registry.interfaces.distribution import ( | 39 | from lp.registry.interfaces.distribution import ( |
186 | 35 | IDistribution, | 40 | IDistribution, |
187 | 36 | IDistributionSet, | 41 | IDistributionSet, |
188 | 37 | ) | 42 | ) |
189 | 38 | from lp.registry.interfaces.product import IProductSet | 43 | from lp.registry.interfaces.product import IProductSet |
190 | 39 | from lp.registry.interfaces.projectgroup import IProjectGroupSet | 44 | from lp.registry.interfaces.projectgroup import IProjectGroupSet |
191 | 45 | from lp.registry.model.milestone import Milestone | ||
192 | 46 | from lp.testing import ( | ||
193 | 47 | person_logged_in, | ||
194 | 48 | StormStatementRecorder, | ||
195 | 49 | TestCaseWithFactory, | ||
196 | 50 | ) | ||
197 | 51 | from lp.testing.matchers import HasQueryCount | ||
198 | 40 | 52 | ||
199 | 41 | 53 | ||
200 | 42 | def bugtarget_filebug(bugtarget, summary, status=None): | 54 | def bugtarget_filebug(bugtarget, summary, status=None): |
201 | @@ -176,6 +188,101 @@ | |||
202 | 176 | test.globs['question_target'] = ubuntu.getSourcePackage('mozilla-firefox') | 188 | test.globs['question_target'] = ubuntu.getSourcePackage('mozilla-firefox') |
203 | 177 | 189 | ||
204 | 178 | 190 | ||
205 | 191 | class TestBugTargetSearchTasks(TestCaseWithFactory): | ||
206 | 192 | """Tests of IHasBugs.searchTasks().""" | ||
207 | 193 | |||
208 | 194 | layer = LaunchpadFunctionalLayer | ||
209 | 195 | |||
210 | 196 | def setUp(self): | ||
211 | 197 | super(TestBugTargetSearchTasks, self).setUp() | ||
212 | 198 | self.bug = self.factory.makeBug() | ||
213 | 199 | self.target = self.bug.default_bugtask.target | ||
214 | 200 | self.milestone = self.factory.makeMilestone(product=self.target) | ||
215 | 201 | with person_logged_in(self.target.owner): | ||
216 | 202 | self.bug.default_bugtask.transitionToMilestone( | ||
217 | 203 | self.milestone, self.target.owner) | ||
218 | 204 | self.store = Store.of(self.bug) | ||
219 | 205 | self.store.flush() | ||
220 | 206 | self.store.invalidate() | ||
221 | 207 | |||
222 | 208 | def test_preload_other_objects(self): | ||
223 | 209 | # We can prejoin objects in calls of searchTasks(). | ||
224 | 210 | |||
225 | 211 | # Without prejoining the table Milestone, accessing the | ||
226 | 212 | # BugTask property milestone requires an extra query. | ||
227 | 213 | with StormStatementRecorder() as recorder: | ||
228 | 214 | params = BugTaskSearchParams(user=None) | ||
229 | 215 | found_tasks = self.target.searchTasks(params) | ||
230 | 216 | found_tasks[0].milestone | ||
231 | 217 | self.assertThat(recorder, HasQueryCount(Equals(2))) | ||
232 | 218 | |||
233 | 219 | # When we prejoin Milestone, the milestone of our bugtask is | ||
234 | 220 | # already loaded during the main search query. | ||
235 | 221 | self.store.invalidate() | ||
236 | 222 | with StormStatementRecorder() as recorder: | ||
237 | 223 | params = BugTaskSearchParams(user=None) | ||
238 | 224 | prejoins = [(Milestone, | ||
239 | 225 | LeftJoin(Milestone, | ||
240 | 226 | BugTask.milestone == Milestone.id))] | ||
241 | 227 | found_tasks = self.target.searchTasks(params, prejoins=prejoins) | ||
242 | 228 | found_tasks[0].milestone | ||
243 | 229 | self.assertThat(recorder, HasQueryCount(Equals(1))) | ||
244 | 230 | |||
245 | 231 | def test_preload_other_objects_for_person_search_no_params_passed(self): | ||
246 | 232 | # We can prejoin objects in calls of Person.searchTasks(). | ||
247 | 233 | owner = self.bug.owner | ||
248 | 234 | with StormStatementRecorder() as recorder: | ||
249 | 235 | found_tasks = owner.searchTasks(None, user=None) | ||
250 | 236 | found_tasks[0].milestone | ||
251 | 237 | self.assertThat(recorder, HasQueryCount(Equals(2))) | ||
252 | 238 | |||
253 | 239 | self.store.invalidate() | ||
254 | 240 | with StormStatementRecorder() as recorder: | ||
255 | 241 | prejoins = [(Milestone, | ||
256 | 242 | LeftJoin(Milestone, | ||
257 | 243 | BugTask.milestone == Milestone.id))] | ||
258 | 244 | found_tasks = owner.searchTasks( | ||
259 | 245 | None, user=None, prejoins=prejoins) | ||
260 | 246 | found_tasks[0].milestone | ||
261 | 247 | self.assertThat(recorder, HasQueryCount(Equals(1))) | ||
262 | 248 | |||
263 | 249 | def test_preload_other_objects_for_person_search_no_keywords_passed(self): | ||
264 | 250 | # We can prejoin objects in calls of Person.searchTasks(). | ||
265 | 251 | owner = self.bug.owner | ||
266 | 252 | params = BugTaskSearchParams(user=None, owner=owner) | ||
267 | 253 | with StormStatementRecorder() as recorder: | ||
268 | 254 | found_tasks = owner.searchTasks(params) | ||
269 | 255 | found_tasks[0].milestone | ||
270 | 256 | self.assertThat(recorder, HasQueryCount(Equals(2))) | ||
271 | 257 | |||
272 | 258 | self.store.invalidate() | ||
273 | 259 | with StormStatementRecorder() as recorder: | ||
274 | 260 | prejoins = [(Milestone, | ||
275 | 261 | LeftJoin(Milestone, | ||
276 | 262 | BugTask.milestone == Milestone.id))] | ||
277 | 263 | found_tasks = owner.searchTasks(params, prejoins=prejoins) | ||
278 | 264 | found_tasks[0].milestone | ||
279 | 265 | self.assertThat(recorder, HasQueryCount(Equals(1))) | ||
280 | 266 | |||
281 | 267 | def test_preload_other_objects_for_person_search_keywords_passed(self): | ||
282 | 268 | # We can prejoin objects in calls of Person.searchTasks(). | ||
283 | 269 | owner = self.bug.owner | ||
284 | 270 | params = BugTaskSearchParams(user=None, owner=owner) | ||
285 | 271 | with StormStatementRecorder() as recorder: | ||
286 | 272 | found_tasks = owner.searchTasks(params, order_by=BugTask.id) | ||
287 | 273 | found_tasks[0].milestone | ||
288 | 274 | self.assertThat(recorder, HasQueryCount(Equals(2))) | ||
289 | 275 | |||
290 | 276 | self.store.invalidate() | ||
291 | 277 | with StormStatementRecorder() as recorder: | ||
292 | 278 | prejoins = [(Milestone, | ||
293 | 279 | LeftJoin(Milestone, | ||
294 | 280 | BugTask.milestone == Milestone.id))] | ||
295 | 281 | found_tasks = owner.searchTasks(params, prejoins=prejoins) | ||
296 | 282 | found_tasks[0].milestone | ||
297 | 283 | self.assertThat(recorder, HasQueryCount(Equals(1))) | ||
298 | 284 | |||
299 | 285 | |||
300 | 179 | def test_suite(): | 286 | def test_suite(): |
301 | 180 | """Return the `IBugTarget` TestSuite.""" | 287 | """Return the `IBugTarget` TestSuite.""" |
302 | 181 | suite = unittest.TestSuite() | 288 | suite = unittest.TestSuite() |
303 | @@ -195,7 +302,6 @@ | |||
304 | 195 | layer=LaunchpadFunctionalLayer) | 302 | layer=LaunchpadFunctionalLayer) |
305 | 196 | suite.addTest(test) | 303 | suite.addTest(test) |
306 | 197 | 304 | ||
307 | 198 | |||
308 | 199 | setUpMethods.remove(sourcePackageForQuestionSetUp) | 305 | setUpMethods.remove(sourcePackageForQuestionSetUp) |
309 | 200 | setUpMethods.append(sourcePackageSetUp) | 306 | setUpMethods.append(sourcePackageSetUp) |
310 | 201 | setUpMethods.append(projectSetUp) | 307 | setUpMethods.append(projectSetUp) |
311 | @@ -206,4 +312,5 @@ | |||
312 | 206 | layer=LaunchpadFunctionalLayer) | 312 | layer=LaunchpadFunctionalLayer) |
313 | 207 | suite.addTest(test) | 313 | suite.addTest(test) |
314 | 208 | 314 | ||
315 | 315 | suite.addTest(unittest.TestLoader().loadTestsFromName(__name__)) | ||
316 | 209 | return suite | 316 | return suite |
317 | 210 | 317 | ||
318 | === modified file 'lib/lp/bugs/tests/test_bugtask_search.py' | |||
319 | --- lib/lp/bugs/tests/test_bugtask_search.py 2010-11-12 17:42:43 +0000 | |||
320 | +++ lib/lp/bugs/tests/test_bugtask_search.py 2010-11-23 13:40:38 +0000 | |||
321 | @@ -10,10 +10,14 @@ | |||
322 | 10 | from new import classobj | 10 | from new import classobj |
323 | 11 | import pytz | 11 | import pytz |
324 | 12 | import sys | 12 | import sys |
325 | 13 | from testtools.matchers import Equals | ||
326 | 13 | import unittest | 14 | import unittest |
327 | 14 | 15 | ||
328 | 15 | from zope.component import getUtility | 16 | from zope.component import getUtility |
329 | 16 | 17 | ||
330 | 18 | from storm.expr import Join | ||
331 | 19 | from storm.store import Store | ||
332 | 20 | |||
333 | 17 | from canonical.launchpad.searchbuilder import ( | 21 | from canonical.launchpad.searchbuilder import ( |
334 | 18 | all, | 22 | all, |
335 | 19 | any, | 23 | any, |
336 | @@ -31,6 +35,7 @@ | |||
337 | 31 | BugTaskStatus, | 35 | BugTaskStatus, |
338 | 32 | IBugTaskSet, | 36 | IBugTaskSet, |
339 | 33 | ) | 37 | ) |
340 | 38 | from lp.bugs.model.bugtask import BugTask | ||
341 | 34 | from lp.registry.interfaces.distribution import IDistribution | 39 | from lp.registry.interfaces.distribution import IDistribution |
342 | 35 | from lp.registry.interfaces.distributionsourcepackage import ( | 40 | from lp.registry.interfaces.distributionsourcepackage import ( |
343 | 36 | IDistributionSourcePackage, | 41 | IDistributionSourcePackage, |
344 | @@ -39,10 +44,13 @@ | |||
345 | 39 | from lp.registry.interfaces.person import IPersonSet | 44 | from lp.registry.interfaces.person import IPersonSet |
346 | 40 | from lp.registry.interfaces.product import IProduct | 45 | from lp.registry.interfaces.product import IProduct |
347 | 41 | from lp.registry.interfaces.sourcepackage import ISourcePackage | 46 | from lp.registry.interfaces.sourcepackage import ISourcePackage |
348 | 47 | from lp.registry.model.person import Person | ||
349 | 42 | from lp.testing import ( | 48 | from lp.testing import ( |
350 | 43 | person_logged_in, | 49 | person_logged_in, |
351 | 50 | StormStatementRecorder, | ||
352 | 44 | TestCaseWithFactory, | 51 | TestCaseWithFactory, |
353 | 45 | ) | 52 | ) |
354 | 53 | from lp.testing.matchers import HasQueryCount | ||
355 | 46 | 54 | ||
356 | 47 | 55 | ||
357 | 48 | class SearchTestBase: | 56 | class SearchTestBase: |
358 | @@ -907,13 +915,40 @@ | |||
359 | 907 | def setUp(self): | 915 | def setUp(self): |
360 | 908 | super(PreloadBugtaskTargets, self).setUp() | 916 | super(PreloadBugtaskTargets, self).setUp() |
361 | 909 | 917 | ||
363 | 910 | def runSearch(self, params, *args): | 918 | def runSearch(self, params, *args, **kw): |
364 | 911 | """Run BugTaskSet.search() and preload bugtask target objects.""" | 919 | """Run BugTaskSet.search() and preload bugtask target objects.""" |
366 | 912 | return list(self.bugtask_set.search(params, *args, _noprejoins=False)) | 920 | return list(self.bugtask_set.search( |
367 | 921 | params, *args, _noprejoins=False, **kw)) | ||
368 | 913 | 922 | ||
369 | 914 | def resultValuesForBugtasks(self, expected_bugtasks): | 923 | def resultValuesForBugtasks(self, expected_bugtasks): |
370 | 915 | return expected_bugtasks | 924 | return expected_bugtasks |
371 | 916 | 925 | ||
372 | 926 | def test_preload_additional_objects(self): | ||
373 | 927 | # It is possible to join additional tables in the search query | ||
374 | 928 | # in order to load related Storm objects during the query. | ||
375 | 929 | store = Store.of(self.bugtasks[0]) | ||
376 | 930 | store.invalidate() | ||
377 | 931 | |||
378 | 932 | # If we do not prejoin the owner, two queries a run | ||
379 | 933 | # in order to retrieve the owner of the bugtask. | ||
380 | 934 | with StormStatementRecorder() as recorder: | ||
381 | 935 | params = self.getBugTaskSearchParams(user=None) | ||
382 | 936 | found_tasks = self.runSearch(params) | ||
383 | 937 | found_tasks[0].owner | ||
384 | 938 | self.assertTrue(len(recorder.statements) > 1) | ||
385 | 939 | |||
386 | 940 | # If we join the table person on bugtask.owner == person.id | ||
387 | 941 | # the owner object is loaded in the query that retrieves the | ||
388 | 942 | # bugtasks. | ||
389 | 943 | store.invalidate() | ||
390 | 944 | with StormStatementRecorder() as recorder: | ||
391 | 945 | params = self.getBugTaskSearchParams(user=None) | ||
392 | 946 | found_tasks = self.runSearch( | ||
393 | 947 | params, | ||
394 | 948 | prejoins=[(Person, Join(Person, BugTask.owner == Person.id))]) | ||
395 | 949 | found_tasks[0].owner | ||
396 | 950 | self.assertThat(recorder, HasQueryCount(Equals(1))) | ||
397 | 951 | |||
398 | 917 | 952 | ||
399 | 918 | class NoPreloadBugtaskTargets(MultipleParams): | 953 | class NoPreloadBugtaskTargets(MultipleParams): |
400 | 919 | """Do not preload bug targets during a BugTaskSet.search() query.""" | 954 | """Do not preload bug targets during a BugTaskSet.search() query.""" |
401 | 920 | 955 | ||
402 | === modified file 'lib/lp/registry/model/person.py' | |||
403 | --- lib/lp/registry/model/person.py 2010-11-18 12:05:34 +0000 | |||
404 | +++ lib/lp/registry/model/person.py 2010-11-23 13:40:38 +0000 | |||
405 | @@ -882,6 +882,7 @@ | |||
406 | 882 | 882 | ||
407 | 883 | def searchTasks(self, search_params, *args, **kwargs): | 883 | def searchTasks(self, search_params, *args, **kwargs): |
408 | 884 | """See `IHasBugs`.""" | 884 | """See `IHasBugs`.""" |
409 | 885 | prejoins = kwargs.pop('prejoins', []) | ||
410 | 885 | if search_params is None and len(args) == 0: | 886 | if search_params is None and len(args) == 0: |
411 | 886 | # this method is called via webapi directly | 887 | # this method is called via webapi directly |
412 | 887 | # calling this method on a Person object directly via the | 888 | # calling this method on a Person object directly via the |
413 | @@ -896,15 +897,18 @@ | |||
414 | 896 | # method, see docstring of | 897 | # method, see docstring of |
415 | 897 | # `lazr.restful.declarations.webservice_error()` | 898 | # `lazr.restful.declarations.webservice_error()` |
416 | 898 | raise e | 899 | raise e |
418 | 899 | return getUtility(IBugTaskSet).search(*search_params) | 900 | return getUtility(IBugTaskSet).search( |
419 | 901 | *search_params, prejoins=prejoins) | ||
420 | 900 | if len(kwargs) > 0: | 902 | if len(kwargs) > 0: |
421 | 901 | # if keyword arguments are supplied, use the deault | 903 | # if keyword arguments are supplied, use the deault |
422 | 902 | # implementation in HasBugsBase. | 904 | # implementation in HasBugsBase. |
424 | 903 | return HasBugsBase.searchTasks(self, search_params, **kwargs) | 905 | return HasBugsBase.searchTasks( |
425 | 906 | self, search_params, prejoins=prejoins, **kwargs) | ||
426 | 904 | else: | 907 | else: |
427 | 905 | # Otherwise pass all positional arguments to the | 908 | # Otherwise pass all positional arguments to the |
428 | 906 | # implementation in BugTaskSet. | 909 | # implementation in BugTaskSet. |
430 | 907 | return getUtility(IBugTaskSet).search(search_params, *args) | 910 | return getUtility(IBugTaskSet).search( |
431 | 911 | search_params, *args, prejoins=prejoins) | ||
432 | 908 | 912 | ||
433 | 909 | def getProjectsAndCategoriesContributedTo(self, limit=5): | 913 | def getProjectsAndCategoriesContributedTo(self, limit=5): |
434 | 910 | """See `IPerson`.""" | 914 | """See `IPerson`.""" |