refactor(backend): phase 4 - extract provider service layer
Architecture improvements: - Extract ProviderService with switch/backfill/write methods - Reduce command layer from 160 to 13 lines via delegation - Separate business logic (services) from state management (commands) - Introduce precise error handling with structured validation Refactoring details: - Split Codex/Claude switching into symmetric private methods - Add multi-layer validation for Codex auth field (existence + type) - Extract import_config_from_path for command and test reuse - Expose export_config_to_file and ProviderService in public API Test coverage: - Add 10+ integration tests for Claude/Codex switching flows - Cover import/export success and failure scenarios (JSON parse, missing file) - Verify state consistency on error paths (current remains unchanged) - Test snapshot backfill for both old and new providers after switching
This commit is contained in:
Jason
@@ -1,9 +1,10 @@
|
||||
use std::fs;
|
||||
use std::{fs, path::Path, sync::Mutex};
|
||||
use tauri::async_runtime;
|
||||
use serde_json::json;
|
||||
|
||||
use cc_switch_lib::{
|
||||
create_backup, get_claude_settings_path, read_json_file, sync_current_providers_to_live, AppType,
|
||||
MultiAppConfig, Provider,
|
||||
create_backup, get_claude_settings_path, import_config_from_path, read_json_file,
|
||||
sync_current_providers_to_live, AppError, AppState, AppType, MultiAppConfig, Provider,
|
||||
};
|
||||
|
||||
#[path = "support.rs"]
|
||||
@@ -671,3 +672,188 @@ fn create_backup_retains_only_latest_entries() {
|
||||
"cleanup should keep part of the older backups to maintain history"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn import_config_from_path_overwrites_state_and_creates_backup() {
|
||||
let _guard = test_mutex().lock().expect("acquire test mutex");
|
||||
reset_test_fs();
|
||||
let home = ensure_test_home();
|
||||
|
||||
let config_dir = home.join(".cc-switch");
|
||||
fs::create_dir_all(&config_dir).expect("create config dir");
|
||||
let config_path = config_dir.join("config.json");
|
||||
fs::write(&config_path, r#"{"version":1}"#).expect("seed original config");
|
||||
|
||||
let import_payload = serde_json::json!({
|
||||
"version": 2,
|
||||
"claude": {
|
||||
"providers": {
|
||||
"p-new": {
|
||||
"id": "p-new",
|
||||
"name": "Test Claude",
|
||||
"settingsConfig": {
|
||||
"env": { "ANTHROPIC_API_KEY": "new-key" }
|
||||
}
|
||||
}
|
||||
},
|
||||
"current": "p-new"
|
||||
},
|
||||
"codex": {
|
||||
"providers": {},
|
||||
"current": ""
|
||||
},
|
||||
"mcp": {
|
||||
"claude": { "servers": {} },
|
||||
"codex": { "servers": {} }
|
||||
}
|
||||
});
|
||||
|
||||
let import_path = config_dir.join("import.json");
|
||||
fs::write(
|
||||
&import_path,
|
||||
serde_json::to_string_pretty(&import_payload).expect("serialize import payload"),
|
||||
)
|
||||
.expect("write import file");
|
||||
|
||||
let app_state = AppState {
|
||||
config: Mutex::new(MultiAppConfig::default()),
|
||||
};
|
||||
|
||||
let backup_id =
|
||||
import_config_from_path(&import_path, &app_state).expect("import should succeed");
|
||||
assert!(
|
||||
!backup_id.is_empty(),
|
||||
"expected backup id when original config exists"
|
||||
);
|
||||
|
||||
let backup_path = config_dir.join("backups").join(format!("{backup_id}.json"));
|
||||
assert!(
|
||||
backup_path.exists(),
|
||||
"backup file should exist at {}",
|
||||
backup_path.display()
|
||||
);
|
||||
|
||||
let updated_content = fs::read_to_string(&config_path).expect("read updated config");
|
||||
let parsed: serde_json::Value =
|
||||
serde_json::from_str(&updated_content).expect("parse updated config");
|
||||
assert_eq!(
|
||||
parsed
|
||||
.get("claude")
|
||||
.and_then(|c| c.get("current"))
|
||||
.and_then(|c| c.as_str()),
|
||||
Some("p-new"),
|
||||
"saved config should record new current provider"
|
||||
);
|
||||
|
||||
let guard = app_state
|
||||
.config
|
||||
.lock()
|
||||
.expect("lock state after import");
|
||||
let claude_manager = guard
|
||||
.get_manager(&AppType::Claude)
|
||||
.expect("claude manager in state");
|
||||
assert_eq!(
|
||||
claude_manager.current, "p-new",
|
||||
"state should reflect new current provider"
|
||||
);
|
||||
assert!(
|
||||
claude_manager.providers.contains_key("p-new"),
|
||||
"new provider should exist in state"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn import_config_from_path_invalid_json_returns_error() {
|
||||
let _guard = test_mutex().lock().expect("acquire test mutex");
|
||||
reset_test_fs();
|
||||
let home = ensure_test_home();
|
||||
|
||||
let config_dir = home.join(".cc-switch");
|
||||
fs::create_dir_all(&config_dir).expect("create config dir");
|
||||
|
||||
let invalid_path = config_dir.join("broken.json");
|
||||
fs::write(&invalid_path, "{ not-json ").expect("write invalid json");
|
||||
|
||||
let app_state = AppState {
|
||||
config: Mutex::new(MultiAppConfig::default()),
|
||||
};
|
||||
|
||||
let err =
|
||||
import_config_from_path(&invalid_path, &app_state).expect_err("import should fail");
|
||||
match err {
|
||||
AppError::Json { .. } => {}
|
||||
other => panic!("expected json error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn import_config_from_path_missing_file_produces_io_error() {
|
||||
let _guard = test_mutex().lock().expect("acquire test mutex");
|
||||
reset_test_fs();
|
||||
let _home = ensure_test_home();
|
||||
|
||||
let missing_path = Path::new("/nonexistent/import.json");
|
||||
let app_state = AppState {
|
||||
config: Mutex::new(MultiAppConfig::default()),
|
||||
};
|
||||
|
||||
let err = import_config_from_path(missing_path, &app_state)
|
||||
.expect_err("import should fail for missing file");
|
||||
match err {
|
||||
AppError::Io { .. } => {}
|
||||
other => panic!("expected io error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_config_to_file_writes_target_path() {
|
||||
let _guard = test_mutex().lock().expect("acquire test mutex");
|
||||
reset_test_fs();
|
||||
let home = ensure_test_home();
|
||||
|
||||
let config_dir = home.join(".cc-switch");
|
||||
fs::create_dir_all(&config_dir).expect("create config dir");
|
||||
let config_path = config_dir.join("config.json");
|
||||
fs::write(&config_path, r#"{"version":42,"flag":true}"#).expect("write config");
|
||||
|
||||
let export_path = home.join("exported-config.json");
|
||||
if export_path.exists() {
|
||||
fs::remove_file(&export_path).expect("cleanup export target");
|
||||
}
|
||||
|
||||
let result = async_runtime::block_on(cc_switch_lib::export_config_to_file(
|
||||
export_path.to_string_lossy().to_string(),
|
||||
))
|
||||
.expect("export should succeed");
|
||||
assert_eq!(
|
||||
result.get("success").and_then(|v| v.as_bool()),
|
||||
Some(true)
|
||||
);
|
||||
|
||||
let exported = fs::read_to_string(&export_path).expect("read exported file");
|
||||
assert!(
|
||||
exported.contains(r#""version":42"#) && exported.contains(r#""flag":true"#),
|
||||
"exported file should mirror source config content"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_config_to_file_returns_error_when_source_missing() {
|
||||
let _guard = test_mutex().lock().expect("acquire test mutex");
|
||||
reset_test_fs();
|
||||
let home = ensure_test_home();
|
||||
|
||||
let export_path = home.join("export-missing.json");
|
||||
if export_path.exists() {
|
||||
fs::remove_file(&export_path).expect("cleanup export target");
|
||||
}
|
||||
|
||||
let err = async_runtime::block_on(cc_switch_lib::export_config_to_file(
|
||||
export_path.to_string_lossy().to_string(),
|
||||
))
|
||||
.expect_err("export should fail when config.json missing");
|
||||
assert!(
|
||||
err.contains("IO 错误"),
|
||||
"expected IO error message, got {err}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@ use serde_json::json;
|
||||
|
||||
use cc_switch_lib::{
|
||||
get_codex_auth_path, get_codex_config_path, read_json_file, switch_provider_test_hook,
|
||||
write_codex_live_atomic, AppState, AppType, MultiAppConfig, Provider,
|
||||
write_codex_live_atomic, AppError, AppState, AppType, MultiAppConfig, Provider,
|
||||
};
|
||||
|
||||
#[path = "support.rs"]
|
||||
@@ -157,3 +157,183 @@ fn switch_provider_missing_provider_returns_error() {
|
||||
"error message should mention missing provider"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn switch_provider_updates_claude_live_and_state() {
|
||||
let _guard = test_mutex().lock().expect("acquire test mutex");
|
||||
reset_test_fs();
|
||||
let _home = ensure_test_home();
|
||||
|
||||
let settings_path = cc_switch_lib::get_claude_settings_path();
|
||||
if let Some(parent) = settings_path.parent() {
|
||||
std::fs::create_dir_all(parent).expect("create claude settings dir");
|
||||
}
|
||||
let legacy_live = json!({
|
||||
"env": {
|
||||
"ANTHROPIC_API_KEY": "legacy-key"
|
||||
},
|
||||
"workspace": {
|
||||
"path": "/tmp/workspace"
|
||||
}
|
||||
});
|
||||
std::fs::write(
|
||||
&settings_path,
|
||||
serde_json::to_string_pretty(&legacy_live).expect("serialize legacy live"),
|
||||
)
|
||||
.expect("seed claude live config");
|
||||
|
||||
let mut config = MultiAppConfig::default();
|
||||
{
|
||||
let manager = config
|
||||
.get_manager_mut(&AppType::Claude)
|
||||
.expect("claude manager");
|
||||
manager.current = "old-provider".to_string();
|
||||
manager.providers.insert(
|
||||
"old-provider".to_string(),
|
||||
Provider::with_id(
|
||||
"old-provider".to_string(),
|
||||
"Legacy Claude".to_string(),
|
||||
json!({
|
||||
"env": { "ANTHROPIC_API_KEY": "stale-key" }
|
||||
}),
|
||||
None,
|
||||
),
|
||||
);
|
||||
manager.providers.insert(
|
||||
"new-provider".to_string(),
|
||||
Provider::with_id(
|
||||
"new-provider".to_string(),
|
||||
"Fresh Claude".to_string(),
|
||||
json!({
|
||||
"env": { "ANTHROPIC_API_KEY": "fresh-key" },
|
||||
"workspace": { "path": "/tmp/new-workspace" }
|
||||
}),
|
||||
None,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
let app_state = AppState {
|
||||
config: std::sync::Mutex::new(config),
|
||||
};
|
||||
|
||||
switch_provider_test_hook(&app_state, AppType::Claude, "new-provider")
|
||||
.expect("switch provider should succeed");
|
||||
|
||||
let live_after: serde_json::Value =
|
||||
read_json_file(&settings_path).expect("read claude live settings");
|
||||
assert_eq!(
|
||||
live_after
|
||||
.get("env")
|
||||
.and_then(|env| env.get("ANTHROPIC_API_KEY"))
|
||||
.and_then(|key| key.as_str()),
|
||||
Some("fresh-key"),
|
||||
"live settings.json should reflect new provider auth"
|
||||
);
|
||||
|
||||
let locked = app_state
|
||||
.config
|
||||
.lock()
|
||||
.expect("lock config after switch");
|
||||
let manager = locked
|
||||
.get_manager(&AppType::Claude)
|
||||
.expect("claude manager after switch");
|
||||
assert_eq!(manager.current, "new-provider", "current provider updated");
|
||||
|
||||
let legacy_provider = manager
|
||||
.providers
|
||||
.get("old-provider")
|
||||
.expect("legacy provider still exists");
|
||||
assert_eq!(
|
||||
legacy_provider.settings_config, legacy_live,
|
||||
"previous provider should receive backfilled live config"
|
||||
);
|
||||
|
||||
let new_provider = manager
|
||||
.providers
|
||||
.get("new-provider")
|
||||
.expect("new provider exists");
|
||||
assert_eq!(
|
||||
new_provider
|
||||
.settings_config
|
||||
.get("env")
|
||||
.and_then(|env| env.get("ANTHROPIC_API_KEY"))
|
||||
.and_then(|key| key.as_str()),
|
||||
Some("fresh-key"),
|
||||
"new provider snapshot should retain fresh auth"
|
||||
);
|
||||
|
||||
drop(locked);
|
||||
|
||||
let home_dir =
|
||||
std::env::var("HOME").expect("HOME should be set by ensure_test_home");
|
||||
let config_path = std::path::Path::new(&home_dir)
|
||||
.join(".cc-switch")
|
||||
.join("config.json");
|
||||
assert!(
|
||||
config_path.exists(),
|
||||
"switching provider should persist config.json"
|
||||
);
|
||||
let persisted: serde_json::Value =
|
||||
serde_json::from_str(&std::fs::read_to_string(&config_path).expect("read saved config"))
|
||||
.expect("parse saved config");
|
||||
assert_eq!(
|
||||
persisted
|
||||
.get("claude")
|
||||
.and_then(|claude| claude.get("current"))
|
||||
.and_then(|current| current.as_str()),
|
||||
Some("new-provider"),
|
||||
"saved config.json should record the new current provider"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn switch_provider_codex_missing_auth_returns_error_and_keeps_state() {
|
||||
let _guard = test_mutex().lock().expect("acquire test mutex");
|
||||
reset_test_fs();
|
||||
let _home = ensure_test_home();
|
||||
|
||||
let mut config = MultiAppConfig::default();
|
||||
{
|
||||
let manager = config
|
||||
.get_manager_mut(&AppType::Codex)
|
||||
.expect("codex manager");
|
||||
manager.providers.insert(
|
||||
"invalid".to_string(),
|
||||
Provider::with_id(
|
||||
"invalid".to_string(),
|
||||
"Broken Codex".to_string(),
|
||||
json!({
|
||||
"config": "[mcp_servers.test]\ncommand = \"noop\""
|
||||
}),
|
||||
None,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
let app_state = AppState {
|
||||
config: std::sync::Mutex::new(config),
|
||||
};
|
||||
|
||||
let err = switch_provider_test_hook(&app_state, AppType::Codex, "invalid")
|
||||
.expect_err("switching should fail when auth missing");
|
||||
match err {
|
||||
AppError::Config(msg) => assert!(
|
||||
msg.contains("auth"),
|
||||
"expected auth missing error message, got {msg}"
|
||||
),
|
||||
other => panic!("expected config error, got {other:?}"),
|
||||
}
|
||||
|
||||
let locked = app_state
|
||||
.config
|
||||
.lock()
|
||||
.expect("lock config after failure");
|
||||
let manager = locked
|
||||
.get_manager(&AppType::Codex)
|
||||
.expect("codex manager");
|
||||
assert!(
|
||||
manager.current.is_empty(),
|
||||
"current provider should remain empty on failure"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user