refactor: type safety + dedup cleanup across 13 code smells
- typed PagedResponse/CreatedApiKeyResponse/NotificationSummaryResponse replace json! blocks - extract TagRow/ApiKeyRow/OutboxRow to module level, top_friend uses sqlx flatten - add should_broadcast() helper, inline dead let bindings in federation_event - add UploadContext struct, extract_upload_field, wants_activity_json helpers - rename PostgresFederationRepository→PgFederationRepository, PostgresApUserRepository→PgApUserRepository - add IntoAnyhow trait replacing ~30 .map_err(|e| anyhow!(e)) calls - extract build_ap_service shared between bootstrap and worker factories - add postgres/constants.rs, PartialEq+Eq on PasswordHash
This commit is contained in:
@@ -8,6 +8,10 @@ use domain::{
|
||||
};
|
||||
use std::sync::Arc;
|
||||
|
||||
fn should_broadcast(t: &domain::models::thought::Thought) -> bool {
|
||||
t.local && matches!(t.visibility, Visibility::Public | Visibility::Unlisted)
|
||||
}
|
||||
|
||||
pub struct FederationEventService {
|
||||
pub thoughts: Arc<dyn ThoughtRepository>,
|
||||
pub users: Arc<dyn UserReader>,
|
||||
@@ -32,15 +36,7 @@ impl FederationEventService {
|
||||
..
|
||||
} => {
|
||||
let thought = match self.thoughts.find_by_id(thought_id).await? {
|
||||
Some(t)
|
||||
if t.local
|
||||
&& matches!(
|
||||
t.visibility,
|
||||
Visibility::Public | Visibility::Unlisted
|
||||
) =>
|
||||
{
|
||||
t
|
||||
}
|
||||
Some(t) if should_broadcast(&t) => t,
|
||||
_ => {
|
||||
tracing::debug!(thought_id = %thought_id, "federation: skipping ThoughtCreated (remote or non-public)");
|
||||
return Ok(());
|
||||
@@ -86,15 +82,7 @@ impl FederationEventService {
|
||||
user_id,
|
||||
} => {
|
||||
let thought = match self.thoughts.find_by_id(thought_id).await? {
|
||||
Some(t)
|
||||
if t.local
|
||||
&& matches!(
|
||||
t.visibility,
|
||||
Visibility::Public | Visibility::Unlisted
|
||||
) =>
|
||||
{
|
||||
t
|
||||
}
|
||||
Some(t) if should_broadcast(&t) => t,
|
||||
_ => return Ok(()),
|
||||
};
|
||||
let user = match self.users.find_by_id(user_id).await? {
|
||||
@@ -125,14 +113,10 @@ impl FederationEventService {
|
||||
user_id,
|
||||
thought_id,
|
||||
} => {
|
||||
let booster = match self.users.find_by_id(user_id).await? {
|
||||
Some(u) if u.local => u,
|
||||
_ => {
|
||||
tracing::debug!(user_id = %user_id, "federation: skipping BoostAdded (remote user)");
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
let _ = booster;
|
||||
if !matches!(self.users.find_by_id(user_id).await?, Some(u) if u.local) {
|
||||
tracing::debug!(user_id = %user_id, "federation: skipping BoostAdded (remote user)");
|
||||
return Ok(());
|
||||
}
|
||||
if self.thoughts.find_by_id(thought_id).await?.is_none() {
|
||||
return Ok(());
|
||||
}
|
||||
@@ -160,14 +144,10 @@ impl FederationEventService {
|
||||
user_id,
|
||||
thought_id,
|
||||
} => {
|
||||
let liker = match self.users.find_by_id(user_id).await? {
|
||||
Some(u) if u.local => u,
|
||||
_ => {
|
||||
tracing::debug!(user_id = %user_id, "federation: skipping LikeAdded (remote user)");
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
let _ = liker;
|
||||
if !matches!(self.users.find_by_id(user_id).await?, Some(u) if u.local) {
|
||||
tracing::debug!(user_id = %user_id, "federation: skipping LikeAdded (remote user)");
|
||||
return Ok(());
|
||||
}
|
||||
let thought = match self.thoughts.find_by_id(thought_id).await? {
|
||||
Some(t) => t,
|
||||
_ => return Ok(()),
|
||||
@@ -193,11 +173,9 @@ impl FederationEventService {
|
||||
user_id,
|
||||
thought_id,
|
||||
} => {
|
||||
let liker = match self.users.find_by_id(user_id).await? {
|
||||
Some(u) if u.local => u,
|
||||
_ => return Ok(()),
|
||||
};
|
||||
let _ = liker;
|
||||
if !matches!(self.users.find_by_id(user_id).await?, Some(u) if u.local) {
|
||||
return Ok(());
|
||||
}
|
||||
let thought = match self.thoughts.find_by_id(thought_id).await? {
|
||||
Some(t) => t,
|
||||
_ => return Ok(()),
|
||||
|
||||
@@ -118,17 +118,25 @@ fn mime_to_ext(mime: &str) -> Result<&'static str, DomainError> {
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub struct UploadContext<'a> {
|
||||
pub users: &'a dyn UserRepository,
|
||||
pub media: &'a dyn MediaStore,
|
||||
pub events: &'a dyn EventPublisher,
|
||||
pub upload_config: &'a UploadConfig,
|
||||
pub base_url: &'a str,
|
||||
}
|
||||
|
||||
async fn store_image(
|
||||
media: &dyn MediaStore,
|
||||
base_url: &str,
|
||||
cfg: &UploadConfig,
|
||||
ctx: &UploadContext<'_>,
|
||||
content_type: &str,
|
||||
data: Bytes,
|
||||
user_id: &UserId,
|
||||
key_segment: &str,
|
||||
old_url: Option<&str>,
|
||||
) -> Result<String, DomainError> {
|
||||
let cfg = ctx.upload_config;
|
||||
let media = ctx.media;
|
||||
let base_url = ctx.base_url;
|
||||
if !cfg.allowed_content_types.iter().any(|t| t == content_type) {
|
||||
return Err(DomainError::InvalidInput("unsupported content type".into()));
|
||||
}
|
||||
@@ -148,25 +156,19 @@ async fn store_image(
|
||||
Ok(key)
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub async fn upload_avatar(
|
||||
users: &dyn UserRepository,
|
||||
media: &dyn MediaStore,
|
||||
events: &dyn EventPublisher,
|
||||
ctx: &UploadContext<'_>,
|
||||
user_id: &UserId,
|
||||
base_url: &str,
|
||||
cfg: &UploadConfig,
|
||||
content_type: &str,
|
||||
data: Bytes,
|
||||
) -> Result<(), DomainError> {
|
||||
let current = users
|
||||
let current = ctx
|
||||
.users
|
||||
.find_by_id(user_id)
|
||||
.await?
|
||||
.ok_or(DomainError::NotFound)?;
|
||||
let key = store_image(
|
||||
media,
|
||||
base_url,
|
||||
cfg,
|
||||
ctx,
|
||||
content_type,
|
||||
data,
|
||||
user_id,
|
||||
@@ -174,41 +176,35 @@ pub async fn upload_avatar(
|
||||
current.avatar_url.as_deref(),
|
||||
)
|
||||
.await?;
|
||||
users
|
||||
ctx.users
|
||||
.update_profile(
|
||||
user_id,
|
||||
UpdateProfileInput {
|
||||
avatar_url: Some(format!("{base_url}/media/{key}")),
|
||||
avatar_url: Some(format!("{}/media/{key}", ctx.base_url)),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
events
|
||||
ctx.events
|
||||
.publish(&DomainEvent::ProfileUpdated {
|
||||
user_id: user_id.clone(),
|
||||
})
|
||||
.await
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub async fn upload_banner(
|
||||
users: &dyn UserRepository,
|
||||
media: &dyn MediaStore,
|
||||
events: &dyn EventPublisher,
|
||||
ctx: &UploadContext<'_>,
|
||||
user_id: &UserId,
|
||||
base_url: &str,
|
||||
cfg: &UploadConfig,
|
||||
content_type: &str,
|
||||
data: Bytes,
|
||||
) -> Result<(), DomainError> {
|
||||
let current = users
|
||||
let current = ctx
|
||||
.users
|
||||
.find_by_id(user_id)
|
||||
.await?
|
||||
.ok_or(DomainError::NotFound)?;
|
||||
let key = store_image(
|
||||
media,
|
||||
base_url,
|
||||
cfg,
|
||||
ctx,
|
||||
content_type,
|
||||
data,
|
||||
user_id,
|
||||
@@ -216,16 +212,16 @@ pub async fn upload_banner(
|
||||
current.header_url.as_deref(),
|
||||
)
|
||||
.await?;
|
||||
users
|
||||
ctx.users
|
||||
.update_profile(
|
||||
user_id,
|
||||
UpdateProfileInput {
|
||||
header_url: Some(format!("{base_url}/media/{key}")),
|
||||
header_url: Some(format!("{}/media/{key}", ctx.base_url)),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
events
|
||||
ctx.events
|
||||
.publish(&DomainEvent::ProfileUpdated {
|
||||
user_id: user_id.clone(),
|
||||
})
|
||||
|
||||
@@ -113,24 +113,31 @@ fn default_cfg() -> UploadConfig {
|
||||
UploadConfig::default()
|
||||
}
|
||||
|
||||
fn make_ctx<'a>(
|
||||
store: &'a TestStore,
|
||||
media: &'a MockMedia,
|
||||
cfg: &'a UploadConfig,
|
||||
) -> UploadContext<'a> {
|
||||
UploadContext {
|
||||
users: store,
|
||||
media,
|
||||
events: store,
|
||||
upload_config: cfg,
|
||||
base_url: "http://localhost",
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn upload_avatar_rejects_unsupported_mime() {
|
||||
let store = TestStore::default();
|
||||
let media = MockMedia::default();
|
||||
let user = make_user();
|
||||
store.users.lock().unwrap().push(user.clone());
|
||||
let err = upload_avatar(
|
||||
&store,
|
||||
&media,
|
||||
&store,
|
||||
&user.id,
|
||||
"http://localhost",
|
||||
&default_cfg(),
|
||||
"text/plain",
|
||||
Bytes::from("hi"),
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
let cfg = default_cfg();
|
||||
let ctx = make_ctx(&store, &media, &cfg);
|
||||
let err = upload_avatar(&ctx, &user.id, "text/plain", Bytes::from("hi"))
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(err, DomainError::InvalidInput(_)));
|
||||
}
|
||||
|
||||
@@ -141,18 +148,11 @@ async fn upload_avatar_rejects_oversized_data() {
|
||||
let user = make_user();
|
||||
store.users.lock().unwrap().push(user.clone());
|
||||
let big = Bytes::from(vec![0u8; 6 * 1024 * 1024]);
|
||||
let err = upload_avatar(
|
||||
&store,
|
||||
&media,
|
||||
&store,
|
||||
&user.id,
|
||||
"http://localhost",
|
||||
&default_cfg(),
|
||||
"image/jpeg",
|
||||
big,
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
let cfg = default_cfg();
|
||||
let ctx = make_ctx(&store, &media, &cfg);
|
||||
let err = upload_avatar(&ctx, &user.id, "image/jpeg", big)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(err, DomainError::InvalidInput(_)));
|
||||
}
|
||||
|
||||
@@ -162,18 +162,11 @@ async fn upload_avatar_stores_file_and_updates_url() {
|
||||
let media = MockMedia::default();
|
||||
let user = make_user();
|
||||
store.users.lock().unwrap().push(user.clone());
|
||||
upload_avatar(
|
||||
&store,
|
||||
&media,
|
||||
&store,
|
||||
&user.id,
|
||||
"http://localhost",
|
||||
&default_cfg(),
|
||||
"image/jpeg",
|
||||
Bytes::from("img"),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let cfg = default_cfg();
|
||||
let ctx = make_ctx(&store, &media, &cfg);
|
||||
upload_avatar(&ctx, &user.id, "image/jpeg", Bytes::from("img"))
|
||||
.await
|
||||
.unwrap();
|
||||
let key = format!("users/{}/avatar.jpg", user.id.as_uuid());
|
||||
assert!(media.store.lock().unwrap().contains_key(&key));
|
||||
let saved = store
|
||||
@@ -203,18 +196,11 @@ async fn upload_avatar_deletes_old_file_on_reupload() {
|
||||
.lock()
|
||||
.unwrap()
|
||||
.insert(old_key.clone(), Bytes::from("old"));
|
||||
upload_avatar(
|
||||
&store,
|
||||
&media,
|
||||
&store,
|
||||
&user.id,
|
||||
"http://localhost",
|
||||
&default_cfg(),
|
||||
"image/jpeg",
|
||||
Bytes::from("new"),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let cfg = default_cfg();
|
||||
let ctx = make_ctx(&store, &media, &cfg);
|
||||
upload_avatar(&ctx, &user.id, "image/jpeg", Bytes::from("new"))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!media.store.lock().unwrap().contains_key(&old_key));
|
||||
assert!(media.deleted.lock().unwrap().contains(&old_key));
|
||||
}
|
||||
@@ -225,18 +211,11 @@ async fn upload_banner_stores_file_and_updates_header_url() {
|
||||
let media = MockMedia::default();
|
||||
let user = make_user();
|
||||
store.users.lock().unwrap().push(user.clone());
|
||||
upload_banner(
|
||||
&store,
|
||||
&media,
|
||||
&store,
|
||||
&user.id,
|
||||
"http://localhost",
|
||||
&default_cfg(),
|
||||
"image/png",
|
||||
Bytes::from("banner"),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let cfg = default_cfg();
|
||||
let ctx = make_ctx(&store, &media, &cfg);
|
||||
upload_banner(&ctx, &user.id, "image/png", Bytes::from("banner"))
|
||||
.await
|
||||
.unwrap();
|
||||
let key = format!("users/{}/banner.png", user.id.as_uuid());
|
||||
assert!(media.store.lock().unwrap().contains_key(&key));
|
||||
let saved = store
|
||||
@@ -266,18 +245,11 @@ async fn upload_banner_deletes_old_file_on_reupload() {
|
||||
.lock()
|
||||
.unwrap()
|
||||
.insert(old_key.clone(), Bytes::from("old"));
|
||||
upload_banner(
|
||||
&store,
|
||||
&media,
|
||||
&store,
|
||||
&user.id,
|
||||
"http://localhost",
|
||||
&default_cfg(),
|
||||
"image/png",
|
||||
Bytes::from("new"),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let cfg = default_cfg();
|
||||
let ctx = make_ctx(&store, &media, &cfg);
|
||||
upload_banner(&ctx, &user.id, "image/png", Bytes::from("new"))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!media.store.lock().unwrap().contains_key(&old_key));
|
||||
assert!(media.deleted.lock().unwrap().contains(&old_key));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user