Skip to content

Commit

Permalink
.Net: During OpenAPI import use payload parameter if specified (#5874)
Browse files Browse the repository at this point in the history
### Motivation and Context

Closes #5870 

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [ ] The code builds clean without any errors or warnings
- [ ] 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
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
  • Loading branch information
markwallace-microsoft committed Apr 16, 2024
1 parent 8b3203a commit 9d0f631
Show file tree
Hide file tree
Showing 10 changed files with 540 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Text.RegularExpressions;

namespace Microsoft.SemanticKernel.Plugins.OpenApi;
Expand Down Expand Up @@ -34,7 +33,7 @@ internal static class RestApiOperationExtensions
var parameters = new List<RestApiOperationParameter>(operation.Parameters);

// Add payload parameters
if (operation.Method == HttpMethod.Put || operation.Method == HttpMethod.Post)
if (operation.Payload is not null)
{
parameters.AddRange(GetPayloadParameters(operation, addPayloadParamsFromMetadata, enablePayloadNamespacing));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ private static async Task<RestApiOperationResponse> SerializeResponseContentAsyn
/// <returns>The HttpContent representing the payload.</returns>
private HttpContent? BuildOperationPayload(RestApiOperation operation, IDictionary<string, object?> arguments)
{
if (operation?.Method != HttpMethod.Put && operation?.Method != HttpMethod.Post)
if (operation.Payload is null && !arguments.ContainsKey(RestApiOperation.PayloadArgumentName))
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
<EmbeddedResource Include="OpenApi\TestPlugins\nonCompliant_documentV3_0.json" />
<EmbeddedResource Include="OpenApi\TestPlugins\documentV3_1.yaml" />
<EmbeddedResource Include="OpenApi\TestPlugins\documentV3_0.json" />
<EmbeddedResource Include="OpenApi\TestPlugins\repair-service.json" />
<EmbeddedResource Include="OpenApi\TestResponses\ObjectResponseSchema.json" />
<EmbeddedResource Include="OpenApi\TestResponses\ProductResponseSchema.json" />
<EmbeddedResource Include="OpenApi\TestResponses\ValidProductContent.json" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,39 @@ public async Task ItShouldSanitizeOperationNameAsync()
Assert.True(plugin.TryGetFunction("IssuesCreatemilestone", out var _));
}

[Fact]
public async Task ItCanIncludeOpenApiDeleteAndPatchOperationsAsync()
{
// Arrange
var openApiDocument = ResourcePluginsProvider.LoadFromResource("repair-service.json");

// Act
var plugin = await this._kernel.ImportPluginFromOpenApiAsync("repairServicePlugin", openApiDocument, this._executionParameters);

// Assert
Assert.NotNull(plugin);
var functionsMetadata = plugin.GetFunctionsMetadata();
Assert.Equal(4, functionsMetadata.Count);
AssertPayloadParameters(plugin, "updateRepair");
AssertPayloadParameters(plugin, "deleteRepair");
}

public void Dispose()
{
this._openApiDocument.Dispose();
}

#region private ================================================================================

private static void AssertPayloadParameters(KernelPlugin plugin, string functionName)
{
Assert.True(plugin.TryGetFunction(functionName, out var function));
Assert.NotNull(function.Metadata.Parameters);
Assert.Equal(2, function.Metadata.Parameters.Count);
Assert.Equal("payload", function.Metadata.Parameters[0].Name);
Assert.Equal("content_type", function.Metadata.Parameters[1].Name);
}

private KernelArguments GetFakeFunctionArguments()
{
return new KernelArguments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System;
using System.Linq;
using System.Net.Http;
using Microsoft.SemanticKernel;
using Microsoft.SemanticKernel.Plugins.OpenApi;
using Xunit;

Expand Down Expand Up @@ -225,18 +224,6 @@ public void ItShouldAddNamespaceToParametersDeclaredInPayloadMetadata(string met
Assert.Null(hasMagicWards.Description);
}

[Theory]
[InlineData("PUT")]
[InlineData("POST")]
public void ItShouldThrowExceptionIfPayloadMetadataDescribingParametersIsMissing(string method)
{
//Arrange
var operation = CreateTestOperation(method, null);

//Act
Assert.Throws<KernelException>(() => operation.GetParameters(addPayloadParamsFromMetadata: true, enablePayloadNamespacing: true));
}

[Theory]
[InlineData("PUT")]
[InlineData("POST")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,24 @@ public RestApiOperationRunnerTests()
this._httpClient = new HttpClient(this._httpMessageHandlerStub);
}

[Fact]
public async Task ItCanRunCreateAndUpdateOperationsWithJsonPayloadSuccessfullyAsync()
[Theory]
[InlineData("POST")]
[InlineData("PUT")]
[InlineData("PATCH")]
[InlineData("DELETE")]
[InlineData("GET")]
public async Task ItCanRunCreateAndUpdateOperationsWithJsonPayloadSuccessfullyAsync(string method)
{
// Arrange
this._httpMessageHandlerStub.ResponseToReturn.Content = new StringContent("fake-content", Encoding.UTF8, MediaTypeNames.Application.Json);

var httpMethod = new HttpMethod(method);

var operation = new RestApiOperation(
"fake-id",
new Uri("https://fake-random-test-host"),
"fake-path",
HttpMethod.Post,
httpMethod,
"fake-description",
[],
payload: null
Expand Down Expand Up @@ -91,7 +98,7 @@ public async Task ItCanRunCreateAndUpdateOperationsWithJsonPayloadSuccessfullyAs
Assert.NotNull(this._httpMessageHandlerStub.RequestUri);
Assert.Equal("https://fake-random-test-host/fake-path", this._httpMessageHandlerStub.RequestUri.AbsoluteUri);

Assert.Equal(HttpMethod.Post, this._httpMessageHandlerStub.Method);
Assert.Equal(httpMethod, this._httpMessageHandlerStub.Method);

Assert.NotNull(this._httpMessageHandlerStub.ContentHeaders);
Assert.Contains(this._httpMessageHandlerStub.ContentHeaders, h => h.Key == "Content-Type" && h.Value.Contains("application/json; charset=utf-8"));
Expand Down Expand Up @@ -122,17 +129,24 @@ public async Task ItCanRunCreateAndUpdateOperationsWithJsonPayloadSuccessfullyAs
this._authenticationHandlerMock.Verify(x => x(It.IsAny<HttpRequestMessage>(), It.IsAny<CancellationToken>()), Times.Once);
}

[Fact]
public async Task ItCanRunCreateAndUpdateOperationsWithPlainTextPayloadSuccessfullyAsync()
[Theory]
[InlineData("POST")]
[InlineData("PUT")]
[InlineData("PATCH")]
[InlineData("DELETE")]
[InlineData("GET")]
public async Task ItCanRunCreateAndUpdateOperationsWithPlainTextPayloadSuccessfullyAsync(string method)
{
// Arrange
this._httpMessageHandlerStub.ResponseToReturn.Content = new StringContent("fake-content", Encoding.UTF8, MediaTypeNames.Text.Plain);

var httpMethod = new HttpMethod(method);

var operation = new RestApiOperation(
"fake-id",
new Uri("https://fake-random-test-host"),
"fake-path",
HttpMethod.Post,
httpMethod,
"fake-description",
[],
payload: null
Expand All @@ -153,7 +167,7 @@ public async Task ItCanRunCreateAndUpdateOperationsWithPlainTextPayloadSuccessfu
Assert.NotNull(this._httpMessageHandlerStub.RequestUri);
Assert.Equal("https://fake-random-test-host/fake-path", this._httpMessageHandlerStub.RequestUri.AbsoluteUri);

Assert.Equal(HttpMethod.Post, this._httpMessageHandlerStub.Method);
Assert.Equal(httpMethod, this._httpMessageHandlerStub.Method);

Assert.NotNull(this._httpMessageHandlerStub.ContentHeaders);
Assert.Contains(this._httpMessageHandlerStub.ContentHeaders, h => h.Key == "Content-Type" && h.Value.Contains("text/plain; charset=utf-8"));
Expand Down Expand Up @@ -537,7 +551,7 @@ public async Task ItShouldThrowExceptionIfPayloadMetadataDoesNotHaveContentTypeA
payload: null
);

var arguments = new KernelArguments();
KernelArguments arguments = new() { { RestApiOperation.PayloadArgumentName, "fake-content" } };

var sut = new RestApiOperationRunner(
this._httpClient,
Expand All @@ -564,7 +578,7 @@ public async Task ItShouldThrowExceptionIfContentTypeArgumentIsNotProvidedAsync(
payload: null
);

var arguments = new KernelArguments();
KernelArguments arguments = new() { { RestApiOperation.PayloadArgumentName, "fake-content" } };

var sut = new RestApiOperationRunner(
this._httpClient,
Expand Down
Loading

0 comments on commit 9d0f631

Please sign in to comment.