launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging lp:~twom/launchpad/allow-none-in-snapbuild-metadata into lp:launchpad.

Commit message:
Allow store_upload_metadata to be None

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/allow-none-in-snapbuild-metadata/+merge/361072

Add tests for same.
Handle the possible None case at point of access in SnapUploadJob.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~twom/launchpad/allow-none-in-snapbuild-metadata into lp:launchpad.

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2018-12-12 10:33:29 +0000
+++ lib/lp/snappy/model/snapbuild.py	2018-12-18 14:25:16 +0000
@@ -175,7 +175,7 @@
 
     failure_count = Int(name='failure_count', allow_none=False)
 
-    store_upload_metadata = JSON('store_upload_json_data', allow_none=False)
+    store_upload_metadata = JSON('store_upload_json_data', allow_none=True)
 
     def __init__(self, build_farm_job, requester, snap, archive,
                  distro_arch_series, pocket, channels, processor, virtualized,

=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	2018-12-13 11:00:18 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2018-12-18 14:25:16 +0000
@@ -216,7 +216,7 @@
         """See `ISnapStoreUploadJob`."""
         intermediate = {}
         intermediate.update(self.metadata)
-        intermediate.update(self.snapbuild.store_upload_metadata)
+        intermediate.update(self.snapbuild.store_upload_metadata or {})
         return intermediate
 
     @property

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2018-10-09 10:35:44 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2018-12-18 14:25:16 +0000
@@ -35,6 +35,7 @@
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.registry.enums import PersonVisibility
 from lp.services.config import config
+from lp.services.database.interfaces import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
@@ -49,6 +50,7 @@
     SnapBuildStoreUploadStatus,
     )
 from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
+from lp.snappy.model.snapbuild import SnapBuild
 from lp.soyuz.enums import ArchivePurpose
 from lp.testing import (
     ANONYMOUS,
@@ -774,3 +776,12 @@
         browser = self.getNonRedirectingBrowser(user=self.person)
         for file_url in file_urls:
             self.assertCanOpenRedirectedUrl(browser, file_url)
+
+    def test_store_upload_metadata_existing_as_none(self):
+        db_build = self.factory.makeSnapBuild(requester=self.person)
+        unsecure_db_build = removeSecurityProxy(db_build)
+        unsecure_db_build.store_upload_metadata = None
+        store = IStore(SnapBuild)
+        store.flush()
+        loaded_build = store.find(SnapBuild, id=unsecure_db_build.id).one()
+        self.assertIsNone(loaded_build.store_upload_metadata)

=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	2018-12-12 10:37:06 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2018-12-18 14:25:16 +0000
@@ -18,9 +18,11 @@
     MatchesStructure,
     )
 from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
 
 from lp.buildmaster.enums import BuildStatus
 from lp.services.config import config
+from lp.services.database.interfaces import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.runner import JobRunner
@@ -39,6 +41,7 @@
     UploadFailedResponse,
     UploadNotScannedYetResponse,
     )
+from lp.snappy.model.snapbuild import SnapBuild
 from lp.snappy.model.snapbuildjob import (
     SnapBuildJob,
     SnapBuildJobType,
@@ -678,3 +681,14 @@
         self.assertEqual(previous_checkStatus, client.checkStatus.calls)
         self.assertEqual(len_previous_release + 1, len(client.release.calls))
         self.assertIsNone(job.error_message)
+
+    def test_with_snapbuild_metadata_as_none(self):
+        db_build = self.factory.makeSnapBuild()
+        unsecure_db_build = removeSecurityProxy(db_build)
+        unsecure_db_build.store_upload_metadata = None
+        store = IStore(SnapBuild)
+        store.flush()
+        loaded_build = store.find(SnapBuild, id=unsecure_db_build.id).one()
+
+        job = SnapStoreUploadJob.create(loaded_build)
+        self.assertEqual({}, job.store_metadata)