Launchpad itself

Merge lp:~jml/launchpad/narrow-commercial-celebrity into lp:launchpad

  1. narrow-commercial-celebrity
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Approved by: Richard Harding
Approved revision: no longer in the source branch.
Merged at revision: 15185
Proposed branch: lp:~jml/launchpad/narrow-commercial-celebrity
Merge into: lp:launchpad
Diff against target: 509 lines (+100/-213)
10 files modified
lib/lp/_schema_circular_imports.py (+3/-5)
lib/lp/registry/interfaces/distribution.py (+0/-12)
lib/lp/registry/model/distribution.py (+0/-6)
lib/lp/soyuz/doc/archive-commercial.txt (+0/-56)
lib/lp/soyuz/interfaces/archive.py (+3/-10)
lib/lp/soyuz/model/archive.py (+0/-8)
lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt (+3/-16)
lib/lp/soyuz/tests/test_archive_agent.py (+29/-42)
lib/lp/soyuz/tests/test_archive_privacy.py (+54/-57)
lib/lp/testing/factory.py (+8/-1)
To merge this branch: bzr merge lp:~jml/launchpad/narrow-commercial-celebrity
Reviewer Review Type Date Requested Status
Richard Harding (community) Approve
Review via email: mp+104236@code.launchpad.net

Commit message

Remove IDistribution.getCommercialPPAs and IArchiveSet.getCommercialPPAs

Description of the change

We don't need getCommercialPPAs at all. It was added for us, we are the only users, and we don't use it. Therefore, it should be deleted. Thus let it be written, thus let it be done.

This branch deletes them both. It also deletes a doctest, archive-commercial.txt, which doesn't really do much once getCommercialPPAs is deleted. The sole remaining test went into test_archive_privacy, which I also went to work on with a pair of pliers and a blowtorch, moving stuff out of setUp, using simpler layers and hopefully making tests more clear.

I've updated the documentation in IArchive for commercial to match our current understanding.

If it lands as-is, this branch will leave me 113 deletions in credit (by diffstat's measure). I'm going to need them.

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

Fresh revisions make better use of the new 'commercial' parameter on LaunchpadObjectFactory.makeArchive. I also did to test_archive_agent what I did to the other test modules: kill unnecessary helpers, use better helpers from the common library, stop using setUp.

Revision history for this message
Richard Harding (rharding) wrote :

Looks good, updated your negative LoC count to 113 after revisions.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/_schema_circular_imports.py'
2--- lib/lp/_schema_circular_imports.py 2012-03-13 00:45:33 +0000
3+++ lib/lp/_schema_circular_imports.py 2012-05-01 12:28:21 +0000
4@@ -470,8 +470,6 @@
5 IDistribution, 'getSourcePackage', IDistributionSourcePackage)
6 patch_collection_return_type(
7 IDistribution, 'searchSourcePackages', IDistributionSourcePackage)
8-patch_collection_return_type(
9- IDistribution, 'getCommercialPPAs', IArchive)
10 patch_reference_property(
11 IDistribution, 'main_archive', IArchive)
12 IDistribution['all_distro_archives'].value_type.schema = IArchive
13@@ -861,9 +859,9 @@
14
15 # IDistribution
16 patch_operations_explicit_version(
17- IDistribution, 'beta', "getArchive", "getCommercialPPAs",
18- "getCountryMirror", "getDevelopmentSeries", "getMirrorByName",
19- "getSeries", "getSourcePackage", "searchSourcePackages")
20+ IDistribution, 'beta', "getArchive", "getCountryMirror",
21+ "getDevelopmentSeries", "getMirrorByName", "getSeries",
22+ "getSourcePackage", "searchSourcePackages")
23
24 # IDistributionMirror
25 patch_entry_explicit_version(IDistributionMirror, 'beta')
26
27=== modified file 'lib/lp/registry/interfaces/distribution.py'
28--- lib/lp/registry/interfaces/distribution.py 2011-12-24 16:54:44 +0000
29+++ lib/lp/registry/interfaces/distribution.py 2012-05-01 12:28:21 +0000
30@@ -570,18 +570,6 @@
31 def getAllPPAs():
32 """Return all PPAs for this distribution."""
33
34- # Really returns IArchive, see
35- # _schema_circular_imports.py.
36- @operation_returns_collection_of(Interface)
37- @export_read_operation()
38- def getCommercialPPAs():
39- """Return all commercial PPAs.
40-
41- Commercial PPAs are private, but explicitly flagged up as commercial
42- so that they are discoverable by people who wish to buy items
43- from them.
44- """
45-
46 def searchPPAs(text=None, show_inactive=False):
47 """Return all PPAs matching the given text in this distribution.
48
49
50=== modified file 'lib/lp/registry/model/distribution.py'
51--- lib/lp/registry/model/distribution.py 2012-03-08 01:07:47 +0000
52+++ lib/lp/registry/model/distribution.py 2012-05-01 12:28:21 +0000
53@@ -1316,12 +1316,6 @@
54 distribution=self,
55 purpose=ArchivePurpose.PPA).order_by('id')
56
57- def getCommercialPPAs(self):
58- """See `IDistribution`."""
59- # If we ever see non-Ubuntu PPAs, this will return more than
60- # just the PPAs for the Ubuntu context.
61- return getUtility(IArchiveSet).getCommercialPPAs()
62-
63 def searchPPAs(self, text=None, show_inactive=False, user=None):
64 """See `IDistribution`."""
65 clauses = ["""
66
67=== removed file 'lib/lp/soyuz/doc/archive-commercial.txt'
68--- lib/lp/soyuz/doc/archive-commercial.txt 2010-06-30 13:50:25 +0000
69+++ lib/lp/soyuz/doc/archive-commercial.txt 1970-01-01 00:00:00 +0000
70@@ -1,56 +0,0 @@
71-===============
72-Commercial PPAs
73-===============
74-
75-Commercial PPAs are to all intents and purposes identical to private PPAs.
76-They contain a 'commercial' flag which indicates that the PPA is actively
77-selling software and should be more discoverable.
78-
79-We have a private PPA for demonstration purposes:
80-
81- >>> ppa = factory.makeArchive(private=True, name="pppa")
82-
83-
84-Discovering Commercial PPAs
85----------------------------
86-
87-IArchive.getCommercialPPAs() retrieves the subset of all private PPAs that are
88-marked for commercial usage.
89-
90- >>> from lp.soyuz.interfaces.archive import IArchiveSet
91- >>> archive_set = getUtility(IArchiveSet)
92- >>> for commercial_ppa in archive_set.getCommercialPPAs():
93- ... print commercial_ppa.name
94-
95-There are none yet - if we mark "ppa" as commercial it will be
96-returned:
97-
98- >>> # Setting 'commercial' is restricted to admins but we don't want to
99- >>> # worry about that here. See lib/lp/soyuz/tests/test_archive.py
100- >>> from zope.security.proxy import removeSecurityProxy
101- >>> removeSecurityProxy(ppa).commercial = True
102- >>> for commercial_ppa in archive_set.getCommercialPPAs():
103- ... print commercial_ppa.name
104- pppa
105-
106-
107-Security
108---------
109-
110-Anyone is allowed to retrieve the list of commercial PPAs. Normal security
111-restrictions apply to individual PPA access though.
112-
113- >>> login(ANONYMOUS)
114- >>> ppas = archive_set.getCommercialPPAs()
115-
116-'name is a public attribute:
117-
118- >>> print ppas[0].name
119- pppa
120-
121-'description' is not, however.
122-
123- >>> ppas[0].description
124- Traceback (most recent call last):
125- ...
126- Unauthorized:...
127
128=== modified file 'lib/lp/soyuz/interfaces/archive.py'
129--- lib/lp/soyuz/interfaces/archive.py 2012-04-03 15:34:50 +0000
130+++ lib/lp/soyuz/interfaces/archive.py 2012-05-01 12:28:21 +0000
131@@ -324,8 +324,9 @@
132 title=_("Commercial"),
133 required=True,
134 description=_(
135- "Display the archive in Software Center's commercial "
136- "listings. Only private archives can be commercial.")))
137+ "True if the archive is for commercial applications in the "
138+ "Ubuntu Software Centre. Governs whether subscribers or "
139+ "uploaders get mail from Launchpad about archive events.")))
140
141 def checkArchivePermission(person, component_or_package=None):
142 """Check to see if person is allowed to upload to component.
143@@ -1886,14 +1887,6 @@
144 def getPrivatePPAs():
145 """Return a result set containing all private PPAs."""
146
147- def getCommercialPPAs():
148- """Return a result set containing all commercial PPAs.
149-
150- Commercial PPAs are private, but explicitly flagged up as commercial
151- so that they are discoverable by people who wish to buy items
152- from them.
153- """
154-
155 def getPublicationsInArchives(source_package_name, archive_list,
156 distribution):
157 """Return a result set of publishing records for the source package.
158
159=== modified file 'lib/lp/soyuz/model/archive.py'
160--- lib/lp/soyuz/model/archive.py 2012-04-04 13:36:29 +0000
161+++ lib/lp/soyuz/model/archive.py 2012-05-01 12:28:21 +0000
162@@ -2404,14 +2404,6 @@
163 Archive._private == True,
164 Archive.purpose == ArchivePurpose.PPA)
165
166- def getCommercialPPAs(self):
167- """See `IArchiveSet`."""
168- store = IStore(Archive)
169- return store.find(
170- Archive,
171- Archive.commercial == True,
172- Archive.purpose == ArchivePurpose.PPA)
173-
174 def getArchivesForDistribution(self, distribution, name=None,
175 purposes=None, user=None,
176 exclude_disabled=True):
177
178=== modified file 'lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt'
179--- lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt 2011-12-24 17:49:30 +0000
180+++ lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt 2012-05-01 12:28:21 +0000
181@@ -8,24 +8,11 @@
182 Start by making a commercial PPA that we can fetch via the webservice:
183
184 >>> login("admin@canonical.com")
185- >>> ppa = factory.makeArchive(private=True, name="cpppa")
186- >>> from zope.security.proxy import removeSecurityProxy
187- >>> removeSecurityProxy(ppa).commercial = True
188+ >>> ppa = factory.makeArchive(private=True, name="cpppa", commercial=True)
189 >>> import transaction
190 >>> transaction.commit()
191 >>> logout()
192
193-Commercial archives can be found with a custom method 'getCommercialPPAs'
194-that lives on the distribution object.
195-
196-Using webservice directly:
197-
198- >>> ubuntu = webservice.get("/ubuntu").jsonBody()
199- >>> cppas = webservice.named_get(
200- ... ubuntu['self_link'], 'getCommercialPPAs').jsonBody()
201- >>> for entry in cppas['entries']:
202- ... print entry['name']
203- cpppa
204
205 == Software Center Agent ==
206
207@@ -35,8 +22,8 @@
208 >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
209 >>> login('admin@canonical.com')
210 >>> celebrity = getUtility(ILaunchpadCelebrities).software_center_agent
211- >>> archive = factory.makeArchive(name='commercial', private=True)
212- >>> archive.commercial = True
213+ >>> archive = factory.makeArchive(
214+ ... name='commercial', private=True, commercial=True)
215 >>> url = "/~%s/+archive/commercial" % archive.owner.name
216 >>> person = factory.makePerson(name='joe')
217 >>> logout()
218
219=== modified file 'lib/lp/soyuz/tests/test_archive_agent.py'
220--- lib/lp/soyuz/tests/test_archive_agent.py 2012-01-01 02:58:52 +0000
221+++ lib/lp/soyuz/tests/test_archive_agent.py 2012-05-01 12:28:21 +0000
222@@ -1,67 +1,54 @@
223-# Copyright 2010 Canonical Ltd. This software is licensed under the
224-# GNU Affero General Public License version 3 (see the file LICENSE).
225+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the GNU
226+# Affero General Public License version 3 (see the file LICENSE).
227
228 """Test Archive software center agent celebrity."""
229
230 from zope.component import getUtility
231
232-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
233 from lp.services.webapp.authorization import check_permission
234 from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
235 from lp.testing import (
236- login,
237- login_person,
238+ celebrity_logged_in,
239 TestCaseWithFactory,
240 )
241-from lp.testing.layers import LaunchpadFunctionalLayer
242+from lp.testing.layers import DatabaseFunctionalLayer
243
244
245 class TestArchivePrivacy(TestCaseWithFactory):
246- layer = LaunchpadFunctionalLayer
247-
248- def setUp(self):
249- super(TestArchivePrivacy, self).setUp()
250- self.ppa = self._makePrivateArchive()
251- self.ppa.commercial = True
252- self.agent = getUtility(ILaunchpadCelebrities).software_center_agent
253- self.joe = self.factory.makePerson(name='joe')
254-
255- def _makePrivateArchive(self):
256- ppa = self.factory.makeArchive()
257- login('admin@canonical.com')
258- ppa.buildd_secret = 'blah'
259- ppa.private = True
260- return ppa
261+
262+ layer = DatabaseFunctionalLayer
263
264 def test_check_permission(self):
265 """The software center agent has the relevant permissions for a
266 commercial archive, but not a private one.
267 """
268- login_person(self.agent)
269- self.assertEqual(
270- check_permission('launchpad.View', self.ppa), True)
271- self.assertEqual(
272- check_permission('launchpad.Append', self.ppa), True)
273+ ppa = self.factory.makeArchive(private=True, commercial=True)
274+ with celebrity_logged_in('software_center_agent'):
275+ self.assertEqual(check_permission('launchpad.View', ppa), True)
276+ self.assertEqual(check_permission('launchpad.Append', ppa), True)
277
278 def test_check_permission_private(self):
279- ppa = self._makePrivateArchive()
280- login_person(self.agent)
281- self.assertEqual(check_permission('launchpad.View', ppa), False)
282- self.assertEqual(check_permission('launchpad.Append', ppa), False)
283+ ppa = self.factory.makeArchive(private=True, commercial=False)
284+ with celebrity_logged_in('software_center_agent'):
285+ self.assertEqual(check_permission('launchpad.View', ppa), False)
286+ self.assertEqual(check_permission('launchpad.Append', ppa), False)
287
288 def test_add_subscription(self):
289- login_person(self.agent)
290- self.ppa.newSubscription(self.joe, self.agent)
291- subscription = getUtility(
292- IArchiveSubscriberSet).getBySubscriber(
293- self.joe, archive=self.ppa).one()
294- self.assertEqual(subscription.registrant, self.agent)
295- self.assertEqual(subscription.subscriber, self.joe)
296+ person = self.factory.makePerson()
297+ ppa = self.factory.makeArchive(private=True, commercial=True)
298+ with celebrity_logged_in('software_center_agent') as agent:
299+ ppa.newSubscription(person, agent)
300+ subscription = getUtility(IArchiveSubscriberSet).getBySubscriber(
301+ person, archive=ppa).one()
302+ self.assertEqual(subscription.registrant, agent)
303+ self.assertEqual(subscription.subscriber, person)
304
305 def test_getArchiveSubscriptionURL(self):
306- login_person(self.agent)
307- sources = self.joe.getArchiveSubscriptionURL(self.agent, self.ppa)
308- authtoken = self.ppa.getAuthToken(self.joe).token
309- url = self.ppa.archive_url.split('http://')[1]
310- new_url = "http://joe:%s@%s" % (authtoken, url)
311+ ppa = self.factory.makeArchive(private=True, commercial=True)
312+ person = self.factory.makePerson()
313+ with celebrity_logged_in('software_center_agent') as agent:
314+ sources = person.getArchiveSubscriptionURL(agent, ppa)
315+ authtoken = ppa.getAuthToken(person).token
316+ url = ppa.archive_url.split('http://')[1]
317+ new_url = "http://%s:%s@%s" % (person.name, authtoken, url)
318 self.assertEqual(sources, new_url)
319
320=== modified file 'lib/lp/soyuz/tests/test_archive_privacy.py'
321--- lib/lp/soyuz/tests/test_archive_privacy.py 2012-01-01 02:58:52 +0000
322+++ lib/lp/soyuz/tests/test_archive_privacy.py 2012-05-01 12:28:21 +0000
323@@ -3,98 +3,95 @@
324
325 """Test Archive privacy features."""
326
327-from zope.component import getUtility
328 from zope.security.interfaces import Unauthorized
329
330-from lp.soyuz.interfaces.archive import (
331- CannotSwitchPrivacy,
332- IArchiveSet,
333- )
334+from lp.soyuz.interfaces.archive import CannotSwitchPrivacy
335 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
336 from lp.testing import (
337- login,
338- login_person,
339+ person_logged_in,
340 TestCaseWithFactory,
341 )
342 from lp.testing.layers import (
343- LaunchpadFunctionalLayer,
344+ DatabaseFunctionalLayer,
345 LaunchpadZopelessLayer,
346 )
347
348
349 class TestArchivePrivacy(TestCaseWithFactory):
350- layer = LaunchpadFunctionalLayer
351-
352- def setUp(self):
353- super(TestArchivePrivacy, self).setUp()
354- self.private_ppa = self.factory.makeArchive(description='Foo')
355- login('admin@canonical.com')
356- self.private_ppa.private = True
357- self.joe = self.factory.makePerson(name='joe')
358- self.fred = self.factory.makePerson(name='fred')
359- login_person(self.private_ppa.owner)
360- self.private_ppa.newSubscription(self.joe, self.private_ppa.owner)
361-
362- def _getDescription(self, p3a):
363- return p3a.description
364+
365+ layer = DatabaseFunctionalLayer
366
367 def test_no_subscription(self):
368- login_person(self.fred)
369- p3a = getUtility(IArchiveSet).get(self.private_ppa.id)
370- self.assertRaises(Unauthorized, self._getDescription, p3a)
371+ # You cannot access private PPAs without a subscription.
372+ ppa = self.factory.makeArchive(private=True)
373+ non_subscriber = self.factory.makePerson()
374+ with person_logged_in(non_subscriber):
375+ self.assertRaises(Unauthorized, getattr, ppa, 'description')
376
377 def test_subscription(self):
378- login_person(self.joe)
379- p3a = getUtility(IArchiveSet).get(self.private_ppa.id)
380- self.assertEqual(self._getDescription(p3a), "Foo")
381+ # Once you have a subscription, you can access private PPAs.
382+ ppa = self.factory.makeArchive(private=True, description="Foo")
383+ subscriber = self.factory.makePerson()
384+ with person_logged_in(ppa.owner):
385+ ppa.newSubscription(subscriber, ppa.owner)
386+ with person_logged_in(subscriber):
387+ self.assertEqual(ppa.description, "Foo")
388+
389+ def test_commercial_security(self):
390+ # Commercial private PPAs cannot be accessed by non-subscribers.
391+ ppa_name = self.factory.getUniqueString()
392+ ppa = self.factory.makeArchive(
393+ private=True, commercial=True, name=ppa_name)
394+ non_subscriber = self.factory.makePerson()
395+ with person_logged_in(non_subscriber):
396+ self.assertEqual(ppa_name, ppa.name)
397+ self.assertRaises(Unauthorized, getattr, ppa, 'description')
398
399
400 class TestArchivePrivacySwitching(TestCaseWithFactory):
401
402 layer = LaunchpadZopelessLayer
403
404- def setUp(self):
405- """Create a public and a private PPA."""
406- super(TestArchivePrivacySwitching, self).setUp()
407- self.public_ppa = self.factory.makeArchive()
408- self.private_ppa = self.factory.makeArchive()
409- self.private_ppa.private = True
410-
411- def make_ppa_private(self, ppa):
412+ def set_ppa_privacy(self, ppa, private):
413 """Helper method to privatise a ppa."""
414- ppa.private = True
415-
416- def make_ppa_public(self, ppa):
417- """Helper method to make a PPA public (and use for assertRaises)."""
418- ppa.private = False
419+ ppa.private = private
420
421 def test_switch_privacy_no_pubs_succeeds(self):
422 # Changing the privacy is fine if there are no publishing
423 # records.
424- self.make_ppa_private(self.public_ppa)
425- self.assertTrue(self.public_ppa.private)
426+ public_ppa = self.factory.makeArchive()
427+ self.set_ppa_privacy(public_ppa, private=True)
428+ self.assertTrue(public_ppa.private)
429
430- self.private_ppa.private = False
431- self.assertFalse(self.private_ppa.private)
432+ private_ppa = self.factory.makeArchive(private=True)
433+ self.set_ppa_privacy(private_ppa, private=False)
434+ self.assertFalse(private_ppa.private)
435
436 def test_switch_privacy_with_pubs_fails(self):
437 # Changing the privacy is not possible when the archive already
438 # has published sources.
439+ public_ppa = self.factory.makeArchive(private=False)
440 publisher = SoyuzTestPublisher()
441 publisher.prepareBreezyAutotest()
442- publisher.getPubSource(archive=self.public_ppa)
443- publisher.getPubSource(archive=self.private_ppa)
444-
445- self.assertRaises(
446- CannotSwitchPrivacy, self.make_ppa_private, self.public_ppa)
447-
448- self.assertRaises(
449- CannotSwitchPrivacy, self.make_ppa_public, self.private_ppa)
450+
451+ private_ppa = self.factory.makeArchive(private=True)
452+ publisher.getPubSource(archive=public_ppa)
453+ publisher.getPubSource(archive=private_ppa)
454+
455+ self.assertRaises(
456+ CannotSwitchPrivacy,
457+ self.set_ppa_privacy, public_ppa, private=True)
458+
459+ self.assertRaises(
460+ CannotSwitchPrivacy,
461+ self.set_ppa_privacy, private_ppa, private=False)
462
463 def test_buildd_secret_was_generated(self):
464- self.make_ppa_private(self.public_ppa)
465- self.assertNotEqual(self.public_ppa.buildd_secret, None)
466+ public_ppa = self.factory.makeArchive()
467+ self.set_ppa_privacy(public_ppa, private=True)
468+ self.assertNotEqual(public_ppa.buildd_secret, None)
469
470 def test_discard_buildd_was_discarded(self):
471- self.make_ppa_public(self.private_ppa)
472- self.assertEqual(self.private_ppa.buildd_secret, None)
473+ private_ppa = self.factory.makeArchive(private=True)
474+ self.set_ppa_privacy(private_ppa, private=False)
475+ self.assertEqual(private_ppa.buildd_secret, None)
476
477=== modified file 'lib/lp/testing/factory.py'
478--- lib/lp/testing/factory.py 2012-04-27 18:50:49 +0000
479+++ lib/lp/testing/factory.py 2012-05-01 12:28:21 +0000
480@@ -2663,7 +2663,8 @@
481
482 def makeArchive(self, distribution=None, owner=None, name=None,
483 purpose=None, enabled=True, private=False,
484- virtualized=True, description=None, displayname=None):
485+ virtualized=True, description=None, displayname=None,
486+ commercial=False):
487 """Create and return a new arbitrary archive.
488
489 :param distribution: Supply IDistribution, defaults to a new one
490@@ -2676,6 +2677,8 @@
491 :param private: Whether the archive is created private.
492 :param virtualized: Whether the archive is virtualized.
493 :param description: A description of the archive.
494+ :param commercial: Whether the archive is commercial. Defaults to
495+ False.
496 """
497 if purpose is None:
498 purpose = ArchivePurpose.PPA
499@@ -2714,6 +2717,10 @@
500 naked_archive.private = True
501 naked_archive.buildd_secret = "sekrit"
502
503+ if commercial:
504+ naked_archive = removeSecurityProxy(archive)
505+ naked_archive.commercial = True
506+
507 return archive
508
509 def makeArchiveAdmin(self, archive=None):