Skip to content

Commit

Permalink
.Net: OpenAPI parameter resolution mechanism - part 1 (microsoft#9668)
Browse files Browse the repository at this point in the history
### Motivation and Context
Today, the SK OpenAPI plugin's functionality does not support OpenAPI
operations that include parameters with the same name. For example, a
parameter named "id" can be used in the operation path, payload, header,
and query string, each representing the ID of a different entity in
those contexts. An attempt to import an OpenAPI operation containing
multiple parameters with the same name will fail with the error message:
"The function has two or more parameters with the same name '{parameter
name}'." Therefore, the SK OpenAPI plugin needs a way to gracefully
manage operations that include parameters with identical names.

### Description
This PR adds the `ArgumentName` property to the `RestApiPayloadProperty`
and `RestApiServerVariable` classes and renames the
`RestApiParameter.AlternativeName` property to
`RestApiParameter.ArgumentName` to better represent its purpose, as it
was used as an argument name before. The property allows associating
argument names with OpenAPI parameters and is used by the
`RestApiOperationRunner` to look up values for them in the arguments the
function was invoked with.
  
The next PR will allow consumers to access the OpenAPI document model
and provide argument names for the "clashing" parameters.

Additionally, this PR adds functionality that allows "freezing" the
classes to prevent their modification after the OpenAPI document has
been parsed and imported, and its operations have become available via
the `KernelFunction.Metadata.AdditionalProperties["operation"]` item.

Contributes to: microsoft#4666
  • Loading branch information
SergeyMenshykh authored Nov 13, 2024
1 parent d604e55 commit 50bc6f3
Show file tree
Hide file tree
Showing 14 changed files with 894 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,14 @@ public static IReadOnlyList<RestApiParameter> GetParameters(
parameters.AddRange(GetPayloadParameters(operation, addPayloadParamsFromMetadata, enablePayloadNamespacing));
}

// Create a property alternative name without special symbols that are not supported by SK template language.
foreach (var parameter in parameters)
{
parameter.AlternativeName = InvalidSymbolsRegex().Replace(parameter.Name, "_");
// The functionality of replacing invalid symbols and setting the argument name
// was introduced to handle dashes allowed in OpenAPI parameter names and
// not supported by SK at that time. More context -
// https://github.com/microsoft/semantic-kernel/pull/283#discussion_r1156286780
// It's kept for backward compatibility only.
parameter.ArgumentName ??= InvalidSymbolsRegex().Replace(parameter.Name, "_");
}

return parameters;
Expand Down Expand Up @@ -181,7 +185,10 @@ private static List<RestApiParameter> GetParametersFromPayloadMetadata(IReadOnly
defaultValue: property.DefaultValue,
description: property.Description,
format: property.Format,
schema: property.Schema));
schema: property.Schema)
{
ArgumentName = property.ArgumentName
});
}

parameters.AddRange(GetParametersFromPayloadMetadata(property.Properties, enableNamespacing, parameterName));
Expand Down
101 changes: 71 additions & 30 deletions dotnet/src/Functions/Functions.OpenApi/Model/RestApiOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,10 @@ internal IDictionary<string, string> BuildHeaders(IDictionary<string, object?> a

foreach (var parameter in parameters)
{
if (!arguments.TryGetValue(parameter.Name, out object? argument) || argument is null)
var argument = this.GetArgumentForParameter(arguments, parameter);
if (argument == null)
{
// Throw an exception if the parameter is a required one but no value is provided.
if (parameter.IsRequired)
{
throw new KernelException($"No argument is provided for the '{parameter.Name}' required parameter of the operation - '{this.Id}'.");
}

// Skipping not required parameter if no argument provided for it.
// Skipping not required parameter if no argument provided for it.
continue;
}

Expand Down Expand Up @@ -187,15 +182,10 @@ internal string BuildQueryString(IDictionary<string, object?> arguments)

foreach (var parameter in parameters)
{
if (!arguments.TryGetValue(parameter.Name, out object? argument) || argument is null)
var argument = this.GetArgumentForParameter(arguments, parameter);
if (argument == null)
{
// Throw an exception if the parameter is a required one but no value is provided.
if (parameter.IsRequired)
{
throw new KernelException($"No argument or value is provided for the '{parameter.Name}' required parameter of the operation - '{this.Id}'.");
}

// Skipping not required parameter if no argument provided for it.
// Skipping not required parameter if no argument provided for it.
continue;
}

Expand All @@ -215,6 +205,24 @@ internal string BuildQueryString(IDictionary<string, object?> arguments)
return string.Join("&", segments);
}

/// <summary>
/// Makes the current instance unmodifiable.
/// </summary>
internal void Freeze()
{
this.Payload?.Freeze();

foreach (var parameter in this.Parameters)
{
parameter.Freeze();
}

foreach (var server in this.Servers)
{
server.Freeze();
}
}

#region private

/// <summary>
Expand All @@ -229,15 +237,10 @@ private string BuildPath(string pathTemplate, IDictionary<string, object?> argum

foreach (var parameter in parameters)
{
if (!arguments.TryGetValue(parameter.Name, out object? argument) || argument is null)
var argument = this.GetArgumentForParameter(arguments, parameter);
if (argument == null)
{
// Throw an exception if the parameter is a required one but no value is provided.
if (parameter.IsRequired)
{
throw new KernelException($"No argument is provided for the '{parameter.Name}' required parameter of the operation - '{this.Id}'.");
}

// Skipping not required parameter if no argument provided for it.
// Skipping not required parameter if no argument provided for it.
continue;
}

Expand All @@ -257,6 +260,31 @@ private string BuildPath(string pathTemplate, IDictionary<string, object?> argum
return pathTemplate;
}

private object? GetArgumentForParameter(IDictionary<string, object?> arguments, RestApiParameter parameter)
{
// Try to get the parameter value by the argument name.
if (!string.IsNullOrEmpty(parameter.ArgumentName) &&
arguments.TryGetValue(parameter.ArgumentName!, out object? argument) &&
argument is not null)
{
return argument;
}

// Try to get the parameter value by the parameter name.
if (arguments.TryGetValue(parameter.Name, out argument) &&
argument is not null)
{
return argument;
}

if (parameter.IsRequired)
{
throw new KernelException($"No argument '{parameter.ArgumentName ?? parameter.Name}' is provided for the '{parameter.Name}' required parameter of the operation - '{this.Id}'.");
}

return null;
}

/// <summary>
/// Returns operation server Url.
/// </summary>
Expand All @@ -275,21 +303,34 @@ private Uri GetServerUrl(Uri? serverUrlOverride, Uri? apiHostUrl, IDictionary<st
else if (this.Servers is { Count: > 0 } servers && servers[0].Url is { } url)
{
serverUrlString = url;

foreach (var variable in servers[0].Variables)
{
arguments.TryGetValue(variable.Key, out object? value);
string? strValue = value as string;
if (strValue is not null && variable.Value.IsValid(strValue))
var variableName = variable.Key;

// Try to get the variable value by the argument name.
if (!string.IsNullOrEmpty(variable.Value.ArgumentName) &&
arguments.TryGetValue(variable.Value.ArgumentName!, out object? value) &&
value is string { } argStrValue && variable.Value.IsValid(argStrValue))
{
serverUrlString = url.Replace($"{{{variableName}}}", argStrValue);
}
// Try to get the variable value by the variable name.
else if (arguments.TryGetValue(variableName, out value) &&
value is string { } strValue &&
variable.Value.IsValid(strValue))
{
serverUrlString = serverUrlString.Replace($"{{{variable.Key}}}", strValue);
serverUrlString = url.Replace($"{{{variableName}}}", strValue);
}
// Use the default value if no argument is provided.
else if (variable.Value.Default is not null)
{
serverUrlString = serverUrlString.Replace($"{{{variable.Key}}}", variable.Value.Default);
serverUrlString = url.Replace($"{{{variableName}}}", variable.Value.Default);
}
// Throw an exception if there's no value for the variable.
else
{
throw new KernelException($"No value provided for the '{variable.Key}' server variable of the operation - '{this.Id}'.");
throw new KernelException($"No argument '{variable.Value.ArgumentName ?? variableName}' provided for the '{variableName}' server variable of the operation - '{this.Id}'.");
}
}
}
Expand Down
43 changes: 41 additions & 2 deletions dotnet/src/Functions/Functions.OpenApi/Model/RestApiParameter.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Diagnostics.CodeAnalysis;

namespace Microsoft.SemanticKernel.Plugins.OpenApi;
Expand All @@ -16,9 +17,19 @@ public sealed class RestApiParameter
public string Name { get; }

/// <summary>
/// The property alternative name. It can be used as an alternative name in contexts where the original name can't be used.
/// The parameter argument name.
/// If provided, the argument name will be used to search for the parameter value in function arguments.
/// If no value is found using the argument name, the original name - <see cref="RestApiParameter.Name"/> will be used for the search instead.
/// </summary>
public string? AlternativeName { get; internal set; }
public string? ArgumentName
{
get => this._argumentName;
set
{
this.ThrowIfFrozen();
this._argumentName = value;
}
}

/// <summary>
/// The parameter type - string, integer, number, boolean, array and object.
Expand Down Expand Up @@ -111,4 +122,32 @@ internal RestApiParameter(
this.Format = format;
this.Schema = schema;
}

/// <summary>
/// Makes the current instance unmodifiable.
/// </summary>
internal void Freeze()
{
if (this._isFrozen)
{
return;
}

this._isFrozen = true;
}

/// <summary>
/// Throws an <see cref="InvalidOperationException"/> if the <see cref="RestApiParameter"/> is frozen.
/// </summary>
/// <exception cref="InvalidOperationException"></exception>
private void ThrowIfFrozen()
{
if (this._isFrozen)
{
throw new InvalidOperationException("RestApiOperationParameter is frozen and cannot be modified.");
}
}

private string? _argumentName;
private bool _isFrozen = false;
}
11 changes: 11 additions & 0 deletions dotnet/src/Functions/Functions.OpenApi/Model/RestApiPayload.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,15 @@ internal RestApiPayload(string mediaType, IReadOnlyList<RestApiPayloadProperty>
this.Description = description;
this.Schema = schema;
}

/// <summary>
/// Makes the current instance unmodifiable.
/// </summary>
internal void Freeze()
{
foreach (var property in this.Properties)
{
property.Freeze();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

Expand All @@ -16,6 +17,21 @@ public sealed class RestApiPayloadProperty
/// </summary>
public string Name { get; }

/// <summary>
/// The property argument name.
/// If provided, the argument name will be used to search for the corresponding property value in function arguments.
/// If no property value is found using the argument name, the original name - <see cref="RestApiPayloadProperty.Name"/> will be used for the search instead.
/// </summary>
public string? ArgumentName
{
get => this._argumentName;
set
{
this.ThrowIfFrozen();
this._argumentName = value;
}
}

/// <summary>
/// The property type.
/// </summary>
Expand Down Expand Up @@ -84,4 +100,32 @@ internal RestApiPayloadProperty(
this.Format = format;
this.DefaultValue = defaultValue;
}

/// <summary>
/// Makes the current instance unmodifiable.
/// </summary>
internal void Freeze()
{
if (this._isFrozen)
{
return;
}

this._isFrozen = true;
}

/// <summary>
/// Throws an <see cref="InvalidOperationException"/> if the <see cref="RestApiPayloadProperty"/> is frozen.
/// </summary>
/// <exception cref="InvalidOperationException"></exception>
private void ThrowIfFrozen()
{
if (this._isFrozen)
{
throw new InvalidOperationException("RestApiOperationPayloadProperty is frozen and cannot be modified.");
}
}

private string? _argumentName;
private bool _isFrozen = false;
}
11 changes: 11 additions & 0 deletions dotnet/src/Functions/Functions.OpenApi/Model/RestApiServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,15 @@ internal RestApiServer(string? url = null, IDictionary<string, RestApiServerVari
this.Url = string.IsNullOrEmpty(url) ? null : url;
this.Variables = variables ?? new Dictionary<string, RestApiServerVariable>();
}

/// <summary>
/// Makes the current instance unmodifiable.
/// </summary>
internal void Freeze()
{
foreach (var variable in this.Variables.Values)
{
variable.Freeze();
}
}
}
Loading

0 comments on commit 50bc6f3

Please sign in to comment.