Pull Request for Container onActivation

[tonyhallett]

Description

Added onActivation method to the Container class. This will add the handler to all the bindings that match the filter and optionally will do the same for ancestor containers. Default behaviour is to wrap existing handlers and there is an option to overwrite them.

Related Issue

#471

Motivation and Context

As per #471, where there is common onActivation options are to store the syntax in array and iterate or to use middleware. This enables doing so with a single method call.

How Has This Been Tested?

TDD - checking that all/some of the Binding instances in the Container binding dictionary have had the onActivation handler present whether as is or wrapped with existing. The filter too has been tested. Tested on windows machine with current inversify test setup - mocha, sinon and chai. Change does not affect other code.

Types of changes

• Updated docs / Refactor code / Added a tests case (non-breaking change)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have updated the changelog.

I will happily update the docs if the pull request is accepted.

My Container code is using a private function getBindingValue. I think this should be a public method of the Binding interface but I did not want to change that api without agreement. Perhaps Binding should change so that there are not separate properties for the different types.

[parisholley]

just noticed this PR as I was making some tweaks to my fork/branch for Async/Promise support (#1074) which includes a follow up for async onActivation (#1086)

one of the advantages to my implementation that differs from this is the ability to act more like middleware and allow you to register onActivation for things that have not been bounded yet.

there are no tests associated with this PR, but I assume it would not be compatible with autoBindInjectable:true (which generates bindings at runtime based on the class requested).

[tonyhallett]

There are tests. You are correct though that autoBindInjectable is not supported.

[parisholley]

you right, i need more sleep 😃

[tonyhallett]

Code now covers autoBindInjectable.
@parisholley I have looked at your code and it is a different use case so I would expect that both ideas could exist together.

Mine is a shortcut to apply onActivation to multiple bindings that pass a filter ( not service identifier dependent ), potentially for bindings in ancestor containers.

Aside from the async nature my understanding of your onActivation is:

Register for onActivation for any binding with specified service identifier.
Order of onActivation calls :
Specified on the binding ( with the fluent syntax )
Top most container - all that have been applied through container.onActivation
Descend down containers and use their activation handlers - if current container ( starting with top most ) does not own the binding.

[parisholley]

correct, i believe the order in which onActivation/onDeactivation is important when you consider the implications of certain patterns (eg: transactions). if one part of code base is closing connections before other activators/deactivators fire, it can be a problem. i think allowing people to change the order of operations is more appropriate for middleware (which has certain "brute force" implications), whereas the simpler API should be very restrictive to prevent people from shooting themselves in the foot.

i also went with the route of keeping the semantics (based on name) the same between the binding onActivation and global. I would vote for using a different name if the container/global based approach uses a different signature/filtering.

i did a refactor of all my PRs into a much simpler implementation that does not change the core code to much so it should be easier to grok: #1132

my two cents on this PR would be to structure more like a simpler version of middleware, than a onActivation implementation

[dcavanagh]

@tonyhallett can you squash the commits to separate the features / fixes that you added? thanks

[tonyhallett]

@dcavanagh Git is not my strong point. I assume that you mean - for each pull request - pull commits made by others then rebase squash so there is a single commit ? Is this something that you can do ?

[jakehamtexas]

@tonyhallett @dcavanagh Just trying to clean up the PRs list in this repo if possible, please reopen if I closed this in error. It looks outdated to me.