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
=== modified file 'lib/lp/app/browser/stringformatter.py'
--- lib/lp/app/browser/stringformatter.py 2010-06-14 04:29:01 +0000
+++ lib/lp/app/browser/stringformatter.py 2010-08-12 19:37:44 +0000
@@ -156,6 +156,7 @@
156156
157 The text may contain entity references or HTML tags.157 The text may contain entity references or HTML tags.
158 """158 """
159
159 def replace(match):160 def replace(match):
160 if match.group('tag'):161 if match.group('tag'):
161 return match.group()162 return match.group()
@@ -176,7 +177,7 @@
176177
177 def nl_to_br(self):178 def nl_to_br(self):
178 """Quote HTML characters, then replace newlines with <br /> tags."""179 """Quote HTML characters, then replace newlines with <br /> tags."""
179 return cgi.escape(self._stringtoformat).replace('\n','<br />\n')180 return cgi.escape(self._stringtoformat).replace('\n', '<br />\n')
180181
181 def escape(self):182 def escape(self):
182 return escape(self._stringtoformat)183 return escape(self._stringtoformat)
@@ -580,7 +581,7 @@
580 if in_fold and line.endswith('</p>') and in_false_paragraph:581 if in_fold and line.endswith('</p>') and in_false_paragraph:
581 # The line ends with a false paragraph in a PGP signature.582 # The line ends with a false paragraph in a PGP signature.
582 # Restore the line break to join with the next paragraph.583 # Restore the line break to join with the next paragraph.
583 line = '%s<br />\n<br />' % strip_trailing_p_tag(line)584 line = '%s<br />\n<br />' % strip_trailing_p_tag(line)
584 elif (in_quoted and self._re_quoted.match(line) is None):585 elif (in_quoted and self._re_quoted.match(line) is None):
585 # The line is not quoted like the previous line.586 # The line is not quoted like the previous line.
586 # End fold before we append this line.587 # End fold before we append this line.
@@ -684,7 +685,8 @@
684 if person is not None and not person.hide_email_addresses:685 if person is not None and not person.hide_email_addresses:
685 # Circular dependancies now. Should be resolved by moving the686 # Circular dependancies now. Should be resolved by moving the
686 # object image display api.687 # object image display api.
687 from canonical.launchpad.webapp.tales import ObjectImageDisplayAPI688 from canonical.launchpad.webapp.tales import (
689 ObjectImageDisplayAPI)
688 css_sprite = ObjectImageDisplayAPI(person).sprite_css()690 css_sprite = ObjectImageDisplayAPI(person).sprite_css()
689 text = text.replace(691 text = text.replace(
690 address, '<a href="%s" class="%s">%s</a>' % (692 address, '<a href="%s" class="%s">%s</a>' % (
@@ -770,7 +772,7 @@
770 letter.772 letter.
771773
772 :param prefix: an optional string to prefix to the id. It can be774 :param prefix: an optional string to prefix to the id. It can be
773 used to ensure that the start of the id is predicable.775 used to ensure that the start of the id is predictable.
774 """776 """
775 if prefix is not None:777 if prefix is not None:
776 raw_text = prefix + self._stringtoformat778 raw_text = prefix + self._stringtoformat
777779
=== modified file 'lib/lp/app/browser/tests/root-views.txt'
--- lib/lp/app/browser/tests/root-views.txt 2010-01-08 21:23:15 +0000
+++ lib/lp/app/browser/tests/root-views.txt 2010-08-12 19:37:44 +0000
@@ -4,7 +4,7 @@
4The launchpad front page uses the LaunchpadRootIndexView to provide the4The launchpad front page uses the LaunchpadRootIndexView to provide the
5special data needed for the layout.5special data needed for the layout.
66
7 # The _get_day_of_year() method must be hacked to return a predicable day7 # The _get_day_of_year() method must be hacked to return a predictable day
8 # to testing the view.8 # to testing the view.
9 >>> from lp.app.browser.root import LaunchpadRootIndexView9 >>> from lp.app.browser.root import LaunchpadRootIndexView
10 >>> def day():10 >>> def day():
1111
=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py 2010-08-06 11:52:05 +0000
+++ lib/lp/registry/interfaces/distribution.py 2010-08-12 19:37:44 +0000
@@ -23,6 +23,7 @@
23from zope.schema import Bool, Choice, Datetime, List, Object, Text, TextLine23from zope.schema import Bool, Choice, Datetime, List, Object, Text, TextLine
24from zope.interface import Attribute, Interface24from zope.interface import Attribute, Interface
2525
26from lazr.lifecycle.snapshot import doNotSnapshot
26from lazr.restful.fields import CollectionField, Reference27from lazr.restful.fields import CollectionField, Reference
27from lazr.restful.declarations import (28from lazr.restful.declarations import (
28 collection_default_content, export_as_webservice_collection,29 collection_default_content, export_as_webservice_collection,
@@ -200,25 +201,27 @@
200 lucilleconfig = TextLine(201 lucilleconfig = TextLine(
201 title=_("Lucille Config"),202 title=_("Lucille Config"),
202 description=_("The Lucille Config."), required=False)203 description=_("The Lucille Config."), required=False)
203 archive_mirrors = exported(CollectionField(204 archive_mirrors = exported(doNotSnapshot(
204 description=_("All enabled and official ARCHIVE mirrors of this "205 CollectionField(
205 "Distribution."),206 description=_("All enabled and official ARCHIVE mirrors "
206 readonly=True, value_type=Object(schema=IDistributionMirror)))207 "of this Distribution."),
207 cdimage_mirrors = exported(CollectionField(208 readonly=True, value_type=Object(schema=IDistributionMirror))))
208 description=_("All enabled and official RELEASE mirrors of this "209 cdimage_mirrors = exported(doNotSnapshot(
209 "Distribution."),210 CollectionField(
210 readonly=True, value_type=Object(schema=IDistributionMirror)))211 description=_("All enabled and official RELEASE mirrors "
212 "of this Distribution."),
213 readonly=True, value_type=Object(schema=IDistributionMirror))))
211 disabled_mirrors = Attribute(214 disabled_mirrors = Attribute(
212 "All disabled and official mirrors of this Distribution.")215 "All disabled and official mirrors of this Distribution.")
213 unofficial_mirrors = Attribute(216 unofficial_mirrors = Attribute(
214 "All unofficial mirrors of this Distribution.")217 "All unofficial mirrors of this Distribution.")
215 pending_review_mirrors = Attribute(218 pending_review_mirrors = Attribute(
216 "All mirrors of this Distribution that haven't been reviewed yet.")219 "All mirrors of this Distribution that haven't been reviewed yet.")
217 series = exported(220 series = exported(doNotSnapshot(
218 CollectionField(221 CollectionField(
219 title=_("DistroSeries inside this Distribution"),222 title=_("DistroSeries inside this Distribution"),
220 # Really IDistroSeries, see _schema_circular_imports.py.223 # Really IDistroSeries, see _schema_circular_imports.py.
221 value_type=Reference(schema=Interface)))224 value_type=Reference(schema=Interface))))
222 architectures = List(225 architectures = List(
223 title=_("DistroArchSeries inside this Distribution"))226 title=_("DistroArchSeries inside this Distribution"))
224 uploaders = Attribute(_(227 uploaders = Attribute(_(
@@ -262,11 +265,11 @@
262 # Really IArchive, see _schema_circular_imports.py.265 # Really IArchive, see _schema_circular_imports.py.
263 schema=Interface))266 schema=Interface))
264267
265 all_distro_archives = exported(268 all_distro_archives = exported(doNotSnapshot(
266 CollectionField(269 CollectionField(
267 title=_("A sequence of the distribution's non-PPA Archives."),270 title=_("A sequence of the distribution's non-PPA Archives."),
268 readonly=True, required=False,271 readonly=True, required=False,
269 value_type=Reference(schema=Interface)),272 value_type=Reference(schema=Interface))),
270 # Really IArchive, see _schema_circular_imports.py.273 # Really IArchive, see _schema_circular_imports.py.
271 exported_as='archives')274 exported_as='archives')
272275
273276
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2010-08-06 14:49:35 +0000
+++ lib/lp/registry/interfaces/product.py 2010-08-12 19:37:44 +0000
@@ -74,6 +74,7 @@
74 ITranslationPolicy)74 ITranslationPolicy)
75from canonical.launchpad.validators import LaunchpadValidationError75from canonical.launchpad.validators import LaunchpadValidationError
76from canonical.launchpad.validators.name import name_validator76from canonical.launchpad.validators.name import name_validator
77from lazr.lifecycle.snapshot import doNotSnapshot
77from lazr.restful.fields import CollectionField, Reference, ReferenceChoice78from lazr.restful.fields import CollectionField, Reference, ReferenceChoice
78from lazr.restful.declarations import (79from lazr.restful.declarations import (
79 REQUEST_USER, call_with, collection_default_content,80 REQUEST_USER, call_with, collection_default_content,
@@ -585,7 +586,8 @@
585 "this product"))586 "this product"))
586587
587 series = exported(588 series = exported(
588 CollectionField(value_type=Object(schema=IProductSeries)))589 doNotSnapshot(
590 CollectionField(value_type=Object(schema=IProductSeries))))
589591
590 development_focus = exported(592 development_focus = exported(
591 ReferenceChoice(593 ReferenceChoice(
@@ -602,10 +604,12 @@
602 "product; otherwise, simply returns the product name."))604 "product; otherwise, simply returns the product name."))
603605
604 releases = exported(606 releases = exported(
605 CollectionField(607 doNotSnapshot(
606 title=_("An iterator over the ProductReleases for this product."),608 CollectionField(
607 readonly=True,609 title=_("An iterator over the ProductReleases for "
608 value_type=Reference(schema=IProductRelease)))610 "this product."),
611 readonly=True,
612 value_type=Reference(schema=IProductRelease))))
609613
610 translation_focus = exported(614 translation_focus = exported(
611 ReferenceChoice(615 ReferenceChoice(
612616
=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py 2010-08-03 15:15:16 +0000
+++ lib/lp/registry/tests/test_distribution.py 2010-08-12 19:37:44 +0000
@@ -7,10 +7,13 @@
77
8from zope.security.proxy import removeSecurityProxy8from zope.security.proxy import removeSecurityProxy
99
10from lazr.lifecycle.snapshot import Snapshot
10from lp.registry.tests.test_distroseries import (11from lp.registry.tests.test_distroseries import (
11 TestDistroSeriesCurrentSourceReleases)12 TestDistroSeriesCurrentSourceReleases)
12from lp.registry.interfaces.distroseries import NoSuchDistroSeries13from lp.registry.interfaces.distroseries import NoSuchDistroSeries
13from lp.registry.interfaces.series import SeriesStatus14from lp.registry.interfaces.series import SeriesStatus
15from lp.registry.interfaces.distribution import IDistribution
16
14from lp.soyuz.interfaces.distributionsourcepackagerelease import (17from lp.soyuz.interfaces.distributionsourcepackagerelease import (
15 IDistributionSourcePackageRelease)18 IDistributionSourcePackageRelease)
16from lp.testing import TestCaseWithFactory19from lp.testing import TestCaseWithFactory
@@ -140,3 +143,31 @@
140 series = self.factory.makeDistroSeries(distribution=distro,143 series = self.factory.makeDistroSeries(distribution=distro,
141 name="dappere", version="42.6")144 name="dappere", version="42.6")
142 self.assertEquals(series, distro.getSeries("42.6"))145 self.assertEquals(series, distro.getSeries("42.6"))
146
147
148class DistroSnapshotTestCase(TestCaseWithFactory):
149 """A TestCase for distribution snapshots."""
150
151 layer = LaunchpadFunctionalLayer
152
153 def setUp(self):
154 super(DistroSnapshotTestCase, self).setUp()
155 self.distribution = self.factory.makeDistribution(name="boobuntu")
156
157 def test_snapshot(self):
158 """Snapshots of products should not include marked attribues.
159
160 Wrap an export with 'doNotSnapshot' to force the snapshot to not
161 include that attribute.
162 """
163 snapshot = Snapshot(self.distribution, providing=IDistribution)
164 omitted = [
165 'archive_mirrors',
166 'cdimage_mirrors',
167 'series',
168 'all_distro_archives',
169 ]
170 for attribute in omitted:
171 self.assertFalse(
172 hasattr(snapshot, attribute),
173 "Snapshot should not include %s." % attribute)
143174
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py 2010-08-05 13:23:52 +0000
+++ lib/lp/registry/tests/test_person.py 2010-08-12 19:37:44 +0000
@@ -249,8 +249,8 @@
249 transaction.commit()249 transaction.commit()
250 logout()250 logout()
251251
252 def _get_testible_account(self, person, date_created, openid_identifier):252 def _get_testable_account(self, person, date_created, openid_identifier):
253 # Return a naked account with predicable attributes.253 # Return a naked account with predictable attributes.
254 account = removeSecurityProxy(person.account)254 account = removeSecurityProxy(person.account)
255 account.date_created = date_created255 account.date_created = date_created
256 account.openid_identifier = openid_identifier256 account.openid_identifier = openid_identifier
@@ -264,7 +264,7 @@
264 2010, 04, 01, 0, 0, 0, 0, tzinfo=pytz.timezone('UTC'))264 2010, 04, 01, 0, 0, 0, 0, tzinfo=pytz.timezone('UTC'))
265 # Free an OpenID identifier using merge.265 # Free an OpenID identifier using merge.
266 first_duplicate = self.factory.makePerson()266 first_duplicate = self.factory.makePerson()
267 first_account = self._get_testible_account(267 first_account = self._get_testable_account(
268 first_duplicate, test_date, test_identifier)268 first_duplicate, test_date, test_identifier)
269 first_person = self.factory.makePerson()269 first_person = self.factory.makePerson()
270 self._do_premerge(first_duplicate, first_person)270 self._do_premerge(first_duplicate, first_person)
@@ -276,7 +276,7 @@
276 # Create an account that reuses the freed OpenID_identifier.276 # Create an account that reuses the freed OpenID_identifier.
277 test_date = test_date.replace(2010, 05)277 test_date = test_date.replace(2010, 05)
278 second_duplicate = self.factory.makePerson()278 second_duplicate = self.factory.makePerson()
279 second_account = self._get_testible_account(279 second_account = self._get_testable_account(
280 second_duplicate, test_date, test_identifier)280 second_duplicate, test_date, test_identifier)
281 second_person = self.factory.makePerson()281 second_person = self.factory.makePerson()
282 self._do_premerge(second_duplicate, second_person)282 self._do_premerge(second_duplicate, second_person)
283283
=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2010-08-02 20:24:24 +0000
+++ lib/lp/registry/tests/test_product.py 2010-08-12 19:37:44 +0000
@@ -18,14 +18,16 @@
1818
19from canonical.launchpad.ftests import syncUpdate19from canonical.launchpad.ftests import syncUpdate
2020
21from lazr.lifecycle.snapshot import Snapshot
21from lp.registry.interfaces.person import IPersonSet22from lp.registry.interfaces.person import IPersonSet
22from lp.registry.interfaces.product import License23from lp.registry.interfaces.product import IProduct, License
23from lp.registry.model.product import Product24from lp.registry.model.product import Product
24from lp.registry.model.productlicense import ProductLicense25from lp.registry.model.productlicense import ProductLicense
25from lp.registry.model.commercialsubscription import (26from lp.registry.model.commercialsubscription import (
26 CommercialSubscription)27 CommercialSubscription)
27from lp.testing import TestCaseWithFactory28from lp.testing import TestCaseWithFactory
2829
30
29class TestProduct(TestCaseWithFactory):31class TestProduct(TestCaseWithFactory):
30 """Tests product object."""32 """Tests product object."""
3133
@@ -178,6 +180,32 @@
178 'new')180 'new')
179181
180182
183class ProductSnapshotTestCase(TestCaseWithFactory):
184 """A TestCase for product snapshots."""
185
186 layer = LaunchpadFunctionalLayer
187
188 def setUp(self):
189 super(ProductSnapshotTestCase, self).setUp()
190 self.product = self.factory.makeProduct(name="shamwow")
191
192 def test_snapshot(self):
193 """Snapshots of products should not include marked attribues.
194
195 Wrap an export with 'doNotSnapshot' to force the snapshot to not
196 include that attribute.
197 """
198 snapshot = Snapshot(self.product, providing=IProduct)
199 omitted = [
200 'series',
201 'releases',
202 ]
203 for attribute in omitted:
204 self.assertFalse(
205 hasattr(snapshot, attribute),
206 "Snapshot should not include %s." % attribute)
207
208
181class BugSupervisorTestCase(TestCaseWithFactory):209class BugSupervisorTestCase(TestCaseWithFactory):
182 """A TestCase for bug supervisor management."""210 """A TestCase for bug supervisor management."""
183211