.Net: fix payload parameter value resolution (#9997)

### Motivation and Context The payload property value lookup mechanism was broken in this PR - #9668, preventing the resolution of payload property values by sanitized payload property names, leading to exceptions such as - "No argument is found for the 'customerid_[[email protected]](mailto:[email protected])' payload property." ### Description This PR fixes the issue and adds the necessary tests to ensure that the functionality won't break in the future. --------- Co-authored-by: Roger Barreto <[email protected]>
microsoft · Jan 6, 2025 · d91c734 · d91c734
1 parent d201b5e
commit d91c734
Show file tree

Hide file tree

Showing 9 changed files with 265 additions and 8 deletions.
diff --git 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<RestApiParameter> 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 '[email protected]' 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
@@ -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
@@ -236,7 +236,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]
@@ -419,7 +419,7 @@ public async Task ItCanParsePropertiesOfObjectDataTypeAsync()
     public async Task ItCanFilterOutSpecifiedOperationsAsync()
     {
         // Arrange
-        var operationsToExclude = new[] { "Excuses", "TestDefaultValues", "OpenApiExtensions", "TestParameterDataTypes" };
+        string[] operationsToExclude = ["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
@@ -237,7 +237,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
@@ -237,7 +237,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
@@ -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<float>());
+    }
+
+    [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<float>());
+    }
+
     /// <summary>
     /// Generate theory data for ItAddSecurityMetadataToOperationAsync
     /// </summary>

diff --git 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
@@ -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
@@ -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: