Merge lp:~al-maisan/launchpad/clog-oops-452070 into lp:launchpad

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
Reviewer Review Type Date Requested Status
Eleanor Berger (community) Approve
Review via email: mp+13789@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

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
    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
    FormattersAPI.linkify_email() in order to avoid many/repeated
    database lookups.

Please note that

  - the bulk of the code was merely refactored i.e. most of the private
    `SourcePackageReleaseView` methods have been converted into
    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-distributionsourcepackagerelease-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%

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
=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py 2009-09-18 21:54:45 +0000
+++ lib/canonical/launchpad/webapp/tales.py 2009-10-22 21:21:16 +0000
@@ -2876,7 +2876,7 @@
2876 "<<email address hidden>>", "<email address hidden>")2876 "<<email address hidden>>", "<email address hidden>")
2877 return text2877 return text
28782878
2879 def linkify_email(self):2879 def linkify_email(self, preloaded_person_data=None):
2880 """Linkify any email address recognised in Launchpad.2880 """Linkify any email address recognised in Launchpad.
28812881
2882 If an email address is recognised as one registered in Launchpad,2882 If an email address is recognised as one registered in Launchpad,
@@ -2891,7 +2891,14 @@
2891 matches = re.finditer(self._re_email, text)2891 matches = re.finditer(self._re_email, text)
2892 for match in matches:2892 for match in matches:
2893 address = match.group()2893 address = match.group()
2894 person = getUtility(IPersonSet).getByEmail(address)2894 person = None
2895 # First try to find the person required in the preloaded person
2896 # data dictionary.
2897 if preloaded_person_data is not None:
2898 person = preloaded_person_data.get(address, None)
2899 else:
2900 # No pre-loaded data -> we need to perform a database lookup.
2901 person = getUtility(IPersonSet).getByEmail(address)
2895 # Only linkify if person exists and does not want to hide2902 # Only linkify if person exists and does not want to hide
2896 # their email addresses.2903 # their email addresses.
2897 if person is not None and not person.hide_email_addresses:2904 if person is not None and not person.hide_email_addresses:
28982905
=== modified file 'lib/lp/bugs/doc/bug.txt'
--- lib/lp/bugs/doc/bug.txt 2009-07-08 03:14:59 +0000
+++ lib/lp/bugs/doc/bug.txt 2009-10-22 21:21:16 +0000
@@ -37,6 +37,34 @@
37 ...37 ...
38 NotFoundError: 'Unable to locate bug with nickname +bugs.'38 NotFoundError: 'Unable to locate bug with nickname +bugs.'
3939
40It is also possible to retrieve a number of bugs by specifying the bug numbers
41of interest.
42
43 >>> result_set = bugset.getByNumbers([6, 1234])
44 >>> print result_set.count()
45 1
46
47 >>> [the_bug_found] = result_set
48 >>> print the_bug_found.title
49 Firefox crashes when Save As dialog for a nonexistent window is closed
50
51 >>> result_set = bugset.getByNumbers([6, 4321, 1])
52 >>> print result_set.count()
53 2
54
55 >>> second_bug_found = result_set[1]
56 >>> print second_bug_found.title
57 Firefox does not support SVG
58
59If no bug numbers are specified an empty result set is returned.
60
61 >>> result_set = bugset.getByNumbers(None)
62 >>> print result_set.count()
63 0
64 >>> result_set = bugset.getByNumbers([])
65 >>> print result_set.count()
66 0
67
40== Bug creation events ==68== Bug creation events ==
4169
42IObjectCreatedEvent events are fired off when a bug is created. First70IObjectCreatedEvent events are fired off when a bug is created. First
4371
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2009-08-26 11:06:05 +0000
+++ lib/lp/bugs/interfaces/bug.py 2009-10-22 21:21:16 +0000
@@ -937,6 +937,13 @@
937 :param limit: The number of distinct Bugs to return.937 :param limit: The number of distinct Bugs to return.
938 """938 """
939939
940 def getByNumbers(bug_numbers):
941 """Get `IBug` instances identified by the `bug_numbers` iterable.
942
943 :param bug_numbers: An iterable of bug numbers for which we should
944 return Bugs.
945 """
946
940947
941class InvalidBugTargetType(Exception):948class InvalidBugTargetType(Exception):
942 """Bug target's type is not valid."""949 """Bug target's type is not valid."""
943950
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2009-08-26 01:54:39 +0000
+++ lib/lp/bugs/model/bug.py 2009-10-22 21:21:16 +0000
@@ -33,7 +33,7 @@
33from sqlobject import SQLMultipleJoin, SQLRelatedJoin33from sqlobject import SQLMultipleJoin, SQLRelatedJoin
34from sqlobject import SQLObjectNotFound34from sqlobject import SQLObjectNotFound
35from storm.expr import And, Count, In, LeftJoin, Select, SQLRaw, Func35from storm.expr import And, Count, In, LeftJoin, Select, SQLRaw, Func
36from storm.store import Store36from storm.store import EmptyResultSet, Store
3737
38from lazr.lifecycle.event import (38from lazr.lifecycle.event import (
39 ObjectCreatedEvent, ObjectDeletedEvent, ObjectModifiedEvent)39 ObjectCreatedEvent, ObjectDeletedEvent, ObjectModifiedEvent)
@@ -49,6 +49,7 @@
49from canonical.launchpad.interfaces.hwdb import IHWSubmissionBugSet49from canonical.launchpad.interfaces.hwdb import IHWSubmissionBugSet
50from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities50from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
51from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet51from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
52from canonical.launchpad.interfaces.lpstorm import IStore
52from canonical.launchpad.interfaces.message import (53from canonical.launchpad.interfaces.message import (
53 IMessage, IndexedMessage)54 IMessage, IndexedMessage)
54from canonical.launchpad.interfaces.structuralsubscription import (55from canonical.launchpad.interfaces.structuralsubscription import (
@@ -1627,6 +1628,14 @@
16271628
1628 return bugs1629 return bugs
16291630
1631 def getByNumbers(self, bug_numbers):
1632 """see `IBugSet`."""
1633 if bug_numbers is None or len(bug_numbers) < 1:
1634 return EmptyResultSet()
1635 store = IStore(Bug)
1636 result_set = store.find(Bug, In(Bug.id, bug_numbers))
1637 return result_set
1638
16301639
1631class BugAffectsPerson(SQLBase):1640class BugAffectsPerson(SQLBase):
1632 """A bug is marked as affecting a user."""1641 """A bug is marked as affecting a user."""
16331642
=== modified file 'lib/lp/registry/browser/distributionsourcepackage.py'
--- lib/lp/registry/browser/distributionsourcepackage.py 2009-10-20 17:57:51 +0000
+++ lib/lp/registry/browser/distributionsourcepackage.py 2009-10-22 21:21:16 +0000
@@ -27,6 +27,7 @@
2727
28from canonical.cachedproperty import cachedproperty28from canonical.cachedproperty import cachedproperty
29from canonical.launchpad import _29from canonical.launchpad import _
30from canonical.launchpad.interfaces import IBugSet
30from lp.answers.interfaces.questionenums import QuestionStatus31from lp.answers.interfaces.questionenums import QuestionStatus
31from lp.soyuz.interfaces.archive import IArchiveSet32from lp.soyuz.interfaces.archive import IArchiveSet
32from lp.soyuz.interfaces.distributionsourcepackagerelease import (33from lp.soyuz.interfaces.distributionsourcepackagerelease import (
@@ -49,6 +50,8 @@
49from canonical.launchpad.webapp.breadcrumb import Breadcrumb50from canonical.launchpad.webapp.breadcrumb import Breadcrumb
5051
51from lazr.delegates import delegates52from lazr.delegates import delegates
53from lp.soyuz.browser.sourcepackagerelease import (
54 extract_bug_numbers, extract_email_addresses, linkify_changelog)
52from canonical.lazr.utils import smartquote55from canonical.lazr.utils import smartquote
5356
5457
@@ -134,11 +137,14 @@
134 """137 """
135 delegates(IDistributionSourcePackageRelease, 'context')138 delegates(IDistributionSourcePackageRelease, 'context')
136139
137 def __init__(self, distributionsourcepackagerelease,140 def __init__(
138 publishing_history, package_diffs):141 self, distributionsourcepackagerelease, publishing_history,
142 package_diffs, person_data, user):
139 self.context = distributionsourcepackagerelease143 self.context = distributionsourcepackagerelease
140 self._publishing_history = publishing_history144 self._publishing_history = publishing_history
141 self._package_diffs = package_diffs145 self._package_diffs = package_diffs
146 self._person_data = person_data
147 self._user = user
142148
143 @property149 @property
144 def publishing_history(self):150 def publishing_history(self):
@@ -150,6 +156,12 @@
150 """ See `ISourcePackageRelease`."""156 """ See `ISourcePackageRelease`."""
151 return self._package_diffs157 return self._package_diffs
152158
159 @property
160 def change_summary(self):
161 """ See `ISourcePackageRelease`."""
162 return linkify_changelog(
163 self._user, self.context.change_summary, self._person_data)
164
153165
154class IDistributionSourcePackageActionMenu(Interface):166class IDistributionSourcePackageActionMenu(Interface):
155 """Marker interface for the action menu."""167 """Marker interface for the action menu."""
@@ -172,14 +184,43 @@
172 """Common features to all `DistributionSourcePackage` views."""184 """Common features to all `DistributionSourcePackage` views."""
173185
174 def releases(self):186 def releases(self):
187 def not_empty(text):
188 return (
189 text is not None and isinstance(text, basestring)
190 and len(text.strip()) > 0)
191
175 dspr_pubs = self.context.getReleasesAndPublishingHistory()192 dspr_pubs = self.context.getReleasesAndPublishingHistory()
176193
177 # Return as early as possible to avoid unnecessary processing.194 # Return as early as possible to avoid unnecessary processing.
178 if len(dspr_pubs) == 0:195 if len(dspr_pubs) == 0:
179 return []196 return []
180197
198 sprs = [dspr.sourcepackagerelease for (dspr, spphs) in dspr_pubs]
199 # Pre-load the bugs and persons referenced by the +changelog page from
200 # the database.
201 # This will improve the performance of the ensuing changelog
202 # linkification.
203 the_changelog = '\n'.join(
204 [spr.changelog_entry for spr in sprs
205 if not_empty(spr.changelog_entry)])
206 unique_bugs = extract_bug_numbers(the_changelog)
207 self._bug_data = list(
208 getUtility(IBugSet).getByNumbers(unique_bugs.keys()))
209 # Preload email/person data only if user is logged on. In the opposite
210 # case the emails in the changelog will be obfuscated anyway and thus
211 # cause no database lookups.
212 if self.user:
213 unique_emails = extract_email_addresses(the_changelog)
214 # The method below returns a [(EmailAddress,Person]] result set.
215 result_set = self.context.getPersonsByEmail(unique_emails)
216 # Ignore the persons who want their email addresses hidden.
217 self._person_data = dict(
218 [(email.email,person) for (email,person) in result_set
219 if not person.hide_email_addresses])
220 else:
221 self._person_data = None
222
181 # Collate diffs for relevant SourcePackageReleases223 # Collate diffs for relevant SourcePackageReleases
182 sprs = [dspr.sourcepackagerelease for (dspr, spphs) in dspr_pubs]
183 pkg_diffs = getUtility(IPackageDiffSet).getDiffsToReleases(sprs)224 pkg_diffs = getUtility(IPackageDiffSet).getDiffsToReleases(sprs)
184 spr_diffs = {}225 spr_diffs = {}
185 for spr, diffs in itertools.groupby(pkg_diffs,226 for spr, diffs in itertools.groupby(pkg_diffs,
@@ -188,7 +229,8 @@
188229
189 return [230 return [
190 DecoratedDistributionSourcePackageRelease(231 DecoratedDistributionSourcePackageRelease(
191 dspr, spphs, spr_diffs.get(dspr.sourcepackagerelease, []))232 dspr, spphs, spr_diffs.get(dspr.sourcepackagerelease, []),
233 self._person_data, self.user)
192 for (dspr, spphs) in dspr_pubs]234 for (dspr, spphs) in dspr_pubs]
193235
194236
195237
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2009-10-22 09:24:53 +0000
+++ lib/lp/registry/configure.zcml 2009-10-22 21:21:16 +0000
@@ -392,6 +392,7 @@
392 getSubscription392 getSubscription
393 parent_subscription_target393 parent_subscription_target
394 getBugNotificationsRecipients394 getBugNotificationsRecipients
395 getPersonsByEmail
395 getReleasesAndPublishingHistory396 getReleasesAndPublishingHistory
396 upstream_product397 upstream_product
397 target_type_display398 target_type_display
398399
=== modified file 'lib/lp/registry/doc/sourcepackage.txt'
--- lib/lp/registry/doc/sourcepackage.txt 2009-10-19 21:20:31 +0000
+++ lib/lp/registry/doc/sourcepackage.txt 2009-10-22 21:21:16 +0000
@@ -370,32 +370,34 @@
370370
371Zero or more spaces may appear between the ":" and the "#".371Zero or more spaces may appear between the ":" and the "#".
372372
373 >>> spr_view._linkify_bug_numbers("LP: #10")373 >>> from lp.soyuz.browser.sourcepackagerelease import (
374 ... linkify_bug_numbers)
375 >>> linkify_bug_numbers("LP: #10")
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>'
375377
376 >>> spr_view._linkify_bug_numbers("LP: #10")378 >>> linkify_bug_numbers("LP: #10")
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>'
378380
379 >>> spr_view._linkify_bug_numbers("LP:#10")381 >>> linkify_bug_numbers("LP:#10")
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>'
381383
382 >>> spr_view._linkify_bug_numbers("LP: #999")384 >>> linkify_bug_numbers("LP: #999")
383 'LP: <a href="/bugs/999" title="No such bug">#999</a>'385 'LP: <a href="/bugs/999" title="No such bug">#999</a>'
384386
385 >>> spr_view._linkify_bug_numbers("garbage")387 >>> linkify_bug_numbers("garbage")
386 'garbage'388 'garbage'
387389
388 >>> spr_view._linkify_bug_numbers("LP: #10, #7")390 >>> linkify_bug_numbers("LP: #10, #7")
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>,
390 <a href="/bugs/7" title="A test bug">#7</a>'392 <a href="/bugs/7" title="A test bug">#7</a>'
391393
392 >>> spr_view._linkify_bug_numbers("LP: #10 LP: #10")394 >>> linkify_bug_numbers("LP: #10 LP: #10")
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>
394 LP: <a href="/bugs/10" title="another test bug">#10</a>'396 LP: <a href="/bugs/10" title="another test bug">#10</a>'
395397
396The regex is case-insensitive, so "lp: #nnn" also works.398The regex is case-insensitive, so "lp: #nnn" also works.
397399
398 >>> spr_view._linkify_bug_numbers("lp: #10")400 >>> linkify_bug_numbers("lp: #10")
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>'
400402
401403
402404
=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py 2009-09-16 04:31:39 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py 2009-10-22 21:21:16 +0000
@@ -15,7 +15,8 @@
15import operator15import operator
1616
17from sqlobject.sqlbuilder import SQLConstant17from sqlobject.sqlbuilder import SQLConstant
18from storm.expr import And, Desc, In18from storm.expr import And, Desc, In, Join, Lower
19from storm.store import EmptyResultSet
19from storm.locals import Int, Reference, Store, Storm, Unicode20from storm.locals import Int, Reference, Store, Storm, Unicode
20from zope.interface import implements21from zope.interface import implements
2122
@@ -36,10 +37,13 @@
36from lp.soyuz.model.sourcepackagerelease import (37from lp.soyuz.model.sourcepackagerelease import (
37 SourcePackageRelease)38 SourcePackageRelease)
38from lp.registry.model.karma import KarmaTotalCache39from lp.registry.model.karma import KarmaTotalCache
40from lp.registry.model.person import Person
39from lp.registry.model.sourcepackage import (41from lp.registry.model.sourcepackage import (
40 SourcePackage, SourcePackageQuestionTargetMixin)42 SourcePackage, SourcePackageQuestionTargetMixin)
43from canonical.launchpad.database.emailaddress import EmailAddress
41from canonical.launchpad.database.structuralsubscription import (44from canonical.launchpad.database.structuralsubscription import (
42 StructuralSubscriptionTargetMixin)45 StructuralSubscriptionTargetMixin)
46from canonical.launchpad.interfaces.lpstorm import IStore
4347
44from canonical.lazr.utils import smartquote48from canonical.lazr.utils import smartquote
4549
@@ -408,6 +412,23 @@
408 'BugTask.distribution = %s AND BugTask.sourcepackagename = %s' %412 'BugTask.distribution = %s AND BugTask.sourcepackagename = %s' %
409 sqlvalues(self.distribution, self.sourcepackagename))413 sqlvalues(self.distribution, self.sourcepackagename))
410414
415 @staticmethod
416 def getPersonsByEmail(email_addresses):
417 """[(EmailAddress,Person), ..] iterable for given email addresses."""
418 if email_addresses is None or len(email_addresses) < 1:
419 return EmptyResultSet()
420 # Perform basic sanitization of email addresses.
421 email_addresses = [
422 address.lower().strip() for address in email_addresses]
423 store = IStore(Person)
424 origin = [
425 Person, Join(EmailAddress, EmailAddress.personID == Person.id)]
426 # Get all persons whose email addresses are in the list.
427 result_set = store.using(*origin).find(
428 (EmailAddress, Person),
429 In(Lower(EmailAddress.email), email_addresses))
430 return result_set
431
411432
412class DistributionSourcePackageInDatabase(Storm):433class DistributionSourcePackageInDatabase(Storm):
413 """Temporary class to allow access to the database."""434 """Temporary class to allow access to the database."""
414435
=== modified file 'lib/lp/soyuz/browser/configure.zcml'
--- lib/lp/soyuz/browser/configure.zcml 2009-09-29 07:21:40 +0000
+++ lib/lp/soyuz/browser/configure.zcml 2009-10-22 21:21:16 +0000
@@ -112,9 +112,6 @@
112 name="+files"112 name="+files"
113 facet="overview"113 facet="overview"
114 template="../templates/sourcepackagerelease-files.pt"/>114 template="../templates/sourcepackagerelease-files.pt"/>
115 <browser:page
116 name="+change_summary"
117 template="../templates/sourcepackagerelease-change-summary.pt"/>
118 </browser:pages>115 </browser:pages>
119 <facet116 <facet
120 facet="overview">117 facet="overview">
121118
=== modified file 'lib/lp/soyuz/browser/sourcepackagerelease.py'
--- lib/lp/soyuz/browser/sourcepackagerelease.py 2009-09-09 11:52:55 +0000
+++ lib/lp/soyuz/browser/sourcepackagerelease.py 2009-10-22 21:21:16 +0000
@@ -16,108 +16,129 @@
16from canonical.launchpad.webapp.tales import FormattersAPI16from canonical.launchpad.webapp.tales import FormattersAPI
1717
1818
19def extract_bug_numbers(text):
20 '''Unique bug numbers matching the "LP: #n(, #n)*" pattern in the text.'''
21 # FormattersAPI._linkify_substitution requires a match object
22 # that has named groups "bug" and "bugnum". The matching text for
23 # the "bug" group is used as the link text and "bugnum" forms part
24 # of the URL for the link to the bug. Example:
25 # >>> bm.groupdict( )
26 # {'bugnum': '400686', 'bug': '#400686'}
27
28 # We need to match bug numbers of the form:
29 # LP: #1, #2, #3
30 # #4, #5
31 # over multiple lines.
32 #
33 # Writing a single catch-all regex for this has proved rather hard
34 # so I am taking the strategy of matching LP:(group) first, and
35 # feeding the result into another regex to pull out the bug and
36 # bugnum groups.
37 unique_bug_matches = dict()
38
39 line_matches = re.finditer(
40 'LP:\s*(?P<buglist>(.+?[^,]))($|\n)', text,
41 re.DOTALL | re.IGNORECASE)
42
43 for line_match in line_matches:
44 bug_matches = re.finditer(
45 '\s*((?P<bug>#(?P<bugnum>\d+)),?\s*)',
46 line_match.group('buglist'))
47
48 for bug_match in bug_matches:
49 bugnum = bug_match.group('bugnum')
50 if bugnum in unique_bug_matches:
51 # We got this bug already, ignore it.
52 continue
53 unique_bug_matches[bugnum] = bug_match
54
55 return unique_bug_matches
56
57
58def linkify_bug_numbers(text):
59 """Linkify to a bug if LP: #number appears in the (changelog) text."""
60 unique_bug_matches = extract_bug_numbers(text)
61 for bug_match in unique_bug_matches.values():
62 replace_text = bug_match.group('bug')
63 if replace_text is not None:
64 # XXX julian 2008-01-10
65 # Note that re.sub would be far more efficient to use
66 # instead of string.replace() but this requires a regex
67 # that matches everything in one go. We're also at danger
68 # of replacing the wrong thing if string.replace() finds
69 # other matching substrings. So for example in the
70 # string:
71 # "LP: #9, #999"
72 # replacing #9 with some HTML would also interfere with
73 # #999. The liklihood of this happening is very, very
74 # small, however.
75 text = text.replace(
76 replace_text,
77 FormattersAPI._linkify_substitution(bug_match))
78 return text
79
80
81def extract_email_addresses(text):
82 '''Unique email adresses in the text.'''
83 matches = re.finditer(FormattersAPI._re_email, text)
84 return list(set([match.group() for match in matches]))
85
86
87def obfuscate_email(user, text):
88 """Obfuscate email addresses if the user is not logged in."""
89 if not text:
90 # If there is nothing to obfuscate, the FormattersAPI
91 # will blow up, so just return.
92 return text
93 formatter = FormattersAPI(text)
94 if user:
95 return text
96 else:
97 return formatter.obfuscate_email()
98
99
100def linkify_email(text, preloaded_person_data):
101 """Email addresses are linkified to point to the person's profile."""
102 formatter = FormattersAPI(text)
103 return formatter.linkify_email(preloaded_person_data)
104
105
106def linkify_changelog(user, changelog, preloaded_person_data=None):
107 """Linkify the changelog.
108
109 This obfuscates email addresses to anonymous users, linkifies
110 them for non-anonymous and links to the bug page for any bug
111 numbers mentioned.
112 """
113 if changelog is None:
114 return ''
115
116 # Remove any email addresses if the user is not logged in.
117 changelog = obfuscate_email(user, changelog)
118
119 # CGI Escape the changelog here before further replacements
120 # insert HTML. Email obfuscation does not insert HTML but can insert
121 # characters that must be escaped.
122 changelog = cgi.escape(changelog)
123
124 # Any email addresses remaining in the changelog were not obfuscated,
125 # so we linkify them here.
126 changelog = linkify_email(changelog, preloaded_person_data)
127
128 # Ensure any bug numbers are linkified to the bug page.
129 changelog = linkify_bug_numbers(changelog)
130
131 return changelog
132
133
19class SourcePackageReleaseView(LaunchpadView):134class SourcePackageReleaseView(LaunchpadView):
20135
21 @property136 @property
22 def changelog_entry(self):137 def changelog_entry(self):
23 """Return a linkified changelog entry."""138 """Return a linkified changelog entry."""
24 return self._linkify_changelog(self.context.changelog_entry)139 return linkify_changelog(self.user, self.context.changelog_entry)
25140
26 @property141 @property
27 def change_summary(self):142 def change_summary(self):
28 """Return a linkified change summary."""143 """Return a linkified change summary."""
29 return self._linkify_changelog(self.context.change_summary)144 return linkify_changelog(self.user, self.context.change_summary)
30
31 def _obfuscate_email(self, text):
32 """Obfuscate email addresses if the user is not logged in."""
33 if not text:
34 # If there is nothing to obfuscate, the FormattersAPI
35 # will blow up, so just return.
36 return text
37 formatter = FormattersAPI(text)
38 if self.user:
39 return text
40 else:
41 return formatter.obfuscate_email()
42
43 def _linkify_email(self, text):
44 """Email addresses are linkified to point to the person's profile."""
45 formatter = FormattersAPI(text)
46 return formatter.linkify_email()
47
48 def _linkify_bug_numbers(self, changelog):
49 """Linkify to a bug if LP: #number appears in the changelog text."""
50 # FormattersAPI._linkify_substitution requires a match object
51 # that has named groups "bug" and "bugnum". The matching text for
52 # the "bug" group is used as the link text and "bugnum" forms part
53 # of the URL for the link to the bug.
54
55 # We need to match bug numbers of the form:
56 # LP: #1, #2, #3
57 # #4, #5
58 # over multiple lines.
59 #
60 # Writing a single catch-all regex for this has proved rather hard
61 # so I am taking the strategy of matching LP:(group) first, and
62 # feeding the result into another regex to pull out the bug and
63 # bugnum groups.
64 line_matches = re.finditer(
65 'LP:\s*(?P<buglist>(.+?[^,]))($|\n)', changelog,
66 re.DOTALL | re.IGNORECASE)
67 seen_bugnums = set()
68 for line_match in line_matches:
69 bug_matches = re.finditer(
70 '\s*((?P<bug>#(?P<bugnum>\d+)),?\s*)',
71 line_match.group('buglist'))
72 for bug_match in bug_matches:
73 bugnum = bug_match.group('bug')
74 if bugnum in seen_bugnums:
75 # This bug was already replaced across the entire text of
76 # the changelog.
77 continue
78 seen_bugnums.add(bugnum)
79 replace_text = bug_match.group('bug')
80 if replace_text is not None:
81 # XXX julian 2008-01-10
82 # Note that re.sub would be far more efficient to use
83 # instead of string.replace() but this requires a regex
84 # that matches everything in one go. We're also at danger
85 # of replacing the wrong thing if string.replace() finds
86 # other matching substrings. So for example in the
87 # string:
88 # "LP: #9, #999"
89 # replacing #9 with some HTML would also interfere with
90 # #999. The liklihood of this happening is very, very
91 # small, however.
92 changelog = changelog.replace(
93 replace_text,
94 FormattersAPI._linkify_substitution(bug_match))
95 return changelog
96
97 def _linkify_changelog(self, changelog):
98 """Linkify the changelog.
99
100 This obfuscates email addresses to anonymous users, linkifies
101 them for non-anonymous and links to the bug page for any bug
102 numbers mentioned.
103 """
104 if changelog is None:
105 return ''
106
107 # Remove any email addresses if the user is not logged in.
108 changelog = self._obfuscate_email(changelog)
109
110 # CGI Escape the changelog here before further replacements
111 # insert HTML. Email obfuscation does not insert HTML but can insert
112 # characters that must be escaped.
113 changelog = cgi.escape(changelog)
114
115 # Any email addresses remaining in the changelog were not obfuscated,
116 # so we linkify them here.
117 changelog = self._linkify_email(changelog)
118
119 # Ensure any bug numbers are linkified to the bug page.
120 changelog = self._linkify_bug_numbers(changelog)
121
122 return changelog
123
124145
=== modified file 'lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt'
--- lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt 2009-09-23 14:58:12 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-distributionsourcepackagerelease-pages.txt 2009-10-22 21:21:16 +0000
@@ -270,3 +270,37 @@
270 Binary packages built by this source270 Binary packages built by this source
271 foo-bin: Foo app is great271 foo-bin: Foo app is great
272 Well ... it does nothing, though272 Well ... it does nothing, though
273
274The full change log can be viewed as follows:
275
276 >>> user_browser.open(
277 ... 'http://launchpad.dev/ubuntutest/+source/testing-dspr')
278 >>> user_browser.getLink('View full change log').click()
279 >>> print user_browser.url
280 http://launchpad.dev/ubuntutest/+source/testing-dspr/+changelog
281
282 >>> print_tag_with_id(user_browser.contents, 'body_testing-dspr_1.0')
283 Testing!!!
284 biscuit@canonical.com
285 LP: #1234
286 celso.providelo@canonical.com
287
288 >>> print extract_text(find_main_content(user_browser.contents))
289 Change log for “testing-dspr” package in ubuntutest
290 ubuntutest
291 “testing-dspr” package
292 Change log
293 0.9
294 Pending in breezy-autotest-release
295 since ...
296 None
297 1.0
298 Published in breezy-autotest-release
299 2 seconds ago
300 Testing!!!
301 biscuit@canonical.com
302 LP: #1234
303 celso.providelo@canonical.com
304 Available diffs
305 0.9 to 1.0
306 (7 bytes)
273307
=== modified file 'lib/lp/soyuz/templates/distributionsourcepackagerelease-changes.pt'
--- lib/lp/soyuz/templates/distributionsourcepackagerelease-changes.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/soyuz/templates/distributionsourcepackagerelease-changes.pt 2009-10-22 21:21:16 +0000
@@ -20,10 +20,11 @@
20 </div>20 </div>
2121
22 <div class="boardCommentBody">22 <div class="boardCommentBody">
23 <div23 <div tal:attributes="id string:body_${context/name}_${context/version}">
24 tal:attributes="id string:body_${context/name}_${context/version}"24 <pre style="margin: 0 0 0.3em 0"
25 tal:content="structure context/sourcepackagerelease/@@+change_summary" />25 tal:attributes="id string:${context/name}_${context/version}"
26 tal:content="structure context/change_summary" />
27 </div>
26 <tal:diffs replace="structure context/@@+diffs" />28 <tal:diffs replace="structure context/@@+diffs" />
27 </div>29 </div>
28
29</div>30</div>
3031
=== removed file 'lib/lp/soyuz/templates/sourcepackagerelease-change-summary.pt'
--- lib/lp/soyuz/templates/sourcepackagerelease-change-summary.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/soyuz/templates/sourcepackagerelease-change-summary.pt 1970-01-01 00:00:00 +0000
@@ -1,10 +0,0 @@
1<tal:root
2 xmlns:tal="http://xml.zope.org/namespaces/tal"
3 xmlns:metal="http://xml.zope.org/namespaces/metal"
4 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
5 omit-tag="">
6
7<pre style="margin: 0 0 0.3em 0"
8 tal:attributes="id string:${context/name}_${context/version}"
9 tal:content="structure view/change_summary" />
10</tal:root>