From 96b14ef64da192910ec4b682548cf44d5c028d4c Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 17 Dec 2024 10:11:47 +0000 Subject: [PATCH 1/2] fix payload parameter value resolution --- .../Extensions/RestApiOperationExtensions.cs | 6 ++ .../OpenApiDocumentParserExtensionsTests.cs | 2 +- .../OpenApi/OpenApiDocumentParserV20Tests.cs | 4 +- .../OpenApi/OpenApiDocumentParserV30Tests.cs | 2 +- .../OpenApi/OpenApiDocumentParserV31Tests.cs | 2 +- .../OpenApiKernelPluginFactoryTests.cs | 93 ++++++++++++++++++- .../OpenApi/TestPlugins/documentV2_0.json | 57 ++++++++++++ .../OpenApi/TestPlugins/documentV3_0.json | 61 ++++++++++++ .../OpenApi/TestPlugins/documentV3_1.yaml | 46 ++++++++- 9 files changed, 265 insertions(+), 8 deletions(-) diff --git a/dotnet/src/Functions/Functions.OpenApi/Extensions/RestApiOperationExtensions.cs b/dotnet/src/Functions/Functions.OpenApi/Extensions/RestApiOperationExtensions.cs index 478306812f10..ad2a7a7c5101 100644 --- a/dotnet/src/Functions/Functions.OpenApi/Extensions/RestApiOperationExtensions.cs +++ b/dotnet/src/Functions/Functions.OpenApi/Extensions/RestApiOperationExtensions.cs @@ -183,6 +183,12 @@ private static List GetParametersFromPayloadMetadata(RestApiOp if (!property.Properties.Any()) { + // Assign an argument name (sanitized form of the property name) so that the parameter value look-up / resolution functionality in the RestApiOperationRunner + // class can find the value for the parameter by the argument name in the arguments dictionary. If the argument name is not assigned here, the resolution mechanism + // will try to find the parameter value by the parameter's original name. However, because the parameter was advertised with the sanitized name by the RestApiOperationExtensions.GetParameters + // method, no value will be found, and an exception will be thrown: "No argument is found for the 'customerid_contact@odata.bind' payload property." + property.ArgumentName ??= InvalidSymbolsRegex().Replace(parameterName, "_"); + var parameter = new RestApiParameter( name: parameterName, type: property.Type, diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserExtensionsTests.cs b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserExtensionsTests.cs index 88cb52d183e6..52fdd6371496 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserExtensionsTests.cs +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserExtensionsTests.cs @@ -91,7 +91,7 @@ public async Task ItCanParseMediaTypeAsync(string documentName) // Assert. Assert.NotNull(restApi.Operations); - Assert.Equal(7, restApi.Operations.Count); + Assert.Equal(8, restApi.Operations.Count); var operation = restApi.Operations.Single(o => o.Id == "Joke"); Assert.NotNull(operation); Assert.Equal("application/json; x-api-version=2.0", operation.Payload?.MediaType); diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs index 625420e2f956..82dfc5512ad9 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs @@ -235,7 +235,7 @@ public async Task ItCanExtractAllPathsAsOperationsAsync() var restApi = await this._sut.ParseAsync(this._openApiDocument); // Assert - Assert.Equal(6, restApi.Operations.Count); + Assert.Equal(7, restApi.Operations.Count); } [Fact] @@ -418,7 +418,7 @@ public async Task ItCanParsePropertiesOfObjectDataTypeAsync() public async Task ItCanFilterOutSpecifiedOperationsAsync() { // Arrange - var operationsToExclude = new[] { "Excuses", "TestDefaultValues", "OpenApiExtensions", "TestParameterDataTypes" }; + var operationsToExclude = new[] { "Excuses", "TestDefaultValues", "OpenApiExtensions", "TestParameterDataTypes", "TestParameterNamesSanitization" }; var options = new OpenApiDocumentParserOptions { diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV30Tests.cs b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV30Tests.cs index 8728771ac54a..009605a9f6b7 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV30Tests.cs +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV30Tests.cs @@ -236,7 +236,7 @@ public async Task ItCanExtractAllPathsAsOperationsAsync() var restApi = await this._sut.ParseAsync(this._openApiDocument); // Assert - Assert.Equal(7, restApi.Operations.Count); + Assert.Equal(8, restApi.Operations.Count); } [Fact] diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV31Tests.cs b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV31Tests.cs index 6455b95dd34b..b27257f46533 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV31Tests.cs +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV31Tests.cs @@ -236,7 +236,7 @@ public async Task ItCanExtractAllPathsAsOperationsAsync() var restApi = await this._sut.ParseAsync(this._openApiDocument); // Assert - Assert.Equal(7, restApi.Operations.Count); + Assert.Equal(8, restApi.Operations.Count); } [Fact] diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiKernelPluginFactoryTests.cs b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiKernelPluginFactoryTests.cs index 2242f5032610..c85002493181 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiKernelPluginFactoryTests.cs +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiKernelPluginFactoryTests.cs @@ -8,6 +8,7 @@ using System.Net.Http; using System.Net.Mime; using System.Text; +using System.Text.Json.Nodes; using System.Threading.Tasks; using Microsoft.SemanticKernel; using Microsoft.SemanticKernel.Plugins.OpenApi; @@ -353,7 +354,7 @@ public async Task ItShouldHandleEmptyOperationNameAsync() var plugin = await OpenApiKernelPluginFactory.CreateFromOpenApiAsync("fakePlugin", content, this._executionParameters); // Assert - Assert.Equal(7, plugin.Count()); + Assert.Equal(8, plugin.Count()); Assert.True(plugin.TryGetFunction("GetSecretsSecretname", out var _)); } @@ -372,7 +373,7 @@ public async Task ItShouldHandleNullOperationNameAsync() var plugin = await OpenApiKernelPluginFactory.CreateFromOpenApiAsync("fakePlugin", content, this._executionParameters); // Assert - Assert.Equal(7, plugin.Count()); + Assert.Equal(8, plugin.Count()); Assert.True(plugin.TryGetFunction("GetSecretsSecretname", out var _)); } @@ -517,6 +518,94 @@ public void ItCreatesPluginFromOpenApiSpecificationModel() Assert.Same(operations[0], function.Metadata.AdditionalProperties["operation"]); } + [Fact] + public async Task ItShouldResolveArgumentsByParameterNamesAsync() + { + // Arrange + using var messageHandlerStub = new HttpMessageHandlerStub(); + using var httpClient = new HttpClient(messageHandlerStub, false); + + this._executionParameters.EnableDynamicPayload = true; + this._executionParameters.HttpClient = httpClient; + + var arguments = new KernelArguments + { + ["string_parameter"] = "fake-secret-name", + ["boolean@parameter"] = true, + ["integer+parameter"] = 6, + ["float?parameter"] = 23.4f + }; + + var kernel = new Kernel(); + + var openApiDocument = ResourcePluginsProvider.LoadFromResource("documentV3_0.json"); + + var plugin = await OpenApiKernelPluginFactory.CreateFromOpenApiAsync("fakePlugin", openApiDocument, this._executionParameters); + + // Act + var result = await kernel.InvokeAsync(plugin["TestParameterNamesSanitization"], arguments); + + // Assert path and query parameters added to the request uri + Assert.NotNull(messageHandlerStub.RequestUri); + Assert.Equal("https://my-key-vault.vault.azure.net/test-parameter-names-sanitization/fake-secret-name?boolean@parameter=true", messageHandlerStub.RequestUri.AbsoluteUri); + + // Assert header parameters added to the request + Assert.Equal("6", messageHandlerStub.RequestHeaders!.GetValues("integer+parameter").First()); + + // Assert payload parameters added to the request + var messageContent = messageHandlerStub.RequestContent; + Assert.NotNull(messageContent); + + var deserializedPayload = await JsonNode.ParseAsync(new MemoryStream(messageContent)); + Assert.NotNull(deserializedPayload); + + Assert.Equal(23.4f, deserializedPayload["float?parameter"]!.GetValue()); + } + + [Fact] + public async Task ItShouldResolveArgumentsBySanitizedParameterNamesAsync() + { + // Arrange + using var messageHandlerStub = new HttpMessageHandlerStub(); + using var httpClient = new HttpClient(messageHandlerStub, false); + + this._executionParameters.EnableDynamicPayload = true; + this._executionParameters.HttpClient = httpClient; + + var arguments = new KernelArguments + { + ["string_parameter"] = "fake-secret-name", // Original parameter name - string-parameter + ["boolean_parameter"] = true, // Original parameter name - boolean@parameter + ["integer_parameter"] = 6, // Original parameter name - integer+parameter + ["float_parameter"] = 23.4f // Original parameter name - float?parameter + }; + + var kernel = new Kernel(); + + var openApiDocument = ResourcePluginsProvider.LoadFromResource("documentV3_0.json"); + + var plugin = await OpenApiKernelPluginFactory.CreateFromOpenApiAsync("fakePlugin", openApiDocument, this._executionParameters); + + // Act + var result = await kernel.InvokeAsync(plugin["TestParameterNamesSanitization"], arguments); + + // Assert path and query parameters added to the request uri + Assert.NotNull(messageHandlerStub.RequestUri); + Assert.Equal("https://my-key-vault.vault.azure.net/test-parameter-names-sanitization/fake-secret-name?boolean@parameter=true", messageHandlerStub.RequestUri.AbsoluteUri); + + // Assert header parameters added to the request + Assert.Equal("6", messageHandlerStub.RequestHeaders!.GetValues("integer+parameter").First()); + + // Assert payload parameters added to the request + var messageContent = messageHandlerStub.RequestContent; + Assert.NotNull(messageContent); + + var deserializedPayload = await JsonNode.ParseAsync(new MemoryStream(messageContent)); + Assert.NotNull(deserializedPayload); + + Assert.Equal(23.4f, deserializedPayload["float?parameter"]!.GetValue()); + } + /// /// Generate theory data for ItAddSecurityMetadataToOperationAsync /// diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV2_0.json b/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV2_0.json index cc71b0f46737..cb83ae5a809a 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV2_0.json +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV2_0.json @@ -434,6 +434,63 @@ }, "summary": "Get secret" } + }, + "/test-parameter-names-sanitization/{string-parameter}": { + "put": { + "summary": "Operation to test parameter names sanitization.", + "description": "Operation to test that forbidden characters in parameter names are sanitized.", + "operationId": "TestParameterNamesSanitization", + "consumes": [ + "application/json" + ], + "parameters": [ + { + "in": "path", + "name": "string-parameter", + "required": true, + "type": "string", + "default": "string-value" + }, + { + "in": "query", + "name": "boolean@parameter", + "required": true, + "type": "boolean", + "default": true + }, + { + "in": "header", + "name": "integer+parameter", + "required": true, + "type": "integer", + "format": "int32", + "default": 281 + }, + { + "in": "body", + "name": "body", + "required": true, + "schema": { + "required": [ + "float?parameter" + ], + "type": "object", + "properties": { + "float?parameter": { + "format": "float", + "default": 12.01, + "type": "number" + } + } + } + } + ], + "responses": { + "200": { + "description": "The OK response" + } + } + } } }, "produces": [], diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV3_0.json b/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV3_0.json index a2990fb86f90..abc7eef32825 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV3_0.json +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV3_0.json @@ -486,6 +486,67 @@ } } } + }, + "/test-parameter-names-sanitization/{string-parameter}": { + "put": { + "summary": "Operation to test parameter names sanitization.", + "description": "Operation to test that forbidden characters in parameter names are sanitized.", + "operationId": "TestParameterNamesSanitization", + "parameters": [ + { + "name": "string-parameter", + "in": "path", + "required": true, + "schema": { + "type": "string", + "default": "string-value" + } + }, + { + "name": "boolean@parameter", + "in": "query", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + }, + { + "name": "integer+parameter", + "in": "header", + "required": true, + "schema": { + "type": "integer", + "format": "int32", + "default": 281 + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "float?parameter": { + "type": "number", + "format": "float", + "default": 12.01 + } + }, + "required": [ "float?parameter" ] + } + } + }, + "required": true, + "x-bodyName": "body" + }, + "responses": { + "200": { + "description": "The OK response" + } + } + } } }, "components": { diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV3_1.yaml b/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV3_1.yaml index 8c250db741cb..3bf24812684a 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV3_1.yaml +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/TestPlugins/documentV3_1.yaml @@ -140,7 +140,7 @@ paths: type: string /FunPlugin/Joke: post: - summary: Gneerate a funny joke + summary: Generate a funny joke operationId: Joke requestBody: description: Joke subject @@ -244,6 +244,8 @@ paths: description: The OK response /api-with-open-api-extensions: get: + summary: Get API with open-api specification extensions + description: 'For more information on specification extensions see the specification extensions section of the open api spec: https://swagger.io/specification/v3/' operationId: OpenApiExtensions responses: '200': @@ -318,6 +320,48 @@ paths: responses: '200': description: The OK response + '/test-parameter-names-sanitization/{string-parameter}': + put: + summary: Operation to test parameter names sanitization. + description: Operation to test that forbidden characters in parameter names are sanitized. + operationId: TestParameterNamesSanitization + parameters: + - name: string-parameter + in: path + required: true + schema: + type: string + default: string-value + - name: boolean@parameter + in: query + required: true + schema: + type: boolean + default: true + - name: integer+parameter + in: header + required: true + schema: + type: integer + format: int32 + default: 281 + requestBody: + content: + application/json: + schema: + required: + - float?parameter + type: object + properties: + float?parameter: + type: number + format: float + default: 12.01 + required: true + x-bodyName: body + responses: + '200': + description: The OK response components: securitySchemes: oauth2_auth: From 79fbe61619d57415b12802a652887fb97a853dcb Mon Sep 17 00:00:00 2001 From: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com> Date: Mon, 6 Jan 2025 11:42:06 +0000 Subject: [PATCH 2/2] Update dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs Co-authored-by: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> --- .../OpenApi/OpenApiDocumentParserV20Tests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs index 58506faf963d..ae60700f5811 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs @@ -419,7 +419,7 @@ public async Task ItCanParsePropertiesOfObjectDataTypeAsync() public async Task ItCanFilterOutSpecifiedOperationsAsync() { // Arrange - var operationsToExclude = new[] { "Excuses", "TestDefaultValues", "OpenApiExtensions", "TestParameterDataTypes", "TestParameterNamesSanitization" }; + string[] operationsToExclude = ["Excuses", "TestDefaultValues", "OpenApiExtensions", "TestParameterDataTypes", "TestParameterNamesSanitization"]; var options = new OpenApiDocumentParserOptions {