NancyFx / Nancy

Lightweight, low-ceremony, framework for building HTTP based services on .Net and Mono
http://nancyfx.org
MIT License
7.15k stars 1.47k forks source link

Model Binding fails on abstract class even with blacklist #1574

Open wilhen01 opened 10 years ago

wilhen01 commented 10 years ago

I have a resource type which contains a heterogenous collection of activities, where activities all inherit from a base abstract class. For illustrative purposes, something like this:

public abstract class Activity
{
    public string ActivityProperty { get; set; }
}

public class Resource
{
    public string Property1 { get; set; }

    public string Property2 { get; set; }

    public IList<Activity> Activities { get; set; } 
}

When using this.Bind<>() I get an exception:

System.Reflection.TargetInvocationException occurred
  HResult=-2146232828
  Message=Exception has been thrown by the target of an invocation.
  Source=mscorlib
  StackTrace:
       at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
       at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
       at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
       at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
       at Nancy.ModelBinding.DefaultBodyDeserializers.JsonBodyDeserializer.Deserialize(String contentType, Stream bodyStream, BindingContext context)
       at Nancy.ModelBinding.DefaultBinder.DeserializeRequestBody(BindingContext context)
       at Nancy.ModelBinding.DefaultBinder.Bind(NancyContext context, Type modelType, Object instance, BindingConfig configuration, String[] blackList)
       at Nancy.ModelBinding.DynamicModelBinderAdapter.TryConvert(ConvertBinder binder, Object& result)
       at CallSite.Target(Closure , CallSite , Object )
       at System.Dynamic.UpdateDelegates.UpdateAndExecute1[T0,TRet](CallSite site, T0 arg0)
       at Nancy.ModelBinding.ModuleExtensions.Bind[TModel](INancyModule module, String[] blacklistedProperties)
       at Linn.Events.Service.Modules.EventsModule.PostEvent(Object parameters) in C:\Projects\events\Service\Modules\EventsModule.cs:line 99
  InnerException: System.MissingMethodException
       HResult=-2146233069
       Message=Cannot create an abstract class.
       Source=mscorlib
       StackTrace:
            at System.RuntimeTypeHandle.CreateInstance(RuntimeType type, Boolean publicOnly, Boolean noCheck, Boolean& canBeCached, RuntimeMethodHandleInternal& ctor, Boolean& bNeedSecurityCheck)
            at System.RuntimeType.CreateInstanceSlow(Boolean publicOnly, Boolean skipCheckThis, Boolean fillCache, StackCrawlMark& stackMark)
            at System.RuntimeType.CreateInstanceDefaultCtor(Boolean publicOnly, Boolean skipCheckThis, Boolean fillCache, StackCrawlMark& stackMark)
            at System.Activator.CreateInstance(Type type, Boolean nonPublic)
            at Nancy.Json.JavaScriptSerializer.ConvertToObject(IDictionary`2 dict, Type type)
            at Nancy.Json.JavaScriptSerializer.ConvertToType(Type type, Object obj)
            at Nancy.Json.JavaScriptSerializer.ConvertToList(ArrayList col, Type type)
            at Nancy.Json.JavaScriptSerializer.ConvertToType(Type type, Object obj)
            at Nancy.Json.JavaScriptSerializer.ConvertToObject(IDictionary`2 dict, Type type)
            at Nancy.Json.JavaScriptSerializer.ConvertToType(Type type, Object obj)
            at Nancy.Json.JavaScriptSerializer.ConvertToType[T](Object obj)
            at Nancy.Json.JavaScriptSerializer.Deserialize[T](String input)
       InnerException: 

The same exception still gets thrown using blacklisting, either by string or lambda

var resource = this.Bind<Resource>("Activities");
var resource = this.Bind<Resource>(ignore => ignore.Activities);

I think this must be a bug, unless I'm missing something? I'm using Nancy 0.23.

jchannon commented 10 years ago

Out of interest, does making it not abstract work?

wilhen01 commented 10 years ago

@jchannon Not sure, will try and find time to test that.

wilhen01 commented 10 years ago

@jchannon Just did some testing, and yes, if you remove the abstract modifier from the class the blacklist works correctly. That's not a great workaround though!

LukeForder commented 10 years ago

I'm running into the same issue deserializing a collection of interfaces using with Json.NET. Having created a JsonConverter to deserialize to an implementation of the interface I still get an error System.MissingMethodException: Cannot create an instance of an interface..

Looking at the source, it seems the problem is caused by;

  private static void HandleReferenceTypeCollectionElement(BindingContext bindingContext, IList model, int count, object o)
{
   // If the instance specified in the binder contains the n-th element use that otherwise make a new one.
   object genericTypeInstance;
   if (model.Count > count)
   {
        genericTypeInstance = model[count];
   }
   else
   {
        genericTypeInstance = Activator.CreateInstance(bindingContext.GenericType);
        model.Add(genericTypeInstance);
   }
 ...

HandleReferenceTypeCollectionElement attempts to create a new instance of the generic collection type before copying valid properties from the original deserialized object (the o parameter) into it. So will always fail for generic collections whose generic type is either abstract or an interface.

LukeForder commented 10 years ago

In the end I created a custom implementation an IModelBinder to handle the deserialization of the collection.

luisrudge commented 8 years ago

I'm having this issue as well :( Even when I blacklist the property, it still fails to bind

phillip-haydon commented 8 years ago

Binding to an abstract class??? Stop that. We don't create proxy objects for binding to.

luisrudge commented 8 years ago

I'm binding to a concrete class which has a property with an abstract type as it's type. I'm trying to ignore this property, but it doesn't work.

wilhen01 commented 8 years ago

@phillip-haydon I think my original post illustrates a perfectly legitimate example. As pointed out by @luisrudge we're actually trying to ignore the abstract type, not bind to it. Blacklisting properties shouldn't be dependent on their modifiers.

phillip-haydon commented 8 years ago

@wilhen01 ok, I understand now, I was in hospital reading through my Nancy emails so I didn't fully understand the initial problem.

This MAY be solved when #2061 is complete.