From c2450080877b535992abea8b18f808cd31c180e8 Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 20 Apr 2026 12:58:11 -0700 Subject: [PATCH 01/25] #404 add branching support --- netbox_custom_objects/__init__.py | 9 ++ netbox_custom_objects/branching.py | 191 +++++++++++++++++++++++++++ netbox_custom_objects/field_types.py | 5 +- netbox_custom_objects/models.py | 41 ++++-- 4 files changed, 236 insertions(+), 10 deletions(-) create mode 100644 netbox_custom_objects/branching.py diff --git a/netbox_custom_objects/__init__.py b/netbox_custom_objects/__init__.py index 2e862e9..598cda2 100644 --- a/netbox_custom_objects/__init__.py +++ b/netbox_custom_objects/__init__.py @@ -175,6 +175,15 @@ def ready(self): pre_migrate.connect(_migration_started) post_migrate.connect(_migration_finished) + # Wire into the netbox-branching lifecycle when that plugin is installed. + # Import is lazy so the plugin remains functional without branching. + try: + from netbox_branching.signals import post_migrate as branching_post_migrate + from netbox_custom_objects.branching import on_branch_migrated + branching_post_migrate.connect(on_branch_migrated) + except ImportError: + pass + # Patch ObjectSelectorView to support dynamically-generated custom object models _patch_object_selector_view() diff --git a/netbox_custom_objects/branching.py b/netbox_custom_objects/branching.py new file mode 100644 index 0000000..63832f4 --- /dev/null +++ b/netbox_custom_objects/branching.py @@ -0,0 +1,191 @@ +""" +Branching support for NetBox Custom Objects. + +When netbox-branching runs ``Branch.migrate()``, the Django migration pass only +handles normal app migrations. Custom object field changes (add/alter/remove +column) are applied directly via the schema editor and are therefore invisible +to Django's migration framework. + +To close that gap, ``on_branch_migrated`` fires after the migration pass and +reconciles each custom object type's physical schema in the branch against the +current field definitions in main. The reconciliation mirrors what +``CustomObjectTypeField.save()`` already does when detecting old-vs-new field +state: it compares the branch's ``CustomObjectTypeField`` rows (snapshot from +provision time) with main's current rows, matched by primary key, and calls +``add_field`` / ``alter_field`` / ``remove_field`` for any differences. + +``CustomObjectTypeField`` has ``ChangeLoggingMixin``, so its rows are replicated +into the branch schema at provision time — the same source-of-truth mechanism +used for all other branchable models. +""" + +import logging + +from django.db import connections, models + +logger = logging.getLogger('netbox_custom_objects.branching') + + +def _fields_schema_differ(branch_f, main_f): + """ + Return True if the two ``CustomObjectTypeField`` instances differ in any + attribute that affects the physical DB column, meaning an ALTER TABLE is + needed to bring the branch schema up to date. + """ + return ( + branch_f.name != main_f.name + or branch_f.type != main_f.type + or branch_f.required != main_f.required + or branch_f.default != main_f.default + or branch_f.unique != main_f.unique + or branch_f.related_object_type_id != main_f.related_object_type_id + ) + + +def on_branch_migrated(sender, branch, user, **kwargs): + """ + Reconcile each custom object type's physical schema in the branch against + the current field definitions in main. + + For each ``CustomObjectType`` whose table exists in the branch schema: + - Fields present in main but absent from the branch → ``add_field`` + - Fields absent from main but present in the branch → ``remove_field`` + - Fields present in both with differing definitions → ``alter_field`` + + Matching is done by primary key so renames are detected correctly (the pk + exists in both, with different ``name`` values) rather than being treated + as an unrelated delete + add. + """ + from extras.choices import CustomFieldTypeChoices + from netbox_branching.utilities import activate_branch + from netbox_custom_objects.constants import APP_LABEL + from netbox_custom_objects.field_types import FIELD_TYPE_CLASS + from netbox_custom_objects.models import CustomObjectType + from netbox_custom_objects.utilities import generate_model + + branch_connection = connections[branch.connection_name] + + with branch_connection.cursor() as cursor: + branch_tables = branch_connection.introspection.table_names(cursor) + + for cot in CustomObjectType.objects.all(): + db_table = cot.get_database_table_name() + if db_table not in branch_tables: + # Table absent — COT was created after this branch was provisioned + # and the branch hasn't been synced yet. Skip; the user needs to + # sync first to pull in the new table. + logger.debug('Skipping %s — table %r not in branch schema', cot, db_table) + continue + + # Main's current field definitions (queried from the public schema since + # active_branch is not set here). + main_fields = { + f.pk: f + for f in cot.fields.select_related('related_object_type', 'choice_set').all() + } + + # Branch's field snapshot (as of provision time, or last sync). + with activate_branch(branch): + branch_fields = { + f.pk: f + for f in cot.fields.select_related('related_object_type', 'choice_set').all() + } + + main_pks = set(main_fields) + branch_pks = set(branch_fields) + + to_add = [main_fields[pk] for pk in main_pks - branch_pks] + to_remove = [branch_fields[pk] for pk in branch_pks - main_pks] + to_alter = [ + (branch_fields[pk], main_fields[pk]) # (old, new) + for pk in main_pks & branch_pks + if _fields_schema_differ(branch_fields[pk], main_fields[pk]) + ] + + if not (to_add or to_remove or to_alter): + continue + + logger.info( + 'Migrating branch schema for %s: %d add, %d remove, %d alter', + cot, len(to_add), len(to_remove), len(to_alter), + ) + + model = cot.get_model() + + with branch_connection.schema_editor() as schema_editor: + + # ── add_field ──────────────────────────────────────────────────── + for fi in to_add: + logger.debug('add_field %r on %s', fi.name, cot) + ft = FIELD_TYPE_CLASS[fi.type]() + mf = ft.get_model_field(fi) + mf.contribute_to_class(model, fi.name) + schema_editor.add_field(model, mf) + if fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: + ft.create_m2m_table(fi, model, fi.name, schema_conn=branch_connection) + + # ── remove_field ───────────────────────────────────────────────── + for fi in to_remove: + logger.debug('remove_field %r on %s', fi.name, cot) + ft = FIELD_TYPE_CLASS[fi.type]() + mf = ft.get_model_field(fi) + mf.contribute_to_class(model, fi.name) + if fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: + through_table = f'custom_objects_{cot.pk}_{fi.name}' + if through_table in branch_tables: + ThroughMeta = type( + 'Meta', (), + {'db_table': through_table, 'app_label': APP_LABEL, 'managed': True}, + ) + through_model = type( + f'_TempThrough_{through_table}', + (models.Model,), + {'Meta': ThroughMeta, '__module__': 'netbox_custom_objects.models'}, + ) + schema_editor.delete_model(through_model) + schema_editor.remove_field(model, mf) + + # ── alter_field ────────────────────────────────────────────────── + for old_fi, new_fi in to_alter: + logger.debug( + 'alter_field %r → %r on %s', + old_fi.name, new_fi.name, cot, + ) + old_mf = FIELD_TYPE_CLASS[old_fi.type]().get_model_field(old_fi) + new_mf = FIELD_TYPE_CLASS[new_fi.type]().get_model_field(new_fi) + old_mf.contribute_to_class(model, old_fi.name) + new_mf.contribute_to_class(model, new_fi.name) + + # When a MULTIOBJECT field is renamed, the through table must + # be renamed first (same logic as CustomObjectTypeField.save()). + if ( + new_fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT + and old_fi.name != new_fi.name + ): + old_through = f'custom_objects_{cot.pk}_{old_fi.name}' + new_through = f'custom_objects_{cot.pk}_{new_fi.name}' + if old_through in branch_tables: + OldThroughMeta = type( + 'Meta', (), + {'db_table': old_through, 'app_label': APP_LABEL, 'managed': True}, + ) + old_through_model = generate_model( + f'_TempOldThrough_{old_through}', + (models.Model,), + { + '__module__': 'netbox_custom_objects.models', + 'Meta': OldThroughMeta, + 'id': models.AutoField(primary_key=True), + 'source': models.ForeignKey( + model, on_delete=models.CASCADE, + db_column='source_id', related_name='+', + ), + 'target': models.ForeignKey( + model, on_delete=models.CASCADE, + db_column='target_id', related_name='+', + ), + }, + ) + schema_editor.alter_db_table(old_through_model, old_through, new_through) + + schema_editor.alter_field(model, old_mf, new_mf) diff --git a/netbox_custom_objects/field_types.py b/netbox_custom_objects/field_types.py index c89cca3..83d5e2a 100644 --- a/netbox_custom_objects/field_types.py +++ b/netbox_custom_objects/field_types.py @@ -935,11 +935,12 @@ def after_model_generation(self, instance, model, field_name): target_field.remote_field.model = to_model target_field.related_model = to_model - def create_m2m_table(self, instance, model, field_name): + def create_m2m_table(self, instance, model, field_name, schema_conn=None): """ Creates the actual M2M table after models are fully generated """ - from django.db import connection + from django.db import connection as default_connection + connection = schema_conn if schema_conn is not None else default_connection # Get the field instance field = model._meta.get_field(field_name) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index cc0cf58..577b87b 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -66,6 +66,25 @@ class UniquenessConstraintTestError(Exception): USER_TABLE_DATABASE_NAME_PREFIX = "custom_objects_" +def _get_schema_connection(): + """ + Return the active branch's DB connection when called within a branch context, + otherwise return the default (main-schema) connection. + + Used so that schema-editor operations (add/alter/remove column) target the + correct PostgreSQL schema without requiring every call-site to be branch-aware. + """ + try: + from netbox_branching.contextvars import active_branch + branch = active_branch.get() + if branch is not None: + from django.db import connections + return connections[branch.connection_name] + except ImportError: + pass + return connection + + class CustomObject( BookmarksMixin, ChangeLoggingMixin, @@ -681,8 +700,6 @@ def create_model(self): # Ensure the ContentType exists and is immediately available features = get_model_features(model) - if 'branching' in features: - features.remove('branching') self.object_type.features = features self.object_type.public = True self.object_type.save() @@ -1608,16 +1625,21 @@ def through_model_name(self): def save(self, *args, **kwargs): is_new = self._state.adding + + # Use the branch connection when operating inside a branch so that schema + # editor operations target the branch schema rather than main. + schema_conn = _get_schema_connection() + field_type = FIELD_TYPE_CLASS[self.type]() model_field = field_type.get_model_field(self) model = self.custom_object_type.get_model() model_field.contribute_to_class(model, self.name) - with connection.schema_editor() as schema_editor: + with schema_conn.schema_editor() as schema_editor: if self._state.adding: schema_editor.add_field(model, model_field) if self.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: - field_type.create_m2m_table(self, model, self.name) + field_type.create_m2m_table(self, model, self.name, schema_conn=schema_conn) else: old_field = field_type.get_model_field(self.original) old_field.contribute_to_class(model, self._original_name) @@ -1632,8 +1654,8 @@ def save(self, *args, **kwargs): new_through_table_name = self.through_table_name # Check if old through table exists - with connection.cursor() as cursor: - tables = connection.introspection.table_names(cursor) + with schema_conn.cursor() as cursor: + tables = schema_conn.introspection.table_names(cursor) old_table_exists = old_through_table_name in tables if old_table_exists: @@ -1709,7 +1731,7 @@ def save(self, *args, **kwargs): ) else: # No old table exists, create the new through table - field_type.create_m2m_table(self, model, self.name) + field_type.create_m2m_table(self, model, self.name, schema_conn=schema_conn) # Alter the field normally (this updates the field definition) schema_editor.alter_field(model, old_field, model_field) @@ -1762,12 +1784,15 @@ def ensure_constraint(): transaction.on_commit(lambda: ReindexCustomObjectTypeJob.enqueue(cot_id=_cot_id)) def delete(self, *args, **kwargs): + # Use the branch connection when operating inside a branch. + schema_conn = _get_schema_connection() + field_type = FIELD_TYPE_CLASS[self.type]() model_field = field_type.get_model_field(self) model = self.custom_object_type.get_model() model_field.contribute_to_class(model, self.name) - with connection.schema_editor() as schema_editor: + with schema_conn.schema_editor() as schema_editor: if self.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: apps = model._meta.apps through_model = apps.get_model(APP_LABEL, self.through_model_name) From 81565b6154c1a16c7fb6f8b2ff6fcfaaad68337e Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 20 Apr 2026 13:21:21 -0700 Subject: [PATCH 02/25] #404 check for branch drift --- netbox_custom_objects/__init__.py | 14 ++++- netbox_custom_objects/branching.py | 95 ++++++++++++++++++++++++++++++ netbox_custom_objects/models.py | 36 +++++++++++ 3 files changed, 144 insertions(+), 1 deletion(-) diff --git a/netbox_custom_objects/__init__.py b/netbox_custom_objects/__init__.py index 598cda2..392aa25 100644 --- a/netbox_custom_objects/__init__.py +++ b/netbox_custom_objects/__init__.py @@ -178,9 +178,13 @@ def ready(self): # Wire into the netbox-branching lifecycle when that plugin is installed. # Import is lazy so the plugin remains functional without branching. try: + from django.db.models.signals import post_delete, post_save from netbox_branching.signals import post_migrate as branching_post_migrate - from netbox_custom_objects.branching import on_branch_migrated + from netbox_custom_objects.branching import on_branch_migrated, on_custom_object_field_changed + from netbox_custom_objects.models import CustomObjectTypeField branching_post_migrate.connect(on_branch_migrated) + post_save.connect(on_custom_object_field_changed, sender=CustomObjectTypeField) + post_delete.connect(on_custom_object_field_changed, sender=CustomObjectTypeField) except ImportError: pass @@ -213,6 +217,14 @@ def ready(self): super().ready() return + # Scan for branches with pre-existing custom object schema drift + # (covers upgrades and any changes made while the app was offline). + try: + from netbox_custom_objects.branching import check_pending_branch_migrations + check_pending_branch_migrations() + except Exception: + pass + super().ready() def get_model(self, model_name, require_ready=True): diff --git a/netbox_custom_objects/branching.py b/netbox_custom_objects/branching.py index 63832f4..06db203 100644 --- a/netbox_custom_objects/branching.py +++ b/netbox_custom_objects/branching.py @@ -26,6 +26,101 @@ logger = logging.getLogger('netbox_custom_objects.branching') +def check_pending_branch_migrations(): + """ + Scan all READY branches at startup and mark any that have custom object + schema drift as PENDING_MIGRATIONS. + + This catches drift that pre-dates the signal handler — for example, after + upgrading the plugin on an instance that already has branches, or if field + changes were applied while the application was not running. + + Called once from ``CustomObjectsPluginConfig.ready()`` after the DB is + confirmed ready. + """ + try: + from netbox_branching.choices import BranchStatusChoices + from netbox_branching.models import Branch + from netbox_custom_objects.models import CustomObjectType + except ImportError: + return + + ready_branches = list(Branch.objects.filter(status=BranchStatusChoices.READY)) + if not ready_branches: + return + + cots = list(CustomObjectType.objects.all()) + if not cots: + return + + to_update = [] + for branch in ready_branches: + branch_connection = connections[branch.connection_name] + with branch_connection.cursor() as cursor: + branch_tables = branch_connection.introspection.table_names(cursor) + + for cot in cots: + if cot.get_database_table_name() not in branch_tables: + continue + if cot.has_branch_schema_drift(branch): + branch.status = BranchStatusChoices.PENDING_MIGRATIONS + to_update.append(branch) + break # One drifted COT is enough — no need to check the rest + + if to_update: + Branch.objects.bulk_update(to_update, ['status']) + logger.info( + 'Marked %d branch(es) as PENDING_MIGRATIONS at startup due to custom object schema drift', + len(to_update), + ) + + +def on_custom_object_field_changed(sender, instance, **kwargs): + """ + Mark any READY branches that contain the affected custom object type's table + as PENDING_MIGRATIONS when a ``CustomObjectTypeField`` is created, modified, + or deleted in the main schema. + + This surfaces the pending state in the branching UI exactly like a normal + Django-migration, prompting users to click "Migrate Branch". That action + calls ``Branch.migrate()``, which fires ``on_branch_migrated`` and reconciles + the physical column differences. + + Skipped when the change happens inside a branch context — the field edit only + affects that branch's schema, not main, so no other branches need updating. + """ + try: + from netbox_branching.contextvars import active_branch + if active_branch.get() is not None: + return + except ImportError: + return + + from netbox_branching.choices import BranchStatusChoices + from netbox_branching.models import Branch + + cot = instance.custom_object_type + db_table = cot.get_database_table_name() + + ready_branches = list(Branch.objects.filter(status=BranchStatusChoices.READY)) + to_update = [] + for branch in ready_branches: + branch_connection = connections[branch.connection_name] + with branch_connection.cursor() as cursor: + branch_tables = branch_connection.introspection.table_names(cursor) + if db_table in branch_tables: + branch.status = BranchStatusChoices.PENDING_MIGRATIONS + to_update.append(branch) + + if to_update: + Branch.objects.bulk_update(to_update, ['status']) + logger.info( + 'Marked %d branch(es) as PENDING_MIGRATIONS due to field changes on %s', + len(to_update), + cot, + ) + + def _fields_schema_differ(branch_f, main_f): """ Return True if the two ``CustomObjectTypeField`` instances differ in any diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 577b87b..12e9aff 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -710,6 +710,42 @@ def create_model(self): get_serializer_class(model) self.register_custom_object_search_index(model) + def has_branch_schema_drift(self, branch) -> bool: + """ + Return True if this custom object type's physical schema in *branch* + differs from the current field definitions in main. + + Drift means at least one of: + - A field exists in main but not in the branch snapshot (needs add_field) + - A field exists in the branch snapshot but not in main (needs remove_field) + - A field exists in both but has a different name, type, or constraint + that affects the DB column (needs alter_field) + + Returns False when netbox-branching is not installed. + """ + try: + from netbox_branching.utilities import activate_branch + except ImportError: + return False + + main_fields = {f.pk: f for f in self.fields.all()} + with activate_branch(branch): + branch_fields = {f.pk: f for f in self.fields.all()} + + main_pks = set(main_fields) + branch_pks = set(branch_fields) + + if main_pks != branch_pks: + return True + + def _schema_key(f): + return (f.name, f.type, f.required, f.default, f.unique, f.related_object_type_id) + + return any( + _schema_key(branch_fields[pk]) != _schema_key(main_fields[pk]) + for pk in main_pks + ) + def save(self, *args, **kwargs): needs_db_create = self._state.adding From 3f5e92502945d85c89832da396e8df1dcccfbeb7 Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 20 Apr 2026 14:31:12 -0700 Subject: [PATCH 03/25] remove branch warning --- README.md | 14 ---------- netbox_custom_objects/api/views.py | 9 ------ ...omobjecttypefield_related_name_and_more.py | 2 +- netbox_custom_objects/models.py | 28 ++++++++++++------- .../custom_object_bulk_edit.html | 6 +--- .../custom_object_bulk_import.html | 10 ++----- .../customobject_edit.html | 10 ++----- .../inc/branch_warning.html | 12 -------- netbox_custom_objects/utilities.py | 15 ---------- netbox_custom_objects/views.py | 4 --- 10 files changed, 26 insertions(+), 84 deletions(-) delete mode 100644 netbox_custom_objects/templates/netbox_custom_objects/inc/branch_warning.html diff --git a/README.md b/README.md index 1a3cf5f..68796f7 100644 --- a/README.md +++ b/README.md @@ -32,20 +32,6 @@ $ ./manage.py migrate sudo systemctl restart netbox netbox-rq ``` -> [!NOTE] -> If you are using NetBox Custom Objects with NetBox Branching, you need to insert the following into your `configuration.py`. See the docs for a full description of how NetBox Custom Objects currently works with NetBox Branching. - -``` -PLUGINS_CONFIG = { - 'netbox_branching': { - 'exempt_models': [ - 'netbox_custom_objects.customobjecttype', - 'netbox_custom_objects.customobjecttypefield', - ], - }, -} -``` - ## Known Limitations NetBox Custom Objects is now Generally Available which means you can use it in production and migrations to future versions will work. There are many upcoming features including GraphQL support - the best place to see what's on the way is the [issues](https://github.com/netboxlabs/netbox-custom-objects/issues) list on the GitHub repository. diff --git a/netbox_custom_objects/api/views.py b/netbox_custom_objects/api/views.py index 1729377..274be01 100644 --- a/netbox_custom_objects/api/views.py +++ b/netbox_custom_objects/api/views.py @@ -11,13 +11,8 @@ from netbox_custom_objects.filtersets import get_filterset_class from netbox_custom_objects.models import CustomObjectType, CustomObjectTypeField -from netbox_custom_objects.utilities import is_in_branch - from . import serializers -# Constants -BRANCH_ACTIVE_ERROR_MESSAGE = _("Please switch to the main branch to perform this operation.") - class RootView(APIRootView): def get_view_name(self): @@ -71,13 +66,9 @@ def list(self, request, *args, **kwargs): return super().list(request, *args, **kwargs) def create(self, request, *args, **kwargs): - if is_in_branch(): - raise ValidationError(BRANCH_ACTIVE_ERROR_MESSAGE) return super().create(request, *args, **kwargs) def update(self, request, *args, **kwargs): - if is_in_branch(): - raise ValidationError(BRANCH_ACTIVE_ERROR_MESSAGE) # Replicate DRF's UpdateModelMixin.update() so we can snapshot the instance # before the serializer is constructed. Calling super().update() would invoke # get_object() a second time and return a fresh, un-snapshotted instance. diff --git a/netbox_custom_objects/migrations/0005_customobjecttypefield_related_name_and_more.py b/netbox_custom_objects/migrations/0005_customobjecttypefield_related_name_and_more.py index 5251280..8eb63d6 100644 --- a/netbox_custom_objects/migrations/0005_customobjecttypefield_related_name_and_more.py +++ b/netbox_custom_objects/migrations/0005_customobjecttypefield_related_name_and_more.py @@ -10,7 +10,7 @@ class Migration(migrations.Migration): dependencies = [ ('core', '0021_job_queue_name'), ('extras', '0134_owner'), - ('netbox_custom_objects', '0004_customobjecttype_group_name'), + ('netbox_custom_objects', '0005_customobjecttypefield_context'), ] operations = [ diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index f34b9da..22ceb92 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -714,7 +714,7 @@ def create_model(self): self.object_type.public = True self.object_type.save() - with connection.schema_editor() as schema_editor: + with _get_schema_connection().schema_editor() as schema_editor: schema_editor.create_model(model) get_serializer_class(model) @@ -772,20 +772,31 @@ def delete(self, *args, **kwargs): self.clear_model_cache(self.id) model = self.get_model() + schema_conn = _get_schema_connection() + in_branch = schema_conn is not connection # Delete all CustomObjectTypeFields that reference this CustomObjectType for field in CustomObjectTypeField.objects.filter(related_object_type=self.object_type): field.delete() object_type = ObjectType.objects.get_for_model(model) - ObjectChange.objects.filter(changed_object_type=object_type).delete() + + # ObjectChange and ObjectType records live in the main schema. Only clean + # them up when operating outside a branch; inside a branch they belong to + # main and must not be touched until the deletion is merged. + if not in_branch: + ObjectChange.objects.filter(changed_object_type=object_type).delete() + super().delete(*args, **kwargs) - # Temporarily disconnect the pre_delete handler to skip the ObjectType deletion - # TODO: Remove this disconnect/reconnect after ObjectType has been exempted from handle_deleted_object - pre_delete.disconnect(handle_deleted_object) - object_type.delete() - with connection.schema_editor() as schema_editor: + if not in_branch: + # Temporarily disconnect the pre_delete handler to skip the ObjectType deletion + # TODO: Remove this disconnect/reconnect after ObjectType has been exempted from handle_deleted_object + pre_delete.disconnect(handle_deleted_object) + object_type.delete() + pre_delete.connect(handle_deleted_object) + + with schema_conn.schema_editor() as schema_editor: schema_editor.delete_model(model) # Unregister the model and its through-models from Django's app registry so @@ -811,9 +822,6 @@ def delete(self, *args, **kwargs): # Re-clear the model cache to remove re-cached model from get_model. self.clear_model_cache(self.id) - # Reconnect the pre_delete handler after all cleanup is done. - pre_delete.connect(handle_deleted_object) - @receiver(post_save, sender=CustomObjectType) def custom_object_type_post_save_handler(sender, instance, created, **kwargs): diff --git a/netbox_custom_objects/templates/netbox_custom_objects/custom_object_bulk_edit.html b/netbox_custom_objects/templates/netbox_custom_objects/custom_object_bulk_edit.html index 2ed1a51..7371ebe 100644 --- a/netbox_custom_objects/templates/netbox_custom_objects/custom_object_bulk_edit.html +++ b/netbox_custom_objects/templates/netbox_custom_objects/custom_object_bulk_edit.html @@ -9,10 +9,6 @@ {# Edit form #}
- {% if branch_warning %} - {% include 'netbox_custom_objects/inc/branch_warning.html' %} - {% endif %} -
{% csrf_token %} @@ -85,7 +81,7 @@

{% trans "Comments" %}

{% trans "Cancel" %} - +
diff --git a/netbox_custom_objects/templates/netbox_custom_objects/custom_object_bulk_import.html b/netbox_custom_objects/templates/netbox_custom_objects/custom_object_bulk_import.html index 3ae447b..8bce96e 100644 --- a/netbox_custom_objects/templates/netbox_custom_objects/custom_object_bulk_import.html +++ b/netbox_custom_objects/templates/netbox_custom_objects/custom_object_bulk_import.html @@ -10,10 +10,6 @@
- {% if branch_warning %} - {% include 'netbox_custom_objects/inc/branch_warning.html' %} - {% endif %} -
{% csrf_token %} @@ -36,7 +32,7 @@ {% if return_url %} {% trans "Cancel" %} {% endif %} - +
@@ -68,7 +64,7 @@ {% if return_url %} {% trans "Cancel" %} {% endif %} - +
@@ -101,7 +97,7 @@ {% if return_url %} {% trans "Cancel" %} {% endif %} - + diff --git a/netbox_custom_objects/templates/netbox_custom_objects/customobject_edit.html b/netbox_custom_objects/templates/netbox_custom_objects/customobject_edit.html index 6cba146..fdf8257 100644 --- a/netbox_custom_objects/templates/netbox_custom_objects/customobject_edit.html +++ b/netbox_custom_objects/templates/netbox_custom_objects/customobject_edit.html @@ -13,10 +13,6 @@ {% endblock title %} {% block form %} - {% if branch_warning %} - {% include 'netbox_custom_objects/inc/branch_warning.html' %} - {% endif %} - {# Render hidden fields #} {% for field in form.hidden_fields %} {{ field }} @@ -50,15 +46,15 @@

{{ group }}

{% block buttons %} {% trans "Cancel" %} {% if object.pk %} - {% else %}
- -
diff --git a/netbox_custom_objects/templates/netbox_custom_objects/inc/branch_warning.html b/netbox_custom_objects/templates/netbox_custom_objects/inc/branch_warning.html deleted file mode 100644 index ed7d027..0000000 --- a/netbox_custom_objects/templates/netbox_custom_objects/inc/branch_warning.html +++ /dev/null @@ -1,12 +0,0 @@ -{% load i18n %} - - diff --git a/netbox_custom_objects/utilities.py b/netbox_custom_objects/utilities.py index d8d901f..7a16cf4 100644 --- a/netbox_custom_objects/utilities.py +++ b/netbox_custom_objects/utilities.py @@ -8,7 +8,6 @@ "AppsProxy", "generate_model", "get_viewname", - "is_in_branch", ) @@ -110,17 +109,3 @@ def generate_model(*args, **kwargs): return model - -def is_in_branch(): - """ - Check if currently operating within a branch. - - Returns: - bool: True if currently in a branch, False otherwise. - """ - try: - from netbox_branching.contextvars import active_branch - return active_branch.get() is not None - except ImportError: - # Branching plugin not installed - return False diff --git a/netbox_custom_objects/views.py b/netbox_custom_objects/views.py index af646a0..5ede42e 100644 --- a/netbox_custom_objects/views.py +++ b/netbox_custom_objects/views.py @@ -29,7 +29,6 @@ from extras.choices import CustomFieldTypeChoices from netbox_custom_objects.constants import APP_LABEL from netbox_custom_objects.dynamic_forms import build_filterset_form_class -from netbox_custom_objects.utilities import is_in_branch logger = logging.getLogger("netbox_custom_objects.views") @@ -588,7 +587,6 @@ def custom_save(self, commit=True): def get_extra_context(self, request, obj): return { - 'branch_warning': is_in_branch(), } @@ -702,7 +700,6 @@ def get_form(self, queryset): def get_extra_context(self, request): return { - 'branch_warning': is_in_branch(), } @@ -793,7 +790,6 @@ def get_model_form(self, queryset): def get_extra_context(self, request): return { - 'branch_warning': is_in_branch(), } From 45315518972b5fec1e03e0028a1e0899ffc897f7 Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 20 Apr 2026 15:41:17 -0700 Subject: [PATCH 04/25] get merge working --- netbox_custom_objects/models.py | 72 +++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 22ceb92..ba54dda 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -720,6 +720,48 @@ def create_model(self): get_serializer_class(model) self.register_custom_object_search_index(model) + @classmethod + def deserialize_object(cls, data, pk=None): + """ + Custom deserialization hook for netbox-branching's merge/revert engine. + + ``ObjectChange.apply()`` normally uses ``DeserializedObject.save()``, which + calls ``Model.save_base(raw=True)`` — bypassing our ``save()`` override and + all ``post_save`` signals. That means ``create_model()`` never runs and the + physical table is never created in the destination schema. + + By implementing this classmethod the apply engine calls our version instead, + returning a wrapper whose ``save()`` invokes the full ``CustomObjectType.save()`` + lifecycle (signals included) so that the table is created as a side effect of + replaying the ObjectChange. + + ``object_type`` is cleared before saving so the ``custom_object_type_post_save_handler`` + can re-create and link it correctly in the destination schema, avoiding any FK + mismatch between the branch and main ``ObjectType`` pks. + """ + from utilities.serialization import deserialize_object as _deserialize + + inner = _deserialize(cls, data, pk=pk) + + class _SchemaAwareDeserialized: + def __init__(self, deserialized): + self._inner = deserialized + self.object = deserialized.object + + def save(self, using=None, **kwargs): + # Clear the ObjectType FK — it may not exist in main yet. + # custom_object_type_post_save_handler re-sets it after INSERT. + self.object.object_type = None + self.object.object_type_id = None + self.object.save() + # Re-apply any M2M data (tags, etc.) that was stripped during deserialization. + if self._inner.m2m_data: + for accessor_name, object_list in self._inner.m2m_data.items(): + getattr(self.object, accessor_name).set(object_list) + self._inner.m2m_data = None + + return _SchemaAwareDeserialized(inner) + def has_branch_schema_drift(self, branch) -> bool: """ Return True if this custom object type's physical schema in *branch* @@ -1677,6 +1719,36 @@ def through_table_name(self): def through_model_name(self): return f"Through_{self.through_table_name}" + @classmethod + def deserialize_object(cls, data, pk=None): + """ + Custom deserialization hook for netbox-branching's merge/revert engine. + + Same problem as ``CustomObjectType.deserialize_object``: the default + ``DeserializedObject.save(raw=True)`` bypasses ``CustomObjectTypeField.save()``, + so the physical column is never added to the custom object table. + + This wrapper calls the real ``save()`` so that ``add_field`` runs as a side + effect of replaying the CREATE ObjectChange during a merge. + """ + from utilities.serialization import deserialize_object as _deserialize + + inner = _deserialize(cls, data, pk=pk) + + class _SchemaAwareDeserialized: + def __init__(self, deserialized): + self._inner = deserialized + self.object = deserialized.object + + def save(self, using=None, **kwargs): + self.object.save() + if self._inner.m2m_data: + for accessor_name, object_list in self._inner.m2m_data.items(): + getattr(self.object, accessor_name).set(object_list) + self._inner.m2m_data = None + + return _SchemaAwareDeserialized(inner) + def save(self, *args, **kwargs): is_new = self._state.adding From e425d0c67b1007563e263395557b0eea6c29ae44 Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 20 Apr 2026 15:42:27 -0700 Subject: [PATCH 05/25] get merge working --- netbox_custom_objects/utilities.py | 1 - 1 file changed, 1 deletion(-) diff --git a/netbox_custom_objects/utilities.py b/netbox_custom_objects/utilities.py index 7a16cf4..45fdac3 100644 --- a/netbox_custom_objects/utilities.py +++ b/netbox_custom_objects/utilities.py @@ -108,4 +108,3 @@ def generate_model(*args, **kwargs): apps.clear_cache = apps.clear_cache return model - From de5840474c0771accc6b6afd29457ba412cc760f Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 20 Apr 2026 16:50:46 -0700 Subject: [PATCH 06/25] test updates --- netbox_custom_objects/models.py | 129 +++++++++++++++++++++++++++- netbox_custom_objects/tests/base.py | 2 + 2 files changed, 130 insertions(+), 1 deletion(-) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index ba54dda..aa486f2 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -86,6 +86,59 @@ def _get_schema_connection(): return connection +def _apply_deferred_co_field(field_instance): + """ + Apply any deferred CO field values after a column is added to the DB. + + Called by CustomObjectTypeField.save() after schema_editor.add_field() so that + custom object rows inserted before their columns existed (squash merge ordering) + receive their correct values via a raw UPDATE. + + ``CustomObject._deferred_field_data`` has the shape:: + + {db_table: {co_pk: {'using': alias, 'data': {field_name: value}}}} + + For TYPE_OBJECT fields the postchange_data key is ``{name}`` but the DB column + is ``{name}_id`` — this function maps accordingly. + For TYPE_MULTIOBJECT fields there is no column on the main table, so they are + skipped entirely. + """ + from extras.choices import CustomFieldTypeChoices + + # No deferred data at all — fast path. + if not CustomObject._deferred_field_data: + return + + cot = field_instance.custom_object_type + table_name = cot.get_database_table_name() + per_table = CustomObject._deferred_field_data.get(table_name) + if not per_table: + return + + # M2M has no column on the main table — nothing to UPDATE. + if field_instance.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: + return + + # For TYPE_OBJECT the data key is the field name but the DB column ends with _id. + data_key = field_instance.name + if field_instance.type == CustomFieldTypeChoices.TYPE_OBJECT: + col_name = f'{field_instance.name}_id' + else: + col_name = field_instance.name + + schema_conn = _get_schema_connection() + + with schema_conn.cursor() as cursor: + for co_pk, entry in per_table.items(): + value = entry['data'].get(data_key) + if value is None: + continue + cursor.execute( + f'UPDATE "{table_name}" SET "{col_name}" = %s WHERE id = %s', + [value, co_pk], + ) + + class CustomObject( BookmarksMixin, ChangeLoggingMixin, @@ -115,9 +168,75 @@ class CustomObject( objects = RestrictedQuerySet.as_manager() + # Pending custom-field values for CO rows created before their columns existed. + # Populated by deserialize_object(); consumed by _apply_deferred_co_field(). + # Format: {db_table: {co_pk: {'using': alias, 'data': {field_name: value}}}} + _deferred_field_data: dict = {} + class Meta: abstract = True + @classmethod + def deserialize_object(cls, data, pk=None): + """ + Hook called by ObjectChange.apply() for CREATE actions. + + The squash merge strategy may apply a CO's CREATE before its + CustomObjectTypeField rows are in main (the dependency graph has no FK + from CO to fields). When that happens, the standard Django + deserialization would INSERT the CO with NULL custom-field values + because the columns don't exist yet. + + This hook: + 1. Deserializes the CO using a fresh model (re-queried from DB). + 2. Does save_base(raw=True) as normal. + 3. Stores the full postchange_data in _deferred_field_data so that + CustomObjectTypeField.save() can UPDATE the row after each column is + added (handles the squash ordering case). + """ + from utilities.serialization import deserialize_object as _deserialize + + # Derive the COT primary key from the model class name (e.g. 'Table1Model' → 1) + model_name = cls.__name__ # e.g. 'Table1Model' + try: + cot_id = int(model_name.lower().replace('table', '').replace('model', '')) + except ValueError: + # Not a generated model name — fall back to standard deserialization. + return _deserialize(cls, data, pk=pk) + + # Refresh the model cache so we pick up any fields already applied to main. + # (In the squash case the cache may still point to a zero-field model.) + from netbox_custom_objects.models import CustomObjectType as _COT # noqa: F401 + _COT.clear_model_cache(cot_id) + try: + cot = _COT.objects.get(pk=cot_id) + fresh_model = cot.get_model() + except _COT.DoesNotExist: + fresh_model = cls + + inner = _deserialize(fresh_model, data, pk=pk) + obj = inner.object + obj_pk = pk if pk is not None else obj.pk + table_name = fresh_model._meta.db_table + full_data = dict(data) + + class _Deserialized: + object = obj + + def save(self, using=None, **_kwargs): + from django.db import DEFAULT_DB_ALIAS + _using = using or DEFAULT_DB_ALIAS + models.Model.save_base(obj, using=_using, raw=True) + # Register full data for deferred column updates (squash ordering fix). + if table_name not in cls._deferred_field_data: + cls._deferred_field_data[table_name] = {} + cls._deferred_field_data[table_name][obj_pk] = { + 'using': _using, + 'data': full_data, + } + + return _Deserialized() + def __str__(self): # Find the field with primary=True and return that field's "name" as the name of the object primary_field = self._field_objects.get(self._primary_field_id, None) @@ -664,7 +783,7 @@ def _ensure_field_fk_constraint(self, model, field_name): related_table = related_model._meta.db_table column_name = model_field.column - with connection.cursor() as cursor: + with _get_schema_connection().cursor() as cursor: # Drop existing FK constraint if it exists # Query for existing constraints cursor.execute(""" @@ -832,6 +951,13 @@ def delete(self, *args, **kwargs): super().delete(*args, **kwargs) if not in_branch: + # ChangeDiff has a PROTECT FK to ContentType/ObjectType — delete those + # records first so object_type.delete() is not blocked. + try: + from netbox_branching.models import ChangeDiff + ChangeDiff.objects.filter(object_type=object_type).delete() + except ImportError: + pass # Temporarily disconnect the pre_delete handler to skip the ObjectType deletion # TODO: Remove this disconnect/reconnect after ObjectType has been exempted from handle_deleted_object pre_delete.disconnect(handle_deleted_object) @@ -1764,6 +1890,7 @@ def save(self, *args, **kwargs): with schema_conn.schema_editor() as schema_editor: if self._state.adding: schema_editor.add_field(model, model_field) + _apply_deferred_co_field(self) if self.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: field_type.create_m2m_table(self, model, self.name, schema_conn=schema_conn) else: diff --git a/netbox_custom_objects/tests/base.py b/netbox_custom_objects/tests/base.py index ae48fc5..c9295ce 100644 --- a/netbox_custom_objects/tests/base.py +++ b/netbox_custom_objects/tests/base.py @@ -15,6 +15,8 @@ class TransactionCleanupMixin: """ def tearDown(self): + from netbox_custom_objects.models import CustomObject + CustomObject._deferred_field_data.clear() for cot in CustomObjectType.objects.all(): try: cot.delete() From a9c0219fd9251d9193fc60b55dd45340169bc298 Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 20 Apr 2026 17:03:01 -0700 Subject: [PATCH 07/25] add branching tests --- netbox_custom_objects/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox_custom_objects/views.py b/netbox_custom_objects/views.py index d6d5aa1..4f4f497 100644 --- a/netbox_custom_objects/views.py +++ b/netbox_custom_objects/views.py @@ -29,7 +29,7 @@ from extras.choices import CustomFieldTypeChoices from netbox_custom_objects.constants import APP_LABEL from netbox_custom_objects.dynamic_forms import build_filterset_form_class -from netbox_custom_objects.utilities import extract_cot_id_from_model_name, is_in_branch +from netbox_custom_objects.utilities import extract_cot_id_from_model_name logger = logging.getLogger("netbox_custom_objects.views") From f24dcaa91cdc178edd2c5aaa801c39aced6a7fdd Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 08:32:39 -0700 Subject: [PATCH 08/25] fix tests --- netbox_custom_objects/models.py | 32 ++++++++++++++++++++++++----- netbox_custom_objects/tests/base.py | 8 ++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 34fd029..69634b1 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -376,6 +376,9 @@ def __str__(self): return self.display_name def clean(self): + # Guard against None (can arrive via update_object during branch revert) + if self.custom_field_data is None: + self.custom_field_data = {} super().clean() if not self.slug: @@ -807,7 +810,9 @@ def _ensure_field_fk_constraint(self, model, field_name): constraint_name = row[0] cursor.execute(f'ALTER TABLE "{table_name}" DROP CONSTRAINT IF EXISTS "{constraint_name}"') - # Create new FK constraint with ON DELETE CASCADE + # Create new FK constraint with ON DELETE CASCADE. + # Not DEFERRABLE: a deferred constraint leaves pending trigger events that block + # subsequent ALTER TABLE calls (e.g. during branch revert remove_field). constraint_name = f"{table_name}_{column_name}_fk_cascade" cursor.execute(f""" ALTER TABLE "{table_name}" @@ -815,7 +820,6 @@ def _ensure_field_fk_constraint(self, model, field_name): FOREIGN KEY ("{column_name}") REFERENCES "{related_table}" ("id") ON DELETE CASCADE - DEFERRABLE INITIALLY DEFERRED """) def _ensure_all_fk_constraints(self, model): @@ -1012,6 +1016,9 @@ def custom_object_type_post_save_handler(sender, instance, created, **kwargs): app_label=APP_LABEL, model=content_type_name ) + # Snapshot before modifying so change logging records a correct pre-state. + # Without this, diff()['pre'] would set all fields to None during branch revert. + instance.snapshot() instance.object_type = ct instance.save() @@ -2024,7 +2031,10 @@ def save(self, *args, **kwargs): # Clear and refresh the model cache for this CustomObjectType when a field is modified self.custom_object_type.clear_model_cache(self.custom_object_type.id) - # Update parent's cache_timestamp to invalidate cache across all workers + # Update parent's cache_timestamp to invalidate cache across all workers. + # snapshot() must be called first so that change logging has a correct pre-state; + # without it, diff()['pre'] would set ALL fields to None during branch revert. + self.custom_object_type.snapshot() self.custom_object_type.save(update_fields=['cache_timestamp']) super().save(*args, **kwargs) @@ -2062,18 +2072,30 @@ def delete(self, *args, **kwargs): apps = model._meta.apps through_model = apps.get_model(APP_LABEL, self.through_model_name) schema_editor.delete_model(through_model) + # Flush any pending DEFERRABLE FK trigger events before ALTER TABLE; + # otherwise PostgreSQL raises "pending trigger events" when removing a FK field. + schema_editor.execute('SET CONSTRAINTS ALL IMMEDIATE') schema_editor.remove_field(model, model_field) # Clear the model cache for this CustomObjectType when a field is deleted self.custom_object_type.clear_model_cache(self.custom_object_type.id) - # Update parent's cache_timestamp to invalidate cache across all workers + # Update parent's cache_timestamp to invalidate cache across all workers. + # snapshot() must be called first so that change logging has a correct pre-state. + self.custom_object_type.snapshot() self.custom_object_type.save(update_fields=['cache_timestamp']) super().delete(*args, **kwargs) + # Regenerate and re-register the model so the app registry no longer includes + # the removed field. During squash revert the squash strategy may try to query + # CO rows (model.objects.get(pk=...)) after undoing this field but before undoing + # the CO itself. If the stale model class is still in the app registry it will + # include the now-absent column in its SELECT, causing ProgrammingError. + updated_model = self.custom_object_type.get_model() + # Reregister SearchIndex with new set of searchable fields - self.custom_object_type.register_custom_object_search_index(model) + self.custom_object_type.register_custom_object_search_index(updated_model) # Reindex all objects of this type since a searchable field was removed if self.search_weight > 0: diff --git a/netbox_custom_objects/tests/base.py b/netbox_custom_objects/tests/base.py index c9295ce..55c0718 100644 --- a/netbox_custom_objects/tests/base.py +++ b/netbox_custom_objects/tests/base.py @@ -15,13 +15,21 @@ class TransactionCleanupMixin: """ def tearDown(self): + from core.models import ObjectChange from netbox_custom_objects.models import CustomObject + # Flush deferred CO field data so it doesn't bleed into the next test. CustomObject._deferred_field_data.clear() + # Delete COTs and their backing tables before the DB flush. for cot in CustomObjectType.objects.all(): try: cot.delete() except Exception as exc: print(f"WARNING: tearDown could not delete COT {cot.pk}: {exc}") + # Remove any ObjectChange records created during the test (merge/revert creates + # them in main with the test user's ID). If left in place, the serialized_rollback + # snapshot accumulates them and restoring it after the next flush produces FK + # violations (user referenced by ObjectChange no longer exists). + ObjectChange.objects.all().delete() super().tearDown() From ed9ec9e873a85380d82a838f2c8b103531be9f8f Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 08:52:19 -0700 Subject: [PATCH 09/25] refactor --- netbox_custom_objects/branching.py | 90 ++--------- netbox_custom_objects/models.py | 230 +++++++++++++++-------------- 2 files changed, 131 insertions(+), 189 deletions(-) diff --git a/netbox_custom_objects/branching.py b/netbox_custom_objects/branching.py index 06db203..651f4ac 100644 --- a/netbox_custom_objects/branching.py +++ b/netbox_custom_objects/branching.py @@ -21,7 +21,7 @@ import logging -from django.db import connections, models +from django.db import connections logger = logging.getLogger('netbox_custom_objects.branching') @@ -151,12 +151,13 @@ def on_branch_migrated(sender, branch, user, **kwargs): exists in both, with different ``name`` values) rather than being treated as an unrelated delete + add. """ - from extras.choices import CustomFieldTypeChoices from netbox_branching.utilities import activate_branch - from netbox_custom_objects.constants import APP_LABEL - from netbox_custom_objects.field_types import FIELD_TYPE_CLASS - from netbox_custom_objects.models import CustomObjectType - from netbox_custom_objects.utilities import generate_model + from netbox_custom_objects.models import ( + CustomObjectType, + _schema_add_field, + _schema_alter_field, + _schema_remove_field, + ) branch_connection = connections[branch.connection_name] @@ -209,78 +210,17 @@ def on_branch_migrated(sender, branch, user, **kwargs): with branch_connection.schema_editor() as schema_editor: - # ── add_field ──────────────────────────────────────────────────── for fi in to_add: logger.debug('add_field %r on %s', fi.name, cot) - ft = FIELD_TYPE_CLASS[fi.type]() - mf = ft.get_model_field(fi) - mf.contribute_to_class(model, fi.name) - schema_editor.add_field(model, mf) - if fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: - ft.create_m2m_table(fi, model, fi.name, schema_conn=branch_connection) - - # ── remove_field ───────────────────────────────────────────────── + _schema_add_field(fi, model, schema_editor, branch_connection) + for fi in to_remove: logger.debug('remove_field %r on %s', fi.name, cot) - ft = FIELD_TYPE_CLASS[fi.type]() - mf = ft.get_model_field(fi) - mf.contribute_to_class(model, fi.name) - if fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: - through_table = f'custom_objects_{cot.pk}_{fi.name}' - if through_table in branch_tables: - ThroughMeta = type( - 'Meta', (), - {'db_table': through_table, 'app_label': APP_LABEL, 'managed': True}, - ) - through_model = type( - f'_TempThrough_{through_table}', - (models.Model,), - {'Meta': ThroughMeta, '__module__': 'netbox_custom_objects.models'}, - ) - schema_editor.delete_model(through_model) - schema_editor.remove_field(model, mf) - - # ── alter_field ────────────────────────────────────────────────── + _schema_remove_field(fi, model, schema_editor, existing_tables=branch_tables) + for old_fi, new_fi in to_alter: - logger.debug( - 'alter_field %r → %r on %s', - old_fi.name, new_fi.name, cot, + logger.debug('alter_field %r → %r on %s', old_fi.name, new_fi.name, cot) + _schema_alter_field( + old_fi, new_fi, model, schema_editor, branch_connection, + existing_tables=branch_tables, ) - old_mf = FIELD_TYPE_CLASS[old_fi.type]().get_model_field(old_fi) - new_mf = FIELD_TYPE_CLASS[new_fi.type]().get_model_field(new_fi) - old_mf.contribute_to_class(model, old_fi.name) - new_mf.contribute_to_class(model, new_fi.name) - - # When a MULTIOBJECT field is renamed, the through table must - # be renamed first (same logic as CustomObjectTypeField.save()). - if ( - new_fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT - and old_fi.name != new_fi.name - ): - old_through = f'custom_objects_{cot.pk}_{old_fi.name}' - new_through = f'custom_objects_{cot.pk}_{new_fi.name}' - if old_through in branch_tables: - OldThroughMeta = type( - 'Meta', (), - {'db_table': old_through, 'app_label': APP_LABEL, 'managed': True}, - ) - old_through_model = generate_model( - f'_TempOldThrough_{old_through}', - (models.Model,), - { - '__module__': 'netbox_custom_objects.models', - 'Meta': OldThroughMeta, - 'id': models.AutoField(primary_key=True), - 'source': models.ForeignKey( - model, on_delete=models.CASCADE, - db_column='source_id', related_name='+', - ), - 'target': models.ForeignKey( - model, on_delete=models.CASCADE, - db_column='target_id', related_name='+', - ), - }, - ) - schema_editor.alter_db_table(old_through_model, old_through, new_through) - - schema_editor.alter_field(model, old_mf, new_mf) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 69634b1..0035496 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -139,6 +139,119 @@ def _apply_deferred_co_field(field_instance): ) +def _schema_add_field(fi, model, schema_editor, schema_conn): + """ + Issue ``add_field`` against the physical schema for *fi*. + + Handles through-table creation for MULTIOBJECT fields. Does NOT apply + deferred CO field data — callers that need that (squash merge context) must + call ``_apply_deferred_co_field(fi)`` separately after this returns. + """ + ft = FIELD_TYPE_CLASS[fi.type]() + mf = ft.get_model_field(fi) + mf.contribute_to_class(model, fi.name) + schema_editor.add_field(model, mf) + if fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: + ft.create_m2m_table(fi, model, fi.name, schema_conn=schema_conn) + + +def _schema_remove_field(fi, model, schema_editor, existing_tables=None): + """ + Issue ``remove_field`` against the physical schema for *fi*. + + For MULTIOBJECT fields the through table is dropped first. When + *existing_tables* is a pre-fetched list only tables present in it are + dropped; when it is ``None`` (main-schema context) the drop is always + attempted. + + Always issues ``SET CONSTRAINTS ALL IMMEDIATE`` before ``remove_field`` to + flush any DEFERRABLE FK trigger events that would otherwise cause PostgreSQL + to reject the subsequent ALTER TABLE. + """ + ft = FIELD_TYPE_CLASS[fi.type]() + mf = ft.get_model_field(fi) + mf.contribute_to_class(model, fi.name) + + if fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: + through_table = fi.through_table_name + if existing_tables is None or through_table in existing_tables: + through_meta = type( + 'Meta', (), + {'db_table': through_table, 'app_label': APP_LABEL, 'managed': True}, + ) + through_model = type( + f'_TempThrough_{through_table}', + (models.Model,), + {'Meta': through_meta, '__module__': 'netbox_custom_objects.models'}, + ) + schema_editor.delete_model(through_model) + + # Flush any pending DEFERRABLE FK trigger events before ALTER TABLE; + # otherwise PostgreSQL raises "pending trigger events" when removing a FK field. + schema_editor.execute('SET CONSTRAINTS ALL IMMEDIATE') + schema_editor.remove_field(model, mf) + + +def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, existing_tables=None): + """ + Issue ``alter_field`` against the physical schema, updating *old_fi* to *new_fi*. + + For MULTIOBJECT fields whose name changes the through table is renamed before + ``alter_field`` is called. When the old through table is absent (e.g. the + branch has never seen this field) the new through table is created from scratch + instead. + + *existing_tables* — optional pre-fetched table name list from the target + connection. When given, through-table operations are guarded by membership + checks. When ``None`` (main-schema context) the schema_conn is introspected + once on demand. + """ + old_mf = FIELD_TYPE_CLASS[old_fi.type]().get_model_field(old_fi) + new_mf = FIELD_TYPE_CLASS[new_fi.type]().get_model_field(new_fi) + old_mf.contribute_to_class(model, old_fi.name) + new_mf.contribute_to_class(model, new_fi.name) + + if ( + new_fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT + and old_fi.name != new_fi.name + ): + old_through = old_fi.through_table_name + new_through = new_fi.through_table_name + + tables = existing_tables + if tables is None: + with schema_conn.cursor() as cursor: + tables = schema_conn.introspection.table_names(cursor) + + if old_through in tables: + old_through_meta = type( + 'Meta', (), + {'db_table': old_through, 'app_label': APP_LABEL, 'managed': True}, + ) + old_through_model = generate_model( + f'_TempOldThrough_{old_through}', + (models.Model,), + { + '__module__': 'netbox_custom_objects.models', + 'Meta': old_through_meta, + 'id': models.AutoField(primary_key=True), + 'source': models.ForeignKey( + model, on_delete=models.CASCADE, db_column='source_id', related_name='+', + ), + 'target': models.ForeignKey( + model, on_delete=models.CASCADE, db_column='target_id', related_name='+', + ), + }, + ) + schema_editor.alter_db_table(old_through_model, old_through, new_through) + else: + # Old through table absent — create the new one from scratch + ft = FIELD_TYPE_CLASS[new_fi.type]() + ft.create_m2m_table(new_fi, model, new_fi.name, schema_conn=schema_conn) + + schema_editor.alter_field(model, old_mf, new_mf) + + class CustomObject( BookmarksMixin, ChangeLoggingMixin, @@ -1901,115 +2014,14 @@ def save(self, *args, **kwargs): # editor operations target the branch schema rather than main. schema_conn = _get_schema_connection() - field_type = FIELD_TYPE_CLASS[self.type]() - model_field = field_type.get_model_field(self) model = self.custom_object_type.get_model() - model_field.contribute_to_class(model, self.name) with schema_conn.schema_editor() as schema_editor: if self._state.adding: - schema_editor.add_field(model, model_field) + _schema_add_field(self, model, schema_editor, schema_conn) _apply_deferred_co_field(self) - if self.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: - field_type.create_m2m_table(self, model, self.name, schema_conn=schema_conn) else: - old_field = field_type.get_model_field(self.original) - old_field.contribute_to_class(model, self._original_name) - - # Special handling for MultiObject fields when the name changes - if ( - self.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT - and self.name != self._original_name - ): - # For renamed MultiObject fields, we just need to rename the through table - old_through_table_name = self.original.through_table_name - new_through_table_name = self.through_table_name - - # Check if old through table exists - with schema_conn.cursor() as cursor: - tables = schema_conn.introspection.table_names(cursor) - old_table_exists = old_through_table_name in tables - - if old_table_exists: - # Create temporary models to represent the old and new through table states - old_through_meta = type( - "Meta", - (), - { - "db_table": old_through_table_name, - "app_label": APP_LABEL, - "managed": True, - }, - ) - old_through_model = generate_model( - f"TempOld{self.original.through_model_name}", - (models.Model,), - { - "__module__": "netbox_custom_objects.models", - "Meta": old_through_meta, - "id": models.AutoField(primary_key=True), - "source": models.ForeignKey( - model, - on_delete=models.CASCADE, - db_column="source_id", - related_name="+", - ), - "target": models.ForeignKey( - model, - on_delete=models.CASCADE, - db_column="target_id", - related_name="+", - ), - }, - ) - - new_through_meta = type( - "Meta", - (), - { - "db_table": new_through_table_name, - "app_label": APP_LABEL, - "managed": True, - }, - ) - new_through_model = generate_model( - f"TempNew{self.through_model_name}", - (models.Model,), - { - "__module__": "netbox_custom_objects.models", - "Meta": new_through_meta, - "id": models.AutoField(primary_key=True), - "source": models.ForeignKey( - model, - on_delete=models.CASCADE, - db_column="source_id", - related_name="+", - ), - "target": models.ForeignKey( - model, - on_delete=models.CASCADE, - db_column="target_id", - related_name="+", - ), - }, - ) - new_through_model # To silence ruff error - - # Rename the table using Django's schema editor - schema_editor.alter_db_table( - old_through_model, - old_through_table_name, - new_through_table_name, - ) - else: - # No old table exists, create the new through table - field_type.create_m2m_table(self, model, self.name, schema_conn=schema_conn) - - # Alter the field normally (this updates the field definition) - schema_editor.alter_field(model, old_field, model_field) - else: - # Normal field alteration - schema_editor.alter_field(model, old_field, model_field) + _schema_alter_field(self.original, self, model, schema_editor, schema_conn) # Ensure FK constraints are properly created for OBJECT fields with CASCADE behavior should_ensure_fk = False @@ -2062,20 +2074,10 @@ def delete(self, *args, **kwargs): # Use the branch connection when operating inside a branch. schema_conn = _get_schema_connection() - field_type = FIELD_TYPE_CLASS[self.type]() - model_field = field_type.get_model_field(self) model = self.custom_object_type.get_model() - model_field.contribute_to_class(model, self.name) with schema_conn.schema_editor() as schema_editor: - if self.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: - apps = model._meta.apps - through_model = apps.get_model(APP_LABEL, self.through_model_name) - schema_editor.delete_model(through_model) - # Flush any pending DEFERRABLE FK trigger events before ALTER TABLE; - # otherwise PostgreSQL raises "pending trigger events" when removing a FK field. - schema_editor.execute('SET CONSTRAINTS ALL IMMEDIATE') - schema_editor.remove_field(model, model_field) + _schema_remove_field(self, model, schema_editor) # Clear the model cache for this CustomObjectType when a field is deleted self.custom_object_type.clear_model_cache(self.custom_object_type.id) From 8cd04a00906916b502cb43fedd6a6a84e74cc0e1 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 09:10:12 -0700 Subject: [PATCH 10/25] test cleanup --- netbox_custom_objects/filtersets.py | 3 +++ netbox_custom_objects/tests/test_field_types.py | 7 ++++--- netbox_custom_objects/tests/test_filtersets.py | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/netbox_custom_objects/filtersets.py b/netbox_custom_objects/filtersets.py index 740e207..f5cbced 100644 --- a/netbox_custom_objects/filtersets.py +++ b/netbox_custom_objects/filtersets.py @@ -3,6 +3,7 @@ from django.contrib.postgres.fields import ArrayField from django.db.models import JSONField, Q from django.utils.dateparse import parse_date, parse_datetime +from django.utils.timezone import make_aware, is_aware from extras.choices import CustomFieldTypeChoices from netbox.filtersets import NetBoxModelFilterSet @@ -89,6 +90,8 @@ def search(self, queryset, name, value): elif field.type == CustomFieldTypeChoices.TYPE_DATETIME: parsed = parse_datetime(value) if parsed is not None: + if not is_aware(parsed): + parsed = make_aware(parsed) q |= Q(**{f"{field.name}__exact": parsed}) if not q: return queryset.none() diff --git a/netbox_custom_objects/tests/test_field_types.py b/netbox_custom_objects/tests/test_field_types.py index 4dff11d..c23c0f9 100644 --- a/netbox_custom_objects/tests/test_field_types.py +++ b/netbox_custom_objects/tests/test_field_types.py @@ -356,11 +356,11 @@ def test_datetime_field_creation(self): name="created_datetime", label="Created DateTime", type="datetime", - default="2023-01-01T12:00:00" + default="2023-01-01T12:00:00+00:00" ) self.assertEqual(field.type, "datetime") - self.assertEqual(field.default, "2023-01-01T12:00:00") + self.assertEqual(field.default, "2023-01-01T12:00:00+00:00") def test_datetime_field_validation(self): """Test datetime field validation.""" @@ -390,8 +390,9 @@ def test_datetime_field_model_generation(self): type="datetime" ) + import datetime as dt model = self.custom_object_type.get_model() - test_datetime = datetime(2023, 1, 1, 12, 0, 0) + test_datetime = datetime(2023, 1, 1, 12, 0, 0, tzinfo=dt.timezone.utc) instance = model.objects.create(name="Test", created_datetime=test_datetime) self.assertEqual(instance.created_datetime, test_datetime) diff --git a/netbox_custom_objects/tests/test_filtersets.py b/netbox_custom_objects/tests/test_filtersets.py index 2965c5c..7d903f5 100644 --- a/netbox_custom_objects/tests/test_filtersets.py +++ b/netbox_custom_objects/tests/test_filtersets.py @@ -573,8 +573,8 @@ def setUpTestData(cls): ) model = cls.cot.get_model() - cls.obj_morning = model.objects.create(ts=datetime.datetime(2025, 3, 10, 9, 0, 0)) - cls.obj_evening = model.objects.create(ts=datetime.datetime(2025, 3, 10, 18, 30, 0)) + cls.obj_morning = model.objects.create(ts=datetime.datetime(2025, 3, 10, 9, 0, 0, tzinfo=datetime.timezone.utc)) + cls.obj_evening = model.objects.create(ts=datetime.datetime(2025, 3, 10, 18, 30, 0, tzinfo=datetime.timezone.utc)) def _search(self, value): model = self.cot.get_model() From 0aed016e232527751c1524dcda83877a39a9879c Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 09:10:48 -0700 Subject: [PATCH 11/25] test cleanup --- netbox_custom_objects/tests/test_filtersets.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/netbox_custom_objects/tests/test_filtersets.py b/netbox_custom_objects/tests/test_filtersets.py index 7d903f5..b33ed65 100644 --- a/netbox_custom_objects/tests/test_filtersets.py +++ b/netbox_custom_objects/tests/test_filtersets.py @@ -573,8 +573,9 @@ def setUpTestData(cls): ) model = cls.cot.get_model() - cls.obj_morning = model.objects.create(ts=datetime.datetime(2025, 3, 10, 9, 0, 0, tzinfo=datetime.timezone.utc)) - cls.obj_evening = model.objects.create(ts=datetime.datetime(2025, 3, 10, 18, 30, 0, tzinfo=datetime.timezone.utc)) + utc = datetime.timezone.utc + cls.obj_morning = model.objects.create(ts=datetime.datetime(2025, 3, 10, 9, 0, 0, tzinfo=utc)) + cls.obj_evening = model.objects.create(ts=datetime.datetime(2025, 3, 10, 18, 30, 0, tzinfo=utc)) def _search(self, value): model = self.cot.get_model() From f47ac66b417b652dde64412be6b31d7a7262e5e7 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 10:08:08 -0700 Subject: [PATCH 12/25] test cleanup --- netbox_custom_objects/tests/base.py | 64 +++++++++++++++++++ netbox_custom_objects/tests/test_branching.py | 9 +-- netbox_custom_objects/tests/test_deletion.py | 16 +---- 3 files changed, 71 insertions(+), 18 deletions(-) diff --git a/netbox_custom_objects/tests/base.py b/netbox_custom_objects/tests/base.py index 55c0718..73cb3bd 100644 --- a/netbox_custom_objects/tests/base.py +++ b/netbox_custom_objects/tests/base.py @@ -1,12 +1,55 @@ # Test utilities for netbox_custom_objects plugin +from django.apps import apps as django_apps +from django.contrib.contenttypes.management import create_contenttypes from django.test import Client from core.models import ObjectType from extras.models import CustomFieldChoiceSet from utilities.testing import create_test_user +from netbox_custom_objects.constants import APP_LABEL from netbox_custom_objects.models import CustomObjectType, CustomObjectTypeField +def _recreate_contenttypes(): + """Recreate ContentType rows for all installed apps using get_or_create. + + Called after a TransactionTestCase flush so that subsequent test classes — + whether TransactionTestCase or regular TestCase — can look up ContentTypes. + Using create_contenttypes (get_or_create) avoids the duplicate-key + violations that serialized_rollback causes in the parallel test runner. + """ + for app_config in django_apps.get_app_configs(): + create_contenttypes(app_config, verbosity=0) + + +def _purge_stale_generated_models(): + """Remove dynamically generated CustomObject models from the app registry. + + Regular TestCase subclasses wrap each test in a transaction that is rolled + back at the end. Rolling back the transaction drops any tables created by + DDL inside the test (e.g. CREATE TABLE custom_objects_1), but Django's + in-memory model registry is NOT rolled back. The stale model entry then + causes problems for subsequent TransactionTestCase tests: + + - netbox-branching's get_tables_to_replicate() iterates over registered + models to build the list of tables to clone into the branch schema. + A stale model entry makes it try to COPY a table that no longer exists. + - Django's cascade-delete collector queries non-existent tables. + + Calling this in setUp() of every TransactionTestCase prevents both failure + modes. + """ + stale = [ + name + for name, model in list(django_apps.all_models.get(APP_LABEL, {}).items()) + if getattr(model, '_generated_table_model', False) + ] + for name in stale: + django_apps.all_models[APP_LABEL].pop(name, None) + if stale: + django_apps.clear_cache() + + class TransactionCleanupMixin: """Mixin for TransactionTestCase subclasses that create CustomObjectType instances. @@ -14,6 +57,14 @@ class TransactionCleanupMixin: database flush that TransactionTestCase performs between tests. """ + def setUp(self): + # Purge stale in-memory model registrations left by earlier TestCase + # classes whose rolled-back transactions dropped the backing tables. + # Must run before any code that iterates the model registry (e.g. + # netbox-branching's get_tables_to_replicate() during provisioning). + _purge_stale_generated_models() + super().setUp() + def tearDown(self): from core.models import ObjectChange from netbox_custom_objects.models import CustomObject @@ -32,6 +83,19 @@ def tearDown(self): ObjectChange.objects.all().delete() super().tearDown() + def _fixture_teardown(self): + """Flush tables and restore ContentTypes for the next test class. + + TransactionTestCase._fixture_teardown() TRUNCATEs all tables after each + test. Any TestCase class that follows on the same worker then finds no + ContentTypes and fails trying to look up ObjectType rows. Recreating + them here (idempotently, via get_or_create) avoids that without the + duplicate-key violations that serialized_rollback=True causes when the + parallel runner tries to INSERT rows that already exist. + """ + super()._fixture_teardown() + _recreate_contenttypes() + class CustomObjectsTestCase: """ diff --git a/netbox_custom_objects/tests/test_branching.py b/netbox_custom_objects/tests/test_branching.py index 85ff377..364632b 100644 --- a/netbox_custom_objects/tests/test_branching.py +++ b/netbox_custom_objects/tests/test_branching.py @@ -28,7 +28,7 @@ HAS_BRANCHING = False from netbox_custom_objects.models import CustomObjectType, CustomObjectTypeField -from netbox_custom_objects.tests.base import TransactionCleanupMixin +from netbox_custom_objects.tests.base import TransactionCleanupMixin, _recreate_contenttypes User = get_user_model() @@ -83,9 +83,10 @@ class IterativeBranchingTestCase(BaseBranchingTests, TransactionTestCase): """ MERGE_STRATEGY = None - serialized_rollback = True def setUp(self): + super().setUp() # → TransactionCleanupMixin.setUp() → _purge_stale_generated_models() + _recreate_contenttypes() self.user = User.objects.create_user(username='testuser') self.request = _make_request(self.user) @@ -327,9 +328,9 @@ class BranchSyncTestCase(TransactionCleanupMixin, TransactionTestCase): available in the branch after sync. """ - serialized_rollback = True - def setUp(self): + super().setUp() # → TransactionCleanupMixin.setUp() → _purge_stale_generated_models() + _recreate_contenttypes() self.user = User.objects.create_user(username='testuser') self.request = _make_request(self.user) diff --git a/netbox_custom_objects/tests/test_deletion.py b/netbox_custom_objects/tests/test_deletion.py index 047aee4..4dcf5f8 100644 --- a/netbox_custom_objects/tests/test_deletion.py +++ b/netbox_custom_objects/tests/test_deletion.py @@ -21,21 +21,9 @@ class DeletionTestCase(TransactionCleanupMixin, CustomObjectsTestCase, Transacti """Test deletion scenarios with cascading effects.""" def setUp(self): - """Purge stale dynamic models left in the app registry by earlier TestCase - classes whose setUpTestData transaction was rolled back (dropping the backing - tables). Leaving them registered causes Django's cascade-delete collector to - query non-existent tables when a related core object is deleted. - """ + # TransactionCleanupMixin.setUp() purges stale generated models and + # CustomObjectsTestCase.setUp() creates the test user and client. super().setUp() - stale = [ - name - for name, model in list(django_apps.all_models.get(APP_LABEL, {}).items()) - if getattr(model, '_generated_table_model', False) - ] - for name in stale: - django_apps.all_models[APP_LABEL].pop(name, None) - if stale: - django_apps.clear_cache() # ------------------------------------------------------------------ # Helpers From 539416dad19d6a66332aa50cb9cd0008d53e5165 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 10:35:45 -0700 Subject: [PATCH 13/25] update drift check --- netbox_custom_objects/branching.py | 8 ++++++-- netbox_custom_objects/models.py | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/netbox_custom_objects/branching.py b/netbox_custom_objects/branching.py index 651f4ac..b969b46 100644 --- a/netbox_custom_objects/branching.py +++ b/netbox_custom_objects/branching.py @@ -126,12 +126,16 @@ def _fields_schema_differ(branch_f, main_f): Return True if the two ``CustomObjectTypeField`` instances differ in any attribute that affects the physical DB column, meaning an ALTER TABLE is needed to bring the branch schema up to date. + + Excluded (application-level only, no DB impact): + - required: enforced by forms/serializers; all field types use null=True + regardless, so required never maps to a NOT NULL column constraint. + - default: Python-level default applied by the ORM, not a DB DEFAULT + clause; changing it on an existing column needs no ALTER TABLE. """ return ( branch_f.name != main_f.name or branch_f.type != main_f.type - or branch_f.required != main_f.required - or branch_f.default != main_f.default or branch_f.unique != main_f.unique or branch_f.related_object_type_id != main_f.related_object_type_id ) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 0035496..339d4e8 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -1036,7 +1036,12 @@ def has_branch_schema_drift(self, branch) -> bool: return True def _schema_key(f): - return (f.name, f.type, f.required, f.default, f.unique, f.related_object_type_id) + # Only attributes that affect the physical DB column are included. + # - required: enforced at form/serializer level only; all field types + # use null=True regardless, so required never maps to NOT NULL. + # - default: Python-level default, not a DB DEFAULT clause; changing + # it on an existing column requires no ALTER TABLE. + return (f.name, f.type, f.unique, f.related_object_type_id) return any( _schema_key(branch_fields[pk]) != _schema_key(main_fields[pk]) From 549f267234a424a008f766da0192dc13daf35785 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 10:46:20 -0700 Subject: [PATCH 14/25] cleanup --- netbox_custom_objects/__init__.py | 6 +++++- netbox_custom_objects/branching.py | 31 ++++++++++++++++++------------ netbox_custom_objects/models.py | 20 +++++++++---------- netbox_custom_objects/views.py | 9 +++++---- 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/netbox_custom_objects/__init__.py b/netbox_custom_objects/__init__.py index 4cbf3a0..a661afd 100644 --- a/netbox_custom_objects/__init__.py +++ b/netbox_custom_objects/__init__.py @@ -1,4 +1,5 @@ import contextvars +import logging import sys import warnings @@ -228,7 +229,10 @@ def ready(self): from netbox_custom_objects.branching import check_pending_branch_migrations check_pending_branch_migrations() except Exception: - pass + logging.getLogger('netbox_custom_objects').exception( + 'check_pending_branch_migrations() failed at startup; ' + 'some branches may not be marked as PENDING_MIGRATIONS' + ) super().ready() diff --git a/netbox_custom_objects/branching.py b/netbox_custom_objects/branching.py index b969b46..8d5fb0b 100644 --- a/netbox_custom_objects/branching.py +++ b/netbox_custom_objects/branching.py @@ -121,24 +121,31 @@ def on_custom_object_field_changed(sender, instance, **kwargs): ) +def _field_schema_key(f): + """ + Return the subset of ``CustomObjectTypeField`` attributes that affect the + physical DB column schema. + + Excluded (application-level only, no physical DB impact): + - required: all field types use null=True regardless; required never maps + to a NOT NULL column constraint. + - default: Python-level default applied by the ORM, not a SQL DEFAULT + clause; changing it on an existing column needs no ALTER TABLE. + + Used by both ``_fields_schema_differ()`` and + ``CustomObjectType.has_branch_schema_drift()`` to ensure the two callers + always agree on what constitutes a schema-affecting change. + """ + return (f.name, f.type, f.unique, f.related_object_type_id) + + def _fields_schema_differ(branch_f, main_f): """ Return True if the two ``CustomObjectTypeField`` instances differ in any attribute that affects the physical DB column, meaning an ALTER TABLE is needed to bring the branch schema up to date. - - Excluded (application-level only, no DB impact): - - required: enforced by forms/serializers; all field types use null=True - regardless, so required never maps to a NOT NULL column constraint. - - default: Python-level default applied by the ORM, not a DB DEFAULT - clause; changing it on an existing column needs no ALTER TABLE. """ - return ( - branch_f.name != main_f.name - or branch_f.type != main_f.type - or branch_f.unique != main_f.unique - or branch_f.related_object_type_id != main_f.related_object_type_id - ) + return _field_schema_key(branch_f) != _field_schema_key(main_f) def on_branch_migrated(sender, branch, user, **kwargs): diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 339d4e8..b598769 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -133,6 +133,10 @@ def _apply_deferred_co_field(field_instance): value = entry['data'].get(data_key) if value is None: continue + # table_name comes from get_database_table_name() (controlled by our + # code) and col_name from field.name, which is validated by the + # ^[a-z0-9_]+$ regex — no double-quote characters are possible, so + # the f-string interpolation is safe against SQL injection here. cursor.execute( f'UPDATE "{table_name}" SET "{col_name}" = %s WHERE id = %s', [value, co_pk], @@ -329,7 +333,6 @@ def deserialize_object(cls, data, pk=None): inner = _deserialize(fresh_model, data, pk=pk) obj = inner.object - obj_pk = pk if pk is not None else obj.pk table_name = fresh_model._meta.db_table full_data = dict(data) @@ -340,6 +343,9 @@ def save(self, using=None, **_kwargs): from django.db import DEFAULT_DB_ALIAS _using = using or DEFAULT_DB_ALIAS models.Model.save_base(obj, using=_using, raw=True) + # Read pk after save_base so that auto-assigned PKs are captured. + # (If pk was None before save_base, obj.pk is now the DB-assigned id.) + obj_pk = obj.pk # Register full data for deferred column updates (squash ordering fix). if table_name not in cls._deferred_field_data: cls._deferred_field_data[table_name] = {} @@ -1025,6 +1031,8 @@ def has_branch_schema_drift(self, branch) -> bool: except ImportError: return False + from netbox_custom_objects.branching import _field_schema_key + main_fields = {f.pk: f for f in self.fields.all()} with activate_branch(branch): branch_fields = {f.pk: f for f in self.fields.all()} @@ -1035,16 +1043,8 @@ def has_branch_schema_drift(self, branch) -> bool: if main_pks != branch_pks: return True - def _schema_key(f): - # Only attributes that affect the physical DB column are included. - # - required: enforced at form/serializer level only; all field types - # use null=True regardless, so required never maps to NOT NULL. - # - default: Python-level default, not a DB DEFAULT clause; changing - # it on an existing column requires no ALTER TABLE. - return (f.name, f.type, f.unique, f.related_object_type_id) - return any( - _schema_key(branch_fields[pk]) != _schema_key(main_fields[pk]) + _field_schema_key(branch_fields[pk]) != _field_schema_key(main_fields[pk]) for pk in main_pks ) diff --git a/netbox_custom_objects/views.py b/netbox_custom_objects/views.py index 4f4f497..15ea6f4 100644 --- a/netbox_custom_objects/views.py +++ b/netbox_custom_objects/views.py @@ -529,10 +529,11 @@ def custom_init(self, *args, **kwargs): # Custom object type from netbox_custom_objects.models import CustomObjectType custom_object_type_id = extract_cot_id_from_model_name(content_type.model) - assert custom_object_type_id is not None, ( - f"Expected tablemodel name for {APP_LABEL} content type, " - f"got {content_type.model!r}" - ) + if custom_object_type_id is None: + raise ValueError( + f"Expected tablemodel name for {APP_LABEL} content type, " + f"got {content_type.model!r}" + ) custom_object_type = CustomObjectType.objects.get(pk=custom_object_type_id) model = custom_object_type.get_model(skip_object_fields=True) else: From 493aacbf436d6c86443ba38d86c1c6281020a697 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 11:00:57 -0700 Subject: [PATCH 15/25] cleanup --- netbox_custom_objects/branching.py | 34 ++++++++----------- netbox_custom_objects/tests/test_branching.py | 23 ++++++++++--- netbox_custom_objects/views.py | 12 ------- 3 files changed, 32 insertions(+), 37 deletions(-) diff --git a/netbox_custom_objects/branching.py b/netbox_custom_objects/branching.py index 8d5fb0b..98ca3ac 100644 --- a/netbox_custom_objects/branching.py +++ b/netbox_custom_objects/branching.py @@ -77,15 +77,20 @@ def check_pending_branch_migrations(): def on_custom_object_field_changed(sender, instance, **kwargs): """ - Mark any READY branches that contain the affected custom object type's table - as PENDING_MIGRATIONS when a ``CustomObjectTypeField`` is created, modified, - or deleted in the main schema. + Mark all READY branches as PENDING_MIGRATIONS when a ``CustomObjectTypeField`` + is created, modified, or deleted in the main schema. This surfaces the pending state in the branching UI exactly like a normal Django-migration, prompting users to click "Migrate Branch". That action calls ``Branch.migrate()``, which fires ``on_branch_migrated`` and reconciles the physical column differences. + Branches provisioned before this COT's table existed will be over-marked as + PENDING_MIGRATIONS but ``on_branch_migrated`` will skip them (no table in + branch) so the extra marking is harmless. The alternative — opening a + connection per branch to check whether the table exists — adds O(branches) + latency to every field save, which is worse at scale. + Skipped when the change happens inside a branch context — the field edit only affects that branch's schema, not main, so no other branches need updating. """ @@ -99,25 +104,14 @@ def on_custom_object_field_changed(sender, instance, **kwargs): from netbox_branching.choices import BranchStatusChoices from netbox_branching.models import Branch - cot = instance.custom_object_type - db_table = cot.get_database_table_name() - - ready_branches = list(Branch.objects.filter(status=BranchStatusChoices.READY)) - to_update = [] - for branch in ready_branches: - branch_connection = connections[branch.connection_name] - with branch_connection.cursor() as cursor: - branch_tables = branch_connection.introspection.table_names(cursor) - if db_table in branch_tables: - branch.status = BranchStatusChoices.PENDING_MIGRATIONS - to_update.append(branch) - - if to_update: - Branch.objects.bulk_update(to_update, ['status']) + updated = Branch.objects.filter(status=BranchStatusChoices.READY).update( + status=BranchStatusChoices.PENDING_MIGRATIONS, + ) + if updated: logger.info( 'Marked %d branch(es) as PENDING_MIGRATIONS due to field changes on %s', - len(to_update), - cot, + updated, + instance.custom_object_type, ) diff --git a/netbox_custom_objects/tests/test_branching.py b/netbox_custom_objects/tests/test_branching.py index 364632b..f5c7e0f 100644 --- a/netbox_custom_objects/tests/test_branching.py +++ b/netbox_custom_objects/tests/test_branching.py @@ -9,6 +9,8 @@ in separate PostgreSQL schemas backed by distinct database connections that cannot be rolled back inside a single SAVEPOINT-based transaction. """ +import datetime +import decimal import time import unittest import uuid @@ -186,11 +188,13 @@ def test_comprehensive_merge_and_revert(self): Field types ----------- - text — plain VARCHAR column - integer — INTEGER column - boolean — BOOLEAN column - select — VARCHAR column with a ChoiceSet - object — ForeignKey column (to dcim.Site) + text — plain VARCHAR column + integer — INTEGER column + decimal — DECIMAL column (exercises numeric precision handling) + boolean — BOOLEAN column + datetime — TIMESTAMPTZ column (exercises timezone-aware handling) + select — VARCHAR column with a ChoiceSet + object — ForeignKey column (to dcim.Site) multiobject — M2M through-table (to dcim.Site) This exercises every distinct schema-editor operation @@ -216,6 +220,9 @@ def test_comprehensive_merge_and_revert(self): field_pks = {} co_pk = None + test_dt = datetime.datetime(2024, 6, 15, 12, 0, 0, tzinfo=datetime.timezone.utc) + test_decimal = decimal.Decimal('3.14') + # ── create inside branch ────────────────────────────────────────── with activate_branch(branch), event_tracking(request): choice_set = CustomFieldChoiceSet.objects.create( @@ -228,7 +235,9 @@ def test_comprehensive_merge_and_revert(self): field_specs = [ ('text_field', {'type': 'text'}), ('int_field', {'type': 'integer'}), + ('dec_field', {'type': 'decimal'}), ('bool_field', {'type': 'boolean'}), + ('dt_field', {'type': 'datetime'}), ('select_field', {'type': 'select', 'choice_set': choice_set}), ('obj_field', {'type': 'object', 'related_object_type': site_ot}), ('multi_field', {'type': 'multiobject', 'related_object_type': site_ot}), @@ -246,7 +255,9 @@ def test_comprehensive_merge_and_revert(self): co = Model.objects.create( text_field='hello', int_field=42, + dec_field=test_decimal, bool_field=True, + dt_field=test_dt, select_field='active', obj_field_id=site.pk, # multi_field left empty — through-table creation is still exercised @@ -284,7 +295,9 @@ def test_comprehensive_merge_and_revert(self): co_main = cot_main.get_model().objects.get(pk=co_pk) self.assertEqual(co_main.text_field, 'hello') self.assertEqual(co_main.int_field, 42) + self.assertEqual(co_main.dec_field, test_decimal) self.assertTrue(co_main.bool_field) + self.assertEqual(co_main.dt_field, test_dt) self.assertEqual(co_main.select_field, 'active') self.assertEqual(co_main.obj_field_id, site.pk) diff --git a/netbox_custom_objects/views.py b/netbox_custom_objects/views.py index 15ea6f4..4af7494 100644 --- a/netbox_custom_objects/views.py +++ b/netbox_custom_objects/views.py @@ -591,10 +591,6 @@ def custom_save(self, commit=True): return form_class - def get_extra_context(self, request, obj): - return { - } - @register_model_view(CustomObject, "delete") class CustomObjectDeleteView(generic.ObjectDeleteView): @@ -704,10 +700,6 @@ def get_form(self, queryset): return form - def get_extra_context(self, request): - return { - } - @register_model_view(CustomObject, "bulk_delete", path="delete", detail=False) class CustomObjectBulkDeleteView(CustomObjectTableMixin, generic.BulkDeleteView): @@ -794,10 +786,6 @@ def get_model_form(self, queryset): return form - def get_extra_context(self, request): - return { - } - class CustomObjectJournalView(ConditionalLoginRequiredMixin, View): """ From 0480531a338cb3883dbacc431f2db66d7917250c Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 11:01:08 -0700 Subject: [PATCH 16/25] cleanup --- .../0007_fix_object_field_fk_deferrable.py | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 netbox_custom_objects/migrations/0007_fix_object_field_fk_deferrable.py diff --git a/netbox_custom_objects/migrations/0007_fix_object_field_fk_deferrable.py b/netbox_custom_objects/migrations/0007_fix_object_field_fk_deferrable.py new file mode 100644 index 0000000..c03b7ca --- /dev/null +++ b/netbox_custom_objects/migrations/0007_fix_object_field_fk_deferrable.py @@ -0,0 +1,74 @@ +""" +Drop DEFERRABLE INITIALLY DEFERRED from FK constraints on custom object tables. + +Prior to this migration, _ensure_field_fk_constraint() created FK constraints +with DEFERRABLE INITIALLY DEFERRED. That attribute causes PostgreSQL to queue +trigger events that block subsequent ALTER TABLE calls (e.g. remove_field during +a branch revert), raising "cannot ALTER TABLE because it has pending trigger +events". + +This migration finds all DEFERRABLE FK constraints on tables whose names start +with "custom_objects_" and recreates them as non-DEFERRABLE with ON DELETE +CASCADE, matching the behaviour of the updated _ensure_field_fk_constraint(). +""" + +from django.db import migrations + + +def fix_deferrable_fk_constraints(apps, schema_editor): + """ + Re-create any DEFERRABLE FK constraints on custom object tables as + non-DEFERRABLE. Uses information_schema so no Django model loading + is required — safe to run during the migration pass even though dynamic + models are not yet registered. + """ + with schema_editor.connection.cursor() as cursor: + # Find all DEFERRABLE FK constraints on custom_objects_* tables. + cursor.execute(""" + SELECT + tc.table_name, + tc.constraint_name, + kcu.column_name, + ccu.table_name AS foreign_table_name + FROM information_schema.table_constraints AS tc + JOIN information_schema.key_column_usage AS kcu + ON tc.constraint_name = kcu.constraint_name + AND tc.table_schema = kcu.table_schema + JOIN information_schema.constraint_column_usage AS ccu + ON ccu.constraint_name = tc.constraint_name + AND ccu.table_schema = tc.table_schema + JOIN information_schema.referential_constraints AS rc + ON tc.constraint_name = rc.constraint_name + AND tc.table_schema = rc.constraint_schema + WHERE tc.constraint_type = 'FOREIGN KEY' + AND tc.table_name LIKE 'custom_objects\\_%%' + AND rc.is_deferrable = 'YES' + """) + rows = cursor.fetchall() + + for table_name, constraint_name, column_name, foreign_table in rows: + new_constraint_name = f'{table_name}_{column_name}_fk_cascade' + cursor.execute( + f'ALTER TABLE "{table_name}" DROP CONSTRAINT "{constraint_name}"' + ) + cursor.execute(f""" + ALTER TABLE "{table_name}" + ADD CONSTRAINT "{new_constraint_name}" + FOREIGN KEY ("{column_name}") + REFERENCES "{foreign_table}" ("id") + ON DELETE CASCADE + """) + + +class Migration(migrations.Migration): + + dependencies = [ + ('netbox_custom_objects', '0006_customobjecttypefield_related_name_and_more'), + ] + + operations = [ + migrations.RunPython( + fix_deferrable_fk_constraints, + migrations.RunPython.noop, + ), + ] From a3b0d8a9b953cae02434e0d2988c0ffd2e4a61fa Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 11:04:01 -0700 Subject: [PATCH 17/25] fix squash merge --- netbox_custom_objects/models.py | 37 ++++++++++++++++++----------- netbox_custom_objects/tests/base.py | 6 ++--- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index b598769..09315b6 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -1,3 +1,4 @@ +import contextvars import decimal import re import threading @@ -66,6 +67,14 @@ class UniquenessConstraintTestError(Exception): USER_TABLE_DATABASE_NAME_PREFIX = "custom_objects_" +# Per-context storage for CO field values deferred during squash merge. +# Using ContextVar instead of a class-level dict so that concurrent merges +# (different threads or coroutines) each get an isolated copy. +# Shape: {db_table: {co_pk: {'using': alias, 'data': {field_name: value}}}} +_deferred_co_field_data: contextvars.ContextVar[dict | None] = contextvars.ContextVar( + '_deferred_co_field_data', default=None +) + def _get_schema_connection(): """ @@ -94,7 +103,7 @@ def _apply_deferred_co_field(field_instance): custom object rows inserted before their columns existed (squash merge ordering) receive their correct values via a raw UPDATE. - ``CustomObject._deferred_field_data`` has the shape:: + ``_deferred_co_field_data`` (ContextVar) has the shape:: {db_table: {co_pk: {'using': alias, 'data': {field_name: value}}}} @@ -106,12 +115,13 @@ def _apply_deferred_co_field(field_instance): from extras.choices import CustomFieldTypeChoices # No deferred data at all — fast path. - if not CustomObject._deferred_field_data: + deferred = _deferred_co_field_data.get() + if not deferred: return cot = field_instance.custom_object_type table_name = cot.get_database_table_name() - per_table = CustomObject._deferred_field_data.get(table_name) + per_table = deferred.get(table_name) if not per_table: return @@ -285,11 +295,6 @@ class CustomObject( objects = RestrictedQuerySet.as_manager() - # Pending custom-field values for CO rows created before their columns existed. - # Populated by deserialize_object(); consumed by _apply_deferred_co_field(). - # Format: {db_table: {co_pk: {'using': alias, 'data': {field_name: value}}}} - _deferred_field_data: dict = {} - class Meta: abstract = True @@ -307,9 +312,9 @@ def deserialize_object(cls, data, pk=None): This hook: 1. Deserializes the CO using a fresh model (re-queried from DB). 2. Does save_base(raw=True) as normal. - 3. Stores the full postchange_data in _deferred_field_data so that - CustomObjectTypeField.save() can UPDATE the row after each column is - added (handles the squash ordering case). + 3. Stores the full postchange_data in _deferred_co_field_data (ContextVar) + so that CustomObjectTypeField.save() can UPDATE the row after each + column is added (handles the squash ordering case). """ from utilities.serialization import deserialize_object as _deserialize from netbox_custom_objects.utilities import extract_cot_id_from_model_name @@ -347,9 +352,13 @@ def save(self, using=None, **_kwargs): # (If pk was None before save_base, obj.pk is now the DB-assigned id.) obj_pk = obj.pk # Register full data for deferred column updates (squash ordering fix). - if table_name not in cls._deferred_field_data: - cls._deferred_field_data[table_name] = {} - cls._deferred_field_data[table_name][obj_pk] = { + deferred = _deferred_co_field_data.get() + if deferred is None: + deferred = {} + _deferred_co_field_data.set(deferred) + if table_name not in deferred: + deferred[table_name] = {} + deferred[table_name][obj_pk] = { 'using': _using, 'data': full_data, } diff --git a/netbox_custom_objects/tests/base.py b/netbox_custom_objects/tests/base.py index 73cb3bd..e5c052d 100644 --- a/netbox_custom_objects/tests/base.py +++ b/netbox_custom_objects/tests/base.py @@ -67,9 +67,9 @@ def setUp(self): def tearDown(self): from core.models import ObjectChange - from netbox_custom_objects.models import CustomObject - # Flush deferred CO field data so it doesn't bleed into the next test. - CustomObject._deferred_field_data.clear() + from netbox_custom_objects.models import _deferred_co_field_data + # Reset deferred CO field data so it doesn't bleed into the next test. + _deferred_co_field_data.set(None) # Delete COTs and their backing tables before the DB flush. for cot in CustomObjectType.objects.all(): try: From 38366696792a4559e8ee98f9d1482167ec6a4413 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 21 Apr 2026 13:37:10 -0700 Subject: [PATCH 18/25] add tests --- .../0007_fix_object_field_fk_deferrable.py | 2 +- netbox_custom_objects/models.py | 6 +- netbox_custom_objects/tests/test_branching.py | 558 ++++++++++++++++++ 3 files changed, 563 insertions(+), 3 deletions(-) diff --git a/netbox_custom_objects/migrations/0007_fix_object_field_fk_deferrable.py b/netbox_custom_objects/migrations/0007_fix_object_field_fk_deferrable.py index c03b7ca..6e57b53 100644 --- a/netbox_custom_objects/migrations/0007_fix_object_field_fk_deferrable.py +++ b/netbox_custom_objects/migrations/0007_fix_object_field_fk_deferrable.py @@ -42,7 +42,7 @@ def fix_deferrable_fk_constraints(apps, schema_editor): AND tc.table_schema = rc.constraint_schema WHERE tc.constraint_type = 'FOREIGN KEY' AND tc.table_name LIKE 'custom_objects\\_%%' - AND rc.is_deferrable = 'YES' + AND tc.is_deferrable = 'YES' """) rows = cursor.fetchall() diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 09315b6..8078507 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -1508,12 +1508,14 @@ def clean(self): {"unique": _("Uniqueness cannot be enforced for boolean or multiobject fields")} ) - # Check if uniqueness constraint can be applied when changing from non-unique to unique + # Check if uniqueness constraint can be applied when changing from non-unique to unique. + # Skip when _original is absent (e.g. during deserialization in branch merge/revert). if ( self.pk and self.unique - and not self.original.unique and not self._state.adding + and hasattr(self, '_original') + and not self.original.unique ): field_type = FIELD_TYPE_CLASS[self.type]() model_field = field_type.get_model_field(self) diff --git a/netbox_custom_objects/tests/test_branching.py b/netbox_custom_objects/tests/test_branching.py index f5c7e0f..2f869e0 100644 --- a/netbox_custom_objects/tests/test_branching.py +++ b/netbox_custom_objects/tests/test_branching.py @@ -316,6 +316,281 @@ def test_comprehensive_merge_and_revert(self): f'Field {name!r} must not be in main after revert', ) + # ── object modified inside branch ───────────────────────────────────── + + def test_object_modified_merge_and_revert(self): + """ + CO that exists in main is modified inside a branch. Merge brings the + new value to main; revert restores the original. + + The COT and field are created in main before provisioning so the branch + has a full schema copy. Only the CO data changes inside the branch. + """ + request = _make_request(self.user) + + with event_tracking(request): + cot = CustomObjectType.objects.create(name='modify_cot', slug='modify-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='notes', + label='Notes', + type='text', + ) + Model = cot.get_model() + co = Model.objects.create(notes='original value') + + co_pk = co.pk + branch = _provision_branch('Modify Branch', self.MERGE_STRATEGY, self.user) + branch_request = _make_request(self.user) + + # Modify CO inside the branch. + with activate_branch(branch), event_tracking(branch_request): + branch_co = cot.get_model().objects.get(pk=co_pk) + branch_co.snapshot() # captures pre-change state for ObjectChange.diff()['pre'] during revert + branch_co.notes = 'modified in branch' + branch_co.save() + + # Main must not see the modification yet. + co.refresh_from_db() + self.assertEqual(co.notes, 'original value', 'Main must not see branch modification before merge') + + # ── merge ───────────────────────────────────────────────────────── + branch.merge(user=self.user, commit=True) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.MERGED) + + co.refresh_from_db() + self.assertEqual(co.notes, 'modified in branch', 'Main must see modified value after merge') + + # ── revert ──────────────────────────────────────────────────────── + branch.revert(user=self.user, commit=True) + + co.refresh_from_db() + self.assertEqual(co.notes, 'original value', 'Main must have original value after revert') + + # ── object deleted inside branch ────────────────────────────────────── + + def test_object_deleted_merge_and_revert(self): + """ + CO that exists in main is deleted inside a branch. Merge removes it + from main; revert restores it. + """ + request = _make_request(self.user) + + with event_tracking(request): + cot = CustomObjectType.objects.create(name='delete_cot', slug='delete-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='notes', + label='Notes', + type='text', + ) + Model = cot.get_model() + co = Model.objects.create(notes='will be deleted') + + co_pk = co.pk + branch = _provision_branch('Delete Branch', self.MERGE_STRATEGY, self.user) + branch_request = _make_request(self.user) + + with activate_branch(branch), event_tracking(branch_request): + cot.get_model().objects.get(pk=co_pk).delete() + + # CO must still exist in main before merge. + self.assertTrue( + cot.get_model().objects.filter(pk=co_pk).exists(), + 'CO must still exist in main before merge', + ) + + # ── merge ───────────────────────────────────────────────────────── + branch.merge(user=self.user, commit=True) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.MERGED) + + self.assertFalse( + cot.get_model().objects.filter(pk=co_pk).exists(), + 'CO must be deleted from main after merge', + ) + + # ── revert ──────────────────────────────────────────────────────── + branch.revert(user=self.user, commit=True) + + self.assertTrue( + cot.get_model().objects.filter(pk=co_pk).exists(), + 'CO must be restored in main after revert', + ) + + # ── field renamed inside branch → merge ─────────────────────────────── + + def test_field_rename_merge_and_revert(self): + """ + Field created and then renamed inside a branch. Merge brings the COT + with the renamed column to main; revert removes it. + + Exercises _schema_alter_field via the merge deserialization path using + PK-based rename detection (same PK, different name values). + """ + branch = _provision_branch('Rename Branch', self.MERGE_STRATEGY, self.user) + request = _make_request(self.user) + + with activate_branch(branch), event_tracking(request): + cot = CustomObjectType.objects.create(name='rename_cot', slug='rename-cot') + field = CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='old_name', + label='Old Name', + type='text', + ) + # Load from DB so _original is set, then rename. + field = CustomObjectTypeField.objects.get(pk=field.pk) + field.snapshot() # captures pre-change state for ObjectChange.diff()['pre'] during revert + field.name = 'new_name' + field.label = 'New Name' + field.save() + Model = cot.get_model() + co = Model.objects.create(new_name='value after rename') + + field_pk, cot_pk, co_pk = field.pk, cot.pk, co.pk + + self.assertFalse( + CustomObjectType.objects.filter(pk=cot_pk).exists(), + 'COT must not exist in main before merge', + ) + + # ── merge ───────────────────────────────────────────────────────── + branch.merge(user=self.user, commit=True) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.MERGED) + + self.assertTrue(CustomObjectTypeField.objects.filter(pk=field_pk).exists()) + field_main = CustomObjectTypeField.objects.get(pk=field_pk) + self.assertEqual(field_main.name, 'new_name', 'Field must have new name in main after merge') + + cot_main = CustomObjectType.objects.get(pk=cot_pk) + co_main = cot_main.get_model().objects.get(pk=co_pk) + self.assertEqual(co_main.new_name, 'value after rename') + + # ── revert ──────────────────────────────────────────────────────── + branch.revert(user=self.user, commit=True) + + self.assertFalse(CustomObjectType.objects.filter(pk=cot_pk).exists()) + self.assertFalse(CustomObjectTypeField.objects.filter(pk=field_pk).exists()) + + # ── unique constraint toggled inside branch → merge ─────────────────── + + def test_field_unique_toggle_merge_and_revert(self): + """ + Field created without a unique constraint inside a branch, then + toggled to unique=True. Merge brings the COT with the UNIQUE + constraint to main; revert removes it. + + Exercises alter_field for constraint-only changes via the merge path. + """ + from django.db import connection as main_conn + + branch = _provision_branch('Unique Branch', self.MERGE_STRATEGY, self.user) + request = _make_request(self.user) + + with activate_branch(branch), event_tracking(request): + cot = CustomObjectType.objects.create(name='unique_cot', slug='unique-cot') + field = CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='code', + label='Code', + type='text', + unique=False, + ) + # Load from DB so _original is set, then enable unique. + field = CustomObjectTypeField.objects.get(pk=field.pk) + field.snapshot() # captures pre-change state for ObjectChange.diff()['pre'] during revert + field.unique = True + field.save() + Model = cot.get_model() + co_pk = Model.objects.create(code='ABC').pk + + field_pk, cot_pk = field.pk, cot.pk + + # ── merge ───────────────────────────────────────────────────────── + branch.merge(user=self.user, commit=True) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.MERGED) + + field_main = CustomObjectTypeField.objects.get(pk=field_pk) + self.assertTrue(field_main.unique, 'Field must have unique=True in main after merge') + + # Verify the UNIQUE constraint exists in main's physical schema. + cot_main = CustomObjectType.objects.get(pk=cot_pk) + table_name = cot_main.get_database_table_name() + with main_conn.cursor() as cursor: + constraints = main_conn.introspection.get_constraints(cursor, table_name) + self.assertTrue( + any(c['unique'] and c.get('columns') == ['code'] for c in constraints.values()), + 'UNIQUE constraint on "code" must exist in main schema after merge', + ) + + # CO with code='ABC' must have survived the merge. + cot_main2 = CustomObjectType.objects.get(pk=cot_pk) + self.assertTrue(cot_main2.get_model().objects.filter(pk=co_pk).exists()) + + # ── revert ──────────────────────────────────────────────────────── + branch.revert(user=self.user, commit=True) + + self.assertFalse(CustomObjectType.objects.filter(pk=cot_pk).exists()) + + # ── non-schema field attributes inside branch → merge ───────────────── + + def test_field_non_schema_attrs_merge_and_revert(self): + """ + Field attributes that do not affect the physical schema (label, + primary, required, description) survive a branch merge and revert + without causing schema errors or spurious ALTER TABLE calls. + + These attributes are excluded from _field_schema_key and exist only + at the application layer. This test confirms that altering them in + a branch and merging correctly updates the ORM-level field record in + main without touching the DB column definition. + """ + branch = _provision_branch('Attrs Branch', self.MERGE_STRATEGY, self.user) + request = _make_request(self.user) + + with activate_branch(branch), event_tracking(request): + cot = CustomObjectType.objects.create(name='attrs_cot', slug='attrs-cot') + field = CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='title', + label='Title', + type='text', + primary=False, + required=False, + description='original description', + ) + # Load from DB so _original is set, then mutate non-schema attrs. + field = CustomObjectTypeField.objects.get(pk=field.pk) + field.snapshot() # captures pre-change state for ObjectChange.diff()['pre'] during revert + field.label = 'Updated Title' + field.primary = True + field.required = True + field.description = 'updated description' + field.save() + + field_pk, cot_pk = field.pk, cot.pk + + # ── merge ───────────────────────────────────────────────────────── + branch.merge(user=self.user, commit=True) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.MERGED) + + field_main = CustomObjectTypeField.objects.get(pk=field_pk) + self.assertEqual(field_main.label, 'Updated Title') + self.assertTrue(field_main.primary) + self.assertTrue(field_main.required) + self.assertEqual(field_main.description, 'updated description') + + # ── revert ──────────────────────────────────────────────────────── + branch.revert(user=self.user, commit=True) + + self.assertFalse(CustomObjectType.objects.filter(pk=cot_pk).exists()) + self.assertFalse(CustomObjectTypeField.objects.filter(pk=field_pk).exists()) + # ── Concrete test classes (one per merge strategy) ──────────────────────────── @@ -411,3 +686,286 @@ def test_main_changes_synced_to_branch(self): BranchModel = cot_branch.get_model() co_branch = BranchModel.objects.get(pk=co_pk) self.assertEqual(co_branch.title, 'main object') + + +# ── Drift detection and Branch.migrate() tests ──────────────────────────────── + +@unittest.skipUnless(HAS_BRANCHING, 'netbox-branching is not installed') +class BranchMigrateTestCase(TransactionCleanupMixin, TransactionTestCase): + """ + Tests for the drift-detection and schema-reconciliation path. + + Scenario: + 1. A COT and field(s) are created in main and a branch is provisioned + (branch has a full schema copy at provision time). + 2. A field is then modified in main (added, removed, renamed, or + unique-toggled). on_custom_object_field_changed fires and marks the + branch PENDING_MIGRATIONS. + 3. branch.migrate(user) runs the normal Django migration pass and then + emits post_migrate, which fires on_branch_migrated. That handler + reconciles the branch's physical schema against main's current field + definitions. + 4. The branch is back to READY and its physical column layout matches main. + + These tests do NOT use merge/revert — they isolate the separate + on_branch_migrated reconciliation path for branches that pre-date a main + schema change. They use the iterative strategy only because the strategy + affects merge order, not schema reconciliation. + """ + + def setUp(self): + super().setUp() + _recreate_contenttypes() + self.user = User.objects.create_user(username='testuser') + self.request = _make_request(self.user) + + def tearDown(self): + _close_branch_connections() + super().tearDown() + + def _get_branch_columns(self, branch, table_name): + """Return the set of column names on table_name in the branch schema.""" + conn = connections[branch.connection_name] + with conn.cursor() as cursor: + return {col.name for col in conn.introspection.get_table_description(cursor, table_name)} + + def _get_branch_tables(self, branch): + """Return the set of table names in the branch schema.""" + conn = connections[branch.connection_name] + with conn.cursor() as cursor: + return set(conn.introspection.table_names(cursor)) + + def _get_branch_constraints(self, branch, table_name): + """Return the constraints dict for table_name in the branch schema.""" + conn = connections[branch.connection_name] + with conn.cursor() as cursor: + return conn.introspection.get_constraints(cursor, table_name) + + # ── field added to main ─────────────────────────────────────────────── + + def test_field_added_to_main_triggers_branch_migrate(self): + """ + Field added to a COT in main marks the branch PENDING_MIGRATIONS. + branch.migrate() fires on_branch_migrated which calls add_field to + create the new column in the branch schema. + """ + cot = CustomObjectType.objects.create(name='drift_add_cot', slug='drift-add-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='existing_field', + label='Existing', + type='text', + ) + + branch = _provision_branch('Drift Add Branch', 'iterative', self.user) + table_name = cot.get_database_table_name() + + self.assertEqual(branch.status, BranchStatusChoices.READY) + self.assertIn('existing_field', self._get_branch_columns(branch, table_name)) + + # Add a new field to main — on_custom_object_field_changed marks branch PENDING. + CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='new_field', + label='New Field', + type='integer', + ) + + branch.refresh_from_db() + self.assertEqual( + branch.status, BranchStatusChoices.PENDING_MIGRATIONS, + 'Branch must be PENDING_MIGRATIONS after field added to main', + ) + self.assertNotIn( + 'new_field', self._get_branch_columns(branch, table_name), + 'New column must not exist in branch before migrate', + ) + + # migrate() fires on_branch_migrated → add_field runs on the branch schema. + branch.migrate(user=self.user) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.READY) + + self.assertIn( + 'new_field', self._get_branch_columns(branch, table_name), + 'New column must be present in branch schema after migrate', + ) + + # ── field deleted from main ─────────────────────────────────────────── + + def test_field_deleted_from_main_triggers_branch_migrate(self): + """ + Field deleted from a COT in main marks the branch PENDING_MIGRATIONS. + branch.migrate() fires on_branch_migrated which calls remove_field to + drop the column from the branch schema. + """ + cot = CustomObjectType.objects.create(name='drift_del_cot', slug='drift-del-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='keep_field', + label='Keep', + type='text', + ) + drop_field = CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='drop_field', + label='Drop', + type='text', + ) + + branch = _provision_branch('Drift Del Branch', 'iterative', self.user) + table_name = cot.get_database_table_name() + + self.assertIn('drop_field', self._get_branch_columns(branch, table_name)) + + # Delete field from main. + drop_field.delete() + + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.PENDING_MIGRATIONS) + + # Column still present in branch before migrate. + self.assertIn('drop_field', self._get_branch_columns(branch, table_name)) + + branch.migrate(user=self.user) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.READY) + + cols = self._get_branch_columns(branch, table_name) + self.assertNotIn( + 'drop_field', cols, + 'Deleted column must be absent from branch schema after migrate', + ) + self.assertIn('keep_field', cols, 'Unmodified column must remain in branch schema') + + # ── field renamed in main ───────────────────────────────────────────── + + def test_field_renamed_in_main_triggers_branch_migrate(self): + """ + Field renamed in main marks the branch PENDING_MIGRATIONS. + branch.migrate() fires on_branch_migrated which calls alter_field to + rename the column in the branch schema using PK-based matching (same + PK in both main and branch, different name values). + """ + cot = CustomObjectType.objects.create(name='drift_ren_cot', slug='drift-ren-cot') + field = CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='old_col', + label='Old', + type='text', + ) + + branch = _provision_branch('Drift Ren Branch', 'iterative', self.user) + table_name = cot.get_database_table_name() + + self.assertIn('old_col', self._get_branch_columns(branch, table_name)) + + # Rename in main — load from DB so _original is set before modifying. + field = CustomObjectTypeField.objects.get(pk=field.pk) + field.name = 'new_col' + field.label = 'New' + field.save() + + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.PENDING_MIGRATIONS) + + branch.migrate(user=self.user) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.READY) + + cols = self._get_branch_columns(branch, table_name) + self.assertIn('new_col', cols, 'Renamed column must exist in branch schema after migrate') + self.assertNotIn('old_col', cols, 'Old column name must be absent from branch schema after migrate') + + # ── unique constraint toggled in main ───────────────────────────────── + + def test_unique_toggled_in_main_triggers_branch_migrate(self): + """ + Field's unique constraint toggled in main marks the branch + PENDING_MIGRATIONS. branch.migrate() reconciles the constraint in + the branch's physical schema. + """ + cot = CustomObjectType.objects.create(name='drift_uniq_cot', slug='drift-uniq-cot') + field = CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='code', + label='Code', + type='text', + unique=False, + ) + + branch = _provision_branch('Drift Uniq Branch', 'iterative', self.user) + table_name = cot.get_database_table_name() + + # Branch must not have a unique constraint on 'code' initially. + constraints_before = self._get_branch_constraints(branch, table_name) + self.assertFalse( + any(c['unique'] and c.get('columns') == ['code'] for c in constraints_before.values()), + 'Branch must not have UNIQUE on "code" before toggle', + ) + + # Toggle unique=True in main. + field = CustomObjectTypeField.objects.get(pk=field.pk) + field.unique = True + field.save() + + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.PENDING_MIGRATIONS) + + branch.migrate(user=self.user) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.READY) + + constraints_after = self._get_branch_constraints(branch, table_name) + self.assertTrue( + any(c['unique'] and c.get('columns') == ['code'] for c in constraints_after.values()), + 'Branch must have UNIQUE constraint on "code" after migrate', + ) + + # ── multiobject field renamed in main (through-table rename) ───────── + + def test_multiobject_field_renamed_in_main_triggers_branch_migrate(self): + """ + MULTIOBJECT field renamed in main marks the branch PENDING_MIGRATIONS. + branch.migrate() fires on_branch_migrated which renames the through + table in the branch schema in addition to the column alter_field. + + Exercises the MULTIOBJECT rename branch of _schema_alter_field. + """ + from core.models import ObjectType + + site_ot = ObjectType.objects.get(app_label='dcim', model='site') + + cot = CustomObjectType.objects.create(name='drift_m2m_cot', slug='drift-m2m-cot') + field = CustomObjectTypeField.objects.create( + custom_object_type=cot, + name='related_sites', + label='Related Sites', + type='multiobject', + related_object_type=site_ot, + ) + + old_through = field.through_table_name + + branch = _provision_branch('Drift M2M Branch', 'iterative', self.user) + + self.assertIn(old_through, self._get_branch_tables(branch)) + + # Rename in main. + field = CustomObjectTypeField.objects.get(pk=field.pk) + field.name = 'linked_sites' + field.label = 'Linked Sites' + field.save() + + new_through = field.through_table_name + + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.PENDING_MIGRATIONS) + + branch.migrate(user=self.user) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.READY) + + branch_tables = self._get_branch_tables(branch) + self.assertIn(new_through, branch_tables, 'Renamed through table must exist in branch after migrate') + self.assertNotIn(old_through, branch_tables, 'Old through table must be absent from branch after migrate') From 1b1584ca9bf1492a9d59f1cb5fa39b7a50a6fdf9 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 23 Apr 2026 11:53:35 -0700 Subject: [PATCH 19/25] update changelog entries on rename, remove branch migrate --- netbox_custom_objects/__init__.py | 6 +-- netbox_custom_objects/branching.py | 61 ++++++--------------- netbox_custom_objects/models.py | 85 ++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 51 deletions(-) diff --git a/netbox_custom_objects/__init__.py b/netbox_custom_objects/__init__.py index a661afd..0e7e04a 100644 --- a/netbox_custom_objects/__init__.py +++ b/netbox_custom_objects/__init__.py @@ -184,13 +184,9 @@ def ready(self): # Wire into the netbox-branching lifecycle when that plugin is installed. # Import is lazy so the plugin remains functional without branching. try: - from django.db.models.signals import post_delete, post_save from netbox_branching.signals import post_migrate as branching_post_migrate - from netbox_custom_objects.branching import on_branch_migrated, on_custom_object_field_changed - from netbox_custom_objects.models import CustomObjectTypeField + from netbox_custom_objects.branching import on_branch_migrated branching_post_migrate.connect(on_branch_migrated) - post_save.connect(on_custom_object_field_changed, sender=CustomObjectTypeField) - post_delete.connect(on_custom_object_field_changed, sender=CustomObjectTypeField) except ImportError: pass diff --git a/netbox_custom_objects/branching.py b/netbox_custom_objects/branching.py index 98ca3ac..488f699 100644 --- a/netbox_custom_objects/branching.py +++ b/netbox_custom_objects/branching.py @@ -8,15 +8,24 @@ To close that gap, ``on_branch_migrated`` fires after the migration pass and reconciles each custom object type's physical schema in the branch against the -current field definitions in main. The reconciliation mirrors what -``CustomObjectTypeField.save()`` already does when detecting old-vs-new field -state: it compares the branch's ``CustomObjectTypeField`` rows (snapshot from -provision time) with main's current rows, matched by primary key, and calls -``add_field`` / ``alter_field`` / ``remove_field`` for any differences. +current field definitions in main. It compares the branch's +``CustomObjectTypeField`` rows (snapshot from provision time) with main's +current rows, matched by primary key, and calls ``add_field`` / ``alter_field`` +/ ``remove_field`` for any differences. + +Schema ordering during sync and merge is handled naturally by the chronological +ObjectChange replay in ``Branch.sync()`` / ``Branch.merge()``: +``CustomObjectTypeField`` changes always precede ``CustomObject`` data changes +in the log. The schema helpers (``_schema_add_field``, ``_schema_alter_field``) +are idempotent, so replaying an ObjectChange after ``on_branch_migrated`` has +already applied the same schema change is safe. ``CustomObjectTypeField`` has ``ChangeLoggingMixin``, so its rows are replicated into the branch schema at provision time — the same source-of-truth mechanism -used for all other branchable models. +used for all other branchable models. Field-rename conflicts (same field PK +renamed differently in main and branch) surface as ``ChangeDiff`` conflicts and +require user acknowledgment before merge/sync proceeds; the existing branching +conflict mechanism handles this without any special casing. """ import logging @@ -75,46 +84,6 @@ def check_pending_branch_migrations(): ) -def on_custom_object_field_changed(sender, instance, **kwargs): - """ - Mark all READY branches as PENDING_MIGRATIONS when a ``CustomObjectTypeField`` - is created, modified, or deleted in the main schema. - - This surfaces the pending state in the branching UI exactly like a normal - Django-migration, prompting users to click "Migrate Branch". That action - calls ``Branch.migrate()``, which fires ``on_branch_migrated`` and reconciles - the physical column differences. - - Branches provisioned before this COT's table existed will be over-marked as - PENDING_MIGRATIONS but ``on_branch_migrated`` will skip them (no table in - branch) so the extra marking is harmless. The alternative — opening a - connection per branch to check whether the table exists — adds O(branches) - latency to every field save, which is worse at scale. - - Skipped when the change happens inside a branch context — the field edit only - affects that branch's schema, not main, so no other branches need updating. - """ - try: - from netbox_branching.contextvars import active_branch - if active_branch.get() is not None: - return - except ImportError: - return - - from netbox_branching.choices import BranchStatusChoices - from netbox_branching.models import Branch - - updated = Branch.objects.filter(status=BranchStatusChoices.READY).update( - status=BranchStatusChoices.PENDING_MIGRATIONS, - ) - if updated: - logger.info( - 'Marked %d branch(es) as PENDING_MIGRATIONS due to field changes on %s', - updated, - instance.custom_object_type, - ) - - def _field_schema_key(f): """ Return the subset of ``CustomObjectTypeField`` attributes that affect the diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 8078507..5f7deea 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -1,5 +1,6 @@ import contextvars import decimal +import logging import re import threading from datetime import date, datetime @@ -59,6 +60,9 @@ from netbox_custom_objects.utilities import _suppress_clear_cache, generate_model +logger = logging.getLogger('netbox_custom_objects.models') + + class UniquenessConstraintTestError(Exception): """Custom exception used to signal successful uniqueness constraint test.""" @@ -160,10 +164,23 @@ def _schema_add_field(fi, model, schema_editor, schema_conn): Handles through-table creation for MULTIOBJECT fields. Does NOT apply deferred CO field data — callers that need that (squash merge context) must call ``_apply_deferred_co_field(fi)`` separately after this returns. + + Idempotent: skips the ALTER TABLE if the column already exists (e.g. when + ``on_branch_migrated`` pre-added it and sync later replays the ObjectChange). """ ft = FIELD_TYPE_CLASS[fi.type]() mf = ft.get_model_field(fi) mf.contribute_to_class(model, fi.name) + + with schema_conn.cursor() as cursor: + existing_cols = { + col.name + for col in schema_conn.introspection.get_table_description(cursor, model._meta.db_table) + } + if mf.column in existing_cols: + logger.debug('_schema_add_field: %r already exists on %s, skipping', mf.column, model._meta.db_table) + return + schema_editor.add_field(model, mf) if fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: ft.create_m2m_table(fi, model, fi.name, schema_conn=schema_conn) @@ -219,12 +236,28 @@ def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, exist connection. When given, through-table operations are guarded by membership checks. When ``None`` (main-schema context) the schema_conn is introspected once on demand. + + Idempotent: skips the ALTER TABLE if the old column is already gone and the + new column already exists (e.g. when ``on_branch_migrated`` pre-applied the + rename and sync later replays the ObjectChange). """ old_mf = FIELD_TYPE_CLASS[old_fi.type]().get_model_field(old_fi) new_mf = FIELD_TYPE_CLASS[new_fi.type]().get_model_field(new_fi) old_mf.contribute_to_class(model, old_fi.name) new_mf.contribute_to_class(model, new_fi.name) + with schema_conn.cursor() as cursor: + existing_cols = { + col.name + for col in schema_conn.introspection.get_table_description(cursor, model._meta.db_table) + } + if old_mf.column not in existing_cols and new_mf.column in existing_cols: + logger.debug( + '_schema_alter_field: %r already renamed to %r on %s, skipping', + old_mf.column, new_mf.column, model._meta.db_table, + ) + return + if ( new_fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT and old_fi.name != new_fi.name @@ -1150,6 +1183,53 @@ def custom_object_type_post_save_handler(sender, instance, created, **kwargs): instance.save() +def _rename_objectchange_field_key(fi, old_name, new_name): + """ + Rename a JSON key in all ObjectChange records for CustomObject instances of + this field's type, reflecting a field rename from *old_name* to *new_name*. + + Updates both ``prechange_data`` and ``postchange_data`` in the ObjectChange + table, and ``original``/``modified``/``current`` in netbox-branching's + ChangeDiff table when that plugin is installed. + + Field names are validated with ``^[a-z0-9_]+$`` so string formatting of the + column names here is safe against SQL injection. + + This runs inside the same ``transaction.atomic()`` block as + ``CustomObjectTypeField.save()``, so it rolls back cleanly if the enclosing + transaction is aborted. + """ + from django.db import connections + + cot = fi.custom_object_type + model = cot.get_model() + ct = ContentType.objects.get_for_model(model) + conn = _get_schema_connection() + + oc_sql = ( + 'UPDATE core_objectchange ' + 'SET {col} = ({col} - %s) || jsonb_build_object(%s, {col}->%s) ' + 'WHERE changed_object_type_id = %s AND {col} ? %s' + ) + with connections[conn.alias].cursor() as cursor: + for json_col in ('prechange_data', 'postchange_data'): + cursor.execute(oc_sql.format(col=json_col), [old_name, new_name, old_name, ct.id, old_name]) + + logger.debug('_rename_objectchange_field_key: %r → %r for %s', old_name, new_name, ct) + + try: + cd_sql = ( + 'UPDATE netbox_branching_changediff ' + 'SET {col} = ({col} - %s) || jsonb_build_object(%s, {col}->%s) ' + 'WHERE object_type_id = %s AND {col} IS NOT NULL AND {col} ? %s' + ) + with connections[conn.alias].cursor() as cursor: + for json_col in ('original', 'modified', 'current'): + cursor.execute(cd_sql.format(col=json_col), [old_name, new_name, old_name, ct.id, old_name]) + except Exception: + pass # netbox-branching not installed or table absent + + class CustomObjectTypeField(CloningMixin, ExportTemplatesMixin, ChangeLoggedModel): custom_object_type = models.ForeignKey( CustomObjectType, on_delete=models.CASCADE, related_name="fields" @@ -2039,6 +2119,11 @@ def save(self, *args, **kwargs): else: _schema_alter_field(self.original, self, model, schema_editor, schema_conn) + # When the field is renamed, update ObjectChange / ChangeDiff JSON keys so + # historical audit records and branch diffs stay consistent with the new name. + if not self._state.adding and self._original_name != self.name: + _rename_objectchange_field_key(self, self._original_name, self.name) + # Ensure FK constraints are properly created for OBJECT fields with CASCADE behavior should_ensure_fk = False if self.type == CustomFieldTypeChoices.TYPE_OBJECT: From ba8eda3b850d238d9ca761fa1e3a0e25862ea025 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 23 Apr 2026 13:09:13 -0700 Subject: [PATCH 20/25] cleanup --- netbox_custom_objects/models.py | 44 ++++++++++++++++++- netbox_custom_objects/tests/test_branching.py | 10 +++-- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 5f7deea..90f16a6 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -156,6 +156,21 @@ def _apply_deferred_co_field(field_instance): [value, co_pk], ) + # Remove the consumed key from each entry so that processed field data does + # not persist in the ContextVar beyond its useful lifetime (e.g. on a retry + # after a partial failure, stale data from a previous attempt is avoided). + for entry in per_table.values(): + entry['data'].pop(data_key, None) + + # Prune entries whose data dict is now exhausted. + exhausted = [pk for pk, entry in per_table.items() if not entry['data']] + for pk in exhausted: + del per_table[pk] + if not per_table: + del deferred[table_name] + if not deferred: + _deferred_co_field_data.set(None) + def _schema_add_field(fi, model, schema_editor, schema_conn): """ @@ -241,6 +256,22 @@ def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, exist new column already exists (e.g. when ``on_branch_migrated`` pre-applied the rename and sync later replays the ObjectChange). """ + old_is_m2m = old_fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT + new_is_m2m = new_fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT + + # A type change between MULTIOBJECT and a scalar type (or vice versa) is not + # a simple column rename/alter — the storage representation is fundamentally + # different (through-table vs column). Attempting alter_field in this case + # would fail at the DB level. Log and skip; the caller is expected to handle + # such changes as remove + add rather than alter. + if old_is_m2m != new_is_m2m: + logger.warning( + '_schema_alter_field: skipping unsupported type change %r→%r on %s ' + '(MULTIOBJECT ↔ scalar changes require remove+add, not alter)', + old_fi.type, new_fi.type, model._meta.db_table, + ) + return + old_mf = FIELD_TYPE_CLASS[old_fi.type]().get_model_field(old_fi) new_mf = FIELD_TYPE_CLASS[new_fi.type]().get_model_field(new_fi) old_mf.contribute_to_class(model, old_fi.name) @@ -259,7 +290,7 @@ def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, exist return if ( - new_fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT + new_is_m2m and old_fi.name != new_fi.name ): old_through = old_fi.through_table_name @@ -1042,6 +1073,9 @@ def __init__(self, deserialized): self.object = deserialized.object def save(self, using=None, **kwargs): + # Snapshot before modifying so that diff()['pre'] records the + # current state rather than showing all fields as None on revert. + self.object.snapshot() # Clear the ObjectType FK — it may not exist in main yet. # custom_object_type_post_save_handler re-sets it after INSERT. self.object.object_type = None @@ -1218,6 +1252,7 @@ def _rename_objectchange_field_key(fi, old_name, new_name): logger.debug('_rename_objectchange_field_key: %r → %r for %s', old_name, new_name, ct) try: + from netbox_branching.models import ChangeDiff # noqa: F401 — presence check only cd_sql = ( 'UPDATE netbox_branching_changediff ' 'SET {col} = ({col} - %s) || jsonb_build_object(%s, {col}->%s) ' @@ -1226,8 +1261,13 @@ def _rename_objectchange_field_key(fi, old_name, new_name): with connections[conn.alias].cursor() as cursor: for json_col in ('original', 'modified', 'current'): cursor.execute(cd_sql.format(col=json_col), [old_name, new_name, old_name, ct.id, old_name]) + except ImportError: + pass # netbox-branching not installed except Exception: - pass # netbox-branching not installed or table absent + logger.debug( + '_rename_objectchange_field_key: ChangeDiff rename failed for %r → %r', + old_name, new_name, exc_info=True, + ) class CustomObjectTypeField(CloningMixin, ExportTemplatesMixin, ChangeLoggedModel): diff --git a/netbox_custom_objects/tests/test_branching.py b/netbox_custom_objects/tests/test_branching.py index 2f869e0..fdc3ff4 100644 --- a/netbox_custom_objects/tests/test_branching.py +++ b/netbox_custom_objects/tests/test_branching.py @@ -63,8 +63,10 @@ def _provision_branch(name, merge_strategy, user): def _close_branch_connections(): """Close any open branch database connections.""" for branch in Branch.objects.all(): - if hasattr(connections, branch.connection_name): + try: connections[branch.connection_name].close() + except Exception: + pass # ── Shared merge/revert tests (strategy-agnostic) ──────────────────────────── @@ -699,8 +701,8 @@ class BranchMigrateTestCase(TransactionCleanupMixin, TransactionTestCase): 1. A COT and field(s) are created in main and a branch is provisioned (branch has a full schema copy at provision time). 2. A field is then modified in main (added, removed, renamed, or - unique-toggled). on_custom_object_field_changed fires and marks the - branch PENDING_MIGRATIONS. + unique-toggled). netbox-branching detects the change to the non-exempt + CustomObjectTypeField model and marks the branch PENDING_MIGRATIONS. 3. branch.migrate(user) runs the normal Django migration pass and then emits post_migrate, which fires on_branch_migrated. That handler reconciles the branch's physical schema against main's current field @@ -763,7 +765,7 @@ def test_field_added_to_main_triggers_branch_migrate(self): self.assertEqual(branch.status, BranchStatusChoices.READY) self.assertIn('existing_field', self._get_branch_columns(branch, table_name)) - # Add a new field to main — on_custom_object_field_changed marks branch PENDING. + # Add a new field to main — netbox-branching detects the change and marks branch PENDING. CustomObjectTypeField.objects.create( custom_object_type=cot, name='new_field', From 993fae7e05fc63d3bc6898ad49e02126ca9f5826 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 23 Apr 2026 13:46:33 -0700 Subject: [PATCH 21/25] refactor --- netbox_custom_objects/__init__.py | 21 - netbox_custom_objects/branching.py | 200 ----- netbox_custom_objects/models.py | 87 +- netbox_custom_objects/tests/test_branching.py | 843 +++++++++++++----- 4 files changed, 679 insertions(+), 472 deletions(-) delete mode 100644 netbox_custom_objects/branching.py diff --git a/netbox_custom_objects/__init__.py b/netbox_custom_objects/__init__.py index df17acf..af41097 100644 --- a/netbox_custom_objects/__init__.py +++ b/netbox_custom_objects/__init__.py @@ -1,5 +1,4 @@ import contextvars -import logging import sys import warnings @@ -181,15 +180,6 @@ def ready(self): pre_migrate.connect(_migration_started) post_migrate.connect(_migration_finished) - # Wire into the netbox-branching lifecycle when that plugin is installed. - # Import is lazy so the plugin remains functional without branching. - try: - from netbox_branching.signals import post_migrate as branching_post_migrate - from netbox_custom_objects.branching import on_branch_migrated - branching_post_migrate.connect(on_branch_migrated) - except ImportError: - pass - # Patch ObjectSelectorView to support dynamically-generated custom object models _patch_object_selector_view() @@ -219,17 +209,6 @@ def ready(self): super().ready() return - # Scan for branches with pre-existing custom object schema drift - # (covers upgrades and any changes made while the app was offline). - try: - from netbox_custom_objects.branching import check_pending_branch_migrations - check_pending_branch_migrations() - except Exception: - logging.getLogger('netbox_custom_objects').exception( - 'check_pending_branch_migrations() failed at startup; ' - 'some branches may not be marked as PENDING_MIGRATIONS' - ) - super().ready() def get_model(self, model_name, require_ready=True): diff --git a/netbox_custom_objects/branching.py b/netbox_custom_objects/branching.py deleted file mode 100644 index 488f699..0000000 --- a/netbox_custom_objects/branching.py +++ /dev/null @@ -1,200 +0,0 @@ -""" -Branching support for NetBox Custom Objects. - -When netbox-branching runs ``Branch.migrate()``, the Django migration pass only -handles normal app migrations. Custom object field changes (add/alter/remove -column) are applied directly via the schema editor and are therefore invisible -to Django's migration framework. - -To close that gap, ``on_branch_migrated`` fires after the migration pass and -reconciles each custom object type's physical schema in the branch against the -current field definitions in main. It compares the branch's -``CustomObjectTypeField`` rows (snapshot from provision time) with main's -current rows, matched by primary key, and calls ``add_field`` / ``alter_field`` -/ ``remove_field`` for any differences. - -Schema ordering during sync and merge is handled naturally by the chronological -ObjectChange replay in ``Branch.sync()`` / ``Branch.merge()``: -``CustomObjectTypeField`` changes always precede ``CustomObject`` data changes -in the log. The schema helpers (``_schema_add_field``, ``_schema_alter_field``) -are idempotent, so replaying an ObjectChange after ``on_branch_migrated`` has -already applied the same schema change is safe. - -``CustomObjectTypeField`` has ``ChangeLoggingMixin``, so its rows are replicated -into the branch schema at provision time — the same source-of-truth mechanism -used for all other branchable models. Field-rename conflicts (same field PK -renamed differently in main and branch) surface as ``ChangeDiff`` conflicts and -require user acknowledgment before merge/sync proceeds; the existing branching -conflict mechanism handles this without any special casing. -""" - -import logging - -from django.db import connections - -logger = logging.getLogger('netbox_custom_objects.branching') - - -def check_pending_branch_migrations(): - """ - Scan all READY branches at startup and mark any that have custom object - schema drift as PENDING_MIGRATIONS. - - This catches drift that pre-dates the signal handler — for example, after - upgrading the plugin on an instance that already has branches, or if field - changes were applied while the application was not running. - - Called once from ``CustomObjectsPluginConfig.ready()`` after the DB is - confirmed ready. - """ - try: - from netbox_branching.choices import BranchStatusChoices - from netbox_branching.models import Branch - from netbox_custom_objects.models import CustomObjectType - except ImportError: - return - - ready_branches = list(Branch.objects.filter(status=BranchStatusChoices.READY)) - if not ready_branches: - return - - cots = list(CustomObjectType.objects.all()) - if not cots: - return - - to_update = [] - for branch in ready_branches: - branch_connection = connections[branch.connection_name] - with branch_connection.cursor() as cursor: - branch_tables = branch_connection.introspection.table_names(cursor) - - for cot in cots: - if cot.get_database_table_name() not in branch_tables: - continue - if cot.has_branch_schema_drift(branch): - branch.status = BranchStatusChoices.PENDING_MIGRATIONS - to_update.append(branch) - break # One drifted COT is enough — no need to check the rest - - if to_update: - Branch.objects.bulk_update(to_update, ['status']) - logger.info( - 'Marked %d branch(es) as PENDING_MIGRATIONS at startup due to custom object schema drift', - len(to_update), - ) - - -def _field_schema_key(f): - """ - Return the subset of ``CustomObjectTypeField`` attributes that affect the - physical DB column schema. - - Excluded (application-level only, no physical DB impact): - - required: all field types use null=True regardless; required never maps - to a NOT NULL column constraint. - - default: Python-level default applied by the ORM, not a SQL DEFAULT - clause; changing it on an existing column needs no ALTER TABLE. - - Used by both ``_fields_schema_differ()`` and - ``CustomObjectType.has_branch_schema_drift()`` to ensure the two callers - always agree on what constitutes a schema-affecting change. - """ - return (f.name, f.type, f.unique, f.related_object_type_id) - - -def _fields_schema_differ(branch_f, main_f): - """ - Return True if the two ``CustomObjectTypeField`` instances differ in any - attribute that affects the physical DB column, meaning an ALTER TABLE is - needed to bring the branch schema up to date. - """ - return _field_schema_key(branch_f) != _field_schema_key(main_f) - - -def on_branch_migrated(sender, branch, user, **kwargs): - """ - Reconcile each custom object type's physical schema in the branch against - the current field definitions in main. - - For each ``CustomObjectType`` whose table exists in the branch schema: - - Fields present in main but absent from the branch → ``add_field`` - - Fields absent from main but present in the branch → ``remove_field`` - - Fields present in both with differing definitions → ``alter_field`` - - Matching is done by primary key so renames are detected correctly (the pk - exists in both, with different ``name`` values) rather than being treated - as an unrelated delete + add. - """ - from netbox_branching.utilities import activate_branch - from netbox_custom_objects.models import ( - CustomObjectType, - _schema_add_field, - _schema_alter_field, - _schema_remove_field, - ) - - branch_connection = connections[branch.connection_name] - - with branch_connection.cursor() as cursor: - branch_tables = branch_connection.introspection.table_names(cursor) - - for cot in CustomObjectType.objects.all(): - db_table = cot.get_database_table_name() - if db_table not in branch_tables: - # Table absent — COT was created after this branch was provisioned - # and the branch hasn't been synced yet. Skip; the user needs to - # sync first to pull in the new table. - logger.debug('Skipping %s — table %r not in branch schema', cot, db_table) - continue - - # Main's current field definitions (queried from the public schema since - # active_branch is not set here). - main_fields = { - f.pk: f - for f in cot.fields.select_related('related_object_type', 'choice_set').all() - } - - # Branch's field snapshot (as of provision time, or last sync). - with activate_branch(branch): - branch_fields = { - f.pk: f - for f in cot.fields.select_related('related_object_type', 'choice_set').all() - } - - main_pks = set(main_fields) - branch_pks = set(branch_fields) - - to_add = [main_fields[pk] for pk in main_pks - branch_pks] - to_remove = [branch_fields[pk] for pk in branch_pks - main_pks] - to_alter = [ - (branch_fields[pk], main_fields[pk]) # (old, new) - for pk in main_pks & branch_pks - if _fields_schema_differ(branch_fields[pk], main_fields[pk]) - ] - - if not (to_add or to_remove or to_alter): - continue - - logger.info( - 'Migrating branch schema for %s: %d add, %d remove, %d alter', - cot, len(to_add), len(to_remove), len(to_alter), - ) - - model = cot.get_model() - - with branch_connection.schema_editor() as schema_editor: - - for fi in to_add: - logger.debug('add_field %r on %s', fi.name, cot) - _schema_add_field(fi, model, schema_editor, branch_connection) - - for fi in to_remove: - logger.debug('remove_field %r on %s', fi.name, cot) - _schema_remove_field(fi, model, schema_editor, existing_tables=branch_tables) - - for old_fi, new_fi in to_alter: - logger.debug('alter_field %r → %r on %s', old_fi.name, new_fi.name, cot) - _schema_alter_field( - old_fi, new_fi, model, schema_editor, branch_connection, - existing_tables=branch_tables, - ) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 90f16a6..733c280 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -181,7 +181,7 @@ def _schema_add_field(fi, model, schema_editor, schema_conn): call ``_apply_deferred_co_field(fi)`` separately after this returns. Idempotent: skips the ALTER TABLE if the column already exists (e.g. when - ``on_branch_migrated`` pre-added it and sync later replays the ObjectChange). + sync/merge replays an ObjectChange that was already applied). """ ft = FIELD_TYPE_CLASS[fi.type]() mf = ft.get_model_field(fi) @@ -253,8 +253,14 @@ def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, exist once on demand. Idempotent: skips the ALTER TABLE if the old column is already gone and the - new column already exists (e.g. when ``on_branch_migrated`` pre-applied the - rename and sync later replays the ObjectChange). + new column already exists (e.g. when sync/merge replays an ObjectChange that + was already applied). + + Conflict resolution: when neither the old nor the new column exists (the field + was independently renamed in the target schema — e.g. branch renamed A→X while + main renamed A→Y), the live field record is looked up from the target schema to + find the actual current column name, which is then renamed to the new target. + A warning is logged to flag the conflict. """ old_is_m2m = old_fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT new_is_m2m = new_fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT @@ -282,11 +288,43 @@ def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, exist col.name for col in schema_conn.introspection.get_table_description(cursor, model._meta.db_table) } - if old_mf.column not in existing_cols and new_mf.column in existing_cols: - logger.debug( - '_schema_alter_field: %r already renamed to %r on %s, skipping', - old_mf.column, new_mf.column, model._meta.db_table, + if old_mf.column not in existing_cols: + if new_mf.column in existing_cols: + logger.debug( + '_schema_alter_field: %r already renamed to %r on %s, skipping', + old_mf.column, new_mf.column, model._meta.db_table, + ) + return + if old_is_m2m: + # M2M fields have no physical column; the old through table is absent. + return + # Scalar field: neither the old nor the new column exists. The field was + # independently renamed in this schema (e.g. branch renamed A→X while main + # renamed A→Y; now applying main's rename to the branch). Look up the live + # field record in the target schema to find the actual column and rename it. + logger.warning( + '_schema_alter_field: rename conflict on %s — source column %r and ' + 'target column %r are both absent; field pk=%d was independently renamed ' + 'in this schema; resolving by looking up live column', + model._meta.db_table, old_mf.column, new_mf.column, new_fi.pk, ) + try: + live_fi = CustomObjectTypeField.objects.using(schema_conn.alias).get(pk=new_fi.pk) + except CustomObjectTypeField.DoesNotExist: + logger.debug( + '_schema_alter_field: field pk=%d not found in %s; skipping', + new_fi.pk, schema_conn.alias, + ) + return + live_mf = FIELD_TYPE_CLASS[live_fi.type]().get_model_field(live_fi) + live_mf.contribute_to_class(model, live_fi.name) + if live_mf.column not in existing_cols: + logger.debug( + '_schema_alter_field: live column %r also absent on %s; skipping', + live_mf.column, model._meta.db_table, + ) + return + schema_editor.alter_field(model, live_mf, new_mf) return if ( @@ -1089,41 +1127,6 @@ def save(self, using=None, **kwargs): return _SchemaAwareDeserialized(inner) - def has_branch_schema_drift(self, branch) -> bool: - """ - Return True if this custom object type's physical schema in *branch* - differs from the current field definitions in main. - - Drift means at least one of: - - A field exists in main but not in the branch snapshot (needs add_field) - - A field exists in the branch snapshot but not in main (needs remove_field) - - A field exists in both but has a different name, type, or constraint - that affects the DB column (needs alter_field) - - Returns False when netbox-branching is not installed. - """ - try: - from netbox_branching.utilities import activate_branch - except ImportError: - return False - - from netbox_custom_objects.branching import _field_schema_key - - main_fields = {f.pk: f for f in self.fields.all()} - with activate_branch(branch): - branch_fields = {f.pk: f for f in self.fields.all()} - - main_pks = set(main_fields) - branch_pks = set(branch_fields) - - if main_pks != branch_pks: - return True - - return any( - _field_schema_key(branch_fields[pk]) != _field_schema_key(main_fields[pk]) - for pk in main_pks - ) - def save(self, *args, **kwargs): needs_db_create = self._state.adding diff --git a/netbox_custom_objects/tests/test_branching.py b/netbox_custom_objects/tests/test_branching.py index fdc3ff4..88309f8 100644 --- a/netbox_custom_objects/tests/test_branching.py +++ b/netbox_custom_objects/tests/test_branching.py @@ -690,29 +690,16 @@ def test_main_changes_synced_to_branch(self): self.assertEqual(co_branch.title, 'main object') -# ── Drift detection and Branch.migrate() tests ──────────────────────────────── +# ── Concurrent-edit tests (both main and branch modified before sync/merge) ─── @unittest.skipUnless(HAS_BRANCHING, 'netbox-branching is not installed') -class BranchMigrateTestCase(TransactionCleanupMixin, TransactionTestCase): +class ConcurrentEditSyncTestCase(TransactionCleanupMixin, TransactionTestCase): """ - Tests for the drift-detection and schema-reconciliation path. - - Scenario: - 1. A COT and field(s) are created in main and a branch is provisioned - (branch has a full schema copy at provision time). - 2. A field is then modified in main (added, removed, renamed, or - unique-toggled). netbox-branching detects the change to the non-exempt - CustomObjectTypeField model and marks the branch PENDING_MIGRATIONS. - 3. branch.migrate(user) runs the normal Django migration pass and then - emits post_migrate, which fires on_branch_migrated. That handler - reconciles the branch's physical schema against main's current field - definitions. - 4. The branch is back to READY and its physical column layout matches main. - - These tests do NOT use merge/revert — they isolate the separate - on_branch_migrated reconciliation path for branches that pre-date a main - schema change. They use the iterative strategy only because the strategy - affects merge order, not schema reconciliation. + Sync scenarios where both main and branch accumulate changes before sync(). + + Mirrors netbox-branching's test_sync_m2m_tags_concurrent_changes pattern: + after sync(), main's ObjectChanges are applied on top of whatever the branch + did, so main's post-change state takes precedence for any conflicting record. """ def setUp(self): @@ -725,249 +712,687 @@ def tearDown(self): _close_branch_connections() super().tearDown() - def _get_branch_columns(self, branch, table_name): - """Return the set of column names on table_name in the branch schema.""" - conn = connections[branch.connection_name] - with conn.cursor() as cursor: - return {col.name for col in conn.introspection.get_table_description(cursor, table_name)} - - def _get_branch_tables(self, branch): - """Return the set of table names in the branch schema.""" - conn = connections[branch.connection_name] - with conn.cursor() as cursor: - return set(conn.introspection.table_names(cursor)) - - def _get_branch_constraints(self, branch, table_name): - """Return the constraints dict for table_name in the branch schema.""" - conn = connections[branch.connection_name] - with conn.cursor() as cursor: - return conn.introspection.get_constraints(cursor, table_name) - - # ── field added to main ─────────────────────────────────────────────── - - def test_field_added_to_main_triggers_branch_migrate(self): - """ - Field added to a COT in main marks the branch PENDING_MIGRATIONS. - branch.migrate() fires on_branch_migrated which calls add_field to - create the new column in the branch schema. - """ - cot = CustomObjectType.objects.create(name='drift_add_cot', slug='drift-add-cot') - CustomObjectTypeField.objects.create( - custom_object_type=cot, - name='existing_field', - label='Existing', - type='text', - ) + def test_co_values_modified_in_both_sync(self): + """ + CO field values modified in both main and branch before sync. - branch = _provision_branch('Drift Add Branch', 'iterative', self.user) - table_name = cot.get_database_table_name() + Scenario + -------- + 1. Create COT + field 'notes' + two COs in main. + 2. Provision branch (branch sees same COs). + 3. In branch: update shared CO to 'modified in branch'. + 4. In main: update a different CO; create a brand-new CO. + 5. sync() applies main's ObjectChanges to the branch. + + Expected after sync + ------------------- + - Main's new CO is visible in the branch. + - CO modified in main has main's value in the branch. + - Branch's own CO modification is overwritten by main's replay for + the same PK (main wins on sync, same as tag-conflict behaviour). + """ + request = _make_request(self.user) - self.assertEqual(branch.status, BranchStatusChoices.READY) - self.assertIn('existing_field', self._get_branch_columns(branch, table_name)) + with event_tracking(request): + cot = CustomObjectType.objects.create(name='sync_co_cot', slug='sync-co-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='notes', label='Notes', type='text', + ) + Model = cot.get_model() + co_shared = Model.objects.create(notes='original shared') + co_main_only = Model.objects.create(notes='main only original') - # Add a new field to main — netbox-branching detects the change and marks branch PENDING. - CustomObjectTypeField.objects.create( - custom_object_type=cot, - name='new_field', - label='New Field', - type='integer', - ) + co_shared_pk = co_shared.pk + co_main_only_pk = co_main_only.pk + branch = _provision_branch('Sync CO Both', 'iterative', self.user) + branch_request = _make_request(self.user) - branch.refresh_from_db() - self.assertEqual( - branch.status, BranchStatusChoices.PENDING_MIGRATIONS, - 'Branch must be PENDING_MIGRATIONS after field added to main', - ) - self.assertNotIn( - 'new_field', self._get_branch_columns(branch, table_name), - 'New column must not exist in branch before migrate', - ) + # ── branch: update the shared CO ───────────────────────────────── + with activate_branch(branch), event_tracking(branch_request): + BM = cot.get_model() + co = BM.objects.get(pk=co_shared_pk) + co.snapshot() + co.notes = 'modified in branch' + co.save() + branch_new = BM.objects.create(notes='new in branch') + branch_new_pk = branch_new.pk + + # ── main: update the other CO; add a new CO ─────────────────────── + with event_tracking(request): + MM = cot.get_model() + co = MM.objects.get(pk=co_main_only_pk) + co.snapshot() + co.notes = 'modified in main' + co.save() + main_new = MM.objects.create(notes='new in main') + main_new_pk = main_new.pk - # migrate() fires on_branch_migrated → add_field runs on the branch schema. - branch.migrate(user=self.user) - branch.refresh_from_db() - self.assertEqual(branch.status, BranchStatusChoices.READY) + # ── sync ────────────────────────────────────────────────────────── + branch.sync(user=self.user, commit=True) - self.assertIn( - 'new_field', self._get_branch_columns(branch, table_name), - 'New column must be present in branch schema after migrate', - ) + with activate_branch(branch): + SyncedCOT = CustomObjectType.objects.get(pk=cot.pk) + SM = SyncedCOT.get_model() + + # CO modified in main must reflect main's value. + self.assertEqual(SM.objects.get(pk=co_main_only_pk).notes, 'modified in main') + # CO created in main must be visible in branch after sync. + self.assertTrue(SM.objects.filter(pk=main_new_pk).exists(), + 'CO created in main must appear in branch after sync') + # CO created in branch was not deleted by sync. + self.assertTrue(SM.objects.filter(pk=branch_new_pk).exists(), + 'CO created in branch must still exist after sync') + + def test_field_rename_in_branch_co_add_in_main_sync(self): + """ + Field renamed inside a branch; new CO added in main (no rename in main). + + Scenario + -------- + 1. Create COT + field 'alpha' + CO in main. + 2. Provision branch. + 3. In branch: rename 'alpha' → 'branch_alpha'; create a CO. + 4. In main: create a new CO using the original field name 'alpha'. + 5. sync() replays main's CO-create on top of the branch. + + Expected after sync + ------------------- + - Branch retains the renamed field ('branch_alpha'). + - CO created in main is present in branch. + - CO created in branch still exists. + + This exercises the schema-mismatch path: main's CO ObjectChange carries + the original field key 'alpha' while the branch schema uses 'branch_alpha'. + The CO data from main is applied by the branching engine regardless of the + column name divergence (branching replays at the data layer). + """ + request = _make_request(self.user) + + with event_tracking(request): + cot = CustomObjectType.objects.create(name='sync_rename_cot', slug='sync-rename-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='alpha', label='Alpha', type='text', + ) + cot.get_model().objects.create(alpha='original') + + branch = _provision_branch('Sync Rename Branch', 'iterative', self.user) + branch_request = _make_request(self.user) - # ── field deleted from main ─────────────────────────────────────────── + # ── branch: rename alpha → branch_alpha; create CO ──────────────── + with activate_branch(branch), event_tracking(branch_request): + field = CustomObjectTypeField.objects.get(custom_object_type=cot, name='alpha') + field.snapshot() + field.name = 'branch_alpha' + field.label = 'Branch Alpha' + field.save() + BM = cot.get_model() + branch_new = BM.objects.create(branch_alpha='new in branch') + branch_new_pk = branch_new.pk - def test_field_deleted_from_main_triggers_branch_migrate(self): + # ── main: add a CO (no field rename) ────────────────────────────── + with event_tracking(request): + cot.get_model().objects.create(alpha='new in main') + + # ── sync ────────────────────────────────────────────────────────── + branch.sync(user=self.user, commit=True) + + with activate_branch(branch): + cot_b = CustomObjectType.objects.get(pk=cot.pk) + field_b = CustomObjectTypeField.objects.get(custom_object_type=cot_b) + # Branch field rename must be preserved (main did not rename). + self.assertEqual(field_b.name, 'branch_alpha', + 'Branch field rename must be preserved after sync') + BM = cot_b.get_model() + self.assertTrue(BM.objects.filter(pk=branch_new_pk).exists(), + 'CO created in branch must survive sync') + + def test_concurrent_field_rename_sync_no_crash(self): """ - Field deleted from a COT in main marks the branch PENDING_MIGRATIONS. - branch.migrate() fires on_branch_migrated which calls remove_field to - drop the column from the branch schema. + Field renamed to different names in both main and branch before sync. + + The same CustomObjectTypeField PK was modified in both schemas. + _schema_alter_field detects the conflict (neither the original 'alpha' + column nor the target 'main_alpha' column exists in the branch), looks up + the live column name in the branch ('branch_alpha'), and renames it to + 'main_alpha' to converge the branch schema on main's post-sync state. + + Scenario + -------- + 1. Create COT + field 'alpha' in main. + 2. Provision branch. + 3. Branch renames 'alpha' → 'branch_alpha'. + 4. Main renames 'alpha' → 'main_alpha'. + 5. sync() applies main's rename ObjectChange to the branch. + _schema_alter_field resolves the conflict: 'branch_alpha' → 'main_alpha'. + + Expected + -------- + - sync() completes without raising a DB error. + - Branch physical column is 'main_alpha' (converged to main's rename). + - 'branch_alpha' and 'alpha' columns are absent from the branch table. """ - cot = CustomObjectType.objects.create(name='drift_del_cot', slug='drift-del-cot') - CustomObjectTypeField.objects.create( - custom_object_type=cot, - name='keep_field', - label='Keep', - type='text', - ) - drop_field = CustomObjectTypeField.objects.create( - custom_object_type=cot, - name='drop_field', - label='Drop', - type='text', - ) + request = _make_request(self.user) + + with event_tracking(request): + cot = CustomObjectType.objects.create(name='confl_sync_cot', slug='confl-sync-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='alpha', label='Alpha', type='text', + ) - branch = _provision_branch('Drift Del Branch', 'iterative', self.user) - table_name = cot.get_database_table_name() + branch = _provision_branch('Conflict Sync Branch', 'iterative', self.user) + branch_request = _make_request(self.user) - self.assertIn('drop_field', self._get_branch_columns(branch, table_name)) + # ── branch: rename alpha → branch_alpha ─────────────────────────── + with activate_branch(branch), event_tracking(branch_request): + f = CustomObjectTypeField.objects.get(custom_object_type=cot, name='alpha') + f.snapshot() + f.name = 'branch_alpha' + f.label = 'Branch Alpha' + f.save() - # Delete field from main. - drop_field.delete() + # ── main: rename alpha → main_alpha ─────────────────────────────── + with event_tracking(request): + f = CustomObjectTypeField.objects.get(custom_object_type=cot, name='alpha') + f.name = 'main_alpha' + f.label = 'Main Alpha' + f.save() + + # ── sync — must not raise ────────────────────────────────────────── + try: + branch.sync(user=self.user, commit=True) + except Exception as exc: + self.fail(f'sync() raised an unexpected exception: {exc!r}') branch.refresh_from_db() - self.assertEqual(branch.status, BranchStatusChoices.PENDING_MIGRATIONS) - # Column still present in branch before migrate. - self.assertIn('drop_field', self._get_branch_columns(branch, table_name)) + # Branch column should now be 'main_alpha' — the conflict was resolved by + # renaming the branch's live 'branch_alpha' column to main's target name. + branch_conn = connections[branch.connection_name] + with branch_conn.cursor() as cursor: + branch_cols = { + col.name + for col in branch_conn.introspection.get_table_description( + cursor, cot.get_database_table_name(), + ) + } + self.assertIn('main_alpha', branch_cols, 'Branch column must converge to main_alpha after sync') + self.assertNotIn('branch_alpha', branch_cols, 'branch_alpha column must be gone after sync') + self.assertNotIn('alpha', branch_cols, 'alpha column must be gone after sync') - branch.migrate(user=self.user) - branch.refresh_from_db() - self.assertEqual(branch.status, BranchStatusChoices.READY) - cols = self._get_branch_columns(branch, table_name) - self.assertNotIn( - 'drop_field', cols, - 'Deleted column must be absent from branch schema after migrate', - ) - self.assertIn('keep_field', cols, 'Unmodified column must remain in branch schema') +# ── Concurrent-edit merge tests ─────────────────────────────────────────────── - # ── field renamed in main ───────────────────────────────────────────── +@unittest.skipUnless(HAS_BRANCHING, 'netbox-branching is not installed') +class ConcurrentEditMergeTestCase(TransactionCleanupMixin, TransactionTestCase): + """ + Merge scenarios where both main and branch accumulate changes before merge(). + """ - def test_field_renamed_in_main_triggers_branch_migrate(self): + def setUp(self): + super().setUp() + _recreate_contenttypes() + self.user = User.objects.create_user(username='testuser') + self.request = _make_request(self.user) + + def tearDown(self): + _close_branch_connections() + super().tearDown() + + def test_field_rename_in_branch_co_changes_merge_iterative(self): """ - Field renamed in main marks the branch PENDING_MIGRATIONS. - branch.migrate() fires on_branch_migrated which calls alter_field to - rename the column in the branch schema using PK-based matching (same - PK in both main and branch, different name values). + Field renamed inside a branch; COs added/updated in branch; main adds a CO. + Iterative merge brings the branch rename and CO changes into main. + + Scenario + -------- + 1. Create COT + field 'alpha' + CO in main. + 2. Provision branch. + 3. In branch: rename 'alpha' → 'beta'; update the existing CO; create a CO. + 4. In main: add a CO (field name 'alpha', no rename). + 5. merge() → revert(). + + Expected after merge + -------------------- + - Field name in main is 'beta'. + - Existing CO has the branch-updated value. + - CO created in branch is present in main. + + Expected after revert + --------------------- + - Field name is 'alpha' again. + - CO created in branch is gone. + - Existing CO has its original value. """ - cot = CustomObjectType.objects.create(name='drift_ren_cot', slug='drift-ren-cot') - field = CustomObjectTypeField.objects.create( - custom_object_type=cot, - name='old_col', - label='Old', - type='text', - ) + request = _make_request(self.user) - branch = _provision_branch('Drift Ren Branch', 'iterative', self.user) - table_name = cot.get_database_table_name() + with event_tracking(request): + cot = CustomObjectType.objects.create(name='merge_rename_cot', slug='merge-rename-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='alpha', label='Alpha', type='text', + ) + Model = cot.get_model() + co_existing = Model.objects.create(alpha='original') - self.assertIn('old_col', self._get_branch_columns(branch, table_name)) + co_existing_pk = co_existing.pk + branch = _provision_branch('Merge Rename Branch', 'iterative', self.user) + branch_request = _make_request(self.user) - # Rename in main — load from DB so _original is set before modifying. - field = CustomObjectTypeField.objects.get(pk=field.pk) - field.name = 'new_col' - field.label = 'New' - field.save() + # ── branch: rename; update CO; create CO ────────────────────────── + with activate_branch(branch), event_tracking(branch_request): + field = CustomObjectTypeField.objects.get(custom_object_type=cot, name='alpha') + field.snapshot() + field.name = 'beta' + field.label = 'Beta' + field.save() + BM = cot.get_model() + co = BM.objects.get(pk=co_existing_pk) + co.snapshot() + co.beta = 'updated in branch' + co.save() + branch_new = BM.objects.create(beta='new in branch') + branch_new_pk = branch_new.pk + + # ── main: add a CO (no schema change) ───────────────────────────── + with event_tracking(request): + main_new = cot.get_model().objects.create(alpha='new in main') + main_new_pk = main_new.pk + # ── merge ───────────────────────────────────────────────────────── + branch.merge(user=self.user, commit=True) branch.refresh_from_db() - self.assertEqual(branch.status, BranchStatusChoices.PENDING_MIGRATIONS) + self.assertEqual(branch.status, BranchStatusChoices.MERGED) - branch.migrate(user=self.user) - branch.refresh_from_db() - self.assertEqual(branch.status, BranchStatusChoices.READY) + MergedModel = cot.get_model() + field_main = CustomObjectTypeField.objects.get(custom_object_type=cot) + self.assertEqual(field_main.name, 'beta', 'Field must be "beta" in main after merge') + self.assertEqual(MergedModel.objects.get(pk=co_existing_pk).beta, 'updated in branch') + self.assertTrue(MergedModel.objects.filter(pk=branch_new_pk).exists(), + 'CO created in branch must be in main after merge') + self.assertTrue(MergedModel.objects.filter(pk=main_new_pk).exists(), + 'CO added in main must still be present after merge') - cols = self._get_branch_columns(branch, table_name) - self.assertIn('new_col', cols, 'Renamed column must exist in branch schema after migrate') - self.assertNotIn('old_col', cols, 'Old column name must be absent from branch schema after migrate') + # ── revert ──────────────────────────────────────────────────────── + branch.revert(user=self.user, commit=True) - # ── unique constraint toggled in main ───────────────────────────────── + field_reverted = CustomObjectTypeField.objects.get(custom_object_type=cot) + self.assertEqual(field_reverted.name, 'alpha', 'Field must be "alpha" after revert') + RevertedModel = cot.get_model() + self.assertEqual(RevertedModel.objects.get(pk=co_existing_pk).alpha, 'original') + self.assertFalse(RevertedModel.objects.filter(pk=branch_new_pk).exists(), + 'CO created in branch must be gone after revert') - def test_unique_toggled_in_main_triggers_branch_migrate(self): + def test_co_values_modified_in_both_merge_iterative(self): """ - Field's unique constraint toggled in main marks the branch - PENDING_MIGRATIONS. branch.migrate() reconciles the constraint in - the branch's physical schema. + CO values modified in both main and branch before merge. + Branch changes win because merge applies branch ObjectChanges to main. + + Scenario + -------- + 1. Create COT + field 'notes' + shared CO in main. + 2. Provision branch. + 3. Branch updates shared CO to 'modified in branch'; creates a CO. + 4. Main creates a separate CO. + 5. merge() → revert(). + + Expected after merge: branch CO changes are in main, main CO preserved. + Expected after revert: shared CO back to original, branch CO gone. """ - cot = CustomObjectType.objects.create(name='drift_uniq_cot', slug='drift-uniq-cot') - field = CustomObjectTypeField.objects.create( - custom_object_type=cot, - name='code', - label='Code', - type='text', - unique=False, - ) + request = _make_request(self.user) - branch = _provision_branch('Drift Uniq Branch', 'iterative', self.user) - table_name = cot.get_database_table_name() + with event_tracking(request): + cot = CustomObjectType.objects.create(name='merge_co_cot', slug='merge-co-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='notes', label='Notes', type='text', + ) + Model = cot.get_model() + co_shared = Model.objects.create(notes='original') - # Branch must not have a unique constraint on 'code' initially. - constraints_before = self._get_branch_constraints(branch, table_name) - self.assertFalse( - any(c['unique'] and c.get('columns') == ['code'] for c in constraints_before.values()), - 'Branch must not have UNIQUE on "code" before toggle', - ) + co_shared_pk = co_shared.pk + branch = _provision_branch('Merge CO Both', 'iterative', self.user) + branch_request = _make_request(self.user) - # Toggle unique=True in main. - field = CustomObjectTypeField.objects.get(pk=field.pk) - field.unique = True - field.save() + # ── branch ──────────────────────────────────────────────────────── + with activate_branch(branch), event_tracking(branch_request): + BM = cot.get_model() + co = BM.objects.get(pk=co_shared_pk) + co.snapshot() + co.notes = 'modified in branch' + co.save() + branch_new = BM.objects.create(notes='new in branch') + branch_new_pk = branch_new.pk + + # ── main ────────────────────────────────────────────────────────── + with event_tracking(request): + main_new = cot.get_model().objects.create(notes='new in main') + main_new_pk = main_new.pk + # ── merge ───────────────────────────────────────────────────────── + branch.merge(user=self.user, commit=True) branch.refresh_from_db() - self.assertEqual(branch.status, BranchStatusChoices.PENDING_MIGRATIONS) + self.assertEqual(branch.status, BranchStatusChoices.MERGED) - branch.migrate(user=self.user) - branch.refresh_from_db() - self.assertEqual(branch.status, BranchStatusChoices.READY) + MM = cot.get_model() + self.assertEqual(MM.objects.get(pk=co_shared_pk).notes, 'modified in branch') + self.assertTrue(MM.objects.filter(pk=branch_new_pk).exists()) + self.assertTrue(MM.objects.filter(pk=main_new_pk).exists(), + 'CO added to main must survive the merge') + + # ── revert ──────────────────────────────────────────────────────── + branch.revert(user=self.user, commit=True) + + RM = cot.get_model() + self.assertEqual(RM.objects.get(pk=co_shared_pk).notes, 'original') + self.assertFalse(RM.objects.filter(pk=branch_new_pk).exists()) - constraints_after = self._get_branch_constraints(branch, table_name) - self.assertTrue( - any(c['unique'] and c.get('columns') == ['code'] for c in constraints_after.values()), - 'Branch must have UNIQUE constraint on "code" after migrate', - ) - # ── multiobject field renamed in main (through-table rename) ───────── +# ── Sequential multi-rename tests ───────────────────────────────────────────── + +@unittest.skipUnless(HAS_BRANCHING, 'netbox-branching is not installed') +class SequentialRenameTestCase(TransactionCleanupMixin, TransactionTestCase): + """ + Tests for sequential field renames (A→B→C) in a branch with CO changes at + each step, plus independent changes in main. + + Exercises the iterative ObjectChange replay order: the rename chain must be + applied in the right sequence so that each CO update sees the correct column + name at merge time. + + Run with both iterative and squash strategies to verify that squash correctly + collapses the A→B→C chain to a single A→C alter. + """ + + MERGE_STRATEGY = 'iterative' + + def setUp(self): + super().setUp() + _recreate_contenttypes() + self.user = User.objects.create_user(username='testuser') + self.request = _make_request(self.user) + + def tearDown(self): + _close_branch_connections() + super().tearDown() - def test_multiobject_field_renamed_in_main_triggers_branch_migrate(self): + def _run_sequential_rename_merge(self, cot_name, cot_slug): """ - MULTIOBJECT field renamed in main marks the branch PENDING_MIGRATIONS. - branch.migrate() fires on_branch_migrated which renames the through - table in the branch schema in addition to the column alter_field. + Shared implementation for the sequential rename merge test. - Exercises the MULTIOBJECT rename branch of _schema_alter_field. + Branch: rename alpha→beta (update+create CO), rename beta→gamma (update+create CO). + Main: add a new independent field + CO (no rename of alpha). + merge() then revert(). """ - from core.models import ObjectType + request = _make_request(self.user) - site_ot = ObjectType.objects.get(app_label='dcim', model='site') + with event_tracking(request): + cot = CustomObjectType.objects.create(name=cot_name, slug=cot_slug) + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='alpha', label='Alpha', type='text', + ) + Model = cot.get_model() + co_original = Model.objects.create(alpha='original value') - cot = CustomObjectType.objects.create(name='drift_m2m_cot', slug='drift-m2m-cot') - field = CustomObjectTypeField.objects.create( - custom_object_type=cot, - name='related_sites', - label='Related Sites', - type='multiobject', - related_object_type=site_ot, - ) + co_original_pk = co_original.pk + branch = _provision_branch(f'{cot_name} branch', self.MERGE_STRATEGY, self.user) + branch_request = _make_request(self.user) - old_through = field.through_table_name + # ── branch: alpha → beta; update CO; create CO ──────────────────── + with activate_branch(branch), event_tracking(branch_request): + field = CustomObjectTypeField.objects.get(custom_object_type=cot, name='alpha') + field.snapshot() + field.name = 'beta' + field.label = 'Beta' + field.save() + BM = cot.get_model() + co = BM.objects.get(pk=co_original_pk) + co.snapshot() + co.beta = 'after rename to beta' + co.save() + co_at_beta = BM.objects.create(beta='created at beta') + co_at_beta_pk = co_at_beta.pk + + # ── branch: beta → gamma; update CO; create CO ──────────────────── + with activate_branch(branch), event_tracking(branch_request): + field = CustomObjectTypeField.objects.get(custom_object_type=cot, name='beta') + field.snapshot() + field.name = 'gamma' + field.label = 'Gamma' + field.save() + BM = cot.get_model() + co = BM.objects.get(pk=co_original_pk) + co.snapshot() + co.gamma = 'after rename to gamma' + co.save() + co_at_gamma = BM.objects.create(gamma='created at gamma') + co_at_gamma_pk = co_at_gamma.pk + + # ── main: add a new independent field + CO ──────────────────────── + with event_tracking(request): + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='extra', label='Extra', type='text', + ) + co_main = cot.get_model().objects.create(alpha='main added', extra='extra val') + co_main_pk = co_main.pk - branch = _provision_branch('Drift M2M Branch', 'iterative', self.user) + # ── merge ───────────────────────────────────────────────────────── + branch.merge(user=self.user, commit=True) + branch.refresh_from_db() + self.assertEqual(branch.status, BranchStatusChoices.MERGED) + + field_names = {f.name for f in CustomObjectTypeField.objects.filter(custom_object_type=cot)} + self.assertIn('gamma', field_names, 'Final field name must be "gamma" after merge') + self.assertNotIn('alpha', field_names, '"alpha" must be absent after merge') + self.assertNotIn('beta', field_names, '"beta" must be absent after merge') + self.assertIn('extra', field_names, '"extra" field from main must be present after merge') + + MergedModel = cot.get_model() + self.assertEqual(MergedModel.objects.get(pk=co_original_pk).gamma, 'after rename to gamma') + self.assertTrue(MergedModel.objects.filter(pk=co_at_beta_pk).exists(), + 'CO created at beta step must survive merge') + self.assertTrue(MergedModel.objects.filter(pk=co_at_gamma_pk).exists(), + 'CO created at gamma step must survive merge') + self.assertTrue(MergedModel.objects.filter(pk=co_main_pk).exists(), + 'CO added in main must survive merge') + + # ── revert ──────────────────────────────────────────────────────── + branch.revert(user=self.user, commit=True) - self.assertIn(old_through, self._get_branch_tables(branch)) + field_names_r = {f.name for f in CustomObjectTypeField.objects.filter(custom_object_type=cot)} + self.assertIn('alpha', field_names_r, '"alpha" must be restored after revert') + self.assertNotIn('gamma', field_names_r, '"gamma" must be gone after revert') - # Rename in main. - field = CustomObjectTypeField.objects.get(pk=field.pk) - field.name = 'linked_sites' - field.label = 'Linked Sites' - field.save() + RevertedModel = cot.get_model() + self.assertEqual(RevertedModel.objects.get(pk=co_original_pk).alpha, 'original value', + 'Original CO value must be restored after revert') + self.assertFalse(RevertedModel.objects.filter(pk=co_at_beta_pk).exists()) + self.assertFalse(RevertedModel.objects.filter(pk=co_at_gamma_pk).exists()) - new_through = field.through_table_name + def test_sequential_renames_alpha_beta_gamma_merge(self): + """Field renamed A→B→C in branch with CO changes at each step; merge + revert.""" + self._run_sequential_rename_merge('seq_iter_cot', 'seq-iter-cot') + + def test_sequential_renames_both_sides_sync(self): + """ + Branch renames A→B→C while main renames A→D. + Both schemas independently rename the same field to different names. + + After sync(), main's rename (A→D) is applied on top of the branch's + state. Because 'alpha' no longer exists in the branch (it was renamed + to 'gamma' via beta), _schema_alter_field detects the conflict, looks up + the live column name in the branch ('gamma'), and renames it to 'delta' + to converge the branch schema on main's post-sync state. + + Scenario + -------- + 1. Create COT + field 'alpha' + CO in main. + 2. Provision branch. + 3. Branch: alpha→beta (update CO), beta→gamma (update CO, add CO). + 4. Main: alpha→delta (update CO, add CO). + 5. sync(): apply main's changes to branch — column converges to 'delta'. + """ + request = _make_request(self.user) + + with event_tracking(request): + cot = CustomObjectType.objects.create(name='seq_sync_cot', slug='seq-sync-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='alpha', label='Alpha', type='text', + ) + co = cot.get_model().objects.create(alpha='original') + + co_pk = co.pk + branch = _provision_branch('Seq Sync Branch', 'iterative', self.user) + branch_request = _make_request(self.user) + + # ── branch: alpha → beta → gamma ────────────────────────────────── + with activate_branch(branch), event_tracking(branch_request): + f = CustomObjectTypeField.objects.get(custom_object_type=cot, name='alpha') + f.snapshot() + f.name = 'beta' + f.label = 'Beta' + f.save() + BM = cot.get_model() + co_b = BM.objects.get(pk=co_pk) + co_b.snapshot() + co_b.beta = 'at beta' + co_b.save() + + with activate_branch(branch), event_tracking(branch_request): + f = CustomObjectTypeField.objects.get(custom_object_type=cot, name='beta') + f.snapshot() + f.name = 'gamma' + f.label = 'Gamma' + f.save() + BM = cot.get_model() + co_g = BM.objects.get(pk=co_pk) + co_g.snapshot() + co_g.gamma = 'at gamma' + co_g.save() + BM.objects.create(gamma='new in branch') + + # ── main: alpha → delta ─────────────────────────────────────────── + with event_tracking(request): + f = CustomObjectTypeField.objects.get(custom_object_type=cot, name='alpha') + f.name = 'delta' + f.label = 'Delta' + f.save() + MM = cot.get_model() + co_m = MM.objects.get(pk=co_pk) + co_m.snapshot() + co_m.delta = 'updated in main' + co_m.save() + MM.objects.create(delta='main new') + + # ── sync ────────────────────────────────────────────────────────── + try: + branch.sync(user=self.user, commit=True) + except Exception as exc: + self.fail(f'sync() must not raise when schemas have conflicting renames: {exc!r}') branch.refresh_from_db() - self.assertEqual(branch.status, BranchStatusChoices.PENDING_MIGRATIONS) - branch.migrate(user=self.user) + # _schema_alter_field resolved the conflict by looking up the live column + # ('gamma') in the branch and renaming it to main's target name ('delta'). + branch_conn = connections[branch.connection_name] + with branch_conn.cursor() as cursor: + branch_cols = { + col.name + for col in branch_conn.introspection.get_table_description( + cursor, cot.get_database_table_name(), + ) + } + self.assertIn('delta', branch_cols, 'Branch column must converge to delta after sync') + self.assertNotIn('gamma', branch_cols, 'gamma column must be gone after sync') + self.assertNotIn('beta', branch_cols, 'beta column must be gone after sync') + self.assertNotIn('alpha', branch_cols, 'alpha column must be gone after sync') + + def test_sequential_renames_both_sides_merge(self): + """ + Branch renames A→B→C; main renames A→D independently. + merge() applies branch's rename chain to main. + + When merging the first branch rename (alpha→beta) into main, 'alpha' no + longer exists in main (it was renamed to 'delta') and 'beta' doesn't exist + either. _schema_alter_field detects the conflict, looks up the live column + in main ('delta'), and renames it to 'beta'. The second branch rename + (beta→gamma) then finds 'beta' in main and renames it normally to 'gamma'. + + Expected after merge: main's physical column is 'gamma'. + """ + request = _make_request(self.user) + + with event_tracking(request): + cot = CustomObjectType.objects.create(name='seq_merge_cot', slug='seq-merge-cot') + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='alpha', label='Alpha', type='text', + ) + co = cot.get_model().objects.create(alpha='original') + + co_pk = co.pk + branch = _provision_branch('Seq Merge Conflict Branch', 'iterative', self.user) + branch_request = _make_request(self.user) + + # ── branch: alpha → beta → gamma ────────────────────────────────── + with activate_branch(branch), event_tracking(branch_request): + f = CustomObjectTypeField.objects.get(custom_object_type=cot, name='alpha') + f.snapshot() + f.name = 'beta' + f.label = 'Beta' + f.save() + BM = cot.get_model() + co_b = BM.objects.get(pk=co_pk) + co_b.snapshot() + co_b.beta = 'at beta' + co_b.save() + + with activate_branch(branch), event_tracking(branch_request): + f = CustomObjectTypeField.objects.get(custom_object_type=cot, name='beta') + f.snapshot() + f.name = 'gamma' + f.label = 'Gamma' + f.save() + BM = cot.get_model() + co_g = BM.objects.get(pk=co_pk) + co_g.snapshot() + co_g.gamma = 'at gamma' + co_g.save() + BM.objects.create(gamma='new in branch') + + # ── main: alpha → delta ─────────────────────────────────────────── + with event_tracking(request): + f = CustomObjectTypeField.objects.get(custom_object_type=cot, name='alpha') + f.name = 'delta' + f.label = 'Delta' + f.save() + + # ── merge ───────────────────────────────────────────────────────── + try: + branch.merge(user=self.user, commit=True) + except Exception as exc: + self.fail(f'merge() must not raise when schemas have conflicting renames: {exc!r}') + branch.refresh_from_db() - self.assertEqual(branch.status, BranchStatusChoices.READY) - branch_tables = self._get_branch_tables(branch) - self.assertIn(new_through, branch_tables, 'Renamed through table must exist in branch after migrate') - self.assertNotIn(old_through, branch_tables, 'Old through table must be absent from branch after migrate') + # _schema_alter_field resolved the conflict for the first branch rename + # (alpha→beta): it found 'delta' (main's live column) and renamed it to + # 'beta'. The second rename (beta→gamma) then proceeded normally. + # Main's final physical column should be 'gamma'. + from django.db import connection as main_conn + with main_conn.cursor() as cursor: + main_cols = { + col.name + for col in main_conn.introspection.get_table_description( + cursor, cot.get_database_table_name(), + ) + } + self.assertIn('gamma', main_cols, 'Main column must be gamma after merge') + self.assertNotIn('delta', main_cols, 'delta column must be gone after merge') + self.assertNotIn('beta', main_cols, 'beta column must be gone after merge') + self.assertNotIn('alpha', main_cols, 'alpha column must be gone after merge') + + +@unittest.skipUnless(HAS_BRANCHING, 'netbox-branching is not installed') +class SequentialRenameSquashTestCase(SequentialRenameTestCase, TransactionTestCase): + """Run SequentialRenameTestCase with the squash merge strategy.""" + MERGE_STRATEGY = 'squash' + + def test_sequential_renames_alpha_beta_gamma_merge(self): + self._run_sequential_rename_merge('seq_squash_cot', 'seq-squash-cot') From 5431a3870c11ed89255d425dbfc042f9549a9dcc Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 23 Apr 2026 14:30:55 -0700 Subject: [PATCH 22/25] freeze db column name --- .../0008_customobjecttypefield_db_column.py | 46 +++++++++++++++++++ netbox_custom_objects/models.py | 37 ++++++++++++--- 2 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 netbox_custom_objects/migrations/0008_customobjecttypefield_db_column.py diff --git a/netbox_custom_objects/migrations/0008_customobjecttypefield_db_column.py b/netbox_custom_objects/migrations/0008_customobjecttypefield_db_column.py new file mode 100644 index 0000000..1a40504 --- /dev/null +++ b/netbox_custom_objects/migrations/0008_customobjecttypefield_db_column.py @@ -0,0 +1,46 @@ +""" +Add ``db_column`` to CustomObjectTypeField and back-fill it from ``name``. + +``db_column`` is frozen at field creation time so that subsequent renames are +pure metadata operations — the physical database column name never changes. +This prevents cross-schema column-name mismatches when a field is renamed in +one schema (e.g. a branch) and the model is then used to query a different +schema (e.g. main) that still has the original column name. + +The data migration sets ``db_column = name`` for all existing fields so that +``effective_db_column`` returns the same value as before the migration. +""" + +from django.db import migrations, models + + +def backfill_db_column(apps, schema_editor): + """Set db_column = name for all existing CustomObjectTypeField rows.""" + CustomObjectTypeField = apps.get_model('netbox_custom_objects', 'CustomObjectTypeField') + CustomObjectTypeField.objects.filter(db_column='').update(db_column=models.F('name')) + + +class Migration(migrations.Migration): + + dependencies = [ + ('netbox_custom_objects', '0007_fix_object_field_fk_deferrable'), + ] + + operations = [ + migrations.AddField( + model_name='customobjecttypefield', + name='db_column', + field=models.CharField( + blank=True, + default='', + help_text='Physical database column name. Set once at creation and never changed, so renames are pure metadata changes that do not require DDL.', + max_length=50, + verbose_name='database column', + ), + preserve_default=False, + ), + migrations.RunPython( + backfill_db_column, + migrations.RunPython.noop, + ), + ] diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 733c280..3a8bc8b 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -184,7 +184,7 @@ def _schema_add_field(fi, model, schema_editor, schema_conn): sync/merge replays an ObjectChange that was already applied). """ ft = FIELD_TYPE_CLASS[fi.type]() - mf = ft.get_model_field(fi) + mf = ft.get_model_field(fi, db_column=fi.effective_db_column) mf.contribute_to_class(model, fi.name) with schema_conn.cursor() as cursor: @@ -215,7 +215,7 @@ def _schema_remove_field(fi, model, schema_editor, existing_tables=None): to reject the subsequent ALTER TABLE. """ ft = FIELD_TYPE_CLASS[fi.type]() - mf = ft.get_model_field(fi) + mf = ft.get_model_field(fi, db_column=fi.effective_db_column) mf.contribute_to_class(model, fi.name) if fi.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT: @@ -278,8 +278,8 @@ def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, exist ) return - old_mf = FIELD_TYPE_CLASS[old_fi.type]().get_model_field(old_fi) - new_mf = FIELD_TYPE_CLASS[new_fi.type]().get_model_field(new_fi) + old_mf = FIELD_TYPE_CLASS[old_fi.type]().get_model_field(old_fi, db_column=old_fi.effective_db_column) + new_mf = FIELD_TYPE_CLASS[new_fi.type]().get_model_field(new_fi, db_column=new_fi.effective_db_column) old_mf.contribute_to_class(model, old_fi.name) new_mf.contribute_to_class(model, new_fi.name) @@ -316,7 +316,7 @@ def _schema_alter_field(old_fi, new_fi, model, schema_editor, schema_conn, exist new_fi.pk, schema_conn.alias, ) return - live_mf = FIELD_TYPE_CLASS[live_fi.type]().get_model_field(live_fi) + live_mf = FIELD_TYPE_CLASS[live_fi.type]().get_model_field(live_fi, db_column=live_fi.effective_db_column) live_mf.contribute_to_class(model, live_fi.name) if live_mf.column not in existing_cols: logger.debug( @@ -750,7 +750,7 @@ def _fetch_and_generate_field_attrs( field_name = field.name field_attrs[field.name] = field_type.get_model_field( - field, + field, db_column=field.effective_db_column, ) # Add to field objects only if the field was successfully generated @@ -1324,6 +1324,15 @@ class CustomObjectTypeField(CloningMixin, ExportTemplatesMixin, ChangeLoggedMode ), ), ) + db_column = models.CharField( + verbose_name=_("database column"), + max_length=50, + blank=True, + help_text=_( + "Physical database column name. Set once at creation and never changed, " + "so renames are pure metadata changes that do not require DDL." + ), + ) label = models.CharField( verbose_name=_("label"), max_length=50, @@ -1517,6 +1526,16 @@ def is_single_value(self): def many(self): return self.type in ["multiobject"] + @property + def effective_db_column(self): + """ + Return the physical database column name for this field. + + ``db_column`` is frozen at creation time so that renames are pure + metadata operations — the physical column name never changes. + """ + return self.db_column + def get_child_relations(self, instance): return instance.get_field_value(self) @@ -2149,6 +2168,12 @@ def save(self, using=None, **kwargs): def save(self, *args, **kwargs): is_new = self._state.adding + # Freeze the physical column name at creation. db_column is set once + # here and never updated, so subsequent renames only update the ORM + # field name — no DDL is required for renames. + if self._state.adding and not self.db_column: + self.db_column = self.name + # Use the branch connection when operating inside a branch so that schema # editor operations target the branch schema rather than main. schema_conn = _get_schema_connection() From 44f834a8287e0ea24d46cf99fcb7810109ef707a Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 23 Apr 2026 16:14:52 -0700 Subject: [PATCH 23/25] freeze db column name --- netbox_custom_objects/__init__.py | 49 +++++++++++++++++++++++++++++++ netbox_custom_objects/forms.py | 1 + netbox_custom_objects/models.py | 4 +-- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/netbox_custom_objects/__init__.py b/netbox_custom_objects/__init__.py index af41097..0b2d104 100644 --- a/netbox_custom_objects/__init__.py +++ b/netbox_custom_objects/__init__.py @@ -32,6 +32,51 @@ def _migration_finished(sender, **kwargs): _migrations_checked = None +def _patch_check_object_accessible_in_branch(): + """ + Patch check_object_accessible_in_branch to use an existence check instead of + a full SELECT for custom object models. + + The original implementation does model.objects.get(pk=object_id) which issues + SELECT * including every custom field column. If a field was renamed in the + branch but the stable db_column is not yet reflected in the model (e.g. due to + a stale cache), this can raise ProgrammingError. For custom objects we only + need to know whether the row exists, so filter(pk=...).exists() is sufficient + and avoids referencing any column other than the primary key. + """ + try: + import netbox_branching.signal_receivers as _sr + from netbox_branching.utilities import deactivate_branch + from netbox_branching.models import ChangeDiff + from core.choices import ObjectChangeActionChoices + from django.contrib.contenttypes.models import ContentType + + _original = _sr.check_object_accessible_in_branch + + def _patched(branch, model, object_id): + if model._meta.app_label != APP_LABEL: + return _original(branch, model, object_id) + + # Check existence in main using only the pk — avoids SELECT on + # renamed columns that may not yet exist in main. + with deactivate_branch(): + if model.objects.filter(pk=object_id).exists(): + return True + + # Not in main — was it created in this branch? + content_type = ContentType.objects.get_for_model(model) + return ChangeDiff.objects.filter( + branch=branch, + object_type=content_type, + object_id=object_id, + action=ObjectChangeActionChoices.ACTION_CREATE, + ).exists() + + _sr.check_object_accessible_in_branch = _patched + except (ImportError, AttributeError): + pass + + def _patch_object_selector_view(): """ Patch ObjectSelectorView to support dynamically-generated custom object models. @@ -183,6 +228,10 @@ def ready(self): # Patch ObjectSelectorView to support dynamically-generated custom object models _patch_object_selector_view() + # Patch check_object_accessible_in_branch to use pk-only existence check, + # avoiding SELECT * which references renamed columns that may not exist in main. + _patch_check_object_accessible_in_branch() + # Suppress warnings about database calls during app initialization with warnings.catch_warnings(): warnings.filterwarnings( diff --git a/netbox_custom_objects/forms.py b/netbox_custom_objects/forms.py index 0642ca3..1157c23 100644 --- a/netbox_custom_objects/forms.py +++ b/netbox_custom_objects/forms.py @@ -176,6 +176,7 @@ class CustomObjectTypeFieldForm(CustomFieldForm): class Meta: model = CustomObjectTypeField fields = "__all__" + exclude = ('db_column',) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index 3a8bc8b..ba1ac9b 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -1660,11 +1660,11 @@ def clean(self): and not self.original.unique ): field_type = FIELD_TYPE_CLASS[self.type]() - model_field = field_type.get_model_field(self) + model_field = field_type.get_model_field(self, db_column=self.effective_db_column) model = self.custom_object_type.get_model() model_field.contribute_to_class(model, self.name) - old_field = field_type.get_model_field(self.original) + old_field = field_type.get_model_field(self.original, db_column=self.original.effective_db_column) old_field.contribute_to_class(model, self._original_name) try: From fe534f25edf77f89a06c69e3525094f196b0d14b Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 23 Apr 2026 17:32:30 -0700 Subject: [PATCH 24/25] freeze db column name --- netbox_custom_objects/__init__.py | 43 +++++++++++++++++++++++++++++++ netbox_custom_objects/models.py | 7 +++++ 2 files changed, 50 insertions(+) diff --git a/netbox_custom_objects/__init__.py b/netbox_custom_objects/__init__.py index 0b2d104..91f86e4 100644 --- a/netbox_custom_objects/__init__.py +++ b/netbox_custom_objects/__init__.py @@ -32,6 +32,45 @@ def _migration_finished(sender, **kwargs): _migrations_checked = None +def _patch_get_serializer_for_model(): + """ + Patch utilities.api.get_serializer_for_model to handle dynamically-generated + custom object models. + + The default implementation resolves serializers by import path convention + (e.g. netbox_custom_objects.api.serializers.Table1ModelSerializer). Dynamic + models have no importable serializer at that path, so the call raises + SerializerNotFound. This patch intercepts the lookup for APP_LABEL models and + delegates to get_serializer_class(), which generates the serializer on the fly. + """ + import utilities.api as _api_utils + from netbox.api.exceptions import SerializerNotFound + + _original = _api_utils.get_serializer_for_model + + def _patched(model, prefix=''): + # Only intercept dynamically-generated custom object models (Table1Model, + # Table2Model, …) identified by their Table{n}Model name pattern. + # CustomObjectType and CustomObjectTypeField live in the same app but + # have importable serializers and must go through the normal path. + if getattr(model, '_meta', None) and model._meta.app_label == APP_LABEL \ + and extract_cot_id_from_model_name(model.__name__.lower()) is not None: + from netbox_custom_objects.api.serializers import get_serializer_class + return get_serializer_class(model) + return _original(model, prefix=prefix) + + _api_utils.get_serializer_for_model = _patched + + # Also patch the reference already imported into extras.events (and anywhere + # else that did `from utilities.api import get_serializer_for_model` before + # our patch ran). + try: + import extras.events as _extras_events + _extras_events.get_serializer_for_model = _patched + except (ImportError, AttributeError): + pass + + def _patch_check_object_accessible_in_branch(): """ Patch check_object_accessible_in_branch to use an existence check instead of @@ -228,6 +267,10 @@ def ready(self): # Patch ObjectSelectorView to support dynamically-generated custom object models _patch_object_selector_view() + # Patch get_serializer_for_model so event rules, job serializers, etc. can + # resolve serializers for dynamically-generated custom object models. + _patch_get_serializer_for_model() + # Patch check_object_accessible_in_branch to use pk-only existence check, # avoiding SELECT * which references renamed columns that may not exist in main. _patch_check_object_accessible_in_branch() diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index ba1ac9b..e5dba2f 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -708,6 +708,13 @@ def get_cached_through_models(cls, custom_object_type_id): """ return cls._through_model_cache.get(custom_object_type_id, {}) + def serialize_object(self, exclude=None): + # cache_timestamp is an internal cache-invalidation field; exclude it + # from ObjectChange records so it doesn't appear as a tracked change. + extra = ['cache_timestamp'] + combined = list(exclude or []) + extra + return super().serialize_object(exclude=combined) + def get_absolute_url(self): return reverse("plugins:netbox_custom_objects:customobjecttype", args=[self.pk]) From df61efbbc49086d4ed623b86f3a6f7758ec05750 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 23 Apr 2026 17:44:58 -0700 Subject: [PATCH 25/25] freeze db column name --- netbox_custom_objects/__init__.py | 1 - .../0008_customobjecttypefield_db_column.py | 5 ++++- netbox_custom_objects/models.py | 12 +++++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/netbox_custom_objects/__init__.py b/netbox_custom_objects/__init__.py index 91f86e4..b1ec966 100644 --- a/netbox_custom_objects/__init__.py +++ b/netbox_custom_objects/__init__.py @@ -44,7 +44,6 @@ def _patch_get_serializer_for_model(): delegates to get_serializer_class(), which generates the serializer on the fly. """ import utilities.api as _api_utils - from netbox.api.exceptions import SerializerNotFound _original = _api_utils.get_serializer_for_model diff --git a/netbox_custom_objects/migrations/0008_customobjecttypefield_db_column.py b/netbox_custom_objects/migrations/0008_customobjecttypefield_db_column.py index 1a40504..70ff491 100644 --- a/netbox_custom_objects/migrations/0008_customobjecttypefield_db_column.py +++ b/netbox_custom_objects/migrations/0008_customobjecttypefield_db_column.py @@ -33,7 +33,10 @@ class Migration(migrations.Migration): field=models.CharField( blank=True, default='', - help_text='Physical database column name. Set once at creation and never changed, so renames are pure metadata changes that do not require DDL.', + help_text=( + 'Physical database column name. Set once at creation and never changed, ' + 'so renames are pure metadata changes that do not require DDL.' + ), max_length=50, verbose_name='database column', ), diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index e5dba2f..ba0c27b 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -2234,8 +2234,18 @@ def ensure_constraint(): transaction.on_commit(ensure_constraint) + # Regenerate the model from DB now that the field change is persisted. + # _schema_alter_field adds both old and new field names to the model + # class via contribute_to_class, and something between clear_model_cache + # and super().save() (e.g. a post_save signal on the COT) can call + # get_model() while the DB still has the old name, caching a stale model + # without the new name. Forcing a no_cache regeneration here (after + # super().save() committed the new name) ensures apps.all_models holds a + # clean model with exactly the current DB field list. + updated_model = self.custom_object_type.get_model(no_cache=True) + # Reregister SearchIndex with new set of searchable fields - self.custom_object_type.register_custom_object_search_index(model) + self.custom_object_type.register_custom_object_search_index(updated_model) # Reindex all objects of this type if search indexing was affected if is_new: