From e4f1aab0442b68fdaf617cc1456a04a7d0376249 Mon Sep 17 00:00:00 2001 From: Sander Steffann Date: Sun, 20 Oct 2019 19:16:32 +0200 Subject: [PATCH] Just store contenttype and pk, don't cache attributes Otherwise we'll have to update the trace cache way too often --- netbox/circuits/models.py | 44 ++++++--- netbox/circuits/signals.py | 5 + .../0077_migrate_connected_endpoint.py | 46 +-------- netbox/dcim/models.py | 96 +++++++++++-------- netbox/dcim/signals.py | 2 +- 5 files changed, 99 insertions(+), 94 deletions(-) diff --git a/netbox/circuits/models.py b/netbox/circuits/models.py index 8b4e972ae..af89e496c 100644 --- a/netbox/circuits/models.py +++ b/netbox/circuits/models.py @@ -7,7 +7,7 @@ from taggit.managers import TaggableManager from dcim.constants import CONNECTION_STATUS_CHOICES, STATUS_CLASSES, CABLE_TERMINATION_TYPES from dcim.fields import ASNField -from dcim.models import CableTermination +from dcim.models import CableTermination, CachedTraceModel from extras.models import CustomFieldModel, ObjectChange, TaggedItem from utilities.models import ChangeLoggedModel from utilities.utils import serialize_object @@ -213,8 +213,37 @@ class Circuit(ChangeLoggedModel, CustomFieldModel): def termination_z(self): return self._get_termination('Z') + def get_related_endpoints(self): + """ + Traverse both ends of a circuit and return a list of all related endpoints. + """ + from dcim.models import FrontPort, RearPort -class CircuitTermination(CableTermination): + # Termination points trace from themselves, through the circuit and beyond. Tracing from the Z termination + # therefore traces in the direction of A [(termination_z, circuit, termination_a), (...)] and vice versa. + # Every path therefore also has at least one segment (the current circuit). + terminations = [self.termination_a, self.termination_z] + paths = [termination.trace() for termination in terminations if termination] + + # Use a dict here to avoid storing duplicates. The same object retrieved twice will have different identities. + endpoints = {} + while paths: + path = paths.pop() + for left, cable, right in path: + if right is not None: + key = '{cls}-{pk}'.format(cls=right.__class__.__name__, pk=right.pk) + endpoints[key] = right + + # If a path ends in a RearPort, then everything connected through its FrontPorts is related as well + if isinstance(path[-1][2], RearPort): + front_ports = FrontPort.objects.filter(rear_port=path[-1][2]) + for front_port in front_ports: + paths.append(front_port.trace()) + + return list(endpoints.values()) + + +class CircuitTermination(CableTermination, CachedTraceModel): circuit = models.ForeignKey( to='circuits.Circuit', on_delete=models.CASCADE, @@ -246,9 +275,6 @@ class CircuitTermination(CableTermination): ct_field='connected_endpoint_type', fk_field='connected_endpoint_id' ) - _trace = JSONField( - default=list - ) connection_status = models.NullBooleanField( choices=CONNECTION_STATUS_CHOICES, @@ -314,11 +340,3 @@ class CircuitTermination(CableTermination): def get_peer_port(self): return self.get_peer_termination() - - def get_endpoint_attributes(self): - return { - **super().get_endpoint_attributes(), - 'cid': self.circuit.cid, - 'provider': self.circuit.provider.name, - 'site': self.site.name, - } diff --git a/netbox/circuits/signals.py b/netbox/circuits/signals.py index 86db21400..c19e23577 100644 --- a/netbox/circuits/signals.py +++ b/netbox/circuits/signals.py @@ -2,6 +2,7 @@ from django.db.models.signals import post_delete, post_save from django.dispatch import receiver from django.utils import timezone +from dcim.signals import update_endpoints from .models import Circuit, CircuitTermination @@ -15,3 +16,7 @@ def update_circuit(instance, **kwargs): for circuit in circuits: circuit.last_updated = time circuit.save() + + # Update all endpoints affected by this cable + endpoints = instance.circuit.get_related_endpoints() + update_endpoints(endpoints) diff --git a/netbox/dcim/migrations/0077_migrate_connected_endpoint.py b/netbox/dcim/migrations/0077_migrate_connected_endpoint.py index 6dd5d9945..9a2a60870 100644 --- a/netbox/dcim/migrations/0077_migrate_connected_endpoint.py +++ b/netbox/dcim/migrations/0077_migrate_connected_endpoint.py @@ -1,6 +1,7 @@ # Generated by Django 2.2.5 on 2019-10-06 19:01 from itertools import chain +from django.contrib.contenttypes.models import ContentType from django.db import migrations @@ -86,46 +87,6 @@ def migration_trace(apps, endpoint, cable_history=None): return path -def migration_get_endpoint_attributes(endpoint): - """ - This is a stand-alone version of CableTermination.get_endpoint_attributes() that is safe to use in migrations. - """ - - if not endpoint: - return {} - - # We can't use isinstance because migrations give us fake classes - attributes = { - 'id': endpoint.pk, - 'type': endpoint.__class__.__name__, - } - - if endpoint.__class__.__name__ == 'CircuitTermination': - attributes['cid'] = endpoint.circuit.cid - attributes['provider'] = endpoint.circuit.provider.name - attributes['site'] = endpoint.site.name - attributes['site_slug'] = endpoint.site.slug - elif endpoint.__class__.__name__ in ('Interface', 'FrontPort', 'RearPort'): - attributes['name'] = endpoint.name - - parent = endpoint.device or endpoint.virtual_machine - if parent.name: - attributes['device'] = parent.name - attributes['device_id'] = parent.pk - elif parent.virtual_chassis and parent.virtual_chassis.master.name: - attributes['device'] = "{}:{}".format(parent.virtual_chassis.master, parent.vc_position) - attributes['device_id'] = parent.virtual_chassis.master.pk - elif hasattr(parent, 'device_type'): - attributes['device'] = "{}".format(parent.device_type) - attributes['device_id'] = parent.pk - else: - attributes['device'] = "" - - attributes['site'] = parent.site.name - - return attributes - - def to_generic_connected_endpoint(apps, schema_editor): print("\nReconstructing all endpoints...", end='') @@ -156,7 +117,10 @@ def to_generic_connected_endpoint(apps, schema_editor): endpoint.connected_endpoint_type = None endpoint.connected_endpoint_id = None - endpoint._trace = [migration_get_endpoint_attributes(endpoint) for endpoint in endpoints] + endpoint._trace = [] + for step in endpoints[:-1]: + endpoint_contenttype = ContentType.objects.get_for_model(step) + endpoint._trace.append((endpoint_contenttype.natural_key(), step.pk)) endpoint.save() print(".", end='', flush=True) diff --git a/netbox/dcim/models.py b/netbox/dcim/models.py index 9ba75e670..02039f805 100644 --- a/netbox/dcim/models.py +++ b/netbox/dcim/models.py @@ -26,6 +26,62 @@ from .fields import ASNField, MACAddressField from .managers import InterfaceManager +class CachedTraceModel(models.Model): + _trace = JSONField( + default=list + ) + + class Meta: + abstract = True + + @property + def via_endpoints(self): + # Cache the result as we'll be iterating over the resulting list multiple times + if hasattr(self, '__cached_via_endpoints'): + return self.__cached_via_endpoints[:] + + # Collect all the primary keys per model + fetch_list = {} + for model_key, key in self._trace: + fetch_list.setdefault(tuple(model_key), []).append(key) + + endpoints = {} + for model_key, keys in fetch_list.items(): + endpoint_contenttype = ContentType.objects.get_by_natural_key(*model_key) + queryset = endpoint_contenttype.model_class().objects + if hasattr(queryset.model, 'circuit'): + queryset = queryset.select_related('circuit__provider') + if hasattr(queryset.model, 'device'): + queryset = queryset.select_related('device') + + endpoints[model_key] = { + endpoint.pk: endpoint + for endpoint in queryset.filter(pk__in=keys) + } + + trace = [] + for model_key, key in self._trace: + endpoint = endpoints[tuple(model_key)].get(key, None) + if endpoint: + trace.append(endpoint) + + self.__cached_via_endpoints = trace + return trace[:] + + @via_endpoints.setter + def via_endpoints(self, endpoints): + # Invalidate the cache + if hasattr(self, '__cached_via_endpoints'): + del self.__cached_via_endpoints + + trace = [] + for step in endpoints: + endpoint_contenttype = ContentType.objects.get_for_model(step) + trace.append((endpoint_contenttype.natural_key(), step.pk)) + + self._trace = trace + + class ComponentTemplateModel(models.Model): class Meta: @@ -160,11 +216,6 @@ class CableTermination(models.Model): if self._cabled_as_b.exists(): return self.cable.termination_a - def get_endpoint_attributes(self): - return { - 'id': self.pk, - 'type': self.__class__.__name__, - } # # Regions @@ -2134,7 +2185,7 @@ class PowerOutlet(CableTermination, ComponentModel): # Interfaces # -class Interface(CableTermination, ComponentModel): +class Interface(CableTermination, ComponentModel, CachedTraceModel): """ A network interface within a Device or VirtualMachine. A physical Interface can connect to exactly one other Interface. @@ -2173,9 +2224,6 @@ class Interface(CableTermination, ComponentModel): ct_field='connected_endpoint_type', fk_field='connected_endpoint_id' ) - _trace = JSONField( - default=list - ) connected_interface = GenericRelation( to='self', @@ -2390,16 +2438,6 @@ class Interface(CableTermination, ComponentModel): def count_ipaddresses(self): return self.ip_addresses.count() - def get_endpoint_attributes(self): - return { - **super().get_endpoint_attributes(), - 'name': self.name, - 'device': self.parent.display_name, - 'device_id': self.parent.pk, - 'site': self.parent.site.name, - 'site_slug': self.parent.site.slug, - } - # # Pass-through ports @@ -2474,16 +2512,6 @@ class FrontPort(CableTermination, ComponentModel): def get_peer_port(self): return self.rear_port - def get_endpoint_attributes(self): - return { - **super().get_endpoint_attributes(), - 'name': self.name, - 'device': self.parent.display_name, - 'device_id': self.parent.pk, - 'site': self.parent.site.name, - 'site_slug': self.parent.site.slug, - } - class RearPort(CableTermination, ComponentModel): """ @@ -2541,16 +2569,6 @@ class RearPort(CableTermination, ComponentModel): except ObjectDoesNotExist: return None - def get_endpoint_attributes(self): - return { - **super().get_endpoint_attributes(), - 'name': self.name, - 'device': self.parent.display_name, - 'device_id': self.parent.pk, - 'site': self.parent.site.name, - 'site_slug': self.parent.site.slug, - } - # # Device bays diff --git a/netbox/dcim/signals.py b/netbox/dcim/signals.py index 86d820ef2..0d27ffbe7 100644 --- a/netbox/dcim/signals.py +++ b/netbox/dcim/signals.py @@ -91,5 +91,5 @@ def update_endpoints(endpoints, without_cable=None): ][1:] endpoint.connected_endpoint = endpoints[-1] if endpoints else None - endpoint._trace = [endpoint.get_endpoint_attributes() for endpoint in endpoints] + endpoint.via_endpoints = endpoints[:-1] endpoint.save()