launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07483
[Merge] lp:~jml/launchpad/narrow-commercial-celebrity into lp:launchpad
Jonathan Lange has proposed merging lp:~jml/launchpad/narrow-commercial-celebrity into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jml/launchpad/narrow-commercial-celebrity/+merge/104236
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 98 deletions in credit (by diffstat's measure). I'm going to need them.
--
https://code.launchpad.net/~jml/launchpad/narrow-commercial-celebrity/+merge/104236
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jml/launchpad/narrow-commercial-celebrity into lp:launchpad.
=== modified file 'lib/lp/_schema_circular_imports.py'
--- lib/lp/_schema_circular_imports.py 2012-03-13 00:45:33 +0000
+++ lib/lp/_schema_circular_imports.py 2012-05-01 11:30:33 +0000
@@ -470,8 +470,6 @@
IDistribution, 'getSourcePackage', IDistributionSourcePackage)
patch_collection_return_type(
IDistribution, 'searchSourcePackages', IDistributionSourcePackage)
-patch_collection_return_type(
- IDistribution, 'getCommercialPPAs', IArchive)
patch_reference_property(
IDistribution, 'main_archive', IArchive)
IDistribution['all_distro_archives'].value_type.schema = IArchive
@@ -861,9 +859,9 @@
# IDistribution
patch_operations_explicit_version(
- IDistribution, 'beta', "getArchive", "getCommercialPPAs",
- "getCountryMirror", "getDevelopmentSeries", "getMirrorByName",
- "getSeries", "getSourcePackage", "searchSourcePackages")
+ IDistribution, 'beta', "getArchive", "getCountryMirror",
+ "getDevelopmentSeries", "getMirrorByName", "getSeries",
+ "getSourcePackage", "searchSourcePackages")
# IDistributionMirror
patch_entry_explicit_version(IDistributionMirror, 'beta')
=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py 2011-12-24 16:54:44 +0000
+++ lib/lp/registry/interfaces/distribution.py 2012-05-01 11:30:33 +0000
@@ -570,18 +570,6 @@
def getAllPPAs():
"""Return all PPAs for this distribution."""
- # Really returns IArchive, see
- # _schema_circular_imports.py.
- @operation_returns_collection_of(Interface)
- @export_read_operation()
- def getCommercialPPAs():
- """Return all commercial PPAs.
-
- Commercial PPAs are private, but explicitly flagged up as commercial
- so that they are discoverable by people who wish to buy items
- from them.
- """
-
def searchPPAs(text=None, show_inactive=False):
"""Return all PPAs matching the given text in this distribution.
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2012-03-08 01:07:47 +0000
+++ lib/lp/registry/model/distribution.py 2012-05-01 11:30:33 +0000
@@ -1316,12 +1316,6 @@
distribution=self,
purpose=ArchivePurpose.PPA).order_by('id')
- def getCommercialPPAs(self):
- """See `IDistribution`."""
- # If we ever see non-Ubuntu PPAs, this will return more than
- # just the PPAs for the Ubuntu context.
- return getUtility(IArchiveSet).getCommercialPPAs()
-
def searchPPAs(self, text=None, show_inactive=False, user=None):
"""See `IDistribution`."""
clauses = ["""
=== removed file 'lib/lp/soyuz/doc/archive-commercial.txt'
--- lib/lp/soyuz/doc/archive-commercial.txt 2010-06-30 13:50:25 +0000
+++ lib/lp/soyuz/doc/archive-commercial.txt 1970-01-01 00:00:00 +0000
@@ -1,56 +0,0 @@
-===============
-Commercial PPAs
-===============
-
-Commercial PPAs are to all intents and purposes identical to private PPAs.
-They contain a 'commercial' flag which indicates that the PPA is actively
-selling software and should be more discoverable.
-
-We have a private PPA for demonstration purposes:
-
- >>> ppa = factory.makeArchive(private=True, name="pppa")
-
-
-Discovering Commercial PPAs
----------------------------
-
-IArchive.getCommercialPPAs() retrieves the subset of all private PPAs that are
-marked for commercial usage.
-
- >>> from lp.soyuz.interfaces.archive import IArchiveSet
- >>> archive_set = getUtility(IArchiveSet)
- >>> for commercial_ppa in archive_set.getCommercialPPAs():
- ... print commercial_ppa.name
-
-There are none yet - if we mark "ppa" as commercial it will be
-returned:
-
- >>> # Setting 'commercial' is restricted to admins but we don't want to
- >>> # worry about that here. See lib/lp/soyuz/tests/test_archive.py
- >>> from zope.security.proxy import removeSecurityProxy
- >>> removeSecurityProxy(ppa).commercial = True
- >>> for commercial_ppa in archive_set.getCommercialPPAs():
- ... print commercial_ppa.name
- pppa
-
-
-Security
---------
-
-Anyone is allowed to retrieve the list of commercial PPAs. Normal security
-restrictions apply to individual PPA access though.
-
- >>> login(ANONYMOUS)
- >>> ppas = archive_set.getCommercialPPAs()
-
-'name is a public attribute:
-
- >>> print ppas[0].name
- pppa
-
-'description' is not, however.
-
- >>> ppas[0].description
- Traceback (most recent call last):
- ...
- Unauthorized:...
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2012-04-03 15:34:50 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2012-05-01 11:30:33 +0000
@@ -324,8 +324,9 @@
title=_("Commercial"),
required=True,
description=_(
- "Display the archive in Software Center's commercial "
- "listings. Only private archives can be commercial.")))
+ "True if the archive is for commercial applications in the "
+ "Ubuntu Software Centre. Governs whether subscribers or "
+ "uploaders get mail from Launchpad about archive events.")))
def checkArchivePermission(person, component_or_package=None):
"""Check to see if person is allowed to upload to component.
@@ -1886,14 +1887,6 @@
def getPrivatePPAs():
"""Return a result set containing all private PPAs."""
- def getCommercialPPAs():
- """Return a result set containing all commercial PPAs.
-
- Commercial PPAs are private, but explicitly flagged up as commercial
- so that they are discoverable by people who wish to buy items
- from them.
- """
-
def getPublicationsInArchives(source_package_name, archive_list,
distribution):
"""Return a result set of publishing records for the source package.
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2012-04-04 13:36:29 +0000
+++ lib/lp/soyuz/model/archive.py 2012-05-01 11:30:33 +0000
@@ -2404,14 +2404,6 @@
Archive._private == True,
Archive.purpose == ArchivePurpose.PPA)
- def getCommercialPPAs(self):
- """See `IArchiveSet`."""
- store = IStore(Archive)
- return store.find(
- Archive,
- Archive.commercial == True,
- Archive.purpose == ArchivePurpose.PPA)
-
def getArchivesForDistribution(self, distribution, name=None,
purposes=None, user=None,
exclude_disabled=True):
=== modified file 'lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt'
--- lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/soyuz/stories/webservice/xx-archive-commercial.txt 2012-05-01 11:30:33 +0000
@@ -15,17 +15,6 @@
>>> transaction.commit()
>>> logout()
-Commercial archives can be found with a custom method 'getCommercialPPAs'
-that lives on the distribution object.
-
-Using webservice directly:
-
- >>> ubuntu = webservice.get("/ubuntu").jsonBody()
- >>> cppas = webservice.named_get(
- ... ubuntu['self_link'], 'getCommercialPPAs').jsonBody()
- >>> for entry in cppas['entries']:
- ... print entry['name']
- cpppa
== Software Center Agent ==
=== modified file 'lib/lp/soyuz/tests/test_archive_privacy.py'
--- lib/lp/soyuz/tests/test_archive_privacy.py 2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/tests/test_archive_privacy.py 2012-05-01 11:30:33 +0000
@@ -3,98 +3,95 @@
"""Test Archive privacy features."""
-from zope.component import getUtility
from zope.security.interfaces import Unauthorized
-from lp.soyuz.interfaces.archive import (
- CannotSwitchPrivacy,
- IArchiveSet,
- )
+from lp.soyuz.interfaces.archive import CannotSwitchPrivacy
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.testing import (
- login,
- login_person,
+ person_logged_in,
TestCaseWithFactory,
)
from lp.testing.layers import (
- LaunchpadFunctionalLayer,
+ DatabaseFunctionalLayer,
LaunchpadZopelessLayer,
)
class TestArchivePrivacy(TestCaseWithFactory):
- layer = LaunchpadFunctionalLayer
-
- def setUp(self):
- super(TestArchivePrivacy, self).setUp()
- self.private_ppa = self.factory.makeArchive(description='Foo')
- login('admin@xxxxxxxxxxxxx')
- self.private_ppa.private = True
- self.joe = self.factory.makePerson(name='joe')
- self.fred = self.factory.makePerson(name='fred')
- login_person(self.private_ppa.owner)
- self.private_ppa.newSubscription(self.joe, self.private_ppa.owner)
-
- def _getDescription(self, p3a):
- return p3a.description
+
+ layer = DatabaseFunctionalLayer
def test_no_subscription(self):
- login_person(self.fred)
- p3a = getUtility(IArchiveSet).get(self.private_ppa.id)
- self.assertRaises(Unauthorized, self._getDescription, p3a)
+ # You cannot access private PPAs without a subscription.
+ ppa = self.factory.makeArchive(private=True)
+ non_subscriber = self.factory.makePerson()
+ with person_logged_in(non_subscriber):
+ self.assertRaises(Unauthorized, getattr, ppa, 'description')
def test_subscription(self):
- login_person(self.joe)
- p3a = getUtility(IArchiveSet).get(self.private_ppa.id)
- self.assertEqual(self._getDescription(p3a), "Foo")
+ # Once you have a subscription, you can access private PPAs.
+ ppa = self.factory.makeArchive(private=True, description="Foo")
+ subscriber = self.factory.makePerson()
+ with person_logged_in(ppa.owner):
+ ppa.newSubscription(subscriber, ppa.owner)
+ with person_logged_in(subscriber):
+ self.assertEqual(ppa.description, "Foo")
+
+ def test_commercial_security(self):
+ # Commercial private PPAs cannot be accessed by non-subscribers.
+ ppa_name = self.factory.getUniqueString()
+ ppa = self.factory.makeArchive(
+ private=True, commercial=True, name=ppa_name)
+ non_subscriber = self.factory.makePerson()
+ with person_logged_in(non_subscriber):
+ self.assertEqual(ppa_name, ppa.name)
+ self.assertRaises(Unauthorized, getattr, ppa, 'description')
class TestArchivePrivacySwitching(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
- def setUp(self):
- """Create a public and a private PPA."""
- super(TestArchivePrivacySwitching, self).setUp()
- self.public_ppa = self.factory.makeArchive()
- self.private_ppa = self.factory.makeArchive()
- self.private_ppa.private = True
-
- def make_ppa_private(self, ppa):
+ def set_ppa_privacy(self, ppa, private):
"""Helper method to privatise a ppa."""
- ppa.private = True
-
- def make_ppa_public(self, ppa):
- """Helper method to make a PPA public (and use for assertRaises)."""
- ppa.private = False
+ ppa.private = private
def test_switch_privacy_no_pubs_succeeds(self):
# Changing the privacy is fine if there are no publishing
# records.
- self.make_ppa_private(self.public_ppa)
- self.assertTrue(self.public_ppa.private)
+ public_ppa = self.factory.makeArchive()
+ self.set_ppa_privacy(public_ppa, private=True)
+ self.assertTrue(public_ppa.private)
- self.private_ppa.private = False
- self.assertFalse(self.private_ppa.private)
+ private_ppa = self.factory.makeArchive(private=True)
+ self.set_ppa_privacy(private_ppa, private=False)
+ self.assertFalse(private_ppa.private)
def test_switch_privacy_with_pubs_fails(self):
# Changing the privacy is not possible when the archive already
# has published sources.
+ public_ppa = self.factory.makeArchive(private=False)
publisher = SoyuzTestPublisher()
publisher.prepareBreezyAutotest()
- publisher.getPubSource(archive=self.public_ppa)
- publisher.getPubSource(archive=self.private_ppa)
-
- self.assertRaises(
- CannotSwitchPrivacy, self.make_ppa_private, self.public_ppa)
-
- self.assertRaises(
- CannotSwitchPrivacy, self.make_ppa_public, self.private_ppa)
+
+ private_ppa = self.factory.makeArchive(private=True)
+ publisher.getPubSource(archive=public_ppa)
+ publisher.getPubSource(archive=private_ppa)
+
+ self.assertRaises(
+ CannotSwitchPrivacy,
+ self.set_ppa_privacy, public_ppa, private=True)
+
+ self.assertRaises(
+ CannotSwitchPrivacy,
+ self.set_ppa_privacy, private_ppa, private=False)
def test_buildd_secret_was_generated(self):
- self.make_ppa_private(self.public_ppa)
- self.assertNotEqual(self.public_ppa.buildd_secret, None)
+ public_ppa = self.factory.makeArchive()
+ self.set_ppa_privacy(public_ppa, private=True)
+ self.assertNotEqual(public_ppa.buildd_secret, None)
def test_discard_buildd_was_discarded(self):
- self.make_ppa_public(self.private_ppa)
- self.assertEqual(self.private_ppa.buildd_secret, None)
+ private_ppa = self.factory.makeArchive(private=True)
+ self.set_ppa_privacy(private_ppa, private=False)
+ self.assertEqual(private_ppa.buildd_secret, None)
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-04-27 18:50:49 +0000
+++ lib/lp/testing/factory.py 2012-05-01 11:30:33 +0000
@@ -2663,7 +2663,8 @@
def makeArchive(self, distribution=None, owner=None, name=None,
purpose=None, enabled=True, private=False,
- virtualized=True, description=None, displayname=None):
+ virtualized=True, description=None, displayname=None,
+ commercial=False):
"""Create and return a new arbitrary archive.
:param distribution: Supply IDistribution, defaults to a new one
@@ -2676,6 +2677,8 @@
:param private: Whether the archive is created private.
:param virtualized: Whether the archive is virtualized.
:param description: A description of the archive.
+ :param commercial: Whether the archive is commercial. Defaults to
+ False.
"""
if purpose is None:
purpose = ArchivePurpose.PPA
@@ -2714,6 +2717,10 @@
naked_archive.private = True
naked_archive.buildd_secret = "sekrit"
+ if commercial:
+ naked_archive = removeSecurityProxy(archive)
+ naked_archive.commercial = True
+
return archive
def makeArchiveAdmin(self, archive=None):
Follow ups
