diff --git a/changes/457.added b/changes/457.added new file mode 100644 index 00000000..f67ad91f --- /dev/null +++ b/changes/457.added @@ -0,0 +1,3 @@ +Added `sort_relationships()` helper function +Added tests for `sort_relationships()` helper function +Added call to `sort_relationships()` function in contrib `NautobotAdapter` \ No newline at end of file diff --git a/docs/dev/jobs.md b/docs/dev/jobs.md index 17deadab..b6607d2e 100644 --- a/docs/dev/jobs.md +++ b/docs/dev/jobs.md @@ -168,7 +168,7 @@ The methods [`calculate_diff`][nautobot_ssot.jobs.base.DataSyncBaseJob.calculate Optionally, on your Job class, also implement the [`lookup_object`][nautobot_ssot.jobs.base.DataSyncBaseJob.lookup_object], [`data_mapping`][nautobot_ssot.jobs.base.DataSyncBaseJob.data_mappings], and/or [`config_information`][nautobot_ssot.jobs.base.DataSyncBaseJob.config_information] APIs (to provide more information to the end user about the details of this Job), as well as the various metadata properties on your Job's Meta inner class. Refer to the example Jobs provided in this Nautobot app for examples and further details. Install your Job via any of the supported Nautobot methods (installation into the `JOBS_ROOT` directory, inclusion in a Git repository, or packaging as part of an app) and it should automatically become available! -### Extra Step: Implementing `create`, `update` and `delete` +### Extra Step 1: Implementing `create`, `update` and `delete` If you are synchronizing data _to_ Nautobot and not _from_ Nautobot, you can entirely skip this step. The `nautobot_ssot.contrib.NautobotModel` class provides this functionality automatically. @@ -179,3 +179,51 @@ If you need to perform the `create`, `update` and `delete` operations on the rem !!! warning Special care should be taken when synchronizing new Devices with children Interfaces into a Nautobot instance that also defines Device Types with Interface components of the same name. When the new Device is created in Nautobot, its Interfaces will also be created as defined in the respective Device Type. As a result, when SSoT will attempt to create the children Interfaces loaded by the remote adapter, these will already exist in the target Nautobot system. In this scenario, if not properly handled, the sync will fail! Possible remediation steps may vary depending on the specific use-case, therefore this is left as an exercise to the reader/developer to solve for their specific context. + +### Extra Step 2: Sorting Many-to-Many and Many-to-One Relationships + +If you are not syncing any many-to-many relationships (M2M) or many-to-one (N:1) relationships from the many side, you can skip this step. + +Loading M2M and N:1 relationship data from source and target destinations are typically not in the same order as each other. For example, the order of a device's interfaces from the source data may differ compared to the order Nautobot loads the data. + +To resolve this, each relationships must be properly sorted before the source and target are compared against eachater. An additional attribute called `sorted_relationships` must be defined in both the source and target adapters. This attribute must be identical between both adapters. + +M2M and N:1 relationships are stored in the DiffSync store as a list of dictionaries. To sort a list of dictionaries, we must specify a dictionary key to sort by and do so by using code similar to the following: + +``` +for obj in diffsync.get_all("model_name"): + sorted_data = sorted( + obj.attribute_name, + key=lamda x: x["sort_by_key_name"] + ) + obj.attribute_name = sorted_data + diffsync.update(obj) +``` + +The `sorted_relationships` attribute was added is a tuple of tuples. Each entry must have three string values indicating: + +1. Name of the model with attribute to be sorted +2. Attribute within the model to sort +3. Dictionary key name to sort by + +The helper function `sort_relationships` has been added to contrib to assist in sorting relationships. The `NautobotAdapter` will automatically call this function and process any entries added to `sorted_relationships`. + +For integrations other than the `NautobotAdapter`, you must also import and add the `sort_relationships` into into the `load()` method and simply pass the DiffSync/Adapter object through using `self`. This must be done after all other loading logic is completed. + +Example: +``` +from nautobot_ssot.contrib import sort_relationships + +class SourceAdapter(DiffSync): + + sorted_relationships = ( + ("tenant", "relationship", "name"), + ) + + def load(self): + ... + # Primary load logic + ... + sort_relationships(self) +``` + diff --git a/docs/user/modeling.md b/docs/user/modeling.md index d106ad31..cb6ee6bc 100644 --- a/docs/user/modeling.md +++ b/docs/user/modeling.md @@ -167,6 +167,54 @@ Through us defining the model, Nautobot will now be able to dynamically load IP !!! note Although `Interface.ip_addresses` is a generic relation, there is only one content type (i.e. `ipam.ipaddress`) that may be related through this relation, therefore we don't have to specific this in any way. +## Sorting Relationships + +One-to-many (from the one side) and many-to-many relationships are typically stored as lists in the DiffSync object. A problem with false updates during the diff arises when the data source and target load these lists in a different order from each other. + +### The DiffSync Model + +The first step is to add the appropriate metadata to the attributes in the DiffSync Model using annotations. The `FieldType` enum is used to identify what fields in the model are to be sorted. + +```python +from typing import Annotated, List + +from nautobot.dcim.models import Location +from nautobot_ssot.contrib import NautobotModel +from nautobot_ssot.contrib.types import FieldType + + +class TagDict(TypedDict): + name: str + color: str + + +class LocationModel(NautobotModel): + _model = Location + _modelname = "location" + _identifiers = ("name",) + _attributes("tags",) + + name: str + tags: Annotated[List[TagDict], FieldType.SORTED_FIELD] +``` + +A model attribute will not be sorted without the `FieldType.SORTED_FIELD` in the annotation. For lists of simple objects (such as strings, integers, etc.), no additional configuration is required. The sorting process is done automatically. + +### Sorting Dictionaries + +Sorting lists of more complex objects requires an additional configuration to properly sort. In the case of dictionaries, you must specify which key the list of dictionaries should be sorted by. + +The existing DiffSync process uses lists of `TypedDict` objects to represent some one-to-many and many-to-many relationships. Looking at the example from above, we can see that the `TagDict` class doesn't have any specification as to which key entry the list of `TagDict` objects should be sorted by. We'll update the class by adding the `FieldType.SORT_BY` marker as follows: + +```python +... + +class TagDict(TypedDict): + name: Annotated[str, FieldType.SORT_BY] + color: str + +... +``` ## Filtering Objects Loaded From Nautobot diff --git a/nautobot_ssot/contrib/adapter.py b/nautobot_ssot/contrib/adapter.py index a5555509..54757ffc 100644 --- a/nautobot_ssot/contrib/adapter.py +++ b/nautobot_ssot/contrib/adapter.py @@ -7,7 +7,7 @@ from typing import DefaultDict, Dict, FrozenSet, Hashable, Tuple, Type, get_args import pydantic -from diffsync import Adapter +from diffsync import DiffSync from diffsync.exceptions import ObjectCrudException from django.contrib.contenttypes.models import ContentType from django.db.models import Model @@ -34,7 +34,7 @@ ParameterSet = FrozenSet[Tuple[str, Hashable]] -class NautobotAdapter(Adapter): +class NautobotAdapter(DiffSync): """ Adapter for loading data from Nautobot through the ORM. @@ -44,6 +44,7 @@ class NautobotAdapter(Adapter): # This dictionary acts as an ORM cache. _cache: DefaultDict[str, Dict[ParameterSet, Model]] _cache_hits: DefaultDict[str, int] = defaultdict(int) + sorted_relationships = () def __init__(self, *args, job, sync=None, **kwargs): """Instantiate this class, but do not load data immediately from the local system.""" @@ -174,6 +175,8 @@ def load(self): # for this specific model class as well as its children without returning anything. self._load_objects(diffsync_model) + # sort_relationships(self) + def _get_diffsync_class(self, model_name): """Given a model name, return the diffsync class.""" try: diff --git a/nautobot_ssot/contrib/sorting.py b/nautobot_ssot/contrib/sorting.py new file mode 100644 index 00000000..3d3a8be0 --- /dev/null +++ b/nautobot_ssot/contrib/sorting.py @@ -0,0 +1,105 @@ +"""Functions for sorting DiffSync model lists ensuring they are sorted to prevent false actions.""" + +from diffsync import Adapter, DiffSyncModel +from typing_extensions import get_type_hints + +from nautobot_ssot.contrib.types import FieldType + + +def is_sortable_field(attribute_type_hints): + """Checks type hints to verify if field labled as sortable or not.""" + if attribute_type_hints.__name__ != "Annotated" or not hasattr(attribute_type_hints, "__metadata__"): + return False + for metadata in attribute_type_hints.__metadata__: + if metadata == FieldType.SORTED_FIELD: + return True + return False + + +def get_sortable_obj_type(attribute_type_hints): + """Get the object type of a sortable list based on the type hints.""" + if not hasattr(attribute_type_hints, "__args__"): + return None + if not attribute_type_hints.__args__: + return None + attr_type = attribute_type_hints.__args__[0] + if not hasattr(attr_type, "__args__"): + return None + attr_type_args = getattr(attr_type, "__args__") + if attr_type_args: + return attr_type_args[0] + return None + + +def get_sortable_obj_sort_key(sortable_obj_type): + """Get the sort key from a TypedDict type if set in the metadata.""" + content_obj_type_hints = get_type_hints(sortable_obj_type, include_extras=True) + for key, value in content_obj_type_hints.items(): + if not value.__name__ == "Annotated": + continue + for metadata in getattr(value, "__metadata__", ()): + if metadata == FieldType.SORT_BY: + return key + return None + + +def get_sortable_fields_from_model(model: DiffSyncModel): + """Get a list of sortable fields and their sort key from a DiffSync model.""" + sortable_fields = [] + model_type_hints = get_type_hints(model, include_extras=True) + + for attribute_name in model._attributes: # pylint: disable=protected-access + attribute_type_hints = model_type_hints.get(attribute_name) + if not is_sortable_field(attribute_type_hints): + continue + + sortable_obj_type = get_sortable_obj_type(attribute_type_hints) + sort_key = get_sortable_obj_sort_key(sortable_obj_type) + + sortable_fields.append( + { + "attribute": attribute_name, + "sort_key": sort_key, + } + ) + return sortable_fields + + +def sort_diffsync_object(obj, attribute, key): + """Update the sortable attribute in a DiffSync object.""" + sorted_data = None + if key: + sorted_data = sorted( + getattr(obj, attribute), + key=lambda x: x[key], + ) + else: + sorted_data = sorted(getattr(obj, attribute)) + if sorted_data: + setattr(obj, attribute, sorted_data) + return obj + + +def sort_relationships(source: Adapter, target: Adapter): + """Sort relationships based on the metadata defined in the DiffSync model.""" + if not source or not target: + return + # Loop through Top Level entries + for level in getattr(target, "top_level", []): + # Get the DiffSync Model + model = getattr(target, level) + if not model: + continue + + # Get sortable fields from model + sortable_fields = get_sortable_fields_from_model(target) + if not sortable_fields: + continue + + for sortable in sortable_fields: + attribute = sortable["attribute"] + key = sortable["sort_key"] + + for adapter in (source, target): + for obj in adapter.get_all(attribute): + adapter.update(sort_diffsync_object(obj, attribute, key)) diff --git a/nautobot_ssot/contrib/typeddicts.py b/nautobot_ssot/contrib/typeddicts.py index 83269678..a6b08102 100644 --- a/nautobot_ssot/contrib/typeddicts.py +++ b/nautobot_ssot/contrib/typeddicts.py @@ -1,32 +1,34 @@ """Common TypedDict definitions used in Many-to-Many relationships.""" -from typing import TypedDict +from typing import Annotated, TypedDict + +from nautobot_ssot.contrib.types import FieldType class ContentTypeDict(TypedDict): """TypedDict for Django Content Types.""" app_label: str - model: str + model: Annotated[str, FieldType.SORT_BY] class TagDict(TypedDict): """TypedDict for Nautobot Tags.""" - name: str + name: Annotated[str, FieldType.SORT_BY] class LocationDict(TypedDict): """TypedDict for DCIM Locations.""" - name: str + name: Annotated[str, FieldType.SORT_BY] parent__name: str class DeviceDict(TypedDict): """TypedDict for DCIM Devices.""" - name: str + name: Annotated[str, FieldType.SORT_BY] location__name: str tenant__name: str rack__name: str @@ -40,14 +42,14 @@ class DeviceDict(TypedDict): class InterfaceDict(TypedDict): """TypedDict for DCIM INterfaces.""" - name: str + name: Annotated[str, FieldType.SORT_BY] device__name: str class PrefixDict(TypedDict): """TypedDict for IPAM Prefixes.""" - network: str + network: Annotated[str, FieldType.SORT_BY] prefix_length: int namespace__name: str @@ -55,20 +57,20 @@ class PrefixDict(TypedDict): class VLANDict(TypedDict): """TypedDict for IPAM VLANs.""" - vid: int + vid: Annotated[int, FieldType.SORT_BY] vlan_group__name: str class IPAddressDict(TypedDict): """TypedDict for IPAM IP Address.""" - host: str + host: Annotated[str, FieldType.SORT_BY] prefix_length: int class VirtualMachineDict(TypedDict): """TypedDict for IPAM .""" - name: str + name: Annotated[str, FieldType.SORT_BY] cluster__name: str tenant__name: str diff --git a/nautobot_ssot/contrib/types.py b/nautobot_ssot/contrib/types.py index 36d2cbc8..7c03bb1b 100644 --- a/nautobot_ssot/contrib/types.py +++ b/nautobot_ssot/contrib/types.py @@ -8,6 +8,16 @@ from typing import Optional +class FieldType(Enum): + """Enum to specify field types for DiffSynModels and TypedDicts.""" + + # Indicates if a model field is to be sorted + SORTED_FIELD = 1 + + # Indicates what field to sort by for lists of dictionaries + SORT_BY = 2 + + class RelationshipSideEnum(Enum): """This details which side of a custom relationship the model it's defined on is on.""" diff --git a/nautobot_ssot/jobs/base.py b/nautobot_ssot/jobs/base.py index a2d775cf..d7a77415 100644 --- a/nautobot_ssot/jobs/base.py +++ b/nautobot_ssot/jobs/base.py @@ -17,6 +17,7 @@ from nautobot.extras.jobs import BooleanVar, DryRunVar, Job from nautobot_ssot.choices import SyncLogEntryActionChoices +from nautobot_ssot.contrib.sorting import sort_relationships from nautobot_ssot.models import BaseModel, Sync, SyncLogEntry DataMapping = namedtuple("DataMapping", ["source_name", "source_url", "target_name", "target_url"]) @@ -180,6 +181,9 @@ def record_memory_trace(step: str): if memory_profiling: record_memory_trace("target_load") + # Sorting relationships must be done before calculating diffs. + sort_relationships(self.source_adapter, self.target_adapter) + self.logger.info("Calculating diffs...") self.calculate_diff() calculate_diff_time = datetime.now() diff --git a/nautobot_ssot/tests/contrib/__init__.py b/nautobot_ssot/tests/contrib/__init__.py new file mode 100644 index 00000000..fd7c7fde --- /dev/null +++ b/nautobot_ssot/tests/contrib/__init__.py @@ -0,0 +1 @@ +"""Unit tests for contrib.""" diff --git a/nautobot_ssot/tests/contrib/test_sorting.py b/nautobot_ssot/tests/contrib/test_sorting.py new file mode 100644 index 00000000..e2f147ee --- /dev/null +++ b/nautobot_ssot/tests/contrib/test_sorting.py @@ -0,0 +1,132 @@ +"""Unit tests for contrib sorting.""" + +from typing import List, Optional + +from django.test import TestCase +from typing_extensions import Annotated, TypedDict, get_type_hints + +from nautobot_ssot.contrib.sorting import ( + get_sortable_fields_from_model, + get_sortable_obj_sort_key, + get_sortable_obj_type, + is_sortable_field, + sort_diffsync_object, +) +from nautobot_ssot.contrib.types import FieldType +from nautobot_ssot.tests.contrib_base_classes import NautobotTenant as BasicNautobotTenant +from nautobot_ssot.tests.contrib_base_classes import TagDict as BasicTagDict + + +class TagDict(BasicTagDict): + """Many-to-many relationship typed dict explaining which fields are interesting.""" + + id: int + name: Annotated[str, FieldType.SORT_BY] + description: Optional[str] = "" + + +class NautobotTenant(BasicNautobotTenant): + """A updated tenant model for testing the `NautobotModel` base class.""" + + tags: Annotated[List[TagDict], FieldType.SORTED_FIELD] = [] + + +class TestCaseIsSortableFieldFunction(TestCase): + """Tests for `_is_sortable_field` function.""" + + @classmethod + def setUpTestData(cls): + cls.type_hints = get_type_hints(NautobotTenant, include_extras=True) + + def test_non_sortable_field(self): + test = is_sortable_field(self.type_hints["name"]) + self.assertFalse(test) + + def test_sortable_field(self): + test = is_sortable_field(self.type_hints["tags"]) + self.assertTrue(test) + + +class TestGetSortKeyFunction(TestCase): + """Tests for `_get_sortable_obj_key` function.""" + + def test_get_sort_key(self): + test = get_sortable_obj_sort_key(TagDict) + self.assertEqual(test, "name") + + def test_no_sort_key(self): + """Test function with no wort key.""" + + class TestClass(TypedDict): # pylint: disable=missing-class-docstring + id: str + name: str + + test = get_sortable_obj_sort_key(TestClass) + self.assertIsNone(test) + + +class TestCaseGetSortableFieldsFromModelFunction(TestCase): + """Tests for `_get_sortable_fields_from_model` function.""" + + def test_with_sortable_fields(self): + """Test get sortable fields with one sortable field identified.""" + test = get_sortable_fields_from_model(NautobotTenant) + self.assertEqual(len(test), 1) + + def test_without_sortable_fields(self): + """Test get sortable fields with no sortable fields identified.""" + test = get_sortable_fields_from_model(BasicNautobotTenant) + self.assertEqual(len(test), 0) + + +class TestSortDiffSyncObjectFunction(TestCase): + """Tests for `_sort_diffsync_object` function.""" + + @classmethod + def setUpTestData(cls): + cls.obj_1 = NautobotTenant( + name="", + description="DiffSync object with a sortable field.", + tags=[ + TagDict( + id=1, + name="b", + description="", + ), + TagDict( + id=2, + name="a", + description="", + ), + ], + ) + + def test_with_sortable_field(self): + """Test to make sure `_sort_diffsync_object` sorts attribute.""" + self.assertEqual( + self.obj_1.tags[0]["name"], + "b", + msg="List of `TagDict` entries must start with `name='b'` to verify proper sorting.", + ) + test = sort_diffsync_object(self.obj_1, "tags", "name") + self.assertEqual(test.tags[0]["name"], "a") + + +class TestGetSortableObjectTypeFunction(TestCase): + """Tests for `_get_sortable_object_type` function.""" + + def test_get_sortable_object_type(self): + """Test to validate `_get_sortable_obj_type` function returns correct object type.""" + type_hints = get_type_hints(NautobotTenant, include_extras=True) + test = get_sortable_obj_type(type_hints.get("tags")) + self.assertTrue(test == TagDict) + + def test_get_nonsortable_object_type(self): + """Test to validate `_get_sortable_obj_type` function returns None.""" + type_hints = get_type_hints(BasicNautobotTenant, include_extras=True) + test = get_sortable_obj_type(type_hints["tags"]) + self.assertIsNone(test) + + +class TestSortRelationships(TestCase): + """Tests for `sort_relationships` function.""" diff --git a/nautobot_ssot/tests/contrib_base_classes.py b/nautobot_ssot/tests/contrib_base_classes.py index be9af316..239afa9d 100644 --- a/nautobot_ssot/tests/contrib_base_classes.py +++ b/nautobot_ssot/tests/contrib_base_classes.py @@ -528,3 +528,21 @@ class ProviderModelCustomRelationship(NautobotModel): tenants: Annotated[ List[TenantDict], CustomRelationshipAnnotation(name="Test Relationship", side=RelationshipSideEnum.SOURCE) ] = [] + + +class CustomRelationshipTypedDict(TypedDict): + """Typed dictionary for testing custom many to many relationships.""" + + name: str + + +class TenantModelCustomManyTomanyRelationship(NautobotModel): + """Model for testing sorting custom relationships.""" + + _model = tenancy_models.Tenant + _modelname = "tenant" + _identifiers = ("name",) + _attributes = ("tenants",) + + name: str + tenants: List[TenantDict] = [] diff --git a/nautobot_ssot/tests/test_contrib_adapter.py b/nautobot_ssot/tests/test_contrib_adapter.py index 3c21e86f..321cd931 100644 --- a/nautobot_ssot/tests/test_contrib_adapter.py +++ b/nautobot_ssot/tests/test_contrib_adapter.py @@ -25,6 +25,7 @@ NautobotTenant, NautobotTenantGroup, ProviderModelCustomRelationship, + TenantModelCustomManyTomanyRelationship, TenantModelCustomRelationship, TestAdapter, TestCaseWithDeviceData, @@ -353,3 +354,17 @@ class Adapter(NautobotAdapter): self.assertEqual(amount_of_vlans, len(diffsync_vlan_group.vlans)) for vlan in diffsync_vlan_group.vlans: self.assertEqual(location.name, vlan["location__name"]) + + +class AdapterCustomRelationshipSortingTest(NautobotAdapter): + """Adapter for testing custom many-to-many relationship sorting.""" + + top_level = ["tenant"] + tenant = TenantModelCustomManyTomanyRelationship + sorted_relationships = ( + ( + "tenant", + "tenants", + "name", + ), + )