Ninject 2 is out ... awesome ... but ... I may have found a bug!

57 views
Skip to first unread message

x97mdr

unread,
Feb 25, 2010, 7:13:15 PM2/25/10
to ninject
Hey guys,

I'm really happy Ninject2 was released! Great work. I love this
framework and it keeps getting better. I don't mean to rain on
anyone's parade however ... but I have devised a unit test to make
explicit a bug in Ninject2 that makes it impossible for me to
implement. It has to do with getting a copy of the kernel within the
When() method of a binding. I do something like this with Ninject1

Bind<IService>().To<FlatFileService>().OnlyIf(context =>
context.Kernel.Get<Settings>().UseFlatFiles);

I do this a lot to conditionally bind implementations to services
depending on what some user entered as a setting. When I try to do
this in Ninject I get NREs because ParentContext is null.

Bind<IService>().To<FlatFileService>().When(request =>
request.ParentContext.Kernel.Get<Settings>().UseFlatFiles);

The test class is attached below (ugly, but does the trick ... I
really had no idea how to test the Bindings otherwise!!). I don't
want to be one of those whiners who gripes at open source authors
rather than contributes so I would really like to take a stab at
repairing it but, not being terribly familiar with the code base, I
wonder if someone could point me in the general direction? Heck, am I
even getting the IKernel in the correct way from an IRequest object??

public class WhenResolvingObject
{
protected readonly IKernel kernel = new StandardKernel();

[Fact]
public void ParentContextIsNotNull()
{
kernel.Bind<IWarrior>().To<Ninja>().When(request =>
{
request.ParentContext.ShouldNotBeNull();
return true;
});

var warrior = kernel.Get<IWarrior>();


warrior.ShouldNotBeNull();
warrior.ShouldNotBeInstanceOf<Ninja>();
}
}

Thanks!

Nate Kohari

unread,
Feb 26, 2010, 9:25:37 AM2/26/10
to nin...@googlegroups.com
Jeffrey,

In Ninject 1, the context controlled the entire activation process. In Ninject 2, it's been split into the request, which defines the nature of the dependency that's being requested, and the context, which holds information about the instance being activated. The context represents a combination of an request and an a binding. In the case of a conditional binding, the ParentContext property of the request isn't set when checking it against the condition.

Off the top of my head, the easiest fix would be for us to add a reference to the kernel to the request, which is set before the conditions are compared. That way you could do:

Bind<IService>().To<FlatFileService>().When(r => r.Kernel.Get<Settings>().UseFlatFiles);

That being said, you can get around this now if you resolve Settings before you declare the binding, like this:

var settings = Kernel.Get<Settings>();

if (settings.UseFlatFiles)
  Bind<IService>().To<FlatFileService>();
else
  Bind<IService>().To<OtherService>();

...although then it obviously would be fixed for the lifetime of the app.



Thanks,
Nate


--
You received this message because you are subscribed to the Google Groups "ninject" group.
To post to this group, send email to nin...@googlegroups.com.
To unsubscribe from this group, send email to ninject+u...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/ninject?hl=en.


Simone Chiaretta

unread,
Feb 26, 2010, 9:33:37 AM2/26/10
to ninject
Unrelated to that problem: has Ninject.MVC been updated to support MVC v2?
Simone
--
Simone Chiaretta
Microsoft MVP ASP.NET - ASPInsider
Blog: http://codeclimber.net.nz
RSS: http://feeds2.feedburner.com/codeclimber
twitter: @simonech

Any sufficiently advanced technology is indistinguishable from magic
"Life is short, play hard"

Nate Kohari

unread,
Feb 26, 2010, 9:36:26 AM2/26/10
to nin...@googlegroups.com
Simone:

Not yet. There are a couple of patches available, but they break MVC1. Since MVC1 is still the current release, we haven't moved yet. Most likely, I'll be splitting the extension into MVC1 and MVC2 support in the near future.


Thanks,
Nate

Simone Chiaretta

unread,
Feb 26, 2010, 9:39:41 AM2/26/10
to ninject
OK.. great.. probably you are aware of the issue, but the current way you create a controller in MVC1 will not work on MVC2 because of Areas.

I wrote about this in a blog post a few months ago:
http://codeclimber.net.nz/archive/2009/12/11/asp.net-mvc-controllerfactory-for-unity-and-the-reasoning-behind-it.aspx

I wanted to contribute with a patch, but never had time to do it, sorry
Simo

Ian Davis

unread,
Feb 26, 2010, 10:39:40 AM2/26/10
to nin...@googlegroups.com
You can also do something like this:

Bind<IService>().To<FlatFileService>().When(request => Kernel.Get<Settings>().UseFlatFiles);

When you call Bind(), you are in a module and can access the kernel, or you are using kernel.Bind(), either way you already have access to the kernel and don't need to pull it out of the request.

-Ian

Nate Kohari

unread,
Feb 26, 2010, 10:44:28 AM2/26/10
to nin...@googlegroups.com
Good point Ian. Hadn't thought about closing over the Kernel property of the binding itself. That's probably a better solution than us adding it to the request, actually.

-Nate

x97mdr

unread,
Feb 26, 2010, 12:08:05 PM2/26/10
to ninject
OK Thanks Nate.

In my case having the object fixed initially shouldn't be a problem
since the Settings are Singletons anyway and they're usually the only
thing I use when conditionally resolving things.

What's your opinion on adding a Kernel property to the IRequest? I
might take a stab at it if you thought it advisable. It would be nice
to have a way to get at the IKernel that generated the request from
within the IRequest itself. I could do the same logic inside of a
provider since you do get an IContext with a provider, but I find too
many providers make for a smell and the logic I need is very simple
and make providers seem like overkill.

Thanks!

> > ninject+u...@googlegroups.com<ninject%2Bunsu...@googlegroups.com >

Jeffrey Cameron

unread,
Feb 26, 2010, 12:13:22 PM2/26/10
to nin...@googlegroups.com
Oh indeed ... hand't thought of that!! Thanks Ian!

On Fri, Feb 26, 2010 at 10:39 AM, Ian Davis <ida...@innovatian.com> wrote:
Reply all
Reply to author
Forward
0 new messages