Merge lp:~al-maisan/launchpad/clog-oops-452070 into lp:launchpad
- clog-oops-452070
- Merge into devel
Proposed by
Muharem Hrnjadovic
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Eleanor Berger | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | not available | ||||
Proposed branch: | lp:~al-maisan/launchpad/clog-oops-452070 | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
659 lines 13 files modified
lib/canonical/launchpad/webapp/tales.py (+9/-2) lib/lp/bugs/doc/bug.txt (+28/-0) lib/lp/bugs/interfaces/bug.py (+7/-0) lib/lp/bugs/model/bug.py (+10/-1) lib/lp/registry/browser/distributionsourcepackage.py (+46/-4) lib/lp/registry/configure.zcml (+1/-0) lib/lp/registry/doc/sourcepackage.txt (+10/-8) lib/lp/registry/model/distributionsourcepackage.py (+22/-1) lib/lp/soyuz/browser/configure.zcml (+0/-3) lib/lp/soyuz/browser/sourcepackagerelease.py (+117/-96) lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt (+34/-0) lib/lp/soyuz/templates/distributionsourcepackagerelease-changes.pt (+5/-4) lib/lp/soyuz/templates/sourcepackagerelease-change-summary.pt (+0/-10) |
||||
To merge this branch: | bzr merge lp:~al-maisan/launchpad/clog-oops-452070 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Eleanor Berger (community) | Approve | ||
Review via email: mp+13789@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote : | # |
Revision history for this message
Eleanor Berger (intellectronica) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/canonical/launchpad/webapp/tales.py' | |||
2 | --- lib/canonical/launchpad/webapp/tales.py 2009-09-18 21:54:45 +0000 | |||
3 | +++ lib/canonical/launchpad/webapp/tales.py 2009-10-22 21:21:16 +0000 | |||
4 | @@ -2876,7 +2876,7 @@ | |||
5 | 2876 | "<<email address hidden>>", "<email address hidden>") | 2876 | "<<email address hidden>>", "<email address hidden>") |
6 | 2877 | return text | 2877 | return text |
7 | 2878 | 2878 | ||
9 | 2879 | def linkify_email(self): | 2879 | def linkify_email(self, preloaded_person_data=None): |
10 | 2880 | """Linkify any email address recognised in Launchpad. | 2880 | """Linkify any email address recognised in Launchpad. |
11 | 2881 | 2881 | ||
12 | 2882 | If an email address is recognised as one registered in Launchpad, | 2882 | If an email address is recognised as one registered in Launchpad, |
13 | @@ -2891,7 +2891,14 @@ | |||
14 | 2891 | matches = re.finditer(self._re_email, text) | 2891 | matches = re.finditer(self._re_email, text) |
15 | 2892 | for match in matches: | 2892 | for match in matches: |
16 | 2893 | address = match.group() | 2893 | address = match.group() |
18 | 2894 | person = getUtility(IPersonSet).getByEmail(address) | 2894 | person = None |
19 | 2895 | # First try to find the person required in the preloaded person | ||
20 | 2896 | # data dictionary. | ||
21 | 2897 | if preloaded_person_data is not None: | ||
22 | 2898 | person = preloaded_person_data.get(address, None) | ||
23 | 2899 | else: | ||
24 | 2900 | # No pre-loaded data -> we need to perform a database lookup. | ||
25 | 2901 | person = getUtility(IPersonSet).getByEmail(address) | ||
26 | 2895 | # Only linkify if person exists and does not want to hide | 2902 | # Only linkify if person exists and does not want to hide |
27 | 2896 | # their email addresses. | 2903 | # their email addresses. |
28 | 2897 | if person is not None and not person.hide_email_addresses: | 2904 | if person is not None and not person.hide_email_addresses: |
29 | 2898 | 2905 | ||
30 | === modified file 'lib/lp/bugs/doc/bug.txt' | |||
31 | --- lib/lp/bugs/doc/bug.txt 2009-07-08 03:14:59 +0000 | |||
32 | +++ lib/lp/bugs/doc/bug.txt 2009-10-22 21:21:16 +0000 | |||
33 | @@ -37,6 +37,34 @@ | |||
34 | 37 | ... | 37 | ... |
35 | 38 | NotFoundError: 'Unable to locate bug with nickname +bugs.' | 38 | NotFoundError: 'Unable to locate bug with nickname +bugs.' |
36 | 39 | 39 | ||
37 | 40 | It is also possible to retrieve a number of bugs by specifying the bug numbers | ||
38 | 41 | of interest. | ||
39 | 42 | |||
40 | 43 | >>> result_set = bugset.getByNumbers([6, 1234]) | ||
41 | 44 | >>> print result_set.count() | ||
42 | 45 | 1 | ||
43 | 46 | |||
44 | 47 | >>> [the_bug_found] = result_set | ||
45 | 48 | >>> print the_bug_found.title | ||
46 | 49 | Firefox crashes when Save As dialog for a nonexistent window is closed | ||
47 | 50 | |||
48 | 51 | >>> result_set = bugset.getByNumbers([6, 4321, 1]) | ||
49 | 52 | >>> print result_set.count() | ||
50 | 53 | 2 | ||
51 | 54 | |||
52 | 55 | >>> second_bug_found = result_set[1] | ||
53 | 56 | >>> print second_bug_found.title | ||
54 | 57 | Firefox does not support SVG | ||
55 | 58 | |||
56 | 59 | If no bug numbers are specified an empty result set is returned. | ||
57 | 60 | |||
58 | 61 | >>> result_set = bugset.getByNumbers(None) | ||
59 | 62 | >>> print result_set.count() | ||
60 | 63 | 0 | ||
61 | 64 | >>> result_set = bugset.getByNumbers([]) | ||
62 | 65 | >>> print result_set.count() | ||
63 | 66 | 0 | ||
64 | 67 | |||
65 | 40 | == Bug creation events == | 68 | == Bug creation events == |
66 | 41 | 69 | ||
67 | 42 | IObjectCreatedEvent events are fired off when a bug is created. First | 70 | IObjectCreatedEvent events are fired off when a bug is created. First |
68 | 43 | 71 | ||
69 | === modified file 'lib/lp/bugs/interfaces/bug.py' | |||
70 | --- lib/lp/bugs/interfaces/bug.py 2009-08-26 11:06:05 +0000 | |||
71 | +++ lib/lp/bugs/interfaces/bug.py 2009-10-22 21:21:16 +0000 | |||
72 | @@ -937,6 +937,13 @@ | |||
73 | 937 | :param limit: The number of distinct Bugs to return. | 937 | :param limit: The number of distinct Bugs to return. |
74 | 938 | """ | 938 | """ |
75 | 939 | 939 | ||
76 | 940 | def getByNumbers(bug_numbers): | ||
77 | 941 | """Get `IBug` instances identified by the `bug_numbers` iterable. | ||
78 | 942 | |||
79 | 943 | :param bug_numbers: An iterable of bug numbers for which we should | ||
80 | 944 | return Bugs. | ||
81 | 945 | """ | ||
82 | 946 | |||
83 | 940 | 947 | ||
84 | 941 | class InvalidBugTargetType(Exception): | 948 | class InvalidBugTargetType(Exception): |
85 | 942 | """Bug target's type is not valid.""" | 949 | """Bug target's type is not valid.""" |
86 | 943 | 950 | ||
87 | === modified file 'lib/lp/bugs/model/bug.py' | |||
88 | --- lib/lp/bugs/model/bug.py 2009-08-26 01:54:39 +0000 | |||
89 | +++ lib/lp/bugs/model/bug.py 2009-10-22 21:21:16 +0000 | |||
90 | @@ -33,7 +33,7 @@ | |||
91 | 33 | from sqlobject import SQLMultipleJoin, SQLRelatedJoin | 33 | from sqlobject import SQLMultipleJoin, SQLRelatedJoin |
92 | 34 | from sqlobject import SQLObjectNotFound | 34 | from sqlobject import SQLObjectNotFound |
93 | 35 | from storm.expr import And, Count, In, LeftJoin, Select, SQLRaw, Func | 35 | from storm.expr import And, Count, In, LeftJoin, Select, SQLRaw, Func |
95 | 36 | from storm.store import Store | 36 | from storm.store import EmptyResultSet, Store |
96 | 37 | 37 | ||
97 | 38 | from lazr.lifecycle.event import ( | 38 | from lazr.lifecycle.event import ( |
98 | 39 | ObjectCreatedEvent, ObjectDeletedEvent, ObjectModifiedEvent) | 39 | ObjectCreatedEvent, ObjectDeletedEvent, ObjectModifiedEvent) |
99 | @@ -49,6 +49,7 @@ | |||
100 | 49 | from canonical.launchpad.interfaces.hwdb import IHWSubmissionBugSet | 49 | from canonical.launchpad.interfaces.hwdb import IHWSubmissionBugSet |
101 | 50 | from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities | 50 | from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities |
102 | 51 | from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet | 51 | from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet |
103 | 52 | from canonical.launchpad.interfaces.lpstorm import IStore | ||
104 | 52 | from canonical.launchpad.interfaces.message import ( | 53 | from canonical.launchpad.interfaces.message import ( |
105 | 53 | IMessage, IndexedMessage) | 54 | IMessage, IndexedMessage) |
106 | 54 | from canonical.launchpad.interfaces.structuralsubscription import ( | 55 | from canonical.launchpad.interfaces.structuralsubscription import ( |
107 | @@ -1627,6 +1628,14 @@ | |||
108 | 1627 | 1628 | ||
109 | 1628 | return bugs | 1629 | return bugs |
110 | 1629 | 1630 | ||
111 | 1631 | def getByNumbers(self, bug_numbers): | ||
112 | 1632 | """see `IBugSet`.""" | ||
113 | 1633 | if bug_numbers is None or len(bug_numbers) < 1: | ||
114 | 1634 | return EmptyResultSet() | ||
115 | 1635 | store = IStore(Bug) | ||
116 | 1636 | result_set = store.find(Bug, In(Bug.id, bug_numbers)) | ||
117 | 1637 | return result_set | ||
118 | 1638 | |||
119 | 1630 | 1639 | ||
120 | 1631 | class BugAffectsPerson(SQLBase): | 1640 | class BugAffectsPerson(SQLBase): |
121 | 1632 | """A bug is marked as affecting a user.""" | 1641 | """A bug is marked as affecting a user.""" |
122 | 1633 | 1642 | ||
123 | === modified file 'lib/lp/registry/browser/distributionsourcepackage.py' | |||
124 | --- lib/lp/registry/browser/distributionsourcepackage.py 2009-10-20 17:57:51 +0000 | |||
125 | +++ lib/lp/registry/browser/distributionsourcepackage.py 2009-10-22 21:21:16 +0000 | |||
126 | @@ -27,6 +27,7 @@ | |||
127 | 27 | 27 | ||
128 | 28 | from canonical.cachedproperty import cachedproperty | 28 | from canonical.cachedproperty import cachedproperty |
129 | 29 | from canonical.launchpad import _ | 29 | from canonical.launchpad import _ |
130 | 30 | from canonical.launchpad.interfaces import IBugSet | ||
131 | 30 | from lp.answers.interfaces.questionenums import QuestionStatus | 31 | from lp.answers.interfaces.questionenums import QuestionStatus |
132 | 31 | from lp.soyuz.interfaces.archive import IArchiveSet | 32 | from lp.soyuz.interfaces.archive import IArchiveSet |
133 | 32 | from lp.soyuz.interfaces.distributionsourcepackagerelease import ( | 33 | from lp.soyuz.interfaces.distributionsourcepackagerelease import ( |
134 | @@ -49,6 +50,8 @@ | |||
135 | 49 | from canonical.launchpad.webapp.breadcrumb import Breadcrumb | 50 | from canonical.launchpad.webapp.breadcrumb import Breadcrumb |
136 | 50 | 51 | ||
137 | 51 | from lazr.delegates import delegates | 52 | from lazr.delegates import delegates |
138 | 53 | from lp.soyuz.browser.sourcepackagerelease import ( | ||
139 | 54 | extract_bug_numbers, extract_email_addresses, linkify_changelog) | ||
140 | 52 | from canonical.lazr.utils import smartquote | 55 | from canonical.lazr.utils import smartquote |
141 | 53 | 56 | ||
142 | 54 | 57 | ||
143 | @@ -134,11 +137,14 @@ | |||
144 | 134 | """ | 137 | """ |
145 | 135 | delegates(IDistributionSourcePackageRelease, 'context') | 138 | delegates(IDistributionSourcePackageRelease, 'context') |
146 | 136 | 139 | ||
149 | 137 | def __init__(self, distributionsourcepackagerelease, | 140 | def __init__( |
150 | 138 | publishing_history, package_diffs): | 141 | self, distributionsourcepackagerelease, publishing_history, |
151 | 142 | package_diffs, person_data, user): | ||
152 | 139 | self.context = distributionsourcepackagerelease | 143 | self.context = distributionsourcepackagerelease |
153 | 140 | self._publishing_history = publishing_history | 144 | self._publishing_history = publishing_history |
154 | 141 | self._package_diffs = package_diffs | 145 | self._package_diffs = package_diffs |
155 | 146 | self._person_data = person_data | ||
156 | 147 | self._user = user | ||
157 | 142 | 148 | ||
158 | 143 | @property | 149 | @property |
159 | 144 | def publishing_history(self): | 150 | def publishing_history(self): |
160 | @@ -150,6 +156,12 @@ | |||
161 | 150 | """ See `ISourcePackageRelease`.""" | 156 | """ See `ISourcePackageRelease`.""" |
162 | 151 | return self._package_diffs | 157 | return self._package_diffs |
163 | 152 | 158 | ||
164 | 159 | @property | ||
165 | 160 | def change_summary(self): | ||
166 | 161 | """ See `ISourcePackageRelease`.""" | ||
167 | 162 | return linkify_changelog( | ||
168 | 163 | self._user, self.context.change_summary, self._person_data) | ||
169 | 164 | |||
170 | 153 | 165 | ||
171 | 154 | class IDistributionSourcePackageActionMenu(Interface): | 166 | class IDistributionSourcePackageActionMenu(Interface): |
172 | 155 | """Marker interface for the action menu.""" | 167 | """Marker interface for the action menu.""" |
173 | @@ -172,14 +184,43 @@ | |||
174 | 172 | """Common features to all `DistributionSourcePackage` views.""" | 184 | """Common features to all `DistributionSourcePackage` views.""" |
175 | 173 | 185 | ||
176 | 174 | def releases(self): | 186 | def releases(self): |
177 | 187 | def not_empty(text): | ||
178 | 188 | return ( | ||
179 | 189 | text is not None and isinstance(text, basestring) | ||
180 | 190 | and len(text.strip()) > 0) | ||
181 | 191 | |||
182 | 175 | dspr_pubs = self.context.getReleasesAndPublishingHistory() | 192 | dspr_pubs = self.context.getReleasesAndPublishingHistory() |
183 | 176 | 193 | ||
184 | 177 | # Return as early as possible to avoid unnecessary processing. | 194 | # Return as early as possible to avoid unnecessary processing. |
185 | 178 | if len(dspr_pubs) == 0: | 195 | if len(dspr_pubs) == 0: |
186 | 179 | return [] | 196 | return [] |
187 | 180 | 197 | ||
188 | 198 | sprs = [dspr.sourcepackagerelease for (dspr, spphs) in dspr_pubs] | ||
189 | 199 | # Pre-load the bugs and persons referenced by the +changelog page from | ||
190 | 200 | # the database. | ||
191 | 201 | # This will improve the performance of the ensuing changelog | ||
192 | 202 | # linkification. | ||
193 | 203 | the_changelog = '\n'.join( | ||
194 | 204 | [spr.changelog_entry for spr in sprs | ||
195 | 205 | if not_empty(spr.changelog_entry)]) | ||
196 | 206 | unique_bugs = extract_bug_numbers(the_changelog) | ||
197 | 207 | self._bug_data = list( | ||
198 | 208 | getUtility(IBugSet).getByNumbers(unique_bugs.keys())) | ||
199 | 209 | # Preload email/person data only if user is logged on. In the opposite | ||
200 | 210 | # case the emails in the changelog will be obfuscated anyway and thus | ||
201 | 211 | # cause no database lookups. | ||
202 | 212 | if self.user: | ||
203 | 213 | unique_emails = extract_email_addresses(the_changelog) | ||
204 | 214 | # The method below returns a [(EmailAddress,Person]] result set. | ||
205 | 215 | result_set = self.context.getPersonsByEmail(unique_emails) | ||
206 | 216 | # Ignore the persons who want their email addresses hidden. | ||
207 | 217 | self._person_data = dict( | ||
208 | 218 | [(email.email,person) for (email,person) in result_set | ||
209 | 219 | if not person.hide_email_addresses]) | ||
210 | 220 | else: | ||
211 | 221 | self._person_data = None | ||
212 | 222 | |||
213 | 181 | # Collate diffs for relevant SourcePackageReleases | 223 | # Collate diffs for relevant SourcePackageReleases |
214 | 182 | sprs = [dspr.sourcepackagerelease for (dspr, spphs) in dspr_pubs] | ||
215 | 183 | pkg_diffs = getUtility(IPackageDiffSet).getDiffsToReleases(sprs) | 224 | pkg_diffs = getUtility(IPackageDiffSet).getDiffsToReleases(sprs) |
216 | 184 | spr_diffs = {} | 225 | spr_diffs = {} |
217 | 185 | for spr, diffs in itertools.groupby(pkg_diffs, | 226 | for spr, diffs in itertools.groupby(pkg_diffs, |
218 | @@ -188,7 +229,8 @@ | |||
219 | 188 | 229 | ||
220 | 189 | return [ | 230 | return [ |
221 | 190 | DecoratedDistributionSourcePackageRelease( | 231 | DecoratedDistributionSourcePackageRelease( |
223 | 191 | dspr, spphs, spr_diffs.get(dspr.sourcepackagerelease, [])) | 232 | dspr, spphs, spr_diffs.get(dspr.sourcepackagerelease, []), |
224 | 233 | self._person_data, self.user) | ||
225 | 192 | for (dspr, spphs) in dspr_pubs] | 234 | for (dspr, spphs) in dspr_pubs] |
226 | 193 | 235 | ||
227 | 194 | 236 | ||
228 | 195 | 237 | ||
229 | === modified file 'lib/lp/registry/configure.zcml' | |||
230 | --- lib/lp/registry/configure.zcml 2009-10-22 09:24:53 +0000 | |||
231 | +++ lib/lp/registry/configure.zcml 2009-10-22 21:21:16 +0000 | |||
232 | @@ -392,6 +392,7 @@ | |||
233 | 392 | getSubscription | 392 | getSubscription |
234 | 393 | parent_subscription_target | 393 | parent_subscription_target |
235 | 394 | getBugNotificationsRecipients | 394 | getBugNotificationsRecipients |
236 | 395 | getPersonsByEmail | ||
237 | 395 | getReleasesAndPublishingHistory | 396 | getReleasesAndPublishingHistory |
238 | 396 | upstream_product | 397 | upstream_product |
239 | 397 | target_type_display | 398 | target_type_display |
240 | 398 | 399 | ||
241 | === modified file 'lib/lp/registry/doc/sourcepackage.txt' | |||
242 | --- lib/lp/registry/doc/sourcepackage.txt 2009-10-19 21:20:31 +0000 | |||
243 | +++ lib/lp/registry/doc/sourcepackage.txt 2009-10-22 21:21:16 +0000 | |||
244 | @@ -370,32 +370,34 @@ | |||
245 | 370 | 370 | ||
246 | 371 | Zero or more spaces may appear between the ":" and the "#". | 371 | Zero or more spaces may appear between the ":" and the "#". |
247 | 372 | 372 | ||
249 | 373 | >>> spr_view._linkify_bug_numbers("LP: #10") | 373 | >>> from lp.soyuz.browser.sourcepackagerelease import ( |
250 | 374 | ... linkify_bug_numbers) | ||
251 | 375 | >>> linkify_bug_numbers("LP: #10") | ||
252 | 374 | u'LP: <a href="/bugs/10" title="another test bug">#10</a>' | 376 | u'LP: <a href="/bugs/10" title="another test bug">#10</a>' |
253 | 375 | 377 | ||
255 | 376 | >>> spr_view._linkify_bug_numbers("LP: #10") | 378 | >>> linkify_bug_numbers("LP: #10") |
256 | 377 | u'LP: <a href="/bugs/10" title="another test bug">#10</a>' | 379 | u'LP: <a href="/bugs/10" title="another test bug">#10</a>' |
257 | 378 | 380 | ||
259 | 379 | >>> spr_view._linkify_bug_numbers("LP:#10") | 381 | >>> linkify_bug_numbers("LP:#10") |
260 | 380 | u'LP:<a href="/bugs/10" title="another test bug">#10</a>' | 382 | u'LP:<a href="/bugs/10" title="another test bug">#10</a>' |
261 | 381 | 383 | ||
263 | 382 | >>> spr_view._linkify_bug_numbers("LP: #999") | 384 | >>> linkify_bug_numbers("LP: #999") |
264 | 383 | 'LP: <a href="/bugs/999" title="No such bug">#999</a>' | 385 | 'LP: <a href="/bugs/999" title="No such bug">#999</a>' |
265 | 384 | 386 | ||
267 | 385 | >>> spr_view._linkify_bug_numbers("garbage") | 387 | >>> linkify_bug_numbers("garbage") |
268 | 386 | 'garbage' | 388 | 'garbage' |
269 | 387 | 389 | ||
271 | 388 | >>> spr_view._linkify_bug_numbers("LP: #10, #7") | 390 | >>> linkify_bug_numbers("LP: #10, #7") |
272 | 389 | u'LP: <a href="/bugs/10" title="another test bug">#10</a>, | 391 | u'LP: <a href="/bugs/10" title="another test bug">#10</a>, |
273 | 390 | <a href="/bugs/7" title="A test bug">#7</a>' | 392 | <a href="/bugs/7" title="A test bug">#7</a>' |
274 | 391 | 393 | ||
276 | 392 | >>> spr_view._linkify_bug_numbers("LP: #10 LP: #10") | 394 | >>> linkify_bug_numbers("LP: #10 LP: #10") |
277 | 393 | u'LP: <a href="/bugs/10" title="another test bug">#10</a> | 395 | u'LP: <a href="/bugs/10" title="another test bug">#10</a> |
278 | 394 | LP: <a href="/bugs/10" title="another test bug">#10</a>' | 396 | LP: <a href="/bugs/10" title="another test bug">#10</a>' |
279 | 395 | 397 | ||
280 | 396 | The regex is case-insensitive, so "lp: #nnn" also works. | 398 | The regex is case-insensitive, so "lp: #nnn" also works. |
281 | 397 | 399 | ||
283 | 398 | >>> spr_view._linkify_bug_numbers("lp: #10") | 400 | >>> linkify_bug_numbers("lp: #10") |
284 | 399 | u'lp: <a href="/bugs/10" title="another test bug">#10</a>' | 401 | u'lp: <a href="/bugs/10" title="another test bug">#10</a>' |
285 | 400 | 402 | ||
286 | 401 | 403 | ||
287 | 402 | 404 | ||
288 | === modified file 'lib/lp/registry/model/distributionsourcepackage.py' | |||
289 | --- lib/lp/registry/model/distributionsourcepackage.py 2009-09-16 04:31:39 +0000 | |||
290 | +++ lib/lp/registry/model/distributionsourcepackage.py 2009-10-22 21:21:16 +0000 | |||
291 | @@ -15,7 +15,8 @@ | |||
292 | 15 | import operator | 15 | import operator |
293 | 16 | 16 | ||
294 | 17 | from sqlobject.sqlbuilder import SQLConstant | 17 | from sqlobject.sqlbuilder import SQLConstant |
296 | 18 | from storm.expr import And, Desc, In | 18 | from storm.expr import And, Desc, In, Join, Lower |
297 | 19 | from storm.store import EmptyResultSet | ||
298 | 19 | from storm.locals import Int, Reference, Store, Storm, Unicode | 20 | from storm.locals import Int, Reference, Store, Storm, Unicode |
299 | 20 | from zope.interface import implements | 21 | from zope.interface import implements |
300 | 21 | 22 | ||
301 | @@ -36,10 +37,13 @@ | |||
302 | 36 | from lp.soyuz.model.sourcepackagerelease import ( | 37 | from lp.soyuz.model.sourcepackagerelease import ( |
303 | 37 | SourcePackageRelease) | 38 | SourcePackageRelease) |
304 | 38 | from lp.registry.model.karma import KarmaTotalCache | 39 | from lp.registry.model.karma import KarmaTotalCache |
305 | 40 | from lp.registry.model.person import Person | ||
306 | 39 | from lp.registry.model.sourcepackage import ( | 41 | from lp.registry.model.sourcepackage import ( |
307 | 40 | SourcePackage, SourcePackageQuestionTargetMixin) | 42 | SourcePackage, SourcePackageQuestionTargetMixin) |
308 | 43 | from canonical.launchpad.database.emailaddress import EmailAddress | ||
309 | 41 | from canonical.launchpad.database.structuralsubscription import ( | 44 | from canonical.launchpad.database.structuralsubscription import ( |
310 | 42 | StructuralSubscriptionTargetMixin) | 45 | StructuralSubscriptionTargetMixin) |
311 | 46 | from canonical.launchpad.interfaces.lpstorm import IStore | ||
312 | 43 | 47 | ||
313 | 44 | from canonical.lazr.utils import smartquote | 48 | from canonical.lazr.utils import smartquote |
314 | 45 | 49 | ||
315 | @@ -408,6 +412,23 @@ | |||
316 | 408 | 'BugTask.distribution = %s AND BugTask.sourcepackagename = %s' % | 412 | 'BugTask.distribution = %s AND BugTask.sourcepackagename = %s' % |
317 | 409 | sqlvalues(self.distribution, self.sourcepackagename)) | 413 | sqlvalues(self.distribution, self.sourcepackagename)) |
318 | 410 | 414 | ||
319 | 415 | @staticmethod | ||
320 | 416 | def getPersonsByEmail(email_addresses): | ||
321 | 417 | """[(EmailAddress,Person), ..] iterable for given email addresses.""" | ||
322 | 418 | if email_addresses is None or len(email_addresses) < 1: | ||
323 | 419 | return EmptyResultSet() | ||
324 | 420 | # Perform basic sanitization of email addresses. | ||
325 | 421 | email_addresses = [ | ||
326 | 422 | address.lower().strip() for address in email_addresses] | ||
327 | 423 | store = IStore(Person) | ||
328 | 424 | origin = [ | ||
329 | 425 | Person, Join(EmailAddress, EmailAddress.personID == Person.id)] | ||
330 | 426 | # Get all persons whose email addresses are in the list. | ||
331 | 427 | result_set = store.using(*origin).find( | ||
332 | 428 | (EmailAddress, Person), | ||
333 | 429 | In(Lower(EmailAddress.email), email_addresses)) | ||
334 | 430 | return result_set | ||
335 | 431 | |||
336 | 411 | 432 | ||
337 | 412 | class DistributionSourcePackageInDatabase(Storm): | 433 | class DistributionSourcePackageInDatabase(Storm): |
338 | 413 | """Temporary class to allow access to the database.""" | 434 | """Temporary class to allow access to the database.""" |
339 | 414 | 435 | ||
340 | === modified file 'lib/lp/soyuz/browser/configure.zcml' | |||
341 | --- lib/lp/soyuz/browser/configure.zcml 2009-09-29 07:21:40 +0000 | |||
342 | +++ lib/lp/soyuz/browser/configure.zcml 2009-10-22 21:21:16 +0000 | |||
343 | @@ -112,9 +112,6 @@ | |||
344 | 112 | name="+files" | 112 | name="+files" |
345 | 113 | facet="overview" | 113 | facet="overview" |
346 | 114 | template="../templates/sourcepackagerelease-files.pt"/> | 114 | template="../templates/sourcepackagerelease-files.pt"/> |
347 | 115 | <browser:page | ||
348 | 116 | name="+change_summary" | ||
349 | 117 | template="../templates/sourcepackagerelease-change-summary.pt"/> | ||
350 | 118 | </browser:pages> | 115 | </browser:pages> |
351 | 119 | <facet | 116 | <facet |
352 | 120 | facet="overview"> | 117 | facet="overview"> |
353 | 121 | 118 | ||
354 | === modified file 'lib/lp/soyuz/browser/sourcepackagerelease.py' | |||
355 | --- lib/lp/soyuz/browser/sourcepackagerelease.py 2009-09-09 11:52:55 +0000 | |||
356 | +++ lib/lp/soyuz/browser/sourcepackagerelease.py 2009-10-22 21:21:16 +0000 | |||
357 | @@ -16,108 +16,129 @@ | |||
358 | 16 | from canonical.launchpad.webapp.tales import FormattersAPI | 16 | from canonical.launchpad.webapp.tales import FormattersAPI |
359 | 17 | 17 | ||
360 | 18 | 18 | ||
361 | 19 | def extract_bug_numbers(text): | ||
362 | 20 | '''Unique bug numbers matching the "LP: #n(, #n)*" pattern in the text.''' | ||
363 | 21 | # FormattersAPI._linkify_substitution requires a match object | ||
364 | 22 | # that has named groups "bug" and "bugnum". The matching text for | ||
365 | 23 | # the "bug" group is used as the link text and "bugnum" forms part | ||
366 | 24 | # of the URL for the link to the bug. Example: | ||
367 | 25 | # >>> bm.groupdict( ) | ||
368 | 26 | # {'bugnum': '400686', 'bug': '#400686'} | ||
369 | 27 | |||
370 | 28 | # We need to match bug numbers of the form: | ||
371 | 29 | # LP: #1, #2, #3 | ||
372 | 30 | # #4, #5 | ||
373 | 31 | # over multiple lines. | ||
374 | 32 | # | ||
375 | 33 | # Writing a single catch-all regex for this has proved rather hard | ||
376 | 34 | # so I am taking the strategy of matching LP:(group) first, and | ||
377 | 35 | # feeding the result into another regex to pull out the bug and | ||
378 | 36 | # bugnum groups. | ||
379 | 37 | unique_bug_matches = dict() | ||
380 | 38 | |||
381 | 39 | line_matches = re.finditer( | ||
382 | 40 | 'LP:\s*(?P<buglist>(.+?[^,]))($|\n)', text, | ||
383 | 41 | re.DOTALL | re.IGNORECASE) | ||
384 | 42 | |||
385 | 43 | for line_match in line_matches: | ||
386 | 44 | bug_matches = re.finditer( | ||
387 | 45 | '\s*((?P<bug>#(?P<bugnum>\d+)),?\s*)', | ||
388 | 46 | line_match.group('buglist')) | ||
389 | 47 | |||
390 | 48 | for bug_match in bug_matches: | ||
391 | 49 | bugnum = bug_match.group('bugnum') | ||
392 | 50 | if bugnum in unique_bug_matches: | ||
393 | 51 | # We got this bug already, ignore it. | ||
394 | 52 | continue | ||
395 | 53 | unique_bug_matches[bugnum] = bug_match | ||
396 | 54 | |||
397 | 55 | return unique_bug_matches | ||
398 | 56 | |||
399 | 57 | |||
400 | 58 | def linkify_bug_numbers(text): | ||
401 | 59 | """Linkify to a bug if LP: #number appears in the (changelog) text.""" | ||
402 | 60 | unique_bug_matches = extract_bug_numbers(text) | ||
403 | 61 | for bug_match in unique_bug_matches.values(): | ||
404 | 62 | replace_text = bug_match.group('bug') | ||
405 | 63 | if replace_text is not None: | ||
406 | 64 | # XXX julian 2008-01-10 | ||
407 | 65 | # Note that re.sub would be far more efficient to use | ||
408 | 66 | # instead of string.replace() but this requires a regex | ||
409 | 67 | # that matches everything in one go. We're also at danger | ||
410 | 68 | # of replacing the wrong thing if string.replace() finds | ||
411 | 69 | # other matching substrings. So for example in the | ||
412 | 70 | # string: | ||
413 | 71 | # "LP: #9, #999" | ||
414 | 72 | # replacing #9 with some HTML would also interfere with | ||
415 | 73 | # #999. The liklihood of this happening is very, very | ||
416 | 74 | # small, however. | ||
417 | 75 | text = text.replace( | ||
418 | 76 | replace_text, | ||
419 | 77 | FormattersAPI._linkify_substitution(bug_match)) | ||
420 | 78 | return text | ||
421 | 79 | |||
422 | 80 | |||
423 | 81 | def extract_email_addresses(text): | ||
424 | 82 | '''Unique email adresses in the text.''' | ||
425 | 83 | matches = re.finditer(FormattersAPI._re_email, text) | ||
426 | 84 | return list(set([match.group() for match in matches])) | ||
427 | 85 | |||
428 | 86 | |||
429 | 87 | def obfuscate_email(user, text): | ||
430 | 88 | """Obfuscate email addresses if the user is not logged in.""" | ||
431 | 89 | if not text: | ||
432 | 90 | # If there is nothing to obfuscate, the FormattersAPI | ||
433 | 91 | # will blow up, so just return. | ||
434 | 92 | return text | ||
435 | 93 | formatter = FormattersAPI(text) | ||
436 | 94 | if user: | ||
437 | 95 | return text | ||
438 | 96 | else: | ||
439 | 97 | return formatter.obfuscate_email() | ||
440 | 98 | |||
441 | 99 | |||
442 | 100 | def linkify_email(text, preloaded_person_data): | ||
443 | 101 | """Email addresses are linkified to point to the person's profile.""" | ||
444 | 102 | formatter = FormattersAPI(text) | ||
445 | 103 | return formatter.linkify_email(preloaded_person_data) | ||
446 | 104 | |||
447 | 105 | |||
448 | 106 | def linkify_changelog(user, changelog, preloaded_person_data=None): | ||
449 | 107 | """Linkify the changelog. | ||
450 | 108 | |||
451 | 109 | This obfuscates email addresses to anonymous users, linkifies | ||
452 | 110 | them for non-anonymous and links to the bug page for any bug | ||
453 | 111 | numbers mentioned. | ||
454 | 112 | """ | ||
455 | 113 | if changelog is None: | ||
456 | 114 | return '' | ||
457 | 115 | |||
458 | 116 | # Remove any email addresses if the user is not logged in. | ||
459 | 117 | changelog = obfuscate_email(user, changelog) | ||
460 | 118 | |||
461 | 119 | # CGI Escape the changelog here before further replacements | ||
462 | 120 | # insert HTML. Email obfuscation does not insert HTML but can insert | ||
463 | 121 | # characters that must be escaped. | ||
464 | 122 | changelog = cgi.escape(changelog) | ||
465 | 123 | |||
466 | 124 | # Any email addresses remaining in the changelog were not obfuscated, | ||
467 | 125 | # so we linkify them here. | ||
468 | 126 | changelog = linkify_email(changelog, preloaded_person_data) | ||
469 | 127 | |||
470 | 128 | # Ensure any bug numbers are linkified to the bug page. | ||
471 | 129 | changelog = linkify_bug_numbers(changelog) | ||
472 | 130 | |||
473 | 131 | return changelog | ||
474 | 132 | |||
475 | 133 | |||
476 | 19 | class SourcePackageReleaseView(LaunchpadView): | 134 | class SourcePackageReleaseView(LaunchpadView): |
477 | 20 | 135 | ||
478 | 21 | @property | 136 | @property |
479 | 22 | def changelog_entry(self): | 137 | def changelog_entry(self): |
480 | 23 | """Return a linkified changelog entry.""" | 138 | """Return a linkified changelog entry.""" |
482 | 24 | return self._linkify_changelog(self.context.changelog_entry) | 139 | return linkify_changelog(self.user, self.context.changelog_entry) |
483 | 25 | 140 | ||
484 | 26 | @property | 141 | @property |
485 | 27 | def change_summary(self): | 142 | def change_summary(self): |
486 | 28 | """Return a linkified change summary.""" | 143 | """Return a linkified change summary.""" |
582 | 29 | return self._linkify_changelog(self.context.change_summary) | 144 | return linkify_changelog(self.user, self.context.change_summary) |
488 | 30 | |||
489 | 31 | def _obfuscate_email(self, text): | ||
490 | 32 | """Obfuscate email addresses if the user is not logged in.""" | ||
491 | 33 | if not text: | ||
492 | 34 | # If there is nothing to obfuscate, the FormattersAPI | ||
493 | 35 | # will blow up, so just return. | ||
494 | 36 | return text | ||
495 | 37 | formatter = FormattersAPI(text) | ||
496 | 38 | if self.user: | ||
497 | 39 | return text | ||
498 | 40 | else: | ||
499 | 41 | return formatter.obfuscate_email() | ||
500 | 42 | |||
501 | 43 | def _linkify_email(self, text): | ||
502 | 44 | """Email addresses are linkified to point to the person's profile.""" | ||
503 | 45 | formatter = FormattersAPI(text) | ||
504 | 46 | return formatter.linkify_email() | ||
505 | 47 | |||
506 | 48 | def _linkify_bug_numbers(self, changelog): | ||
507 | 49 | """Linkify to a bug if LP: #number appears in the changelog text.""" | ||
508 | 50 | # FormattersAPI._linkify_substitution requires a match object | ||
509 | 51 | # that has named groups "bug" and "bugnum". The matching text for | ||
510 | 52 | # the "bug" group is used as the link text and "bugnum" forms part | ||
511 | 53 | # of the URL for the link to the bug. | ||
512 | 54 | |||
513 | 55 | # We need to match bug numbers of the form: | ||
514 | 56 | # LP: #1, #2, #3 | ||
515 | 57 | # #4, #5 | ||
516 | 58 | # over multiple lines. | ||
517 | 59 | # | ||
518 | 60 | # Writing a single catch-all regex for this has proved rather hard | ||
519 | 61 | # so I am taking the strategy of matching LP:(group) first, and | ||
520 | 62 | # feeding the result into another regex to pull out the bug and | ||
521 | 63 | # bugnum groups. | ||
522 | 64 | line_matches = re.finditer( | ||
523 | 65 | 'LP:\s*(?P<buglist>(.+?[^,]))($|\n)', changelog, | ||
524 | 66 | re.DOTALL | re.IGNORECASE) | ||
525 | 67 | seen_bugnums = set() | ||
526 | 68 | for line_match in line_matches: | ||
527 | 69 | bug_matches = re.finditer( | ||
528 | 70 | '\s*((?P<bug>#(?P<bugnum>\d+)),?\s*)', | ||
529 | 71 | line_match.group('buglist')) | ||
530 | 72 | for bug_match in bug_matches: | ||
531 | 73 | bugnum = bug_match.group('bug') | ||
532 | 74 | if bugnum in seen_bugnums: | ||
533 | 75 | # This bug was already replaced across the entire text of | ||
534 | 76 | # the changelog. | ||
535 | 77 | continue | ||
536 | 78 | seen_bugnums.add(bugnum) | ||
537 | 79 | replace_text = bug_match.group('bug') | ||
538 | 80 | if replace_text is not None: | ||
539 | 81 | # XXX julian 2008-01-10 | ||
540 | 82 | # Note that re.sub would be far more efficient to use | ||
541 | 83 | # instead of string.replace() but this requires a regex | ||
542 | 84 | # that matches everything in one go. We're also at danger | ||
543 | 85 | # of replacing the wrong thing if string.replace() finds | ||
544 | 86 | # other matching substrings. So for example in the | ||
545 | 87 | # string: | ||
546 | 88 | # "LP: #9, #999" | ||
547 | 89 | # replacing #9 with some HTML would also interfere with | ||
548 | 90 | # #999. The liklihood of this happening is very, very | ||
549 | 91 | # small, however. | ||
550 | 92 | changelog = changelog.replace( | ||
551 | 93 | replace_text, | ||
552 | 94 | FormattersAPI._linkify_substitution(bug_match)) | ||
553 | 95 | return changelog | ||
554 | 96 | |||
555 | 97 | def _linkify_changelog(self, changelog): | ||
556 | 98 | """Linkify the changelog. | ||
557 | 99 | |||
558 | 100 | This obfuscates email addresses to anonymous users, linkifies | ||
559 | 101 | them for non-anonymous and links to the bug page for any bug | ||
560 | 102 | numbers mentioned. | ||
561 | 103 | """ | ||
562 | 104 | if changelog is None: | ||
563 | 105 | return '' | ||
564 | 106 | |||
565 | 107 | # Remove any email addresses if the user is not logged in. | ||
566 | 108 | changelog = self._obfuscate_email(changelog) | ||
567 | 109 | |||
568 | 110 | # CGI Escape the changelog here before further replacements | ||
569 | 111 | # insert HTML. Email obfuscation does not insert HTML but can insert | ||
570 | 112 | # characters that must be escaped. | ||
571 | 113 | changelog = cgi.escape(changelog) | ||
572 | 114 | |||
573 | 115 | # Any email addresses remaining in the changelog were not obfuscated, | ||
574 | 116 | # so we linkify them here. | ||
575 | 117 | changelog = self._linkify_email(changelog) | ||
576 | 118 | |||
577 | 119 | # Ensure any bug numbers are linkified to the bug page. | ||
578 | 120 | changelog = self._linkify_bug_numbers(changelog) | ||
579 | 121 | |||
580 | 122 | return changelog | ||
581 | 123 | |||
583 | 124 | 145 | ||
584 | === modified file 'lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt' | |||
585 | --- lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt 2009-09-23 14:58:12 +0000 | |||
586 | +++ lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt 2009-10-22 21:21:16 +0000 | |||
587 | @@ -270,3 +270,37 @@ | |||
588 | 270 | Binary packages built by this source | 270 | Binary packages built by this source |
589 | 271 | foo-bin: Foo app is great | 271 | foo-bin: Foo app is great |
590 | 272 | Well ... it does nothing, though | 272 | Well ... it does nothing, though |
591 | 273 | |||
592 | 274 | The full change log can be viewed as follows: | ||
593 | 275 | |||
594 | 276 | >>> user_browser.open( | ||
595 | 277 | ... 'http://launchpad.dev/ubuntutest/+source/testing-dspr') | ||
596 | 278 | >>> user_browser.getLink('View full change log').click() | ||
597 | 279 | >>> print user_browser.url | ||
598 | 280 | http://launchpad.dev/ubuntutest/+source/testing-dspr/+changelog | ||
599 | 281 | |||
600 | 282 | >>> print_tag_with_id(user_browser.contents, 'body_testing-dspr_1.0') | ||
601 | 283 | Testing!!! | ||
602 | 284 | biscuit@canonical.com | ||
603 | 285 | LP: #1234 | ||
604 | 286 | celso.providelo@canonical.com | ||
605 | 287 | |||
606 | 288 | >>> print extract_text(find_main_content(user_browser.contents)) | ||
607 | 289 | Change log for “testing-dspr” package in ubuntutest | ||
608 | 290 | ubuntutest | ||
609 | 291 | “testing-dspr” package | ||
610 | 292 | Change log | ||
611 | 293 | 0.9 | ||
612 | 294 | Pending in breezy-autotest-release | ||
613 | 295 | since ... | ||
614 | 296 | None | ||
615 | 297 | 1.0 | ||
616 | 298 | Published in breezy-autotest-release | ||
617 | 299 | 2 seconds ago | ||
618 | 300 | Testing!!! | ||
619 | 301 | biscuit@canonical.com | ||
620 | 302 | LP: #1234 | ||
621 | 303 | celso.providelo@canonical.com | ||
622 | 304 | Available diffs | ||
623 | 305 | 0.9 to 1.0 | ||
624 | 306 | (7 bytes) | ||
625 | 273 | 307 | ||
626 | === modified file 'lib/lp/soyuz/templates/distributionsourcepackagerelease-changes.pt' | |||
627 | --- lib/lp/soyuz/templates/distributionsourcepackagerelease-changes.pt 2009-07-17 17:59:07 +0000 | |||
628 | +++ lib/lp/soyuz/templates/distributionsourcepackagerelease-changes.pt 2009-10-22 21:21:16 +0000 | |||
629 | @@ -20,10 +20,11 @@ | |||
630 | 20 | </div> | 20 | </div> |
631 | 21 | 21 | ||
632 | 22 | <div class="boardCommentBody"> | 22 | <div class="boardCommentBody"> |
636 | 23 | <div | 23 | <div tal:attributes="id string:body_${context/name}_${context/version}"> |
637 | 24 | tal:attributes="id string:body_${context/name}_${context/version}" | 24 | <pre style="margin: 0 0 0.3em 0" |
638 | 25 | tal:content="structure context/sourcepackagerelease/@@+change_summary" /> | 25 | tal:attributes="id string:${context/name}_${context/version}" |
639 | 26 | tal:content="structure context/change_summary" /> | ||
640 | 27 | </div> | ||
641 | 26 | <tal:diffs replace="structure context/@@+diffs" /> | 28 | <tal:diffs replace="structure context/@@+diffs" /> |
642 | 27 | </div> | 29 | </div> |
643 | 28 | |||
644 | 29 | </div> | 30 | </div> |
645 | 30 | 31 | ||
646 | === removed file 'lib/lp/soyuz/templates/sourcepackagerelease-change-summary.pt' | |||
647 | --- lib/lp/soyuz/templates/sourcepackagerelease-change-summary.pt 2009-07-17 17:59:07 +0000 | |||
648 | +++ lib/lp/soyuz/templates/sourcepackagerelease-change-summary.pt 1970-01-01 00:00:00 +0000 | |||
649 | @@ -1,10 +0,0 @@ | |||
650 | 1 | <tal:root | ||
651 | 2 | xmlns:tal="http://xml.zope.org/namespaces/tal" | ||
652 | 3 | xmlns:metal="http://xml.zope.org/namespaces/metal" | ||
653 | 4 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" | ||
654 | 5 | omit-tag=""> | ||
655 | 6 | |||
656 | 7 | <pre style="margin: 0 0 0.3em 0" | ||
657 | 8 | tal:attributes="id string:${context/name}_${context/version}" | ||
658 | 9 | tal:content="structure view/change_summary" /> | ||
659 | 10 | </tal:root> |
Hello there!
The branch at hand optimises pages with a long changelog/history like:
https:/ /launchpad. net/ubuntu/ +source/ linux/+ changelog
These pages time out on a regular basis due to the number of database
queries issued in order to fetch the bugs/persons needed for the so
called "linkification".
You can see what such a page looks like by following the link below
(since it's less prone to time out).
https:/ /launchpad. net/ubuntu/ +source/ bash/+changelog
The optimisations introduced are twofold:
- the numbers of the bugs that appear on the page are all extracted PI.linkify_ email() in order to avoid many/repeated
and a *single* database query that pre-loads them into the storm
cache is issued before the linkification starts.
- Analogously, the email addresses are extracted from the page and an
email address/person cache is provided to
FormattersA
database lookups.
Please note that
- the bulk of the code was merely refactored i.e. most of the private ageReleaseView` methods have been converted into
`SourcePack
functions so they can be reused.
- Deryck consented to having the getByNumbers() method added to IBugSet.
Pre-implementation calls with bigjools and jtv.
Tests to run:
bin/test -v -t xx-distribution sourcepackagere lease-pages. txt -t sourcepackage
More importantly, this branch was tested thoroughly on the Soyuz dogfood
system. The resulting improvements are as follows:
- the number of database queries was reduced by approx. 66%
- the overall page loading time was reduced by approx. 33%