AliSoftware / Dip

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

Autowiring should prioritize the definition with the matching tag #153

Closed nmarkovic04 closed 7 years ago

nmarkovic04 commented 7 years ago

Problem:

// Add interactor
container.register(.shared) { (entryDataAdapter: EntryDataAdapter, dateFormatter: DateFormatter) -> AbstractAddEntryInteractor in
   return AddEntryInteractor(entryDataAdapter: entryDataAdapter, dateFormatter: dateFormatter)
}.resolvingProperties({ (container, interactor) in
   interactor.presenter = try! container.resolve(tag: container.context.tag)
})

// Same module, different interactor; tag "editEntry"
container.register(tag: "editEntry", factory: { (selectedEntryID: Int, entryDataAdapter: EntryDataAdapter) -> AbstractAddEntryInteractor in
    return EditEntryInteractor(selectedEntryID: selectedEntryID, dataAdapter: entryDataAdapter) as AbstractAddEntryInteractor
}).resolvingProperties({ (container, interactor) in
   interactor.presenter = try! container.resolve(tag: container.context.tag)
})

// this is where interactor gets resolved
container.register(.shared) { () -> AbstractAddEntryPresenter in
   return AddEntryPresenter()
}.resolvingProperties({ (container, presenter) in
   presenter.interactor = try! container.resolve(tag: container.context.tag)
   presenter.view = try! container.resolve(tag: container.context.tag)
   presenter.routing = try! container.resolve(tag: container.context.tag)
})

Autowiring fails with DipError.ambiguousDefinitions while resolving graph with "editEntry" tag because there are multiple definitions with different arguments, specifically this line

// AutoWiring.swift, line 67
if definitions.count > 1 && definitions[0].key.typeOfArguments != definitions[1].key.typeOfArguments

even though only one definition matches the tag, so the instance should correctly get resolved.

ilyapuchka commented 7 years ago

Hi @nmarkovic04! Thanks for your contribution! Looks like there is an issue in this case indeed, thanks for pointing it out! While your fix solves it, I think it should be address in a different way. Currently autowiring happens in one pass and it searches for definitions either matching tag or with nil tag (which we use as a fallback) and selects one with most number of arguments. When tag is used it looks like this strategy can lead to other cases when wrong definition is used.

Basically auto-wiring should happen in two passes, first should search for definitions for resolved type strictly matching tag, second (if it's needed, when tag is not nil) should search for definitions for resolved type with nil tag. This way it will be more inline with a fallback strategy used in "normal" resolution flow.

So I will close this and will do PR for release/5.1 shortly Thanks again for reporting!