Fix: containers with common collaborator share resolved instances for their own definitions

[ilyapuchka]

Resolves #168

[jtwigg]

@ilyapuchka This fixes the original test where there's two isolated containers but the following test fails.

a) There is a rootContainer b) There are two isolated child containers, each collaborates with root. i ) not logged in has a valid registry of Password ii) logged in has NO valid registry

When resolving password on both containers, the first one should pass and the second resolution should fail BUT

The two child containers are leaking into each other. The second container has access to the first containers registry.

  class ServiceA {}

  class Password {
    let text: String
    let service : ServiceA

    init(text:String, service: ServiceA) {
      self.text = text
      self.service = service
    }
  }

  func testIsolationContainer2() {

    let rootContainer = DependencyContainer()

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

  //NOTE: Nothing is registered
    let loggedIn = DependencyContainer()

    let unloggedIn = DependencyContainer()
    unloggedIn.register() { Password(text: "-none-", service:$0) }

    // NOTE: Isolated containers
    loggedIn.collaborate(with: rootContainer) //Isolated containers
    unloggedIn.collaborate(with: rootContainer)//Isolated Container

    let passwordA = try! unloggedIn.resolve() as Password

    XCTAssert(passwordA.text == "-none-")
    XCTAssert(count == 1) ////<< Works

    let passwordB : Password? = try? loggedIn.resolve() as Password

    XCTAssertNil(passwordB) //<<<< FAILS. Should be nil,  but its "-none-"
    XCTAssert(count == 1)
  }

[jtwigg]

@ilyapuchka Here's a second failure in the tests you applied

Root Containers should not have access to their children's registry

I added a check in the test you created:

  func testThatContainersShareTheirSingletonsOnlyWithCollaborators() {
    let container = DependencyContainer()
    container.register(.singleton) { RootService() }

    let collaborator1 = DependencyContainer()
    collaborator1.register(.singleton) {
      ServiceClient(name: "1", service: $0)
    }

    let collaborator2 = DependencyContainer()
    collaborator2.register(.singleton) {
      ServiceClient(name: "2", service: $0)
    }

    collaborator1.collaborate(with: container)
    collaborator2.collaborate(with: container)

    //Root client should not have access to its childrens services
    let rootClient = try? container.resolve() as ServiceClient
    XCTAssertNil(rootClient) //<<<<<< FAILS

    let client2 = try! collaborator2.resolve() as ServiceClient
    let client1 = try! collaborator1.resolve() as ServiceClient

    XCTAssertEqual(client1.name, "1")
    XCTAssertEqual(client2.name, "2")
    XCTAssertTrue(client1.service === client2.service)
  }

I believe this is do to the change you made where adding a collaborator to yourself ALSO forces that to collaborate with you.

public func collaborate(with containers: [DependencyContainer]) {
    _collaborators += containers
    for container in containers {
      container._collaborators += [self] //<<<<<<<<<<< Might not be correct
      container.resolvedInstances.sharedSingletonsBox = self.resolvedInstances.sharedSingletonsBox
      container.resolvedInstances.sharedWeakSingletonsBox = self.resolvedInstances.sharedWeakSingletonsBox
      updateCollaborationReferences(between: container, and: self)
    }
  }

[ilyapuchka]

Thanks for adding those test cases, I'll take a look later. Regarding root container having access to others - it is by design. It's not parent-child relationship, collaboration is bidirectional.

[jtwigg]

I agree, you did make it bidirectional in this commit, e7d8fb41e1c8a852c603d7227ddc5f4f2bc1f816 but I think that means we'll never have isolated containers.

So I can see now that collaborations has a completely different use case. The wiki talks about Modules (good) but its clear now you didn't include isolation as a property of a module, since its bi directional. That is something I was expecting and rather counting on.

My question stands: https://github.com/AliSoftware/Dip/issues/168 How do you intend people to create isolated hierarchical containers? Or is that simply not supported?

[ilyapuchka]

My vision on that is that you can not have everything (or we need to design a new feature). If your container collaborate their are connected into a bidirectional graph, and library goes around this graph to resolve instance. In in this graph there are several ambiguous definitions and there is a rout to both of them - there is no way to say what variant will be used (it depends on the order of containers references). This PRs solves issue when collaboration was attempted instead of trying to resolve using called container, and that instances were shared even though they don't need to, but it does not solve ambiguities (i.e. different shared instances will be reused depending on the order in which collaborators resolve them, first wins now).

In your case I would suggest either to use named definitions (for that you will need to store tag that represents state of your app, like loggedIn and notLoggedIn) and use tag to disambiguate your requests at runtime, or do not keep both containers around at the same time, but this will make you to recreate containers graph. We can also have a method to stop collaboration which can make this task easier. And somehow make bidirectional connection opt in instead of default.

Also if you don't want collaboration to happen and return instance that you do not expect don't resolve component using container that did not register it. And you can always fallback to explicit calls to other containers from within registered factories instead of collaboration. Probably for complicated containers graphs its the best option.