diff --git a/src/Microsoft.AspNet.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs b/src/Microsoft.AspNet.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs index a6cd7a7ea2..f0ee860ef9 100644 --- a/src/Microsoft.AspNet.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs +++ b/src/Microsoft.AspNet.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs @@ -325,8 +325,8 @@ namespace Microsoft.AspNet.Mvc.ApiExplorer if (responseFormatMetadataProvider != null) { var supportedTypes = responseFormatMetadataProvider.GetSupportedContentTypes( - declaredType, - runtimeType, + declaredType, + runtimeType, contentType); if (supportedTypes != null) @@ -496,34 +496,17 @@ namespace Microsoft.AspNet.Mvc.ApiExplorer // // The default is ModelBinding (aka all default value providers) var source = BindingSource.ModelBinding; - if (!Visit(context, source, containerName: string.Empty)) - { - // If we get here, then it means we didn't find a match for any of the model. This means that it's - // likely 'model-bound' in the traditional MVC sense (formdata + query string + route data) and - // doesn't use any IBinderMetadata. - // - // Add a single 'default' parameter description for the model. - Context.Results.Add(CreateResult(context, source, containerName: string.Empty)); - } + Visit(context, source, containerName: string.Empty); } /// /// Visits a node in a model, and attempts to create for any - /// model properties where we can definitely compute an answer. + /// model properties that are leaf nodes (see comments). /// /// The metadata for the model. /// The from the ambient context. /// The current name prefix (to prepend to property names). - /// - /// true if the set of objects were created for the model. - /// false if no objects were created for the model. - /// - /// - /// Its the reponsibility of this method to create a parameter description for ALL of the current model - /// or NONE of it. If a parameter description is created for ANY sub-properties of the model, then a parameter - /// description will be created for ALL of them. - /// - private bool Visit( + private void Visit( ApiParameterDescriptionContext bindingContext, BindingSource ambientSource, string containerName) @@ -534,8 +517,7 @@ namespace Microsoft.AspNet.Mvc.ApiExplorer // We have a definite answer for this model. This is a greedy source like // [FromBody] so there's no need to consider properties. Context.Results.Add(CreateResult(bindingContext, source, containerName)); - - return true; + return; } var modelMetadata = bindingContext.ModelMetadata; @@ -554,25 +536,11 @@ namespace Microsoft.AspNet.Mvc.ApiExplorer !modelMetadata.IsComplexType || !modelMetadata.Properties.Any()) { - if (source == null || source == ambientSource) - { - // If it's a leaf node, and we have no new source then we don't know how to bind this. - // Return without creating any parameters, so that this can be included in the parent model. - return false; - } - else - { - // We found a new source, and this model has no properties. This is probabaly - // a simple type with an attribute like [FromQuery]. - Context.Results.Add(CreateResult(bindingContext, source, containerName)); - return true; - } + Context.Results.Add(CreateResult(bindingContext, source ?? ambientSource, containerName)); + return; } // This will come from composite model binding - so investigate what's going on with each property. - // - // Basically once we find something that we know how to bind, we want to treat all properties at that - // level (and higher levels) as separate parameters. // // Ex: // @@ -592,8 +560,6 @@ namespace Microsoft.AspNet.Mvc.ApiExplorer // Order - source: Body // - var unboundProperties = new HashSet(); - // We don't want to append the **parameter** name when building a model name. var newContainerName = containerName; if (modelMetadata.ContainerType != null) @@ -609,45 +575,17 @@ namespace Microsoft.AspNet.Mvc.ApiExplorer propertyMetadata, bindingInfo: null, propertyName: null); + if (Visited.Add(key)) { - if (!Visit(propertyContext, source ?? ambientSource, newContainerName)) - { - unboundProperties.Add(propertyContext); - } + Visit(propertyContext, source ?? ambientSource, newContainerName); } else { - unboundProperties.Add(propertyContext); + // This is cycle, so just add a result rather than traversing. + Context.Results.Add(CreateResult(propertyContext, source ?? ambientSource, newContainerName)); } } - - if (unboundProperties.Count == modelMetadata.Properties.Count) - { - if (source == null || source == ambientSource) - { - // No properties were bound and we didn't find a new source, let the caller handle it. - return false; - } - else - { - // We found a new source, and didn't create a result for any of the properties yet, - // so create a result for the current object. - Context.Results.Add(CreateResult(bindingContext, source, containerName)); - return true; - } - } - else - { - // This model was only partially bound, so create a result for all the other properties - foreach (var property in unboundProperties) - { - // Create a 'default' description for each property - Context.Results.Add(CreateResult(property, source ?? ambientSource, newContainerName)); - } - - return true; - } } private ApiParameterDescription CreateResult( diff --git a/test/Microsoft.AspNet.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs b/test/Microsoft.AspNet.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs index bad01946d3..4a915c387f 100644 --- a/test/Microsoft.AspNet.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs @@ -543,9 +543,20 @@ namespace Microsoft.AspNet.Mvc.Description // Assert var description = Assert.Single(descriptions); - var parameter = Assert.Single(description.ParameterDescriptions); - Assert.Equal("product", parameter.Name); + var parameters = description.ParameterDescriptions; + Assert.Equal(3, parameters.Count); + + var parameter = Assert.Single(parameters, p => p.Name == "ProductId"); Assert.Same(BindingSource.ModelBinding, parameter.Source); + Assert.Equal(typeof(int), parameter.Type); + + parameter = Assert.Single(parameters, p => p.Name == "Name"); + Assert.Same(BindingSource.ModelBinding, parameter.Source); + Assert.Equal(typeof(string), parameter.Type); + + parameter = Assert.Single(parameters, p => p.Name == "Description"); + Assert.Same(BindingSource.ModelBinding, parameter.Source); + Assert.Equal(typeof(string), parameter.Type); } [Fact] @@ -611,9 +622,20 @@ namespace Microsoft.AspNet.Mvc.Description // Assert var description = Assert.Single(descriptions); - var parameter = Assert.Single(description.ParameterDescriptions); - Assert.Equal("product", parameter.Name); + var parameters = description.ParameterDescriptions; + Assert.Equal(3, parameters.Count); + + var parameter = Assert.Single(parameters, p => p.Name == "ProductId"); Assert.Same(BindingSource.Form, parameter.Source); + Assert.Equal(typeof(int), parameter.Type); + + parameter = Assert.Single(parameters, p => p.Name == "Name"); + Assert.Same(BindingSource.Form, parameter.Source); + Assert.Equal(typeof(string), parameter.Type); + + parameter = Assert.Single(parameters, p => p.Name == "Description"); + Assert.Same(BindingSource.Form, parameter.Source); + Assert.Equal(typeof(string), parameter.Type); } [Fact] @@ -677,9 +699,20 @@ namespace Microsoft.AspNet.Mvc.Description // Assert var description = Assert.Single(descriptions); - var parameter = Assert.Single(description.ParameterDescriptions); - Assert.Equal("product", parameter.Name); + var parameters = description.ParameterDescriptions; + Assert.Equal(3, parameters.Count); + + var parameter = Assert.Single(parameters, p => p.Name == "ProductId"); Assert.Same(BindingSource.ModelBinding, parameter.Source); + Assert.Equal(typeof(int), parameter.Type); + + parameter = Assert.Single(parameters, p => p.Name == "Name"); + Assert.Same(BindingSource.ModelBinding, parameter.Source); + Assert.Equal(typeof(string), parameter.Type); + + parameter = Assert.Single(parameters, p => p.Name == "Description"); + Assert.Same(BindingSource.ModelBinding, parameter.Source); + Assert.Equal(typeof(string), parameter.Type); } [Fact] @@ -789,7 +822,7 @@ namespace Microsoft.AspNet.Mvc.Description // Assert var description = Assert.Single(descriptions); - Assert.Equal(3, description.ParameterDescriptions.Count); + Assert.Equal(4, description.ParameterDescriptions.Count); var id = Assert.Single(description.ParameterDescriptions, p => p.Name == "Id"); Assert.Same(BindingSource.Path, id.Source); @@ -799,9 +832,13 @@ namespace Microsoft.AspNet.Mvc.Description Assert.Same(BindingSource.Query, quantity.Source); Assert.Equal(typeof(int), quantity.Type); - var product = Assert.Single(description.ParameterDescriptions, p => p.Name == "Product"); - Assert.Same(BindingSource.Query, product.Source); - Assert.Equal(typeof(OrderProductDTO), product.Type); + var productId = Assert.Single(description.ParameterDescriptions, p => p.Name == "Product.Id"); + Assert.Same(BindingSource.Query, productId.Source); + Assert.Equal(typeof(int), productId.Type); + + var productPrice = Assert.Single(description.ParameterDescriptions, p => p.Name == "Product.Price"); + Assert.Same(BindingSource.Query, productPrice.Source); + Assert.Equal(typeof(decimal), productPrice.Type); } [Fact] @@ -819,7 +856,7 @@ namespace Microsoft.AspNet.Mvc.Description var c = Assert.Single(description.ParameterDescriptions); Assert.Same(BindingSource.Query, c.Source); - Assert.Equal("C.C", c.Name); + Assert.Equal("C.C.C.C", c.Name); Assert.Equal(typeof(Cycle1), c.Type); } @@ -856,14 +893,14 @@ namespace Microsoft.AspNet.Mvc.Description // Assert var description = Assert.Single(descriptions); - var c = Assert.Single(description.ParameterDescriptions); - Assert.Same(BindingSource.ModelBinding, c.Source); - Assert.Equal("c", c.Name); - Assert.Equal(typeof(HasCollection_Complex), c.Type); + var items = Assert.Single(description.ParameterDescriptions); + Assert.Same(BindingSource.ModelBinding, items.Source); + Assert.Equal("Items", items.Name); + Assert.Equal(typeof(Child[]), items.Type); } [Fact] - public void GetApiDescription_ParameterDescription_RedundentMetadataMergedWithParent() + public void GetApiDescription_ParameterDescription_RedundentMetadata_NotMergedWithParent() { // Arrange var action = CreateActionDescriptor(nameof(AcceptsRedundentMetadata)); @@ -875,10 +912,16 @@ namespace Microsoft.AspNet.Mvc.Description // Assert var description = Assert.Single(descriptions); - var r = Assert.Single(description.ParameterDescriptions); - Assert.Same(BindingSource.Query, r.Source); - Assert.Equal("r", r.Name); - Assert.Equal(typeof(RedundentMetadata), r.Type); + var parameters = description.ParameterDescriptions; + Assert.Equal(2, parameters.Count); + + var id = Assert.Single(parameters, p => p.Name == "Id"); + Assert.Same(BindingSource.Query, id.Source); + Assert.Equal(typeof(int), id.Type); + + var name = Assert.Single(parameters, p => p.Name == "Name"); + Assert.Same(BindingSource.Query, name.Source); + Assert.Equal(typeof(string), name.Type); } [Fact] diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ApiExplorerTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ApiExplorerTest.cs index 3660a1c3ac..be1958ee11 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ApiExplorerTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ApiExplorerTest.cs @@ -744,11 +744,15 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var description = Assert.Single(result); var parameters = description.ParameterDescriptions; - Assert.Equal(1, parameters.Count); + Assert.Equal(2, parameters.Count); - var product = Assert.Single(parameters, p => p.Name == "product"); - Assert.Equal(BindingSource.ModelBinding.Id, product.Source); - Assert.Equal(typeof(ApiExplorerWebSite.Product).FullName, product.Type); + var id = Assert.Single(parameters, p => p.Name == "Id"); + Assert.Equal(BindingSource.ModelBinding.Id, id.Source); + Assert.Equal(typeof(int).FullName, id.Type); + + var name = Assert.Single(parameters, p => p.Name == "Name"); + Assert.Equal(BindingSource.ModelBinding.Id, name.Source); + Assert.Equal(typeof(string).FullName, name.Type); } [Fact] @@ -796,7 +800,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var description = Assert.Single(result); var parameters = description.ParameterDescriptions; - Assert.Equal(6, parameters.Count); + Assert.Equal(7, parameters.Count); var customerId = Assert.Single(parameters, p => p.Name == "CustomerId"); Assert.Equal(BindingSource.Query.Id, customerId.Source); @@ -810,9 +814,13 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal(BindingSource.Form.Id, quantity.Source); Assert.Equal(typeof(int).FullName, quantity.Type); - var product = Assert.Single(parameters, p => p.Name == "Details.Product"); - Assert.Equal(BindingSource.Form.Id, product.Source); - Assert.Equal(typeof(ApiExplorerWebSite.Product).FullName, product.Type); + var productId = Assert.Single(parameters, p => p.Name == "Details.Product.Id"); + Assert.Equal(BindingSource.Form.Id, productId.Source); + Assert.Equal(typeof(int).FullName, productId.Type); + + var productName = Assert.Single(parameters, p => p.Name == "Details.Product.Name"); + Assert.Equal(BindingSource.Form.Id, productName.Source); + Assert.Equal(typeof(string).FullName, productName.Type); var shippingInstructions = Assert.Single(parameters, p => p.Name == "Comments.ShippingInstructions"); Assert.Equal(BindingSource.Query.Id, shippingInstructions.Source);