diff --git a/doc/changelog.rst b/doc/changelog.rst index 60c5fd8..ee70f8a 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -4,6 +4,14 @@ Changelog [0.6.8] - Unreleased -------------------- +Added +^^^^^ +- :class:`~scim2_models.MutabilityException` handler in framework integration examples (FastAPI, Flask, Django). + +Deprecated +^^^^^^^^^^ +- The ``original`` parameter of :meth:`~scim2_models.base.BaseModel.model_validate` is deprecated. Use :meth:`~scim2_models.Resource.replace` on the validated instance instead. Will be removed in 0.8.0. + Fixed ^^^^^ - PATCH operations on :attr:`~scim2_models.Mutability.immutable` fields are now validated at runtime per :rfc:`RFC 7644 §3.5.2 <7644#section-3.5.2>`: ``add`` is only allowed when the field has no previous value, ``replace`` is only allowed with the same value, and ``remove`` is only allowed on unset fields. diff --git a/doc/guides/_examples/django_example.py b/doc/guides/_examples/django_example.py index eb6777f..099525e 100644 --- a/doc/guides/_examples/django_example.py +++ b/doc/guides/_examples/django_example.py @@ -16,9 +16,9 @@ from scim2_models import PatchOp from scim2_models import ResourceType from scim2_models import ResponseParameters +from scim2_models import SCIMException from scim2_models import Schema from scim2_models import SearchRequest -from scim2_models import UniquenessException from scim2_models import User from .integrations import check_etag @@ -81,12 +81,12 @@ def scim_validation_error(error): # -- validation-helper-end -- -# -- uniqueness-helper-start -- -def scim_uniqueness_error(error): - """Turn uniqueness errors into a SCIM 409 response.""" - scim_error = UniquenessException(detail=str(error)).to_error() - return scim_response(scim_error.model_dump_json(), HTTPStatus.CONFLICT) -# -- uniqueness-helper-end -- +# -- scim-exception-helper-start -- +def scim_exception_error(error): + """Turn SCIM exceptions into a SCIM error response.""" + scim_error = error.to_error() + return scim_response(scim_error.model_dump_json(), scim_error.status) +# -- scim-exception-helper-end -- # -- precondition-helper-start -- @@ -152,17 +152,19 @@ def put(self, request, app_record): replacement = User.model_validate( json.loads(request.body), scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, - original=existing_user, ) + replacement.replace(existing_user) except ValidationError as error: return scim_validation_error(error) + except SCIMException as error: + return scim_exception_error(error) replacement.id = existing_user.id updated_record = from_scim_user(replacement) try: save_record(updated_record) - except ValueError as error: - return scim_uniqueness_error(error) + except SCIMException as error: + return scim_exception_error(error) response_user = to_scim_user(updated_record, resource_location(request, updated_record)) resp = scim_response( @@ -192,8 +194,8 @@ def patch(self, request, app_record): updated_record = from_scim_user(scim_user) try: save_record(updated_record) - except ValueError as error: - return scim_uniqueness_error(error) + except SCIMException as error: + return scim_exception_error(error) resp = scim_response( scim_user.model_dump_json(scim_ctx=Context.RESOURCE_PATCH_RESPONSE) @@ -242,8 +244,8 @@ def post(self, request): app_record = from_scim_user(request_user) try: save_record(app_record) - except ValueError as error: - return scim_uniqueness_error(error) + except SCIMException as error: + return scim_exception_error(error) response_user = to_scim_user(app_record, resource_location(request, app_record)) resp = scim_response( diff --git a/doc/guides/_examples/fastapi_example.py b/doc/guides/_examples/fastapi_example.py index 62b8a6c..e89e076 100644 --- a/doc/guides/_examples/fastapi_example.py +++ b/doc/guides/_examples/fastapi_example.py @@ -14,9 +14,9 @@ from scim2_models import PatchOp from scim2_models import ResourceType from scim2_models import ResponseParameters +from scim2_models import SCIMException from scim2_models import Schema from scim2_models import SearchRequest -from scim2_models import UniquenessException from scim2_models import User from .integrations import PreconditionFailed @@ -79,11 +79,11 @@ async def handle_http_exception(request, error): return Response(scim_error.model_dump_json(), status_code=error.status_code) -@app.exception_handler(ValueError) -async def handle_value_error(request, error): - """Turn uniqueness errors into SCIM 409 responses.""" - scim_error = UniquenessException(detail=str(error)).to_error() - return Response(scim_error.model_dump_json(), status_code=HTTPStatus.CONFLICT) +@app.exception_handler(SCIMException) +async def handle_scim_error(request, error): + """Turn SCIM exceptions into SCIM error responses.""" + scim_error = error.to_error() + return Response(scim_error.model_dump_json(), status_code=scim_error.status) @app.exception_handler(PreconditionFailed) @@ -151,8 +151,8 @@ async def replace_user(request: Request, app_record: dict = Depends(resolve_user replacement = User.model_validate( await request.json(), scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, - original=existing_user, ) + replacement.replace(existing_user) replacement.id = existing_user.id updated_record = from_scim_user(replacement) diff --git a/doc/guides/_examples/flask_example.py b/doc/guides/_examples/flask_example.py index 217d795..e2aa34c 100644 --- a/doc/guides/_examples/flask_example.py +++ b/doc/guides/_examples/flask_example.py @@ -14,9 +14,9 @@ from scim2_models import PatchOp from scim2_models import ResourceType from scim2_models import ResponseParameters +from scim2_models import SCIMException from scim2_models import Schema from scim2_models import SearchRequest -from scim2_models import UniquenessException from scim2_models import User from .integrations import check_etag @@ -87,11 +87,11 @@ def handle_not_found(error): return scim_error.model_dump_json(), HTTPStatus.NOT_FOUND -@bp.errorhandler(ValueError) -def handle_value_error(error): - """Turn uniqueness errors into SCIM 409 responses.""" - scim_error = UniquenessException(detail=str(error)).to_error() - return scim_error.model_dump_json(), HTTPStatus.CONFLICT +@bp.errorhandler(SCIMException) +def handle_scim_error(error): + """Turn SCIM exceptions into SCIM error responses.""" + scim_error = error.to_error() + return scim_error.model_dump_json(), scim_error.status @bp.errorhandler(PreconditionFailed) @@ -156,8 +156,8 @@ def replace_user(app_record): replacement = User.model_validate( request.get_json(), scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, - original=existing_user, ) + replacement.replace(existing_user) replacement.id = existing_user.id updated_record = from_scim_user(replacement) diff --git a/doc/guides/_examples/integrations.py b/doc/guides/_examples/integrations.py index 81adb4c..563ed10 100644 --- a/doc/guides/_examples/integrations.py +++ b/doc/guides/_examples/integrations.py @@ -15,6 +15,7 @@ from scim2_models import ResourceType from scim2_models import ServiceProviderConfig from scim2_models import Sort +from scim2_models import UniquenessException from scim2_models import User # -- storage-start -- @@ -40,12 +41,14 @@ def list_records(start=None, stop=None): def save_record(record): - """Persist *record*, raising ValueError if its userName is already taken.""" + """Persist *record*, raising UniquenessException if its userName is already taken.""" if not record.get("id"): record["id"] = str(uuid4()) for existing in records.values(): if existing["id"] != record["id"] and existing["user_name"] == record["user_name"]: - raise ValueError(f"userName {record['user_name']!r} is already taken") + raise UniquenessException( + detail=f"userName {record['user_name']!r} is already taken" + ) now = datetime.now(timezone.utc) record.setdefault("created_at", now) record["updated_at"] = now diff --git a/doc/guides/django.rst b/doc/guides/django.rst index c8c9141..ff7958c 100644 --- a/doc/guides/django.rst +++ b/doc/guides/django.rst @@ -67,16 +67,16 @@ If :meth:`~scim2_models.Resource.model_validate` or :class:`~pydantic.ValidationError` and return a SCIM :class:`~scim2_models.Error` response. -Uniqueness error helper -^^^^^^^^^^^^^^^^^^^^^^^ +SCIM exception helper +^^^^^^^^^^^^^^^^^^^^^ -``scim_uniqueness_error`` catches the ``ValueError`` raised by ``save_record`` and returns a -409 with ``scimType: uniqueness`` using :class:`~scim2_models.UniquenessException`. +``scim_exception_error`` converts any :class:`~scim2_models.SCIMException` +(uniqueness, mutability, …) into a SCIM error response. .. literalinclude:: _examples/django_example.py :language: python - :start-after: # -- uniqueness-helper-start -- - :end-before: # -- uniqueness-helper-end -- + :start-after: # -- scim-exception-helper-start -- + :end-before: # -- scim-exception-helper-end -- Precondition error helper ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -121,8 +121,9 @@ For ``GET``, parse query parameters with :class:`~scim2_models.ResponseParameter SCIM resource, and serialize with :attr:`~scim2_models.Context.RESOURCE_QUERY_RESPONSE`. For ``DELETE``, remove the record and return an empty 204 response. For ``PUT``, validate the full replacement payload with -:attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_REQUEST`, passing the ``original`` resource -so that immutable attributes are checked for unintended modifications. +:attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_REQUEST`, then call +:meth:`~scim2_models.Resource.replace` to verify that immutable attributes +have not been modified. Convert back to native and persist, then serialize with :attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_RESPONSE`. For ``PATCH``, validate the payload with :attr:`~scim2_models.Context.RESOURCE_PATCH_REQUEST`, diff --git a/doc/guides/fastapi.rst b/doc/guides/fastapi.rst index 269a469..3ac3d07 100644 --- a/doc/guides/fastapi.rst +++ b/doc/guides/fastapi.rst @@ -72,8 +72,8 @@ validation errors, HTTP exceptions, and application errors aligned with SCIM res response. ``handle_http_exception`` catches HTTP errors such as the 404 raised by the dependency and wraps them in a SCIM :class:`~scim2_models.Error`. -``handle_value_error`` catches the ``ValueError`` raised by ``save_record`` and returns a 409 -with ``scimType: uniqueness`` using :class:`~scim2_models.UniquenessException`. +``handle_scim_error`` catches any :class:`~scim2_models.SCIMException` (uniqueness, mutability, …) +and returns the appropriate SCIM :class:`~scim2_models.Error` response. ``handle_precondition_failed`` catches :class:`~doc.guides._examples.integrations.PreconditionFailed` errors raised by the :ref:`ETag helpers ` and returns a 412. @@ -129,10 +129,9 @@ PUT /Users/ ^^^^^^^^^^^^^^^ Validate the full replacement payload with -:attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_REQUEST`, passing the ``original`` resource -so that immutable attributes are checked for unintended modifications. -Convert back to native and persist, then serialize the result with -:attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_RESPONSE`. +:attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_REQUEST`, then call +:meth:`~scim2_models.Resource.replace` to verify that immutable attributes +have not been modified. .. literalinclude:: _examples/fastapi_example.py :language: python diff --git a/doc/guides/flask.rst b/doc/guides/flask.rst index e3477ba..4694533 100644 --- a/doc/guides/flask.rst +++ b/doc/guides/flask.rst @@ -66,8 +66,8 @@ responses. If :meth:`~scim2_models.Resource.model_validate`, Flask routes the :class:`~pydantic.ValidationError` to ``handle_validation_error`` and the client receives a SCIM :class:`~scim2_models.Error` response. -``handle_value_error`` catches the ``ValueError`` raised by ``save_record`` and returns a 409 -with ``scimType: uniqueness`` using :class:`~scim2_models.UniquenessException`. +``handle_scim_error`` catches any :class:`~scim2_models.SCIMException` (uniqueness, mutability, …) +and returns the appropriate SCIM :class:`~scim2_models.Error` response. ``handle_precondition_failed`` catches :class:`~doc.guides._examples.integrations.PreconditionFailed` errors raised by the :ref:`ETag helpers ` and returns a 412. @@ -122,10 +122,9 @@ PUT /Users/ ^^^^^^^^^^^^^^^ Validate the full replacement payload with -:attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_REQUEST`, passing the ``original`` resource -so that immutable attributes are checked for unintended modifications. -Convert back to native and persist, then serialize the result with -:attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_RESPONSE`. +:attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_REQUEST`, then call +:meth:`~scim2_models.Resource.replace` to verify that immutable attributes +have not been modified. .. literalinclude:: _examples/flask_example.py :language: python diff --git a/doc/tutorial.rst b/doc/tutorial.rst index 56be359..8c875be 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -124,13 +124,6 @@ fields with unexpected values will raise :class:`~pydantic.ValidationError`: ... except pydantic.ValidationError: ... obj = Error(...) -.. note:: - - With the :attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_REQUEST` context, - :meth:`~scim2_models.BaseModel.model_validate` takes an additional - :paramref:`~scim2_models.BaseModel.model_validate.original` argument that is used to compare - :attr:`~scim2_models.Mutability.immutable` attributes, and raise an exception when they have mutated. - Attributes inclusions and exclusions ==================================== @@ -479,6 +472,29 @@ Client applications can use this to dynamically discover server resources by bro :language: json :caption: schema-group.json +Replace operations +================== + +When handling a ``PUT`` request, validate the incoming payload with the +:attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_REQUEST` context, then call +:meth:`~scim2_models.Resource.replace` against the existing resource to +verify that :attr:`~scim2_models.Mutability.immutable` attributes have not been +modified. + +.. doctest:: + + >>> from scim2_models import User, Context + >>> from scim2_models.exceptions import MutabilityException + >>> existing = User(user_name="bjensen") + >>> replacement = User.model_validate( + ... {"userName": "bjensen"}, + ... scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + ... ) + >>> replacement.replace(existing) + +If an immutable attribute differs, a :class:`~scim2_models.MutabilityException` +is raised. + Patch operations ================ diff --git a/scim2_models/base.py b/scim2_models/base.py index 1e286eb..af1bec3 100644 --- a/scim2_models/base.py +++ b/scim2_models/base.py @@ -1,3 +1,4 @@ +import warnings from inspect import isclass from typing import Any from typing import Optional @@ -23,6 +24,7 @@ from scim2_models.annotations import Required from scim2_models.annotations import Returned from scim2_models.context import Context +from scim2_models.exceptions import MutabilityException from scim2_models.utils import UNION_TYPES from scim2_models.utils import _find_field_name from scim2_models.utils import _normalize_attribute_name @@ -410,7 +412,10 @@ def check_replacement_request_mutability( and issubclass(cls, Resource) and original is not None ): - cls._check_mutability_issues(original, obj) + try: + obj._check_immutable_fields(original) + except MutabilityException as exc: + raise exc.as_pydantic_error() from exc return obj @model_validator(mode="after") @@ -456,35 +461,30 @@ def check_primary_attribute_uniqueness(self, info: ValidationInfo) -> Self: return self - @classmethod - def _check_mutability_issues( - cls, original: "BaseModel", replacement: "BaseModel" - ) -> None: - """Compare two instances, and check for differences of values on the fields marked as immutable.""" + def _check_immutable_fields(self, original: Self) -> None: + """Check that immutable fields have not been modified compared to *original*. + + Recursively checks nested single-valued complex attributes. + """ from .attributes import is_complex_attribute - model = replacement.__class__ - for field_name in model.model_fields: - mutability = model.get_field_annotation(field_name, Mutability) + for field_name in type(self).model_fields: + mutability = type(self).get_field_annotation(field_name, Mutability) if mutability == Mutability.immutable and getattr( original, field_name - ) != getattr(replacement, field_name): - raise PydanticCustomError( - "mutability_error", - "Field '{field_name}' is immutable but the request value is different than the original value.", - {"field_name": field_name}, - ) + ) != getattr(self, field_name): + raise MutabilityException(attribute=field_name, mutability="immutable") - attr_type = model.get_field_root_type(field_name) + attr_type = type(self).get_field_root_type(field_name) if ( attr_type and is_complex_attribute(attr_type) - and not model.get_field_multiplicity(field_name) + and not type(self).get_field_multiplicity(field_name) ): original_val = getattr(original, field_name) - replacement_value = getattr(replacement, field_name) - if original_val is not None and replacement_value is not None: - cls._check_mutability_issues(original_val, replacement_value) + replacement_val = getattr(self, field_name) + if original_val is not None and replacement_val is not None: + replacement_val._check_immutable_fields(original_val) def _set_complex_attribute_urns(self) -> None: """Navigate through attributes and sub-attributes of type ComplexAttribute, and mark them with a '_attribute_urn' attribute. @@ -611,22 +611,30 @@ def model_validate( original: Optional["BaseModel"] = None, **kwargs: Any, ) -> Self: - """Validate SCIM payloads and generate model representation by using Pydantic :code:`BaseModel.model_validate`. + """Validate SCIM payloads and generate model representation by using Pydantic :meth:`~pydantic.BaseModel.model_validate`. :param scim_ctx: The SCIM :class:`~scim2_models.Context` in which the validation happens. :param original: If this parameter is set during :attr:`~Context.RESOURCE_REPLACEMENT_REQUEST`, :attr:`~scim2_models.Mutability.immutable` parameters will be compared against the *original* model value. An exception is raised if values are different. + + .. deprecated:: 0.6.7 + Use :meth:`replace` on the validated instance instead. + Will be removed in 0.8.0. """ + if original is not None: + warnings.warn( + "The 'original' parameter is deprecated, " + "use the 'replace' method on the validated instance instead. " + "Will be removed in 0.8.0.", + DeprecationWarning, + stacklevel=2, + ) + context = kwargs.setdefault("context", {}) context.setdefault("scim", scim_ctx) context.setdefault("original", original) - if scim_ctx == Context.RESOURCE_REPLACEMENT_REQUEST and original is None: - raise ValueError( - "Resource queries replacement validation must compare to an original resource" - ) - return super().model_validate(*args, **kwargs) def get_attribute_urn(self, field_name: str) -> str: diff --git a/scim2_models/resources/resource.py b/scim2_models/resources/resource.py index a38204b..1798d14 100644 --- a/scim2_models/resources/resource.py +++ b/scim2_models/resources/resource.py @@ -153,6 +153,20 @@ class Resource(ScimObject, Generic[AnyExtension]): meta: Annotated[Meta | None, Mutability.read_only, Returned.default] = None """A complex attribute containing resource metadata.""" + def replace(self, original: Self) -> None: + """Verify that no immutable field has been modified compared to *original*. + + Intended to be called after parsing a PUT request body, to enforce + :rfc:`RFC 7644 §3.5.1 <7644#section-3.5.1>`: if one or more values + are already set for an immutable attribute, the input values MUST match. + + Recursively checks nested single-valued complex attributes. + + :param original: The original resource state to compare against. + :raises MutabilityException: If an immutable field value differs. + """ + self._check_immutable_fields(original) + @classmethod def __class_getitem__(cls, item: Any) -> type["Resource[Any]"]: """Create a Resource class with extension fields dynamically added.""" diff --git a/tests/test_model_validation.py b/tests/test_model_validation.py index 7973656..7957653 100644 --- a/tests/test_model_validation.py +++ b/tests/test_model_validation.py @@ -147,56 +147,46 @@ def test_validate_replacement_request_mutability(): - Mutability.immutable raise a ValidationError if different than the 'original' item. - Mutability.read_only are ignored """ - with pytest.raises( - ValueError, - match="Resource queries replacement validation must compare to an original resource", - ): - MutResource.model_validate( + original = MutResource(read_only="y", read_write="y", write_only="y", immutable="y") + with pytest.warns(DeprecationWarning, match="original"): + assert MutResource.model_validate( { "readOnly": "x", "readWrite": "x", "writeOnly": "x", + "immutable": "y", }, scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + original=original, + ) == MutResource( + schemas=["org:example:MutResource"], + readWrite="x", + writeOnly="x", + immutable="y", ) - original = MutResource(read_only="y", read_write="y", write_only="y", immutable="y") - assert MutResource.model_validate( - { - "readOnly": "x", - "readWrite": "x", - "writeOnly": "x", - "immutable": "y", - }, - scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, - original=original, - ) == MutResource( - schemas=["org:example:MutResource"], - readWrite="x", - writeOnly="x", - immutable="y", - ) - - MutResource.model_validate( - { - "immutable": "y", - }, - scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, - original=original, - ) - - with pytest.raises( - ValidationError, - match="Field 'immutable' is immutable but the request value is different than the original value.", - ): + with pytest.warns(DeprecationWarning, match="original"): MutResource.model_validate( { - "immutable": "x", + "immutable": "y", }, scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, original=original, ) + with pytest.warns(DeprecationWarning, match="original"): + with pytest.raises( + ValidationError, + match="mutability", + ): + MutResource.model_validate( + { + "immutable": "x", + }, + scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + original=original, + ) + def test_validate_replacement_request_mutability_sub_attributes(): """Test query validation for resource model replacement requests. @@ -214,45 +204,123 @@ class Super(Resource): sub: Sub | None = None original = Super(sub=Sub(immutable="y")) - assert Super.model_validate( - { - "sub": { - "immutable": "y", - } - }, - scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, - original=original, - ) == Super( - schemas=["org:example:Super"], - sub=Sub( - immutable="y", - ), - ) - - Super.model_validate( - { - "sub": { - "immutable": "y", - } - }, - scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, - original=original, - ) + with pytest.warns(DeprecationWarning, match="original"): + assert Super.model_validate( + { + "sub": { + "immutable": "y", + } + }, + scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + original=original, + ) == Super( + schemas=["org:example:Super"], + sub=Sub( + immutable="y", + ), + ) - with pytest.raises( - ValidationError, - match="Field 'immutable' is immutable but the request value is different than the original value.", - ): + with pytest.warns(DeprecationWarning, match="original"): Super.model_validate( { "sub": { - "immutable": "x", + "immutable": "y", } }, scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, original=original, ) + with pytest.warns(DeprecationWarning, match="original"): + with pytest.raises( + ValidationError, + match="mutability", + ): + Super.model_validate( + { + "sub": { + "immutable": "x", + } + }, + scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + original=original, + ) + + +def test_replace_detects_changed_field(): + """Replace raises MutabilityException when an immutable field differs.""" + from scim2_models.exceptions import MutabilityException + + original = MutResource(immutable="y") + replacement = MutResource(immutable="x") + with pytest.raises(MutabilityException): + replacement.replace(original) + + +def test_replace_allows_identical_values(): + """Replace passes when immutable fields are unchanged.""" + original = MutResource(immutable="y") + replacement = MutResource(immutable="y") + replacement.replace(original) + + +def test_replace_recurses_into_complex_attributes(): + """Replace detects changes in nested complex attribute immutable fields.""" + from scim2_models.exceptions import MutabilityException + + class Sub(ComplexAttribute): + immutable: Annotated[str | None, Mutability.immutable] = None + + class Super(Resource): + schemas: Annotated[list[str], Required.true] = ["org:example:Super"] + sub: Sub | None = None + + original = Super(sub=Sub(immutable="y")) + replacement = Super(sub=Sub(immutable="x")) + with pytest.raises(MutabilityException): + replacement.replace(original) + + +def test_replace_ignores_readwrite_changes(): + """Replace does not raise when readWrite fields change.""" + original = MutResource(read_write="y") + replacement = MutResource(read_write="x") + replacement.replace(original) + + +def test_original_parameter_emits_deprecation_warning(): + """Passing 'original' to model_validate emits a DeprecationWarning.""" + original = MutResource(immutable="y") + with pytest.warns(DeprecationWarning, match="original"): + MutResource.model_validate( + {"immutable": "y"}, + scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + original=original, + ) + + +def test_replacement_request_without_original_parameter(): + """Replacement requests work without 'original' when using replace manually.""" + from scim2_models.exceptions import MutabilityException + + original = MutResource(immutable="y") + replacement = MutResource.model_validate( + {"immutable": "x"}, + scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + ) + with pytest.raises(MutabilityException): + replacement.replace(original) + + +def test_replacement_request_without_original_allows_matching_values(): + """Replacement requests validate and replace succeeds with identical immutable values.""" + original = MutResource(immutable="y") + replacement = MutResource.model_validate( + {"immutable": "y"}, + scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + ) + replacement.replace(original) + def test_validate_search_request_mutability(): """Test query validation for resource query request. @@ -460,14 +528,12 @@ def test_validate_creation_and_replacement_request_necessity(context): Attributes marked as: - Required.true and missing raise a ValidationError """ - original = MutResource(read_only="y", read_write="y", write_only="y", immutable="y") assert ReqResource.model_validate( { "required": "x", "optional": "x", }, scim_ctx=context, - original=original, ) == ReqResource( schemas=["org:example:ReqResource"], required="x", @@ -479,7 +545,6 @@ def test_validate_creation_and_replacement_request_necessity(context): "required": "x", }, scim_ctx=context, - original=original, ) == ReqResource( schemas=["org:example:ReqResource"], required="x", @@ -493,7 +558,6 @@ def test_validate_creation_and_replacement_request_necessity(context): { "optional": "x", }, - original=original, scim_ctx=context, ) diff --git a/tests/test_patch_op_validation.py b/tests/test_patch_op_validation.py index 69bd319..9f9ea8d 100644 --- a/tests/test_patch_op_validation.py +++ b/tests/test_patch_op_validation.py @@ -248,24 +248,21 @@ def test_patch_operation_validation_contexts(): def test_validate_mutability_readonly_error(): - """Test mutability validation error for readOnly attributes.""" - # Test add operation on readOnly field - with pytest.raises(ValidationError, match="mutability"): - PatchOp[User].model_validate( - {"operations": [{"op": "add", "path": "id", "value": "new-id"}]}, - context={"scim": Context.RESOURCE_PATCH_REQUEST}, - ) - - # Test replace operation on readOnly field - with pytest.raises(ValidationError, match="mutability"): - PatchOp[User].model_validate( - {"operations": [{"op": "replace", "path": "id", "value": "new-id"}]}, - context={"scim": Context.RESOURCE_PATCH_REQUEST}, - ) + """All PATCH operations on readOnly attributes are rejected at parse-time.""" + for op, extra in [ + ("add", {"value": "x"}), + ("replace", {"value": "x"}), + ("remove", {}), + ]: + with pytest.raises(ValidationError, match="mutability"): + PatchOp[User].model_validate( + {"operations": [{"op": op, "path": "id", **extra}]}, + context={"scim": Context.RESOURCE_PATCH_REQUEST}, + ) -def test_validate_mutability_readonly_replace_via_complex_path(): - """Replacing a readOnly complex attribute path is rejected.""" +def test_validate_mutability_readonly_via_complex_path(): + """Replacing a readOnly complex attribute path is rejected at parse-time.""" with pytest.raises(ValidationError, match="mutability"): PatchOp[User].model_validate( { @@ -282,7 +279,7 @@ def test_validate_mutability_readonly_replace_via_complex_path(): def test_patch_remove_on_immutable_field_with_value_is_rejected(): - """Removing an existing immutable attribute via PATCH is rejected.""" + """Removing an existing immutable attribute via PATCH is rejected at runtime.""" resource = ImmutableFieldResource.model_construct(locked="existing") patch_op = PatchOp[ImmutableFieldResource].model_validate( {"operations": [{"op": "remove", "path": "locked"}]},