Skip to content

Commit

Permalink
Better messages for overload and missing binding exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
@domaindrivendev
domaindrivendev committed Nov 1, 2016
1 parent ff7e05a commit 9bf2cb4
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 73 deletions.
1 change: 1 addition & 0 deletions src/Swashbuckle.Swagger/Model/SwaggerDocument.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ public class BodyParameter : IParameter
{
public BodyParameter()
{
In = "body";
Extensions = new Dictionary<string, object>();
}

Expand Down
57 changes: 34 additions & 23 deletions src/Swashbuckle.SwaggerGen/Generator/ApiDescriptionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,26 @@ namespace Swashbuckle.SwaggerGen.Generator
{
public static class ApiDescriptionExtensions
{
internal static string RelativePathSansQueryString(this ApiDescription apiDescription)
public static IEnumerable<object> ControllerAttributes(this ApiDescription apiDescription)
{
return apiDescription.RelativePath.Split('?').First();
var controllerActionDescriptor = apiDescription.ControllerActionDescriptor();
return (controllerActionDescriptor == null)
? Enumerable.Empty<object>()
: controllerActionDescriptor.ControllerTypeInfo.GetCustomAttributes(false);
}

public static IEnumerable<object> ActionAttributes(this ApiDescription apiDescription)
{
var controllerActionDescriptor = apiDescription.ControllerActionDescriptor();
return (controllerActionDescriptor == null)
? Enumerable.Empty<object>()
: controllerActionDescriptor.MethodInfo.GetCustomAttributes(false);
}

internal static string ControllerName(this ApiDescription apiDescription)
{
var controllerActionDescriptor = apiDescription.ControllerActionDescriptor();
return (controllerActionDescriptor == null) ? null : controllerActionDescriptor.ControllerName;
}

internal static string FriendlyId(this ApiDescription apiDescription)
Expand All @@ -24,7 +41,6 @@ internal static string FriendlyId(this ApiDescription apiDescription)
foreach (var part in parts)
{
var trimmed = part.Trim('{', '}');

builder.AppendFormat("{0}{1}",
(part.StartsWith("{") ? "By" : string.Empty),
trimmed.ToTitleCase()
Expand All @@ -34,6 +50,11 @@ internal static string FriendlyId(this ApiDescription apiDescription)
return builder.ToString();
}

internal static string RelativePathSansQueryString(this ApiDescription apiDescription)
{
return apiDescription.RelativePath.Split('?').First();
}

internal static IEnumerable<string> SupportedRequestMediaTypes(this ApiDescription apiDescription)
{
return apiDescription.SupportedRequestFormats
Expand All @@ -48,31 +69,21 @@ internal static IEnumerable<string> SupportedResponseMediaTypes(this ApiDescript
.Distinct();
}

public static string ControllerName(this ApiDescription apiDescription)
{
var actionDescriptor = apiDescription.ActionDescriptor as ControllerActionDescriptor;
return (actionDescriptor == null) ? null : actionDescriptor.ControllerName;
}

public static IEnumerable<object> ControllerAttributes(this ApiDescription apiDescription)
internal static bool IsObsolete(this ApiDescription apiDescription)
{
var actionDescriptor = apiDescription.ActionDescriptor as ControllerActionDescriptor;
return (actionDescriptor == null)
? Enumerable.Empty<object>()
: actionDescriptor.ControllerTypeInfo.GetCustomAttributes(false);
return apiDescription.ActionAttributes().OfType<ObsoleteAttribute>().Any();
}

public static IEnumerable<object> ActionAttributes(this ApiDescription apiDescription)
private static ControllerActionDescriptor ControllerActionDescriptor(this ApiDescription apiDescription)
{
var actionDescriptor = apiDescription.ActionDescriptor as ControllerActionDescriptor;
return (actionDescriptor == null)
? Enumerable.Empty<object>()
: actionDescriptor.MethodInfo.GetCustomAttributes(false);
var controllerActionDescriptor = apiDescription.GetProperty<ControllerActionDescriptor>();
if (controllerActionDescriptor == null)
{
controllerActionDescriptor = apiDescription.ActionDescriptor as ControllerActionDescriptor;
apiDescription.SetProperty(controllerActionDescriptor);
}
return controllerActionDescriptor;
}

internal static bool IsObsolete(this ApiDescription apiDescription)
{
return apiDescription.ActionAttributes().OfType<ObsoleteAttribute>().Any();
}
}
}
91 changes: 50 additions & 41 deletions src/Swashbuckle.SwaggerGen/Generator/SwaggerGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class SwaggerGenerator : ISwaggerProvider
var apiDescriptions = _apiDescriptionsProvider.ApiDescriptionGroups.Items
.SelectMany(group => group.Items)
.Where(apiDesc => documentDescriptor.IncludeActionPredicate(apiDesc))
.Where(apiDesc => !(_settings.IgnoreObsoleteActions && apiDesc.IsObsolete()))
.Where(apiDesc => !_settings.IgnoreObsoleteActions || !apiDesc.IsObsolete())
.OrderBy(_settings.TagSelector, _settings.TagComparer);

var paths = apiDescriptions
Expand Down Expand Up @@ -83,13 +83,17 @@ private PathItem CreatePathItem(IEnumerable<ApiDescription> apiDescriptions, ISc

if (httpMethod == null)
throw new NotSupportedException(string.Format(
"Unbounded HTTP verbs for path '{0}'. Are you missing an HttpMethodAttribute?",
group.First().RelativePathSansQueryString()));
"Ambiguous HTTP method for action - {0}. " +
"Actions require an explicit HttpMethod binding for Swagger",
group.First().ActionDescriptor.DisplayName));

if (group.Count() > 1)
throw new NotSupportedException(string.Format(
"Multiple operations with path '{0}' and method '{1}'. Are you overloading action methods?",
group.First().RelativePathSansQueryString(), httpMethod));
"HTTP method \"{0}\" & path \"{1}\" overloaded by actions - {2}. " +
"Actions require unique method/path combination for Swagger",
httpMethod,
group.First().RelativePathSansQueryString(),
string.Join(",", group.Select(apiDesc => apiDesc.ActionDescriptor.DisplayName))));

var apiDescription = group.Single();

Expand Down Expand Up @@ -126,7 +130,7 @@ private Operation CreateOperation(ApiDescription apiDescription, ISchemaRegistry
{
var parameters = apiDescription.ParameterDescriptions
.Where(paramDesc => paramDesc.Source.IsFromRequest)
.Select(paramDesc => CreateParameter(paramDesc, schemaRegistry))
.Select(paramDesc => CreateParameter(apiDescription, paramDesc, schemaRegistry))
.ToList();

var responses = apiDescription.SupportedResponseTypes
Expand Down Expand Up @@ -156,53 +160,58 @@ private Operation CreateOperation(ApiDescription apiDescription, ISchemaRegistry
return operation;
}

private string GetParameterLocation(ApiParameterDescription paramDesc)
private IParameter CreateParameter(
ApiDescription apiDescription,
ApiParameterDescription paramDescription,
ISchemaRegistry schemaRegistry)
{
if (paramDesc.Source == BindingSource.Form)
return "formData";
else if (paramDesc.Source == BindingSource.Body)
return "body";
else if (paramDesc.Source == BindingSource.Header)
return "header";
else if (paramDesc.Source == BindingSource.Path)
return "path";
else
return "query";
}
var location = GetParameterLocation(apiDescription, paramDescription);
var schema = (paramDescription.Type == null) ? null : schemaRegistry.GetOrRegister(paramDescription.Type);

private IParameter CreateParameter(ApiParameterDescription paramDesc, ISchemaRegistry schemaRegistry)
{
var source = paramDesc.Source.Id.ToLower();
var schema = (paramDesc.Type == null) ? null : schemaRegistry.GetOrRegister(paramDesc.Type);

if (source == "body")
if (location == "body")
{
return new BodyParameter
{
Name = paramDesc.Name,
In = source,
Name = paramDescription.Name,
Schema = schema
};
}
else

var nonBodyParam = new NonBodyParameter
{
var nonBodyParam = new NonBodyParameter
{
Name = paramDesc.Name,
In = GetParameterLocation(paramDesc),
Required = (source == "path")
};
Name = paramDescription.Name,
In = location,
Required = (location == "path")
};

if (schema == null)
nonBodyParam.Type = "string";
else
nonBodyParam.PopulateFrom(schema);
if (schema == null)
nonBodyParam.Type = "string";
else
nonBodyParam.PopulateFrom(schema);

if (nonBodyParam.Type == "array")
nonBodyParam.CollectionFormat = "multi";
if (nonBodyParam.Type == "array")
nonBodyParam.CollectionFormat = "multi";

return nonBodyParam;
}
return nonBodyParam;
}

private string GetParameterLocation(ApiDescription apiDescription, ApiParameterDescription paramDescription)
{
if (paramDescription.Source == BindingSource.Form)
return "formData";
else if (paramDescription.Source == BindingSource.Body)
return "body";
else if (paramDescription.Source == BindingSource.Header)
return "header";
else if (paramDescription.Source == BindingSource.Path)
return "path";
else if (paramDescription.Source == BindingSource.Query)
return "query";

throw new NotSupportedException(string.Format(
"Ambiguous location (path, query etc.) for one or more parameters in action - {0}. " +
"Action parameters require an explicit \"From\" binding for Swagger",
apiDescription.ActionDescriptor.DisplayName));
}

private Response CreateResponse(ApiResponseType apiResponseType, ISchemaRegistry schemaRegistry)
Expand Down
32 changes: 25 additions & 7 deletions test/Swashbuckle.SwaggerGen.Test/Generator/SwaggerGeneratorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public void GetSwagger_SetsParametersToNull_ForParameterlessActions()
[InlineData("collection", nameof(FakeActions.AcceptsStringFromQuery), "query")]
[InlineData("collection", nameof(FakeActions.AcceptsStringFromHeader), "header")]
[InlineData("collection", nameof(FakeActions.AcceptsStringFromForm), "formData")]
[InlineData("collection", nameof(FakeActions.AcceptStringFromDefault), "query")]
[InlineData("collection", nameof(FakeActions.AcceptsStringFromQuery), "query")]
public void GetSwagger_GeneratesNonBodyParameters_ForNonBodyParams(
string routeTemplate,
string actionFixtureName,
Expand Down Expand Up @@ -496,7 +496,20 @@ public void GetSwagger_HandlesUnboundRouteParams()
}

[Fact]
public void GetSwagger_ThrowsInformativeException_IfActionsAreOverloaded()
public void GetSwagger_ThrowsInformativeException_IfHttpMethodAttributeNotPresent()
{
var subject = Subject(setupApis: apis => apis
.Add(null, "collection", nameof(FakeActions.AcceptsNothing)));

var exception = Assert.Throws<NotSupportedException>(() => subject.GetSwagger("v1"));
Assert.Equal(
"Ambiguous HTTP method for action - Swashbuckle.SwaggerGen.TestFixtures.FakeControllers+NotAnnotated.AcceptsNothing (Swashbuckle.SwaggerGen.Test). " +
"Actions require an explicit HttpMethod binding for Swagger",
exception.Message);
}

[Fact]
public void GetSwagger_ThrowsInformativeException_IfHttpMethodAndPathAreOverloaded()
{
var subject = Subject(setupApis: apis => apis
.Add("GET", "collection", nameof(FakeActions.AcceptsNothing))
Expand All @@ -505,22 +518,27 @@ public void GetSwagger_ThrowsInformativeException_IfActionsAreOverloaded()

var exception = Assert.Throws<NotSupportedException>(() => subject.GetSwagger("v1"));
Assert.Equal(
"Multiple operations with path 'collection' and method 'GET'. Are you overloading action methods?",
"HTTP method \"GET\" & path \"collection\" overloaded by actions - " +
"Swashbuckle.SwaggerGen.TestFixtures.FakeControllers+NotAnnotated.AcceptsNothing (Swashbuckle.SwaggerGen.Test)," +
"Swashbuckle.SwaggerGen.TestFixtures.FakeControllers+NotAnnotated.AcceptsStringFromQuery (Swashbuckle.SwaggerGen.Test). " +
"Actions require unique method/path combination for Swagger",
exception.Message);
}

[Fact]
public void GetSwagger_ThrowsInformativeException_IfHttpMethodAttributeNotPresent()
public void GetSwagger_ThrowsInformativeException_IfParameterBindingNotPresent()
{
var subject = Subject(setupApis: apis => apis
.Add(null, "collection", nameof(FakeActions.AcceptsNothing)));
.Add("GET", "collection", nameof(FakeActions.AcceptsUnboundParameter)));

var exception = Assert.Throws<NotSupportedException>(() => subject.GetSwagger("v1"));
Assert.Equal(
"Unbounded HTTP verbs for path 'collection'. Are you missing an HttpMethodAttribute?",
"Ambiguous location (path, query etc.) for one or more parameters in action - " +
"Swashbuckle.SwaggerGen.TestFixtures.FakeControllers+NotAnnotated.AcceptsUnboundParameter (Swashbuckle.SwaggerGen.Test). " +
"Action parameters require an explicit \"From\" binding for Swagger",
exception.Message);
}

private SwaggerGenerator Subject(
Action<FakeApiDescriptionGroupCollectionProvider> setupApis = null,
Action<SwaggerGeneratorSettings> configure = null)
Expand Down
4 changes: 2 additions & 2 deletions test/Swashbuckle.SwaggerGen.Test/TestFixtures/Fakes/FakeActions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ public void AcceptsStringFromHeader([FromHeader]string param)
public void AcceptsStringFromForm([FromForm]string param)
{ }

public void AcceptStringFromDefault(string param)
public void AcceptsComplexTypeFromBody([FromBody]ComplexType param)
{ }

public void AcceptsComplexTypeFromBody([FromBody]ComplexType param)
public void AcceptsUnboundParameter(string param)
{ }

[Obsolete]
Expand Down

0 comments on commit 9bf2cb4

Please sign in to comment.