Conversation
3288be8 to
2742d15
Compare
| public init() {} | ||
|
|
||
| public func handle(_ request: Request, context: Context, next: (Request, Context) async throws -> Response) async throws -> Response { | ||
| guard context.identity != nil else { |
There was a problem hiding this comment.
This can be simplified to try context.requireIndentity()
There was a problem hiding this comment.
In this example I'm not interested in the value behind the user's identity, just if they're authenticated. Should I add a comment for that?
| ```swift | ||
| protocol RateLimitedRequestContext { | ||
| var rateLimitStatus: RateLimitStatus { get } | ||
| var ipAddress: String { get } |
There was a problem hiding this comment.
Should we elaborate a little more here because using the NIO channel to extract the ip address will not wield the desired data (ie a user's ip if the service is behind and load balancer or proxy.
There was a problem hiding this comment.
I guess so, though this was really intended as a reference of what RequestContext could potentially be used for, not a full example. Should I clarify the intent here more?
There was a problem hiding this comment.
@thoven87 I'm not sure getting into details about how to retrieve ip addresses is needed here
There was a problem hiding this comment.
@thoven87 I'm not sure getting into details about how to retrieve ip addresses is needed here
Understood, I think this should be documented somewhere though.
| @@ -0,0 +1,342 @@ | |||
| ## Dependencies | |||
There was a problem hiding this comment.
We need a top level heading for the documentation to link correctly eg Recommendations, a byline and an overview
We should then have some general text saying something of the sort
# Development Guidelines
Recommendations for building applications with Hummingbird
The Hummingbird web framework has been designed to be as flexible as possible. We try to impose as little structure on how it should be used. But some people like structure. This document provides some direction on how you can organise and implement good practices when developing your Hummingbird application.|
|
||
| ## Project Structure | ||
|
|
||
| The project structure is a recommended project layout, defining what modules are recommended to create and how these modules are structured in terms of folders and files. |
There was a problem hiding this comment.
Mention the template providing a degree of initial structure
|
|
||
| ### Handling Bodies | ||
|
|
||
| If you're using standard libraries like Foundation's `JSONDecoder`, limit the body size to "reasonable" sizes. What is considered reasonable depends per application, but lower is generally better. |
There was a problem hiding this comment.
A lot of this is covered in the router documentation.
Recommendations about not collecting response in middleware, setting realistic limits when collecting into a single buffer, although preferring to stick to streamed version if possible.
There was a problem hiding this comment.
I'll make things a little more bullet-pointy here
| } | ||
| ``` | ||
|
|
||
| ### Child Request Contexts |
There was a problem hiding this comment.
What are actual guidelines here?
There was a problem hiding this comment.
Oops, forgot to add that bit
Co-authored-by: Moritz Lang <16192401+slashmo@users.noreply.github.com>
| - [IkigaJSON](https://github.com/orlandos-nl/ikigajson) supports streaming JSON lines or JSON arrays. | ||
| - [Server Sent Events](https://github.com/orlandos-nl/ssekit) supports streaming Server Sent Events. | ||
| - [Swift-WebSocket](https://github.com/hummingbird-project/swift-websocket) and [Hummingbird-WebSocket](https://github.com/hummingbird-project/hummingbird-websocket) support streaming over WebSocket connections. | ||
| - [PostgreNIO](https://github.com/vapor/posrtgresnio), [Valkey-Swift](https://github.com/valkey-io/valkey-swift) and [MongoKitten](https://github.com/orlandos-nl/mongokitten) all support streaming in their database operations. |
There was a problem hiding this comment.
correct link: https://github.com/vapor/postgres-nio
adam-fowler
left a comment
There was a problem hiding this comment.
Some more comments. I'm going to add a few additions to the docs based off what this PR has revealed is missing
| @@ -0,0 +1,392 @@ | |||
| # Tips for Building Hummingbird Apps | |||
|
|
|||
| When you start a new app, especially with a new framework, you will find many useful tips and tools. Here are lessons we learned from building Hummingbird and from our community. Please send us feedback or suggestions for this document anytime! | |||
There was a problem hiding this comment.
This feels too informal. Prefer something like
Hummingbird was built to be flexible as possible, so you can use it as you feel is suitable. This document though provides some recommendations on how to structure a Hummingbird app. Many of these come from lessons we learned while building applications using Hummingbird and from the experience of our community.
|
|
||
| When you start a new app, especially with a new framework, you will find many useful tips and tools. Here are lessons we learned from building Hummingbird and from our community. Please send us feedback or suggestions for this document anytime! | ||
|
|
||
| ## Dependencies |
There was a problem hiding this comment.
I assume by dependencies you mean things like Postgres, redis/valkey etc if so I think this should be part of a different article. I feel this should concentrate on how to structure applications
| } | ||
| ``` | ||
|
|
||
| Controllers can specify dependencies on services they need, like a database connection. To do so, add these properties on the controller: |
There was a problem hiding this comment.
Can we get a subheading here, something like
Dependencies
| ``` | ||
|
|
||
|
|
||
| If a route needs something special, like authentication, use _protocol composition_ to refine the RequestContext. |
There was a problem hiding this comment.
Mention hummingbird-auth here, as this provides a lot of the framework for being authenticated services
| let router = Router(context: MyContext.self) | ||
| router.add("users", routes: UserController().routes) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Add a link to the general documentation here <doc:RouterGuide>
| return try await next(request, context) | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Add a link to general documentation <doc:MiddlewareGuide>
| - Avoid collecting the data into a single buffer, whenever possible. Use streaming APIs instead. | ||
| - Try to move the CPU and RAM intensive work of providing the correct format to the client if possible. | ||
| - This means your API only needs to validate the data before processing/saving. | ||
| - Use libraries that support streaming data if your dataset is large. |
There was a problem hiding this comment.
Add
- Use your RequestContext to define an in-memory size when decoding requests. I was going to say add a link to the relevant docs, but apparently we only have reference documentation for that
| - This means your API only needs to validate the data before processing/saving. | ||
| - Use libraries that support streaming data if your dataset is large. | ||
|
|
||
| Some libraries that play well into this: |
There was a problem hiding this comment.
This should be in a separate document
| - [Swift-WebSocket](https://github.com/hummingbird-project/swift-websocket) and [Hummingbird-WebSocket](https://github.com/hummingbird-project/hummingbird-websocket) support streaming over WebSocket connections. | ||
| - [PostgreNIO](https://github.com/vapor/posrtgresnio), [Valkey-Swift](https://github.com/valkey-io/valkey-swift) and [MongoKitten](https://github.com/orlandos-nl/mongokitten) all support streaming in their database operations. | ||
|
|
||
| You can handle the request and response bodies as an `AsyncSequence`. |
There was a problem hiding this comment.
This is repeating what is already in the documentation https://docs.hummingbird.codes/2.0/documentation/hummingbird/routerguide#Request-Body
Co-authored-by: Adam Fowler <adamfowler71@gmail.com>
We should expand this later to include databases (Valkey, Postgres) and Deployment. Though those can very well be separate guides