Updated to support WebApi RC

[azzlack]

A lot of fixes.
You should look at the changes and see if it suits your plans before merging. :-)

[remogloor]

Unfortunately your dependency resolver implementation is not correct. That way all object are singletons within the scope. What you want is that just objects InRequestScope are affected by the scope.

[azzlack]

Ok. I'll fix that and do another pull request. (And clean up the commits while I'm at it)

What about the nuget package restore, iisexpress and line endings commits? Should I leave them out?

[azzlack]

Fixed. I noticed there is another pull request that basically does the same thing. (Probably looked at the same articles) :-)

[niemyjski]

Whats the current status on this pull request?

[azzlack]

Status is that no-one seems to know where Remo Gloor is... :S

[sirkirby]

I'm hoping this gets accepted and pushed to nuget...I've ended up rolling my own, but I would certainly prefer using this package.

[azzlack]

I already posted it up on NuGet while I wait for Remo to release a proper version.

https://nuget.org/packages/Ninject.Web.WebApi-RC

[danielmarbach]

Hy

Remo got married two weeks ago and is currently on a honeymoon trip. I'm in vacation too (althoug not marriage related). Maybe Ian can merge this. I will contact him.

Daniel

Am 30.08.2012 um 09:48 schrieb Ove Andersen [email protected]:

I already posted it up on NuGet while I wait for Remo to release a proper version.

https://nuget.org/packages/Ninject.Web.WebApi-RC

—
Reply to this email directly or view it on GitHub.

[azzlack]

Cool! Congratulations to Remo then! :-)

Would be awesome to have an official release of the Ninject WebAPI package.

[danielmarbach]

I can understand! I have the rights to merge, Ian and others also. Releasing is Remo's job :( but I'm only occasionally online and have no time to do it properly. But I contacted Ian...

Am 30.08.2012 um 10:13 schrieb Ove Andersen [email protected]:

Congratulations to Remo then! :-)

Would be awesome to have an official release of the Ninject WebAPI package.

—
Reply to this email directly or view it on GitHub.

[idavis]

I'll try to review and integrate this weekend.

[remogloor]

I don't understand that impl either. I'd stick with the dependency resolver code from the previous version for that part.

@idavis I expect not to have any Internet connection before Sunday and no regular connection for two weeks more.

[remogloor]

The kernel is the correct resolution root at least for IIS hosting
because that way the same bindings can be used for MVC and WebAPI.

When it comes to self hosting things change quite a bit. I was working
on that before my vacation. I have a working solution but it still
needs some time to finish mainly because of NuGet packageing as there
will be a base, iis hosting and a self hosting package for most of the
web extensions in future.

Also this solution won't be backward compatible to this pull request.
But since it will take at least three weeks until I am ready I'm fine
with releasing this as beta.

[bradwilson]

The scoping mechanism is a cleanup system. The request-local scope created by Web API is how the controller (and all its dependent services) will get resolved; the scope will be disposed when the request is completed.

[azzlack]

@idavis Yeah. I noticed now the comment is wrong in the Dispose function. Line 35 should have been like this: "We don't want Ninject to dispose the kernel, as .NET handles this for us".

Also, line 51 should have been removed.

[azzlack]

@idavis The implementation that @PProvost made looks like it works, but unfortunately makes all objects singletons within the scope. (As noted by @remogloor => #1 (comment)).
I tried using that implementation together with the log4net ninject package, and can confirm his comment.

[azzlack]

@idavis As for swallowing the exception, I think I agree with you. I've updated the fork in that regard.

[bradwilson]

As for exceptions, the contract of the dependency resolver and dependency scope interfaces it that they should NEVER throw. GetService() should return null when it cannot find a service, and GetServices() should return a non-null empty collection when it cannot find any services.

[idavis]

@azzlack, based on @bradwilson's and @remogloor's advice, I think we need to change the NinjectDependencyScope to:

    public class NinjectDependencyScope : IDependencyScope
    {
        private readonly IResolutionRoot resolutionRoot;

        internal NinjectDependencyScope(IResolutionRoot resolutionRoot)
        {
            Contract.Assert(resolutionRoot != null);

            this.resolutionRoot = resolutionRoot;
        }

        public void Dispose()
        {
            // Do nothing. 
        }

        public object GetService(Type serviceType)
        {
            try
            {
                return this.resolutionRoot.Get(serviceType);
            }
            catch (Exception ex)
            {
                return null;
            }
        }

        public IEnumerable<object> GetServices(Type serviceType)
        {
            try
            {
                return this.resolutionRoot.GetAll(serviceType);
            }
            catch (Exception ex)
            {
                return new object[0];
            }
        }
    }

[danielmarbach]

You can use TryGet

Daniel

Am 06.09.2012 um 17:47 schrieb Ian Davis [email protected]:

@azzlack, based on @bradwilson's and @remogloor's advice, I think we need to change the NinjectDependencyScope to:

public class NinjectDependencyScope : IDependencyScope
{
    private readonly IResolutionRoot resolutionRoot;

    internal NinjectDependencyScope(IResolutionRoot resolutionRoot)
    {
        Contract.Assert(resolutionRoot != null);

        this.resolutionRoot = resolutionRoot;
    }

    public void Dispose()
    {
        // Do nothing. 
        // We don't want Ninject to dispose the kernel, as .NET handles this for us
    }

    public object GetService(Type serviceType)
    {
        try
        {
            return this.resolutionRoot.Get(serviceType);
        }
        catch (Exception ex)
        {
            return null;
        }
    }

    public IEnumerable<object> GetServices(Type serviceType)
    {
        try
        {
            return this.resolutionRoot.GetAll(serviceType);
        }
        catch (Exception ex)
        {
            return new object[0];
        }
    }
}

—
Reply to this email directly or view it on GitHub.

[idavis]

@danielmarbach TryGet only catches ActivationException and doesn't have a non-generic version IIRC. Also ResolutionExtensions::GetAll doesn't follow the contract of returning an empty collection for GetServices should an exception occur.

[danielmarbach]

Have you looked at

https://github.com/NServiceBus/NServiceBus/tree/master/src/impl/ObjectBuilder/ObjectBuilder.Ninject

ChildContainer also also like a dependency scope. The ninject child container implements DisposeNotifyingObject

Daniel

Am 06.09.2012 um 18:01 schrieb Ian Davis [email protected]:

@danielmarbach TryGet only catches ActivationException and doesn't have a non-generic version IIRC. Also ResolutionExtensions::GetAll doesn't follow the contract of returning an empty collection for GetServices should an exception occur.

—
Reply to this email directly or view it on GitHub.

[azzlack]

@idavis Yup. Fixed in new commit. :-)

[idavis]

@danielmarbach @bartelink Can anyone else verify that this code is working correctly? I'll continue to review other parts of the pull.

[bartelink]

@danielmarbach @idavis Sorry - can't add anything; I hope Daniel can as I don't have a ninject dev space rigged up at the moment (Though I'll need it for work in a few weeks at which time I'll be making the time no matter what :D)

[bradwilson]

If TryGet is avoiding the throwing of some exceptions, I'd still use it, even though you need try/catch for the others (purely for performance reasons). If, on the other hand, TryGet is just doing a try/catch itself to mask an unpreventable exception, then it's not worth bothering with.

[danielmarbach]

    private static T TryGet<T>(IEnumerable<T> iterator)

    {

        try

        {

            return iterator.SingleOrDefault();

        }

        catch (ActivationException)

        {

            return default(T);

        }

    }

From: Brad Wilson [mailto:[email protected]]
Sent: Freitag, 7. September 2012 21:08
To: ninject/Ninject.Web.WebApi
Cc: danielmarbach
Subject: Re: [Ninject.Web.WebApi] Updated to support WebApi RC (#1)

If TryGet is avoiding the throwing of some exceptions, I'd still use it, even though you need try/catch for the others (purely for performance reasons). If, on the other hand, TryGet is just doing a try/catch itself to mask an unpreventable exception, then it's not worth bothering with.

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

https://github.com/notifications/beacon/J6T91GIPIyhU-8ti4GCGP48lRAJ2JmVnij8EsBH6WB6wXk9sHvTVARdxxpAGIIxm.gif

[azzlack]

@Idacos I 've been using this code in test and production for 4 weeks, if that is any proof that it works :-)

[danielmarbach]

Hy together
I had time to look a bit into it. I would rather go for a seperate class for the NinjectDependencyScope or a nested private class. I like the implementation of http://www.strathweb.com/2012/05/using-ninject-with-the-latest-asp-net-web-api-source/ And there are also many other commits going on in this pull requests (including line ending stuff). I recommend against taking in the pull request and reimplement the changes.

Daniel

[remogloor]

@bradwilson I question the general rule not to throw any exceptions a
bit. So far our implementation of the DR was to return null or an
empty enumerator when we do not know about the requested service e.g.
some MVC interface like IControllerFactory But if we do know about it
but something goes wrong e.g. a dependency is not correctly configured
or an exception occurs in the constructor then we throw. This makes
far more sense imo as those exceptions are bugs and give the
developpers hints what is wrong. By swallowing those exceptions we
just make it harder to find and fix those problems.

[bradwilson]

There's really nothing to be done about it now, since the contract is
already fixed by the Web API RTM.

Note that throwing exception during app startup -- which is the most likely
time for it happen for the services consumed by Web API -- are extremely
hard to debug. Feel free to throw exceptions if you think it's going to be
useful, but I'm extremely doubtful, based on past experience. Also don't be
surprised if we throw up in interesting ways when you violate the contract.
:-p

[remogloor]

@bradwilson In that case I'd really rethink the contract for next
version. I doubt that it is much more easy to debug if someone
configured a service in the IoC container but the IoC container
returns null because it can't build up the service because some other
configuration is missing which makes the WebApi using the default
implementation.

[tbertenshaw]

Hi Guys,
got a bit lost in all those updates. Is the situation that it needs a fair bit of work prior to a official release? or is an official release possible soon.

cheers
Tim

[angularsen]

I'm using this branch with MVC4 web api with great success, thanks a lot.
A few changes I made:
NinjectDependencyScope.cs lines 54-67:

1. Remove try-catch.
2. Replace this.resolver.Get(serviceType) with this.resolver.TryGet(serviceType)

I was getting a lot of first-chance exceptions due to MVC assemblies not being resolved by ninject. I believe TryGet() is preferred here instead of swallowing the exception this way.

I sure hope an MVC4 web api feature is release for ninject soon. Thanks for the great work guys.

[remogloor]

Added support for MVC4 final in c0b1a97

[niemyjski]

Any ideas when the NuGet package will be updated :D?

[remogloor]

After updating docu, testing, ....

So long you can get a beta version from the teamcity server nuget feed:

https://teamcity.bbv.ch/httpAuth/app/nuget/v1/FeedService.svc/