Launchpad itself

Merge lp:~bac/launchpad/bug-590813 into lp:launchpad

  1. bug-590813
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 11332
Proposed branch: lp:~bac/launchpad/bug-590813
Merge into: lp:launchpad
Diff against target: 301 lines (+95/-27)
7 files modified
lib/lp/app/browser/stringformatter.py (+6/-4)
lib/lp/app/browser/tests/root-views.txt (+1/-1)
lib/lp/registry/interfaces/distribution.py (+15/-12)
lib/lp/registry/interfaces/product.py (+9/-5)
lib/lp/registry/tests/test_distribution.py (+31/-0)
lib/lp/registry/tests/test_person.py (+4/-4)
lib/lp/registry/tests/test_product.py (+29/-1)
To merge this branch: bzr merge lp:~bac/launchpad/bug-590813
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) Approve
Review via email: mp+32248@code.launchpad.net

Commit message

Don't snapshot collections in IProduct and IDistribution.

Description of the change

= Summary =

Some products have lots of series...sometimes over 1000, which breaks
ShortList. Snapshotting those series is a huge waste of time.

== Proposed fix ==

Add 'doNotSnapshot' to the interface definition of CollectionFields in
IProduct and IDistribution.

Also did some drive-by lint, spelling, and grammar fixes.

== Pre-implementation notes ==

Talk with Curtis.

== Implementation details ==

As above.

== Tests ==

bin/test -vvt test_snapshot

== Demo and Q/A ==

Nope

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/browser/stringformatter.py
  lib/lp/registry/interfaces/product.py
  lib/lp/registry/interfaces/distribution.py
  lib/lp/registry/tests/test_distribution.py
  lib/lp/registry/tests/test_person.py
  lib/lp/registry/tests/test_product.py
  lib/lp/app/browser/tests/root-views.txt

./lib/lp/app/browser/stringformatter.py
     405: E501 line too long (90 characters)
     409: E501 line too long (88 characters)
     405: Line exceeds 78 characters.
     409: Line exceeds 78 characters.
./lib/lp/registry/interfaces/product.py
     903: E301 expected 1 blank line, found 2

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/browser/stringformatter.py'
2--- lib/lp/app/browser/stringformatter.py 2010-06-14 04:29:01 +0000
3+++ lib/lp/app/browser/stringformatter.py 2010-08-12 19:37:44 +0000
4@@ -156,6 +156,7 @@
5
6 The text may contain entity references or HTML tags.
7 """
8+
9 def replace(match):
10 if match.group('tag'):
11 return match.group()
12@@ -176,7 +177,7 @@
13
14 def nl_to_br(self):
15 """Quote HTML characters, then replace newlines with <br /> tags."""
16- return cgi.escape(self._stringtoformat).replace('\n','<br />\n')
17+ return cgi.escape(self._stringtoformat).replace('\n', '<br />\n')
18
19 def escape(self):
20 return escape(self._stringtoformat)
21@@ -580,7 +581,7 @@
22 if in_fold and line.endswith('</p>') and in_false_paragraph:
23 # The line ends with a false paragraph in a PGP signature.
24 # Restore the line break to join with the next paragraph.
25- line = '%s<br />\n<br />' % strip_trailing_p_tag(line)
26+ line = '%s<br />\n<br />' % strip_trailing_p_tag(line)
27 elif (in_quoted and self._re_quoted.match(line) is None):
28 # The line is not quoted like the previous line.
29 # End fold before we append this line.
30@@ -684,7 +685,8 @@
31 if person is not None and not person.hide_email_addresses:
32 # Circular dependancies now. Should be resolved by moving the
33 # object image display api.
34- from canonical.launchpad.webapp.tales import ObjectImageDisplayAPI
35+ from canonical.launchpad.webapp.tales import (
36+ ObjectImageDisplayAPI)
37 css_sprite = ObjectImageDisplayAPI(person).sprite_css()
38 text = text.replace(
39 address, '<a href="%s" class="%s">%s</a>' % (
40@@ -770,7 +772,7 @@
41 letter.
42
43 :param prefix: an optional string to prefix to the id. It can be
44- used to ensure that the start of the id is predicable.
45+ used to ensure that the start of the id is predictable.
46 """
47 if prefix is not None:
48 raw_text = prefix + self._stringtoformat
49
50=== modified file 'lib/lp/app/browser/tests/root-views.txt'
51--- lib/lp/app/browser/tests/root-views.txt 2010-01-08 21:23:15 +0000
52+++ lib/lp/app/browser/tests/root-views.txt 2010-08-12 19:37:44 +0000
53@@ -4,7 +4,7 @@
54 The launchpad front page uses the LaunchpadRootIndexView to provide the
55 special data needed for the layout.
56
57- # The _get_day_of_year() method must be hacked to return a predicable day
58+ # The _get_day_of_year() method must be hacked to return a predictable day
59 # to testing the view.
60 >>> from lp.app.browser.root import LaunchpadRootIndexView
61 >>> def day():
62
63=== modified file 'lib/lp/registry/interfaces/distribution.py'
64--- lib/lp/registry/interfaces/distribution.py 2010-08-06 11:52:05 +0000
65+++ lib/lp/registry/interfaces/distribution.py 2010-08-12 19:37:44 +0000
66@@ -23,6 +23,7 @@
67 from zope.schema import Bool, Choice, Datetime, List, Object, Text, TextLine
68 from zope.interface import Attribute, Interface
69
70+from lazr.lifecycle.snapshot import doNotSnapshot
71 from lazr.restful.fields import CollectionField, Reference
72 from lazr.restful.declarations import (
73 collection_default_content, export_as_webservice_collection,
74@@ -200,25 +201,27 @@
75 lucilleconfig = TextLine(
76 title=_("Lucille Config"),
77 description=_("The Lucille Config."), required=False)
78- archive_mirrors = exported(CollectionField(
79- description=_("All enabled and official ARCHIVE mirrors of this "
80- "Distribution."),
81- readonly=True, value_type=Object(schema=IDistributionMirror)))
82- cdimage_mirrors = exported(CollectionField(
83- description=_("All enabled and official RELEASE mirrors of this "
84- "Distribution."),
85- readonly=True, value_type=Object(schema=IDistributionMirror)))
86+ archive_mirrors = exported(doNotSnapshot(
87+ CollectionField(
88+ description=_("All enabled and official ARCHIVE mirrors "
89+ "of this Distribution."),
90+ readonly=True, value_type=Object(schema=IDistributionMirror))))
91+ cdimage_mirrors = exported(doNotSnapshot(
92+ CollectionField(
93+ description=_("All enabled and official RELEASE mirrors "
94+ "of this Distribution."),
95+ readonly=True, value_type=Object(schema=IDistributionMirror))))
96 disabled_mirrors = Attribute(
97 "All disabled and official mirrors of this Distribution.")
98 unofficial_mirrors = Attribute(
99 "All unofficial mirrors of this Distribution.")
100 pending_review_mirrors = Attribute(
101 "All mirrors of this Distribution that haven't been reviewed yet.")
102- series = exported(
103+ series = exported(doNotSnapshot(
104 CollectionField(
105 title=_("DistroSeries inside this Distribution"),
106 # Really IDistroSeries, see _schema_circular_imports.py.
107- value_type=Reference(schema=Interface)))
108+ value_type=Reference(schema=Interface))))
109 architectures = List(
110 title=_("DistroArchSeries inside this Distribution"))
111 uploaders = Attribute(_(
112@@ -262,11 +265,11 @@
113 # Really IArchive, see _schema_circular_imports.py.
114 schema=Interface))
115
116- all_distro_archives = exported(
117+ all_distro_archives = exported(doNotSnapshot(
118 CollectionField(
119 title=_("A sequence of the distribution's non-PPA Archives."),
120 readonly=True, required=False,
121- value_type=Reference(schema=Interface)),
122+ value_type=Reference(schema=Interface))),
123 # Really IArchive, see _schema_circular_imports.py.
124 exported_as='archives')
125
126
127=== modified file 'lib/lp/registry/interfaces/product.py'
128--- lib/lp/registry/interfaces/product.py 2010-08-06 14:49:35 +0000
129+++ lib/lp/registry/interfaces/product.py 2010-08-12 19:37:44 +0000
130@@ -74,6 +74,7 @@
131 ITranslationPolicy)
132 from canonical.launchpad.validators import LaunchpadValidationError
133 from canonical.launchpad.validators.name import name_validator
134+from lazr.lifecycle.snapshot import doNotSnapshot
135 from lazr.restful.fields import CollectionField, Reference, ReferenceChoice
136 from lazr.restful.declarations import (
137 REQUEST_USER, call_with, collection_default_content,
138@@ -585,7 +586,8 @@
139 "this product"))
140
141 series = exported(
142- CollectionField(value_type=Object(schema=IProductSeries)))
143+ doNotSnapshot(
144+ CollectionField(value_type=Object(schema=IProductSeries))))
145
146 development_focus = exported(
147 ReferenceChoice(
148@@ -602,10 +604,12 @@
149 "product; otherwise, simply returns the product name."))
150
151 releases = exported(
152- CollectionField(
153- title=_("An iterator over the ProductReleases for this product."),
154- readonly=True,
155- value_type=Reference(schema=IProductRelease)))
156+ doNotSnapshot(
157+ CollectionField(
158+ title=_("An iterator over the ProductReleases for "
159+ "this product."),
160+ readonly=True,
161+ value_type=Reference(schema=IProductRelease))))
162
163 translation_focus = exported(
164 ReferenceChoice(
165
166=== modified file 'lib/lp/registry/tests/test_distribution.py'
167--- lib/lp/registry/tests/test_distribution.py 2010-08-03 15:15:16 +0000
168+++ lib/lp/registry/tests/test_distribution.py 2010-08-12 19:37:44 +0000
169@@ -7,10 +7,13 @@
170
171 from zope.security.proxy import removeSecurityProxy
172
173+from lazr.lifecycle.snapshot import Snapshot
174 from lp.registry.tests.test_distroseries import (
175 TestDistroSeriesCurrentSourceReleases)
176 from lp.registry.interfaces.distroseries import NoSuchDistroSeries
177 from lp.registry.interfaces.series import SeriesStatus
178+from lp.registry.interfaces.distribution import IDistribution
179+
180 from lp.soyuz.interfaces.distributionsourcepackagerelease import (
181 IDistributionSourcePackageRelease)
182 from lp.testing import TestCaseWithFactory
183@@ -140,3 +143,31 @@
184 series = self.factory.makeDistroSeries(distribution=distro,
185 name="dappere", version="42.6")
186 self.assertEquals(series, distro.getSeries("42.6"))
187+
188+
189+class DistroSnapshotTestCase(TestCaseWithFactory):
190+ """A TestCase for distribution snapshots."""
191+
192+ layer = LaunchpadFunctionalLayer
193+
194+ def setUp(self):
195+ super(DistroSnapshotTestCase, self).setUp()
196+ self.distribution = self.factory.makeDistribution(name="boobuntu")
197+
198+ def test_snapshot(self):
199+ """Snapshots of products should not include marked attribues.
200+
201+ Wrap an export with 'doNotSnapshot' to force the snapshot to not
202+ include that attribute.
203+ """
204+ snapshot = Snapshot(self.distribution, providing=IDistribution)
205+ omitted = [
206+ 'archive_mirrors',
207+ 'cdimage_mirrors',
208+ 'series',
209+ 'all_distro_archives',
210+ ]
211+ for attribute in omitted:
212+ self.assertFalse(
213+ hasattr(snapshot, attribute),
214+ "Snapshot should not include %s." % attribute)
215
216=== modified file 'lib/lp/registry/tests/test_person.py'
217--- lib/lp/registry/tests/test_person.py 2010-08-05 13:23:52 +0000
218+++ lib/lp/registry/tests/test_person.py 2010-08-12 19:37:44 +0000
219@@ -249,8 +249,8 @@
220 transaction.commit()
221 logout()
222
223- def _get_testible_account(self, person, date_created, openid_identifier):
224- # Return a naked account with predicable attributes.
225+ def _get_testable_account(self, person, date_created, openid_identifier):
226+ # Return a naked account with predictable attributes.
227 account = removeSecurityProxy(person.account)
228 account.date_created = date_created
229 account.openid_identifier = openid_identifier
230@@ -264,7 +264,7 @@
231 2010, 04, 01, 0, 0, 0, 0, tzinfo=pytz.timezone('UTC'))
232 # Free an OpenID identifier using merge.
233 first_duplicate = self.factory.makePerson()
234- first_account = self._get_testible_account(
235+ first_account = self._get_testable_account(
236 first_duplicate, test_date, test_identifier)
237 first_person = self.factory.makePerson()
238 self._do_premerge(first_duplicate, first_person)
239@@ -276,7 +276,7 @@
240 # Create an account that reuses the freed OpenID_identifier.
241 test_date = test_date.replace(2010, 05)
242 second_duplicate = self.factory.makePerson()
243- second_account = self._get_testible_account(
244+ second_account = self._get_testable_account(
245 second_duplicate, test_date, test_identifier)
246 second_person = self.factory.makePerson()
247 self._do_premerge(second_duplicate, second_person)
248
249=== modified file 'lib/lp/registry/tests/test_product.py'
250--- lib/lp/registry/tests/test_product.py 2010-08-02 20:24:24 +0000
251+++ lib/lp/registry/tests/test_product.py 2010-08-12 19:37:44 +0000
252@@ -18,14 +18,16 @@
253
254 from canonical.launchpad.ftests import syncUpdate
255
256+from lazr.lifecycle.snapshot import Snapshot
257 from lp.registry.interfaces.person import IPersonSet
258-from lp.registry.interfaces.product import License
259+from lp.registry.interfaces.product import IProduct, License
260 from lp.registry.model.product import Product
261 from lp.registry.model.productlicense import ProductLicense
262 from lp.registry.model.commercialsubscription import (
263 CommercialSubscription)
264 from lp.testing import TestCaseWithFactory
265
266+
267 class TestProduct(TestCaseWithFactory):
268 """Tests product object."""
269
270@@ -178,6 +180,32 @@
271 'new')
272
273
274+class ProductSnapshotTestCase(TestCaseWithFactory):
275+ """A TestCase for product snapshots."""
276+
277+ layer = LaunchpadFunctionalLayer
278+
279+ def setUp(self):
280+ super(ProductSnapshotTestCase, self).setUp()
281+ self.product = self.factory.makeProduct(name="shamwow")
282+
283+ def test_snapshot(self):
284+ """Snapshots of products should not include marked attribues.
285+
286+ Wrap an export with 'doNotSnapshot' to force the snapshot to not
287+ include that attribute.
288+ """
289+ snapshot = Snapshot(self.product, providing=IProduct)
290+ omitted = [
291+ 'series',
292+ 'releases',
293+ ]
294+ for attribute in omitted:
295+ self.assertFalse(
296+ hasattr(snapshot, attribute),
297+ "Snapshot should not include %s." % attribute)
298+
299+
300 class BugSupervisorTestCase(TestCaseWithFactory):
301 """A TestCase for bug supervisor management."""
302