Introduce base-dir constructors for isolation; update tests to avoid env var dependence; ensure directories exist before I/O; all tests green (including performance)
This commit is contained in:
@@ -70,6 +70,17 @@ impl DocxHandler {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Create a handler that stores temporary documents under the provided base directory
|
||||||
|
pub fn new_with_base_dir<P: AsRef<Path>>(base_dir: P) -> Result<Self> {
|
||||||
|
let temp_dir = base_dir.as_ref().join("docx-mcp");
|
||||||
|
fs::create_dir_all(&temp_dir)?;
|
||||||
|
Ok(Self {
|
||||||
|
temp_dir,
|
||||||
|
documents: std::collections::HashMap::new(),
|
||||||
|
in_memory_ops: std::collections::HashMap::new(),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
pub fn new_with_temp_dir(temp_dir: &Path) -> Result<Self> {
|
pub fn new_with_temp_dir(temp_dir: &Path) -> Result<Self> {
|
||||||
let temp_dir = temp_dir.to_path_buf();
|
let temp_dir = temp_dir.to_path_buf();
|
||||||
|
|||||||
@@ -36,6 +36,23 @@ impl DocxToolsProvider {
|
|||||||
security_config,
|
security_config,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Create a provider that stores temporary documents under the provided base directory
|
||||||
|
pub fn with_base_dir<P: AsRef<std::path::Path>>(base_dir: P) -> Self {
|
||||||
|
Self::with_base_dir_and_security(base_dir, SecurityConfig::default())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Create a provider with a base directory and explicit security config
|
||||||
|
pub fn with_base_dir_and_security<P: AsRef<std::path::Path>>(base_dir: P, security_config: SecurityConfig) -> Self {
|
||||||
|
Self {
|
||||||
|
handler: Arc::new(Mutex::new(DocxHandler::new_with_base_dir(base_dir).expect("Failed to create DocxHandler"))),
|
||||||
|
converter: Arc::new(DocumentConverter::new()),
|
||||||
|
#[cfg(feature = "advanced-docx")]
|
||||||
|
advanced: Arc::new(AdvancedDocxHandler::new()),
|
||||||
|
security: Arc::new(SecurityMiddleware::new(security_config.clone())),
|
||||||
|
security_config,
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl DocxToolsProvider {
|
impl DocxToolsProvider {
|
||||||
|
|||||||
@@ -34,9 +34,7 @@ async fn tool_result(provider: &DocxToolsProvider, name: &str, args: Value) -> T
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_complete_document_workflow() -> Result<()> {
|
async fn test_complete_document_workflow() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
let provider = DocxToolsProvider::with_base_dir(temp_dir.path());
|
||||||
|
|
||||||
let provider = DocxToolsProvider::new();
|
|
||||||
|
|
||||||
// Step 1: Create a new document
|
// Step 1: Create a new document
|
||||||
let create_result = tool_result(&provider, "create_document", json!({})).await;
|
let create_result = tool_result(&provider, "create_document", json!({})).await;
|
||||||
@@ -234,9 +232,7 @@ async fn test_complete_document_workflow() -> Result<()> {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_document_editing_workflow() -> Result<()> {
|
async fn test_document_editing_workflow() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
let provider = DocxToolsProvider::with_base_dir(temp_dir.path());
|
||||||
|
|
||||||
let provider = DocxToolsProvider::new();
|
|
||||||
|
|
||||||
// Create initial document
|
// Create initial document
|
||||||
let create_result = tool_result(&provider, "create_document", json!({})).await;
|
let create_result = tool_result(&provider, "create_document", json!({})).await;
|
||||||
@@ -424,9 +420,7 @@ async fn test_document_editing_workflow() -> Result<()> {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_collaborative_workflow() -> Result<()> {
|
async fn test_collaborative_workflow() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
let provider = DocxToolsProvider::with_base_dir(temp_dir.path());
|
||||||
|
|
||||||
let provider = DocxToolsProvider::new();
|
|
||||||
let mut document_ids = Vec::new();
|
let mut document_ids = Vec::new();
|
||||||
|
|
||||||
// Simulate multiple team members creating documents
|
// Simulate multiple team members creating documents
|
||||||
@@ -642,8 +636,6 @@ async fn test_collaborative_workflow() -> Result<()> {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_security_restricted_workflow() -> Result<()> {
|
async fn test_security_restricted_workflow() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
|
||||||
|
|
||||||
// Create a restrictive security configuration
|
// Create a restrictive security configuration
|
||||||
let mut whitelist = HashSet::new();
|
let mut whitelist = HashSet::new();
|
||||||
whitelist.insert("open_document".to_string());
|
whitelist.insert("open_document".to_string());
|
||||||
@@ -665,7 +657,7 @@ async fn test_security_restricted_workflow() -> Result<()> {
|
|||||||
allow_network: false,
|
allow_network: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
let provider = DocxToolsProvider::new_with_security(security_config);
|
let provider = DocxToolsProvider::with_base_dir_and_security(temp_dir.path(), security_config);
|
||||||
|
|
||||||
// Test security info
|
// Test security info
|
||||||
let security_info = tool_result(&provider, "get_security_info", json!({})).await;
|
let security_info = tool_result(&provider, "get_security_info", json!({})).await;
|
||||||
@@ -855,9 +847,7 @@ async fn test_security_restricted_workflow() -> Result<()> {
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_error_recovery_workflow() -> Result<()> {
|
async fn test_error_recovery_workflow() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
let provider = DocxToolsProvider::with_base_dir(temp_dir.path());
|
||||||
|
|
||||||
let provider = DocxToolsProvider::new();
|
|
||||||
|
|
||||||
// Test recovery from invalid document ID
|
// Test recovery from invalid document ID
|
||||||
let invalid_ops = vec![
|
let invalid_ops = vec![
|
||||||
|
|||||||
@@ -26,18 +26,13 @@ async fn tool_result(provider: &DocxToolsProvider, name: &str, args: serde_json:
|
|||||||
|
|
||||||
async fn create_test_provider() -> (DocxToolsProvider, TempDir) {
|
async fn create_test_provider() -> (DocxToolsProvider, TempDir) {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
// Ensure our handler uses this path for its own temp files
|
let provider = DocxToolsProvider::with_base_dir(temp_dir.path());
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
|
||||||
|
|
||||||
let provider = DocxToolsProvider::new();
|
|
||||||
(provider, temp_dir)
|
(provider, temp_dir)
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn create_test_provider_with_security(config: SecurityConfig) -> (DocxToolsProvider, TempDir) {
|
async fn create_test_provider_with_security(config: SecurityConfig) -> (DocxToolsProvider, TempDir) {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
let provider = DocxToolsProvider::with_base_dir_and_security(temp_dir.path(), config);
|
||||||
|
|
||||||
let provider = DocxToolsProvider::new_with_security(config);
|
|
||||||
(provider, temp_dir)
|
(provider, temp_dir)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -17,7 +17,7 @@ const STRESS_TEST_ITERATIONS: usize = 100;
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_large_document_performance() -> Result<()> {
|
fn test_large_document_performance() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
let mut handler = DocxHandler::new().unwrap();
|
let mut handler = DocxHandler::new_with_base_dir(temp_dir.path()).unwrap();
|
||||||
|
|
||||||
let start = Instant::now();
|
let start = Instant::now();
|
||||||
let doc_id = handler.create_document().unwrap();
|
let doc_id = handler.create_document().unwrap();
|
||||||
@@ -104,7 +104,7 @@ fn test_concurrent_document_stress() -> Result<()> {
|
|||||||
let results = Arc::clone(&results);
|
let results = Arc::clone(&results);
|
||||||
|
|
||||||
thread::spawn(move || -> Result<()> {
|
thread::spawn(move || -> Result<()> {
|
||||||
let mut handler = DocxHandler::new()?;
|
let mut handler = DocxHandler::new_with_base_dir(&*temp_path)?;
|
||||||
let mut local_results = Vec::new();
|
let mut local_results = Vec::new();
|
||||||
|
|
||||||
for op_id in 0..operations_per_thread {
|
for op_id in 0..operations_per_thread {
|
||||||
@@ -181,7 +181,7 @@ fn test_concurrent_document_stress() -> Result<()> {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_memory_intensive_operations() -> Result<()> {
|
fn test_memory_intensive_operations() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
let mut handler = DocxHandler::new().unwrap();
|
let mut handler = DocxHandler::new_with_base_dir(temp_dir.path()).unwrap();
|
||||||
|
|
||||||
let mut doc_ids = Vec::new();
|
let mut doc_ids = Vec::new();
|
||||||
|
|
||||||
@@ -256,9 +256,7 @@ fn test_memory_intensive_operations() -> Result<()> {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_mcp_tool_performance() -> Result<()> {
|
fn test_mcp_tool_performance() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
let provider = DocxToolsProvider::with_base_dir(temp_dir.path());
|
||||||
|
|
||||||
let provider = DocxToolsProvider::new();
|
|
||||||
let mut operation_times = Vec::new();
|
let mut operation_times = Vec::new();
|
||||||
|
|
||||||
// Test document creation performance
|
// Test document creation performance
|
||||||
@@ -385,10 +383,9 @@ fn test_mcp_tool_performance() -> Result<()> {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_security_overhead_performance() -> Result<()> {
|
fn test_security_overhead_performance() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
|
||||||
|
|
||||||
// Test with default (permissive) security
|
// Test with default (permissive) security
|
||||||
let default_provider = DocxToolsProvider::new();
|
let default_provider = DocxToolsProvider::with_base_dir(temp_dir.path());
|
||||||
|
|
||||||
// Test with restrictive security
|
// Test with restrictive security
|
||||||
let restrictive_config = SecurityConfig {
|
let restrictive_config = SecurityConfig {
|
||||||
@@ -400,7 +397,7 @@ fn test_security_overhead_performance() -> Result<()> {
|
|||||||
allow_network: false,
|
allow_network: false,
|
||||||
..Default::default()
|
..Default::default()
|
||||||
};
|
};
|
||||||
let restrictive_provider = DocxToolsProvider::new_with_security(restrictive_config);
|
let restrictive_provider = DocxToolsProvider::with_base_dir_and_security(temp_dir.path(), restrictive_config);
|
||||||
|
|
||||||
let operations = vec![
|
let operations = vec![
|
||||||
("list_documents", json!({})),
|
("list_documents", json!({})),
|
||||||
@@ -442,7 +439,7 @@ fn test_conversion_performance_scaling() -> Result<()> {
|
|||||||
let mut performance_data = Vec::new();
|
let mut performance_data = Vec::new();
|
||||||
|
|
||||||
for &size in &document_sizes {
|
for &size in &document_sizes {
|
||||||
let mut handler = DocxHandler::new()?;
|
let mut handler = DocxHandler::new_with_base_dir(temp_dir.path())?;
|
||||||
let doc_id = handler.create_document()?;
|
let doc_id = handler.create_document()?;
|
||||||
|
|
||||||
// Create document with specified number of paragraphs
|
// Create document with specified number of paragraphs
|
||||||
@@ -501,9 +498,7 @@ fn test_conversion_performance_scaling() -> Result<()> {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_error_handling_performance() -> Result<()> {
|
fn test_error_handling_performance() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
std::env::set_var("DOCX_MCP_TEMP", temp_dir.path());
|
let provider = DocxToolsProvider::with_base_dir(temp_dir.path());
|
||||||
|
|
||||||
let provider = DocxToolsProvider::new();
|
|
||||||
let error_operations = vec![
|
let error_operations = vec![
|
||||||
("extract_text", json!({"document_id": "nonexistent"})),
|
("extract_text", json!({"document_id": "nonexistent"})),
|
||||||
("add_paragraph", json!({"document_id": "fake", "text": "test"})),
|
("add_paragraph", json!({"document_id": "fake", "text": "test"})),
|
||||||
@@ -535,7 +530,7 @@ fn test_error_handling_performance() -> Result<()> {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_resource_cleanup_performance() -> Result<()> {
|
fn test_resource_cleanup_performance() -> Result<()> {
|
||||||
let temp_dir = TempDir::new().unwrap();
|
let temp_dir = TempDir::new().unwrap();
|
||||||
let mut handler = DocxHandler::new()?;
|
let mut handler = DocxHandler::new_with_base_dir(temp_dir.path())?;
|
||||||
|
|
||||||
let document_count = 50;
|
let document_count = 50;
|
||||||
let mut doc_ids = Vec::new();
|
let mut doc_ids = Vec::new();
|
||||||
|
|||||||
Reference in New Issue
Block a user