Skip to content

Commit a3271df

Browse files
committed
refactor: extract helper methods in JsonFileRepository for better readability
Extract long, complex operations in save(), load(), and delete() methods into dedicated private helper methods with self-documenting names: - acquire_lock(): Centralizes file lock acquisition pattern - serialize_to_json(): Handles JSON serialization - read_file_content(): Handles file reading - deserialize_from_json(): Handles JSON deserialization Benefits: - Improved readability - each method reads like a high-level algorithm - Better maintainability - complex logic isolated in dedicated methods - DRY principle - lock acquisition pattern no longer duplicated - Self-documenting code - method names describe operations clearly
1 parent c7c6ccd commit a3271df

File tree

1 file changed

+49
-26
lines changed

1 file changed

+49
-26
lines changed

src/infrastructure/persistence/filesystem/json_file_repository.rs

Lines changed: 49 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -271,19 +271,12 @@ impl JsonFileRepository {
271271
/// # Ok::<(), Box<dyn std::error::Error>>(())
272272
/// ```
273273
pub fn save<T: Serialize>(&self, file_path: &Path, entity: &T) -> Result<(), JsonFileError> {
274-
// Ensure parent directory exists
275274
Self::ensure_parent_dir(file_path)?;
276275

277-
// Acquire lock
278-
let _lock = FileLock::acquire(file_path, self.lock_timeout)
279-
.map_err(|e| Self::convert_lock_error(e, file_path, "save"))?;
276+
let _lock = self.acquire_lock(file_path, "save")?;
280277

281-
// Serialize to JSON
282-
let json_content = serde_json::to_string_pretty(entity)
283-
.context("Failed to serialize entity to JSON")
284-
.map_err(JsonFileError::Internal)?;
278+
let json_content = Self::serialize_to_json(entity)?;
285279

286-
// Write atomically
287280
Self::write_atomic(file_path, &json_content)?;
288281

289282
Ok(())
@@ -310,24 +303,15 @@ impl JsonFileRepository {
310303
&self,
311304
file_path: &Path,
312305
) -> Result<Option<T>, JsonFileError> {
313-
// Check if file exists
314306
if !file_path.exists() {
315307
return Ok(None);
316308
}
317309

318-
// Acquire lock for reading
319-
let _lock = FileLock::acquire(file_path, self.lock_timeout)
320-
.map_err(|e| Self::convert_lock_error(e, file_path, "load"))?;
310+
let _lock = self.acquire_lock(file_path, "load")?;
321311

322-
// Read file content
323-
let content = fs::read_to_string(file_path)
324-
.with_context(|| format!("Failed to read file: {}", file_path.display()))
325-
.map_err(JsonFileError::Internal)?;
312+
let content = Self::read_file_content(file_path)?;
326313

327-
// Deserialize from JSON
328-
let entity: T = serde_json::from_str(&content)
329-
.with_context(|| format!("Failed to deserialize JSON from: {}", file_path.display()))
330-
.map_err(JsonFileError::Internal)?;
314+
let entity = Self::deserialize_from_json(&content, file_path)?;
331315

332316
Ok(Some(entity))
333317
// Lock is automatically released when _lock goes out of scope
@@ -360,16 +344,12 @@ impl JsonFileRepository {
360344
/// Returns `JsonFileError::Conflict` if the file is locked by another process.
361345
/// Returns `JsonFileError::Internal` for I/O errors.
362346
pub fn delete(&self, file_path: &Path) -> Result<(), JsonFileError> {
363-
// If file doesn't exist, operation is successful (idempotent)
364347
if !file_path.exists() {
365348
return Ok(());
366349
}
367350

368-
// Acquire lock before deletion
369-
let _lock = FileLock::acquire(file_path, self.lock_timeout)
370-
.map_err(|e| Self::convert_lock_error(e, file_path, "delete"))?;
351+
let _lock = self.acquire_lock(file_path, "delete")?;
371352

372-
// Delete the file
373353
fs::remove_file(file_path)
374354
.with_context(|| format!("Failed to delete file: {}", file_path.display()))
375355
.map_err(JsonFileError::Internal)?;
@@ -434,6 +414,49 @@ impl JsonFileRepository {
434414
.map_err(JsonFileError::Internal)
435415
}
436416

417+
/// Acquire lock for file operation
418+
///
419+
/// Acquires a file lock with timeout and converts lock errors to JSON file errors.
420+
///
421+
/// # Arguments
422+
///
423+
/// * `file_path` - Path to the file to lock
424+
/// * `operation` - Description of the operation being performed (for error context)
425+
fn acquire_lock(&self, file_path: &Path, operation: &str) -> Result<FileLock, JsonFileError> {
426+
FileLock::acquire(file_path, self.lock_timeout)
427+
.map_err(|e| Self::convert_lock_error(e, file_path, operation))
428+
}
429+
430+
/// Serialize entity to JSON
431+
///
432+
/// Converts an entity to a pretty-printed JSON string.
433+
fn serialize_to_json<T: Serialize>(entity: &T) -> Result<String, JsonFileError> {
434+
serde_json::to_string_pretty(entity)
435+
.context("Failed to serialize entity to JSON")
436+
.map_err(JsonFileError::Internal)
437+
}
438+
439+
/// Read file content
440+
///
441+
/// Reads the entire file content as a string.
442+
fn read_file_content(file_path: &Path) -> Result<String, JsonFileError> {
443+
fs::read_to_string(file_path)
444+
.with_context(|| format!("Failed to read file: {}", file_path.display()))
445+
.map_err(JsonFileError::Internal)
446+
}
447+
448+
/// Deserialize from JSON
449+
///
450+
/// Parses JSON content into the target entity type.
451+
fn deserialize_from_json<T: for<'de> Deserialize<'de>>(
452+
content: &str,
453+
file_path: &Path,
454+
) -> Result<T, JsonFileError> {
455+
serde_json::from_str(content)
456+
.with_context(|| format!("Failed to deserialize JSON from: {}", file_path.display()))
457+
.map_err(JsonFileError::Internal)
458+
}
459+
437460
/// Convert `FileLockError` to `JsonFileError` with operation context
438461
///
439462
/// Lock acquisition timeouts are mapped to `Conflict` errors, while other

0 commit comments

Comments
 (0)