Address review: drop module_bay_types CSV import and cross-manufacturer resolution
Per review, ditch dedupe_module_bay_types_by_manufacturer() and any import logic that resolves module_bay_types by name alone across manufacturers. ModuleBayType's unique constraint is (manufacturer, name), not name alone, so resolving a bare name against an unscoped, potentially cross-manufacturer pool -- guessing via a preference order, rejecting only on a genuine tie -- is not a reliable way to identify a specific object. CSV import forms have no way to qualify an M2M reference beyond a bare name, so module_bay_types is no longer exposed there at all (ModuleTypeImportForm, ModuleBayImportForm in bulk_import.py): it's acceptable not to support this rather than resolve it unreliably. This also reverts the netbox/tables/columns.py export_transform API addition and the three tables' use of it, which existed only to make the CSV round trip work. The one import path that survives is ModuleBayTemplateImportForm (the YAML device/module type "Import Components" flow), because it can reliably scope module_bay_types' queryset to the parent device/module type's own manufacturer plus global (manufacturer-less) types *before* resolving by name -- so a name collision is never cross-manufacturer, only "this manufacturer's own type vs. a global one of the same name," which ModuleBayType's own uniqueness constraint makes unambiguous. A name matching only some other manufacturer's type doesn't resolve at all, rather than being coerced to an arbitrary guess. Kept: the ModuleBayTemplateImportForm.enabled field/clean_enabled() fix (default=True was previously lost on YAML re-import; unrelated to the above), and to_yaml()'s export of module_bay_types on both ModuleType and ModuleBayTemplate, plus the export_yaml() prefetch optimizations -- none of these involve resolving an object's identity from an ambiguous attribute. Trimmed the model docs to match: the modulebay.md and moduletype.md paragraphs described capabilities (CSV import, cross-manufacturer YAML import) that no longer exist and are removed; modulebaytemplate.md's note is rewritten to describe the actual (manufacturer-or-global-scoped) resolution behavior.
This commit is contained in:
Brian Tiemann
parent
e5c0d60d09
commit
f65c72da9c
|
|
@ -34,8 +34,6 @@ The numeric position in which this module bay is situated. For example, this wou
|
|||
|
||||
Zero or more [module bay types](./modulebaytype.md) assigned to this bay. When at least one bay type is set, only module types that share a common bay type may be installed. Leave empty to allow any module type.
|
||||
|
||||
Bay types are importable via CSV, referenced by name. A bay type belonging to a manufacturer other than the module bay's own device may be referenced; this mirrors the UI and REST API, which likewise place no manufacturer restriction on the assignment. Since a bay type's uniqueness is scoped to `(manufacturer, name)` rather than name alone, more than one bay type may share a name; import prefers, in order, an exact match on the device's own manufacturer, then a global (manufacturer-less) bay type. If a name instead matches two or more bay types belonging to *other* manufacturers, with neither the device's own manufacturer nor a global type available to break the tie, the import is rejected rather than resolved to an arbitrary one.
|
||||
|
||||
### Enabled
|
||||
|
||||
Whether this module bay is enabled. Disabled module bays are not available for installation.
|
||||
|
|
|
|||
|
|
@ -4,4 +4,4 @@ A template for a module bay that will be created on all instantiations of the pa
|
|||
|
||||
[Bay types](./modulebaytype.md) assigned to a module bay template are copied to each instantiated module bay, so constraints defined on the device type propagate automatically to all devices of that type.
|
||||
|
||||
Bay types are importable and exportable as part of a device type's YAML definition (`module-bays[].module_bay_types`), referenced by name. A bay type belonging to a manufacturer other than the device type's own may be referenced; this mirrors the UI and REST API, which likewise place no manufacturer restriction on the assignment. Since a bay type's uniqueness is scoped to `(manufacturer, name)` rather than name alone, more than one bay type may share a name; import prefers, in order, an exact match on the device type's own manufacturer, then a global (manufacturer-less) bay type. If a name instead matches two or more bay types belonging to *other* manufacturers, with neither the device type's own manufacturer nor a global type available to break the tie, the import is rejected rather than resolved to an arbitrary one.
|
||||
Bay types are importable and exportable as part of a device type's YAML definition (`module-bays[].module_bay_types`), referenced by name. A referenced name is resolved against bay types belonging to the device type's own manufacturer or with no manufacturer set (global); a name may match both, since a bay type's uniqueness is scoped to `(manufacturer, name)` rather than name alone, in which case the manufacturer-specific type takes precedence.
|
||||
|
|
|
|||
|
|
@ -91,8 +91,6 @@ The assigned [profile](./moduletypeprofile.md) for the type of module. Profiles
|
|||
|
||||
Zero or more [module bay types](./modulebaytype.md) that this module type is compatible with. When at least one bay type is set, the module type may only be installed into bays that share a common type. Leave empty to allow installation into any bay.
|
||||
|
||||
Bay types are importable and exportable as part of a module type's YAML definition (`module_bay_types`), referenced by name. A bay type belonging to a manufacturer other than the module type's own may be referenced -- e.g. a third-party module declaring compatibility with another manufacturer's proprietary bay type -- mirroring the UI and REST API, which likewise place no manufacturer restriction on the assignment. Since a bay type's uniqueness is scoped to `(manufacturer, name)` rather than name alone, more than one bay type may share a name; import prefers, in order, an exact match on the module type's own manufacturer, then a global (manufacturer-less) bay type. If a name instead matches two or more bay types belonging to *other* manufacturers, with neither the module type's own manufacturer nor a global type available to break the tie, the import is rejected rather than resolved to an arbitrary one.
|
||||
|
||||
### Attributes
|
||||
|
||||
Depending on the module type's assigned [profile](./moduletypeprofile.md) (if any), one or more user-defined attributes may be available to configure.
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ from django import forms
|
|||
from django.contrib.contenttypes.models import ContentType
|
||||
from django.contrib.postgres.forms.array import SimpleArrayField
|
||||
from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist
|
||||
from django.db.models import Q
|
||||
from django.utils.functional import lazy
|
||||
from django.utils.html import format_html
|
||||
from django.utils.safestring import SafeString, mark_safe
|
||||
|
|
@ -11,7 +10,7 @@ from django.utils.translation import gettext_lazy as _
|
|||
from dcim.choices import *
|
||||
from dcim.constants import *
|
||||
from dcim.models import *
|
||||
from dcim.utils import dedupe_module_bay_types_by_manufacturer, reconcile_port_mappings
|
||||
from dcim.utils import reconcile_port_mappings
|
||||
from extras.models import ConfigTemplate
|
||||
from ipam.choices import VLANQinQRoleChoices
|
||||
from ipam.models import VLAN, VRF, IPAddress, VLANGroup
|
||||
|
|
@ -551,19 +550,12 @@ class ModuleTypeImportForm(PrimaryModelImportForm):
|
|||
required=False,
|
||||
help_text=_('Attribute values for the assigned profile, passed as a dictionary')
|
||||
)
|
||||
module_bay_types = CSVModelMultipleChoiceField(
|
||||
label=_('Module bay types'),
|
||||
queryset=ModuleBayType.objects.all(),
|
||||
to_field_name='name',
|
||||
required=False,
|
||||
help_text=_('Types of module bays this module type can be installed in (empty = unconstrained)'),
|
||||
)
|
||||
|
||||
class Meta:
|
||||
model = ModuleType
|
||||
fields = [
|
||||
'manufacturer', 'model', 'part_number', 'description', 'cooling_method', 'airflow', 'weight', 'weight_unit',
|
||||
'end_of_life', 'profile', 'attribute_data', 'owner', 'comments', 'tags', 'module_bay_types',
|
||||
'end_of_life', 'profile', 'attribute_data', 'owner', 'comments', 'tags',
|
||||
]
|
||||
|
||||
def clean(self):
|
||||
|
|
@ -577,11 +569,6 @@ class ModuleTypeImportForm(PrimaryModelImportForm):
|
|||
if self.cleaned_data.get('profile') and not self.cleaned_data.get('attribute_data'):
|
||||
self.cleaned_data['attribute_data'] = {}
|
||||
|
||||
if module_bay_types := self.cleaned_data.get('module_bay_types'):
|
||||
self.cleaned_data['module_bay_types'] = dedupe_module_bay_types_by_manufacturer(
|
||||
module_bay_types, self.cleaned_data.get('manufacturer'),
|
||||
)
|
||||
|
||||
|
||||
class DeviceRoleImportForm(NestedGroupModelImportForm):
|
||||
parent = CSVModelChoiceField(
|
||||
|
|
@ -1439,19 +1426,10 @@ class ModuleBayImportForm(OwnerCSVMixin, NetBoxModelImportForm):
|
|||
queryset=Device.objects.all(),
|
||||
to_field_name='name'
|
||||
)
|
||||
module_bay_types = CSVModelMultipleChoiceField(
|
||||
label=_('Module bay types'),
|
||||
queryset=ModuleBayType.objects.all(),
|
||||
to_field_name='name',
|
||||
required=False,
|
||||
help_text=_('Types of module bays this bay accepts (empty = unconstrained)'),
|
||||
)
|
||||
|
||||
class Meta:
|
||||
model = ModuleBay
|
||||
fields = (
|
||||
'device', 'name', 'label', 'position', 'enabled', 'description', 'owner', 'tags', 'module_bay_types',
|
||||
)
|
||||
fields = ('device', 'name', 'label', 'position', 'enabled', 'description', 'owner', 'tags')
|
||||
|
||||
def clean_enabled(self):
|
||||
# Make sure enabled is True when it's not included in the uploaded data
|
||||
|
|
@ -1459,16 +1437,6 @@ class ModuleBayImportForm(OwnerCSVMixin, NetBoxModelImportForm):
|
|||
return True
|
||||
return self.cleaned_data['enabled']
|
||||
|
||||
def clean(self):
|
||||
super().clean()
|
||||
|
||||
if module_bay_types := self.cleaned_data.get('module_bay_types'):
|
||||
device = self.cleaned_data.get('device')
|
||||
manufacturer = device.device_type.manufacturer if device else None
|
||||
self.cleaned_data['module_bay_types'] = dedupe_module_bay_types_by_manufacturer(
|
||||
module_bay_types, manufacturer,
|
||||
)
|
||||
|
||||
|
||||
class DeviceBayImportForm(OwnerCSVMixin, NetBoxModelImportForm):
|
||||
device = CSVModelChoiceField(
|
||||
|
|
|
|||
|
|
@ -1,10 +1,9 @@
|
|||
from django import forms
|
||||
from django.db.models import Q
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
|
||||
from dcim.choices import InterfacePoEModeChoices, InterfacePoETypeChoices, InterfaceTypeChoices, PortTypeChoices
|
||||
from dcim.models import *
|
||||
from dcim.utils import dedupe_module_bay_types_by_manufacturer
|
||||
from utilities.forms.fields import CSVModelMultipleChoiceField
|
||||
from wireless.choices import WirelessRoleChoices
|
||||
|
||||
__all__ = (
|
||||
|
|
@ -215,10 +214,7 @@ class PortTemplateMappingImportForm(forms.ModelForm):
|
|||
|
||||
|
||||
class ModuleBayTemplateImportForm(forms.ModelForm):
|
||||
# CSVModelMultipleChoiceField, not the plain ModelMultipleChoiceField used elsewhere in
|
||||
# this file, so a scalar name is accepted alongside a list -- matches ModuleTypeImportForm's
|
||||
# equivalent field, which also serves plain CSV import.
|
||||
module_bay_types = CSVModelMultipleChoiceField(
|
||||
module_bay_types = forms.ModelMultipleChoiceField(
|
||||
label=_('Module bay types'),
|
||||
queryset=ModuleBayType.objects.all(),
|
||||
to_field_name='name',
|
||||
|
|
@ -227,6 +223,8 @@ class ModuleBayTemplateImportForm(forms.ModelForm):
|
|||
|
||||
class Meta:
|
||||
model = ModuleBayTemplate
|
||||
# module_bay_types must stay last: clean_device_type/clean_module_type narrow its queryset by
|
||||
# manufacturer before it is itself cleaned, and Django cleans fields in this order.
|
||||
fields = [
|
||||
'device_type', 'module_type', 'name', 'label', 'position', 'enabled', 'description',
|
||||
'module_bay_types',
|
||||
|
|
@ -239,20 +237,42 @@ class ModuleBayTemplateImportForm(forms.ModelForm):
|
|||
return True
|
||||
return self.cleaned_data['enabled']
|
||||
|
||||
def clean(self):
|
||||
cleaned_data = super().clean()
|
||||
def _scope_module_bay_types(self, manufacturer):
|
||||
module_bay_types = self.fields['module_bay_types']
|
||||
module_bay_types.queryset = module_bay_types.queryset.filter(
|
||||
Q(manufacturer__isnull=True) | Q(manufacturer=manufacturer)
|
||||
)
|
||||
|
||||
if module_bay_types := cleaned_data.get('module_bay_types'):
|
||||
device_type = cleaned_data.get('device_type')
|
||||
module_type = cleaned_data.get('module_type')
|
||||
manufacturer = device_type.manufacturer if device_type else (
|
||||
module_type.manufacturer if module_type else None
|
||||
)
|
||||
cleaned_data['module_bay_types'] = dedupe_module_bay_types_by_manufacturer(
|
||||
module_bay_types, manufacturer,
|
||||
)
|
||||
def clean_device_type(self):
|
||||
if device_type := self.cleaned_data['device_type']:
|
||||
self._scope_module_bay_types(device_type.manufacturer)
|
||||
|
||||
return cleaned_data
|
||||
return device_type
|
||||
|
||||
def clean_module_type(self):
|
||||
if module_type := self.cleaned_data['module_type']:
|
||||
self._scope_module_bay_types(module_type.manufacturer)
|
||||
|
||||
return module_type
|
||||
|
||||
def clean_module_bay_types(self):
|
||||
"""
|
||||
Collapse to one match per name, preferring a manufacturer-specific match over a global
|
||||
one. ModuleBayType's unique constraint is on (manufacturer, name), not name alone, so a
|
||||
name can legitimately collide between a global type and one scoped to this template's
|
||||
own manufacturer (narrowed by clean_device_type/clean_module_type above); the field's
|
||||
default name-based lookup resolves both matches into cleaned_data rather than picking
|
||||
one, since it has no way to know which is meant.
|
||||
"""
|
||||
module_bay_types = self.cleaned_data['module_bay_types']
|
||||
|
||||
by_name = {}
|
||||
for module_bay_type in module_bay_types:
|
||||
existing = by_name.get(module_bay_type.name)
|
||||
if existing is None or module_bay_type.manufacturer_id is not None:
|
||||
by_name[module_bay_type.name] = module_bay_type
|
||||
|
||||
return list(by_name.values())
|
||||
|
||||
|
||||
class DeviceBayTemplateImportForm(forms.ModelForm):
|
||||
|
|
|
|||
|
|
@ -1028,8 +1028,6 @@ class ModuleBayTable(ModularDeviceComponentTable):
|
|||
module_bay_types = columns.ManyToManyColumn(
|
||||
verbose_name=_('Bay Types'),
|
||||
linkify_item=True,
|
||||
# Export as bare name (import resolves by name alone); UI keeps str()'s manufacturer prefix.
|
||||
export_transform=lambda obj: obj.name,
|
||||
)
|
||||
|
||||
class Meta(ModularDeviceComponentTable.Meta):
|
||||
|
|
|
|||
|
|
@ -305,8 +305,6 @@ class ModuleBayTemplateTable(ComponentTemplateTable):
|
|||
module_bay_types = columns.ManyToManyColumn(
|
||||
verbose_name=_('Bay Types'),
|
||||
linkify_item=True,
|
||||
# Export as bare name (import resolves by name alone); UI keeps str()'s manufacturer prefix.
|
||||
export_transform=lambda obj: obj.name,
|
||||
)
|
||||
actions = columns.ActionsColumn(
|
||||
actions=('edit', 'delete')
|
||||
|
|
|
|||
|
|
@ -78,8 +78,6 @@ class ModuleTypeTable(PrimaryModelTable):
|
|||
module_bay_types = columns.ManyToManyColumn(
|
||||
verbose_name=_('Bay Types'),
|
||||
linkify_item=True,
|
||||
# Export as bare name (import resolves by name alone); UI keeps str()'s manufacturer prefix.
|
||||
export_transform=lambda obj: obj.name,
|
||||
)
|
||||
model = tables.Column(
|
||||
linkify=True,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
from unittest.mock import patch
|
||||
|
||||
import yaml
|
||||
from django import forms
|
||||
from django.test import TestCase
|
||||
|
||||
|
|
@ -17,7 +16,6 @@ from dcim.choices import (
|
|||
)
|
||||
from dcim.forms import *
|
||||
from dcim.models import *
|
||||
from dcim.tables.modules import ModuleTypeTable
|
||||
from dcim.tests.test_module_moves import fail_after
|
||||
from ipam.models import ASN, RIR, VLAN
|
||||
from utilities.exceptions import AbortRequest
|
||||
|
|
@ -350,252 +348,29 @@ class ModuleBayTemplateImportFormTestCase(TestCase):
|
|||
set(original.module_bay_types.values_list('name', flat=True)),
|
||||
)
|
||||
|
||||
def test_module_bay_types_permits_a_different_manufacturers_type(self):
|
||||
"""The UI/API place no manufacturer restriction on module_bay_types; import must match."""
|
||||
def test_module_bay_types_name_belonging_only_to_other_manufacturers_is_unresolvable(self):
|
||||
"""
|
||||
A name that exists only for manufacturers other than the device type's own (and isn't
|
||||
global) must not resolve at all -- module_bay_types is scoped to the device type's own
|
||||
manufacturer plus global types, with no cross-manufacturer fallback.
|
||||
"""
|
||||
juniper = Manufacturer.objects.create(name='Juniper', slug='juniper')
|
||||
cisco = Manufacturer.objects.create(name='Cisco', slug='cisco')
|
||||
cisco_bay_type = ModuleBayType.objects.create(name='SFP28', slug='sfp28-cisco', manufacturer=cisco)
|
||||
ModuleBayType.objects.create(name='SFP28', slug='sfp28-cisco', manufacturer=cisco)
|
||||
device_type = DeviceType.objects.create(
|
||||
manufacturer=juniper, model='Juniper Device Type', slug='juniper-device-type',
|
||||
)
|
||||
|
||||
form = ModuleBayTemplateImportForm({
|
||||
'device_type': device_type.pk,
|
||||
'name': 'Module Bay 1',
|
||||
'module_bay_types': ['SFP28'],
|
||||
})
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
|
||||
module_bay_template = form.save()
|
||||
self.assertEqual(list(module_bay_template.module_bay_types.all()), [cisco_bay_type])
|
||||
|
||||
exported = module_bay_template.to_yaml()['module_bay_types']
|
||||
reimport_form = ModuleBayTemplateImportForm({
|
||||
'device_type': device_type.pk,
|
||||
'name': 'Module Bay 2',
|
||||
'module_bay_types': exported,
|
||||
})
|
||||
self.assertTrue(reimport_form.is_valid(), reimport_form.errors)
|
||||
self.assertEqual(list(reimport_form.save().module_bay_types.all()), [cisco_bay_type])
|
||||
|
||||
def test_module_bay_types_rejects_ambiguous_name_across_two_foreign_manufacturers(self):
|
||||
juniper = Manufacturer.objects.create(name='Juniper', slug='juniper')
|
||||
cisco = Manufacturer.objects.create(name='Cisco', slug='cisco')
|
||||
arista = Manufacturer.objects.create(name='Arista', slug='arista')
|
||||
ModuleBayType.objects.create(name='SFP28', slug='sfp28-cisco', manufacturer=cisco)
|
||||
ModuleBayType.objects.create(name='SFP28', slug='sfp28-arista', manufacturer=arista)
|
||||
device_type = DeviceType.objects.create(
|
||||
manufacturer=juniper, model='Juniper Device Type 2', slug='juniper-device-type-2',
|
||||
)
|
||||
|
||||
form = ModuleBayTemplateImportForm({
|
||||
'device_type': device_type.pk,
|
||||
'name': 'Module Bay 1',
|
||||
'module_bay_types': ['SFP28'],
|
||||
})
|
||||
self.assertFalse(form.is_valid())
|
||||
# Must name both manufacturers, not just error -- a plain invalid_choice would also
|
||||
# pass assertIn() and mask a regression of the scoping removed in 6f3c537.
|
||||
errors = form.errors.get('module_bay_types', [])
|
||||
self.assertTrue(any('Cisco' in e and 'Arista' in e for e in errors), errors)
|
||||
|
||||
|
||||
class ModuleBayImportFormTestCase(TestCase):
|
||||
"""Covers real ModuleBay instances via CSV, as opposed to templates via ModuleBayTemplateImportForm."""
|
||||
|
||||
def test_module_bay_types_csv_import(self):
|
||||
device = create_test_device('Module Bay Import Device')
|
||||
bay_type = ModuleBayType.objects.create(name='SFP28', slug='sfp28')
|
||||
|
||||
form = ModuleBayImportForm({
|
||||
'device': device.name,
|
||||
'name': 'Bay 1',
|
||||
'module_bay_types': 'SFP28',
|
||||
})
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
module_bay = form.save()
|
||||
self.assertEqual(list(module_bay.module_bay_types.all()), [bay_type])
|
||||
|
||||
def test_module_bay_types_prefers_devices_own_manufacturer(self):
|
||||
device = create_test_device('Module Bay Import Device')
|
||||
own_manufacturer = device.device_type.manufacturer
|
||||
other_manufacturer = Manufacturer.objects.create(name='Other Mfr', slug='other-mfr')
|
||||
own_type = ModuleBayType.objects.create(name='SFP28', slug='sfp28-own', manufacturer=own_manufacturer)
|
||||
ModuleBayType.objects.create(name='SFP28', slug='sfp28-other', manufacturer=other_manufacturer)
|
||||
|
||||
form = ModuleBayImportForm({
|
||||
'device': device.name,
|
||||
'name': 'Bay 1',
|
||||
'module_bay_types': 'SFP28',
|
||||
})
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
self.assertEqual(list(form.save().module_bay_types.all()), [own_type])
|
||||
|
||||
def test_module_bay_types_rejects_ambiguous_name_across_two_foreign_manufacturers(self):
|
||||
device = create_test_device('Module Bay Import Device')
|
||||
cisco = Manufacturer.objects.create(name='Cisco', slug='cisco')
|
||||
arista = Manufacturer.objects.create(name='Arista', slug='arista')
|
||||
ModuleBayType.objects.create(name='SFP28', slug='sfp28-cisco', manufacturer=cisco)
|
||||
ModuleBayType.objects.create(name='SFP28', slug='sfp28-arista', manufacturer=arista)
|
||||
|
||||
form = ModuleBayImportForm({
|
||||
'device': device.name,
|
||||
'name': 'Bay 1',
|
||||
'module_bay_types': 'SFP28',
|
||||
})
|
||||
self.assertFalse(form.is_valid())
|
||||
# Must name both manufacturers, not just error -- a plain invalid_choice would also
|
||||
# pass assertIn() and mask a regression of the scoping removed in 6f3c537.
|
||||
errors = form.errors.get('module_bay_types', [])
|
||||
self.assertTrue(any('Cisco' in e and 'Arista' in e for e in errors), errors)
|
||||
|
||||
|
||||
class ModuleTypeImportFormTestCase(TestCase):
|
||||
|
||||
def test_module_bay_types_round_trip(self):
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
|
||||
bay_type = ModuleBayType.objects.create(name='SFP28', slug='sfp28', manufacturer=manufacturer)
|
||||
|
||||
form = ModuleTypeImportForm({
|
||||
'manufacturer': manufacturer.name,
|
||||
'model': 'Module Type 1',
|
||||
'module_bay_types': ['SFP28'],
|
||||
})
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
|
||||
module_type = form.save()
|
||||
self.assertEqual(list(module_type.module_bay_types.all()), [bay_type])
|
||||
self.assertEqual(yaml.safe_load(module_type.to_yaml())['module_bay_types'], ['SFP28'])
|
||||
|
||||
def test_module_bay_types_prefers_manufacturer_specific_match_over_global(self):
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
|
||||
global_type = ModuleBayType.objects.create(name='SFP28', slug='sfp28-global')
|
||||
scoped_type = ModuleBayType.objects.create(
|
||||
name='SFP28', slug='sfp28-scoped', manufacturer=manufacturer,
|
||||
)
|
||||
|
||||
form = ModuleTypeImportForm({
|
||||
'manufacturer': manufacturer.name,
|
||||
'model': 'Module Type 1',
|
||||
'module_bay_types': ['SFP28'],
|
||||
})
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
|
||||
module_type = form.save()
|
||||
self.assertEqual(list(module_type.module_bay_types.all()), [scoped_type])
|
||||
self.assertNotIn(global_type, module_type.module_bay_types.all())
|
||||
|
||||
def test_module_bay_types_accepts_csv_comma_separated_string(self):
|
||||
"""This form also serves plain CSV import, where the value is a string, not a list."""
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
|
||||
bay_type_a = ModuleBayType.objects.create(name='SFP28', slug='sfp28')
|
||||
bay_type_b = ModuleBayType.objects.create(name='QSFP28', slug='qsfp28')
|
||||
|
||||
form = ModuleTypeImportForm({
|
||||
'manufacturer': manufacturer.name,
|
||||
'model': 'Module Type 1',
|
||||
'module_bay_types': 'SFP28,QSFP28',
|
||||
})
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
|
||||
module_type = form.save()
|
||||
self.assertEqual(
|
||||
set(module_type.module_bay_types.values_list('name', flat=True)),
|
||||
{bay_type_a.name, bay_type_b.name},
|
||||
form.errors.as_data()['module_bay_types'][0].code, 'invalid_choice',
|
||||
)
|
||||
|
||||
def test_module_bay_types_accepts_empty_csv_string(self):
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
|
||||
|
||||
form = ModuleTypeImportForm({
|
||||
'manufacturer': manufacturer.name,
|
||||
'model': 'Module Type 1',
|
||||
'module_bay_types': '',
|
||||
})
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
self.assertFalse(form.save().module_bay_types.exists())
|
||||
|
||||
def test_module_bay_types_round_trips_through_the_table_column_export_value(self):
|
||||
"""The table's CSV export (multi-value separator, name-only transform) must be re-importable,
|
||||
without changing the rendered UI column, which should keep str()'s manufacturer prefix."""
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
|
||||
bay_type_a = ModuleBayType.objects.create(name='SFP28', slug='sfp28', manufacturer=manufacturer)
|
||||
bay_type_b = ModuleBayType.objects.create(name='QSFP28', slug='qsfp28', manufacturer=manufacturer)
|
||||
original = ModuleType.objects.create(manufacturer=manufacturer, model='Module Type 1')
|
||||
original.module_bay_types.set([bay_type_a, bay_type_b])
|
||||
|
||||
column = ModuleTypeTable([original]).columns['module_bay_types'].column
|
||||
exported_value = column.value(original.module_bay_types.all())
|
||||
self.assertEqual(exported_value, 'QSFP28, SFP28')
|
||||
self.assertIn(str(manufacturer), str(column.render(original.module_bay_types.all())))
|
||||
|
||||
form = ModuleTypeImportForm({
|
||||
'manufacturer': manufacturer.name,
|
||||
'model': 'Module Type 2',
|
||||
'module_bay_types': exported_value,
|
||||
})
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
self.assertEqual(
|
||||
set(form.save().module_bay_types.values_list('name', flat=True)),
|
||||
{bay_type_a.name, bay_type_b.name},
|
||||
)
|
||||
|
||||
def test_module_bay_types_non_string_scalar_is_a_validation_error_not_a_crash(self):
|
||||
"""A non-string scalar (e.g. from YAML) must produce a form error, not a crash."""
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
|
||||
|
||||
form = ModuleTypeImportForm({
|
||||
'manufacturer': manufacturer.name,
|
||||
'model': 'Module Type 1',
|
||||
'module_bay_types': 100,
|
||||
})
|
||||
self.assertFalse(form.is_valid())
|
||||
self.assertIn('module_bay_types', form.errors)
|
||||
|
||||
def test_module_bay_types_permits_a_different_manufacturers_type(self):
|
||||
"""The UI/API place no manufacturer restriction on module_bay_types; import must match."""
|
||||
juniper = Manufacturer.objects.create(name='Juniper', slug='juniper')
|
||||
cisco = Manufacturer.objects.create(name='Cisco', slug='cisco')
|
||||
cisco_bay_type = ModuleBayType.objects.create(name='SFP28', slug='sfp28-cisco', manufacturer=cisco)
|
||||
|
||||
form = ModuleTypeImportForm({
|
||||
'manufacturer': juniper.name,
|
||||
'model': 'Juniper Line Card',
|
||||
'module_bay_types': ['SFP28'],
|
||||
})
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
|
||||
module_type = form.save()
|
||||
self.assertEqual(list(module_type.module_bay_types.all()), [cisco_bay_type])
|
||||
|
||||
exported = yaml.safe_load(module_type.to_yaml())['module_bay_types']
|
||||
reimport_form = ModuleTypeImportForm({
|
||||
'manufacturer': juniper.name,
|
||||
'model': 'Juniper Line Card 2',
|
||||
'module_bay_types': exported,
|
||||
})
|
||||
self.assertTrue(reimport_form.is_valid(), reimport_form.errors)
|
||||
self.assertEqual(list(reimport_form.save().module_bay_types.all()), [cisco_bay_type])
|
||||
|
||||
def test_module_bay_types_rejects_ambiguous_name_across_two_foreign_manufacturers(self):
|
||||
"""A name matching two different foreign manufacturers must be refused, not resolved arbitrarily."""
|
||||
juniper = Manufacturer.objects.create(name='Juniper', slug='juniper')
|
||||
cisco = Manufacturer.objects.create(name='Cisco', slug='cisco')
|
||||
arista = Manufacturer.objects.create(name='Arista', slug='arista')
|
||||
ModuleBayType.objects.create(name='SFP28', slug='sfp28-cisco', manufacturer=cisco)
|
||||
ModuleBayType.objects.create(name='SFP28', slug='sfp28-arista', manufacturer=arista)
|
||||
|
||||
form = ModuleTypeImportForm({
|
||||
'manufacturer': juniper.name,
|
||||
'model': 'Juniper Line Card',
|
||||
'module_bay_types': ['SFP28'],
|
||||
})
|
||||
self.assertFalse(form.is_valid())
|
||||
# Must name both manufacturers, not just error -- a plain invalid_choice would also
|
||||
# pass assertIn() and mask a regression of the scoping removed in 6f3c537.
|
||||
errors = form.errors.get('module_bay_types', [])
|
||||
self.assertTrue(any('Cisco' in e and 'Arista' in e for e in errors), errors)
|
||||
|
||||
|
||||
class ModuleFormTestCase(TestCase):
|
||||
|
||||
|
|
|
|||
|
|
@ -2,57 +2,12 @@ from collections import defaultdict
|
|||
|
||||
from django.apps import apps
|
||||
from django.contrib.contenttypes.models import ContentType
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.db import router, transaction
|
||||
from django.utils.translation import gettext as _
|
||||
|
||||
from dcim.constants import MODULE_TOKEN
|
||||
|
||||
|
||||
def dedupe_module_bay_types_by_manufacturer(module_bay_types, manufacturer=None):
|
||||
"""
|
||||
Collapse ModuleBayType instances resolved by name to one per name, preferring an exact
|
||||
match on *manufacturer*, then a global (manufacturer-less) type. Names aren't globally
|
||||
unique -- uniqueness is scoped to (manufacturer, name) -- and callers must not scope the
|
||||
queryset by manufacturer themselves, since a type may legitimately belong to another
|
||||
manufacturer entirely; only this preference order is manufacturer-aware.
|
||||
|
||||
Raises ValidationError if a name ties across two or more non-preferred manufacturers,
|
||||
rather than picking one arbitrarily.
|
||||
"""
|
||||
manufacturer_id = manufacturer.pk if manufacturer else None
|
||||
|
||||
def preference(module_bay_type):
|
||||
if module_bay_type.manufacturer_id == manufacturer_id:
|
||||
return 0
|
||||
if module_bay_type.manufacturer_id is None:
|
||||
return 1
|
||||
return 2
|
||||
|
||||
# Keyed by pk so a caller passing the same row twice can't manufacture a false tie below.
|
||||
by_name = defaultdict(dict)
|
||||
for module_bay_type in module_bay_types:
|
||||
by_name[module_bay_type.name][module_bay_type.pk] = module_bay_type
|
||||
|
||||
resolved = []
|
||||
for name, candidates_by_pk in by_name.items():
|
||||
candidates = list(candidates_by_pk.values())
|
||||
best_rank = min(preference(c) for c in candidates)
|
||||
best = [c for c in candidates if preference(c) == best_rank]
|
||||
if len(best) > 1:
|
||||
manufacturers = ', '.join(sorted(c.manufacturer.name for c in best))
|
||||
raise ValidationError({
|
||||
'module_bay_types': _(
|
||||
"Module bay type \"{name}\" is ambiguous: it belongs to more than one "
|
||||
"manufacturer ({manufacturers}), none of which is this type's own "
|
||||
"manufacturer."
|
||||
).format(name=name, manufacturers=manufacturers)
|
||||
})
|
||||
resolved.append(best[0])
|
||||
|
||||
return resolved
|
||||
|
||||
|
||||
def inherit_module_token(position, parent_positions):
|
||||
"""
|
||||
Resolve a single {module} token in a bay position by inheriting from the position
|
||||
|
|
|
|||
|
|
@ -131,17 +131,9 @@ class DurationColumn(tables.Column):
|
|||
class ManyToManyColumn(tables.ManyToManyColumn):
|
||||
"""
|
||||
Overrides django-tables2's stock ManyToManyColumn to ensure that value() returns only plaintext data.
|
||||
|
||||
export_transform: optional callable used only for value() (CSV/table export), letting export use a
|
||||
different representation than the rendered column (e.g. a bare name where the UI shows str(obj)).
|
||||
Defaults to transform, matching the stock behavior of exporting the same text that's rendered.
|
||||
"""
|
||||
def __init__(self, *args, export_transform=None, **kwargs):
|
||||
super().__init__(*args, **kwargs)
|
||||
self.export_transform = export_transform or self.transform
|
||||
|
||||
def value(self, value):
|
||||
items = [self.export_transform(item) for item in self.filter(value)]
|
||||
items = [self.transform(item) for item in self.filter(value)]
|
||||
return self.separator.join(items)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -100,9 +100,7 @@ class CSVModelMultipleChoiceField(forms.ModelMultipleChoiceField):
|
|||
|
||||
def clean(self, value):
|
||||
if not isinstance(value, list):
|
||||
# str(): a non-CSV caller (e.g. YAML) may pass a non-string scalar. strip(): a
|
||||
# table's default ManyToManyColumn export separator is ", ", not ",".
|
||||
value = [v.strip() for v in str(value).split(',')] if value else []
|
||||
value = value.split(',') if value else []
|
||||
return super().clean(value)
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue