Merge lp:~cjwatson/launchpad/export-git-repositories-new into lp:launchpad

[Colin Watson]

Export IGitRepositorySet.new on the webservice.

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.

[Colin Watson (cjwatson)]

Superseded by https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/373763.

Unmerged revisions

19055. By Colin Watson on 2019-09-26

Fix GitRepositorySet.new, and export it on the webservice.

This was previously untested, and had been broken since r18222.

Now that namespace.createRepository can create the repository on the hosting
service, we can safely export this to create a bare repository via the API.
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.

19054. By Colin Watson on 2019-09-26

Push Git hosting creation down from XML-RPC endpoint to model.

Having the creation of the actual repository on disk be done by the XML-RPC
endpoint makes it difficult to expose other methods of creating
repositories.

I rearranged how target and owner defaults are set. We have to create the
repository on the hosting service as the last step to avoid problems with
rolling back transactions, so pushing this part down to the model also
requires pushing down target/owner default handling, and the callback
mechanism previously in place wasn't very suited to that.

I had to adjust the permission check in
GitRepositorySet.setDefaultRepository slightly, because pushing down
target/owner default handling is easier if the Unauthorized exception
message is more accurate. setDefaultRepositoryForOwner already raised
exceptions with accurate messages.

=== 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(