refactor(application): fix 4 code smells — validate username input, extract ownership guard and dedup helpers
Some checks failed
lint / lint (push) Has been cancelled
test / unit (push) Has been cancelled
test / integration (push) Has been cancelled
lint / lint (pull_request) Failing after 5m12s
test / unit (pull_request) Successful in 16m23s
test / integration (pull_request) Failing after 17m26s

This commit is contained in:
2026-05-14 16:27:03 +02:00
parent dd7beb7ab4
commit e6f4a6256f
4 changed files with 28 additions and 14 deletions

View File

@@ -2,8 +2,9 @@ use std::sync::Arc;
use domain::{
errors::DomainError,
events::DomainEvent,
models::thought::Visibility,
models::thought::{Thought, Visibility},
ports::{OutboundFederationPort, ThoughtRepository, UserRepository},
value_objects::ThoughtId,
};
pub struct FederationEventService {
@@ -14,6 +15,12 @@ pub struct FederationEventService {
}
impl FederationEventService {
fn object_ap_id(&self, thought: &Thought, thought_id: &ThoughtId) -> String {
thought.ap_id.clone().unwrap_or_else(|| {
format!("{}/thoughts/{}", self.base_url, thought_id)
})
}
pub async fn process(&self, event: &DomainEvent) -> Result<(), DomainError> {
match event {
DomainEvent::ThoughtCreated { thought_id, user_id, .. } => {
@@ -52,9 +59,7 @@ impl FederationEventService {
Some(t) => t,
None => return Ok(()),
};
let object_ap_id = thought.ap_id.clone().unwrap_or_else(|| {
format!("{}/thoughts/{}", self.base_url, thought_id)
});
let object_ap_id = self.object_ap_id(&thought, thought_id);
self.ap.broadcast_announce(user_id, &object_ap_id).await
}
@@ -63,9 +68,7 @@ impl FederationEventService {
Some(t) => t,
None => return Ok(()),
};
let object_ap_id = thought.ap_id.clone().unwrap_or_else(|| {
format!("{}/thoughts/{}", self.base_url, thought_id)
});
let object_ap_id = self.object_ap_id(&thought, thought_id);
self.ap.broadcast_undo_announce(user_id, &object_ap_id).await
}

View File

@@ -5,7 +5,7 @@ use domain::{
events::DomainEvent,
models::notification::{Notification, NotificationType},
ports::{NotificationRepository, ThoughtRepository},
value_objects::NotificationId,
value_objects::{NotificationId, UserId},
};
pub struct NotificationEventService {
@@ -13,6 +13,10 @@ pub struct NotificationEventService {
pub notifications: Arc<dyn NotificationRepository>,
}
fn is_self_action(thought_author: &UserId, actor: &UserId) -> bool {
thought_author == actor
}
impl NotificationEventService {
pub async fn process(&self, event: &DomainEvent) -> Result<(), DomainError> {
match event {
@@ -21,7 +25,7 @@ impl NotificationEventService {
Some(t) => t,
None => return Ok(()),
};
if thought.user_id == *user_id { return Ok(()); }
if is_self_action(&thought.user_id, user_id) { return Ok(()); }
self.notifications.save(&Notification {
id: NotificationId::new(),
user_id: thought.user_id,
@@ -37,7 +41,7 @@ impl NotificationEventService {
Some(t) => t,
None => return Ok(()),
};
if thought.user_id == *user_id { return Ok(()); }
if is_self_action(&thought.user_id, user_id) { return Ok(()); }
self.notifications.save(&Notification {
id: NotificationId::new(),
user_id: thought.user_id,
@@ -68,7 +72,7 @@ impl NotificationEventService {
Some(t) => t,
None => return Ok(()),
};
if original.user_id == *user_id { return Ok(()); }
if is_self_action(&original.user_id, user_id) { return Ok(()); }
self.notifications.save(&Notification {
id: NotificationId::new(),
user_id: original.user_id,

View File

@@ -10,7 +10,7 @@ pub async fn get_user(users: &dyn UserRepository, user_id: &UserId) -> Result<Us
}
pub async fn get_user_by_username(users: &dyn UserRepository, username: &str) -> Result<User, DomainError> {
let username = Username::from_trusted(username.to_string());
let username = Username::new(username).map_err(|_| DomainError::NotFound)?;
users.find_by_username(&username).await?.ok_or(DomainError::NotFound)
}

View File

@@ -6,6 +6,13 @@ use domain::{
value_objects::{Content, ThoughtId, UserId},
};
fn require_owner(thought: &Thought, user_id: &UserId) -> Result<(), DomainError> {
if thought.user_id != *user_id {
return Err(DomainError::NotFound);
}
Ok(())
}
pub struct CreateThoughtInput {
pub user_id: UserId,
pub content: String,
@@ -45,7 +52,7 @@ pub async fn delete_thought(
user_id: &UserId,
) -> Result<(), DomainError> {
let thought = thoughts.find_by_id(id).await?.ok_or(DomainError::NotFound)?;
if thought.user_id != *user_id { return Err(DomainError::NotFound); }
require_owner(&thought, user_id)?;
thoughts.delete(id, user_id).await?;
events.publish(&DomainEvent::ThoughtDeleted { thought_id: id.clone(), user_id: user_id.clone() }).await?;
Ok(())
@@ -59,7 +66,7 @@ pub async fn edit_thought(
new_content: String,
) -> Result<(), DomainError> {
let thought = thoughts.find_by_id(id).await?.ok_or(DomainError::NotFound)?;
if thought.user_id != *user_id { return Err(DomainError::NotFound); }
require_owner(&thought, user_id)?;
let content = Content::new_local(new_content)?;
thoughts.update_content(id, &content).await?;
events.publish(&DomainEvent::ThoughtUpdated { thought_id: id.clone(), user_id: user_id.clone() }).await?;