launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07952
[Merge] lp:~stevenk/launchpad/branch-use-information_type into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/branch-use-information_type into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #933768 in Launchpad itself: "Update branch to use information_visibility_policy"
https://bugs.launchpad.net/launchpad/+bug/933768
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/branch-use-information_type/+merge/106109
Switch Branch to using information_type by default. I have also moved the code from setPrivate to transitionToInformationType and made setPrivate a wrapper around it. Changing the information_type of a branch that is stacked on a non-public branch is now forbidden. IBranch.branchChanged() now forces the branch that is being stacked on top of an existing branch will have its information_type set to the stacked on information_type if it's not public.
I have changed the factory method makeBranch to take an information_type argument, and have also made it call IBranch.branchChanged() if stacked_on is set, which more closely mirrors what happens in production.
This necessitated changing a fair number of tests, which naively just set explicitly_private by hand. I've converted them to setting information_type, but am willing to make use of IBranch.setPrivate() or IBranch.transistionToInformationType() instead.
--
https://code.launchpad.net/~stevenk/launchpad/branch-use-information_type/+merge/106109
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/branch-use-information_type into lp:launchpad.
=== modified file 'lib/lp/code/adapters/branch.py'
--- lib/lp/code/adapters/branch.py 2011-09-26 20:00:50 +0000
+++ lib/lp/code/adapters/branch.py 2012-05-17 04:48:18 +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).
"""Components related to branches."""
@@ -8,6 +8,7 @@
"BranchDelta",
"BranchMergeProposalDelta",
"BranchMergeProposalNoPreviewDiffDelta",
+ "convert_to_information_type",
]
from contextlib import contextmanager
@@ -23,6 +24,7 @@
IBranchDelta,
)
from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
+from lp.registry.enums import InformationType
# XXX: thumper 2006-12-20: This needs to be extended
# to cover bugs and specs linked and unlinked, as
@@ -150,3 +152,10 @@
new_values = tuple(
name for name in BranchMergeProposalDelta.new_values
if name != "preview_diff")
+
+
+def convert_to_information_type(private):
+ if private:
+ return InformationType.USERDATA
+ else:
+ return InformationType.PUBLIC
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2012-05-02 05:25:11 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2012-05-17 04:48:18 +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-02 05:25:11 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2012-05-17 04:48:18 +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-02-21 22:46:28 +0000
+++ lib/lp/code/errors.py 2012-05-17 04:48:18 +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 branch cannot change its information type."""
+
+
class InvalidBranchException(Exception):
"""Base exception for an error resolving a branch for a component.
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2012-05-04 04:52:24 +0000
+++ lib/lp/code/interfaces/branch.py 2012-05-17 04:48:18 +0000
@@ -53,6 +53,7 @@
Reference,
ReferenceChoice,
)
+from lazr.restful.interface import copy_field
from zope.component import getUtility
from zope.interface import (
Attribute,
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2012-05-04 04:52:24 +0000
+++ lib/lp/code/model/branch.py 2012-05-17 04:48:18 +0000
@@ -61,6 +61,7 @@
)
from lp.bugs.interfaces.bugtaskfilter import filter_bugtasks_by_context
from lp.buildmaster.model.buildqueue import BuildQueue
+from lp.code.adapters.branch import convert_to_information_type
from lp.code.bzr import (
BranchFormat,
ControlFormat,
@@ -77,6 +78,7 @@
AlreadyLatestFormat,
BranchCannotBePrivate,
BranchCannotBePublic,
+ BranchCannotChangeInformationType,
BranchMergeProposalExists,
BranchTargetError,
BranchTypeError,
@@ -128,7 +130,10 @@
)
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,
+ )
from lp.registry.interfaces.person import (
validate_person,
validate_public_person,
@@ -186,34 +191,45 @@
enum=InformationType, default=InformationType.PUBLIC)
@property
+ def _private(self):
+ return self.transitively_private
+
+ @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:
+ return self.transitionToInformationType(
+ convert_to_information_type(private), 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 !=
+ InformationType.PUBLIC and information_type !=
+ self.stacked_on.information_type):
+ 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
+ # 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 +1068,10 @@
self.mirror_status_message = (
'Invalid stacked on location: ' + stacked_on_url)
self.stacked_on = stacked_on_branch
+ if (
+ self.stacked_on and self.stacked_on.information_type !=
+ InformationType.PUBLIC):
+ self.information_type = self.stacked_on.information_type
if self.branch_type == BranchType.HOSTED:
self.last_mirrored = UTC_NOW
else:
@@ -1207,7 +1227,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-04 04:52:24 +0000
+++ lib/lp/code/model/tests/test_branch.py 2012-05-17 04:48:18 +0000
@@ -2340,7 +2340,7 @@
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)
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py 2012-05-02 05:25:11 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py 2012-05-17 04:48:18 +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)
=== modified file 'lib/lp/code/model/tests/test_branchvisibility.py'
--- lib/lp/code/model/tests/test_branchvisibility.py 2012-01-15 17:43:05 +0000
+++ lib/lp/code/model/tests/test_branchvisibility.py 2012-05-17 04:48:18 +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,10 +134,9 @@
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(
+ name='branch_%s' % x, private=x > 2)
test_branches.append(branch)
test_branches.append(
self.factory.makeBranch(
=== modified file 'lib/lp/code/xmlrpc/tests/test_branch.py'
--- lib/lp/code/xmlrpc/tests/test_branch.py 2012-01-01 02:58:52 +0000
+++ lib/lp/code/xmlrpc/tests/test_branch.py 2012-05-17 04:48:18 +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/testing/factory.py'
--- lib/lp/testing/factory.py 2012-05-04 14:52:44 +0000
+++ lib/lp/testing/factory.py 2012-05-17 04:48:18 +0000
@@ -1072,8 +1072,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=False, 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 +1119,17 @@
branch = namespace.createBranch(
branch_type=branch_type, name=name, registrant=registrant,
url=url, **optional_branch_args)
+ if information_type:
+ removeSecurityProxy(branch).information_type = information_type
if private:
removeSecurityProxy(branch).explicitly_private = True
removeSecurityProxy(branch).transitively_private = True
+ if not information_type:
+ removeSecurityProxy(branch).information_type = (
+ InformationType.USERDATA)
if stacked_on is not None:
- removeSecurityProxy(branch).stacked_on = stacked_on
+ removeSecurityProxy(branch).branchChanged(
+ stacked_on.unique_name, 'rev1', None, None, None)
if reviewer is not None:
removeSecurityProxy(branch).reviewer = reviewer
return branch
Follow ups
