feat: Alert users when they have requested a header type which is not allowed

Author: dbanty

openapi_python_client/parser/openapi.py

@@ -287,9 +287,6 @@ def add_parameters(
             if isinstance(param, oai.Reference) or param.param_schema is None:
                 continue
 
-            if param.param_in == oai.ParameterLocation.PATH and not param.required:
-                return ParseError(data=param, detail="Path parameter must be required"), schemas
-
             unique_param = (param.name, param.param_in)
             if unique_param in unique_parameters:
                 duplication_detail = (
@@ -300,7 +297,7 @@ def add_parameters(
                 return ParseError(data=data, detail=duplication_detail), schemas
             unique_parameters.add(unique_param)
 
-            prop, schemas = property_from_data(
+            prop, new_schemas = property_from_data(
                 name=param.name,
                 required=param.required,
                 data=param.param_schema,
@@ -310,6 +307,11 @@ def add_parameters(
             )
             if isinstance(prop, ParseError):
                 return ParseError(detail=f"cannot parse parameter of endpoint {endpoint.name}", data=prop.data), schemas
+            location_error = prop.validate_location(param.param_in)
+            if location_error is not None:
+                location_error.data = param
+                return location_error, schemas
+            schemas = new_schemas
             if prop.name in parameters_by_location[param.param_in]:
                 # This parameter was defined in the Operation, so ignore the PathItem definition
                 continue

openapi_python_client/parser/properties/__init__.py

@@ -49,6 +49,12 @@ class StringProperty(Property):
     pattern: Optional[str] = None
     _type_string: ClassVar[str] = "str"
     _json_type_string: ClassVar[str] = "str"
+    _allowed_locations: ClassVar[Set[oai.ParameterLocation]] = {
+        oai.ParameterLocation.QUERY,
+        oai.ParameterLocation.PATH,
+        oai.ParameterLocation.COOKIE,
+        oai.ParameterLocation.HEADER,
+    }
 
 
 @attr.s(auto_attribs=True, frozen=True)
@@ -123,6 +129,12 @@ class FloatProperty(Property):
 
     _type_string: ClassVar[str] = "float"
     _json_type_string: ClassVar[str] = "float"
+    _allowed_locations: ClassVar[Set[oai.ParameterLocation]] = {
+        oai.ParameterLocation.QUERY,
+        oai.ParameterLocation.PATH,
+        oai.ParameterLocation.COOKIE,
+        oai.ParameterLocation.HEADER,
+    }
     template: ClassVar[str] = "float_property.py.jinja"
 
 
@@ -132,6 +144,12 @@ class IntProperty(Property):
 
     _type_string: ClassVar[str] = "int"
     _json_type_string: ClassVar[str] = "int"
+    _allowed_locations: ClassVar[Set[oai.ParameterLocation]] = {
+        oai.ParameterLocation.QUERY,
+        oai.ParameterLocation.PATH,
+        oai.ParameterLocation.COOKIE,
+        oai.ParameterLocation.HEADER,
+    }
     template: ClassVar[str] = "int_property.py.jinja"
 
 
@@ -141,6 +159,12 @@ class BooleanProperty(Property):
 
     _type_string: ClassVar[str] = "bool"
     _json_type_string: ClassVar[str] = "bool"
+    _allowed_locations: ClassVar[Set[oai.ParameterLocation]] = {
+        oai.ParameterLocation.QUERY,
+        oai.ParameterLocation.PATH,
+        oai.ParameterLocation.COOKIE,
+        oai.ParameterLocation.HEADER,
+    }
     template: ClassVar[str] = "boolean_property.py.jinja"
 
 

openapi_python_client/parser/properties/property.py

@@ -5,7 +5,9 @@
 import attr
 
 from ... import Config
+from ... import schema as oai
 from ...utils import PythonIdentifier
+from ..errors import ParseError
 
 
 @attr.s(auto_attribs=True, frozen=True)
@@ -28,6 +30,11 @@ class Property:
     nullable: bool
     _type_string: ClassVar[str] = ""
     _json_type_string: ClassVar[str] = ""  # Type of the property after JSON serialization
+    _allowed_locations: ClassVar[Set[oai.ParameterLocation]] = {
+        oai.ParameterLocation.QUERY,
+        oai.ParameterLocation.PATH,
+        oai.ParameterLocation.COOKIE,
+    }
     default: Optional[str] = attr.ib()
     python_name: PythonIdentifier
     description: Optional[str] = attr.ib()
@@ -36,6 +43,14 @@ class Property:
     template: ClassVar[str] = "any_property.py.jinja"
     json_is_dict: ClassVar[bool] = False
 
+    def validate_location(self, location: oai.ParameterLocation) -> Optional[ParseError]:
+        """Returns an error if this type of property is not allowed in the given location"""
+        if location not in self._allowed_locations:
+            return ParseError(detail=f"{self.get_type_string()} is not allowed in {location}")
+        if location == oai.ParameterLocation.PATH and not self.required:
+            return ParseError(detail="Path parameter must be required")
+        return None
+
     def set_python_name(self, new_name: str, config: Config) -> None:
         """Mutates this Property to set a new python_name.
 

tests/test_parser/test_openapi.py

@@ -507,9 +507,38 @@ def test_add_parameters_parse_error(self, mocker):
         )
         assert result == (
             ParseError(data=parse_error.data, detail=f"cannot parse parameter of endpoint {endpoint.name}"),
-            property_schemas,
+            initial_schemas,
         )
 
+    @pytest.mark.parametrize(
+        "data_type, allowed",
+        [
+            (oai.DataType.STRING, True),
+            (oai.DataType.INTEGER, True),
+            (oai.DataType.NUMBER, True),
+            (oai.DataType.BOOLEAN, True),
+            (oai.DataType.ARRAY, False),
+            (oai.DataType.OBJECT, False),
+        ],
+    )
+    def test_add_parameters_header_types(self, data_type, allowed):
+        from openapi_python_client.parser.openapi import Endpoint
+
+        endpoint = self.make_endpoint()
+        initial_schemas = Schemas()
+        param = oai.Parameter.construct(
+            name="test", required=True, param_schema=oai.Schema(type=data_type), param_in=oai.ParameterLocation.HEADER
+        )
+        config = Config()
+
+        result = Endpoint.add_parameters(
+            endpoint=endpoint, data=oai.Operation.construct(parameters=[param]), schemas=initial_schemas, config=config
+        )
+        if allowed:
+            assert isinstance(result[0], Endpoint)
+        else:
+            assert isinstance(result[0], ParseError)
+
     def test__add_parameters_parse_error_on_non_required_path_param(self):
         endpoint = self.make_endpoint()
         param = oai.Parameter.construct(
@@ -534,13 +563,12 @@ def test_validation_error_when_location_not_supported(self, mocker):
     def test__add_parameters_with_location_postfix_conflict1(self, mocker, property_factory):
         """Checks when the PythonIdentifier of new parameter already used."""
         from openapi_python_client.parser.openapi import Endpoint
-        from openapi_python_client.parser.properties import Property
 
         endpoint = self.make_endpoint()
 
-        path_prop_conflicted = property_factory(name="prop_name_path", required=False, nullable=False, default=None)
-        query_prop = property_factory(name="prop_name", required=False, nullable=False, default=None)
-        path_prop = property_factory(name="prop_name", required=False, nullable=False, default=None)
+        path_prop_conflicted = property_factory(name="prop_name_path", required=True, nullable=False, default=None)
+        query_prop = property_factory(name="prop_name", required=True, nullable=False, default=None)
+        path_prop = property_factory(name="prop_name", required=True, nullable=False, default=None)
 
         schemas_1 = mocker.MagicMock()
         schemas_2 = mocker.MagicMock()
@@ -582,9 +610,9 @@ def test__add_parameters_with_location_postfix_conflict2(self, mocker, property_
         from openapi_python_client.parser.openapi import Endpoint
 
         endpoint = self.make_endpoint()
-        path_prop_conflicted = property_factory(name="prop_name_path", required=False, nullable=False, default=None)
-        path_prop = property_factory(name="prop_name", required=False, nullable=False, default=None)
-        query_prop = property_factory(name="prop_name", required=False, nullable=False, default=None)
+        path_prop_conflicted = property_factory(name="prop_name_path", required=True, nullable=False, default=None)
+        path_prop = property_factory(name="prop_name", required=True, nullable=False, default=None)
+        query_prop = property_factory(name="prop_name", required=True, nullable=False, default=None)
         schemas_1 = mocker.MagicMock()
         schemas_2 = mocker.MagicMock()
         schemas_3 = mocker.MagicMock()