Invalid dynamic collection binding and validation

[anfomin]

@dougbu asked to create new issue in discussion #2268. Current CollectionModelBinder can't properly bind collections with dynamic indexes. It returns invalid results in 2 cases.

1. I've class with collection property:

class RootObject
{
    public IList<AnotherObject> Collection { get; set; }
    // other code goes here
}

class AnotherObject
{
    public string Name { get; set; }
}

Binding form data:

Collection.index = key1,key2
Collection[key1].Name = name1
Collection[key2].Name = name2

If I try to bind RootObject then Collection property is bound successfully, but ModelState contains unvalidated states for Collection[key1].Name and Collection[key2].Name:

var model = new RoomObject();
TryUpdateModel(model);
/// ModelState.IsValid returns false - wrong value.

2. I've the same classes. If I try to bind collection of AnotherObject directly with prefix, result collection is still empty:

var collection = new List<AnotherObject>();
TryUpdateModel(collection, "Collection");
// collection is still empty, even if there is data

I'm using latest beta5 MVC version.

[danroth27]

@anfomin Are you literally sending Collection.index = key1,key2 or is this just a shorthand for sending Collection.index = key1 and then Collection.index = key2?

[danroth27]

@dougbu We should at least be consistent with the MVC 5 behavior.

[anfomin]

@danroth27 yes, in previous message I wrote a shorthand. I have 2 inputs: Collection.index = key1 and Collection.index = key2.

[dougbu]

@anfomin could you please upload a small sample of what you're trying to do? Want to avoid testing a slightly different scenario.

A GitHub repository is easiest for me but feel free to send a Zip file offline or whatever.

[anfomin]

@dougbu ok, I'll prepare sample tomorrow.

[anfomin]

@dougbu sample is ready: https://github.com/anfomin/MvcCollectionBindingSample

[dougbu]

thanks @anfomin. I am able to reproduce the tightly-related issues you reported. both appear to be regressions compared to MVC 5.

one workaround for the ModelState.IsValid issue is to remove the "Collection.index" fields entirely. instead name the fields using the C# indices e.g.

for (var index = 0; index < Model.Collection.Count; index++)
{
    @Html.LabelFor(model => model.Collection[index])
    <div>
        @Html.EditorFor(model => model.Collection[index])
        @Html.ValidationMessageFor(model => model.Collection[index])
    </div>
}

have not found a workaround for the problem binding directly to a collection (except not doing that at all).

[anfomin]

@dougbu I can't use workaround with numeric indexes because collection items can be added or deleted with JS in real app. So resulting bound collection will have gaps in indexes. I'm using custom IModelBinder until issue is fixed.

[dougbu]

1. the root cause of the first problem @anfomin reported is model binding adds ModelState entries for the Guid-indexed collection entries e.g.
   
   but these remain Unvalidated because validation checks the created model and its int-indexed entries e.g.
   
   (all entries above except Collection[d6dc29c5].Name and Collection[8d51753e].Name are either Skipped (e.g. TempData) or Valid.)
   in MVC 5, the final ModelState instead looks like the following and both entries are Valid:
   
2. the root cause of the second problem is MVC 6 lacks support for copying entries into an existing collection. properties such as public IList<AnotherObject> Collection { get; } = new List<AnotherObject>(); and calls such as TryUpdateModel(collection, "Collection"); work fine in MVC 5 but MVC 6 does not support either case.

[dougbu]

addressed item (2), the lack of support for copying entries into a collection in 3fd4991

[dougbu]

@harshgMSFT if you get back to this before I do, I have a branch and a few thoughts to share...