Add dict() methods on Diff and DiffElement

glennmatthews · glennmatthews · commit c1341fcab303 · 2020-10-23T14:30:11.000-04:00
diff --git a/dsync/__init__.py b/dsync/__init__.py
@@ -222,7 +222,7 @@ def str(self, include_children: bool = True, indent: int = 0) -> str:
         return output
 
     @classmethod
-    def create(cls, dsync: "DSync", ids: Dict, attrs: Dict) -> Optional["DSyncModel"]:
+    def create(cls, dsync: "DSync", ids: Mapping, attrs: Mapping) -> Optional["DSyncModel"]:
         """Instantiate this class, along with any platform-specific data creation.
 
         Args:
@@ -239,7 +239,7 @@ def create(cls, dsync: "DSync", ids: Dict, attrs: Dict) -> Optional["DSyncModel"
         """
         return cls(**ids, dsync=dsync, **attrs)
 
-    def update(self, attrs: Dict) -> Optional["DSyncModel"]:
+    def update(self, attrs: Mapping) -> Optional["DSyncModel"]:
         """Update the attributes of this instance, along with any platform-specific data updates.
 
         Args:
@@ -292,15 +292,15 @@ def get_children_mapping(cls) -> Mapping[Text, Text]:
         """Get the mapping of types to fieldnames for child models of this model."""
         return cls._children
 
-    def get_identifiers(self) -> Dict:
+    def get_identifiers(self) -> Mapping:
         """Get a dict of all identifiers (primary keys) and their values for this object.
 
         Returns:
             dict: dictionary containing all primary keys for this device, as defined in _identifiers
         """
         return self.dict(include=set(self._identifiers))
 
-    def get_attrs(self) -> Dict:
+    def get_attrs(self) -> Mapping:
         """Get all the non-primary-key attributes or parameters for this object.
 
         Similar to Pydantic's `BaseModel.dict()` method, with the following key differences:
@@ -441,9 +441,9 @@ def load(self):
         """Load all desired data from whatever backend data source into this instance."""
         # No-op in this generic class
 
-    def dict(self, exclude_defaults: bool = True, **kwargs) -> dict:
+    def dict(self, exclude_defaults: bool = True, **kwargs) -> Mapping:
         """Represent the DSync contents as a dict, as if it were a Pydantic model."""
-        data: Dict[str, Dict[str, dict]] = {}
+        data: Dict[str, Dict[str, Dict]] = {}
         for modelname in self._data:
             data[modelname] = {}
             for unique_id, model in self._data[modelname].items():
@@ -531,13 +531,13 @@ def _sync_from_diff_element(
                 log.debug("Attempting object creation")
                 if obj:
                     raise ObjectNotCreated(f"Failed to create {object_class.get_type()} {element.keys} - it exists!")
-                obj = object_class.create(dsync=self, ids=element.keys, attrs={key: diffs[key]["src"] for key in diffs})
+                obj = object_class.create(dsync=self, ids=element.keys, attrs=diffs["src"])
                 log.info("Created successfully", status="success")
             elif element.action == "update":
                 log.debug("Attempting object update")
                 if not obj:
                     raise ObjectNotUpdated(f"Failed to update {object_class.get_type()} {element.keys} - not found!")
-                obj = obj.update(attrs={key: diffs[key]["src"] for key in diffs})
+                obj = obj.update(attrs=diffs["src"])
                 log.info("Updated successfully", status="success")
             elif element.action == "delete":
                 log.debug("Attempting object deletion")
@@ -606,7 +606,7 @@ def diff_to(self, target: "DSync", diff_class: Type[Diff] = Diff, flags: DSyncFl
     # ------------------------------------------------------------------------------
 
     def get(
-        self, obj: Union[Text, DSyncModel, Type[DSyncModel]], identifier: Union[Text, Dict]
+        self, obj: Union[Text, DSyncModel, Type[DSyncModel]], identifier: Union[Text, Mapping]
     ) -> Optional[DSyncModel]:
         """Get one object from the data store based on its unique id.
 
diff --git a/dsync/diff.py b/dsync/diff.py
@@ -87,7 +87,7 @@ def get_children(self) -> Iterator["DiffElement"]:
             yield from order_method(self.children[group])
 
     @classmethod
-    def order_children_default(cls, children: dict) -> Iterator["DiffElement"]:
+    def order_children_default(cls, children: Mapping) -> Iterator["DiffElement"]:
         """Default method to an Iterator for children.
 
         Since children is already an OrderedDefaultDict, this method is not doing anything special.
@@ -112,23 +112,31 @@ def str(self, indent: int = 0):
             result = "(no diffs)"
         return result
 
+    def dict(self) -> Mapping[Text, Mapping[Text, Mapping]]:
+        """Build a dictionary representation of this Diff."""
+        result = OrderedDefaultDict(dict)
+        for child in self.get_children():
+            if child.has_diffs(include_children=True):
+                result[child.type][child.name] = child.dict()
+        return dict(result)
+
 
 @total_ordering
 class DiffElement:  # pylint: disable=too-many-instance-attributes
     """DiffElement object, designed to represent a single item/object that may or may not have any diffs."""
 
     def __init__(
-        self, obj_type: Text, name: Text, keys: dict, source_name: Text = "source", dest_name: Text = "dest"
+        self, obj_type: Text, name: Text, keys: Mapping, source_name: Text = "source", dest_name: Text = "dest"
     ):  # pylint: disable=too-many-arguments
         """Instantiate a DiffElement.
 
         Args:
-            obj_type (str): Name of the object type being described, as in DSyncModel.get_type().
-            name (str): Human-readable name of the object being described, as in DSyncModel.get_shortname().
+            obj_type: Name of the object type being described, as in DSyncModel.get_type().
+            name: Human-readable name of the object being described, as in DSyncModel.get_shortname().
                 This name must be unique within the context of the Diff that is the direct parent of this DiffElement.
-            keys (dict): Primary keys and values uniquely describing this object, as in DSyncModel.get_identifiers().
-            source_name (str): Name of the source DSync object
-            dest_name (str): Name of the destination DSync object
+            keys: Primary keys and values uniquely describing this object, as in DSyncModel.get_identifiers().
+            source_name: Name of the source DSync object
+            dest_name: Name of the destination DSync object
         """
         if not isinstance(obj_type, str):
             raise ValueError(f"obj_type must be a string (not {type(obj_type)})")
@@ -142,8 +150,8 @@ def __init__(
         self.source_name = source_name
         self.dest_name = dest_name
         # Note: *_attrs == None if no target object exists; it'll be an empty dict if it exists but has no _attributes
-        self.source_attrs: Optional[dict] = None
-        self.dest_attrs: Optional[dict] = None
+        self.source_attrs: Optional[Mapping] = None
+        self.dest_attrs: Optional[Mapping] = None
         self.child_diff = Diff()
 
     def __lt__(self, other):
@@ -194,7 +202,7 @@ def action(self) -> Optional[Text]:
         return None
 
     # TODO: separate into set_source_attrs() and set_dest_attrs() methods, or just use direct property access instead?
-    def add_attrs(self, source: Optional[dict] = None, dest: Optional[dict] = None):
+    def add_attrs(self, source: Optional[Mapping] = None, dest: Optional[Mapping] = None):
         """Set additional attributes of a source and/or destination item that may result in diffs."""
         # TODO: should source_attrs and dest_attrs be "write-once" properties, or is it OK to overwrite them once set?
         if source is not None:
@@ -218,25 +226,31 @@ def get_attrs_keys(self) -> Iterable[Text]:
             return self.source_attrs.keys()
         return []
 
-    # The below would be more accurate but typing.Literal is only in Python 3.8 and later
-    # def get_attrs_diffs(self) -> Mapping[Text, Mapping[Literal["src", "dst"], Any]]:
     def get_attrs_diffs(self) -> Mapping[Text, Mapping[Text, Any]]:
         """Get the dict of actual attribute diffs between source_attrs and dest_attrs.
 
         Returns:
-            dict: of the form `{key: {src: <value>, dst: <value>}, key2: ...}`
+            dict: of the form `{src: {key1: <value>, key2: ...}, dst: {key1: <value>, key2: ...}}`,
+            where the `src` or `dst` dicts may be empty.
         """
         if self.source_attrs is not None and self.dest_attrs is not None:
             return {
-                key: dict(src=self.source_attrs[key], dst=self.dest_attrs[key])
-                for key in self.get_attrs_keys()
-                if self.source_attrs[key] != self.dest_attrs[key]
+                "src": {
+                    key: self.source_attrs[key]
+                    for key in self.get_attrs_keys()
+                    if self.source_attrs[key] != self.dest_attrs[key]
+                },
+                "dst": {
+                    key: self.dest_attrs[key]
+                    for key in self.get_attrs_keys()
+                    if self.source_attrs[key] != self.dest_attrs[key]
+                },
             }
         if self.source_attrs is None and self.dest_attrs is not None:
-            return {key: dict(src=None, dst=self.dest_attrs[key]) for key in self.get_attrs_keys()}
+            return {"src": {}, "dst": {key: self.dest_attrs[key] for key in self.get_attrs_keys()}}
         if self.source_attrs is not None and self.dest_attrs is None:
-            return {key: dict(src=self.source_attrs[key], dst=None) for key in self.get_attrs_keys()}
-        return {}
+            return {"src": {key: self.source_attrs[key] for key in self.get_attrs_keys()}, "dst": {}}
+        return {"src": {}, "dst": {}}
 
     def add_child(self, element: "DiffElement"):
         """Attach a child object of type DiffElement.
@@ -279,11 +293,12 @@ def str(self, indent: int = 0):
         result = f"{margin}{self.type}: {self.name}"
         if self.source_attrs is not None and self.dest_attrs is not None:
             # Only print attrs that have meaning in both source and dest
-            for attr, item in self.get_attrs_diffs().items():
+            attrs_diffs = self.get_attrs_diffs()
+            for attr in attrs_diffs["src"]:
                 result += (
                     f"\n{margin}  {attr}"
-                    f"    {self.source_name}({item.get('src')})"
-                    f"    {self.dest_name}({item.get('dst')})"
+                    f"    {self.source_name}({attrs_diffs['src'][attr]})"
+                    f"    {self.dest_name}({attrs_diffs['dst'][attr]})"
                 )
         elif self.dest_attrs is not None:
             result += f" MISSING in {self.source_name}"
@@ -295,3 +310,15 @@ def str(self, indent: int = 0):
         elif self.source_attrs is None and self.dest_attrs is None:
             result += " (no diffs)"
         return result
+
+    def dict(self) -> Mapping[Text, Mapping[Text, Any]]:
+        """Build a dictionary representation of this DiffElement and its children."""
+        attrs_diffs = self.get_attrs_diffs()
+        result = {}
+        if attrs_diffs.get("src"):
+            result["_src"] = attrs_diffs["src"]
+        if attrs_diffs.get("dst"):
+            result["_dst"] = attrs_diffs["dst"]
+        if self.child_diff.has_diffs():
+            result.update(self.child_diff.dict())
+        return result
diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py
@@ -14,7 +14,7 @@
 See the License for the specific language governing permissions and
 limitations under the License.
 """
-from typing import ClassVar, List, Optional, Tuple
+from typing import ClassVar, List, Mapping, Optional, Tuple
 
 import pytest
 
@@ -23,17 +23,6 @@
 from dsync.exceptions import ObjectNotCreated, ObjectNotUpdated, ObjectNotDeleted
 
 
-@pytest.fixture()
-def give_me_success():
-    """
-    Provides True to make tests pass
-
-    Returns:
-        (bool): Returns True
-    """
-    return True
-
-
 @pytest.fixture
 def generic_dsync_model():
     """Provide a generic DSyncModel instance."""
@@ -46,14 +35,14 @@ class ErrorProneModel(DSyncModel):
     _counter: ClassVar[int] = 0
 
     @classmethod
-    def create(cls, dsync: DSync, ids: dict, attrs: dict):
+    def create(cls, dsync: DSync, ids: Mapping, attrs: Mapping):
         """As DSyncModel.create(), but periodically throw exceptions."""
         cls._counter += 1
         if not cls._counter % 3:
             raise ObjectNotCreated("Random creation error!")
         return super().create(dsync, ids, attrs)
 
-    def update(self, attrs: dict):
+    def update(self, attrs: Mapping):
         """As DSyncModel.update(), but periodically throw exceptions."""
         # pylint: disable=protected-access
         self.__class__._counter += 1
diff --git a/tests/unit/test_diff.py b/tests/unit/test_diff.py
@@ -37,6 +37,12 @@ def test_diff_str_with_no_diffs():
     assert diff.str() == "(no diffs)"
 
 
+def test_diff_dict_with_no_diffs():
+    diff = Diff()
+
+    assert diff.dict() == {}
+
+
 def test_diff_children():
     """Test the basic functionality of the Diff class when adding child elements."""
     diff = Diff()
@@ -89,6 +95,21 @@ def test_diff_str_with_diffs():
     )
 
 
+def test_diff_dict_with_diffs():
+    diff = Diff()
+    device_element = DiffElement("device", "device1", {"name": "device1"})
+    diff.add(device_element)
+    intf_element = DiffElement("interface", "eth0", {"device_name": "device1", "name": "eth0"})
+    source_attrs = {"interface_type": "ethernet", "description": "my interface"}
+    dest_attrs = {"description": "your interface"}
+    intf_element.add_attrs(source=source_attrs, dest=dest_attrs)
+    diff.add(intf_element)
+
+    assert diff.dict() == {
+        "interface": {"eth0": {"_dst": {"description": "your interface"}, "_src": {"description": "my interface"}}},
+    }
+
+
 def test_order_children_default(backend_a, backend_b):
     """Test that order_children_default is properly called when calling get_children."""
 
diff --git a/tests/unit/test_diff_element.py b/tests/unit/test_diff_element.py
@@ -47,6 +47,11 @@ def test_diff_element_str_with_no_diffs():
     assert element.str() == "interface: eth0 (no diffs)"
 
 
+def test_diff_element_dict_with_no_diffs():
+    element = DiffElement("interface", "eth0", {"device_name": "device1", "name": "eth0"})
+    assert element.dict() == {}
+
+
 def test_diff_element_attrs():
     """Test the basic functionality of the DiffElement class when setting and retrieving attrs."""
     element = DiffElement("interface", "eth0", {"device_name": "device1", "name": "eth0"})
@@ -85,6 +90,14 @@ def test_diff_element_str_with_diffs():
     )
 
 
+def test_diff_element_dict_with_diffs():
+    element = DiffElement("interface", "eth0", {"device_name": "device1", "name": "eth0"})
+    element.add_attrs(source={"interface_type": "ethernet", "description": "my interface"})
+    assert element.dict() == {"_src": {"description": "my interface", "interface_type": "ethernet"}}
+    element.add_attrs(dest={"description": "your interface"})
+    assert element.dict() == {"_dst": {"description": "your interface"}, "_src": {"description": "my interface"}}
+
+
 def test_diff_element_children():
     """Test the basic functionality of the DiffElement class when storing and retrieving child elements."""
     child_element = DiffElement("interface", "eth0", {"device_name": "device1", "name": "eth0"})
@@ -122,3 +135,16 @@ def test_diff_element_str_with_child_diffs():
       description    source(my interface)    dest(your interface)\
 """
     )
+
+
+def test_diff_element_dict_with_child_diffs():
+    child_element = DiffElement("interface", "eth0", {"device_name": "device1", "name": "eth0"})
+    parent_element = DiffElement("device", "device1", {"name": "device1"})
+    parent_element.add_child(child_element)
+    source_attrs = {"interface_type": "ethernet", "description": "my interface"}
+    dest_attrs = {"description": "your interface"}
+    child_element.add_attrs(source=source_attrs, dest=dest_attrs)
+
+    assert parent_element.dict() == {
+        "interface": {"eth0": {"_dst": {"description": "your interface"}, "_src": {"description": "my interface"}}},
+    }
