45931f7f579e (#633, released in 1.3) changed ValidatedModelSerializer.validate()
to validate a copy of the instance on the update path:
else:
# Validate incoming PUT/PATCH data without mutating the DB instance.instance=copy(instance)
Applying the incoming data before validation is a real fix and 1.2.x was wrong
not to do it. But validating a copy silently breaks models whose clean()
corrects state by assignment: DRF's ModelSerializer.update() saves the original instance with no update_fields, so under 1.2.x those assignments
reached the database, while under 1.3 they are written to a throwaway and lost.
openwisp-controller's own Template.clean() depends on this in three places
(openwisp_controller/config/base/template.py, 1.2.3):
Create a template with type="vpn" and a VPN attached.
PATCH /api/v1/controller/template/<pk>/ with
{"type": "generic", "config": {"interfaces": []}} → 200.
PATCH /api/v1/controller/template/<pk>/ with {"name": "renamed"} → 200.
Re-read the row and inspect vpn.
Measured on identical code, varying only ValidatedModelSerializer.validate
(the 1.3 body applied via mock.patch):
after step 2
after step 3
utils 1.2.2
type=generic, vpn=<uuid>
vpn=None — corrected
utils 1.3
type=generic, vpn=<uuid>
vpn=<uuid> — never corrected
PATCH {"required": true} on any template shows the same shape: default stays
False permanently instead of being forced to True.
Expected behavior
A write should be validated against the state it will save, and any correction
clean() makes to that state should be what gets persisted. Note neither
version validates the submitted state at the moment of the write — step 2 is
validated while the row is still type=vpn and saves an inconsistent row under
both versions. The difference is that 1.2.x recovered on the next write and 1.3
does not recover at all.
Three possible directions:
apply the data to the real instance, accepting clean()-as-normalisation,
which is what callers already rely on;
keep the copy, but copy cleaned values back onto the instance for concrete
local fields;
declare that clean() must not mutate, and fix the affected
openwisp-controller models — cleanest contract, but breaking for
downstreams.
We hit this on a downstream model with a type-scoped permission field: under the
1.3 body, a template converted away from that type keeps granting device access
indefinitely, because the clean() that strips the generated credentials only
ever runs against a discarded copy. Fail-unsafe, and invisible — every status
code stays 200.
openwisp-controller 1.2.3 (latest on PyPI) pins openwisp-utils~=1.2.0, so
this pairing isn't reachable from a released controller yet, which is why it
seems worth catching before a 1.3-compatible controller ships.
Screenshots
N/A.
System Informatioon:
OS: Pop!_OS 24.04 LTS
Python Version: Python 3.13.5
Django Version: Django 5.2.16
Browser and Browser Version (if applicable): N/A (REST API)
Also: openwisp-utils 1.2.2 (1.3 behaviour reproduced by patching in the 1.3
validate body), openwisp-controller 1.2.3, djangorestframework 3.16.1.
I reproduced the update-path problem: ValidatedModelSerializer.validate() makes a shallow copy of self.instance, runs full_clean() on that copy, and corrections made by Model.clean() are therefore discarded before DRF saves the original instance.
My current understanding is that the smallest fix is to validate the real instance for the update path, restoring the previous behavior. I also confirmed the issue with an API-level PATCH regression case.
Before I start implementing, could a maintainer confirm that #760 is validated/ready for contribution and that this direction is acceptable?
Activity
AbhijitDoesCoding commented on Sep 30, 2026
Hi, I'd like to work on this issue.
I reproduced the update-path problem: ValidatedModelSerializer.validate() makes a shallow copy of self.instance, runs full_clean() on that copy, and corrections made by Model.clean() are therefore discarded before DRF saves the original instance.
My current understanding is that the smallest fix is to validate the real instance for the update path, restoring the previous behavior. I also confirmed the issue with an API-level PATCH regression case.
Before I start implementing, could a maintainer confirm that #760 is validated/ready for contribution and that this direction is acceptable?