Skip to content

Commit

Permalink
Fix exceptions generating default values (#2895)
Browse files Browse the repository at this point in the history
- Fix SwaggerGeneratorException if the type of a `[DefaultValue]` does not match the type of the property when using System.Text.Json for serialization to resolve #2885.
- Fix schema generation of default values for nullable enums with System.Text.Json to resolve #2904.
- Resolve some IDE refactoring suggestions.
- Render the response if an integration test fails.
  • Loading branch information
martincostello authored May 21, 2024
1 parent d5cd28e commit fe87e6d
Show file tree
Hide file tree
Showing 15 changed files with 198 additions and 34 deletions.
7 changes: 7 additions & 0 deletions Swashbuckle.AspNetCore.sln
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "WebApi.Aot", "test\WebSites
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "MinimalAppWithHostedServices", "test\WebSites\MinimalAppWithHostedServices\MinimalAppWithHostedServices.csproj", "{D06A88E8-6F42-4F40-943A-E266C0AE6EC9}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "MvcWithNullable", "test\WebSites\MvcWithNullable\MvcWithNullable.csproj", "{F88B6070-BE3C-45F9-978C-2ECBA9518C24}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -265,6 +267,10 @@ Global
{D06A88E8-6F42-4F40-943A-E266C0AE6EC9}.Debug|Any CPU.Build.0 = Debug|Any CPU
{D06A88E8-6F42-4F40-943A-E266C0AE6EC9}.Release|Any CPU.ActiveCfg = Release|Any CPU
{D06A88E8-6F42-4F40-943A-E266C0AE6EC9}.Release|Any CPU.Build.0 = Release|Any CPU
{F88B6070-BE3C-45F9-978C-2ECBA9518C24}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{F88B6070-BE3C-45F9-978C-2ECBA9518C24}.Debug|Any CPU.Build.0 = Debug|Any CPU
{F88B6070-BE3C-45F9-978C-2ECBA9518C24}.Release|Any CPU.ActiveCfg = Release|Any CPU
{F88B6070-BE3C-45F9-978C-2ECBA9518C24}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -309,6 +315,7 @@ Global
{DE1D77F8-3916-4DEE-A57D-6DDC357F64C6} = {DB3F57FC-1472-4F03-B551-43394DA3C5EB}
{07BB09CF-6C6F-4D00-A459-93586345C921} = {DB3F57FC-1472-4F03-B551-43394DA3C5EB}
{D06A88E8-6F42-4F40-943A-E266C0AE6EC9} = {DB3F57FC-1472-4F03-B551-43394DA3C5EB}
{F88B6070-BE3C-45F9-978C-2ECBA9518C24} = {DB3F57FC-1472-4F03-B551-43394DA3C5EB}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {36FC6A67-247D-4149-8EDD-79FFD1A75F51}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ private string JsonConverterFunc(object value)
return JsonConvert.SerializeObject(value, _serializerSettings);
}

private IEnumerable<DataProperty> GetDataPropertiesFor(JsonObjectContract jsonObjectContract, out Type extensionDataType)
private List<DataProperty> GetDataPropertiesFor(JsonObjectContract jsonObjectContract, out Type extensionDataType)
{
var dataProperties = new List<DataProperty>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,19 @@ public DataContract GetDataContractForType(Type type)
var enumValues = type.GetEnumValues();

// Test to determine if the serializer will treat as string
var serializeAsString = (enumValues.Length > 0)
&& JsonConverterFunc(enumValues.GetValue(0), type).StartsWith("\"");
var serializeAsString =
enumValues.Length > 0 &&
#if NET5_0_OR_GREATER
JsonConverterFunc(enumValues.GetValue(0), type).StartsWith('\"');
#else
JsonConverterFunc(enumValues.GetValue(0), type).StartsWith("\"");
#endif

var exampleType = serializeAsString ?
typeof(string) :
type.GetEnumUnderlyingType();

primitiveTypeAndFormat = serializeAsString
? PrimitiveTypesAndFormats[typeof(string)]
: PrimitiveTypesAndFormats[type.GetEnumUnderlyingType()];
primitiveTypeAndFormat = PrimitiveTypesAndFormats[exampleType];

return DataContract.ForPrimitive(
underlyingType: type,
Expand Down Expand Up @@ -144,7 +151,7 @@ public bool IsSupportedCollection(Type type, out Type itemType)
return false;
}

private IEnumerable<DataProperty> GetDataPropertiesFor(Type objectType, out Type extensionDataType)
private List<DataProperty> GetDataPropertiesFor(Type objectType, out Type extensionDataType)
{
extensionDataType = null;

Expand Down Expand Up @@ -177,7 +184,7 @@ private IEnumerable<DataProperty> GetDataPropertiesFor(Type objectType, out Type

return
(property.IsPubliclyReadable() || property.IsPubliclyWritable()) &&
!(property.GetIndexParameters().Any()) &&
!(property.GetIndexParameters().Length > 0) &&
!(property.HasAttribute<JsonIgnoreAttribute>() && isIgnoredViaNet5Attribute) &&
!(property.HasAttribute<SwaggerIgnoreAttribute>()) &&
!(_serializerOptions.IgnoreReadOnlyProperties && !property.IsPubliclyWritable());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ApiExplorer;
using Microsoft.OpenApi.Any;
using Microsoft.OpenApi.Models;

namespace Swashbuckle.AspNetCore.SwaggerGen
Expand Down Expand Up @@ -55,7 +56,7 @@ private OpenApiSchema GenerateSchemaForMember(

if (_generatorOptions.UseAllOfToExtendReferenceSchemas && schema.Reference != null)
{
schema.AllOf = new[] { new OpenApiSchema { Reference = schema.Reference } };
schema.AllOf = [new OpenApiSchema { Reference = schema.Reference }];
schema.Reference = null;
}

Expand All @@ -80,8 +81,7 @@ private OpenApiSchema GenerateSchemaForMember(
var defaultValueAttribute = customAttributes.OfType<DefaultValueAttribute>().FirstOrDefault();
if (defaultValueAttribute != null)
{
var defaultAsJson = dataContract.JsonConverter(defaultValueAttribute.Value);
schema.Default = OpenApiAnyFactory.CreateFromJson(defaultAsJson);
schema.Default = GenerateDefaultValue(dataContract, modelType, defaultValueAttribute.Value);
}

var obsoleteAttribute = customAttributes.OfType<ObsoleteAttribute>().FirstOrDefault();
Expand All @@ -90,8 +90,9 @@ private OpenApiSchema GenerateSchemaForMember(
schema.Deprecated = true;
}

// NullableAttribute behaves diffrently for Dictionaries
if (schema.AdditionalPropertiesAllowed && modelType.IsGenericType && modelType.GetGenericTypeDefinition() == typeof(Dictionary<,>))
// NullableAttribute behaves differently for Dictionaries
if (schema.AdditionalPropertiesAllowed && modelType.IsGenericType &&
modelType.GetGenericTypeDefinition() == typeof(Dictionary<,>))
{
schema.AdditionalProperties.Nullable = !memberInfo.IsDictionaryValueNonNullable();
}
Expand All @@ -118,7 +119,7 @@ private OpenApiSchema GenerateSchemaForParameter(

if (_generatorOptions.UseAllOfToExtendReferenceSchemas && schema.Reference != null)
{
schema.AllOf = new[] { new OpenApiSchema { Reference = schema.Reference } };
schema.AllOf = [new OpenApiSchema { Reference = schema.Reference }];
schema.Reference = null;
}

Expand All @@ -132,8 +133,7 @@ private OpenApiSchema GenerateSchemaForParameter(

if (defaultValue != null)
{
var defaultAsJson = dataContract.JsonConverter(defaultValue);
schema.Default = OpenApiAnyFactory.CreateFromJson(defaultAsJson);
schema.Default = GenerateDefaultValue(dataContract, modelType, defaultValue);
}

schema.ApplyValidationAttributes(customAttributes);
Expand Down Expand Up @@ -200,15 +200,15 @@ private OpenApiSchema GeneratePolymorphicSchema(
};
}

private static readonly Type[] BinaryStringTypes = new[]
{
private static readonly Type[] BinaryStringTypes =
[
typeof(IFormFile),
typeof(FileResult),
typeof(System.IO.Stream),
#if NETCOREAPP3_0_OR_GREATER
typeof(System.IO.Pipelines.PipeReader),
#endif
};
];

private OpenApiSchema GenerateConcreteSchema(DataContract dataContract, SchemaRepository schemaRepository)
{
Expand Down Expand Up @@ -277,7 +277,7 @@ private bool TryGetCustomTypeMapping(Type modelType, out Func<OpenApiSchema> sch
|| (modelType.IsConstructedGenericType && _generatorOptions.CustomTypeMappings.TryGetValue(modelType.GetGenericTypeDefinition(), out schemaFactory));
}

private OpenApiSchema CreatePrimitiveSchema(DataContract dataContract)
private static OpenApiSchema CreatePrimitiveSchema(DataContract dataContract)
{
var schema = new OpenApiSchema
{
Expand All @@ -292,7 +292,7 @@ private OpenApiSchema CreatePrimitiveSchema(DataContract dataContract)
schema.Enum = dataContract.EnumValues
.Select(value => JsonSerializer.Serialize(value))
.Distinct()
.Select(valueAsJson => OpenApiAnyFactory.CreateFromJson(valueAsJson))
.Select(OpenApiAnyFactory.CreateFromJson)
.ToList();

return schema;
Expand Down Expand Up @@ -402,7 +402,7 @@ private OpenApiSchema CreateObjectSchema(DataContract dataContract, SchemaReposi

foreach (var dataProperty in applicableDataProperties)
{
var customAttributes = dataProperty.MemberInfo?.GetInlineAndMetadataAttributes() ?? Enumerable.Empty<object>();
var customAttributes = dataProperty.MemberInfo?.GetInlineAndMetadataAttributes() ?? [];

if (_generatorOptions.IgnoreObsoleteProperties && customAttributes.OfType<ObsoleteAttribute>().Any())
continue;
Expand Down Expand Up @@ -519,5 +519,24 @@ private void ApplyFilters(
filter.Apply(schema, filterContext);
}
}

private IOpenApiAny GenerateDefaultValue(
DataContract dataContract,
Type modelType,
object defaultValue)
{
// If the types do not match (e.g. a default which is an integer is specified for a double),
// attempt to coerce the default value to the correct type so that it can be serialized correctly.
// See https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/2885 and
// https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/2904.
var defaultValueType = defaultValue?.GetType();
if (defaultValueType != null && defaultValueType != modelType)
{
dataContract = GetDataContractFor(defaultValueType);
}

var defaultAsJson = dataContract.JsonConverter(defaultValue);
return OpenApiAnyFactory.CreateFromJson(defaultAsJson);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,21 @@ public async Task SwaggerMiddleware_CanBeConfiguredMultipleTimes(
public async Task SwaggerEndpoint_ReturnsValidSwaggerJson_For_WebApi(
string swaggerRequestUri)
{
await SwaggerEndpointReturnsValidSwaggerJson<Program>(swaggerRequestUri);
await SwaggerEndpointReturnsValidSwaggerJson<WebApi.Program>(swaggerRequestUri);
}

[Theory]
[InlineData("/swagger/v1/swagger.json")]
public async Task SwaggerEndpoint_ReturnsValidSwaggerJson_For_Mvc(
string swaggerRequestUri)
{
await SwaggerEndpointReturnsValidSwaggerJson<MvcWithNullable.Program>(swaggerRequestUri);
}

[Fact]
public async Task TypesAreRenderedCorrectly()
{
using var application = new TestApplication<Program>();
using var application = new TestApplication<WebApi.Program>();
using var client = application.CreateDefaultClient();

using var swaggerResponse = await client.GetFromJsonAsync<JsonDocument>("/swagger/v1/swagger.json");
Expand Down Expand Up @@ -153,7 +161,7 @@ public async Task TypesAreRenderedCorrectly()
]);
}

private async Task SwaggerEndpointReturnsValidSwaggerJson<TEntryPoint>(string swaggerRequestUri)
private static async Task SwaggerEndpointReturnsValidSwaggerJson<TEntryPoint>(string swaggerRequestUri)
where TEntryPoint : class
{
using var application = new TestApplication<TEntryPoint>();
Expand All @@ -163,11 +171,11 @@ private async Task SwaggerEndpointReturnsValidSwaggerJson<TEntryPoint>(string sw
}
#endif

private async Task AssertValidSwaggerJson(HttpClient client, string swaggerRequestUri)
private static async Task AssertValidSwaggerJson(HttpClient client, string swaggerRequestUri)
{
using var swaggerResponse = await client.GetAsync(swaggerRequestUri);

swaggerResponse.EnsureSuccessStatusCode();
Assert.True(swaggerResponse.IsSuccessStatusCode, await swaggerResponse.Content.ReadAsStringAsync());
using var contentStream = await swaggerResponse.Content.ReadAsStreamAsync();
new OpenApiStreamReader().Read(contentStream, out OpenApiDiagnostic diagnostic);
Assert.Empty(diagnostic.Errors);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<AssemblyOriginatorKeyFile>$(MSBuildThisFileDirectory)..\..\src\Swashbuckle.AspNetCore.Swagger\Swashbuckle.AspNetCore.Swagger.snk</AssemblyOriginatorKeyFile>
Expand All @@ -23,10 +23,7 @@
</ItemGroup>

<ItemGroup Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net8.0'))">
<ProjectReference Include="..\WebSites\WebApi\WebApi.csproj" />
</ItemGroup>

<ItemGroup Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net8.0'))">
<ProjectReference Include="..\WebSites\MvcWithNullable\MvcWithNullable.csproj" />
<ProjectReference Include="..\WebSites\WebApi\WebApi.csproj" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ public void GenerateSchema_SetsNullableFlag_IfPropertyIsReferenceOrNullableType(
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.LongWithDefault), "9223372036854775807")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.FloatWithDefault), "3.4028235E+38")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.DoubleWithDefault), "1.7976931348623157E+308")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.DoubleWithDefaultOfDifferentType), "1")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.StringWithDefault), "\"foobar\"")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.IntArrayWithDefault), "[\n 1,\n 2,\n 3\n]")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.StringArrayWithDefault), "[\n \"foo\",\n \"bar\"\n]")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ public void GenerateSchema_SetsNullableFlag_IfPropertyIsReferenceOrNullableType(
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.LongWithDefault), "9223372036854775807")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.FloatWithDefault), "3.4028235E+38")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.DoubleWithDefault), "1.7976931348623157E+308")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.DoubleWithDefaultOfDifferentType), "1")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.StringWithDefault), "\"foobar\"")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.IntArrayWithDefault), "[\n 1,\n 2,\n 3\n]")]
[InlineData(typeof(TypeWithDefaultAttributes), nameof(TypeWithDefaultAttributes.StringArrayWithDefault), "[\n \"foo\",\n \"bar\"\n]")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ public class TypeWithDefaultAttributes
[DefaultValue(double.MaxValue)]
public double DoubleWithDefault { get; set; }

[DefaultValue(1)] // Repro for https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/2885
public double DoubleWithDefaultOfDifferentType { get; set; }

[DefaultValue("foobar")]
public string StringWithDefault { get; set; }

Expand Down
20 changes: 20 additions & 0 deletions test/WebSites/MvcWithNullable/MvcWithNullable.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<ImplicitUsings>enable</ImplicitUsings>
<NoWarn>$(NoWarn);CA1050</NoWarn>
<Nullable>enable</Nullable>
<RootNamespace>WebApi</RootNamespace>
<TargetFramework>net8.0</TargetFramework>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\..\..\src\Swashbuckle.AspNetCore.SwaggerGen\Swashbuckle.AspNetCore.SwaggerGen.csproj" />
<ProjectReference Include="..\..\..\src\Swashbuckle.AspNetCore.SwaggerUI\Swashbuckle.AspNetCore.SwaggerUI.csproj" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.OpenApi" />
</ItemGroup>

</Project>
40 changes: 40 additions & 0 deletions test/WebSites/MvcWithNullable/Program.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddControllers();

builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen(options =>
{
options.UseAllOfToExtendReferenceSchemas();
});

var app = builder.Build();
app.UseHttpsRedirection();

if (app.Environment.IsDevelopment())
{
_ = app.UseSwagger();
_ = app.UseSwaggerUI();
}

app.MapControllers();

app.Run();

[ApiController]
[Route("api/[controller]")]
public class EnumController : ControllerBase
{
[HttpGet]
public IActionResult Get(LogLevel? logLevel = LogLevel.Error) => Ok(new { logLevel });
}

namespace MvcWithNullable
{
public partial class Program
{
// Expose the Program class for use with WebApplicationFactory<T>
}
}
Loading

0 comments on commit fe87e6d

Please sign in to comment.