From 77a53bae86f808638568a0294bb11f78fce7b047 Mon Sep 17 00:00:00 2001 From: Maxime Van Hees Date: Thu, 25 Sep 2025 16:25:08 +0200 Subject: [PATCH] don't use strings for paths --- src/admin_meta.rs | 51 ++++++++++++++++---------------- src/cmd.rs | 2 +- src/main.rs | 3 +- src/options.rs | 4 ++- src/rpc.rs | 7 +++-- src/rpc_server.rs | 5 ++-- tests/debug_hset.rs | 3 +- tests/debug_hset_simple.rs | 3 +- tests/redis_tests.rs | 3 +- tests/rpc_tests.rs | 5 ++-- tests/simple_integration_test.rs | 3 +- tests/simple_redis_test.rs | 3 +- tests/usage_suite.rs | 3 +- 13 files changed, 54 insertions(+), 41 deletions(-) diff --git a/src/admin_meta.rs b/src/admin_meta.rs index d6454b3..4eb2d9e 100644 --- a/src/admin_meta.rs +++ b/src/admin_meta.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::{Arc, OnceLock, Mutex, RwLock}; use std::collections::HashMap; @@ -35,11 +35,11 @@ static DATA_STORAGES: OnceLock>>> = static DATA_INIT_LOCK: Mutex<()> = Mutex::new(()); fn init_admin_storage( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, ) -> Result, DBError> { - let db_file = PathBuf::from(base_dir).join("0.db"); + let db_file = base_dir.join("0.db"); if let Some(parent_dir) = db_file.parent() { std::fs::create_dir_all(parent_dir).map_err(|e| { DBError(format!("Failed to create directory {}: {}", parent_dir.display(), e)) @@ -57,24 +57,25 @@ fn init_admin_storage( // Get or initialize a cached handle to admin DB 0 per base_dir (thread-safe, no double-open race) pub fn open_admin_storage( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, ) -> Result, DBError> { let map = ADMIN_STORAGES.get_or_init(|| RwLock::new(HashMap::new())); + let key = base_dir.display().to_string(); // Fast path - if let Some(st) = map.read().unwrap().get(base_dir) { + if let Some(st) = map.read().unwrap().get(&key) { return Ok(st.clone()); } // Slow path with write lock { let mut w = map.write().unwrap(); - if let Some(st) = w.get(base_dir) { + if let Some(st) = w.get(&key) { return Ok(st.clone()); } // Detect existing 0.db backend by filesystem, if present. - let admin_path = PathBuf::from(base_dir).join("0.db"); + let admin_path = base_dir.join("0.db"); let detected = if admin_path.exists() { if admin_path.is_file() { Some(options::BackendType::Redb) @@ -102,14 +103,14 @@ pub fn open_admin_storage( }; let st = init_admin_storage(base_dir, effective_backend, admin_secret)?; - w.insert(base_dir.to_string(), st.clone()); + w.insert(key, st.clone()); Ok(st) } } // Ensure admin structures exist in encrypted DB 0 pub fn ensure_bootstrap( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, ) -> Result<(), DBError> { @@ -125,7 +126,7 @@ pub fn ensure_bootstrap( // Get or initialize a shared handle to a data DB (> 0), avoiding double-open across subsystems pub fn open_data_storage( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -159,7 +160,7 @@ pub fn open_data_storage( // 2) If missing, sniff filesystem (file => Redb, dir => Sled), then persist into admin meta // 3) Fallback to requested 'backend' (startup default) if nothing else is known let meta_backend = get_database_backend(base_dir, backend.clone(), admin_secret, id).ok().flatten(); - let db_path = PathBuf::from(base_dir).join(format!("{}.db", id)); + let db_path = base_dir.join(format!("{}.db", id)); let sniffed_backend = if db_path.exists() { if db_path.is_file() { Some(options::BackendType::Redb) @@ -214,7 +215,7 @@ pub fn open_data_storage( // Allocate the next DB id and persist new pointer pub fn allocate_next_id( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, ) -> Result { @@ -238,7 +239,7 @@ pub fn allocate_next_id( // Check existence of a db id in admin:dbs pub fn db_exists( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -249,7 +250,7 @@ pub fn db_exists( // Get per-db encryption key, if any pub fn get_enc_key( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -260,7 +261,7 @@ pub fn get_enc_key( // Set per-db encryption key (called during create) pub fn set_enc_key( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -272,7 +273,7 @@ pub fn set_enc_key( // Set database public flag pub fn set_database_public( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -286,7 +287,7 @@ pub fn set_database_public( // Persist per-db backend type in admin metadata (module-scope) pub fn set_database_backend( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -304,7 +305,7 @@ pub fn set_database_backend( } pub fn get_database_backend( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -321,7 +322,7 @@ pub fn get_database_backend( // Set database name pub fn set_database_name( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -335,7 +336,7 @@ pub fn set_database_name( // Get database name pub fn get_database_name( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -359,7 +360,7 @@ fn load_public( // Add access key for db (value format: "Read:ts" or "ReadWrite:ts") pub fn add_access_key( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -378,7 +379,7 @@ pub fn add_access_key( // Delete access key by hash pub fn delete_access_key( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -391,7 +392,7 @@ pub fn delete_access_key( // List access keys, returning (hash, perms, created_at_secs) pub fn list_access_keys( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -411,7 +412,7 @@ pub fn list_access_keys( // - Ok(Some(Permissions)) when access is allowed // - Ok(None) when not allowed or db missing (caller can distinguish by calling db_exists) pub fn verify_access( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, id: u64, @@ -456,7 +457,7 @@ pub fn verify_access( // Enumerate all db ids pub fn list_dbs( - base_dir: &str, + base_dir: &Path, backend: options::BackendType, admin_secret: &str, ) -> Result, DBError> { diff --git a/src/cmd.rs b/src/cmd.rs index 45f0fea..aca492d 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -1427,7 +1427,7 @@ async fn incr_cmd(server: &Server, key: &String) -> Result { fn config_get_cmd(name: &String, server: &Server) -> Result { let value = match name.as_str() { - "dir" => Some(server.option.dir.clone()), + "dir" => Some(server.option.dir.display().to_string()), "dbfilename" => Some(format!("{}.db", server.selected_db)), "databases" => Some("16".to_string()), // Hardcoded as per original logic _ => None, diff --git a/src/main.rs b/src/main.rs index 3a59b09..d5bacba 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,6 @@ // #![allow(unused_imports)] +use std::path::PathBuf; use tokio::net::TcpListener; use herodb::server; @@ -13,7 +14,7 @@ use clap::Parser; struct Args { /// The directory of Redis DB file #[arg(long)] - dir: String, + dir: PathBuf, /// The port of the Redis server, default is 6379 if not specified #[arg(long)] diff --git a/src/options.rs b/src/options.rs index a7a0216..b7d686b 100644 --- a/src/options.rs +++ b/src/options.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + #[derive(Debug, Clone, PartialEq, Eq)] pub enum BackendType { Redb, @@ -7,7 +9,7 @@ pub enum BackendType { #[derive(Debug, Clone)] pub struct DBOption { - pub dir: String, + pub dir: PathBuf, pub port: u16, pub debug: bool, // Deprecated for data DBs; retained for backward-compat on CLI parsing diff --git a/src/rpc.rs b/src/rpc.rs index c601535..86815b5 100644 --- a/src/rpc.rs +++ b/src/rpc.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::path::PathBuf; use std::sync::Arc; use tokio::sync::RwLock; use jsonrpsee::{core::RpcResult, proc_macros::rpc}; @@ -165,7 +166,7 @@ pub trait Rpc { /// RPC Server implementation pub struct RpcServerImpl { /// Base directory for database files - base_dir: String, + base_dir: PathBuf, /// Managed database servers servers: Arc>>>, /// Default backend type @@ -176,7 +177,7 @@ pub struct RpcServerImpl { impl RpcServerImpl { /// Create a new RPC server instance - pub fn new(base_dir: String, backend: crate::options::BackendType, admin_secret: String) -> Self { + pub fn new(base_dir: PathBuf, backend: crate::options::BackendType, admin_secret: String) -> Self { Self { base_dir, servers: Arc::new(RwLock::new(HashMap::new())), @@ -351,7 +352,7 @@ impl RpcServerImpl { backend, encrypted, redis_version: Some("7.0".to_string()), - storage_path: Some(server.option.dir.clone()), + storage_path: Some(server.option.dir.display().to_string()), size_on_disk, key_count, created_at, diff --git a/src/rpc_server.rs b/src/rpc_server.rs index eaabc5b..8000833 100644 --- a/src/rpc_server.rs +++ b/src/rpc_server.rs @@ -1,11 +1,12 @@ use std::net::SocketAddr; +use std::path::PathBuf; use jsonrpsee::server::{ServerBuilder, ServerHandle}; use jsonrpsee::RpcModule; use crate::rpc::{RpcServer, RpcServerImpl}; /// Start the RPC server on the specified address -pub async fn start_rpc_server(addr: SocketAddr, base_dir: String, backend: crate::options::BackendType, admin_secret: String) -> Result> { +pub async fn start_rpc_server(addr: SocketAddr, base_dir: PathBuf, backend: crate::options::BackendType, admin_secret: String) -> Result> { // Create the RPC server implementation let rpc_impl = RpcServerImpl::new(base_dir, backend, admin_secret); @@ -34,7 +35,7 @@ mod tests { #[tokio::test] async fn test_rpc_server_startup() { let addr = "127.0.0.1:0".parse().unwrap(); // Use port 0 for auto-assignment - let base_dir = "/tmp/test_rpc".to_string(); + let base_dir = PathBuf::from("/tmp/test_rpc"); let backend = crate::options::BackendType::Redb; // Default for test let handle = start_rpc_server(addr, base_dir, backend, "test-admin".to_string()).await.unwrap(); diff --git a/tests/debug_hset.rs b/tests/debug_hset.rs index 44cd39b..21613c5 100644 --- a/tests/debug_hset.rs +++ b/tests/debug_hset.rs @@ -1,4 +1,5 @@ use herodb::{server::Server, options::DBOption}; +use std::path::PathBuf; use std::time::Duration; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::TcpStream; @@ -22,7 +23,7 @@ async fn debug_hset_simple() { let port = 16500; let option = DBOption { - dir: test_dir.to_string(), + dir: PathBuf::from(test_dir), port, debug: false, encrypt: false, diff --git a/tests/debug_hset_simple.rs b/tests/debug_hset_simple.rs index fe99f0f..97dd5e1 100644 --- a/tests/debug_hset_simple.rs +++ b/tests/debug_hset_simple.rs @@ -1,4 +1,5 @@ use herodb::{server::Server, options::DBOption}; +use std::path::PathBuf; use std::time::Duration; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::TcpStream; @@ -13,7 +14,7 @@ async fn debug_hset_return_value() { std::fs::create_dir_all(&test_dir).unwrap(); let option = DBOption { - dir: test_dir.to_string(), + dir: PathBuf::from(test_dir), port: 16390, debug: false, encrypt: false, diff --git a/tests/redis_tests.rs b/tests/redis_tests.rs index 724704c..67981ee 100644 --- a/tests/redis_tests.rs +++ b/tests/redis_tests.rs @@ -1,4 +1,5 @@ use herodb::{server::Server, options::DBOption}; +use std::path::PathBuf; use std::time::Duration; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::TcpStream; @@ -17,7 +18,7 @@ async fn start_test_server(test_name: &str) -> (Server, u16) { std::fs::create_dir_all(&test_dir).unwrap(); let option = DBOption { - dir: test_dir, + dir: PathBuf::from(test_dir), port, debug: true, encrypt: false, diff --git a/tests/rpc_tests.rs b/tests/rpc_tests.rs index 8f8e6ce..da1f3a9 100644 --- a/tests/rpc_tests.rs +++ b/tests/rpc_tests.rs @@ -1,6 +1,7 @@ use herodb::rpc::{BackendType, DatabaseConfig}; use herodb::admin_meta; use herodb::options::BackendType as OptionsBackendType; +use std::path::Path; #[tokio::test] async fn test_rpc_server_basic() { @@ -70,11 +71,11 @@ async fn test_database_name_persistence() { let _ = std::fs::remove_dir_all(base_dir); // Set the database name - admin_meta::set_database_name(base_dir, backend.clone(), admin_secret, db_id, test_name) + admin_meta::set_database_name(Path::new(base_dir), backend.clone(), admin_secret, db_id, test_name) .expect("Failed to set database name"); // Retrieve the database name - let retrieved_name = admin_meta::get_database_name(base_dir, backend, admin_secret, db_id) + let retrieved_name = admin_meta::get_database_name(Path::new(base_dir), backend, admin_secret, db_id) .expect("Failed to get database name"); // Verify the name matches diff --git a/tests/simple_integration_test.rs b/tests/simple_integration_test.rs index 706c9cb..d99a74a 100644 --- a/tests/simple_integration_test.rs +++ b/tests/simple_integration_test.rs @@ -1,4 +1,5 @@ use herodb::{server::Server, options::DBOption}; +use std::path::PathBuf; use std::time::Duration; use tokio::time::sleep; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -19,7 +20,7 @@ async fn start_test_server(test_name: &str) -> (Server, u16) { std::fs::create_dir_all(&test_dir).unwrap(); let option = DBOption { - dir: test_dir, + dir: PathBuf::from(test_dir), port, debug: true, encrypt: false, diff --git a/tests/simple_redis_test.rs b/tests/simple_redis_test.rs index cd8c0a7..69a377d 100644 --- a/tests/simple_redis_test.rs +++ b/tests/simple_redis_test.rs @@ -1,4 +1,5 @@ use herodb::{server::Server, options::DBOption}; +use std::path::PathBuf; use std::time::Duration; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::TcpStream; @@ -17,7 +18,7 @@ async fn start_test_server(test_name: &str) -> (Server, u16) { std::fs::create_dir_all(&test_dir).unwrap(); let option = DBOption { - dir: test_dir, + dir: PathBuf::from(test_dir), port, debug: false, encrypt: false, diff --git a/tests/usage_suite.rs b/tests/usage_suite.rs index 3754906..3524c3b 100644 --- a/tests/usage_suite.rs +++ b/tests/usage_suite.rs @@ -1,4 +1,5 @@ use herodb::{options::DBOption, server::Server}; +use std::path::PathBuf; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::TcpStream; use tokio::time::{sleep, Duration}; @@ -17,7 +18,7 @@ async fn start_test_server(test_name: &str) -> (Server, u16) { std::fs::create_dir_all(&test_dir).unwrap(); let option = DBOption { - dir: test_dir, + dir: PathBuf::from(test_dir), port, debug: false, encrypt: false,