From f3be0ad01dbb4b9084e2c9060e1e95cc5077d075 Mon Sep 17 00:00:00 2001 From: Misaka_Company Date: Tue, 3 Mar 2026 14:36:50 +0800 Subject: [PATCH] feat: implement savePartialSettings with validation and rollback Implements the core savePartialSettings method that: - Validates fields against UI_EDITABLE_FIELDS whitelist - Deep merges partial updates with current settings - Creates backup before saving - Restores backup on save failure - Reloads .env file to populate cache with correct keys Added comprehensive tests: - Partial update preserves existing fields - Rejects non-whitelisted fields - Handles nested object updates - Restores backup on save failure Fixed cache key inconsistency bug by clearing cache in loadEnvFile() and reloading after save to ensure proper cache population. Co-Authored-By: Claude Sonnet 4.5 --- src/main/services/config/config-manager.ts | 65 +++++++++ .../services/config/config-manager.test.ts | 126 ++++++++++++++++-- 2 files changed, 179 insertions(+), 12 deletions(-) diff --git a/src/main/services/config/config-manager.ts b/src/main/services/config/config-manager.ts index 1281c7a..7432089 100644 --- a/src/main/services/config/config-manager.ts +++ b/src/main/services/config/config-manager.ts @@ -183,6 +183,9 @@ export class ConfigManager { */ private async loadEnvFile(): Promise { try { + // Clear cache before loading + this.configCache.clear() + if (fs.existsSync(this.envPath)) { const content = fs.readFileSync(this.envPath, 'utf-8') const lines = content.split('\n') @@ -550,6 +553,68 @@ export class ConfigManager { return this.save() } + /** + * Save partial settings (only update provided fields) + * Preserves all existing fields not included in the update + */ + public async savePartialSettings( + settings: Partial + ): Promise<{ success: boolean; error?: string }> { + try { + // Step 1: Validate field whitelist + const validation = validateEditableFields(settings) + if (!validation.valid) { + log.warn('Attempted to save non-editable fields', { + invalidFields: validation.invalidFields + }) + return { + success: false, + error: `包含不允许修改的字段：${validation.invalidFields.join(', ')}` + } + } + + // Step 2: Read current settings + const currentSettings = this.getAllSettings() + + // Step 3: Deep merge + const mergedSettings = deepMerge(currentSettings, settings) + + // Step 4: Backup and save + const backupSuccess = await this.backupEnvFile() + if (!backupSuccess) { + log.warn('Failed to backup .env file, proceeding with caution') + } + + const saveSuccess = await this.saveAllSettings(mergedSettings) + + if (!saveSuccess) { + // Save failed, attempt restore + await this.restoreBackup() + return { + success: false, + error: '保存配置失败，已恢复原配置' + } + } + + // Step 5: Reload from disk to populate cache with correct keys (ERP_URL instead of erp.url) + await this.loadEnvFile() + + log.info('Settings saved successfully', { + updatedFields: Object.keys(settings) + }) + + return { success: true } + } catch (error) { + const message = error instanceof Error ? error.message : 'Unknown error' + log.error('Error in savePartialSettings', { error: message }) + await this.restoreBackup() + return { + success: false, + error: `保存配置时发生错误：${message}` + } + } + } + /** * Reset to default settings */ diff --git a/tests/main/services/config/config-manager.test.ts b/tests/main/services/config/config-manager.test.ts index 37ed857..1a5f5f6 100644 --- a/tests/main/services/config/config-manager.test.ts +++ b/tests/main/services/config/config-manager.test.ts @@ -6,6 +6,8 @@ describe('ConfigManager - deep merge utilities', () => { it('should deep merge objects, updating only specified fields', async () => { const manager = ConfigManager.getInstance() await manager.initialize() + // Reload to ensure clean state from previous tests + await manager['loadEnvFile']() // Setup initial state const initial: SettingsData = { @@ -47,6 +49,8 @@ describe('ConfigManager - deep merge utilities', () => { // Load initial settings await manager.saveAllSettings(initial) + // Reload from disk to populate cache + await manager['loadEnvFile']() // Partial update const partial = { @@ -86,30 +90,128 @@ describe('ConfigManager - backup and restore', () => { expect(fs.existsSync(backupPath)).toBe(true) }) - it('should restore from backup when save fails', async () => { + // Note: Skipping fs.writeFileSync mock test due to ESM limitations in Vitest + // The restoreBackup functionality is tested indirectly through the savePartialSettings rollback test +}) + +describe('ConfigManager.savePartialSettings', () => { + it('should save only specified fields and preserve others', async () => { const manager = ConfigManager.getInstance() await manager.initialize() - // Create initial state - const initial = manager.getAllSettings() - const originalUrl = initial.erp.url + // Setup initial state with multiple categories + await manager.saveAllSettings({ + erp: { url: 'http://old.com', username: 'user1', password: 'pass1', headless: true, ignoreHttpsErrors: true, autoCloseBrowser: true }, + database: { dbType: 'mysql', server: '', mysqlHost: '192.168.1.1', mysqlPort: 3306, database: 'testdb', username: 'dbuser', password: '' }, + paths: { dataDir: '/old/path', defaultOutput: 'out.xlsx', validationOutput: 'val.xlsx' }, + extraction: { batchSize: 50, verbose: true, autoConvert: true, mergeBatches: true, enableDbPersistence: true }, + validation: { dataSource: 'database_full', batchSize: 1000, matchMode: 'exact', enableCrud: false, defaultManager: '' }, + ui: { fontFamily: 'Tahoma', fontSize: 14, productionIdInputWidth: 25 }, + execution: { dryRun: true } + }) + // Reload from disk to populate cache + await manager['loadEnvFile']() - // Mock fs.writeFileSync to fail - const fs = await import('fs') - const writeFileSyncSpy = vi.spyOn(fs, 'writeFileSync').mockImplementation(() => { - throw new Error('Disk full') + // Update only ERP URL + const result = await manager.savePartialSettings({ + erp: { url: 'http://new.com' } }) + expect(result.success).toBe(true) + + const current = manager.getAllSettings() + + // Verify updated field + expect(current.erp.url).toBe('http://new.com') + + // Verify preserved ERP fields + expect(current.erp.username).toBe('user1') + expect(current.erp.password).toBe('pass1') + + // Verify preserved other categories + expect(current.database.dbType).toBe('mysql') + expect(current.database.mysqlHost).toBe('192.168.1.1') + expect(current.paths.dataDir).toBe('/old/path') + expect(current.extraction.batchSize).toBe(50) + expect(current.ui.fontFamily).toBe('Tahoma') + }) + + it('should reject updates to non-whitelisted fields', async () => { + const manager = ConfigManager.getInstance() + await manager.initialize() + // Reset to ensure clean state + manager.resetToDefaults() + await manager.save() + const result = await manager.savePartialSettings({ - erp: { url: 'http://should-not-save.com' } + database: { dbType: 'postgres' } }) expect(result.success).toBe(false) + expect(result.error).toContain('不允许修改') + expect(result.error).toContain('database.dbType') + }) + + it('should handle nested object updates correctly', async () => { + const manager = ConfigManager.getInstance() + await manager.initialize() + // Reset to ensure clean state + manager.resetToDefaults() + await manager.save() + + await manager.saveAllSettings({ + erp: { url: 'http://test.com', username: 'u', password: 'p', headless: false, ignoreHttpsErrors: false, autoCloseBrowser: false }, + database: { dbType: 'mysql', server: '', mysqlHost: 'localhost', mysqlPort: 3306, database: 'db', username: 'user', password: '' }, + paths: { dataDir: '/data', defaultOutput: 'out.xlsx', validationOutput: 'val.xlsx' }, + extraction: { batchSize: 100, verbose: true, autoConvert: true, mergeBatches: true, enableDbPersistence: true }, + validation: { dataSource: 'database_full', batchSize: 2000, matchMode: 'substring', enableCrud: false, defaultManager: '' }, + ui: { fontFamily: 'Arial', fontSize: 12, productionIdInputWidth: 20 }, + execution: { dryRun: false } + }) + // Reload from disk to populate cache + await manager['loadEnvFile']() + + // Update multiple ERP fields at once + const result = await manager.savePartialSettings({ + erp: { + url: 'http://updated.com', + username: 'newuser', + password: 'newpass' + } + }) + + expect(result.success).toBe(true) - // Restore should have happened const current = manager.getAllSettings() - expect(current.erp.url).toBe(originalUrl) - writeFileSyncSpy.mockRestore() + expect(current.erp.url).toBe('http://updated.com') + expect(current.erp.username).toBe('newuser') + expect(current.erp.password).toBe('newpass') + expect(current.erp.headless).toBe(false) // preserved + }) + + it('should restore backup on save failure', async () => { + const manager = ConfigManager.getInstance() + await manager.initialize() + // Reset to ensure clean state + manager.resetToDefaults() + await manager.save() + + const originalUrl = manager.getAllSettings().erp.url + + // Mock save to fail + vi.spyOn(manager, 'save').mockResolvedValueOnce(false) + + const result = await manager.savePartialSettings({ + erp: { url: 'http://should-not-apply.com' } + }) + + expect(result.success).toBe(false) + expect(result.error).toContain('保存配置失败') + + // Verify rollback + expect(manager.getAllSettings().erp.url).toBe(originalUrl) + + manager.save.mockRestore() }) })