Inversifyjs: The isBound method from Container class is not checking parent on a child Container

[tiagoschenkel]

The Container class allows the creation of a child container. When a serviceIdentifier is defined on the parent container, it's possible to get the service from the child container, but strangely enough, it's not possible to check if the child container can resolve the same serviceIdentifier through isBound method.

A simple test case scenario is provided bellow, the last test will fail:

it("Should be able to check if serviceIdentifier is bound from child container", () => {
    @injectable()
    class Katana {}

    let weaponIdentifier = "Weapon";
    let container = new Container();
    container.bind(weaponIdentifier).to(Katana);

    let childContainer = container.createChild();

    // it is possible to get service from parent container
    expect(container.get(weaponIdentifier)).to.be.instanceOf(Katana);

    // it is possible to get service from child container
    expect(childContainer.get(weaponIdentifier)).to.be.instanceOf(Katana);

    // it is possible to check if the service if bound from parent container
    expect(container.isBound(weaponIdentifier)).to.be.equal(true);

    // but it is failing to check if the service is bound from child container - test will fail!
    expect(childContainer.isBound(weaponIdentifier)).to.be.equal(true);
});

Expected Behavior

The isBound method should check if the parent container could solve the serviceIdentifier when the child container can't solve it directly.

Current Behavior

The isBound method is only checking it's current own bindingDictionary, when it should also check the bindingDictionary from the parent container.

[remojansen]

@tiagoschenkel thanks for reporting this issue, would you like to send a PR or you want one of us to look at this?

[tiagoschenkel]

@remojansen, a quick solution for the issue can be find here:
Implements #611

Since is possible to get a service from a parent container, it should also be possible to check if the child container can solve the binding, directly or indirectly.

But before closing this issue, I would like to ask if the the methods getTagged and getNamed should have the same behaviour, that means, check if a parent can also solves the request. If yes, the methods isBoundNamed and isBoundTagged should also verify if a parent container can solve the binding.

But then, because I don't know all the details of the architecture of the binding system, I'm unsure if we can generalize this without breaking any previous feature or intended behaviour.

Also, I was doing some experiments with the isBoundNamed and isBoundTagged methods, but then I realised that I was generating performance problems. The time to create a binding was increased and was higher than 1 ms. Off course, since the method need to check also the parent, the time to solve the request will grow and will depend of how many leves are concatenated though the entire three. Checking all the three up will create performance issues, there is no other way around.

Let me know if we can quickly solve this issue, or if we should discuss also the behaviour of other similar methods.

[remojansen]

Hi thanks for all the details, I think both isBoundNamed and isBoundTagged should check as well in parent/child containers. We can try to find ways to increase the binding performance (the lookup is not using a particularly fancy algorithm at the moment). However, I don't think to register a binding should be extremely fast. The important thing is to resolve bindings very fast.

Your changes look good :+1:

[theodesp]

tiagoschenkel Your changes look good. I think it's important to give readability some credit first and then look for alternatives in terms of performance. Maybe we need to warn if we detect a deep hierarchical system of containers.

[tiagoschenkel]

There is this comment above the isBoundTagged:

// Note: we can only identify basic tagged bindings not complex constraints (e.g ancerstors)
// Users can try-catch calls to container.get<T>("T") if they really need to do check if a
// binding with a complex constraint is available.

This method throws a error if the serviceIdentifier is not directly available from the binding dictionary of the current container. At least for me, add a try / catch statement only to prevent this erros doesn't look to be a proper solution.

As far as I understood, the isBoundTagged method get a list of services that could be returned from the serviceIdentifier parameter, then check if one of these bindings could also match the key and value parameters. These parameters are used as a filter.

But then, if the current container can't solve the binding, we need to check if the parent can solve it, but then a try / catch statement is needed because we only know that the current container can't solve it if a error is generated.

@remojansen, is there another way to know if the current container can solve the binding without force the generation of a error?

[tiagoschenkel]

I implemented a way to solve the issue without trowing a error. The isBound methods are checking if a parent container can also sove the request.

I already created a pull request

[remojansen]

PR merged and 4.3.0 released :tada:

[jpsfs]

Hi!

Having the isBound method checking the parent seems a great feature to me (to be honest I thought it already worked like this).

Now the issue is that unbind only does this for the current container, and throws an error if there is nothing to unbind. With this change, the following pattern doesn't work:

        if (container.isBound("a")) {
            container.unbind("a");
        }
        container.bind("a").toConstantValue(entity);

Any idea?