diff --git a/README.md b/README.md index efcd31ba..d4453450 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ B.load() # it will show the difference between both systems diff_a_b = A.diff_from(B) -diff.print_detailed() +print(diff.str()) # it will update System A to align with the current status of system B A.sync_from(B) diff --git a/dsync/__init__.py b/dsync/__init__.py index 0bcaba9c..74728dff 100644 --- a/dsync/__init__.py +++ b/dsync/__init__.py @@ -18,7 +18,7 @@ from collections.abc import Iterable as ABCIterable, Mapping as ABCMapping import enum from inspect import isclass -from typing import ClassVar, Iterable, List, Mapping, MutableMapping, Optional, Tuple, Type, Union +from typing import ClassVar, Dict, Iterable, List, Mapping, MutableMapping, Optional, Text, Tuple, Type, Union from pydantic import BaseModel import structlog # type: ignore @@ -187,28 +187,42 @@ def __repr__(self): def __str__(self): return self.get_unique_id() - def print_detailed(self, dsync: "Optional[DSync]" = None, indent: int = 0): - """Print this model and its children.""" + def dict(self, **kwargs) -> dict: + """Convert this DSyncModel to a dict, excluding the dsync field by default as it is not serializable.""" + if "exclude" not in kwargs: + kwargs["exclude"] = {"dsync"} + return super().dict(**kwargs) + + def json(self, **kwargs) -> str: + """Convert this DSyncModel to a JSON string, excluding the dsync field by default as it is not serializable.""" + if "exclude" not in kwargs: + kwargs["exclude"] = {"dsync"} + if "exclude_defaults" not in kwargs: + kwargs["exclude_defaults"] = True + return super().json(**kwargs) + + def str(self, include_children: bool = True, indent: int = 0) -> str: + """Build a detailed string representation of this DSyncModel and optionally its children.""" margin = " " * indent - if not dsync: - dsync = self.dsync - print(f"{margin}{self.get_type()}: {self.get_unique_id()}") + output = f"{margin}{self.get_type()}: {self.get_unique_id()}: {self.get_attrs()}" for modelname, fieldname in self._children.items(): - print(f"{margin} {modelname}") + output += f"\n{margin} {fieldname}" child_ids = getattr(self, fieldname) if not child_ids: - print(f"{margin} (none)") - for child_id in child_ids: - child = None - if dsync: - child = dsync.get(modelname, child_id) - if not child: - print(f"{margin} {child_id} (no details available)") - else: - child.print_detailed(dsync, indent + 4) + output += ": []" + elif not self.dsync or not include_children: + output += f": {child_ids}" + else: + for child_id in child_ids: + child = self.dsync.get(modelname, child_id) + if not child: + output += f"\n{margin} {child_id} (details unavailable)" + else: + output += "\n" + child.str(include_children=include_children, indent=indent + 4) + 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: @@ -225,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: @@ -256,7 +270,7 @@ def delete(self) -> Optional["DSyncModel"]: return self @classmethod - def get_type(cls) -> str: + def get_type(cls) -> Text: """Return the type AKA modelname of the object or the class Returns: @@ -265,7 +279,7 @@ def get_type(cls) -> str: return cls._modelname @classmethod - def create_unique_id(cls, **identifiers) -> str: + def create_unique_id(cls, **identifiers) -> Text: """Construct a unique identifier for this model class. Args: @@ -274,11 +288,11 @@ def create_unique_id(cls, **identifiers) -> str: return "__".join(str(identifiers[key]) for key in cls._identifiers) @classmethod - def get_children_mapping(cls) -> Mapping[str, str]: + 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: @@ -286,7 +300,7 @@ def get_identifiers(self) -> dict: """ 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: @@ -299,7 +313,7 @@ def get_attrs(self) -> dict: """ return self.dict(include=set(self._attributes)) - def get_unique_id(self) -> str: + def get_unique_id(self) -> Text: """Get the unique ID of an object. By default the unique ID is built based on all the primary keys defined in `_identifiers`. @@ -309,7 +323,7 @@ def get_unique_id(self) -> str: """ return self.create_unique_id(**self.get_identifiers()) - def get_shortname(self) -> str: + def get_shortname(self) -> Text: """Get the (not guaranteed-unique) shortname of an object, if any. By default the shortname is built based on all the keys defined in `_shortname`. @@ -427,16 +441,28 @@ def load(self): """Load all desired data from whatever backend data source into this instance.""" # No-op in this generic class - def print_detailed(self, indent: int = 0): - """Recursively print this DSync and its contained models.""" + 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]] = {} + for modelname in self._data: + data[modelname] = {} + for unique_id, model in self._data[modelname].items(): + data[modelname][unique_id] = model.dict(exclude_defaults=exclude_defaults, **kwargs) + return data + + def str(self, indent: int = 0) -> str: + """Build a detailed string representation of this DSync.""" margin = " " * indent + output = "" for modelname in self.top_level: - print(f"{margin}{modelname}") + output += f"{margin}{modelname}" models = self.get_all(modelname) if not models: - print(f"{margin} (none)") - for model in models: - model.print_detailed(self, indent + 2) + output += ": []" + else: + for model in models: + output += "\n" + model.str(indent=indent + 2) + return output # ------------------------------------------------------------------------------ # Synchronization between DSync instances @@ -505,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") @@ -579,7 +605,9 @@ def diff_to(self, target: "DSync", diff_class: Type[Diff] = Diff, flags: DSyncFl # Object Storage Management # ------------------------------------------------------------------------------ - def get(self, obj: Union[str, DSyncModel, Type[DSyncModel]], identifier: Union[str, dict]) -> Optional[DSyncModel]: + def get( + 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. Args: @@ -589,16 +617,23 @@ def get(self, obj: Union[str, DSyncModel, Type[DSyncModel]], identifier: Union[s if isinstance(obj, str): modelname = obj if not hasattr(self, obj): - return None - object_class = getattr(self, obj) + object_class = None + else: + object_class = getattr(self, obj) else: object_class = obj modelname = obj.get_type() if isinstance(identifier, str): uid = identifier - else: + elif object_class: uid = object_class.create_unique_id(**identifier) + else: + self._log.warning( + f"Tried to look up a {modelname} by identifier {identifier}, " + "but don't know how to convert that to a uid string", + ) + return None return self._data[modelname].get(uid) @@ -618,7 +653,7 @@ def get_all(self, obj): return self._data[modelname].values() - def get_by_uids(self, uids: List[str], obj) -> List[DSyncModel]: + def get_by_uids(self, uids: List[Text], obj) -> List[DSyncModel]: """Get multiple objects from the store by their unique IDs/Keys and type. Args: diff --git a/dsync/diff.py b/dsync/diff.py index 44aab641..19a9973b 100644 --- a/dsync/diff.py +++ b/dsync/diff.py @@ -16,7 +16,7 @@ """ from functools import total_ordering -from typing import Any, Iterator, Iterable, Mapping, Optional +from typing import Any, Iterator, Iterable, Mapping, Optional, Text from .exceptions import ObjectAlreadyExists from .utils import intersection, OrderedDefaultDict @@ -64,7 +64,7 @@ def has_diffs(self) -> bool: """ for group in self.groups(): for child in self.children[group].values(): - if child.has_diffs(): + if child.has_diffs(include_children=True): return True return False @@ -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. @@ -95,18 +95,30 @@ def order_children_default(cls, children: dict) -> Iterator["DiffElement"]: for child in children.values(): yield child - def print_detailed(self, indent: int = 0): - """Print all diffs to screen for all child elements. - - Args: - indent (int, optional): Indentation to use when printing to screen. Defaults to 0. - """ + def str(self, indent: int = 0): + """Build a detailed string representation of this Diff and its child DiffElements.""" margin = " " * indent + output = [] for group in self.groups(): - print(f"{margin}{group}") + group_heading_added = False for child in self.children[group].values(): - if child.has_diffs(): - child.print_detailed(indent + 2) + if child.has_diffs(include_children=True): + if not group_heading_added: + output.append(f"{margin}{group}") + group_heading_added = True + output.append(child.str(indent + 2)) + result = "\n".join(output) + if not result: + 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 @@ -114,17 +126,17 @@ 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: str, name: str, keys: dict, source_name: str = "source", dest_name: str = "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)})") @@ -138,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): @@ -170,7 +182,7 @@ def __str__(self): return f'{self.type} "{self.name}" : {self.keys} : {self.source_name} → {self.dest_name} : {self.get_attrs_diffs()}' @property - def action(self) -> Optional[str]: + def action(self) -> Optional[Text]: """Action, if any, that should be taken to remediate the diffs described by this element. Returns: @@ -190,7 +202,7 @@ def action(self) -> Optional[str]: 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: @@ -199,7 +211,7 @@ def add_attrs(self, source: Optional[dict] = None, dest: Optional[dict] = None): if dest is not None: self.dest_attrs = dest - def get_attrs_keys(self) -> Iterable[str]: + def get_attrs_keys(self) -> Iterable[Text]: """Get the list of shared attrs between source and dest, or the attrs of source or dest if only one is present. - If source_attrs is not set, return the keys of dest_attrs @@ -214,25 +226,31 @@ def get_attrs_keys(self) -> Iterable[str]: 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[str, Mapping[Literal["src", "dst"], Any]]: - def get_attrs_diffs(self) -> Mapping[str, Mapping[str, 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: , dst: }, key2: ...}` + dict: of the form `{src: {key1: , key2: ...}, dst: {key1: , 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. @@ -269,22 +287,38 @@ def has_diffs(self, include_children: bool = True) -> bool: return False - def print_detailed(self, indent: int = 0): - """Print status on screen for current object and all children. - - Args: - indent: Default value = 0 - """ + def str(self, indent: int = 0): + """Build a detailed string representation of this DiffElement and its children.""" margin = " " * indent - - if self.source_attrs is None: - print(f"{margin}{self.type}: {self.name} MISSING in {self.source_name}") - elif self.dest_attrs is None: - print(f"{margin}{self.type}: {self.name} MISSING in {self.dest_name}") - else: - print(f"{margin}{self.type}: {self.name}") + 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(): - print(f"{margin} {attr} {self.source_name}({item.get('src')}) {self.dest_name}({item.get('dst')})") - - self.child_diff.print_detailed(indent + 2) + attrs_diffs = self.get_attrs_diffs() + for attr in attrs_diffs["src"]: + result += ( + f"\n{margin} {attr}" + 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}" + elif self.source_attrs is not None: + result += f" MISSING in {self.dest_name}" + + if self.child_diff.has_diffs(): + result += "\n" + self.child_diff.str(indent + 2) + 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/examples/example1/README.md b/examples/example1/README.md index 54e87fb9..f7299f53 100644 --- a/examples/example1/README.md +++ b/examples/example1/README.md @@ -16,15 +16,15 @@ from backend_c import BackendC a = BackendA() a.load() -a.print_detailed() +print(a.str()) b = BackendB() b.load() -b.print_detailed() +print(b.str()) c = BackendC() c.load() -c.print_detailed() +print(c.str()) ``` Configure verbosity of DSync's structured logging to console; the default is full verbosity (all logs including debugging) @@ -38,25 +38,25 @@ enable_console_logging(verbosity=0) # Show WARNING and ERROR logs only Show the differences between A and B ```python diff_a_b = a.diff_to(b) -diff_a_b.print_detailed() +print(diff_a_b.str()) ``` Show the differences between B and C ```python diff_b_c = c.diff_from(b) -diff_b_c.print_detailed() +print(diff_b_c.str()) ``` Synchronize A and B (update B with the contents of A) ```python a.sync_to(b) -a.diff_to(b).print_detailed() +print(a.diff_to(b).str()) ``` Now A and B will show no differences ```python diff_a_b = a.diff_to(b) -diff_a_b.print_detailed() +print(diff_a_b.str()) ``` > In the Device model, the `site_name` and `role` are not included in the `_attributes`, so they are not shown when we are comparing the different objects, even if the value is different. diff --git a/examples/example1/main.py b/examples/example1/main.py index 41e95d12..2a5bd8d2 100644 --- a/examples/example1/main.py +++ b/examples/example1/main.py @@ -47,26 +47,26 @@ def main(): print("Initializing and loading Backend A...") backend_a = BackendA(name="Backend-A") backend_a.load() - backend_a.print_detailed() + print(backend_a.str()) print("Initializing and loading Backend B...") backend_b = BackendB(name="Backend-B") backend_b.load() - backend_b.print_detailed() + print(backend_b.str()) print("Initializing and loading Backend C...") backend_c = BackendC() backend_c.load() - backend_c.print_detailed() + print(backend_c.str()) print("Getting diffs from Backend A to Backend B...") diff_a_b = backend_a.diff_to(backend_b, diff_class=MyDiff) - diff_a_b.print_detailed() + print(diff_a_b.str()) print("Syncing changes from Backend A to Backend B...") backend_a.sync_to(backend_b) print("Getting updated diffs from Backend A to Backend B...") - backend_a.diff_to(backend_b).print_detailed() + print(backend_a.diff_to(backend_b).str()) if __name__ == "__main__": diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index d8ed456e..9695fed5 100644 --- 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 @@ -85,11 +74,11 @@ class Site(DSyncModel): def make_site(): """Factory for Site instances.""" - def site(name="site1", devices=None): + def site(name="site1", devices=None, **kwargs): """Provide an instance of a Site model.""" if not devices: devices = [] - return Site(name=name, devices=devices) + return Site(name=name, devices=devices, **kwargs) return site diff --git a/tests/unit/test_diff.py b/tests/unit/test_diff.py index cc5b1703..6961e16d 100644 --- a/tests/unit/test_diff.py +++ b/tests/unit/test_diff.py @@ -30,7 +30,17 @@ def test_diff_empty(): assert not diff.has_diffs() assert list(diff.get_children()) == [] - # TODO: test print_detailed + +def test_diff_str_with_no_diffs(): + diff = Diff() + + assert diff.str() == "(no diffs)" + + +def test_diff_dict_with_no_diffs(): + diff = Diff() + + assert diff.dict() == {} def test_diff_children(): @@ -63,7 +73,41 @@ def test_diff_children(): assert diff.has_diffs() - # TODO: test print_detailed + +def test_diff_str_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) + + # Since device_element has no diffs, we don't have any "device" entry in the diff string: + assert ( + diff.str() + == """\ +interface + interface: eth0 + description source(my interface) dest(your interface)\ +""" + ) + + +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): diff --git a/tests/unit/test_diff_element.py b/tests/unit/test_diff_element.py index 0ab85ec0..c48e6b89 100644 --- a/tests/unit/test_diff_element.py +++ b/tests/unit/test_diff_element.py @@ -40,7 +40,16 @@ def test_diff_element_empty(): ) assert element2.source_name == "S1" assert element2.dest_name == "D1" - # TODO: test print_detailed + + +def test_diff_element_str_with_no_diffs(): + element = DiffElement("interface", "eth0", {"device_name": "device1", "name": "eth0"}) + 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(): @@ -66,7 +75,27 @@ def test_diff_element_attrs(): assert element.has_diffs(include_children=False) assert element.get_attrs_keys() == ["description"] # intersection of source_attrs.keys() and dest_attrs.keys() - # TODO: test print_detailed + +def test_diff_element_str_with_diffs(): + element = DiffElement("interface", "eth0", {"device_name": "device1", "name": "eth0"}) + element.add_attrs(source={"interface_type": "ethernet", "description": "my interface"}) + assert element.str() == "interface: eth0 MISSING in dest" + element.add_attrs(dest={"description": "your interface"}) + assert ( + element.str() + == """\ +interface: eth0 + description source(my interface) dest(your interface)\ +""" + ) + + +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(): @@ -88,4 +117,34 @@ def test_diff_element_children(): assert parent_element.has_diffs(include_children=True) assert not parent_element.has_diffs(include_children=False) - # TODO: test print_detailed + +def test_diff_element_str_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.str() + == """\ +device: device1 + interface + interface: eth0 + 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"}}}, + } diff --git a/tests/unit/test_dsync.py b/tests/unit/test_dsync.py index 5540c194..2f0e9ded 100644 --- a/tests/unit/test_dsync.py +++ b/tests/unit/test_dsync.py @@ -33,6 +33,14 @@ def test_dsync_generic_load_is_noop(generic_dsync): assert len(generic_dsync._data) == 0 # pylint: disable=protected-access +def test_dsync_dict_with_no_data(generic_dsync): + assert generic_dsync.dict() == {} + + +def test_dsync_str_with_no_data(generic_dsync): + assert generic_dsync.str() == "" + + def test_dsync_diff_self_with_no_data_has_no_diffs(generic_dsync): assert generic_dsync.diff_from(generic_dsync).has_diffs() is False assert generic_dsync.diff_to(generic_dsync).has_diffs() is False @@ -69,8 +77,9 @@ def test_dsync_get_with_generic_model(generic_dsync, generic_dsync_model): generic_dsync.add(generic_dsync_model) # The generic_dsync_model has an empty identifier/unique-id assert generic_dsync.get(DSyncModel, "") == generic_dsync_model - # DSync doesn't know what a "dsyncmodel" is - assert generic_dsync.get(DSyncModel.get_type(), "") is None + assert generic_dsync.get(DSyncModel.get_type(), "") == generic_dsync_model + # DSync doesn't know how to construct a uid str for a "dsyncmodel" + assert generic_dsync.get(DSyncModel.get_type(), {}) is None # Wrong object-type - no match assert generic_dsync.get("", "") is None # Wrong unique-id - no match @@ -121,6 +130,113 @@ class BadElementName(DSync): assert "dev_class" in str(excinfo.value) +def test_dsync_dict_with_data(backend_a): + assert backend_a.dict() == { + "device": { + "nyc-spine1": { + "interfaces": ["nyc-spine1__eth0", "nyc-spine1__eth1"], + "name": "nyc-spine1", + "role": "spine", + "site_name": "nyc", + }, + "nyc-spine2": { + "interfaces": ["nyc-spine2__eth0", "nyc-spine2__eth1"], + "name": "nyc-spine2", + "role": "spine", + "site_name": "nyc", + }, + "rdu-spine1": { + "interfaces": ["rdu-spine1__eth0", "rdu-spine1__eth1"], + "name": "rdu-spine1", + "role": "spine", + "site_name": "rdu", + }, + "rdu-spine2": { + "interfaces": ["rdu-spine2__eth0", "rdu-spine2__eth1"], + "name": "rdu-spine2", + "role": "spine", + "site_name": "rdu", + }, + "sfo-spine1": { + "interfaces": ["sfo-spine1__eth0", "sfo-spine1__eth1"], + "name": "sfo-spine1", + "role": "spine", + "site_name": "sfo", + }, + "sfo-spine2": { + "interfaces": ["sfo-spine2__eth0", "sfo-spine2__eth1", "sfo-spine2__eth2"], + "name": "sfo-spine2", + "role": "spine", + "site_name": "sfo", + }, + }, + "interface": { + "nyc-spine1__eth0": {"description": "Interface 0", "device_name": "nyc-spine1", "name": "eth0"}, + "nyc-spine1__eth1": {"description": "Interface 1", "device_name": "nyc-spine1", "name": "eth1"}, + "nyc-spine2__eth0": {"description": "Interface 0", "device_name": "nyc-spine2", "name": "eth0"}, + "nyc-spine2__eth1": {"description": "Interface 1", "device_name": "nyc-spine2", "name": "eth1"}, + "rdu-spine1__eth0": {"description": "Interface 0", "device_name": "rdu-spine1", "name": "eth0"}, + "rdu-spine1__eth1": {"description": "Interface 1", "device_name": "rdu-spine1", "name": "eth1"}, + "rdu-spine2__eth0": {"description": "Interface 0", "device_name": "rdu-spine2", "name": "eth0"}, + "rdu-spine2__eth1": {"description": "Interface 1", "device_name": "rdu-spine2", "name": "eth1"}, + "sfo-spine1__eth0": {"description": "Interface 0", "device_name": "sfo-spine1", "name": "eth0"}, + "sfo-spine1__eth1": {"description": "Interface 1", "device_name": "sfo-spine1", "name": "eth1"}, + "sfo-spine2__eth0": {"description": "TBD", "device_name": "sfo-spine2", "name": "eth0"}, + "sfo-spine2__eth1": {"description": "ddd", "device_name": "sfo-spine2", "name": "eth1"}, + "sfo-spine2__eth2": {"description": "Interface 2", "device_name": "sfo-spine2", "name": "eth2"}, + }, + "person": {"Glenn Matthews": {"name": "Glenn Matthews"}}, + "site": { + "nyc": {"devices": ["nyc-spine1", "nyc-spine2"], "name": "nyc"}, + "rdu": {"devices": ["rdu-spine1", "rdu-spine2"], "name": "rdu", "people": ["Glenn Matthews"]}, + "sfo": {"devices": ["sfo-spine1", "sfo-spine2"], "name": "sfo"}, + }, + } + + +def test_dsync_str_with_data(backend_a): + assert ( + backend_a.str() + == """\ +site + site: nyc: {} + devices + device: nyc-spine1: {'role': 'spine', 'tag': ''} + interfaces + interface: nyc-spine1__eth0: {'interface_type': 'ethernet', 'description': 'Interface 0'} + interface: nyc-spine1__eth1: {'interface_type': 'ethernet', 'description': 'Interface 1'} + device: nyc-spine2: {'role': 'spine', 'tag': ''} + interfaces + interface: nyc-spine2__eth0: {'interface_type': 'ethernet', 'description': 'Interface 0'} + interface: nyc-spine2__eth1: {'interface_type': 'ethernet', 'description': 'Interface 1'} + people: [] + site: sfo: {} + devices + device: sfo-spine1: {'role': 'spine', 'tag': ''} + interfaces + interface: sfo-spine1__eth0: {'interface_type': 'ethernet', 'description': 'Interface 0'} + interface: sfo-spine1__eth1: {'interface_type': 'ethernet', 'description': 'Interface 1'} + device: sfo-spine2: {'role': 'spine', 'tag': ''} + interfaces + interface: sfo-spine2__eth0: {'interface_type': 'ethernet', 'description': 'TBD'} + interface: sfo-spine2__eth1: {'interface_type': 'ethernet', 'description': 'ddd'} + interface: sfo-spine2__eth2: {'interface_type': 'ethernet', 'description': 'Interface 2'} + people: [] + site: rdu: {} + devices + device: rdu-spine1: {'role': 'spine', 'tag': ''} + interfaces + interface: rdu-spine1__eth0: {'interface_type': 'ethernet', 'description': 'Interface 0'} + interface: rdu-spine1__eth1: {'interface_type': 'ethernet', 'description': 'Interface 1'} + device: rdu-spine2: {'role': 'spine', 'tag': ''} + interfaces + interface: rdu-spine2__eth0: {'interface_type': 'ethernet', 'description': 'Interface 0'} + interface: rdu-spine2__eth1: {'interface_type': 'ethernet', 'description': 'Interface 1'} + people + person: Glenn Matthews: {}""" + ) + + def test_dsync_diff_self_with_data_has_no_diffs(backend_a): # Self diff should always show no diffs! assert backend_a.diff_from(backend_a).has_diffs() is False @@ -230,7 +346,7 @@ def test_dsync_sync_from_with_continue_on_failure_flag(log, error_prone_backend_ error_prone_backend_a.sync_from(backend_b, flags=DSyncFlags.CONTINUE_ON_FAILURE) # Not all sync operations succeeded on the first try remaining_diffs = error_prone_backend_a.diff_from(backend_b) - remaining_diffs.print_detailed() + print(remaining_diffs.str()) # for debugging of any failure assert remaining_diffs.has_diffs() # At least some operations of each type should have succeeded @@ -248,7 +364,7 @@ def test_dsync_sync_from_with_continue_on_failure_flag(log, error_prone_backend_ print(f"Sync retry #{i}") error_prone_backend_a.sync_from(backend_b, flags=DSyncFlags.CONTINUE_ON_FAILURE) remaining_diffs = error_prone_backend_a.diff_from(backend_b) - remaining_diffs.print_detailed() + print(remaining_diffs.str()) # for debugging of any failure if remaining_diffs.has_diffs(): # If we still have diffs, some ERROR messages should have been logged assert [event for event in log.events if event["level"] == "error"] != [] @@ -311,7 +427,7 @@ def test_dsync_diff_with_ignore_flag_on_source_models(backend_a, backend_a_with_ backend_a_with_extra_models.get(backend_a_with_extra_models.site, "nyc").model_flags |= DSyncModelFlags.IGNORE diff = backend_a.diff_from(backend_a_with_extra_models) - diff.print_detailed() + print(diff.str()) # for debugging of any failure assert not diff.has_diffs() @@ -322,7 +438,7 @@ def test_dsync_diff_with_ignore_flag_on_target_models(backend_a, backend_a_minus backend_a.get(backend_a.site, "sfo").model_flags |= DSyncModelFlags.IGNORE diff = backend_a.diff_from(backend_a_minus_some_models) - diff.print_detailed() + print(diff.str()) # for debugging of any failure assert not diff.has_diffs() @@ -355,5 +471,5 @@ class NoDeleteInterfaceDSync(BackendA): assert extra_models.get(extra_models.interface, extra_interface.get_unique_id()) is None # The sync should be complete, regardless diff = extra_models.diff_from(backend_a) - diff.print_detailed() + print(diff.str()) # for debugging of any failure assert not diff.has_diffs() diff --git a/tests/unit/test_dsync_model.py b/tests/unit/test_dsync_model.py index bc7be614..5c69d805 100644 --- a/tests/unit/test_dsync_model.py +++ b/tests/unit/test_dsync_model.py @@ -19,7 +19,7 @@ import pytest -from dsync import DSyncModel +from dsync import DSyncModel, DSyncModelFlags from dsync.exceptions import ObjectStoreWrongType, ObjectAlreadyExists, ObjectNotFound from .conftest import Device, Interface @@ -40,6 +40,18 @@ def test_generic_dsync_model_methods(generic_dsync_model, make_site): generic_dsync_model.add_child(make_site()) +def test_dsync_model_dict_with_no_data(generic_dsync_model): + assert generic_dsync_model.dict() == {"model_flags": DSyncModelFlags.NONE} + + +def test_dsync_model_json_with_no_data(generic_dsync_model): + assert generic_dsync_model.json() == "{}" + + +def test_dsync_model_str_with_no_data(generic_dsync_model): + assert generic_dsync_model.str() == "dsyncmodel: : {}" + + def test_dsync_model_subclass_getters(make_site, make_device, make_interface): """Check that the DSyncModel APIs work correctly for various subclasses.""" site1 = make_site() @@ -79,6 +91,30 @@ def test_dsync_model_subclass_getters(make_site, make_device, make_interface): assert device1_eth0.get_shortname() == "eth0" +def test_dsync_model_dict_with_data(make_interface): + intf = make_interface() + # dict() includes all fields, even those set to default values + assert intf.dict() == { + "description": None, + "device_name": "device1", + "interface_type": "ethernet", + "model_flags": DSyncModelFlags.NONE, + "name": "eth0", + } + + +def test_dsync_model_json_with_data(make_interface): + intf = make_interface() + # json() omits default values for brevity + assert intf.json() == '{"device_name": "device1", "name": "eth0"}' + + +def test_dsync_model_str_with_data(make_interface): + intf = make_interface() + # str() only includes _attributes + assert intf.str() == "interface: device1__eth0: {'interface_type': 'ethernet', 'description': None}" + + def test_dsync_model_subclass_add_remove(make_site, make_device, make_interface): """Check that the DSyncModel add_child/remove_child APIs work correctly for various subclasses.""" site1 = make_site() @@ -116,6 +152,62 @@ def test_dsync_model_subclass_add_remove(make_site, make_device, make_interface) device1.remove_child(device1_eth0) +def test_dsync_model_dict_with_children(generic_dsync, make_site, make_device, make_interface): + site1 = make_site(dsync=generic_dsync) + device1 = make_device(dsync=generic_dsync) + device1_eth0 = make_interface(dsync=generic_dsync) + site1.add_child(device1) + device1.add_child(device1_eth0) + # test error handling - dsync knows about site and device but not interface + generic_dsync.add(site1) + generic_dsync.add(device1) + + assert site1.dict() == {"devices": ["device1"], "model_flags": DSyncModelFlags.NONE, "name": "site1"} + + +def test_dsync_model_json_with_children(generic_dsync, make_site, make_device, make_interface): + site1 = make_site(dsync=generic_dsync) + device1 = make_device(dsync=generic_dsync) + device1_eth0 = make_interface(dsync=generic_dsync) + site1.add_child(device1) + device1.add_child(device1_eth0) + # test error handling - dsync knows about site and device but not interface + generic_dsync.add(site1) + generic_dsync.add(device1) + + assert site1.json() == '{"name": "site1", "devices": ["device1"]}' + + +def test_dsync_model_str_with_children(generic_dsync, make_site, make_device, make_interface): + site1 = make_site(dsync=generic_dsync) + device1 = make_device(dsync=generic_dsync) + device1_eth0 = make_interface(dsync=generic_dsync) + site1.add_child(device1) + device1.add_child(device1_eth0) + # test error handling - dsync knows about site and device but not interface + generic_dsync.add(site1) + generic_dsync.add(device1) + + assert ( + site1.str() + == """\ +site: site1: {} + devices + device: device1: {'role': 'default'} + interfaces + device1__eth0 (details unavailable)\ +""" + ) + + assert ( + site1.str(include_children=False) + == """\ +site: site1: {} + devices: ['device1']\ +""" + ) + + def test_dsync_model_subclass_crud(generic_dsync): """Test basic CRUD operations on generic DSyncModel subclasses.""" device1 = Device.create(generic_dsync, {"name": "device1"}, {"role": "spine"})