plugs/slots: match output format read in snapcraft.yaml (#2884)

Snapcraft recently started always writing the long-form
of plugs for consistency, where the `interface` attribute
is always defined.

Unfortunately this seems to have tripped up a bug in review-tools
that won't likely be fixed for a while.  This commit restores
the 3.8 behavior to ensure the output snap.yaml matches the syntax
used in the snapcraft.yaml.

Signed-off-by: Chris Patterson <[email protected]>

snapcraft/internal/meta/plugs.py

@@ -27,9 +27,20 @@
 class Plug:
     """Generic plug."""
 
-    def __init__(self, *, plug_name: str) -> None:
+    def __init__(
+        self,
+        *,
+        plug_name: str,
+        plug_dict: Optional[Dict[str, Any]] = None,
+        use_string_representation: bool = False,
+    ) -> None:
         self._plug_name = plug_name
-        self._plug_dict: Dict[str, Any] = {"interface": plug_name}
+        if plug_dict is None:
+            self._plug_dict: Dict[str, Any] = dict()
+        else:
+            self._plug_dict = plug_dict
+
+        self.use_string_representation = use_string_representation
 
     @property
     def plug_name(self) -> str:
@@ -40,15 +51,8 @@ def plug_name(self) -> str:
     def validate(self) -> None:
         """Validate plug, raising an exception on failure."""
 
-        if not self._plug_dict:
-            raise PlugValidationError(
-                plug_name=self.plug_name, message="plug has no defined attributes"
-            )
-
-        if "interface" not in self._plug_dict:
-            raise PlugValidationError(
-                plug_name=self.plug_name, message="plug has no defined interface"
-            )
+        # Nothing to validate (yet).
+        return
 
     @classmethod
     def from_dict(cls, *, plug_dict: Dict[str, Any], plug_name: str) -> "Plug":
@@ -69,16 +73,22 @@ def from_object(cls, *, plug_object: Any, plug_name: str) -> "Plug":
         if plug_object is None:
             return Plug(plug_name=plug_name)
         elif isinstance(plug_object, str):
-            plug = Plug(plug_name=plug_name)
+            plug = Plug(plug_name=plug_name, use_string_representation=True)
             plug._plug_dict["interface"] = plug_object
             return plug
         elif isinstance(plug_object, dict):
             return Plug.from_dict(plug_dict=plug_object, plug_name=plug_name)
 
         raise RuntimeError(f"unknown syntax for plug {plug_name!r}: {plug_object!r}")
 
-    def to_dict(self) -> Dict[str, Any]:
-        """Create dictionary from plug."""
+    def to_yaml_object(self) -> Any:
+        # To output string short-form: "slot-name: interface-type"
+        if self.use_string_representation:
+            return self._plug_dict["interface"]
+
+        # To output shortest-form: "slot-name-is-interface: <empty>"
+        if not self._plug_dict:
+            return None
 
         return OrderedDict(deepcopy(self._plug_dict))
 
@@ -160,7 +170,7 @@ def from_dict(cls, *, plug_dict: Dict[str, str], plug_name: str) -> "ContentPlug
             default_provider=plug_dict.get("default-provider", None),
         )
 
-    def to_dict(self) -> Dict[str, str]:
+    def to_yaml_object(self) -> Dict[str, str]:
         props = [("interface", self.interface)]
 
         # Only include content if set explicitly.

snapcraft/internal/meta/slots.py

@@ -29,9 +29,21 @@
 class Slot:
     """Representation of a generic snap slot."""
 
-    def __init__(self, *, slot_name: str) -> None:
+    def __init__(
+        self,
+        *,
+        slot_name: str,
+        slot_dict: Optional[Dict[str, Any]] = None,
+        use_string_representation: bool = False,
+    ) -> None:
         self._slot_name = slot_name
-        self._slot_dict: Dict[str, Any] = {"interface": slot_name}
+
+        if slot_dict is None:
+            self._slot_dict: Dict[str, Any] = dict()
+        else:
+            self._slot_dict = slot_dict
+
+        self.use_string_representation = use_string_representation
 
     @property
     def slot_name(self) -> str:
@@ -41,15 +53,8 @@ def slot_name(self) -> str:
     def validate(self) -> None:
         """Validate slot, raising exception on error."""
 
-        if not self._slot_dict:
-            raise SlotValidationError(
-                slot_name=self.slot_name, message="slot has no defined attributes"
-            )
-
-        if "interface" not in self._slot_dict:
-            raise SlotValidationError(
-                slot_name=self.slot_name, message="slot has no defined interface"
-            )
+        # Nothing to validate (yet).
+        return
 
     @classmethod
     def from_dict(cls, *, slot_dict: Dict[str, Any], slot_name: str) -> "Slot":
@@ -62,24 +67,30 @@ def from_dict(cls, *, slot_dict: Dict[str, Any], slot_name: str) -> "Slot":
             return slot_class.from_dict(slot_dict=slot_dict, slot_name=slot_name)
 
         # Handle the general case.
-        slot = Slot(slot_name=slot_name)
-        slot._slot_dict.update(slot_dict)
-        return slot
+        return Slot(slot_name=slot_name, slot_dict=slot_dict)
 
     @classmethod
     def from_object(cls, *, slot_object: Any, slot_name: str) -> "Slot":
         if slot_object is None:
             return Slot(slot_name=slot_name)
         elif isinstance(slot_object, str):
-            slot = Slot(slot_name=slot_name)
+            slot = Slot(slot_name=slot_name, use_string_representation=True)
             slot._slot_dict["interface"] = slot_object
             return slot
         elif isinstance(slot_object, dict):
             return Slot.from_dict(slot_dict=slot_object, slot_name=slot_name)
 
         raise RuntimeError(f"unknown syntax for slot {slot_name!r}: {slot_object!r}")
 
-    def to_dict(self) -> Dict[str, Any]:
+    def to_yaml_object(self) -> Optional[Dict[str, Any]]:
+        # To output string short-form: "slot-name: interface-type"
+        if self.use_string_representation:
+            return self._slot_dict["interface"]
+
+        # To output shortest-form: "slot-name-is-interface: <empty>"
+        if not self._slot_dict:
+            return None
+
         return OrderedDict(deepcopy(self._slot_dict))
 
     def __repr__(self) -> str:
@@ -175,7 +186,7 @@ def from_dict(cls, *, slot_dict: Dict[str, Any], slot_name: str) -> "ContentSlot
 
         return slot
 
-    def to_dict(self) -> Dict[str, Any]:
+    def to_yaml_object(self) -> Dict[str, Any]:
         props: List[Tuple[str, Any]] = [("interface", self.interface)]
 
         # Only include content if set explicitly.
@@ -249,7 +260,7 @@ def validate(self) -> None:
                 message="valid `name` is required for dbus slot",
             )
 
-    def to_dict(self) -> Dict[str, Any]:
+    def to_yaml_object(self) -> Dict[str, Any]:
         """Return dict of dbus slot."""
 
         return OrderedDict(

snapcraft/internal/meta/snap.py

@@ -265,11 +265,11 @@ def to_dict(self):  # noqa: C901
             elif key == "plugs":
                 snap_dict[key] = dict()
                 for name, plug in sorted(self.plugs.items()):
-                    snap_dict[key][name] = plug.to_dict()
+                    snap_dict[key][name] = plug.to_yaml_object()
             elif key == "slots":
                 snap_dict[key] = dict()
                 for name, slot in sorted(self.slots.items()):
-                    snap_dict[key][name] = slot.to_dict()
+                    snap_dict[key][name] = slot.to_yaml_object()
             else:
                 snap_dict[key] = deepcopy(self.__dict__[key])
 

tests/unit/meta/test_plugs.py

@@ -35,14 +35,6 @@ def test_plug_name(self):
         self.assertEqual(plug_name, plug.plug_name)
         self.assertEqual(plug_name, plug_from_dict.plug_name)
 
-    def test_invalid_raises_exception(self):
-        plug_name = "plug-test"
-
-        plug = Plug(plug_name=plug_name)
-        plug._plug_dict = dict({})
-
-        self.assertRaises(errors.PlugValidationError, plug.validate)
-
     def test_from_empty_dict(self):
         plug_dict = OrderedDict({})
         plug_name = "plug-test"
@@ -63,13 +55,14 @@ def test_from_object_none(self):
         plug = Plug.from_object(plug_name="plug-name", plug_object=None)
         plug.validate()
 
-        self.assertThat(plug._plug_dict["interface"], Equals("plug-name"))
+        self.assertThat(plug._plug_dict, Equals(dict()))
 
     def test_from_object_string(self):
         plug = Plug.from_object(plug_name="plug-name", plug_object="some-interface")
         plug.validate()
 
         self.assertThat(plug._plug_dict["interface"], Equals("some-interface"))
+        self.assertThat(plug.use_string_representation, Equals(True))
 
     def test_from_object_dict(self):
         plug_dict = OrderedDict(
@@ -120,7 +113,7 @@ def test_basic_from_dict(self):
         transformed_dict = plug_dict.copy()
         transformed_dict["default-provider"] = plug_provider
 
-        self.assertEqual(transformed_dict, plug.to_dict())
+        self.assertEqual(transformed_dict, plug.to_yaml_object())
         self.assertEqual(plug_name, plug.plug_name)
         self.assertEqual(plug_dict["content"], plug.content)
         self.assertEqual(plug_dict["target"], plug.target)

tests/unit/meta/test_slots.py

@@ -35,14 +35,6 @@ def test_slot_name(self):
         self.assertEqual(slot_name, slot.slot_name)
         self.assertEqual(slot_name, slot_from_dict.slot_name)
 
-    def test_invalid_raises_exception(self):
-        slot_name = "slot-test"
-
-        slot = Slot(slot_name=slot_name)
-        slot._slot_dict = dict({})
-
-        self.assertRaises(errors.SlotValidationError, slot.validate)
-
     def test_from_empty_dict(self):
         slot_dict = OrderedDict({})
         slot_name = "slot-test"
@@ -63,13 +55,14 @@ def test_from_object_none(self):
         slot = Slot.from_object(slot_name="slot-name", slot_object=None)
         slot.validate()
 
-        self.assertThat(slot._slot_dict["interface"], Equals("slot-name"))
+        self.assertThat(slot._slot_dict, Equals(dict()))
 
     def test_from_object_string(self):
         slot = Slot.from_object(slot_name="slot-name", slot_object="some-interface")
         slot.validate()
 
         self.assertThat(slot._slot_dict["interface"], Equals("some-interface"))
+        self.assertThat(slot.use_string_representation, Equals(True))
 
     def test_from_object_dict(self):
         slot_dict = OrderedDict(
@@ -80,7 +73,9 @@ def test_from_object_dict(self):
         slot = Slot.from_object(slot_object=slot_dict, slot_name=slot_name)
 
         slot.validate()
+
         self.assertThat(slot._slot_dict["interface"], Equals("some-interface"))
+        self.assertThat(slot._slot_dict["someprop"], Equals("somevalue"))
 
 
 class ContentSlotTests(unit.TestCase):
@@ -94,7 +89,7 @@ def test_empty(self):
         transformed_dict = slot_dict.copy()
         transformed_dict["source"] = {}
 
-        self.assertEqual(transformed_dict, slot.to_dict())
+        self.assertEqual(transformed_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertRaises(errors.SlotValidationError, slot.validate)
         self.assertEqual(set(), slot.get_content_dirs(installed_path=""))
@@ -106,7 +101,7 @@ def test_empty_from_dict(self):
         slot = Slot.from_dict(slot_dict=slot_dict, slot_name=slot_name)
 
         self.assertIsInstance(slot, ContentSlot)
-        self.assertEqual(slot_dict, slot.to_dict())
+        self.assertEqual(slot_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertRaises(errors.SlotValidationError, slot.validate)
         self.assertEqual(set(), slot.get_content_dirs(installed_path=""))
@@ -117,7 +112,7 @@ def test_empty_force_no_source(self):
 
         slot = ContentSlot(use_source_key=False, slot_name=slot_name)
 
-        self.assertEqual(slot_dict, slot.to_dict())
+        self.assertEqual(slot_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertRaises(errors.SlotValidationError, slot.validate)
         self.assertEqual(set(), slot.get_content_dirs(installed_path=""))
@@ -129,7 +124,7 @@ def test_read_from_dict(self):
         slot = ContentSlot.from_dict(slot_dict=slot_dict, slot_name=slot_name)
         slot.validate()
 
-        self.assertEqual(slot_dict, slot.to_dict())
+        self.assertEqual(slot_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertEqual(slot_dict["read"], slot.read)
         self.assertEqual(
@@ -147,7 +142,7 @@ def test_read_from_dict_force_source_key(self):
         transformed_dict["source"] = dict()
         transformed_dict["source"]["read"] = transformed_dict.pop("read")
 
-        self.assertEqual(transformed_dict, slot.to_dict())
+        self.assertEqual(transformed_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertEqual(slot_dict["read"], slot.read)
         self.assertEqual(
@@ -163,7 +158,7 @@ def test_source_read_from_dict(self):
         slot = ContentSlot.from_dict(slot_dict=slot_dict, slot_name=slot_name)
         slot.validate()
 
-        self.assertEqual(slot_dict, slot.to_dict())
+        self.assertEqual(slot_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertEqual(slot_dict["source"]["read"], slot.read)
         self.assertEqual(
@@ -177,7 +172,7 @@ def test_write_from_dict(self):
         slot = ContentSlot.from_dict(slot_dict=slot_dict, slot_name=slot_name)
         slot.validate()
 
-        self.assertEqual(slot_dict, slot.to_dict())
+        self.assertEqual(slot_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertEqual(slot_dict["write"], slot.write)
         self.assertEqual(
@@ -195,7 +190,7 @@ def test_write_from_dict_force_source_key(self):
         transformed_dict["source"] = dict()
         transformed_dict["source"]["write"] = transformed_dict.pop("write")
 
-        self.assertEqual(transformed_dict, slot.to_dict())
+        self.assertEqual(transformed_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertEqual(slot_dict["write"], slot.write)
         self.assertEqual(
@@ -211,7 +206,7 @@ def test_source_write_from_dict(self):
         slot = ContentSlot.from_dict(slot_dict=slot_dict, slot_name=slot_name)
         slot.validate()
 
-        self.assertEqual(slot_dict, slot.to_dict())
+        self.assertEqual(slot_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertEqual(slot_dict["source"]["write"], slot.write)
         self.assertEqual(
@@ -251,7 +246,7 @@ def test_basic(self):
         )
         slot.validate()
 
-        self.assertEqual(slot_dict, slot.to_dict())
+        self.assertEqual(slot_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertEqual(slot_dict["bus"], slot.bus)
         self.assertEqual(slot_dict["name"], slot.name)
@@ -263,7 +258,7 @@ def test_basic_from_dict(self):
         slot = DbusSlot.from_dict(slot_dict=slot_dict, slot_name=slot_name)
         slot.validate()
 
-        self.assertEqual(slot_dict, slot.to_dict())
+        self.assertEqual(slot_dict, slot.to_yaml_object())
         self.assertEqual(slot_name, slot.slot_name)
         self.assertEqual(slot_dict["bus"], slot.bus)
         self.assertEqual(slot_dict["name"], slot.name)