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.

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

Path grouping strategy #3152

Merged
merged 8 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<UseArtifactsOutput>true</UseArtifactsOutput>
<VersionPrefix>7.1.1</VersionPrefix>
<VersionPrefix>7.2.0</VersionPrefix>
<WarnOnPackingNonPackableProject>false</WarnOnPackingNonPackableProject>
</PropertyGroup>
<PropertyGroup Condition=" '$(GITHUB_ACTIONS)' != '' ">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public void DeepCopy(SwaggerGeneratorOptions source, SwaggerGeneratorOptions tar
target.RequestBodyFilters = new List<IRequestBodyFilter>(source.RequestBodyFilters);
target.RequestBodyAsyncFilters = new List<IRequestBodyAsyncFilter>(source.RequestBodyAsyncFilters);
target.SecuritySchemesSelector = source.SecuritySchemesSelector;
target.PathGroupSelector = source.PathGroupSelector;
}

private TFilter GetOrCreateFilter<TFilter>(FilterDescriptor filterDescriptor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ static Microsoft.Extensions.DependencyInjection.SwaggerGenOptionsExtensions.Docu
static Microsoft.Extensions.DependencyInjection.SwaggerGenOptionsExtensions.OperationAsyncFilter<TFilter>(this Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenOptions swaggerGenOptions, params object[] arguments) -> void
static Microsoft.Extensions.DependencyInjection.SwaggerGenOptionsExtensions.ParameterAsyncFilter<TFilter>(this Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenOptions swaggerGenOptions, params object[] arguments) -> void
static Microsoft.Extensions.DependencyInjection.SwaggerGenOptionsExtensions.RequestBodyAsyncFilter<TFilter>(this Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenOptions swaggerGenOptions, params object[] arguments) -> void
Swashbuckle.AspNetCore.SwaggerGen.SwaggerGeneratorOptions.PathGroupSelector.get -> System.Func<Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescription, string>
Swashbuckle.AspNetCore.SwaggerGen.SwaggerGeneratorOptions.PathGroupSelector.set -> void
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using System;
using System.Linq;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Microsoft.AspNetCore.Mvc.ApiExplorer;
using Microsoft.AspNetCore.Mvc.Controllers;
Expand Down Expand Up @@ -41,7 +41,7 @@ public static IEnumerable<object> CustomAttributes(this ApiDescription apiDescri
.Union(methodInfo.DeclaringType.GetCustomAttributes(true));
}

return Enumerable.Empty<object>();
return [];
}

[Obsolete("Use TryGetMethodInfo() and CustomAttributes() instead")]
Expand All @@ -57,7 +57,7 @@ public static void GetAdditionalMetadata(this ApiDescription apiDescription,
return;
}

customAttributes = Enumerable.Empty<object>();
customAttributes = [];
}

internal static string RelativePathSansParameterConstraints(this ApiDescription apiDescription)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ public static PropertyInfo PropertyInfo(this ApiParameterDescription apiParamete
{
var modelMetadata = apiParameter.ModelMetadata;

return (modelMetadata?.ContainerType != null)
? modelMetadata.ContainerType.GetProperty(modelMetadata.PropertyName)
: null;
return modelMetadata?.ContainerType?.GetProperty(modelMetadata.PropertyName);
}

public static IEnumerable<object> CustomAttributes(this ApiParameterDescription apiParameter)
Expand Down Expand Up @@ -103,12 +101,12 @@ internal static void GetAdditionalMetadata(

internal static bool IsFromPath(this ApiParameterDescription apiParameter)
{
return (apiParameter.Source == BindingSource.Path);
return apiParameter.Source == BindingSource.Path;
}

internal static bool IsFromBody(this ApiParameterDescription apiParameter)
{
return (apiParameter.Source == BindingSource.Body);
return apiParameter.Source == BindingSource.Body;
}

internal static bool IsFromForm(this ApiParameterDescription apiParameter)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public async Task<OpenApiDocument> GetSwaggerAsync(
swaggerDoc.Paths = await GeneratePathsAsync(filterContext.ApiDescriptions, filterContext.SchemaRepository);
swaggerDoc.Components.SecuritySchemes = await GetSecuritySchemesAsync();

// NOTE: Filter processing moved here so they may effect generated security schemes
// NOTE: Filter processing moved here so they may affect generated security schemes
foreach (var filter in _options.DocumentAsyncFilters)
{
await filter.ApplyAsync(swaggerDoc, filterContext, CancellationToken.None);
Expand All @@ -81,7 +81,7 @@ public OpenApiDocument GetSwagger(string documentName, string host = null, strin
swaggerDoc.Paths = GeneratePaths(filterContext.ApiDescriptions, filterContext.SchemaRepository);
swaggerDoc.Components.SecuritySchemes = GetSecuritySchemesAsync().Result;

// NOTE: Filter processing moved here so they may effect generated security schemes
// NOTE: Filter processing moved here so they may affect generated security schemes
foreach (var filter in _options.DocumentFilters)
{
filter.Apply(swaggerDoc, filterContext);
Expand Down Expand Up @@ -199,7 +199,7 @@ private async Task<OpenApiPaths> GeneratePathsAsync(
{
var apiDescriptionsByPath = apiDescriptions
.OrderBy(_options.SortKeySelector)
.GroupBy(apiDesc => apiDesc.RelativePathSansParameterConstraints());
.GroupBy(_options.PathGroupSelector);

var paths = new OpenApiPaths();
foreach (var group in apiDescriptionsByPath)
Expand Down Expand Up @@ -284,7 +284,7 @@ private async Task<Dictionary<OperationType, OpenApiOperation>> GenerateOperatio
{
throw new SwaggerGeneratorException(string.Format(
"Conflicting method/path combination \"{0} {1}\" for actions - {2}. " +
"Actions require a unique method/path combination for Swagger/OpenAPI 3.0. Use ConflictingActionsResolver as a workaround",
"Actions require a unique method/path combination for Swagger/OpenAPI 2.0 and 3.0. Use ConflictingActionsResolver as a workaround or provide your own implementation of PathGroupSelector.",
httpMethod,
group.First().RelativePath,
string.Join(",", group.Select(apiDesc => apiDesc.ActionDescriptor.DisplayName))));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public SwaggerGeneratorOptions()
OperationIdSelector = DefaultOperationIdSelector;
TagsSelector = DefaultTagsSelector;
SortKeySelector = DefaultSortKeySelector;
PathGroupSelector = DefaultPathGroupSelector;
SecuritySchemesSelector = null;
SchemaComparer = StringComparer.Ordinal;
Servers = new List<OpenApiServer>();
Expand Down Expand Up @@ -49,6 +50,8 @@ public SwaggerGeneratorOptions()

public Func<ApiDescription, string> SortKeySelector { get; set; }

public Func<ApiDescription, string> PathGroupSelector { get; set; }

public bool InferSecuritySchemes { get; set; }

public Func<IEnumerable<AuthenticationScheme>, IDictionary<string, OpenApiSecurityScheme>> SecuritySchemesSelector { get; set; }
Expand Down Expand Up @@ -120,5 +123,10 @@ private string DefaultSortKeySelector(ApiDescription apiDescription)
{
return TagsSelector(apiDescription).First();
}

private static string DefaultPathGroupSelector(ApiDescription apiDescription)
{
return apiDescription.RelativePathSansParameterConstraints();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public static void DeepCopy_Copies_All_Properties()

// If this assertion fails, it means that a new property has been added
// to SwaggerGeneratorOptions and ConfigureSwaggerGeneratorOptions.DeepCopy() needs to be updated
Saibamen marked this conversation as resolved.
Show resolved Hide resolved
Assert.Equal(22, publicProperties.Length);
Assert.Equal(23, publicProperties.Length);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ public void ActionWithIntParameterWithDefaultValue(int param = 1)
public void ActionWithIntParameterWithDefaultValueAttribute([DefaultValue(3)] int param)
{ }

public void ActionWithIntFromQueryParameter([FromQuery] int param)
{ }

public void ActionWithIntParameterWithRequiredAttribute([Required] int param)
{ }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,37 @@ public void GetSwagger_ThrowsSwaggerGeneratorException_IfActionsHaveConflictingH
"Conflicting method/path combination \"POST resource\" for actions - " +
"Swashbuckle.AspNetCore.SwaggerGen.Test.FakeController.ActionWithNoParameters (Swashbuckle.AspNetCore.SwaggerGen.Test)," +
"Swashbuckle.AspNetCore.SwaggerGen.Test.FakeController.ActionWithNoParameters (Swashbuckle.AspNetCore.SwaggerGen.Test). " +
"Actions require a unique method/path combination for Swagger/OpenAPI 3.0. Use ConflictingActionsResolver as a workaround",
"Actions require a unique method/path combination for Swagger/OpenAPI 2.0 and 3.0. Use ConflictingActionsResolver as a workaround or provide your own implementation of PathGroupSelector.",
exception.Message);
}

[Fact]
public void GetSwagger_ThrowsSwaggerGeneratorException_IfActionsHaveConflictingHttpMethodAndPathWithDifferentParameters()
{
var subject = Subject(
apiDescriptions:
[
ApiDescriptionFactory.Create<FakeController>(
c => nameof(c.ActionWithNoParameters), groupName: "v1", httpMethod: "GET", relativePath: "resource"),

ApiDescriptionFactory.Create<FakeController>(
c => nameof(c.ActionWithIntFromQueryParameter), groupName: "v1", httpMethod: "GET", relativePath: "resource", new ApiParameterDescription[]
{
new()
{
Name = "id",
Source = BindingSource.Query,
}
}),
]
);

var exception = Assert.Throws<SwaggerGeneratorException>(() => subject.GetSwagger("v1"));
Assert.Equal(
"Conflicting method/path combination \"GET resource\" for actions - " +
"Swashbuckle.AspNetCore.SwaggerGen.Test.FakeController.ActionWithNoParameters (Swashbuckle.AspNetCore.SwaggerGen.Test)," +
"Swashbuckle.AspNetCore.SwaggerGen.Test.FakeController.ActionWithIntFromQueryParameter (Swashbuckle.AspNetCore.SwaggerGen.Test). " +
"Actions require a unique method/path combination for Swagger/OpenAPI 2.0 and 3.0. Use ConflictingActionsResolver as a workaround or provide your own implementation of PathGroupSelector.",
exception.Message);
}

Expand Down