Fix optional id route param inferred as FromQuery (#11557)

* Fix optional id route param inferred as FromQuery

Closes #11554

* Prevent breaking change, UmbracoJsonModelBinderConvention is public class

* Set missing binding source for complex types

* Update UmbracoApiBehaviorApplicationModelProvider.cs

Co-authored-by: Elitsa Marinovska <[email protected]>
Co-authored-by: Elitsa Marinovska <[email protected]>

Author: Paul Johnson

src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs

@@ -18,6 +18,7 @@ namespace Umbraco.Cms.Web.Common.ApplicationModels
     /// <para>
     /// This is nearly a copy of aspnetcore's ApiBehaviorApplicationModelProvider which supplies a convention for the
     /// [ApiController] attribute, however that convention is too strict for our purposes so we will have our own.
+    /// Uses UmbracoJsonModelBinder for complex parameters and those with BindingSource of Body, but leaves the rest alone see GH #11554
     /// </para>
     /// <para>
     /// See https://shazwazza.com/post/custom-body-model-binding-per-controller-in-asp-net-core/
@@ -41,14 +42,12 @@ public UmbracoApiBehaviorApplicationModelProvider(IModelMetadataProvider modelMe
             {
                 new ClientErrorResultFilterConvention(), // Ensures the responses without any body is converted into a simple json object with info instead of a string like "Status Code: 404; Not Found"
                 new ConsumesConstraintForFormFileParameterConvention(), // If an controller accepts files, it must accept multipart/form-data.
-                new InferParameterBindingInfoConvention(modelMetadataProvider), // no need for [FromBody] everywhere, A complex type parameter is assigned to FromBody
 
-                // This ensures that all parameters of type BindingSource.Body (based on the above InferParameterBindingInfoConvention) are bound
+                // This ensures that all parameters of type BindingSource.Body and those of complex type are bound
                 // using our own UmbracoJsonModelBinder
-                new UmbracoJsonModelBinderConvention()
+                new UmbracoJsonModelBinderConvention(modelMetadataProvider)
             };
 
-            // TODO: Need to determine exactly how this affects errors
             var defaultErrorType = typeof(ProblemDetails);
             var defaultErrorTypeAttribute = new ProducesErrorResponseTypeAttribute(defaultErrorType);
             _actionModelConventions.Add(new ApiConventionApplicationModelConvention(defaultErrorTypeAttribute));
@@ -68,25 +67,24 @@ public void OnProvidersExecuted(ApplicationModelProviderContext context)
         /// <inheritdoc/>
         public void OnProvidersExecuting(ApplicationModelProviderContext context)
         {
-            foreach (var controller in context.Result.Controllers)
+            foreach (ControllerModel controller in context.Result.Controllers)
             {
                 if (!IsUmbracoApiController(controller))
                 {
                     continue;
                 }
 
-                foreach (var action in controller.Actions)
+                foreach (ActionModel action in controller.Actions)
                 {
-                    foreach (var convention in _actionModelConventions)
+                    foreach (IActionModelConvention convention in _actionModelConventions)
                     {
                         convention.Apply(action);
                     }
                 }
-
             }
         }
 
-        private bool IsUmbracoApiController(ControllerModel controller)
+        private static bool IsUmbracoApiController(ICommonModel controller)
             => controller.Attributes.OfType<UmbracoApiControllerAttribute>().Any();
     }
 }

src/Umbraco.Web.Common/ApplicationModels/UmbracoJsonModelBinderConvention.cs

@@ -1,25 +1,61 @@
-using System.Linq;
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
+using Microsoft.Extensions.DependencyInjection;
+using Umbraco.Cms.Web.Common.DependencyInjection;
 using Umbraco.Cms.Web.Common.ModelBinders;
 
 namespace Umbraco.Cms.Web.Common.ApplicationModels
 {
     /// <summary>
-    /// Applies the <see cref="UmbracoJsonModelBinder"/> body model binder to any parameter binding source of type <see cref="BindingSource.Body"/>
+    /// Applies the <see cref="UmbracoJsonModelBinder"/> body model binder to any complex parameter and those with a
+    /// binding source of type <see cref="BindingSource.Body"/>
     /// </summary>
-    /// <remarks>
-    /// For this to work Microsoft's own <see cref="InferParameterBindingInfoConvention"/> convention must be executed before this one
-    /// </remarks>
     public class UmbracoJsonModelBinderConvention : IActionModelConvention
     {
+        private readonly IModelMetadataProvider _modelMetadataProvider;
+
+        public UmbracoJsonModelBinderConvention()
+            : this(StaticServiceProvider.Instance.GetRequiredService<IModelMetadataProvider>())
+        {
+        }
+
+        public UmbracoJsonModelBinderConvention(IModelMetadataProvider modelMetadataProvider)
+        {
+            _modelMetadataProvider = modelMetadataProvider;
+        }
+
         /// <inheritdoc/>
         public void Apply(ActionModel action)
         {
-            foreach (ParameterModel p in action.Parameters.Where(p => p.BindingInfo?.BindingSource == BindingSource.Body))
+            foreach (ParameterModel p in action.Parameters)
             {
-                p.BindingInfo.BinderType = typeof(UmbracoJsonModelBinder);
+                if (p.BindingInfo == null)
+                {
+                    if (IsComplexTypeParameter(p))
+                    {
+                        p.BindingInfo = new BindingInfo
+                        {
+                            BindingSource = BindingSource.Body,
+                            BinderType = typeof(UmbracoJsonModelBinder)
+                        };
+                    }
+
+                    continue;
+                }
+
+                if (p.BindingInfo.BindingSource == BindingSource.Body)
+                {
+                    p.BindingInfo.BinderType = typeof(UmbracoJsonModelBinder);
+                }
             }
         }
+
+        private bool IsComplexTypeParameter(ParameterModel parameter)
+        {
+            // No need for information from attributes on the parameter. Just use its type.
+            ModelMetadata metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType);
+
+            return metadata.IsComplexType;
+        }
     }
 }