refactor(backend): implement transaction mechanism and i18n errors for provider service
This commit completes phase 4 service layer extraction by introducing: 1. **Transaction mechanism with 2PC (Two-Phase Commit)**: - Introduced `run_transaction()` wrapper with snapshot-based rollback - Implemented `LiveSnapshot` enum to capture and restore live config files - Added `PostCommitAction` to separate config.json persistence from live file writes - Applied to critical operations: add, update, switch providers - Ensures atomicity: memory + config.json + live files stay consistent 2. **Internationalized error handling**: - Added `AppError::Localized` variant with key + zh + en messages - Implemented `AppError::localized()` helper function - Migrated 24 error sites to use i18n-ready errors - Enables frontend to display errors in user's preferred language 3. **Concurrency optimization**: - Fixed `get_custom_endpoints()` to use read lock instead of write lock - Ensured async IO operations (usage query) execute outside lock scope - Added defensive RAII lock management with explicit scope blocks 4. **Code organization improvements**: - Reduced commands/provider.rs from ~800 to ~320 lines (-60%) - Expanded services/provider.rs with transaction infrastructure - Added unit tests for validation and credential extraction - Documented legacy file cleanup logic with inline comments 5. **Backfill mechanism refinement**: - Ensured live config is synced back to memory before switching - Maintains SSOT (Single Source of Truth) architecture principle - Handles Codex dual-file (auth.json + config.toml) atomically Breaking changes: None (internal refactoring only) Performance: Improved read concurrency, no measurable overhead from snapshots Test coverage: Added validation tests, updated service layer tests
This commit is contained in:
@@ -76,6 +76,10 @@ fn import_default_config_without_live_file_returns_error() {
|
||||
let err = import_default_config_test_hook(&state, AppType::Claude)
|
||||
.expect_err("missing live file should error");
|
||||
match err {
|
||||
AppError::Localized { zh, .. } => assert!(
|
||||
zh.contains("Claude Code 配置文件不存在"),
|
||||
"unexpected error message: {zh}"
|
||||
),
|
||||
AppError::Message(msg) => assert!(
|
||||
msg.contains("Claude Code 配置文件不存在"),
|
||||
"unexpected error message: {msg}"
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
use serde_json::json;
|
||||
use std::sync::RwLock;
|
||||
|
||||
use cc_switch_lib::{
|
||||
get_claude_settings_path, read_json_file, write_codex_live_atomic, AppError, AppType,
|
||||
get_claude_settings_path, read_json_file, write_codex_live_atomic, AppError, AppState, AppType,
|
||||
MultiAppConfig, Provider, ProviderService,
|
||||
};
|
||||
|
||||
@@ -33,9 +34,9 @@ command = "echo"
|
||||
write_codex_live_atomic(&legacy_auth, Some(legacy_config))
|
||||
.expect("seed existing codex live config");
|
||||
|
||||
let mut config = MultiAppConfig::default();
|
||||
let mut initial_config = MultiAppConfig::default();
|
||||
{
|
||||
let manager = config
|
||||
let manager = initial_config
|
||||
.get_manager_mut(&AppType::Codex)
|
||||
.expect("codex manager");
|
||||
manager.current = "old-provider".to_string();
|
||||
@@ -68,7 +69,7 @@ command = "say"
|
||||
);
|
||||
}
|
||||
|
||||
config.mcp.codex.servers.insert(
|
||||
initial_config.mcp.codex.servers.insert(
|
||||
"echo-server".into(),
|
||||
json!({
|
||||
"id": "echo-server",
|
||||
@@ -80,7 +81,11 @@ command = "say"
|
||||
}),
|
||||
);
|
||||
|
||||
ProviderService::switch(&mut config, AppType::Codex, "new-provider")
|
||||
let state = AppState {
|
||||
config: RwLock::new(initial_config),
|
||||
};
|
||||
|
||||
ProviderService::switch(&state, AppType::Codex, "new-provider")
|
||||
.expect("switch provider should succeed");
|
||||
|
||||
let auth_value: serde_json::Value =
|
||||
@@ -98,7 +103,8 @@ command = "say"
|
||||
"config.toml should contain synced MCP servers"
|
||||
);
|
||||
|
||||
let manager = config
|
||||
let guard = state.config.read().expect("read config after switch");
|
||||
let manager = guard
|
||||
.get_manager(&AppType::Codex)
|
||||
.expect("codex manager after switch");
|
||||
assert_eq!(manager.current, "new-provider", "current provider updated");
|
||||
@@ -188,7 +194,11 @@ fn provider_service_switch_claude_updates_live_and_state() {
|
||||
);
|
||||
}
|
||||
|
||||
ProviderService::switch(&mut config, AppType::Claude, "new-provider")
|
||||
let state = AppState {
|
||||
config: RwLock::new(config),
|
||||
};
|
||||
|
||||
ProviderService::switch(&state, AppType::Claude, "new-provider")
|
||||
.expect("switch provider should succeed");
|
||||
|
||||
let live_after: serde_json::Value =
|
||||
@@ -202,7 +212,11 @@ fn provider_service_switch_claude_updates_live_and_state() {
|
||||
"live settings.json should reflect new provider auth"
|
||||
);
|
||||
|
||||
let manager = config
|
||||
let guard = state
|
||||
.config
|
||||
.read()
|
||||
.expect("read claude config after switch");
|
||||
let manager = guard
|
||||
.get_manager(&AppType::Claude)
|
||||
.expect("claude manager after switch");
|
||||
assert_eq!(manager.current, "new-provider", "current provider updated");
|
||||
@@ -219,9 +233,11 @@ fn provider_service_switch_claude_updates_live_and_state() {
|
||||
|
||||
#[test]
|
||||
fn provider_service_switch_missing_provider_returns_error() {
|
||||
let mut config = MultiAppConfig::default();
|
||||
let state = AppState {
|
||||
config: RwLock::new(MultiAppConfig::default()),
|
||||
};
|
||||
|
||||
let err = ProviderService::switch(&mut config, AppType::Claude, "missing")
|
||||
let err = ProviderService::switch(&state, AppType::Claude, "missing")
|
||||
.expect_err("switching missing provider should fail");
|
||||
match err {
|
||||
AppError::ProviderNotFound(id) => assert_eq!(id, "missing"),
|
||||
@@ -249,7 +265,11 @@ fn provider_service_switch_codex_missing_auth_returns_error() {
|
||||
);
|
||||
}
|
||||
|
||||
let err = ProviderService::switch(&mut config, AppType::Codex, "invalid")
|
||||
let state = AppState {
|
||||
config: RwLock::new(config),
|
||||
};
|
||||
|
||||
let err = ProviderService::switch(&state, AppType::Codex, "invalid")
|
||||
.expect_err("switching should fail without auth");
|
||||
match err {
|
||||
AppError::Config(msg) => assert!(
|
||||
@@ -404,6 +424,10 @@ fn provider_service_delete_current_provider_returns_error() {
|
||||
let err = ProviderService::delete(&mut config, AppType::Claude, "keep")
|
||||
.expect_err("deleting current provider should fail");
|
||||
match err {
|
||||
AppError::Localized { zh, .. } => assert!(
|
||||
zh.contains("不能删除当前正在使用的供应商"),
|
||||
"unexpected message: {zh}"
|
||||
),
|
||||
AppError::Config(msg) => assert!(
|
||||
msg.contains("不能删除当前正在使用的供应商"),
|
||||
"unexpected message: {msg}"
|
||||
|
||||
Reference in New Issue
Block a user