diff --git a/netbox_diode_plugin/api/applier.py b/netbox_diode_plugin/api/applier.py index 4c974d8..3f1b041 100644 --- a/netbox_diode_plugin/api/applier.py +++ b/netbox_diode_plugin/api/applier.py @@ -23,7 +23,7 @@ def apply_changeset(change_set: ChangeSet, request) -> ChangeSetResult: _validate_change_set(change_set) created = {} - for i, change in enumerate(change_set.changes): + for change in change_set.changes: change_type = change.change_type object_type = change.object_type @@ -35,9 +35,14 @@ def apply_changeset(change_set: ChangeSet, request) -> ChangeSetResult: data = _pre_apply(model_class, change, created) _apply_change(data, model_class, change, created, request) except ValidationError as e: - raise _err_from_validation_error(e, f"changes[{i}]") + raise _err_from_validation_error(e, object_type) except ObjectDoesNotExist: - raise _err(f"{object_type} with id {change.object_id} does not exist", f"changes[{i}]", "object_id") + raise _err(f"{object_type} with id {change.object_id} does not exist", object_type, "object_id") + except TypeError as e: + # this indicates a problem in model validation (should raise ValidationError) + # but raised non-validation error (TypeError) -- we don't know which field trigged it. + logger.error(f"invalid data type for unspecified field (validation raised non-validation error): {data}: {e}") + raise _err("invalid data type for field", object_type, "__all__") # ConstraintViolationError ? # ... @@ -113,13 +118,15 @@ def _validate_change_set(change_set: ChangeSet): if not change_set.changes: raise _err("Changes are required", "changeset", "changes") - for i, change in enumerate(change_set.changes): + for change in change_set.changes: if change.object_id is None and change.ref_id is None: - raise _err("Object ID or Ref ID must be provided", f"changes[{i}]", NON_FIELD_ERRORS) + raise _err("Object ID or Ref ID must be provided", change.object_type, NON_FIELD_ERRORS) if change.change_type not in ChangeType: - raise _err(f"Unsupported change type '{change.change_type}'", f"changes[{i}]", "change_type") + raise _err(f"Unsupported change type '{change.change_type}'", change.object_type, "change_type") def _err(message, object_name, field): + if not object_name: + object_name = "__all__" return ChangeSetException(message, errors={object_name: {field: [message]}}) def _err_from_validation_error(e, object_name): diff --git a/netbox_diode_plugin/api/differ.py b/netbox_diode_plugin/api/differ.py index 55a990f..1da1843 100644 --- a/netbox_diode_plugin/api/differ.py +++ b/netbox_diode_plugin/api/differ.py @@ -10,7 +10,7 @@ from django.core.exceptions import ValidationError from utilities.data import shallow_compare_dict -from .common import Change, ChangeSet, ChangeSetException, ChangeSetResult, ChangeType +from .common import Change, ChangeSet, ChangeSetException, ChangeSetResult, ChangeType, UnresolvedReference from .plugin_utils import get_primary_value, legal_fields from .supported_models import extract_supported_models from .transformer import cleanup_unresolved_references, set_custom_field_defaults, transform_proto_json @@ -84,12 +84,20 @@ def prechange_data_from_instance(instance) -> dict: # noqa: C901 def _harmonize_formats(prechange_data: dict, postchange_data: dict): for k, v in prechange_data.items(): + if k.startswith('_'): + continue if isinstance(v, datetime.datetime): prechange_data[k] = v.strftime("%Y-%m-%dT%H:%M:%SZ") elif isinstance(v, datetime.date): prechange_data[k] = v.strftime("%Y-%m-%d") elif isinstance(v, int) and k in postchange_data: - postchange_data[k] = int(postchange_data[k]) + val = postchange_data[k] + if isinstance(val, UnresolvedReference): + continue + try: + postchange_data[k] = int(val) + except Exception: + continue elif isinstance(v, dict): _harmonize_formats(v, postchange_data.get(k, {})) diff --git a/netbox_diode_plugin/api/transformer.py b/netbox_diode_plugin/api/transformer.py index cfb246d..2dd05cc 100644 --- a/netbox_diode_plugin/api/transformer.py +++ b/netbox_diode_plugin/api/transformer.py @@ -452,7 +452,10 @@ def _prepare_custom_fields(object_type: str, custom_fields: dict) -> tuple[dict, out[key] = value elif value_type == "date": # truncate to YYYY-MM-DD - out[key] = datetime.datetime.fromisoformat(value).strftime("%Y-%m-%d") + try: + out[key] = datetime.datetime.fromisoformat(value).strftime("%Y-%m-%d") + except Exception: + out[key] = value elif value_type == "integer": out[key] = int(value) elif value_type == "json": diff --git a/netbox_diode_plugin/tests/test_api_apply_change_set.py b/netbox_diode_plugin/tests/test_api_apply_change_set.py index b2d27c0..d35e1bb 100644 --- a/netbox_diode_plugin/tests/test_api_apply_change_set.py +++ b/netbox_diode_plugin/tests/test_api_apply_change_set.py @@ -300,7 +300,7 @@ def test_change_type_create_with_error_return_400(self): self.assertIn( 'Expected a list of items but got type "int".', - _get_error(response, "changes[0]", "asns"), + _get_error(response, "dcim.site", "asns"), ) self.assertFalse(site_created.exists()) @@ -334,7 +334,7 @@ def test_change_type_update_with_error_return_400(self): site_updated = Site.objects.get(id=20) self.assertIn( 'Expected a list of items but got type "int".', - _get_error(response, "changes[0]", "asns") + _get_error(response, "dcim.site", "asns") ) self.assertEqual(site_updated.name, "Site 2") @@ -478,7 +478,7 @@ def test_change_type_create_and_update_with_error_in_one_object_return_400(self) self.assertIn( "Related object not found using the provided numeric ID: 3", - _get_error(response, "changes[1]", "device_type"), + _get_error(response, "dcim.device", "device_type"), ) self.assertFalse(site_created.exists()) self.assertFalse(device_created.exists()) @@ -548,7 +548,7 @@ def test_multiples_create_type_error_in_two_objects_return_400(self): self.assertIn( "Related object not found using the provided numeric ID: 3", - _get_error(response, "changes[1]", "device_type"), + _get_error(response, "dcim.device", "device_type"), ) self.assertFalse(site_created.exists()) @@ -587,7 +587,7 @@ def test_change_type_update_with_object_id_not_exist_return_400(self): self.assertIn( "dcim.site with id 30 does not exist", - _get_error(response, "changes[0]", "object_id"), + _get_error(response, "dcim.site", "object_id"), ) self.assertEqual(site_updated.name, "Site 2") @@ -655,7 +655,7 @@ def test_change_type_field_not_provided_return_400( self.assertIn( "Unsupported change type ''", - _get_error(response, "changes[0]", "change_type"), + _get_error(response, "dcim.site", "change_type"), ) def test_change_set_id_field_and_change_set_not_provided_return_400(self): @@ -720,7 +720,7 @@ def test_change_type_and_object_type_provided_return_400( self.assertIn( "Unsupported change type 'None'", - _get_error(response, "changes[0]", "change_type"), + _get_error(response, "__all__", "change_type"), ) # self.assertEqual( # response.json().get("errors")[0].get("change_type"), @@ -992,7 +992,7 @@ def test_create_prefix_with_unknown_site_fails(self): response = self.send_request(payload, status_code=status.HTTP_400_BAD_REQUEST) self.assertIn( 'Please select a site.', - _get_error(response, "changes[0]", "scope"), + _get_error(response, "ipam.prefix", "scope"), ) self.assertFalse(Prefix.objects.filter(prefix="192.168.0.0/24").exists()) diff --git a/netbox_diode_plugin/tests/test_api_diff_and_apply.py b/netbox_diode_plugin/tests/test_api_diff_and_apply.py index 97bc38b..c4b599d 100644 --- a/netbox_diode_plugin/tests/test_api_diff_and_apply.py +++ b/netbox_diode_plugin/tests/test_api_diff_and_apply.py @@ -113,6 +113,64 @@ def test_generate_diff_and_apply_create_interface_with_tags(self): self.assertEqual(new_interface.tags.first().name, "tag 1") + def test_generate_diff_and_apply_create_and_update_device_role(self): + """Test generate diff and apply create and update device role.""" + device_uuid = str(uuid4()) + role_1_uuid = str(uuid4()) + role_2_uuid = str(uuid4()) + site_uuid = str(uuid4()) + payload = { + "timestamp": 1, + "object_type": "dcim.device", + "entity": { + "device": { + "name": f"Device {device_uuid}", + "deviceType": { + "model": f"Device Type {uuid4()}", + "manufacturer": { + "name": f"Manufacturer {uuid4()}" + } + }, + "role": { + "name": f"Role {role_1_uuid}" + }, + "site": { + "name": f"Site {site_uuid}" + } + }, + } + } + _, response = self.diff_and_apply(payload) + new_device = Device.objects.get(name=f"Device {device_uuid}") + self.assertEqual(new_device.site.name, f"Site {site_uuid}") + self.assertEqual(new_device.role.name, f"Role {role_1_uuid}") + payload = { + "timestamp": 1, + "object_type": "dcim.device", + "entity": { + "device": { + "name": f"Device {device_uuid}", + "deviceType": { + "model": f"Device Type {uuid4()}", + "manufacturer": { + "name": f"Manufacturer {uuid4()}" + } + }, + "role": { + "name": f"Role {role_2_uuid}" + }, + "site": { + "name": f"Site {site_uuid}" + } + }, + } + } + _, response = self.diff_and_apply(payload) + device = Device.objects.get(name=f"Device {device_uuid}") + self.assertEqual(device.site.name, f"Site {site_uuid}") + self.assertEqual(device.role.name, f"Role {role_2_uuid}") + + def test_generate_diff_and_apply_create_site_autoslug(self): """Test generate diff and apply create site.""" """Test generate diff create site.""" @@ -321,6 +379,35 @@ def test_generate_diff_and_apply_create_and_update_site_with_custom_field(self): diff = response1.json().get("change_set", {}) self.assertEqual(diff.get("changes", []), []) + def test_generate_diff_wrong_type_date(self): + """Test generate diff wrong type date.""" + payload = { + "timestamp": 1, + "object_type": "dcim.site", + "entity": { + "site": { + "name": "Site Generate Diff 1", + "slug": "site-generate-diff-1", + "customFields": { + "mydate": { + "date": 12, + }, + }, + }, + } + } + response1 = self.client.post( + self.diff_url, data=payload, format="json", **self.user_header + ) + self.assertEqual(response1.status_code, status.HTTP_200_OK) + + diff = response1.json().get("change_set", {}) + + response2 = self.client.post( + self.apply_url, data=diff, format="json", **self.user_header + ) + self.assertEqual(response2.status_code, status.HTTP_400_BAD_REQUEST) + def diff_and_apply(self, payload): """Diff and apply the payload."""