Parent child container support for Dip

john-twigg-ck commented 7 years ago

Support for Parent Child containers

First off, I have to say I love Dip, and you've done an amazing job at the API. I heavily vetted Dip along with Swinject, Cleanse and a few other containers. I've used Unity .Net for over a decade, Guice for years. But yours was perfect for us especially around syntax.

Motivation

This PR takes its inspiration from (Dagger Subcomponents)[https://google.github.io/dagger/subcomponents]

The motivations is to provide support for Container to have a hierarchy, such that some containers can be denoted as Parent containers and others as Child and sibling containers.

Child containers will be isolated from each other, and parent containers will be isolated from the child containers.

Rules

Parent containers cannot directly access the dependencies of a child container.
Child containers can only have one single parent
Child containers can access all the dependencies of a parent container, but not any of the dependancies of a sibling container.
Reuse : Dependencies that are resolved in a parent container are reused during the lifecycle of the overall resolve() invocation, similar to existing behavior
Overriding: All nested calls to resolve start over with the container with whom the resolve call was invoked If both a child and a parent include a registry, the child's registry will always be attempted first, failing up the hierarchy. EVEN if the dependency is auto-wired from within a parent class, all new resolve calls are re-initiated with the child container from which the call originated.
Collaboration containers are attempted first, before parent containers.

Changes to existing behavior

Much effort was made to maintain and only extend the existing behavior, however there were knock on effects with respect to resolving ambiguities that I believe are an overall improvement, making resolution more deterministic and easier to reason about.

Attached is a screenshot of a Unit Test I added before/after this change that focuses on Collaborators.

Before: I believe the collaboration of the containers prior to this change were somewhat ambiguously resolved. In the example resolving the dependency from either collaborated container would bias the results towards the container that contained the AutoWire. In fact order of associating collaborators can have a big impact on the results.
After: With the addition of the behavior that all nested resolves are reinitiated with the originating container, I believe the behavior is now more deterministic and predictable. In this example, the container2 has registered a SerivceImpl2 under its Service protocol. When you resolve using conatiner2, anything requesting a Service will receive a ServiceImpl2 implementation which seems reasonable since you just went to the trouble of registering it and requesting it.

Further Improvements

Perhaps we should restrict the mixture of collaboration and parent child relationships.

ilyapuchka commented 7 years ago

@john-twigg-ck thanks for your work! I think if we go with this we should deprecate collaboration in favour of parent-child. @jtwigg can you please look if this implementation will solve the issues you had with collaboration?

john-twigg-ck commented 7 years ago

@ilyapuchka @jtwigg and @john-twigg-ck are in fact the same people! Sorry for the confusion. I launched this PR from our corp account.

It does solve the problems I had with collaboration.

ilyapuchka commented 7 years ago

I thought so 😂

jtwigg commented 7 years ago

@ilyapuchka I'll try and maintain come consistency here from this account.

So thats great news that you are in favor of the change.

If you want to deprecate collaboration then a few points need to be discussed

1) There's the possibility that a child could have multiple parents. There's nothing excluding it. I would avoid using it in my implementations because it would degrade my ability to understand how the dependancies are being resolved, but it would open up ability to aggregate containers together to create a behavior. You'd have order sensitive initialization.

//Always tries rootContainerFirst
let container  = DependencyContainer(parents: [rootContainer,sideEffectContainer])

2) I think you should consider creating an actual "Module" concept, since thats what collabs were trying to do at some level. Here. Swinject Assemblies or Guice Modules actually do a really good job of this and I'm using this concept in our production app. Something like

let rootContainer = DependancyContainer(module: [RootModule(), LoginModule(),AdsModule()])
let childContainer = DependancyContainer(module : [ChildModule()],parent: rootContainer)

class RootModule : Module {
     register(container: DependancyContainer ) {
         container.register(.singleton) { ...} 
     }
}

3) Getting more specific, I think that your

self.resolvedInstances.resolvedInstances 
self.resolvedInstances.resolvableInstances

should actually be

self.context.resolvedInstances 
self.context.resolvableInstances

since context shares the lifecycle of the resolve() call and resolvedInstances shares the lifecycle of the container (good for singletons). Thoughts?

ilyapuchka commented 7 years ago

I'll be able to get back to this and really discuss this only on a weekend, unfortunately, so keep this thought =)

jtwigg commented 6 years ago

@ilyapuchka Awesome. Thanks for merging. I'll post another PR with our latest updates and fixes. We've been using it in anger exponentially more over the last half year.

AliSoftware / Dip