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

Make Services in Vapor Usable #2901

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Make Services in Vapor Usable #2901

wants to merge 3 commits into from

Conversation

0xTim
Copy link
Member

@0xTim 0xTim commented Oct 20, 2022

This adds real support to Vapor to make it easy to integrate different services that can be tested.

For example, if you have a service defined as:

protocol MyService {
    func `for`(_ request: Request) -> MyService
    func doSomething() -> String
}

You may then have a real implementation:

import Vapor

struct MyRealService: MyService {
    let logger: Logger
    let eventLoop: EventLoop
    
    func `for`(_ request: Vapor.Request) -> MyService {
        return MyRealService(logger: request.logger, eventLoop: request.eventLoop)
    }
    
    func doSomething() -> String {
        return "Tada"
    }
    
}

This is a very contrived example, but shows a service that needs a Logger and EventLoop - things that are normally tied to specific requests. Doing this in a safe and testable way involves a lot of boilerplate. This moves the boilerplate into Vapor to make it easier to do.

Now, you can define your services:

extension Application.Services {
    var myService: Application.Service<MyService> {
        .init(application: self.application)
    }
}

extension Request.Services {
    var myService: MyService {
        self.request.application.services.myService.service.for(request)
    }
}

Configure it in configure.swift:

app.services.myService.use { app in
    MyRealService(logger: app.logger, eventLoop: app.eventLoopGroup.next())
}

And then use it in routes:

app.get("myService") { req -> String in
    let thing = req.services.myService.doSomething()
    return thing
}

Testing

This approach makes it very easy to write test services as well. With the protocol defined above, you can create an implementation for testing:

struct MyFakeService: MyService {
    let cannedResponse: String
    let eventLoop: EventLoop
    let logger: Logger
    
    func `for`(_ request: Vapor.Request) -> App.MyService {
        return MyFakeService(cannedResponse: self.cannedResponse, eventLoop: request.eventLoop, logger: request.logger)
    }
    
    func doSomething() -> String {
        return cannedResponse
    }   
}

And then you can configure your test application and use it:

let myFakeServicce = MyFakeService(cannedResponse: testString, eventLoop: app.eventLoopGroup.next(), logger: app.logger)        
app.services.myService.use { _ in
    myFakeServicce
}
    
try app.test(.GET, "myService", afterResponse: { res in
    XCTAssertEqual(res.status, .ok)
    XCTAssertEqual(res.body.string, testString)
})
``

@0xTim 0xTim marked this pull request as ready for review October 20, 2022 13:06
@0xTim 0xTim added the semver-minor Contains new API label Oct 20, 2022
@0xTim
Copy link
Member Author

0xTim commented Oct 20, 2022

One point for comment is it might be worth defaulting to making service access async to avoid immediately deprecating everything when #2873 lands

@0xTim
Copy link
Member Author

0xTim commented Oct 20, 2022

To add some further context, the original code (the generic Service stuff) was taken from a Vapor 4 project that's been running in production for over 2 years with no issues

@@ -4,55 +4,108 @@ final class ServiceTests: XCTestCase {
func testReadOnly() throws {
let app = Application(.testing)
defer { app.shutdown() }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, but there's a lot of whitespace changes.

public struct Provider {
let run: (Application) -> ()

public init(_ run: @escaping (Application) -> ()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we mark these @Sendable from now on?

provider.run(self.application)
}

public func use(_ makeService: @escaping (Application) -> ServiceType) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sendable?


private var storage: Storage {
if self.application.storage[Key.self] == nil {
self.initialize()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this works fine, but can we make initialize() return the newly created entity? Then return that here.

@@ -0,0 +1,53 @@
public extension Application {
struct Service<ServiceType> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public struct instead of public extension? I tend to be explicit in marking things public in libraries/frameworks, so future changes don't accidentally expose internal API.

@Joannis
Copy link
Member

Joannis commented Oct 27, 2022

Great addition btw!

@DanielSincere
Copy link

@0xTim @Joannis @Andrewangeta What's the best way I can help get this merged?

  1. I'm using this code in a project
  2. Tim has copy-pasted this into Discord at least three times

@Joannis
Copy link
Member

Joannis commented Jan 27, 2023

What's the goal of this PR?

@0xTim
Copy link
Member Author

0xTim commented Jan 27, 2023

@Joannis what do you mean?

@Joannis
Copy link
Member

Joannis commented Feb 16, 2023

@0xTim sorry for missing your message;
I mean, what's the intention behind this new feature? To make it easier to work with app.storage? Or is there another goal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor Contains new API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants