AliSoftware / Dip

Simple Swift Dependency container. Use protocols to resolve your dependencies and avoid singletons / sharedInstances!
MIT License
978 stars 75 forks source link

Singleton not honored when collaborating containers #167

Closed jtwigg closed 7 years ago

jtwigg commented 7 years ago

Help! I've found what I believe to be a nasty bug with Collaborating containers and singleton registrations.

If I have a registration of ServiceA in a rootContainer as a singleton registration and I a) Resolve the ServiceA from the rootContainer b) Make a child container the collaborates with the rootContainer c) resolve ServiceA for a second time with rootContainer

The second resolve of ServiceA is a new instance. I think the call to collaborate with rootContainer wipes the registry clean!

  class ServiceA {}

  func testSingletonNotHonored() {
    var count = 0

    let rootContainer = DependencyContainer()
    rootContainer.register(.singleton) { () -> ServiceA in
      count = count + 1
      return ServiceA()
    }

    let s1:  ServiceA = try! rootContainer.resolve()//<<<< Aquire the service

    let childContainer = DependencyContainer()

    childContainer.collaborate(with: rootContainer) // <<< Collaborate with root service

    //NOTE: I believe the root service is now broken. Its forgotten about its
    //singleton registration of ServiceA

    let s2:  ServiceA = try! rootContainer.resolve()

    XCTAssert(s1 === s2) //<<<<<< FAils
    XCTAssert(count == 1) //<<<<< FAILS
  }

NOTE: If you resolve the ServiceA twice After the collaboration, the singleton is honored.

This is important to our team because we want to make modules that are isolated from each other, but have access to root container singletons and this is the only approach that can do this.

I even modified your Collaboration Wiki example to show that the DataStore singleton can be broken.

jtwigg commented 7 years ago

Digging through the collaboration code, I'm worried that this whole approach is never going to work. Whenever anyone collaborates with another container, it a) overrides all its resolved instances in ...

public func collaborate(with containers: [DependencyContainer]) {

(which is almost certainly the bug I'm seeing)

b) It then recurses into the collborators->collaborators and over rights its resolved instances too. It forces them to use the same 'singletonsBox'. This is a sort of coerced collaboration.

An example of this is in your wiki example https://github.com/AliSoftware/Dip/wiki/containers-collaboration

The following collaboration match up

eventsListModule.collaborate(with: addEventModule, rootContainer)
addEventModule.collaborate(with: eventsListModule, rootContainer)

can be replaced with

eventsListModule.collaborate(with: rootContainer)
addEventModule.collaborate(with: rootContainer)

and now eventsListModule & addEventModule are coerced into collaborating with each other.

I would expect the test to fail, but it passes.

ilyapuchka commented 7 years ago

You should first setup your containers and only then resolve.

jtwigg commented 7 years ago

So be it: I'm opening a question regarding child containers as a result.