Simone Pelosi has proposed merging ~pelpsi/launchpad:differentiate-git-push-message-if-mp-exists into launchpad:master.
Commit message: Differentiate git push message if the MP exists Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/448597 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:differentiate-git-push-message-if-mp-exists into launchpad:master.
diff --git a/lib/lp/code/interfaces/branchmergeproposal.py b/lib/lp/code/interfaces/branchmergeproposal.py index e65eef6..1b3c916 100644 --- a/lib/lp/code/interfaces/branchmergeproposal.py +++ b/lib/lp/code/interfaces/branchmergeproposal.py @@ -971,6 +971,13 @@ class IBranchMergeProposalGetter(Interface): def get(id): """Return the BranchMergeProposal with specified id.""" + def activeProposalsForBranches(source, target=None): + """Return BranchMergeProposals having a given source and target. + + :param source: source branch + :param target: target branch + """ + def getProposalsForContext(context, status=None, visible_by_user=None): """Return BranchMergeProposals associated with the context. diff --git a/lib/lp/code/interfaces/gitapi.py b/lib/lp/code/interfaces/gitapi.py index 5495944..f6bb38b 100644 --- a/lib/lp/code/interfaces/gitapi.py +++ b/lib/lp/code/interfaces/gitapi.py @@ -95,6 +95,22 @@ class IGitAPI(Interface): or an `Unauthorized` fault for unauthorized push attempts. """ + def getMergeProposalInfo(translated_path, branch, auth_params): + """Return the info (string) for a merge proposal. + + When a `branch` that is not the default branch in a repository + is pushed, the URL where the merge proposal for that branch can + be opened will be generated and returned if the merge proposal + doesn't exist, otherwise the link of the existing merge proposal + will be returned. + + :returns: The URL to register a merge proposal or the link of + an existing merge proposal for the branch in the + specified repository. A `GitRepositoryNotFound` fault is returned + if no repository can be found for 'translated_path', + or an `Unauthorized` fault for unauthorized push attempts. + """ + def confirmRepoCreation(repository_id): """Confirm that repository creation. diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py index 7f6053f..4f57972 100644 --- a/lib/lp/code/interfaces/gitrepository.py +++ b/lib/lp/code/interfaces/gitrepository.py @@ -787,6 +787,21 @@ class IGitRepositoryView(IHasRecipes): diffs updated. """ + def makeFrozenRef(path, commit_sha1): + """A frozen Git reference. + + This is like a GitRef, but is frozen at a particular commit, even if + the real reference has moved on or has been deleted. + It isn't necessarily backed by a real database object, + and will retrieve columns from the database when required. + Use this when you have a repository/path/commit_sha1 that you want + to pass around as a single object, + but don't necessarily know that the ref still exists. + + :param path: the repository reference path. + :param commit_sha1: the commit sha1 for that repository reference path. + """ + def markRecipesStale(paths): """Mark recipes associated with this repository as stale. diff --git a/lib/lp/code/model/branchmergeproposal.py b/lib/lp/code/model/branchmergeproposal.py index 924658c..17ecffc 100644 --- a/lib/lp/code/model/branchmergeproposal.py +++ b/lib/lp/code/model/branchmergeproposal.py @@ -1805,7 +1805,7 @@ class BranchMergeProposalGetter: return result @staticmethod - def activeProposalsForBranches(source, target): + def activeProposalsForBranches(source, target=None): clauses = [Not(BranchMergeProposal.queue_status.is_in(FINAL_STATES))] if IGitRef.providedBy(source): clauses.extend( @@ -1813,11 +1813,18 @@ class BranchMergeProposalGetter: BranchMergeProposal.source_git_repository == source.repository, BranchMergeProposal.source_git_path == source.path, - BranchMergeProposal.target_git_repository - == target.repository, - BranchMergeProposal.target_git_path == target.path, ] ) + + if target: + clauses.extend( + [ + BranchMergeProposal.target_git_repository + == target.repository, + BranchMergeProposal.target_git_path == target.path, + ] + ) + else: clauses.extend( [ @@ -1825,4 +1832,12 @@ class BranchMergeProposalGetter: BranchMergeProposal.target_branch == target, ] ) + + if target: + clauses.extend( + [ + BranchMergeProposal.source_branch == source, + BranchMergeProposal.target_branch == target, + ] + ) return IStore(BranchMergeProposal).find(BranchMergeProposal, *clauses) diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py index 87abb5b..a91df2e 100644 --- a/lib/lp/code/model/gitrepository.py +++ b/lib/lp/code/model/gitrepository.py @@ -98,7 +98,7 @@ from lp.code.interfaces.revisionstatus import ( from lp.code.mail.branch import send_git_repository_modified_notifications from lp.code.model.branchmergeproposal import BranchMergeProposal from lp.code.model.gitactivity import GitActivity -from lp.code.model.gitref import GitRef, GitRefDefault +from lp.code.model.gitref import GitRef, GitRefDefault, GitRefFrozen from lp.code.model.gitrule import GitRule, GitRuleGrant from lp.code.model.gitsubscription import GitSubscription from lp.code.model.revisionstatus import RevisionStatusReport @@ -1478,6 +1478,13 @@ class GitRepository( jobs.extend(merge_proposal.scheduleDiffUpdates()) return jobs + def makeFrozenRef(self, path, commit_sha1): + return GitRefFrozen( + self, + path, + commit_sha1, + ) + def _getRecipes(self, paths=None): """Undecorated version of recipes for use by `markRecipesStale`.""" from lp.code.model.sourcepackagerecipedata import ( diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py index 3b97a85..3ecdaee 100644 --- a/lib/lp/code/xmlrpc/git.py +++ b/lib/lp/code/xmlrpc/git.py @@ -38,6 +38,7 @@ from lp.code.errors import ( GitRepositoryExists, InvalidNamespace, ) +from lp.code.interfaces.branchmergeproposal import IBranchMergeProposalGetter from lp.code.interfaces.codehosting import ( LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES, @@ -699,6 +700,73 @@ class GitAPI(LaunchpadXMLRPCView): del result @return_fault + def _getMergeProposalInfo( + self, requester, translated_path, ref, auth_params + ): + if requester == LAUNCHPAD_ANONYMOUS: + requester = None + repository = removeSecurityProxy( + getUtility(IGitLookup).getByHostingPath(translated_path) + ) + if repository is None: + raise faults.GitRepositoryNotFound(translated_path) + + verified = self._verifyAuthParams(requester, repository, auth_params) + if verified is not None and verified.user is NO_USER: + # Showing a merge proposal URL may be useful to ordinary users, + # but it doesn't make sense in the context of an internal service. + return None + + frozen_ref = repository.makeFrozenRef(path=ref, commit_sha1="") + branch = frozen_ref.name + merge_proposals = getUtility( + IBranchMergeProposalGetter + ).activeProposalsForBranches(source=frozen_ref, target=None) + + merge_proposal = merge_proposals.one() + if merge_proposal: + return ( + "Updated existing merge proposal for %s on Launchpad\n" + % quote(branch) + + merge_proposal.address + ) + else: + base_url = canonical_url(repository, rootsite="code") + mp_url = "%s/+ref/%s/+register-merge" % (base_url, quote(branch)) + return ( + "Create a merge proposal for '%s' on Launchpad by" + " visiting:\n" % branch + mp_url + ) + + def getMergeProposalInfo(self, translated_path, ref, auth_params): + """See `IGitAPI`.""" + logger = self._getLogger(auth_params.get("request-id")) + requester_id = _get_requester_id(auth_params) + logger.info( + "Request received: getMergeProposalURL('%s, %s') for %s", + translated_path, + ref, + requester_id, + ) + + result = run_with_login( + requester_id, + self._getMergeProposalInfo, + translated_path, + ref, + auth_params, + ) + try: + if isinstance(result, xmlrpc.client.Fault): + logger.error("getMergeProposalInfo failed: %r", result) + else: + logger.info("getMergeProposalInfo succeeded: %s" % result) + return result + finally: + # Avoid traceback reference cycles. + del result + + @return_fault def _authenticateWithPassword(self, username, password): """Authenticate a user by username and password. diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py index 1f69b8e..80cf50b 100644 --- a/lib/lp/code/xmlrpc/tests/test_git.py +++ b/lib/lp/code/xmlrpc/tests/test_git.py @@ -660,6 +660,34 @@ class TestGitAPIMixin: ) self.assertEqual(expected_mp_url, result) + def assertHasMergeProposalInfo( + self, repository, pushed_branch, auth_params, mp=None + ): + base_url = canonical_url(repository, rootsite="code") + expected_mp_url = ( + "Create a merge proposal for 'branch1' on " + "Launchpad by visiting:\n" + "%s/+ref/%s/+register-merge" + % ( + base_url, + quote(pushed_branch), + ) + ) + + if mp: + expected_mp_url = ( + "Updated existing merge proposal for %s on Launchpad\n" + % quote(pushed_branch) + + mp.address + ) + + result = self.git_api.getMergeProposalInfo( + repository.getInternalPath(), + "refs/heads/%s" % pushed_branch, + auth_params, + ) + self.assertEqual(expected_mp_url, result) + def test_translatePath_private_repository(self): requester = self.factory.makePerson() repository = removeSecurityProxy( @@ -2760,6 +2788,34 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory): access_token_id="string", ) + def test_getMergeProposalInfo(self): + # A merge proposal URL is returned by LP for a non-default branch + # pushed by a user that has their ordinary privileges on the + # corresponding repository. + requester_owner = self.factory.makePerson() + repository = self.factory.makeGitRepository(owner=requester_owner) + [ref] = self.factory.makeGitRefs( + repository=repository, + paths=["refs/heads/master"], + ) + [source_ref] = self.factory.makeGitRefs( + repository=repository, + paths=["refs/heads/branch1"], + ) + removeSecurityProxy(repository).default_branch = "refs/heads/master" + pushed_branch = "branch1" + self.assertHasMergeProposalInfo( + repository, pushed_branch, {"uid": requester_owner.id} + ) + + mp = self.factory.makeBranchMergeProposalForGit( + source_ref=source_ref, target_ref=ref + ) + + self.assertHasMergeProposalInfo( + repository, pushed_branch, {"uid": requester_owner.id}, mp + ) + def test_getMergeProposalURL_user(self): # A merge proposal URL is returned by LP for a non-default branch # pushed by a user that has their ordinary privileges on the
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp