diff options
| author | Owen Jacobson <owen@grimoire.ca> | 2024-10-11 22:57:56 -0400 |
|---|---|---|
| committer | Owen Jacobson <owen@grimoire.ca> | 2024-10-11 22:57:56 -0400 |
| commit | 756863f298f9e4277863f9e8758e253c5ae95923 (patch) | |
| tree | 82a9c6643b360b035f4630f2b1db138d9937c8d8 | |
| parent | 812f1cafe3f8a68bf45b677fade7417c30d92eac (diff) | |
Return a distinct error when an invite username is in use.
I've also aligned channel creation with this (it's 409 Conflict). To make server setup more distinct, it now returns 503 Service Unavailable if setup has not been completed.
| -rw-r--r-- | docs/api.md | 8 | ||||
| -rw-r--r-- | src/channel/app.rs | 16 | ||||
| -rw-r--r-- | src/channel/routes.rs | 2 | ||||
| -rw-r--r-- | src/db/mod.rs | 30 | ||||
| -rw-r--r-- | src/invite/app.rs | 10 | ||||
| -rw-r--r-- | src/invite/routes.rs | 3 | ||||
| -rw-r--r-- | src/setup/middleware.rs | 6 | ||||
| -rw-r--r-- | ui/lib/apiServer.js | 2 | ||||
| -rw-r--r-- | ui/routes/(app)/+layout.svelte | 2 |
9 files changed, 57 insertions, 22 deletions
diff --git a/docs/api.md b/docs/api.md index f91780e..d544689 100644 --- a/docs/api.md +++ b/docs/api.md @@ -10,7 +10,7 @@ Requests that require a JSON body must include a `content-type: application/json ## Initial setup -The `hi` service requires setup before it can enter service. This setup is performed online, via the `hi` API. Any request to an API endpoint before setup has been completed will return a 409 Conflict response, unless the endpoint is documented as allowing requests before setup. +The `hi` service requires setup before it can enter service. This setup is performed online, via the `hi` API. Any request to an API endpoint before setup has been completed will return a 503 Service Unavailable response, unless the endpoint is documented as allowing requests before setup. ### `POST /api/setup` @@ -197,6 +197,10 @@ This endpoint returns a 204 No Content response on success, with a `Set-Cookie` This endpoint returns a 404 Not Found response if the invite ID in the path does not exist, or has already been accepted. +#### Name already used + +This endpoint returns a 409 Conflict response if the requested login name has already been taken. The invite can be re-accepted with a different name. + ## Working with channels Channels are the containers for conversations. The API supports listing channels, creating new channels, and send messages to an existing channel. @@ -224,7 +228,7 @@ Creates a channel. #### On duplicate channel name -Channel names must be unique. If a channel with the same name already exists, this will return a 400 Bad Request error. +Channel names must be unique. If a channel with the same name already exists, this will return a 409 Conflict error. ## Events diff --git a/src/channel/app.rs b/src/channel/app.rs index 7c0b107..5d6cada 100644 --- a/src/channel/app.rs +++ b/src/channel/app.rs @@ -5,7 +5,7 @@ use sqlx::sqlite::SqlitePool; use super::{repo::Provider as _, Channel, History, Id}; use crate::{ clock::DateTime, - db::NotFound, + db::{Duplicate as _, NotFound as _}, event::{repo::Provider as _, Broadcaster, Event, Sequence}, message::repo::Provider as _, }; @@ -27,7 +27,7 @@ impl<'a> Channels<'a> { .channels() .create(name, &created) .await - .map_err(|err| CreateError::from_duplicate_name(err, name))?; + .duplicate(|| CreateError::DuplicateName(name.into()))?; tx.commit().await?; self.events @@ -133,18 +133,6 @@ pub enum Error { DatabaseError(#[from] sqlx::Error), } -impl CreateError { - fn from_duplicate_name(error: sqlx::Error, name: &str) -> Self { - if let Some(error) = error.as_database_error() { - if error.is_unique_violation() { - return Self::DuplicateName(name.into()); - } - } - - Self::from(error) - } -} - #[derive(Debug, thiserror::Error)] pub enum InternalError { #[error(transparent)] diff --git a/src/channel/routes.rs b/src/channel/routes.rs index e97c447..eaf7962 100644 --- a/src/channel/routes.rs +++ b/src/channel/routes.rs @@ -53,7 +53,7 @@ impl IntoResponse for CreateError { let Self(error) = self; match error { duplicate @ app::CreateError::DuplicateName(_) => { - (StatusCode::BAD_REQUEST, duplicate.to_string()).into_response() + (StatusCode::CONFLICT, duplicate.to_string()).into_response() } other => Internal::from(other).into_response(), } diff --git a/src/db/mod.rs b/src/db/mod.rs index fa3d74e..6005813 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -4,6 +4,7 @@ use std::str::FromStr; use hex_literal::hex; use sqlx::{ + error::{DatabaseError, ErrorKind}, migrate::{Migrate as _, MigrateDatabase as _}, sqlite::{Sqlite, SqliteConnectOptions, SqlitePool, SqlitePoolOptions}, }; @@ -161,3 +162,32 @@ impl<T> NotFound for Result<T, sqlx::Error> { self.optional()?.ok_or_else(map) } } + +pub trait Duplicate { + type Ok; + type Error; + + fn duplicate<E, F>(self, map: F) -> Result<Self::Ok, E> + where + E: From<Self::Error>, + F: FnOnce() -> E; +} + +impl<T> Duplicate for Result<T, sqlx::Error> { + type Ok = T; + type Error = sqlx::Error; + + fn duplicate<E, F>(self, map: F) -> Result<T, E> + where + E: From<sqlx::Error>, + F: FnOnce() -> E, + { + match self { + Ok(value) => Ok(value), + Err(error) => match error.as_database_error().map(DatabaseError::kind) { + Some(ErrorKind::UniqueViolation) => Err(map()), + _ => Err(error.into()), + }, + } + } +} diff --git a/src/invite/app.rs b/src/invite/app.rs index 998b4f1..6800d72 100644 --- a/src/invite/app.rs +++ b/src/invite/app.rs @@ -4,7 +4,7 @@ use sqlx::sqlite::SqlitePool; use super::{repo::Provider as _, Id, Invite, Summary}; use crate::{ clock::DateTime, - db::NotFound as _, + db::{Duplicate as _, NotFound as _}, event::repo::Provider as _, login::{repo::Provider as _, Login, Password}, token::{repo::Provider as _, Secret}, @@ -68,7 +68,11 @@ impl<'a> Invites<'a> { // catch it. tx.invites().accept(&invite).await?; let created = tx.sequence().next(accepted_at).await?; - let login = tx.logins().create(name, &password_hash, &created).await?; + let login = tx + .logins() + .create(name, &password_hash, &created) + .await + .duplicate(|| AcceptError::DuplicateLogin(name.into()))?; let secret = tx.tokens().issue(&login, accepted_at).await?; tx.commit().await?; @@ -99,6 +103,8 @@ pub enum Error { pub enum AcceptError { #[error("invite not found: {0}")] NotFound(Id), + #[error("name in use: {0}")] + DuplicateLogin(String), #[error(transparent)] Database(#[from] sqlx::Error), #[error(transparent)] diff --git a/src/invite/routes.rs b/src/invite/routes.rs index 3384e10..977fe9b 100644 --- a/src/invite/routes.rs +++ b/src/invite/routes.rs @@ -88,6 +88,9 @@ impl IntoResponse for AcceptError { let Self(error) = self; match error { error @ app::AcceptError::NotFound(_) => NotFound(error).into_response(), + error @ app::AcceptError::DuplicateLogin(_) => { + (StatusCode::CONFLICT, error.to_string()).into_response() + } other => Internal::from(other).into_response(), } } diff --git a/src/setup/middleware.rs b/src/setup/middleware.rs index a5f9070..5f9996b 100644 --- a/src/setup/middleware.rs +++ b/src/setup/middleware.rs @@ -10,7 +10,11 @@ use crate::{app::App, error::Internal}; pub async fn setup_required(State(app): State<App>, request: Request, next: Next) -> Response { match app.setup().completed().await { Ok(true) => next.run(request).await, - Ok(false) => (StatusCode::CONFLICT, "initial setup not completed").into_response(), + Ok(false) => ( + StatusCode::SERVICE_UNAVAILABLE, + "initial setup not completed", + ) + .into_response(), Err(error) => Internal::from(error).into_response(), } } diff --git a/ui/lib/apiServer.js b/ui/lib/apiServer.js index 46fcb53..76ecb87 100644 --- a/ui/lib/apiServer.js +++ b/ui/lib/apiServer.js @@ -3,7 +3,7 @@ import { channelsList, logins, messages } from '$lib/store'; export const apiServer = axios.create({ baseURL: '/api/', - validateStatus: (status) => status >= 200 && status < 500, + validateStatus: () => true, }); export async function boot() { diff --git a/ui/routes/(app)/+layout.svelte b/ui/routes/(app)/+layout.svelte index 38df9b9..9abaaf4 100644 --- a/ui/routes/(app)/+layout.svelte +++ b/ui/routes/(app)/+layout.svelte @@ -36,7 +36,7 @@ currentUser.update(() => null); goto('/login'); break; - case 409: + case 503: currentUser.update(() => null); goto('/setup'); break; |
