Remove unnecessary minimal_model code and use LazyForeignKey for both logic branches in single object fields

Author: bctiemann

netbox_custom_objects/field_types.py

@@ -14,14 +14,19 @@
 from django.utils.translation import gettext_lazy as _
 from extras.choices import CustomFieldTypeChoices, CustomFieldUIEditableChoices
 from utilities.api import get_serializer_for_model
-from utilities.forms.fields import (CSVChoiceField, CSVModelChoiceField,
-                                    CSVModelMultipleChoiceField, CSVMultipleChoiceField,
-                                    DynamicChoiceField,
-                                    DynamicMultipleChoiceField, JSONField,
-                                    LaxURLField)
+from utilities.forms.fields import (
+    CSVChoiceField, CSVModelChoiceField,
+    CSVModelMultipleChoiceField, CSVMultipleChoiceField,
+    DynamicChoiceField, DynamicModelChoiceField,
+    DynamicModelMultipleChoiceField,
+    DynamicMultipleChoiceField, JSONField,
+    LaxURLField,
+)
 from utilities.forms.utils import add_blank_choice
-from utilities.forms.widgets import (APISelect, APISelectMultiple, DatePicker,
-                                     DateTimePicker)
+from utilities.forms.widgets import (
+    APISelect, APISelectMultiple, DatePicker,
+    DateTimePicker,
+)
 from utilities.templatetags.builtins.filters import linkify, render_markdown
 
 from netbox_custom_objects.constants import APP_LABEL
@@ -73,12 +78,6 @@ def get_filterform_field(self, field, **kwargs):
     def get_form_field(self, field, **kwargs):
         raise NotImplementedError
 
-    def _filter_field_kwargs(self, kwargs):
-        """
-        Filter out custom parameters that shouldn't be passed to Django field constructors.
-        """
-        return {k: v for k, v in kwargs.items() if not k.startswith('_') and k != 'generating_models'}
-
     def _safe_kwargs(self, **kwargs):
         """
         Create a safe kwargs dict that can be passed to Django field constructors.
@@ -389,8 +388,7 @@ def get_model_field(self, field, **kwargs):
         to_model = content_type.model
 
         # Extract our custom parameters and keep only Django field parameters
-        generating_models = kwargs.pop('_generating_models', getattr(self, '_generating_models', set()))
-        field_kwargs = {k: v for k, v in kwargs.items() if not k.startswith('_')}
+        field_kwargs = self._safe_kwargs(**kwargs)
         field_kwargs.update({"default": field.default, "unique": field.unique})
 
         # Handle self-referential fields by using string references
@@ -402,35 +400,15 @@ def get_model_field(self, field, **kwargs):
             )
             custom_object_type = CustomObjectType.objects.get(pk=custom_object_type_id)
 
-            # Check if this is a self-referential field
-            if custom_object_type.id == field.custom_object_type.id:
-                # For self-referential fields, use LazyForeignKey to defer resolution
-                model_name = f"{APP_LABEL}.{custom_object_type.get_table_model_name(custom_object_type.id)}"
-                f = LazyForeignKey(
-                    model_name,
-                    null=True,
-                    blank=True,
-                    on_delete=models.CASCADE,
-                    **field_kwargs
-                )
-                return f
-            else:
-                # For cross-referential fields, use skip_object_fields to avoid infinite loops
-                # Check if we're in a recursion situation using the parameter or stored attribute
-                if generating_models and custom_object_type.id in generating_models:
-                    # We're in a circular reference, don't call get_model() to prevent recursion
-                    # Use a string reference instead
-                    model_name = f"{APP_LABEL}.{custom_object_type.get_table_model_name(custom_object_type.id)}"
-                    f = models.ForeignKey(
-                        model_name,
-                        null=True,
-                        blank=True,
-                        on_delete=models.CASCADE,
-                        **field_kwargs
-                    )
-                    return f
-                else:
-                    model = custom_object_type.get_model(skip_object_fields=True)
+            model_name = f"{APP_LABEL}.{custom_object_type.get_table_model_name(custom_object_type.id)}"
+            f = LazyForeignKey(
+                model_name,
+                null=True,
+                blank=True,
+                on_delete=models.CASCADE,
+                **field_kwargs
+            )
+            return f
         else:
             # to_model = content_type.model_class()._meta.object_name
             to_ct = f"{content_type.app_label}.{to_model}"
@@ -464,7 +442,6 @@ def get_form_field(self, field, for_csv_import=False, **kwargs):
             if generating_models and custom_object_type.id in generating_models:
                 # We're in a circular reference, don't call get_model() to prevent recursion
                 # Use a minimal approach or return a basic field
-                from utilities.forms.fields import DynamicModelChoiceField
                 return DynamicModelChoiceField(
                     queryset=custom_object_type.get_model(skip_object_fields=True).objects.all(),
                     required=field.required,
@@ -488,7 +465,6 @@ def get_form_field(self, field, for_csv_import=False, **kwargs):
                 to_field_name=to_field_name,
             )
         else:
-            from utilities.forms.fields import DynamicModelChoiceField
             field_class = DynamicModelChoiceField
             return field_class(
                 queryset=model.objects.all(),
@@ -523,8 +499,7 @@ def after_model_generation(self, instance, model, field_name):
         This ensures that self-referential fields point to the correct model class.
         """
         # Check if this field has a resolution method
-        resolve_method = getattr(model, f'_resolve_{field_name}_model', None)
-        if resolve_method:
+        if resolve_method := getattr(model, f'_resolve_{field_name}_model', None):
             resolve_method(model)
 
 
@@ -790,7 +765,6 @@ def get_form_field(self, field, for_csv_import=False, **kwargs):
             if generating_models and custom_object_type.id in generating_models:
                 # We're in a circular reference, don't call get_model() to prevent recursion
                 # Use a minimal approach or return a basic field
-                from utilities.forms.fields import DynamicModelMultipleChoiceField
                 return DynamicModelMultipleChoiceField(
                     queryset=custom_object_type.get_model(skip_object_fields=True).objects.all(),
                     required=field.required,
@@ -814,7 +788,6 @@ def get_form_field(self, field, for_csv_import=False, **kwargs):
                 to_field_name=to_field_name,
             )
         else:
-            from utilities.forms.fields import DynamicModelMultipleChoiceField
             field_class = DynamicModelMultipleChoiceField
             return field_class(
                 queryset=model.objects.all(),
@@ -839,8 +812,6 @@ def get_serializer_field(self, field, **kwargs):
         if related_model_class._meta.app_label == APP_LABEL:
             from netbox_custom_objects.api.serializers import get_serializer_class
             serializer = get_serializer_class(related_model_class, skip_object_fields=True)
-        # if not related_model_class:
-        #     raise NotImplementedError("Custom object serializers not implemented")
         else:
             serializer = get_serializer_for_model(related_model_class)
         return serializer(required=field.required, nested=True, many=True)

netbox_custom_objects/models.py

@@ -378,7 +378,6 @@ def _fetch_and_generate_field_attrs(
                             model_name = content_type.model
 
                             # Try to extract the ID from the model name
-                            import re
                             id_match = re.search(r'table(\d+)model', model_name, re.IGNORECASE)
                             if id_match:
                                 custom_object_type_id = int(id_match.group(1))
@@ -539,15 +538,6 @@ def get_model(
 
         if _generating_models is None:
             _generating_models = CustomObjectType._global_generating_models
-        # Check if we're already generating this model (circular reference)
-        if self.id in _generating_models:
-            # We have a circular reference, return a minimal model to break the cycle
-            return self._get_minimal_model(_generating_models)
-
-        # Check recursion depth to prevent infinite loops
-        if len(_generating_models) > 1:
-            # We're too deep in recursion, return a minimal model
-            return self._get_minimal_model(_generating_models)
 
         # Add this model to the set of models being generated
         _generating_models.add(self.id)
@@ -667,58 +657,6 @@ def wrapped_post_through_setup(self, cls):
 
         return model
 
-    def _get_minimal_model(self, _generating_models=None):
-        """
-        Creates a minimal model with only basic fields when recursion is detected.
-        This breaks infinite recursion cycles by providing a basic model structure.
-        """
-        model_name = self.get_table_model_name(self.pk)
-
-        meta = type(
-            "Meta",
-            (),
-            {
-                "apps": apps,
-                "managed": False,
-                "db_table": self.get_database_table_name(),
-                "app_label": APP_LABEL,
-                "ordering": ["id"],
-                "verbose_name": self.get_verbose_name(),
-                "verbose_name_plural": self.get_verbose_name_plural(),
-            },
-        )
-
-        attrs = {
-            "Meta": meta,
-            "__module__": "database.models",
-            "custom_object_type": self,
-            "custom_object_type_id": self.id,
-        }
-
-        # Don't call field generation methods to avoid further recursion
-        # Just create a basic model with no custom fields
-        # attrs.update(**field_attrs)
-
-        # Create the minimal model
-        model = type(
-            str(model_name),
-            (CustomObject, models.Model),
-            attrs,
-        )
-
-        # Register the model with Django's app registry
-        try:
-            existing_model = apps.get_model(APP_LABEL, model_name)
-            if existing_model is not model:
-                model = existing_model
-        except LookupError:
-            apps.register_model(APP_LABEL, model)
-
-        # Cache this minimal model
-        self._model_cache[self.id] = model
-
-        return model
-
     def create_model(self):
         # Get the model and ensure it's registered
         model = self.get_model()