Skip to content

Commit

Permalink
.Net: Handle empty response with no content type in RestApiOperationR…
Browse files Browse the repository at this point in the history
…unner (#6632)

### Motivation and Context

Closes #6427 

### 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 authored Jun 11, 2024
1 parent 49374d9 commit b8c277e
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ private static void ProcessPositionalArguments(KernelFunctionMetadata functionMe
// Deserialize any JSON content or return the content as a string
if (restApiOperationResponse.ContentType?.IndexOf("application/json", StringComparison.OrdinalIgnoreCase) >= 0)
{
var parsedJson = JsonValue.Parse(restApiOperationResponse.Content.ToString() ?? string.Empty);
var parsedJson = JsonValue.Parse(restApiOperationResponse.Content?.ToString() ?? string.Empty);
return KernelHelpersUtils.DeserializeJsonNode(parsedJson);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static bool IsValid(this RestApiOperationResponse response)
return true;
}

return response.ContentType switch
return response.ContentType! switch
{
var ct when ct.StartsWith("application/json", StringComparison.OrdinalIgnoreCase) => ValidateJson(response),
var ct when ct.StartsWith("application/xml", StringComparison.OrdinalIgnoreCase) => ValidateXml(response),
Expand All @@ -47,7 +47,7 @@ private static bool ValidateJson(RestApiOperationResponse response)
try
{
var jsonSchema = JsonSchema.FromText(JsonSerializer.Serialize(response.ExpectedSchema));
using var contentDoc = JsonDocument.Parse(response.Content.ToString() ?? "");
using var contentDoc = JsonDocument.Parse(response.Content?.ToString() ?? string.Empty);
var result = jsonSchema.Evaluate(contentDoc);
return result.IsValid;
}
Expand Down
20 changes: 15 additions & 5 deletions dotnet/src/Functions/Functions.OpenApi/RestApiOperationRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ private async Task<RestApiOperationResponse> SendAsync(
{
using var responseMessage = await this._httpClient.SendWithSuccessCheckAsync(requestMessage, cancellationToken).ConfigureAwait(false);

var response = await SerializeResponseContentAsync(requestMessage, payload, responseMessage.Content).ConfigureAwait(false);
var response = await SerializeResponseContentAsync(requestMessage, payload, responseMessage).ConfigureAwait(false);

response.ExpectedSchema ??= GetExpectedSchema(expectedSchemas, responseMessage.StatusCode);

Expand Down Expand Up @@ -228,11 +228,21 @@ private async Task<RestApiOperationResponse> SendAsync(
/// </summary>
/// <param name="request">The HttpRequestMessage associated with the HTTP request.</param>
/// <param name="payload">The payload sent in the HTTP request.</param>
/// <param name="content">The HttpContent object containing the response content to be serialized.</param>
/// <param name="responseMessage">The HttpResponseMessage object containing the response content to be serialized.</param>
/// <returns>The serialized content.</returns>
private static async Task<RestApiOperationResponse> SerializeResponseContentAsync(HttpRequestMessage request, object? payload, HttpContent content)
private static async Task<RestApiOperationResponse> SerializeResponseContentAsync(HttpRequestMessage request, object? payload, HttpResponseMessage responseMessage)
{
var contentType = content.Headers.ContentType;
if (responseMessage.StatusCode == HttpStatusCode.NoContent)
{
return new RestApiOperationResponse(null, null)
{
RequestMethod = request.Method.Method,
RequestUri = request.RequestUri,
RequestPayload = payload,
};
}

var contentType = responseMessage.Content.Headers.ContentType;

var mediaType = contentType?.MediaType ?? throw new KernelException("No media type available.");

Expand All @@ -256,7 +266,7 @@ private static async Task<RestApiOperationResponse> SerializeResponseContentAsyn
}

// Serialize response content and return it
var serializedContent = await serializer.Invoke(content).ConfigureAwait(false);
var serializedContent = await serializer.Invoke(responseMessage.Content).ConfigureAwait(false);

return new RestApiOperationResponse(serializedContent, contentType!.ToString())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,54 @@ public async Task ItShouldReturnRequestUriAndContentAsync()
Assert.Equal("{\"name\":\"fake-name-value\",\"attributes\":{\"enabled\":true}}", ((JsonObject)result.RequestPayload).ToJsonString());
}

[Fact]
public async Task ItShouldHandleNoContentAsync()
{
// Arrange
this._httpMessageHandlerStub!.ResponseToReturn = new HttpResponseMessage(System.Net.HttpStatusCode.NoContent);

List<RestApiOperationPayloadProperty> payloadProperties =
[
new("name", "string", true, []),
new("attributes", "object", false,
[
new("enabled", "boolean", false, []),
])
];

var payload = new RestApiOperationPayload(MediaTypeNames.Application.Json, payloadProperties);

var operation = new RestApiOperation(
"fake-id",
new Uri("https://fake-random-test-host"),
"fake-path",
HttpMethod.Post,
"fake-description",
[],
payload
);

var arguments = new KernelArguments
{
{ "name", "fake-name-value" },
{ "enabled", true }
};

var sut = new RestApiOperationRunner(this._httpClient, this._authenticationHandlerMock.Object, enableDynamicPayload: true);

// Act
var result = await sut.RunAsync(operation, arguments);

// Assert
Assert.NotNull(result.RequestMethod);
Assert.Equal(HttpMethod.Post.Method, result.RequestMethod);
Assert.NotNull(result.RequestUri);
Assert.Equal("https://fake-random-test-host/fake-path", result.RequestUri.AbsoluteUri);
Assert.NotNull(result.RequestPayload);
Assert.IsType<JsonObject>(result.RequestPayload);
Assert.Equal("{\"name\":\"fake-name-value\",\"attributes\":{\"enabled\":true}}", ((JsonObject)result.RequestPayload).ToJsonString());
}

public class SchemaTestData : IEnumerable<object[]>
{
public IEnumerator<object[]> GetEnumerator()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ public sealed class RestApiOperationResponse
/// <summary>
/// Gets the content of the response.
/// </summary>
public object Content { get; }
public object? Content { get; }

/// <summary>
/// Gets the content type of the response.
/// </summary>
public string ContentType { get; }
public string? ContentType { get; }

/// <summary>
/// The expected schema of the response as advertised in the OpenAPI operation.
Expand Down Expand Up @@ -47,7 +47,7 @@ public sealed class RestApiOperationResponse
/// <param name="content">The content of the response.</param>
/// <param name="contentType">The content type of the response.</param>
/// <param name="expectedSchema">The schema against which the response body should be validated.</param>
public RestApiOperationResponse(object content, string contentType, KernelJsonSchema? expectedSchema = null)
public RestApiOperationResponse(object? content, string? contentType, KernelJsonSchema? expectedSchema = null)
{
this.Content = content;
this.ContentType = contentType;
Expand Down

0 comments on commit b8c277e

Please sign in to comment.