Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix generation when [DefaultValue]'s type differs #2895

Merged
merged 4 commits into from
May 21, 2024
Merged

Fix generation when [DefaultValue]'s type differs #2895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb030a7
Fix generation when [DefaultValue]'s type differs
martincostello May 15, 2024
5e22cd0
Fix generation for nullable enums
martincostello May 16, 2024
b10358c
Unify implementations
martincostello May 19, 2024
dd47198
Fix null check
martincostello May 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Swashbuckle.AspNetCore.sln
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/Swashbuckle.AspNetCore.Newtonsoft/SchemaGenerator/NewtonsoftDataContractResolver.cs
View file
Open in desktop
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)
domaindrivendev marked this conversation as resolved.
Show resolved Hide resolved
private List<DataProperty> GetDataPropertiesFor(JsonObjectContract jsonObjectContract, out Type extensionDataType)
{
var dataProperties = new List<DataProperty>();

Expand Down
21 changes: 14 additions & 7 deletions src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/JsonSerializerDataContractResolver.cs
View file
Open in desktop
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)
domaindrivendev marked this conversation as resolved.
Show resolved Hide resolved
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) &&
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martincostello is this another performance-related suggestion? Personally, I prefer the readability of Any()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely yes. See CA1860

(Note: I also don't like that rule. I've disabled it in my projects since I don't write high performance code.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is a new analyser rule related to performance as well.

!(property.HasAttribute<JsonIgnoreAttribute>() && isIgnoredViaNet5Attribute) &&
!(property.HasAttribute<SwaggerIgnoreAttribute>()) &&
!(_serializerOptions.IgnoreReadOnlyProperties && !property.IsPubliclyWritable());
Expand Down
47 changes: 33 additions & 14 deletions src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/SchemaGenerator.cs
View file
Open in desktop
domaindrivendev marked this conversation as resolved.
Show resolved Hide resolved
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)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

.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);
}
}
}
18 changes: 13 additions & 5 deletions test/Swashbuckle.AspNetCore.IntegrationTests/SwaggerIntegrationTests.cs
View file
Open in desktop
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
7 changes: 2 additions & 5 deletions test/Swashbuckle.AspNetCore.IntegrationTests/Swashbuckle.AspNetCore.IntegrationTests.csproj
View file
Open in desktop
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
1 change: 1 addition & 0 deletions .../Swashbuckle.AspNetCore.Newtonsoft.Test/SchemaGenerator/NewtonsoftSchemaGeneratorTests.cs
View file
Open in desktop
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
1 change: 1 addition & 0 deletions ...shbuckle.AspNetCore.SwaggerGen.Test/SchemaGenerator/JsonSerializerSchemaGeneratorTests.cs
View file
Open in desktop
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
3 changes: 3 additions & 0 deletions test/Swashbuckle.AspNetCore.TestSupport/Fixtures/TypeWithDefaultAttributes.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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