-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
base: main
Are you sure you want to change the base?
Conversation
One point for comment is it might be worth defaulting to making service access |
To add some further context, the original code (the generic |
@@ -4,55 +4,108 @@ final class ServiceTests: XCTestCase { | |||
func testReadOnly() throws { | |||
let app = Application(.testing) | |||
defer { app.shutdown() } | |||
|
|||
There was a problem hiding this comment.
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) -> ()) { |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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> { |
There was a problem hiding this comment.
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.
Great addition btw! |
@0xTim @Joannis @Andrewangeta What's the best way I can help get this merged?
|
What's the goal of this PR? |
@Joannis what do you mean? |
@0xTim sorry for missing your message; |
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:
You may then have a real implementation:
This is a very contrived example, but shows a service that needs a
Logger
andEventLoop
- 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:
Configure it in configure.swift:
And then use it in routes:
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:
And then you can configure your test application and use it: