launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21592
[Merge] lp:~cjwatson/launchpad/remove-branch-set-api into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-branch-set-api into lp:launchpad.
Commit message:
Remove BranchSetAPI. Since the calling code in bzr relies on password authentication, it can't have worked since at least 2012.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-branch-set-api/+merge/324474
https://code.launchpad.net/~cjwatson/bzr/remove-register-branch/+merge/324472 removes the bzr side. Ordinarily we'd probably need to go through a deprecation cycle or something, but since this API has been unusable for years we've effectively done so already, albeit by accident.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-branch-set-api into lp:launchpad.
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2017-05-04 16:02:40 +0000
+++ lib/lp/code/configure.zcml 2017-05-23 12:56:33 +0000
@@ -182,11 +182,6 @@
<xmlrpc:view
for="lp.code.interfaces.codehosting.IBazaarApplication"
- interface="lp.code.xmlrpc.branch.IBranchSetAPI"
- class="lp.code.xmlrpc.branch.BranchSetAPI"
- permission="launchpad.AnyPerson"/>
- <xmlrpc:view
- for="lp.code.interfaces.codehosting.IBazaarApplication"
interface="lp.code.xmlrpc.branch.IPublicCodehostingAPI"
class="lp.code.xmlrpc.branch.PublicCodehostingAPI"
permission="zope.Public"/>
=== modified file 'lib/lp/code/doc/branch-xmlrpc.txt'
--- lib/lp/code/doc/branch-xmlrpc.txt 2016-02-04 04:59:12 +0000
+++ lib/lp/code/doc/branch-xmlrpc.txt 2017-05-23 12:56:33 +0000
@@ -1,310 +1,8 @@
-IBranchSetAPI
-=============
+IPublicCodehostingAPI
+=====================
- >>> from zope.security.proxy import removeSecurityProxy
- >>> from datetime import datetime
- >>> import pytz
>>> import xmlrpclib
>>> from lp.testing.xmlrpc import XMLRPCTestTransport
- >>> branchset_api = xmlrpclib.ServerProxy(
- ... 'http://foo.bar@xxxxxxxxxxxxx:test@xxxxxxxxxxxxxxxxxxxx/bazaar/',
- ... transport=XMLRPCTestTransport())
-
-
-register_branch
-===============
-
-register_branch(branch_url, branch_name, branch_title,
- branch_description, author_email, product_name,
- owner_name='')
-
-Let's register a branch using register_branch(). First we make sure that
-there is no branch with the URL we are going to register.
-
- >>> from lp.code.interfaces.branchlookup import IBranchLookup
- >>> branch_url = 'http://foo.com/branch'
- >>> getUtility(IBranchLookup).getByUrl(branch_url) is None
- True
-
-The only thing that we have to supply is the branch url. The rest is
-optional, so we pass '' for the rest of the parameters. The reason we
-pass '' is that XMLRPC doesn't allow optional parameters, and None is an
-extension to XMLRPC which we don't want to rely on. We use the currently
-authenticated user as the owner of the branch.
-
- >>> branchset_api.register_branch(
- ... 'http://foo.com/branch', '', '', '', '', '')
- 'http://code.launchpad.dev/~name16/+junk/branch'
-
-Now we can see that a branch got registered:
-
- >>> new_branch = getUtility(IBranchLookup).getByUrl(branch_url)
-
-The author_email is now ignored as the author attribute has been
-removed from the branch object.
-
- >>> print new_branch.owner.displayname
- Foo Bar
-
-The name got set to the last component of the URL since it wasn't
-specified:
-
- >>> print new_branch.name
- branch
-
-Since we didn't specify product, title or description, they are None:
-
- >>> new_branch.product is None
- True
-
-Also, Launchpad is scheduled to mirror this branch on its next mirror run:
-
- >>> removeSecurityProxy(new_branch).sync()
- >>> print new_branch.next_mirror_time is not None
- True
- >>> print new_branch.next_mirror_time < datetime.now(pytz.timezone('UTC'))
- True
-
-(We remove the security proxy in order to ensure that next_mirror_time has a
-real object in it, rather than an expression builder.)
-
-Let's register a branch specifying the name, description and product as
-well:
-
- >>> other_branch_url = 'http://foo.com/other_branch'
- >>> branchset_api.register_branch(
- ... other_branch_url, 'branch_name', 'branch_title',
- ... 'branch description', 'test@xxxxxxxxxxxxx', 'evolution')
- 'http://code.launchpad.dev/~name16/evolution/branch_name'
-
- >>> new_branch = getUtility(IBranchLookup).getByUrl(other_branch_url)
- >>> print new_branch.owner.displayname
- Foo Bar
-
- >>> print new_branch.name
- branch_name
- >>> print new_branch.product.displayname
- Evolution
-
-If we try to register a branch with a branch name that is already used:
-
- >>> yet_another_branch_url = 'http://foo.com/yet_another_branch'
- >>> branchset_api.register_branch(
- ... yet_another_branch_url, 'branch_name', 'branch_title',
- ... 'branch description', 'test@xxxxxxxxxxxxx', 'evolution')
- Traceback (most recent call last):
- ...
- Fault: <Fault 220: 'Branch name already in use:
- A branch with the name "branch_name" already exists
- for name16 in evolution.'>
-
-However a branch can be registered with a name used by a different user.
-
- >>> evo_main_url = 'http://foo.example.com/evo/main'
- >>> branchset_api.register_branch(
- ... evo_main_url, 'main', 'branch_title',
- ... 'branch description', 'test@xxxxxxxxxxxxx', 'evolution')
- 'http://code.launchpad.dev/~name16/evolution/main'
-
-This check must also detect branch name conflict for +junk branches:
-
- >>> branchset_api.register_branch(
- ... yet_another_branch_url, 'branch', '', '', '', '')
- Traceback (most recent call last):
- ...
- Fault: <Fault 220: 'Branch name already in use:
- A junk branch with the name "branch" already exists for name16.'>
-
-If we give a product name that doesn't exist, we get an error:
-
- >>> new_branch_url = 'http://foo.com/new_branch'
- >>> branchset_api.register_branch(
- ... new_branch_url, 'branch_name', 'branch_title',
- ... 'branch description', 'test@xxxxxxxxxxxxx', 'non-existing')
- Traceback (most recent call last):
- ...
- Fault: <Fault 10: 'No such project: non-existing'>
-
-If we try to register a branch with an invalid URI:
-
- >>> invalid_uri = '.'
- >>> branchset_api.register_branch(
- ... invalid_uri, 'branch_name', 'branch title', 'branch description',
- ... 'test@xxxxxxxxxxxxx', 'evolution')
- Traceback (most recent call last):
- ...
- Fault: <Fault 120: 'Invalid URL: .\n"." is not a valid URI'>
-
-If we try to register a URL with a unsupported scheme:
-
- >>> bad_scheme = 'svn://chinstrap.ubuntu.com/foo/bar/baz'
- >>> branchset_api.register_branch(
- ... bad_scheme, 'branch_name', 'branch title', 'branch description',
- ... 'test@xxxxxxxxxxxxx', 'evolution')
- Traceback (most recent call last):
- ...
- Fault: <Fault 120: 'Invalid URL:
- svn://chinstrap.ubuntu.com/foo/bar/baz\nThe URI scheme
- "svn" is not allowed. Only URIs with the following
- schemes may be used: bzr+ssh, ftp, http, https, sftp'>
-
-If we try to register a root URL:
-
- >>> root_url = 'http://example.com/'
- >>> branchset_api.register_branch(
- ... root_url, 'branch_name', 'branch title', 'branch description',
- ... 'test@xxxxxxxxxxxxx', 'evolution')
- Traceback (most recent call last):
- ...
- Fault: <Fault 120: 'Invalid URL: http://example.com\nURLs for branches
- cannot point to the root of a site.'>
-
-If we try to register a URL in launchpad.net (actually, launchpad.dev because
-we are in the testing environment):
-
- >>> launchpad_url = 'https://blueprints.launchpad.dev/foo/bar/baz'
- >>> branchset_api.register_branch(
- ... launchpad_url, 'branch_name', 'branch title',
- ... 'branch description', 'test@xxxxxxxxxxxxx', 'evolution')
- Traceback (most recent call last):
- ...
- Fault: <Fault 120:
- 'Invalid URL: https://blueprints.launchpad.dev/foo/bar/baz\nFor
- Launchpad to mirror a branch, the original branch cannot be on
- <code>launchpad.dev</code>.'>
-
-Registering a branch name that ends in a slash works, but the slash
-gets removed from the stored URL:
-
- >>> branchset_api.register_branch(
- ... new_branch_url + '/', 'new_branch', 'branch_title',
- ... 'branch_description', 'test@xxxxxxxxxxxxx', 'evolution')
- 'http://code.launchpad.dev/~name16/evolution/new_branch'
-
- >>> new_branch = getUtility(IBranchLookup).getByUrl(new_branch_url)
- >>> print new_branch.url
- http://foo.com/new_branch
- >>> print new_branch.owner.name
- name16
- >>> print new_branch.product.name
- evolution
- >>> print new_branch.name
- new_branch
-
-
-If we try to register a branch which is already registered in Launchpad:
-
- >>> getUtility(IBranchLookup).getByUrl(other_branch_url) is not None
- True
- >>> branchset_api.register_branch(
- ... other_branch_url, 'branch_name', 'branch_title',
- ... 'branch description', 'test@xxxxxxxxxxxxx', 'evolution')
- Traceback (most recent call last):
- ...
- Fault: <Fault 50: 'http://foo.com/other_branch is already registered.'>
-
-
-We get the same error if we try to register the branch with a slash appended:
- >>> branchset_api.register_branch(
- ... other_branch_url + '/', 'branch_name', 'branch_title',
- ... 'branch description', 'test@xxxxxxxxxxxxx', 'evolution')
- Traceback (most recent call last):
- ...
- Fault: <Fault 50: 'http://foo.com/other_branch is already registered.'>
-
-Attempting to register a branch for a product where the branch visibility
-policy is forbidden will cause a fault result.
-
- >>> test_user_api = xmlrpclib.ServerProxy(
- ... 'http://no-priv@xxxxxxxxxxxxx:test@xxxxxxxxxxxxxxxxxxxx/bazaar/',
- ... transport=XMLRPCTestTransport())
- >>> test_user_api.register_branch(
- ... 'http://foo.com/bar', 'branch_name', 'branch_title',
- ... 'branch description', 'no-priv@xxxxxxxxxxxxx', 'landscape')
- Traceback (most recent call last):
- ...
- Fault: <Fault 110: 'You are not allowed to create a branch for project:
- The Landscape Project'>
-
-We can register a branch as being owned by a team, rather than just an
-individual:
-
- >>> branchset_api.register_branch(
- ... 'http://example.com/unregistered-as-yet', 'team-branch',
- ... 'new-title', 'new-description', 'test@xxxxxxxxxxxxx',
- ... 'evolution', 'name18')
- 'http://code.launchpad.dev/~name18/evolution/team-branch'
-
-If the team doesn't exist, we raise a Fault that says so:
-
- >>> branchset_api.register_branch(
- ... 'http://example.com/team-dont-exist', 'team-dont-exist',
- ... 'title', 'description', 'test@xxxxxxxxxxxxx', 'evolution',
- ... 'no-existy-teamo')
- Traceback (most recent call last):
- ...
- Fault: <Fault 200: 'No such person or team: no-existy-teamo'>
-
-We can only register branches as belonging to a team that we are a
-member of. Doing otherwise raises a Fault:
-
- >>> branchset_api.register_branch(
- ... 'http://example.com/invalid-team', 'invalid-team', 'title',
- ... 'description', 'test@xxxxxxxxxxxxx', 'evolution',
- ... 'landscape-developers')
- Traceback (most recent call last):
- ...
- Fault: <Fault 250: 'name16 is not a member of landscape-developers.'>
-
-Trying to register a branch with an invalid name will fail with a fault that
-explains the problem:
-
- >>> branchset_api.register_branch(
- ... 'http://example.com/invalid-branch-name', '.bzr', 'title', 'desc',
- ... 'test@xxxxxxxxxxxxx', 'evolution')
- Traceback (most recent call last):
- ...
- Fault: <Fault 260: 'Invalid branch name '.bzr'. Branch names must ...
-
-Crazy unicode characters are also invalid:
-
- >>> branchset_api.register_branch(
- ... 'http://example.com/invalid-branch-name',
- ... u'\N{latin small letter e with acute}', 'title', 'desc',
- ... 'test@xxxxxxxxxxxxx', 'evolution')
- Traceback (most recent call last):
- ...
- Fault: <Fault 260: u'Invalid branch name '\xe9'. Branch names must ...
-
-
-link_branch_to_bug(branch_url, bug_id)
-======================================
-
-link_branch_to_bug() associates a branch with a bug. Let's associate
-the branch we registered with bug 1. If the link is successful, the bug
-URL is returned.
-
- >>> branchset_api.link_branch_to_bug(other_branch_url, '1')
- 'http://bugs.launchpad.dev/bugs/1'
-
-Let's take a look to see that the branch actually got linked to the bug:
-
- >>> from lp.bugs.interfaces.bug import IBugSet
- >>> bug_one = getUtility(IBugSet).get(1)
- >>> for branch in bug_one.linked_branches:
- ... print branch.url
- http://foo.com/other_branch
-
-We get an error if we try to specify a non-existant branch or bug:
-
- >>> branchset_api.link_branch_to_bug('http://foo.com/unknown', '1')
- Traceback (most recent call last):
- ...
- Fault: <Fault 30: 'No such branch: http://foo.com/unknown'>
- >>> branchset_api.link_branch_to_bug(branch_url, '99')
- Traceback (most recent call last):
- ...
- Fault: <Fault 40: 'No such bug: 99'>
resolve_lp_path
@@ -319,9 +17,7 @@
This API is deprecated, and will eventually be replaced with an
equivalent method in the new Launchpad API infrastructure.
-Note that authentication is not required to use this method.
-
- >>> branchset_api = xmlrpclib.ServerProxy(
+ >>> public_codehosting_api = xmlrpclib.ServerProxy(
... 'http://xmlrpc.launchpad.dev/bazaar/',
... transport=XMLRPCTestTransport())
@@ -329,7 +25,8 @@
On success, resolve_lp_path returns a dict containing a single key,
'urls':
- >>> results = branchset_api.resolve_lp_path('~vcs-imports/evolution/main')
+ >>> results = public_codehosting_api.resolve_lp_path(
+ ... '~vcs-imports/evolution/main')
>>> print results.keys()
['urls']
@@ -337,7 +34,8 @@
The value of a key is a list of URLs from which the branch can be
accessed:
- >>> results = branchset_api.resolve_lp_path('~vcs-imports/evolution/main')
+ >>> results = public_codehosting_api.resolve_lp_path(
+ ... '~vcs-imports/evolution/main')
>>> for url in results['urls']:
... print url
bzr+ssh://bazaar.launchpad.dev/~vcs-imports/evolution/main
=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py 2015-10-09 15:49:14 +0000
+++ lib/lp/code/model/branchnamespace.py 2017-05-23 12:56:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Implementations of `IBranchNamespace`."""
@@ -199,10 +199,10 @@
raise BranchExists(existing_branch)
# Not all code paths that lead to branch creation go via a
- # schema-validated form (e.g. the register_branch XML-RPC call or
- # pushing a new branch to codehosting), so we validate the branch name
- # here to give a nicer error message than 'ERROR: new row for relation
- # "branch" violates check constraint "valid_name"...'.
+ # schema-validated form (e.g. pushing a new branch to codehosting),
+ # so we validate the branch name here to give a nicer error message
+ # than 'ERROR: new row for relation "branch" violates check
+ # constraint "valid_name"...'.
IBranch['name'].validate(unicode(name))
def validateMove(self, branch, mover, name=None):
=== modified file 'lib/lp/code/xmlrpc/branch.py'
--- lib/lp/code/xmlrpc/branch.py 2015-09-28 17:38:45 +0000
+++ lib/lp/code/xmlrpc/branch.py 2017-05-23 12:56:33 +0000
@@ -1,12 +1,10 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Branch XMLRPC API."""
__metaclass__ = type
__all__ = [
- 'BranchSetAPI',
- 'IBranchSetAPI',
'IPublicCodehostingAPI',
'PublicCodehostingAPI',
]
@@ -21,21 +19,14 @@
Interface,
)
-from lp.app.errors import NotFoundError
-from lp.app.validators import LaunchpadValidationError
-from lp.bugs.interfaces.bug import IBugSet
from lp.code.enums import BranchType
from lp.code.errors import (
- BranchCreationException,
- BranchCreationForbidden,
CannotHaveLinkedBranch,
InvalidNamespace,
NoLinkedBranch,
NoSuchBranch,
)
-from lp.code.interfaces.branch import IBranch
from lp.code.interfaces.branchlookup import IBranchLookup
-from lp.code.interfaces.branchnamespace import get_branch_namespace
from lp.code.interfaces.codehosting import (
BRANCH_ALIAS_PREFIX,
compose_public_url,
@@ -45,134 +36,18 @@
NoSuchDistroSeries,
NoSuchSourcePackageName,
)
-from lp.registry.interfaces.person import (
- IPersonSet,
- NoSuchPerson,
- )
+from lp.registry.interfaces.person import NoSuchPerson
from lp.registry.interfaces.product import (
InvalidProductName,
- IProductSet,
NoSuchProduct,
)
from lp.registry.interfaces.productseries import NoSuchProductSeries
from lp.services.config import config
-from lp.services.webapp import (
- canonical_url,
- LaunchpadXMLRPCView,
- )
-from lp.services.webapp.interfaces import ILaunchBag
+from lp.services.webapp import LaunchpadXMLRPCView
from lp.xmlrpc import faults
from lp.xmlrpc.helpers import return_fault
-class IBranchSetAPI(Interface):
- """An XMLRPC interface for dealing with branches.
-
- This XML-RPC interface was introduced to support Bazaar 0.8-2, which is
- included in Ubuntu 6.06. This interface cannot be removed until Ubuntu
- 6.06 is end-of-lifed.
- """
-
- def register_branch(branch_url, branch_name, branch_title,
- branch_description, author_email, product_name,
- owner_name=''):
- """Register a new branch in Launchpad."""
-
- def link_branch_to_bug(branch_url, bug_id):
- """Link the branch to the bug."""
-
-
-@implementer(IBranchSetAPI)
-class BranchSetAPI(LaunchpadXMLRPCView):
-
- def register_branch(self, branch_url, branch_name, branch_title,
- branch_description, author_email, product_name,
- owner_name=''):
- """See IBranchSetAPI."""
- registrant = getUtility(ILaunchBag).user
- assert registrant is not None, (
- "register_branch shouldn't be accessible to unauthenicated"
- " requests.")
-
- person_set = getUtility(IPersonSet)
- if owner_name:
- owner = person_set.getByName(owner_name)
- if owner is None:
- return faults.NoSuchPersonWithName(owner_name)
- if not registrant.inTeam(owner):
- return faults.NotInTeam(registrant.name, owner_name)
- else:
- owner = registrant
-
- if product_name:
- product = getUtility(IProductSet).getByName(product_name)
- if product is None:
- return faults.NoSuchProduct(product_name)
- else:
- product = None
-
- # Branch URLs in Launchpad do not end in a slash, so strip any
- # slashes from the end of the URL.
- branch_url = branch_url.rstrip('/')
-
- branch_lookup = getUtility(IBranchLookup)
- existing_branch = branch_lookup.getByUrl(branch_url)
- if existing_branch is not None:
- return faults.BranchAlreadyRegistered(branch_url)
-
- try:
- unicode_branch_url = branch_url.decode('utf-8')
- IBranch['url'].validate(unicode_branch_url)
- except LaunchpadValidationError as exc:
- return faults.InvalidBranchUrl(branch_url, exc)
-
- # We want it to be None in the database, not ''.
- if not branch_description:
- branch_description = None
- if not branch_title:
- branch_title = None
-
- if not branch_name:
- branch_name = unicode_branch_url.split('/')[-1]
-
- try:
- if branch_url:
- branch_type = BranchType.MIRRORED
- else:
- branch_type = BranchType.HOSTED
- namespace = get_branch_namespace(owner, product)
- branch = namespace.createBranch(
- branch_type=branch_type,
- name=branch_name, registrant=registrant,
- url=branch_url, title=branch_title,
- summary=branch_description)
- if branch_type == BranchType.MIRRORED:
- branch.requestMirror()
- except BranchCreationForbidden:
- return faults.BranchCreationForbidden(product.displayname)
- except BranchCreationException as err:
- return faults.BranchNameInUse(err)
- except LaunchpadValidationError as err:
- return faults.InvalidBranchName(err)
-
- return canonical_url(branch)
-
- def link_branch_to_bug(self, branch_url, bug_id):
- """See IBranchSetAPI."""
- branch = getUtility(IBranchLookup).getByUrl(url=branch_url)
- if branch is None:
- return faults.NoSuchBranch(branch_url)
- try:
- bug = getUtility(IBugSet).get(bug_id)
- except NotFoundError:
- return faults.NoSuchBug(bug_id)
- # Since this API is controlled using launchpad.AnyPerson there must be
- # an authenticated person, so use this person as the registrant.
- registrant = getUtility(ILaunchBag).user
- bug.linkBranch(branch, registrant=registrant)
- return canonical_url(bug)
-
-
class IPublicCodehostingAPI(Interface):
"""The public codehosting API."""
Follow ups
