diff --git a/doc/changelog.rst b/doc/changelog.rst index 386e7d6..1286e9b 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -1,14 +1,28 @@ Changelog ========= +[0.3.0] - Unreleased +-------------------- + +Changed +^^^^^^^ +- Add a :paramref:`~scim2_models.BaseModel.model_validate.original` + parameter to :meth:`~scim2_models.BaseModel.model_validate` + mandatory for :attr:`~scim2_models.Context.RESOURCE_REPLACEMENT_REQUEST`. + This *original* value is used to look if :attr:`~scim2_models.Mutability.immutable` + parameters have mutated. + :issue:`86` + [0.2.12] - 2024-12-09 --------------------- + Added ^^^^^ - Implement :meth:`Attribute.get_attribute `. [0.2.11] - 2024-12-08 --------------------- + Added ^^^^^ - Implement :meth:`Schema.get_attribute `. diff --git a/doc/conf.py b/doc/conf.py index e53f700..db49ad4 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -17,6 +17,7 @@ "sphinx.ext.viewcode", "sphinxcontrib.autodoc_pydantic", "sphinx_issues", + "sphinx_paramlinks", "sphinx_togglebutton", "myst_parser", ] diff --git a/doc/tutorial.rst b/doc/tutorial.rst index 8efcaa2..49d8e6e 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -102,6 +102,13 @@ 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 ==================================== diff --git a/pyproject.toml b/pyproject.toml index b21387a..a6c1023 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,7 @@ doc = [ "autodoc-pydantic>=2.2.0", "myst-parser>=3.0.1", "shibuya>=2024.5.15", + "sphinx-paramlinks>=0.6.0", "sphinx>=7.3.7", "sphinx-issues >= 5.0.0", "sphinx-togglebutton>=0.3.2", diff --git a/scim2_models/base.py b/scim2_models/base.py index 426484b..ffff680 100644 --- a/scim2_models/base.py +++ b/scim2_models/base.py @@ -233,9 +233,9 @@ class Context(Enum): Should be used for clients building a payload for a resource replacement request, and servers validating resource replacement request payloads. - - When used for serialization, it will not dump attributes annotated with :attr:`~scim2_models.Mutability.read_only` and :attr:`~scim2_models.Mutability.immutable`. + - When used for serialization, it will not dump attributes annotated with :attr:`~scim2_models.Mutability.read_only`. - When used for validation, it will ignore attributes annotated with :attr:`scim2_models.Mutability.read_only` and raise a :class:`~pydantic.ValidationError`: - - when finding attributes annotated with :attr:`~scim2_models.Mutability.immutable`, + - when finding attributes annotated with :attr:`~scim2_models.Mutability.immutable` different than :paramref:`~scim2_models.BaseModel.model_validate.original`: - when attributes annotated with :attr:`Required.true ` are missing on null. """ @@ -492,12 +492,6 @@ def check_request_attributes_mutability( ): raise exc - if ( - context == Context.RESOURCE_REPLACEMENT_REQUEST - and mutability == Mutability.immutable - ): - raise exc - if ( context in (Context.RESOURCE_CREATION_REQUEST, Context.RESOURCE_REPLACEMENT_REQUEST) @@ -604,8 +598,55 @@ def check_response_attributes_necessity( return value + @model_validator(mode="wrap") + @classmethod + def check_replacement_request_mutability( + cls, value: Any, handler: ValidatorFunctionWrapHandler, info: ValidationInfo + ) -> Self: + """Check if 'immutable' attributes have been mutated in replacement requests.""" + from scim2_models.rfc7643.resource import Resource + + value = handler(value) + + context = info.context.get("scim") if info.context else None + original = info.context.get("original") if info.context else None + if ( + context == Context.RESOURCE_REPLACEMENT_REQUEST + and issubclass(cls, Resource) + and original is not None + ): + cls.check_mutability_issues(original, value) + return value + + @classmethod + def check_mutability_issues(cls, original: "BaseModel", replacement: "BaseModel"): + """Compare two instances, and check for differences of values on the fields marked as immutable.""" + model = replacement.__class__ + for field_name in model.model_fields: + mutability = model.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}, + ) + + attr_type = model.get_field_root_type(field_name) + if is_complex_attribute(attr_type) and not model.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) + def mark_with_schema(self): - """Navigate through attributes and sub-attributes of type ComplexAttribute, and mark them with a '_schema' attribute. '_schema' will later be used by 'get_attribute_urn'.""" + """Navigate through attributes and sub-attributes of type ComplexAttribute, and mark them with a '_schema' attribute. + + '_schema' will later be used by 'get_attribute_urn'. + """ from scim2_models.rfc7643.resource import Resource for field_name in self.model_fields: @@ -653,7 +694,8 @@ def scim_request_serializer(self, value: Any, info: SerializationInfo) -> Any: scim_ctx = info.context.get("scim") if info.context else None if ( - scim_ctx == Context.RESOURCE_CREATION_REQUEST + scim_ctx + in (Context.RESOURCE_CREATION_REQUEST, Context.RESOURCE_REPLACEMENT_REQUEST) and mutability == Mutability.read_only ): return None @@ -668,12 +710,6 @@ def scim_request_serializer(self, value: Any, info: SerializationInfo) -> Any: ): return None - if scim_ctx == Context.RESOURCE_REPLACEMENT_REQUEST and mutability in ( - Mutability.immutable, - Mutability.read_only, - ): - return None - return value def scim_response_serializer(self, value: Any, info: SerializationInfo) -> Any: @@ -719,10 +755,28 @@ def model_serializer_exclude_none( @classmethod def model_validate( - cls, *args, scim_ctx: Optional[Context] = Context.DEFAULT, **kwargs + cls, + *args, + scim_ctx: Optional[Context] = Context.DEFAULT, + original: Optional["BaseModel"] = None, + **kwargs, ) -> Self: - """Validate SCIM payloads and generate model representation by using Pydantic :code:`BaseModel.model_validate`.""" - kwargs.setdefault("context", {}).setdefault("scim", scim_ctx) + """Validate SCIM payloads and generate model representation by using Pydantic :code:`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. + """ + 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 _prepare_model_dump( diff --git a/tests/test_model_serialization.py b/tests/test_model_serialization.py index d3572fb..458acae 100644 --- a/tests/test_model_serialization.py +++ b/tests/test_model_serialization.py @@ -151,6 +151,7 @@ def test_dump_replacement_request(mut_resource): "schemas": ["org:example:MutResource"], "readWrite": "x", "writeOnly": "x", + "immutable": "x", } diff --git a/tests/test_model_validation.py b/tests/test_model_validation.py index d54a7e8..62bdc76 100644 --- a/tests/test_model_validation.py +++ b/tests/test_model_validation.py @@ -4,6 +4,7 @@ import pytest from pydantic import ValidationError +from scim2_models.base import ComplexAttribute from scim2_models.base import Context from scim2_models.base import Mutability from scim2_models.base import Required @@ -144,31 +145,113 @@ def test_validate_replacement_request_mutability(): """Test query validation for resource model replacement requests. Attributes marked as: - - Mutability.immutable raise a ValidationError + - 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( + { + "readOnly": "x", + "readWrite": "x", + "writeOnly": "x", + }, + scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + ) + + 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' has mutability 'immutable' but this in not valid in resource replacement request context", + match="Field 'immutable' is immutable but the request value is different than the original value.", ): 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. + + Sub-attributes marked as: + - Mutability.immutable raise a ValidationError if different than the 'original' item. + - Mutability.read_only are ignored + """ + + class Sub(ComplexAttribute): + immutable: Annotated[Optional[str], Mutability.immutable] = None + + class Super(Resource): + schemas: Annotated[list[str], Required.true] = ["org:example:Super"] + sub: Optional[Sub] = 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.raises( + ValidationError, + match="Field 'immutable' is immutable but the request value is different than the original value.", + ): + Super.model_validate( + { + "sub": { + "immutable": "x", + } + }, + scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, + original=original, ) @@ -378,12 +461,14 @@ 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", @@ -395,6 +480,7 @@ def test_validate_creation_and_replacement_request_necessity(context): "required": "x", }, scim_ctx=context, + original=original, ) == ReqResource( schemas=["org:example:ReqResource"], required="x", @@ -408,6 +494,7 @@ def test_validate_creation_and_replacement_request_necessity(context): { "optional": "x", }, + original=original, scim_ctx=context, ) diff --git a/tests/test_patch_op.py b/tests/test_patch_op.py index b249147..e3fdf8f 100644 --- a/tests/test_patch_op.py +++ b/tests/test_patch_op.py @@ -3,7 +3,6 @@ from scim2_models import PatchOp from scim2_models import PatchOperation -from scim2_models.base import Context def test_validate_patchop_case_insensitivith(): @@ -16,7 +15,6 @@ def test_validate_patchop_case_insensitivith(): {"op": "ReMove", "path": "userName", "value": "Rivard"}, ], }, - scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, ) == PatchOp( operations=[ PatchOperation( @@ -36,5 +34,4 @@ def test_validate_patchop_case_insensitivith(): { "operations": [{"op": 42, "path": "userName", "value": "Rivard"}], }, - scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST, ) diff --git a/uv.lock b/uv.lock index d1caf89..98bed61 100644 --- a/uv.lock +++ b/uv.lock @@ -873,6 +873,7 @@ doc = [ { name = "shibuya" }, { name = "sphinx" }, { name = "sphinx-issues" }, + { name = "sphinx-paramlinks" }, { name = "sphinx-togglebutton" }, ] @@ -893,6 +894,7 @@ doc = [ { name = "shibuya", specifier = ">=2024.5.15" }, { name = "sphinx", specifier = ">=7.3.7" }, { name = "sphinx-issues", specifier = ">=5.0.0" }, + { name = "sphinx-paramlinks", specifier = ">=0.6.0" }, { name = "sphinx-togglebutton", specifier = ">=0.3.2" }, ] @@ -967,6 +969,16 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/1d/14/a1b38212db8d327f53f2f56f0ed81c8b80ee2c2160e8819069c0d329d0a9/sphinx_issues-5.0.0-py3-none-any.whl", hash = "sha256:d80704a01c8af3d76586771a67a9e48f2d1a6091a0377458c49908460a6a31ea", size = 8063 }, ] +[[package]] +name = "sphinx-paramlinks" +version = "0.6.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "docutils" }, + { name = "sphinx" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/ae/21/62d3a58ff7bd02bbb9245a63d1f0d2e0455522a11a78951d16088569fca8/sphinx-paramlinks-0.6.0.tar.gz", hash = "sha256:746a0816860aa3fff5d8d746efcbec4deead421f152687411db1d613d29f915e", size = 12363 } + [[package]] name = "sphinx-togglebutton" version = "0.3.2"