launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #23985
[Merge] lp:~cjwatson/launchpad/export-git-repositories-new into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/export-git-repositories-new into lp:launchpad.
Commit message:
Export IGitRepositorySet.new on the webservice.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1824399 in Launchpad itself: "Add Git HTTPS push tokens for snapcraft experiment"
https://bugs.launchpad.net/launchpad/+bug/1824399
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/export-git-repositories-new/+merge/373267
For example, this allows snapcraft to create a temporary repository, issue an access token to it, and push code to it, all without needing to configure an SSH key.
In order to make this possible, I had to do some preliminary refactoring to push on-disk repository creation down from the XML-RPC endpoint to the model.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/export-git-repositories-new into lp:launchpad.
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2018-08-17 11:46:36 +0000
+++ lib/lp/code/errors.py 2019-09-26 15:27:30 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Errors used in the lp/code modules."""
@@ -432,6 +432,10 @@
class GitRepositoryCreationFault(Exception):
"""Raised when there is a hosting fault creating a Git repository."""
+ def __init__(self, message, path):
+ super(GitRepositoryCreationFault, self).__init__(message)
+ self.path = path
+
class GitRepositoryScanFault(Exception):
"""Raised when there is a fault scanning a repository."""
=== modified file 'lib/lp/code/interfaces/gitnamespace.py'
--- lib/lp/code/interfaces/gitnamespace.py 2018-07-10 11:26:57 +0000
+++ lib/lp/code/interfaces/gitnamespace.py 2019-09-26 15:27:30 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interface for a Git repository namespace."""
@@ -32,10 +32,14 @@
name = Attribute(
"The name of the namespace. This is prepended to the repository name.")
+ owner = Attribute("The `IPerson` who owns this namespace.")
+
target = Attribute("The `IHasGitRepositories` for this namespace.")
def createRepository(repository_type, registrant, name,
- information_type=None, date_created=None):
+ information_type=None, date_created=None,
+ target_default=False, owner_default=False,
+ with_hosting=False):
"""Create and return an `IGitRepository` in this namespace."""
def isNameUsed(name):
=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py 2019-05-11 11:16:16 +0000
+++ lib/lp/code/interfaces/gitrepository.py 2019-09-26 15:27:30 +0000
@@ -25,6 +25,7 @@
export_as_webservice_collection,
export_as_webservice_entry,
export_destructor_operation,
+ export_factory_operation,
export_operation_as,
export_read_operation,
export_write_operation,
@@ -927,10 +928,21 @@
export_as_webservice_collection(IGitRepository)
- def new(registrant, owner, target, name, information_type=None,
- date_created=None):
+ @call_with(
+ repository_type=GitRepositoryType.HOSTED,
+ registrant=REQUEST_USER,
+ with_hosting=True)
+ @operation_parameters(
+ information_type=copy_field(
+ IGitRepositoryView["information_type"], required=False))
+ @export_factory_operation(IGitRepository, ["owner", "target", "name"])
+ @operation_for_version("devel")
+ def new(repository_type, registrant, owner, target, name,
+ information_type=None, date_created=None, with_hosting=False):
"""Create a Git repository and return it.
+ :param repository_type: The `GitRepositoryType` of the new
+ repository.
:param registrant: The `IPerson` who registered the new repository.
:param owner: The `IPerson` who owns the new repository.
:param target: The `IProduct`, `IDistributionSourcePackage`, or
@@ -939,6 +951,7 @@
:param information_type: Set the repository's information type to
one different from the target's default. The type must conform
to the target's code sharing policy. (optional)
+ :param with_hosting: Create the repository on the hosting service.
"""
# Marker for references to Git URL layouts: ##GITNAMESPACE##
=== modified file 'lib/lp/code/model/githosting.py'
--- lib/lp/code/model/githosting.py 2018-11-22 16:35:28 +0000
+++ lib/lp/code/model/githosting.py 2019-09-26 15:27:30 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Communication with the Git hosting service."""
@@ -98,7 +98,7 @@
self._post("/repo", json=request)
except requests.RequestException as e:
raise GitRepositoryCreationFault(
- "Failed to create Git repository: %s" % unicode(e))
+ "Failed to create Git repository: %s" % unicode(e), path)
def getProperties(self, path):
"""See `IGitHostingClient`."""
=== modified file 'lib/lp/code/model/gitnamespace.py'
--- lib/lp/code/model/gitnamespace.py 2018-07-10 16:36:54 +0000
+++ lib/lp/code/model/gitnamespace.py 2019-09-26 15:27:30 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Implementations of `IGitNamespace`."""
@@ -39,6 +39,7 @@
GitRepositoryCreatorNotMemberOfOwnerTeam,
GitRepositoryCreatorNotOwner,
GitRepositoryExists,
+ GitTargetError,
)
from lp.code.interfaces.gitcollection import IAllGitRepositories
from lp.code.interfaces.gitnamespace import (
@@ -72,8 +73,11 @@
def createRepository(self, repository_type, registrant, name,
reviewer=None, information_type=None,
- date_created=DEFAULT, description=None):
+ date_created=DEFAULT, description=None,
+ target_default=False, owner_default=False,
+ with_hosting=False):
"""See `IGitNamespace`."""
+ repository_set = getUtility(IGitRepositorySet)
self.validateRegistrant(registrant)
self.validateRepositoryName(name)
@@ -102,6 +106,42 @@
notify(ObjectCreatedEvent(repository))
+ if target_default:
+ repository_set.setDefaultRepository(self.target, repository)
+ if owner_default:
+ repository_set.setDefaultRepositoryForOwner(
+ self.owner, self.target, repository, registrant)
+
+ if with_hosting:
+ # Ask the hosting service to create the repository on disk. Do
+ # this as late as possible, since if this succeeds it can't
+ # easily be rolled back with the rest of the transaction.
+
+ # Flush to make sure that repository.id is populated.
+ IStore(repository).flush()
+ assert repository.id is not None
+
+ # If repository has target_default, clone from default.
+ clone_from_repository = None
+ try:
+ default = repository_set.getDefaultRepository(
+ repository.target)
+ if default is not None and default.visibleByUser(registrant):
+ clone_from_repository = default
+ else:
+ default = repository_set.getDefaultRepositoryForOwner(
+ repository.owner, repository.target)
+ if (default is not None and
+ default.visibleByUser(registrant)):
+ clone_from_repository = default
+ except GitTargetError:
+ pass # Ignore Personal repositories.
+ if clone_from_repository == repository:
+ clone_from_repository = None
+
+ repository._createOnHostingService(
+ clone_from_repository=clone_from_repository)
+
return repository
def isNameUsed(self, repository_name):
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2019-09-16 09:05:01 +0000
+++ lib/lp/code/model/gitrepository.py 2019-09-26 15:27:30 +0000
@@ -197,7 +197,7 @@
cachedproperty,
get_property_cache,
)
-from lp.services.webapp.authorization import available_with_permission
+from lp.services.webapp.authorization import check_permission
from lp.services.webapp.interfaces import ILaunchBag
from lp.services.webhooks.interfaces import IWebhookSet
from lp.services.webhooks.model import WebhookTargetMixin
@@ -342,6 +342,16 @@
self.owner_default = False
self.target_default = False
+ def _createOnHostingService(self, clone_from_repository=None):
+ """Create this repository on the hosting service."""
+ hosting_path = self.getInternalPath()
+ if clone_from_repository is not None:
+ clone_from_path = clone_from_repository.getInternalPath()
+ else:
+ clone_from_path = None
+ getUtility(IGitHostingClient).create(
+ hosting_path, clone_from=clone_from_path)
+
@property
def valid_webhook_event_types(self):
return ["git:push:0.1", "merge-proposal:0.1"]
@@ -1648,13 +1658,15 @@
class GitRepositorySet:
"""See `IGitRepositorySet`."""
- def new(self, registrant, owner, target, name, information_type=None,
- date_created=DEFAULT, description=None):
+ def new(self, repository_type, registrant, owner, target, name,
+ information_type=None, date_created=DEFAULT, description=None,
+ with_hosting=False):
"""See `IGitRepositorySet`."""
namespace = get_git_namespace(target, owner)
return namespace.createRepository(
- registrant, name, information_type=information_type,
- date_created=date_created, description=description)
+ repository_type, registrant, name,
+ information_type=information_type, date_created=date_created,
+ description=description, with_hosting=with_hosting)
def getByPath(self, user, path):
"""See `IGitRepositorySet`."""
@@ -1724,13 +1736,16 @@
"Personal repositories cannot be defaults for any target.")
return IStore(GitRepository).find(GitRepository, *clauses).one()
- @available_with_permission('launchpad.Edit', 'target')
def setDefaultRepository(self, target, repository):
"""See `IGitRepositorySet`."""
if IPerson.providedBy(target):
raise GitTargetError(
"Cannot set a default Git repository for a person, only "
"for a project or a package.")
+ if not check_permission("launchpad.Edit", target):
+ raise Unauthorized(
+ "You cannot set the default Git repository for %s." %
+ target.display_name)
if repository is not None and repository.target != target:
raise GitTargetError(
"Cannot set default Git repository to one attached to "
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py 2019-09-16 09:05:01 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py 2019-09-26 15:27:30 +0000
@@ -26,6 +26,7 @@
from storm.store import Store
from testtools.matchers import (
AnyMatch,
+ ContainsDict,
EndsWith,
Equals,
Is,
@@ -2990,6 +2991,35 @@
super(TestGitRepositorySet, self).setUp()
self.repository_set = getUtility(IGitRepositorySet)
+ def test_new(self):
+ # By default, GitRepositorySet.new creates a new repository in the
+ # database but not on the hosting service.
+ hosting_fixture = self.useFixture(GitHostingFixture())
+ owner = self.factory.makePerson()
+ target = self.factory.makeProduct()
+ name = self.factory.getUniqueUnicode()
+ repository = self.repository_set.new(
+ GitRepositoryType.HOSTED, owner, owner, target, name)
+ self.assertThat(repository, MatchesStructure.byEquality(
+ registrant=owner, owner=owner, target=target, name=name))
+ self.assertEqual(0, hosting_fixture.create.call_count)
+
+ def test_new_with_hosting(self):
+ # GitRepositorySet.new(with_hosting=True) creates a new repository
+ # in both the database and the hosting service.
+ hosting_fixture = self.useFixture(GitHostingFixture())
+ owner = self.factory.makePerson()
+ target = self.factory.makeProduct()
+ name = self.factory.getUniqueUnicode()
+ repository = self.repository_set.new(
+ GitRepositoryType.HOSTED, owner, owner, target, name,
+ with_hosting=True)
+ self.assertThat(repository, MatchesStructure.byEquality(
+ registrant=owner, owner=owner, target=target, name=name))
+ self.assertEqual(
+ [((repository.getInternalPath(),), {"clone_from": None})],
+ hosting_fixture.create.calls)
+
def test_provides_IGitRepositorySet(self):
# GitRepositorySet instances provide IGitRepositorySet.
verifyObject(IGitRepositorySet, self.repository_set)
@@ -3370,6 +3400,42 @@
"git+ssh://git.launchpad.test/~person/project/+git/repository",
repository["git_ssh_url"])
+ def assertNewWorks(self, target_db):
+ hosting_fixture = self.useFixture(GitHostingFixture())
+ if IPerson.providedBy(target_db):
+ owner_db = target_db
+ else:
+ owner_db = self.factory.makePerson()
+ owner_url = api_url(owner_db)
+ target_url = api_url(target_db)
+ name = "repository"
+ webservice = webservice_for_person(
+ owner_db, permission=OAuthPermission.WRITE_PUBLIC)
+ webservice.default_api_version = "devel"
+ response = webservice.named_post(
+ "/+git", "new", owner=owner_url, target=target_url, name=name)
+ self.assertEqual(201, response.status)
+ repository = webservice.get(response.getHeader("Location")).jsonBody()
+ self.assertThat(repository, ContainsDict({
+ "repository_type": Equals("Hosted"),
+ "registrant_link": EndsWith(owner_url),
+ "owner_link": EndsWith(owner_url),
+ "target_link": EndsWith(target_url),
+ "name": Equals(name),
+ "owner_default": Is(False),
+ "target_default": Is(False),
+ }))
+ self.assertEqual(1, hosting_fixture.create.call_count)
+
+ def test_new_project(self):
+ self.assertNewWorks(self.factory.makeProduct())
+
+ def test_new_package(self):
+ self.assertNewWorks(self.factory.makeDistributionSourcePackage())
+
+ def test_new_person(self):
+ self.assertNewWorks(self.factory.makePerson())
+
def assertGetRepositoriesWorks(self, target_db):
if IPerson.providedBy(target_db):
owner_db = target_db
=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py 2019-09-10 09:58:52 +0000
+++ lib/lp/code/xmlrpc/git.py 2019-09-26 15:27:30 +0000
@@ -13,7 +13,6 @@
from pymacaroons import Macaroon
import six
from six.moves import xmlrpc_client
-from storm.store import Store
import transaction
from zope.component import (
ComponentLookupError,
@@ -36,7 +35,6 @@
GitRepositoryCreationFault,
GitRepositoryCreationForbidden,
GitRepositoryExists,
- GitTargetError,
InvalidNamespace,
)
from lp.code.interfaces.codehosting import (
@@ -44,7 +42,6 @@
LAUNCHPAD_SERVICES,
)
from lp.code.interfaces.gitapi import IGitAPI
-from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitjob import IGitRefScanJobSource
from lp.code.interfaces.gitlookup import (
IGitLookup,
@@ -282,20 +279,15 @@
if repository_name is None and not namespace.has_defaults:
raise InvalidNamespace(path)
if repository_name is None:
- def default_func(new_repository):
- if owner is None:
- self.repository_set.setDefaultRepository(
- target, new_repository)
- if (owner is not None or
- self.repository_set.getDefaultRepositoryForOwner(
- repository_owner, target) is None):
- self.repository_set.setDefaultRepositoryForOwner(
- repository_owner, target, new_repository, requester)
-
repository_name = namespace.findUnusedName(target.name)
- return namespace, repository_name, default_func
+ target_default = owner is None
+ owner_default = (
+ owner is None or
+ self.repository_set.getDefaultRepositoryForOwner(
+ repository_owner, target) is None)
+ return namespace, repository_name, target_default, owner_default
else:
- return namespace, repository_name, None
+ return namespace, repository_name, False, False
def _reportError(self, path, exception, hosting_path=None):
properties = [
@@ -310,7 +302,7 @@
def _createRepository(self, requester, path, clone_from=None):
try:
- namespace, repository_name, default_func = (
+ namespace, repository_name, target_default, owner_default = (
self._getGitNamespaceExtras(path, requester))
except InvalidNamespace:
raise faults.PermissionDenied(
@@ -331,56 +323,27 @@
raise faults.PermissionDenied(unicode(e))
try:
- repository = namespace.createRepository(
- GitRepositoryType.HOSTED, requester, repository_name)
- except LaunchpadValidationError as e:
- # Despite the fault name, this just passes through the exception
- # text so there's no need for a new Git-specific fault.
- raise faults.InvalidBranchName(e)
- except GitRepositoryExists as e:
- # We should never get here, as we just tried to translate the
- # path and found nothing (not even an inaccessible private
- # repository). Log an OOPS for investigation.
- self._reportError(path, e)
- except GitRepositoryCreationException as e:
- raise faults.PermissionDenied(unicode(e))
-
- try:
- if default_func:
- try:
- default_func(repository)
- except Unauthorized:
- raise faults.PermissionDenied(
- "You cannot set the default Git repository for '%s'." %
- path)
-
- # Flush to make sure that repository.id is populated.
- Store.of(repository).flush()
- assert repository.id is not None
-
- # If repository has target_default, clone from default.
- target_path = None
- try:
- default = self.repository_set.getDefaultRepository(
- repository.target)
- if default is not None and default.visibleByUser(requester):
- target_path = default.getInternalPath()
- else:
- default = self.repository_set.getDefaultRepositoryForOwner(
- repository.owner, repository.target)
- if (default is not None and
- default.visibleByUser(requester)):
- target_path = default.getInternalPath()
- except GitTargetError:
- pass # Ignore Personal repositories.
-
- hosting_path = repository.getInternalPath()
- try:
- getUtility(IGitHostingClient).create(
- hosting_path, clone_from=target_path)
+ try:
+ namespace.createRepository(
+ GitRepositoryType.HOSTED, requester, repository_name,
+ target_default=target_default, owner_default=owner_default,
+ with_hosting=True)
+ except LaunchpadValidationError as e:
+ # Despite the fault name, this just passes through the
+ # exception text so there's no need for a new Git-specific
+ # fault.
+ raise faults.InvalidBranchName(e)
+ except GitRepositoryExists as e:
+ # We should never get here, as we just tried to translate
+ # the path and found nothing (not even an inaccessible
+ # private repository). Log an OOPS for investigation.
+ self._reportError(path, e)
+ except (GitRepositoryCreationException, Unauthorized) as e:
+ raise faults.PermissionDenied(unicode(e))
except GitRepositoryCreationFault as e:
# The hosting service failed. Log an OOPS for investigation.
- self._reportError(path, e, hosting_path=hosting_path)
+ self._reportError(path, e, hosting_path=e.path)
+ raise
except Exception:
# We don't want to keep the repository we created.
transaction.abort()
=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
--- lib/lp/code/xmlrpc/tests/test_git.py 2019-09-05 11:29:00 +0000
+++ lib/lp/code/xmlrpc/tests/test_git.py 2019-09-26 15:27:30 +0000
@@ -908,7 +908,7 @@
# If the hosting service is down, trying to create a repository
# fails and doesn't leave junk around in the Launchpad database.
self.hosting_fixture.create.failure = GitRepositoryCreationFault(
- "nothing here")
+ "nothing here", path="123")
requester = self.factory.makePerson()
initial_count = getUtility(IAllGitRepositories).count()
oops_id = self.assertOopsOccurred(
Follow ups
