umco / umbraco-ditto

Ditto - the friendly view-model mapper for Umbraco
http://our.umbraco.org/projects/developer-tools/ditto
MIT License
79 stars 33 forks source link

InvalidCastException #176

Closed sniffdk closed 7 years ago

sniffdk commented 8 years ago

Hi Ditto-people

I've just upgraded to Ditto 0.9.0 and have started seeing an issue where an InvalidCastException is thrown, something like this: Unable to cast object of type 'Web.ViewModels.ItemViewModel[]' to type 'System.Collections.Generic.List1[Web.ViewModels.ItemViewModel]'.`

I'm using Ditto in conjunction with Nested Content doing some fairly simple stuff like:

var items = publishedNode
    .GetPropertyValue<IEnumerable<IPublishedContent>>("items")
    .As<ItemViewModel>()
    .ToList();

I tried to remove the .ToList() part and run it through a manual for-each, but the exception is still thrown. The Ditto stack trace looks like this:

at lambda_method(Closure , Object , Object )
at Our.Umbraco.Ditto.PropertyInfoInvocations.SetValue(PropertyInfo property, Object instance, Object value)
at Our.Umbraco.Ditto.PublishedContentExtensions.ConvertContent(IPublishedContent content, Type type, CultureInfo culture, Object instance, IEnumerable`1 processorContexts, Action`1 onConverting, Action`1 onConverted)
at Our.Umbraco.Ditto.PublishedContentExtensions.As(IPublishedContent content, Type type, CultureInfo culture, Object instance, IEnumerable`1 processorContexts, Action`1 onConverting, Action`1 onConverted)
at Our.Umbraco.Ditto.PublishedContentExtensions.<>c__DisplayClass3.<As>b__1(IPublishedContent x)

And apparently the Source is mentioned as being Anonymously Hosted DynamicMethods Assembly, gotta :heart: dynamics :smile: This is tested in Umbraco 7.4.3, both on .net 4.5.2 and 4.6.1 and downgrading to Ditto 0.8.4 made the exception go away.

I don't know anything about the Ditto internals, but do let me know if I can provide any more help or clues.

Cheers Mads

sniffdk commented 8 years ago

Ok, did some more testing. First a small sample of the model setup I'm using:

public class ListViewModel
{    
    public string Name { get; set; }
    ...........

    public List<ItemViewModel> Items { get; set; }

    public ListViewModel()
    {
        Items = new List<ItemViewModel>();
    }
}

Now, this fails in version 0.9.0 on PropertyInfoInvocations.SetValue which is some of the new performance optimizing stuff. If I change the List<ItemViewModel> to be IEnumerable<ItemViewModel> it converts correctly and no exception is thrown.

I can understand why the cast fails, you can't cast an array to a list, but I haven't yet figured out why this cast is happening in the first place.

Cheers

sniffdk commented 8 years ago

Ok, more investigation. It seems that the error is actually happening in the EnumerableInvocations.Empty method. When providing a property of type List<T>, the Empty method will return an Array[0] instead. I'm having some difficulties figuring out why that is happening though.

Upon entering EnumerableConverterAttribute.ProcessValue the variable propertyIsEnumerableType is set to true. At this point this.Value is null, so we end up calling EnumerableInvocations.Empty(this.Context.PropertyDescriptor.PropertyType.GenericTypeArguments.First()).

In the Emptymethod the key retrieved is a Our.Umbraco.Ditto.MethodBaseCacheItem with the MethodBase property set to "Empty" and the Type set to ItemViewModel. When calling f(type) the returned value is an ItemViewModel[0]. My expectation would be that it would have return an empty List<ItemViewModel>.

As far as I can tell, the entire EnumerableConverterAttribute class is new in 0.9.0, so I'm not sure how this used to work. Don't think I can get much closer to the problem now.

Cheers

leekelleher commented 8 years ago

@sniffdk Thanks for all the info. Just want to say that we're not ignoring you, (we're just caught up at CGRT16). We'll definitely take a look once we can; would be great to resolve this scenario.

sniffdk commented 8 years ago

I'm fully aware of that @leekelleher , have a great time :smile:

JimBobSquarePants commented 8 years ago

@sniffdk @leekelleher

I'll have a look at this asap. I'm sure I can figure out what the difference is between the two approaches.

mattbrailsford commented 8 years ago

@JimBobSquarePants you had chance to look at this yet?

JimBobSquarePants commented 8 years ago

Not yet, will try to grab some time over the the weekend.

JimBobSquarePants commented 8 years ago

Still struggling with this. I can handle empty results but not non-empty ones.

Its not the setter, it's the stuff before I think that is the issue.

Have to say though, viewmodels should be immutable and not be using lists

mattbrailsford commented 8 years ago

Do we have a failing unit test for this?

sniffdk commented 7 years ago

ehm, no, we don't ... 😔

leekelleher commented 7 years ago

@sniffdk It's looking like @JimBobSquarePants has resolved this with #195 ... we'll verify before closing off this issue 👍

sniffdk commented 7 years ago

Yeah, just saw that, awesome :)