refactor(parser): Schema datatypes validation by pydantic [openapi-generators#478]. Thanks @mtovts!

mtovts · web-flow · commit 2d74640a6a6a · 2021-08-23T06:53:32.000-06:00
diff --git a/openapi_python_client/parser/properties/__init__.py b/openapi_python_client/parser/properties/__init__.py
@@ -535,9 +535,9 @@ def _property_from_data(
         return build_union_property(
             data=data, name=name, required=required, schemas=schemas, parent_name=parent_name, config=config
         )
-    if data.type == "string":
+    if data.type == oai.DataType.STRING:
         return _string_based_property(name=name, required=required, data=data, config=config), schemas
-    if data.type == "number":
+    if data.type == oai.DataType.NUMBER:
         return (
             FloatProperty(
                 name=name,
@@ -548,7 +548,7 @@ def _property_from_data(
             ),
             schemas,
         )
-    if data.type == "integer":
+    if data.type == oai.DataType.INTEGER:
         return (
             IntProperty(
                 name=name,
@@ -559,7 +559,7 @@ def _property_from_data(
             ),
             schemas,
         )
-    if data.type == "boolean":
+    if data.type == oai.DataType.BOOLEAN:
         return (
             BooleanProperty(
                 name=name,
@@ -570,26 +570,24 @@ def _property_from_data(
             ),
             schemas,
         )
-    if data.type == "array":
+    if data.type == oai.DataType.ARRAY:
         return build_list_property(
             data=data, name=name, required=required, schemas=schemas, parent_name=parent_name, config=config
         )
-    if data.type == "object" or data.allOf:
+    if data.type == oai.DataType.OBJECT or data.allOf:
         return build_model_property(
             data=data, name=name, schemas=schemas, required=required, parent_name=parent_name, config=config
         )
-    if not data.type:
-        return (
-            AnyProperty(
-                name=name,
-                required=required,
-                nullable=False,
-                default=None,
-                python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
-            ),
-            schemas,
-        )
-    return PropertyError(data=data, detail=f"unknown type {data.type}"), schemas
+    return (
+        AnyProperty(
+            name=name,
+            required=required,
+            nullable=False,
+            default=None,
+            python_name=utils.PythonIdentifier(value=name, prefix=config.field_prefix),
+        ),
+        schemas,
+    )
 
 
 def property_from_data(
diff --git a/openapi_python_client/schema/__init__.py b/openapi_python_client/schema/__init__.py
@@ -4,6 +4,7 @@
     "Operation",
     "Parameter",
     "ParameterLocation",
+    "DataType",
     "PathItem",
     "Reference",
     "RequestBody",
@@ -13,6 +14,7 @@
 ]
 
 
+from .data_type import DataType
 from .openapi_schema_pydantic import (
     MediaType,
     OpenAPI,
diff --git a/openapi_python_client/schema/data_type.py b/openapi_python_client/schema/data_type.py
@@ -0,0 +1,16 @@
+from enum import Enum
+
+
+class DataType(str, Enum):
+    """The data type of a schema is defined by the type keyword
+
+    References:
+        - https://swagger.io/docs/specification/data-models/data-types/
+    """
+
+    STRING = "string"
+    NUMBER = "number"
+    INTEGER = "integer"
+    BOOLEAN = "boolean"
+    ARRAY = "array"
+    OBJECT = "object"
diff --git a/openapi_python_client/schema/openapi_schema_pydantic/schema.py b/openapi_python_client/schema/openapi_schema_pydantic/schema.py
@@ -2,6 +2,7 @@
 
 from pydantic import BaseModel, Field
 
+from ..data_type import DataType
 from .discriminator import Discriminator
 from .external_documentation import ExternalDocumentation
 from .reference import Reference
@@ -35,7 +36,7 @@ class Schema(BaseModel):
     minProperties: Optional[int] = Field(default=None, ge=0)
     required: Optional[List[str]] = Field(default=None, min_items=1)
     enum: Optional[List[Any]] = Field(default=None, min_items=1)
-    type: Optional[str] = None
+    type: Optional[DataType] = Field(default=None)
     allOf: Optional[List[Union[Reference, "Schema"]]] = None
     oneOf: List[Union[Reference, "Schema"]] = []
     anyOf: List[Union[Reference, "Schema"]] = []
diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py
@@ -605,21 +605,6 @@ def test_property_from_data_union_of_one_element(self, mocker, model_property_fa
         assert prop == attr.evolve(existing_model, name=name, required=required, nullable=nullable, python_name=name)
         build_union_property.assert_not_called()
 
-    def test_property_from_data_unsupported_type(self, mocker):
-        name = mocker.MagicMock()
-        required = mocker.MagicMock()
-        data = oai.Schema.construct(type=mocker.MagicMock())
-
-        from openapi_python_client.parser.errors import PropertyError
-        from openapi_python_client.parser.properties import Schemas, property_from_data
-
-        assert property_from_data(
-            name=name, required=required, data=data, schemas=Schemas(), parent_name="parent", config=MagicMock()
-        ) == (
-            PropertyError(data=data, detail=f"unknown type {data.type}"),
-            Schemas(),
-        )
-
     def test_property_from_data_no_valid_props_in_data(self):
         from openapi_python_client.parser.properties import AnyProperty, Schemas, property_from_data
 
@@ -744,19 +729,19 @@ def test_property_from_data_union(
         assert p == expected
         assert s == Schemas()
 
-    def test_property_from_data_union_bad_type(self, mocker):
+    def test_build_union_property_invalid_property(self, mocker):
         name = "bad_union"
         required = mocker.MagicMock()
-        data = oai.Schema(anyOf=[{"type": "garbage"}])
+        reference = oai.Reference.construct(ref="#/components/schema/NotExist")
+        data = oai.Schema(anyOf=[reference])
         mocker.patch("openapi_python_client.utils.remove_string_escapes", return_value=name)
 
-        from openapi_python_client.parser.properties import Schemas, property_from_data
+        from openapi_python_client.parser.properties import Schemas, build_union_property
 
-        p, s = property_from_data(
+        p, s = build_union_property(
             name=name, required=required, data=data, schemas=Schemas(), parent_name="parent", config=MagicMock()
         )
-
-        assert p == PropertyError(detail=f"Invalid property in union {name}", data=oai.Schema(type="garbage"))
+        assert p == PropertyError(detail=f"Invalid property in union {name}", data=reference)
 
 
 class TestStringBasedProperty:
diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py
@@ -140,41 +140,33 @@ def test_model_name_conflict(self):
         assert new_schemas == schemas
         assert err == PropertyError(detail='Attempted to generate duplicate models with name "OtherModel"', data=data)
 
-    def test_bad_props_return_error(self):
+    def test_model_bad_properties(self):
         from openapi_python_client.parser.properties import Schemas, build_model_property
 
         data = oai.Schema(
             properties={
-                "bad": oai.Schema(type="not_real"),
+                "bad": oai.Reference.construct(ref="#/components/schema/NotExist"),
             },
         )
-        schemas = Schemas()
-
-        err, new_schemas = build_model_property(
-            data=data, name="prop", schemas=schemas, required=True, parent_name=None, config=Config()
-        )
-
-        assert new_schemas == schemas
-        assert err == PropertyError(detail="unknown type not_real", data=oai.Schema(type="not_real"))
+        result = build_model_property(
+            data=data, name="prop", schemas=Schemas(), required=True, parent_name="parent", config=Config()
+        )[0]
+        assert isinstance(result, PropertyError)
 
-    def test_bad_additional_props_return_error(self):
-        from openapi_python_client.parser.properties import Config, Schemas, build_model_property
+    def test_model_bad_additional_properties(self):
+        from openapi_python_client.parser.properties import Schemas, build_model_property
 
         additional_properties = oai.Schema(
             type="object",
             properties={
-                "bad": oai.Schema(type="not_real"),
+                "bad": oai.Reference(ref="#/components/schemas/not_exist"),
             },
         )
         data = oai.Schema(additionalProperties=additional_properties)
-        schemas = Schemas()
-
-        err, new_schemas = build_model_property(
-            data=data, name="prop", schemas=schemas, required=True, parent_name=None, config=Config()
-        )
-
-        assert new_schemas == schemas
-        assert err == PropertyError(detail="unknown type not_real", data=oai.Schema(type="not_real"))
+        result = build_model_property(
+            data=data, name="prop", schemas=Schemas(), required=True, parent_name="parent", config=Config()
+        )[0]
+        assert isinstance(result, PropertyError)
 
 
 class TestProcessProperties:
@@ -198,6 +190,20 @@ def test_conflicting_properties_different_types(
 
         assert isinstance(result, PropertyError)
 
+    def test_process_properties_reference_not_exist(self):
+        from openapi_python_client.parser.properties import Schemas
+        from openapi_python_client.parser.properties.model_property import _process_properties
+
+        data = oai.Schema(
+            properties={
+                "bad": oai.Reference.construct(ref="#/components/schema/NotExist"),
+            },
+        )
+
+        result = _process_properties(data=data, class_name="", schemas=Schemas(), config=Config())
+
+        assert isinstance(result, PropertyError)
+
     def test_invalid_reference(self, model_property_factory):
         from openapi_python_client.parser.properties import Schemas
         from openapi_python_client.parser.properties.model_property import _process_properties
diff --git a/tests/test_schema/test_data_type.py b/tests/test_schema/test_data_type.py
@@ -0,0 +1,35 @@
+import pytest
+
+import openapi_python_client.schema as oai
+
+
+class TestDataType:
+    def test_schema_bad_types(self):
+        import pydantic
+
+        with pytest.raises(pydantic.ValidationError):
+            oai.Schema(type="bad_type")
+
+        with pytest.raises(pydantic.ValidationError):
+            oai.Schema(anyOf=[{"type": "garbage"}])
+
+        with pytest.raises(pydantic.ValidationError):
+            oai.Schema(
+                properties={
+                    "bad": oai.Schema(type="not_real"),
+                },
+            )
+
+    @pytest.mark.parametrize(
+        "type_",
+        (
+            "string",
+            "number",
+            "integer",
+            "boolean",
+            "array",
+            "object",
+        ),
+    )
+    def test_schema_happy(self, type_):
+        assert oai.Schema(type=type_).type == type_
