Skip to content

Commit

Permalink
fix: refactor ToIdentifier() to normalize flaggable enums (#2156)
Browse files Browse the repository at this point in the history 
* fix: refactor ToIdentifier() to normalize flaggable enums

* fix: add support for casting type array to a flaggable enum

* chore: add tests and update public API

* Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

Co-authored-by: Vincent Biret <[email protected]>

* chore: address PR feedback

* chore: make method internal; update public API

* Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

Co-authored-by: Darrel <[email protected]>

* Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

Co-authored-by: Darrel <[email protected]>

* fix: address more PR feedback, add and fix tests

* Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

Co-authored-by: Vincent Biret <[email protected]>

* Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

Co-authored-by: Vincent Biret <[email protected]>

* Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

Co-authored-by: Vincent Biret <[email protected]>

* chore: clean up

---------

Co-authored-by: Vincent Biret <[email protected]>
Co-authored-by: Darrel <[email protected]>
  • Loading branch information
@MaggieKimani1 @baywet @darrelmiller
3 people authored Feb 20, 2025
1 parent 75d7a66 commit b80e934
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 62 deletions.
135 changes: 91 additions & 44 deletions src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.OpenApi.Exceptions;
using Microsoft.OpenApi.Models;

Expand All @@ -19,34 +20,61 @@ public static class OpenApiTypeMapper
/// </summary>
/// <param name="schemaType"></param>
/// <returns></returns>
public static string? ToIdentifier(this JsonSchemaType? schemaType)
public static string[]? ToIdentifiers(this JsonSchemaType? schemaType)
{
if (schemaType is null)
{
return null;
}
return schemaType.Value.ToIdentifier();
return schemaType.Value.ToIdentifiers();
}

/// <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[] ToIdentifiers(this JsonSchemaType schemaType)
{
return schemaType switch
{
JsonSchemaType.Null => "null",
JsonSchemaType.Boolean => "boolean",
JsonSchemaType.Integer => "integer",
JsonSchemaType.Number => "number",
JsonSchemaType.String => "string",
JsonSchemaType.Array => "array",
JsonSchemaType.Object => "object",
_ => null,
};
return schemaType.ToIdentifiersInternal().ToArray();
}

private static readonly Dictionary<JsonSchemaType, string> allSchemaTypes = new()
{
{ JsonSchemaType.Boolean, "boolean" },
{ JsonSchemaType.Integer, "integer" },
{ JsonSchemaType.Number, "number" },
{ JsonSchemaType.String, "string" },
{ JsonSchemaType.Object, "object" },
{ JsonSchemaType.Array, "array" },
{ JsonSchemaType.Null, "null" }
};

private static IEnumerable<string> ToIdentifiersInternal(this JsonSchemaType schemaType)
{
return allSchemaTypes.Where(kvp => schemaType.HasFlag(kvp.Key)).Select(static kvp => kvp.Value);
}

/// <summary>
/// Returns the first identifier from a string array.
/// </summary>
/// <param name="schemaType"></param>
/// <returns></returns>
internal static string ToFirstIdentifier(this JsonSchemaType schemaType)
{
return schemaType.ToIdentifiersInternal().First();
}

/// <summary>
/// Returns a single identifier from an array with only one item.
/// </summary>
/// <param name="schemaType"></param>
/// <returns></returns>
internal static string ToSingleIdentifier(this JsonSchemaType schemaType)
{
return schemaType.ToIdentifiersInternal().Single();
}

#nullable restore

/// <summary>
Expand All @@ -70,6 +98,26 @@ public static JsonSchemaType ToJsonSchemaType(this string identifier)
};
}

/// <summary>
/// Converts a schema type's identifier into the enum equivalent
/// </summary>
/// <param name="identifier"></param>
/// <returns></returns>
public static JsonSchemaType? ToJsonSchemaType(this string[] identifier)
{
if (identifier == null)
{
return null;
}

JsonSchemaType type = 0;
foreach (var id in identifier)
{
type |= id.ToJsonSchemaType();
}
return type;
}

private static readonly Dictionary<Type, Func<OpenApiSchema>> _simpleTypeToOpenApiSchema = new()
{
[typeof(bool)] = () => new() { Type = JsonSchemaType.Boolean },
Expand Down Expand Up @@ -141,7 +189,7 @@ public static OpenApiSchema MapTypeToOpenApiPrimitiveType(this Type type)
}

/// <summary>
/// Maps an JsonSchema data type and format to a simple type.
/// Maps a JsonSchema data type and format to a simple type.
/// </summary>
/// <param name="schema">The OpenApi data type</param>
/// <returns>The simple type</returns>
Expand All @@ -153,37 +201,36 @@ public static Type MapOpenApiPrimitiveTypeToSimpleType(this OpenApiSchema schema
throw new ArgumentNullException(nameof(schema));
}

var type = ((schema.Type & ~JsonSchemaType.Null).ToIdentifier(), schema.Format?.ToLowerInvariant(), schema.Type & JsonSchemaType.Null) switch
var type = (schema.Type, schema.Format?.ToLowerInvariant()) switch
{
("integer" or "number", "int32", JsonSchemaType.Null) => typeof(int?),
("integer" or "number", "int64", JsonSchemaType.Null) => typeof(long?),
("integer", null, JsonSchemaType.Null) => typeof(long?),
("number", "float", JsonSchemaType.Null) => typeof(float?),
("number", "double", JsonSchemaType.Null) => typeof(double?),
("number", null, JsonSchemaType.Null) => typeof(double?),
("number", "decimal", JsonSchemaType.Null) => typeof(decimal?),
("string", "byte", JsonSchemaType.Null) => typeof(byte?),
("string", "date-time", JsonSchemaType.Null) => typeof(DateTimeOffset?),
("string", "uuid", JsonSchemaType.Null) => typeof(Guid?),
("string", "char", JsonSchemaType.Null) => typeof(char?),
("boolean", null, JsonSchemaType.Null) => typeof(bool?),
("boolean", null, _) => typeof(bool),
(JsonSchemaType.Integer | JsonSchemaType.Null or JsonSchemaType.Number | JsonSchemaType.Null, "int32") => typeof(int?),
(JsonSchemaType.Integer | JsonSchemaType.Null or JsonSchemaType.Number | JsonSchemaType.Null, "int64") => typeof(long?),
(JsonSchemaType.Integer | JsonSchemaType.Null, null) => typeof(long?),
(JsonSchemaType.Number | JsonSchemaType.Null, "float") => typeof(float?),
(JsonSchemaType.Number | JsonSchemaType.Null, "double") => typeof(double?),
(JsonSchemaType.Number | JsonSchemaType.Null, null) => typeof(double?),
(JsonSchemaType.Number | JsonSchemaType.Null, "decimal") => typeof(decimal?),
(JsonSchemaType.String | JsonSchemaType.Null, "byte") => typeof(byte?),
(JsonSchemaType.String | JsonSchemaType.Null, "date-time") => typeof(DateTimeOffset?),
(JsonSchemaType.String | JsonSchemaType.Null, "uuid") => typeof(Guid?),
(JsonSchemaType.String | JsonSchemaType.Null, "char") => typeof(char?),
(JsonSchemaType.Boolean | JsonSchemaType.Null, null) => typeof(bool?),
(JsonSchemaType.Boolean, null) => typeof(bool),
// integer is technically not valid with format, but we must provide some compatibility
("integer" or "number", "int32", _) => typeof(int),
("integer" or "number", "int64", _) => typeof(long),
("integer", null, _) => typeof(long),
("number", "float", _) => typeof(float),
("number", "double", _) => typeof(double),
("number", "decimal", _) => typeof(decimal),
("number", null, _) => typeof(double),
("string", "byte", _) => typeof(byte),
("string", "date-time", _) => typeof(DateTimeOffset),
("string", "uuid", _) => typeof(Guid),
("string", "duration", _) => typeof(TimeSpan),
("string", "char", _) => typeof(char),
("string", null, _) => typeof(string),
("object", null, _) => typeof(object),
("string", "uri", _) => typeof(Uri),
(JsonSchemaType.Integer or JsonSchemaType.Number, "int32") => typeof(int),
(JsonSchemaType.Integer or JsonSchemaType.Number, "int64") => typeof(long),
(JsonSchemaType.Integer, null) => typeof(long),
(JsonSchemaType.Number, "float") => typeof(float),
(JsonSchemaType.Number, "double") => typeof(double),
(JsonSchemaType.Number, "decimal") => typeof(decimal),
(JsonSchemaType.Number, null) => typeof(double),
(JsonSchemaType.String, "byte") => typeof(byte),
(JsonSchemaType.String, "date-time") => typeof(DateTimeOffset),
(JsonSchemaType.String, "uuid") => typeof(Guid),
(JsonSchemaType.String, "char") => typeof(char),
(JsonSchemaType.String, null) => typeof(string),
(JsonSchemaType.Object, null) => typeof(object),
(JsonSchemaType.String, "uri") => typeof(Uri),
_ => typeof(string),
};

Expand Down
26 changes: 16 additions & 10 deletions src/Microsoft.OpenApi/Models/OpenApiSchema.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ internal void WriteJsonSchemaKeywords(IOpenApiWriter writer)
internal void WriteAsItemsProperties(IOpenApiWriter writer)
{
// type
writer.WriteProperty(OpenApiConstants.Type, (Type & ~JsonSchemaType.Null).ToIdentifier());
writer.WriteProperty(OpenApiConstants.Type, (Type & ~JsonSchemaType.Null)?.ToFirstIdentifier());

// format
WriteFormatProperty(writer);
Expand Down Expand Up @@ -634,10 +634,10 @@ private void SerializeAsV2(
private void SerializeTypeProperty(JsonSchemaType? type, IOpenApiWriter writer, OpenApiSpecVersion version)
{
// check whether nullable is true for upcasting purposes
var isNullable = (Type.HasValue && Type.Value.HasFlag(JsonSchemaType.Null)) ||
var isNullable = (Type.HasValue && Type.Value.HasFlag(JsonSchemaType.Null)) ||
Extensions is not null &&
Extensions.TryGetValue(OpenApiConstants.NullableExtension, out var nullExtRawValue) &&
nullExtRawValue is OpenApiAny { Node: JsonNode jsonNode} &&
nullExtRawValue is OpenApiAny { Node: JsonNode jsonNode } &&
jsonNode.GetValueKind() is JsonValueKind.True;
if (type is null)
{
Expand All @@ -656,14 +656,14 @@ Extensions is not null &&
break;
case OpenApiSpecVersion.OpenApi3_0 when isNullable && type.Value == JsonSchemaType.Null:
writer.WriteProperty(OpenApiConstants.Nullable, true);
writer.WriteProperty(OpenApiConstants.Type, JsonSchemaType.Object.ToIdentifier());
writer.WriteProperty(OpenApiConstants.Type, JsonSchemaType.Object.ToFirstIdentifier());
break;
case OpenApiSpecVersion.OpenApi3_0 when isNullable && type.Value != JsonSchemaType.Null:
writer.WriteProperty(OpenApiConstants.Nullable, true);
writer.WriteProperty(OpenApiConstants.Type, type.Value.ToIdentifier());
writer.WriteProperty(OpenApiConstants.Type, type.Value.ToFirstIdentifier());
break;
default:
writer.WriteProperty(OpenApiConstants.Type, type.Value.ToIdentifier());
writer.WriteProperty(OpenApiConstants.Type, type.Value.ToFirstIdentifier());
break;
}
}
Expand All @@ -679,7 +679,13 @@ Extensions is not null &&
var list = (from JsonSchemaType flag in jsonSchemaTypeValues
where type.Value.HasFlag(flag)
select flag).ToList();
writer.WriteOptionalCollection(OpenApiConstants.Type, list, (w, s) => w.WriteValue(s.ToIdentifier()));
writer.WriteOptionalCollection(OpenApiConstants.Type, list, (w, s) =>
{
foreach(var item in s.ToIdentifiers())
{
w.WriteValue(item);
}
});
}
}
}
Expand All @@ -702,7 +708,7 @@ private static void UpCastSchemaTypeToV31(JsonSchemaType type, IOpenApiWriter wr
var temporaryType = type | JsonSchemaType.Null;
var list = (from JsonSchemaType flag in jsonSchemaTypeValues// Check if the flag is set in 'type' using a bitwise AND operation
where temporaryType.HasFlag(flag)
select flag.ToIdentifier()).ToList();
select flag.ToFirstIdentifier()).ToList();
if (list.Count > 1)
{
writer.WriteOptionalCollection(OpenApiConstants.Type, list, (w, s) => w.WriteValue(s));
Expand Down Expand Up @@ -739,7 +745,7 @@ private void DowncastTypeArrayToV2OrV3(JsonSchemaType schemaType, IOpenApiWriter
if (schemaType.HasFlag(flag) && flag != JsonSchemaType.Null)
{
// Write the non-null flag value to the writer
writer.WriteProperty(OpenApiConstants.Type, flag.ToIdentifier());
writer.WriteProperty(OpenApiConstants.Type, flag.ToFirstIdentifier());
}
}
writer.WriteProperty(nullableProp, true);
Expand All @@ -752,7 +758,7 @@ private void DowncastTypeArrayToV2OrV3(JsonSchemaType schemaType, IOpenApiWriter
}
else
{
writer.WriteProperty(OpenApiConstants.Type, schemaType.ToIdentifier());
writer.WriteProperty(OpenApiConstants.Type, schemaType.ToFirstIdentifier());
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/Microsoft.OpenApi/Validations/Rules/RuleHelpers.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System.Linq;
using System.Text.Json;
using System.Text.Json.Nodes;
using Microsoft.OpenApi.Extensions;
Expand Down Expand Up @@ -55,7 +56,7 @@ public static void ValidateDataTypeMismatch(
// convert value to JsonElement and access the ValueKind property to determine the type.
var valueKind = value.GetValueKind();

var type = schema.Type.ToIdentifier();
var type = (schema.Type & ~JsonSchemaType.Null)?.ToFirstIdentifier();
var format = schema.Format;

// Before checking the type, check first if the schema allows null.
Expand Down
54 changes: 49 additions & 5 deletions test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiSchemaTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
using FluentAssertions;
using FluentAssertions.Equivalency;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Extensions;
using Microsoft.OpenApi.Models.Interfaces;
using Microsoft.OpenApi.Reader;
using Microsoft.OpenApi.Tests;
using Microsoft.OpenApi.Writers;
using Xunit;
using Microsoft.OpenApi.Exceptions;
using System;

namespace Microsoft.OpenApi.Readers.Tests.V31Tests
{
Expand All @@ -31,7 +34,7 @@ public static MemoryStream GetMemoryStream(string fileName)

public OpenApiSchemaTests()
{
OpenApiReaderRegistry.RegisterReader("yaml", new OpenApiYamlReader());
OpenApiReaderRegistry.RegisterReader("yaml", new OpenApiYamlReader());
}

[Fact]
Expand Down Expand Up @@ -317,8 +320,8 @@ public void CloningSchemaWithExamplesAndEnumsShouldSucceed()
clone.Default = 6;

// Assert
Assert.Equivalent(new int[] {1, 2, 3, 4}, clone.Enum.Select(static x => x.GetValue<int>()).ToArray());
Assert.Equivalent(new int[] {2, 3, 4}, clone.Examples.Select(static x => x.GetValue<int>()).ToArray());
Assert.Equivalent(new int[] { 1, 2, 3, 4 }, clone.Enum.Select(static x => x.GetValue<int>()).ToArray());
Assert.Equivalent(new int[] { 2, 3, 4 }, clone.Examples.Select(static x => x.GetValue<int>()).ToArray());
Assert.Equivalent(6, clone.Default.GetValue<int>());
}

Expand Down Expand Up @@ -417,7 +420,7 @@ public void SerializeSchemaWithTypeArrayAndNullableDoesntEmitType()
schema.SerializeAsV2(new OpenApiYamlWriter(writer));
var schemaString = writer.ToString();

Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), schemaString.MakeLineBreaksEnvironmentNeutral());
Assert.Equal(expected.MakeLineBreaksEnvironmentNeutral(), schemaString.MakeLineBreaksEnvironmentNeutral());
}

[Theory]
Expand Down Expand Up @@ -525,7 +528,7 @@ public async Task ParseSchemaWithConstWorks()
}

[Fact]
public void ParseSchemaWithUnrecognizedKeywordsWorks()
public void ParseSchemaWithUnrecognizedKeywordsWorks()
{
var input = @"{
""type"": ""string"",
Expand All @@ -539,5 +542,46 @@ public void ParseSchemaWithUnrecognizedKeywordsWorks()
Assert.Equal(2, schema.UnrecognizedKeywords.Count);
}

[Theory]
[InlineData(JsonSchemaType.Integer | JsonSchemaType.String, new[] { "integer", "string" })]
[InlineData(JsonSchemaType.Integer | JsonSchemaType.Null, new[] { "integer", "null" })]
[InlineData(JsonSchemaType.Integer, new[] { "integer" })]
public void NormalizeFlaggableJsonSchemaTypeEnumWorks(JsonSchemaType type, string[] expected)
{
var schema = new OpenApiSchema
{
Type = type
};

var actual = schema.Type.ToIdentifiers();
Assert.Equal(expected, actual);
}

[Theory]
[InlineData(new[] { "integer", "string" }, JsonSchemaType.Integer | JsonSchemaType.String)]
[InlineData(new[] { "integer", "null" }, JsonSchemaType.Integer | JsonSchemaType.Null)]
[InlineData(new[] { "integer" }, JsonSchemaType.Integer)]
public void ArrayIdentifierToEnumConversionWorks(string[] type, JsonSchemaType expected)
{
var actual = type.ToJsonSchemaType();
Assert.Equal(expected, actual);
}

[Fact]
public void StringIdentifierToEnumConversionWorks()
{
var actual = "integer".ToJsonSchemaType();
Assert.Equal(JsonSchemaType.Integer, actual);
}

[Fact]
public void ReturnSingleIdentifierWorks()
{
var type = JsonSchemaType.Integer;
var types = JsonSchemaType.Integer | JsonSchemaType.Null;

Assert.Equal("integer", type.ToSingleIdentifier());
Assert.Throws<InvalidOperationException>(() => types.ToSingleIdentifier());
}
}
}
5 changes: 3 additions & 2 deletions test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,10 @@ 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[] ToIdentifiers(this Microsoft.OpenApi.Models.JsonSchemaType schemaType) { }
public static string[]? ToIdentifiers(this Microsoft.OpenApi.Models.JsonSchemaType? schemaType) { }
public static Microsoft.OpenApi.Models.JsonSchemaType ToJsonSchemaType(this string identifier) { }
public static Microsoft.OpenApi.Models.JsonSchemaType? ToJsonSchemaType(this string[] identifier) { }
}
}
namespace Microsoft.OpenApi.Interfaces
Expand Down

0 comments on commit b80e934

Please sign in to comment.