#9102: Enable creating terminations in conjunction with cables via REST API

docs/release-notes/version-3.3.md

@@ -6,12 +6,12 @@
 
 * Device position and rack unit values are now reported as decimals (e.g. `1.0` or `1.5`) to support modeling half-height rack units.
 * The `nat_outside` relation on the IP address model now returns a list of zero or more related IP addresses, rather than a single instance (or None).
-* Several fields on the cable API serializers have been altered to support multiple-object cable terminations:
+* Several fields on the cable API serializers have been altered or removed to support multiple-object cable terminations:
 
 | Old Name             | Old Type | New Name              | New Type |
 |----------------------|----------|-----------------------|----------|
-| `termination_a_type` | string   | `a_terminations_type` | string   |
-| `termination_b_type` | string   | `b_terminations_type` | string   |
+| `termination_a_type` | string   | _Removed_             | -        |
+| `termination_b_type` | string   | _Removed_             | -        |
 | `termination_a_id`   | integer  | _Removed_             | -        |
 | `termination_b_id`   | integer  | _Removed_             | -        |
 | `termination_a`      | object   | `a_terminations`      | list     |

netbox/dcim/api/serializers.py

@@ -15,7 +15,8 @@ from ipam.api.nested_serializers import (
 from ipam.models import ASN, VLAN
 from netbox.api import ChoiceField, ContentTypeField, SerializedPKRelatedField
 from netbox.api.serializers import (
-    NestedGroupModelSerializer, NetBoxModelSerializer, ValidatedModelSerializer, WritableNestedSerializer,
+    GenericObjectSerializer, NestedGroupModelSerializer, NetBoxModelSerializer, ValidatedModelSerializer,
+    WritableNestedSerializer,
 )
 from netbox.config import ConfigItem
 from tenancy.api.nested_serializers import NestedTenantSerializer
@@ -994,10 +995,8 @@ class InventoryItemRoleSerializer(NetBoxModelSerializer):
 
 class CableSerializer(NetBoxModelSerializer):
     url = serializers.HyperlinkedIdentityField(view_name='dcim-api:cable-detail')
-    a_terminations_type = serializers.SerializerMethodField(read_only=True)
-    b_terminations_type = serializers.SerializerMethodField(read_only=True)
-    a_terminations = serializers.SerializerMethodField(read_only=True)
-    b_terminations = serializers.SerializerMethodField(read_only=True)
+    a_terminations = GenericObjectSerializer(many=True, required=False)
+    b_terminations = GenericObjectSerializer(many=True, required=False)
     status = ChoiceField(choices=LinkStatusChoices, required=False)
     tenant = NestedTenantSerializer(required=False, allow_null=True)
     length_unit = ChoiceField(choices=CableLengthUnitChoices, allow_blank=True, required=False)
@@ -1005,47 +1004,10 @@ class CableSerializer(NetBoxModelSerializer):
     class Meta:
         model = Cable
         fields = [
-            'id', 'url', 'display', 'type', 'a_terminations_type', 'a_terminations', 'b_terminations_type',
-            'b_terminations', 'status', 'tenant', 'label', 'color', 'length', 'length_unit', 'tags', 'custom_fields',
-            'created', 'last_updated',
+            'id', 'url', 'display', 'type', 'a_terminations', 'b_terminations', 'status', 'tenant', 'label', 'color',
+            'length', 'length_unit', 'tags', 'custom_fields', 'created', 'last_updated',
         ]
 
-    def _get_terminations_type(self, obj, side):
-        assert side in CableEndChoices.values()
-        terms = getattr(obj, f'get_{side.lower()}_terminations')()
-        if terms:
-            ct = ContentType.objects.get_for_model(terms[0])
-            return f"{ct.app_label}.{ct.model}"
-
-    def _get_terminations(self, obj, side):
-        assert side in CableEndChoices.values()
-        terms = getattr(obj, f'get_{side.lower()}_terminations')()
-        if not terms:
-            return []
-
-        termination_type = ContentType.objects.get_for_model(terms[0])
-        serializer = get_serializer_for_model(termination_type.model_class(), prefix='Nested')
-        context = {'request': self.context['request']}
-        data = serializer(terms, context=context, many=True).data
-
-        return data
-
-    @swagger_serializer_method(serializer_or_field=serializers.CharField)
-    def get_a_terminations_type(self, obj):
-        return self._get_terminations_type(obj, CableEndChoices.SIDE_A)
-
-    @swagger_serializer_method(serializer_or_field=serializers.CharField)
-    def get_b_terminations_type(self, obj):
-        return self._get_terminations_type(obj, CableEndChoices.SIDE_B)
-
-    @swagger_serializer_method(serializer_or_field=serializers.DictField)
-    def get_a_terminations(self, obj):
-        return self._get_terminations(obj, CableEndChoices.SIDE_A)
-
-    @swagger_serializer_method(serializer_or_field=serializers.DictField)
-    def get_b_terminations(self, obj):
-        return self._get_terminations(obj, CableEndChoices.SIDE_B)
-
 
 class TracedCableSerializer(serializers.ModelSerializer):
     """

netbox/dcim/forms/bulk_import.py

@@ -955,7 +955,7 @@ class CableCSVForm(NetBoxModelCSVForm):
         except ObjectDoesNotExist:
             raise forms.ValidationError(f"{side.upper()} side termination not found: {device} {name}")
 
-        setattr(self.instance, f'termination_{side}', termination_object)
+        setattr(self.instance, f'{side}_terminations', [termination_object])
         return termination_object
 
     def clean_side_a_name(self):

netbox/dcim/forms/connections.py

@@ -157,8 +157,8 @@ def get_cable_form(a_type, b_type):
 
             if self.instance and self.instance.pk:
                 # Initialize A/B terminations when modifying an existing Cable instance
-                self.initial['a_terminations'] = self.instance.get_a_terminations()
-                self.initial['b_terminations'] = self.instance.get_b_terminations()
+                self.initial['a_terminations'] = self.instance.a_terminations
+                self.initial['b_terminations'] = self.instance.b_terminations
 
         def save(self, *args, **kwargs):
 

netbox/dcim/models/cables.py

@@ -93,11 +93,12 @@ class Cable(NetBoxModel):
         # Cache the original status so we can check later if it's been changed
         self._orig_status = self.status
 
-        # Assign any *new* CableTerminations for the instance. These will replace any existing
-        # terminations on save().
-        if a_terminations is not None:
+        self._terminations_modified = False
+
+        # Assign or retrieve A/B terminations
+        if a_terminations:
             self.a_terminations = a_terminations
-        if b_terminations is not None:
+        if b_terminations:
             self.b_terminations = b_terminations
 
     def __str__(self):
@@ -107,6 +108,34 @@ class Cable(NetBoxModel):
     def get_absolute_url(self):
         return reverse('dcim:cable', args=[self.pk])
 
+    @property
+    def a_terminations(self):
+        if hasattr(self, '_a_terminations'):
+            return self._a_terminations
+        # Query self.terminations.all() to leverage cached results
+        return [
+            ct.termination for ct in self.terminations.all() if ct.cable_end == CableEndChoices.SIDE_A
+        ]
+
+    @a_terminations.setter
+    def a_terminations(self, value):
+        self._terminations_modified = True
+        self._a_terminations = value
+
+    @property
+    def b_terminations(self):
+        if hasattr(self, '_b_terminations'):
+            return self._b_terminations
+        # Query self.terminations.all() to leverage cached results
+        return [
+            ct.termination for ct in self.terminations.all() if ct.cable_end == CableEndChoices.SIDE_B
+        ]
+
+    @b_terminations.setter
+    def b_terminations(self, value):
+        self._terminations_modified = True
+        self._b_terminations = value
+
     def clean(self):
         super().clean()
 
@@ -116,30 +145,28 @@ class Cable(NetBoxModel):
         elif self.length is None:
             self.length_unit = ''
 
-        a_terminations = [
-            CableTermination(cable=self, cable_end='A', termination=t)
-            for t in getattr(self, 'a_terminations', [])
-        ]
-        b_terminations = [
-            CableTermination(cable=self, cable_end='B', termination=t)
-            for t in getattr(self, 'b_terminations', [])
-        ]
+        if self.pk is None and (not self.a_terminations or not self.b_terminations):
+            raise ValidationError("Must define A and B terminations when creating a new cable.")
 
-        # Check that all termination objects for either end are of the same type
-        for terms in (a_terminations, b_terminations):
-            if len(terms) > 1 and not all(t.termination_type == terms[0].termination_type for t in terms[1:]):
-                raise ValidationError("Cannot connect different termination types to same end of cable.")
+        if self._terminations_modified:
 
-        # Check that termination types are compatible
-        if a_terminations and b_terminations:
-            a_type = a_terminations[0].termination_type.model
-            b_type = b_terminations[0].termination_type.model
-            if b_type not in COMPATIBLE_TERMINATION_TYPES.get(a_type):
-                raise ValidationError(f"Incompatible termination types: {a_type} and {b_type}")
+            # Check that all termination objects for either end are of the same type
+            for terms in (self.a_terminations, self.b_terminations):
+                if len(terms) > 1 and not all(isinstance(t, type(terms[0])) for t in terms[1:]):
+                    raise ValidationError("Cannot connect different termination types to same end of cable.")
 
-        # Run clean() on any new CableTerminations
-        for cabletermination in [*a_terminations, *b_terminations]:
-            cabletermination.clean()
+            # Check that termination types are compatible
+            if self.a_terminations and self.b_terminations:
+                a_type = self.a_terminations[0]._meta.model_name
+                b_type = self.b_terminations[0]._meta.model_name
+                if b_type not in COMPATIBLE_TERMINATION_TYPES.get(a_type):
+                    raise ValidationError(f"Incompatible termination types: {a_type} and {b_type}")
+
+            # Run clean() on any new CableTerminations
+            for termination in self.a_terminations:
+                CableTermination(cable=self, cable_end='A', termination=termination).clean()
+            for termination in self.b_terminations:
+                CableTermination(cable=self, cable_end='B', termination=termination).clean()
 
     def save(self, *args, **kwargs):
         _created = self.pk is None
@@ -160,23 +187,21 @@ class Cable(NetBoxModel):
         b_terminations = {ct.termination: ct for ct in self.terminations.filter(cable_end='B')}
 
         # Delete stale CableTerminations
-        if hasattr(self, 'a_terminations'):
+        if self._terminations_modified:
             for termination, ct in a_terminations.items():
-                if termination not in self.a_terminations:
+                if termination.pk and termination not in self.a_terminations:
                     ct.delete()
-        if hasattr(self, 'b_terminations'):
             for termination, ct in b_terminations.items():
-                if termination not in self.b_terminations:
+                if termination.pk and termination not in self.b_terminations:
                     ct.delete()
 
         # Save new CableTerminations (if any)
-        if hasattr(self, 'a_terminations'):
+        if self._terminations_modified:
             for termination in self.a_terminations:
-                if termination not in a_terminations:
+                if not termination.pk or termination not in a_terminations:
                     CableTermination(cable=self, cable_end='A', termination=termination).save()
-        if hasattr(self, 'b_terminations'):
             for termination in self.b_terminations:
-                if termination not in b_terminations:
+                if not termination.pk or termination not in b_terminations:
                     CableTermination(cable=self, cable_end='B', termination=termination).save()
 
         trace_paths.send(Cable, instance=self, created=_created)
@@ -184,18 +209,6 @@ class Cable(NetBoxModel):
     def get_status_color(self):
         return LinkStatusChoices.colors.get(self.status)
 
-    def get_a_terminations(self):
-        # Query self.terminations.all() to leverage cached results
-        return [
-            ct.termination for ct in self.terminations.all() if ct.cable_end == CableEndChoices.SIDE_A
-        ]
-
-    def get_b_terminations(self):
-        # Query self.terminations.all() to leverage cached results
-        return [
-            ct.termination for ct in self.terminations.all() if ct.cable_end == CableEndChoices.SIDE_B
-        ]
-
 
 class CableTermination(models.Model):
     """

netbox/dcim/signals.py

@@ -79,7 +79,7 @@ def update_connected_endpoints(instance, created, raw=False, **kwargs):
         return
 
     # Update cable paths if new terminations have been set
-    if hasattr(instance, 'a_terminations') or hasattr(instance, 'b_terminations'):
+    if instance._terminations_modified:
         a_terminations = []
         b_terminations = []
         for t in instance.terminations.all():

netbox/dcim/tests/test_api.py

@@ -7,6 +7,7 @@ from dcim.choices import *
 from dcim.constants import *
 from dcim.models import *
 from ipam.models import ASN, RIR, VLAN, VRF
+from netbox.api.serializers import GenericObjectSerializer
 from utilities.testing import APITestCase, APIViewTestCases, create_test_device
 from virtualization.models import Cluster, ClusterType
 from wireless.choices import WirelessChannelChoices
@@ -1864,6 +1865,17 @@ class CableTest(APIViewTestCases.APIViewTestCase):
     # TODO: Allow updating cable terminations
     test_update_object = None
 
+    def model_to_dict(self, *args, **kwargs):
+        data = super().model_to_dict(*args, **kwargs)
+
+        # Serialize termination objects
+        if 'a_terminations' in data:
+            data['a_terminations'] = GenericObjectSerializer(data['a_terminations'], many=True).data
+        if 'b_terminations' in data:
+            data['b_terminations'] = GenericObjectSerializer(data['b_terminations'], many=True).data
+
+        return data
+
     @classmethod
     def setUpTestData(cls):
         site = Site.objects.create(name='Site 1', slug='site-1')
@@ -1893,24 +1905,36 @@ class CableTest(APIViewTestCases.APIViewTestCase):
 
         cls.create_data = [
             {
-                'a_terminations_type': 'dcim.interface',
-                'a_terminations': [interfaces[4].pk],
-                'b_terminations_type': 'dcim.interface',
-                'b_terminations': [interfaces[14].pk],
+                'a_terminations': [{
+                    'object_type': 'dcim.interface',
+                    'object_id': interfaces[4].pk,
+                }],
+                'b_terminations': [{
+                    'object_type': 'dcim.interface',
+                    'object_id': interfaces[14].pk,
+                }],
                 'label': 'Cable 4',
             },
             {
-                'a_terminations_type': 'dcim.interface',
-                'a_terminations': [interfaces[5].pk],
-                'b_terminations_type': 'dcim.interface',
-                'b_terminations': [interfaces[15].pk],
+                'a_terminations': [{
+                    'object_type': 'dcim.interface',
+                    'object_id': interfaces[5].pk,
+                }],
+                'b_terminations': [{
+                    'object_type': 'dcim.interface',
+                    'object_id': interfaces[15].pk,
+                }],
                 'label': 'Cable 5',
             },
             {
-                'a_terminations_type': 'dcim.interface',
-                'a_terminations': [interfaces[6].pk],
-                'b_terminations_type': 'dcim.interface',
-                'b_terminations': [interfaces[16].pk],
+                'a_terminations': [{
+                    'object_type': 'dcim.interface',
+                    'object_id': interfaces[6].pk,
+                }],
+                'b_terminations': [{
+                    'object_type': 'dcim.interface',
+                    'object_id': interfaces[16].pk,
+                }],
                 'label': 'Cable 6',
             },
         ]

netbox/dcim/tests/test_views.py

@@ -12,6 +12,7 @@ from dcim.choices import *
 from dcim.constants import *
 from dcim.models import *
 from ipam.models import ASN, RIR, VLAN, VRF
+from netbox.api.serializers import GenericObjectSerializer
 from tenancy.models import Tenant
 from utilities.testing import ViewTestCases, create_tags, create_test_device, post_data
 from wireless.models import WirelessLAN
@@ -2640,8 +2641,8 @@ class CableTestCase(
         cls.form_data = {
             # TODO: Revisit this limitation
             # Changing terminations not supported when editing an existing Cable
-            'a_terminations': interfaces[0].pk,
-            'b_terminations': interfaces[3].pk,
+            'a_terminations': [interfaces[0].pk],
+            'b_terminations': [interfaces[3].pk],
             'type': CableTypeChoices.TYPE_CAT6,
             'status': LinkStatusChoices.STATUS_PLANNED,
             'label': 'Label',
@@ -2667,6 +2668,17 @@ class CableTestCase(
             'length_unit': CableLengthUnitChoices.UNIT_METER,
         }
 
+    def model_to_dict(self, *args, **kwargs):
+        data = super().model_to_dict(*args, **kwargs)
+
+        # Serialize termination objects
+        if 'a_terminations' in data:
+            data['a_terminations'] = [obj.pk for obj in data['a_terminations']]
+        if 'b_terminations' in data:
+            data['b_terminations'] = [obj.pk for obj in data['b_terminations']]
+
+        return data
+
 
 class VirtualChassisTestCase(ViewTestCases.PrimaryObjectViewTestCase):
     model = VirtualChassis

netbox/templates/dcim/cable.html

@@ -63,13 +63,13 @@
       <div class="card">
         <h5 class="card-header">Termination A</h5>
         <div class="card-body">
-          {% include 'dcim/inc/cable_termination.html' with terminations=object.get_a_terminations %}
+          {% include 'dcim/inc/cable_termination.html' with terminations=object.a_terminations %}
         </div>
       </div>
       <div class="card">
         <h5 class="card-header">Termination B</h5>
         <div class="card-body">
-          {% include 'dcim/inc/cable_termination.html' with terminations=object.get_b_terminations %}
+          {% include 'dcim/inc/cable_termination.html' with terminations=object.b_terminations %}
         </div>
       </div>
       {% plugin_right_page object %}