add unassign role to consistent updates (#275)

* Enhance Consistent Updates feature with new role unassignment endpoints

- Updated `FactsClient` to support optional query parameters in `build_forward_request` and `send_forward_request` methods, improving flexibility in request handling.
- Added new DELETE endpoints for unassigning user roles and deleting role assignments in the `facts_router`, allowing for better management of user roles with a `return_deleted` query parameter.
- Refactored `forward_request_then_wait_for_update` to accommodate the new query parameters, ensuring consistent behavior across requests.

* Refactor FactsClient methods to remove optional query parameters

- Updated `build_forward_request` and `send_forward_request` methods in `FactsClient` to eliminate the optional `query_params` argument, simplifying the request handling process.
- Adjusted related calls in `facts_router` to align with the new method signatures, ensuring consistent behavior across the application.

* fix pre-commit

* Enhance FactsClient and router with query parameter support

- Updated `FactsClient` methods to accept optional `query_params`, improving flexibility in request handling.
- Introduced a `pre_return_callback` parameter to `convert_response` for custom response processing.
- Modified `facts_router` endpoints to utilize the new query parameter functionality, specifically for handling `return_deleted` logic in user role management.
- Added a utility function to convert 200 responses to 204, enhancing response consistency.

* Refactor FactsClient and router to remove pre_return_callback parameter

- Eliminated the `pre_return_callback` parameter from `convert_response` in `FactsClient`, simplifying response handling.
- Updated `unassign_user_role` and `delete_role_assignment` functions in `facts_router` to remove references to `pre_return_callback`, ensuring consistent response processing.
- Adjusted `forward_request_then_wait_for_update` to align with the new method signature, enhancing clarity and maintainability.

* Refactor query parameter handling in FactsClient

- Updated the `FactsClient` to use a separate variable for merging query parameters, improving code clarity.
- Simplified the request parameter construction by directly using the new variable, enhancing maintainability.

Author: omer9564

charts/pdp/Chart.yaml

@@ -1,4 +1,4 @@
 apiVersion: v2
 name: pdp
 description: An official Helm chart for Permit.io PDP (Policy Decision Point)
-version: 0.0.4
+version: 0.0.4

horizon/facts/client.py

@@ -1,4 +1,4 @@
-from typing import Annotated
+from typing import Annotated, Any
 from urllib.parse import urljoin
 
 from fastapi import Depends, HTTPException
@@ -30,7 +30,13 @@ def client(self) -> AsyncClient:
             )
         return self._client
 
-    async def build_forward_request(self, request: FastApiRequest, path: str) -> HttpxRequest:
+    async def build_forward_request(
+        self,
+        request: FastApiRequest,
+        path: str,
+        *,
+        query_params: dict[str, Any] | None = None,
+    ) -> HttpxRequest:
         """
         Build an HTTPX request from a FastAPI request to forward to the facts service.
         :param request: FastAPI request
@@ -50,10 +56,11 @@ async def build_forward_request(self, request: FastApiRequest, path: str) -> Htt
             )
 
         full_path = urljoin(f"/v2/facts/{project_id}/{environment_id}/", path.removeprefix("/"))
+        _query_params = {**request.query_params, **(query_params or {})}
         return self.client.build_request(
             method=request.method,
             url=full_path,
-            params=request.query_params,
+            params=_query_params,
             headers=forward_headers,
             content=request.stream(),
         )
@@ -62,18 +69,28 @@ async def send(self, request: HttpxRequest, *, stream: bool = False) -> HttpxRes
         logger.info(f"Forwarding facts request: {request.method} {request.url}")
         return await self.client.send(request, stream=stream)
 
-    async def send_forward_request(self, request: FastApiRequest, path: str) -> HttpxResponse:
+    async def send_forward_request(
+        self,
+        request: FastApiRequest,
+        path: str,
+        *,
+        query_params: dict[str, Any] | None = None,
+    ) -> HttpxResponse:
         """
         Send a forward request to the facts service.
         :param request: FastAPI request
         :param path: Backend facts service path to forward to
         :return: HTTPX response
         """
-        forward_request = await self.build_forward_request(request, path)
+        forward_request = await self.build_forward_request(request, path, query_params=query_params)
         return await self.send(forward_request)
 
     @staticmethod
-    def convert_response(response: HttpxResponse, *, stream: bool = False) -> FastApiResponse:
+    def convert_response(
+        response: HttpxResponse,
+        *,
+        stream: bool = False,
+    ) -> FastApiResponse:
         """
         Convert an HTTPX response to a FastAPI response.
         :param response: HTTPX response
@@ -103,6 +120,8 @@ def extract_body(response: HttpxResponse):
             return None
 
         try:
+            if response.status_code == 204:
+                return None
             body = response.json()
         except Exception:  # noqa: BLE001
             logger.exception("Failed to parse response body as JSON, skipping wait for update.")

horizon/facts/router.py

@@ -192,6 +192,27 @@ async def assign_user_role(
     )
 
 
+@facts_router.delete("/users/{user_id}/roles")
+async def unassign_user_role(
+    request: FastApiRequest,
+    client: FactsClientDependency,
+    update_subscriber: DataUpdateSubscriberDependency,
+    wait_timeout: WaitTimeoutDependency,
+    timeout_policy: TimeoutPolicyDependency,
+    user_id: str,
+):
+    return await forward_request_then_wait_for_update(
+        client,
+        request,
+        update_subscriber,
+        wait_timeout,
+        path=f"/users/{user_id}/roles",
+        query_params={"return_deleted": True},
+        entries_callback=create_role_assignment_data_entries,
+        timeout_policy=timeout_policy,
+    )
+
+
 @facts_router.post("/role_assignments")
 async def create_role_assignment(
     request: FastApiRequest,
@@ -211,6 +232,26 @@ async def create_role_assignment(
     )
 
 
+@facts_router.delete("/role_assignments")
+async def delete_role_assignment(
+    request: FastApiRequest,
+    client: FactsClientDependency,
+    update_subscriber: DataUpdateSubscriberDependency,
+    wait_timeout: WaitTimeoutDependency,
+    timeout_policy: TimeoutPolicyDependency,
+):
+    return await forward_request_then_wait_for_update(
+        client,
+        request,
+        update_subscriber,
+        wait_timeout,
+        path="/role_assignments",
+        entries_callback=create_role_assignment_data_entries,
+        timeout_policy=timeout_policy,
+        query_params={"return_deleted": True},
+    )
+
+
 @facts_router.post("/resource_instances")
 async def create_resource_instance(
     request: FastApiRequest,
@@ -293,6 +334,12 @@ async def create_relationship_tuple(
     )
 
 
+def cast_delete_200_to_204(response: Response) -> Response:
+    if response.status_code == 200:
+        return Response(status_code=204)
+    return response
+
+
 async def forward_request_then_wait_for_update(
     client: FactsClient,
     request: FastApiRequest,
@@ -303,9 +350,10 @@ async def forward_request_then_wait_for_update(
     update_id: UUID | None = None,
     entries_callback: Callable[[FastApiRequest, dict[str, Any], UUID | None], Iterable[DataSourceEntry]],
     timeout_policy: TimeoutPolicy = TimeoutPolicy.IGNORE,
+    query_params: dict[str, Any] | None = None,
 ) -> Response:
     _update_id = update_id or uuid4()
-    response = await client.send_forward_request(request, path)
+    response = await client.send_forward_request(request, path, query_params=query_params)
     body = client.extract_body(response)
     if body is None:
         return client.convert_response(response)