fix: address review issues for custom db path

This commit is contained in:
Sergey 2025-06-12 01:46:54 -04:00
parent e5e812310c
commit 88cbd4458a
4 changed files with 118 additions and 76 deletions

View File

@ -117,7 +117,9 @@ export default function CustomDatabaseLocationSettings() {
} catch (error) {
console.error('Failed to create pastebar-data directory:', error)
setOperationError(
t('Failed to create directory. Please check permissions and try again.', { ns: 'settings' })
t('Failed to create directory. Please check permissions and try again.', {
ns: 'settings',
})
)
return
}
@ -201,6 +203,12 @@ export default function CustomDatabaseLocationSettings() {
await applyCustomDbPath(selectedPathForChangeDialog, dbOperationForChangeDialog)
relaunchApp()
} catch (error: any) {
// Attempt rollback on failure
try {
await revertToDefaultDbPath()
} catch (_) {
// Ignore rollback errors
}
setOperationError(
error.message ||
t('Failed to apply custom database location.', { ns: 'settings' })
@ -287,7 +295,9 @@ export default function CustomDatabaseLocationSettings() {
} catch (error) {
console.error('Failed to create pastebar-data directory:', error)
setOperationError(
t('Failed to create directory. Please check permissions and try again.', { ns: 'settings' })
t('Failed to create directory. Please check permissions and try again.', {
ns: 'settings',
})
)
return
}
@ -317,9 +327,7 @@ export default function CustomDatabaseLocationSettings() {
// Handle applying the initial setup
const handleApplySetup = async () => {
if (!selectedPathForChangeDialog) {
setOperationError(
t('Please select a directory first.', { ns: 'settings' })
)
setOperationError(t('Please select a directory first.', { ns: 'settings' }))
return
}
setIsApplyingChange(true)
@ -367,6 +375,9 @@ export default function CustomDatabaseLocationSettings() {
await applyCustomDbPath(selectedPathForChangeDialog, dbOperationForChangeDialog)
relaunchApp()
} catch (error: any) {
try {
await revertToDefaultDbPath()
} catch (_) {}
setOperationError(
error.message ||
t('Failed to apply custom database location.', { ns: 'settings' })
@ -424,7 +435,9 @@ export default function CustomDatabaseLocationSettings() {
} catch (error) {
console.error('Failed to create pastebar-data directory:', error)
setOperationError(
t('Failed to create directory. Please check permissions and try again.', { ns: 'settings' })
t('Failed to create directory. Please check permissions and try again.', {
ns: 'settings',
})
)
setIsProcessing(false)
return false
@ -462,9 +475,19 @@ export default function CustomDatabaseLocationSettings() {
)
if (confirmed) {
await applyCustomDbPath(finalPath, 'none') // 'none' for initial setup
relaunchApp()
pathSuccessfullySet = true // Path will be set by store, app restarts
try {
await applyCustomDbPath(finalPath, 'none') // 'none' for initial setup
relaunchApp()
pathSuccessfullySet = true // Path will be set by store, app restarts
} catch (error) {
try {
await revertToDefaultDbPath()
} catch (_) {}
setOperationError(
(error as any).message ||
t('Failed to apply custom database location.', { ns: 'settings' })
)
}
}
}
} catch (error) {
@ -506,7 +529,11 @@ export default function CustomDatabaseLocationSettings() {
return (
<Box className="animate-in fade-in max-w-xl mt-4">
<Card
className={`${!customDbPath && !isSetupSectionExpanded ? 'opacity-80 bg-gray-100 dark:bg-gray-900/80' : ''}`}
className={`${
!customDbPath && !isSetupSectionExpanded
? 'opacity-80 bg-gray-100 dark:bg-gray-900/80'
: ''
}`}
>
<CardHeader className="flex flex-row items-center justify-between space-y-0 pb-1">
<CardTitle className="animate-in fade-in text-md font-medium">
@ -569,7 +596,7 @@ export default function CustomDatabaseLocationSettings() {
) : null}
{t('Change Custom Data Folder...', { ns: 'settings' })}
</Button>
<Button
onClick={handleRevertFromContent}
disabled={isLoading}
@ -685,10 +712,9 @@ export default function CustomDatabaseLocationSettings() {
{isProcessing && !isApplyingChange ? (
<Icons.spinner className="mr-2 h-4 w-4 animate-spin" />
) : null}
{selectedPathForChangeDialog
{selectedPathForChangeDialog
? t('Change Selected Folder...', { ns: 'settings' })
: t('Select Data Folder...', { ns: 'settings' })
}
: t('Select Data Folder...', { ns: 'settings' })}
</Button>
{selectedPathForChangeDialog && (

View File

@ -15,6 +15,22 @@ use crate::services::user_settings_service::{
use fs_extra::dir::{copy, CopyOptions};
use std::path::PathBuf;
fn rollback_moves(items: &[(PathBuf, PathBuf)]) {
for (src, dest) in items.iter().rev() {
if dest.exists() {
if dest.is_dir() {
let mut options = CopyOptions::new();
options.overwrite = true;
let _ = copy(dest, src, &options);
let _ = fs::remove_dir_all(dest);
} else {
let _ = fs::copy(dest, src);
let _ = fs::remove_file(dest);
}
}
}
}
#[derive(serde::Serialize)]
pub enum PathStatus {
Empty,
@ -50,9 +66,11 @@ pub fn cmd_check_custom_data_path(path_str: String) -> Result<PathStatus, String
let pastebar_data_subfolder = path.join("pastebar-data");
if pastebar_data_subfolder.exists() && pastebar_data_subfolder.is_dir() {
// Check if the pastebar-data subfolder contains database files
let has_dev_db_in_subfolder = pastebar_data_subfolder.join("local.pastebar-db.data").exists();
let has_dev_db_in_subfolder = pastebar_data_subfolder
.join("local.pastebar-db.data")
.exists();
let has_prod_db_in_subfolder = pastebar_data_subfolder.join("pastebar-db.data").exists();
if has_dev_db_in_subfolder || has_prod_db_in_subfolder {
return Ok(PathStatus::IsPastebarDataAndNotEmpty);
} else {
@ -80,51 +98,33 @@ pub fn cmd_get_custom_db_path() -> Option<String> {
#[command]
pub fn cmd_create_directory(path_str: String) -> Result<(), String> {
let path = Path::new(&path_str);
fs::create_dir_all(&path).map_err(|e| {
format!(
"Failed to create directory {}: {}",
path.display(),
e
)
})?;
fs::create_dir_all(&path)
.map_err(|e| format!("Failed to create directory {}: {}", path.display(), e))?;
Ok(())
}
/// Validates if the provided path is a writable directory.
#[command]
pub fn cmd_validate_custom_db_path(path_str: String) -> Result<bool, String> {
let path = Path::new(&path_str);
if !path.exists() {
// Attempt to create it if it doesn't exist, to check writability of parent
if let Some(parent) = path.parent() {
if !parent.exists() {
fs::create_dir_all(parent).map_err(|e| {
format!(
"Failed to create parent directory {}: {}",
parent.display(),
e
)
})?;
}
}
// Check if we can create the directory itself (simulates future db file creation in this dir)
fs::create_dir_all(&path).map_err(|e| {
format!(
"Path {} is not a valid directory or cannot be created: {}",
path.display(),
e
)
})?;
// Clean up by removing the directory if we created it for validation
fs::remove_dir(&path).map_err(|e| {
format!(
"Failed to clean up validation directory {}: {}",
path.display(),
e
)
})?;
} else if !path.is_dir() {
return Err(format!("Path {} is not a directory.", path_str));
let input_path = PathBuf::from(&path_str);
if input_path
.components()
.any(|c| matches!(c, std::path::Component::ParentDir))
{
return Err("Path traversal not allowed".to_string());
}
let path = if input_path.exists() {
input_path
.canonicalize()
.map_err(|e| format!("Invalid path: {}", e))?
} else {
input_path
};
if path.exists() && !path.is_dir() {
return Err(format!("Path {} is not a directory.", path.display()));
}
// Check writability by trying to create a temporary file
@ -153,6 +153,8 @@ pub fn cmd_set_and_relocate_data(
let items_to_relocate = vec!["pastebar-db.data", "clip-images", "clipboard-images"];
let mut moved_items: Vec<(PathBuf, PathBuf)> = Vec::new();
for item_name in items_to_relocate {
let source_path = current_data_dir.join(item_name);
let dest_path = new_data_dir.join(item_name);
@ -168,24 +170,24 @@ pub fn cmd_set_and_relocate_data(
match operation.as_str() {
"move" => {
if source_path.is_dir() {
fs::rename(&source_path, &dest_path).map_err(|e| {
format!(
"Failed to move directory {} to {}: {}",
source_path.display(),
dest_path.display(),
e
)
})?;
let mut options = CopyOptions::new();
options.overwrite = true;
copy(&source_path, &dest_path, &options)
.map_err(|e| format!("Failed to copy directory: {}", e))?;
if let Err(e) = fs::remove_dir_all(&source_path) {
let _ = fs_extra::dir::remove(&dest_path);
rollback_moves(&moved_items);
return Err(format!("Failed to remove original directory: {}", e));
}
} else {
fs::rename(&source_path, &dest_path).map_err(|e| {
format!(
"Failed to move file {} to {}: {}",
source_path.display(),
dest_path.display(),
e
)
})?;
fs::copy(&source_path, &dest_path).map_err(|e| format!("Failed to copy file: {}", e))?;
if let Err(e) = fs::remove_file(&source_path) {
let _ = fs::remove_file(&dest_path);
rollback_moves(&moved_items);
return Err(format!("Failed to remove original file: {}", e));
}
}
moved_items.push((source_path.clone(), dest_path.clone()));
}
"copy" => {
if source_path.is_dir() {
@ -205,6 +207,7 @@ pub fn cmd_set_and_relocate_data(
}
user_settings_service::set_custom_db_path(&new_parent_dir_path)?;
crate::db::reinitialize_connection_pool();
Ok(format!(
"Data successfully {} to {}. Please restart the application.",
@ -218,6 +221,7 @@ pub fn cmd_set_and_relocate_data(
pub fn cmd_revert_to_default_data_location() -> Result<String, String> {
// Simply remove the custom database path setting
remove_custom_db_path()?;
crate::db::reinitialize_connection_pool();
Ok("Custom database location setting removed successfully.".to_string())
}

View File

@ -4,6 +4,7 @@ use serde::Serialize;
use std::fs;
use std::path::Path;
use std::path::PathBuf;
use std::sync::RwLock;
use std::time::Duration;
use diesel::connection::SimpleConnection;
@ -39,7 +40,7 @@ pub struct ConnectionOptions {
}
lazy_static! {
pub static ref DB_POOL_CONNECTION: Pool = init_connection_pool();
pub static ref DB_POOL_CONNECTION: RwLock<Pool> = RwLock::new(init_connection_pool());
}
impl diesel::r2d2::CustomizeConnection<SqliteConnection, diesel::r2d2::Error>
@ -92,6 +93,12 @@ fn init_connection_pool() -> Pool {
.expect("Failed to create db pool.")
}
pub fn reinitialize_connection_pool() {
let new_pool = init_connection_pool();
let mut pool_lock = DB_POOL_CONNECTION.write().unwrap();
*pool_lock = new_pool;
}
pub fn init(app: &mut tauri::App) {
let config = app.config().clone();
@ -182,6 +189,7 @@ pub fn establish_pool_db_connection(
});
DB_POOL_CONNECTION
.read()
.get()
.unwrap_or_else(|_| panic!("Error connecting to db pool"))
}
@ -268,10 +276,11 @@ pub fn get_default_db_path_string() -> String {
pub fn to_relative_image_path(absolute_path: &str) -> String {
let data_dir = get_data_dir();
let data_dir_str = data_dir.to_string_lossy();
if absolute_path.starts_with(&data_dir_str.as_ref()) {
// Remove the data directory prefix and replace with placeholder
let relative_path = absolute_path.strip_prefix(&data_dir_str.as_ref())
let relative_path = absolute_path
.strip_prefix(&data_dir_str.as_ref())
.unwrap_or(absolute_path)
.trim_start_matches('/')
.trim_start_matches('\\');
@ -291,7 +300,10 @@ pub fn to_absolute_image_path(relative_path: &str) -> String {
.unwrap_or(relative_path)
.trim_start_matches('/')
.trim_start_matches('\\');
data_dir.join(path_without_placeholder).to_string_lossy().into_owned()
data_dir
.join(path_without_placeholder)
.to_string_lossy()
.into_owned()
} else {
// If path doesn't have placeholder, return as is
relative_path.to_string()

View File

@ -39,7 +39,7 @@ pub fn insert_or_update_setting_by_name(
setting: &Setting,
app_handle: tauri::AppHandle,
) -> Result<String, Error> {
let connection = &mut DB_POOL_CONNECTION.get().unwrap();
let connection = &mut DB_POOL_CONNECTION.read().unwrap().get().unwrap();
match settings
.filter(name.eq(&setting.name))