Merge lp:~cjwatson/launchpad/snap-webhook-store-status into lp:launchpad

[Colin Watson]

Extend snap webhooks to include store_upload_status.

This is needed for build.snapcraft.io, so that it can increment metrics when snaps are released to edge; but in general it seems reasonable to include store upload status as well as build status in the webhook payload and trigger deliveries when it changes.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2016-11-23 03:42:51 +0000
+++ database/schema/security.cfg	2017-03-20 00:43:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 #
 # Possible permissions: SELECT, INSERT, UPDATE, EXECUTE
@@ -2565,7 +2565,7 @@
 public.distroarchseries                 = SELECT
 public.distroseries                     = SELECT
 public.emailaddress                     = SELECT
-public.job                              = SELECT, UPDATE
+public.job                              = SELECT, INSERT, UPDATE
 public.libraryfilealias                 = SELECT
 public.libraryfilecontent               = SELECT
 public.person                           = SELECT
@@ -2576,3 +2576,5 @@
 public.snapfile                         = SELECT
 public.snappyseries                     = SELECT
 public.teammembership                   = SELECT
+public.webhook                          = SELECT
+public.webhookjob                       = SELECT, INSERT

=== modified file 'lib/lp/snappy/configure.zcml'
--- lib/lp/snappy/configure.zcml	2016-06-20 21:17:58 +0000
+++ lib/lp/snappy/configure.zcml	2017-03-20 00:43:16 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -139,6 +139,10 @@
         <allow interface="lp.snappy.interfaces.snapbuildjob.ISnapBuildJob" />
         <allow interface="lp.snappy.interfaces.snapbuildjob.ISnapStoreUploadJob" />
     </class>
+    <subscriber
+        for="lp.snappy.interfaces.snapbuild.ISnapBuild
+             lp.snappy.interfaces.snapbuildjob.ISnapBuildStoreUploadStatusChangedEvent"
+        handler="lp.snappy.subscribers.snapbuild.snap_build_store_upload_status_changed" />
 
     <webservice:register module="lp.snappy.interfaces.webservice" />
 

=== modified file 'lib/lp/snappy/interfaces/snapbuildjob.py'
--- lib/lp/snappy/interfaces/snapbuildjob.py	2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/interfaces/snapbuildjob.py	2017-03-20 00:43:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Snap build job interfaces."""
@@ -8,11 +8,13 @@
 __metaclass__ = type
 __all__ = [
     'ISnapBuildJob',
+    'ISnapBuildStoreUploadStatusChangedEvent',
     'ISnapStoreUploadJob',
     'ISnapStoreUploadJobSource',
     ]
 
 from lazr.restful.fields import Reference
+from zope.component.interfaces import IObjectEvent
 from zope.interface import (
     Attribute,
     Interface,
@@ -42,6 +44,10 @@
     metadata = Attribute(_("A dict of data about the job."))
 
 
+class ISnapBuildStoreUploadStatusChangedEvent(IObjectEvent):
+    """The store upload status of a snap package build changed."""
+
+
 class ISnapStoreUploadJob(IRunnableJob):
     """A Job that uploads a snap build to the store."""
 

=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	2016-10-16 22:04:39 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2017-03-20 00:43:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Snap build jobs."""
@@ -9,6 +9,7 @@
 __all__ = [
     'SnapBuildJob',
     'SnapBuildJobType',
+    'SnapBuildStoreUploadStatusChangedEvent',
     'SnapStoreUploadJob',
     ]
 
@@ -26,6 +27,8 @@
     )
 import transaction
 from zope.component import getUtility
+from zope.component.interfaces import ObjectEvent
+from zope.event import notify
 from zope.interface import (
     implementer,
     provider,
@@ -44,8 +47,10 @@
     Job,
     )
 from lp.services.job.runner import BaseRunnableJob
+from lp.services.propertycache import get_property_cache
 from lp.snappy.interfaces.snapbuildjob import (
     ISnapBuildJob,
+    ISnapBuildStoreUploadStatusChangedEvent,
     ISnapStoreUploadJob,
     ISnapStoreUploadJobSource,
     )
@@ -164,6 +169,11 @@
     pass
 
 
+@implementer(ISnapBuildStoreUploadStatusChangedEvent)
+class SnapBuildStoreUploadStatusChangedEvent(ObjectEvent):
+    """See `ISnapBuildStoreUploadStatusChangedEvent`."""
+
+
 @implementer(ISnapStoreUploadJob)
 @provider(ISnapStoreUploadJobSource)
 class SnapStoreUploadJob(SnapBuildJobDerived):
@@ -191,6 +201,8 @@
         snap_build_job = SnapBuildJob(snapbuild, cls.class_job_type, {})
         job = cls(snap_build_job)
         job.celeryRunOnCommit()
+        del get_property_cache(snapbuild).last_store_upload_job
+        notify(SnapBuildStoreUploadStatusChangedEvent(snapbuild))
         return job
 
     @property
@@ -213,6 +225,33 @@
         """See `ISnapStoreUploadJob`."""
         self.metadata["store_url"] = url
 
+    # Ideally we'd just override Job._set_status or similar, but
+    # lazr.delegates makes that difficult, so we use this to override all
+    # the individual Job lifecycle methods instead.
+    def _do_lifecycle(self, method, *args, **kwargs):
+        old_store_upload_status = self.snapbuild.store_upload_status
+        method(*args, **kwargs)
+        if self.snapbuild.store_upload_status != old_store_upload_status:
+            notify(SnapBuildStoreUploadStatusChangedEvent(self.snapbuild))
+
+    def start(self, *args, **kwargs):
+        self._do_lifecycle(self.job.start, *args, **kwargs)
+
+    def complete(self, *args, **kwargs):
+        self._do_lifecycle(self.job.complete, *args, **kwargs)
+
+    def fail(self, *args, **kwargs):
+        self._do_lifecycle(self.job.fail, *args, **kwargs)
+
+    def queue(self, *args, **kwargs):
+        self._do_lifecycle(self.job.queue, *args, **kwargs)
+
+    def suspend(self, *args, **kwargs):
+        self._do_lifecycle(self.job.suspend, *args, **kwargs)
+
+    def resume(self, *args, **kwargs):
+        self._do_lifecycle(self.job.resume, *args, **kwargs)
+
     def run(self):
         """See `IRunnableJob`."""
         client = getUtility(ISnapStoreClient)

=== modified file 'lib/lp/snappy/subscribers/snapbuild.py'
--- lib/lp/snappy/subscribers/snapbuild.py	2016-07-19 16:32:46 +0000
+++ lib/lp/snappy/subscribers/snapbuild.py	2017-03-20 00:43:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Event subscribers for snap builds."""
@@ -19,18 +19,27 @@
 from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
 
 
-def snap_build_status_changed(snapbuild, event):
-    """Trigger events when snap package build statuses change."""
+def _trigger_snap_build_webhook(snapbuild):
     if getFeatureFlag(SNAP_WEBHOOKS_FEATURE_FLAG):
         payload = {
             "snap_build": canonical_url(snapbuild, force_local_path=True),
             "action": "status-changed",
             }
         payload.update(compose_webhook_payload(
-            ISnapBuild, snapbuild, ["snap", "status"]))
+            ISnapBuild, snapbuild, ["snap", "status", "store_upload_status"]))
         getUtility(IWebhookSet).trigger(
             snapbuild.snap, "snap:build:0.1", payload)
 
+
+def snap_build_status_changed(snapbuild, event):
+    """Trigger events when snap package build statuses change."""
+    _trigger_snap_build_webhook(snapbuild)
+
     if (snapbuild.snap.can_upload_to_store and snapbuild.snap.store_upload and
             snapbuild.status == BuildStatus.FULLYBUILT):
         getUtility(ISnapStoreUploadJobSource).create(snapbuild)
+
+
+def snap_build_store_upload_status_changed(snapbuild, event):
+    """Trigger events when snap package build store upload statuses change."""
+    _trigger_snap_build_webhook(snapbuild)

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2017-02-27 18:46:38 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2017-03-20 00:43:16 +0000
@@ -231,6 +231,7 @@
             "snap": Equals(
                 canonical_url(self.build.snap, force_local_path=True)),
             "status": Equals("Successfully built"),
+            "store_upload_status": Equals("Unscheduled"),
             }
         delivery = hook.deliveries.one()
         self.assertThat(
@@ -471,6 +472,37 @@
         self.assertEqual(
             [], list(getUtility(ISnapStoreUploadJobSource).iterReady()))
 
+    def test_scheduleStoreUpload_triggers_webhooks(self):
+        # Scheduling a store upload triggers webhooks on the corresponding
+        # snap.
+        self.setUpStoreUpload()
+        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        self.factory.makeSnapFile(
+            snapbuild=self.build,
+            libraryfile=self.factory.makeLibraryFileAlias(db_only=True))
+        hook = self.factory.makeWebhook(
+            target=self.build.snap, event_types=["snap:build:0.1"])
+        self.build.scheduleStoreUpload()
+        expected_payload = {
+            "snap_build": Equals(
+                canonical_url(self.build, force_local_path=True)),
+            "action": Equals("status-changed"),
+            "snap": Equals(
+                canonical_url(self.build.snap, force_local_path=True)),
+            "status": Equals("Successfully built"),
+            "store_upload_status": Equals("Pending"),
+            }
+        delivery = hook.deliveries.one()
+        self.assertThat(
+            delivery, MatchesStructure(
+                event_type=Equals("snap:build:0.1"),
+                payload=MatchesDict(expected_payload)))
+        with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+            self.assertEqual(
+                "<WebhookDeliveryJob for webhook %d on %r>" % (
+                    hook.id, hook.target),
+                repr(delivery))
+
 
 class TestSnapBuildSet(TestCaseWithFactory):
 

=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	2017-01-04 20:52:12 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2017-03-20 00:43:16 +0000
@@ -8,12 +8,20 @@
 __metaclass__ = type
 
 from fixtures import FakeLogger
+from testtools.matchers import (
+    Equals,
+    MatchesDict,
+    MatchesListwise,
+    MatchesStructure,
+    )
 from zope.interface import implementer
 
+from lp.buildmaster.enums import BuildStatus
 from lp.services.config import config
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.runner import JobRunner
+from lp.services.webapp.publisher import canonical_url
 from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
 from lp.snappy.interfaces.snapbuildjob import (
     ISnapBuildJob,
@@ -92,10 +100,45 @@
         self.assertEqual(
             "<SnapStoreUploadJob for %s>" % snapbuild.title, repr(job))
 
+    def makeSnapBuild(self, **kwargs):
+        # Make a build with a builder and a webhook.
+        snapbuild = self.factory.makeSnapBuild(
+            builder=self.factory.makeBuilder(), **kwargs)
+        snapbuild.updateStatus(BuildStatus.FULLYBUILT)
+        self.factory.makeWebhook(
+            target=snapbuild.snap, event_types=["snap:build:0.1"])
+        return snapbuild
+
+    def assertWebhookDeliveries(self, snapbuild,
+                                expected_store_upload_statuses):
+        hook = snapbuild.snap.webhooks.one()
+        deliveries = list(hook.deliveries)
+        deliveries.reverse()
+        expected_payloads = [{
+            "snap_build": Equals(
+                canonical_url(snapbuild, force_local_path=True)),
+            "action": Equals("status-changed"),
+            "snap": Equals(
+                canonical_url(snapbuild.snap, force_local_path=True)),
+            "status": Equals("Successfully built"),
+            "store_upload_status": Equals(expected),
+            } for expected in expected_store_upload_statuses]
+        matchers = [
+            MatchesStructure(
+                event_type=Equals("snap:build:0.1"),
+                payload=MatchesDict(expected_payload))
+            for expected_payload in expected_payloads]
+        self.assertThat(deliveries, MatchesListwise(matchers))
+        with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+            for delivery in deliveries:
+                self.assertEqual(
+                    "<WebhookDeliveryJob for webhook %d on %r>" % (
+                        hook.id, hook.target),
+                    repr(delivery))
+
     def test_run(self):
         # The job uploads the build to the store and records the store URL.
-        snapbuild = self.factory.makeSnapBuild(
-            builder=self.factory.makeBuilder())
+        snapbuild = self.makeSnapBuild()
         self.assertContentEqual([], snapbuild.store_upload_jobs)
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
@@ -111,11 +154,11 @@
         self.assertEqual(self.store_url, job.store_url)
         self.assertIsNone(job.error_message)
         self.assertEqual([], pop_notifications())
+        self.assertWebhookDeliveries(snapbuild, ["Pending", "Uploaded"])
 
     def test_run_failed(self):
         # A failed run sets the store upload status to FAILED.
-        snapbuild = self.factory.makeSnapBuild(
-            builder=self.factory.makeBuilder())
+        snapbuild = self.makeSnapBuild()
         self.assertContentEqual([], snapbuild.store_upload_jobs)
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
@@ -130,15 +173,16 @@
         self.assertIsNone(job.store_url)
         self.assertEqual("An upload failure", job.error_message)
         self.assertEqual([], pop_notifications())
+        self.assertWebhookDeliveries(
+            snapbuild, ["Pending", "Failed to upload"])
 
     def test_run_unauthorized_notifies(self):
         # A run that gets 401 from the store sends mail.
         requester = self.factory.makePerson(name="requester")
         requester_team = self.factory.makeTeam(
             owner=requester, name="requester-team", members=[requester])
-        snapbuild = self.factory.makeSnapBuild(
-            requester=requester_team, name="test-snap", owner=requester_team,
-            builder=self.factory.makeBuilder())
+        snapbuild = self.makeSnapBuild(
+            requester=requester_team, name="test-snap", owner=requester_team)
         self.assertContentEqual([], snapbuild.store_upload_jobs)
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
@@ -177,6 +221,8 @@
             "http://launchpad.dev/~requester-team/+snap/test-snap/+build/%d\n"
             "Your team Requester Team is the requester of the build.\n" %
             snapbuild.id, footer)
+        self.assertWebhookDeliveries(
+            snapbuild, ["Pending", "Failed to upload"])
 
     def test_run_upload_failure_notifies(self):
         # A run that gets some other upload failure from the store sends
@@ -184,9 +230,8 @@
         requester = self.factory.makePerson(name="requester")
         requester_team = self.factory.makeTeam(
             owner=requester, name="requester-team", members=[requester])
-        snapbuild = self.factory.makeSnapBuild(
-            requester=requester_team, name="test-snap", owner=requester_team,
-            builder=self.factory.makeBuilder())
+        snapbuild = self.makeSnapBuild(
+            requester=requester_team, name="test-snap", owner=requester_team)
         self.assertContentEqual([], snapbuild.store_upload_jobs)
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
@@ -225,13 +270,14 @@
         self.assertEqual(
             "%s\nYour team Requester Team is the requester of the build.\n" %
             build_url, footer)
+        self.assertWebhookDeliveries(
+            snapbuild, ["Pending", "Failed to upload"])
 
     def test_run_scan_pending_retries(self):
         # A run that finds that the store has not yet finished scanning the
         # package schedules itself to be retried.
         self.useFixture(FakeLogger())
-        snapbuild = self.factory.makeSnapBuild(
-            builder=self.factory.makeBuilder())
+        snapbuild = self.makeSnapBuild()
         self.assertContentEqual([], snapbuild.store_upload_jobs)
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
@@ -248,6 +294,7 @@
         self.assertIsNone(job.error_message)
         self.assertEqual([], pop_notifications())
         self.assertEqual(JobStatus.WAITING, job.job.status)
+        self.assertWebhookDeliveries(snapbuild, ["Pending"])
         # Try again.  The upload part of the job is not retried, and this
         # time the scan completes.
         job.lease_expires = None
@@ -266,15 +313,15 @@
         self.assertIsNone(job.error_message)
         self.assertEqual([], pop_notifications())
         self.assertEqual(JobStatus.COMPLETED, job.job.status)
+        self.assertWebhookDeliveries(snapbuild, ["Pending", "Uploaded"])
 
     def test_run_scan_failure_notifies(self):
         # A run that gets a scan failure from the store sends mail.
         requester = self.factory.makePerson(name="requester")
         requester_team = self.factory.makeTeam(
             owner=requester, name="requester-team", members=[requester])
-        snapbuild = self.factory.makeSnapBuild(
-            requester=requester_team, name="test-snap", owner=requester_team,
-            builder=self.factory.makeBuilder())
+        snapbuild = self.makeSnapBuild(
+            requester=requester_team, name="test-snap", owner=requester_team)
         self.assertContentEqual([], snapbuild.store_upload_jobs)
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
@@ -311,12 +358,13 @@
             "http://launchpad.dev/~requester-team/+snap/test-snap/+build/%d\n"
             "Your team Requester Team is the requester of the build.\n" %
             snapbuild.id, footer)
+        self.assertWebhookDeliveries(
+            snapbuild, ["Pending", "Failed to upload"])
 
     def test_run_release(self):
         # A run configured to automatically release the package to certain
         # channels does so.
-        snapbuild = self.factory.makeSnapBuild(
-            store_channels=["stable", "edge"])
+        snapbuild = self.makeSnapBuild(store_channels=["stable", "edge"])
         self.assertContentEqual([], snapbuild.store_upload_jobs)
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
@@ -332,6 +380,7 @@
         self.assertEqual(self.store_url, job.store_url)
         self.assertIsNone(job.error_message)
         self.assertEqual([], pop_notifications())
+        self.assertWebhookDeliveries(snapbuild, ["Pending", "Uploaded"])
 
     def test_run_release_manual_review_notifies(self):
         # A run configured to automatically release the package to certain
@@ -340,7 +389,7 @@
         requester = self.factory.makePerson(name="requester")
         requester_team = self.factory.makeTeam(
             owner=requester, name="requester-team", members=[requester])
-        snapbuild = self.factory.makeSnapBuild(
+        snapbuild = self.makeSnapBuild(
             requester=requester_team, name="test-snap", owner=requester_team,
             store_channels=["stable", "edge"])
         self.assertContentEqual([], snapbuild.store_upload_jobs)
@@ -382,6 +431,8 @@
             "http://launchpad.dev/~requester-team/+snap/test-snap/+build/%d\n"
             "Your team Requester Team is the requester of the build.\n" %
             snapbuild.id, footer)
+        self.assertWebhookDeliveries(
+            snapbuild, ["Pending", "Failed to release to channels"])
 
     def test_run_release_failure_notifies(self):
         # A run configured to automatically release the package to certain
@@ -389,7 +440,7 @@
         requester = self.factory.makePerson(name="requester")
         requester_team = self.factory.makeTeam(
             owner=requester, name="requester-team", members=[requester])
-        snapbuild = self.factory.makeSnapBuild(
+        snapbuild = self.makeSnapBuild(
             requester=requester_team, name="test-snap", owner=requester_team,
             store_channels=["stable", "edge"])
         self.assertContentEqual([], snapbuild.store_upload_jobs)
@@ -430,3 +481,5 @@
             "http://launchpad.dev/~requester-team/+snap/test-snap/+build/%d\n"
             "Your team Requester Team is the requester of the build.\n" %
             snapbuild.id, footer)
+        self.assertWebhookDeliveries(
+            snapbuild, ["Pending", "Failed to release to channels"])