Merge ~andrey-fedoseev/launchpad:snap-base-features into launchpad:master

[Andrey Fedoseev]

Fix the case when `SnapBase.feature` is `None`

[Jürgen Gmach (jugmac00)]

I do not know what a SnapBase is, and the docstring for ISnapBase isn't helpful at all ("A base for snaps."), but I still wonder whether we could initialize `features` as an empty dict, so we could save us those `is None` checks.

[Andrey Fedoseev (andrey-fedoseev)]

None is an existing value in the database that we need to account for.

On Fri, Oct 7, 2022, 11:17 Jürgen Gmach <email address hidden>
wrote:

> I do not know what a SnapBase is, and the docstring for ISnapBase isn't
> helpful at all ("A base for snaps."), but I still wonder whether we could
> initialize `features` as an empty dict, so we could save us those `is None`
> checks.
> --
>
> https://code.launchpad.net/~andrey-fedoseev/launchpad/+git/launchpad/+merge/431181
> You are the owner of ~andrey-fedoseev/launchpad:snap-base-features.
>
>

[Jürgen Gmach (jugmac00)]

How did they land in the database in the first place?

Could we convert them to empty dicts?

[Andrey Fedoseev (andrey-fedoseev)]

This is a new nullable column that's been added recently

On Fri, Oct 7, 2022, 11:59 Jürgen Gmach <email address hidden>
wrote:

> How did they land in the database in the first place?
>
> Could we convert them to empty dicts?
> --
>
> https://code.launchpad.net/~andrey-fedoseev/launchpad/+git/launchpad/+merge/431181
> You are the owner of ~andrey-fedoseev/launchpad:snap-base-features.
>
>

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/snappy/model/snapbase.py b/lib/lp/snappy/model/snapbase.py
index b6e8b33..a67159b 100644
--- a/lib/lp/snappy/model/snapbase.py
+++ b/lib/lp/snappy/model/snapbase.py
@@ -74,7 +74,7 @@ class SnapBase(Storm):
 
     is_default = Bool(name="is_default", allow_none=False)
 
-    _features = PgJSON(name="features", allow_none=False)
+    _features = PgJSON(name="features", allow_none=True)
 
     def __init__(
         self,
@@ -98,6 +98,8 @@ class SnapBase(Storm):
 
     @property
     def features(self) -> Dict[Item, bool]:
+        if self._features is None:
+            return {}
         features = {}
         for token, is_enabled in self._features.items():
             try:
diff --git a/lib/lp/snappy/tests/test_snapbase.py b/lib/lp/snappy/tests/test_snapbase.py
index 978e8b6..e7cd522 100644
--- a/lib/lp/snappy/tests/test_snapbase.py
+++ b/lib/lp/snappy/tests/test_snapbase.py
@@ -93,6 +93,11 @@ class TestSnapBase(TestCaseWithFactory):
             snap_base.features,
         )
 
+    def test_blank_features(self):
+        snap_base = self.factory.makeSnapBase(name="foo")
+        removeSecurityProxy(snap_base)._features = None
+        self.assertEqual({}, snap_base.features)
+
 
 class TestSnapBaseProcessors(TestCaseWithFactory):