diff options
| author | Owen Jacobson <owen@grimoire.ca> | 2024-10-19 00:57:20 -0400 |
|---|---|---|
| committer | Owen Jacobson <owen@grimoire.ca> | 2024-10-19 00:57:20 -0400 |
| commit | ad00b553d845dba8af7b0e9fa2930209aee1dd62 (patch) | |
| tree | 0a91c3c0113b2618730d3160f311c083e95b0581 | |
| parent | 635d92eb4ffc5a1c94cba784a2a4f18e1cb5effc (diff) | |
Make the responses for various data creation requests more consistent.
In general:
* If the client can only assume the response is immediately valid (mostly, login creation, where the client cannot monitor the event stream), then 200 Okay, with data describing the server's view of the request.
* If the client can monitor for completion by watching the event stream, then 202 Accepted, with data describing the server's view of the request.
This comes on the heels of a comment I made on Discord:
> hrm
>
> creating a login: 204 No Content, no body
> sending a message: 202 Accepted, no body
> creating a channel: 200 Okay, has a body
>
> past me, what were you on
There wasn't any principled reason for this inconsistency; it happened as the endpoints were written at different times and with different states of mind.
| -rw-r--r-- | docs/api/authentication.md | 16 | ||||
| -rw-r--r-- | docs/api/channels-messages.md | 38 | ||||
| -rw-r--r-- | docs/api/events.md | 5 | ||||
| -rw-r--r-- | docs/api/initial-setup.md | 16 | ||||
| -rw-r--r-- | docs/api/invitations.md | 16 | ||||
| -rw-r--r-- | src/channel/routes/channel/post.rs | 23 | ||||
| -rw-r--r-- | src/channel/routes/channel/test.rs | 2 | ||||
| -rw-r--r-- | src/channel/routes/post.rs | 14 | ||||
| -rw-r--r-- | src/channel/routes/test.rs | 16 | ||||
| -rw-r--r-- | src/invite/app.rs | 4 | ||||
| -rw-r--r-- | src/invite/routes/invite/post.rs | 8 | ||||
| -rw-r--r-- | src/login/history.rs | 6 | ||||
| -rw-r--r-- | src/login/routes/login/post.rs | 10 | ||||
| -rw-r--r-- | src/login/routes/login/test.rs | 22 | ||||
| -rw-r--r-- | src/setup/app.rs | 6 | ||||
| -rw-r--r-- | src/setup/routes/post.rs | 12 | ||||
| -rw-r--r-- | src/test/fixtures/identity.rs | 12 | ||||
| -rw-r--r-- | src/test/fixtures/login.rs | 7 | ||||
| -rw-r--r-- | src/token/app.rs | 6 |
19 files changed, 174 insertions, 65 deletions
diff --git a/docs/api/authentication.md b/docs/api/authentication.md index d4c1f70..7e05443 100644 --- a/docs/api/authentication.md +++ b/docs/api/authentication.md @@ -56,7 +56,21 @@ The request must have the following fields: <!-- This prose is duplicated by 03-initial-setup.md and in 04-invitations.md, with small changes for context. If you edit it here, edit it there, too. --> -This endpoint will respond with a status of `204 No Content` when successful. +This endpoint will respond with a status of `200 Okay` when successful. The body of the response will be a JSON object describing the authenticated login: + +```json +{ + "id": "Labcd1234", + "name": "Andrea" +} +``` + +The response will include the following fields: + +| Field | Type | Description | +|:------------|:-------|:--| +| `id` | string | The authenticated login's ID. | +| `name` | string | The authenticated login's name. | The response will include a `Set-Cookie` header for the `identity` cookie, providing the client with a newly-minted identity token associated with the login identified in the request. This token's value must be kept confidential. diff --git a/docs/api/channels-messages.md b/docs/api/channels-messages.md index 99a525e..69cadcb 100644 --- a/docs/api/channels-messages.md +++ b/docs/api/channels-messages.md @@ -54,12 +54,12 @@ The request must have the following fields: ### Success -This endpoint will respond with a status of `200 Okay` when successful. The body of the response will be a JSON object describing the new channel: +This endpoint will respond with a status of `202 Accepted` when successful. The body of the response will be a JSON object describing the new channel: ```json { - "name": "a unique channel name", "id": "C9876cyyz" + "name": "a unique channel name", } ``` @@ -67,8 +67,10 @@ The response will have the following fields: | Field | Type | Description | |:-------|:-------|:--| +| `id` | string | A unique identifier for the channel. This can be used to associate the channel with events, or to make API calls targeting the channel. | | `name` | string | The channel's name. | -| `id` | string | A unique identifier for the channel. This can be used to associate the channel with other events, or to make API calls targeting the channel. | + +When completed, the service will emit a [channel created](events.md#channel-created) event with the channel's ID. ### Duplicate channel name @@ -101,14 +103,35 @@ The request must have the following fields: ### Success -This endpoint will respond with a status of `202 Accepted` when successful. The response will not include a body. +This endpoint will respond with a status of `202 Accepted` when successful. The body of the response will be a JSON object describing the newly-sent message: + +```json +{ + "at": "2024-10-19T04:37:09.467325Z", + "channel": "Cfqdn1234", + "sender": "Labcd1234", + "id": "Mgh98yp75", + "body": "an elaborate example message" +} +``` + +The response will have the following fields: + +| Field | Type | Description | +|:----------|:----------|:--| +| `at` | timestamp | The moment the message was sent. | +| `channel` | string | The ID of the channel the message was sent to. | +| `sender` | string | The ID of the login that sent the message. | +| `id` | string | A unique identifier for the message. This can be used to associate the message with events, or to make API calls targeting the message. | +| `body` | string | The message's body. | + +When completed, the service will emit a [message sent](events.md#message-sent) event with the message's ID. ### Invalid channel ID This endpoint will respond with a status of `404 Not Found` if the channel ID is not valid. - ## `DELETE /api/channels/:id` Deletes a channel (and all messages in it). @@ -123,6 +146,9 @@ This endpoint requires the following path parameter: This endpoint will respond with a status of `202 Accepted` when successful. The response will not include a body. +When completed, the service will emit a [channel deleted](events.md#channel-deleted) event with the channel's ID. + + ### Invalid channel ID This endpoint will respond with a status of `404 Not Found` if the channel ID is not valid. @@ -142,6 +168,8 @@ This endpoint requires the following path parameter: This endpoint will respond with a status of `202 Accepted` when successful. The response will not include a body. +When completed, the service will emit a [message deleted](events.md#message-deleted) event with the channel's ID. + ### Invalid message ID This endpoint will respond with a status of `404 Not Found` if the message ID is not valid. diff --git a/docs/api/events.md b/docs/api/events.md index 2f48df1..b08e971 100644 --- a/docs/api/events.md +++ b/docs/api/events.md @@ -31,6 +31,11 @@ sequenceDiagram The core of the service is to facilitate conversations between logins. Conversational activity is delivered to clients using _events_. Each event notifies interested clients of activity sent to the service through its API. +## Asynchronous completion + +A number of endpoints return `202 Accepted` responses. The actions performed by those endpoints will be completed before events are delivered. To await the completion of an operation which returns this response, clients must monitor the event stream for the corresponding event. + + ## `GET /api/events` Subscribes to events. diff --git a/docs/api/initial-setup.md b/docs/api/initial-setup.md index ad57089..3c5a8a6 100644 --- a/docs/api/initial-setup.md +++ b/docs/api/initial-setup.md @@ -55,7 +55,21 @@ The request must have the following fields: <!-- This prose is duplicated from authentication.md, with small changes for context. If you edit it here, edit it there, too. --> -This endpoint will respond with a status of `204 No Content` when successful. +This endpoint will respond with a status of `200 Okay` when successful. The body of the response will be a JSON object describing the newly-created login: + +```json +{ + "id": "Labcd1234", + "name": "Andrea" +} +``` + +The response will include the following fields: + +| Field | Type | Description | +|:------------|:-------|:--| +| `id` | string | A unique identifier for the newly-created login. This can be used to associate the login with other events, or to make API calls targeting the login. | +| `name` | string | The login's name. | The response will include a `Set-Cookie` header for the `identity` cookie, providing the client with a newly-minted identity token associated with the initial login created for this request. See the [authentication](./authentication) section for details on how this cookie may be used. diff --git a/docs/api/invitations.md b/docs/api/invitations.md index f75e30b..d3431d7 100644 --- a/docs/api/invitations.md +++ b/docs/api/invitations.md @@ -134,7 +134,21 @@ The request must have the following fields: <!-- This prose is duplicated from authentication.md, with small changes for context. If you edit it here, edit it there, too. --> -This endpoint will respond with a status of `204 No Content` when successful. +This endpoint will respond with a status of `200 Okay` when successful. The body of the response will be a JSON object describing the newly-created login: + +```json +{ + "id": "Labcd1234", + "name": "Andrea" +} +``` + +The response will include the following fields: + +| Field | Type | Description | +|:------------|:-------|:--| +| `id` | string | A unique identifier for the newly-created login. This can be used to associate the login with other events, or to make API calls targeting the login. | +| `name` | string | The login's name. | The response will include a `Set-Cookie` header for the `identity` cookie, providing the client with a newly-minted identity token associated with the login created for this request. See the [authentication](./authentication.md) section for details on how this cookie may be used. diff --git a/src/channel/routes/channel/post.rs b/src/channel/routes/channel/post.rs index a71a3a0..b489a77 100644 --- a/src/channel/routes/channel/post.rs +++ b/src/channel/routes/channel/post.rs @@ -1,7 +1,7 @@ use axum::{ extract::{Json, Path, State}, http::StatusCode, - response::{IntoResponse, Response}, + response::{self, IntoResponse}, }; use crate::{ @@ -9,7 +9,7 @@ use crate::{ clock::RequestedAt, error::{Internal, NotFound}, login::Login, - message::app::SendError, + message::{app::SendError, Message}, }; pub async fn handler( @@ -18,12 +18,13 @@ pub async fn handler( RequestedAt(sent_at): RequestedAt, login: Login, Json(request): Json<Request>, -) -> Result<StatusCode, Error> { - app.messages() +) -> Result<Response, Error> { + let message = app + .messages() .send(&channel, &login, &sent_at, &request.body) .await?; - Ok(StatusCode::ACCEPTED) + Ok(Response(message)) } #[derive(serde::Deserialize)] @@ -31,12 +32,22 @@ pub struct Request { pub body: String, } +#[derive(Debug)] +pub struct Response(pub Message); + +impl IntoResponse for Response { + fn into_response(self) -> response::Response { + let Self(message) = self; + (StatusCode::ACCEPTED, Json(message)).into_response() + } +} + #[derive(Debug, thiserror::Error)] #[error(transparent)] pub struct Error(#[from] pub SendError); impl IntoResponse for Error { - fn into_response(self) -> Response { + fn into_response(self) -> response::Response { let Self(error) = self; #[allow(clippy::match_wildcard_for_single_variants)] match error { diff --git a/src/channel/routes/channel/test.rs b/src/channel/routes/channel/test.rs index c6541cd..b895b69 100644 --- a/src/channel/routes/channel/test.rs +++ b/src/channel/routes/channel/test.rs @@ -27,7 +27,7 @@ async fn messages_in_order() { for (sent_at, body) in &requests { let request = post::Request { body: body.clone() }; - post::handler( + let _ = post::handler( State(app.clone()), Path(channel.id.clone()), sent_at.clone(), diff --git a/src/channel/routes/post.rs b/src/channel/routes/post.rs index d694f8b..a05c312 100644 --- a/src/channel/routes/post.rs +++ b/src/channel/routes/post.rs @@ -17,14 +17,14 @@ pub async fn handler( _: Login, // requires auth, but doesn't actually care who you are RequestedAt(created_at): RequestedAt, Json(request): Json<Request>, -) -> Result<Json<Channel>, Error> { +) -> Result<Response, Error> { let channel = app .channels() .create(&request.name, &created_at) .await .map_err(Error)?; - Ok(Json(channel)) + Ok(Response(channel)) } #[derive(serde::Deserialize)] @@ -33,6 +33,16 @@ pub struct Request { } #[derive(Debug)] +pub struct Response(pub Channel); + +impl IntoResponse for Response { + fn into_response(self) -> response::Response { + let Self(channel) = self; + (StatusCode::ACCEPTED, Json(channel)).into_response() + } +} + +#[derive(Debug)] pub struct Error(pub app::CreateError); impl IntoResponse for Error { diff --git a/src/channel/routes/test.rs b/src/channel/routes/test.rs index ffd8484..7879ba0 100644 --- a/src/channel/routes/test.rs +++ b/src/channel/routes/test.rs @@ -20,9 +20,10 @@ async fn new_channel() { let name = fixtures::channel::propose(); let request = post::Request { name: name.clone() }; - let Json(response) = post::handler(State(app.clone()), creator, fixtures::now(), Json(request)) - .await - .expect("creating a channel in an empty app succeeds"); + let post::Response(response) = + post::handler(State(app.clone()), creator, fixtures::now(), Json(request)) + .await + .expect("creating a channel in an empty app succeeds"); // Verify the structure of the response @@ -96,7 +97,7 @@ async fn name_reusable_after_delete() { // Call the endpoint (first time) let request = post::Request { name: name.clone() }; - let Json(response) = post::handler( + let post::Response(response) = post::handler( State(app.clone()), creator.clone(), fixtures::now(), @@ -115,9 +116,10 @@ async fn name_reusable_after_delete() { // Call the endpoint (second time) let request = post::Request { name: name.clone() }; - let Json(response) = post::handler(State(app.clone()), creator, fixtures::now(), Json(request)) - .await - .expect("new channel in an empty app"); + let post::Response(response) = + post::handler(State(app.clone()), creator, fixtures::now(), Json(request)) + .await + .expect("new channel in an empty app"); // Verify the structure of the response diff --git a/src/invite/app.rs b/src/invite/app.rs index 4162470..ee7f74f 100644 --- a/src/invite/app.rs +++ b/src/invite/app.rs @@ -45,7 +45,7 @@ impl<'a> Invites<'a> { name: &str, password: &Password, accepted_at: &DateTime, - ) -> Result<Secret, AcceptError> { + ) -> Result<(Login, Secret), AcceptError> { let mut tx = self.db.begin().await?; let invite = tx .invites() @@ -72,7 +72,7 @@ impl<'a> Invites<'a> { let secret = tx.tokens().issue(&login, accepted_at).await?; tx.commit().await?; - Ok(secret) + Ok((login.as_created(), secret)) } pub async fn expire(&self, relative_to: &DateTime) -> Result<(), sqlx::Error> { diff --git a/src/invite/routes/invite/post.rs b/src/invite/routes/invite/post.rs index 12c2e21..c072929 100644 --- a/src/invite/routes/invite/post.rs +++ b/src/invite/routes/invite/post.rs @@ -9,7 +9,7 @@ use crate::{ clock::RequestedAt, error::{Internal, NotFound}, invite::app, - login::Password, + login::{Login, Password}, token::extract::IdentityToken, }; @@ -19,14 +19,14 @@ pub async fn handler( identity: IdentityToken, Path(invite): Path<super::PathInfo>, Json(request): Json<Request>, -) -> Result<(IdentityToken, StatusCode), Error> { - let secret = app +) -> Result<(IdentityToken, Json<Login>), Error> { + let (login, secret) = app .invites() .accept(&invite, &request.name, &request.password, &accepted_at) .await .map_err(Error)?; let identity = identity.set(secret); - Ok((identity, StatusCode::NO_CONTENT)) + Ok((identity, Json(login))) } #[derive(serde::Deserialize)] diff --git a/src/login/history.rs b/src/login/history.rs index f8d81bb..daad579 100644 --- a/src/login/history.rs +++ b/src/login/history.rs @@ -20,7 +20,6 @@ impl History { // if this returns a redacted or modified version of the login. If we implement // renames by redacting the original name, then this should return the edited // login, not the original, even if that's not how it was "as created.") - #[cfg(test)] pub fn as_created(&self) -> Login { self.login.clone() } @@ -30,6 +29,11 @@ impl History { .filter(Sequence::up_to(resume_point.into())) .collect() } + + // Snapshot of this login, as of all events recorded in this history. + pub fn as_snapshot(&self) -> Option<Login> { + self.events().collect() + } } // Events interface diff --git a/src/login/routes/login/post.rs b/src/login/routes/login/post.rs index e33acad..67eaa6d 100644 --- a/src/login/routes/login/post.rs +++ b/src/login/routes/login/post.rs @@ -8,7 +8,7 @@ use crate::{ app::App, clock::RequestedAt, error::Internal, - login::Password, + login::{Login, Password}, token::{app, extract::IdentityToken}, }; @@ -17,14 +17,14 @@ pub async fn handler( RequestedAt(now): RequestedAt, identity: IdentityToken, Json(request): Json<Request>, -) -> Result<(IdentityToken, StatusCode), Error> { - let token = app +) -> Result<(IdentityToken, Json<Login>), Error> { + let (login, secret) = app .tokens() .login(&request.name, &request.password, &now) .await .map_err(Error)?; - let identity = identity.set(token); - Ok((identity, StatusCode::NO_CONTENT)) + let identity = identity.set(secret); + Ok((identity, Json(login))) } #[derive(serde::Deserialize)] diff --git a/src/login/routes/login/test.rs b/src/login/routes/login/test.rs index d431612..c94f14c 100644 --- a/src/login/routes/login/test.rs +++ b/src/login/routes/login/test.rs @@ -1,7 +1,4 @@ -use axum::{ - extract::{Json, State}, - http::StatusCode, -}; +use axum::extract::{Json, State}; use super::post; use crate::{test::fixtures, token::app}; @@ -11,24 +8,24 @@ async fn correct_credentials() { // Set up the environment let app = fixtures::scratch_app().await; - let (name, password) = fixtures::login::create_with_password(&app, &fixtures::now()).await; + let (login, password) = fixtures::login::create_with_password(&app, &fixtures::now()).await; // Call the endpoint let identity = fixtures::identity::not_logged_in(); let logged_in_at = fixtures::now(); let request = post::Request { - name: name.clone(), + name: login.name.clone(), password, }; - let (identity, status) = + let (identity, Json(response)) = post::handler(State(app.clone()), logged_in_at, identity, Json(request)) .await .expect("logged in with valid credentials"); // Verify the return value's basic structure - assert_eq!(StatusCode::NO_CONTENT, status); + assert_eq!(login, response); let secret = identity.secret().expect("logged in with valid credentials"); // Verify the semantics @@ -40,7 +37,7 @@ async fn correct_credentials() { .await .expect("identity secret is valid"); - assert_eq!(name, validated_login.name); + assert_eq!(login, validated_login); } #[tokio::test] @@ -98,13 +95,16 @@ async fn token_expires() { // Set up the environment let app = fixtures::scratch_app().await; - let (name, password) = fixtures::login::create_with_password(&app, &fixtures::now()).await; + let (login, password) = fixtures::login::create_with_password(&app, &fixtures::now()).await; // Call the endpoint let logged_in_at = fixtures::ancient(); let identity = fixtures::identity::not_logged_in(); - let request = post::Request { name, password }; + let request = post::Request { + name: login.name.clone(), + password, + }; let (identity, _) = post::handler(State(app.clone()), logged_in_at, identity, Json(request)) .await .expect("logged in with valid credentials"); diff --git a/src/setup/app.rs b/src/setup/app.rs index 24e0010..d015813 100644 --- a/src/setup/app.rs +++ b/src/setup/app.rs @@ -4,7 +4,7 @@ use super::repo::Provider as _; use crate::{ clock::DateTime, event::{repo::Provider as _, Broadcaster, Event}, - login::{repo::Provider as _, Password}, + login::{repo::Provider as _, Login, Password}, token::{repo::Provider as _, Secret}, }; @@ -23,7 +23,7 @@ impl<'a> Setup<'a> { name: &str, password: &Password, created_at: &DateTime, - ) -> Result<Secret, Error> { + ) -> Result<(Login, Secret), Error> { let password_hash = password.hash()?; let mut tx = self.db.begin().await?; @@ -39,7 +39,7 @@ impl<'a> Setup<'a> { self.events .broadcast(login.events().map(Event::from).collect::<Vec<_>>()); - Ok(secret) + Ok((login.as_created(), secret)) } pub async fn completed(&self) -> Result<bool, sqlx::Error> { diff --git a/src/setup/routes/post.rs b/src/setup/routes/post.rs index 9e6776f..34f4ed2 100644 --- a/src/setup/routes/post.rs +++ b/src/setup/routes/post.rs @@ -5,7 +5,11 @@ use axum::{ }; use crate::{ - app::App, clock::RequestedAt, error::Internal, login::Password, setup::app, + app::App, + clock::RequestedAt, + error::Internal, + login::{Login, Password}, + setup::app, token::extract::IdentityToken, }; @@ -14,14 +18,14 @@ pub async fn handler( RequestedAt(setup_at): RequestedAt, identity: IdentityToken, Json(request): Json<Request>, -) -> Result<(IdentityToken, StatusCode), Error> { - let secret = app +) -> Result<(IdentityToken, Json<Login>), Error> { + let (login, secret) = app .setup() .initial(&request.name, &request.password, &setup_at) .await .map_err(Error)?; let identity = identity.set(secret); - Ok((identity, StatusCode::NO_CONTENT)) + Ok((identity, Json(login))) } #[derive(serde::Deserialize)] diff --git a/src/test/fixtures/identity.rs b/src/test/fixtures/identity.rs index 56b4ffa..c434473 100644 --- a/src/test/fixtures/identity.rs +++ b/src/test/fixtures/identity.rs @@ -3,7 +3,7 @@ use uuid::Uuid; use crate::{ app::App, clock::RequestedAt, - login::Password, + login::{Login, Password}, token::{ extract::{Identity, IdentityToken}, Secret, @@ -14,11 +14,11 @@ pub fn not_logged_in() -> IdentityToken { IdentityToken::new() } -pub async fn logged_in(app: &App, login: &(String, Password), now: &RequestedAt) -> IdentityToken { - let (name, password) = login; - let token = app +pub async fn logged_in(app: &App, login: &(Login, Password), now: &RequestedAt) -> IdentityToken { + let (login, password) = login; + let (_, token) = app .tokens() - .login(name, password, now) + .login(&login.name, password, now) .await .expect("should succeed given known-valid credentials"); @@ -36,7 +36,7 @@ pub async fn from_token(app: &App, token: &IdentityToken, issued_at: &RequestedA Identity { token, login } } -pub async fn identity(app: &App, login: &(String, Password), issued_at: &RequestedAt) -> Identity { +pub async fn identity(app: &App, login: &(Login, Password), issued_at: &RequestedAt) -> Identity { let secret = logged_in(app, login, issued_at).await; from_token(app, &secret, issued_at).await } diff --git a/src/test/fixtures/login.rs b/src/test/fixtures/login.rs index e5ac716..b6766fe 100644 --- a/src/test/fixtures/login.rs +++ b/src/test/fixtures/login.rs @@ -7,14 +7,15 @@ use crate::{ login::{self, Login, Password}, }; -pub async fn create_with_password(app: &App, created_at: &RequestedAt) -> (String, Password) { +pub async fn create_with_password(app: &App, created_at: &RequestedAt) -> (Login, Password) { let (name, password) = propose(); - app.logins() + let login = app + .logins() .create(&name, &password, created_at) .await .expect("should always succeed if the login is actually new"); - (name, password) + (login, password) } pub async fn create(app: &App, created_at: &RequestedAt) -> Login { diff --git a/src/token/app.rs b/src/token/app.rs index cb5d75f..0dc1a46 100644 --- a/src/token/app.rs +++ b/src/token/app.rs @@ -30,7 +30,7 @@ impl<'a> Tokens<'a> { name: &str, password: &Password, login_at: &DateTime, - ) -> Result<Secret, LoginError> { + ) -> Result<(Login, Secret), LoginError> { let mut tx = self.db.begin().await?; let (login, stored_hash) = tx .auth() @@ -45,6 +45,8 @@ impl<'a> Tokens<'a> { // if the account is deleted during that time. tx.commit().await?; + let snapshot = login.as_snapshot().ok_or(LoginError::Rejected)?; + let token = if stored_hash.verify(password)? { let mut tx = self.db.begin().await?; let token = tx.tokens().issue(&login, login_at).await?; @@ -54,7 +56,7 @@ impl<'a> Tokens<'a> { Err(LoginError::Rejected)? }; - Ok(token) + Ok((snapshot, token)) } pub async fn validate( |
