From b80e9342018cf136cc54b900bb95832a6867e982 Mon Sep 17 00:00:00 2001 From: Maggie Kimani Date: Thu, 20 Feb 2025 19:54:19 +0300 Subject: [PATCH] fix: refactor ToIdentifier() to normalize flaggable enums (#2156) * 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 * chore: address PR feedback * chore: make method internal; update public API * Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs Co-authored-by: Darrel * Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs Co-authored-by: Darrel * fix: address more PR feedback, add and fix tests * Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs Co-authored-by: Vincent Biret * Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs Co-authored-by: Vincent Biret * Update src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs Co-authored-by: Vincent Biret * chore: clean up --------- Co-authored-by: Vincent Biret Co-authored-by: Darrel --- .../Extensions/OpenApiTypeMapper.cs | 135 ++++++++++++------ src/Microsoft.OpenApi/Models/OpenApiSchema.cs | 26 ++-- .../Validations/Rules/RuleHelpers.cs | 3 +- .../V31Tests/OpenApiSchemaTests.cs | 54 ++++++- .../PublicApi/PublicApi.approved.txt | 5 +- 5 files changed, 161 insertions(+), 62 deletions(-) diff --git a/src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs b/src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs index eea41be49..3ae417022 100644 --- a/src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs +++ b/src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using Microsoft.OpenApi.Exceptions; using Microsoft.OpenApi.Models; @@ -19,13 +20,13 @@ public static class OpenApiTypeMapper /// /// /// - 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(); } /// @@ -33,20 +34,47 @@ public static class OpenApiTypeMapper /// /// /// - 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 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 ToIdentifiersInternal(this JsonSchemaType schemaType) + { + return allSchemaTypes.Where(kvp => schemaType.HasFlag(kvp.Key)).Select(static kvp => kvp.Value); + } + + /// + /// Returns the first identifier from a string array. + /// + /// + /// + internal static string ToFirstIdentifier(this JsonSchemaType schemaType) + { + return schemaType.ToIdentifiersInternal().First(); + } + + /// + /// Returns a single identifier from an array with only one item. + /// + /// + /// + internal static string ToSingleIdentifier(this JsonSchemaType schemaType) + { + return schemaType.ToIdentifiersInternal().Single(); } + #nullable restore /// @@ -70,6 +98,26 @@ public static JsonSchemaType ToJsonSchemaType(this string identifier) }; } + /// + /// Converts a schema type's identifier into the enum equivalent + /// + /// + /// + 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> _simpleTypeToOpenApiSchema = new() { [typeof(bool)] = () => new() { Type = JsonSchemaType.Boolean }, @@ -141,7 +189,7 @@ public static OpenApiSchema MapTypeToOpenApiPrimitiveType(this Type type) } /// - /// Maps an JsonSchema data type and format to a simple type. + /// Maps a JsonSchema data type and format to a simple type. /// /// The OpenApi data type /// The simple type @@ -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), }; diff --git a/src/Microsoft.OpenApi/Models/OpenApiSchema.cs b/src/Microsoft.OpenApi/Models/OpenApiSchema.cs index da93b17ca..e524cfe41 100644 --- a/src/Microsoft.OpenApi/Models/OpenApiSchema.cs +++ b/src/Microsoft.OpenApi/Models/OpenApiSchema.cs @@ -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); @@ -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) { @@ -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; } } @@ -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); + } + }); } } } @@ -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)); @@ -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); @@ -752,7 +758,7 @@ private void DowncastTypeArrayToV2OrV3(JsonSchemaType schemaType, IOpenApiWriter } else { - writer.WriteProperty(OpenApiConstants.Type, schemaType.ToIdentifier()); + writer.WriteProperty(OpenApiConstants.Type, schemaType.ToFirstIdentifier()); } } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/RuleHelpers.cs b/src/Microsoft.OpenApi/Validations/Rules/RuleHelpers.cs index 63ca4d05e..71f46255f 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/RuleHelpers.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/RuleHelpers.cs @@ -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; @@ -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. diff --git a/test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiSchemaTests.cs b/test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiSchemaTests.cs index 127cbe689..f7c30f65e 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiSchemaTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/V31Tests/OpenApiSchemaTests.cs @@ -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 { @@ -31,7 +34,7 @@ public static MemoryStream GetMemoryStream(string fileName) public OpenApiSchemaTests() { - OpenApiReaderRegistry.RegisterReader("yaml", new OpenApiYamlReader()); + OpenApiReaderRegistry.RegisterReader("yaml", new OpenApiYamlReader()); } [Fact] @@ -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()).ToArray()); - Assert.Equivalent(new int[] {2, 3, 4}, clone.Examples.Select(static x => x.GetValue()).ToArray()); + Assert.Equivalent(new int[] { 1, 2, 3, 4 }, clone.Enum.Select(static x => x.GetValue()).ToArray()); + Assert.Equivalent(new int[] { 2, 3, 4 }, clone.Examples.Select(static x => x.GetValue()).ToArray()); Assert.Equivalent(6, clone.Default.GetValue()); } @@ -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] @@ -525,7 +528,7 @@ public async Task ParseSchemaWithConstWorks() } [Fact] - public void ParseSchemaWithUnrecognizedKeywordsWorks() + public void ParseSchemaWithUnrecognizedKeywordsWorks() { var input = @"{ ""type"": ""string"", @@ -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(() => types.ToSingleIdentifier()); + } } } diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt index d08428d5d..ca8c30327 100644 --- a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt +++ b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt @@ -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