atomic operations refactoring, new coverage

mahenzon · mahenzon · commit eabb8f4953ac · 2023-09-13T16:42:03.000+03:00
diff --git a/fastapi_jsonapi/atomic/atomic_handler.py b/fastapi_jsonapi/atomic/atomic_handler.py
@@ -2,9 +2,12 @@
 
 import logging
 from collections import defaultdict
+from functools import wraps
 from typing import (
     TYPE_CHECKING,
     Any,
+    Awaitable,
+    Callable,
     Dict,
     List,
     Optional,
@@ -18,7 +21,7 @@
 
 from fastapi_jsonapi import RoutersJSONAPI
 from fastapi_jsonapi.atomic.prepared_atomic_operation import LocalIdsType, OperationBase
-from fastapi_jsonapi.atomic.schemas import AtomicOperationRequest, AtomicResultResponse
+from fastapi_jsonapi.atomic.schemas import AtomicOperation, AtomicOperationRequest, AtomicResultResponse
 from fastapi_jsonapi.utils.dependency_helper import DependencyHelper
 from fastapi_jsonapi.views.utils import HTTPMethodConfig
 
@@ -29,6 +32,37 @@
 AtomicResponseDict = TypedDict("AtomicResponseDict", {"atomic:results": List[Any]})
 
 
+def catch_exc_on_operation_handle(func: Callable[..., Awaitable]):
+    @wraps(func)
+    async def wrapper(*a, operation: OperationBase, **kw):
+        try:
+            return await func(*a, operation=operation, **kw)
+        except (ValidationError, ValueError) as ex:
+            log.exception(
+                "Validation error on atomic action ref=%s, data=%s",
+                operation.ref,
+                operation.data,
+            )
+            errors_details = {
+                "message": f"Validation error on operation {operation.op_type}",
+                "ref": operation.ref,
+                "data": operation.data.dict(),
+            }
+            if isinstance(ex, ValidationError):
+                errors_details.update(errors=ex.errors())
+            elif isinstance(ex, ValueError):
+                errors_details.update(error=str(ex))
+            else:
+                raise
+            # TODO: json:api exception
+            raise HTTPException(
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+                detail=errors_details,
+            )
+
+    return wrapper
+
+
 class AtomicViewHandler:
     def __init__(
         self,
@@ -37,6 +71,7 @@ def __init__(
     ):
         self.request = request
         self.operations_request = operations_request
+        self.local_ids_cache: LocalIdsType = defaultdict(dict)
 
     async def handle_view_dependencies(
         self,
@@ -55,68 +90,64 @@ def handle_dependencies(**dep_kwargs):
         dependencies_result: Dict[str, Any] = await DependencyHelper(request=self.request).run(handle_dependencies)
         return dependencies_result
 
+    async def prepare_one_operation(self, operation: AtomicOperation):
+        """
+        :param operation:
+        :return:
+        """
+        operation_type = operation.ref and operation.ref.type or operation.data and operation.data.type
+        assert operation_type
+        if operation_type not in RoutersJSONAPI.all_jsonapi_routers:
+            msg = f"Unknown resource type {operation_type!r}. Register it via RoutersJSONAPI"
+            raise ValueError(msg)
+        jsonapi = RoutersJSONAPI.all_jsonapi_routers[operation_type]
+
+        dependencies_result: Dict[str, Any] = await self.handle_view_dependencies(
+            jsonapi=jsonapi,
+        )
+        one_operation = OperationBase.prepare(
+            action=operation.op,
+            request=self.request,
+            jsonapi=jsonapi,
+            ref=operation.ref,
+            data=operation.data,
+            data_layer_view_dependencies=dependencies_result,
+        )
+        return one_operation
+
     async def prepare_operations(self) -> List[OperationBase]:
         prepared_operations: List[OperationBase] = []
 
         for operation in self.operations_request.operations:
-            operation_type = operation.ref and operation.ref.type or operation.data and operation.data.type
-            assert operation_type
-            jsonapi = RoutersJSONAPI.all_jsonapi_routers[operation_type]
-
-            dependencies_result: Dict[str, Any] = await self.handle_view_dependencies(
-                jsonapi=jsonapi,
-            )
-            one_operation = OperationBase.prepare(
-                action=operation.op,
-                request=self.request,
-                jsonapi=jsonapi,
-                ref=operation.ref,
-                data=operation.data,
-                data_layer_view_dependencies=dependencies_result,
-            )
+            one_operation = await self.prepare_one_operation(operation)
             prepared_operations.append(one_operation)
 
         return prepared_operations
 
+    @catch_exc_on_operation_handle
+    async def process_one_operation(
+        self,
+        dl: BaseDataLayer,
+        operation: OperationBase,
+    ):
+        operation.update_relationships_with_lid(local_ids=self.local_ids_cache)
+        return await operation.handle(dl=dl)
+
     async def handle(self) -> Union[AtomicResponseDict, AtomicResultResponse, None]:
         prepared_operations = await self.prepare_operations()
         results = []
-
-        # TODO: try/except, catch schema ValidationError
-
         only_empty_responses = True
-        local_ids_cache: LocalIdsType = defaultdict(dict)
         success = True
         previous_dl: Optional[BaseDataLayer] = None
-        for idx, operation in enumerate(prepared_operations, start=1):
+        for operation in prepared_operations:
             dl: BaseDataLayer = await operation.get_data_layer()
             await dl.atomic_start(previous_dl=previous_dl)
+            response = await self.process_one_operation(
+                dl=dl,
+                operation=operation,
+            )
             previous_dl = dl
-            try:
-                operation.update_relationships_with_lid(local_ids=local_ids_cache)
-                response = await operation.handle(dl=dl)
-            except (ValidationError, ValueError) as ex:
-                log.exception(
-                    "Validation error on atomic action ref=%s, data=%s",
-                    operation.ref,
-                    operation.data,
-                )
-                errors_details = {
-                    "message": f"Validation error on operation #{idx}",
-                    "ref": operation.ref,
-                    "data": operation.data.dict(),
-                }
-                if isinstance(ex, ValidationError):
-                    errors_details.update(errors=ex.errors())
-                elif isinstance(ex, ValueError):
-                    errors_details.update(error=str(ex))
-                else:
-                    raise
-                # TODO: json:api exception
-                raise HTTPException(
-                    status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
-                    detail=errors_details,
-                )
+
             # response.data.id
             if not response:
                 # https://jsonapi.org/ext/atomic/#result-objects
@@ -127,7 +158,7 @@ async def handle(self) -> Union[AtomicResponseDict, AtomicResultResponse, None]:
             only_empty_responses = False
             results.append({"data": response.data})
             if operation.data.lid and response.data:
-                local_ids_cache[operation.data.type][operation.data.lid] = response.data.id
+                self.local_ids_cache[operation.data.type][operation.data.lid] = response.data.id
 
         if previous_dl:
             await previous_dl.atomic_end(success=success)
diff --git a/fastapi_jsonapi/atomic/prepared_atomic_operation.py b/fastapi_jsonapi/atomic/prepared_atomic_operation.py
@@ -24,6 +24,7 @@ class OperationBase:
     ref: Optional[AtomicOperationRef]
     data: OperationDataType
     data_layer_view_dependencies: Dict[str, Any]
+    op_type: str
 
     @classmethod
     def prepare(
@@ -56,6 +57,7 @@ def prepare(
             ref=ref,
             data=data,
             data_layer_view_dependencies=data_layer_view_dependencies,
+            op_type=action,
         )
 
     async def get_data_layer(self) -> BaseDataLayer:
diff --git a/tests/test_atomic/test_create_objects.py b/tests/test_atomic/test_create_objects.py
@@ -680,7 +680,7 @@ async def test_resource_type_with_local_id_not_found(
                     "lid": None,
                 },
                 "error": expected_error_text,
-                "message": "Validation error on operation #2",
+                "message": f"Validation error on operation {action_1['op']}",
                 "ref": None,
             },
         }
@@ -780,7 +780,7 @@ async def test_local_id_not_found(
                     "lid": None,
                 },
                 "error": expected_error_text,
-                "message": "Validation error on operation #2",
+                "message": f"Validation error on operation {action_2['op']}",
                 "ref": None,
             },
         }
@@ -897,6 +897,47 @@ async def test_create_and_associate_many_to_many(
             ],
         }
 
+    async def test_create_object_schema_validation_error(
+        self,
+        client: AsyncClient,
+    ):
+        action_add = {
+            "op": "add",
+            "data": {
+                "type": "user",
+                # not passing the required `name` attribute
+                "attributes": {},
+            },
+        }
+        data_atomic_request = {
+            "atomic:operations": [
+                action_add,
+            ],
+        }
+
+        response = await client.post("/operations", json=data_atomic_request)
+        assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY, response.text
+        # TODO: json:api exception
+        assert response.json() == {
+            "detail": {
+                "data": {
+                    **action_add["data"],
+                    "id": None,
+                    "lid": None,
+                    "relationships": None,
+                },
+                "errors": [
+                    {
+                        "loc": ["data", "attributes", "name"],
+                        "msg": "field required",
+                        "type": "value_error.missing",
+                    },
+                ],
+                "message": f"Validation error on operation {action_add['op']}",
+                "ref": None,
+            },
+        }
+
     @pytest.mark.skip("not ready yet")
     async def test_update_to_many_relationship_with_local_id(
         self,
