Merge lp:~cjwatson/launchpad/snappy-series-can-guess-distro-series into lp:launchpad

[Colin Watson]

Support SnappySeries that can guess the distro series from snapcraft.yaml.

This lets us offer better defaults when creating or editing a Snap.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py	2019-02-07 16:35:34 +0000
+++ lib/lp/snappy/browser/snap.py	2019-02-07 16:35:34 +0000
@@ -490,7 +490,9 @@
                 store_name = snapcraft_data.get('name')
 
         store_series = getUtility(ISnappySeriesSet).getAll().first()
-        if store_series.preferred_distro_series is not None:
+        if store_series.can_infer_distro_series:
+            distro_series = None
+        elif store_series.preferred_distro_series is not None:
             distro_series = store_series.preferred_distro_series
         else:
             distro_series = store_series.usable_distro_series.first()

=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py	2019-02-07 16:35:34 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py	2019-02-07 16:35:34 +0000
@@ -32,6 +32,7 @@
     MatchesListwise,
     MatchesSetwise,
     MatchesStructure,
+    Not,
     )
 import transaction
 from zope.component import getUtility
@@ -226,6 +227,24 @@
             MatchesStructure.byEquality(
                 snappy_series=newest, distro_series=lts))
 
+    def test_initial_store_distro_series_can_infer_distro_series(self):
+        # If the latest snappy series supports inferring the distro series
+        # from snapcraft.yaml, then we default to that.
+        self.useFixture(BranchHostingFixture(blob=b""))
+        lts = self.factory.makeUbuntuDistroSeries(
+            version="16.04", status=SeriesStatus.CURRENT)
+        with admin_logged_in():
+            self.factory.makeSnappySeries(usable_distro_series=[lts])
+            newest = self.factory.makeSnappySeries(
+                preferred_distro_series=lts, can_infer_distro_series=True)
+        branch = self.factory.makeAnyBranch()
+        with person_logged_in(self.person):
+            view = create_initialized_view(branch, "+new-snap")
+        self.assertThat(
+            view.initial_values["store_distro_series"],
+            MatchesStructure(
+                snappy_series=Equals(newest), distro_series=Is(None)))
+
     def test_create_new_snap_not_logged_in(self):
         branch = self.factory.makeAnyBranch()
         self.assertRaises(
@@ -532,6 +551,26 @@
         self.assertContentEqual(
             ["386", "amd64"], [proc.name for proc in snap.processors])
 
+    def test_create_new_snap_infer_distro_series(self):
+        self.useFixture(BranchHostingFixture(blob=b""))
+        with admin_logged_in():
+            self.snappyseries.can_infer_distro_series = True
+        branch = self.factory.makeAnyBranch()
+        browser = self.getViewBrowser(
+            branch, view_name="+new-snap", user=self.person)
+        browser.getControl(name="field.name").value = "snap-name"
+        self.assertEqual(
+            [self.snappyseries.name],
+            browser.getControl(name="field.store_distro_series").value)
+        self.assertEqual(
+            self.snappyseries.name,
+            browser.getControl(name="field.store_distro_series").options[0])
+        browser.getControl("Create snap package").click()
+
+        content = find_main_content(browser.contents)
+        self.assertEqual("snap-name", extract_text(content.h1))
+        self.assertIsNone(find_tag_by_id(content, "distro_series"))
+
     def test_initial_name_extraction_bzr_success(self):
         self.useFixture(BranchHostingFixture(
             file_list={"snapcraft.yaml": "file-id"}, blob=b"name: test-snap"))
@@ -1404,6 +1443,31 @@
                 store_upload_tag,
                 soupmatchers.Tag("store name", "span", text=snap.store_name))))
 
+    def test_index_store_upload_no_distro_series(self):
+        # If the snap package is to be automatically uploaded to the store
+        # and is configured to infer an appropriate distro series from
+        # snapcraft.yaml, the index page shows details of this.
+        with admin_logged_in():
+            snappyseries = self.factory.makeSnappySeries(
+                usable_distro_series=[self.distroseries],
+                can_infer_distro_series=True)
+        snap = self.makeSnap(
+            distroseries=None, store_upload=True, store_series=snappyseries,
+            store_name=self.getUniqueString("store-name"))
+        view = create_initialized_view(snap, "+index")
+        store_upload_tag = soupmatchers.Tag(
+            "store upload", "div", attrs={"id": "store_upload"})
+        self.assertThat(view(), soupmatchers.HTMLContains(
+            Not(soupmatchers.Tag(
+                "distribution series", "dl", attrs={"id": "distro_series"})),
+            soupmatchers.Within(
+                store_upload_tag,
+                soupmatchers.Tag(
+                    "store series name", "span", text=snappyseries.title)),
+            soupmatchers.Within(
+                store_upload_tag,
+                soupmatchers.Tag("store name", "span", text=snap.store_name))))
+
     def setStatus(self, build, status):
         build.updateStatus(
             BuildStatus.BUILDING, date_started=build.date_created)

=== modified file 'lib/lp/snappy/interfaces/snappyseries.py'
--- lib/lp/snappy/interfaces/snappyseries.py	2019-01-30 18:00:09 +0000
+++ lib/lp/snappy/interfaces/snappyseries.py	2019-02-07 16:35:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Snappy series interfaces."""
@@ -86,7 +86,7 @@
     registrant = exported(PublicPersonChoice(
         title=_("Registrant"), required=True, readonly=True,
         vocabulary="ValidPersonOrTeam",
-        description=_("The person who registered this snap package.")))
+        description=_("The person who registered this snappy series.")))
 
 
 class ISnappySeriesEditableAttributes(Interface):
@@ -118,6 +118,12 @@
         value_type=Reference(schema=IDistroSeries),
         required=True, readonly=False))
 
+    can_infer_distro_series = exported(Bool(
+        title=_("Can infer distro series?"), required=True, readonly=False,
+        description=_(
+            "True if inferring a distro series from snapcraft.yaml is "
+            "supported for this snappy series.")))
+
 
 class ISnappySeries(ISnappySeriesView, ISnappySeriesEditableAttributes):
     """A series for snap packages in the store."""
@@ -134,7 +140,7 @@
     snappy_series = Reference(
         ISnappySeries, title=_("Snappy series"), readonly=True)
     distro_series = Reference(
-        IDistroSeries, title=_("Distro series"), readonly=True)
+        IDistroSeries, title=_("Distro series"), required=False, readonly=True)
     preferred = Bool(
         title=_("Preferred"),
         required=True, readonly=False,

=== modified file 'lib/lp/snappy/model/snappyseries.py'
--- lib/lp/snappy/model/snappyseries.py	2019-01-30 13:57:06 +0000
+++ lib/lp/snappy/model/snappyseries.py	2019-02-07 16:35:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Snappy series."""
@@ -138,18 +138,44 @@
                 link = SnappyDistroSeries(self, distro_series)
                 Store.of(self).add(link)
 
+    @cachedproperty
+    def _can_infer_distro_series(self):
+        return not Store.of(self).find(
+            SnappyDistroSeries,
+            SnappyDistroSeries.snappy_series == self,
+            SnappyDistroSeries.distro_series == None).is_empty()
+
+    @property
+    def can_infer_distro_series(self):
+        return self._can_infer_distro_series
+
+    @can_infer_distro_series.setter
+    def can_infer_distro_series(self, value):
+        store = Store.of(self)
+        current = store.find(
+            SnappyDistroSeries,
+            SnappyDistroSeries.snappy_series == self,
+            SnappyDistroSeries.distro_series == None).one()
+        if current is None and value is True:
+            store.add(SnappyDistroSeries(self, None))
+            get_property_cache(self)._can_infer_distro_series = True
+        elif current is not None and value is False:
+            store.remove(current)
+            get_property_cache(self)._can_infer_distro_series = False
+
 
 @implementer(ISnappyDistroSeries)
 class SnappyDistroSeries(Storm):
     """Link table between `SnappySeries` and `DistroSeries`."""
 
     __storm_table__ = 'SnappyDistroSeries'
-    __storm_primary__ = ('snappy_series_id', 'distro_series_id')
+
+    id = Int(primary=True)
 
     snappy_series_id = Int(name='snappy_series', allow_none=False)
     snappy_series = Reference(snappy_series_id, 'SnappySeries.id')
 
-    distro_series_id = Int(name='distro_series', allow_none=False)
+    distro_series_id = Int(name='distro_series', allow_none=True)
     distro_series = Reference(distro_series_id, 'DistroSeries.id')
 
     preferred = Bool(name='preferred', allow_none=False)
@@ -162,8 +188,11 @@
 
     @property
     def title(self):
-        return "%s, for %s" % (
-            self.distro_series.fullseriesname, self.snappy_series.title)
+        if self.distro_series is not None:
+            return "%s, for %s" % (
+                self.distro_series.fullseriesname, self.snappy_series.title)
+        else:
+            return self.snappy_series.title
 
 
 @implementer(ISnappySeriesSet)

=== modified file 'lib/lp/snappy/tests/test_snappyseries.py'
--- lib/lp/snappy/tests/test_snappyseries.py	2019-01-30 18:00:09 +0000
+++ lib/lp/snappy/tests/test_snappyseries.py	2019-02-07 16:35:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snappy series."""
@@ -86,6 +86,20 @@
         self.assertContentEqual([], snappy_series.usable_distro_series)
         self.assertIsNone(snappy_series.preferred_distro_series)
 
+    def test_can_infer_distro_series(self):
+        # Setting can_infer_distro_series indicates that this snappy series
+        # supports inferring the distro series from snapcraft.yaml.
+        snappy_series = self.factory.makeSnappySeries()
+        sds_set = getUtility(ISnappyDistroSeriesSet)
+        self.assertFalse(snappy_series.can_infer_distro_series)
+        self.assertIsNone(sds_set.getByBothSeries(snappy_series, None))
+        snappy_series.can_infer_distro_series = True
+        self.assertTrue(snappy_series.can_infer_distro_series)
+        self.assertIsNotNone(sds_set.getByBothSeries(snappy_series, None))
+        snappy_series.can_infer_distro_series = False
+        self.assertFalse(snappy_series.can_infer_distro_series)
+        self.assertIsNone(sds_set.getByBothSeries(snappy_series, None))
+
     def test_anonymous(self):
         # Anyone can view an `ISnappySeries`.
         series = self.factory.makeSnappySeries(name="dummy")
@@ -235,10 +249,21 @@
         self.assertThat(
             sds_set.getByBothSeries(snappy_serieses[0], dses[0]),
             MatchesStructure.byEquality(
-                snappy_series=snappy_serieses[0], distro_series=dses[0]))
+                snappy_series=snappy_serieses[0], distro_series=dses[0],
+                title="%s, for %s" % (
+                    dses[0].fullseriesname, snappy_serieses[0].title)))
         self.assertIsNone(sds_set.getByBothSeries(snappy_serieses[0], dses[1]))
         self.assertIsNone(sds_set.getByBothSeries(snappy_serieses[1], dses[0]))
         self.assertIsNone(sds_set.getByBothSeries(snappy_serieses[1], dses[1]))
+        self.assertIsNone(sds_set.getByBothSeries(snappy_serieses[0], None))
+        self.assertIsNone(sds_set.getByBothSeries(snappy_serieses[1], None))
+        snappy_serieses[0].can_infer_distro_series = True
+        self.assertThat(
+            sds_set.getByBothSeries(snappy_serieses[0], None),
+            MatchesStructure.byEquality(
+                snappy_series=snappy_serieses[0], distro_series=None,
+                title=snappy_serieses[0].title))
+        self.assertIsNone(sds_set.getByBothSeries(snappy_serieses[1], None))
 
     def test_getAll(self):
         sdses = list(IStore(SnappyDistroSeries).find(SnappyDistroSeries))

=== modified file 'lib/lp/snappy/vocabularies.py'
--- lib/lp/snappy/vocabularies.py	2017-01-04 20:52:12 +0000
+++ lib/lp/snappy/vocabularies.py	2019-02-07 16:35:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the GNU
+# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the GNU
 # Affero General Public License version 3 (see the file LICENSE).
 
 """Snappy vocabularies."""
@@ -6,12 +6,20 @@
 __metaclass__ = type
 
 __all__ = [
+    'BuildableSnappyDistroSeriesVocabulary',
     'SnapDistroArchSeriesVocabulary',
+    'SnappyDistroSeriesVocabulary',
     'SnappySeriesVocabulary',
+    'SnapStoreChannel',
+    'SnapStoreChannelVocabulary',
     ]
 
 from lazr.restful.interfaces import IJSONPublishable
-from storm.locals import Desc
+from storm.expr import LeftJoin
+from storm.locals import (
+    Desc,
+    Not,
+    )
 from zope.component import getUtility
 from zope.interface import implementer
 from zope.schema.vocabulary import (
@@ -24,6 +32,10 @@
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.series import ACTIVE_STATUSES
 from lp.services.database.interfaces import IStore
+from lp.services.database.stormexpr import (
+    IsDistinctFrom,
+    NullsFirst,
+    )
 from lp.services.webapp.vocabulary import StormVocabularyBase
 from lp.snappy.interfaces.snap import ISnap
 from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
@@ -62,26 +74,33 @@
     """A vocabulary for searching snappy/distro series combinations."""
 
     _table = SnappyDistroSeries
-    _clauses = [
-        SnappyDistroSeries.snappy_series_id == SnappySeries.id,
-        SnappyDistroSeries.distro_series_id == DistroSeries.id,
-        DistroSeries.distributionID == Distribution.id,
+    _origin = [
+        SnappyDistroSeries,
+        LeftJoin(
+            DistroSeries,
+            SnappyDistroSeries.distro_series_id == DistroSeries.id),
+        LeftJoin(Distribution, DistroSeries.distributionID == Distribution.id),
+        SnappySeries,
         ]
+    _clauses = [SnappyDistroSeries.snappy_series_id == SnappySeries.id]
 
     @property
     def _entries(self):
-        tables = [SnappyDistroSeries, SnappySeries, DistroSeries, Distribution]
-        entries = IStore(self._table).using(*tables).find(
+        entries = IStore(self._table).using(*self._origin).find(
             self._table, *self._clauses)
         return entries.order_by(
-            Distribution.display_name, Desc(DistroSeries.date_created),
+            NullsFirst(Distribution.display_name),
+            Desc(DistroSeries.date_created),
             Desc(SnappySeries.date_created))
 
     def toTerm(self, obj):
         """See `IVocabulary`."""
-        token = "%s/%s/%s" % (
-            obj.distro_series.distribution.name, obj.distro_series.name,
-            obj.snappy_series.name)
+        if obj.distro_series is None:
+            token = obj.snappy_series.name
+        else:
+            token = "%s/%s/%s" % (
+                obj.distro_series.distribution.name, obj.distro_series.name,
+                obj.snappy_series.name)
         return SimpleTerm(obj, token, obj.title)
 
     def __contains__(self, value):
@@ -96,15 +115,20 @@
 
     def getTermByToken(self, token):
         """See `IVocabularyTokenized`."""
-        try:
-            distribution_name, distro_series_name, snappy_series_name = (
-                token.split("/", 2))
-        except ValueError:
-            raise LookupError(token)
-        entry = IStore(self._table).find(
+        if "/" in token:
+            try:
+                distribution_name, distro_series_name, snappy_series_name = (
+                    token.split("/", 2))
+            except ValueError:
+                raise LookupError(token)
+        else:
+            distribution_name = None
+            distro_series_name = None
+            snappy_series_name = token
+        entry = IStore(self._table).using(*self._origin).find(
             self._table,
-            Distribution.name == distribution_name,
-            DistroSeries.name == distro_series_name,
+            Not(IsDistinctFrom(Distribution.name, distribution_name)),
+            Not(IsDistinctFrom(DistroSeries.name, distro_series_name)),
             SnappySeries.name == snappy_series_name,
             *self._clauses).one()
         if entry is None:

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2019-02-07 16:35:34 +0000
+++ lib/lp/testing/factory.py	2019-02-07 16:35:34 +0000
@@ -4824,7 +4824,8 @@
     def makeSnappySeries(self, registrant=None, name=None, display_name=None,
                          status=SeriesStatus.DEVELOPMENT,
                          preferred_distro_series=None, date_created=DEFAULT,
-                         usable_distro_series=None):
+                         usable_distro_series=None,
+                         can_infer_distro_series=False):
         """Make a new SnappySeries."""
         if registrant is None:
             registrant = self.makePerson()
@@ -4841,6 +4842,8 @@
             snappy_series.usable_distro_series = usable_distro_series
         elif preferred_distro_series is not None:
             snappy_series.usable_distro_series = [preferred_distro_series]
+        if can_infer_distro_series:
+            snappy_series.can_infer_distro_series = True
         IStore(snappy_series).flush()
         return snappy_series