refactor atomic handler, create separate operation types, use enum for op action validation

mahenzon · mahenzon · commit 60145716b0f1 · 2023-09-04T20:25:39.000+03:00
diff --git a/fastapi_jsonapi/atomic/atomic_handler.py b/fastapi_jsonapi/atomic/atomic_handler.py
@@ -1,16 +1,25 @@
-from typing import Any, Dict, List, Optional, Type, TypedDict, Union
+from __future__ import annotations
+
+from typing import (
+    TYPE_CHECKING,
+    Any,
+    Dict,
+    List,
+    Optional,
+    TypedDict,
+    Union,
+)
 
 from starlette.requests import Request
 
 from fastapi_jsonapi import RoutersJSONAPI
-from fastapi_jsonapi.atomic.prepared_atomic_operation import PreparedOperation
+from fastapi_jsonapi.atomic.prepared_atomic_operation import OperationBase
 from fastapi_jsonapi.atomic.schemas import AtomicOperationRequest, AtomicResultResponse
-from fastapi_jsonapi.data_layers.base import BaseDataLayer
 from fastapi_jsonapi.utils.dependency_helper import DependencyHelper
-from fastapi_jsonapi.views.detail_view import DetailViewBase
-from fastapi_jsonapi.views.list_view import ListViewBase
 from fastapi_jsonapi.views.utils import HTTPMethodConfig
-from fastapi_jsonapi.views.view_base import ViewBase
+
+if TYPE_CHECKING:
+    from fastapi_jsonapi.data_layers.base import BaseDataLayer
 
 AtomicResponseDict = TypedDict("AtomicResponseDict", {"atomic:results": List[Any]})
 
@@ -26,7 +35,6 @@ def __init__(
 
     async def handle_view_dependencies(
         self,
-        request: Request,
         jsonapi: RoutersJSONAPI,
     ) -> Dict[str, Any]:
         method_config: HTTPMethodConfig = jsonapi.get_method_config_for_create()
@@ -39,30 +47,24 @@ def handle_dependencies(**dep_kwargs):
             method_config=method_config,
         )
 
-        dependencies_result: Dict[str, Any] = await DependencyHelper(request=request).run(handle_dependencies)
+        dependencies_result: Dict[str, Any] = await DependencyHelper(request=self.request).run(handle_dependencies)
         return dependencies_result
 
-    async def prepare_operations(self) -> List[PreparedOperation]:
-        prepared_operations: List[PreparedOperation] = []
+    async def prepare_operations(self) -> List[OperationBase]:
+        prepared_operations: List[OperationBase] = []
 
         for operation in self.operations_request.operations:
             jsonapi = RoutersJSONAPI.all_jsonapi_routers[operation.data.type]
-            view_cls: Type["ViewBase"] = jsonapi.detail_view_resource
-            if operation.op == "add":
-                view_cls = jsonapi.list_view_resource
-            view = view_cls(request=self.request, jsonapi=jsonapi)
+
             dependencies_result: Dict[str, Any] = await self.handle_view_dependencies(
-                request=self.request,
                 jsonapi=jsonapi,
             )
-            dl: "BaseDataLayer" = await view.get_data_layer(dependencies_result)
-
-            one_operation = PreparedOperation(
+            one_operation = OperationBase.prepare(
                 action=operation.op,
-                data_layer=dl,
-                view=view,
+                request=self.request,
                 jsonapi=jsonapi,
                 data=operation.data,
+                data_layer_view_dependencies=dependencies_result,
             )
             prepared_operations.append(one_operation)
 
@@ -74,42 +76,17 @@ async def handle(self) -> Union[AtomicResponseDict, AtomicResultResponse]:
 
         # TODO: try/except, catch schema ValidationError
 
-        previous_dl: Optional["BaseDataLayer"] = None
+        success = True
+        previous_dl: Optional[BaseDataLayer] = None
         for operation in prepared_operations:
-            dl = operation.data_layer
+            dl: BaseDataLayer = await operation.get_data_layer()
             await dl.atomic_start(previous_dl=previous_dl)
             previous_dl = dl
-            if operation.action == "add":
-                data_in = operation.jsonapi.schema_in_post(data=operation.data)
-                assert isinstance(operation.view, ListViewBase)
-                view: "ListViewBase" = operation.view
-                response = await view.process_create_object(dl=operation.data_layer, data_create=data_in.data)
-                # response.data.id
-                results.append({"data": response.data})
-            elif operation.action == "update":
-                data_in = operation.jsonapi.schema_in_patch(data=operation.data)
-                assert isinstance(operation.view, DetailViewBase)
-                view: "DetailViewBase" = operation.view
-                response = await view.process_update_object(
-                    dl=dl,
-                    obj_id=data_in.data.id,
-                    data_update=data_in.data,
-                )
-                # response.data.id
-                results.append({"data": response.data})
-            elif operation.action == "remove":
-                assert isinstance(operation.view, DetailViewBase)
-                view: "DetailViewBase" = operation.view
-                response = await view.process_delete_object(
-                    dl=dl,
-                    obj_id=operation.data.id,
-                )
-                results.append({"data": response.data})
-            else:
-                msg = f"unknown action {operation.action!r}"
-                raise ValueError(msg)
+            response = await operation.handle(dl=dl)
+            # response.data.id
+            results.append({"data": response.data})
 
         if previous_dl:
-            await previous_dl.atomic_end(success=True)
+            await previous_dl.atomic_end(success=success)
 
         return {"atomic:results": results}
diff --git a/fastapi_jsonapi/atomic/prepared_atomic_operation.py b/fastapi_jsonapi/atomic/prepared_atomic_operation.py
@@ -1,26 +1,101 @@
+from __future__ import annotations
+
 from dataclasses import dataclass
-from typing import List, Literal, Union
+from typing import TYPE_CHECKING, Any, Dict, Type
+
+from fastapi import Request
 
 from fastapi_jsonapi import RoutersJSONAPI
-from fastapi_jsonapi.atomic.schemas import OperationItemInSchema, OperationRelationshipSchema
-from fastapi_jsonapi.data_layers.base import BaseDataLayer
-from fastapi_jsonapi.views.view_base import ViewBase
+from fastapi_jsonapi.atomic.schemas import AtomicOperationAction, OperationDataType
+
+if TYPE_CHECKING:
+    from fastapi_jsonapi.data_layers.base import BaseDataLayer
+    from fastapi_jsonapi.views.detail_view import DetailViewBase
+    from fastapi_jsonapi.views.list_view import ListViewBase
+    from fastapi_jsonapi.views.view_base import ViewBase
 
 
 @dataclass
-class PreparedOperation:
-    action: Literal["add", "update", "remove"]
-    data_layer: "BaseDataLayer"
-    view: "ViewBase"
+class OperationBase:
     jsonapi: RoutersJSONAPI
-    data: Union[
-        # from biggest to smallest!
-        # any object creation
-        OperationItemInSchema,
-        # to-many relationship
-        List[OperationRelationshipSchema],
-        # to-one relationship
-        OperationRelationshipSchema,
-        # not required
-        None,
-    ] = None
+    view: ViewBase
+    data: OperationDataType
+    data_layer_view_dependencies: Dict[str, Any]
+
+    @classmethod
+    def prepare(
+        cls,
+        action: str,
+        request: Request,
+        jsonapi: RoutersJSONAPI,
+        data: OperationDataType,
+        data_layer_view_dependencies: Dict[str, Any],
+    ) -> "OperationBase":
+        view_cls: Type[ViewBase] = jsonapi.detail_view_resource
+
+        if action == AtomicOperationAction.add:
+            operation_cls = OperationAdd
+            view_cls = jsonapi.list_view_resource
+        elif action == AtomicOperationAction.update:
+            operation_cls = OperationUpdate
+        elif action == AtomicOperationAction.remove:
+            operation_cls = OperationRemove
+        else:
+            msg = f"Unknown operation {action!r}"
+            raise ValueError(msg)
+
+        view = view_cls(request=request, jsonapi=jsonapi)
+
+        return operation_cls(
+            jsonapi=jsonapi,
+            view=view,
+            data=data,
+            data_layer_view_dependencies=data_layer_view_dependencies,
+        )
+
+    async def get_data_layer(self) -> BaseDataLayer:
+        return await self.view.get_data_layer(self.data_layer_view_dependencies)
+
+    async def handle(self, dl: BaseDataLayer):
+        raise NotImplementedError
+
+
+class ListOperationBase(OperationBase):
+    view: ListViewBase
+
+
+class DetailOperationBase(OperationBase):
+    view: DetailViewBase
+
+
+class OperationAdd(ListOperationBase):
+    async def handle(self, dl: BaseDataLayer):
+        data_in = self.jsonapi.schema_in_post(data=self.data)
+        response = await self.view.process_create_object(
+            dl=dl,
+            data_create=data_in.data,
+        )
+        return response
+
+
+class OperationUpdate(DetailOperationBase):
+    async def handle(self, dl: BaseDataLayer):
+        data_in = self.jsonapi.schema_in_patch(data=self.data)
+        response = await self.view.process_update_object(
+            dl=dl,
+            obj_id=data_in.data.id,
+            data_update=data_in.data,
+        )
+        return response
+
+
+class OperationRemove(DetailOperationBase):
+    async def handle(
+        self,
+        dl: BaseDataLayer,
+    ):
+        response = await self.view.process_delete_object(
+            dl=dl,
+            obj_id=self.data.id,
+        )
+        return response
diff --git a/fastapi_jsonapi/atomic/schemas.py b/fastapi_jsonapi/atomic/schemas.py
@@ -1,4 +1,7 @@
-from typing import List, Literal, Optional, Union
+from __future__ import annotations
+
+from enum import Enum
+from typing import List, Optional, Union
 
 from pydantic import BaseModel, Field, root_validator
 from starlette.datastructures import URLPath
@@ -20,6 +23,19 @@ class OperationItemInSchema(BaseModel):
     relationships: Optional[dict] = Field(None, description="Resource object relationships")
 
 
+OperationDataType = Union[
+    # from biggest to smallest!
+    # any object creation
+    OperationItemInSchema,
+    # to-many relationship
+    List[OperationRelationshipSchema],
+    # to-one relationship
+    OperationRelationshipSchema,
+    # not required
+    None,
+]
+
+
 class AtomicOperationRef(BaseModel):
     """
 
@@ -65,6 +81,12 @@ def validate_atomic_operation_ref(cls, values: dict):
         raise ValueError(msg)
 
 
+class AtomicOperationAction(str, Enum):
+    add = "add"
+    update = "update"
+    remove = "remove"
+
+
 class AtomicOperation(BaseModel):
     """
     An operation object MUST contain the following member: op
@@ -79,7 +101,7 @@ class AtomicOperation(BaseModel):
     https://jsonapi.org/ext/atomic/#operation-objects
     """
 
-    op: Literal["add", "update", "remove"] = Field(
+    op: AtomicOperationAction = Field(
         default=...,
         description="an operation code, expressed as a string, that indicates the type of operation to perform.",
     )
@@ -89,17 +111,10 @@ class AtomicOperation(BaseModel):
         description="a string that contains a URI-reference that identifies the target of the operation.",
     )
 
-    data: Union[
-        # from biggest to smallest!
-        # any object creation
-        OperationItemInSchema,
-        # to-many relationship
-        List[OperationRelationshipSchema],
-        # to-one relationship
-        OperationRelationshipSchema,
-        # not required
-        None,
-    ] = Field(default=None, description="the operation’s “primary data”.")
+    data: OperationDataType = Field(
+        default=None,
+        description="the operation’s “primary data”.",
+    )
 
     meta: Optional[dict] = Field(
         default=None,
diff --git a/tests/test_atomic/conftest.py b/tests/test_atomic/conftest.py
@@ -0,0 +1,15 @@
+from __future__ import annotations
+
+import pytest
+
+from fastapi_jsonapi.atomic.schemas import AtomicOperationAction
+
+
+@pytest.fixture()
+def allowed_atomic_actions_list() -> list[str]:
+    return [op.value for op in AtomicOperationAction]
+
+
+@pytest.fixture()
+def allowed_atomic_actions_as_string(allowed_atomic_actions_list) -> str:
+    return ", ".join(repr(op) for op in allowed_atomic_actions_list)
diff --git a/tests/test_atomic/test_mixed_atomic.py b/tests/test_atomic/test_mixed_atomic.py
@@ -17,6 +17,45 @@
 
 
 class TestAtomicMixedActions:
+    async def test_schema_validation_error(
+        self,
+        client: AsyncClient,
+        allowed_atomic_actions_list: list[str],
+        allowed_atomic_actions_as_string: str,
+    ):
+        operation_name = fake.word()
+        atomic_request_data = {
+            "atomic:operations": [
+                {
+                    "op": operation_name,
+                    "href": "/any",
+                    "data": {
+                        "type": "any",
+                        "attributes": {
+                            "any": "any",
+                        },
+                    },
+                },
+            ],
+        }
+        response = await client.post("/operations", json=atomic_request_data)
+        assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY, response.text
+        response_data = response.json()
+
+        assert response_data == {
+            # TODO: jsonapi exception?
+            "detail": [
+                {
+                    "loc": ["body", "atomic:operations", 0, "op"],
+                    "msg": f"value is not a valid enumeration member; permitted: {allowed_atomic_actions_as_string}",
+                    "type": "type_error.enum",
+                    "ctx": {
+                        "enum_values": allowed_atomic_actions_list,
+                    },
+                },
+            ],
+        }
+
     async def test_create_and_update_atomic_success(
         self,
         client: AsyncClient,
diff --git a/tests/test_atomic/test_request.py b/tests/test_atomic/test_request.py
