diff --git a/doc/changelog.rst b/doc/changelog.rst index dc18d1c..3aed4af 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -1,6 +1,22 @@ Changelog ========= +[0.7.6] - Unreleased +-------------------- + +Added +^^^^^ +- Support for ETags: ``replace``, ``modify`` and ``delete`` automatically send + an ``If-Match`` header when the server advertises ETag support and the resource + has a ``meta.version``. :issue:`47` + +Breaking changes +^^^^^^^^^^^^^^^^ +- ``delete`` now takes a resource instance instead of a resource type and id. + :issue:`13` +- ``modify`` now takes a resource instance and a patch operation instead of a + resource type, id and patch operation. :issue:`13` + [0.7.5] - 2026-04-02 -------------------- diff --git a/doc/tutorial.rst b/doc/tutorial.rst index 31d8884..059eab8 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -124,7 +124,8 @@ The :meth:`~scim2_client.BaseSyncSCIMClient.modify` method allows you to perform patch_op = PatchOp[User](operations=[operation]) # Apply the patch - response = scim.modify(User, user_id, patch_op) + user = scim.query(User, user_id) + response = scim.modify(user, patch_op) if response: # Server returned 200 with updated resource print(f"User updated: {response.display_name}") else: # Server returned 204 (no content) @@ -155,7 +156,7 @@ You can include multiple operations in a single PATCH request: ) ] patch_op = PatchOp[User](operations=operations) - response = scim.modify(User, user_id, patch_op) + response = scim.modify(user, patch_op) Patch Operation Types ~~~~~~~~~~~~~~~~~~~~~ @@ -201,6 +202,39 @@ To achieve this, all the methods provide the following parameters, all are :data which value will excluded from the request payload, and which values are expected in the response payload. +Resource versioning (ETags) +========================== + +SCIM supports resource versioning through HTTP ETags +(:rfc:`RFC 7644 §3.14 <7644#section-3.14>`). +When the server advertises ETag support in its +:class:`~scim2_models.ServiceProviderConfig`, scim2-client automatically sends +an ``If-Match`` header on write operations +(:meth:`~scim2_client.BaseSyncSCIMClient.replace`, +:meth:`~scim2_client.BaseSyncSCIMClient.modify`, +:meth:`~scim2_client.BaseSyncSCIMClient.delete`) +using the :attr:`meta.version ` value from the resource. + +This enables optimistic concurrency control: the server will reject the request +with ``412 Precondition Failed`` if the resource has been modified since it was +last read. + +.. code-block:: python + + # Read a resource — meta.version is populated by the server + user = scim.query(User, user_id) + + # Modify it — If-Match is sent automatically + user.display_name = "Updated Name" + updated_user = scim.replace(user) + + # Delete it — If-Match is sent automatically + scim.delete(user) + +No additional configuration is needed. If the server does not advertise ETag +support, or if the resource has no :attr:`meta.version `, no +``If-Match`` header is sent. + Engines ======= diff --git a/scim2_client/client.py b/scim2_client/client.py index 76bb2be..bc5a488 100644 --- a/scim2_client/client.py +++ b/scim2_client/client.py @@ -195,6 +195,19 @@ def __init__( self.check_response_status_codes = check_response_status_codes self.raise_scim_errors = raise_scim_errors + @property + def _etag_supported(self) -> bool: + spc = self.service_provider_config + return bool(spc and spc.etag and spc.etag.supported) + + def _set_if_match(self, req: RequestPayload, resource: Resource) -> None: + """Add ``If-Match`` header to the request if the server supports ETags.""" + if not self._etag_supported: + return + if resource.meta and resource.meta.version: + headers = req.request_kwargs.setdefault("headers", {}) + headers.setdefault("If-Match", resource.meta.version) + def get_resource_model(self, name: str) -> type[Resource] | None: """Get a registered model by its name or its schema.""" for resource_model in self.resource_models: @@ -515,8 +528,7 @@ def _prepare_search_request( def _prepare_delete_request( self, - resource_model: type[Resource], - id: str, + resource: Resource, expected_status_codes: list[int] | None = None, **kwargs, ) -> RequestPayload: @@ -525,9 +537,13 @@ def _prepare_delete_request( request_kwargs=kwargs, ) + resource_model = type(resource) self._check_resource_model(resource_model) - delete_url = self.resource_endpoint(resource_model) + f"/{id}" + if not resource.id: + raise SCIMRequestError("Resource must have an id", source=resource) + delete_url = self.resource_endpoint(resource_model) + f"/{resource.id}" req.url = req.request_kwargs.pop("url", delete_url) + self._set_if_match(req, resource) return req def _prepare_replace_request( @@ -575,6 +591,7 @@ def _prepare_replace_request( raise SCIMRequestError("Resource must have an id", source=resource) req.expected_types = [resource.__class__] + self._set_if_match(req, resource) req.payload = resource.model_dump( scim_ctx=Context.RESOURCE_REPLACEMENT_REQUEST ) @@ -586,29 +603,15 @@ def _prepare_replace_request( def _prepare_patch_request( self, - resource_model: type[ResourceT], - id: str, - patch_op: PatchOp[ResourceT] | dict, + resource: Resource, + patch_op: PatchOp | dict, check_request_payload: bool | None = None, expected_status_codes: list[int] | None = None, **kwargs, ) -> RequestPayload: - """Prepare a PATCH request payload. - - :param resource_model: The resource type to modify (e.g., User, Group). - :param id: The resource ID. - :param patch_op: A PatchOp instance parameterized with the same resource type as resource_model - (e.g., PatchOp[User] when resource_model is User), or a dict representation. - :param check_request_payload: If :data:`False`, :code:`patch_op` is expected to be a dict - that will be passed as-is in the request. This value can be - overwritten in methods. - :param expected_status_codes: List of HTTP status codes expected for this request. - :param raise_scim_errors: If :data:`True` and the server returned an - :class:`~scim2_models.Error` object during a request, a - :class:`~scim2_client.SCIMResponseErrorObject` exception will be raised. - :param kwargs: Additional request parameters. - :return: The prepared request payload. - """ + """Prepare a PATCH request payload.""" + resource_model = type(resource) + id = resource.id req = RequestPayload( expected_status_codes=expected_status_codes, request_kwargs=kwargs, @@ -644,12 +647,12 @@ def _prepare_patch_request( ) req.expected_types = [resource_model] + self._set_if_match(req, resource) return req def modify( self, - resource_model: type[ResourceT], - id: str, + resource: ResourceT, patch_op: PatchOp[ResourceT] | dict, **kwargs, ) -> ResourceT | Error | dict | None: @@ -858,18 +861,19 @@ def search( def delete( self, - resource_model: type, - id: str, + resource: Resource, check_response_payload: bool | None = None, expected_status_codes: list[int] | None = SCIMClient.DELETION_RESPONSE_STATUS_CODES, raise_scim_errors: bool | None = None, **kwargs, ) -> Error | dict | None: - """Perform a DELETE request to create, as defined in :rfc:`RFC7644 §3.6 <7644#section-3.6>`. + """Perform a DELETE request, as defined in :rfc:`RFC7644 §3.6 <7644#section-3.6>`. - :param resource_model: The type of the resource to delete. - :param id: The type id the resource to delete. + :param resource: The resource to delete. If the server supports ETags + and the resource has ``meta.version``, an ``If-Match`` header is sent. + :param resource_model: Deprecated. The type of the resource to delete. + :param id: Deprecated. The id of the resource to delete. :param check_response_payload: If set, overwrites :paramref:`scim2_client.SCIMClient.check_response_payload`. :param expected_status_codes: The list of expected status codes form the response. If :data:`None` any status code is accepted. @@ -884,11 +888,10 @@ def delete( :usage: .. code-block:: python - :caption: Deleting an `User` which `id` is `foobar` + :caption: Deleting a User resource - from scim2_models import User, SearchRequest - - response = scim.delete(User, "foobar") + user = scim.query(User, "foobar") + response = scim.delete(resource=user) # 'response' may be None, or an Error object """ raise NotImplementedError() @@ -941,8 +944,7 @@ def replace( def modify( self, - resource_model: type[ResourceT], - id: str, + resource: ResourceT, patch_op: PatchOp[ResourceT] | dict, check_request_payload: bool | None = None, check_response_payload: bool | None = None, @@ -953,11 +955,11 @@ def modify( ) -> ResourceT | Error | dict | None: """Perform a PATCH request to modify a resource, as defined in :rfc:`RFC7644 §3.5.2 <7644#section-3.5.2>`. - :param resource_model: The type of the resource to modify. - :param id: The id of the resource to modify. + :param resource: The resource to modify. If the server supports ETags + and the resource has ``meta.version``, an ``If-Match`` header is sent. :param patch_op: The :class:`~scim2_models.PatchOp` object describing the modifications. - Must be parameterized with the same resource type as ``resource_model`` - (e.g., :code:`PatchOp[User]` when ``resource_model`` is :code:`User`). + :param resource_model: Deprecated. The type of the resource to modify. + :param id: Deprecated. The id of the resource to modify. :param check_request_payload: If set, overwrites :paramref:`scim2_client.SCIMClient.check_request_payload`. :param check_response_payload: If set, overwrites :paramref:`scim2_client.SCIMClient.check_response_payload`. :param expected_status_codes: The list of expected status codes form the response. @@ -978,11 +980,12 @@ def modify( from scim2_models import User, PatchOp, PatchOperation + user = scim.query(User, "my-user-id") operation = PatchOperation( op="replace", path="displayName", value="New Display Name" ) patch_op = PatchOp[User](operations=[operation]) - response = scim.modify(User, "my-user-id", patch_op) + response = scim.modify(resource=user, patch_op=patch_op) # 'response' may be a User, None, or an Error object .. tip:: @@ -1193,18 +1196,19 @@ async def search( async def delete( self, - resource_model: type, - id: str, + resource: Resource, check_response_payload: bool | None = None, expected_status_codes: list[int] | None = SCIMClient.DELETION_RESPONSE_STATUS_CODES, raise_scim_errors: bool | None = None, **kwargs, ) -> Error | dict | None: - """Perform a DELETE request to create, as defined in :rfc:`RFC7644 §3.6 <7644#section-3.6>`. + """Perform a DELETE request, as defined in :rfc:`RFC7644 §3.6 <7644#section-3.6>`. - :param resource_model: The type of the resource to delete. - :param id: The type id the resource to delete. + :param resource: The resource to delete. If the server supports ETags + and the resource has ``meta.version``, an ``If-Match`` header is sent. + :param resource_model: Deprecated. The type of the resource to delete. + :param id: Deprecated. The id of the resource to delete. :param check_response_payload: If set, overwrites :paramref:`scim2_client.SCIMClient.check_response_payload`. :param expected_status_codes: The list of expected status codes form the response. If :data:`None` any status code is accepted. @@ -1219,11 +1223,10 @@ async def delete( :usage: .. code-block:: python - :caption: Deleting an `User` which `id` is `foobar` - - from scim2_models import User, SearchRequest + :caption: Deleting a User resource - response = scim.delete(User, "foobar") + user = scim.query(User, "foobar") + response = await scim.delete(resource=user) # 'response' may be None, or an Error object """ raise NotImplementedError() @@ -1276,8 +1279,7 @@ async def replace( async def modify( self, - resource_model: type[ResourceT], - id: str, + resource: ResourceT, patch_op: PatchOp[ResourceT] | dict, check_request_payload: bool | None = None, check_response_payload: bool | None = None, @@ -1288,11 +1290,11 @@ async def modify( ) -> ResourceT | Error | dict | None: """Perform a PATCH request to modify a resource, as defined in :rfc:`RFC7644 §3.5.2 <7644#section-3.5.2>`. - :param resource_model: The type of the resource to modify. - :param id: The id of the resource to modify. + :param resource: The resource to modify. If the server supports ETags + and the resource has ``meta.version``, an ``If-Match`` header is sent. :param patch_op: The :class:`~scim2_models.PatchOp` object describing the modifications. - Must be parameterized with the same resource type as ``resource_model`` - (e.g., :code:`PatchOp[User]` when ``resource_model`` is :code:`User`). + :param resource_model: Deprecated. The type of the resource to modify. + :param id: Deprecated. The id of the resource to modify. :param check_request_payload: If set, overwrites :paramref:`scim2_client.SCIMClient.check_request_payload`. :param check_response_payload: If set, overwrites :paramref:`scim2_client.SCIMClient.check_response_payload`. :param expected_status_codes: The list of expected status codes form the response. @@ -1313,11 +1315,12 @@ async def modify( from scim2_models import User, PatchOp, PatchOperation + user = scim.query(User, "my-user-id") operation = PatchOperation( op="replace", path="displayName", value="New Display Name" ) patch_op = PatchOp[User](operations=[operation]) - response = await scim.modify(User, "my-user-id", patch_op) + response = await scim.modify(resource=user, patch_op=patch_op) # 'response' may be a User, None, or an Error object .. tip:: diff --git a/scim2_client/engines/httpx.py b/scim2_client/engines/httpx.py index 874c428..4ea2a8e 100644 --- a/scim2_client/engines/httpx.py +++ b/scim2_client/engines/httpx.py @@ -178,8 +178,7 @@ def search( def delete( self, - resource_model: type[Resource], - id: str, + resource: Resource, check_response_payload: bool | None = None, expected_status_codes: list[int] | None = BaseSyncSCIMClient.DELETION_RESPONSE_STATUS_CODES, @@ -187,8 +186,7 @@ def delete( **kwargs, ) -> Error | dict | None: req = self._prepare_delete_request( - resource_model=resource_model, - id=id, + resource=resource, expected_status_codes=expected_status_codes, **kwargs, ) @@ -240,8 +238,7 @@ def replace( def modify( self, - resource_model: type[ResourceT], - id: str, + resource: ResourceT, patch_op: PatchOp[ResourceT] | dict, check_request_payload: bool | None = None, check_response_payload: bool | None = None, @@ -251,8 +248,7 @@ def modify( **kwargs, ) -> ResourceT | Error | dict | None: req = self._prepare_patch_request( - resource_model=resource_model, - id=id, + resource=resource, patch_op=patch_op, check_request_payload=check_request_payload, expected_status_codes=expected_status_codes, @@ -410,8 +406,7 @@ async def search( async def delete( self, - resource_model: type[Resource], - id: str, + resource: Resource, check_response_payload: bool | None = None, expected_status_codes: list[int] | None = BaseAsyncSCIMClient.DELETION_RESPONSE_STATUS_CODES, @@ -419,8 +414,7 @@ async def delete( **kwargs, ) -> Error | dict | None: req = self._prepare_delete_request( - resource_model=resource_model, - id=id, + resource=resource, expected_status_codes=expected_status_codes, **kwargs, ) @@ -474,8 +468,7 @@ async def replace( async def modify( self, - resource_model: type[ResourceT], - id: str, + resource: ResourceT, patch_op: PatchOp[ResourceT] | dict, check_request_payload: bool | None = None, check_response_payload: bool | None = None, @@ -485,8 +478,7 @@ async def modify( **kwargs, ) -> ResourceT | Error | dict | None: req = self._prepare_patch_request( - resource_model=resource_model, - id=id, + resource=resource, patch_op=patch_op, check_request_payload=check_request_payload, expected_status_codes=expected_status_codes, diff --git a/scim2_client/engines/werkzeug.py b/scim2_client/engines/werkzeug.py index 3d4dad6..9c963c4 100644 --- a/scim2_client/engines/werkzeug.py +++ b/scim2_client/engines/werkzeug.py @@ -215,8 +215,7 @@ def search( def delete( self, - resource_model: type[Resource], - id: str, + resource: Resource, check_response_payload: bool | None = None, expected_status_codes: list[int] | None = BaseSyncSCIMClient.DELETION_RESPONSE_STATUS_CODES, @@ -224,8 +223,7 @@ def delete( **kwargs, ) -> Error | dict | None: req = self._prepare_delete_request( - resource_model=resource_model, - id=id, + resource=resource, expected_status_codes=expected_status_codes, **kwargs, ) @@ -277,8 +275,7 @@ def replace( def modify( self, - resource_model: type[ResourceT], - id: str, + resource: ResourceT, patch_op: PatchOp[ResourceT] | dict, check_request_payload: bool | None = None, check_response_payload: bool | None = None, @@ -288,8 +285,7 @@ def modify( **kwargs, ) -> ResourceT | Error | dict | None: req = self._prepare_patch_request( - resource_model=resource_model, - id=id, + resource=resource, patch_op=patch_op, check_request_payload=check_request_payload, expected_status_codes=expected_status_codes, diff --git a/tests/conftest.py b/tests/conftest.py index ebcbab7..2f3af09 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,5 @@ +import uuid + import pytest from httpx import Client from scim2_models import Group @@ -15,3 +17,17 @@ def sync_client(httpserver): ) scim_client.register_naive_resource_types() yield scim_client + + +@pytest.fixture +def user(): + u = User(user_name="bjensen@example.com") + u.id = str(uuid.uuid4()) + return u + + +@pytest.fixture +def group(): + g = Group(display_name="Tour Guides") + g.id = str(uuid.uuid4()) + return g diff --git a/tests/engines/test_httpx.py b/tests/engines/test_httpx.py index 8d001a7..8d65f3a 100644 --- a/tests/engines/test_httpx.py +++ b/tests/engines/test_httpx.py @@ -86,13 +86,13 @@ def test_sync_engine(server): op=PatchOperation.Op.replace_, path="displayName", value="patched name" ) patch_op = PatchOp[User](operations=[operation]) - scim_client.modify(User, response_user.id, patch_op) + scim_client.modify(response_user, patch_op) # Verify patch result with query queried_user = scim_client.query(User, response_user.id) assert queried_user.display_name == "patched name" - scim_client.delete(User, response_user.id) + scim_client.delete(response_user) with pytest.raises(SCIMResponseErrorObject): scim_client.query(User, response_user.id) @@ -145,12 +145,12 @@ async def test_async_engine(server): op=PatchOperation.Op.replace_, path="displayName", value="async patched name" ) patch_op = PatchOp[User](operations=[operation]) - await scim_client.modify(User, response_user.id, patch_op) + await scim_client.modify(response_user, patch_op) # Verify patch result with query queried_user = await scim_client.query(User, response_user.id) assert queried_user.display_name == "async patched name" - await scim_client.delete(User, response_user.id) + await scim_client.delete(response_user) with pytest.raises(SCIMResponseErrorObject): await scim_client.query(User, response_user.id) diff --git a/tests/engines/test_werkzeug.py b/tests/engines/test_werkzeug.py index 1cd07bb..d45ba65 100644 --- a/tests/engines/test_werkzeug.py +++ b/tests/engines/test_werkzeug.py @@ -67,13 +67,13 @@ def test_werkzeug_engine(scim_client): op=PatchOperation.Op.replace_, path="displayName", value="werkzeug patched" ) patch_op = PatchOp[User](operations=[operation]) - scim_client.modify(User, response_user.id, patch_op) + scim_client.modify(response_user, patch_op) # Verify patch result with query queried_user = scim_client.query(User, response_user.id) assert queried_user.display_name == "werkzeug patched" - scim_client.delete(User, response_user.id) + scim_client.delete(response_user) with pytest.raises(SCIMResponseErrorObject): scim_client.query(User, response_user.id) diff --git a/tests/test_delete.py b/tests/test_delete.py index 465e3d5..f72f516 100644 --- a/tests/test_delete.py +++ b/tests/test_delete.py @@ -11,32 +11,30 @@ class UnregisteredResource(Resource): __schema__ = "urn:test:schemas:UnregisteredResource" -def test_delete_user(httpserver, sync_client): +def test_delete_user(httpserver, sync_client, user): """Nominal case for a User deletion.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="DELETE" - ).respond_with_data(status=204, content_type="application/scim+json") + httpserver.expect_request(f"/Users/{user.id}", method="DELETE").respond_with_data( + status=204, content_type="application/scim+json" + ) - response = sync_client.delete(User, "2819c223-7f76-453a-919d-413861904646") + response = sync_client.delete(user) assert response is None -def test_delete_user_without_content_type_header(httpserver, sync_client): +def test_delete_user_without_content_type_header(httpserver, sync_client, user): """Server returns 204 without Content-Type header, which is valid per RFC 7231.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="DELETE" - ).respond_with_data(status=204) + httpserver.expect_request(f"/Users/{user.id}", method="DELETE").respond_with_data( + status=204 + ) - response = sync_client.delete(User, "2819c223-7f76-453a-919d-413861904646") + response = sync_client.delete(user) assert response is None @pytest.mark.parametrize("code", [400, 401, 403, 404, 412, 500, 501]) -def test_errors(httpserver, code, sync_client): +def test_errors(httpserver, code, sync_client, user): """Test error cases defined in RFC7644.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="DELETE" - ).respond_with_json( + httpserver.expect_request(f"/Users/{user.id}", method="DELETE").respond_with_json( { "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], "status": str(code), @@ -45,9 +43,7 @@ def test_errors(httpserver, code, sync_client): status=code, ) - response = sync_client.delete( - User, "2819c223-7f76-453a-919d-413861904646", raise_scim_errors=False - ) + response = sync_client.delete(user, raise_scim_errors=False) assert response == Error( schemas=["urn:ietf:params:scim:api:messages:2.0:Error"], @@ -56,17 +52,24 @@ def test_errors(httpserver, code, sync_client): ) +def test_delete_resource_without_id(sync_client): + """Deleting a resource without an id raises an error.""" + no_id_user = User(user_name="no-id") + with pytest.raises(SCIMRequestError, match="Resource must have an id"): + sync_client.delete(no_id_user) + + def test_invalid_resource_model(httpserver, sync_client): """Test that resource_models passed to the method must be part of SCIMClient.resource_models.""" + unregistered = UnregisteredResource() + unregistered.id = "foobar" with pytest.raises(SCIMRequestError, match=r"Unknown resource type"): - sync_client.delete(UnregisteredResource, id="foobar") + sync_client.delete(unregistered) -def test_dont_check_response_payload(httpserver, sync_client): +def test_dont_check_response_payload(httpserver, sync_client, user): """Test the check_response_payload attribute.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="DELETE" - ).respond_with_json( + httpserver.expect_request(f"/Users/{user.id}", method="DELETE").respond_with_json( { "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], "status": "404", @@ -75,9 +78,7 @@ def test_dont_check_response_payload(httpserver, sync_client): status=404, ) - response = sync_client.delete( - User, "2819c223-7f76-453a-919d-413861904646", check_response_payload=False - ) + response = sync_client.delete(user, check_response_payload=False) assert response == { "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], "status": "404", @@ -85,9 +86,9 @@ def test_dont_check_response_payload(httpserver, sync_client): } -def test_request_network_error(httpserver, sync_client): +def test_request_network_error(httpserver, sync_client, user): """Test that httpx exceptions are transformed in RequestNetworkError.""" with pytest.raises( RequestNetworkError, match="Network error happened during request" ): - sync_client.delete(User, "anything", url="http://invalid.test") + sync_client.delete(user, url="http://invalid.test") diff --git a/tests/test_modify.py b/tests/test_modify.py index cef4956..65214db 100644 --- a/tests/test_modify.py +++ b/tests/test_modify.py @@ -11,14 +11,12 @@ from scim2_client import SCIMRequestError -def test_modify_user_200(httpserver, sync_client): +def test_modify_user_200(httpserver, sync_client, user): """Nominal case for a User modification with 200 response (resource returned).""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_json( + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_json( { "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], - "id": "2819c223-7f76-453a-919d-413861904646", + "id": user.id, "userName": "bjensen@example.com", "displayName": "Updated Display Name", "meta": { @@ -26,7 +24,7 @@ def test_modify_user_200(httpserver, sync_client): "created": "2010-01-23T04:56:22Z", "lastModified": "2011-05-13T04:42:34Z", "version": 'W\\/"3694e05e9dff590"', - "location": "https://example.com/v2/Users/2819c223-7f76-453a-919d-413861904646", + "location": f"https://example.com/v2/Users/{user.id}", }, }, status=200, @@ -38,20 +36,16 @@ def test_modify_user_200(httpserver, sync_client): ) patch_op = PatchOp[User](operations=[operation]) - response = sync_client.modify( - User, "2819c223-7f76-453a-919d-413861904646", patch_op - ) + response = sync_client.modify(user, patch_op) assert isinstance(response, User) - assert response.id == "2819c223-7f76-453a-919d-413861904646" + assert response.id == user.id assert response.display_name == "Updated Display Name" -def test_modify_user_204(httpserver, sync_client): +def test_modify_user_204(httpserver, sync_client, user): """Nominal case for a User modification with 204 response (no content).""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_data( + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_data( "", status=204, content_type="application/scim+json", @@ -62,18 +56,14 @@ def test_modify_user_204(httpserver, sync_client): ) patch_op = PatchOp[User](operations=[operation]) - response = sync_client.modify( - User, "2819c223-7f76-453a-919d-413861904646", patch_op - ) + response = sync_client.modify(user, patch_op) assert response is None -def test_modify_user_204_without_content_type_header(httpserver, sync_client): +def test_modify_user_204_without_content_type_header(httpserver, sync_client, user): """Server returns 204 without Content-Type header, which is valid per RFC 7231.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_data( + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_data( "", status=204, ) @@ -83,21 +73,17 @@ def test_modify_user_204_without_content_type_header(httpserver, sync_client): ) patch_op = PatchOp[User](operations=[operation]) - response = sync_client.modify( - User, "2819c223-7f76-453a-919d-413861904646", patch_op - ) + response = sync_client.modify(user, patch_op) assert response is None -def test_modify_user_multiple_operations(httpserver, sync_client): +def test_modify_user_multiple_operations(httpserver, sync_client, user): """Test User modification with multiple patch operations.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_json( + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_json( { "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], - "id": "2819c223-7f76-453a-919d-413861904646", + "id": user.id, "userName": "bjensen@example.com", "displayName": "Betty Jane", "active": False, @@ -106,7 +92,7 @@ def test_modify_user_multiple_operations(httpserver, sync_client): "created": "2010-01-23T04:56:22Z", "lastModified": "2011-05-13T04:42:34Z", "version": 'W\\/"3694e05e9dff591"', - "location": "https://example.com/v2/Users/2819c223-7f76-453a-919d-413861904646", + "location": f"https://example.com/v2/Users/{user.id}", }, }, status=200, @@ -121,23 +107,19 @@ def test_modify_user_multiple_operations(httpserver, sync_client): ] patch_op = PatchOp[User](operations=operations) - response = sync_client.modify( - User, "2819c223-7f76-453a-919d-413861904646", patch_op - ) + response = sync_client.modify(user, patch_op) assert isinstance(response, User) assert response.display_name == "Betty Jane" assert response.active is False -def test_modify_user_add_operation(httpserver, sync_client): +def test_modify_user_add_operation(httpserver, sync_client, user): """Test User modification with add operation.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_json( + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_json( { "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], - "id": "2819c223-7f76-453a-919d-413861904646", + "id": user.id, "userName": "bjensen@example.com", "emails": [{"value": "bjensen@example.com", "primary": True}], "meta": { @@ -145,7 +127,7 @@ def test_modify_user_add_operation(httpserver, sync_client): "created": "2010-01-23T04:56:22Z", "lastModified": "2011-05-13T04:42:34Z", "version": 'W\\/"3694e05e9dff591"', - "location": "https://example.com/v2/Users/2819c223-7f76-453a-919d-413861904646", + "location": f"https://example.com/v2/Users/{user.id}", }, }, status=200, @@ -159,30 +141,26 @@ def test_modify_user_add_operation(httpserver, sync_client): ) patch_op = PatchOp[User](operations=[operation]) - response = sync_client.modify( - User, "2819c223-7f76-453a-919d-413861904646", patch_op - ) + response = sync_client.modify(user, patch_op) assert isinstance(response, User) assert len(response.emails) == 1 assert response.emails[0].value == "bjensen@example.com" -def test_modify_user_remove_operation(httpserver, sync_client): +def test_modify_user_remove_operation(httpserver, sync_client, user): """Test User modification with remove operation.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_json( + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_json( { "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], - "id": "2819c223-7f76-453a-919d-413861904646", + "id": user.id, "userName": "bjensen@example.com", "meta": { "resourceType": "User", "created": "2010-01-23T04:56:22Z", "lastModified": "2011-05-13T04:42:34Z", "version": 'W\\/"3694e05e9dff591"', - "location": "https://example.com/v2/Users/2819c223-7f76-453a-919d-413861904646", + "location": f"https://example.com/v2/Users/{user.id}", }, }, status=200, @@ -192,29 +170,25 @@ def test_modify_user_remove_operation(httpserver, sync_client): operation = PatchOperation(op=PatchOperation.Op.remove, path="displayName") patch_op = PatchOp[User](operations=[operation]) - response = sync_client.modify( - User, "2819c223-7f76-453a-919d-413861904646", patch_op - ) + response = sync_client.modify(user, patch_op) assert isinstance(response, User) assert response.display_name is None -def test_modify_group(httpserver, sync_client): +def test_modify_group(httpserver, sync_client, group): """Test Group modification.""" - httpserver.expect_request( - "/Groups/e9e30dba-f08f-4109-8486-d5c6a331660a", method="PATCH" - ).respond_with_json( + httpserver.expect_request(f"/Groups/{group.id}", method="PATCH").respond_with_json( { "schemas": ["urn:ietf:params:scim:schemas:core:2.0:Group"], - "id": "e9e30dba-f08f-4109-8486-d5c6a331660a", + "id": group.id, "displayName": "Updated Tour Guides", "meta": { "resourceType": "Group", "created": "2010-01-23T04:56:22Z", "lastModified": "2011-05-13T04:42:34Z", "version": 'W\\/"3694e05e9dff592"', - "location": "https://example.com/v2/Groups/e9e30dba-f08f-4109-8486-d5c6a331660a", + "location": f"https://example.com/v2/Groups/{group.id}", }, }, status=200, @@ -226,19 +200,17 @@ def test_modify_group(httpserver, sync_client): ) patch_op = PatchOp[Group](operations=[operation]) - response = sync_client.modify( - Group, "e9e30dba-f08f-4109-8486-d5c6a331660a", patch_op - ) + response = sync_client.modify(group, patch_op) assert isinstance(response, Group) assert response.display_name == "Updated Tour Guides" -def test_dont_check_response_payload(httpserver, sync_client): +def test_dont_check_response_payload(httpserver, sync_client, user): """Test the check_response_payload attribute.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_json({"foo": "bar"}, status=200) + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_json( + {"foo": "bar"}, status=200 + ) operation = PatchOperation( op=PatchOperation.Op.replace_, path="displayName", value="Test" @@ -246,22 +218,19 @@ def test_dont_check_response_payload(httpserver, sync_client): patch_op = PatchOp[User](operations=[operation]) response = sync_client.modify( - User, - "2819c223-7f76-453a-919d-413861904646", + user, patch_op, check_response_payload=False, ) assert response == {"foo": "bar"} -def test_dont_check_request_payload(httpserver, sync_client): +def test_dont_check_request_payload(httpserver, sync_client, user): """Test the check_request_payload attribute.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_json( + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_json( { "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], - "id": "2819c223-7f76-453a-919d-413861904646", + "id": user.id, "userName": "bjensen@example.com", "displayName": "Updated Name", }, @@ -277,21 +246,18 @@ def test_dont_check_request_payload(httpserver, sync_client): } response = sync_client.modify( - User, - "2819c223-7f76-453a-919d-413861904646", + user, patch_op_dict, check_request_payload=False, ) - assert response.id == "2819c223-7f76-453a-919d-413861904646" + assert response.id == user.id assert response.display_name == "Updated Name" @pytest.mark.parametrize("code", [400, 401, 403, 404, 409, 412, 500, 501]) -def test_errors(httpserver, code, sync_client): +def test_errors(httpserver, code, sync_client, user): """Test error cases defined in RFC7644.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_json( + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_json( { "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], "status": str(code), @@ -306,9 +272,7 @@ def test_errors(httpserver, code, sync_client): ) patch_op = PatchOp[User](operations=[operation]) - response = sync_client.modify( - User, "2819c223-7f76-453a-919d-413861904646", patch_op, raise_scim_errors=False - ) + response = sync_client.modify(user, patch_op, raise_scim_errors=False) assert response == Error( schemas=["urn:ietf:params:scim:api:messages:2.0:Error"], @@ -317,7 +281,7 @@ def test_errors(httpserver, code, sync_client): ) -def test_invalid_resource_model(httpserver, sync_client): +def test_invalid_resource_model(httpserver, sync_client, group): """Test that resource_models passed to the method must be part of SCIMClient.resource_models.""" sync_client.resource_models = (User,) sync_client.resource_types = [ResourceType.from_resource(User)] @@ -328,10 +292,10 @@ def test_invalid_resource_model(httpserver, sync_client): patch_op = PatchOp[Group](operations=[operation]) with pytest.raises(SCIMRequestError, match=r"Unknown resource type"): - sync_client.modify(Group, "some-id", patch_op) + sync_client.modify(group, patch_op) -def test_request_validation_error(httpserver, sync_client): +def test_request_validation_error(httpserver, sync_client, user): """Test that incorrect PatchOp creation raises a validation error.""" # Test with a PatchOp that has invalid data - this should fail during model_dump in prepare_patch_request with pytest.raises( @@ -344,10 +308,10 @@ def test_request_validation_error(httpserver, sync_client): invalid_patch_op = Mock() invalid_patch_op.model_dump.side_effect = ValueError("Invalid operation type") - sync_client.modify(User, "some-id", invalid_patch_op) + sync_client.modify(user, invalid_patch_op) -def test_request_network_error(httpserver, sync_client): +def test_request_network_error(httpserver, sync_client, user): """Test that httpx exceptions are transformed in RequestNetworkError.""" operation = PatchOperation( op=PatchOperation.Op.replace_, path="displayName", value="Test" @@ -357,10 +321,10 @@ def test_request_network_error(httpserver, sync_client): with pytest.raises( RequestNetworkError, match="Network error happened during request" ): - sync_client.modify(User, "some-id", patch_op, url="http://invalid.test") + sync_client.modify(user, patch_op, url="http://invalid.test") -def test_custom_url(httpserver, sync_client): +def test_custom_url(httpserver, sync_client, user): """Test modify with custom URL.""" httpserver.expect_request( "/custom/path/users/123", method="PATCH" @@ -375,16 +339,14 @@ def test_custom_url(httpserver, sync_client): ) patch_op = PatchOp[User](operations=[operation]) - response = sync_client.modify(User, "123", patch_op, url="/custom/path/users/123") + response = sync_client.modify(user, patch_op, url="/custom/path/users/123") assert response is None -def test_modify_with_dict_patch_op(httpserver, sync_client): +def test_modify_with_dict_patch_op(httpserver, sync_client, user): """Test modify with dict patch_op.""" - httpserver.expect_request( - "/Users/2819c223-7f76-453a-919d-413861904646", method="PATCH" - ).respond_with_data( + httpserver.expect_request(f"/Users/{user.id}", method="PATCH").respond_with_data( "", status=204, content_type="application/scim+json", @@ -397,8 +359,7 @@ def test_modify_with_dict_patch_op(httpserver, sync_client): } response = sync_client.modify( - User, - "2819c223-7f76-453a-919d-413861904646", + user, patch_op_dict, check_request_payload=True, ) @@ -406,7 +367,7 @@ def test_modify_with_dict_patch_op(httpserver, sync_client): assert response is None -def test_modify_validation_error(httpserver, sync_client): +def test_modify_validation_error(httpserver, sync_client, user): """Test that PatchOp validation errors are handled properly.""" from unittest.mock import Mock @@ -426,4 +387,4 @@ def test_modify_validation_error(httpserver, sync_client): RequestPayloadValidationError, match="Server request payload validation error", ): - sync_client.modify(User, "some-id", invalid_patch_op) + sync_client.modify(user, invalid_patch_op)