From ac4f394b1f9fe9af246b43092218a062c4367d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=B5=E3=81=81=E3=83=BC?= <47295014+ymuichiro@users.noreply.github.com> Date: Wed, 18 Sep 2024 05:06:38 +0900 Subject: [PATCH] Python: Fixed an issue where the schema property was missing when using the OpenAPI plugin with the get method. (#8502) Fixed an issue where the schema property was missing when using the OpenAPI plugin with the get method. ### Motivation and Context This Pull Request is based on Issue https://github.com/microsoft/semantic-kernel/issues/8423. When loading an openapi plugin that uses the get method, properties other than the "type" key in parameters.schema is missing. ### Description The parameters.schema includes properties other than the "type" key, such as "description", which is used for the llm to make judgments during function calling. Therefore, I modified it to include this information. [openapi schema](https://github.com/OAI/OpenAPI-Specification/blob/4e9d2b3ec859beef08309c414f895c73a793b958/schemas/v3.0/schema.yaml#L658) ### Contribution Checklist - [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 :smile: --------- Co-authored-by: Tao Chen Co-authored-by: Evan Mattson <35585003+moonbox3@users.noreply.github.com> --- .../models/rest_api_operation_parameter.py | 2 +- .../openapi_plugin/openapi_parser.py | 17 ++--- .../unit/connectors/openapi/openapi_todo.yaml | 57 +++++++++++++++++ .../connectors/openapi/test_openapi_parser.py | 62 ++++++++++++++++++- 4 files changed, 128 insertions(+), 10 deletions(-) create mode 100644 python/tests/unit/connectors/openapi/openapi_todo.yaml diff --git a/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation_parameter.py b/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation_parameter.py index 761e390c9d4c..0f8745e08f2e 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation_parameter.py +++ b/python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation_parameter.py @@ -28,7 +28,7 @@ def __init__( description: str | None = None, is_required: bool = False, default_value: Any | None = None, - schema: str | None = None, + schema: str | dict | None = None, response: RestApiOperationExpectedResponse | None = None, ): """Initialize the RestApiOperationParameter.""" diff --git a/python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py b/python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py index 85f13a096908..984f120e837b 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py +++ b/python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py @@ -63,26 +63,27 @@ def _parse_parameters(self, parameters: list[dict[str, Any]]): """Parse the parameters from the OpenAPI document.""" result: list[RestApiOperationParameter] = [] for param in parameters: - name = param["name"] - type = param["schema"]["type"] + name: str = param["name"] if not param.get("in"): raise PluginInitializationError(f"Parameter {name} is missing 'in' field") + if param.get("content", None) is not None: + # The schema and content fields are mutually exclusive. + raise PluginInitializationError(f"Parameter {name} cannot have a 'content' field. Expected: schema.") location = RestApiOperationParameterLocation(param["in"]) - description = param.get("description", None) - is_required = param.get("required", False) + description: str = param.get("description", None) + is_required: bool = param.get("required", False) default_value = param.get("default", None) - schema = param.get("schema", None) - schema_type = schema.get("type", None) if schema else "string" + schema: dict[str, Any] | None = param.get("schema", None) result.append( RestApiOperationParameter( name=name, - type=type, + type=schema.get("type", "string") if schema else "string", location=location, description=description, is_required=is_required, default_value=default_value, - schema=schema_type, + schema=schema if schema else {"type": "string"}, ) ) return result diff --git a/python/tests/unit/connectors/openapi/openapi_todo.yaml b/python/tests/unit/connectors/openapi/openapi_todo.yaml new file mode 100644 index 000000000000..3afd713b809e --- /dev/null +++ b/python/tests/unit/connectors/openapi/openapi_todo.yaml @@ -0,0 +1,57 @@ +openapi: 3.0.0 +info: + title: Todo List API + version: 1.0.0 + description: API for managing todo lists +paths: + /list: + get: + summary: Get todo list + operationId: get_todo_list + description: get todo list from specific group + parameters: + - name: listName + in: query + required: true + description: todo list group name description + schema: + type: string + description: todo list group name + responses: + "200": + description: Successful response + content: + application/json: + schema: + type: array + items: + type: object + properties: + task: + type: string + listName: + type: string + + /add: + post: + summary: Add a task to a list + operationId: add_todo_list + description: add todo to specific group + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - task + properties: + task: + type: string + description: task name + listName: + type: string + description: task group name + responses: + "201": + description: Task added successfully diff --git a/python/tests/unit/connectors/openapi/test_openapi_parser.py b/python/tests/unit/connectors/openapi/test_openapi_parser.py index 71548537e30a..0e4d278a1667 100644 --- a/python/tests/unit/connectors/openapi/test_openapi_parser.py +++ b/python/tests/unit/connectors/openapi/test_openapi_parser.py @@ -1,10 +1,14 @@ # Copyright (c) Microsoft. All rights reserved. +import os import pytest -from semantic_kernel.connectors.openapi_plugin.openapi_manager import OpenApiParser +from semantic_kernel.connectors.openapi_plugin.openapi_manager import OpenApiParser, create_functions_from_openapi from semantic_kernel.exceptions.function_exceptions import PluginInitializationError +from semantic_kernel.functions import KernelFunctionFromMethod, KernelFunctionMetadata, KernelParameterMetadata + +current_dir = os.path.dirname(os.path.abspath(__file__)) def test_parse_parameters_missing_in_field(): @@ -14,6 +18,62 @@ def test_parse_parameters_missing_in_field(): parser._parse_parameters(parameters) +def test_parse_parameters_get_query(): + """Verify whether the get request query parameter can be successfully parsed""" + openapi_fcs: list[KernelFunctionFromMethod] = create_functions_from_openapi( + plugin_name="todo", + openapi_document_path=os.path.join(current_dir, "openapi_todo.yaml"), + execution_settings=None, + ) + + get_todo_list: list[KernelFunctionMetadata] = [ + f.metadata for f in openapi_fcs if f.metadata.name == "get_todo_list" + ] + + assert get_todo_list + + get_todo_params: list[KernelParameterMetadata] = get_todo_list[0].parameters + assert get_todo_params + assert get_todo_params[0].name == "listName" + assert get_todo_params[0].description == "todo list group name description" + assert get_todo_params[0].is_required + assert get_todo_params[0].schema_data + assert get_todo_params[0].schema_data.get("type") == "string" + assert get_todo_params[0].schema_data.get("description") == "todo list group name" + + +def test_parse_parameters_post_request_body(): + """Verify whether the post request body parameter can be successfully parsed""" + openapi_fcs: list[KernelFunctionFromMethod] = create_functions_from_openapi( + plugin_name="todo", + openapi_document_path=os.path.join(current_dir, "openapi_todo.yaml"), + execution_settings=None, + ) + + add_todo_list: list[KernelFunctionMetadata] = [ + f.metadata for f in openapi_fcs if f.metadata.name == "add_todo_list" + ] + + assert add_todo_list + + add_todo_params: list[KernelParameterMetadata] = add_todo_list[0].parameters + + assert add_todo_params + assert add_todo_params[0].name == "task" + assert add_todo_params[0].description == "task name" + assert add_todo_params[0].is_required + assert add_todo_params[0].schema_data + assert add_todo_params[0].schema_data.get("type") == "string" + assert add_todo_params[0].schema_data.get("description") == "task name" + + assert add_todo_params[1].name == "listName" + assert add_todo_params[1].description == "task group name" + assert not add_todo_params[1].is_required + assert add_todo_params[1].schema_data + assert add_todo_params[1].schema_data.get("type") == "string" + assert add_todo_params[1].schema_data.get("description") == "task group name" + + def test_get_payload_properties_schema_none(): parser = OpenApiParser() properties = parser._get_payload_properties("operation_id", None, [])