There appears to be an interaction problem with FactoryAdapter<T> and LazyFactory that causes issues with factory classes that implement IFactory<T> with property injection and custom factories (and perhaps other cases).
Given the sample:
public interface IThing {}
[Factory(typeof(IThing)] public class ThingFactory : IFactory<IThing>() { public IThing CreateInstance(IFactoryRequest request) { return new Thing(); } }
[Implements(ISomethingElse)] public class SomethingElse : ISomethingElse { [Inject] public IThing Thing { get; set; } }
given the above calling GetService<ISomethingElse>() will fail trying to inject the Thing property.
The problem starts out in FactoryAttributeLoader.GetFactory():
// Lazy-instantiate the factories so that they can be injected by the
container var lazyFactory = new LazyFactory(getStronglyTypedFactory);
if (implementedInterfaces.Contains(genericType))
{
// Convert the IFactory<T> instance down to an IFactory
// instance so that it can be used by the target container
Type adapterType =
typeof(FactoryAdapter<>).MakeGenericType(currentServiceType); result = (IFactory)Activator.CreateInstance(adapterType, new[] { lazyFactory }); return result; }
The above snippet attempts to create an adapter to adapt the factory's IFactory<T> to IFactory. Which is great in theory except that the created adapter is really adapting LazyFactory (note that lazyFactory is passed to the adapter as the factory to adapt). LazyFactory, however, implements IFactory not IFactory<T>.
FactoryAdapter<T>.CreateInstance doesn't cope with this:
public object CreateInstance(IFactoryRequest request)
{
if (_factory == null)
return default(T);
var factory = _factory as IFactory<T>;
if (factory == null)
return default(T);
return factory.CreateInstance(request);
}
It assumes that it's adapting IFactory<T>, but in this case it's adapting LazyFactory's IFactory which ends up returning default(T).
There are two ways to fix it:
- Change FactoryAdapter to be smart enough to be a no-op if the object it's adapting implements IFactory (IMO this should be done whether #2 is done or not for robustness).
- Change FactoryAttributeLoader to not bother making an adapter as it uses LazyLoader which makes the adapter unnecessary.
Comment #1
Posted on Feb 2, 2009 by Happy GiraffeI was able to fix this in a local copy by changing the getStronglyTypedFactory lambda in FactoryAttributeLoader.GetResults from:
Func getStronglyTypedFactory = request => getFactory(request) as IFactory;
to:
Func<IFactoryRequest, IFactory> getStronglyTypedFactory =
request =>
{
object result = getFactory(request);
if (result is IFactory)
// If the object is IFactory then we can just return it (what the old code did).
return (IFactory)result;
else
{
// Check to see if the object is IFactory<T>, if so we need to adapt it to
// IFactory.
Type genericType = typeof(IFactory<>).MakeGenericType(currentServiceType);
if (genericType.IsInstanceOfType(result))
{
// HERE is the correct place to adapt IFactory<T> to IFactory.
Type adapterType = typeof(FactoryAdapter<>).MakeGenericType(currentServiceType);
IFactory adapter = (IFactory)Activator.CreateInstance(adapterType, new[] {
result }); return adapter; } else // It isn't an IFactory or IFactory, who knows what it is, return null. return null; } };
Also, the code in FactoryAttributeLoader.GetFactory that creates the FactoryAdapter wrapper object can be removed (I commented it out) as it is too late at this point to create the adapter, it must be done in the lambda.
This fixed my bug and all the unit tests still pass, so (presumably) it didn't break anything.
Comment #2
Posted on Feb 8, 2009 by Massive Bird(No comment was entered for this change.)
Comment #3
Posted on Feb 9, 2009 by Massive BirdIssue 8 has been merged into this issue.
Status: Done
Labels:
Type-Defect
Priority-Medium