Merge lp:~cjwatson/launchpad/snap-upload-poll-status into lp:launchpad

[Colin Watson]

Make SnapStoreUploadJob check the store for status of scanning uploaded packages.

Make SnapStoreUploadJob check the store for status of scanning uploaded packages. Figuring out what happened to store uploads without this is very difficult even for staff, let alone for ordinary users.

The current need to poll is unfortunate, and we don't want to block Launchpad's job runners on the store indefinitely. For the meantime, lacking much in the way of data, I've gone for a gut estimate of trying once a minute for 20 minutes, and we'll see how that goes.

This handles both the current status URL data format and the new one implemented by https://code.launchpad.net/~facundo/software-center-agent/doc-status-url/+merge/296780. We'll need the new format in order to implement automatic releasing to channels, but either is good enough for just finding out whether a package is scanned successfully.

Future work ought to include adding a store upload retry facility to the UI.

[Celso Providelo (cprov)]

Colin,

Thanks for this new feature, it looks very good. It's unfortunate that we ended up having to supporting old and new status response because SCA hasn't settled yet. I am assuming the cleanup (removing the old format) will be quick.

I don't understand the LP jobs mechanics very well, but the code changes seem to reflect the MP description and I am happy to see it in action.

review: Approve

=== modified file 'lib/lp/snappy/browser/snapbuild.py'
--- lib/lp/snappy/browser/snapbuild.py	2016-05-11 21:40:12 +0000
+++ lib/lp/snappy/browser/snapbuild.py	2016-06-27 13:20:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """SnapBuild views."""
@@ -16,6 +16,7 @@
     action,
     LaunchpadFormView,
     )
+from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.browser import (
     FileNavigationMixin,
     ProxiedLibraryFileAlias,
@@ -28,6 +29,7 @@
     LaunchpadView,
     Link,
     Navigation,
+    structured,
     )
 from lp.snappy.interfaces.snapbuild import ISnapBuild
 from lp.soyuz.interfaces.binarypackagebuild import IBuildRescoreForm
@@ -82,6 +84,20 @@
     def has_files(self):
         return bool(self.files)
 
+    @cachedproperty
+    def store_upload_status(self):
+        job = self.context.store_upload_jobs.first()
+        if job is None:
+            return None
+        elif job.job.status in (JobStatus.WAITING, JobStatus.RUNNING):
+            return "Store upload in progress"
+        elif job.job.status == JobStatus.COMPLETED:
+            return structured(
+                '<a href="%s">Manage this package in the store</a>',
+                job.store_url)
+        else:
+            return structured("Store upload failed: %s", job.error_message)
+
 
 class SnapBuildCancelView(LaunchpadFormView):
     """View for cancelling a snap package build."""

=== modified file 'lib/lp/snappy/browser/tests/test_snapbuild.py'
--- lib/lp/snappy/browser/tests/test_snapbuild.py	2016-05-11 21:40:12 +0000
+++ lib/lp/snappy/browser/tests/test_snapbuild.py	2016-06-27 13:20:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snap package build views."""
@@ -17,8 +17,10 @@
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.enums import BuildStatus
 from lp.services.features.testing import FeatureFixture
+from lp.services.job.interfaces.job import JobStatus
 from lp.services.webapp import canonical_url
 from lp.snappy.interfaces.snap import SNAP_FEATURE_FLAG
+from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
 from lp.testing import (
     admin_logged_in,
     ANONYMOUS,
@@ -81,6 +83,36 @@
         build_view = create_initialized_view(build, "+index")
         self.assertEqual([], build_view.files)
 
+    def test_store_upload_status_in_progress(self):
+        build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+        getUtility(ISnapStoreUploadJobSource).create(build)
+        build_view = create_initialized_view(build, "+index")
+        self.assertEqual(
+            "Store upload in progress", build_view.store_upload_status)
+
+    def test_store_upload_status_completed(self):
+        build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+        job = getUtility(ISnapStoreUploadJobSource).create(build)
+        naked_job = removeSecurityProxy(job)
+        naked_job.job._status = JobStatus.COMPLETED
+        naked_job.store_url = "http://sca.example/dev/click-apps/1/rev/1/"
+        build_view = create_initialized_view(build, "+index")
+        self.assertEqual(
+            '<a href="%s">Manage this package in the store</a>' % (
+                job.store_url),
+            build_view.store_upload_status.escapedtext)
+
+    def test_store_upload_status_failed(self):
+        build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+        job = getUtility(ISnapStoreUploadJobSource).create(build)
+        naked_job = removeSecurityProxy(job)
+        naked_job.job._status = JobStatus.FAILED
+        naked_job.error_message = "Scan failed."
+        build_view = create_initialized_view(build, "+index")
+        self.assertEqual(
+            "Store upload failed: Scan failed.",
+            build_view.store_upload_status.escapedtext)
+
 
 class TestSnapBuildOperations(BrowserTestCase):
 

=== added file 'lib/lp/snappy/emailtemplates/snapbuild-scanfailed.txt'
--- lib/lp/snappy/emailtemplates/snapbuild-scanfailed.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/emailtemplates/snapbuild-scanfailed.txt	2016-06-27 13:20:12 +0000
@@ -0,0 +1,4 @@
+Launchpad uploaded this snap package to the store, but the store failed to
+scan it:
+
+  %(store_error_message)s

=== modified file 'lib/lp/snappy/interfaces/snapbuildjob.py'
--- lib/lp/snappy/interfaces/snapbuildjob.py	2016-05-06 16:34:21 +0000
+++ lib/lp/snappy/interfaces/snapbuildjob.py	2016-06-27 13:20:12 +0000
@@ -48,6 +48,10 @@
     error_message = TextLine(
         title=_("Error message"), required=False, readonly=True)
 
+    store_url = TextLine(
+        title=_("The URL on the store corresponding to this build"),
+        required=False, readonly=True)
+
 
 class ISnapStoreUploadJobSource(IJobSource):
 

=== modified file 'lib/lp/snappy/interfaces/snapstoreclient.py'
--- lib/lp/snappy/interfaces/snapstoreclient.py	2016-06-03 14:49:49 +0000
+++ lib/lp/snappy/interfaces/snapstoreclient.py	2016-06-27 13:20:12 +0000
@@ -9,10 +9,13 @@
 __all__ = [
     'BadRefreshResponse',
     'BadRequestPackageUploadResponse',
+    'BadScanStatusResponse',
     'BadUploadResponse',
     'ISnapStoreClient',
     'NeedsRefreshResponse',
+    'ScanFailedResponse',
     'UnauthorizedUploadResponse',
+    'UploadNotScannedYetResponse',
     ]
 
 from zope.interface import Interface
@@ -38,6 +41,18 @@
     pass
 
 
+class BadScanStatusResponse(Exception):
+    pass
+
+
+class UploadNotScannedYetResponse(Exception):
+    pass
+
+
+class ScanFailedResponse(Exception):
+    pass
+
+
 class ISnapStoreClient(Interface):
     """Interface for the API provided by the snap store."""
 
@@ -59,6 +74,7 @@
         """Upload a snap build to the store.
 
         :param snapbuild: The `ISnapBuild` to upload.
+        :return: A URL to poll for upload processing status.
         """
 
     def refreshDischargeMacaroon(snap):
@@ -66,3 +82,15 @@
 
         :param snap: An `ISnap` whose discharge macaroon needs to be refreshed.
         """
+
+    def checkStatus(status_url):
+        """Poll the store once for upload scan status.
+
+        :param status_url: A URL as returned by `upload`.
+        :raises UploadNotScannedYetResponse: if the store has not yet
+            scanned the upload.
+        :raises BadScanStatusResponse: if the store failed to scan the
+            upload.
+        :return: A URL on the store with further information about this
+            upload.
+        """

=== modified file 'lib/lp/snappy/mail/snapbuild.py'
--- lib/lp/snappy/mail/snapbuild.py	2016-06-03 14:49:49 +0000
+++ lib/lp/snappy/mail/snapbuild.py	2016-06-27 13:20:12 +0000
@@ -46,6 +46,20 @@
             config.canonical.noreply_from_address,
             "snap-build-upload-unauthorized", build)
 
+    @classmethod
+    def forUploadScanFailure(cls, build):
+        """Create a mailer for notifying about store upload scan failures.
+
+        :param build: The relevant build.
+        """
+        requester = build.requester
+        recipients = {requester: RecipientReason.forBuildRequester(requester)}
+        return cls(
+            "Store upload scan failed for %(snap_name)s",
+            "snapbuild-scanfailed.txt", recipients,
+            config.canonical.noreply_from_address,
+            "snap-build-upload-scan-failed", build)
+
     def __init__(self, subject, template_name, recipients, from_address,
                  notification_type, build):
         super(SnapBuildMailer, self).__init__(
@@ -62,6 +76,11 @@
     def _getTemplateParams(self, email, recipient):
         """See `BaseMailer`."""
         build = self.build
+        upload_job = build.store_upload_jobs.first()
+        if upload_job is None or upload_job.error_message is None:
+            error_message = ""
+        else:
+            error_message = upload_job.error_message
         params = super(SnapBuildMailer, self)._getTemplateParams(
             email, recipient)
         params.update({
@@ -80,6 +99,7 @@
             "build_url": canonical_url(build),
             "snap_authorize_url": canonical_url(
                 build.snap, view_name="+authorize"),
+            "store_error_message": error_message,
             })
         if build.duration is not None:
             duration_formatter = DurationFormatterAPI(build.duration)

=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	2016-06-03 14:49:49 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2016-06-27 13:20:12 +0000
@@ -12,6 +12,8 @@
     'SnapStoreUploadJob',
     ]
 
+from datetime import timedelta
+
 from lazr.delegates import delegate_to
 from lazr.enum import (
     DBEnumeratedType,
@@ -48,8 +50,11 @@
     ISnapStoreUploadJobSource,
     )
 from lp.snappy.interfaces.snapstoreclient import (
+    BadScanStatusResponse,
     ISnapStoreClient,
+    ScanFailedResponse,
     UnauthorizedUploadResponse,
+    UploadNotScannedYetResponse,
     )
 from lp.snappy.mail.snapbuild import SnapBuildMailer
 
@@ -159,7 +164,12 @@
 
     class_job_type = SnapBuildJobType.STORE_UPLOAD
 
+    user_error_types = (UnauthorizedUploadResponse, ScanFailedResponse)
+
     # XXX cjwatson 2016-05-04: identify transient upload failures and retry
+    retry_error_types = (UploadNotScannedYetResponse,)
+    retry_delay = timedelta(minutes=1)
+    max_retries = 20
 
     config = config.ISnapStoreUploadJobSource
 
@@ -181,11 +191,26 @@
         """See `ISnapStoreUploadJob`."""
         self.metadata["error_message"] = message
 
+    @property
+    def store_url(self):
+        """See `ISnapStoreUploadJob`."""
+        return self.metadata.get("store_url")
+
+    @store_url.setter
+    def store_url(self, url):
+        """See `ISnapStoreUploadJob`."""
+        self.metadata["store_url"] = url
+
     def run(self):
         """See `IRunnableJob`."""
+        client = getUtility(ISnapStoreClient)
         try:
-            getUtility(ISnapStoreClient).upload(self.snapbuild)
+            if "status_url" not in self.metadata:
+                self.metadata["status_url"] = client.upload(self.snapbuild)
+            self.store_url = client.checkStatus(self.metadata["status_url"])
             self.error_message = None
+        except self.retry_error_types:
+            raise
         except Exception as e:
             # Abort work done so far, but make sure that we commit the error
             # message.
@@ -194,5 +219,8 @@
             if isinstance(e, UnauthorizedUploadResponse):
                 mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)
                 mailer.sendAll()
+            elif isinstance(e, (BadScanStatusResponse, ScanFailedResponse)):
+                mailer = SnapBuildMailer.forUploadScanFailure(self.snapbuild)
+                mailer.sendAll()
             transaction.commit()
             raise

=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py	2016-06-06 13:03:31 +0000
+++ lib/lp/snappy/model/snapstoreclient.py	2016-06-27 13:20:12 +0000
@@ -25,10 +25,13 @@
 from lp.snappy.interfaces.snapstoreclient import (
     BadRefreshResponse,
     BadRequestPackageUploadResponse,
+    BadScanStatusResponse,
     BadUploadResponse,
     ISnapStoreClient,
     NeedsRefreshResponse,
+    ScanFailedResponse,
     UnauthorizedUploadResponse,
+    UploadNotScannedYetResponse,
     )
 
 
@@ -166,11 +169,16 @@
         # that's currently difficult in jobs.
         try:
             assert snap.store_secrets is not None
-            urlfetch(
+            response = urlfetch(
                 upload_url, method="POST", json=data,
                 auth=MacaroonAuth(
                     snap.store_secrets["root"],
                     snap.store_secrets["discharge"]))
+            response_data = response.json()
+            if "status_details_url" in response_data:
+                return response_data["status_details_url"]
+            else:
+                return response_data["status_url"]
         except requests.HTTPError as e:
             if e.response.status_code == 401:
                 if (e.response.headers.get("WWW-Authenticate") ==
@@ -186,12 +194,12 @@
         for _, lfa, lfc in snapbuild.getFiles():
             upload_data = self._uploadFile(lfa, lfc)
             try:
-                self._uploadApp(snapbuild.snap, upload_data)
+                return self._uploadApp(snapbuild.snap, upload_data)
             except NeedsRefreshResponse:
                 # Try to automatically refresh the discharge macaroon and
                 # retry the upload.
                 self.refreshDischargeMacaroon(snapbuild.snap)
-                self._uploadApp(snapbuild.snap, upload_data)
+                return self._uploadApp(snapbuild.snap, upload_data)
 
     @classmethod
     def refreshDischargeMacaroon(cls, snap):
@@ -211,3 +219,29 @@
             snap.store_secrets = new_secrets
         except requests.HTTPError as e:
             raise BadRefreshResponse(e.args[0])
+
+    @classmethod
+    def checkStatus(cls, status_url):
+        try:
+            response = urlfetch(status_url)
+            response_data = response.json()
+            if "completed" in response_data:
+                # Old status format.
+                if not response_data["completed"]:
+                    raise UploadNotScannedYetResponse()
+                elif not response_data["application_url"]:
+                    raise ScanFailedResponse(response_data["message"])
+                else:
+                    return response_data["application_url"]
+            else:
+                # New status format.
+                if not response_data["processed"]:
+                    raise UploadNotScannedYetResponse()
+                elif "errors" in response_data:
+                    error_message = "\n".join(
+                        error["message"] for error in response_data["errors"])
+                    raise ScanFailedResponse(error_message)
+                else:
+                    return response_data["url"]
+        except requests.HTTPError as e:
+            raise BadScanStatusResponse(e.args[0])

=== modified file 'lib/lp/snappy/templates/snapbuild-index.pt'
--- lib/lp/snappy/templates/snapbuild-index.pt	2016-05-11 21:40:12 +0000
+++ lib/lp/snappy/templates/snapbuild-index.pt	2016-06-27 13:20:12 +0000
@@ -159,6 +159,9 @@
            tal:attributes="href context/upload_log_url">uploadlog</a>
         (<span tal:replace="file/content/filesize/fmt:bytes" />)
       </li>
+      <li tal:define="store_upload_status view/store_upload_status"
+          tal:condition="store_upload_status"
+          tal:content="structure store_upload_status" />
     </ul>
 
     <div

=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	2016-06-03 14:49:49 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2016-06-27 13:20:12 +0000
@@ -7,10 +7,12 @@
 
 __metaclass__ = type
 
+from fixtures import FakeLogger
 from zope.interface import implementer
 
 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.snappy.interfaces.snap import SNAP_TESTING_FLAGS
 from lp.snappy.interfaces.snapbuildjob import (
@@ -18,8 +20,10 @@
     ISnapStoreUploadJob,
     )
 from lp.snappy.interfaces.snapstoreclient import (
+    BadScanStatusResponse,
     ISnapStoreClient,
     UnauthorizedUploadResponse,
+    UploadNotScannedYetResponse,
     )
 from lp.snappy.model.snapbuildjob import (
     SnapBuildJob,
@@ -42,6 +46,7 @@
 
     def __init__(self):
         self.upload = FakeMethod()
+        self.checkStatus = FakeMethod()
 
 
 class TestSnapBuildJob(TestCaseWithFactory):
@@ -67,6 +72,8 @@
     def setUp(self):
         super(TestSnapStoreUploadJob, self).setUp()
         self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+        self.status_url = "http://sca.example/dev/api/snaps/1/builds/1/status"
+        self.store_url = "http://sca.example/dev/click-apps/1/rev/1/"
 
     def test_provides_interface(self):
         # `SnapStoreUploadJob` objects provide `ISnapStoreUploadJob`.
@@ -82,16 +89,20 @@
             "<SnapStoreUploadJob for %s>" % snapbuild.title, repr(job))
 
     def test_run(self):
-        # The job uploads the build to the store.
+        # The job uploads the build to the store and records the store URL.
         snapbuild = self.factory.makeSnapBuild()
         self.assertContentEqual([], snapbuild.store_upload_jobs)
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
+        client.upload.result = self.status_url
+        client.checkStatus.result = self.store_url
         self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):
             JobRunner([job]).runAll()
         self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertEqual(self.store_url, job.store_url)
         self.assertIsNone(job.error_message)
         self.assertEqual([], pop_notifications())
 
@@ -106,7 +117,9 @@
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):
             JobRunner([job]).runAll()
         self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertEqual([], client.checkStatus.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertIsNone(job.store_url)
         self.assertEqual("An upload failure", job.error_message)
         self.assertEqual([], pop_notifications())
 
@@ -124,7 +137,9 @@
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):
             JobRunner([job]).runAll()
         self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertEqual([], client.checkStatus.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertIsNone(job.store_url)
         self.assertEqual("Authorization failed.", job.error_message)
         [notification] = pop_notifications()
         self.assertEqual(
@@ -147,3 +162,80 @@
         self.assertEqual(
             "http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n"
             "You are the requester of the build.\n" % snapbuild.id, footer)
+
+    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()
+        self.assertContentEqual([], snapbuild.store_upload_jobs)
+        job = SnapStoreUploadJob.create(snapbuild)
+        client = FakeSnapStoreClient()
+        client.upload.result = self.status_url
+        client.checkStatus.failure = UploadNotScannedYetResponse()
+        self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            JobRunner([job]).runAll()
+        self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertIsNone(job.store_url)
+        self.assertIsNone(job.error_message)
+        self.assertEqual([], pop_notifications())
+        self.assertEqual(JobStatus.WAITING, job.job.status)
+        # Try again.  The upload part of the job is not retried, and this
+        # time the scan completes.
+        job.lease_expires = None
+        job.scheduled_start = None
+        client.upload.calls = []
+        client.checkStatus.calls = []
+        client.checkStatus.failure = None
+        client.checkStatus.result = self.store_url
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            JobRunner([job]).runAll()
+        self.assertEqual([], client.upload.calls)
+        self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertEqual(self.store_url, job.store_url)
+        self.assertIsNone(job.error_message)
+        self.assertEqual([], pop_notifications())
+        self.assertEqual(JobStatus.COMPLETED, job.job.status)
+
+    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")
+        snapbuild = self.factory.makeSnapBuild(
+            requester=requester, name="test-snap", owner=requester)
+        self.assertContentEqual([], snapbuild.store_upload_jobs)
+        job = SnapStoreUploadJob.create(snapbuild)
+        client = FakeSnapStoreClient()
+        client.upload.result = self.status_url
+        client.checkStatus.failure = BadScanStatusResponse("Scan failed.")
+        self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            JobRunner([job]).runAll()
+        self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertIsNone(job.store_url)
+        self.assertEqual("Scan failed.", job.error_message)
+        [notification] = pop_notifications()
+        self.assertEqual(
+            config.canonical.noreply_from_address, notification["From"])
+        self.assertEqual(
+            "Requester <%s>" % requester.preferredemail.email,
+            notification["To"])
+        subject = notification["Subject"].replace("\n ", " ")
+        self.assertEqual("Store upload scan failed for test-snap", subject)
+        self.assertEqual(
+            "Requester", notification["X-Launchpad-Message-Rationale"])
+        self.assertEqual(
+            requester.name, notification["X-Launchpad-Message-For"])
+        self.assertEqual(
+            "snap-build-upload-scan-failed",
+            notification["X-Launchpad-Notification-Type"])
+        body, footer = notification.get_payload(decode=True).split("\n-- \n")
+        self.assertIn("Scan failed.", body)
+        self.assertEqual(
+            "http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n"
+            "You are the requester of the build.\n" % snapbuild.id, footer)

=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
--- lib/lp/snappy/tests/test_snapstoreclient.py	2016-06-06 13:03:31 +0000
+++ lib/lp/snappy/tests/test_snapstoreclient.py	2016-06-27 13:20:12 +0000
@@ -43,8 +43,11 @@
 from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
 from lp.snappy.interfaces.snapstoreclient import (
     BadRequestPackageUploadResponse,
+    BadScanStatusResponse,
     ISnapStoreClient,
+    ScanFailedResponse,
     UnauthorizedUploadResponse,
+    UploadNotScannedYetResponse,
     )
 from lp.snappy.model.snapstoreclient import (
     InvalidStoreSecretsError,
@@ -199,7 +202,16 @@
     @urlmatch(path=r".*/snap-push/$")
     def _snap_push_handler(self, url, request):
         self.snap_push_request = request
-        return {"status_code": 202, "content": {"success": True}}
+        return {
+            "status_code": 202,
+            "content": {
+                "success": True,
+                "status_url": (
+                    "http://sca.example/dev/api/"
+                    "click-scan-complete/updown/1/"),
+                "status_details_url": (
+                    "http://sca.example/dev/api/snaps/1/builds/1/status"),
+                }}
 
     @urlmatch(path=r".*/api/v2/tokens/refresh$")
     def _macaroon_refresh_handler(self, url, request):
@@ -273,7 +285,9 @@
         self.factory.makeSnapFile(snapbuild=snapbuild, libraryfile=lfa)
         transaction.commit()
         with HTTMock(self._unscanned_upload_handler, self._snap_push_handler):
-            self.client.upload(snapbuild)
+            self.assertEqual(
+                "http://sca.example/dev/api/snaps/1/builds/1/status",
+                self.client.upload(snapbuild))
         self.assertThat(self.unscanned_upload_request, RequestMatches(
             url=Equals("http://updown.example/unscanned-upload/"),
             method=Equals("POST"),
@@ -340,11 +354,38 @@
         transaction.commit()
         with HTTMock(self._unscanned_upload_handler, snap_push_handler,
                      self._macaroon_refresh_handler):
-            self.client.upload(snapbuild)
+            self.assertEqual(
+                "http://sca.example/dev/api/snaps/1/builds/1/status",
+                self.client.upload(snapbuild))
         self.assertEqual(2, snap_push_handler.call_count)
         self.assertNotEqual(
             store_secrets["discharge"], snap.store_secrets["discharge"])
 
+    def test_upload_old_status_url(self):
+        @urlmatch(path=r".*/snap-push/$")
+        def snap_push_handler(url, request):
+            return {
+                "status_code": 202,
+                "content": {
+                    "success": True,
+                    "status_url": (
+                        "http://sca.example/dev/api/"
+                        "click-scan-complete/updown/1/"),
+                    }}
+
+        snap = self.factory.makeSnap(
+            store_upload=True,
+            store_series=self.factory.makeSnappySeries(name="rolling"),
+            store_name="test-snap", store_secrets=self._make_store_secrets())
+        snapbuild = self.factory.makeSnapBuild(snap=snap)
+        lfa = self.factory.makeLibraryFileAlias(content="dummy snap content")
+        self.factory.makeSnapFile(snapbuild=snapbuild, libraryfile=lfa)
+        transaction.commit()
+        with HTTMock(self._unscanned_upload_handler, snap_push_handler):
+            self.assertEqual(
+                "http://sca.example/dev/api/click-scan-complete/updown/1/",
+                self.client.upload(snapbuild))
+
     def test_refresh_discharge_macaroon(self):
         store_secrets = self._make_store_secrets()
         snap = self.factory.makeSnap(
@@ -361,3 +402,139 @@
             json_data={"discharge_macaroon": store_secrets["discharge"]}))
         self.assertNotEqual(
             store_secrets["discharge"], snap.store_secrets["discharge"])
+
+    def test_checkStatus_old_pending(self):
+        @all_requests
+        def handler(url, request):
+            return {
+                "status_code": 200,
+                "content": {
+                    "completed": False, "application_url": "",
+                    "revision": None,
+                    "message": "Task 1 is waiting for execution.",
+                    "package_name": None,
+                    }}
+
+        status_url = "http://sca.example/dev/api/click-scan-complete/updown/1/"
+        with HTTMock(handler):
+            self.assertRaises(
+                UploadNotScannedYetResponse, self.client.checkStatus,
+                status_url)
+
+    def test_checkStatus_old_error(self):
+        @all_requests
+        def handler(url, request):
+            return {
+                "status_code": 200,
+                "content": {
+                    "completed": True, "application_url": "", "revision": None,
+                    "message": "You cannot use that reserved namespace.",
+                    "package_name": None,
+                    }}
+
+        status_url = "http://sca.example/dev/api/click-scan-complete/updown/1/"
+        with HTTMock(handler):
+            self.assertRaisesWithContent(
+                ScanFailedResponse, b"You cannot use that reserved namespace.",
+                self.client.checkStatus, status_url)
+
+    def test_checkStatus_old_success(self):
+        @all_requests
+        def handler(url, request):
+            return {
+                "status_code": 200,
+                "content": {
+                    "completed": True,
+                    "application_url": "http://sca.example/dev/click-apps/1/",
+                    "revision": 1, "message": "", "package_name": "test",
+                    }}
+
+        status_url = "http://sca.example/dev/api/click-scan-complete/updown/1/"
+        with HTTMock(handler):
+            self.assertEqual(
+                "http://sca.example/dev/click-apps/1/",
+                self.client.checkStatus(status_url))
+
+    def test_checkStatus_new_pending(self):
+        @all_requests
+        def handler(url, request):
+            return {
+                "status_code": 200,
+                "content": {
+                    "code": "being_processed", "processed": False,
+                    "can_release": False,
+                    }}
+
+        status_url = "http://sca.example/dev/api/snaps/1/builds/1/status"
+        with HTTMock(handler):
+            self.assertRaises(
+                UploadNotScannedYetResponse, self.client.checkStatus,
+                status_url)
+
+    def test_checkStatus_new_error(self):
+        @all_requests
+        def handler(url, request):
+            return {
+                "status_code": 200,
+                "content": {
+                    "code": "processing_error", "processed": True,
+                    "can_release": False,
+                    "errors": [
+                        {"code": None,
+                         "message": "You cannot use that reserved namespace.",
+                         }],
+                    }}
+
+        status_url = "http://sca.example/dev/api/snaps/1/builds/1/status"
+        with HTTMock(handler):
+            self.assertRaisesWithContent(
+                ScanFailedResponse,
+                b"You cannot use that reserved namespace.",
+                self.client.checkStatus, status_url)
+
+    def test_checkStatus_new_review_error(self):
+        @all_requests
+        def handler(url, request):
+            return {
+                "status_code": 200,
+                "content": {
+                    "code": "processing_error", "processed": True,
+                    "can_release": False,
+                    "errors": [{"code": None, "message": "Review failed."}],
+                    "url": "http://sca.example/dev/click-apps/1/rev/1/",
+                    }}
+
+        status_url = "http://sca.example/dev/api/snaps/1/builds/1/status"
+        with HTTMock(handler):
+            self.assertRaisesWithContent(
+                ScanFailedResponse, b"Review failed.",
+                self.client.checkStatus, status_url)
+
+    def test_checkStatus_new_complete(self):
+        @all_requests
+        def handler(url, request):
+            return {
+                "status_code": 200,
+                "content": {
+                    "code": "ready_to_release", "processed": True,
+                    "can_release": True,
+                    "url": "http://sca.example/dev/click-apps/1/rev/1/",
+                    "revision": 1,
+                    }}
+
+        status_url = "http://sca.example/dev/api/snaps/1/builds/1/status"
+        with HTTMock(handler):
+            self.assertEqual(
+                "http://sca.example/dev/click-apps/1/rev/1/",
+                self.client.checkStatus(status_url))
+
+    def test_checkStatus_404(self):
+        @all_requests
+        def handler(url, request):
+            return {"status_code": 404, "reason": b"Not found"}
+
+        status_url = "http://sca.example/dev/api/snaps/1/builds/1/status"
+        with HTTMock(handler):
+            self.assertRaisesWithContent(
+                BadScanStatusResponse, b"404 Client Error: Not found",
+                self.client.checkStatus, status_url)