Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.Net: fix payload parameter value resolution #9997

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 'customerid_contact@odata.bind' payload property."
property.ArgumentName ??= InvalidSymbolsRegex().Replace(parameterName, "_");

var parameter = new RestApiParameter(
name: parameterName,
type: property.Type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -419,7 +419,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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 _));
}

Expand All @@ -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 _));
}

Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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':
Expand Down Expand Up @@ -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:
Expand Down
Loading