feat: Added ability to set attributes on cables from connect_to (#70)

* feat: Added ability to set attributes on cables from `connect_to`

The cable connection plugin can now accept attributes to be set for the newly created cable. Also updated the following:

- Updated pylint disable lines for removed checks

- Renamed object_creator/ObjectCreator references in all of the
  extensions to align with builder/Builder

`cable_connection` now requires a `to` field with the `termination_b` query filter

* refactor: Converted two methods to static

* test: Trying to fix BGP extensions tests

design_builder/context.py

@@ -27,7 +27,7 @@ def __init__(self, root: "_Node"):
     def _compare(self, subscripts, other):
         """Compare 'other' to the node's data store."""
         for i in subscripts:
-            value = self.__getitem__(i)
+            value = self[i]
             if value != other[i]:
                 return False
         return True

design_builder/contrib/ext.py

@@ -7,21 +7,22 @@
 from django.db.models import Q
 
 from nautobot.dcim.models import Cable
-from nautobot.extras.models import Status
 from nautobot.ipam.models import Prefix
 
 import netaddr
 from design_builder.design import Builder
 from design_builder.design import ModelInstance
 
-from design_builder.errors import DesignImplementationError
+from design_builder.errors import DesignImplementationError, MultipleObjectsReturnedError, DoesNotExistError
 from design_builder.ext import Extension
 from design_builder.jinja2 import network_offset
 
 
 class LookupMixin:
     """A helper mixin that provides a way to lookup objects."""
 
+    builder: Builder
+
     def lookup_by_content_type(self, app_label, model_name, query):
         """Perform a query on a model.
 
@@ -46,29 +47,31 @@ def lookup_by_content_type(self, app_label, model_name, query):
 
         return self.lookup(queryset, query)
 
-    def lookup(self, queryset, query):  # pylint: disable=R0201
+    def lookup(self, queryset, query, parent=None):
         """Perform a single object lookup from a queryset.
 
         Args:
             queryset: Queryset (e.g. Status.objects.all) from which to query.
             query: Query params to filter by.
+            parent: Optional field used for better error reporting. Set this
+            value to the model instance that is semantically the parent so
+            that DesignModelErrors raised are more easily debugged.
 
         Raises:
-            DesignImplementationError: If either no object is found, or multiple objects are found.
+            DoesNotExistError: If either no object is found.
+            MultipleObjectsReturnedError: if multiple objects are found.
 
         Returns:
             Any: The object matching the query.
         """
-        # TODO: Convert this to lookup via ModelInstance
-        for key, value in query.items():
-            if hasattr(value, "instance"):
-                query[key] = value.instance
+        self.builder.resolve_values(query, unwrap_model_instances=True)
+
         try:
             return queryset.get(**query)
         except ObjectDoesNotExist:
-            raise DesignImplementationError(f"no {queryset.model.__name__} matching {query}")
+            raise DoesNotExistError(queryset.model, query_filter=query, parent=parent)
         except MultipleObjectsReturned:
-            raise DesignImplementationError(f"Multiple {queryset.model.__name__} objects match {query}")
+            raise MultipleObjectsReturnedError(queryset.model, query=query, parent=parent)
 
 
 class LookupExtension(Extension, LookupMixin):
@@ -143,12 +146,13 @@ def attribute(self, value, model_instance) -> None:
         """Connect a cable termination to another cable termination.
 
         Args:
-            value: Query for the `termination_b` side. This dictionary must
-            include a field `status` or `status__<lookup param>` that is either
-            a reference to a status object (former) or a lookup key/value to
-            get a status (latter). The query must also include enough
-            differentiating lookup params to retrieve a single matching termination
-            of the same type as the `termination_a` side.
+            value: Dictionary with details about the cable. At a minimum
+            the dictionary must have a `to` key which includes a query
+            dictionary that will return exactly one object to be added to the
+            `termination_b` side of the cable. All other attributes map
+            directly to the cable attributes. Cables require a status,
+            so the `status` field is mandatory and follows typical design
+            builder query lookup.
 
         Raises:
             DesignImplementationError: If no `status` was provided, or no matching
@@ -172,38 +176,49 @@ def attribute(self, value, model_instance) -> None:
                   status__name: "Active"
                   "!connect_cable":
                     status__name: "Planned"
-                    device: "!ref:device1"
-                    name: "GigabitEthernet1"
+                    to:
+                        device: "!ref:device1"
+                        name: "GigabitEthernet1"
             ```
         """
-        query = {**value}
-        status = query.pop("status", None)
-        if status is None:
-            for key in list(query.keys()):
-                if key.startswith("status__"):
-                    status_lookup = key[len("status__") :]  # noqa: E203
-                    status = Status.objects.get(**{status_lookup: query.pop(key)})
-                    break
-        elif isinstance(status, dict):
-            status = Status.objects.get(**status)
-        elif hasattr(status, "instance"):
-            status = status.instance
-
-        if status is None:
-            raise DesignImplementationError("No status given for cable connection")
-
-        remote_instance = self.lookup(model_instance.model_class.objects, query)
+        if "to" not in value:
+            raise DesignImplementationError(
+                f"`connect_cable` must have a `to` field indicating what to terminate to. {value}"
+            )
+
+        cable_attributes = {**value}
+        termination_query = cable_attributes.pop("to")
+
+        # status = query.pop("status", None)
+        # if status is None:
+        #     for key in list(query.keys()):
+        #         if key.startswith("status__"):
+        #             status_lookup = key[len("status__") :]  # noqa: E203
+        #             status = Status.objects.get(**{status_lookup: query.pop(key)})
+        #             break
+        # elif isinstance(status, dict):
+        #     status = Status.objects.get(**status)
+        # elif hasattr(status, "instance"):
+        #     status = status.instance
+
+        # if status is None:
+        #     raise DesignImplementationError("No status given for cable connection")
+
+        remote_instance = self.lookup(model_instance.model_class.objects, termination_query)
+        cable_attributes.update(
+            {
+                "termination_a": model_instance,
+                "!create_or_update:termination_b_type": ContentType.objects.get_for_model(remote_instance),
+                "!create_or_update:termination_b_id": remote_instance.id,
+            }
+        )
+
         model_instance.deferred.append("cable")
         model_instance.deferred_attributes["cable"] = [
             model_instance.__class__(
-                self.object_creator,
+                self.builder,
                 model_class=Cable,
-                attributes={
-                    "status": status,
-                    "termination_a": model_instance,
-                    "!create_or_update:termination_b_type": ContentType.objects.get_for_model(remote_instance),
-                    "!create_or_update:termination_b_id": remote_instance.id,
-                },
+                attributes=cable_attributes,
             )
         ]
 
@@ -276,7 +291,8 @@ def attribute(self, value: dict, model_instance) -> None:
         prefixes = Prefix.objects.filter(query)
         return "prefix", self._get_next(prefixes, length)
 
-    def _get_next(self, prefixes, length) -> str:  # pylint:disable=no-self-use
+    @staticmethod
+    def _get_next(prefixes, length) -> str:
         """Return the next available prefix from a parent prefix.
 
         Args:
@@ -364,7 +380,7 @@ class BGPPeeringExtension(Extension):
 
     attribute_tag = "bgp_peering"
 
-    def __init__(self, object_creator: Builder):
+    def __init__(self, builder: Builder):
         """Initialize the BGPPeeringExtension.
 
         This initializer will import the necessary BGP models. If the
@@ -373,7 +389,7 @@ def __init__(self, object_creator: Builder):
         Raises:
             DesignImplementationError: Raised when the BGP Models Plugin is not installed.
         """
-        super().__init__(object_creator)
+        super().__init__(builder)
         try:
             from nautobot_bgp_models.models import PeerEndpoint, Peering  # pylint:disable=import-outside-toplevel
 
@@ -440,8 +456,8 @@ def attribute(self, value, model_instance) -> None:
         # copy the value so it won't be modified in later
         # use
         retval = {**value}
-        endpoint_a = ModelInstance(self.object_creator, self.PeerEndpoint, retval.pop("endpoint_a"))
-        endpoint_z = ModelInstance(self.object_creator, self.PeerEndpoint, retval.pop("endpoint_z"))
+        endpoint_a = ModelInstance(self.builder, self.PeerEndpoint, retval.pop("endpoint_a"))
+        endpoint_z = ModelInstance(self.builder, self.PeerEndpoint, retval.pop("endpoint_z"))
         peering_a = None
         peering_z = None
         try:

design_builder/contrib/tests/test_ext.py

@@ -88,8 +88,9 @@ def test_connect_cable(self):
                   status__name: "Active"
                   "!connect_cable":
                     status__name: "Planned"
-                    device: "!ref:device1"
-                    name: "GigabitEthernet1"
+                    to:
+                      device: "!ref:device1"
+                      name: "GigabitEthernet1"
         """
 
         design_template_v2 = """
@@ -134,8 +135,9 @@ def test_connect_cable(self):
                   status__name: "Active"
                   "!connect_cable":
                     status__name: "Planned"
-                    device: "!ref:device1"
-                    name: "GigabitEthernet1"
+                    to:
+                      device: "!ref:device1"
+                      name: "GigabitEthernet1"
         """
 
         if nautobot_version < "2.0.0":
@@ -330,6 +332,7 @@ def test_creation(self):
           bgp_routing_instances:
           - "!create_or_update:autonomous_system__asn": 64500
             "!ref": "device1-instance"
+            "status__name": "Active"
 
         - "!create_or_update:name": "device2"
           status__name: "Active"
@@ -346,6 +349,7 @@ def test_creation(self):
           bgp_routing_instances:
           - "!create_or_update:autonomous_system__asn": 64500
             "!ref": "device2-instance"
+            "status__name": "Active"
 
         bgp_peerings:
         - "!bgp_peering":

design_builder/design.py

@@ -471,7 +471,7 @@ def implement_design(self, design, commit=False):
             self.roll_back()
             raise ex
 
-    def resolve_value(self, value):
+    def resolve_value(self, value, unwrap_model_instance=False):
         """Resolve a value using extensions, if needed."""
         if isinstance(value, str) and value.startswith("!"):
             (action, arg) = value.lstrip("!").split(":", 1)
@@ -480,9 +480,11 @@ def resolve_value(self, value):
                 value = extn.value(arg)
             else:
                 raise errors.DesignImplementationError(f"Unknown attribute extension {value}")
+        if unwrap_model_instance and isinstance(value, ModelInstance):
+            value = value.instance
         return value
 
-    def resolve_values(self, value):
+    def resolve_values(self, value, unwrap_model_instances=False):
         """Resolve a value, or values, using extensions.
 
         Args:
@@ -492,13 +494,13 @@ def resolve_values(self, value):
             Any: The resolved value.
         """
         if isinstance(value, str):
-            value = self.resolve_value(value)
+            value = self.resolve_value(value, unwrap_model_instances)
         elif isinstance(value, list):
             for i, item in enumerate(value):
-                value[i] = self.resolve_value(item)
+                value[i] = self.resolve_value(item, unwrap_model_instances)
         elif isinstance(value, dict):
             for k, item in value.items():
-                value[k] = self.resolve_value(item)
+                value[k] = self.resolve_value(item, unwrap_model_instances)
         return value
 
     def _create_objects(self, model_cls, objects):

design_builder/errors.py

@@ -3,6 +3,7 @@
 from inspect import isclass
 
 from django.core.exceptions import ValidationError
+from django.db.models import Model
 
 
 def _error_msg(validation_error):
@@ -33,24 +34,35 @@ def __init__(self, message, model=None):
 class DesignModelError(Exception):
     """Parent class for all model related design errors."""
 
-    def __init__(self, model=None) -> None:
+    def __init__(self, model=None, parent=None) -> None:
         """Initialize a DesignError with optional model_stack.
 
         Args:
             model: The model that generated the error.
+            parent: If model is a django model (as opposed to a design
+            builder ModelInstance) then a parent can be specified
+            in order to better represent the relationship of the
+            model within the design.
         """
         super().__init__()
         self.model = model
+        self.parent = parent
 
     @staticmethod
     def _model_str(model):
         instance_str = None
-        if not hasattr(model, "instance"):
+        if not isinstance(model, Model) and not hasattr(model, "instance"):
             return str(model)
 
-        if model.instance:
-            instance_str = str(model.instance)
-        model_str = model.model_class._meta.verbose_name.capitalize()
+        model_class = model.__class__
+        # if it looks like a duck...
+        if hasattr(model, "instance"):
+            model_class = model.model_class
+            model = model.instance
+
+        if model:
+            instance_str = str(model)
+        model_str = model_class._meta.verbose_name.capitalize()
         if instance_str:
             model_str = f"{model_str} {instance_str}"
         return model_str
@@ -83,6 +95,9 @@ def path_str(self):
             path_msg.insert(0, DesignModelError._model_str(model))
             if hasattr(model, "parent"):
                 model = model.parent
+            elif self.parent:
+                model = self.parent
+                self.parent = None
             else:
                 model = None
         # don't include the top level model in the ancestry
@@ -137,6 +152,16 @@ def __str__(self) -> str:
 class DesignQueryError(DesignModelError):
     """Exception indicating design builder could not find the object."""
 
+    def __init__(self, model=None, query_filter=None, **kwargs):
+        """Initialize a design query error.
+
+        Args:
+            model: Model or model class this query error corresponds to.
+            query_filter: Query filter the generated the error.
+        """
+        super().__init__(model=model, **kwargs)
+        self.query_filter = query_filter
+
     def __str__(self) -> str:
         """The string representation of an object of the DoesNotExistError class."""
         msg = []
@@ -146,6 +171,8 @@ def __str__(self) -> str:
         msg.append(f"{indentation}- {self.model_str}:")
         if hasattr(self.model, "query_filter"):
             msg.append(DesignModelError._object_to_markdown(self.model.query_filter, indentation=f"{indentation}    "))
+        elif self.query_filter:
+            msg.append(DesignModelError._object_to_markdown(self.query_filter, indentation=f"{indentation}    "))
         else:
             msg.append(DesignModelError._object_to_markdown(self.model.filter, indentation=f"{indentation}    "))
         return "\n".join(msg)