From 00890c9e58fbd86985a8337da3587c7da4ab355a Mon Sep 17 00:00:00 2001 From: martincostello Date: Fri, 25 Oct 2024 12:36:59 +0100 Subject: [PATCH] Avoid exception checking nullability - Do not throw if we cannot determine the nullability of a dictionary. - Clean-up some code analysis suggestions. Resolves #3070. Resolves #2793. --- .../SchemaGenerator/MemberInfoExtensions.cs | 27 +++--- .../SchemaGenerator/SchemaGenerator.cs | 9 +- .../MemberInfoExtensionsTests.cs | 86 +++++++++++++++++++ 3 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/MemberInfoExtensionsTests.cs diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/MemberInfoExtensions.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/MemberInfoExtensions.cs index 01d5981320..d530a3a9aa 100644 --- a/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/MemberInfoExtensions.cs +++ b/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/MemberInfoExtensions.cs @@ -85,16 +85,19 @@ public static bool IsDictionaryValueNonNullable(this MemberInfo memberInfo) { #if NET6_0_OR_GREATER var nullableInfo = GetNullabilityInfo(memberInfo); - if (nullableInfo.GenericTypeArguments.Length != 2) - { - var length = nullableInfo.GenericTypeArguments.Length; - var type = nullableInfo.Type.FullName; - var container = memberInfo.DeclaringType.FullName; - var member = memberInfo.Name; - throw new InvalidOperationException($"Expected Dictionary to have two generic type arguments but it had {length}. Member: {container}.{member} Type: {type}."); - } - return nullableInfo.GenericTypeArguments[1].ReadState == NullabilityState.NotNull; + // Assume one generic argument means TKey and TValue are the same type. + // Assume two generic arguments match TKey and TValue for a dictionary. + // A better solution would be to inspect the type declaration (base types, + // interfaces, etc.) to determine if the type is a dictionary, but the + // nullability information is not available to be able to do that. + // See https://stackoverflow.com/q/75786306/1064169. + return nullableInfo.GenericTypeArguments.Length switch + { + 1 => nullableInfo.GenericTypeArguments[0].ReadState == NullabilityState.NotNull, + 2 => nullableInfo.GenericTypeArguments[1].ReadState == NullabilityState.NotNull, + _ => false, + }; #else var memberType = memberInfo.MemberType == MemberTypes.Field ? ((FieldInfo)memberInfo).FieldType @@ -156,11 +159,11 @@ private static bool GetNullableFallbackValue(this MemberInfo memberInfo) { var declaringTypes = memberInfo.DeclaringType.IsNested ? GetDeclaringTypeChain(memberInfo) - : new List(1) { memberInfo.DeclaringType }; + : [memberInfo.DeclaringType]; foreach (var declaringType in declaringTypes) { - var attributes = (IEnumerable)declaringType.GetCustomAttributes(false); + IEnumerable attributes = declaringType.GetCustomAttributes(false); var nullableContext = attributes .FirstOrDefault(attr => string.Equals(attr.GetType().FullName, NullableContextAttributeFullTypeName)); @@ -168,7 +171,7 @@ private static bool GetNullableFallbackValue(this MemberInfo memberInfo) if (nullableContext != null) { if (nullableContext.GetType().GetField(FlagFieldName) is FieldInfo field && - field.GetValue(nullableContext) is byte flag && flag == NotAnnotated) + field.GetValue(nullableContext) is byte flag && flag == NotAnnotated) { return true; } diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs index 247c33e456..7d6c79ec1f 100644 --- a/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs +++ b/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs @@ -27,7 +27,10 @@ public SchemaGenerator(SchemaGeneratorOptions generatorOptions, ISerializerDataC { } - public SchemaGenerator(SchemaGeneratorOptions generatorOptions, ISerializerDataContractResolver serializerDataContractResolver, IOptions mvcOptions) + public SchemaGenerator( + SchemaGeneratorOptions generatorOptions, + ISerializerDataContractResolver serializerDataContractResolver, + IOptions mvcOptions) { _generatorOptions = generatorOptions; _serializerDataContractResolver = serializerDataContractResolver; @@ -104,7 +107,7 @@ private OpenApiSchema GenerateSchemaForMember( var genericTypes = modelType .GetInterfaces() #if NETSTANDARD2_0 - .Concat(new[] { modelType }) + .Concat([modelType]) #else .Append(modelType) #endif @@ -309,7 +312,7 @@ private static OpenApiSchema CreatePrimitiveSchema(DataContract dataContract) }; #pragma warning disable CS0618 // Type or member is obsolete - // For backcompat only - EnumValues is obsolete + // For backwards compatibility only - EnumValues is obsolete if (dataContract.EnumValues != null) { schema.Enum = dataContract.EnumValues diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/MemberInfoExtensionsTests.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/MemberInfoExtensionsTests.cs new file mode 100644 index 0000000000..fa389b464e --- /dev/null +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/MemberInfoExtensionsTests.cs @@ -0,0 +1,86 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Xunit; + +namespace Swashbuckle.AspNetCore.SwaggerGen; + +#nullable enable + +public static class MemberInfoExtensionsTests +{ + [Theory] + [InlineData(typeof(MyClass), nameof(MyClass.DictionaryInt32NonNullable), true)] + [InlineData(typeof(MyClass), nameof(MyClass.DictionaryInt32Nullable), false)] + [InlineData(typeof(MyClass), nameof(MyClass.DictionaryStringNonNullable), true)] + [InlineData(typeof(MyClass), nameof(MyClass.DictionaryStringNullable), false)] + [InlineData(typeof(MyClass), nameof(MyClass.IDictionaryInt32NonNullable), true)] + [InlineData(typeof(MyClass), nameof(MyClass.IDictionaryInt32Nullable), false)] + [InlineData(typeof(MyClass), nameof(MyClass.IDictionaryStringNonNullable), true)] + [InlineData(typeof(MyClass), nameof(MyClass.IDictionaryStringNullable), false)] + [InlineData(typeof(MyClass), nameof(MyClass.IReadOnlyDictionaryInt32NonNullable), true)] + [InlineData(typeof(MyClass), nameof(MyClass.IReadOnlyDictionaryInt32Nullable), false)] + [InlineData(typeof(MyClass), nameof(MyClass.IReadOnlyDictionaryStringNonNullable), true)] + [InlineData(typeof(MyClass), nameof(MyClass.IReadOnlyDictionaryStringNullable), false)] + [InlineData(typeof(MyClass), nameof(MyClass.StringDictionary), false)] // There is no way to inspect the nullability of the base class' TValue argument + [InlineData(typeof(MyClass), nameof(MyClass.NullableStringDictionary), false)] + [InlineData(typeof(MyClass), nameof(MyClass.SameTypesDictionary), true)] + [InlineData(typeof(MyClass), nameof(MyClass.CustomDictionaryStringNullable), false)] + [InlineData(typeof(MyClass), nameof(MyClass.CustomDictionaryStringNonNullable), true)] + public static void IsDictionaryValueNonNullable_Returns_Correct_Value(Type type, string memberName, bool expected) + { + // Arrange + var memberInfo = type.GetMember(memberName).First(); + + // Act + var actual = memberInfo.IsDictionaryValueNonNullable(); + + // Assert + Assert.Equal(expected, actual); + } + + public class MyClass + { + public Dictionary DictionaryInt32NonNullable { get; set; } = []; + + public Dictionary DictionaryInt32Nullable { get; set; } = []; + + public Dictionary DictionaryStringNonNullable { get; set; } = []; + + public Dictionary DictionaryStringNullable { get; set; } = []; + + public IDictionary IDictionaryInt32NonNullable { get; set; } = new Dictionary(); + + public IDictionary IDictionaryInt32Nullable { get; set; } = new Dictionary(); + + public IDictionary IDictionaryStringNonNullable { get; set; } = new Dictionary(); + + public IDictionary IDictionaryStringNullable { get; set; } = new Dictionary(); + + public IReadOnlyDictionary IReadOnlyDictionaryInt32NonNullable { get; set; } = new Dictionary(); + + public IReadOnlyDictionary IReadOnlyDictionaryInt32Nullable { get; set; } = new Dictionary(); + + public IReadOnlyDictionary IReadOnlyDictionaryStringNonNullable { get; set; } = new Dictionary(); + + public IReadOnlyDictionary IReadOnlyDictionaryStringNullable { get; set; } = new Dictionary(); + + public StringDictionary StringDictionary { get; set; } = []; + + public NullableStringDictionary NullableStringDictionary { get; set; } = []; + + public SameTypesDictionary SameTypesDictionary { get; set; } = []; + + public CustomDictionary CustomDictionaryStringNullable { get; set; } = []; + + public CustomDictionary CustomDictionaryStringNonNullable { get; set; } = []; + } + + public class StringDictionary : Dictionary; + + public class NullableStringDictionary : Dictionary; + + public class SameTypesDictionary : Dictionary where T : notnull; + + public class CustomDictionary : Dictionary where TKey : notnull; +}