From 5bcb1d1268626db73362a0413eea5dd50ebcdf4b Mon Sep 17 00:00:00 2001 From: Havunen Date: Sat, 17 Feb 2024 19:42:33 +0200 Subject: [PATCH] Fixes an issue where nullable information gets lost /~https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/2731 for /~https://github.com/Havunen/DotSwashbuckle/issues/3 --- .../SchemaGenerator/SchemaGenerator.cs | 89 +++++++++++++------ .../NewtonsoftSchemaGeneratorTests.cs | 50 ++++++----- .../Fixtures/TypeWithNullableProperties.cs | 5 +- .../Dummy/Controllers/Controller (1).cs | 13 +++ 4 files changed, 104 insertions(+), 53 deletions(-) diff --git a/src/DotSwashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs b/src/DotSwashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs index 296570b..4c2edfc 100644 --- a/src/DotSwashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs +++ b/src/DotSwashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs @@ -3,6 +3,7 @@ using System.Collections.ObjectModel; using System.ComponentModel; using System.ComponentModel.DataAnnotations; +using System.Data.SqlTypes; using System.Globalization; using System.Linq; using System.Reflection; @@ -48,11 +49,11 @@ private OpenApiSchema GenerateSchemaForMember( MemberInfo memberInfo, DataProperty dataProperty = null) { - var dataContract = GetDataContractFor(modelType); + var (dataContract, isNullable) = GetDataContractFor(modelType); var schema = _generatorOptions.UseOneOfForPolymorphism && IsBaseTypeWithKnownTypesDefined(dataContract, out var knownTypesDataContracts) - ? GeneratePolymorphicSchema(dataContract, schemaRepository, knownTypesDataContracts) - : GenerateConcreteSchema(dataContract, schemaRepository); + ? GeneratePolymorphicSchema(schemaRepository, knownTypesDataContracts) + : GenerateConcreteSchema(dataContract, schemaRepository, isNullable); if (_generatorOptions.UseAllOfToExtendReferenceSchemas && schema.Reference != null) { @@ -111,11 +112,11 @@ private OpenApiSchema GenerateSchemaForParameter( ParameterInfo parameterInfo, ApiParameterRouteInfo routeInfo) { - var dataContract = GetDataContractFor(modelType); + var (dataContract, isNullable) = GetDataContractFor(modelType); var schema = _generatorOptions.UseOneOfForPolymorphism && IsBaseTypeWithKnownTypesDefined(dataContract, out var knownTypesDataContracts) - ? GeneratePolymorphicSchema(dataContract, schemaRepository, knownTypesDataContracts) - : GenerateConcreteSchema(dataContract, schemaRepository); + ? GeneratePolymorphicSchema(schemaRepository, knownTypesDataContracts) + : GenerateConcreteSchema(dataContract, schemaRepository, isNullable); if (_generatorOptions.UseAllOfToExtendReferenceSchemas && schema.Reference != null) { @@ -151,11 +152,11 @@ private OpenApiSchema GenerateSchemaForParameter( private OpenApiSchema GenerateSchemaForType(Type modelType, SchemaRepository schemaRepository, bool isEffectiveTypeNeeded = true) { - var dataContract = GetDataContractFor(modelType, isEffectiveTypeNeeded); + var (dataContract, isNullable) = GetDataContractFor(modelType, isEffectiveTypeNeeded); var schema = _generatorOptions.UseOneOfForPolymorphism && IsBaseTypeWithKnownTypesDefined(dataContract, out var knownTypesDataContracts) - ? GeneratePolymorphicSchema(dataContract, schemaRepository, knownTypesDataContracts) - : GenerateConcreteSchema(dataContract, schemaRepository); + ? GeneratePolymorphicSchema(schemaRepository, knownTypesDataContracts) + : GenerateConcreteSchema(dataContract, schemaRepository, isNullable); if (schema.Reference == null) { @@ -165,13 +166,28 @@ private OpenApiSchema GenerateSchemaForType(Type modelType, SchemaRepository sch return schema; } - private DataContract GetDataContractFor(Type modelType, bool isEffectiveTypeNeeded = true) + private (DataContract dataContract, bool isNullable) GetDataContractFor(Type modelType, bool isEffectiveTypeNeeded = true) { - var effectiveType = isEffectiveTypeNeeded ? Nullable.GetUnderlyingType(modelType) ?? modelType : modelType; - return _serializerDataContractResolver.GetDataContractForType(effectiveType); + Type effectiveType = null; + var isNullable = false; + + if (isEffectiveTypeNeeded) + { + effectiveType = Nullable.GetUnderlyingType(modelType); + isNullable = effectiveType != null; + effectiveType ??= modelType; + } + + effectiveType ??= modelType; + + + return ( + _serializerDataContractResolver.GetDataContractForType(effectiveType), + isNullable + ); } - private bool IsBaseTypeWithKnownTypesDefined(DataContract dataContract, out IEnumerable knownTypesDataContracts) + private bool IsBaseTypeWithKnownTypesDefined(DataContract dataContract, out IEnumerable<(DataContract dataContract, bool isNullable)> knownTypesDataContracts) { knownTypesDataContracts = null; @@ -192,19 +208,26 @@ private bool IsBaseTypeWithKnownTypesDefined(DataContract dataContract, out IEnu } private OpenApiSchema GeneratePolymorphicSchema( - DataContract dataContract, SchemaRepository schemaRepository, - IEnumerable knownTypesDataContracts) + IEnumerable<(DataContract, bool)> knownTypesDataContracts) { return new OpenApiSchema { OneOf = knownTypesDataContracts - .Select(allowedTypeDataContract => GenerateConcreteSchema(allowedTypeDataContract, schemaRepository)) + .Select(dataContractNullableTuple => GenerateConcreteSchema( + dataContractNullableTuple.Item1, + schemaRepository, + dataContractNullableTuple.Item2 + )) .ToList() }; } - private OpenApiSchema GenerateConcreteSchema(DataContract dataContract, SchemaRepository schemaRepository) + private OpenApiSchema GenerateConcreteSchema( + DataContract dataContract, + SchemaRepository schemaRepository, + bool isNullable + ) { if (TryGetCustomTypeMapping(dataContract.UnderlyingType, out Func customSchemaFactory)) { @@ -226,7 +249,7 @@ private OpenApiSchema GenerateConcreteSchema(DataContract dataContract, SchemaRe case DataType.Number: case DataType.String: { - schemaFactory = () => CreatePrimitiveSchema(dataContract); + schemaFactory = () => CreatePrimitiveSchema(dataContract, isNullable); returnAsReference = dataContract.UnderlyingType.IsEnum && !_generatorOptions.UseInlineDefinitionsForEnums; break; } @@ -271,13 +294,13 @@ private bool TryGetCustomTypeMapping(Type modelType, out Func sch || (modelType.IsConstructedGenericType && _generatorOptions.CustomTypeMappings.TryGetValue(modelType.GetGenericTypeDefinition(), out schemaFactory)); } - private OpenApiSchema CreatePrimitiveSchema(DataContract dataContract) + private OpenApiSchema CreatePrimitiveSchema(DataContract dataContract, bool isNullable) { var schema = new OpenApiSchema { Type = dataContract.DataType.ToString().ToLower(CultureInfo.InvariantCulture), Format = dataContract.DataFormat, - Nullable = Nullable.GetUnderlyingType(dataContract.UnderlyingType) != null + Nullable = isNullable || Nullable.GetUnderlyingType(dataContract.UnderlyingType) != null }; if (dataContract.UnderlyingType.IsEnum) @@ -346,9 +369,13 @@ private OpenApiSchema CreateObjectSchema(DataContract dataContract, SchemaReposi if (_generatorOptions.UseAllOfForInheritance || _generatorOptions.UseOneOfForPolymorphism) { - if (IsKnownSubType(dataContract, out var baseTypeDataContract)) + if (IsKnownSubType(dataContract, out var baseTypeDataContractTuple)) { - var baseTypeSchema = GenerateConcreteSchema(baseTypeDataContract, schemaRepository); + var baseTypeSchema = GenerateConcreteSchema( + baseTypeDataContractTuple.Item1, + schemaRepository, + baseTypeDataContractTuple.Item2 + ); schema.AllOf.Add(baseTypeSchema); @@ -361,7 +388,7 @@ private OpenApiSchema CreateObjectSchema(DataContract dataContract, SchemaReposi foreach (var knownTypeDataContract in knownTypesDataContracts) { // Ensure schema is generated for all known types - GenerateConcreteSchema(knownTypeDataContract, schemaRepository); + GenerateConcreteSchema(knownTypeDataContract.dataContract, schemaRepository, knownTypeDataContract.isNullable); } if (TryGetDiscriminatorFor(dataContract, schemaRepository, knownTypesDataContracts, out var discriminator)) @@ -403,9 +430,9 @@ private OpenApiSchema CreateObjectSchema(DataContract dataContract, SchemaReposi return schema; } - private bool IsKnownSubType(DataContract dataContract, out DataContract baseTypeDataContract) + private bool IsKnownSubType(DataContract dataContract, out (DataContract, bool) baseTypeDataContract) { - baseTypeDataContract = null; + baseTypeDataContract = (null, false); var baseType = dataContract.UnderlyingType.BaseType; @@ -427,7 +454,7 @@ private bool IsKnownSubType(DataContract dataContract, out DataContract baseType private bool TryGetDiscriminatorFor( DataContract dataContract, SchemaRepository schemaRepository, - IEnumerable knownTypesDataContracts, + IEnumerable<(DataContract dataContract, bool isNullable)> knownTypesDataContracts, out OpenApiDiscriminator discriminator) { discriminator = null; @@ -444,12 +471,16 @@ private bool TryGetDiscriminatorFor( foreach (var knownTypeDataContract in knownTypesDataContracts) { - var discriminatorValue = _generatorOptions.DiscriminatorValueSelector(knownTypeDataContract.UnderlyingType) - ?? knownTypeDataContract.ObjectTypeNameValue; + var discriminatorValue = _generatorOptions.DiscriminatorValueSelector(knownTypeDataContract.dataContract.UnderlyingType) + ?? knownTypeDataContract.dataContract.ObjectTypeNameValue; if (discriminatorValue == null) continue; - discriminator.Mapping.Add(discriminatorValue, GenerateConcreteSchema(knownTypeDataContract, schemaRepository).Reference.ReferenceV3); + discriminator.Mapping.Add(discriminatorValue, GenerateConcreteSchema( + knownTypeDataContract.dataContract, + schemaRepository, + knownTypeDataContract.isNullable + ).Reference.ReferenceV3); } return true; diff --git a/test/DotSwashbuckle.AspNetCore.Newtonsoft.Test/SchemaGenerator/NewtonsoftSchemaGeneratorTests.cs b/test/DotSwashbuckle.AspNetCore.Newtonsoft.Test/SchemaGenerator/NewtonsoftSchemaGeneratorTests.cs index 6ca77b5..386dcf0 100644 --- a/test/DotSwashbuckle.AspNetCore.Newtonsoft.Test/SchemaGenerator/NewtonsoftSchemaGeneratorTests.cs +++ b/test/DotSwashbuckle.AspNetCore.Newtonsoft.Test/SchemaGenerator/NewtonsoftSchemaGeneratorTests.cs @@ -42,37 +42,40 @@ public void GenerateSchema_GeneratesFileSchema_IfFormFileOrFileResultType(Type t } [Theory] - [InlineData(typeof(byte), "integer", "int32")] - [InlineData(typeof(sbyte), "integer", "int32")] - [InlineData(typeof(short), "integer", "int32")] - [InlineData(typeof(ushort), "integer", "int32")] - [InlineData(typeof(int), "integer", "int32")] - [InlineData(typeof(uint), "integer", "int32")] - [InlineData(typeof(long), "integer", "int64")] - [InlineData(typeof(ulong), "integer", "int64")] - [InlineData(typeof(float), "number", "float")] - [InlineData(typeof(double), "number", "double")] - [InlineData(typeof(decimal), "number", "double")] - [InlineData(typeof(string), "string", null)] - [InlineData(typeof(char), "string", null)] - [InlineData(typeof(byte[]), "string", "byte")] - [InlineData(typeof(DateTime), "string", "date-time")] - [InlineData(typeof(DateTimeOffset), "string", "date-time")] - [InlineData(typeof(Guid), "string", "uuid")] - [InlineData(typeof(TimeSpan), "string", "date-span")] - [InlineData(typeof(Version), "string", null)] - [InlineData(typeof(bool?), "boolean", null)] - [InlineData(typeof(int?), "integer", "int32")] - [InlineData(typeof(DateTime?), "string", "date-time")] + [InlineData(typeof(byte), "integer", "int32", false)] + [InlineData(typeof(sbyte), "integer", "int32", false)] + [InlineData(typeof(short), "integer", "int32", false)] + [InlineData(typeof(ushort), "integer", "int32", false)] + [InlineData(typeof(int), "integer", "int32", false)] + [InlineData(typeof(uint), "integer", "int32", false)] + [InlineData(typeof(long), "integer", "int64", false)] + [InlineData(typeof(ulong), "integer", "int64", false)] + [InlineData(typeof(float), "number", "float", false)] + [InlineData(typeof(double), "number", "double", false)] + [InlineData(typeof(decimal), "number", "double", false)] + [InlineData(typeof(string), "string", null, false)] + [InlineData(typeof(char), "string", null, false)] + [InlineData(typeof(byte[]), "string", "byte", false)] + [InlineData(typeof(DateTime), "string", "date-time", false)] + [InlineData(typeof(DateTimeOffset), "string", "date-time", false)] + [InlineData(typeof(Guid), "string", "uuid", false)] + [InlineData(typeof(TimeSpan), "string", "date-span", false)] + [InlineData(typeof(Version), "string", null, false)] + [InlineData(typeof(bool?), "boolean", null, true)] + [InlineData(typeof(int?), "integer", "int32", true)] + [InlineData(typeof(DateTime?), "string", "date-time", true)] public void GenerateSchema_GeneratesPrimitiveSchema_IfPrimitiveOrNullablePrimitiveType( Type type, string expectedSchemaType, - string expectedFormat) + string expectedFormat, + bool expectedNullable + ) { var schema = Subject().GenerateSchema(type, new SchemaRepository()); Assert.Equal(expectedSchemaType, schema.Type); Assert.Equal(expectedFormat, schema.Format); + Assert.Equal(expectedNullable, schema.Nullable); } [Theory] @@ -276,6 +279,7 @@ public void GenerateSchema_ExcludesIndexerProperties_IfComplexTypeIsIndexed() [InlineData(typeof(TypeWithNullableProperties), nameof(TypeWithNullableProperties.IntProperty), false)] [InlineData(typeof(TypeWithNullableProperties), nameof(TypeWithNullableProperties.StringProperty), true)] [InlineData(typeof(TypeWithNullableProperties), nameof(TypeWithNullableProperties.NullableIntProperty), true)] + [InlineData(typeof(TypeWithNullableProperties), nameof(TypeWithNullableProperties.SysNullableIntProperty), true)] public void GenerateSchema_SetsNullableFlag_IfPropertyIsReferenceOrNullableType( Type declaringType, string propertyName, diff --git a/test/DotSwashbuckle.AspNetCore.TestSupport/Fixtures/TypeWithNullableProperties.cs b/test/DotSwashbuckle.AspNetCore.TestSupport/Fixtures/TypeWithNullableProperties.cs index e92feec..2234da7 100644 --- a/test/DotSwashbuckle.AspNetCore.TestSupport/Fixtures/TypeWithNullableProperties.cs +++ b/test/DotSwashbuckle.AspNetCore.TestSupport/Fixtures/TypeWithNullableProperties.cs @@ -1,4 +1,6 @@ -namespace DotSwashbuckle.AspNetCore.TestSupport +using System; + +namespace DotSwashbuckle.AspNetCore.TestSupport { public class TypeWithNullableProperties { @@ -7,5 +9,6 @@ public class TypeWithNullableProperties public string StringProperty { get; set; } public int? NullableIntProperty { get; set; } + public Nullable SysNullableIntProperty { get; set; } } } diff --git a/test/WebSites/Dummy/Controllers/Controller (1).cs b/test/WebSites/Dummy/Controllers/Controller (1).cs index bb14f1d..79597c9 100644 --- a/test/WebSites/Dummy/Controllers/Controller (1).cs +++ b/test/WebSites/Dummy/Controllers/Controller (1).cs @@ -2,7 +2,10 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; using System; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using DotSwashbuckle.AspNetCore.Annotations; +using Microsoft.AspNetCore.Http; namespace Dummy.Controllers { @@ -40,5 +43,15 @@ [FromRoute] [MinLength(1)] [MaxLength(4)] [Length(2, 5)] string minMaxCat2 = "CC { throw new NotImplementedException(); } + + [Route("intnull")] + [ProducesResponseType(StatusCodes.Status200OK)] + [HttpGet] + [SwaggerOperation(OperationId = "IntNull")] + [SwaggerResponse(200, "nullable int", typeof(Nullable))] //this makes no diffrence + public async Task>> IntNull([FromQuery] Dictionary test) + { + return this.Ok(null); + } } }