Skip to content

Commit

Permalink
fix payload parameter value resolution
Browse files Browse the repository at this point in the history
  • Loading branch information
SergeyMenshykh committed Dec 17, 2024
1 parent e883843 commit 96b14ef
Show file tree
Hide file tree
Showing 9 changed files with 265 additions and 8 deletions.
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 @@ -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]
Expand Down Expand Up @@ -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
{
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(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 @@ -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]
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

0 comments on commit 96b14ef

Please sign in to comment.