launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08156
[Merge] lp:~stevenk/launchpad/branch-use-information_type-redux into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/branch-use-information_type-redux into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/branch-use-information_type-redux/+merge/107150
Building on https://code.launchpad.net/~stevenk/launchpad/branch-use-information_type/+merge/106109 which got reverted due to test failures.
I have fixed the test failures, and have changed transitionToInformationType so it will change the information_type of branches stacked on the branch which information_type is being changed.
--
https://code.launchpad.net/~stevenk/launchpad/branch-use-information_type-redux/+merge/107150
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/branch-use-information_type-redux into lp:launchpad.
=== modified file 'lib/lp/app/browser/tests/test_launchpad.py'
--- lib/lp/app/browser/tests/test_launchpad.py 2012-02-23 23:44:00 +0000
+++ lib/lp/app/browser/tests/test_launchpad.py 2012-05-24 02:20:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for traversal from the root branch object."""
@@ -20,6 +20,7 @@
from lp.app.errors import GoneError
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
+from lp.registry.enums import InformationType
from lp.registry.interfaces.person import (
IPersonSet,
PersonVisibility,
@@ -192,7 +193,8 @@
branch = self.factory.makeProductBranch()
naked_product = removeSecurityProxy(branch.product)
ICanHasLinkedBranch(naked_product).setBranch(branch)
- removeSecurityProxy(branch).explicitly_private = True
+ removeSecurityProxy(branch).information_type = (
+ InformationType.USERDATA)
login(ANONYMOUS)
requiredMessage = (
u"The target %s does not have a linked branch." %
@@ -218,7 +220,8 @@
branch = self.factory.makeProductBranch()
naked_product = removeSecurityProxy(branch.product)
ICanHasLinkedBranch(naked_product).setBranch(branch)
- removeSecurityProxy(branch).explicitly_private = True
+ removeSecurityProxy(branch).information_type = (
+ InformationType.USERDATA)
login(ANONYMOUS)
self.assertNotFound(naked_product.name, use_default_referer=False)
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2012-05-23 22:27:56 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2012-05-24 02:20:24 +0000
@@ -758,7 +758,8 @@
# If the target is private, the landing targets should not include it.
bmp = self.factory.makeBranchMergeProposal()
branch = bmp.source_branch
- removeSecurityProxy(bmp.target_branch).explicitly_private = True
+ removeSecurityProxy(bmp.target_branch).information_type = (
+ InformationType.USERDATA)
view = BranchView(branch, LaunchpadTestRequest())
self.assertTrue(view.no_merges)
self.assertEqual([], view.landing_targets)
@@ -779,7 +780,8 @@
# it.
bmp = self.factory.makeBranchMergeProposal()
branch = bmp.target_branch
- removeSecurityProxy(bmp.source_branch).explicitly_private = True
+ removeSecurityProxy(bmp.source_branch).information_type = (
+ InformationType.USERDATA)
view = BranchView(branch, LaunchpadTestRequest())
self.assertTrue(view.no_merges)
self.assertEqual([], view.landing_candidates)
@@ -799,7 +801,8 @@
# the target is private, then the dependent_branches are not shown.
branch = self.factory.makeProductBranch()
bmp = self.factory.makeBranchMergeProposal(prerequisite_branch=branch)
- removeSecurityProxy(bmp.source_branch).explicitly_private = True
+ removeSecurityProxy(bmp.source_branch).information_type = (
+ InformationType.USERDATA)
view = BranchView(branch, LaunchpadTestRequest())
self.assertTrue(view.no_merges)
self.assertEqual([], view.dependent_branches)
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2012-05-23 22:27:56 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2012-05-24 02:20:24 +0000
@@ -1290,7 +1290,8 @@
bmp2 = self.factory.makeBranchMergeProposal(
date_created=(
datetime(year=2008, month=10, day=10, tzinfo=pytz.UTC)))
- removeSecurityProxy(bmp2.source_branch).explicitly_private = True
+ removeSecurityProxy(bmp2.source_branch).information_type = (
+ InformationType.USERDATA)
self.assertEqual(
[bmp1], latest_proposals_for_each_branch([bmp1, bmp2]))
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2012-05-23 22:27:56 +0000
+++ lib/lp/code/errors.py 2012-05-24 02:20:24 +0000
@@ -10,6 +10,7 @@
'BadStateTransition',
'BranchCannotBePrivate',
'BranchCannotBePublic',
+ 'BranchCannotChangeInformationType',
'BranchCreationException',
'BranchCreationForbidden',
'BranchCreatorNotMemberOfOwnerTeam',
@@ -155,6 +156,10 @@
"""The branch cannot be made private."""
+class BranchCannotChangeInformationType(Exception):
+ """The information type of this branch cannot be changed."""
+
+
class InvalidBranchException(Exception):
"""Base exception for an error resolving a branch for a component.
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2012-05-23 22:27:56 +0000
+++ lib/lp/code/model/branch.py 2012-05-24 02:20:24 +0000
@@ -77,6 +77,7 @@
AlreadyLatestFormat,
BranchCannotBePrivate,
BranchCannotBePublic,
+ BranchCannotChangeInformationType,
BranchMergeProposalExists,
BranchTargetError,
BranchTypeError,
@@ -128,7 +129,11 @@
)
from lp.code.model.seriessourcepackagebranch import SeriesSourcePackageBranch
from lp.codehosting.safe_open import safe_open
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+ InformationType,
+ PRIVATE_INFORMATION_TYPES,
+ PUBLIC_INFORMATION_TYPES,
+ )
from lp.registry.interfaces.person import (
validate_person,
validate_public_person,
@@ -187,33 +192,47 @@
@property
def private(self):
- return self.transitively_private
+ return self.information_type in PRIVATE_INFORMATION_TYPES
def setPrivate(self, private, user):
"""See `IBranch`."""
- if private == self.explicitly_private:
+ if private:
+ information_type = InformationType.USERDATA
+ else:
+ information_type = InformationType.PUBLIC
+ return self.transitionToInformationType(information_type, user)
+
+ def transitionToInformationType(self, information_type, who):
+ """See `IBranch`."""
+ if self.information_type == information_type:
return
+ if (self.stacked_on
+ and self.stacked_on.information_type in PRIVATE_INFORMATION_TYPES
+ and information_type in PUBLIC_INFORMATION_TYPES):
+ raise BranchCannotChangeInformationType()
+ private = information_type in PRIVATE_INFORMATION_TYPES
# Only check the privacy policy if the user is not special.
- if (not user_has_special_branch_access(user)):
+ if (not user_has_special_branch_access(who)):
policy = IBranchNamespacePolicy(self.namespace)
if private and not policy.canBranchesBePrivate():
raise BranchCannotBePrivate()
if not private and not policy.canBranchesBePublic():
raise BranchCannotBePublic()
+ self.information_type = information_type
+ # Change the information_type of any stacked branches.
+ children = list(self.getStackedBranches())
+ for child in children:
+ children.extend(list(child.getStackedBranches()))
+ child.information_type = information_type
+ # Set the legacy values for now.
self.explicitly_private = private
# If this branch is private, then it is also transitively_private
# otherwise we need to reload the value.
if private:
self.transitively_private = True
- self.information_type = InformationType.USERDATA
else:
self.transitively_private = AutoReload
- self.information_type = InformationType.PUBLIC
-
- def transitionToInformationType(self, information_type, who):
- """See `IBranch`."""
- self.information_type = information_type
registrant = ForeignKey(
dbName='registrant', foreignKey='Person',
@@ -1052,6 +1071,13 @@
self.mirror_status_message = (
'Invalid stacked on location: ' + stacked_on_url)
self.stacked_on = stacked_on_branch
+ # If the branch we are stacking on is not public, and we are,
+ # set our information_type to the stacked on's, since having a
+ # public branch stacked on a private branch does not make sense.
+ if (self.stacked_on
+ and self.stacked_on.information_type in PRIVATE_INFORMATION_TYPES
+ and self.information_type in PUBLIC_INFORMATION_TYPES):
+ self.information_type = self.stacked_on.information_type
if self.branch_type == BranchType.HOSTED:
self.last_mirrored = UTC_NOW
else:
@@ -1207,7 +1233,7 @@
This method doesn't check the stacked upon branch. That is handled by
the `visibleByUser` method.
"""
- if not self.explicitly_private:
+ if self.information_type not in PRIVATE_INFORMATION_TYPES:
return True
if user is None:
return False
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2012-05-23 22:27:56 +0000
+++ lib/lp/code/model/tests/test_branch.py 2012-05-24 02:20:24 +0000
@@ -56,6 +56,7 @@
AlreadyLatestFormat,
BranchCannotBePrivate,
BranchCannotBePublic,
+ BranchCannotChangeInformationType,
BranchCreatorNotMemberOfOwnerTeam,
BranchCreatorNotOwner,
BranchTargetError,
@@ -2334,16 +2335,21 @@
def test_public_stacked_on_private_is_private(self):
# A public branch stacked on a private branch is private.
stacked_on = self.factory.makeBranch(private=True)
- branch = self.factory.makeBranch(stacked_on=stacked_on, private=False)
+ branch = self.factory.makeBranch(
+ stacked_on=stacked_on, private=False)
self.assertTrue(branch.private)
+ self.assertEqual(
+ stacked_on.information_type, branch.information_type)
self.assertTrue(removeSecurityProxy(branch).transitively_private)
self.assertFalse(branch.explicitly_private)
def test_private_stacked_on_public_is_private(self):
- # A public branch stacked on a private branch is private.
+ # A private branch stacked on a public branch is private.
stacked_on = self.factory.makeBranch(private=False)
branch = self.factory.makeBranch(stacked_on=stacked_on, private=True)
self.assertTrue(branch.private)
+ self.assertNotEqual(
+ stacked_on.information_type, branch.information_type)
self.assertTrue(removeSecurityProxy(branch).transitively_private)
self.assertTrue(branch.explicitly_private)
@@ -2436,6 +2442,26 @@
branch.setPrivate,
False, branch.owner)
+ def test_cannot_transition_with_private_stacked_on(self):
+ # If a public branch is stacked on a private branch, it can not
+ # change its information_type to public.
+ stacked_on = self.factory.makeBranch(private=True)
+ branch = self.factory.makeBranch(stacked_on=stacked_on)
+ self.assertRaises(
+ BranchCannotChangeInformationType,
+ branch.transitionToInformationType, InformationType.PUBLIC,
+ branch.owner)
+
+ def test_can_transition_with_public_stacked_on(self):
+ # If a private branch is stacked on a public branch, it can change
+ # its information_type.
+ stacked_on = self.factory.makeBranch()
+ branch = self.factory.makeBranch(stacked_on=stacked_on, private=True)
+ branch.transitionToInformationType(
+ InformationType.UNEMBARGOEDSECURITY, branch.owner)
+ self.assertEqual(
+ InformationType.UNEMBARGOEDSECURITY, branch.information_type)
+
class TestBranchCommitsForDays(TestCaseWithFactory):
"""Tests for `Branch.commitsForDays`."""
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py 2012-05-23 22:27:56 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py 2012-05-24 02:20:24 +0000
@@ -929,8 +929,9 @@
charlie, BranchSubscriptionNotificationLevel.NOEMAIL, None,
CodeReviewNotificationLevel.FULL, charlie)
# Make both branches private.
- removeSecurityProxy(bmp.source_branch).explicitly_private = True
- removeSecurityProxy(bmp.target_branch).explicitly_private = True
+ for branch in (bmp.source_branch, bmp.target_branch):
+ removeSecurityProxy(branch).information_type = (
+ InformationType.USERDATA)
recipients = bmp.getNotificationRecipients(
CodeReviewNotificationLevel.FULL)
self.assertFalse(bob in recipients)
@@ -1505,7 +1506,7 @@
base_branch = self.factory.makeBranch(
owner=owner, private=True, product=product)
source_branch = self.factory.makeBranch(
- stacked_on=base_branch, product=product)
+ stacked_on=base_branch, product=product, owner=owner)
target_branch = self.factory.makeBranch(owner=owner, product=product)
target_branch.product.setBranchVisibilityTeamPolicy(
owner, BranchVisibilityRule.PRIVATE)
=== modified file 'lib/lp/code/model/tests/test_branchvisibility.py'
--- lib/lp/code/model/tests/test_branchvisibility.py 2012-05-23 22:27:56 +0000
+++ lib/lp/code/model/tests/test_branchvisibility.py 2012-05-24 02:20:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for visibility of branches.
@@ -134,14 +134,11 @@
private_owner = self.factory.makePerson()
test_branches = []
for x in range(5):
- # We want the first 3 public and the last 3 private
- branch = self.factory.makeBranch(name='branch_%s' % x)
- # The 3rd, 4th and 5th will be explicitly private.
- branch.explicitly_private = x > 2
+ # We want the first 3 public and the last 3 private.
+ branch = self.factory.makeBranch(private=x > 2)
test_branches.append(branch)
test_branches.append(
- self.factory.makeBranch(
- name='branch_5', private=True, owner=private_owner))
+ self.factory.makeBranch(private=True, owner=private_owner))
# Anonymous users see just the public branches.
branch_info = [(branch, branch.private)
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py 2012-02-28 11:14:44 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py 2012-05-24 02:20:24 +0000
@@ -50,6 +50,7 @@
)
from lp.code.model.sourcepackagerecipedata import SourcePackageRecipeData
from lp.code.tests.helpers import recipe_parser_newest_version
+from lp.registry.enums import InformationType
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.database.bulk import load_referencing
from lp.services.database.constants import UTC_NOW
@@ -529,7 +530,8 @@
with person_logged_in(owner):
recipe = self.factory.makeSourcePackageRecipe(branches=[branch])
self.assertTrue(check_permission('launchpad.View', recipe))
- removeSecurityProxy(branch).explicitly_private = True
+ removeSecurityProxy(branch).information_type = (
+ InformationType.USERDATA)
with person_logged_in(self.factory.makePerson()):
self.assertFalse(check_permission('launchpad.View', recipe))
self.assertFalse(check_permission('launchpad.View', recipe))
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2012-04-24 06:44:30 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2012-05-24 02:20:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for source package builds."""
@@ -38,6 +38,7 @@
SourcePackageRecipeBuildMailer,
)
from lp.code.model.sourcepackagerecipebuild import SourcePackageRecipeBuild
+from lp.registry.enums import InformationType
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
from lp.services.database.lpstorm import IStore
@@ -176,7 +177,8 @@
job = build.makeJob()
self.assertTrue(check_permission('launchpad.View', build))
self.assertTrue(check_permission('launchpad.View', job))
- removeSecurityProxy(branch).explicitly_private = True
+ removeSecurityProxy(branch).information_type = (
+ InformationType.USERDATA)
with person_logged_in(self.factory.makePerson()):
self.assertFalse(check_permission('launchpad.View', build))
self.assertFalse(check_permission('launchpad.View', job))
=== modified file 'lib/lp/code/xmlrpc/tests/test_branch.py'
--- lib/lp/code/xmlrpc/tests/test_branch.py 2012-05-23 22:27:56 +0000
+++ lib/lp/code/xmlrpc/tests/test_branch.py 2012-05-24 02:20:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit tests for the public codehosting API."""
@@ -16,6 +16,7 @@
from lp.code.interfaces.codehosting import BRANCH_ALIAS_PREFIX
from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
from lp.code.xmlrpc.branch import PublicCodehostingAPI
+from lp.registry.enums import InformationType
from lp.services.xmlrpc import LaunchpadFault
from lp.testing import (
person_logged_in,
@@ -323,7 +324,8 @@
def test_private_branch_as_development_focus(self):
# We resolve private linked branches using the writable alias.
product, trunk = self.makeProdutWithTrunk()
- removeSecurityProxy(trunk).explicitly_private = True
+ removeSecurityProxy(trunk).information_type = (
+ InformationType.USERDATA)
self.assertOnlyWritableResolves(product.name)
def test_private_branch_as_user(self):
=== modified file 'lib/lp/registry/stories/product/xx-product-development-focus.txt'
--- lib/lp/registry/stories/product/xx-product-development-focus.txt 2012-01-15 13:32:27 +0000
+++ lib/lp/registry/stories/product/xx-product-development-focus.txt 2012-05-24 02:20:24 +0000
@@ -14,7 +14,9 @@
>>> eric = factory.makePerson(name='eric', email='eric@xxxxxxxxxxx')
>>> fooix = factory.makeProduct(name='fooix', owner=eric)
>>> branch = factory.makeBranch(owner=eric, product=fooix, name='trunk')
- >>> # Make revisions for the branch so it has a codebrowse link.
+
+Make revisions for the branch so it has a codebrowse link.
+
>>> factory.makeRevisionsForBranch(branch)
>>> logout()
@@ -145,7 +147,9 @@
>>> login('admin@xxxxxxxxxxxxx')
>>> from zope.security.proxy import removeSecurityProxy
- >>> removeSecurityProxy(branch).explicitly_private = True
+ >>> from lp.registry.enums import InformationType
+ >>> removeSecurityProxy(branch).information_type = (
+ ... InformationType.USERDATA)
>>> logout()
>>> anon_browser.open('http://launchpad.dev/fooix')
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-05-23 22:27:56 +0000
+++ lib/lp/testing/factory.py 2012-05-24 02:20:24 +0000
@@ -140,6 +140,7 @@
DistroSeriesDifferenceStatus,
DistroSeriesDifferenceType,
InformationType,
+ PRIVATE_INFORMATION_TYPES,
)
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactGrantSource,
@@ -1072,8 +1073,8 @@
def makeBranch(self, branch_type=None, owner=None,
name=None, product=_DEFAULT, url=_DEFAULT, registrant=None,
- private=False, stacked_on=None, sourcepackage=None,
- reviewer=None, **optional_branch_args):
+ private=None, information_type=None, stacked_on=None,
+ sourcepackage=None, reviewer=None, **optional_branch_args):
"""Create and return a new, arbitrary Branch of the given type.
Any parameters for `IBranchNamespace.createBranch` can be specified to
@@ -1119,11 +1120,23 @@
branch = namespace.createBranch(
branch_type=branch_type, name=name, registrant=registrant,
url=url, **optional_branch_args)
- if private:
- removeSecurityProxy(branch).explicitly_private = True
- removeSecurityProxy(branch).transitively_private = True
+ assert information_type is None or private is None, (
+ "Can not specify both information_type and private")
+ if information_type is not None or private is not None:
+ if information_type:
+ private = information_type in PRIVATE_INFORMATION_TYPES
+ else:
+ information_type = (
+ InformationType.USERDATA if private else
+ InformationType.PUBLIC)
+ if private:
+ removeSecurityProxy(branch).explicitly_private = True
+ removeSecurityProxy(branch).transitively_private = True
+ removeSecurityProxy(branch).information_type = information_type
if stacked_on is not None:
- removeSecurityProxy(branch).stacked_on = stacked_on
+ removeSecurityProxy(branch).branchChanged(
+ removeSecurityProxy(stacked_on).unique_name, 'rev1', None,
+ None, None)
if reviewer is not None:
removeSecurityProxy(branch).reviewer = reviewer
return branch
=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildjob.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2012-01-01 02:58:52 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildjob.py 2012-05-24 02:20:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -17,6 +17,7 @@
from lp.code.model.branchjob import BranchJob
from lp.code.model.directbranchcommit import DirectBranchCommit
from lp.codehosting.scanner import events
+from lp.registry.enums import InformationType
from lp.services.job.model.job import Job
from lp.services.webapp.interfaces import (
DEFAULT_FLAVOR,
@@ -249,7 +250,8 @@
def test_private_branch(self):
# We don't generate templates for private branches.
branch = self._makeTranslationBranch(fake_pottery_compatible=True)
- removeSecurityProxy(branch).explicitly_private = True
+ removeSecurityProxy(branch).information_type = (
+ InformationType.USERDATA)
self.assertFalse(self.jobsource.generatesTemplates(branch))
def test_scheduleTranslationTemplatesBuild_subscribed(self):
Follow ups
