launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:xmlrpc-git-confirmRepoCreation into launchpad:master with ~pappacena/launchpad:gitrepo-status as a prerequisite.

Commit message:
New XML-RPC to allow code hosting service to confirm a git repository creation on code hosting side.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/385301
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:xmlrpc-git-confirmRepoCreation into launchpad:master.

diff --git a/lib/lp/code/interfaces/gitapi.py b/lib/lp/code/interfaces/gitapi.py
index c289dee..f90de58 100644
--- a/lib/lp/code/interfaces/gitapi.py
+++ b/lib/lp/code/interfaces/gitapi.py
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces for internal Git APIs."""
@@ -92,3 +92,15 @@ class IGitAPI(Interface):
             if no repository can be found for 'translated_path',
             or an `Unauthorized` fault for unauthorized push attempts.
         """
+
+    def confirmRepoCreation(translated_path):
+        """Confirm that repository creation.
+
+        When code hosting finishes creating the repository locally,
+        it should call back this method to confirm that the repository was
+        created, and Launchpad should make the repository available for end
+        users.
+
+        :param repository_id: The database ID of the repository, provided by
+                    translatePath call when repo creation is necessary.
+        """
diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py
index b34307a..a012695 100644
--- a/lib/lp/code/xmlrpc/git.py
+++ b/lib/lp/code/xmlrpc/git.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Implementations of the XML-RPC APIs for Git."""
@@ -31,6 +31,7 @@ from lp.app.validators import LaunchpadValidationError
 from lp.code.enums import (
     GitGranteeType,
     GitPermissionType,
+    GitRepositoryStatus,
     GitRepositoryType,
     )
 from lp.code.errors import (
@@ -69,6 +70,7 @@ from lp.registry.interfaces.product import (
     NoSuchProduct,
     )
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
+from lp.services.database.interfaces import IStore
 from lp.services.macaroons.interfaces import (
     IMacaroonIssuer,
     NO_USER,
@@ -584,3 +586,45 @@ class GitAPI(LaunchpadXMLRPCView):
                 [(ref_path.data, permissions)
                  for ref_path, permissions in result])
         return result
+
+    def _confirmRepoCreation(self, requester, repository_id, auth_params):
+        if requester == LAUNCHPAD_ANONYMOUS:
+            requester = None
+        # We remove the security proxy here because we are in a controlled,
+        # internal environment, and "status" is a read-only attribute for
+        # users.
+        naked_repo = removeSecurityProxy(
+            getUtility(IGitLookup).get(repository_id))
+        if naked_repo is None:
+            raise faults.GitRepositoryNotFound(str(repository_id))
+
+        verified = self._verifyAuthParams(requester, naked_repo, auth_params)
+        if verified is not None and verified.user is NO_USER:
+            if not _can_internal_issuer_write(verified):
+                raise faults.Unauthorized()
+
+        # Only the repository owner can mark it as "available".
+        if requester != naked_repo.owner:
+            raise faults.GitRepositoryNotFound(str(repository_id))
+
+        naked_repo.status = GitRepositoryStatus.AVAILABLE
+        IStore(naked_repo).flush()
+        transaction.commit()
+
+    def confirmRepoCreation(self, repository_id, auth_params):
+        """See `IGitAPI`."""
+        logger = self._getLogger(auth_params.get("request-id"))
+        requester_id = _get_requester_id(auth_params)
+        logger.info(
+            "Request received: confirmRepoCreation('%s')", repository_id)
+        try:
+            result = run_with_login(
+                requester_id, self._confirmRepoCreation,
+                repository_id, auth_params)
+        except Exception as e:
+            result = e
+        if isinstance(result, xmlrpc_client.Fault):
+            logger.error("confirmRepoCreation failed: %r", result)
+        else:
+            logger.info("confirmRepoCreation succeeded: %s" % result)
+        return result
diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
index 6f12f1f..a57a809 100644
--- a/lib/lp/code/xmlrpc/tests/test_git.py
+++ b/lib/lp/code/xmlrpc/tests/test_git.py
@@ -12,6 +12,7 @@ from pymacaroons import Macaroon
 import six
 from six.moves import xmlrpc_client
 from six.moves.urllib.parse import quote
+from storm.store import Store
 from testtools.matchers import (
     Equals,
     IsInstance,
@@ -29,6 +30,7 @@ from lp.app.enums import InformationType
 from lp.buildmaster.enums import BuildStatus
 from lp.code.enums import (
     GitGranteeType,
+    GitRepositoryStatus,
     GitRepositoryType,
     TargetRevisionControlSystems,
     )
@@ -44,6 +46,7 @@ from lp.code.interfaces.gitrepository import (
     )
 from lp.code.tests.helpers import GitHostingFixture
 from lp.registry.enums import TeamMembershipPolicy
+from lp.services.compat import mock
 from lp.services.config import config
 from lp.services.features.testing import FeatureFixture
 from lp.services.macaroons.interfaces import (
@@ -280,6 +283,32 @@ class TestGitAPIMixin:
              "writable": writable, "trailing": trailing, "private": private},
             translation)
 
+    def assertConfirmsRepoCreation(self, requester, git_repository,
+                                   can_authenticate=True):
+        auth_params = _make_auth_params(
+            requester, can_authenticate=can_authenticate)
+        request_id = auth_params["request-id"]
+        result = self.assertDoesNotFault(
+            request_id, "confirmRepoCreation", git_repository.id, auth_params)
+        login(ANONYMOUS)
+        self.assertIsNone(result)
+        Store.of(git_repository).invalidate(git_repository)
+        self.assertEqual(git_repository.status, GitRepositoryStatus.AVAILABLE)
+
+    def assertConfirmRepoCreationFails(
+            self, failure, requester, git_repository, can_authenticate=True):
+        auth_params = _make_auth_params(
+            requester, can_authenticate=can_authenticate)
+        request_id = auth_params["request-id"]
+        original_status = git_repository.status
+        self.assertFault(
+            failure, request_id, "confirmRepoCreation", git_repository.id,
+            auth_params)
+        store = Store.of(git_repository)
+        if store:
+            store.invalidate(git_repository)
+        self.assertEqual(original_status, git_repository.status)
+
     def assertCreates(self, requester, path, can_authenticate=False,
                       private=False):
         auth_params = _make_auth_params(
@@ -660,6 +689,29 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
+    def test_confirm_git_repository_creation(self):
+        owner = self.factory.makePerson()
+        repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner))
+        repo.status = GitRepositoryStatus.CREATING
+        self.assertConfirmsRepoCreation(owner, repo)
+
+    def test_only_owner_can_confirm_git_repository_creation(self):
+        requester = self.factory.makePerson()
+        owner = self.factory.makePerson()
+        repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner))
+        repo.status = GitRepositoryStatus.CREATING
+
+        expected_failure = faults.GitRepositoryNotFound(str(repo.id))
+        self.assertConfirmRepoCreationFails(expected_failure, requester, repo)
+
+    def test_confirm_git_repository_creation_of_non_existing_repository(self):
+        requester = self.factory.makePerson()
+        repo = mock.Mock()
+        repo.id = 99999
+
+        expected_failure = faults.GitRepositoryNotFound('99999')
+        self.assertConfirmRepoCreationFails(expected_failure, requester, repo)
+
     def test_translatePath_cannot_translate(self):
         # Sometimes translatePath will not know how to translate a path.
         # When this happens, it returns a Fault saying so, including the