Merge lp:~twom/launchpad/allow-none-in-snapbuild-metadata into lp:launchpad

[Tom Wardill]

Set the metadata to {} on each access

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2018-12-18 17:20:04 +0000
+++ lib/lp/snappy/model/snapbuild.py	2018-12-18 18:24:19 +0000
@@ -175,9 +175,7 @@
 
     failure_count = Int(name='failure_count', allow_none=False)
 
-    # Private variable that is used by the `store_upload_metadata` to
-    # set as an empty dict rather than None, to handle pre-existing data.
-    _store_upload_metadata = JSON('store_upload_json_data', allow_none=True)
+    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,
@@ -194,22 +192,12 @@
         self.processor = processor
         self.virtualized = virtualized
         self.date_created = date_created
-        self._store_upload_metadata = store_upload_metadata
+        self.store_upload_metadata = store_upload_metadata
         if build_request is not None:
             self.build_request_id = build_request.id
         self.status = BuildStatus.NEEDSBUILD
 
     @property
-    def store_upload_metadata(self):
-        if self._store_upload_metadata is None:
-            self._store_upload_metadata = {}
-        return self._store_upload_metadata
-
-    @store_upload_metadata.setter
-    def store_upload_metadata(self, value):
-        self._store_upload_metadata = value
-
-    @property
     def build_request(self):
         """See `ISnapBuild`."""
         if self.build_request_id is not None:

=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	2018-12-18 17:15:21 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2018-12-18 18:24:19 +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
@@ -257,6 +257,8 @@
     @store_url.setter
     def store_url(self, url):
         """See `ISnapStoreUploadJob`."""
+        if self.snapbuild.store_upload_metadata is None:
+            self.snapbuild.store_upload_metadata = {}
         self.snapbuild.store_upload_metadata["store_url"] = url
 
     @property
@@ -267,6 +269,8 @@
     @store_revision.setter
     def store_revision(self, revision):
         """See `ISnapStoreUploadJob`."""
+        if self.snapbuild.store_upload_metadata is None:
+            self.snapbuild.store_upload_metadata = {}
         self.snapbuild.store_upload_metadata["store_revision"] = revision
 
     @property
@@ -276,6 +280,8 @@
 
     @status_url.setter
     def status_url(self, url):
+        if self.snapbuild.store_upload_metadata is None:
+            self.snapbuild.store_upload_metadata = {}
         self.snapbuild.store_upload_metadata["status_url"] = url
 
     # Ideally we'd just override Job._set_status or similar, but

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2018-12-18 17:15:21 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2018-12-18 18:24:19 +0000
@@ -35,7 +35,6 @@
 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
@@ -50,7 +49,6 @@
     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,
@@ -776,13 +774,3 @@
         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)
-        self.assertIsNotNone(loaded_build.store_upload_metadata)

=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	2018-12-18 17:15:21 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2018-12-18 18:24:19 +0000
@@ -685,7 +685,7 @@
     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
+        unsecure_db_build.store_upload_metadata = None
         store = IStore(SnapBuild)
         store.flush()
         loaded_build = store.find(SnapBuild, id=unsecure_db_build.id).one()
@@ -696,7 +696,7 @@
     def test_with_snapbuild_metadata_as_none_set_status(self):
         db_build = self.factory.makeSnapBuild()
         unsecure_db_build = removeSecurityProxy(db_build)
-        unsecure_db_build._store_upload_metadata = None
+        unsecure_db_build.store_upload_metadata = None
         store = IStore(SnapBuild)
         store.flush()
         loaded_build = store.find(SnapBuild, id=unsecure_db_build.id).one()