Skip to content
Merged
1 change: 1 addition & 0 deletions proto/sync.proto
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ message RenameFileRequest {
optional string operation_type = 11; // e.g., "RENAME" or "MOVE"
ConflictInfo.ResolutionStrategy conflict_resolution = 12; // How to resolve conflicts (default: MANUAL)
google.protobuf.Timestamp updated_time = 13; // Client's last known update time for timestamp-based conflict resolution
optional bool overwrite = 14; // If true, tombstone conflicting target before rename/move
}

// NEW: Rename file response
Expand Down
61 changes: 52 additions & 9 deletions src/handlers/file/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,13 @@ async fn process_upload_operation(
}
}
Err(e) => {
error!("Upload operation {} failed: {}", operation_id, e);
let error_msg = e.message();
error!("Upload operation {} failed: {}", operation_id, error_msg);
let error_code = extract_error_code_from_message(error_msg);
create_error_result(
operation_id,
&format!("Upload failed: {}", e),
ErrorCode::StorageFailed,
&format!("Upload failed: {}", error_msg),
error_code,
)
}
}
Expand Down Expand Up @@ -211,11 +213,13 @@ async fn process_delete_operation(
}
}
Err(e) => {
error!("Delete operation {} failed: {}", operation_id, e);
let error_msg = e.message();
error!("Delete operation {} failed: {}", operation_id, error_msg);
let error_code = extract_error_code_from_message(error_msg);
create_error_result(
operation_id,
&format!("Delete failed: {}", e),
ErrorCode::StorageFailed,
&format!("Delete failed: {}", error_msg),
error_code,
)
}
}
Expand Down Expand Up @@ -243,11 +247,13 @@ async fn process_rename_operation(
}
}
Err(e) => {
error!("Rename operation {} failed: {}", operation_id, e);
let error_msg = e.message();
error!("Rename operation {} failed: {}", operation_id, error_msg);
let error_code = extract_error_code_from_message(error_msg);
create_error_result(
operation_id,
&format!("Rename failed: {}", e),
ErrorCode::StorageFailed,
&format!("Rename failed: {}", error_msg),
error_code,
)
}
}
Expand All @@ -266,3 +272,40 @@ fn create_error_result(
result: None,
}
}

/// Extract appropriate ErrorCode from error message for client-side retry logic classification
fn extract_error_code_from_message(msg: &str) -> ErrorCode {
let msg_lower = msg.to_lowercase();

// Permanent errors - client should not retry
if msg_lower.contains("permission denied") || msg_lower.contains("access denied") {
ErrorCode::AuthFailed
} else if msg_lower.contains("already deleted") {
ErrorCode::FileAlreadyDeleted
} else if msg_lower.contains("path conflict") || msg_lower.contains("already exists") {
ErrorCode::PathConflict
} else if msg_lower.contains("quota") || msg_lower.contains("storage limit") {
ErrorCode::QuotaExceeded
} else if msg_lower.contains("invalid") || msg_lower.contains("validation") {
ErrorCode::InvalidRequest
}
// Conflict errors - client should sync and retry
else if msg_lower.contains("revision") || msg_lower.contains("conflict") {
ErrorCode::RevisionConflict
}
// File errors
else if msg_lower.contains("not found") {
ErrorCode::FileNotFound
}
// Schema/DB errors - permanent
else if msg_lower.contains("schema")
|| msg_lower.contains("varbinary")
|| msg_lower.contains("varchar")
{
ErrorCode::DbSchemaError
}
// Default: transient error - client can retry with backoff
else {
ErrorCode::StorageFailed
}
}
19 changes: 14 additions & 5 deletions src/handlers/file/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,20 @@ pub async fn handle_delete_file(
}))
}
Ok(DeleteFileResult::AlreadyDeleted) => {
info!("File already deleted: file_id={}", file_id);
Ok(Response::new(response::file_delete_error_with_code(
"File already deleted",
ErrorCode::FileNotFound,
)))
// Idempotent success: if the file is already deleted, the client's goal is achieved
info!(
"File already deleted (idempotent success): file_id={}",
file_id
);
Ok(Response::new(DeleteFileResponse {
success: true,
return_message: "File already deleted".to_string(),
delete_record_id: 0,
new_revision: 0,
conflict: None,
error_code: ErrorCode::Success as i32,
current_revision: 0,
}))
}
Err(e) => {
// Determine error category for logging and client classification
Expand Down
12 changes: 6 additions & 6 deletions src/handlers/file/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,18 +280,18 @@ pub async fn handle_rename_file(
}

// 6. Broadcast rename notification to other devices
// Fix P0 #3: Include updated file info with Server IDs
// Fix P0 #3: Use SERVER IDs for cross-watcher MOVE
let mut broadcast_file_info = file_info.clone();
broadcast_file_info.file_path = req.new_file_path.clone();
broadcast_file_info.revision = new_revision;
broadcast_file_info.updated_time.seconds = req.timestamp;
broadcast_file_info.updated_time.nanos = 0;

if let Some(gid) = new_group_id {
broadcast_file_info.group_id = gid;
}
if let Some(wid) = new_watcher_id {
broadcast_file_info.watcher_id = wid;
// CRITICAL: For cross-watcher moves, use SERVER IDs (not client IDs)
// so other devices can properly locate the file
if is_cross_watcher_move {
broadcast_file_info.group_id = server_group_id;
broadcast_file_info.watcher_id = server_watcher_id;
}

if let Err(e) = handler
Expand Down
12 changes: 10 additions & 2 deletions src/server/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1675,7 +1675,11 @@ impl SyncService for SyncServiceImpl {
quota_reset_date: summary.quota_reset_date.map(|d| {
use prost_types::Timestamp;
Timestamp {
seconds: d.and_hms_opt(0, 0, 0).unwrap().and_utc().timestamp(),
seconds: d
.and_hms_opt(0, 0, 0)
.expect("Valid time: midnight")
.and_utc()
.timestamp(),
nanos: 0,
}
}),
Expand Down Expand Up @@ -1752,7 +1756,11 @@ impl SyncService for SyncServiceImpl {
period_start: None,
period_end: None,
quota_reset_date: summary.quota_reset_date.map(|d| Timestamp {
seconds: d.and_hms_opt(0, 0, 0).unwrap().and_utc().timestamp(),
seconds: d
.and_hms_opt(0, 0, 0)
.expect("Valid time: midnight")
.and_utc()
.timestamp(),
nanos: 0,
}),
is_transfer_exceeded: summary.transfer_percentage >= 100.0,
Expand Down
8 changes: 4 additions & 4 deletions src/services/usage_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,13 @@ impl UsageService {
));

// Update warning timestamp
let _ = self
if let Some(mysql_storage) = self
.storage
.as_any()
.downcast_ref::<crate::storage::mysql::MySqlStorage>()
.unwrap()
.update_last_warning(account_hash)
.await;
{
let _ = mysql_storage.update_last_warning(account_hash).await;
}
}

Ok((true, None, warnings))
Expand Down
2 changes: 1 addition & 1 deletion src/services/version_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl VersionService for VersionServiceImpl {
.map_err(|e| AppError::storage(format!("Failed to get file history: {}", e)))?;

let total_versions = files.len() as i32;
let has_more = limit.is_some() && files.len() == limit.unwrap() as usize;
let has_more = limit.map_or(false, |l| files.len() == l as usize);

// Convert to FileVersionInfo
let versions: Vec<FileVersionInfo> = files
Expand Down
22 changes: 15 additions & 7 deletions src/storage/mysql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1269,19 +1269,19 @@ END"#;
account_hash: &str,
group_id: i32,
watcher_id: i32,
data: Vec<u8>,
data: &[u8],
) -> String {
let cfg = crate::server::app_state::AppState::get_config();
if let Some(kv) = cfg.server_encode_key.as_ref() {
if kv.len() == 32 {
let key: &[u8; 32] = kv.as_slice().try_into().expect("len checked");
let aad = format!("{}:{}:{}", account_hash, group_id, watcher_id);
if let Ok(pt) = crate::utils::crypto::aead_decrypt(key, &data, aad.as_bytes()) {
if let Ok(pt) = crate::utils::crypto::aead_decrypt(key, data, aad.as_bytes()) {
return String::from_utf8_lossy(&pt).to_string();
}
}
}
String::from_utf8_lossy(&data).to_string()
String::from_utf8_lossy(data).to_string()
}

pub async fn migrate_encrypt_paths(&self, batch_size: usize) -> Result<u64> {
Expand Down Expand Up @@ -1332,8 +1332,14 @@ END"#;
(maybe_plain_path.as_ref(), maybe_plain_name.as_ref())
{
// Already encrypted; recompute indexes from decrypted plaintext
let plain_path =
String::from_utf8_lossy(maybe_plain_path.as_ref().unwrap()).to_string();
let plain_path = match maybe_plain_path.as_ref() {
Some(bytes) => String::from_utf8_lossy(bytes).to_string(),
None => {
return Err(StorageError::Database(
"Missing plain_path during encryption migration".into(),
))
}
};
let salt = crate::utils::crypto::derive_salt(key, "meta-index", &account_hash);
(
file_path_b,
Expand Down Expand Up @@ -2000,9 +2006,11 @@ impl Storage for MySqlStorage {
))
})?;

// Tombstone row uses next revision, new active uses next+1 to avoid UNIQUE(file_id, revision) conflicts
// Both tombstone and new active row share the same revision (current+1)
// This represents a single logical operation (rename/move) with one revision increment
// Note: No UNIQUE(file_id, revision) constraint exists, so this is safe
let tombstone_revision = current_revision + 1;
let new_active_revision = tombstone_revision + 1;
let new_active_revision = current_revision + 1;

sqlx::query(
r#"INSERT INTO files (
Expand Down
Loading