Skip to content

Commit

Permalink
.Net: Prepare OpenAPI model collection properties to be modifiable. (#…
Browse files Browse the repository at this point in the history
…9735)

### Motivation, Context and Description
Currently, OpenAPI model classes use a read-only version of list and
dictionary types as types for collection properties. This is sufficient
for now, but it could become a breaking change later when they need to
be modifiable. In that case, they will have to be changed to
non-read-only types - specifically, IList<T> and IDictionary<TK, TV>.
This PR changes all the collection properties to be modifiable by
altering their types to IList<T> and IDictionary<TK, TV>.
   
~~The RestApiSecurityRequirement model class is not completely
freezeable at the moment and needs to be refactored to implement the
IDictionary interface instead of inheriting from Dictionary. This change
will allow for intercepting mutation calls and throwing an exception if
the class is frozen.~~

Contributes to: #6884
  • Loading branch information
SergeyMenshykh authored Nov 19, 2024
1 parent e6413c3 commit 8da553d
Show file tree
Hide file tree
Showing 19 changed files with 753 additions and 107 deletions.
6 changes: 6 additions & 0 deletions dotnet/SK-dotnet.sln
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,11 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Type", "Type", "{E85EA4D0-B
src\InternalUtilities\src\Type\TypeExtensions.cs = src\InternalUtilities\src\Type\TypeExtensions.cs
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Model", "Model", "{1DBA09D6-0506-4AFA-9DF0-0A0E8D6586B1}"
ProjectSection(SolutionItems) = preProject
src\InternalUtilities\src\Model\Freezable.cs = src\InternalUtilities\src\Model\Freezable.cs
EndProjectSection
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Plugins.Core", "src\Plugins\Plugins.Core\Plugins.Core.csproj", "{0D0C4DAD-E6BC-4504-AE3A-EEA4E35920C1}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Connectors.Memory.Kusto", "src\Connectors\Connectors.Memory.Kusto\Connectors.Memory.Kusto.csproj", "{E07608CC-D710-4655-BB9E-D22CF3CDD193}"
Expand Down Expand Up @@ -1203,6 +1208,7 @@ Global
{1C19D805-3573-4477-BF07-40180FCDE1BD} = {958AD708-F048-4FAF-94ED-D2F2B92748B9}
{3CDE10B2-AE8F-4FC4-8D55-92D4AD32E144} = {958AD708-F048-4FAF-94ED-D2F2B92748B9}
{E85EA4D0-BB7E-4DFD-882F-A76EB8C0B8FF} = {958AD708-F048-4FAF-94ED-D2F2B92748B9}
{1DBA09D6-0506-4AFA-9DF0-0A0E8D6586B1} = {958AD708-F048-4FAF-94ED-D2F2B92748B9}
{0D0C4DAD-E6BC-4504-AE3A-EEA4E35920C1} = {D6D598DF-C17C-46F4-B2B9-CDE82E2DE132}
{E07608CC-D710-4655-BB9E-D22CF3CDD193} = {24503383-A8C4-4255-9998-28D70FE8E99A}
{D6D598DF-C17C-46F4-B2B9-CDE82E2DE132} = {831DDCA2-7D2C-4C31-80DB-6BDB3E1F7AE0}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public static IReadOnlyList<RestApiParameter> GetParameters(
/// <param name="responses">Possible REST API responses.</param>
/// <param name="preferredResponses">The preferred response codes to use when selecting the default response.</param>
/// <returns>The default response, if any.</returns>
private static RestApiExpectedResponse? GetDefaultResponse(IReadOnlyDictionary<string, RestApiExpectedResponse> responses, string[] preferredResponses)
private static RestApiExpectedResponse? GetDefaultResponse(IDictionary<string, RestApiExpectedResponse> responses, string[] preferredResponses)
{
foreach (var code in preferredResponses)
{
Expand Down Expand Up @@ -165,7 +165,7 @@ private static RestApiParameter CreatePayloadArtificialParameter(RestApiOperatio
/// </param>
/// <param name="rootPropertyName">The root property name.</param>
/// <returns>The list of payload parameters.</returns>
private static List<RestApiParameter> GetParametersFromPayloadMetadata(IReadOnlyList<RestApiPayloadProperty> properties, bool enableNamespacing = false, string? rootPropertyName = null)
private static List<RestApiParameter> GetParametersFromPayloadMetadata(IList<RestApiPayloadProperty> properties, bool enableNamespacing = false, string? rootPropertyName = null)
{
var parameters = new List<RestApiParameter>();

Expand Down
14 changes: 13 additions & 1 deletion dotnet/src/Functions/Functions.OpenApi/Model/RestApiOAuthFlow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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

namespace Microsoft.SemanticKernel.Plugins.OpenApi;
Expand Down Expand Up @@ -32,7 +33,11 @@ public sealed class RestApiOAuthFlow
/// <summary>
/// REQUIRED. A map between the scope name and a short description for it.
/// </summary>
public IReadOnlyDictionary<string, string> Scopes { get; init; }
public IDictionary<string, string> Scopes
{
get => this._scopes;
init => this._scopes = value;
}

/// <summary>
/// Creates an instance of a <see cref="RestApiOAuthFlow"/> class.
Expand All @@ -42,4 +47,11 @@ internal RestApiOAuthFlow()
#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable.
{
}

internal void Freeze()
{
this._scopes = new ReadOnlyDictionary<string, string>(this._scopes);
}

private IDictionary<string, string> _scopes;
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,12 @@ public sealed class RestApiOAuthFlows
internal RestApiOAuthFlows()
{
}

internal void Freeze()
{
this.Implicit?.Freeze();
this.Password?.Freeze();
this.ClientCredentials?.Freeze();
this.AuthorizationCode?.Freeze();
}
}
44 changes: 30 additions & 14 deletions dotnet/src/Functions/Functions.OpenApi/Model/RestApiOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Net.Http;
Expand Down Expand Up @@ -54,22 +55,22 @@ public sealed class RestApiOperation
/// <summary>
/// The server.
/// </summary>
public IReadOnlyList<RestApiServer> Servers { get; }
public IList<RestApiServer> Servers { get; private set; }

/// <summary>
/// The security requirements.
/// </summary>
public IReadOnlyList<RestApiSecurityRequirement> SecurityRequirements { get; }
public IList<RestApiSecurityRequirement> SecurityRequirements { get; private set; }

/// <summary>
/// The operation parameters.
/// </summary>
public IReadOnlyList<RestApiParameter> Parameters { get; }
public IList<RestApiParameter> Parameters { get; private set; }

/// <summary>
/// The list of possible operation responses.
/// </summary>
public IReadOnlyDictionary<string, RestApiExpectedResponse> Responses { get; }
public IDictionary<string, RestApiExpectedResponse> Responses { get; private set; }

/// <summary>
/// The operation payload.
Expand All @@ -79,8 +80,11 @@ public sealed class RestApiOperation
/// <summary>
/// Additional unstructured metadata about the operation.
/// </summary>
public IReadOnlyDictionary<string, object?> Extensions { get; init; } = s_emptyDictionary;

public IDictionary<string, object?> Extensions
{
get => this._extensions;
init => this._extensions = value;
}
/// <summary>
/// Creates an instance of a <see cref="RestApiOperation"/> class.
/// </summary>
Expand All @@ -95,13 +99,13 @@ public sealed class RestApiOperation
/// <param name="payload">The operation payload.</param>
internal RestApiOperation(
string? id,
IReadOnlyList<RestApiServer> servers,
IList<RestApiServer> servers,
string path,
HttpMethod method,
string? description,
IReadOnlyList<RestApiParameter> parameters,
IReadOnlyDictionary<string, RestApiExpectedResponse> responses,
IReadOnlyList<RestApiSecurityRequirement> securityRequirements,
IList<RestApiParameter> parameters,
IDictionary<string, RestApiExpectedResponse> responses,
IList<RestApiSecurityRequirement> securityRequirements,
RestApiPayload? payload = null)
{
this.Id = id;
Expand All @@ -110,11 +114,9 @@ internal RestApiOperation(
this.Method = method;
this.Description = description;
this.Parameters = parameters;
this.Responses = responses;
this.SecurityRequirements = securityRequirements;
this.Payload = payload;
this.Responses = responses ?? new Dictionary<string, RestApiExpectedResponse>();
this.SecurityRequirements = securityRequirements;
this.Payload = payload;
}

/// <summary>
Expand Down Expand Up @@ -212,15 +214,27 @@ internal void Freeze()
{
this.Payload?.Freeze();

this.Parameters = new ReadOnlyCollection<RestApiParameter>(this.Parameters);
foreach (var parameter in this.Parameters)
{
parameter.Freeze();
}

this.Servers = new ReadOnlyCollection<RestApiServer>(this.Servers);
foreach (var server in this.Servers)
{
server.Freeze();
}

this.SecurityRequirements = new ReadOnlyCollection<RestApiSecurityRequirement>(this.SecurityRequirements);
foreach (var securityRequirement in this.SecurityRequirements)
{
securityRequirement.Freeze();
}

this.Responses = new ReadOnlyDictionary<string, RestApiExpectedResponse>(this.Responses);

this._extensions = new ReadOnlyDictionary<string, object?>(this._extensions);
}

#region private
Expand Down Expand Up @@ -358,5 +372,7 @@ value is string { } strValue &&
{ RestApiParameterStyle.PipeDelimited, PipeDelimitedStyleParameterSerializer.Serialize }
};

# endregion
private IDictionary<string, object?> _extensions = s_emptyDictionary;

#endregion
}
28 changes: 3 additions & 25 deletions dotnet/src/Functions/Functions.OpenApi/Model/RestApiParameter.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Diagnostics.CodeAnalysis;

namespace Microsoft.SemanticKernel.Plugins.OpenApi;
Expand All @@ -26,7 +25,7 @@ public string? ArgumentName
get => this._argumentName;
set
{
this.ThrowIfFrozen();
this._freezable.ThrowIfFrozen();
this._argumentName = value;
}
}
Expand Down Expand Up @@ -122,32 +121,11 @@ 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.");
}
this._freezable.Freeze();
}

private readonly Freezable _freezable = new();
private string? _argumentName;
private bool _isFrozen = false;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft. All rights reserved.

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

namespace Microsoft.SemanticKernel.Plugins.OpenApi;
Expand All @@ -24,7 +25,7 @@ public sealed class RestApiPayload
/// <summary>
/// The payload properties.
/// </summary>
public IReadOnlyList<RestApiPayloadProperty> Properties { get; }
public IList<RestApiPayloadProperty> Properties { get; private set; }

/// <summary>
/// The schema of the parameter.
Expand All @@ -38,7 +39,7 @@ public sealed class RestApiPayload
/// <param name="properties">The properties.</param>
/// <param name="description">The description.</param>
/// <param name="schema">The JSON Schema.</param>
internal RestApiPayload(string mediaType, IReadOnlyList<RestApiPayloadProperty> properties, string? description = null, KernelJsonSchema? schema = null)
internal RestApiPayload(string mediaType, IList<RestApiPayloadProperty> properties, string? description = null, KernelJsonSchema? schema = null)
{
this.MediaType = mediaType;
this.Properties = properties;
Expand All @@ -51,6 +52,7 @@ internal RestApiPayload(string mediaType, IReadOnlyList<RestApiPayloadProperty>
/// </summary>
internal void Freeze()
{
this.Properties = new ReadOnlyCollection<RestApiPayloadProperty>(this.Properties);
foreach (var property in this.Properties)
{
property.Freeze();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft. All rights reserved.

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

namespace Microsoft.SemanticKernel.Plugins.OpenApi;
Expand All @@ -27,7 +27,7 @@ public string? ArgumentName
get => this._argumentName;
set
{
this.ThrowIfFrozen();
this._freezable.ThrowIfFrozen();
this._argumentName = value;
}
}
Expand Down Expand Up @@ -56,8 +56,7 @@ public string? ArgumentName
/// <summary>
/// The properties.
/// </summary>
public IReadOnlyList<RestApiPayloadProperty> Properties { get; }

public IList<RestApiPayloadProperty> Properties { get; private set; }
/// <summary>
/// The schema of the parameter.
/// </summary>
Expand Down Expand Up @@ -85,7 +84,7 @@ internal RestApiPayloadProperty(
string name,
string type,
bool isRequired,
IReadOnlyList<RestApiPayloadProperty> properties,
IList<RestApiPayloadProperty> properties,
string? description = null,
string? format = null,
KernelJsonSchema? schema = null,
Expand All @@ -106,26 +105,14 @@ internal RestApiPayloadProperty(
/// </summary>
internal void Freeze()
{
if (this._isFrozen)
this.Properties = new ReadOnlyCollection<RestApiPayloadProperty>(this.Properties);
foreach (var property in this.Properties)
{
return;
property.Freeze();
}

this._isFrozen = true;
this._freezable.Freeze();
}

/// <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 readonly Freezable _freezable = new();
private string? _argumentName;
private bool _isFrozen = false;
}
Loading

0 comments on commit 8da553d

Please sign in to comment.