Skip to content

[bug] ValidatedModelSerializer does not correctly validate instances during PUT/PATCH updates #633

@pandafy

Description

@pandafy

Describe the bug
The current implementation of ValidatedModelSerializer in openwisp_utils/api/serializers.py does not correctly perform model validation during update operations (PUT and PATCH).

In Django REST Framework's validation flow, the validate() method is executed before the update() method is called, so self.instance still has the original database values when validation runs.

Currently, the serializer calls instance.full_clean() without applying the incoming data (data) to the instance first. As a result, model validation runs against the previous state of the object instead of the updated state being submitted via the API.

Steps To Reproduce

  1. Perform a PUT or PATCH request to an API endpoint using ValidatedModelSerializer, updating some fields.
  2. Validation will run, but the model will not reflect the new values in the incoming request during validation.
  3. As a result, model clean() logic and unique/constraint validation may not consider the newly submitted values.

Expected behavior
Model validation should run against the merged state of the existing instance and the incoming validated data for update operations, not just the current database values.

Screenshots
N/A

System Informatioon:

  • OS: [e.g. Ubuntu 24.04 LTS]
  • Python Version: [e.g. Python 3.11.2]
  • Django Version: [e.g. Django 4.2.5]
  • Browser and Browser Version (if applicable): [e.g. Chromium v126.0.6478.126]

Details, Investigation, and Suggested Solution

Current implementation

class ValidatedModelSerializer(serializers.ModelSerializer):
    exclude_validation = None

    def validate(self, data):
        """Performs model validation on serialized data.

        Allows to avoid having to duplicate model validation logic in the
        REST API.
        """
        instance = self.instance
        # if instance is empty (eg: creation)
        # simulate for validation purposes
        if not instance:
            Model = self.Meta.model
            instance = Model()
            for key, value in data.items():
                # avoid direct assignment for m2m (not allowed)
                if not isinstance(Model._meta.get_field(key), models.ManyToManyField):
                    setattr(instance, key, value)
        # perform model validation
        instance.full_clean(exclude=self.exclude_validation)
        return data

Problem
For update requests, the serializer does not merge incoming data into the instance before validation, so validation may be performed on outdated values, and logic in the model or unique constraints may fail to catch invalid updates.

Proposed solution
Copy the instance and apply the incoming values before validation. Example:

import copy

def validate(self, data):
    instance = self.instance

    if instance:
        instance = copy.copy(instance)
        for key, value in data.items():
            field = instance._meta.get_field(key)
            if not isinstance(field, models.ManyToManyField):
                setattr(instance, key, value)
    else:
        Model = self.Meta.model
        instance = Model()
        for key, value in data.items():
            field = Model._meta.get_field(key)
            if not isinstance(field, models.ManyToManyField):
                setattr(instance, key, value)

    instance.full_clean(exclude=self.exclude_validation)
    return data

Impact
Model validation may behave incorrectly when updating objects via the API, potentially allowing invalid updates to pass serializer validation.

Suggested tests

  • Model validation works correctly on POST
  • Model validation works correctly on PUT
  • Model validation works correctly on PATCH
  • Validation reflects updated field values

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Status

    To do (Python & Django)

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions