diff --git a/netbox/core/models/change_logging.py b/netbox/core/models/change_logging.py index c5aab1838..b6bf13f7a 100644 --- a/netbox/core/models/change_logging.py +++ b/netbox/core/models/change_logging.py @@ -11,7 +11,7 @@ from mptt.models import MPTTModel from core.choices import ObjectChangeActionChoices from core.querysets import ObjectChangeQuerySet from netbox.models.features import ChangeLoggingMixin, has_feature -from utilities.data import shallow_compare_dict +from utilities.data import deep_compare_dict, shallow_compare_dict __all__ = ( 'ObjectChange', @@ -202,9 +202,11 @@ class ObjectChange(models.Model): elif self.action == ObjectChangeActionChoices.ACTION_DELETE: changed_attrs = sorted(prechange_data.keys()) else: - # TODO: Support deep (recursive) comparison - changed_data = shallow_compare_dict(prechange_data, postchange_data) - changed_attrs = sorted(changed_data.keys()) + diff_added, diff_removed = deep_compare_dict(prechange_data, postchange_data) + return { + 'pre': dict(sorted(diff_removed.items())), + 'post': dict(sorted(diff_added.items())), + } return { 'pre': { diff --git a/netbox/core/views.py b/netbox/core/views.py index 21e68d1b6..a8a45e5cc 100644 --- a/netbox/core/views.py +++ b/netbox/core/views.py @@ -30,7 +30,7 @@ from netbox.views import generic from netbox.views.generic.base import BaseObjectView from netbox.views.generic.mixins import TableMixin from utilities.apps import get_installed_apps -from utilities.data import shallow_compare_dict +from utilities.data import deep_compare_dict, shallow_compare_dict from utilities.forms import ConfirmationForm from utilities.htmx import htmx_partial from utilities.json import ConfigJSONEncoder @@ -273,14 +273,11 @@ class ObjectChangeView(generic.ObjectView): prechange_data = instance.prechange_data_clean if prechange_data and instance.postchange_data: - diff_added = shallow_compare_dict( + diff_added, diff_removed = deep_compare_dict( prechange_data or dict(), instance.postchange_data_clean or dict(), exclude=['last_updated'], ) - diff_removed = { - x: prechange_data.get(x) for x in diff_added - } if prechange_data else {} else: diff_added = None diff_removed = None diff --git a/netbox/utilities/data.py b/netbox/utilities/data.py index 549bf96ef..8ab8e110b 100644 --- a/netbox/utilities/data.py +++ b/netbox/utilities/data.py @@ -7,6 +7,7 @@ __all__ = ( 'array_to_ranges', 'array_to_string', 'check_ranges_overlap', + 'deep_compare_dict', 'deepmerge', 'drange', 'flatten_dict', @@ -83,6 +84,35 @@ def shallow_compare_dict(source_dict, destination_dict, exclude=tuple()): return difference +def deep_compare_dict(source_dict, destination_dict, exclude=tuple()): + """ + Return a two-tuple of dictionaries (added, removed) representing the differences between source_dict and + destination_dict. For values which are themselves dicts, the comparison is performed recursively such that only + the changed keys within the nested dict are included. `exclude` is a list or tuple of keys to be ignored. + """ + added = {} + removed = {} + + all_keys = set(source_dict) | set(destination_dict) + for key in all_keys: + if key in exclude: + continue + src_val = source_dict.get(key) + dst_val = destination_dict.get(key) + if src_val == dst_val: + continue + if isinstance(src_val, dict) and isinstance(dst_val, dict): + sub_added, sub_removed = deep_compare_dict(src_val, dst_val) + if sub_added or sub_removed: + added[key] = sub_added + removed[key] = sub_removed + else: + added[key] = dst_val + removed[key] = src_val + + return added, removed + + # # Array utilities # diff --git a/netbox/utilities/tests/test_data.py b/netbox/utilities/tests/test_data.py index 0d4bd95df..ab48ce496 100644 --- a/netbox/utilities/tests/test_data.py +++ b/netbox/utilities/tests/test_data.py @@ -3,6 +3,7 @@ from django.test import TestCase from utilities.data import ( check_ranges_overlap, + deep_compare_dict, get_config_value_ci, ranges_to_string, ranges_to_string_list, @@ -100,6 +101,66 @@ class RangeFunctionsTestCase(TestCase): ) +class DeepCompareDictTestCase(TestCase): + + def test_no_changes(self): + source = {'a': 1, 'b': 'foo', 'c': {'x': 1, 'y': 2}} + dest = {'a': 1, 'b': 'foo', 'c': {'x': 1, 'y': 2}} + added, removed = deep_compare_dict(source, dest) + self.assertEqual(added, {}) + self.assertEqual(removed, {}) + + def test_scalar_change(self): + source = {'a': 1, 'b': 'foo'} + dest = {'a': 2, 'b': 'foo'} + added, removed = deep_compare_dict(source, dest) + self.assertEqual(added, {'a': 2}) + self.assertEqual(removed, {'a': 1}) + + def test_key_added(self): + source = {'a': 1} + dest = {'a': 1, 'b': 'new'} + added, removed = deep_compare_dict(source, dest) + self.assertEqual(added, {'b': 'new'}) + self.assertEqual(removed, {'b': None}) + + def test_key_removed(self): + source = {'a': 1, 'b': 'old'} + dest = {'a': 1} + added, removed = deep_compare_dict(source, dest) + self.assertEqual(added, {'b': None}) + self.assertEqual(removed, {'b': 'old'}) + + def test_nested_dict_partial_change(self): + """Only changed sub-keys of a nested dict are included.""" + source = {'custom_fields': {'cf1': 'old', 'cf2': 'unchanged'}} + dest = {'custom_fields': {'cf1': 'new', 'cf2': 'unchanged'}} + added, removed = deep_compare_dict(source, dest) + self.assertEqual(added, {'custom_fields': {'cf1': 'new'}}) + self.assertEqual(removed, {'custom_fields': {'cf1': 'old'}}) + + def test_nested_dict_no_change(self): + source = {'name': 'test', 'custom_fields': {'cf1': 'same'}} + dest = {'name': 'test', 'custom_fields': {'cf1': 'same'}} + added, removed = deep_compare_dict(source, dest) + self.assertEqual(added, {}) + self.assertEqual(removed, {}) + + def test_exclude(self): + source = {'a': 1, 'last_updated': '2024-01-01'} + dest = {'a': 2, 'last_updated': '2024-06-01'} + added, removed = deep_compare_dict(source, dest, exclude=['last_updated']) + self.assertEqual(added, {'a': 2}) + self.assertEqual(removed, {'a': 1}) + + def test_deeply_nested(self): + source = {'level1': {'level2': {'val': 'old', 'other': 'same'}}} + dest = {'level1': {'level2': {'val': 'new', 'other': 'same'}}} + added, removed = deep_compare_dict(source, dest) + self.assertEqual(added, {'level1': {'level2': {'val': 'new'}}}) + self.assertEqual(removed, {'level1': {'level2': {'val': 'old'}}}) + + class GetConfigValueCITestCase(TestCase): def test_exact_match(self):