Fix operation ID for class based views (#289)

* Fix operation ID for class based views
* Clean types
* Fix docstrings for `generate_operation_id`.

esmerald/openapi/openapi.py

@@ -130,6 +130,7 @@ def get_openapi_operation(
         operation.description = route.description
 
     operation_id = route.operation_id
+
     if operation_id in operation_ids:
         message = (
             f"Duplicate Operation ID {operation_id} for function " + f"{route.handler.__name__}"

esmerald/routing/base.py

@@ -236,7 +236,7 @@ async def response_content(data: Response, **kwargs: Dict[str, Any]) -> LilyaRes
 
         return response_content
 
-    def starlette_response_handler(
+    def lilya_response_handler(
         self,
         cookies: "ResponseCookies",
         headers: Optional["ResponseHeaders"] = None,
@@ -435,7 +435,7 @@ def get_response_handler(self) -> Callable[[Any], Awaitable[LilyaResponse]]:
                     headers=headers,
                 )
             elif is_class_and_subclass(self.handler_signature.return_annotation, LilyaResponse):
-                handler = self.starlette_response_handler(
+                handler = self.lilya_response_handler(
                     cookies=cookies,
                     headers=headers,
                 )

esmerald/routing/gateways.py

@@ -42,7 +42,40 @@ def handle_middleware(
         return _middleware
 
 
-class Gateway(LilyaPath, BaseInterceptorMixin, BaseMiddleware):
+class GatewayUtil:
+
+    def is_class_based(
+        self, handler: Union["HTTPHandler", "WebSocketHandler", "ParentType"]
+    ) -> bool:
+        return bool(is_class_and_subclass(handler, View) or isinstance(handler, View))
+
+    def is_handler(self, handler: Union["HTTPHandler", "WebSocketHandler", "ParentType"]) -> bool:
+        return bool(not is_class_and_subclass(handler, View) and not isinstance(handler, View))
+
+    def generate_operation_id(
+        self, name: Union[str, None], handler: Union["HTTPHandler", "WebSocketHandler", View]
+    ) -> str:
+        """
+        Generates an unique operation if for the handler.
+
+        We need to be able to handle with edge cases when a view does not default a path like `/format` and a default name needs to be passed when its a class based view.
+        """
+        if self.is_class_based(handler.parent):
+            operation_id = (
+                handler.parent.__class__.__name__.lower() + f"_{name}" + handler.path_format
+            )
+        else:
+            operation_id = name + handler.path_format
+
+        operation_id = re.sub(r"\W", "_", operation_id)
+        methods = list(handler.methods)  # type: ignore
+
+        assert handler.methods  # type: ignore
+        operation_id = f"{operation_id}_{methods[0].lower()}"
+        return operation_id
+
+
+class Gateway(LilyaPath, BaseInterceptorMixin, BaseMiddleware, GatewayUtil):
     """
     `Gateway` object class used by Esmerald routes.
 
@@ -283,32 +316,20 @@ def __init__(
             self.path
         )
 
-        if not is_class_and_subclass(self.handler, View) and not isinstance(self.handler, View):
+        if self.is_handler(self.handler):  # type: ignore
             self.handler.name = self.name
 
             if not handler.operation_id:
-                handler.operation_id = self.generate_operation_id()
+                handler.operation_id = self.generate_operation_id(
+                    name=self.name, handler=self.handler  # type: ignore
+                )
 
     async def handle_dispatch(self, scope: "Scope", receive: "Receive", send: "Send") -> None:
         """
         Handles the interception of messages and calls from the API.
-        if self._middleware:
-            self.is_middleware = True
         """
         await self.app(scope, receive, send)
 
-    def generate_operation_id(self) -> str:
-        """
-        Generates an unique operation if for the handler
-        """
-        operation_id = self.name + self.handler.path_format
-        operation_id = re.sub(r"\W", "_", operation_id)
-        methods = list(self.handler.methods)
-
-        assert self.handler.methods
-        operation_id = f"{operation_id}_{methods[0].lower()}"
-        return cast(str, operation_id)
-
 
 class WebSocketGateway(LilyaWebSocketPath, BaseInterceptorMixin, BaseMiddleware):
     """
@@ -508,7 +529,7 @@ async def handle_dispatch(self, scope: "Scope", receive: "Receive", send: "Send"
         await self.app(scope, receive, send)
 
 
-class WebhookGateway(LilyaPath, BaseInterceptorMixin):
+class WebhookGateway(LilyaPath, BaseInterceptorMixin, GatewayUtil):
     """
     `WebhookGateway` object class used by Esmerald routes.
 
@@ -648,20 +669,10 @@ def __init__(
             self.path
         )
 
-        if not is_class_and_subclass(self.handler, View) and not isinstance(self.handler, View):
+        if self.is_handler(self.handler):  # type: ignore
             self.handler.name = self.name
 
             if not handler.operation_id:
-                handler.operation_id = self.generate_operation_id()
-
-    def generate_operation_id(self) -> str:
-        """
-        Generates an unique operation if for the handler
-        """
-        operation_id = self.name + self.handler.path_format
-        operation_id = re.sub(r"\W", "_", operation_id)
-        methods = list(self.handler.methods)
-
-        assert self.handler.methods
-        operation_id = f"{operation_id}_{methods[0].lower()}"
-        return cast(str, operation_id)
+                handler.operation_id = self.generate_operation_id(
+                    name=self.name, handler=self.handler  # type: ignore
+                )

esmerald/types.py

@@ -13,9 +13,8 @@
     get_args,
 )
 
-from lilya.middleware import DefineMiddleware  # noqa
-from lilya.responses import Response as LilyaResponse  # noqa
-from lilya.types import ASGIApp  # noqa
+from lilya.middleware import DefineMiddleware
+from lilya.types import ASGIApp
 from typing_extensions import Literal
 
 from esmerald.backgound import BackgroundTask, BackgroundTasks
@@ -24,27 +23,31 @@
 from esmerald.routing.router import Include
 
 try:
-    from asyncz.schedulers import AsyncIOScheduler  # noqa
+    from asyncz.schedulers import AsyncIOScheduler
 except ImportError:
     AsyncIOScheduler = Any  # type: ignore
 
 try:
-    from esmerald.config.template import TemplateConfig as TemplateConfig  # noqa
+    from esmerald.config.template import TemplateConfig as TemplateConfig
 except MissingDependency:
     TemplateConfig = Any  # type: ignore
 
 if TYPE_CHECKING:
     from esmerald.applications import Esmerald
-    from esmerald.conf.global_settings import EsmeraldAPISettings  # noqa
-    from esmerald.datastructures import Cookie, ResponseHeader, State  # noqa: TC004
-    from esmerald.injector import Inject  # noqa
-    from esmerald.protocols.middleware import MiddlewareProtocol
-    from esmerald.requests import Request  # noqa
-    from esmerald.responses import Response  # noqa
-    from esmerald.routing.apis.base import View  # noqa
-    from esmerald.routing.gateways import Gateway, WebhookGateway  # noqa
-    from esmerald.routing.router import HTTPHandler, Router, WebSocketHandler  # noqa
-    from esmerald.websockets import WebSocket  # noqa
+    from esmerald.conf.global_settings import EsmeraldAPISettings
+    from esmerald.datastructures import Cookie, ResponseHeader, State as State
+    from esmerald.injector import Inject
+    from esmerald.protocols.middleware import MiddlewareProtocol as MiddlewareProtocol
+    from esmerald.requests import Request
+    from esmerald.responses import Response
+    from esmerald.routing.apis.base import View
+    from esmerald.routing.gateways import Gateway, WebhookGateway
+    from esmerald.routing.router import (
+        HTTPHandler as HTTPHandler,
+        Router,
+        WebSocketHandler as WebSocketHandler,
+    )
+    from esmerald.websockets import WebSocket
 else:
     HTTPHandler = Any
     Message = Any

tests/openapi/test_duplicate_operation_id.py

@@ -0,0 +1,93 @@
+from __future__ import annotations
+
+from esmerald import APIView, Gateway, Include, post
+from esmerald.testclient import create_client
+from tests.settings import TestSettings
+
+
+class UserAPIView(APIView):
+    tags: list[str] = ["User"]
+
+    @post(path="/")
+    async def create(self) -> str: ...
+
+
+class ProfileAPIView(APIView):
+    tags: list[str] = ["Profile"]
+
+    @post(path="/")
+    async def create(self) -> str: ...
+
+
+user_routes = [Gateway(handler=UserAPIView)]
+profile_routes = [Gateway(handler=ProfileAPIView)]
+
+route_patterns = [
+    Include(
+        "/admin",
+        routes=[
+            Include(
+                "/users",
+                namespace="tests.openapi.test_duplicate_operation_id",
+                pattern="user_routes",
+            ),
+            Include(
+                "/profiles",
+                namespace="tests.openapi.test_duplicate_operation_id",
+                pattern="profile_routes",
+            ),
+        ],
+    )
+]
+
+
+def test_open_api_schema(test_client_factory):
+    with create_client(
+        routes=[Include(path="/api/v1", namespace="tests.openapi.test_duplicate_operation_id")],
+        enable_openapi=True,
+        include_in_schema=True,
+        settings_module=TestSettings,
+    ) as client:
+        response = client.get("/openapi.json")
+
+        assert response.json() == {
+            "openapi": "3.1.0",
+            "info": {
+                "title": "Esmerald",
+                "summary": "Esmerald application",
+                "description": "Highly scalable, performant, easy to learn and for every application.",
+                "contact": {"name": "admin", "email": "[email protected]"},
+                "version": client.app.version,
+            },
+            "servers": [{"url": "/"}],
+            "paths": {
+                "/api/v1/admin/users": {
+                    "post": {
+                        "tags": ["User"],
+                        "summary": "Create",
+                        "operationId": "userapiview_create__post",
+                        "responses": {
+                            "201": {
+                                "description": "Successful response",
+                                "content": {"application/json": {"schema": {"type": "string"}}},
+                            }
+                        },
+                        "deprecated": False,
+                    }
+                },
+                "/api/v1/admin/profiles": {
+                    "post": {
+                        "tags": ["Profile"],
+                        "summary": "Create",
+                        "operationId": "profileapiview_create__post",
+                        "responses": {
+                            "201": {
+                                "description": "Successful response",
+                                "content": {"application/json": {"schema": {"type": "string"}}},
+                            }
+                        },
+                        "deprecated": False,
+                    }
+                },
+            },
+        }

tests/openapi/test_include_with_apiview.py

@@ -59,7 +59,7 @@ async def read_item(self) -> JSON:
                     "get": {
                         "summary": "Read Item",
                         "description": "Read an item",
-                        "operationId": "read_item_item_get",
+                        "operationId": "myapi_read_item_item_get",
                         "responses": {
                             "200": {
                                 "description": "The SKU information of an item",
@@ -154,6 +154,6 @@ async def read_item(self) -> JSON:
                         },
                         "deprecated": False,
                     }
-                },
+                }
             },
         }