diff --git a/docs/release-notes/version-3.3.md b/docs/release-notes/version-3.3.md index 3d514ea69..7a093a35b 100644 --- a/docs/release-notes/version-3.3.md +++ b/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 | diff --git a/netbox/dcim/api/serializers.py b/netbox/dcim/api/serializers.py index 08edd2820..66d28c3fb 100644 --- a/netbox/dcim/api/serializers.py +++ b/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): """ diff --git a/netbox/dcim/forms/bulk_import.py b/netbox/dcim/forms/bulk_import.py index d6ec0f6f4..f0fd9bf86 100644 --- a/netbox/dcim/forms/bulk_import.py +++ b/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): diff --git a/netbox/dcim/forms/connections.py b/netbox/dcim/forms/connections.py index f7bfa6431..1b393ec6e 100644 --- a/netbox/dcim/forms/connections.py +++ b/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): diff --git a/netbox/dcim/models/cables.py b/netbox/dcim/models/cables.py index 551521c26..b35864aa0 100644 --- a/netbox/dcim/models/cables.py +++ b/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): """ diff --git a/netbox/dcim/signals.py b/netbox/dcim/signals.py index 7cfdc823d..2293f8840 100644 --- a/netbox/dcim/signals.py +++ b/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(): diff --git a/netbox/dcim/tests/test_api.py b/netbox/dcim/tests/test_api.py index 1c6f45596..a78a98ae5 100644 --- a/netbox/dcim/tests/test_api.py +++ b/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', }, ] diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index c6a531a31..a25267166 100644 --- a/netbox/dcim/tests/test_views.py +++ b/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 diff --git a/netbox/templates/dcim/cable.html b/netbox/templates/dcim/cable.html index f557792c1..e032d7034 100644 --- a/netbox/templates/dcim/cable.html +++ b/netbox/templates/dcim/cable.html @@ -63,13 +63,13 @@