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 <noreply@anthropic.com>
This commit is contained in:
@@ -183,6 +183,9 @@ export class ConfigManager {
|
|||||||
*/
|
*/
|
||||||
private async loadEnvFile(): Promise<void> {
|
private async loadEnvFile(): Promise<void> {
|
||||||
try {
|
try {
|
||||||
|
// Clear cache before loading
|
||||||
|
this.configCache.clear()
|
||||||
|
|
||||||
if (fs.existsSync(this.envPath)) {
|
if (fs.existsSync(this.envPath)) {
|
||||||
const content = fs.readFileSync(this.envPath, 'utf-8')
|
const content = fs.readFileSync(this.envPath, 'utf-8')
|
||||||
const lines = content.split('\n')
|
const lines = content.split('\n')
|
||||||
@@ -550,6 +553,68 @@ export class ConfigManager {
|
|||||||
return this.save()
|
return this.save()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Save partial settings (only update provided fields)
|
||||||
|
* Preserves all existing fields not included in the update
|
||||||
|
*/
|
||||||
|
public async savePartialSettings(
|
||||||
|
settings: Partial<SettingsData>
|
||||||
|
): 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
|
* Reset to default settings
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -6,6 +6,8 @@ describe('ConfigManager - deep merge utilities', () => {
|
|||||||
it('should deep merge objects, updating only specified fields', async () => {
|
it('should deep merge objects, updating only specified fields', async () => {
|
||||||
const manager = ConfigManager.getInstance()
|
const manager = ConfigManager.getInstance()
|
||||||
await manager.initialize()
|
await manager.initialize()
|
||||||
|
// Reload to ensure clean state from previous tests
|
||||||
|
await manager['loadEnvFile']()
|
||||||
|
|
||||||
// Setup initial state
|
// Setup initial state
|
||||||
const initial: SettingsData = {
|
const initial: SettingsData = {
|
||||||
@@ -47,6 +49,8 @@ describe('ConfigManager - deep merge utilities', () => {
|
|||||||
|
|
||||||
// Load initial settings
|
// Load initial settings
|
||||||
await manager.saveAllSettings(initial)
|
await manager.saveAllSettings(initial)
|
||||||
|
// Reload from disk to populate cache
|
||||||
|
await manager['loadEnvFile']()
|
||||||
|
|
||||||
// Partial update
|
// Partial update
|
||||||
const partial = {
|
const partial = {
|
||||||
@@ -86,30 +90,128 @@ describe('ConfigManager - backup and restore', () => {
|
|||||||
expect(fs.existsSync(backupPath)).toBe(true)
|
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()
|
const manager = ConfigManager.getInstance()
|
||||||
await manager.initialize()
|
await manager.initialize()
|
||||||
|
|
||||||
// Create initial state
|
// Setup initial state with multiple categories
|
||||||
const initial = manager.getAllSettings()
|
await manager.saveAllSettings({
|
||||||
const originalUrl = initial.erp.url
|
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
|
// Update only ERP URL
|
||||||
const fs = await import('fs')
|
const result = await manager.savePartialSettings({
|
||||||
const writeFileSyncSpy = vi.spyOn(fs, 'writeFileSync').mockImplementation(() => {
|
erp: { url: 'http://new.com' }
|
||||||
throw new Error('Disk full')
|
|
||||||
})
|
})
|
||||||
|
|
||||||
|
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({
|
const result = await manager.savePartialSettings({
|
||||||
erp: { url: 'http://should-not-save.com' }
|
database: { dbType: 'postgres' }
|
||||||
})
|
})
|
||||||
|
|
||||||
expect(result.success).toBe(false)
|
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()
|
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()
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user