launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30368
[Merge] ~pelpsi/launchpad:differentiate-git-push-message-if-mp-exists into launchpad:master
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
