diff options
| author | ojacobson <ojacobson@noreply.codeberg.org> | 2025-10-28 18:41:41 +0100 |
|---|---|---|
| committer | ojacobson <ojacobson@noreply.codeberg.org> | 2025-10-28 18:41:41 +0100 |
| commit | 0ef69c7d256380e660edc45ace7f1d6151226340 (patch) | |
| tree | a6eea3af13f237393057aac1d80f024bc1b40b10 /src/message | |
| parent | 58e6496558a01052537c5272169aea3e79ccbc8e (diff) | |
| parent | dc7074bbf39aff895ba52abd5e7b7e9bb643cf27 (diff) | |
Use freestanding structs for `App` components.
A "component" is a struct that provides domain-specific operations on the service. `App` largely acts as a way to obtain components for domain-specific operations: for example, given `let app: App = todo!();`, then `app.tokens()` provides a component (`Tokens`) that supports operations on authentication tokens, and `app.conversations()` provides a component (`Conversations`) that supports operations on conversations.
This has been a major piece of the server's internal organization for a long time. Historically, these components have been built as short-lived view structs, which hold onto their functional dependencies by reference. A given component was therefore bound to its source `App`, and had a lifetime bounded by the life of that `App` instance or reference.
This change turns components into freestanding structs - that is, they can outlive the `App` that provided them. They hold their dependencies by value, not by reference; `App` provides clones when creating a component, instead of borrowing its own state. For the functional dependencies we have today, cloning is a supported and cheap way to share access; details are documented in the individual commits.
I'm making this change because we're working on web push, and we discovered while prototyping that it will be useful to be able to support multiple distinct types of web push client. A running Pilcrow server would use a "real" client (which sends real HTTP requests to deliver push messages), while tests would use a client stub (which doesn't). However, to make that work, we'll need to make `App` generic over the client, and the resulting type parameter would then end up in every handler and in most other things that touch the `App` type. This refactoring dramatically reduces the number of places we mention the `App` type, by making most uses rely on specific components instead of relying on `App` generally.
There are still a few places that work on `App` generally, rather than on specific components, because an operation requires the use of two or more components.
I don't love all this cloning, even if I know in my head that it's fine. The alternatives that we looked at include:
* Provider traits, as we have for `Transaction`, that allow endpoints to specify that they want any type that can provide a `Tokens` or a `Conversation` instead of specifically an `App` (`App<PushClient>`). This is wordy enough that we've opted to punt on that approach for now.
* Accept the type parameter as the cost of doing business. This is still an open alternative.
* Use `dyn` dispatch instead of a type parameter for the push client. This is still an open alternative, though not one I love as we'd be incurring function call indirection without getting any generality out of it.
Merges freestanding-app-components into main.
Diffstat (limited to 'src/message')
| -rw-r--r-- | src/message/app.rs | 10 | ||||
| -rw-r--r-- | src/message/handlers/delete/mod.rs | 10 | ||||
| -rw-r--r-- | src/message/handlers/delete/test.rs | 12 |
3 files changed, 17 insertions, 15 deletions
diff --git a/src/message/app.rs b/src/message/app.rs index 647152e..cbcbff9 100644 --- a/src/message/app.rs +++ b/src/message/app.rs @@ -13,13 +13,13 @@ use crate::{ user::{self, repo::Provider as _}, }; -pub struct Messages<'a> { - db: &'a SqlitePool, - events: &'a Broadcaster, +pub struct Messages { + db: SqlitePool, + events: Broadcaster, } -impl<'a> Messages<'a> { - pub const fn new(db: &'a SqlitePool, events: &'a Broadcaster) -> Self { +impl Messages { + pub const fn new(db: SqlitePool, events: Broadcaster) -> Self { Self { db, events } } diff --git a/src/message/handlers/delete/mod.rs b/src/message/handlers/delete/mod.rs index 3e9a212..c09a752 100644 --- a/src/message/handlers/delete/mod.rs +++ b/src/message/handlers/delete/mod.rs @@ -5,10 +5,12 @@ use axum::{ }; use crate::{ - app::App, clock::RequestedAt, error::{Internal, NotFound}, - message::{self, app::DeleteError}, + message::{ + self, + app::{DeleteError, Messages}, + }, token::extract::Identity, }; @@ -16,12 +18,12 @@ use crate::{ mod test; pub async fn handler( - State(app): State<App>, + State(messages): State<Messages>, Path(message): Path<message::Id>, RequestedAt(deleted_at): RequestedAt, identity: Identity, ) -> Result<Response, Error> { - app.messages() + messages .delete(&identity.login, &message, &deleted_at) .await?; diff --git a/src/message/handlers/delete/test.rs b/src/message/handlers/delete/test.rs index 05d9344..198728b 100644 --- a/src/message/handlers/delete/test.rs +++ b/src/message/handlers/delete/test.rs @@ -16,7 +16,7 @@ pub async fn delete_message() { // Send the request let response = super::handler( - State(app.clone()), + State(app.messages()), Path(message.id.clone()), fixtures::now(), sender, @@ -52,7 +52,7 @@ pub async fn delete_invalid_message_id() { let deleter = fixtures::identity::create(&app, &fixtures::now()).await; let message = fixtures::message::fictitious(); let super::Error(error) = super::handler( - State(app.clone()), + State(app.messages()), Path(message.clone()), fixtures::now(), deleter, @@ -83,7 +83,7 @@ pub async fn delete_deleted() { // Send the request let super::Error(error) = super::handler( - State(app.clone()), + State(app.messages()), Path(message.id.clone()), fixtures::now(), sender, @@ -114,7 +114,7 @@ pub async fn delete_expired() { // Send the request let super::Error(error) = super::handler( - State(app.clone()), + State(app.messages()), Path(message.id.clone()), fixtures::now(), sender, @@ -150,7 +150,7 @@ pub async fn delete_purged() { let deleter = fixtures::identity::create(&app, &fixtures::now()).await; let super::Error(error) = super::handler( - State(app.clone()), + State(app.messages()), Path(message.id.clone()), fixtures::now(), deleter, @@ -176,7 +176,7 @@ pub async fn delete_not_sender() { let deleter = fixtures::identity::create(&app, &fixtures::now()).await; let super::Error(error) = super::handler( - State(app.clone()), + State(app.messages()), Path(message.id.clone()), fixtures::now(), deleter.clone(), |
