Closes: #386 - Exportable schema support

[bctiemann]

Closes: #386

Provides the groundwork and core data model for the portable COT schema format. Introduces stable field identifiers (schema_id), field lifecycle metadata (deprecation / scheduled removal), the schema_document store, and the canonical JSON Schema definition (cot_schema_v1.json) against which schema documents are validated.

Model changes — CustomObjectTypeField

Field	Type	Purpose
schema_id	PositiveSmallIntegerField (nullable)	Stable numeric identifier — never reused within a COT, even after deletion
deprecated	BooleanField	Mark a field as read-only / soft-retired in the UI
deprecated_since	CharField	Schema version in which the field was deprecated
scheduled_removal	CharField	Schema version in which the field is planned for removal

Model changes — CustomObjectType

Field	Type	Purpose
schema_document	JSONField (nullable)	Last-applied schema document for this COT; source of truth for tombstoned field records
next_schema_id	PositiveIntegerField (default 0)	Monotonically increasing counter; auto-incremented on field creation via QuerySet.update() to avoid clearing the model cache

schema_id is protected by a UniqueConstraint(fields=["schema_id", "custom_object_type"], condition=Q(schema_id__isnull=False)).

next_schema_id design note

schema_id auto-assignment in CustomObjectTypeField.save() increments next_schema_id via CustomObjectType.objects.filter(pk=...).update(next_schema_id=new_id) rather than cot.save(). This bypasses the post_save signal that clears the in-process model cache, preventing a race where contribute_to_class() targets a freshly regenerated model class rather than the one get_model() already returned to the field-save caller.

cot_schema_v1.json

Formal JSON Schema (draft 2020-12) for the portable document format. Key design points:

• Top-level document requires schema_version: "1" and a types array.
• Each field variant (text_field, integer_field, object_field, …) uses unevaluatedProperties: false to prevent extra keys from silently passing validation — mirroring the strictness already present on removed_field.
• removed_field tombstone entries carry id, name, type, and optional removed_in version.
• A discriminator hint (OpenAPI 3.x) is included for tooling; standard JSON Schema validators use the oneOf branches.

schema_format.py

Shared constants consumed by the exporter, comparator, and executor:

• SCHEMA_FORMAT_VERSION — current format version string ("1")
• CHOICES_TO_SCHEMA_TYPE / SCHEMA_TYPE_TO_CHOICES — bidirectional mapping between CustomFieldTypeChoices values and schema type names
• FIELD_DEFAULTS — canonical default values for all field attributes (omitted from schema documents when equal to default)
• FIELD_TYPE_ATTRS — which type-specific attributes are valid per field type

Tests added

• FieldDefaultsConsistencyTestCase — cross-checks every key in FIELD_DEFAULTS against the corresponding model field default; catches drift between the constants and the model.
• SchemaIdBackfillTestCase — validates the 0005 migration logic: sequential assignment, monotonic counter update, gap-after-deletion prevention.
• Extended SchemaFormatTestCase — unevaluatedProperties rejects extra keys on all 9 field subtypes; removed_field accepts removed_in.

Note: schema_document is added in this PR but left null; the exporter (#388, PR #450) populates it.

[bctiemann]

@claude /review

[claude]

Claude finished @bctiemann's task in 1m 49s —— View job

---

PR Review: Exportable Schema Support (#449)

Tasks:

• Gather context and understand request
• Read changed files
• Analyze code quality, correctness, and style
• Post review feedback

---

Overview

This PR introduces the foundational data model and JSON Schema format for a portable, exportable Custom Object Type (COT) schema. It adds:

• schema_id (stable per-field identity), deprecated/deprecated_since/scheduled_removal fields on CustomObjectTypeField
• schema_document (stored JSON snapshot) and version on CustomObjectType
• schema_format.py — a module of constants/mappings for the schema format
• cot_schema_v1.json — a JSON Schema (Draft 2020-12) validator
• A comprehensive test suite (486 lines)

This is solid foundational work. The design decisions are well-reasoned and the code is clean. Below are observations ranging from correctness issues to minor suggestions.

---

Issues

1. Race condition in schema_id auto-assignment (models.py:1591-1604)

The select_for_update() on the COT row correctly serializes concurrent writes. However, super().save() is called outside the atomic block at line 1738, meaning the schema_id is committed to self.schema_id in memory, but the field row isn't saved yet. If the outer save fails after the next_schema_id counter was incremented, the counter drifts. This is an acknowledged trade-off (the comment mentions IntegrityError as a safety net), but it's worth documenting explicitly that next_schema_id may advance without a corresponding field existing — which is intentional for the "never reuse" guarantee.

The comment is actually there and explains this well — but the code flow might be clearer if the select_for_update block were a helper method.

2. schema_document is stored but never populated (models.py:210-217)

The field is added, the serializer marks it read_only, and tests verify it can store JSON — but nothing in this PR actually writes the schema_document field. There is no exporter yet. This is fine as "groundwork" (the PR description says "core implementation"), but it means schema_document will remain null for all existing and new COTs until a follow-up PR adds the export logic. This should be called out explicitly in the PR description or a TODO comment.

Fix this →

3. PositiveSmallIntegerField for next_schema_id and schema_id limits to 32,767 (migration:18, models.py:943)

Both fields use PositiveSmallIntegerField (max 32,767). For most use cases this is fine, but if a COT ever has more than 32,767 fields created and deleted over its lifetime, the counter would overflow silently. Consider using PositiveIntegerField (max ~2.1 billion) for safety, especially since this counter never decreases. The cost is 2 extra bytes per row.

Fix this →

---

Design Observations

4. identifier pattern in JSON Schema is stricter than model validator (cot_schema_v1.json:16-17)

The JSON Schema identifier pattern ^[a-z0-9]+(_[a-z0-9]+)*$ disallows leading/trailing underscores and consecutive underscores. The model validator on CustomObjectTypeField.name uses ^[a-z0-9_]+$ (plus a separate __ check). This means a field name like _private would be valid in the DB but rejected by the schema validator. The comment in the JSON Schema already documents this discrepancy, which is good — but importers will need to handle this edge case explicitly.

5. removed_field uses additionalProperties: false but field does not (cot_schema_v1.json:251-268)

Active field objects use oneOf with open schemas (no additionalProperties: false). This means an active field can carry arbitrary extra keys without failing validation. If the intent is to be strict about what goes into a schema document, consider adding unevaluatedProperties: false to field_base and the type-specific variants. The asymmetry with removed_field is a bit surprising.

6. schema_document is read-only in the serializer but writable in the model

read_only_fields = ("schema_document",) in CustomObjectTypeSerializer.Meta prevents API clients from writing this field — correct. However, there's no editable=False on the model field itself, so the Django admin and any direct ORM usage can still set it. This is probably fine (the schema exporter will need to write it), but worth noting the asymmetry.

7. deprecated_since / scheduled_removal are free-form strings with no validation

Both fields are CharField(max_length=50, blank=True) with no format validation. The help text says "e.g. '2.0.0'" suggesting semver, but nothing enforces this. A user could set deprecated_since = "next Tuesday" and it would save fine. This might be intentional (flexibility), but if the schema format treats these as version strings, a RegexValidator or clean() check would improve consistency.

---

Code Quality

8. schema_format.py imports from extras.choices at module level

from extras.choices import CustomFieldTypeChoices

This is imported at module level in schema_format.py. Since this module is also used in tests (from netbox_custom_objects.schema_format import ...), this is fine. Just confirm this import doesn't cause issues in any lightweight/offline contexts (e.g., documentation generation, static analysis).

9. FIELD_DEFAULTS in schema_format.py vs. model defaults

The FIELD_DEFAULTS dict (lines 66-90 in schema_format.py) duplicates default values that are also defined on the Django model fields. If a model default ever changes (e.g., weight changes from 100 to 50), FIELD_DEFAULTS could silently become out of sync. Consider adding a test that cross-checks these defaults against the model's field.default attribute.

10. Test: test_schema_id_scoped_to_cot assumes first field always gets ID 1

self.assertEqual(fa.schema_id, fb.schema_id,
    "First field in each COT should both receive schema_id=1.")

This is technically an implementation assumption. The spec says schema IDs are scoped to the COT and sequential, but it doesn't require starting at 1. If the starting point ever changes (e.g., starts at 0 or some other seed), this test would break even if the behavior is correct. Consider weakening the assertion to just check that fa.schema_id == fb.schema_id without assuming the value.

11. COTJsonSchemaTestCase is conditionally skipped — consider marking as optional dependency

jsonschema is an optional dependency. If the intent is to validate schema documents regularly, it might be worth adding jsonschema to the dev/test requirements so these tests always run in CI.

---

Minor / Style

• The discriminator in cot_schema_v1.json (lines 245-248) is well-commented about its non-standard status — good practice.
• schema_format.py is well-documented with a clear format version history section at the top.
• The migration constraint name netbox_custom_objects_customobjecttypefield_unique_schema_id (line 55 of migration) differs from the model constraint name %(app_label)s_%(class)s_unique_schema_id — Django resolves these at migration time so this is fine, but worth verifying they match.

---

Summary

This is a well-designed groundwork PR. The main things to address before merging:

1. PositiveSmallIntegerField → PositiveIntegerField for next_schema_id and schema_id to avoid a 32k field limit (low cost, avoids future pain).
2. Explicit TODO indicating that schema_document population requires a follow-up exporter PR.
3. Consider unevaluatedProperties: false on active field schemas if strict schema documents are desired.

Everything else is minor design commentary or optional improvements.

[bctiemann]

Addressed items 1 and 3 in the summary, and also added a test to cover item 9 in Code Quality. The rest are non-issues for the time being.

[arthanson]

1. Model validators vs. identifier pattern mismatch

CustomObjectType.name and CustomObjectTypeField.name both validate to ^[a-z0-9_]+$ (no double underscores). The JSON Schema identifier pattern (cot_schema_v1.json:675) is ^[a-z0-9]+(_[a-z0-9]+)*$, which additionally disallows leading and trailing underscores.

So a COT or field named foo or bar is valid in the database but would produce an export that fails its own schema validator. The comment on identifier calls this out for field names, but cot_definition.name (schemas/cot_schema_v1.json:943) uses the same pattern — the gap applies to COT names too.

Seems like the validators should match.

2. schema_id - I'm a bit confused on this. On one hand it is an auto-incrementing integer value and it is exported into the json schema. It looks like this is to avoid conflicts between different people making updates to the schema but I think that would break down if they make two completely independent changes at the same time, the generated schema_id would be the same.

If it is not to avoid conflicts between different people, then it seems we could just use the fields pk. If it is to avoid conflicts it seems a UUID would be much better / safer for this.

[bctiemann]

Design note: schema_id integer vs. UUID

While reviewing the portable schema work, a question came up about why CustomObjectTypeField.schema_id uses a small integer rather than a UUID. Capturing the reasoning here for future reference.

Why not a UUID?

Short integers were chosen for:

• Readability — schema documents are JSON that humans read and diff in version control. "id": 3 is cleaner than "id": "a3f2c1d0-...".
• Meaningful ordering — schema_id is monotonically increasing, so field 7 was provably added after field 4. UUIDs carry no ordering.
• No global uniqueness needed — IDs are scoped per-COT and assigned under a SELECT ... FOR UPDATE lock on next_schema_id, so cross-instance collision isn't a concern within the intended workflow.

The hidden assumption

The integer scheme implicitly assumes a single source of truth — one canonical environment that exports schemas, and one or more targets that receive them. In that hub-and-spoke model it works correctly.

If two environments independently add a field to the same COT, both will assign the same next integer ID. When the schemas are later reconciled, the comparator sees the same ID on two semantically different fields — a false match that could silently alter or rename the wrong column.

A UUID would eliminate the false-match class of bug: independently-created fields get distinct IDs and the comparator correctly identifies them as two new additions. However, a UUID doesn't solve the underlying merge problem — it just surfaces it more honestly. The executor is currently designed for apply-from-canonical, not merge, so the unresolved additions would still need a human decision regardless of ID scheme.

Summary

• Current design is correct for the intended one-authoritative-environment workflow.
• If the product ever wants bidirectional sync or multi-master schema evolution, switching schema_id to a UUID would be the right foundation (at the cost of readability and ordering).
• Worth flagging as a known constraint rather than a design oversight.

[bctiemann]

@arthanson Design discussion has settled that we are going with a single-source-of-truth model, i.e. many NetBox instances pull down an authoritative COT schema from a central repository, rather than trying to merge many peer schema changes. So for the schema_id question, we will stick with integers.

Note, this generally follows the pattern established by GRPC/protobuf, which also uses integers for its field IDs and places the onus on users to increment and curate them properly.

[bctiemann]

@arthanson I addressed your concern about the regex validation on the name fields. Also added a documentation file.

The migrations will cause failures until we've merged #472, so that should go in next.

[arthanson]

feature now has the Django 6 change from main so the test should pass but merging feature gets some merge conflicts that need to be cleaned up. Also for #442

[arthanson]

The basics look good - but can't really test much. One note to change before merge - the schema_id is not read-only in the serializer and it should be. It was stated it can't change, but right now someone could post/put to it and change it.