launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/webhook-git-push into lp:launchpad with lp:~wgrant/launchpad/webhook-trigger as a prerequisite.

Commit message:
Trigger git:push:0.1 webhooks as part of GitRefScanJobs.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/webhook-git-push/+merge/267510

Trigger git:push:0.1 webhooks as part of GitRefScanJobs.

The payload is currently quite limited: the git repository's shortened path, and a dict containing old and new SHA-1s for each changed ref. Details of commits aren't readily accessible without some refactoring, and we'll see what people actually want.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/webhook-git-push into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2015-08-07 11:25:04 +0000
+++ database/schema/security.cfg	2015-08-10 12:52:50 +0000
@@ -712,6 +712,8 @@
 public.translationtemplatesbuild          = SELECT, INSERT
 public.validpersoncache                   = SELECT
 public.validpersonorteamcache             = SELECT
+public.webhook                            = SELECT
+public.webhookjob                         = SELECT, INSERT
 type=user
 
 [branch-distro]

=== modified file 'lib/lp/code/model/gitjob.py'
--- lib/lp/code/model/gitjob.py	2015-07-09 20:06:17 +0000
+++ lib/lp/code/model/gitjob.py	2015-08-10 12:52:50 +0000
@@ -51,6 +51,7 @@
     try_advisory_lock,
     )
 from lp.services.database.stormbase import StormBase
+from lp.services.features import getFeatureFlag
 from lp.services.job.model.job import (
     EnumeratedSubclass,
     Job,
@@ -58,6 +59,7 @@
 from lp.services.job.runner import BaseRunnableJob
 from lp.services.mail.sendmail import format_address_for_person
 from lp.services.scripts import log
+from lp.services.webhooks.interfaces import IWebhookSet
 
 
 class GitJobType(DBEnumeratedType):
@@ -196,6 +198,26 @@
         job.celeryRunOnCommit()
         return job
 
+    @staticmethod
+    def composeWebhookPayload(repository, refs_to_upsert, refs_to_remove):
+        old_refs = {ref.path: ref for ref in repository.refs}
+        ref_changes = {}
+        for ref in refs_to_upsert.keys() + list(refs_to_remove):
+            old = (
+                {"commit_sha1": old_refs[ref].commit_sha1}
+                if ref in old_refs else None)
+            new = (
+                {"commit_sha1": refs_to_upsert[ref]['sha1']}
+                if ref in refs_to_upsert else None)
+            # planRefChanges can return an unchanged ref if the cached
+            # commit details differ.
+            if old != new:
+                ref_changes[ref] = {"old": old, "new": new}
+        return {
+            "git_repository_path": repository.shortened_path,
+            "ref_changes": ref_changes,
+            }
+
     def run(self):
         """See `IGitRefScanJob`."""
         try:
@@ -207,6 +229,13 @@
                     self.repository.planRefChanges(hosting_path, logger=log))
                 self.repository.fetchRefCommits(
                     hosting_path, refs_to_upsert, logger=log)
+                # The webhook delivery includes old ref information, so
+                # prepare it before we actually execute the changes.
+                if getFeatureFlag('code.git.webhooks.enabled'):
+                    payload = self.composeWebhookPayload(
+                        self.repository, refs_to_upsert, refs_to_remove)
+                    getUtility(IWebhookSet).trigger(
+                        self.repository, 'git:push:0.1', payload)
                 self.repository.synchroniseRefs(
                     refs_to_upsert, refs_to_remove, logger=log)
                 props = getUtility(IGitHostingClient).getProperties(

=== modified file 'lib/lp/code/model/tests/test_gitjob.py'
--- lib/lp/code/model/tests/test_gitjob.py	2015-07-08 16:05:11 +0000
+++ lib/lp/code/model/tests/test_gitjob.py	2015-08-10 12:52:50 +0000
@@ -13,6 +13,8 @@
 
 import pytz
 from testtools.matchers import (
+    Equals,
+    MatchesDict,
     MatchesSetwise,
     MatchesStructure,
     )
@@ -34,6 +36,7 @@
     ReclaimGitRepositorySpaceJob,
     )
 from lp.services.database.constants import UTC_NOW
+from lp.services.features.testing import FeatureFixture
 from lp.services.job.runner import JobRunner
 from lp.testing import (
     TestCaseWithFactory,
@@ -178,6 +181,68 @@
                 JobRunner([job]).runAll()
         self.assertEqual([], list(repository.refs))
 
+    def test_triggers_webhooks(self):
+        # Jobs trigger any relevant webhooks when they're enabled.
+        self.useFixture(FeatureFixture({'code.git.webhooks.enabled': 'on'}))
+        repository = self.factory.makeGitRepository()
+        self.factory.makeGitRefs(
+            repository, paths=[u'refs/heads/master', u'refs/tags/1.0'])
+        hook = self.factory.makeWebhook(
+            target=repository, event_types=['git:push:0.1'])
+        job = GitRefScanJob.create(repository)
+        paths = (u'refs/heads/master', u'refs/tags/2.0')
+        hosting_client = FakeGitHostingClient(self.makeFakeRefs(paths), [])
+        self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+        with dbuser('branchscanner'):
+            JobRunner([job]).runAll()
+        delivery = hook.deliveries.one()
+        sha1 = lambda s: hashlib.sha1(s).hexdigest()
+        self.assertThat(
+            delivery,
+            MatchesStructure(
+                event_type=Equals('git:push:0.1'),
+                payload=MatchesDict({
+                    'git_repository_path': Equals(repository.unique_name),
+                    'ref_changes': Equals({
+                        'refs/tags/1.0': {
+                            'old': {'commit_sha1': sha1('refs/tags/1.0')},
+                            'new': None},
+                        'refs/tags/2.0': {
+                            'old': None,
+                            'new': {'commit_sha1': sha1('refs/tags/2.0')}},
+                    })})))
+
+    def test_composeWebhookPayload(self):
+        repository = self.factory.makeGitRepository()
+        self.factory.makeGitRefs(
+            repository, paths=[u'refs/heads/master', u'refs/tags/1.0'])
+
+        sha1 = lambda s: hashlib.sha1(s).hexdigest()
+        new_refs = {
+            'refs/heads/master': {
+                'sha1': sha1('master-ng'),
+                'type': 'commit'},
+            'refs/tags/2.0': {
+                'sha1': sha1('2.0'),
+                'type': 'commit'},
+            }
+        removed_refs = ['refs/tags/1.0']
+        payload = GitRefScanJob.composeWebhookPayload(
+            repository, new_refs, removed_refs)
+        self.assertEqual(
+            {'git_repository_path': repository.unique_name,
+             'ref_changes': {
+                'refs/heads/master': {
+                    'old': {'commit_sha1': sha1('refs/heads/master')},
+                    'new': {'commit_sha1': sha1('master-ng')}},
+                'refs/tags/1.0': {
+                    'old': {'commit_sha1': sha1('refs/tags/1.0')},
+                    'new': None},
+                'refs/tags/2.0': {
+                    'old': None,
+                    'new': {'commit_sha1': sha1('2.0')}}}},
+            payload)
+
 
 class TestReclaimGitRepositorySpaceJob(TestCaseWithFactory):
     """Tests for `ReclaimGitRepositorySpaceJob`."""