Merge branch 'release' into dev

Author: dougbu

src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDto.cs

@@ -40,18 +40,22 @@ public virtual async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBind
                 return null;
             }
 
-            EnsureModel(bindingContext);
-            var result = await CreateAndPopulateDto(bindingContext, mutableObjectBinderContext.PropertyMetadata);
+            // Create model first (if necessary) to avoid reporting errors about properties when activation fails.
+            var model = GetModel(bindingContext);
+
+            var results = await BindPropertiesAsync(bindingContext, mutableObjectBinderContext.PropertyMetadata);
 
             var validationNode = new ModelValidationNode(
                 bindingContext.ModelName,
                 bindingContext.ModelMetadata,
-                bindingContext.Model);
+                model);
+
+            // Post-processing e.g. property setters and hooking up validation.
+            bindingContext.Model = model;
+            ProcessResults(bindingContext, results, validationNode);
 
-            // post-processing, e.g. property setters and hooking up validation
-            ProcessDto(bindingContext, (ComplexModelDto)result.Model, validationNode);
             return new ModelBindingResult(
-                bindingContext.Model,
+                model,
                 bindingContext.ModelName,
                 isModelSet: true,
                 validationNode: validationNode);
@@ -192,7 +196,8 @@ private async Task<bool> CanBindValue(ModelBindingContext bindingContext)
             var bindingSource = bindingContext.BindingSource;
             if (bindingSource != null && !bindingSource.IsGreedy)
             {
-                var rootValueProvider = bindingContext.OperationBindingContext.ValueProvider as IBindingSourceValueProvider;
+                var rootValueProvider =
+                    bindingContext.OperationBindingContext.ValueProvider as IBindingSourceValueProvider;
                 if (rootValueProvider != null)
                 {
                     valueProvider = rootValueProvider.Filter(bindingSource);
@@ -225,12 +230,6 @@ private static bool CanBindType(ModelMetadata modelMetadata)
                 return false;
             }
 
-            if (modelMetadata.ModelType == typeof(ComplexModelDto))
-            {
-                // forbidden type - will cause a stack overflow if we try binding this type
-                return false;
-            }
-
             return true;
         }
 
@@ -265,24 +264,50 @@ private static bool CanUpdateReadOnlyProperty(Type propertyType)
             return true;
         }
 
-        private async Task<ModelBindingResult> CreateAndPopulateDto(
+        // Returned dictionary contains entries corresponding to properties against which binding was attempted. If
+        // binding failed, the entry's value will have IsModelSet == false. Binding is attempted for all elements of
+        // propertyMetadatas.
+        private async Task<IDictionary<ModelMetadata, ModelBindingResult>> BindPropertiesAsync(
             ModelBindingContext bindingContext,
             IEnumerable<ModelMetadata> propertyMetadatas)
         {
-            // create a DTO and call into the DTO binder
-            var dto = new ComplexModelDto(bindingContext.ModelMetadata, propertyMetadatas);
-
-            var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider;
-            var dtoMetadata = metadataProvider.GetMetadataForType(typeof(ComplexModelDto));
+            var results = new Dictionary<ModelMetadata, ModelBindingResult>();
+            foreach (var propertyMetadata in propertyMetadatas)
+            {
+                var propertyModelName = ModelNames.CreatePropertyModelName(
+                    bindingContext.ModelName,
+                    propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName);
+                var childContext = ModelBindingContext.GetChildModelBindingContext(
+                    bindingContext,
+                    propertyModelName,
+                    propertyMetadata);
+
+                // ModelBindingContext.Model property values may be non-null when invoked via TryUpdateModel(). Pass
+                // complex (including collection) values down so that binding system does not unnecessarily recreate
+                // instances or overwrite inner properties that are not bound. No need for this with simple values
+                // because they will be overwritten if binding succeeds. Arrays are never reused because they cannot
+                // be resized.
+                //
+                // ModelMetadata.PropertyGetter is not null safe; use it only if Model is non-null.
+                if (bindingContext.Model != null &&
+                    propertyMetadata.PropertyGetter != null &&
+                    propertyMetadata.IsComplexType &&
+                    !propertyMetadata.ModelType.IsArray)
+                {
+                    childContext.Model = propertyMetadata.PropertyGetter(bindingContext.Model);
+                }
 
-            var childContext = ModelBindingContext.GetChildModelBindingContext(
-                bindingContext,
-                bindingContext.ModelName,
-                dtoMetadata);
+                var result = await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(childContext);
+                if (result == null)
+                {
+                    // Could not bind. Let ProcessResult() know explicitly.
+                    result = new ModelBindingResult(model: null, key: propertyModelName, isModelSet: false);
+                }
 
-            childContext.Model = dto;
+                results[propertyMetadata] = result;
+            }
 
-            return await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(childContext);
+            return results;
         }
 
         /// <summary>
@@ -298,16 +323,18 @@ protected virtual object CreateModel([NotNull] ModelBindingContext bindingContex
         }
 
         /// <summary>
-        /// Ensures <see cref="ModelBindingContext.Model"/> is not <c>null</c> in given
-        /// <paramref name="bindingContext"/>.
+        /// Get <see cref="ModelBindingContext.Model"/> if that property is not <c>null</c>. Otherwise activate a
+        /// new instance of <see cref="ModelBindingContext.ModelType"/>.
         /// </summary>
         /// <param name="bindingContext">The <see cref="ModelBindingContext"/>.</param>
-        protected virtual void EnsureModel([NotNull] ModelBindingContext bindingContext)
+        protected virtual object GetModel([NotNull] ModelBindingContext bindingContext)
         {
-            if (bindingContext.Model == null)
+            if (bindingContext.Model != null)
             {
-                bindingContext.Model = CreateModel(bindingContext);
+                return bindingContext.Model;
             }
+
+            return CreateModel(bindingContext);
         }
 
         /// <summary>
@@ -365,17 +392,18 @@ internal static PropertyValidationInfo GetPropertyValidationInfo(ModelBindingCon
         }
 
         // Internal for testing.
-        internal ModelValidationNode ProcessDto(
+        internal ModelValidationNode ProcessResults(
             ModelBindingContext bindingContext,
-            ComplexModelDto dto,
+            IDictionary<ModelMetadata, ModelBindingResult> results,
             ModelValidationNode validationNode)
         {
             var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider;
-            var modelExplorer = metadataProvider.GetModelExplorerForType(bindingContext.ModelType, bindingContext.Model);
+            var modelExplorer =
+                metadataProvider.GetModelExplorerForType(bindingContext.ModelType, bindingContext.Model);
             var validationInfo = GetPropertyValidationInfo(bindingContext);
 
             // Eliminate provided properties from RequiredProperties; leaving just *missing* required properties.
-            var boundProperties = dto.Results.Where(p => p.Value.IsModelSet).Select(p => p.Key.PropertyName);
+            var boundProperties = results.Where(p => p.Value.IsModelSet).Select(p => p.Key.PropertyName);
             validationInfo.RequiredProperties.ExceptWith(boundProperties);
 
             foreach (var missingRequiredProperty in validationInfo.RequiredProperties)
@@ -389,25 +417,25 @@ internal ModelValidationNode ProcessDto(
                     Resources.FormatModelBinding_MissingBindRequiredMember(propertyName));
             }
 
-            // For each property that ComplexModelDtoModelBinder attempted to bind, call the setter, recording
+            // For each property that BindPropertiesAsync() attempted to bind, call the setter, recording
             // exceptions as necessary.
-            foreach (var entry in dto.Results)
+            foreach (var entry in results)
             {
-                var dtoResult = entry.Value;
-                if (dtoResult != null)
+                var result = entry.Value;
+                if (result != null)
                 {
                     var propertyMetadata = entry.Key;
-                    SetProperty(bindingContext, modelExplorer, propertyMetadata, dtoResult);
+                    SetProperty(bindingContext, modelExplorer, propertyMetadata, result);
 
-                    var dtoValidationNode = dtoResult.ValidationNode;
-                    if (dtoValidationNode == null)
+                    var propertyValidationNode = result.ValidationNode;
+                    if (propertyValidationNode == null)
                     {
-                        // Make sure that irrespective of if the properties of the model were bound with a value,
+                        // Make sure that irrespective of whether the properties of the model were bound with a value,
                         // create a validation node so that these get validated.
-                        dtoValidationNode = new ModelValidationNode(dtoResult.Key, entry.Key, dtoResult.Model);
+                        propertyValidationNode = new ModelValidationNode(result.Key, entry.Key, result.Model);
                     }
 
-                    validationNode.ChildNodes.Add(dtoValidationNode);
+                    validationNode.ChildNodes.Add(propertyValidationNode);
                 }
             }
 
@@ -422,71 +450,69 @@ internal ModelValidationNode ProcessDto(
         /// The <see cref="ModelExplorer"/> for the model containing property to set.
         /// </param>
         /// <param name="propertyMetadata">The <see cref="ModelMetadata"/> for the property to set.</param>
-        /// <param name="dtoResult">The <see cref="ModelBindingResult"/> for the property's new value.</param>
+        /// <param name="result">The <see cref="ModelBindingResult"/> for the property's new value.</param>
         /// <remarks>Should succeed in all cases that <see cref="CanUpdateProperty"/> returns <c>true</c>.</remarks>
         protected virtual void SetProperty(
             [NotNull] ModelBindingContext bindingContext,
             [NotNull] ModelExplorer modelExplorer,
             [NotNull] ModelMetadata propertyMetadata,
-            [NotNull] ModelBindingResult dtoResult)
+            [NotNull] ModelBindingResult result)
         {
             var bindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.IgnoreCase;
-            var property = bindingContext.ModelType.GetProperty(
-                propertyMetadata.PropertyName,
-                bindingFlags);
+            var property = bindingContext.ModelType.GetProperty(propertyMetadata.PropertyName, bindingFlags);
 
             if (property == null)
             {
                 // Nothing to do if property does not exist.
                 return;
             }
 
-            if (!property.CanWrite)
+            if (!result.IsModelSet)
             {
-                // Try to handle as a collection if property exists but is not settable.
-                AddToProperty(bindingContext, modelExplorer, property, dtoResult);
+                // If we don't have a value, don't set it on the model and trounce a pre-initialized value.
                 return;
             }
 
-            object value = null;
-            if (dtoResult.IsModelSet)
-            {
-                value = dtoResult.Model;
-            }
-
-            if (!dtoResult.IsModelSet)
+            if (!property.CanWrite)
             {
-                // If we don't have a value, don't set it on the model and trounce a pre-initialized
-                // value.
+                // Try to handle as a collection if property exists but is not settable.
+                AddToProperty(bindingContext, modelExplorer, property, result);
                 return;
             }
 
+            var value = result.Model;
             try
             {
                 propertyMetadata.PropertySetter(bindingContext.Model, value);
             }
             catch (Exception exception)
             {
-                AddModelError(exception, bindingContext, dtoResult);
+                AddModelError(exception, bindingContext, result);
             }
         }
 
         private void AddToProperty(
             ModelBindingContext bindingContext,
             ModelExplorer modelExplorer,
             PropertyInfo property,
-            ModelBindingResult dtoResult)
+            ModelBindingResult result)
         {
             var propertyExplorer = modelExplorer.GetExplorerForProperty(property.Name);
 
             var target = propertyExplorer.Model;
-            var source = dtoResult.Model;
-            if (!dtoResult.IsModelSet || target == null || source == null)
+            var source = result.Model;
+            if (target == null || source == null)
             {
                 // Cannot copy to or from a null collection.
                 return;
             }
 
+            if (target == source)
+            {
+                // Added to the target collection in BindPropertiesAsync().
+                return;
+            }
+
             // Determine T if this is an ICollection<T> property. No need for a T[] case because CanUpdateProperty()
             // ensures property is either settable or not an array. Underlying assumption is that CanUpdateProperty()
             // and SetProperty() are overridden together.
@@ -507,7 +533,7 @@ private void AddToProperty(
             }
             catch (Exception exception)
             {
-                AddModelError(exception, bindingContext, dtoResult);
+                AddModelError(exception, bindingContext, result);
             }
         }
 
@@ -529,7 +555,7 @@ private static void CallPropertyAddRange<TElement>(object target, object source)
         private static void AddModelError(
             Exception exception,
             ModelBindingContext bindingContext,
-            ModelBindingResult dtoResult)
+            ModelBindingResult result)
         {
             var targetInvocationException = exception as TargetInvocationException;
             if (targetInvocationException != null && targetInvocationException.InnerException != null)
@@ -539,7 +565,7 @@ private static void AddModelError(
 
             // Do not add an error message if a binding error has already occurred for this property.
             var modelState = bindingContext.ModelState;
-            var modelStateKey = dtoResult.Key;
+            var modelStateKey = result.Key;
             var validationState = modelState.GetFieldValidationState(modelStateKey);
             if (validationState == ModelValidationState.Unvalidated)
             {