✨ Avoid data schema names collision in OpenAPI schema (#113)

vortico · Sep 19, 2023 · 331a759 · 331a759
1 parent 4c00cc6
commit 331a759
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 55 deletions.
diff --git a/flama/pagination/mixins/limit_offset.py b/flama/pagination/mixins/limit_offset.py
@@ -27,7 +27,7 @@ def __init__(
         offset: t.Optional[t.Union[int, str]] = None,
         limit: t.Optional[t.Union[int, str]] = None,
         count: t.Optional[bool] = True,
-        **kwargs
+        **kwargs,
     ):
         self.offset = int(offset) if offset is not None else 0
         self.limit = int(limit) if limit is not None else self.default_limit
@@ -66,7 +66,7 @@ async def decorator(
             limit: t.Optional[int] = None,
             offset: t.Optional[int] = None,
             count: t.Optional[bool] = False,
-            **kwargs
+            **kwargs,
         ):
             return LimitOffsetResponse(
                 schema=schema, limit=limit, offset=offset, count=count, content=await func(*args, **kwargs)
@@ -82,7 +82,7 @@ def decorator(
             limit: t.Optional[int] = None,
             offset: t.Optional[int] = None,
             count: t.Optional[bool] = False,
-            **kwargs
+            **kwargs,
         ):
             return LimitOffsetResponse(
                 schema=schema, limit=limit, offset=offset, count=count, content=func(*args, **kwargs)
@@ -107,9 +107,13 @@ def limit_offset(self, schema_name: str):
         :return: Decorated view.
         """
 
-        def _inner(func: t.Callable):
+        def _inner(func: t.Callable) -> t.Callable:
             resource_schema = schemas.Schema.from_type(inspect.signature(func).return_annotation).unique_schema
-            paginated_schema_name = "LimitOffsetPaginated" + schema_name
+            try:
+                schema_module, schema_class = schema_name.rsplit(".", 1)
+                paginated_schema_name = f"{schema_module}.LimitOffsetPaginated{schema_class}"
+            except ValueError:
+                paginated_schema_name = f"LimitOffsetPaginated{schema_name}"
             schema = schemas.Schema.build(
                 paginated_schema_name,
                 schema=schemas.schemas.LimitOffset,

diff --git a/flama/pagination/mixins/page_number.py b/flama/pagination/mixins/page_number.py
@@ -29,7 +29,7 @@ def __init__(
         page: t.Optional[t.Union[int, str]] = None,
         page_size: t.Optional[t.Union[int, str]] = None,
         count: t.Optional[bool] = True,
-        **kwargs
+        **kwargs,
     ):
         self.page_number = int(page) if page is not None else 1
         self.page_size = int(page_size) if page_size is not None else self.default_page_size
@@ -73,7 +73,7 @@ async def decorator(
             page: t.Optional[int] = None,
             page_size: t.Optional[int] = None,
             count: t.Optional[bool] = False,
-            **kwargs
+            **kwargs,
         ):
             return PageNumberResponse(
                 schema=schema, page=page, page_size=page_size, count=count, content=await func(*args, **kwargs)
@@ -89,7 +89,7 @@ def decorator(
             page: t.Optional[int] = None,
             page_size: t.Optional[int] = None,
             count: t.Optional[bool] = False,
-            **kwargs
+            **kwargs,
         ):
             return PageNumberResponse(
                 schema=schema, page=page, page_size=page_size, count=count, content=func(*args, **kwargs)
@@ -116,7 +116,11 @@ def page_number(self, schema_name: str) -> t.Callable:
 
         def _inner(func: t.Callable) -> t.Callable:
             resource_schema = schemas.Schema.from_type(inspect.signature(func).return_annotation).unique_schema
-            paginated_schema_name = "PageNumberPaginated" + schema_name
+            try:
+                schema_module, schema_class = schema_name.rsplit(".", 1)
+                paginated_schema_name = f"{schema_module}.PageNumberPaginated{schema_class}"
+            except ValueError:
+                paginated_schema_name = f"PageNumberPaginated{schema_name}"
             schema = schemas.Schema.build(
                 paginated_schema_name,
                 schema=schemas.schemas.PageNumber,

diff --git a/flama/schemas/_libs/marshmallow/schemas.py b/flama/schemas/_libs/marshmallow/schemas.py
@@ -78,12 +78,12 @@ class MLModelOutput(marshmallow.Schema):
 
 
 SCHEMAS = {
-    "APIError": APIError,
-    "DropCollection": DropCollection,
-    "LimitOffsetMeta": LimitOffsetMeta,
-    "LimitOffset": LimitOffset,
-    "PageNumberMeta": PageNumberMeta,
-    "PageNumber": PageNumber,
-    "MLModelInput": MLModelInput,
-    "MLModelOutput": MLModelOutput,
+    "flama.APIError": APIError,
+    "flama.DropCollection": DropCollection,
+    "flama.LimitOffsetMeta": LimitOffsetMeta,
+    "flama.LimitOffset": LimitOffset,
+    "flama.PageNumberMeta": PageNumberMeta,
+    "flama.PageNumber": PageNumber,
+    "flama.MLModelInput": MLModelInput,
+    "flama.MLModelOutput": MLModelOutput,
 }
diff --git a/flama/schemas/_libs/pydantic/schemas.py b/flama/schemas/_libs/pydantic/schemas.py
@@ -56,12 +56,12 @@ class MLModelOutput(BaseModel):
 
 
 SCHEMAS = {
-    "APIError": APIError,
-    "DropCollection": DropCollection,
-    "LimitOffsetMeta": LimitOffsetMeta,
-    "LimitOffset": LimitOffset,
-    "PageNumberMeta": PageNumberMeta,
-    "PageNumber": PageNumber,
-    "MLModelInput": MLModelInput,
-    "MLModelOutput": MLModelOutput,
+    "flama.APIError": APIError,
+    "flama.DropCollection": DropCollection,
+    "flama.LimitOffsetMeta": LimitOffsetMeta,
+    "flama.LimitOffset": LimitOffset,
+    "flama.PageNumberMeta": PageNumberMeta,
+    "flama.PageNumber": PageNumber,
+    "flama.MLModelInput": MLModelInput,
+    "flama.MLModelOutput": MLModelOutput,
 }
diff --git a/flama/schemas/_libs/typesystem/schemas.py b/flama/schemas/_libs/typesystem/schemas.py
@@ -22,15 +22,15 @@
         "error": fields.String(title="type", description="Exception or error type", allow_null=True),
     },
 )
-SCHEMAS["APIError"] = APIError
+SCHEMAS["flama.APIError"] = APIError
 
 DropCollection = Schema(
     title="DropCollection",
     fields={
         "deleted": fields.Integer(title="deleted", description="Number of deleted elements"),
     },
 )
-SCHEMAS["DropCollection"] = DropCollection
+SCHEMAS["flama.DropCollection"] = DropCollection
 
 LimitOffsetMeta = Schema(
     title="LimitOffsetMeta",
@@ -40,16 +40,18 @@
         "count": fields.Integer(title="count", description="Total number of items", allow_null=True),
     },
 )
-SCHEMAS["LimitOffsetMeta"] = LimitOffsetMeta
+SCHEMAS["flama.LimitOffsetMeta"] = LimitOffsetMeta
 
 LimitOffset = Schema(
     title="LimitOffset",
     fields={
-        "meta": Reference(to="LimitOffsetMeta", definitions=SCHEMAS, title="meta", description="Pagination metadata"),
+        "meta": Reference(
+            to="flama.LimitOffsetMeta", definitions=SCHEMAS, title="meta", description="Pagination metadata"
+        ),
         "data": fields.Array(title="data", description="Paginated data"),
     },
 )
-SCHEMAS["LimitOffset"] = LimitOffset
+SCHEMAS["flama.LimitOffset"] = LimitOffset
 
 PageNumberMeta = Schema(
     title="PageNumberMeta",
@@ -59,29 +61,31 @@
         "count": fields.Integer(title="count", description="Total number of items", allow_null=True),
     },
 )
-SCHEMAS["PageNumberMeta"] = PageNumberMeta
+SCHEMAS["flama.PageNumberMeta"] = PageNumberMeta
 
 PageNumber = Schema(
     title="PageNumber",
     fields={
-        "meta": Reference(to="PageNumberMeta", definitions=SCHEMAS, title="meta", description="Pagination metadata"),
+        "meta": Reference(
+            to="flama.PageNumberMeta", definitions=SCHEMAS, title="meta", description="Pagination metadata"
+        ),
         "data": fields.Array(title="data", description="Paginated data"),
     },
 )
-SCHEMAS["PageNumber"] = PageNumber
+SCHEMAS["flama.PageNumber"] = PageNumber
 
 MLModelInput = Schema(
     title="MLModelInput",
     fields={
         "input": fields.Array(title="input", description="Model input"),
     },
 )
-SCHEMAS["MLModelInput"] = MLModelInput
+SCHEMAS["flama.MLModelInput"] = MLModelInput
 
 MLModelOutput = Schema(
     title="MLModelOutput",
     fields={
         "output": fields.Array(title="output", description="Model output"),
     },
 )
-SCHEMAS["MLModelOutput"] = MLModelOutput
+SCHEMAS["flama.MLModelOutput"] = MLModelOutput
diff --git a/flama/schemas/generator.py b/flama/schemas/generator.py
@@ -204,7 +204,11 @@ def register(self, schema: schemas.Schema, name: t.Optional[str] = None) -> int:
                 raise ValueError("Cannot infer schema name.")
 
             try:
-                name = schema_instance.__name__
+                name = (
+                    schema_instance.__qualname__
+                    if schema_instance.__module__ == "builtins"
+                    else f"{schema_instance.__module__}.{schema_instance.__qualname__}"
+                )
             except AttributeError:
                 raise ValueError("Cannot infer schema name.")
 

diff --git a/tests/schemas/test_generator.py b/tests/schemas/test_generator.py
@@ -341,23 +341,27 @@ def test_used(self, registry, foo_schema, spec, operation, output):
         assert registry.used(spec) == expected_output
 
     @pytest.mark.parametrize(
-        "schema,name,exception",
+        ["schema", "name", "expected_name", "exception"],
         [
-            pytest.param(typesystem.Schema(title="Foo", fields={}), "Foo", None, id="typesystem_explicit_name"),
+            pytest.param(typesystem.Schema(title="Foo", fields={}), "Foo", "Foo", None, id="typesystem_explicit_name"),
             pytest.param(
                 typesystem.Schema(title="Foo", fields={}),
                 None,
+                "abc.Foo",
                 ValueError("Cannot infer schema name."),
                 id="typesystem_cannot_infer_name",
             ),
-            pytest.param(type("Foo", (marshmallow.Schema,), {}), None, None, id="marshmallow_infer_name"),
+            pytest.param(type("Foo", (marshmallow.Schema,), {}), None, "abc.Foo", None, id="marshmallow_infer_name"),
+            pytest.param(
+                pydantic.create_model("Foo", name=(str, ...)), None, "pydantic.main.Foo", None, id="pydantic_infer_name"
+            ),
         ],
         indirect=["exception"],
     )
-    def test_register(self, registry, schema, name, exception):
+    def test_register(self, registry, schema, name, expected_name, exception):
         with exception:
             registry.register(schema, name=name)
-            assert registry[schema].name == "Foo"
+            assert registry[schema].name == expected_name
 
     def test_register_already_registered(self, registry, foo_schema):
         with pytest.raises(ValueError, match="Schema is already registered."):
@@ -551,7 +555,7 @@ def test_components_schemas(self, app):
         schemas = app.schema.schema["components"]["schemas"]
 
         # Check declared components are only those that are in use
-        assert set(schemas.keys()) == {"Owner", "Puppy", "BodyParam", "APIError"}
+        assert set(schemas.keys()) == {"Owner", "Puppy", "BodyParam", "flama.APIError"}
 
     @pytest.mark.parametrize(
         "path,verb,expected_schema",
@@ -565,7 +569,9 @@ def test_components_schemas(self, app):
                         "200": {"description": "Param."},
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         },
                     },
                     "parameters": [
@@ -600,7 +606,9 @@ def test_components_schemas(self, app):
                         "200": {"description": "Param."},
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         },
                     },
                     "parameters": [
@@ -623,7 +631,9 @@ def test_components_schemas(self, app):
                         "200": {"description": "Param."},
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         },
                     },
                     "requestBody": {
@@ -653,7 +663,9 @@ def test_components_schemas(self, app):
                         },
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         },
                     },
                 },
@@ -680,7 +692,9 @@ def test_components_schemas(self, app):
                         },
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         },
                     },
                 },
@@ -707,7 +721,9 @@ def test_components_schemas(self, app):
                         },
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         },
                     },
                 },
@@ -734,7 +750,9 @@ def test_components_schemas(self, app):
                         },
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         },
                     },
                 },
@@ -761,7 +779,9 @@ def test_components_schemas(self, app):
                         },
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         },
                     },
                 },
@@ -775,7 +795,9 @@ def test_components_schemas(self, app):
                     "responses": {
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         }
                     },
                 },
@@ -791,7 +813,9 @@ def test_components_schemas(self, app):
                         "400": {"description": "Bad Request."},
                         "default": {
                             "description": "Unexpected error.",
-                            "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}},
+                            "content": {
+                                "application/json": {"schema": {"$ref": "#/components/schemas/flama.APIError"}}
+                            },
                         },
                     },
                 },

diff --git a/tests/test_pagination.py b/tests/test_pagination.py
@@ -38,7 +38,12 @@ def page_number(**kwargs) -> types.Schema[output_schema]:
     def test_registered_schemas(self, app):
         schemas = app.schema.schema["components"]["schemas"]
 
-        assert set(schemas.keys()) == {"OutputSchema", "PageNumberPaginatedOutputSchema", "PageNumberMeta", "APIError"}
+        assert set(schemas.keys()) == {
+            "OutputSchema",
+            "PageNumberPaginatedOutputSchema",
+            "flama.PageNumberMeta",
+            "flama.APIError",
+        }
 
     def test_invalid_view(self, output_schema):
         with pytest.raises(TypeError, match=r"Paginated views must define \*\*kwargs param"):
@@ -167,8 +172,8 @@ def test_registered_schemas(self, app):
         assert set(schemas.keys()) == {
             "OutputSchema",
             "LimitOffsetPaginatedOutputSchema",
-            "LimitOffsetMeta",
-            "APIError",
+            "flama.LimitOffsetMeta",
+            "flama.APIError",
         }
 
     def test_invalid_view(self, app, output_schema):