summaryrefslogtreecommitdiff
path: root/src/conversation/handlers/create
diff options
context:
space:
mode:
authorojacobson <ojacobson@noreply.codeberg.org>2025-10-28 18:41:41 +0100
committerojacobson <ojacobson@noreply.codeberg.org>2025-10-28 18:41:41 +0100
commit0ef69c7d256380e660edc45ace7f1d6151226340 (patch)
treea6eea3af13f237393057aac1d80f024bc1b40b10 /src/conversation/handlers/create
parent58e6496558a01052537c5272169aea3e79ccbc8e (diff)
parentdc7074bbf39aff895ba52abd5e7b7e9bb643cf27 (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/conversation/handlers/create')
-rw-r--r--src/conversation/handlers/create/mod.rs8
-rw-r--r--src/conversation/handlers/create/test.rs66
2 files changed, 46 insertions, 28 deletions
diff --git a/src/conversation/handlers/create/mod.rs b/src/conversation/handlers/create/mod.rs
index 18eca1f..2b7fa39 100644
--- a/src/conversation/handlers/create/mod.rs
+++ b/src/conversation/handlers/create/mod.rs
@@ -5,9 +5,8 @@ use axum::{
};
use crate::{
- app::App,
clock::RequestedAt,
- conversation::{Conversation, app},
+ conversation::{Conversation, app, app::Conversations},
error::Internal,
name::Name,
token::extract::Identity,
@@ -17,13 +16,12 @@ use crate::{
mod test;
pub async fn handler(
- State(app): State<App>,
+ State(conversations): State<Conversations>,
_: Identity, // requires auth, but doesn't actually care who you are
RequestedAt(created_at): RequestedAt,
Json(request): Json<Request>,
) -> Result<Response, Error> {
- let conversation = app
- .conversations()
+ let conversation = conversations
.create(&request.name, &created_at)
.await
.map_err(Error)?;
diff --git a/src/conversation/handlers/create/test.rs b/src/conversation/handlers/create/test.rs
index bc05b00..380bb13 100644
--- a/src/conversation/handlers/create/test.rs
+++ b/src/conversation/handlers/create/test.rs
@@ -22,10 +22,14 @@ async fn new_conversation() {
let name = fixtures::conversation::propose();
let request = super::Request { name: name.clone() };
- let super::Response(response) =
- super::handler(State(app.clone()), creator, fixtures::now(), Json(request))
- .await
- .expect("creating a conversation in an empty app succeeds");
+ let super::Response(response) = super::handler(
+ State(app.conversations()),
+ creator,
+ fixtures::now(),
+ Json(request),
+ )
+ .await
+ .expect("creating a conversation in an empty app succeeds");
// Verify the structure of the response
@@ -77,10 +81,14 @@ async fn duplicate_name() {
let request = super::Request {
name: conversation.name.clone(),
};
- let super::Error(error) =
- super::handler(State(app.clone()), creator, fixtures::now(), Json(request))
- .await
- .expect_err("duplicate conversation name should fail the request");
+ let super::Error(error) = super::handler(
+ State(app.conversations()),
+ creator,
+ fixtures::now(),
+ Json(request),
+ )
+ .await
+ .expect_err("duplicate conversation name should fail the request");
// Verify the structure of the response
@@ -110,10 +118,14 @@ async fn conflicting_canonical_name() {
let request = super::Request {
name: conflicting_name.clone(),
};
- let super::Error(error) =
- super::handler(State(app.clone()), creator, fixtures::now(), Json(request))
- .await
- .expect_err("duplicate conversation name should fail the request");
+ let super::Error(error) = super::handler(
+ State(app.conversations()),
+ creator,
+ fixtures::now(),
+ Json(request),
+ )
+ .await
+ .expect_err("duplicate conversation name should fail the request");
// Verify the structure of the response
@@ -135,7 +147,7 @@ async fn invalid_name() {
let name = fixtures::conversation::propose_invalid_name();
let request = super::Request { name: name.clone() };
let super::Error(error) = crate::conversation::handlers::create::handler(
- State(app.clone()),
+ State(app.conversations()),
creator,
fixtures::now(),
Json(request),
@@ -163,7 +175,7 @@ async fn name_reusable_after_delete() {
let request = super::Request { name: name.clone() };
let super::Response(response) = super::handler(
- State(app.clone()),
+ State(app.conversations()),
creator.clone(),
fixtures::now(),
Json(request),
@@ -181,10 +193,14 @@ async fn name_reusable_after_delete() {
// Call the endpoint (second time)
let request = super::Request { name: name.clone() };
- let super::Response(response) =
- super::handler(State(app.clone()), creator, fixtures::now(), Json(request))
- .await
- .expect("creation succeeds after original conversation deleted");
+ let super::Response(response) = super::handler(
+ State(app.conversations()),
+ creator,
+ fixtures::now(),
+ Json(request),
+ )
+ .await
+ .expect("creation succeeds after original conversation deleted");
// Verify the structure of the response
@@ -212,7 +228,7 @@ async fn name_reusable_after_expiry() {
let request = super::Request { name: name.clone() };
let super::Response(_) = super::handler(
- State(app.clone()),
+ State(app.conversations()),
creator.clone(),
fixtures::ancient(),
Json(request),
@@ -230,10 +246,14 @@ async fn name_reusable_after_expiry() {
// Call the endpoint (second time)
let request = super::Request { name: name.clone() };
- let super::Response(response) =
- super::handler(State(app.clone()), creator, fixtures::now(), Json(request))
- .await
- .expect("creation succeeds after original conversation expired");
+ let super::Response(response) = super::handler(
+ State(app.conversations()),
+ creator,
+ fixtures::now(),
+ Json(request),
+ )
+ .await
+ .expect("creation succeeds after original conversation expired");
// Verify the structure of the response