starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Generic API test harness miscompares ArrayField(IntegerRangeField) values (NumericRange vs inclusive pairs) #11681

New Issue
Closed
opened 2025-12-29 21:48:34 +01:00 by adam · 3 comments
adam commented 2025-12-29 21:48:34 +01:00
Owner
Copy Link

Originally created by @pheus on GitHub (Oct 3, 2025).

Originally assigned to: @bctiemann, @pheus on GitHub.

NetBox Edition

NetBox Community

NetBox Version

v4.4.2

Python Version

3.12

Steps to Reproduce

  1. Define a model with an array of integer ranges: 
    from django.db import models
from django.contrib.postgres.fields import ArrayField, IntegerRangeField

class DemoModel(models.Model):
    name = models.CharField(max_length=50)
    port_ranges = ArrayField(base_field=IntegerRangeField(), blank=True, default=list)
  2. Expose it in the API using the existing range serializer: 
    from netbox.api.serializers import NetBoxModelSerializer
from netbox.api.fields import IntegerRangeSerializer

class DemoSerializer(NetBoxModelSerializer):
    # Inclusive [start, end] pairs; list handled by many=True
    port_ranges = IntegerRangeSerializer(many=True, required=False)

    class Meta:
        model = DemoModel
        fields = ("id", "name", "port_ranges")
  3. Write an APIViewTestCase that creates/updates using inclusive pairs: 
    from netbox.utilities.testing import APIViewTestCases

class DemoAPIViewTestCase(APIViewTestCases.APIViewTestCase):
    model = DemoModel

    @classmethod
    def setUpTestData(cls):
        cls.create_data = [
            {"name": "demo1", "port_ranges": [[22, 22], [443, 443]]}
        ]
  4. Run the tests.

Expected Behavior

When api=True, model_to_dict() should normalize ArrayField(IntegerRangeField) values to inclusive pairs [[lo, hi], ...], matching:

  • the API serializer representation (IntegerRangeSerializer)
  • the established VLANGroup.vid_ranges representation.

Observed Behavior

utilities.testing.api::test_update_object → assertInstanceEqual compares request data to model_to_dict(instance, api=True). For ArrayField(IntegerRangeField), the model dict includes psycopg ranges:

AssertionError:
- 'port_ranges': [Range(22, 23, '[)'), Range(443, 444, '[)')],
+ 'port_ranges': [[22, 22], [443, 443]],

Root Cause

PostgreSQL canonicalizes discrete integer ranges to half‑open [lo, hi). The test harness directly compares these psycopg Range objects against the inclusive JSON pairs submitted by the test case.

Proposed Fix

Normalize arrays of numeric ranges within the api=True branch of utilities.testing.base:model_to_dict():

# utilities/testing/base.py (inside model_to_dict(), within the `api` branch)
elif type(field) is ArrayField and issubclass(type(field.base_field), RangeField):
    # Convert half-open [lo, hi) to inclusive [lo, hi-1]
    model_dict[key] = [[r.lower, r.upper - 1] for r in value]

This mirrors the non‑API branch (which already special‑cases range arrays for forms/CSV) and aligns the generic API tests with the inclusive JSON representation used by NetBox serializers.

Minimal End-to-End Example

  • Model: ArrayField(IntegerRangeField) as above.
  • Serializer: IntegerRangeSerializer(many=True, required=False).
  • Test payload: {"name": "demo1", "port_ranges": [[22, 22], [443, 443]]}.
  • Before fix: Fails: Range(22, 23, '[)') vs [[22, 22]].
  • After fix: Passes: both sides compare as [[22, 22], [443, 443]].

Notes

  • This change affects only the test utility normalization for api=True. It does not alter database storage, API behavior, or public schemas.
  • It brings the generic test harness in line with the inclusive‑pair API contract already used elsewhere (e.g., VLAN ID ranges).
Originally created by @pheus on GitHub (Oct 3, 2025). Originally assigned to: @bctiemann, @pheus on GitHub. ### NetBox Edition NetBox Community ### NetBox Version v4.4.2 ### Python Version 3.12 ### Steps to Reproduce 1. Define a model with an array of integer ranges: ```python from django.db import models from django.contrib.postgres.fields import ArrayField, IntegerRangeField class DemoModel(models.Model): name = models.CharField(max_length=50) port_ranges = ArrayField(base_field=IntegerRangeField(), blank=True, default=list) ``` 2. Expose it in the API using the existing range serializer: ```python from netbox.api.serializers import NetBoxModelSerializer from netbox.api.fields import IntegerRangeSerializer class DemoSerializer(NetBoxModelSerializer): # Inclusive [start, end] pairs; list handled by many=True port_ranges = IntegerRangeSerializer(many=True, required=False) class Meta: model = DemoModel fields = ("id", "name", "port_ranges") ``` 3. Write an `APIViewTestCase` that creates/updates using inclusive pairs: ```python from netbox.utilities.testing import APIViewTestCases class DemoAPIViewTestCase(APIViewTestCases.APIViewTestCase): model = DemoModel @classmethod def setUpTestData(cls): cls.create_data = [ {"name": "demo1", "port_ranges": [[22, 22], [443, 443]]} ] ``` 4. Run the tests. ### Expected Behavior When `api=True`, `model_to_dict()` should normalize `ArrayField(IntegerRangeField)` values to **inclusive pairs** `[[lo, hi], ...]`, matching: - the API serializer representation (`IntegerRangeSerializer`) - the established `VLANGroup.vid_ranges` representation. ### Observed Behavior `utilities.testing.api::test_update_object` → `assertInstanceEqual` compares request data to `model_to_dict(instance, api=True)`. For `ArrayField(IntegerRangeField)`, the model dict includes psycopg ranges: ``` AssertionError: - 'port_ranges': [Range(22, 23, '[)'), Range(443, 444, '[)')], + 'port_ranges': [[22, 22], [443, 443]], ``` ### Root Cause PostgreSQL canonicalizes discrete integer ranges to half‑open `[lo, hi)`. The test harness directly compares these psycopg `Range` objects against the inclusive JSON pairs submitted by the test case. ### Proposed Fix Normalize arrays of numeric ranges within the `api=True` branch of `utilities.testing.base:model_to_dict()`: ```python # utilities/testing/base.py (inside model_to_dict(), within the `api` branch) elif type(field) is ArrayField and issubclass(type(field.base_field), RangeField): # Convert half-open [lo, hi) to inclusive [lo, hi-1] model_dict[key] = [[r.lower, r.upper - 1] for r in value] ``` This mirrors the non‑API branch (which already special‑cases range arrays for forms/CSV) and aligns the generic API tests with the inclusive JSON representation used by NetBox serializers. ### Minimal End-to-End Example - **Model:** `ArrayField(IntegerRangeField)` as above. - **Serializer:** `IntegerRangeSerializer(many=True, required=False)`. - **Test payload:** `{"name": "demo1", "port_ranges": [[22, 22], [443, 443]]}`. - **Before fix:** Fails: `Range(22, 23, '[)')` vs `[[22, 22]]`. - **After fix:** Passes: both sides compare as `[[22, 22], [443, 443]]`. ### Notes - This change affects only the **test utility** normalization for `api=True`. It does **not** alter database storage, API behavior, or public schemas. - It brings the generic test harness in line with the inclusive‑pair API contract already used elsewhere (e.g., VLAN ID ranges).
adam added the type: bugstatus: acceptednetbox labels 2025-12-29 21:48:34 +01:00
adam closed this issue 2025-12-29 21:48:34 +01:00
adam commented 2025-12-29 21:48:35 +01:00
Author
Owner
Copy Link

@pheus commented on GitHub (Oct 3, 2025):

Additional reproduction using core VLANGroupTest (ipam.tests.test_api)

You can reproduce the same mismatch in NetBox core (so this isn’t plugin‑specific) by adding a single vid_ranges entry to the existing API test.

Minimal change

In netbox/ipam/tests/test_api.py, modify VLANGroupTest to include vid_ranges in create_data:

class VLANGroupTest(APIViewTestCases.APIViewTestCase):
    model = VLANGroup
    brief_fields = ['description', 'display', 'id', 'name', 'slug', 'url', 'vlan_count']
    create_data = [
        {
            'name': 'VLAN Group 4',
            'slug': 'vlan-group-4',
            'vid_ranges': [[1, 4094]],  # ← inclusive pair
        },
        {
            'name': 'VLAN Group 5',
            'slug': 'vlan-group-5',
        },
        {
            'name': 'VLAN Group 6',
            'slug': 'vlan-group-6',
        },
    ]
    bulk_update_data = {
        'description': 'New description',
    }

How to run

  • python3 manage.py test ipam.tests.test_api.VLANGroupTest

Observed failure (without the proposed model_to_dict(api=True) normalization)

AssertionError: ...
- 'vid_ranges': [Range(1, 4095, '[)')],
+ 'vid_ranges': [[1, 4094]],

This shows the generic API test harness is comparing the request payload (inclusive pairs) to the model’s raw value (psycopg half‑open ranges). Applying the one‑line normalization in utilities.testing.base:model_to_dict() for api=True:

elif type(field) is ArrayField and issubclass(type(field.base_field), RangeField):
    model_dict[key] = [[r.lower, r.upper - 1] for r in value]

makes this test pass as well, aligning the comparison with the API’s inclusive [lo, hi] representation (as used by IntegerRangeSerializer(many=True, required=False)).

@pheus commented on GitHub (Oct 3, 2025): **Additional reproduction using core `VLANGroupTest` (`ipam.tests.test_api`)** You can reproduce the same mismatch in NetBox core (so this isn’t plugin‑specific) by adding a single `vid_ranges` entry to the existing API test. ### Minimal change In `netbox/ipam/tests/test_api.py`, modify `VLANGroupTest` to include `vid_ranges` in `create_data`: ```python class VLANGroupTest(APIViewTestCases.APIViewTestCase): model = VLANGroup brief_fields = ['description', 'display', 'id', 'name', 'slug', 'url', 'vlan_count'] create_data = [ { 'name': 'VLAN Group 4', 'slug': 'vlan-group-4', 'vid_ranges': [[1, 4094]], # ← inclusive pair }, { 'name': 'VLAN Group 5', 'slug': 'vlan-group-5', }, { 'name': 'VLAN Group 6', 'slug': 'vlan-group-6', }, ] bulk_update_data = { 'description': 'New description', } ``` ### How to run - `python3 manage.py test ipam.tests.test_api.VLANGroupTest` ### Observed failure (without the proposed `model_to_dict(api=True)` normalization) ``` AssertionError: ... - 'vid_ranges': [Range(1, 4095, '[)')], + 'vid_ranges': [[1, 4094]], ``` This shows the generic API test harness is comparing the request payload (inclusive pairs) to the model’s raw value (psycopg half‑open ranges). Applying the one‑line normalization in `utilities.testing.base:model_to_dict()` for `api=True`: ```python elif type(field) is ArrayField and issubclass(type(field.base_field), RangeField): model_dict[key] = [[r.lower, r.upper - 1] for r in value] ``` makes this test pass as well, aligning the comparison with the API’s inclusive `[lo, hi]` representation (as used by `IntegerRangeSerializer(many=True, required=False)`).
adam commented 2025-12-29 21:48:35 +01:00
Author
Owner
Copy Link

@pheus commented on GitHub (Oct 3, 2025):

I’ve identified a root cause and I’m happy to open a PR if this report is accepted.

@pheus commented on GitHub (Oct 3, 2025): I’ve identified a root cause and I’m happy to open a PR if this report is accepted.
adam commented 2025-12-29 21:48:35 +01:00
Author
Owner
Copy Link

@bctiemann commented on GitHub (Dec 10, 2025):

@pheus Since you've already done the research and work on this, maybe it makes sense for you to take this on? Thanks!

@bctiemann commented on GitHub (Dec 10, 2025): @pheus Since you've already done the research and work on this, maybe it makes sense for you to take this on? Thanks!
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label netbox status: accepted type: bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#11681
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?