Python: Turn on mypy for OpenAPI plugins. Increase code coverage. (mi…

…crosoft#7153)

### Motivation and Context

We have mypy enabled on parts of the code base, but not all. The goal is
to enable it across the entire SK python code. As part of this, we've
broken up the work to tackle different sections. Additionally, we're
working to improve the code coverage for these sections of code.

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

This PR:
- turns on mypy for the openapi connector (plugin). Code adjustments
were made per mypy, with the exclusion of two places that contain a
`type: ignore` as the type was already valid, but mypy still complained.
- adds more unit test coverage for the OpenAPI models and classes. All
OpenAPI classes are now >95% covered.
- closes microsoft#7135 

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [X] The code builds clean without any errors or warnings
- [X] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [X] I didn't break anyone 😄

python/mypy.ini

@@ -29,10 +29,6 @@ ignore_errors = true
 ignore_errors = true
 # TODO (eavanvalkenburg): remove this: https://github.com/microsoft/semantic-kernel/issues/7134
 
-[mypy-semantic_kernel.connectors.openapi_plugin.*]
-ignore_errors = true
-# TODO (eavanvalkenburg): remove this: https://github.com/microsoft/semantic-kernel/issues/7135
-
 [mypy-semantic_kernel.connectors.utils.*]
 ignore_errors = true
 # TODO (eavanvalkenburg): remove this: https://github.com/microsoft/semantic-kernel/issues/7136

python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py

@@ -2,7 +2,7 @@
 
 import re
 from typing import Any, Final
-from urllib.parse import urlencode, urljoin, urlparse, urlunparse
+from urllib.parse import ParseResult, urlencode, urljoin, urlparse, urlunparse
 
 from semantic_kernel.connectors.openapi_plugin.models.rest_api_operation_expected_response import (
     RestApiOperationExpectedResponse,
@@ -49,7 +49,7 @@ def __init__(
         self,
         id: str,
         method: str,
-        server_url: str,
+        server_url: str | ParseResult,
         path: str,
         summary: str | None = None,
         description: str | None = None,
@@ -60,11 +60,11 @@ def __init__(
         """Initialize the RestApiOperation."""
         self.id = id
         self.method = method.upper()
-        self.server_url = server_url
+        self.server_url = urlparse(server_url) if isinstance(server_url, str) else server_url
         self.path = path
         self.summary = summary
         self.description = description
-        self.parameters = params
+        self.parameters = params if params else []
         self.request_body = request_body
         self.responses = responses
 
@@ -163,7 +163,7 @@ def get_parameters(
         enable_payload_spacing: bool = False,
     ) -> list["RestApiOperationParameter"]:
         """Get the parameters for the operation."""
-        params = list(operation.parameters)
+        params = list(operation.parameters) if operation.parameters is not None else []
         if operation.request_body is not None:
             params.extend(
                 self.get_payload_parameters(
@@ -221,8 +221,8 @@ def _get_parameters_from_payload_metadata(
     ) -> list["RestApiOperationParameter"]:
         parameters: list[RestApiOperationParameter] = []
         for property in properties:
-            parameter_name = self._get_property_name(property, root_property_name, enable_namespacing)
-            if not property.properties:
+            parameter_name = self._get_property_name(property, root_property_name or False, enable_namespacing)
+            if not hasattr(property, "properties") or not property.properties:
                 parameters.append(
                     RestApiOperationParameter(
                         name=parameter_name,
@@ -234,9 +234,16 @@ def _get_parameters_from_payload_metadata(
                         schema=property.schema,
                     )
                 )
-            parameters.extend(
-                self._get_parameters_from_payload_metadata(property.properties, enable_namespacing, parameter_name)
-            )
+            else:
+                # Handle property.properties as a single instance or a list
+                if isinstance(property.properties, RestApiOperationPayloadProperty):
+                    nested_properties = [property.properties]
+                else:
+                    nested_properties = property.properties
+
+                parameters.extend(
+                    self._get_parameters_from_payload_metadata(nested_properties, enable_namespacing, parameter_name)
+                )
         return parameters
 
     def get_payload_parameters(
@@ -246,7 +253,7 @@ def get_payload_parameters(
         if use_parameters_from_metadata:
             if operation.request_body is None:
                 raise Exception(
-                    f"Payload parameters cannot be retrieved from the `{operation.Id}` "
+                    f"Payload parameters cannot be retrieved from the `{operation.id}` "
                     f"operation payload metadata because it is missing."
                 )
             if operation.request_body.media_type == RestApiOperation.MEDIA_TYPE_TEXT_PLAIN:
@@ -256,7 +263,7 @@ def get_payload_parameters(
 
         return [
             self.create_payload_artificial_parameter(operation),
-            self.create_content_type_artificial_parameter(operation),
+            self.create_content_type_artificial_parameter(),
         ]
 
     def get_default_response(
@@ -276,14 +283,25 @@ def get_default_return_parameter(self, preferred_responses: list[str] | None = N
         if preferred_responses is None:
             preferred_responses = self._preferred_responses
 
-        rest_operation_response = self.get_default_response(self.responses, preferred_responses)
+        responses = self.responses if self.responses is not None else {}
+
+        rest_operation_response = self.get_default_response(responses, preferred_responses)
+
+        schema_type = None
+        if rest_operation_response is not None and rest_operation_response.schema is not None:
+            schema_type = rest_operation_response.schema.get("type")
 
         if rest_operation_response:
             return KernelParameterMetadata(
                 name="return",
                 description=rest_operation_response.description,
-                type_=rest_operation_response.schema.get("type") if rest_operation_response.schema else None,
+                type_=schema_type,
                 schema_data=rest_operation_response.schema,
             )
 
-        return None
+        return KernelParameterMetadata(
+            name="return",
+            description="Default return parameter",
+            type_="string",
+            schema_data={"type": "string"},
+        )

python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation_expected_response.py

@@ -6,7 +6,7 @@
 
 @experimental_class
 class RestApiOperationExpectedResponse:
-    def __init__(self, description: str, media_type: str, schema: str | None = None):
+    def __init__(self, description: str, media_type: str, schema: dict[str, str] | None = None):
         """Initialize the RestApiOperationExpectedResponse."""
         self.description = description
         self.media_type = media_type

python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation_run_options.py

@@ -7,7 +7,7 @@
 class RestApiOperationRunOptions:
     """The options for running the REST API operation."""
 
-    def __init__(self, server_url_override=None, api_host_url=None):
+    def __init__(self, server_url_override=None, api_host_url=None) -> None:
         """Initialize the REST API operation run options."""
         self.server_url_override: str = server_url_override
         self.api_host_url: str = api_host_url

python/semantic_kernel/connectors/openapi_plugin/openapi_manager.py

@@ -46,12 +46,14 @@ def create_functions_from_openapi(
         list[KernelFunctionFromMethod]: the operations as functions
     """
     parser = OpenApiParser()
-    parsed_doc = parser.parse(openapi_document_path)
+    if (parsed_doc := parser.parse(openapi_document_path)) is None:
+        raise FunctionExecutionException(f"Error parsing OpenAPI document: {openapi_document_path}")
     operations = parser.create_rest_api_operations(parsed_doc, execution_settings=execution_settings)
 
     auth_callback = None
     if execution_settings and execution_settings.auth_callback:
         auth_callback = execution_settings.auth_callback
+
     openapi_runner = OpenApiRunner(
         parsed_openapi_document=parsed_doc,
         auth_callback=auth_callback,
@@ -129,11 +131,13 @@ async def run_openapi_operation(
             description=f"{p.description or p.name}",
             default_value=p.default_value or "",
             is_required=p.is_required,
-            type_=p.type if p.type is not None else TYPE_MAPPING.get(p.type, None),
+            type_=p.type if p.type is not None else TYPE_MAPPING.get(p.type, "object"),
             schema_data=(
                 p.schema
                 if p.schema is not None and isinstance(p.schema, dict)
-                else {"type": f"{p.type}"} if p.type else None
+                else {"type": f"{p.type}"}
+                if p.type
+                else None
             ),
         )
         for p in rest_operation_params

python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py

@@ -118,13 +118,19 @@ def _get_payload_properties(self, operation_id, schema, required_properties, lev
 
     def _create_rest_api_operation_payload(
         self, operation_id: str, request_body: dict[str, Any]
-    ) -> RestApiOperationPayload:
+    ) -> RestApiOperationPayload | None:
         if request_body is None or request_body.get("content") is None:
             return None
-        media_type = next((mt for mt in OpenApiParser.SUPPORTED_MEDIA_TYPES if mt in request_body.get("content")), None)
+
+        content = request_body.get("content")
+        if content is None:
+            return None
+
+        media_type = next((mt for mt in OpenApiParser.SUPPORTED_MEDIA_TYPES if mt in content), None)
         if media_type is None:
             raise Exception(f"Neither of the media types of {operation_id} is supported.")
-        media_type_metadata = request_body.get("content")[media_type]
+
+        media_type_metadata = content[media_type]
         payload_properties = self._get_payload_properties(
             operation_id, media_type_metadata["schema"], media_type_metadata["schema"].get("required", set())
         )

python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py

@@ -3,7 +3,8 @@
 import json
 import logging
 from collections import OrderedDict
-from collections.abc import Callable, Mapping
+from collections.abc import Awaitable, Callable, Mapping
+from inspect import isawaitable
 from typing import Any
 from urllib.parse import urlparse, urlunparse
 
@@ -34,13 +35,13 @@ class OpenApiRunner:
     def __init__(
         self,
         parsed_openapi_document: Mapping[str, str],
-        auth_callback: Callable[[dict[str, str]], dict[str, str]] | None = None,
+        auth_callback: Callable[..., dict[str, str] | Awaitable[dict[str, str]]] | None = None,
         http_client: httpx.AsyncClient | None = None,
         enable_dynamic_payload: bool = True,
         enable_payload_namespacing: bool = False,
     ):
         """Initialize the OpenApiRunner."""
-        self.spec = Spec.from_dict(parsed_openapi_document)
+        self.spec = Spec.from_dict(parsed_openapi_document)  # type: ignore
         self.auth_callback = auth_callback
         self.http_client = http_client
         self.enable_dynamic_payload = enable_dynamic_payload
@@ -99,19 +100,27 @@ def build_json_object(self, properties, arguments, property_namespace=None):
                 )
         return result
 
-    def build_operation_payload(self, operation: RestApiOperation, arguments: KernelArguments) -> tuple[str, str]:
+    def build_operation_payload(
+        self, operation: RestApiOperation, arguments: KernelArguments
+    ) -> tuple[str, str] | tuple[None, None]:
         """Build the operation payload."""
         if operation.request_body is None and self.payload_argument_name not in arguments:
             return None, None
-        return self.build_json_payload(operation.request_body, arguments)
+
+        if operation.request_body is not None:
+            return self.build_json_payload(operation.request_body, arguments)
+
+        return None, None
 
     def get_argument_name_for_payload(self, property_name, property_namespace=None):
         """Get argument name for the payload."""
         if not self.enable_payload_namespacing:
             return property_name
         return f"{property_namespace}.{property_name}" if property_namespace else property_name
 
-    def _get_first_response_media_type(self, responses: OrderedDict[str, RestApiOperationExpectedResponse]) -> str:
+    def _get_first_response_media_type(
+        self, responses: OrderedDict[str, RestApiOperationExpectedResponse] | None
+    ) -> str:
         if responses:
             first_response = next(iter(responses.values()))
             return first_response.media_type if first_response.media_type else self.media_type_application_json
@@ -123,30 +132,36 @@ async def run_operation(
         arguments: KernelArguments | None = None,
         options: RestApiOperationRunOptions | None = None,
     ) -> str:
-        """Run the operation."""
+        """Runs the operation defined in the OpenAPI manifest."""
+        if not arguments:
+            arguments = KernelArguments()
         url = self.build_operation_url(
             operation=operation,
             arguments=arguments,
-            server_url_override=options.server_url_override,
-            api_host_url=options.api_host_url,
+            server_url_override=options.server_url_override if options else None,
+            api_host_url=options.api_host_url if options else None,
         )
         headers = operation.build_headers(arguments=arguments)
         payload, _ = self.build_operation_payload(operation=operation, arguments=arguments)
 
-        """Runs the operation defined in the OpenAPI manifest"""
-        if headers is None:
-            headers = {}
-
         if self.auth_callback:
-            headers_update = await self.auth_callback(headers=headers)
-            headers.update(headers_update)
+            headers_update = self.auth_callback(**headers)
+            if isawaitable(headers_update):
+                headers_update = await headers_update
+            # at this point, headers_update is a valid dictionary
+            headers.update(headers_update)  # type: ignore
 
         if APP_INFO:
             headers.update(APP_INFO)
             headers = prepend_semantic_kernel_to_user_agent(headers)
 
         if "Content-Type" not in headers:
-            headers["Content-Type"] = self._get_first_response_media_type(operation.responses)
+            responses = (
+                operation.responses
+                if isinstance(operation.responses, OrderedDict)
+                else OrderedDict(operation.responses or {})
+            )
+            headers["Content-Type"] = self._get_first_response_media_type(responses)
 
         async def fetch():
             async def make_request(client: httpx.AsyncClient):