dongfo / autofac

Automatically exported from code.google.com/p/autofac
0 stars 0 forks source link

MVC ExtensibleActionInvoker.GetParameterValue Can't Be Disabled #368

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
There is an ignored test in the ExtensibleActionInvokerFixture 
"ActionInjectionTurnedOff_DependencyRegistered_ServiceNotRegistered" that will 
currently fail if enabled.

In the 3.0 branch I fixed the issues that were initially causing it to fail 
(handling some issues around MVC dynamic validation shimming) but in doing so 
uncovered the actual functional issue.

The test has a controller with a dependency property. The property is an 
interface type.

When the ExtensibleActionInvoker executes, it determines if it's set to inject 
action method parameters. This happens in 
ExtensibleActionInvoker.GetParameterValue(ControllerContext, 
ParameterDescriptor)

If it's not set to inject, it falls back to the base functionality 
ControllerBase.GetParameterValue.

What ControllerBase.GetParameterValue does is try to use the incoming 
ParameterDescriptor to bind a model to the value. Basically 
DefaultModelBinder.BindModel(ControllerContext, BindingContext).

DefaultModelBinder.BindModel, failing to find any values to bind, will do 
Activator.CreateInstance() on the dependency type.

When the dependency type is an interface, it throws an exception because you 
can't create an instance of an interface.

So... it's not the test that's malfunctioning, it's a use case that isn't 
totally supported.

I think the ExtensibleActionInvoker.GetParameterValue method either needs to 
check for things like this (e.g., see if the type is an interface or abstract 
class and just return null without calling base) or possibly just catch the 
appropriate exceptions and return null.

I've made a comment to this effect in the associated test in the 3.0 branch 
where I'm working.

Original issue reported on code.google.com by travis.illig on 17 May 2012 at 7:31

GoogleCodeExporter commented 8 years ago

Original comment by travis.illig on 21 Sep 2012 at 4:36

GoogleCodeExporter commented 8 years ago
This issue was closed by revision 8767b8b4af03.

Original comment by travis.illig on 8 Oct 2012 at 11:20

GoogleCodeExporter commented 8 years ago
Per https://groups.google.com/d/msg/autofac/-/YilK3MhfdUsJ it appears there may 
need to be a fix made. Post contents:

The current implementation causes parameters of type string to not be handled 
by the DefaultModelBinder. The CanBeActivated method should also return true 
for strings and types convertible from strings.

if (type.IsValueType || type == typeof(string))
{
    // All value types can go through Activator.CreateInstance(type)
    return true;
}

var typeConverter = TypeDescriptor.GetConverter(type);
if (typeConverter.CanConvertFrom(typeof(string)))
{
    return true;
}

Original comment by travis.illig on 17 Nov 2012 at 4:37

GoogleCodeExporter commented 8 years ago

Original comment by travis.illig on 17 Nov 2012 at 4:38

GoogleCodeExporter commented 8 years ago

Original comment by travis.illig on 26 Nov 2012 at 9:42

GoogleCodeExporter commented 8 years ago
This issue was closed by revision 6d4bf71cc47a.

Original comment by travis.illig on 26 Nov 2012 at 11:58

GoogleCodeExporter commented 8 years ago
I think it will never work that way :) Currently, 
IEnumerable<HttpPostedFileBase> is not handled by the DefaultModelBinder 
because ResolveOptional returns empty enumeration (not null).

Original comment by du...@djanosik.cz on 17 Jan 2013 at 4:09

GoogleCodeExporter commented 8 years ago
If you have found something not working, please file an additional issue with 
some details about reproduction and what you expect to happen. Thanks!

Original comment by travis.illig on 17 Jan 2013 at 4:14