fix: a PATCH operation without a path accepts a resource as its value

Author: azmeuk

doc/changelog.rst

@@ -18,6 +18,9 @@ Fixed
   :attr:`Required.true <scim2_models.Required.true>` unassigned answers ``mutability`` instead
   of ``invalidValue``, which is the ``scimType``
   :rfc:`RFC7644 §3.5.2.2 <7644#section-3.5.2.2>` names for it.
+- A PATCH operation carrying no ``path`` accepts a resource as its ``value``, and not only a
+  mapping. Such a resource used to be ignored without a word: neither checked against the
+  model, nor written to the patched resource.
 - A PATCH operation carrying no ``path`` has the attributes of its ``value`` checked, where
   they used to go through unexamined. :rfc:`RFC7644 §3.5.2.3 <7644#section-3.5.2.3>` makes
   them the targets of the operation, so each one answers to §3.5.2 as a named path does: a

scim2_models/messages/patch_op.py

@@ -15,6 +15,7 @@
 from ..annotations import Mutability
 from ..annotations import Required
 from ..attributes import ComplexAttribute
+from ..base import BaseModel
 from ..context import Context
 from ..exceptions import InvalidValueException
 from ..exceptions import MutabilityException
@@ -29,6 +30,22 @@
 ResourceT = TypeVar("ResourceT", bound=Resource[Any])
 
 
+def _targeted_attributes(value: Any) -> dict[str, Any]:
+    """Return the attributes an operation without a path writes.
+
+    :rfc:`RFC7644 §3.5.2.3 <7644#section-3.5.2.3>` has the ``value`` name them
+    when the path is omitted. A client building its payload in Python passes a
+    resource, where a server parses a mapping, and both name the same
+    attributes. Anything else names none.
+    """
+    if isinstance(value, BaseModel):
+        # Dumped out of context on purpose: a payload dumped in the PATCH
+        # context already leaves read-only attributes out, and an operation
+        # naming one is to be refused rather than quietly trimmed.
+        return value.model_dump(exclude_unset=True)
+    return value if isinstance(value, dict) else {}
+
+
 def _resolved_field(
     resource_class: type[Resource[Any]], attr_name: str | None
 ) -> str | None:
@@ -272,8 +289,7 @@ def validate_operations(self, info: ValidationInfo) -> Self:
                 # the attributes to write. Each of them is a target of its own,
                 # and one the model does not declare is left alone, as §3.5.2
                 # asks servers to be tolerant of schema extensions.
-                targeted = operation.value if isinstance(operation.value, dict) else {}
-                for attr_name, written in targeted.items():
+                for attr_name, written in _targeted_attributes(operation.value).items():
                     operation._validate_mutability(resource_class, attr_name)
                     operation._validate_required_attribute(
                         resource_class, attr_name, written
@@ -389,10 +405,17 @@ def _apply_add_replace(
         """Apply an add or replace operation."""
         before_state = self._capture_primary_state(resource)
 
+        value = operation.value
+        if operation.path is None and isinstance(value, BaseModel):
+            # Path("").set writes the attributes a mapping names, so a resource
+            # given as a value is dumped to the payload it stands for, and to
+            # the very attributes the operation was checked against.
+            value = _targeted_attributes(value)
+
         path = operation.path if operation.path is not None else Path("")
         modified = path.set(
             resource,  # type: ignore[arg-type]
-            operation.value,
+            value,
             is_add=operation.op == PatchOperation.Op.add,
         )
 

tests/test_patch_op_validation.py

@@ -786,6 +786,7 @@ def test_a_replace_may_empty_a_required_string():
 
 
 def test_a_replace_may_unassign_an_optional_attribute():
+    """Only a required attribute is protected from becoming unassigned."""
     patch = PatchOp[User].model_validate(
         {
             "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
@@ -813,6 +814,7 @@ def test_an_operation_without_path_cannot_write_a_read_only_attribute():
 
 
 def test_an_operation_without_path_cannot_write_a_read_only_complex_attribute():
+    """Naming a complex attribute is enough to refuse it, without descending into its sub-attributes."""
     with pytest.raises(ValidationError, match="mutability"):
         PatchOp[User].model_validate(
             {
@@ -826,6 +828,7 @@ def test_an_operation_without_path_cannot_write_a_read_only_complex_attribute():
 
 
 def test_an_operation_without_path_cannot_unassign_a_required_attribute():
+    """A null value in the ``value`` unassigns its attribute as surely as a remove does."""
     with pytest.raises(
         ValidationError, match="required attribute cannot be unassigned"
     ):
@@ -839,6 +842,7 @@ def test_an_operation_without_path_cannot_unassign_a_required_attribute():
 
 
 def test_an_add_without_path_cannot_write_a_read_only_attribute():
+    """:rfc:`RFC7644 §3.5.2.1 <7644#section-3.5.2.1>` gives ``add`` the same omitted path as ``replace``."""
     with pytest.raises(ValidationError, match="mutability"):
         PatchOp[User].model_validate(
             {
@@ -898,3 +902,52 @@ def test_unassigning_a_required_attribute_answers_mutability():
                 scim_ctx=Context.RESOURCE_PATCH_REQUEST,
             )
         assert exc_info.value.errors()[0]["type"] == "scim_mutability"
+
+
+def test_an_operation_without_path_takes_a_model_as_value():
+    """A client building its payload in Python passes a resource, where a server parses a dict."""
+    patch = PatchOp[User](
+        operations=[
+            PatchOperation[User](
+                op=PatchOperation.Op.replace_, value=User(display_name="Babs")
+            )
+        ]
+    )
+    user = User(id="srv-1", user_name="bjensen")
+    assert patch.patch(user)
+    assert user.display_name == "Babs"
+    assert user.user_name == "bjensen"
+
+
+def test_a_model_value_answers_to_the_same_constraints_as_a_dict_one():
+    """The form the value takes says nothing about what the operation may write."""
+    with pytest.raises(ValidationError, match="mutability"):
+        PatchOp[User](
+            operations=[
+                PatchOperation[User](
+                    op=PatchOperation.Op.replace_,
+                    value=User(id="chosen-by-the-client"),
+                )
+            ]
+        )
+
+
+def test_a_model_value_carries_the_attributes_its_payload_would():
+    """A null attribute is not serialized, so it is neither written nor refused.
+
+    Unassigning through an operation without a path takes the payload form,
+    ``{"userName": null}``, which :rfc:`RFC7643 §2.5 <7643#section-2.5>` makes
+    equivalent to an unassigned attribute.
+    """
+    patch = PatchOp[User](
+        operations=[
+            PatchOperation[User](
+                op=PatchOperation.Op.replace_,
+                value=User(user_name=None, display_name="Babs"),
+            )
+        ]
+    )
+    user = User(id="srv-1", user_name="bjensen")
+    patch.patch(user)
+    assert user.user_name == "bjensen"
+    assert user.display_name == "Babs"