Owin + WebApi - Ninject 'Provider' does not work honor request scope

[jameshulse]

When using a Ninject Provider<T> (factory) to create a dependency, the dependencies resolved from the IContext passed to the Provider will not honor any 'Per Request' scoping.

When in Owin/WebApi world, ninject creates a named scope per request which is automatically bound to a context but it seems that the context passed to the CreateInstance method is somehow not aware of any global named scope.

To reproduce, create a provider which relies on some per request object:

public class FooProvider : Provider<IFoo>
{
    protected override IFoo CreateInstance(IContext context)
    {
        var bar = context.Kernel.Get<IBar>(); // This should call the Bar constructor
        var bar2 = context.Kernel.Get<IBar>(); // This should return the same instance but is calling the constructor again

        return new Foo(bar, bar2);
    }
}

Registration:

Bind<IBar>().To<Bar>().InRequestScope();
Bind<IFoo>().ToProvider<FooProvider>();

Now you would need a WebApi controller hosted in Owin to rely on an IFoo.

[MisterGoodcat]

Is there any news on this issue?

It's not only related to providers. Request scope doesn't work at all when you're using Owin.
To reproduce:

1.) Create a Ninject web application as per: https://github.com/ninject/Ninject.Web.Common/wiki/Setting-up-a-OWIN-WebApi-application

2.) Create some custom type that you can bind, e.g.

public class TestService : ITestService { }
public interface ITestService { }

3.) Add request scope binding, e.g.

private static StandardKernel CreateKernel()
{
    var kernel = new StandardKernel();
    kernel.Load(Assembly.GetExecutingAssembly());
    kernel.Bind<ITestService>().To<TestService>().InRequestScope();
    return kernel;
}

4.) Test. For example:

app.Use((context, next) =>
{
    Debug.WriteLine(
        webApiConfiguration.DependencyResolver.GetService(typeof(ITestService)) == 
        webApiConfiguration.DependencyResolver.GetService(typeof(ITestService)));
    return next();
});

Output is "False", should be "True".

That's a show-stopper for us at the moment.

[MoonStorm]

It seems that this project is dead.

[jameshulse]

We have subsequently changed to using Autofac and haven't looked back.

[MisterGoodcat]

Maybe you should have left the issue open so others exploring Ninject and its options can find it more easily.

I've always liked Ninject and did some investigation into what needs to be done to fix this, but in the end we also moved on to a different framework. Can't say I miss the NuGet package mess, introduction of breaking changes in bug fix releases and similar things.

[jameshulse]

Good point :)

On Thu, Nov 26, 2015 at 5:38 AM, Peter notifications@github.com wrote:

Maybe you should have left the issue open so others exploring Ninject and
its options can find it more easily.

I've always liked Ninject and did some investigation into what needs to be
done to fix this, but in the end we also moved on to a different framework.
Can't say I miss the NuGet package mess, introduction of breaking changes
in bug fix releases and similar things.

—
Reply to this email directly or view it on GitHub
#17 (comment)
.

[PatrickKunz]

You need to use IResolutionRoot if you want create objects in the scope. Don't use the kernel directly in this case. Also make sure that you have installed the context preservation module.

[MisterGoodcat]

As you can see in my example, I wasn't using the kernel but the built-in dependency resolver which is also used by the runtime itself for i.e. constructor injection. The actual use of the kernel in this case is not under the control of the code in question but hidden behind at least the provided adapter and potentially more layers of code. The context preservation module in this sample is installed automatically as one of the NuGet dependencies, but unfortunately it's still not working.

Side note: With all due respect, I think that these exactly are the problems with Ninject nowadays. Even the simple sample above requires 7(!) Ninject NuGet packages to be installed, and after closely following provided instructions or samples you are still confronted with how to specifically handle details that in my opinion should be internal to the framework itself. A developer's intention is quite clear: have DI for typical web application scenarios. Ninject however seems to get more and more lost in layer after layer of generic and abstracted logic, carefully balanced (read: fragile) interaction of multiple independent modules, and some implicit behind-the-scenes magic. If you hit a road block with all that, good luck, because now you're forced to understand all of that magic the framework is supposed to hide from you, to figure out what you might have missed or used in a way that was not intended. Just read, for example, through the wiki documentation of the context preservation module you were referring to. There is simply no way anyone without deeper knowledge of the inner workings of the framework could make any sense of this highly abstracted information in the context of a problem like "request scoping isn't working as expected". Don't get me wrong, I highly appreciate the work people put into these projects, and I have benefited from Ninject quite a lot in the past. I am thankful for that. It just feels like that at some point the project took a turn for the worse. The fact that an issue like this one, of fundamental nature and reproducible by just a few simple steps, is not only unaddressed but also passed without comment for months, does not really help with that impression.

[MoonStorm]

I'm gonna have to agree with @MisterGoodcat. If before Owin the scope was set to the Http context, people were naturally expecting that the Owin context took its place, which is not the case. That's what InRequestScope should mean, right? Doesn't matter whether I'm inside the Owin middleware pipeline, in a filter or inside a controller. An InRequestScope resolved service should remain the same!

I did take a look at the implementation of this extension, and it's pure and simple wrong. I explained a bit more in #22. The fix is not trivial.

[MoonStorm]

The workaround is to remove this module altogether from the project and set up Ninject's classic HTTP scope handler instead. Obviously this would only work on web server hosted apps.

Alternatively, if this is an option at this stage, use a different DI framework.

[ItWorksOnMyMachine]

This is what I'm testing out and it seems to work. Instead of calling .InRequestScope() I'm now calling InCustomRequestScope. This seems entirely too easy. Anyone see a problem here? And if you don't have HttpContext.Current.Request you could just throw an if block into GetRequestScope and return the correct "scope managing object."

public static IBindingNamedWithOrOnSyntax InCustomRequestScope(this IBindingInSyntax syntax)
{
return syntax.InScope(GetRequestScope());
}
private static Func<IContext, object> GetRequestScope()
{
return new Func<IContext, object>(ctx => HttpContext.Current.Request);
}

[Eirenarch]

I just hit this issue. I was using context.Kernel.Get in a ToMethod binding and was very surprised to find that the services I produce this way are not in request scope but transient. Ended up using this code (thanks to @ItWorksOnMyMachine ) to create my own scope. I hope I did not break something

public static class CustomRequestScope
{
    public static Ninject.Syntax.IBindingNamedWithOrOnSyntax<T> InCustomRequestScope<T>(this Ninject.Syntax.IBindingInSyntax<T> syntax)
    {
        return syntax.InScope(ctx => HttpContext.Current.Handler == null ? null : HttpContext.Current.Request);
    }
}

[skalinets]

2017 and the issue is still there. Switched to autofac

[scott-xu]

WebApiConfiguration.DependencyResolver is a global dependency resolver which is not in request scope.
Controllers are resolved within the scope created by BeginScope.

[DennisVM-D2i]

I just hit this issue. I was using context.Kernel.Get in a ToMethod binding and was very surprised to find that the services I produce this way are not in request scope but transient. Ended up using this code (thanks to @ItWorksOnMyMachine ) to create my own scope. I hope I did not break something

public static class CustomRequestScope
{
    public static Ninject.Syntax.IBindingNamedWithOrOnSyntax<T> InCustomRequestScope<T>(this Ninject.Syntax.IBindingInSyntax<T> syntax)
    {
        return syntax.InScope(ctx => HttpContext.Current.Handler == null ? null : HttpContext.Current.Request);
    }
}

I found (as a replacement for the above / in my case) that this variant seems to be working for me - as 'HttpContext.Current.Handler' always seemed to be set to 'null' (- so using 'HttpContext.Current' or 'HttpContext.Current.Request' instead);:

public static class CustomRequestScope
{
    public static Ninject.Syntax.IBindingNamedWithOrOnSyntax<T> InCustomRequestScope<T>(this Ninject.Syntax.IBindingInSyntax<T> syntax)
    {
        return syntax.InScope(ctx => HttpContext.Current != null ? HttpContext.Current.Request : null);
    }
}

But I do wonder if the use of 'HttpContext.Current.Request' should actually be replaced by 'HttpContext.Current' instead, inline with it's use in the 'OnePerRequestHttpModule' & 'DefaultWebApiRequestScopeProvider' classes (/files):

...\Ninject\Ninject.Web.Common-3.3.0\src\Ninject.Web.Common.WebHost\OnePerRequestHttpModule.cs

...\Ninject\Ninject.Web.WebApi-3.3.0\src\Ninject.Web.WebApi\DefaultWebApiRequestScopeProvider.cs

[DennisVM-D2i]

Is there any news on this issue?

It's not only related to providers. Request scope doesn't work at all when you're using Owin.
To reproduce:

1.) Create a Ninject web application as per: https://github.com/ninject/Ninject.Web.Common/wiki/Setting-up-a-OWIN-WebApi-application

2.) Create some custom type that you can bind, e.g.

public class TestService : ITestService { }
public interface ITestService { }

3.) Add request scope binding, e.g.

private static StandardKernel CreateKernel()
{
    var kernel = new StandardKernel();
    kernel.Load(Assembly.GetExecutingAssembly());
    kernel.Bind<ITestService>().To<TestService>().InRequestScope();
    return kernel;
}

4.) Test. For example:

app.Use((context, next) =>
{
    Debug.WriteLine(
        webApiConfiguration.DependencyResolver.GetService(typeof(ITestService)) == 
        webApiConfiguration.DependencyResolver.GetService(typeof(ITestService)));
    return next();
});

Output is "False", should be "True".

That's a show-stopper for us at the moment.

This is what I'm testing out and it seems to work. Instead of calling .InRequestScope() I'm now calling InCustomRequestScope. This seems entirely too easy. Anyone see a problem here? And if you don't have HttpContext.Current.Request you could just throw an if block into GetRequestScope and return the correct "scope managing object."

public static IBindingNamedWithOrOnSyntax InCustomRequestScope(this IBindingInSyntax syntax)
{
return syntax.InScope(GetRequestScope());
}
private static Func<IContext, object> GetRequestScope()
{
return new Func<IContext, object>(ctx => HttpContext.Current.Request);
}

Based upon my comment (/discovery) above, I wonder if you should be using 'HttpContext.Current' instead of 'HttpContext.Current.Request' (?).

[scott-xu]

I'm considering using IActivationBlock to adopt WebAPI's IDependencyScope. The only drawback is that it doesn't need "InRequestScope" declaring any more. All instances created within Controller are "RequestScope". Anyway, that's by design.

[DennisVM-D2i]

I'm considering using IActivationBlock to adopt WebAPI's IDependencyScope. The only drawback is that it doesn't need "InRequestScope" declaring any more. All instances created within Controller are "RequestScope". Anyway, that's by design.

Just be careful that you're not inadvertently hiding/running way from the problem that I seemed to encounter, whereby the switch between OWIN & ASP.NET (contexts) seemed to cause 2 different request scopes to be used - one per context, in other words the request scope was not been shared.

[scott-xu]

I'm considering using IActivationBlock to adopt WebAPI's IDependencyScope. The only drawback is that it doesn't need "InRequestScope" declaring any more. All instances created within Controller are "RequestScope". Anyway, that's by design.

Just be careful that you're not inadvertently hiding/running way from the problem that I seemed to encounter, whereby the switch between OWIN & ASP.NET (contexts) seemed to cause 2 different request scopes to be used - one per context, in other words the request scope was not been shared.

Thanks for the suggestion. I've closed the IActivationBlock PR and created #29. It will always use HttpContext.Current if it's available. https://github.com/ninject/Ninject.Web.WebApi/tree/Re-implement-IDependencyScope

[scott-xu]

Try Ninject.Web.WebApi 3.3.1 please.

[ISkomorokh]

Try Ninject.Web.WebApi 3.3.1 please.

Unfortunately, it doesn't help.

[scott-xu]

@ISkomorokh What’s the symptom? Would you mind sharing your package list and some code snippets?

[ISkomorokh]

@scott-xu sure.
It is an ASP.NET Web API 2 project.

packages.config

  <package id="Ninject" version="3.3.4" targetFramework="net46" />
  <package id="Ninject.Web.Common" version="3.3.2" targetFramework="net46" />
  <package id="Ninject.Web.Common.WebHost" version="3.3.2" targetFramework="net46" />
  <package id="Ninject.Web.WebApi" version="3.3.1" targetFramework="net46" />
  <package id="Ninject.Web.WebApi.WebHost" version="3.3.1" targetFramework="net46" />

I've a IStateContextProviderFactory bound to a StateContextProviderFactory in InRequestScope . The constructor accepts a couple of other dependencies. The factory is going to be used in some legacy parts of the project where DI isn't being used. Thus, it would be great to simplify the instantiation of the factory. Here is the idea:

internal sealed class StateContextProviderFactory : IStateContextProviderFactory {

        public static IStateContextProviderFactory Instance => (IStateContextProviderFactory)GlobalConfiguration.Configuration.DependencyResolver.GetService(typeof(StateContextProviderFactory));

}

I know, that it is a Service Locator anti-pattern, but still, I need it in order to simplify the instantiation in the legacy part of the app until it gets refactored.
Anyway, every time I access the Instance property, I get a new instance of the factory instead of getting the same instance in the request scope.

This blog post has a very useful passage:

GlobalConfiguration.Configuration.DependencyResolver.GetService(typeof(sometype)) as sometipe;

This is totally fine, except that this will create a new dependency scope for you (!), meaning that the UnitOfWork resolved like this inside a filter will be a different instance from the UnitOfWork resolved through the controller constructor injection.

In order to keep the dependency resolution in request scope we have to use a nice and useful, yet little know extension method on the HttpRequestMessage – GetDependencyScope(), more about which you can read up on MSDN.

Unfortunately, my code flow doesn't deal with HttpRequestMessage.

Hope this helps.

[scott-xu]

@ISkomorokh Try GlobalConfiguration.Configuration.DependencyResolver.GetService(typeof(IStateContextProviderFactory) please.

[scott-xu]

Fixed in Ninject.Web.WebApi 3.3.1