Swinject / SwinjectAutoregistration

Swinject extension to automatically register your services
MIT License
249 stars 45 forks source link

Allow resolution via a protocol for a service #88

Closed esilverberg closed 3 months ago

esilverberg commented 1 year ago

Our company makes heavy use of Swinject and SwinjectAutoregistration with Quick and Nimble.

Problem: repeated boilerplate service resolution when using protocols

When we write tests, we found we used this pattern frequently:

var container: Container!
var mockApi: MockApi!

beforeEach {
  container = Container().customInjectionExtensionCode()
  mockApi = ((container ~> SomeApiProviding.self) as! MockApi)
}

justBeforeEach {
  // call some event that triggers the API
}

context("something") {
  beforeEach {
    mockApi.result = .success(SomeDomainModel.fixture)
  }
}

The code to resolve the MockApi via its protocol was repeated throughout our code.

Proposal: a new operator

Instead of calling:

mockApi = ((container ~> SomeApiProviding.self) as! MockApi)

Now, you can call:

mockApi = container ~~> SomeApiProviding.self

This PR uses type specifiers to determine the assigned type via the infix ~~> operator and cast the return value accordingly.

tkohout commented 1 year ago

Thanks a lot for your contribution!

There are two reasons why I think it might not be a good idea to add this to the repo:

1) ~~> is easily confused with ~> and it's hard to see what it will do just from looking at the code. Also this is only needed when testing, so it shouldn't be in production code. 2) Since you can't constraint that the mock conforms to the resolved protocol it is inherently unsafe. It's ok in testing code, but there's a potential that someone will misuse in production code and cause a crash.

esilverberg commented 1 year ago

Thank you for your quick reply! I understand that ~~> is syntactically too similar to ~>.

I would submit that, in any large-scale project, a significant % of the developer interactions with Swinject are happening inside of BDD Quick/Nimble tests, where a developer is retrieving various UseCase/Logic classes, repositories, and APIs, and then confirming the expected effects, as you can see in my example above.

Are there any other ways I can use SwinjectAutoregistration that would enable me to avoid continually typing out the boilerplate: ((container ~> SomeApiProviding.self) as! MockApi) as I configure my mocks?

bradfol commented 3 months ago

Could you also include a bit more information about your registration set up? It's not entirely clear to me why resolving MockApi directly doesn't work for your situation?

Somewhat separate question: Is there any limitation you are running into where this custom operator needs to exist within SwinjectAutoregistration (rather than just within your own app's repo)?

esilverberg commented 3 months ago

Let me answer the second question first: we do not need to fork this code; we could have also added this change as an extension locally in our own project.

Now for additional data about our registration setup:

Our app is implemented with MVVM using a series of Swift Packages, but regardless of architecture, any app is going to have 3rd party dependencies that require API keys and make network requests. Think:

etc, etc

When writing tests, generally we are not actually wishing to trigger a real request to these services. They may be slow, cost money, or are otherwise software that we don't feel needs unit/journey testing. One of my teammates wrote a blog post summarizing our testing approach here; it's focused on Android but applies equally to iOS.

We implement interfaces in front of every one of these SDKs/services. Often we will just copy the form of protocol supplied by the SDK; sometimes we will adapt the interfaces to Combine. Regardless, when writing unit tests of things like ViewModels, we will use Swinject to register mocked versions of each of our protocols that sit in front of these SDKs/services. In our tests, we will (typically) want to confirm that a specified third-party API actually got triggered when a user took an action.

Here's a simple protocol for playing sounds on iOS: SoundPlayerProviding

public protocol SoundPlayerProviding {
    func register(url: URL, sound: UnsafeMutablePointer<UInt32>)
    func play(sound: UInt32)
    func vibrate()
}

We have a FrameworkProvider package that contains the following class, that implements this protocol as follows:

public final class SoundPlayerProvider: SoundPlayerProviding {
    public func register(url: URL, sound: UnsafeMutablePointer<UInt32>) {
        AudioServicesCreateSystemSoundID(url as CFURL, sound)
    }

    public func play(sound: UInt32) {
        AudioServicesPlaySystemSound(sound)
    }

    public func vibrate() {
        AudioServicesPlayAlertSound(kSystemSoundID_Vibrate)
    }
}

Our app is implemented with MVVM as a series of distinct Swift packages with a chain of dependencies modeled after the CLEAN architecture (see blog post above); it basically looks like:

Screens -> ViewModels -> Logic (aka UseCases) -> Repositories -> FrameworkProviderProtocols

Our main app target has a dependency on FrameworkProviders, which is a package that defines the following Container extension to resolve that 3rd-party protocol:

public extension Container {
    func injectFrameworkProviders() -> Container {
        // snip
        self.autoregister(SoundPlayerProviding.self, initializer: SoundPlayerProvider.init).inObjectScope(.container)
        // etc

Our Tests targets for those intermediate layers (ViewModelsTests, LogicTests, RepositoriesTests, etc) will setup a Container that injects our real intermediate code from that layer down. So, LogicTests will call .injectLogic() and .injectRepositories(), but ViewModelsTests will additionally call .injectViewModels() on top of .injectLogic() and .injectRepositories().

When configuring any of these tests, rather than taking a dependency on FrameworkProviders, however, they take a dependency on FrameworkProviderMocks. This package defines the following Container extension to resolve these 3rd-party protocols:

public extension Container {
    func injectFrameworkProviderMocks() -> Container {
        // etc
        self.autoregister(SoundPlayerProviding.self, initializer: MockSoundProvider.init).inObjectScope(.container)
        // etc

In our tests, we will want to confirm that various methods on MockSoundProvider actually got called. Here is a simple example from a ViewModel test:

    var mockSoundProvider: MockSoundProvider!

    beforeEach {
        mockSoundProvider = container ~~> SoundPlayerProviding.self
    }

    context("Given I received a message") {
        beforeEach {
            viewModel.viewDidAppear()
            let chatMessage = createSimpleChatMessage()
            inboxRepository.chatMessageReceived(chatMessage)
        }

        it("Then a sound has been played") {
            expect(mockSoundProvider.playCount).to(equal(1))
            expect(mockSoundProvider.vibrateCount).to(equal(1))
        }

Cases like this get repeated throughout our (large) test code base; evidently we resolve the Mock version of a protocol in our tests more than 500 times. As a result we very much prefer our shorter syntax:

    mockSoundProvider = container ~~> SoundPlayerProviding.self

to the longer:

    mockSoundProvider = ((container ~> SoundPlayerProviding.self) as! MockSoundProvider)

Given how useful we have found this operator we wanted to suggest it for use by the wider community. Apologies for the longer reply, but I thought it would be useful to offer a full accounting of our approach!

bradfol commented 3 months ago

It seems like you could modify your registration to include .implements(MockSoundProvider.self) which then means resolving either SoundPlayerProviding or MockSoundProvider would produce the same object.

public extension Container {
    func injectFrameworkProviderMocks() -> Container {
        // etc
        self.autoregister(SoundPlayerProviding.self, initializer: MockSoundProvider.init)
            .implements(MockSoundProvider.self)
            .inObjectScope(.container)
        // etc

This approach I think gets what you want from testing niceties while avoiding a custom operator (you can just use the existing ~> operator). Please let me know if I'm missing something.

esilverberg commented 3 months ago

That totally works. Will use this solution -- thank you @bradfol for your help!