From 0d68d0c059f9bb162622861e7c0c482c8bc907ab Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 5 Aug 2020 15:55:47 -0400 Subject: [PATCH 1/5] Replace annotate_depth() with annotate_tree() --- netbox/ipam/querysets.py | 44 ++++++++++++++-------------------------- netbox/ipam/views.py | 13 +++--------- 2 files changed, 18 insertions(+), 39 deletions(-) diff --git a/netbox/ipam/querysets.py b/netbox/ipam/querysets.py index 6d2dc6f33..102954f13 100644 --- a/netbox/ipam/querysets.py +++ b/netbox/ipam/querysets.py @@ -3,34 +3,20 @@ from utilities.querysets import RestrictedQuerySet class PrefixQuerySet(RestrictedQuerySet): - def annotate_depth(self, limit=None): + def annotate_tree(self): """ - Iterate through a QuerySet of Prefixes and annotate the hierarchical level of each. While it would be preferable - to do this using .annotate() on the QuerySet to count the unique parents of each prefix, that approach introduces - performance issues at scale. - - Because we're adding a non-field attribute to the model, annotation must be made *after* any QuerySet - modifications. + Annotate the number of parent and child prefixes for each Prefix. Raw SQL is needed for these subqueries + because we need to cast NULL VRF values to integers for comparison. (NULL != NULL). """ - queryset = self - stack = [] - for p in queryset: - try: - prev_p = stack[-1] - except IndexError: - prev_p = None - if prev_p is not None: - while (p.prefix not in prev_p.prefix) or p.prefix == prev_p.prefix: - stack.pop() - try: - prev_p = stack[-1] - except IndexError: - prev_p = None - break - if prev_p is not None: - prev_p.has_children = True - stack.append(p) - p.depth = len(stack) - 1 - if limit is None: - return queryset - return list(filter(lambda p: p.depth <= limit, queryset)) + return self.extra( + select={ + 'parents': 'SELECT COUNT(U0."prefix") AS "c" ' + 'FROM "ipam_prefix" U0 ' + 'WHERE (U0."prefix" >> "ipam_prefix"."prefix" ' + 'AND COALESCE(U0."vrf_id", 0) = COALESCE("ipam_prefix"."vrf_id", 0))', + 'children': 'SELECT COUNT(U1."prefix") AS "c" ' + 'FROM "ipam_prefix" U1 ' + 'WHERE (U1."prefix" << "ipam_prefix"."prefix" ' + 'AND COALESCE(U1."vrf_id", 0) = COALESCE("ipam_prefix"."vrf_id", 0))', + } + ) diff --git a/netbox/ipam/views.py b/netbox/ipam/views.py index ec067e705..52db68127 100644 --- a/netbox/ipam/views.py +++ b/netbox/ipam/views.py @@ -221,9 +221,7 @@ class AggregateView(ObjectView): 'site', 'role' ).order_by( 'prefix' - ).annotate_depth( - limit=0 - ) + ).annotate_tree() # Add available prefixes to the table if requested if request.GET.get('show_available', 'true') == 'true': @@ -326,11 +324,6 @@ class PrefixListView(ObjectListView): table = tables.PrefixDetailTable template_name = 'ipam/prefix_list.html' - def alter_queryset(self, request): - # Show only top-level prefixes by default (unless searching) - limit = None if request.GET.get('expand') or request.GET.get('q') else 0 - return self.queryset.annotate_depth(limit=limit) - class PrefixView(ObjectView): queryset = Prefix.objects.prefetch_related('vrf', 'site__region', 'tenant__group', 'vlan__group', 'role') @@ -353,7 +346,7 @@ class PrefixView(ObjectView): prefix__net_contains=str(prefix.prefix) ).prefetch_related( 'site', 'role' - ).annotate_depth() + ).annotate_tree() parent_prefix_table = tables.PrefixTable(list(parent_prefixes), orderable=False) parent_prefix_table.exclude = ('vrf',) @@ -386,7 +379,7 @@ class PrefixPrefixesView(ObjectView): # Child prefixes table child_prefixes = prefix.get_child_prefixes().restrict(request.user, 'view').prefetch_related( 'site', 'vlan', 'role', - ).annotate_depth(limit=0) + ).annotate_tree() # Add available prefixes to the table if requested if child_prefixes and request.GET.get('show_available', 'true') == 'true': From e8df9be7020a9b781cc65e86e947fa9f6b9af4dc Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 5 Aug 2020 16:16:51 -0400 Subject: [PATCH 2/5] Add LTE and GTE filters for prefix mask length --- netbox/ipam/filters.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/netbox/ipam/filters.py b/netbox/ipam/filters.py index 0856c9f1c..79fc05334 100644 --- a/netbox/ipam/filters.py +++ b/netbox/ipam/filters.py @@ -144,8 +144,16 @@ class PrefixFilterSet(BaseFilterSet, TenancyFilterSet, CustomFieldFilterSet, Cre label='Prefixes which contain this prefix or IP', ) mask_length = django_filters.NumberFilter( - method='filter_mask_length', - label='Mask length', + field_name='prefix', + lookup_expr='net_mask_length' + ) + mask_length__gte = django_filters.NumberFilter( + field_name='prefix', + lookup_expr='net_mask_length__gte' + ) + mask_length__lte = django_filters.NumberFilter( + field_name='prefix', + lookup_expr='net_mask_length__lte' ) vrf_id = django_filters.ModelMultipleChoiceFilter( queryset=VRF.objects.all(), @@ -262,11 +270,6 @@ class PrefixFilterSet(BaseFilterSet, TenancyFilterSet, CustomFieldFilterSet, Cre except (AddrFormatError, ValueError): return queryset.none() - def filter_mask_length(self, queryset, name, value): - if not value: - return queryset - return queryset.filter(prefix__net_mask_length=value) - class IPAddressFilterSet(BaseFilterSet, TenancyFilterSet, CustomFieldFilterSet, CreatedUpdatedFilterSet): q = django_filters.CharFilter( From d384f25ec222776cdd06c6813fae832f7a705569 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 5 Aug 2020 16:48:45 -0400 Subject: [PATCH 3/5] Replace expand/collpase with max mask length for prefixes list --- netbox/ipam/forms.py | 4 ---- netbox/ipam/tables.py | 12 +++++++----- netbox/ipam/views.py | 2 +- netbox/templates/ipam/prefix_list.html | 15 +++++++++++++-- netbox/utilities/templatetags/helpers.py | 8 ++++++++ 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index ade7fc994..e14f61872 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -511,10 +511,6 @@ class PrefixFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm) choices=BOOLEAN_WITH_BLANK_CHOICES ) ) - expand = forms.BooleanField( - required=False, - label='Expand prefix hierarchy' - ) tag = TagFilterField(model) diff --git a/netbox/ipam/tables.py b/netbox/ipam/tables.py index d20ec1819..37fa1a9eb 100644 --- a/netbox/ipam/tables.py +++ b/netbox/ipam/tables.py @@ -39,10 +39,10 @@ ROLE_VLAN_COUNT = """ """ PREFIX_LINK = """ -{% if record.has_children %} - +{% if record.children %} + {% else %} - + {% endif %} {{ record.prefix }} @@ -336,8 +336,10 @@ class PrefixTable(BaseTable): class Meta(BaseTable.Meta): model = Prefix - fields = ('pk', 'prefix', 'status', 'vrf', 'tenant', 'site', 'vlan', 'role', 'is_pool', 'description') - default_columns = ('pk', 'prefix', 'status', 'vrf', 'tenant', 'site', 'vlan', 'role', 'description') + fields = ( + 'pk', 'prefix', 'children', 'status', 'vrf', 'tenant', 'site', 'vlan', 'role', 'is_pool', 'description', + ) + default_columns = ('pk', 'prefix', 'children', 'status', 'vrf', 'tenant', 'site', 'vlan', 'role', 'description') row_attrs = { 'class': lambda record: 'success' if not record.pk else '', } diff --git a/netbox/ipam/views.py b/netbox/ipam/views.py index 52db68127..8ea33764c 100644 --- a/netbox/ipam/views.py +++ b/netbox/ipam/views.py @@ -318,7 +318,7 @@ class RoleBulkDeleteView(BulkDeleteView): # class PrefixListView(ObjectListView): - queryset = Prefix.objects.prefetch_related('site', 'vrf__tenant', 'tenant', 'vlan', 'role') + queryset = Prefix.objects.prefetch_related('site', 'vrf__tenant', 'tenant', 'vlan', 'role').annotate_tree() filterset = filters.PrefixFilterSet filterset_form = forms.PrefixFilterForm table = tables.PrefixDetailTable diff --git a/netbox/templates/ipam/prefix_list.html b/netbox/templates/ipam/prefix_list.html index 00f0b7fe9..c4c6ea157 100644 --- a/netbox/templates/ipam/prefix_list.html +++ b/netbox/templates/ipam/prefix_list.html @@ -3,7 +3,18 @@ {% block buttons %}
- Collapse - Expand +
+ + +
{% endblock %} diff --git a/netbox/utilities/templatetags/helpers.py b/netbox/utilities/templatetags/helpers.py index 99dd0c65c..151ca99cb 100644 --- a/netbox/utilities/templatetags/helpers.py +++ b/netbox/utilities/templatetags/helpers.py @@ -199,6 +199,14 @@ def has_perms(user, permissions_list): return user.has_perms(permissions_list) +@register.filter() +def split(string, sep=','): + """ + Split a string by the given value (default: comma) + """ + return string.split(sep) + + # # Tags # From c36d8b2256b0af2263c27193c942d04aa473fd27 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 6 Aug 2020 09:14:20 -0400 Subject: [PATCH 4/5] Fix default display of child count in prefix tables --- netbox/ipam/tables.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/netbox/ipam/tables.py b/netbox/ipam/tables.py index 37fa1a9eb..5a4e2c133 100644 --- a/netbox/ipam/tables.py +++ b/netbox/ipam/tables.py @@ -337,9 +337,9 @@ class PrefixTable(BaseTable): class Meta(BaseTable.Meta): model = Prefix fields = ( - 'pk', 'prefix', 'children', 'status', 'vrf', 'tenant', 'site', 'vlan', 'role', 'is_pool', 'description', + 'pk', 'prefix', 'status', 'children', 'vrf', 'tenant', 'site', 'vlan', 'role', 'is_pool', 'description', ) - default_columns = ('pk', 'prefix', 'children', 'status', 'vrf', 'tenant', 'site', 'vlan', 'role', 'description') + default_columns = ('pk', 'prefix', 'status', 'vrf', 'tenant', 'site', 'vlan', 'role', 'description') row_attrs = { 'class': lambda record: 'success' if not record.pk else '', } @@ -359,11 +359,11 @@ class PrefixDetailTable(PrefixTable): class Meta(PrefixTable.Meta): fields = ( - 'pk', 'prefix', 'status', 'vrf', 'utilization', 'tenant', 'site', 'vlan', 'role', 'is_pool', 'description', - 'tags', + 'pk', 'prefix', 'status', 'children', 'vrf', 'utilization', 'tenant', 'site', 'vlan', 'role', 'is_pool', + 'description', 'tags', ) default_columns = ( - 'pk', 'prefix', 'status', 'vrf', 'utilization', 'tenant', 'site', 'vlan', 'role', 'description', + 'pk', 'prefix', 'status', 'children', 'vrf', 'utilization', 'tenant', 'site', 'vlan', 'role', 'description', ) From 0f397fa41086b1fdd008464652d0c516575af29a Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 6 Aug 2020 09:20:20 -0400 Subject: [PATCH 5/5] Replicate 'max length' field in PrefixFilterForm for consistency --- netbox/ipam/forms.py | 3 ++ netbox/templates/inc/search_panel.html | 55 ++++++++++++++------------ 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/netbox/ipam/forms.py b/netbox/ipam/forms.py index e14f61872..15c69c504 100644 --- a/netbox/ipam/forms.py +++ b/netbox/ipam/forms.py @@ -437,6 +437,9 @@ class PrefixFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm) 'q', 'within_include', 'family', 'mask_length', 'vrf_id', 'status', 'region', 'site', 'role', 'tenant_group', 'tenant', 'is_pool', 'expand', ] + mask_length__lte = forms.IntegerField( + widget=forms.HiddenInput() + ) q = forms.CharField( required=False, label='Search' diff --git a/netbox/templates/inc/search_panel.html b/netbox/templates/inc/search_panel.html index b156791ff..5302f71b9 100644 --- a/netbox/templates/inc/search_panel.html +++ b/netbox/templates/inc/search_panel.html @@ -7,33 +7,36 @@
- {% for field in filter_form %} -
- {% if field.name == "q" %} -
- - - - -
- {% elif field|widget_type == 'checkboxinput' %} - - {% else %} - {{ field.label_tag }} - {{ field }} - {% endif %} -
- {% endfor %} -
- - - Clear - + {% for field in filter_form.hidden_fields %} + {{ field }} + {% endfor %} + {% for field in filter_form.visible_fields %} +
+ {% if field.name == "q" %} +
+ + + + +
+ {% elif field|widget_type == 'checkboxinput' %} + + {% else %} + {{ field.label_tag }} + {{ field }} + {% endif %}
+ {% endfor %} +
+ + + Clear + +