launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18771
[Merge] lp:~cjwatson/launchpad/git-detect-merges into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-detect-merges into lp:launchpad.
Commit message:
When Git refs are updated, update merge proposal commit SHA1s and detect merges.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-detect-merges/+merge/261875
When Git refs are updated, update merge proposal commit SHA1s and detect merges.
I also did some refactoring of GitHostingClient to make it less repetitive and more readable.
Note that this can't be landed until we deploy the backend on PS4.5 and make sure that it's up to date with the latest code in lp:turnip.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-detect-merges into lp:launchpad.
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2015-06-06 08:49:54 +0000
+++ lib/lp/code/configure.zcml 2015-06-12 17:58:46 +0000
@@ -938,6 +938,13 @@
<adapter factory="lp.code.model.gitlookup.DistributionGitTraversable" />
<adapter factory="lp.code.model.gitlookup.DistributionSourcePackageGitTraversable" />
+ <!-- Git hosting -->
+ <securedutility
+ class="lp.code.model.githosting.GitHostingClient"
+ provides="lp.code.interfaces.githosting.IGitHostingClient">
+ <allow interface="lp.code.interfaces.githosting.IGitHostingClient" />
+ </securedutility>
+
<!-- Git-related jobs -->
<class class="lp.code.model.gitjob.GitJob">
<allow interface="lp.code.interfaces.gitjob.IGitJob" />
=== modified file 'lib/lp/code/event/git.py'
--- lib/lp/code/event/git.py 2015-06-06 08:49:54 +0000
+++ lib/lp/code/event/git.py 2015-06-12 17:58:46 +0000
@@ -19,6 +19,7 @@
implements(IGitRefsUpdatedEvent)
- def __init__(self, repository, paths):
+ def __init__(self, repository, paths, logger):
super(GitRefsUpdatedEvent, self).__init__(repository)
self.paths = paths
+ self.logger = logger
=== added file 'lib/lp/code/interfaces/githosting.py'
--- lib/lp/code/interfaces/githosting.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/interfaces/githosting.py 2015-06-12 17:58:46 +0000
@@ -0,0 +1,63 @@
+# Copyright 2015 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Interface for communication with the Git hosting service."""
+
+__metaclass__ = type
+__all__ = [
+ 'IGitHostingClient',
+ ]
+
+from zope.interface import Interface
+
+
+class IGitHostingClient(Interface):
+ """Interface for the internal API provided by the Git hosting service."""
+
+ def create(path, clone_from=None):
+ """Create a Git repository.
+
+ :param path: Physical path of the new repository on the hosting
+ service.
+ :param clone_from: If not None, clone the new repository from this
+ other physical path.
+ """
+
+ def getRefs(path):
+ """Get all refs in this repository.
+
+ :param path: Physical path of the repository on the hosting service.
+ :return: A dict mapping ref paths to dicts representing the objects
+ they point to.
+ """
+
+ def getCommits(path, commit_oids, logger=None):
+ """Get details of a list of commits.
+
+ :param path: Physical path of the repository on the hosting service.
+ :param commit_oids: A list of commit OIDs.
+ :param logger: An optional logger.
+ :return: A list of dicts each of which represents one of the
+ requested commits. Non-existent commits will be omitted.
+ """
+
+ def getMergeDiff(path, base, head, logger=None):
+ """Get the merge preview diff between two commits.
+
+ :param path: Physical path of the repository on the hosting service.
+ :param base: The OID of the base commit.
+ :param head: The OID of the commit that we want to merge into
+ 'base'.
+ :param logger: An optional logger.
+ :return: A dict mapping 'commits' to a list of commits between
+ 'base' and 'head' (formatted as with `getCommits`), 'patch' to
+ the text of the diff between 'base' and 'head', and 'conflicts'
+ to a list of conflicted paths.
+ """
+
+ def delete(path, logger=None):
+ """Delete a repository.
+
+ :param path: Physical path of the repository on the hosting service.
+ :param logger: An optional logger.
+ """
=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py 2015-06-12 08:20:17 +0000
+++ lib/lp/code/interfaces/gitrepository.py 2015-06-12 17:58:46 +0000
@@ -249,12 +249,13 @@
:return: An `IGitRef`, or None.
"""
- def createOrUpdateRefs(refs_info, get_objects=False):
+ def createOrUpdateRefs(refs_info, get_objects=False, logger=None):
"""Create or update a set of references in this repository.
:param refs_info: A dict mapping ref paths to
{"sha1": sha1, "type": `GitObjectType`}.
:param get_objects: Return the created/updated references.
+ :param logger: An optional logger.
:return: A list of the created/updated references if get_objects,
otherwise None.
@@ -266,10 +267,9 @@
:params paths: An iterable of paths.
"""
- def planRefChanges(hosting_client, hosting_path, logger=None):
+ def planRefChanges(hosting_path, logger=None):
"""Plan ref changes based on information from the hosting service.
- :param hosting_client: A `GitHostingClient`.
:param hosting_path: A path on the hosting service.
:param logger: An optional logger.
@@ -278,10 +278,9 @@
paths to remove.
"""
- def fetchRefCommits(hosting_client, hosting_path, refs, logger=None):
+ def fetchRefCommits(hosting_path, refs, logger=None):
"""Fetch commit information from the hosting service for a set of refs.
- :param hosting_client: A `GitHostingClient`.
:param hosting_path: A path on the hosting service.
:param refs: A dict mapping ref paths to dictionaries of their
fields; the field dictionaries will be updated with any detailed
@@ -289,13 +288,14 @@
:param logger: An optional logger.
"""
- def synchroniseRefs(refs_to_upsert, refs_to_remove):
+ def synchroniseRefs(refs_to_upsert, refs_to_remove, logger=None):
"""Synchronise references with those from the hosting service.
:param refs_to_upsert: A dictionary mapping ref paths to
dictionaries of their fields; these refs will be created or
updated as appropriate.
:param refs_to_remove: A set of ref paths to remove.
+ :param logger: An optional logger.
"""
def setOwnerDefault(value):
@@ -484,6 +484,16 @@
"Whether there are recent changes in this repository that have not "
"yet been scanned.")
+ def updateMergeCommitIDs(paths):
+ """Update commit SHA1s of merge proposals for this repository.
+
+ The *_git_commit_sha1 columns of merge proposals are stored
+ explicitly in order that merge proposals are still meaningful after
+ associated refs have been deleted. However, active merge proposals
+ where the refs in question still exist should have these columns
+ kept up to date.
+ """
+
def scheduleDiffUpdates(paths):
"""Create UpdatePreviewDiffJobs for landing targets.
@@ -492,6 +502,15 @@
diffs updated.
"""
+ def detectMerges(paths, logger=None):
+ """Detect merges of landing candidates.
+
+ :param paths: A list of reference paths. Any merge proposals whose
+ target is this repository and one of these paths will be
+ checked.
+ :param logger: An optional logger.
+ """
+
class IGitRepositoryModerateAttributes(Interface):
"""IGitRepository attributes that can be edited by more than one community.
=== modified file 'lib/lp/code/model/diff.py'
--- lib/lp/code/model/diff.py 2015-04-30 22:06:11 +0000
+++ lib/lp/code/model/diff.py 2015-06-12 17:58:46 +0000
@@ -42,12 +42,12 @@
from zope.interface import implements
from lp.app.errors import NotFoundError
-from lp.code.githosting import GitHostingClient
from lp.code.interfaces.diff import (
IDiff,
IIncrementalDiff,
IPreviewDiff,
)
+from lp.code.interfaces.githosting import IGitHostingClient
from lp.codehosting.bzrutils import read_locked
from lp.services.config import config
from lp.services.database.constants import UTC_NOW
@@ -417,10 +417,6 @@
def has_conflicts(self):
return self.conflicts is not None and self.conflicts != ''
- @staticmethod
- def getGitHostingClient():
- return GitHostingClient(config.codehosting.internal_git_api_endpoint)
-
@classmethod
def fromBranchMergeProposal(cls, bmp):
"""Create a `PreviewDiff` from a `BranchMergeProposal`.
@@ -449,7 +445,6 @@
preview.conflicts = u''.join(
unicode(conflict) + '\n' for conflict in conflicts)
else:
- hosting_client = cls.getGitHostingClient()
source_repository = bmp.source_git_repository
target_repository = bmp.target_git_repository
if source_repository == target_repository:
@@ -460,7 +455,7 @@
source_repository.getInternalPath())
# XXX cjwatson 2015-04-30: Add prerequisite handling once turnip
# supports it.
- response = hosting_client.getMergeDiff(
+ response = getUtility(IGitHostingClient).getMergeDiff(
path, bmp.target_git_ref.commit_sha1,
bmp.source_git_ref.commit_sha1)
source_revision_id = bmp.source_git_ref.commit_sha1
=== renamed file 'lib/lp/code/githosting.py' => 'lib/lp/code/model/githosting.py'
--- lib/lp/code/githosting.py 2015-05-19 11:29:24 +0000
+++ lib/lp/code/model/githosting.py 2015-06-12 17:58:46 +0000
@@ -12,19 +12,24 @@
from urlparse import urljoin
import requests
+from zope.interface import implements
from lp.code.errors import (
GitRepositoryCreationFault,
GitRepositoryDeletionFault,
GitRepositoryScanFault,
)
+from lp.code.interfaces.githosting import IGitHostingClient
+from lp.services.config import config
class GitHostingClient:
"""A client for the internal API provided by the Git hosting system."""
- def __init__(self, endpoint):
- self.endpoint = endpoint
+ implements(IGitHostingClient)
+
+ def __init__(self):
+ self.endpoint = config.codehosting.internal_git_api_endpoint
def _makeSession(self):
session = requests.Session()
@@ -38,70 +43,59 @@
# over, but is there some more robust way to do this?
return 5.0
+ def _request(self, method, path, json_data=None, **kwargs):
+ session = self._makeSession()
+ if json_data is not None:
+ # XXX cjwatson 2015-03-01: Once we're on requests >= 2.4.2, we
+ # should just pass json through directly and drop the explicit
+ # Content-Type header.
+ kwargs.setdefault("headers", {})["Content-Type"] = (
+ "application/json")
+ kwargs["data"] = json.dumps(json_data)
+ response = getattr(session, method)(
+ urljoin(self.endpoint, path), timeout=self.timeout, **kwargs)
+ if response.status_code != 200:
+ raise Exception(response.text)
+ return response.json()
+
+ def _get(self, path, **kwargs):
+ return self._request("get", path, **kwargs)
+
+ def _post(self, path, **kwargs):
+ return self._request("post", path, **kwargs)
+
+ def _delete(self, path, **kwargs):
+ return self._request("delete", path, **kwargs)
+
def create(self, path, clone_from=None):
try:
- # XXX cjwatson 2015-03-01: Once we're on requests >= 2.4.2, we
- # should just use post(json=) and drop the explicit Content-Type
- # header.
if clone_from:
request = {"repo_path": path, "clone_from": clone_from}
else:
request = {"repo_path": path}
- response = self._makeSession().post(
- urljoin(self.endpoint, "/repo"),
- headers={"Content-Type": "application/json"},
- data=json.dumps(request),
- timeout=self.timeout)
+ self._post("/repo", json_data=request)
except Exception as e:
raise GitRepositoryCreationFault(
"Failed to create Git repository: %s" % unicode(e))
- if response.status_code != 200:
- raise GitRepositoryCreationFault(
- "Failed to create Git repository: %s" % response.text)
def getRefs(self, path):
try:
- response = self._makeSession().get(
- urljoin(self.endpoint, "/repo/%s/refs" % path),
- timeout=self.timeout)
+ return self._get("/repo/%s/refs" % path)
except Exception as e:
raise GitRepositoryScanFault(
"Failed to get refs from Git repository: %s" % unicode(e))
- if response.status_code != 200:
- raise GitRepositoryScanFault(
- "Failed to get refs from Git repository: %s" % response.text)
- try:
- return response.json()
- except ValueError as e:
- raise GitRepositoryScanFault(
- "Failed to decode ref-scan response: %s" % unicode(e))
def getCommits(self, path, commit_oids, logger=None):
commit_oids = list(commit_oids)
try:
- # XXX cjwatson 2015-03-01: Once we're on requests >= 2.4.2, we
- # should just use post(json=) and drop the explicit Content-Type
- # header.
if logger is not None:
logger.info("Requesting commit details for %s" % commit_oids)
- response = self._makeSession().post(
- urljoin(self.endpoint, "/repo/%s/commits" % path),
- headers={"Content-Type": "application/json"},
- data=json.dumps({"commits": commit_oids}),
- timeout=self.timeout)
+ return self._post(
+ "/repo/%s/commits" % path, json_data={"commits": commit_oids})
except Exception as e:
raise GitRepositoryScanFault(
"Failed to get commit details from Git repository: %s" %
unicode(e))
- if response.status_code != 200:
- raise GitRepositoryScanFault(
- "Failed to get commit details from Git repository: %s" %
- response.text)
- try:
- return response.json()
- except ValueError as e:
- raise GitRepositoryScanFault(
- "Failed to decode commit-scan response: %s" % unicode(e))
def getMergeDiff(self, path, base, head, logger=None):
"""Get the merge preview diff between two commits.
@@ -116,35 +110,40 @@
logger.info(
"Requesting merge diff for %s from %s to %s" % (
path, base, head))
- response = self._makeSession().get(
- urljoin(
- self.endpoint,
- "/repo/%s/compare-merge/%s:%s" % (path, base, head)),
- timeout=self.timeout)
+ return self._get(
+ "/repo/%s/compare-merge/%s:%s" % (path, base, head))
except Exception as e:
raise GitRepositoryScanFault(
"Failed to get merge diff from Git repository: %s" %
unicode(e))
- if response.status_code != 200:
- raise GitRepositoryScanFault(
- "Failed to get merge diff from Git repository: %s" %
- response.text)
+
+ def detectMerges(self, path, target, sources, logger=None):
+ """Detect merges of any of 'sources' into 'target'.
+
+ :return: A dict mapping merged commit OIDs from 'sources' to the
+ first commit OID in the left-hand (first parent only) history of
+ 'target' that is a descendant of the corresponding source
+ commit. Unmerged commits are omitted.
+ """
+ sources = list(sources)
try:
- return response.json()
- except ValueError as e:
+ if logger is not None:
+ logger.info(
+ "Detecting merges for %s from %s to %s" % (
+ path, sources, target))
+ return self._post(
+ "/repo/%s/detect-merges/%s" % (path, target),
+ json_data={"sources": sources})
+ except Exception as e:
raise GitRepositoryScanFault(
- "Failed to decode merge-diff response: %s" % unicode(e))
+ "Failed to detect merges in Git repository: %s" % unicode(e))
def delete(self, path, logger=None):
"""Delete a repository."""
try:
if logger is not None:
logger.info("Deleting repository %s" % path)
- response = self._makeSession().delete(
- urljoin(self.endpoint, "/repo/%s" % path))
+ return self._delete("/repo/%s" % path)
except Exception as e:
raise GitRepositoryDeletionFault(
"Failed to delete Git repository: %s" % unicode(e))
- if response.status_code != 200:
- raise GitRepositoryDeletionFault(
- "Failed to delete Git repository: %s" % response.text)
=== modified file 'lib/lp/code/model/gitjob.py'
--- lib/lp/code/model/gitjob.py 2015-05-26 13:55:10 +0000
+++ lib/lp/code/model/gitjob.py 2015-06-12 17:58:46 +0000
@@ -23,13 +23,14 @@
SQL,
Store,
)
+from zope.component import getUtility
from zope.interface import (
classProvides,
implements,
)
from lp.app.errors import NotFoundError
-from lp.code.githosting import GitHostingClient
+from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitjob import (
IGitJob,
IGitRefScanJob,
@@ -198,11 +199,6 @@
job.celeryRunOnCommit()
return job
- def __init__(self, git_job):
- super(GitRefScanJob, self).__init__(git_job)
- self._hosting_client = GitHostingClient(
- config.codehosting.internal_git_api_endpoint)
-
def run(self):
"""See `IGitRefScanJob`."""
try:
@@ -211,12 +207,11 @@
Store.of(self.repository)):
hosting_path = self.repository.getInternalPath()
refs_to_upsert, refs_to_remove = (
- self.repository.planRefChanges(
- self._hosting_client, hosting_path, logger=log))
+ self.repository.planRefChanges(hosting_path, logger=log))
self.repository.fetchRefCommits(
- self._hosting_client, hosting_path, refs_to_upsert,
- logger=log)
- self.repository.synchroniseRefs(refs_to_upsert, refs_to_remove)
+ hosting_path, refs_to_upsert, logger=log)
+ self.repository.synchroniseRefs(
+ refs_to_upsert, refs_to_remove, logger=log)
except LostObjectError:
log.info(
"Skipping repository %s because it has been deleted." %
@@ -250,14 +245,9 @@
job.celeryRunOnCommit()
return job
- def __init__(self, git_job):
- super(ReclaimGitRepositorySpaceJob, self).__init__(git_job)
- self._hosting_client = GitHostingClient(
- config.codehosting.internal_git_api_endpoint)
-
@property
def repository_path(self):
return self.metadata["repository_path"]
def run(self):
- self._hosting_client.delete(self.repository_path, logger=log)
+ getUtility(IGitHostingClient).delete(self.repository_path, logger=log)
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2015-06-12 08:20:17 +0000
+++ lib/lp/code/model/gitrepository.py 2015-06-12 17:58:46 +0000
@@ -10,7 +10,12 @@
from datetime import datetime
import email
-from itertools import chain
+from functools import partial
+from itertools import (
+ chain,
+ groupby,
+ )
+from operator import attrgetter
from bzrlib import urlutils
import pytz
@@ -71,11 +76,13 @@
from lp.code.event.git import GitRefsUpdatedEvent
from lp.code.interfaces.branchmergeproposal import (
BRANCH_MERGE_PROPOSAL_FINAL_STATES,
+ notify_modified,
)
from lp.code.interfaces.gitcollection import (
IAllGitRepositories,
IGitCollection,
)
+from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitlookup import IGitLookup
from lp.code.interfaces.gitnamespace import (
get_git_namespace,
@@ -439,7 +446,7 @@
sha1 = sha1.decode("US-ASCII")
return {"sha1": sha1, "type": object_type_map[obj["type"]]}
- def createOrUpdateRefs(self, refs_info, get_objects=False):
+ def createOrUpdateRefs(self, refs_info, get_objects=False, logger=None):
"""See `IGitRepository`."""
def dbify_values(values):
return [
@@ -508,7 +515,8 @@
self.date_last_modified = UTC_NOW
if updated:
- notify(GitRefsUpdatedEvent(self, [value[1] for value in updated]))
+ notify(GitRefsUpdatedEvent(
+ self, [value[1] for value in updated], logger))
if get_objects:
return bulk.load(GitRef, updated + created)
@@ -519,8 +527,9 @@
GitRef.repository == self, GitRef.path.is_in(paths)).remove()
self.date_last_modified = UTC_NOW
- def planRefChanges(self, hosting_client, hosting_path, logger=None):
+ def planRefChanges(self, hosting_path, logger=None):
"""See `IGitRepository`."""
+ hosting_client = getUtility(IGitHostingClient)
new_refs = {}
for path, info in hosting_client.getRefs(hosting_path).items():
try:
@@ -550,12 +559,12 @@
return refs_to_upsert, refs_to_remove
@staticmethod
- def fetchRefCommits(hosting_client, hosting_path, refs, logger=None):
+ def fetchRefCommits(hosting_path, refs, logger=None):
"""See `IGitRepository`."""
oids = sorted(set(info["sha1"] for info in refs.values()))
commits = {
commit.get("sha1"): commit
- for commit in hosting_client.getCommits(
+ for commit in getUtility(IGitHostingClient).getCommits(
hosting_path, oids, logger=logger)}
authors_to_acquire = []
committers_to_acquire = []
@@ -595,10 +604,10 @@
if committer is not None:
info["committer"] = committer.id
- def synchroniseRefs(self, refs_to_upsert, refs_to_remove):
+ def synchroniseRefs(self, refs_to_upsert, refs_to_remove, logger=None):
"""See `IGitRepository`."""
if refs_to_upsert:
- self.createOrUpdateRefs(refs_to_upsert)
+ self.createOrUpdateRefs(refs_to_upsert, logger=logger)
if refs_to_remove:
self.removeRefs(refs_to_remove)
@@ -810,6 +819,15 @@
Not(BranchMergeProposal.queue_status.is_in(
BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
+ def getActiveLandingCandidates(self, paths):
+ """Merge proposals not in final states where these refs are target."""
+ return Store.of(self).find(
+ BranchMergeProposal,
+ BranchMergeProposal.target_git_repository == self,
+ BranchMergeProposal.target_git_path.is_in(paths),
+ Not(BranchMergeProposal.queue_status.is_in(
+ BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
+
@property
def dependent_landings(self):
"""See `IGitRepository`."""
@@ -842,6 +860,52 @@
Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]))
return not jobs.is_empty()
+ def updateMergeCommitIDs(self, paths):
+ """See `IGitRepository`."""
+ store = Store.of(self)
+ refs = {
+ path: commit_sha1 for path, commit_sha1 in store.find(
+ (GitRef.path, GitRef.commit_sha1),
+ GitRef.repository_id == self.id,
+ GitRef.path.is_in(paths))}
+ updated = set()
+ for kind in ("source", "target", "prerequisite"):
+ repository_name = "%s_git_repositoryID" % kind
+ path_name = "%s_git_path" % kind
+ commit_sha1_name = "%s_git_commit_sha1" % kind
+ old_column = partial(getattr, BranchMergeProposal)
+ db_kind = "dependent" if kind == "prerequisite" else kind
+ column_types = [
+ ("%s_git_path" % db_kind, "text"),
+ ("%s_git_commit_sha1" % db_kind, "character(40)"),
+ ]
+ db_values = [(
+ bulk.dbify_value(old_column(path_name), path),
+ bulk.dbify_value(old_column(commit_sha1_name), commit_sha1)
+ ) for path, commit_sha1 in refs.items()]
+ new_proposals_expr = Values(
+ "new_proposals", column_types, db_values)
+ new_proposals = ClassAlias(BranchMergeProposal, "new_proposals")
+ new_column = partial(getattr, new_proposals)
+ updated_columns = {
+ old_column(commit_sha1_name): new_column(commit_sha1_name)}
+ update_filter = And(
+ old_column(repository_name) == self.id,
+ old_column(path_name) == new_column(path_name),
+ Not(BranchMergeProposal.queue_status.is_in(
+ BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
+ result = store.execute(Returning(BulkUpdate(
+ updated_columns, table=BranchMergeProposal,
+ values=new_proposals_expr, where=update_filter,
+ primary_columns=BranchMergeProposal.id)))
+ updated.update(item[0] for item in result)
+ if updated:
+ # Some existing BranchMergeProposal objects may no longer be
+ # valid. Without knowing which ones we already have, it's
+ # safest to just invalidate everything.
+ store.invalidate()
+ return updated
+
def scheduleDiffUpdates(self, paths):
"""See `IGitRepository`."""
from lp.code.model.branchmergeproposaljob import UpdatePreviewDiffJob
@@ -850,6 +914,33 @@
jobs.append(UpdatePreviewDiffJob.create(merge_proposal))
return jobs
+ def _markProposalMerged(self, proposal, merged_revision_id, logger=None):
+ if logger is not None:
+ logger.info(
+ "Merge detected: %s => %s",
+ proposal.source_git_ref.identity,
+ proposal.target_git_ref.identity)
+ notify_modified(
+ proposal, proposal.markAsMerged,
+ merged_revision_id=merged_revision_id)
+
+ def detectMerges(self, paths, logger=None):
+ """See `IGitRepository`."""
+ hosting_client = getUtility(IGitHostingClient)
+ all_proposals = self.getActiveLandingCandidates(paths).order_by(
+ BranchMergeProposal.target_git_path)
+ for _, group in groupby(all_proposals, attrgetter("target_git_path")):
+ proposals = list(group)
+ merges = hosting_client.detectMerges(
+ self.getInternalPath(), proposals[0].target_git_commit_sha1,
+ set(proposal.source_git_commit_sha1 for proposal in proposals))
+ for proposal in proposals:
+ merged_revision_id = merges.get(
+ proposal.source_git_commit_sha1)
+ if merged_revision_id is not None:
+ self._markProposalMerged(
+ proposal, merged_revision_id, logger=logger)
+
def canBeDeleted(self):
"""See `IGitRepository`."""
# Can't delete if the repository is associated with anything.
=== modified file 'lib/lp/code/model/tests/test_diff.py'
--- lib/lp/code/model/tests/test_diff.py 2015-05-14 13:57:51 +0000
+++ lib/lp/code/model/tests/test_diff.py 2015-06-12 17:58:46 +0000
@@ -16,8 +16,8 @@
parse_patches,
RemoveLine,
)
-from fixtures import MonkeyPatch
import transaction
+from zope.interface import implements
from zope.security.proxy import removeSecurityProxy
from lp.app.errors import NotFoundError
@@ -26,6 +26,7 @@
IIncrementalDiff,
IPreviewDiff,
)
+from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.model.diff import (
Diff,
PreviewDiff,
@@ -43,6 +44,7 @@
verifyObject,
)
from lp.testing.fakemethod import FakeMethod
+from lp.testing.fixture import ZopeUtilityFixture
from lp.testing.layers import (
LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
@@ -90,7 +92,8 @@
class FakeGitHostingClient:
- pass
+
+ implements(IGitHostingClient)
class DiffTestCase(TestCaseWithFactory):
@@ -139,9 +142,7 @@
hosting_client = FakeGitHostingClient()
hosting_client.getMergeDiff = FakeMethod(
result=result, failure=failure)
- self.useFixture(MonkeyPatch(
- "lp.code.model.diff.PreviewDiff.getGitHostingClient",
- staticmethod(lambda: hosting_client)))
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
def createExampleGitMerge(self):
"""Create an example Git-based merge scenario.
=== modified file 'lib/lp/code/model/tests/test_gitjob.py'
--- lib/lp/code/model/tests/test_gitjob.py 2015-05-26 13:35:50 +0000
+++ lib/lp/code/model/tests/test_gitjob.py 2015-06-12 17:58:46 +0000
@@ -16,9 +16,11 @@
MatchesSetwise,
MatchesStructure,
)
+from zope.interface import implements
from zope.security.proxy import removeSecurityProxy
from lp.code.enums import GitObjectType
+from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitjob import (
IGitJob,
IGitRefScanJob,
@@ -39,12 +41,18 @@
)
from lp.testing.dbuser import dbuser
from lp.testing.fakemethod import FakeMethod
+from lp.testing.fixture import ZopeUtilityFixture
from lp.testing.layers import (
DatabaseFunctionalLayer,
LaunchpadZopelessLayer,
)
+class FakeGitHostingClient:
+
+ implements(IGitHostingClient)
+
+
class TestGitJob(TestCaseWithFactory):
"""Tests for `GitJob`."""
@@ -129,26 +137,28 @@
def test_run(self):
# Ensure the job scans the repository.
+ hosting_client = FakeGitHostingClient()
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
repository = self.factory.makeGitRepository()
job = GitRefScanJob.create(repository)
paths = (u"refs/heads/master", u"refs/tags/1.0")
- job._hosting_client.getRefs = FakeMethod(
- result=self.makeFakeRefs(paths))
+ hosting_client.getRefs = FakeMethod(result=self.makeFakeRefs(paths))
author = repository.owner
author_date_start = datetime(2015, 01, 01, tzinfo=pytz.UTC)
author_date_gen = time_counter(author_date_start, timedelta(days=1))
- job._hosting_client.getCommits = FakeMethod(
+ hosting_client.getCommits = FakeMethod(
result=self.makeFakeCommits(author, author_date_gen, paths))
with dbuser("branchscanner"):
JobRunner([job]).runAll()
self.assertRefsMatch(repository.refs, repository, paths)
def test_logs_bad_ref_info(self):
+ hosting_client = FakeGitHostingClient()
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
repository = self.factory.makeGitRepository()
job = GitRefScanJob.create(repository)
- job._hosting_client.getRefs = FakeMethod(
- result={u"refs/heads/master": {}})
- job._hosting_client.getCommits = FakeMethod(result=[])
+ hosting_client.getRefs = FakeMethod(result={u"refs/heads/master": {}})
+ hosting_client.getCommits = FakeMethod(result=[])
expected_message = (
'Unconvertible ref refs/heads/master {}: '
'ref info does not contain "object" key')
@@ -206,15 +216,17 @@
def test_run(self):
# Running a job to reclaim space sends a request to the hosting
# service.
+ hosting_client = FakeGitHostingClient()
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
name = "/~owner/+git/gone"
path = "1"
job = ReclaimGitRepositorySpaceJob.create(name, path)
self.makeJobReady(job)
[job] = list(ReclaimGitRepositorySpaceJob.iterReady())
with dbuser("branchscanner"):
- job._hosting_client.delete = FakeMethod()
+ hosting_client.delete = FakeMethod()
JobRunner([job]).runAll()
- self.assertEqual([(path,)], job._hosting_client.delete.extract_args())
+ self.assertEqual([(path,)], hosting_client.delete.extract_args())
# XXX cjwatson 2015-03-12: We should test that the jobs work via Celery too,
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py 2015-06-12 08:20:17 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py 2015-06-12 17:58:46 +0000
@@ -14,15 +14,15 @@
from bzrlib import urlutils
from lazr.lifecycle.event import ObjectModifiedEvent
from lazr.lifecycle.snapshot import Snapshot
+import pytz
from sqlobject import SQLObjectNotFound
from storm.store import Store
-import transaction
-import pytz
from testtools.matchers import (
EndsWith,
MatchesSetwise,
MatchesStructure,
)
+import transaction
from zope.component import getUtility
from zope.event import notify
from zope.interface import providedBy
@@ -37,6 +37,7 @@
)
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.code.enums import (
+ BranchMergeProposalStatus,
BranchSubscriptionDiffSize,
BranchSubscriptionNotificationLevel,
CodeReviewNotificationLevel,
@@ -53,6 +54,7 @@
BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES,
)
from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository
+from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitjob import IGitRefScanJobSource
from lp.code.interfaces.gitlookup import IGitLookup
from lp.code.interfaces.gitnamespace import (
@@ -100,6 +102,7 @@
from lp.registry.tests.test_accesspolicy import get_policies_for_artifact
from lp.services.config import config
from lp.services.database.constants import UTC_NOW
+from lp.services.database.interfaces import IStore
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.model.job import Job
from lp.services.job.runner import JobRunner
@@ -118,11 +121,13 @@
)
from lp.testing.dbuser import dbuser
from lp.testing.fakemethod import FakeMethod
+from lp.testing.fixture import ZopeUtilityFixture
from lp.testing.layers import (
DatabaseFunctionalLayer,
LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
)
+from lp.testing.mail_helpers import pop_notifications
from lp.testing.pages import webservice_for_person
@@ -1014,6 +1019,9 @@
jobs = self._getWaitingUpdatePreviewDiffJobs(repository)
self.assertEqual(
[bmp, bmp], [job.branch_merge_proposal for job in jobs])
+ self.assertEqual(
+ u"0000000000000000000000000000000000000000",
+ bmp.source_git_commit_sha1)
def test_getRefByPath_without_leading_refs_heads(self):
[ref] = self.factory.makeGitRefs(paths=[u"refs/heads/master"])
@@ -1052,8 +1060,8 @@
},
},
})
- refs_to_upsert, refs_to_remove = repository.planRefChanges(
- hosting_client, "dummy")
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+ refs_to_upsert, refs_to_remove = repository.planRefChanges("dummy")
expected_upsert = {
u"refs/heads/master": {
@@ -1094,8 +1102,8 @@
},
},
})
- self.assertEqual(
- ({}, set()), repository.planRefChanges(hosting_client, "dummy"))
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+ self.assertEqual(({}, set()), repository.planRefChanges("dummy"))
def test_fetchRefCommits(self):
# fetchRefCommits fetches detailed tip commit metadata for the
@@ -1126,6 +1134,7 @@
u"parents": [],
u"tree": unicode(hashlib.sha1("").hexdigest()),
}])
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
refs = {
u"refs/heads/master": {
u"sha1": master_sha1,
@@ -1136,7 +1145,7 @@
u"type": GitObjectType.COMMIT,
},
}
- GitRepository.fetchRefCommits(hosting_client, "dummy", refs)
+ GitRepository.fetchRefCommits("dummy", refs)
expected_oids = [master_sha1, foo_sha1]
[(_, observed_oids)] = hosting_client.getCommits.extract_args()
@@ -1615,6 +1624,66 @@
target=commercial_project, user=owner)
+class TestGitRepositoryUpdateMergeCommitIDs(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_updates_proposals(self):
+ # `updateMergeCommitIDs` updates proposals for specified refs.
+ repository = self.factory.makeGitRepository()
+ paths = [u"refs/heads/1", u"refs/heads/2", u"refs/heads/3"]
+ [ref1, ref2, ref3] = self.factory.makeGitRefs(
+ repository=repository, paths=paths)
+ bmp1 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=ref1, source_ref=ref2)
+ bmp2 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=ref1, source_ref=ref3, prerequisite_ref=ref2)
+ removeSecurityProxy(ref1).commit_sha1 = u"0" * 40
+ removeSecurityProxy(ref2).commit_sha1 = u"1" * 40
+ removeSecurityProxy(ref3).commit_sha1 = u"2" * 40
+ self.assertNotEqual(ref1, bmp1.target_git_ref)
+ self.assertNotEqual(ref2, bmp1.source_git_ref)
+ self.assertNotEqual(ref1, bmp2.target_git_ref)
+ self.assertNotEqual(ref3, bmp2.source_git_ref)
+ self.assertNotEqual(ref2, bmp2.prerequisite_git_ref)
+ self.assertContentEqual(
+ [bmp1.id, bmp2.id], repository.updateMergeCommitIDs(paths))
+ self.assertEqual(ref1, bmp1.target_git_ref)
+ self.assertEqual(ref2, bmp1.source_git_ref)
+ self.assertEqual(ref1, bmp2.target_git_ref)
+ self.assertIsNone(bmp1.prerequisite_git_ref)
+ self.assertEqual(ref3, bmp2.source_git_ref)
+ self.assertEqual(ref2, bmp2.prerequisite_git_ref)
+
+ def test_skips_unspecified_refs(self):
+ # `updateMergeCommitIDs` skips unspecified refs.
+ repository1 = self.factory.makeGitRepository()
+ paths = [u"refs/heads/1", u"refs/heads/2"]
+ [ref1_1, ref1_2] = self.factory.makeGitRefs(
+ repository=repository1, paths=paths)
+ bmp1 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=ref1_1, source_ref=ref1_2)
+ repository2 = self.factory.makeGitRepository(target=repository1.target)
+ [ref2_1, ref2_2] = self.factory.makeGitRefs(
+ repository=repository2, paths=paths)
+ bmp2 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=ref2_1, source_ref=ref2_2)
+ removeSecurityProxy(ref1_1).commit_sha1 = u"0" * 40
+ removeSecurityProxy(ref1_2).commit_sha1 = u"1" * 40
+ removeSecurityProxy(ref2_1).commit_sha1 = u"2" * 40
+ removeSecurityProxy(ref2_2).commit_sha1 = u"3" * 40
+ self.assertNotEqual(ref1_1, bmp1.target_git_ref)
+ self.assertNotEqual(ref1_2, bmp1.source_git_ref)
+ self.assertNotEqual(ref2_1, bmp2.target_git_ref)
+ self.assertNotEqual(ref2_2, bmp2.source_git_ref)
+ self.assertContentEqual(
+ [bmp1.id], repository1.updateMergeCommitIDs([paths[0]]))
+ self.assertEqual(ref1_1, bmp1.target_git_ref)
+ self.assertNotEqual(ref1_2, bmp1.source_git_ref)
+ self.assertNotEqual(ref2_1, bmp2.target_git_ref)
+ self.assertNotEqual(ref2_2, bmp2.source_git_ref)
+
+
class TestGitRepositoryScheduleDiffUpdates(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
@@ -1646,6 +1715,85 @@
self.assertEqual(0, len(jobs))
+class TestGitRepositoryDetectMerges(TestCaseWithFactory):
+
+ layer = LaunchpadZopelessLayer
+
+ def test_markProposalMerged(self):
+ # A merge proposal that is merged is marked as such.
+ proposal = self.factory.makeBranchMergeProposalForGit()
+ self.assertNotEqual(
+ BranchMergeProposalStatus.MERGED, proposal.queue_status)
+ proposal.target_git_repository._markProposalMerged(
+ proposal, proposal.target_git_commit_sha1)
+ self.assertEqual(
+ BranchMergeProposalStatus.MERGED, proposal.queue_status)
+ job = IStore(proposal).find(
+ BranchMergeProposalJob,
+ BranchMergeProposalJob.branch_merge_proposal == proposal,
+ BranchMergeProposalJob.job_type ==
+ BranchMergeProposalJobType.MERGE_PROPOSAL_UPDATED).one()
+ derived_job = job.makeDerived()
+ derived_job.run()
+ notifications = pop_notifications()
+ self.assertIn(
+ "Work in progress => Merged",
+ notifications[0].get_payload(decode=True))
+ self.assertEqual(
+ config.canonical.noreply_from_address, notifications[0]["From"])
+ recipients = set(msg["x-envelope-to"] for msg in notifications)
+ expected = set(
+ [proposal.source_git_repository.registrant.preferredemail.email,
+ proposal.target_git_repository.registrant.preferredemail.email])
+ self.assertEqual(expected, recipients)
+
+ def test_update_detects_merges(self):
+ # Pushing changes to a branch causes a check whether any active
+ # merge proposals with that branch as the target have been merged.
+ repository = self.factory.makeGitRepository()
+ [target_1, target_2, source_1, source_2] = self.factory.makeGitRefs(
+ repository, paths=[
+ u"refs/heads/target-1",
+ u"refs/heads/target-2",
+ u"refs/heads/source-1",
+ u"refs/heads/source-2",
+ ])
+ bmp1 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=target_1, source_ref=source_1)
+ bmp2 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=target_1, source_ref=source_2)
+ bmp3 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=target_2, source_ref=source_1)
+ hosting_client = FakeMethod()
+ hosting_client.detectMerges = FakeMethod(
+ result={source_1.commit_sha1: u"0" * 40})
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+ repository.createOrUpdateRefs({
+ u"refs/heads/target-1": {
+ u"sha1": u"0" * 40,
+ u"type": GitObjectType.COMMIT,
+ },
+ u"refs/heads/target-2": {
+ u"sha1": u"1" * 40,
+ u"type": GitObjectType.COMMIT,
+ }
+ })
+ expected_args = [
+ (repository.getInternalPath(), target_1.commit_sha1,
+ set([source_1.commit_sha1, source_2.commit_sha1])),
+ (repository.getInternalPath(), target_2.commit_sha1,
+ set([source_1.commit_sha1])),
+ ]
+ self.assertContentEqual(
+ expected_args, hosting_client.detectMerges.extract_args())
+ self.assertEqual(BranchMergeProposalStatus.MERGED, bmp1.queue_status)
+ self.assertEqual(u"0" * 40, bmp1.merged_revision_id)
+ self.assertEqual(
+ BranchMergeProposalStatus.WORK_IN_PROGRESS, bmp2.queue_status)
+ self.assertEqual(BranchMergeProposalStatus.MERGED, bmp3.queue_status)
+ self.assertEqual(u"0" * 40, bmp3.merged_revision_id)
+
+
class TestGitRepositorySet(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
=== modified file 'lib/lp/code/subscribers/git.py'
--- lib/lp/code/subscribers/git.py 2015-06-06 08:49:54 +0000
+++ lib/lp/code/subscribers/git.py 2015-06-12 17:58:46 +0000
@@ -8,4 +8,6 @@
def refs_updated(repository, event):
"""Some references in a Git repository have been updated."""
+ repository.updateMergeCommitIDs(event.paths)
repository.scheduleDiffUpdates(event.paths)
+ repository.detectMerges(event.paths, logger=event.logger)
=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py 2015-05-26 10:54:35 +0000
+++ lib/lp/code/xmlrpc/git.py 2015-06-12 17:58:46 +0000
@@ -22,15 +22,16 @@
from lp.app.validators import LaunchpadValidationError
from lp.code.errors import (
GitRepositoryCreationException,
+ GitRepositoryCreationFault,
GitRepositoryCreationForbidden,
- GitRepositoryCreationFault,
GitRepositoryExists,
GitTargetError,
InvalidNamespace,
)
-from lp.code.githosting import GitHostingClient
from lp.code.interfaces.codehosting import LAUNCHPAD_ANONYMOUS
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,
IGitTraverser,
@@ -40,7 +41,6 @@
split_git_unique_name,
)
from lp.code.interfaces.gitrepository import IGitRepositorySet
-from lp.code.interfaces.gitjob import IGitRefScanJobSource
from lp.code.xmlrpc.codehosting import run_with_login
from lp.registry.errors import (
InvalidName,
@@ -52,7 +52,6 @@
NoSuchProduct,
)
from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
-from lp.services.config import config
from lp.services.webapp import LaunchpadXMLRPCView
from lp.services.webapp.authorization import check_permission
from lp.services.webapp.errorlog import ScriptRequest
@@ -67,8 +66,6 @@
def __init__(self, *args, **kwargs):
super(GitAPI, self).__init__(*args, **kwargs)
- self.hosting_client = GitHostingClient(
- config.codehosting.internal_git_api_endpoint)
self.repository_set = getUtility(IGitRepositorySet)
def _performLookup(self, path):
@@ -211,15 +208,16 @@
else:
default = self.repository_set.getDefaultRepositoryForOwner(
repository.owner, repository.target)
- if default is not None and default.visibleByUser(requester):
+ if (default is not None and
+ default.visibleByUser(requester)):
target_path = default.getInternalPath()
except GitTargetError:
pass # Ignore Personal repositories.
hosting_path = repository.getInternalPath()
try:
- self.hosting_client.create(hosting_path,
- clone_from=target_path)
+ getUtility(IGitHostingClient).create(
+ hosting_path, clone_from=target_path)
except GitRepositoryCreationFault as e:
# The hosting service failed. Log an OOPS for investigation.
self._reportError(path, e, hosting_path=hosting_path)
=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
--- lib/lp/code/xmlrpc/tests/test_git.py 2015-05-26 10:54:35 +0000
+++ lib/lp/code/xmlrpc/tests/test_git.py 2015-06-12 17:58:46 +0000
@@ -6,6 +6,7 @@
__metaclass__ = type
from zope.component import getUtility
+from zope.interface import implements
from zope.security.proxy import removeSecurityProxy
from lp.app.enums import InformationType
@@ -15,6 +16,7 @@
LAUNCHPAD_SERVICES,
)
from lp.code.interfaces.gitcollection import IAllGitRepositories
+from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitjob import IGitRefScanJobSource
from lp.code.interfaces.gitrepository import (
GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE,
@@ -30,6 +32,7 @@
person_logged_in,
TestCaseWithFactory,
)
+from lp.testing.fixture import ZopeUtilityFixture
from lp.testing.layers import (
AppServerLayer,
LaunchpadFunctionalLayer,
@@ -40,6 +43,8 @@
class FakeGitHostingClient:
"""A GitHostingClient lookalike that just logs calls."""
+ implements(IGitHostingClient)
+
def __init__(self):
self.calls = []
@@ -50,6 +55,8 @@
class BrokenGitHostingClient:
"""A GitHostingClient lookalike that pretends the remote end is down."""
+ implements(IGitHostingClient)
+
def create(self, path, clone_from=None):
raise GitRepositoryCreationFault("nothing here")
@@ -60,7 +67,9 @@
def setUp(self):
super(TestGitAPIMixin, self).setUp()
self.git_api = GitAPI(None, None)
- self.git_api.hosting_client = FakeGitHostingClient()
+ self.hosting_client = FakeGitHostingClient()
+ self.useFixture(
+ ZopeUtilityFixture(self.hosting_client, IGitHostingClient))
self.repository_set = getUtility(IGitRepositorySet)
def assertPathTranslationError(self, requester, path, permission="read",
@@ -167,15 +176,14 @@
translation)
self.assertEqual(
("create", repository.getInternalPath()),
- self.git_api.hosting_client.calls[0][0:2])
+ self.hosting_client.calls[0][0:2])
return repository
def assertCreatesFromClone(self, requester, path, cloned_from,
can_authenticate=False):
self.assertCreates(requester, path, can_authenticate)
self.assertEqual(
- cloned_from.getInternalPath(),
- self.git_api.hosting_client.calls[0][2])
+ cloned_from.getInternalPath(), self.hosting_client.calls[0][2])
def test_translatePath_private_repository(self):
requester = self.factory.makePerson()
@@ -603,7 +611,8 @@
def test_translatePath_create_broken_hosting_service(self):
# If the hosting service is down, trying to create a repository
# fails and doesn't leave junk around in the Launchpad database.
- self.git_api.hosting_client = BrokenGitHostingClient()
+ hosting_client = BrokenGitHostingClient()
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
requester = self.factory.makePerson()
initial_count = getUtility(IAllGitRepositories).count()
oops_id = self.assertOopsOccurred(
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2015-06-11 08:06:28 +0000
+++ lib/lp/testing/factory.py 2015-06-12 17:58:46 +0000
@@ -1766,8 +1766,11 @@
u"type": GitObjectType.COMMIT,
}
for path in paths}
- return removeSecurityProxy(repository).createOrUpdateRefs(
- refs_info, get_objects=True)
+ refs_by_path = {
+ ref.path: ref
+ for ref in removeSecurityProxy(repository).createOrUpdateRefs(
+ refs_info, get_objects=True)}
+ return [refs_by_path[path] for path in paths]
def makeBug(self, target=None, owner=None, bug_watch_url=None,
information_type=None, date_closed=None, title=None,
=== modified file 'lib/lp/testing/fixture.py'
--- lib/lp/testing/fixture.py 2013-06-20 05:50:00 +0000
+++ lib/lp/testing/fixture.py 2015-06-12 17:58:46 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Launchpad test fixtures that have no better home."""
@@ -228,7 +228,7 @@
class ZopeUtilityFixture(Fixture):
"""A fixture that temporarily registers a different utility."""
- def __init__(self, component, intf, name):
+ def __init__(self, component, intf, name=""):
"""Construct a new fixture.
:param component: An instance of a class that provides this
@@ -244,10 +244,14 @@
def setUp(self):
super(ZopeUtilityFixture, self).setUp()
gsm = getGlobalSiteManager()
+ original = gsm.queryUtility(self.intf, self.name)
gsm.registerUtility(self.component, self.intf, self.name)
self.addCleanup(
gsm.unregisterUtility,
self.component, self.intf, self.name)
+ if original is not None:
+ self.addCleanup(
+ gsm.registerUtility, original, self.intf, self.name)
class Urllib2Fixture(Fixture):
=== modified file 'lib/lp/testing/tests/test_fixture.py'
--- lib/lp/testing/tests/test_fixture.py 2013-06-20 05:50:00 +0000
+++ lib/lp/testing/tests/test_fixture.py 2015-06-12 17:58:46 +0000
@@ -104,16 +104,28 @@
layer = BaseLayer
+ def getMailer(self):
+ return getGlobalSiteManager().getUtility(IMailDelivery, 'Mail')
+
def test_fixture(self):
- def get_mailer():
- return getGlobalSiteManager().getUtility(
- IMailDelivery, 'Mail')
fake = DummyMailer()
# In BaseLayer there should be no mailer by default.
- self.assertRaises(ComponentLookupError, get_mailer)
- with ZopeUtilityFixture(fake, IMailDelivery, 'Mail'):
- self.assertEquals(get_mailer(), fake)
- self.assertRaises(ComponentLookupError, get_mailer)
+ self.assertRaises(ComponentLookupError, self.getMailer)
+ with ZopeUtilityFixture(fake, IMailDelivery, 'Mail'):
+ self.assertEqual(fake, self.getMailer())
+ self.assertRaises(ComponentLookupError, self.getMailer)
+
+ def test_restores_previous_utility(self):
+ # If there was a previous utility, ZopeUtilityFixture restores it on
+ # cleanup.
+ original_fake = DummyMailer()
+ getGlobalSiteManager().registerUtility(
+ original_fake, IMailDelivery, 'Mail')
+ self.assertEqual(original_fake, self.getMailer())
+ fake = DummyMailer()
+ with ZopeUtilityFixture(fake, IMailDelivery, 'Mail'):
+ self.assertEqual(fake, self.getMailer())
+ self.assertEqual(original_fake, self.getMailer())
class TestPGBouncerFixtureWithCA(TestCase):
