refactor(server): file_management_service: move all method without owner check into private, add folder_ports

This commit is contained in:
Edouard Vanbelle
2026-05-20 15:39:53 +02:00
parent ac42a6d3cc
commit dfb082fdf4
17 changed files with 318 additions and 328 deletions
@@ -5,7 +5,7 @@ use crate::application::ports::auth_ports::{
OidcIdClaims, OidcServicePort, PasswordHasherPort, SessionStoragePort, TokenServicePort,
UserStoragePort,
};
use crate::application::ports::inbound::FolderUseCase;
use crate::application::ports::folder_ports::FolderUseCase;
use crate::application::services::folder_service::FolderService;
use crate::common::config::OidcConfig;
use crate::common::errors::{DomainError, ErrorKind};
+16 -10
View File
@@ -12,7 +12,7 @@ use tracing::info;
use crate::application::dtos::file_dto::FileDto;
use crate::application::dtos::folder_dto::{FolderDto, MoveFolderDto};
use crate::application::ports::file_ports::{FileManagementUseCase, FileRetrievalUseCase};
use crate::application::ports::inbound::FolderUseCase;
use crate::application::ports::folder_ports::FolderUseCase;
use crate::application::ports::storage_ports::CopyFolderTreeResult;
use crate::application::ports::trash_ports::TrashUseCase;
use crate::application::services::file_management_service::FileManagementService;
@@ -145,7 +145,7 @@ impl BatchOperationService {
async move {
let copy_result = mgmt
.copy_file_owned(&file_id, user_id, target_folder.map(|s| s.to_string()))
.copy_file_with_perms(&file_id, user_id, target_folder.map(|s| s.to_string()))
.await;
(file_id, copy_result)
}
@@ -211,7 +211,7 @@ impl BatchOperationService {
async move {
let move_result = mgmt
.move_file_owned(&file_id, user_id, target_folder.map(|s| s.to_string()))
.move_file_with_perms(&file_id, user_id, target_folder.map(|s| s.to_string()))
.await;
(file_id, move_result)
}
@@ -270,7 +270,7 @@ impl BatchOperationService {
let mgmt = self.file_management.clone();
async move {
let delete_result = mgmt.delete_file_owned(&file_id, user_id).await;
let delete_result = mgmt.delete_file_with_perms(&file_id, user_id).await;
let id_for_result = file_id.clone();
(file_id, delete_result.map(|_| id_for_result))
}
@@ -390,7 +390,9 @@ impl BatchOperationService {
let folder_service = self.folder_service.clone();
async move {
let delete_result = folder_service.delete_folder(&folder_id, user_id).await;
let delete_result = folder_service
.delete_folder_with_perms(&folder_id, user_id)
.await;
let id_for_result = folder_id.clone();
(folder_id, delete_result.map(|_| id_for_result))
}
@@ -583,7 +585,9 @@ impl BatchOperationService {
let dto = MoveFolderDto {
parent_id: target.map(|s| s.to_string()),
};
let move_result = folder_service.move_folder(&folder_id, dto, user_id).await;
let move_result = folder_service
.move_folder_with_perms(&folder_id, dto, user_id)
.await;
(folder_id, move_result)
}
}))
@@ -644,7 +648,7 @@ impl BatchOperationService {
async move {
let copy_result = file_management
.copy_folder_tree_owned(
.copy_folder_tree_with_perms(
&folder_id,
user_id,
target.map(|s| s.to_string()),
@@ -732,7 +736,7 @@ impl BatchOperationService {
for folder_id in &folder_ids {
match self
.folder_service
.get_folder_owned(folder_id, user_id)
.get_folder_with_perms(folder_id, user_id)
.await
{
Ok(root_folder) => {
@@ -979,7 +983,7 @@ impl BatchOperationService {
name: name.clone(),
parent_id: parent_id.clone(),
};
let create_result = folder_service.create_folder(dto, user_id).await;
let create_result = folder_service.create_folder_with_perms(dto, user_id).await;
let id = format!("{}:{}", name, parent_id.unwrap_or_default());
(id, create_result)
}
@@ -1039,7 +1043,9 @@ impl BatchOperationService {
let folder_service = self.folder_service.clone();
async move {
let get_result = folder_service.get_folder_owned(&folder_id, user_id).await;
let get_result = folder_service
.get_folder_with_perms(&folder_id, user_id)
.await;
(folder_id, get_result)
}
}))
@@ -103,9 +103,8 @@ impl FileManagementService {
};
folder_repo.verify_owner(target, caller_id).await
}
}
impl FileManagementUseCase for FileManagementService {
//impl FileManagementPrivateUseCase for FileManagementService {
async fn move_file(
&self,
file_id: &str,
@@ -135,20 +134,6 @@ impl FileManagementUseCase for FileManagementService {
Ok(FileDto::from(moved_file))
}
async fn move_file_owned(
&self,
file_id: &str,
caller_id: Uuid,
folder_id: Option<String>,
) -> Result<FileDto, DomainError> {
// Verify file ownership first
self.verify_owner(file_id, caller_id).await?;
// Verify target folder ownership (prevents file from "disappearing")
self.verify_target_folder_owner(folder_id.as_deref(), caller_id)
.await?;
self.move_file(file_id, folder_id).await
}
async fn copy_file(
&self,
file_id: &str,
@@ -178,18 +163,6 @@ impl FileManagementUseCase for FileManagementService {
Ok(FileDto::from(copied_file))
}
async fn copy_file_owned(
&self,
file_id: &str,
caller_id: Uuid,
target_folder_id: Option<String>,
) -> Result<FileDto, DomainError> {
self.verify_owner(file_id, caller_id).await?;
self.verify_target_folder_owner(target_folder_id.as_deref(), caller_id)
.await?;
self.copy_file(file_id, target_folder_id).await
}
async fn rename_file(&self, file_id: &str, new_name: &str) -> Result<FileDto, DomainError> {
if let Err(reason) = validate_storage_name(new_name) {
return Err(DomainError::validation_error(format!(
@@ -217,65 +190,7 @@ impl FileManagementUseCase for FileManagementService {
Ok(FileDto::from(renamed_file))
}
async fn rename_file_owned(
&self,
file_id: &str,
caller_id: Uuid,
new_name: &str,
) -> Result<FileDto, DomainError> {
self.verify_owner(file_id, caller_id).await?;
self.rename_file(file_id, new_name).await
}
async fn delete_file(&self, id: &str) -> Result<(), DomainError> {
self.file_repository.delete_file(id).await?;
if let Some(cc) = &self.content_cache {
cc.invalidate(id).await;
}
for hook in &self.file_deleted_hooks {
hook.on_file_deleted(id).await;
}
Ok(())
}
async fn delete_file_owned(&self, id: &str, caller_id: Uuid) -> Result<(), DomainError> {
self.verify_owner(id, caller_id).await?;
self.delete_file(id).await
}
/// Smart delete: trash-first with dedup reference cleanup.
///
/// Blob ref_count bookkeeping is handled entirely by the PG trigger
/// `trg_files_decrement_blob_ref` which fires on DELETE FROM storage.files.
/// We do NOT decrement here — trashing is a soft-delete (UPDATE, not DELETE)
/// so the blob must remain referenced until the file is permanently deleted.
async fn delete_with_cleanup(&self, id: &str, user_id: Uuid) -> Result<bool, DomainError> {
// Step 1: Try trash (soft delete — file row stays, blob stays referenced)
if let Some(trash) = &self.trash_service {
info!("Moving file to trash: {}", id);
match trash.move_to_trash(id, "file", user_id).await {
Ok(_) => {
info!("File successfully moved to trash: {}", id);
// Invalidate content cache — trashed files must not be served.
if let Some(cc) = &self.content_cache {
cc.invalidate(id).await;
}
// Do NOT decrement blob ref here — the file row still exists
// (is_trashed = TRUE). The trigger will decrement when the
// row is actually DELETEd during trash emptying.
return Ok(true); // trashed
}
Err(err) => {
error!("Could not move file to trash: {:?}", err);
warn!("Falling back to permanent delete");
// fall through
}
}
} else {
warn!("Trash service not available, using permanent delete");
}
// Step 2: Permanent delete — trigger handles blob ref_count
warn!("Permanently deleting file: {}", id);
self.file_repository.delete_file(id).await?;
if let Some(cc) = &self.content_cache {
@@ -285,8 +200,7 @@ impl FileManagementUseCase for FileManagementService {
hook.on_file_deleted(id).await;
}
info!("File permanently deleted: {}", id);
Ok(false) // permanently deleted
Ok(())
}
async fn copy_folder_tree(
@@ -319,8 +233,95 @@ impl FileManagementUseCase for FileManagementService {
Ok(result)
}
}
async fn copy_folder_tree_owned(
impl FileManagementUseCase for FileManagementService {
async fn move_file_with_perms(
&self,
file_id: &str,
caller_id: Uuid,
folder_id: Option<String>,
) -> Result<FileDto, DomainError> {
// Verify file ownership first
self.verify_owner(file_id, caller_id).await?;
// Verify target folder ownership (prevents file from "disappearing")
self.verify_target_folder_owner(folder_id.as_deref(), caller_id)
.await?;
self.move_file(file_id, folder_id).await
}
async fn copy_file_with_perms(
&self,
file_id: &str,
caller_id: Uuid,
target_folder_id: Option<String>,
) -> Result<FileDto, DomainError> {
self.verify_owner(file_id, caller_id).await?;
self.verify_target_folder_owner(target_folder_id.as_deref(), caller_id)
.await?;
self.copy_file(file_id, target_folder_id).await
}
async fn rename_file_with_perms(
&self,
file_id: &str,
caller_id: Uuid,
new_name: &str,
) -> Result<FileDto, DomainError> {
self.verify_owner(file_id, caller_id).await?;
self.rename_file(file_id, new_name).await
}
async fn delete_file_with_perms(&self, id: &str, caller_id: Uuid) -> Result<(), DomainError> {
self.verify_owner(id, caller_id).await?;
self.delete_file(id).await
}
/// Smart delete: trash-first with dedup reference cleanup.
///
/// Blob ref_count bookkeeping is handled entirely by the PG trigger
/// `trg_files_decrement_blob_ref` which fires on DELETE FROM storage.files.
/// We do NOT decrement here — trashing is a soft-delete (UPDATE, not DELETE)
/// so the blob must remain referenced until the file is permanently deleted.
async fn delete_and_cleanup_with_perms(
&self,
id: &str,
caller_id: Uuid,
) -> Result<bool, DomainError> {
self.verify_owner(id, caller_id).await?;
// Step 1: Try trash (soft delete — file row stays, blob stays referenced)
if let Some(trash) = &self.trash_service {
info!("Moving file to trash: {}", id);
match trash.move_to_trash(id, "file", caller_id).await {
Ok(_) => {
info!("File successfully moved to trash: {}", id);
// Invalidate content cache — trashed files must not be served.
if let Some(cc) = &self.content_cache {
cc.invalidate(id).await;
}
// Do NOT decrement blob ref here — the file row still exists
// (is_trashed = TRUE). The trigger will decrement when the
// row is actually DELETEd during trash emptying.
return Ok(true); // trashed
}
Err(err) => {
error!("Could not move file to trash: {:?}", err);
warn!("Falling back to permanent delete");
// fall through
}
}
} else {
warn!("Trash service not available, using permanent delete");
}
// Step 2: Permanent delete — trigger handles blob ref_count
self.delete_file(id).await?;
Ok(false) // permanently deleted
}
async fn copy_folder_tree_with_perms(
&self,
source_folder_id: &str,
caller_id: Uuid,
+23 -15
View File
@@ -1,7 +1,7 @@
use crate::application::dtos::folder_dto::{
CreateFolderDto, FolderDto, MoveFolderDto, RenameFolderDto,
};
use crate::application::ports::inbound::FolderUseCase;
use crate::application::ports::folder_ports::FolderUseCase;
use crate::common::errors::{DomainError, ErrorKind};
use crate::domain::repositories::folder_repository::FolderRepository;
use crate::domain::services::path_service::{StoragePath, validate_storage_name};
@@ -25,7 +25,7 @@ impl FolderService {
struct FolderServiceStub;
impl FolderUseCase for FolderServiceStub {
async fn create_folder(
async fn create_folder_with_perms(
&self,
_dto: CreateFolderDto,
_user_id: Uuid,
@@ -37,7 +37,7 @@ impl FolderService {
Ok(FolderDto::empty())
}
async fn get_folder_owned(
async fn get_folder_with_perms(
&self,
_id: &str,
_caller_id: Uuid,
@@ -101,7 +101,7 @@ impl FolderService {
)
}
async fn rename_folder(
async fn rename_folder_with_perms(
&self,
_id: &str,
_dto: RenameFolderDto,
@@ -110,7 +110,7 @@ impl FolderService {
Ok(FolderDto::empty())
}
async fn move_folder(
async fn move_folder_with_perms(
&self,
_id: &str,
_dto: MoveFolderDto,
@@ -119,7 +119,11 @@ impl FolderService {
Ok(FolderDto::empty())
}
async fn delete_folder(&self, _id: &str, _caller_id: Uuid) -> Result<(), DomainError> {
async fn delete_folder_with_perms(
&self,
_id: &str,
_caller_id: Uuid,
) -> Result<(), DomainError> {
Ok(())
}
@@ -138,7 +142,7 @@ impl FolderService {
impl FolderUseCase for FolderService {
/// Creates a new folder
async fn create_folder(
async fn create_folder_with_perms(
&self,
dto: CreateFolderDto,
caller_id: Uuid,
@@ -204,7 +208,11 @@ impl FolderUseCase for FolderService {
}
/// Gets a folder by its ID, enforcing that `caller_id` is the owner.
async fn get_folder_owned(&self, id: &str, caller_id: Uuid) -> Result<FolderDto, DomainError> {
async fn get_folder_with_perms(
&self,
id: &str,
caller_id: Uuid,
) -> Result<FolderDto, DomainError> {
let folder_dto = self.get_folder(id).await?;
if folder_dto.owner_id.as_deref() != Some(&caller_id.to_string()) {
tracing::warn!(
@@ -261,10 +269,6 @@ impl FolderUseCase for FolderService {
parent_id: Option<&str>,
owner_id: Uuid,
) -> Result<Vec<FolderDto>, DomainError> {
let owner_id_short = {
let s = owner_id.to_string();
s[..8.min(s.len())].to_string()
};
let folders = self
.folder_storage
.list_folders_by_owner(parent_id, owner_id)
@@ -286,6 +290,10 @@ impl FolderUseCase for FolderService {
"No root folders found for user {}, creating home folder automatically",
owner_id
);
let owner_id_short = {
let s = owner_id.to_string();
s[..8.min(s.len())].to_string()
};
let folder_name = format!("My Folder - {}", owner_id_short);
match self
.folder_storage
@@ -388,7 +396,7 @@ impl FolderUseCase for FolderService {
}
/// Renames a folder after verifying ownership.
async fn rename_folder(
async fn rename_folder_with_perms(
&self,
id: &str,
dto: RenameFolderDto,
@@ -431,7 +439,7 @@ impl FolderUseCase for FolderService {
}
/// Moves a folder to a new parent after verifying ownership.
async fn move_folder(
async fn move_folder_with_perms(
&self,
id: &str,
dto: MoveFolderDto,
@@ -497,7 +505,7 @@ impl FolderUseCase for FolderService {
}
/// Deletes a folder after verifying ownership.
async fn delete_folder(&self, id: &str, caller_id: Uuid) -> Result<(), DomainError> {
async fn delete_folder_with_perms(&self, id: &str, caller_id: Uuid) -> Result<(), DomainError> {
// Verify the folder exists and belongs to the caller
let folder = self.folder_storage.get_folder(id).await?;
@@ -362,7 +362,7 @@ async fn stub_move_file_owned_returns_ok() {
let user_id = Uuid::new_v4();
let stub = StubFileManagementUseCase;
let result = stub
.move_file_owned("file-1", user_id, Some("folder-2".to_string()))
.move_file_with_perms("file-1", user_id, Some("folder-2".to_string()))
.await;
assert!(result.is_ok(), "stub should return Ok for move_file_owned");
}
@@ -372,7 +372,7 @@ async fn stub_rename_file_owned_returns_ok() {
let user_id = Uuid::new_v4();
let stub = StubFileManagementUseCase;
let result = stub
.rename_file_owned("file-1", user_id, "new-name.txt")
.rename_file_with_perms("file-1", user_id, "new-name.txt")
.await;
assert!(
result.is_ok(),
@@ -6,7 +6,7 @@ use uuid::Uuid;
use crate::application::dtos::folder_listing_dto::FolderListingDto;
use crate::application::ports::file_ports::FileRetrievalUseCase;
use crate::application::ports::inbound::FolderUseCase;
use crate::application::ports::folder_ports::FolderUseCase;
use crate::application::services::file_retrieval_service::FileRetrievalService;
use crate::application::services::folder_service::FolderService;
use crate::application::services::share_service::ShareService;