fix(dcim): Persist normalized cable length on partial saves
Cable.save() recomputed _abs_length in memory but never added it to update_fields, so a save naming length or length_unit left the stored normalized value stale. Derive it from the values the row will hold after the save and persist it alongside its source fields. Fixes #23096
This commit is contained in:
Martin Hauser
parent
7b56158d47
commit
90675dbbab
|
|
@ -9,7 +9,7 @@ from django.contrib.postgres.fields import ArrayField
|
|||
from django.contrib.postgres.indexes import GinIndex
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.core.validators import MaxValueValidator, MinValueValidator
|
||||
from django.db import models
|
||||
from django.db import models, router
|
||||
from django.dispatch import Signal
|
||||
from django.urls import reverse
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
|
|
@ -328,15 +328,32 @@ class Cable(PrimaryModel):
|
|||
}
|
||||
update_fields = normalize_update_fields(save_kwargs)
|
||||
|
||||
# Store the given length (if any) in meters for use in database ordering
|
||||
if self.length is not None and self.length_unit:
|
||||
self._abs_length = to_meters(self.length, self.length_unit)
|
||||
else:
|
||||
self._abs_length = None
|
||||
length_written = update_fields is None or 'length' in update_fields
|
||||
length_unit_written = update_fields is None or 'length_unit' in update_fields
|
||||
|
||||
# Clear length_unit if no length is defined
|
||||
if self.length is None:
|
||||
self.length_unit = None
|
||||
if length_written or length_unit_written:
|
||||
if length_written and length_unit_written:
|
||||
stored = {}
|
||||
else:
|
||||
# Read from the database this save will write, so a router cannot split the two
|
||||
db = using or router.db_for_write(Cable, instance=self)
|
||||
stored = Cable.objects.using(db).filter(pk=self.pk).values('length', 'length_unit').first() or {}
|
||||
length = self.length if length_written else stored.get('length')
|
||||
length_unit = self.length_unit if length_unit_written else stored.get('length_unit')
|
||||
|
||||
# Clear length_unit if no length is defined
|
||||
if length is None and length_unit_written:
|
||||
self.length_unit = None
|
||||
|
||||
# Store the given length (if any) in meters for use in database ordering
|
||||
if length is not None and length_unit:
|
||||
self._abs_length = to_meters(length, length_unit)
|
||||
else:
|
||||
self._abs_length = None
|
||||
|
||||
# _abs_length is a denormalized cache of length and length_unit, so persist them together
|
||||
if update_fields is not None:
|
||||
save_kwargs['update_fields'] = update_fields | {'_abs_length'}
|
||||
|
||||
# A field counts as changed only when this save actually writes it
|
||||
status_written = update_fields is None or 'status' in update_fields
|
||||
|
|
|
|||
|
|
@ -1,9 +1,11 @@
|
|||
from decimal import Decimal
|
||||
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.db import connection
|
||||
from django.db.models import ProtectedError
|
||||
from django.db.models.signals import post_save
|
||||
from django.test import TestCase, tag
|
||||
from django.test.utils import CaptureQueriesContext
|
||||
|
||||
from circuits.models import *
|
||||
from core.models import ObjectType
|
||||
|
|
@ -2700,6 +2702,213 @@ class CableTestCase(TestCase):
|
|||
|
||||
self.assertEqual(cable._abs_length, Decimal('1609343983.9066'))
|
||||
|
||||
def test_partial_save_persists_normalized_length(self):
|
||||
"""
|
||||
A save naming only length must persist the normalized length alongside it.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('1')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
cable.length = Decimal('2')
|
||||
cable.save(update_fields=['length'])
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertEqual(cable.length, Decimal('2.00'))
|
||||
self.assertEqual(cable.length_unit, CableLengthUnitChoices.UNIT_METER)
|
||||
self.assertEqual(cable._abs_length, Decimal('2.0000'))
|
||||
|
||||
def test_partial_save_persists_normalized_length_for_a_unit_change(self):
|
||||
"""
|
||||
A save naming only length_unit must renormalize against the stored length.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('10')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_FOOT
|
||||
cable.save(update_fields=['length_unit'])
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertEqual(cable.length, Decimal('10.00'))
|
||||
self.assertEqual(cable.length_unit, CableLengthUnitChoices.UNIT_FOOT)
|
||||
self.assertEqual(cable._abs_length, Decimal('3.0480'))
|
||||
|
||||
def test_partial_save_persists_normalized_length_for_both_source_fields(self):
|
||||
"""
|
||||
A save naming both source fields must normalize from the values being written.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('2')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_KILOMETER
|
||||
cable.save(update_fields=['length', 'length_unit'])
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertEqual(cable._abs_length, Decimal('2000.0000'))
|
||||
|
||||
def test_partial_save_normalizes_against_an_unwritten_length_unit(self):
|
||||
"""
|
||||
A save naming only length must normalize against the stored unit, not an unwritten one.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('1')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
cable.length = Decimal('2')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_KILOMETER
|
||||
cable.save(update_fields=['length'])
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertEqual(cable.length_unit, CableLengthUnitChoices.UNIT_METER)
|
||||
self.assertEqual(cable._abs_length, Decimal('2.0000'))
|
||||
|
||||
def test_partial_save_normalizes_against_an_unwritten_length(self):
|
||||
"""
|
||||
A save naming only length_unit must normalize against the stored length, not an unwritten one.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('1')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
cable.length = Decimal('2')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_CENTIMETER
|
||||
cable.save(update_fields=['length_unit'])
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertEqual(cable.length, Decimal('1.00'))
|
||||
self.assertEqual(cable.length_unit, CableLengthUnitChoices.UNIT_CENTIMETER)
|
||||
self.assertEqual(cable._abs_length, Decimal('0.0100'))
|
||||
|
||||
def test_partial_save_normalizes_against_an_unwritten_cleared_length(self):
|
||||
"""
|
||||
A save naming only length_unit must keep the unit when the excluded length is cleared in memory.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('1')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
cable.length = None
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_CENTIMETER
|
||||
cable.save(update_fields=['length_unit'])
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertEqual(cable.length, Decimal('1.00'))
|
||||
self.assertEqual(cable.length_unit, CableLengthUnitChoices.UNIT_CENTIMETER)
|
||||
self.assertEqual(cable._abs_length, Decimal('0.0100'))
|
||||
|
||||
def test_partial_save_clearing_length_keeps_the_stored_unit(self):
|
||||
"""
|
||||
A save naming only length must clear the normalized length without writing length_unit.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('1')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
cable.length = None
|
||||
cable.save(update_fields=['length'])
|
||||
|
||||
# The unit was not written, so the instance must still agree with the row
|
||||
self.assertEqual(cable.length_unit, CableLengthUnitChoices.UNIT_METER)
|
||||
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertIsNone(cable.length)
|
||||
self.assertEqual(cable.length_unit, CableLengthUnitChoices.UNIT_METER)
|
||||
self.assertIsNone(cable._abs_length)
|
||||
|
||||
def test_partial_save_leaves_an_unwritten_length_alone(self):
|
||||
"""
|
||||
A save naming an unrelated field must not persist an in-memory length change.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('1')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
cable.length = Decimal('99')
|
||||
cable.label = 'Renamed'
|
||||
cable.save(update_fields=['label'])
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertEqual(cable.label, 'Renamed')
|
||||
self.assertEqual(cable.length, Decimal('1.00'))
|
||||
self.assertEqual(cable._abs_length, Decimal('1.0000'))
|
||||
|
||||
def test_partial_save_normalizes_against_an_out_of_band_length(self):
|
||||
"""
|
||||
A save naming only length_unit must read the stored length, not one cached on the instance.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('1')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
Cable.objects.filter(pk=cable.pk).update(length=Decimal('7'))
|
||||
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_FOOT
|
||||
cable.save(update_fields=['length_unit'])
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertEqual(cable.length, Decimal('7.00'))
|
||||
self.assertEqual(cable._abs_length, Decimal('2.1336'))
|
||||
|
||||
def test_partial_save_clears_a_unit_written_without_a_stored_length(self):
|
||||
"""
|
||||
A save naming only length_unit must drop the unit when the row holds no length.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save(update_fields=['length_unit'])
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertIsNone(cable.length)
|
||||
self.assertIsNone(cable.length_unit)
|
||||
self.assertIsNone(cable._abs_length)
|
||||
|
||||
def test_full_save_clears_the_unit_when_the_length_is_removed(self):
|
||||
"""
|
||||
A full save with no length must clear the stored unit.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('1')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
cable.length = None
|
||||
cable.save()
|
||||
cable.refresh_from_db()
|
||||
|
||||
self.assertIsNone(cable.length)
|
||||
self.assertIsNone(cable.length_unit)
|
||||
self.assertIsNone(cable._abs_length)
|
||||
|
||||
def test_partial_save_reads_the_stored_pair_only_for_an_excluded_field(self):
|
||||
"""
|
||||
A save writing both source fields must normalize without reading the row back.
|
||||
"""
|
||||
cable = Cable.objects.first()
|
||||
cable.length = Decimal('1')
|
||||
cable.length_unit = CableLengthUnitChoices.UNIT_METER
|
||||
cable.save()
|
||||
|
||||
with CaptureQueriesContext(connection) as both_written:
|
||||
cable.save(update_fields=['length', 'length_unit'])
|
||||
with CaptureQueriesContext(connection) as one_written:
|
||||
cable.save(update_fields=['length'])
|
||||
|
||||
def cable_reads(queries):
|
||||
return [q for q in queries if q['sql'].startswith('SELECT') and 'FROM "dcim_cable"' in q['sql']]
|
||||
|
||||
self.assertEqual(cable_reads(both_written), [])
|
||||
self.assertEqual(len(cable_reads(one_written)), 1)
|
||||
|
||||
|
||||
class CableTerminationTestCase(TestCase):
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue