Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overloaded bindings in hierarchy of assemblers are resolved in a incorrect way #518

Open
ursusursus opened this issue Sep 5, 2022 · 0 comments

Comments

@ursusursus
Copy link

ursusursus commented Sep 5, 2022

Hi, nazdar,

I make use of the parent child assemblers, which works great to create semantical scopes.

However one thing that is unexpected/dangerous is how unnamed overloaded bindings are resolved, when in a hierarchy

Say I have a alamofire Interceptor registered in app scope, which provides a common middleware for attaching user agent etc
container.register(Interceptor.self, name: "app") { _ in Interceptor([ some common interceptors ])

This is then wrapped by RequestHelper as to not expose alamofire types

protocol RequestHelper

final class RequestHelperImpl : RequestHelper {
   private let interceptor: Interceptor
}

...
container.register(RequestHelper.self) { r in
   RequestHelper(r.resolve(name: "app")
}

Then a feature 1 has a api client, whose logical scope is app scope
container.register(Feature1ApiClient.self) { r in FeatureApi1Client(requestHelper: r.resolve(RequestHelper.self)

This seems to work fine

Then user scope is added, which obviously needs a way to authenticate the request, while inheriting the app scope behaviour (interceptor)

container.register(Interceptor.self, name: "user") { r in
   let parentInterceptor = r.resolve(Interceptor.self, name: "app")
   return Interceptor([parentInterceptor, AuthenticationInterceptor()
}

container.register(RequestHelper.self) { r in
   RequestHelper(r.resolve(name: "user")
}

Now comes feature 2 which is in the user scope
container.register(Feature2ApiClient.self) { r in FeatureApi2Client(requestHelper: r.resolve(RequestHelper.self)

And the assemblers are instantiated like this (so AppScope is parent to UserScope)

let appScopeAssembler = Assembler([Feature1Assembly()], parent: nil)
let userScopeAssembler = Assembler([Feature2Assembly()], parent: appScopeAssembler)

Ooookay, now, the issue is, that both RequestHelpers are registered anonymously, and this I believe should be okay, as both are NOT reqistered in the same scope but different scopes - yes, app scope is parent to user scope, so user scope technically sees 2 bindings, but, the "scope local" should be the first choice - and it mostly behaves in this way, given the sources

fileprivate func getEntry(for key: ServiceKey) -> ServiceEntryProtocol? {
    if let entry = services[key] {
        return entry
    } else {
        return parent?.getEntry(for: key)
    }
}

The unexpected part comes, when user assembler.resolve is used to resolve a app scope binding, which again, is all legal
let feature1ApiClient = userScopeAssembler.resolve.resolve(Feature1ApiClient.self)

HOWEVER the RequestHelper instance of FeatureApi1Client(requestHelper: r.resolve(RequestHelper.self) is the one registered in user scope, not app scope as indended!

Obviously this all could be alleviated by naming the RequestHelper bindings, however this then breaks encapsulation, and which RequestHelper instance is uses should only be dictated by where the XAssembly is added to.

I did concede in naming the Interceptor bindings as there I need to explicitly as for parent Interceptor, and this only happens once per scope, so total of 2, which I could live with.

But I'm not keen on breaking the encapsulation everywhere as there are going to be 10s of features per scope, and paying such close attention on which scope the instance is resolved is very error prone.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant