Skip to content

fix: multiple performance fixes for type serialization feat: adds to identifier mapping to non nullable enum #1915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 13, 2024
Merged

fix: multiple performance fixes for type serialization feat: adds to identifier mapping to non nullable enum #1915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,41 @@
/// </summary>
public static class OpenApiTypeMapper
{
#nullable enable
/// <summary>
/// Maps a JsonSchema data type to an identifier.
/// </summary>
/// <param name="schemaType"></param>
/// <returns></returns>
public static string ToIdentifier(this JsonSchemaType? schemaType)
public static string? ToIdentifier(this JsonSchemaType? schemaType)
{
if (schemaType is null)
{
return null;
}
return schemaType.Value.ToIdentifier();
}

/// <summary>
/// Maps a JsonSchema data type to an identifier.
/// </summary>
/// <param name="schemaType"></param>
/// <returns></returns>
public static string? ToIdentifier(this JsonSchemaType schemaType)
{
return schemaType switch
{
JsonSchemaType.Null => "null",
JsonSchemaType.Boolean => "boolean",

Check warning on line 41 in src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

View workflow job for this annotation

GitHub Actions / Build 

Define a constant instead of using this literal 'boolean' 4 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
JsonSchemaType.Integer => "integer",

Check warning on line 42 in src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

View workflow job for this annotation

GitHub Actions / Build 

Define a constant instead of using this literal 'integer' 8 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
JsonSchemaType.Number => "number",

Check warning on line 43 in src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

View workflow job for this annotation

GitHub Actions / Build 

Define a constant instead of using this literal 'number' 14 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
JsonSchemaType.String => "string",

Check warning on line 44 in src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

View workflow job for this annotation

GitHub Actions / Build 

Define a constant instead of using this literal 'string' 13 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
JsonSchemaType.Array => "array",
JsonSchemaType.Object => "object",
_ => null,
};
}
#nullable restore

/// <summary>
/// Converts a schema type's identifier into the enum equivalent
Expand All @@ -45,7 +61,7 @@
"null" => JsonSchemaType.Null,
"boolean" => JsonSchemaType.Boolean,
"integer" or "int" => JsonSchemaType.Integer,
"number" or "double" or "float" or "decimal"=> JsonSchemaType.Number,

Check warning on line 64 in src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

View workflow job for this annotation

GitHub Actions / Build 

Define a constant instead of using this literal 'double' 7 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
"string" => JsonSchemaType.String,
"array" => JsonSchemaType.Array,
"object" => JsonSchemaType.Object,
Expand Down
96 changes: 42 additions & 54 deletions src/Microsoft.OpenApi/Models/OpenApiSchema.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,10 +476,7 @@ public void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version,
writer.WriteOptionalCollection(OpenApiConstants.Enum, Enum, (nodeWriter, s) => nodeWriter.WriteAny(s));

// type
if (Type is not null)
{
SerializeTypeProperty(Type, writer, version);
}
SerializeTypeProperty(Type, writer, version);

// allOf
writer.WriteOptionalCollection(OpenApiConstants.AllOf, AllOf, callback);
Expand Down Expand Up @@ -660,10 +657,7 @@ internal void SerializeAsV2(
writer.WriteStartObject();

// type
if (Type is not null)
{
SerializeTypeProperty(Type, writer, OpenApiSpecVersion.OpenApi2_0);
}
SerializeTypeProperty(Type, writer, OpenApiSpecVersion.OpenApi2_0);

// description
writer.WriteProperty(OpenApiConstants.Description, Description);
Expand Down Expand Up @@ -794,8 +788,11 @@ internal void SerializeAsV2(

private void SerializeTypeProperty(JsonSchemaType? type, IOpenApiWriter writer, OpenApiSpecVersion version)
{
var flagsCount = CountEnumSetFlags(type);
if (flagsCount is 1)
if (type is null)
{
return;
}
if (!HasMultipleTypes(type.Value))
{
// check whether nullable is true for upcasting purposes
if (version is OpenApiSpecVersion.OpenApi3_1 && (Nullable || Extensions.ContainsKey(OpenApiConstants.NullableExtension)))
Expand All @@ -804,61 +801,50 @@ private void SerializeTypeProperty(JsonSchemaType? type, IOpenApiWriter writer,
}
else
{
writer.WriteProperty(OpenApiConstants.Type, type.ToIdentifier());
writer.WriteProperty(OpenApiConstants.Type, type.Value.ToIdentifier());
}
}
else if(flagsCount > 1)
else
{
// type
if (version is OpenApiSpecVersion.OpenApi2_0 || version is OpenApiSpecVersion.OpenApi3_0)
{
DowncastTypeArrayToV2OrV3(type, writer, version, flagsCount);
DowncastTypeArrayToV2OrV3(type.Value, writer, version);
}
else
{
if (type is not null)
var list = new List<JsonSchemaType>();
foreach (JsonSchemaType flag in jsonSchemaTypeValues)
{
var list = new List<JsonSchemaType?>();
foreach (JsonSchemaType flag in System.Enum.GetValues(typeof(JsonSchemaType)))
if (type.Value.HasFlag(flag))
{
if (type.Value.HasFlag(flag))
{
list.Add(flag);
}
list.Add(flag);
}
}

writer.WriteOptionalCollection(OpenApiConstants.Type, list, (w, s) => w.WriteValue(s.ToIdentifier()));
}
writer.WriteOptionalCollection(OpenApiConstants.Type, list, (w, s) => w.WriteValue(s.ToIdentifier()));
}
}
}

private static int CountEnumSetFlags(JsonSchemaType? schemaType)
private static bool IsPowerOfTwo(int x)
{
int count = 0;

if (schemaType != null)
{
// Check each flag in the enum
foreach (JsonSchemaType value in System.Enum.GetValues(typeof(JsonSchemaType)))
{
// Check if the flag is set
if (schemaType.Value.HasFlag(value))
{
count++;
}
}
}
return x != 0 && (x & (x - 1)) == 0;
}

return count;
private static bool HasMultipleTypes(JsonSchemaType schemaType)
{
var schemaTypeNumeric = (int)schemaType;
return !IsPowerOfTwo(schemaTypeNumeric) && // Boolean, Integer, Number, String, Array, Object
schemaTypeNumeric != (int)JsonSchemaType.Null;
}

private void UpCastSchemaTypeToV31(JsonSchemaType? type, IOpenApiWriter writer)
{
// create a new array and insert the type and "null" as values
Type = type | JsonSchemaType.Null;
var list = new List<string>();
foreach (JsonSchemaType? flag in System.Enum.GetValues(typeof(JsonSchemaType)))
foreach (JsonSchemaType? flag in jsonSchemaTypeValues)
{
// Check if the flag is set in 'type' using a bitwise AND operation
if (Type.Value.HasFlag(flag))
Expand All @@ -870,7 +856,9 @@ private void UpCastSchemaTypeToV31(JsonSchemaType? type, IOpenApiWriter writer)
writer.WriteOptionalCollection(OpenApiConstants.Type, list, (w, s) => w.WriteValue(s));
}

private void DowncastTypeArrayToV2OrV3(JsonSchemaType? schemaType, IOpenApiWriter writer, OpenApiSpecVersion version, int flagsCount)
private static readonly Array jsonSchemaTypeValues = System.Enum.GetValues(typeof(JsonSchemaType));

private void DowncastTypeArrayToV2OrV3(JsonSchemaType schemaType, IOpenApiWriter writer, OpenApiSpecVersion version)
{
/* If the array has one non-null value, emit Type as string
* If the array has one null value, emit x-nullable as true
Expand All @@ -882,23 +870,12 @@ private void DowncastTypeArrayToV2OrV3(JsonSchemaType? schemaType, IOpenApiWrite
? OpenApiConstants.NullableExtension
: OpenApiConstants.Nullable;

if (flagsCount is 1)
if (!HasMultipleTypes(schemaType ^ JsonSchemaType.Null) && (schemaType & JsonSchemaType.Null) == JsonSchemaType.Null) // checks for two values and one is null
{
if (schemaType is JsonSchemaType.Null)
{
writer.WriteProperty(nullableProp, true);
}
else
{
writer.WriteProperty(OpenApiConstants.Type, schemaType.ToIdentifier());
}
}
else if (flagsCount is 2 && (schemaType & JsonSchemaType.Null) == JsonSchemaType.Null) // checks for two values and one is null
{
foreach (JsonSchemaType? flag in System.Enum.GetValues(typeof(JsonSchemaType)))
foreach (JsonSchemaType? flag in jsonSchemaTypeValues)
{
// Skip if the flag is not set or if it's the Null flag
if (schemaType.Value.HasFlag(flag) && flag != JsonSchemaType.Null)
if (schemaType.HasFlag(flag) && flag != JsonSchemaType.Null)
{
// Write the non-null flag value to the writer
writer.WriteProperty(OpenApiConstants.Type, flag.ToIdentifier());
Expand All @@ -909,6 +886,17 @@ private void DowncastTypeArrayToV2OrV3(JsonSchemaType? schemaType, IOpenApiWrite
writer.WriteProperty(nullableProp, true);
}
}
else if (!HasMultipleTypes(schemaType))
{
if (schemaType is JsonSchemaType.Null)
{
writer.WriteProperty(nullableProp, true);
}
else
{
writer.WriteProperty(OpenApiConstants.Type, schemaType.ToIdentifier());
}
}
}
}
}
3 changes: 2 additions & 1 deletion test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ namespace Microsoft.OpenApi.Extensions
{
public static System.Type MapOpenApiPrimitiveTypeToSimpleType(this Microsoft.OpenApi.Models.OpenApiSchema schema) { }
public static Microsoft.OpenApi.Models.OpenApiSchema MapTypeToOpenApiPrimitiveType(this System.Type type) { }
public static string ToIdentifier(this Microsoft.OpenApi.Models.JsonSchemaType? schemaType) { }
public static string? ToIdentifier(this Microsoft.OpenApi.Models.JsonSchemaType schemaType) { }
public static string? ToIdentifier(this Microsoft.OpenApi.Models.JsonSchemaType? schemaType) { }
public static Microsoft.OpenApi.Models.JsonSchemaType ToJsonSchemaType(this string identifier) { }
}
public static class StringExtensions
Expand Down
Loading