fix: resolve clippy warnings for --all-features CI build
Update share_service test impl to match upstream trait changes (requester_id params, verify_shared_link_password returns ShareDto). Fix map_or, collapsible_if, dead_code, too_many_arguments warnings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -245,7 +245,7 @@ pub trait FileRetrievalUseCase: Send + Sync + 'static {
|
||||
let all = self.list_files_batch(folder_id, offset, limit).await?;
|
||||
Ok(all
|
||||
.into_iter()
|
||||
.filter(|f| f.owner_id.as_deref().map_or(false, |o| o == owner_id))
|
||||
.filter(|f| f.owner_id.as_deref().is_some_and(|o| o == owner_id))
|
||||
.collect())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -58,7 +58,7 @@ pub trait FileReadPort: Send + Sync + 'static {
|
||||
let all = self.list_files(folder_id).await?;
|
||||
Ok(all
|
||||
.into_iter()
|
||||
.filter(|f| f.owner_id().map_or(false, |o| o == owner_id))
|
||||
.filter(|f| f.owner_id().is_some_and(|o| o == owner_id))
|
||||
.collect())
|
||||
}
|
||||
|
||||
@@ -142,7 +142,7 @@ pub trait FileReadPort: Send + Sync + 'static {
|
||||
let all = self.list_files_batch(folder_id, offset, limit).await?;
|
||||
Ok(all
|
||||
.into_iter()
|
||||
.filter(|f| f.owner_id().map_or(false, |o| o == owner_id))
|
||||
.filter(|f| f.owner_id().is_some_and(|o| o == owner_id))
|
||||
.collect())
|
||||
}
|
||||
|
||||
|
||||
@@ -928,11 +928,9 @@ impl BatchOperationService {
|
||||
|
||||
async move {
|
||||
// If a parent is specified, verify the caller owns it
|
||||
if let Some(ref pid) = parent_id {
|
||||
if let Err(e) = folder_service.get_folder_owned(pid, &caller).await {
|
||||
let id = format!("{}:{}", name, pid);
|
||||
return (id, Err(e.into()));
|
||||
}
|
||||
if let Some(ref pid) = parent_id && let Err(e) = folder_service.get_folder_owned(pid, &caller).await {
|
||||
let id = format!("{}:{}", name, pid);
|
||||
return (id, Err(e));
|
||||
}
|
||||
let dto = crate::application::dtos::folder_dto::CreateFolderDto {
|
||||
name: name.clone(),
|
||||
|
||||
@@ -553,10 +553,10 @@ mod tests {
|
||||
Ok(ShareDto::from_entity(&saved_share, &self.config.base_url()))
|
||||
}
|
||||
|
||||
async fn get_shared_link(&self, id: &str) -> Result<ShareDto, DomainError> {
|
||||
async fn get_shared_link(&self, id: &str, requester_id: &str) -> Result<ShareDto, DomainError> {
|
||||
let share = self
|
||||
.share_repository
|
||||
.find_share_by_id(id)
|
||||
.find_share_by_id_for_user(id, requester_id)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
ShareServiceError::NotFound(format!("Share {} not found: {}", id, e))
|
||||
@@ -585,10 +585,11 @@ mod tests {
|
||||
&self,
|
||||
item_id: &str,
|
||||
item_type: &ShareItemType,
|
||||
requester_id: &str,
|
||||
) -> Result<Vec<ShareDto>, DomainError> {
|
||||
let shares = self
|
||||
.share_repository
|
||||
.find_shares_by_item(item_id, item_type)
|
||||
.find_shares_by_item_for_user(item_id, item_type, requester_id)
|
||||
.await
|
||||
.map_err(|e| ShareServiceError::Repository(e.to_string()))?;
|
||||
Ok(shares
|
||||
@@ -601,11 +602,12 @@ mod tests {
|
||||
async fn update_shared_link(
|
||||
&self,
|
||||
id: &str,
|
||||
requester_id: &str,
|
||||
dto: UpdateShareDto,
|
||||
) -> Result<ShareDto, DomainError> {
|
||||
let mut share = self
|
||||
.share_repository
|
||||
.find_share_by_id(id)
|
||||
.find_share_by_id_for_user(id, requester_id)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
ShareServiceError::NotFound(format!("Share {} not found: {}", id, e))
|
||||
@@ -632,9 +634,9 @@ mod tests {
|
||||
Ok(ShareDto::from_entity(&updated, &self.config.base_url()))
|
||||
}
|
||||
|
||||
async fn delete_shared_link(&self, id: &str) -> Result<(), DomainError> {
|
||||
async fn delete_shared_link(&self, id: &str, requester_id: &str) -> Result<(), DomainError> {
|
||||
self.share_repository
|
||||
.delete_share(id)
|
||||
.delete_share_for_user(id, requester_id)
|
||||
.await
|
||||
.map_err(|e| ShareServiceError::Repository(e.to_string()))?;
|
||||
Ok(())
|
||||
@@ -663,7 +665,7 @@ mod tests {
|
||||
&self,
|
||||
token: &str,
|
||||
password: &str,
|
||||
) -> Result<bool, DomainError> {
|
||||
) -> Result<ShareDto, DomainError> {
|
||||
let share = self
|
||||
.share_repository
|
||||
.find_share_by_token(token)
|
||||
@@ -675,8 +677,18 @@ mod tests {
|
||||
return Err(ShareServiceError::Expired.into());
|
||||
}
|
||||
match share.password_hash() {
|
||||
Some(hash) => self.password_hasher.verify_password(password, hash).await,
|
||||
None => Ok(true),
|
||||
Some(hash) => {
|
||||
let valid = self.password_hasher.verify_password(password, hash).await?;
|
||||
if !valid {
|
||||
return Err(DomainError::new(
|
||||
crate::common::errors::ErrorKind::AccessDenied,
|
||||
"Share",
|
||||
"Invalid share password",
|
||||
));
|
||||
}
|
||||
Ok(ShareDto::from_entity(&share, &self.config.base_url()))
|
||||
}
|
||||
None => Ok(ShareDto::from_entity(&share, &self.config.base_url())),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -272,7 +272,7 @@ impl TrashUseCase for TrashService {
|
||||
|
||||
// Ownership check — return NotFound (not Forbidden) to
|
||||
// prevent leaking whether the folder exists.
|
||||
if folder.owner_id().map_or(true, |o| o != user_id) {
|
||||
if folder.owner_id().is_none_or(|o| o != user_id) {
|
||||
return Err(DomainError::not_found(
|
||||
"Folder",
|
||||
format!("Folder not found: {}", item_id),
|
||||
|
||||
@@ -20,6 +20,7 @@ use crate::domain::services::path_service::StoragePath;
|
||||
|
||||
/// Test-only service that mirrors `TrashService` logic but accepts generic repos,
|
||||
/// allowing mock repositories to be injected in unit tests.
|
||||
#[allow(dead_code)]
|
||||
struct TrashServiceForTest<TR, FR, FW, FoR> {
|
||||
trash_repository: Arc<TR>,
|
||||
file_read_port: Arc<FR>,
|
||||
@@ -35,6 +36,7 @@ where
|
||||
FW: FileWritePort,
|
||||
FoR: FolderRepository,
|
||||
{
|
||||
#[allow(dead_code)]
|
||||
fn new(
|
||||
trash_repository: Arc<TR>,
|
||||
file_read_port: Arc<FR>,
|
||||
@@ -308,6 +310,7 @@ where
|
||||
}
|
||||
|
||||
// Mock repositories for testing
|
||||
#[allow(dead_code)]
|
||||
struct MockTrashRepository {
|
||||
trash_items: Mutex<HashMap<Uuid, TrashedItem>>,
|
||||
/// Shared refs to the file/folder trashed maps so `clear_trash` can
|
||||
@@ -317,6 +320,7 @@ struct MockTrashRepository {
|
||||
}
|
||||
|
||||
impl MockTrashRepository {
|
||||
#[allow(dead_code)]
|
||||
fn new(
|
||||
trashed_files: Arc<Mutex<HashMap<String, File>>>,
|
||||
trashed_folders: Arc<Mutex<HashMap<String, Folder>>>,
|
||||
@@ -394,12 +398,14 @@ impl TrashRepository for MockTrashRepository {
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
struct MockFileRepository {
|
||||
files: Mutex<HashMap<String, File>>,
|
||||
trashed_files: Arc<Mutex<HashMap<String, File>>>,
|
||||
}
|
||||
|
||||
impl MockFileRepository {
|
||||
#[allow(dead_code)]
|
||||
fn new(trashed_files: Arc<Mutex<HashMap<String, File>>>) -> Self {
|
||||
Self {
|
||||
files: Mutex::new(HashMap::new()),
|
||||
@@ -407,6 +413,7 @@ impl MockFileRepository {
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
fn add_test_file(&self, id: &str, name: &str, path: &str) {
|
||||
let file = File::new(
|
||||
id.to_string(),
|
||||
@@ -625,12 +632,14 @@ impl FileWritePort for MockFileRepository {
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
struct MockFolderRepository {
|
||||
folders: Mutex<HashMap<String, Folder>>,
|
||||
trashed_folders: Arc<Mutex<HashMap<String, Folder>>>,
|
||||
}
|
||||
|
||||
impl MockFolderRepository {
|
||||
#[allow(dead_code)]
|
||||
fn new(trashed_folders: Arc<Mutex<HashMap<String, Folder>>>) -> Self {
|
||||
Self {
|
||||
folders: Mutex::new(HashMap::new()),
|
||||
@@ -638,6 +647,7 @@ impl MockFolderRepository {
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
fn add_test_folder(&self, id: &str, name: &str, path: &str) {
|
||||
let folder = Folder::new(
|
||||
id.to_string(),
|
||||
|
||||
@@ -434,6 +434,7 @@ async fn handle_propfind(
|
||||
/// (sub-folders and files) are fetched in batches of `PROPFIND_BATCH_SIZE`.
|
||||
/// Each batch is serialised to XML and sent as a chunk, so memory stays
|
||||
/// constant at O(batch_size) regardless of the total number of children.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn build_streaming_propfind_response(
|
||||
folder: FolderDto,
|
||||
folder_id: Option<String>,
|
||||
@@ -1227,10 +1228,8 @@ async fn handle_move(
|
||||
|
||||
if source_parent_path != dest_parent_path {
|
||||
// SECURITY: verify destination parent belongs to caller (V-08)
|
||||
if !dest_parent_path.is_empty() {
|
||||
if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await {
|
||||
assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?;
|
||||
}
|
||||
if !dest_parent_path.is_empty() && let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await {
|
||||
assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?;
|
||||
}
|
||||
file_management_service
|
||||
.move_file(&file.id, Some(dest_parent_path.to_string()))
|
||||
@@ -1328,10 +1327,8 @@ async fn handle_move(
|
||||
|
||||
if source_parent_path != dest_parent_path {
|
||||
// SECURITY: verify destination parent belongs to caller (V-08)
|
||||
if !dest_parent_path.is_empty() {
|
||||
if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await {
|
||||
assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?;
|
||||
}
|
||||
if !dest_parent_path.is_empty() && let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await {
|
||||
assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?;
|
||||
}
|
||||
file_management_service
|
||||
.move_file(&file.id, Some(dest_parent_path.to_string()))
|
||||
|
||||
Reference in New Issue
Block a user