Skip to content

⚡ Optimize FileAuditStore::log_async to use spawn_blocking #100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Tuntii merged 1 commit into from
Feb 7, 2026
Merged

⚡ Optimize FileAuditStore::log_async to use spawn_blocking #100

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 44 additions & 14 deletions crates/rustapi-extras/src/audit/file_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ use super::event::AuditEvent;
use super::query::AuditQuery;
use super::store::{AuditError, AuditResult, AuditStore};
use std::fs::{File, OpenOptions};
use std::future::Future;
use std::io::{BufRead, BufReader, Write};
use std::path::PathBuf;
use std::sync::Mutex;
use std::pin::Pin;
use std::sync::{Arc, Mutex};

/// Configuration for file-based audit store.
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -45,19 +47,29 @@ impl FileAuditStoreConfig {
}
}

/// File-based audit store (JSON Lines format).
pub struct FileAuditStore {
/// Internal state for FileAuditStore
#[derive(Debug)]
struct FileAuditStoreInner {
config: FileAuditStoreConfig,
writer: Mutex<Option<File>>,
}

/// File-based audit store (JSON Lines format).
#[derive(Clone, Debug)]
pub struct FileAuditStore {
inner: Arc<FileAuditStoreInner>,
}

impl FileAuditStore {
/// Create a new file-based audit store.
pub fn new(config: FileAuditStoreConfig) -> AuditResult<Self> {
let store = Self {
let inner = FileAuditStoreInner {
config,
writer: Mutex::new(None),
};
let store = Self {
inner: Arc::new(inner),
};
store.open_writer()?;
Ok(store)
}
Expand All @@ -70,24 +82,25 @@ impl FileAuditStore {
/// Open or create the file writer.
fn open_writer(&self) -> AuditResult<()> {
let mut writer = self
.inner
.writer
.lock()
.map_err(|e| AuditError::WriteError(format!("Failed to acquire lock: {}", e)))?;

// Create parent directories if they don't exist
if let Some(parent) = self.config.file_path.parent() {
if !parent.exists() && self.config.create_if_missing {
if let Some(parent) = self.inner.config.file_path.parent() {
if !parent.exists() && self.inner.config.create_if_missing {
std::fs::create_dir_all(parent).map_err(|e| {
AuditError::IoError(format!("Failed to create directories: {}", e))
})?;
}
}

let file = OpenOptions::new()
.create(self.config.create_if_missing)
.append(self.config.append)
.create(self.inner.config.create_if_missing)
.append(self.inner.config.append)
.write(true)
.open(&self.config.file_path)
.open(&self.inner.config.file_path)
.map_err(|e| AuditError::IoError(format!("Failed to open file: {}", e)))?;

*writer = Some(file);
Expand All @@ -96,8 +109,8 @@ impl FileAuditStore {

/// Check if rotation is needed and perform it.
fn check_rotation(&self) -> AuditResult<()> {
if let Some(max_size) = self.config.max_file_size {
if let Ok(metadata) = std::fs::metadata(&self.config.file_path) {
if let Some(max_size) = self.inner.config.max_file_size {
if let Ok(metadata) = std::fs::metadata(&self.inner.config.file_path) {
if metadata.len() >= max_size {
self.rotate()?;
}
Expand All @@ -109,6 +122,7 @@ impl FileAuditStore {
/// Rotate the log file.
fn rotate(&self) -> AuditResult<()> {
let mut writer = self
.inner
.writer
.lock()
.map_err(|e| AuditError::WriteError(format!("Failed to acquire lock: {}", e)))?;
Expand All @@ -123,12 +137,13 @@ impl FileAuditStore {
.unwrap_or(0);

let rotated_path = self
.inner
.config
.file_path
.with_extension(format!("{}.log", timestamp));

// Rename current file
std::fs::rename(&self.config.file_path, &rotated_path)
std::fs::rename(&self.inner.config.file_path, &rotated_path)
.map_err(|e| AuditError::IoError(format!("Failed to rotate file: {}", e)))?;

// Open new file
Expand All @@ -140,7 +155,7 @@ impl FileAuditStore {

/// Read all events from the file.
fn read_all_events(&self) -> AuditResult<Vec<AuditEvent>> {
let path = &self.config.file_path;
let path = &self.inner.config.file_path;

if !path.exists() {
return Ok(Vec::new());
Expand Down Expand Up @@ -178,6 +193,7 @@ impl AuditStore for FileAuditStore {
self.check_rotation()?;

let mut writer = self
.inner
.writer
.lock()
.map_err(|e| AuditError::WriteError(format!("Failed to acquire lock: {}", e)))?;
Expand All @@ -195,6 +211,18 @@ impl AuditStore for FileAuditStore {
Ok(())
}

fn log_async(
&self,
event: AuditEvent,
) -> Pin<Box<dyn Future<Output = AuditResult<()>> + Send + '_>> {
let store = self.clone();
Box::pin(async move {
tokio::task::spawn_blocking(move || store.log(event))
.await
.map_err(|e| AuditError::IoError(format!("Task join error: {}", e)))?
})
}
Comment on lines +214 to +224
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new log_async method lacks test coverage. Given that the codebase uses comprehensive async testing (as seen in other modules like replay, api_key, circuit_breaker, etc.), and that log_async introduces new behavior with spawn_blocking, it should have dedicated test coverage. Consider adding a test like:

#[tokio::test]
async fn test_file_store_log_async() {
let (store, _dir) = temp_store();
let event = AuditEvent::new(AuditAction::Create).actor("admin");
let id = event.id.clone();
store.log_async(event).await.unwrap();
store.flush().unwrap();
let retrieved = store.get(&id).unwrap();
assert!(retrieved.is_some());
}

This would verify that the spawn_blocking mechanism works correctly and that events logged asynchronously are properly persisted.

Copilot uses AI. Check for mistakes.

fn get(&self, id: &str) -> AuditResult<Option<AuditEvent>> {
let events = self.read_all_events()?;
Ok(events.into_iter().find(|e| e.id == id))
Expand Down Expand Up @@ -238,14 +266,15 @@ impl AuditStore for FileAuditStore {

fn clear(&self) -> AuditResult<()> {
let mut writer = self
.inner
.writer
.lock()
.map_err(|e| AuditError::WriteError(format!("Failed to acquire lock: {}", e)))?;

*writer = None;

// Truncate the file
File::create(&self.config.file_path)
File::create(&self.inner.config.file_path)
.map_err(|e| AuditError::IoError(format!("Failed to clear file: {}", e)))?;

// Reopen
Expand All @@ -257,6 +286,7 @@ impl AuditStore for FileAuditStore {

fn flush(&self) -> AuditResult<()> {
let mut writer = self
.inner
.writer
.lock()
.map_err(|e| AuditError::WriteError(format!("Failed to acquire lock: {}", e)))?;
Expand Down
Loading