Merge lp:~cjwatson/launchpad/git-ref-scanner-commits into lp:launchpad

[Colin Watson]

Scan author/committer/commit-message information from the tip commit of Git references.

Scan author/committer/commit-message information from the tip commit of Git references.

I ended up refactoring GitRepository.synchroniseRefs a bit in the process; it now has a separate planning stage that takes a GitHostingClient so that it can go off and fetch commit information. The method split is mainly to make it easier to test.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py	2015-03-12 15:21:27 +0000
+++ lib/lp/code/errors.py	2015-03-20 13:28:37 +0000
@@ -36,7 +36,7 @@
     'GitRepositoryCreatorNotMemberOfOwnerTeam',
     'GitRepositoryCreatorNotOwner',
     'GitRepositoryExists',
-    'GitRepositoryRefScanFault',
+    'GitRepositoryScanFault',
     'GitTargetError',
     'InvalidBranchMergeProposal',
     'InvalidMergeQueueConfig',
@@ -382,8 +382,8 @@
     """Raised when there is a hosting fault creating a Git repository."""
 
 
-class GitRepositoryRefScanFault(Exception):
-    """Raised when there is a fault getting the refs for a repository."""
+class GitRepositoryScanFault(Exception):
+    """Raised when there is a fault scanning a repository."""
 
 
 class GitTargetError(Exception):

=== modified file 'lib/lp/code/githosting.py'
--- lib/lp/code/githosting.py	2015-03-12 15:21:27 +0000
+++ lib/lp/code/githosting.py	2015-03-20 13:28:37 +0000
@@ -15,7 +15,7 @@
 
 from lp.code.errors import (
     GitRepositoryCreationFault,
-    GitRepositoryRefScanFault,
+    GitRepositoryScanFault,
     )
 
 
@@ -54,19 +54,46 @@
             raise GitRepositoryCreationFault(
                 "Failed to create Git repository: %s" % response.text)
 
-    def get_refs(self, path):
+    def getRefs(self, path):
         try:
             response = self._makeSession().get(
                 urlutils.join(self.endpoint, "repo", path, "refs"),
                 timeout=self.timeout)
         except Exception as e:
-            raise GitRepositoryRefScanFault(
+            raise GitRepositoryScanFault(
                 "Failed to get refs from Git repository: %s" % unicode(e))
         if response.status_code != 200:
-            raise GitRepositoryRefScanFault(
+            raise GitRepositoryScanFault(
                 "Failed to get refs from Git repository: %s" % response.text)
         try:
             return response.json()
         except ValueError as e:
-            raise GitRepositoryRefScanFault(
+            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(
+                urlutils.join(self.endpoint, "repo", path, "commits"),
+                headers={"Content-Type": "application/json"},
+                data=json.dumps({"commits": commit_oids}),
+                timeout=self.timeout)
+        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))

=== modified file 'lib/lp/code/interfaces/gitref.py'
--- lib/lp/code/interfaces/gitref.py	2015-03-13 14:15:24 +0000
+++ lib/lp/code/interfaces/gitref.py	2015-03-20 13:28:37 +0000
@@ -15,6 +15,8 @@
     )
 from zope.schema import (
     Choice,
+    Datetime,
+    Text,
     TextLine,
     )
 
@@ -42,6 +44,24 @@
         title=_("Object type"), required=True, readonly=True,
         vocabulary=GitObjectType)
 
+    author = Attribute(
+        "The author of the commit pointed to by this reference.")
+    author_date = Datetime(
+        title=_("The author date of the commit pointed to by this reference."),
+        required=False, readonly=True)
+
+    committer = Attribute(
+        "The committer of the commit pointed to by this reference.")
+    committer_date = Datetime(
+        title=_(
+            "The committer date of the commit pointed to by this reference."),
+        required=False, readonly=True)
+
+    commit_message = Text(
+        title=_(
+            "The commit message of the commit pointed to by this reference."),
+        required=False, readonly=True)
+
     display_name = TextLine(
         title=_("Display name"), required=True, readonly=True,
         description=_("Display name of the reference."))

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2015-03-17 16:05:54 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2015-03-20 13:28:37 +0000
@@ -213,12 +213,36 @@
         :params paths: An iterable of paths.
         """
 
-    def synchroniseRefs(hosting_refs, logger=None):
+    def planRefChanges(hosting_client, 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.
+
+        :return: A dict of refs to create or update as appropriate, mapping
+            ref paths to dictionaries of their fields; and a set of ref
+            paths to remove.
+        """
+
+    def fetchRefCommits(hosting_client, 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
+            commit information that is available.
+        :param logger: An optional logger.
+        """
+
+    def synchroniseRefs(refs_to_upsert, refs_to_remove):
         """Synchronise references with those from the hosting service.
 
-        :param hosting_refs: A dictionary of reference information returned
-            from the hosting service's `/repo/PATH/refs` collection.
-        :param logger: An optional logger.
+        :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.
         """
 
     def setOwnerDefault(value):

=== modified file 'lib/lp/code/interfaces/revision.py'
--- lib/lp/code/interfaces/revision.py	2013-01-07 02:40:55 +0000
+++ lib/lp/code/interfaces/revision.py	2015-03-20 13:28:37 +0000
@@ -81,6 +81,7 @@
 class IRevisionAuthor(Interface):
     """Committer of a Bazaar revision."""
 
+    id = Int(title=_('The database revision author ID'))
     name = TextLine(title=_("Revision Author Name"), required=True)
     name_without_email = Attribute(
         "Revision author name without email address.")

=== modified file 'lib/lp/code/model/gitjob.py'
--- lib/lp/code/model/gitjob.py	2015-03-17 10:42:24 +0000
+++ lib/lp/code/model/gitjob.py	2015-03-20 13:28:37 +0000
@@ -189,8 +189,13 @@
                     LockType.GIT_REF_SCAN, self.repository.id,
                     Store.of(self.repository)):
                 hosting_path = self.repository.getInternalPath()
-                self.repository.synchroniseRefs(
-                    self._hosting_client.get_refs(hosting_path), logger=log)
+                refs_to_upsert, refs_to_remove = (
+                    self.repository.planRefChanges(
+                        self._hosting_client, 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)
         except LostObjectError:
             log.info(
                 "Skipping repository %s because it has been deleted." %

=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py	2015-03-13 14:15:24 +0000
+++ lib/lp/code/model/gitref.py	2015-03-20 13:28:37 +0000
@@ -6,7 +6,9 @@
     'GitRef',
     ]
 
+import pytz
 from storm.locals import (
+    DateTime,
     Int,
     Reference,
     Unicode,
@@ -36,6 +38,18 @@
 
     object_type = EnumCol(enum=GitObjectType, notNull=True)
 
+    author_id = Int(name='author', allow_none=True)
+    author = Reference(author_id, 'RevisionAuthor.id')
+    author_date = DateTime(
+        name='author_date', tzinfo=pytz.UTC, allow_none=True)
+
+    committer_id = Int(name='committer', allow_none=True)
+    committer = Reference(committer_id, 'RevisionAuthor.id')
+    committer_date = DateTime(
+        name='committer_date', tzinfo=pytz.UTC, allow_none=True)
+
+    commit_message = Unicode(name='commit_message', allow_none=True)
+
     @property
     def display_name(self):
         return self.path.split("/", 2)[-1]

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2015-03-17 16:05:54 +0000
+++ lib/lp/code/model/gitrepository.py	2015-03-20 13:28:37 +0000
@@ -8,6 +8,8 @@
     'GitRepositorySet',
     ]
 
+from datetime import datetime
+import email
 from itertools import chain
 
 from bzrlib import urlutils
@@ -68,6 +70,7 @@
     IGitRepositorySet,
     user_has_special_git_repository_access,
     )
+from lp.code.interfaces.revision import IRevisionSet
 from lp.code.model.gitref import GitRef
 from lp.registry.enums import PersonVisibility
 from lp.registry.errors import CannotChangeInformationType
@@ -361,16 +364,28 @@
         store.flush()
 
         # Try a bulk update first.
-        column_names = ["repository_id", "path", "commit_sha1", "object_type"]
+        column_names = [
+            "repository_id", "path", "commit_sha1", "object_type",
+            "author_id", "author_date", "committer_id", "committer_date",
+            "commit_message",
+            ]
         column_types = [
             ("repository", "integer"),
             ("path", "text"),
             ("commit_sha1", "character(40)"),
             ("object_type", "integer"),
+            ("author", "integer"),
+            ("author_date", "timestamp without time zone"),
+            ("committer", "integer"),
+            ("committer_date", "timestamp without time zone"),
+            ("commit_message", "text"),
             ]
         columns = [getattr(GitRef, name) for name in column_names]
         values = [
-            (self.id, path, info["sha1"], info["type"])
+            (self.id, path, info["sha1"], info["type"],
+             info.get("author"), info.get("author_date"),
+             info.get("committer"), info.get("committer_date"),
+             info.get("commit_message"))
             for path, info in refs_info.items()]
         db_values = dbify_values(values)
         new_refs_expr = Values("new_refs", column_types, db_values)
@@ -412,14 +427,16 @@
             GitRef.repository == self, GitRef.path.is_in(paths)).remove()
         del get_property_cache(self).refs
 
-    def synchroniseRefs(self, hosting_refs, logger=None):
+    def planRefChanges(self, hosting_client, hosting_path, logger=None):
         """See `IGitRepository`."""
         new_refs = {}
-        for path, info in hosting_refs.items():
+        for path, info in hosting_client.getRefs(hosting_path).items():
             try:
                 new_refs[path] = self._convertRefInfo(info)
             except ValueError as e:
-                logger.warning("Unconvertible ref %s %s: %s" % (path, info, e))
+                if logger is not None:
+                    logger.warning(
+                        "Unconvertible ref %s %s: %s" % (path, info, e))
         current_refs = {ref.path: ref for ref in self.refs}
         refs_to_upsert = {}
         for path, info in new_refs.items():
@@ -428,7 +445,66 @@
                 info["sha1"] != current_ref.commit_sha1 or
                 info["type"] != current_ref.object_type):
                 refs_to_upsert[path] = info
+            elif (info["type"] == GitObjectType.COMMIT and
+                  (current_ref.author_id is None or
+                   current_ref.author_date is None or
+                   current_ref.committer_id is None or
+                   current_ref.committer_date is None or
+                   current_ref.commit_message is None)):
+                # Only request detailed commit metadata for refs that point
+                # to commits.
+                refs_to_upsert[path] = info
         refs_to_remove = set(current_refs) - set(new_refs)
+        return refs_to_upsert, refs_to_remove
+
+    @staticmethod
+    def fetchRefCommits(hosting_client, 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(
+                hosting_path, oids, logger=logger)}
+        authors_to_acquire = []
+        committers_to_acquire = []
+        for info in refs.values():
+            commit = commits.get(info["sha1"])
+            if commit is None:
+                continue
+            author = commit.get("author")
+            if author is not None:
+                if "time" in author:
+                    info["author_date"] = datetime.fromtimestamp(
+                        author["time"], tz=pytz.UTC)
+                if "name" in author and "email" in author:
+                    author_addr = email.utils.formataddr(
+                        (author["name"], author["email"]))
+                    info["author_addr"] = author_addr
+                    authors_to_acquire.append(author_addr)
+            committer = commit.get("committer")
+            if committer is not None:
+                if "time" in committer:
+                    info["committer_date"] = datetime.fromtimestamp(
+                        committer["time"], tz=pytz.UTC)
+                if "name" in committer and "email" in committer:
+                    committer_addr = email.utils.formataddr(
+                        (committer["name"], committer["email"]))
+                    info["committer_addr"] = committer_addr
+                    committers_to_acquire.append(committer_addr)
+            if "message" in commit:
+                info["commit_message"] = commit["message"]
+        revision_authors = getUtility(IRevisionSet).acquireRevisionAuthors(
+            authors_to_acquire + committers_to_acquire)
+        for info in refs.values():
+            author = revision_authors.get(info.get("author_addr"))
+            if author is not None:
+                info["author"] = author.id
+            committer = revision_authors.get(info.get("committer_addr"))
+            if committer is not None:
+                info["committer"] = committer.id
+
+    def synchroniseRefs(self, refs_to_upsert, refs_to_remove):
+        """See `IGitRepository`."""
         if refs_to_upsert:
             self.createOrUpdateRefs(refs_to_upsert)
         if refs_to_remove:

=== modified file 'lib/lp/code/model/tests/test_gitjob.py'
--- lib/lp/code/model/tests/test_gitjob.py	2015-03-17 10:51:15 +0000
+++ lib/lp/code/model/tests/test_gitjob.py	2015-03-20 13:28:37 +0000
@@ -5,8 +5,13 @@
 
 __metaclass__ = type
 
+from datetime import (
+    datetime,
+    timedelta,
+    )
 import hashlib
 
+import pytz
 from testtools.matchers import (
     MatchesSetwise,
     MatchesStructure,
@@ -25,7 +30,10 @@
     GitRefScanJob,
     )
 from lp.services.features.testing import FeatureFixture
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    TestCaseWithFactory,
+    time_counter,
+    )
 from lp.testing.dbuser import dbuser
 from lp.testing.fakemethod import FakeMethod
 from lp.testing.layers import (
@@ -72,6 +80,27 @@
                 }}
             for path in paths}
 
+    @staticmethod
+    def makeFakeCommits(author, author_date_gen, paths):
+        epoch = datetime.fromtimestamp(0, tz=pytz.UTC)
+        dates = {path: next(author_date_gen) for path in paths}
+        return [{
+            "sha1": hashlib.sha1(path).hexdigest(),
+            "message": "tip of %s" % path,
+            "author": {
+                "name": author.displayname,
+                "email": author.preferredemail.email,
+                "time": int((dates[path] - epoch).total_seconds()),
+                },
+            "committer": {
+                "name": author.displayname,
+                "email": author.preferredemail.email,
+                "time": int((dates[path] - epoch).total_seconds()),
+                },
+            "parents": [],
+            "tree": hashlib.sha1("").hexdigest(),
+            } for path in paths]
+
     def assertRefsMatch(self, refs, repository, paths):
         matchers = [
             MatchesStructure.byEquality(
@@ -102,8 +131,13 @@
         repository = self.factory.makeGitRepository()
         job = GitRefScanJob.create(repository)
         paths = (u"refs/heads/master", u"refs/tags/1.0")
-        job._hosting_client.get_refs = FakeMethod(
+        job._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(
+            result=self.makeFakeCommits(author, author_date_gen, paths))
         with dbuser("branchscanner"):
             job.run()
         self.assertRefsMatch(repository.refs, repository, paths)
@@ -111,8 +145,9 @@
     def test_logs_bad_ref_info(self):
         repository = self.factory.makeGitRepository()
         job = GitRefScanJob.create(repository)
-        job._hosting_client.get_refs = FakeMethod(
+        job._hosting_client.getRefs = FakeMethod(
             result={u"refs/heads/master": {}})
+        job._hosting_client.getCommits = FakeMethod(result=[])
         expected_message = (
             'Unconvertible ref refs/heads/master {}: '
             'ref info does not contain "object" key')

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2015-03-17 16:05:54 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2015-03-20 13:28:37 +0000
@@ -43,6 +43,7 @@
     IGitRepository,
     IGitRepositorySet,
     )
+from lp.code.interfaces.revision import IRevisionSet
 from lp.code.model.gitrepository import GitRepository
 from lp.registry.enums import (
     BranchSharingPolicy,
@@ -72,6 +73,7 @@
     TestCaseWithFactory,
     verifyObject,
     )
+from lp.testing.fakemethod import FakeMethod
 from lp.testing.layers import (
     DatabaseFunctionalLayer,
     ZopelessDatabaseLayer,
@@ -535,6 +537,150 @@
                 object_type=GitObjectType.BLOB,
                 ))
 
+    def test_planRefChanges(self):
+        # planRefChanges copes with planning changes to refs in a repository
+        # where some refs have been created, some deleted, and some changed.
+        repository = self.factory.makeGitRepository()
+        paths = (u"refs/heads/master", u"refs/heads/foo", u"refs/heads/bar")
+        self.factory.makeGitRefs(repository=repository, paths=paths)
+        self.assertRefsMatch(repository.refs, repository, paths)
+        master_sha1 = repository.getRefByPath(u"refs/heads/master").commit_sha1
+        foo_sha1 = repository.getRefByPath(u"refs/heads/foo").commit_sha1
+        hosting_client = FakeMethod()
+        hosting_client.getRefs = FakeMethod(result={
+            u"refs/heads/master": {
+                u"object": {
+                    u"sha1": u"1111111111111111111111111111111111111111",
+                    u"type": u"commit",
+                    },
+                },
+            u"refs/heads/foo": {
+                u"object": {
+                    u"sha1": foo_sha1,
+                    u"type": u"commit",
+                    },
+                },
+            u"refs/tags/1.0": {
+                u"object": {
+                    u"sha1": master_sha1,
+                    u"type": u"commit",
+                    },
+                },
+            })
+        refs_to_upsert, refs_to_remove = repository.planRefChanges(
+            hosting_client, "dummy")
+
+        expected_upsert = {
+            u"refs/heads/master": {
+                u"sha1": u"1111111111111111111111111111111111111111",
+                u"type": GitObjectType.COMMIT,
+                },
+            u"refs/heads/foo": {
+                u"sha1": unicode(hashlib.sha1(u"refs/heads/foo").hexdigest()),
+                u"type": GitObjectType.COMMIT,
+                },
+            u"refs/tags/1.0": {
+                u"sha1": unicode(
+                    hashlib.sha1(u"refs/heads/master").hexdigest()),
+                u"type": GitObjectType.COMMIT,
+                },
+            }
+        self.assertEqual(expected_upsert, refs_to_upsert)
+        self.assertEqual(set([u"refs/heads/bar"]), refs_to_remove)
+
+    def test_planRefChanges_skips_non_commits(self):
+        # planRefChanges does not attempt to update refs that point to
+        # non-commits.
+        repository = self.factory.makeGitRepository()
+        blob_sha1 = unicode(hashlib.sha1(u"refs/heads/blob").hexdigest())
+        refs_info = {
+            u"refs/heads/blob": {
+                u"sha1": blob_sha1,
+                u"type": GitObjectType.BLOB,
+                },
+            }
+        repository.createOrUpdateRefs(refs_info)
+        hosting_client = FakeMethod()
+        hosting_client.getRefs = FakeMethod(result={
+            u"refs/heads/blob": {
+                u"object": {
+                    u"sha1": blob_sha1,
+                    u"type": u"blob",
+                    },
+                },
+            })
+        self.assertEqual(
+            ({}, set()), repository.planRefChanges(hosting_client, "dummy"))
+
+    def test_fetchRefCommits(self):
+        # fetchRefCommits fetches detailed tip commit metadata for the
+        # requested refs.
+        master_sha1 = unicode(hashlib.sha1(u"refs/heads/master").hexdigest())
+        foo_sha1 = unicode(hashlib.sha1(u"refs/heads/foo").hexdigest())
+        author = self.factory.makePerson()
+        with person_logged_in(author):
+            author_email = author.preferredemail.email
+        epoch = datetime.fromtimestamp(0, tz=pytz.UTC)
+        author_date = datetime(2015, 1, 1, tzinfo=pytz.UTC)
+        committer_date = datetime(2015, 1, 2, tzinfo=pytz.UTC)
+        hosting_client = FakeMethod()
+        hosting_client.getCommits = FakeMethod(result=[
+            {
+                u"sha1": master_sha1,
+                u"message": u"tip of master",
+                u"author": {
+                    u"name": author.displayname,
+                    u"email": author_email,
+                    u"time": int((author_date - epoch).total_seconds()),
+                    },
+                u"committer": {
+                    u"name": u"New Person",
+                    u"email": u"[e-mail]",
+                    u"time": int((committer_date - epoch).total_seconds()),
+                    },
+                u"parents": [],
+                u"tree": unicode(hashlib.sha1("").hexdigest()),
+                }])
+        refs = {
+            u"refs/heads/master": {
+                u"sha1": master_sha1,
+                u"type": GitObjectType.COMMIT,
+                },
+            u"refs/heads/foo": {
+                u"sha1": foo_sha1,
+                u"type": GitObjectType.COMMIT,
+                },
+            }
+        GitRepository.fetchRefCommits(hosting_client, "dummy", refs)
+
+        expected_oids = [master_sha1, foo_sha1]
+        [(_, observed_oids)] = hosting_client.getCommits.extract_args()
+        self.assertContentEqual(expected_oids, observed_oids)
+        expected_author_addr = u"%s <%s>" % (author.displayname, author_email)
+        [expected_author] = getUtility(IRevisionSet).acquireRevisionAuthors(
+            [expected_author_addr]).values()
+        expected_committer_addr = u"New Person <[e-mail]>"
+        [expected_committer] = getUtility(IRevisionSet).acquireRevisionAuthors(
+            [expected_committer_addr]).values()
+        expected_refs = {
+            u"refs/heads/master": {
+                u"sha1": master_sha1,
+                u"type": GitObjectType.COMMIT,
+                u"author": expected_author.id,
+                u"author_addr": expected_author_addr,
+                u"author_date": author_date,
+                u"committer": expected_committer.id,
+                u"committer_addr": expected_committer_addr,
+                u"committer_date": committer_date,
+                u"commit_message": u"tip of master",
+                },
+            u"refs/heads/foo": {
+                u"sha1": foo_sha1,
+                u"type": GitObjectType.COMMIT,
+                },
+            }
+        self.assertEqual(expected_refs, refs)
+
     def test_synchroniseRefs(self):
         # synchroniseRefs copes with synchronising a repository where some
         # refs have been created, some deleted, and some changed.
@@ -542,28 +688,24 @@
         paths = (u"refs/heads/master", u"refs/heads/foo", u"refs/heads/bar")
         self.factory.makeGitRefs(repository=repository, paths=paths)
         self.assertRefsMatch(repository.refs, repository, paths)
-        repository.synchroniseRefs({
+        refs_to_upsert = {
             u"refs/heads/master": {
-                u"object": {
-                    u"sha1": u"1111111111111111111111111111111111111111",
-                    u"type": u"commit",
-                    },
+                u"sha1": u"1111111111111111111111111111111111111111",
+                u"type": GitObjectType.COMMIT,
                 },
             u"refs/heads/foo": {
-                u"object": {
-                    u"sha1": repository.getRefByPath(
-                        u"refs/heads/foo").commit_sha1,
-                    u"type": u"commit",
-                    },
+                u"sha1": repository.getRefByPath(
+                    u"refs/heads/foo").commit_sha1,
+                u"type": GitObjectType.COMMIT,
                 },
             u"refs/tags/1.0": {
-                u"object": {
-                    u"sha1": repository.getRefByPath(
-                        u"refs/heads/master").commit_sha1,
-                    u"type": u"commit",
-                    },
+                u"sha1": repository.getRefByPath(
+                    u"refs/heads/master").commit_sha1,
+                u"type": GitObjectType.COMMIT,
                 },
-            })
+            }
+        refs_to_remove = set([u"refs/heads/bar"])
+        repository.synchroniseRefs(refs_to_upsert, refs_to_remove)
         expected_sha1s = [
             (u"refs/heads/master",
              u"1111111111111111111111111111111111111111"),