Closes #22835: Improve performance when provisioning new custom fields (#22866)

This commit is contained in:
Jeremy Stretch 2026-08-06 15:56:22 -04:00 committed by GitHub
parent b1ee8297d2
commit 80231a9706
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 805 additions and 68 deletions

View File

@ -30,6 +30,15 @@ Marking a field as required will force the user to provide a value for the field
A custom field must be assigned to one or more object types, or models, in NetBox. Once created, custom fields will automatically appear as part of these models in the web UI and REST API. Note that not all models support custom fields.
!!! info "This behavior changed in NetBox v4.6.8."
To improve performance when creating custom fields, empty field values are no longer pre-provisioned.
Unless the field has been assigned a default value, creating a custom field does not write a value to the objects which already exist. An object which has never been assigned a value simply stores nothing for the field, and reports the field as having no value in the web UI, REST API, GraphQL API, and exports, exactly as if it stored an explicit null.
This matters only if you query the underlying `custom_field_data` JSON directly, for example in a custom script. The field's key is absent from an object's data until a value is assigned to it, so read it with `obj.cf['field_name']` or `obj.custom_field_data.get('field_name')` rather than by direct subscript.
Assigning a default value, by contrast, does write that value to every existing object at the time the field is created, so that objects can be filtered by it immediately. On a model with a very large number of objects, this can take some time. Note that a default added to a field which already exists is _not_ backfilled: objects with no value continue to report none until they are next saved.
### Filtering
The filter logic controls how values are matched when filtering objects by the custom field. Loose filtering (the default) matches on a partial value, whereas exact matching requires a complete match of the given string to a field's value. For example, exact filtering with the string "red" will only match the exact value "red", whereas loose filtering will match on the values "red", "red-orange", or "bored". Setting the filter logic to "disabled" disables filtering by the field entirely.

View File

@ -1,13 +1,111 @@
from functools import cache
import django_filters
from django.db.models import Q
from .models import Tag
__all__ = (
'MissingKeyAwareFilterMixin',
'TagFilter',
'TagIDFilter',
'missing_key_aware_filter_factory',
)
class MissingKeyAwareFilterMixin:
"""
Treat a JSON key which is absent as equivalent to one holding a null value: an object storing
no value for a custom field must filter identically however that absence is represented.
Custom field data materializes a key only once a value is assigned to it (see
CustomField.populate_initial_data()), so an object predating a field carries no key for it at
all, whereas one whose value has been cleared holds a JSON null. Postgres treats the two
differently, in two places:
* Django compiles `exclude(custom_field_data__foo='x')` to a bare `NOT (data -> 'foo' = 'x')`.
A row which does not carry the key yields SQL NULL there, so the negation evaluates to NULL
and the row is discarded. A row holding a JSON null fares no better under any of the text
lookups (icontains, istartswith, etc.), which compare `data ->> 'foo'` and so are NULL for a
JSON null as well.
* The null sentinel (`?cf_foo=null`; see FILTERS_NULL_CHOICE_VALUE) asks for the objects holding
no value. MultipleChoiceFilter.filter() translates it to None and hands it to
get_filter_predicate(), which builds a lookup matching a JSON null only -- silently omitting
every object which predates the field.
Both directions are handled: the sentinel is mapped onto "holds no value" rather than onto a
predicate of its own, and a negation is built explicitly so that valueless rows are admitted.
Two constraints on where this may be mixed in, both satisfied by every filter class
CustomField.to_filter() can select:
* filter() is reimplemented rather than delegated to, so any custom filter() on the base class
is bypassed. Do not mix this into a class which overrides filter() (e.g.
MultiValueMACAddressFilter, MultiValueContentTypeFilter).
missing_key_aware_filter_factory() rejects such classes.
* `conjoined` is not honored: multiple values are always OR'ed. Passing it raises TypeError.
"""
def __init__(self, *args, **kwargs):
if kwargs.get('conjoined'):
raise TypeError(
f"{type(self).__name__} does not support conjoined filtering: multiple values are "
f"always OR'ed."
)
super().__init__(*args, **kwargs)
def filter(self, qs, value):
if not value:
return super().filter(qs, value)
# `<key>__isnull` matches only a missing key and `<key>=None` only a JSON null, so together
# they select exactly the objects holding no value. Both are null-safe, which is what makes
# them usable inside the negation below.
unset = Q(**{f'{self.field_name}__isnull': True}) | Q(**{self.field_name: None})
values = set(value)
match_unset = self.null_value in values
values.discard(self.null_value)
q = Q()
for v in values:
q |= Q(**self.get_filter_predicate(v))
if match_unset:
q |= unset
if self.exclude:
# Negate explicitly rather than deferring to exclude(), whose bare NOT discards the
# rows carrying no key. Those rows are admitted, unless holding no value is itself one
# of the things being excluded.
q = ~q if match_unset else ~q | unset
qs = qs.filter(q)
return qs.distinct() if self.distinct else qs
@cache
def missing_key_aware_filter_factory(filter_class):
"""
Return a subclass of the given filter class which treats an absent JSON key as equivalent to a
null one. Results are cached so that each filter class yields a single stable subclass.
The class must inherit MultipleChoiceFilter.filter() unmodified: the mixin reimplements it, so a
filter() of its own (and with it any custom predicate or short-circuit) would be silently
bypassed, yielding a wrong result set rather than an error.
"""
if filter_class.filter is not django_filters.MultipleChoiceFilter.filter:
raise TypeError(
f"{filter_class.__name__} cannot be made missing-key aware: it defines its own "
f"filter(), which MissingKeyAwareFilterMixin would bypass."
)
return type(
f'MissingKeyAware{filter_class.__name__}',
(MissingKeyAwareFilterMixin, filter_class),
{}
)
class TagFilter(django_filters.ModelMultipleChoiceFilter):
"""
Match on one or more assigned tags. If multiple tags are specified (e.g. ?tag=foo&tag=bar), the queryset is filtered

View File

@ -4,7 +4,7 @@ import strawberry
import strawberry_django
from strawberry.types import Info
from extras.models import ImageAttachment, JournalEntry
from extras.models import CustomField, ImageAttachment, JournalEntry
from utilities.querysets import RestrictedPrefetch
__all__ = (
@ -47,7 +47,16 @@ class CustomFieldsMixin:
@strawberry_django.field(only=['custom_field_data'])
def custom_fields(self) -> strawberry.scalars.JSON:
return self.custom_field_data
# Emit a key for every custom field assigned to the model, as the REST API does, rather than
# returning the stored data verbatim. A key is materialized only once a value is assigned
# (see CustomField.populate_initial_data()), so an object which predates a field carries no
# key for it; without this, such a field would be absent from the response instead of null.
# CustomFieldManager.get_for_model() is served from the per-request cache, so this costs one
# query per model rather than one per object.
return {
cf.name: self.custom_field_data.get(cf.name)
for cf in CustomField.objects.get_for_model(self)
}
@strawberry.type

View File

@ -10,7 +10,6 @@ from django.conf import settings
from django.core.validators import RegexValidator, ValidationError
from django.db import models, transaction
from django.db.models import F, Func, Value
from django.db.models.expressions import RawSQL
from django.urls import reverse
from django.utils.html import escape
from django.utils.safestring import mark_safe
@ -326,7 +325,7 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
return None
@staticmethod
def _update_object_data(model, **update_kwargs):
def _update_object_data(model, filters=None, **update_kwargs):
"""
Apply an UPDATE to the custom_field_data of every instance of the given model in batches,
bounding the number of rows touched by each statement. A single unbounded UPDATE across
@ -338,29 +337,41 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
a renamed field landing on only some objects) should the loop be interrupted when not
already running inside a request's transaction. Batching avoids the statement timeout
regardless, as that limit applies per statement rather than per transaction.
:param filters: Optional dict of ORM filters restricting which rows are updated. Callers
which need only to touch rows already holding a given key should pass
`{'custom_field_data__has_key': ...}`; because keys are materialized only when a value
is actually set (see populate_initial_data()), this typically excludes the bulk of the
table.
"""
filters = filters or {}
queryset = model.objects.filter(**filters)
with transaction.atomic():
last_pk = 0
while True:
pks = list(
model.objects.filter(pk__gt=last_pk).order_by('pk')
queryset.filter(pk__gt=last_pk).order_by('pk')
.values_list('pk', flat=True)[:CUSTOMFIELD_DATA_BATCH_SIZE]
)
if not pks:
break
model.objects.filter(pk__in=pks).update(**update_kwargs)
queryset.filter(pk__in=pks).update(**update_kwargs)
last_pk = pks[-1]
def populate_initial_data(self, content_types):
"""
Populate initial custom field data upon either a) the creation of a new CustomField, or
b) the assignment of an existing CustomField to new object types.
Only a non-null default is written. A field with no default has no value to record, and an
absent key is equivalent to a null one everywhere the data is read (see CustomFieldsMixin),
so materializing a JSON null on every object would be a very expensive no-op: on a large
table it can outlast the request. Objects without the key simply report no value until one
is assigned.
"""
if self.default is None:
# We have to convert None to a JSON null for jsonb_set()
value = RawSQL("'null'::jsonb", [])
else:
value = Value(self.default, models.JSONField())
return
value = Value(self.default, models.JSONField())
for ct in content_types:
if model := ct.model_class():
self._update_object_data(
@ -377,11 +388,16 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
"""
Delete custom field data which is no longer relevant (either because the CustomField is
no longer assigned to a model, or because it has been deleted).
Only objects which actually hold a value for the field are rewritten. Because keys are
materialized only when a value is set (see populate_initial_data()), this typically
excludes the bulk of the table.
"""
for ct in content_types:
if model := ct.model_class():
self._update_object_data(
model,
filters={'custom_field_data__has_key': self.name},
custom_field_data=F('custom_field_data') - self.name
)
@ -394,13 +410,15 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
if model := ct.model_class():
self._update_object_data(
model,
filters={'custom_field_data__has_key': old_name},
custom_field_data=Func(
F('custom_field_data') - old_name,
Value([new_name]),
Func(
F('custom_field_data'),
function='jsonb_extract_path_text',
template=f"to_jsonb(%(expressions)s -> '{old_name}')"
Value(old_name),
function='jsonb_extract_path',
output_field=models.JSONField()
),
function='jsonb_set')
)
@ -705,6 +723,9 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
:param lookup_expr: Custom lookup expression (optional)
"""
# Imported locally as extras.filters imports extras.models
from extras.filters import missing_key_aware_filter_factory
kwargs = {
'field_name': f'custom_field_data__{self.name}'
}
@ -770,6 +791,11 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo
else:
return None
# A negated lookup must match objects which carry no key for this field at all; see
# MissingKeyAwareFilterMixin. BooleanFilter is never negated, so it is left alone.
if not issubclass(filter_class, django_filters.BooleanFilter):
filter_class = missing_key_aware_filter_factory(filter_class)
filter_instance = filter_class(**kwargs)
filter_instance.custom_field = self

View File

@ -20,20 +20,33 @@ from .utils import run_validators
#
def handle_cf_added_obj_types(instance, action, pk_set, **kwargs):
def handle_cf_object_types_changed(instance, action, pk_set, reverse, **kwargs):
"""
Handle the population of default/null values when a CustomField is added to one or more ContentTypes.
Handle the stored data of a CustomField as it is assigned to or unassigned from object types.
Only the forward direction is handled: every action below operates on the CustomField, whereas
the reverse of this relation (ContentType.custom_fields) reports the ContentType as the sender's
instance. Nothing in NetBox assigns object types that way.
"""
if reverse or action not in ('pre_clear', 'post_add', 'post_remove'):
return
if action == 'pre_clear':
# clear() unassigns every object type at once. It must be handled before the fact: no
# pk_set is reported for a clear, so the assignments have to be read while they still
# exist. (Note that set() diffs via remove()/add() by default, so it does not land here.)
instance.remove_stale_data(instance.object_types.all())
return
object_types = ContentType.objects.filter(pk__in=pk_set)
if action == 'post_add':
instance.populate_initial_data(ContentType.objects.filter(pk__in=pk_set))
# Populate the field's default value (if any) on all existing objects
instance.populate_initial_data(object_types)
def handle_cf_removed_obj_types(instance, action, pk_set, **kwargs):
"""
Handle the cleanup of old custom field data when a CustomField is removed from one or more ContentTypes.
"""
if action == 'post_remove':
instance.remove_stale_data(ContentType.objects.filter(pk__in=pk_set))
else:
# Remove the field's stored data from objects to which it no longer applies
instance.remove_stale_data(object_types)
def handle_cf_renamed(instance, created, **kwargs):
@ -53,8 +66,7 @@ def handle_cf_deleted(instance, **kwargs):
post_save.connect(handle_cf_renamed, sender=CustomField)
pre_delete.connect(handle_cf_deleted, sender=CustomField)
m2m_changed.connect(handle_cf_added_obj_types, sender=CustomField.object_types.through)
m2m_changed.connect(handle_cf_removed_obj_types, sender=CustomField.object_types.through)
m2m_changed.connect(handle_cf_object_types_changed, sender=CustomField.object_types.through)
#

View File

@ -4,8 +4,12 @@ from collections import defaultdict
from decimal import Decimal
from unittest.mock import patch
import django_filters
from django.core.exceptions import ValidationError
from django.db import connection
from django.db.models import QuerySet
from django.test import tag
from django.test.utils import CaptureQueriesContext
from django.urls import reverse
from rest_framework import status
@ -13,11 +17,14 @@ from core.models import ObjectChange, ObjectType
from dcim.filtersets import SiteFilterSet
from dcim.forms import SiteImportForm
from dcim.models import Manufacturer, Rack, Site
from dcim.tables import SiteTable
from extras.choices import *
from extras.filters import MissingKeyAwareFilterMixin, missing_key_aware_filter_factory
from extras.models import CustomField, CustomFieldChoiceSet
from ipam.models import VLAN
from netbox.choices import CSVDelimiterChoices, ImportFormatChoices
from netbox.context import query_cache
from utilities.filters import MultiValueCharFilter, MultiValueMACAddressFilter
from utilities.testing import APITestCase, TestCase
from virtualization.models import VirtualMachine
@ -49,7 +56,7 @@ class CustomFieldTestCase(TestCase):
def test_text_field(self):
value = 'Foobar!'
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='text_field',
type=CustomFieldTypeChoices.TYPE_TEXT,
@ -57,7 +64,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -74,7 +81,7 @@ class CustomFieldTestCase(TestCase):
def test_longtext_field(self):
value = 'A' * 256
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='longtext_field',
type=CustomFieldTypeChoices.TYPE_LONGTEXT,
@ -82,7 +89,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -98,7 +105,7 @@ class CustomFieldTestCase(TestCase):
def test_integer_field(self):
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='integer_field',
type=CustomFieldTypeChoices.TYPE_INTEGER,
@ -106,7 +113,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
for value in (123456, 0, -123456):
@ -124,7 +131,7 @@ class CustomFieldTestCase(TestCase):
def test_decimal_field(self):
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='decimal_field',
type=CustomFieldTypeChoices.TYPE_DECIMAL,
@ -132,7 +139,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
for value in (123456.54, 0, -123456.78):
@ -150,7 +157,7 @@ class CustomFieldTestCase(TestCase):
def test_boolean_field(self):
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='boolean_field',
type=CustomFieldTypeChoices.TYPE_INTEGER,
@ -158,7 +165,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
for value in (True, False):
@ -177,7 +184,7 @@ class CustomFieldTestCase(TestCase):
def test_date_field(self):
value = datetime.date(2016, 6, 23)
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='date_field',
type=CustomFieldTypeChoices.TYPE_DATE,
@ -185,7 +192,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = cf.serialize(value)
@ -202,7 +209,7 @@ class CustomFieldTestCase(TestCase):
def test_datetime_field(self):
value = datetime.datetime(2016, 6, 23, 9, 45, 0)
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='date_field',
type=CustomFieldTypeChoices.TYPE_DATETIME,
@ -210,7 +217,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = cf.serialize(value)
@ -227,7 +234,7 @@ class CustomFieldTestCase(TestCase):
def test_url_field(self):
value = 'http://example.com/'
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='url_field',
type=CustomFieldTypeChoices.TYPE_URL,
@ -235,7 +242,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -252,7 +259,7 @@ class CustomFieldTestCase(TestCase):
def test_json_field(self):
value = '{"foo": 1, "bar": 2}'
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='json_field',
type=CustomFieldTypeChoices.TYPE_JSON,
@ -260,7 +267,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -342,7 +349,7 @@ class CustomFieldTestCase(TestCase):
extra_choices=CHOICES
)
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='select_field',
type=CustomFieldTypeChoices.TYPE_SELECT,
@ -351,7 +358,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -379,7 +386,7 @@ class CustomFieldTestCase(TestCase):
extra_choices=CHOICES
)
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='multiselect_field',
type=CustomFieldTypeChoices.TYPE_MULTISELECT,
@ -388,7 +395,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -549,7 +556,7 @@ class CustomFieldTestCase(TestCase):
def test_object_field(self):
value = VLAN.objects.create(name='VLAN 1', vid=1).pk
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='object_field',
type=CustomFieldTypeChoices.TYPE_OBJECT,
@ -558,7 +565,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -581,7 +588,7 @@ class CustomFieldTestCase(TestCase):
VLAN.objects.bulk_create(vlans)
value = [vlan.pk for vlan in vlans]
# Create a custom field & check that initial value is null
# Create a custom field & check that no initial data is written
cf = CustomField.objects.create(
name='object_field',
type=CustomFieldTypeChoices.TYPE_MULTIOBJECT,
@ -590,7 +597,7 @@ class CustomFieldTestCase(TestCase):
)
cf.object_types.set([self.object_type])
instance = Site.objects.first()
self.assertIsNone(instance.custom_field_data[cf.name])
self.assertNotIn(cf.name, instance.custom_field_data)
# Assign a value and check that it is saved
instance.custom_field_data[cf.name] = value
@ -665,13 +672,302 @@ class CustomFieldTestCase(TestCase):
0
)
# Removal: the key is stripped from every existing object when the field is deleted
# Removal: deleting the field strips the key from every existing object
cf.delete()
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='renamed_field').count(),
0
)
def test_provisioning_writes_nothing_without_a_default(self):
"""
A field with no default has no value to record, so creating one must not touch any object.
"""
cf = CustomField.objects.create(
name='unset_field',
type=CustomFieldTypeChoices.TYPE_TEXT
)
with CaptureQueriesContext(connection) as queries:
cf.object_types.set([self.object_type])
# No object data is written at all -- the cost of adding a field no longer scales with the
# number of objects it applies to
self.assertFalse([
q['sql'] for q in queries.captured_queries
if q['sql'].lstrip().upper().startswith('UPDATE "DCIM_SITE"'.upper())
])
self.assertEqual(Site.objects.filter(custom_field_data__has_key='unset_field').count(), 0)
for site in Site.objects.all():
self.assertEqual(site.custom_field_data, {})
self.assertIsNone(site.cf['unset_field'])
def test_provisioning_applies_a_default_immediately(self):
"""
A default value, by contrast, must be recorded on every existing object as soon as the
field is created -- it has to be filterable straight away, so it cannot be deferred.
"""
cf = CustomField.objects.create(
name='defaulted_field',
type=CustomFieldTypeChoices.TYPE_TEXT,
default='bar'
)
cf.object_types.set([self.object_type])
self.assertEqual(
Site.objects.filter(custom_field_data__defaulted_field='bar').count(),
Site.objects.count()
)
def test_rename_touches_only_objects_holding_a_value(self):
"""
Renaming rewrites the key only where a value is actually stored. This is what keeps a
rename cheap now that objects are no longer provisioned with a placeholder each.
"""
cf = CustomField.objects.create(
name='sparse_field',
type=CustomFieldTypeChoices.TYPE_TEXT
)
cf.object_types.set([self.object_type])
site = Site.objects.first()
site.custom_field_data['sparse_field'] = 'value'
site.save()
cf.name = 'sparse_renamed'
cf.save()
self.assertEqual(
list(
Site.objects.filter(custom_field_data__has_key='sparse_renamed')
.values_list('pk', flat=True)
),
[site.pk]
)
self.assertEqual(Site.objects.filter(custom_field_data__has_key='sparse_field').count(), 0)
site.refresh_from_db()
self.assertEqual(site.custom_field_data['sparse_renamed'], 'value')
def test_removal_from_object_type_purges_data(self):
"""
Unassigning a field from an object type removes its data from those objects.
"""
cf = CustomField.objects.create(
name='unassigned_field',
type=CustomFieldTypeChoices.TYPE_TEXT,
default='baz'
)
cf.object_types.set([self.object_type])
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='unassigned_field').count(),
Site.objects.count()
)
cf.object_types.remove(self.object_type)
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='unassigned_field').count(),
0
)
def test_clearing_object_types_purges_data(self):
"""
clear() unassigns every object type at once and reports no pk_set, so it must be handled
before the fact. Its data is removed just as remove()'s is.
"""
cf = CustomField.objects.create(
name='cleared_field',
type=CustomFieldTypeChoices.TYPE_TEXT,
default='baz'
)
cf.object_types.set([self.object_type])
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='cleared_field').count(),
Site.objects.count()
)
cf.object_types.clear()
self.assertEqual(
Site.objects.filter(custom_field_data__has_key='cleared_field').count(),
0
)
def test_batch_update_excludes_rows_which_no_longer_match(self):
"""
A caller's filters must constrain the UPDATE as well as the selection of each batch.
rename_object_data() builds a jsonb_set() expression which evaluates to NULL for a row not
holding the key being renamed, so a row which loses it between the two statements would
otherwise have its entire custom_field_data column nulled out.
"""
cf = CustomField.objects.create(
name='drifting_field',
type=CustomFieldTypeChoices.TYPE_TEXT
)
cf.object_types.set([self.object_type])
sites = list(Site.objects.order_by('pk'))
holder, bystander = sites[0], sites[-1]
Site.objects.filter(pk=holder.pk).update(custom_field_data={'drifting_field': 'value'})
Site.objects.filter(pk=bystander.pk).update(custom_field_data={'other': 'untouched'})
# Simulate a concurrent write: the batch selection yields a pk which no longer satisfies
# the has_key filter by the time the UPDATE is issued.
select_pks = QuerySet.values_list
injected = []
def inject_stale_pk(self, *args, **kwargs):
result = select_pks(self, *args, **kwargs)
if self.model is Site and args == ('pk',) and kwargs.get('flat') and not injected:
injected.append(bystander.pk)
return [*result, bystander.pk]
return result
with patch.object(QuerySet, 'values_list', inject_stale_pk):
cf.name = 'drifted_field'
cf.save()
self.assertEqual(injected, [bystander.pk], "the stale pk was never injected")
# The renamed value landed, and the bystander was left entirely alone
holder.refresh_from_db()
self.assertEqual(holder.custom_field_data, {'drifted_field': 'value'})
bystander.refresh_from_db()
self.assertEqual(bystander.custom_field_data, {'other': 'untouched'})
@staticmethod
def order_sites_by(*aliases):
"""
Order a SiteTable by the given column aliases and return the underlying QuerySet.
"""
table = SiteTable(Site.objects.all())
table.order_by = aliases
return table.data.data
def test_table_ordering_groups_objects_with_no_value(self):
"""
Objects holding no value sort together regardless of whether they store a JSON null or
carry no key at all, and numeric fields still sort numerically rather than lexically.
"""
cf = CustomField.objects.create(
name='sort_field',
type=CustomFieldTypeChoices.TYPE_INTEGER
)
cf.object_types.set([self.object_type])
sites = list(Site.objects.order_by('name'))
# Site A holds a value, Site B an explicit null, Site C no key whatsoever
Site.objects.filter(pk=sites[0].pk).update(custom_field_data={'sort_field': 20})
Site.objects.filter(pk=sites[1].pk).update(custom_field_data={'sort_field': None})
Site.objects.filter(pk=sites[2].pk).update(custom_field_data={})
extra = Site.objects.create(
name='Site D', slug='site-d', custom_field_data={'sort_field': 100}
)
ordered = self.order_sites_by('cf_sort_field')
self.assertEqual(
[s.pk for s in ordered][:2],
[sites[0].pk, extra.pk],
"20 must sort before 100 (numerically, not lexically) ahead of the empty rows"
)
self.assertEqual(
{s.pk for s in ordered[2:]},
{sites[1].pk, sites[2].pk},
"the JSON-null and missing-key rows must group together at the end"
)
# Reversing the ordering carries the empty rows to the front, as it would SQL nulls
ordered = self.order_sites_by('-cf_sort_field')
self.assertEqual(
{s.pk for s in ordered[:2]},
{sites[1].pk, sites[2].pk},
"the JSON-null and missing-key rows must still group together"
)
self.assertEqual([s.pk for s in ordered[2:]], [extra.pk, sites[0].pk])
def test_table_ordering_breaks_ties_by_primary_key(self):
"""
Rows tying on the sort value -- every object holding no value ties on both sort keys --
must still be totally ordered, or paginated results may skip or repeat rows between
page requests.
"""
cf = CustomField.objects.create(
name='sort_field',
type=CustomFieldTypeChoices.TYPE_INTEGER
)
cf.object_types.set([self.object_type])
# None of these hold a value for the field, so all of them tie
Site.objects.bulk_create([
Site(name=f'Tied Site {i}', slug=f'tied-site-{i}') for i in range(1, 11)
])
for alias in ('cf_sort_field', '-cf_sort_field'):
ordered = self.order_sites_by(alias)
self.assertEqual(
ordered.query.order_by[-1],
'pk',
"the primary key must be applied as the final sort key"
)
# Paging through the results must yield each object exactly once
expected = [site.pk for site in ordered]
paginated = []
for offset in range(0, len(expected), 4):
paginated.extend(site.pk for site in ordered[offset:offset + 4])
self.assertEqual(paginated, expected)
def test_table_ordering_composes_with_other_columns(self):
"""
A custom field column must contribute its sort keys to a multi-column ordering rather than
replace it. (The sort parameter is read with getlist(), and a saved TableConfig records an
ordering of arbitrary length.)
"""
cf = CustomField.objects.create(
name='sort_field',
type=CustomFieldTypeChoices.TYPE_INTEGER
)
cf.object_types.set([self.object_type])
sites = list(Site.objects.order_by('name'))
# Ordering by the custom field alone would reverse the first two sites
Site.objects.filter(pk=sites[0].pk).update(custom_field_data={'sort_field': 2})
Site.objects.filter(pk=sites[1].pk).update(custom_field_data={'sort_field': 1})
Site.objects.filter(pk=sites[2].pk).update(custom_field_data={'sort_field': 3})
ordered = self.order_sites_by('name', 'cf_sort_field')
self.assertEqual(
[site.pk for site in ordered],
[site.pk for site in sites],
"the preceding sort key must survive the addition of a custom field column"
)
# A column named after the custom field column must likewise still apply
Site.objects.update(custom_field_data={'sort_field': 1})
ordered = self.order_sites_by('cf_sort_field', '-name')
self.assertEqual(
[site.pk for site in ordered],
[site.pk for site in reversed(sites)],
"the trailing sort key must be applied before the primary key tie breaker"
)
def test_table_ordering_tolerates_a_repeated_sort_alias(self):
"""
The sort parameter is read with getlist(), so the same custom field column can appear in
the ordering more than once, applying the same annotation to the queryset twice.
"""
cf = CustomField.objects.create(
name='sort_field',
type=CustomFieldTypeChoices.TYPE_INTEGER
)
cf.object_types.set([self.object_type])
table = SiteTable(Site.objects.all())
table.order_by = ['cf_sort_field', '-cf_sort_field']
self.assertEqual(len(list(table.rows)), Site.objects.count())
def test_default_value_validation(self):
choiceset = CustomFieldChoiceSet.objects.create(
name="Test Choice Set",
@ -1823,6 +2119,77 @@ class CustomFieldModelTestCase(TestCase):
site.custom_field_data['baz'] = 'def'
site.clean()
def test_required_field_enforced_on_existing_objects(self):
"""
Adding a required custom field invalidates the objects which already exist, whether they
carry no key for it -- the normal state now that empty values are not provisioned -- or an
explicit null. Both are rejected, as they were before: every object then held a materialized
null, which CustomField.validate() rejects for a required field.
"""
site = Site.objects.create(name='Test Site', slug='test-site')
cf = CustomField(type=CustomFieldTypeChoices.TYPE_TEXT, name='req', required=True)
cf.save()
cf.object_types.set([ObjectType.objects.get_for_model(Site)])
# No value was provisioned onto the existing object
site.refresh_from_db()
self.assertNotIn('req', site.custom_field_data)
with self.assertRaises(ValidationError):
site.clean()
# An explicit null is rejected identically
site.custom_field_data['req'] = None
with self.assertRaises(ValidationError):
site.clean()
site.custom_field_data['req'] = 'value'
site.clean()
class MissingKeyAwareFilterTestCase(TestCase):
"""
MissingKeyAwareFilterMixin reimplements MultipleChoiceFilter.filter() for the negated case, so
it may only be mixed into a class which inherits that method unmodified and which does not
filter conjoined. Both constraints are enforced, as violating either would yield a wrong result
set rather than an error.
"""
def test_factory_rejects_a_class_which_defines_filter(self):
# MultiValueMACAddressFilter overrides filter() to swallow ValidationError
with self.assertRaises(TypeError):
missing_key_aware_filter_factory(MultiValueMACAddressFilter)
# BooleanFilter does not inherit MultipleChoiceFilter.filter() at all
with self.assertRaises(TypeError):
missing_key_aware_filter_factory(django_filters.BooleanFilter)
def test_factory_accepts_a_class_which_inherits_filter(self):
filter_class = missing_key_aware_filter_factory(MultiValueCharFilter)
self.assertTrue(issubclass(filter_class, MissingKeyAwareFilterMixin))
self.assertTrue(issubclass(filter_class, MultiValueCharFilter))
# The factory is cached, so a class yields a single stable subclass
self.assertIs(filter_class, missing_key_aware_filter_factory(MultiValueCharFilter))
def test_conjoined_filtering_is_rejected(self):
filter_class = missing_key_aware_filter_factory(MultiValueCharFilter)
filter_class(field_name='custom_field_data__foo')
filter_class(field_name='custom_field_data__foo', conjoined=False)
with self.assertRaises(TypeError):
filter_class(field_name='custom_field_data__foo', conjoined=True)
def test_every_supported_custom_field_type_satisfies_the_constraints(self):
"""
The filter classes CustomField.to_filter() selects must all remain admissible.
"""
for cf_type in CustomFieldTypeChoices.values():
with self.subTest(cf_type):
cf = CustomField(name='test', type=cf_type)
# Raises TypeError if the selected filter class violates a constraint
cf.to_filter()
cf.to_filter(lookup_expr='empty')
class CustomFieldModelFilterTestCase(TestCase):
queryset = Site.objects.all()
@ -1979,12 +2346,14 @@ class CustomFieldModelFilterTestCase(TestCase):
'cf11': manufacturers[2].pk,
'cf12': [manufacturers[2].pk, manufacturers[3].pk],
}),
# Carries no custom field data at all. Negated lookups ("is not x") match it, as they
# do an object holding an explicit null; see MissingKeyAwareFilterMixin.
Site(name='Site 4', slug='site-4'),
])
def test_filter_integer(self):
self.assertEqual(self.filterset({'cf_cf1': [100, 200]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf1__n': [200]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf1__n': [200]}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf1__gt': [200]}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf1__gte': [200]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf1__lt': [200]}, self.queryset).qs.count(), 1)
@ -1993,7 +2362,7 @@ class CustomFieldModelFilterTestCase(TestCase):
def test_filter_decimal(self):
self.assertEqual(self.filterset({'cf_cf2': [100.1, 200.2]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf2__n': [200.2]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf2__n': [200.2]}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf2__gt': [200.2]}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf2__gte': [200.2]}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf2__lt': [200.2]}, self.queryset).qs.count(), 1)
@ -2006,15 +2375,15 @@ class CustomFieldModelFilterTestCase(TestCase):
def test_filter_text_strict(self):
self.assertEqual(self.filterset({'cf_cf4': ['foo']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__n': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__n': ['foo']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf4__ic': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__nic': ['foo']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__nic': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__isw': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__nisw': ['foo']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__nisw': ['foo']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__iew': ['bar']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__niew': ['bar']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__niew': ['bar']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__ie': ['FOO']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf4__nie': ['FOO']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf4__nie': ['FOO']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf4__empty': True}, self.queryset).qs.count(), 1)
def test_filter_text_loose(self):
@ -2022,7 +2391,7 @@ class CustomFieldModelFilterTestCase(TestCase):
def test_filter_date(self):
self.assertEqual(self.filterset({'cf_cf6': ['2016-06-26', '2016-06-27']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf6__n': ['2016-06-27']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf6__n': ['2016-06-27']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf6__gt': ['2016-06-27']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf6__gte': ['2016-06-27']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf6__lt': ['2016-06-27']}, self.queryset).qs.count(), 1)
@ -2034,20 +2403,108 @@ class CustomFieldModelFilterTestCase(TestCase):
self.filterset({'cf_cf7': ['http://a.example.com', 'http://b.example.com']}, self.queryset).qs.count(),
2
)
self.assertEqual(self.filterset({'cf_cf7__n': ['http://b.example.com']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf7__n': ['http://b.example.com']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__ic': ['b']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf7__nic': ['b']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf7__nic': ['b']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__isw': ['http://']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__nisw': ['http://']}, self.queryset).qs.count(), 0)
self.assertEqual(self.filterset({'cf_cf7__nisw': ['http://']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf7__iew': ['.com']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__niew': ['.com']}, self.queryset).qs.count(), 0)
self.assertEqual(self.filterset({'cf_cf7__niew': ['.com']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf7__ie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf7__nie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf7__nie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 3)
self.assertEqual(self.filterset({'cf_cf7__empty': True}, self.queryset).qs.count(), 1)
def test_filter_url_loose(self):
self.assertEqual(self.filterset({'cf_cf8': ['example.com']}, self.queryset).qs.count(), 3)
def test_filter_negation_matches_unset_values(self):
"""
A negated lookup must match an object which holds no value for the field, whether that is
recorded as an explicit null or by the absence of the key; see MissingKeyAwareFilterMixin.
"""
no_key = Site.objects.get(slug='site-4')
explicit_null = Site.objects.create(name='Site 5', slug='site-5', custom_field_data={
'cf1': None,
'cf4': None,
'cf6': None,
'cf7': None,
})
for filter_name, value in (
('cf_cf1__n', 100),
('cf_cf4__n', 'foo'),
('cf_cf4__nic', 'foo'),
('cf_cf4__nisw', 'foo'),
('cf_cf4__niew', 'bar'),
('cf_cf4__nie', 'FOO'),
('cf_cf6__n', '2016-06-26'),
('cf_cf7__n', 'http://a.example.com'),
('cf_cf7__nic', 'a'),
('cf_cf7__nisw', 'http://'),
('cf_cf7__niew', '.com'),
):
with self.subTest(filter_name):
pks = set(
self.filterset({filter_name: [value]}, self.queryset).qs.values_list('pk', flat=True)
)
self.assertIn(no_key.pk, pks, "an object carrying no key must match")
self.assertIn(explicit_null.pk, pks, "an object holding a null must match")
def test_filter_null_sentinel_matches_unset_values(self):
"""
The null sentinel (FILTERS_NULL_CHOICE_VALUE) asks for the objects holding no value, which
must include those carrying no key as well as those holding an explicit null. Negating it
must therefore return exactly the objects which do hold a value -- and in particular must
not return the ones it is being asked to exclude.
Only string-backed field types are exercised: a numeric or date field rejects 'null' during
form validation ("Enter a whole number"), so the sentinel never reaches the filter at all.
That is a property of multivalue_field_factory() and is unaffected by this behavior.
"""
no_key = Site.objects.get(slug='site-4')
explicit_null = Site.objects.create(name='Site 5', slug='site-5', custom_field_data={
'cf4': None,
'cf7': None,
'cf9': None,
})
has_value = set(
Site.objects.filter(slug__in=('site-1', 'site-2', 'site-3')).values_list('pk', flat=True)
)
for filter_name in ('cf_cf4', 'cf_cf7', 'cf_cf9'):
with self.subTest(filter_name):
pks = set(
self.filterset({filter_name: ['null']}, self.queryset).qs.values_list('pk', flat=True)
)
self.assertEqual(pks, {no_key.pk, explicit_null.pk})
pks = set(
self.filterset({f'{filter_name}__n': ['null']}, self.queryset)
.qs.values_list('pk', flat=True)
)
self.assertEqual(pks, has_value)
def test_filter_null_sentinel_combined_with_a_value(self):
"""
The sentinel may be passed alongside real values, in which case it widens the match rather
than replacing it. Under negation the valueless objects are then excluded, as they are among
the values being negated.
"""
no_key = Site.objects.get(slug='site-4')
site_1 = Site.objects.get(slug='site-1')
pks = set(
self.filterset({'cf_cf4': ['foo', 'null']}, self.queryset).qs.values_list('pk', flat=True)
)
self.assertIn(site_1.pk, pks, "an object holding the value must match")
self.assertIn(no_key.pk, pks, "an object holding no value must match")
pks = set(
self.filterset({'cf_cf4__n': ['foo', 'null']}, self.queryset).qs.values_list('pk', flat=True)
)
self.assertNotIn(site_1.pk, pks, "an object holding the value must be excluded")
self.assertNotIn(no_key.pk, pks, "an object holding no value must be excluded")
def test_filter_select(self):
self.assertEqual(self.filterset({'cf_cf9': ['A', 'B']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf9__empty': True}, self.queryset).qs.count(), 1)
@ -2055,7 +2512,8 @@ class CustomFieldModelFilterTestCase(TestCase):
def test_filter_multiselect(self):
self.assertEqual(self.filterset({'cf_cf10': ['A']}, self.queryset).qs.count(), 1)
self.assertEqual(self.filterset({'cf_cf10': ['A', 'C']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf10': ['null']}, self.queryset).qs.count(), 1) # Contains a literal null
# Matches both the object holding a literal null and the one carrying no key, as `empty` does
self.assertEqual(self.filterset({'cf_cf10': ['null']}, self.queryset).qs.count(), 2)
self.assertEqual(self.filterset({'cf_cf10__empty': True}, self.queryset).qs.count(), 2)
def test_filter_object(self):

View File

@ -81,8 +81,8 @@ class CustomFieldDeletedSignalTestCase(TestCase):
class CustomFieldObjectTypeSignalTestCase(TestCase):
"""
Verify extras.signals.handle_cf_added_obj_types and handle_cf_removed_obj_types
populate or strip default values when a CustomField's object_types m2m changes.
Verify extras.signals.handle_cf_object_types_changed populates or strips default values when a
CustomField's object_types m2m changes.
"""
def test_adding_object_type_populates_default_value(self):

View File

@ -6,7 +6,7 @@ import django_tables2 as tables
from django.conf import settings
from django.contrib.auth.context_processors import auth
from django.contrib.auth.models import AnonymousUser
from django.db.models import DateField, DateTimeField
from django.db.models import DateField, DateTimeField, Q
from django.template import Context, Template
from django.urls import reverse
from django.utils.dateparse import parse_date
@ -522,9 +522,46 @@ class CustomFieldColumn(tables.Column):
CustomFieldTypeChoices.TYPE_MULTIOBJECT
):
kwargs['orderable'] = False
else:
kwargs.setdefault('order_by', (
self.unset_alias,
f'custom_field_data__{customfield.name}',
))
super().__init__(*args, **kwargs)
@property
def unset_alias(self):
"""
Return the name of the annotation which groups together the objects holding no value for
this field (see get_ordering_annotation()).
The annotation is named for the custom field so that ordering by two custom field columns
cannot produce a duplicate alias. Field names are validated to contain only alphanumerics
and underscores, so the alias is always a legal identifier.
"""
return f'_cf_{self.customfield.name}_unset'
def get_ordering_annotation(self):
"""
Return the annotation by which objects holding no value for this field are sorted together,
as the leading sort key for the column. (BaseTable applies it to the queryset when ordering
by this column.)
An object can lack a value either by storing a JSON null or by carrying no key for the
field at all -- the latter being the normal state for objects which predate it, as data is
no longer provisioned onto existing objects (see CustomField.populate_initial_data()).
Postgres sorts those two apart: a JSON null is the lowest jsonb value, whereas a missing
key yields SQL NULL and sorts last, so the "empty" rows would otherwise land at both ends
of the same column. This key (the `empty` lookup covers both states) groups them at one
end, matching how SQL NULLs are ordered for an ordinary column: last when ascending, first
when descending. The column's second sort key then orders by the raw value, so that numeric
and date fields still sort by type rather than lexically.
"""
return {
self.unset_alias: Q(**{f'custom_field_data__{self.customfield.name}__empty': True})
}
@staticmethod
def _linkify_item(item):
if hasattr(item, 'get_absolute_url'):

View File

@ -12,6 +12,7 @@ from django.urls.exceptions import NoReverseMatch
from django.utils.safestring import mark_safe
from django.utils.translation import gettext_lazy as _
from django_tables2.data import TableQuerysetData
from django_tables2.utils import OrderBy
from core.models import ObjectType
from extras.choices import *
@ -158,6 +159,68 @@ class BaseTable(tables.Table):
prefetch_fields.append('__'.join(prefetch_path))
self.data.data = self.data.data.prefetch_related(*prefetch_fields)
def _get_custom_field_ordering_columns(self, order_by):
"""
Return the custom field columns among those named by the given ordering.
Args:
order_by: An iterable (or comma-separated string) of order by aliases.
"""
order_by = order_by.split(',') if isinstance(order_by, str) else order_by or ()
ordering_columns = []
for alias in order_by:
name = OrderBy(alias).bare
# Ignore any aliases which django-tables2 will itself discard
if name not in self.columns or not self.columns[name].orderable:
continue
if isinstance(column := self.columns[name].column, columns.CustomFieldColumn):
ordering_columns.append(column)
return ordering_columns
def _apply_ordering_annotations(self, ordering_columns):
"""
Dynamically annotate the table's QuerySet with the expressions needed to sort by the given
custom field columns. These are applied only for the columns actually being ordered by, to
avoid burdening every query with expressions it has no use for.
"""
annotations = {}
for column in ordering_columns:
annotations.update(column.get_ordering_annotation())
# Skip any annotations already applied, as when the ordering is set more than once
if annotations := {
name: expr for name, expr in annotations.items()
if name not in self.data.data.query.annotations
}:
self.data.data = self.data.data.annotate(**annotations)
def _apply_ordering_tie_breaker(self):
"""
Append the primary key to the table's ordering as a final sort key, so that the ordering is
total. Rows tying on every preceding key -- and every object holding no value for a custom
field ties on both of that column's keys -- are otherwise free to come back in a different
order for each query, which would cause paginated results to skip or repeat rows from one
page to the next.
"""
ordering = self.data.data.query.order_by
if ordering and not any(OrderBy(o).bare in ('pk', 'id') for o in ordering):
self.data.data = self.data.data.order_by(*ordering, 'pk')
@tables.Table.order_by.setter
def order_by(self, value):
"""
Extend the ordering of the table's data with the support needed by custom field columns.
"""
if not isinstance(self.data, TableQuerysetData):
tables.Table.order_by.fset(self, value)
return
if ordering_columns := self._get_custom_field_ordering_columns(value):
self._apply_ordering_annotations(ordering_columns)
tables.Table.order_by.fset(self, value)
if ordering_columns:
self._apply_ordering_tie_breaker()
def configure(self, request):
"""
Configure the table for a specific request context. This performs pagination and records

View File

@ -419,6 +419,31 @@ class GraphQLAPITestCase(APITestCase):
self.assertNotIn('errors', data)
self.assertEqual(int(data['data']['table_config']['object_type']['id']), site_ct.pk)
def test_graphql_custom_fields_include_unset_fields(self):
"""
CustomFieldsMixin.custom_fields must emit a key for every custom field assigned to the model,
as the REST API does, rather than returning the stored data verbatim. A key is materialized
only once a value is assigned, so an object predating a field carries none; without this such
a field would be absent from the response instead of null. Stale data for a field which no
longer applies is likewise omitted.
"""
self.add_permissions('dcim.view_site')
url = reverse('graphql')
cf = CustomField.objects.create(name='cf1', type=CustomFieldTypeChoices.TYPE_TEXT)
cf.object_types.set([ObjectType.objects.get_for_model(Site)])
site = Site.objects.get(slug='site-1')
self.assertNotIn('cf1', site.custom_field_data)
Site.objects.filter(pk=site.pk).update(custom_field_data={'stale': 'value'})
query = '{ site(id: ' + str(site.pk) + ') { custom_fields } }'
response = self.client.post(url, data={'query': query}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK)
data = json.loads(response.content)
self.assertNotIn('errors', data)
self.assertEqual(data['data']['site']['custom_fields'], {'cf1': None})
@override_settings(LOGIN_REQUIRED=True)
def test_graphql_device_list_tags_are_prefetched(self):
"""