feat(backend): DELETE /avatars removes the row and the S3 object
Frontend "Remove avatar" needs a way to clear the current user's picture without uploading a placeholder. Adds the missing route. - S3Storage::delete wraps aws_sdk_s3 DeleteObject. MinIO's delete is idempotent for missing keys, so a stray miss is silent. - repo::clear_avatar returns true when a row existed so the handler can decide whether to also try the S3 delete (skip for users without an avatar). - handlers::delete is authenticated (GatewayIdentity) and always returns 204, matching how the frontend button should behave after a repeat click. S3 delete is best-effort: if MinIO is unreachable we already committed the DB row removal and log a warn — the object becomes orphaned but is not served, because serve() looks up keys via the DB, not by URL. - Route registered on the existing avatar_routes Router. - 3 tests: happy path (post-then-delete → count==0), idempotent no-op, 401 without identity.
This commit is contained in:
parent
d5f17eb0ac
commit
a597425e45
5 changed files with 122 additions and 0 deletions
|
|
@ -61,6 +61,17 @@ pub async fn set_avatar(pool: &PgPool, account_id: Uuid, s3_key: &str) -> Result
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Deletes the `avatars` row for one account. Returns `true` when a row existed
|
||||||
|
/// (so the caller can decide whether to also delete the S3 object) and `false`
|
||||||
|
/// when the user simply had no avatar — the API still treats that as success.
|
||||||
|
pub async fn clear_avatar(pool: &PgPool, account_id: Uuid) -> Result<bool, sqlx::Error> {
|
||||||
|
let result = sqlx::query("DELETE FROM avatars WHERE account_id = $1")
|
||||||
|
.bind(account_id)
|
||||||
|
.execute(pool)
|
||||||
|
.await?;
|
||||||
|
Ok(result.rows_affected() > 0)
|
||||||
|
}
|
||||||
|
|
||||||
/// Avatar reference for one account: its storage key plus an `uploaded_at`
|
/// Avatar reference for one account: its storage key plus an `uploaded_at`
|
||||||
/// version (epoch seconds) used to cache-bust the served image URL.
|
/// version (epoch seconds) used to cache-bust the served image URL.
|
||||||
pub async fn avatar_ref(
|
pub async fn avatar_ref(
|
||||||
|
|
|
||||||
|
|
@ -70,6 +70,28 @@ pub async fn upload(
|
||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// DELETE /avatars — clears the caller's avatar. Removes the DB row and, if one
|
||||||
|
/// existed, the S3 object. Always returns 204, including when the user had no
|
||||||
|
/// avatar: the operation is idempotent, matching how the frontend's "Remove
|
||||||
|
/// avatar" button should behave after a successful click.
|
||||||
|
pub async fn delete(
|
||||||
|
State(state): State<AvatarState>,
|
||||||
|
identity: GatewayIdentity,
|
||||||
|
) -> Result<Response, AppError> {
|
||||||
|
let account_id = identity.account_id;
|
||||||
|
let existed = repo::clear_avatar(&state.pool, account_id).await?;
|
||||||
|
if existed {
|
||||||
|
let key = format!("avatars/{account_id}.png");
|
||||||
|
// S3 delete is best-effort: if MinIO is unreachable we already committed
|
||||||
|
// the DB row removal, and the object becomes orphaned but not served (the
|
||||||
|
// serve handler looks up keys via the DB, not directly).
|
||||||
|
if let Err(err) = state.storage.delete(&key).await {
|
||||||
|
tracing::warn!("avatar S3 delete failed for {account_id}: {err}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Ok(StatusCode::NO_CONTENT.into_response())
|
||||||
|
}
|
||||||
|
|
||||||
/// Only serve the keys this service writes: `avatars/<account-uuid>.png`.
|
/// Only serve the keys this service writes: `avatars/<account-uuid>.png`.
|
||||||
/// Anything else (traversal, other buckets/paths) is a 404. The gateway blocks
|
/// Anything else (traversal, other buckets/paths) is a 404. The gateway blocks
|
||||||
/// dot-segments upstream, but the service validates independently.
|
/// dot-segments upstream, but the service validates independently.
|
||||||
|
|
|
||||||
|
|
@ -51,4 +51,17 @@ impl S3Storage {
|
||||||
let aggregated = out.body.collect().await?;
|
let aggregated = out.body.collect().await?;
|
||||||
Ok(aggregated.into_bytes().to_vec())
|
Ok(aggregated.into_bytes().to_vec())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Deletes the object at `key`. MinIO's DeleteObject is idempotent — deleting
|
||||||
|
/// a missing key returns Ok. Callers on the app layer are expected to run
|
||||||
|
/// this only after a matching DB row existed, so a stray 404 here is silent.
|
||||||
|
pub async fn delete(&self, key: &str) -> anyhow::Result<()> {
|
||||||
|
self.client
|
||||||
|
.delete_object()
|
||||||
|
.bucket(&self.bucket)
|
||||||
|
.key(key)
|
||||||
|
.send()
|
||||||
|
.await?;
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -89,6 +89,7 @@ pub fn build_app_with_mail(pool: sqlx::PgPool, cfg: &Config, mail: auth::reset::
|
||||||
|
|
||||||
let avatar_routes = Router::new()
|
let avatar_routes = Router::new()
|
||||||
.route("/avatars", post(avatars::handlers::upload))
|
.route("/avatars", post(avatars::handlers::upload))
|
||||||
|
.route("/avatars", axum::routing::delete(avatars::handlers::delete))
|
||||||
// Public avatar read served back through the gateway; the `{*key}`
|
// Public avatar read served back through the gateway; the `{*key}`
|
||||||
// wildcard is the storage key (`avatars/<uuid>.png`).
|
// wildcard is the storage key (`avatars/<uuid>.png`).
|
||||||
.route("/media/{*key}", get(avatars::handlers::serve))
|
.route("/media/{*key}", get(avatars::handlers::serve))
|
||||||
|
|
|
||||||
|
|
@ -121,3 +121,78 @@ async fn upload_without_identity_is_401() {
|
||||||
.await
|
.await
|
||||||
.assert_status(StatusCode::UNAUTHORIZED);
|
.assert_status(StatusCode::UNAUTHORIZED);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn delete_removes_the_row_and_returns_204() {
|
||||||
|
common::init_test_logging();
|
||||||
|
common::ensure_avatar_bucket().await;
|
||||||
|
let pool = common::test_pool().await;
|
||||||
|
let server = common::test_server(accounts_service::build_app(
|
||||||
|
pool.clone(),
|
||||||
|
&common::test_config(),
|
||||||
|
));
|
||||||
|
let (account_id, email) = common::register_account(&server).await;
|
||||||
|
|
||||||
|
server
|
||||||
|
.post("/avatars")
|
||||||
|
.add_header(ACCOUNT_ID_HEADER, account_id.to_string())
|
||||||
|
.multipart(form(png_bytes()))
|
||||||
|
.await
|
||||||
|
.assert_status_ok();
|
||||||
|
|
||||||
|
server
|
||||||
|
.delete("/avatars")
|
||||||
|
.add_header(ACCOUNT_ID_HEADER, account_id.to_string())
|
||||||
|
.await
|
||||||
|
.assert_status(StatusCode::NO_CONTENT);
|
||||||
|
|
||||||
|
let count: i64 = sqlx::query_as::<_, (i64,)>("SELECT COUNT(*) FROM avatars WHERE account_id = $1")
|
||||||
|
.bind(account_id)
|
||||||
|
.fetch_one(&pool)
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.0;
|
||||||
|
assert_eq!(count, 0);
|
||||||
|
|
||||||
|
sqlx::query("DELETE FROM accounts WHERE lower(email) = $1")
|
||||||
|
.bind(email.to_lowercase())
|
||||||
|
.execute(&pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn delete_without_avatar_is_idempotent() {
|
||||||
|
let pool = common::test_pool().await;
|
||||||
|
let server = common::test_server(accounts_service::build_app(
|
||||||
|
pool.clone(),
|
||||||
|
&common::test_config(),
|
||||||
|
));
|
||||||
|
let (account_id, email) = common::register_account(&server).await;
|
||||||
|
|
||||||
|
server
|
||||||
|
.delete("/avatars")
|
||||||
|
.add_header(ACCOUNT_ID_HEADER, account_id.to_string())
|
||||||
|
.await
|
||||||
|
.assert_status(StatusCode::NO_CONTENT);
|
||||||
|
|
||||||
|
sqlx::query("DELETE FROM accounts WHERE lower(email) = $1")
|
||||||
|
.bind(email.to_lowercase())
|
||||||
|
.execute(&pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn delete_without_identity_is_401() {
|
||||||
|
let pool = common::test_pool().await;
|
||||||
|
let server = common::test_server(accounts_service::build_app(
|
||||||
|
pool.clone(),
|
||||||
|
&common::test_config(),
|
||||||
|
));
|
||||||
|
|
||||||
|
server
|
||||||
|
.delete("/avatars")
|
||||||
|
.await
|
||||||
|
.assert_status(StatusCode::UNAUTHORIZED);
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue