fix(config): remove deprecated keys during migration
- commit
- 89ee588d4f02dff4f8ed26576ba14873ad55d3c4
- author
- bnema · · 2025-12-28 18:07 UTC
- parent
- ff5b26e
- signature
- GPG Unverified
2B82207D370127A0The key is unknown
7 files · +526 -44
M CHANGELOG.md +1 -0
| @@ -42,6 +42,7 @@ All notable changes to this project will be documented in this file. | |||
| 42 | 42 | ### Fixed | |
| 43 | 43 | - **Omnibox favorite toggle reactivity**: Fixed space key toggle in omnibox not updating UI reactively. Now properly toggles favorites (add if not favorite, remove if favorite) with immediate visual feedback (yellow indicator) and toast notifications. Business logic moved to `ManageFavoritesUseCase.Toggle()` for testability. | |
| 44 | 44 | - **Config migration type matching**: Fixed migration incorrectly matching keys of different types (e.g., `_width` int keys with `_color` string keys) during rename detection, preventing value swapping. | |
| 45 | + | - **Config migrate removes deprecated settings**: Fixed `dumber config migrate` not removing deprecated settings from the config file. Previously deprecated keys were preserved but unused; now they are properly deleted. Supports TOML, YAML, and JSON config formats. | |
| 45 | 46 | - **Fullscreen video tab bar**: Fixed tab bar remaining visible during fullscreen video playback. Now hides automatically when entering fullscreen and restores based on normal visibility logic when exiting. | |
| 46 | 47 | - **Dark mode navigation flash**: Fixed a brief white flash between the loading skeleton and page content when navigating in dark mode. | |
| 47 | 48 | - **Progress bar stuck state**: Added 30-second timeout to progress bar to auto-hide if page load stalls and never completes. | |
| @@ -42,6 +42,7 @@ All notable changes to this project will be documented in this file. | |||||
| 42 | ### Fixed | 42 | ### Fixed | ||
| 43 | - **Omnibox favorite toggle reactivity**: Fixed space key toggle in omnibox not updating UI reactively. Now properly toggles favorites (add if not favorite, remove if favorite) with immediate visual feedback (yellow indicator) and toast notifications. Business logic moved to `ManageFavoritesUseCase.Toggle()` for testability. | 43 | - **Omnibox favorite toggle reactivity**: Fixed space key toggle in omnibox not updating UI reactively. Now properly toggles favorites (add if not favorite, remove if favorite) with immediate visual feedback (yellow indicator) and toast notifications. Business logic moved to `ManageFavoritesUseCase.Toggle()` for testability. | ||
| 44 | - **Config migration type matching**: Fixed migration incorrectly matching keys of different types (e.g., `_width` int keys with `_color` string keys) during rename detection, preventing value swapping. | 44 | - **Config migration type matching**: Fixed migration incorrectly matching keys of different types (e.g., `_width` int keys with `_color` string keys) during rename detection, preventing value swapping. | ||
| 45 | + | - **Config migrate removes deprecated settings**: Fixed `dumber config migrate` not removing deprecated settings from the config file. Previously deprecated keys were preserved but unused; now they are properly deleted. Supports TOML, YAML, and JSON config formats. | |||
| 45 | - **Fullscreen video tab bar**: Fixed tab bar remaining visible during fullscreen video playback. Now hides automatically when entering fullscreen and restores based on normal visibility logic when exiting. | 46 | - **Fullscreen video tab bar**: Fixed tab bar remaining visible during fullscreen video playback. Now hides automatically when entering fullscreen and restores based on normal visibility logic when exiting. | ||
| 46 | - **Dark mode navigation flash**: Fixed a brief white flash between the loading skeleton and page content when navigating in dark mode. | 47 | - **Dark mode navigation flash**: Fixed a brief white flash between the loading skeleton and page content when navigating in dark mode. | ||
| 47 | - **Progress bar stuck state**: Added 30-second timeout to progress bar to auto-hide if page load stalls and never completes. | 48 | - **Progress bar stuck state**: Added 30-second timeout to progress bar to auto-hide if page load stalls and never completes. | ||
M internal/application/port/config_migrator.go +5 -2
| @@ -63,12 +63,15 @@ type ConfigMigrator interface { | |||
| 63 | 63 | // This provides a detailed diff-like view of what migration would do. | |
| 64 | 64 | DetectChanges() ([]KeyChange, error) | |
| 65 | 65 | ||
| 66 | - | // Migrate adds missing default keys to the user's config file. | |
| 67 | - | // Returns the list of keys that were added/renamed. | |
| 66 | + | // Migrate adds missing default keys and removes deprecated keys from the user's config file. | |
| 67 | + | // Returns the list of keys that were added/renamed/removed. | |
| 68 | 68 | Migrate() ([]string, error) | |
| 69 | 69 | ||
| 70 | 70 | // GetKeyInfo returns detailed information about a config key. | |
| 71 | 71 | GetKeyInfo(key string) KeyInfo | |
| 72 | + | ||
| 73 | + | // GetConfigFile returns the path to the user's config file. | |
| 74 | + | GetConfigFile() (string, error) | |
| 72 | 75 | } | |
| 73 | 76 | ||
| 74 | 77 | // DiffFormatter formats config changes for display. | |
| @@ -63,12 +63,15 @@ type ConfigMigrator interface { | |||||
| 63 | // This provides a detailed diff-like view of what migration would do. | 63 | // This provides a detailed diff-like view of what migration would do. | ||
| 64 | DetectChanges() ([]KeyChange, error) | 64 | DetectChanges() ([]KeyChange, error) | ||
| 65 | 65 | ||||
| 66 | - | // Migrate adds missing default keys to the user's config file. | 66 | + | // Migrate adds missing default keys and removes deprecated keys from the user's config file. |
| 67 | - | // Returns the list of keys that were added/renamed. | 67 | + | // Returns the list of keys that were added/renamed/removed. |
| 68 | Migrate() ([]string, error) | 68 | Migrate() ([]string, error) | ||
| 69 | 69 | ||||
| 70 | // GetKeyInfo returns detailed information about a config key. | 70 | // GetKeyInfo returns detailed information about a config key. | ||
| 71 | GetKeyInfo(key string) KeyInfo | 71 | GetKeyInfo(key string) KeyInfo | ||
| 72 | + | ||||
| 73 | + | // GetConfigFile returns the path to the user's config file. | |||
| 74 | + | GetConfigFile() (string, error) | |||
| 72 | } | 75 | } | ||
| 73 | 76 | ||||
| 74 | // DiffFormatter formats config changes for display. | 77 | // DiffFormatter formats config changes for display. | ||
M internal/application/port/mocks/mocks.go +53 -0
| @@ -151,6 +151,59 @@ func (_c *MockConfigMigrator_DetectChanges_Call) RunAndReturn(run func() ([]port | |||
| 151 | 151 | return _c | |
| 152 | 152 | } | |
| 153 | 153 | ||
| 154 | + | // GetConfigFile provides a mock function for the type MockConfigMigrator | |
| 155 | + | func (_mock *MockConfigMigrator) GetConfigFile() (string, error) { | |
| 156 | + | ret := _mock.Called() | |
| 157 | + | ||
| 158 | + | if len(ret) == 0 { | |
| 159 | + | panic("no return value specified for GetConfigFile") | |
| 160 | + | } | |
| 161 | + | ||
| 162 | + | var r0 string | |
| 163 | + | var r1 error | |
| 164 | + | if returnFunc, ok := ret.Get(0).(func() (string, error)); ok { | |
| 165 | + | return returnFunc() | |
| 166 | + | } | |
| 167 | + | if returnFunc, ok := ret.Get(0).(func() string); ok { | |
| 168 | + | r0 = returnFunc() | |
| 169 | + | } else { | |
| 170 | + | r0 = ret.Get(0).(string) | |
| 171 | + | } | |
| 172 | + | if returnFunc, ok := ret.Get(1).(func() error); ok { | |
| 173 | + | r1 = returnFunc() | |
| 174 | + | } else { | |
| 175 | + | r1 = ret.Error(1) | |
| 176 | + | } | |
| 177 | + | return r0, r1 | |
| 178 | + | } | |
| 179 | + | ||
| 180 | + | // MockConfigMigrator_GetConfigFile_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetConfigFile' | |
| 181 | + | type MockConfigMigrator_GetConfigFile_Call struct { | |
| 182 | + | *mock.Call | |
| 183 | + | } | |
| 184 | + | ||
| 185 | + | // GetConfigFile is a helper method to define mock.On call | |
| 186 | + | func (_e *MockConfigMigrator_Expecter) GetConfigFile() *MockConfigMigrator_GetConfigFile_Call { | |
| 187 | + | return &MockConfigMigrator_GetConfigFile_Call{Call: _e.mock.On("GetConfigFile")} | |
| 188 | + | } | |
| 189 | + | ||
| 190 | + | func (_c *MockConfigMigrator_GetConfigFile_Call) Run(run func()) *MockConfigMigrator_GetConfigFile_Call { | |
| 191 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 192 | + | run() | |
| 193 | + | }) | |
| 194 | + | return _c | |
| 195 | + | } | |
| 196 | + | ||
| 197 | + | func (_c *MockConfigMigrator_GetConfigFile_Call) Return(s string, err error) *MockConfigMigrator_GetConfigFile_Call { | |
| 198 | + | _c.Call.Return(s, err) | |
| 199 | + | return _c | |
| 200 | + | } | |
| 201 | + | ||
| 202 | + | func (_c *MockConfigMigrator_GetConfigFile_Call) RunAndReturn(run func() (string, error)) *MockConfigMigrator_GetConfigFile_Call { | |
| 203 | + | _c.Call.Return(run) | |
| 204 | + | return _c | |
| 205 | + | } | |
| 206 | + | ||
| 154 | 207 | // GetKeyInfo provides a mock function for the type MockConfigMigrator | |
| 155 | 208 | func (_mock *MockConfigMigrator) GetKeyInfo(key string) port.KeyInfo { | |
| 156 | 209 | ret := _mock.Called(key) | |
| @@ -151,6 +151,59 @@ func (_c *MockConfigMigrator_DetectChanges_Call) RunAndReturn(run func() ([]port | |||||
| 151 | return _c | 151 | return _c | ||
| 152 | } | 152 | } | ||
| 153 | 153 | ||||
| 154 | + | // GetConfigFile provides a mock function for the type MockConfigMigrator | |||
| 155 | + | func (_mock *MockConfigMigrator) GetConfigFile() (string, error) { | |||
| 156 | + | ret := _mock.Called() | |||
| 157 | + | ||||
| 158 | + | if len(ret) == 0 { | |||
| 159 | + | panic("no return value specified for GetConfigFile") | |||
| 160 | + | } | |||
| 161 | + | ||||
| 162 | + | var r0 string | |||
| 163 | + | var r1 error | |||
| 164 | + | if returnFunc, ok := ret.Get(0).(func() (string, error)); ok { | |||
| 165 | + | return returnFunc() | |||
| 166 | + | } | |||
| 167 | + | if returnFunc, ok := ret.Get(0).(func() string); ok { | |||
| 168 | + | r0 = returnFunc() | |||
| 169 | + | } else { | |||
| 170 | + | r0 = ret.Get(0).(string) | |||
| 171 | + | } | |||
| 172 | + | if returnFunc, ok := ret.Get(1).(func() error); ok { | |||
| 173 | + | r1 = returnFunc() | |||
| 174 | + | } else { | |||
| 175 | + | r1 = ret.Error(1) | |||
| 176 | + | } | |||
| 177 | + | return r0, r1 | |||
| 178 | + | } | |||
| 179 | + | ||||
| 180 | + | // MockConfigMigrator_GetConfigFile_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetConfigFile' | |||
| 181 | + | type MockConfigMigrator_GetConfigFile_Call struct { | |||
| 182 | + | *mock.Call | |||
| 183 | + | } | |||
| 184 | + | ||||
| 185 | + | // GetConfigFile is a helper method to define mock.On call | |||
| 186 | + | func (_e *MockConfigMigrator_Expecter) GetConfigFile() *MockConfigMigrator_GetConfigFile_Call { | |||
| 187 | + | return &MockConfigMigrator_GetConfigFile_Call{Call: _e.mock.On("GetConfigFile")} | |||
| 188 | + | } | |||
| 189 | + | ||||
| 190 | + | func (_c *MockConfigMigrator_GetConfigFile_Call) Run(run func()) *MockConfigMigrator_GetConfigFile_Call { | |||
| 191 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 192 | + | run() | |||
| 193 | + | }) | |||
| 194 | + | return _c | |||
| 195 | + | } | |||
| 196 | + | ||||
| 197 | + | func (_c *MockConfigMigrator_GetConfigFile_Call) Return(s string, err error) *MockConfigMigrator_GetConfigFile_Call { | |||
| 198 | + | _c.Call.Return(s, err) | |||
| 199 | + | return _c | |||
| 200 | + | } | |||
| 201 | + | ||||
| 202 | + | func (_c *MockConfigMigrator_GetConfigFile_Call) RunAndReturn(run func() (string, error)) *MockConfigMigrator_GetConfigFile_Call { | |||
| 203 | + | _c.Call.Return(run) | |||
| 204 | + | return _c | |||
| 205 | + | } | |||
| 206 | + | ||||
| 154 | // GetKeyInfo provides a mock function for the type MockConfigMigrator | 207 | // GetKeyInfo provides a mock function for the type MockConfigMigrator | ||
| 155 | func (_mock *MockConfigMigrator) GetKeyInfo(key string) port.KeyInfo { | 208 | func (_mock *MockConfigMigrator) GetKeyInfo(key string) port.KeyInfo { | ||
| 156 | ret := _mock.Called(key) | 209 | ret := _mock.Called(key) | ||
M internal/application/usecase/migrate_config.go +12 -9
| @@ -126,17 +126,17 @@ func (uc *MigrateConfigUseCase) DetectChanges(ctx context.Context, _ DetectChang | |||
| 126 | 126 | }, nil | |
| 127 | 127 | } | |
| 128 | 128 | ||
| 129 | - | // Execute adds missing default keys to the user's config file. | |
| 129 | + | // Execute adds missing default keys and removes deprecated keys from the user's config file. | |
| 130 | 130 | func (uc *MigrateConfigUseCase) Execute(ctx context.Context, _ MigrateConfigInput) (*MigrateConfigOutput, error) { | |
| 131 | 131 | log := logging.FromContext(ctx) | |
| 132 | 132 | ||
| 133 | - | // First check if migration is needed | |
| 134 | - | checkResult, err := uc.migrator.CheckMigration() | |
| 133 | + | // Check for any changes (added, removed, or renamed keys) | |
| 134 | + | changes, err := uc.migrator.DetectChanges() | |
| 135 | 135 | if err != nil { | |
| 136 | 136 | return nil, err | |
| 137 | 137 | } | |
| 138 | 138 | ||
| 139 | - | if checkResult == nil || len(checkResult.MissingKeys) == 0 { | |
| 139 | + | if len(changes) == 0 { | |
| 140 | 140 | log.Debug().Msg("no migration needed") | |
| 141 | 141 | return &MigrateConfigOutput{ | |
| 142 | 142 | AddedKeys: nil, | |
| @@ -144,20 +144,23 @@ func (uc *MigrateConfigUseCase) Execute(ctx context.Context, _ MigrateConfigInpu | |||
| 144 | 144 | }, nil | |
| 145 | 145 | } | |
| 146 | 146 | ||
| 147 | + | // Get config file path for output | |
| 148 | + | configFile, _ := uc.migrator.GetConfigFile() | |
| 149 | + | ||
| 147 | 150 | // Perform migration | |
| 148 | - | addedKeys, err := uc.migrator.Migrate() | |
| 151 | + | appliedKeys, err := uc.migrator.Migrate() | |
| 149 | 152 | if err != nil { | |
| 150 | 153 | log.Error().Err(err).Msg("config migration failed") | |
| 151 | 154 | return nil, err | |
| 152 | 155 | } | |
| 153 | 156 | ||
| 154 | 157 | log.Info(). | |
| 155 | - | Int("added_keys", len(addedKeys)). | |
| 156 | - | Str("config_file", checkResult.ConfigFile). | |
| 158 | + | Int("applied_keys", len(appliedKeys)). | |
| 159 | + | Str("config_file", configFile). | |
| 157 | 160 | Msg("config migration completed") | |
| 158 | 161 | ||
| 159 | 162 | return &MigrateConfigOutput{ | |
| 160 | - | AddedKeys: addedKeys, | |
| 161 | - | ConfigFile: checkResult.ConfigFile, | |
| 163 | + | AddedKeys: appliedKeys, | |
| 164 | + | ConfigFile: configFile, | |
| 162 | 165 | }, nil | |
| 163 | 166 | } | |
| @@ -126,17 +126,17 @@ func (uc *MigrateConfigUseCase) DetectChanges(ctx context.Context, _ DetectChang | |||||
| 126 | }, nil | 126 | }, nil | ||
| 127 | } | 127 | } | ||
| 128 | 128 | ||||
| 129 | - | // Execute adds missing default keys to the user's config file. | 129 | + | // Execute adds missing default keys and removes deprecated keys from the user's config file. |
| 130 | func (uc *MigrateConfigUseCase) Execute(ctx context.Context, _ MigrateConfigInput) (*MigrateConfigOutput, error) { | 130 | func (uc *MigrateConfigUseCase) Execute(ctx context.Context, _ MigrateConfigInput) (*MigrateConfigOutput, error) { | ||
| 131 | log := logging.FromContext(ctx) | 131 | log := logging.FromContext(ctx) | ||
| 132 | 132 | ||||
| 133 | - | // First check if migration is needed | 133 | + | // Check for any changes (added, removed, or renamed keys) |
| 134 | - | checkResult, err := uc.migrator.CheckMigration() | 134 | + | changes, err := uc.migrator.DetectChanges() |
| 135 | if err != nil { | 135 | if err != nil { | ||
| 136 | return nil, err | 136 | return nil, err | ||
| 137 | } | 137 | } | ||
| 138 | 138 | ||||
| 139 | - | if checkResult == nil || len(checkResult.MissingKeys) == 0 { | 139 | + | if len(changes) == 0 { |
| 140 | log.Debug().Msg("no migration needed") | 140 | log.Debug().Msg("no migration needed") | ||
| 141 | return &MigrateConfigOutput{ | 141 | return &MigrateConfigOutput{ | ||
| 142 | AddedKeys: nil, | 142 | AddedKeys: nil, | ||
| @@ -144,20 +144,23 @@ func (uc *MigrateConfigUseCase) Execute(ctx context.Context, _ MigrateConfigInpu | |||||
| 144 | }, nil | 144 | }, nil | ||
| 145 | } | 145 | } | ||
| 146 | 146 | ||||
| 147 | + | // Get config file path for output | |||
| 148 | + | configFile, _ := uc.migrator.GetConfigFile() | |||
| 149 | + | ||||
| 147 | // Perform migration | 150 | // Perform migration | ||
| 148 | - | addedKeys, err := uc.migrator.Migrate() | 151 | + | appliedKeys, err := uc.migrator.Migrate() |
| 149 | if err != nil { | 152 | if err != nil { | ||
| 150 | log.Error().Err(err).Msg("config migration failed") | 153 | log.Error().Err(err).Msg("config migration failed") | ||
| 151 | return nil, err | 154 | return nil, err | ||
| 152 | } | 155 | } | ||
| 153 | 156 | ||||
| 154 | log.Info(). | 157 | log.Info(). | ||
| 155 | - | Int("added_keys", len(addedKeys)). | 158 | + | Int("applied_keys", len(appliedKeys)). |
| 156 | - | Str("config_file", checkResult.ConfigFile). | 159 | + | Str("config_file", configFile). |
| 157 | Msg("config migration completed") | 160 | Msg("config migration completed") | ||
| 158 | 161 | ||||
| 159 | return &MigrateConfigOutput{ | 162 | return &MigrateConfigOutput{ | ||
| 160 | - | AddedKeys: addedKeys, | 163 | + | AddedKeys: appliedKeys, |
| 161 | - | ConfigFile: checkResult.ConfigFile, | 164 | + | ConfigFile: configFile, |
| 162 | }, nil | 165 | }, nil | ||
| 163 | } | 166 | } | ||
M internal/application/usecase/migrate_config_test.go +13 -12
| @@ -86,7 +86,7 @@ func TestMigrateConfigUseCase_Check_Error(t *testing.T) { | |||
| 86 | 86 | func TestMigrateConfigUseCase_Execute_NoMigrationNeeded(t *testing.T) { | |
| 87 | 87 | mockMigrator := mocks.NewMockConfigMigrator(t) | |
| 88 | 88 | mockFormatter := mocks.NewMockDiffFormatter(t) | |
| 89 | - | mockMigrator.EXPECT().CheckMigration().Return(nil, nil) | |
| 89 | + | mockMigrator.EXPECT().DetectChanges().Return(nil, nil) | |
| 90 | 90 | ||
| 91 | 91 | uc := NewMigrateConfigUseCase(mockMigrator, mockFormatter) | |
| 92 | 92 | ctx := context.Background() | |
| @@ -101,11 +101,12 @@ func TestMigrateConfigUseCase_Execute_Success(t *testing.T) { | |||
| 101 | 101 | mockMigrator := mocks.NewMockConfigMigrator(t) | |
| 102 | 102 | mockFormatter := mocks.NewMockDiffFormatter(t) | |
| 103 | 103 | ||
| 104 | - | migrationResult := &port.MigrationResult{ | |
| 105 | - | MissingKeys: []string{"key1", "key2"}, | |
| 106 | - | ConfigFile: "/path/to/config.toml", | |
| 104 | + | changes := []port.KeyChange{ | |
| 105 | + | {Type: port.KeyChangeAdded, NewKey: "key1"}, | |
| 106 | + | {Type: port.KeyChangeAdded, NewKey: "key2"}, | |
| 107 | 107 | } | |
| 108 | - | mockMigrator.EXPECT().CheckMigration().Return(migrationResult, nil) | |
| 108 | + | mockMigrator.EXPECT().DetectChanges().Return(changes, nil) | |
| 109 | + | mockMigrator.EXPECT().GetConfigFile().Return("/path/to/config.toml", nil) | |
| 109 | 110 | mockMigrator.EXPECT().Migrate().Return([]string{"key1", "key2"}, nil) | |
| 110 | 111 | ||
| 111 | 112 | uc := NewMigrateConfigUseCase(mockMigrator, mockFormatter) | |
| @@ -122,11 +123,11 @@ func TestMigrateConfigUseCase_Execute_MigrateError(t *testing.T) { | |||
| 122 | 123 | mockMigrator := mocks.NewMockConfigMigrator(t) | |
| 123 | 124 | mockFormatter := mocks.NewMockDiffFormatter(t) | |
| 124 | 125 | ||
| 125 | - | migrationResult := &port.MigrationResult{ | |
| 126 | - | MissingKeys: []string{"key1"}, | |
| 127 | - | ConfigFile: "/path/to/config.toml", | |
| 126 | + | changes := []port.KeyChange{ | |
| 127 | + | {Type: port.KeyChangeAdded, NewKey: "key1"}, | |
| 128 | 128 | } | |
| 129 | - | mockMigrator.EXPECT().CheckMigration().Return(migrationResult, nil) | |
| 129 | + | mockMigrator.EXPECT().DetectChanges().Return(changes, nil) | |
| 130 | + | mockMigrator.EXPECT().GetConfigFile().Return("/path/to/config.toml", nil) | |
| 130 | 131 | ||
| 131 | 132 | expectedErr := errors.New("migrate failed") | |
| 132 | 133 | mockMigrator.EXPECT().Migrate().Return(nil, expectedErr) | |
| @@ -141,12 +142,12 @@ func TestMigrateConfigUseCase_Execute_MigrateError(t *testing.T) { | |||
| 141 | 142 | assert.Equal(t, expectedErr, err) | |
| 142 | 143 | } | |
| 143 | 144 | ||
| 144 | - | func TestMigrateConfigUseCase_Execute_CheckError(t *testing.T) { | |
| 145 | + | func TestMigrateConfigUseCase_Execute_DetectChangesError(t *testing.T) { | |
| 145 | 146 | mockMigrator := mocks.NewMockConfigMigrator(t) | |
| 146 | 147 | mockFormatter := mocks.NewMockDiffFormatter(t) | |
| 147 | 148 | ||
| 148 | - | expectedErr := errors.New("check failed") | |
| 149 | - | mockMigrator.EXPECT().CheckMigration().Return(nil, expectedErr) | |
| 149 | + | expectedErr := errors.New("detect changes failed") | |
| 150 | + | mockMigrator.EXPECT().DetectChanges().Return(nil, expectedErr) | |
| 150 | 151 | ||
| 151 | 152 | uc := NewMigrateConfigUseCase(mockMigrator, mockFormatter) | |
| 152 | 153 | ctx := context.Background() | |
| @@ -86,7 +86,7 @@ func TestMigrateConfigUseCase_Check_Error(t *testing.T) { | |||||
| 86 | func TestMigrateConfigUseCase_Execute_NoMigrationNeeded(t *testing.T) { | 86 | func TestMigrateConfigUseCase_Execute_NoMigrationNeeded(t *testing.T) { | ||
| 87 | mockMigrator := mocks.NewMockConfigMigrator(t) | 87 | mockMigrator := mocks.NewMockConfigMigrator(t) | ||
| 88 | mockFormatter := mocks.NewMockDiffFormatter(t) | 88 | mockFormatter := mocks.NewMockDiffFormatter(t) | ||
| 89 | - | mockMigrator.EXPECT().CheckMigration().Return(nil, nil) | 89 | + | mockMigrator.EXPECT().DetectChanges().Return(nil, nil) |
| 90 | 90 | ||||
| 91 | uc := NewMigrateConfigUseCase(mockMigrator, mockFormatter) | 91 | uc := NewMigrateConfigUseCase(mockMigrator, mockFormatter) | ||
| 92 | ctx := context.Background() | 92 | ctx := context.Background() | ||
| @@ -101,11 +101,12 @@ func TestMigrateConfigUseCase_Execute_Success(t *testing.T) { | |||||
| 101 | mockMigrator := mocks.NewMockConfigMigrator(t) | 101 | mockMigrator := mocks.NewMockConfigMigrator(t) | ||
| 102 | mockFormatter := mocks.NewMockDiffFormatter(t) | 102 | mockFormatter := mocks.NewMockDiffFormatter(t) | ||
| 103 | 103 | ||||
| 104 | - | migrationResult := &port.MigrationResult{ | 104 | + | changes := []port.KeyChange{ |
| 105 | - | MissingKeys: []string{"key1", "key2"}, | 105 | + | {Type: port.KeyChangeAdded, NewKey: "key1"}, |
| 106 | - | ConfigFile: "/path/to/config.toml", | 106 | + | {Type: port.KeyChangeAdded, NewKey: "key2"}, |
| 107 | } | 107 | } | ||
| 108 | - | mockMigrator.EXPECT().CheckMigration().Return(migrationResult, nil) | 108 | + | mockMigrator.EXPECT().DetectChanges().Return(changes, nil) |
| 109 | + | mockMigrator.EXPECT().GetConfigFile().Return("/path/to/config.toml", nil) | |||
| 109 | mockMigrator.EXPECT().Migrate().Return([]string{"key1", "key2"}, nil) | 110 | mockMigrator.EXPECT().Migrate().Return([]string{"key1", "key2"}, nil) | ||
| 110 | 111 | ||||
| 111 | uc := NewMigrateConfigUseCase(mockMigrator, mockFormatter) | 112 | uc := NewMigrateConfigUseCase(mockMigrator, mockFormatter) | ||
| @@ -122,11 +123,11 @@ func TestMigrateConfigUseCase_Execute_MigrateError(t *testing.T) { | |||||
| 122 | mockMigrator := mocks.NewMockConfigMigrator(t) | 123 | mockMigrator := mocks.NewMockConfigMigrator(t) | ||
| 123 | mockFormatter := mocks.NewMockDiffFormatter(t) | 124 | mockFormatter := mocks.NewMockDiffFormatter(t) | ||
| 124 | 125 | ||||
| 125 | - | migrationResult := &port.MigrationResult{ | 126 | + | changes := []port.KeyChange{ |
| 126 | - | MissingKeys: []string{"key1"}, | 127 | + | {Type: port.KeyChangeAdded, NewKey: "key1"}, |
| 127 | - | ConfigFile: "/path/to/config.toml", | |||
| 128 | } | 128 | } | ||
| 129 | - | mockMigrator.EXPECT().CheckMigration().Return(migrationResult, nil) | 129 | + | mockMigrator.EXPECT().DetectChanges().Return(changes, nil) |
| 130 | + | mockMigrator.EXPECT().GetConfigFile().Return("/path/to/config.toml", nil) | |||
| 130 | 131 | ||||
| 131 | expectedErr := errors.New("migrate failed") | 132 | expectedErr := errors.New("migrate failed") | ||
| 132 | mockMigrator.EXPECT().Migrate().Return(nil, expectedErr) | 133 | mockMigrator.EXPECT().Migrate().Return(nil, expectedErr) | ||
| @@ -141,12 +142,12 @@ func TestMigrateConfigUseCase_Execute_MigrateError(t *testing.T) { | |||||
| 141 | assert.Equal(t, expectedErr, err) | 142 | assert.Equal(t, expectedErr, err) | ||
| 142 | } | 143 | } | ||
| 143 | 144 | ||||
| 144 | - | func TestMigrateConfigUseCase_Execute_CheckError(t *testing.T) { | 145 | + | func TestMigrateConfigUseCase_Execute_DetectChangesError(t *testing.T) { |
| 145 | mockMigrator := mocks.NewMockConfigMigrator(t) | 146 | mockMigrator := mocks.NewMockConfigMigrator(t) | ||
| 146 | mockFormatter := mocks.NewMockDiffFormatter(t) | 147 | mockFormatter := mocks.NewMockDiffFormatter(t) | ||
| 147 | 148 | ||||
| 148 | - | expectedErr := errors.New("check failed") | 149 | + | expectedErr := errors.New("detect changes failed") |
| 149 | - | mockMigrator.EXPECT().CheckMigration().Return(nil, expectedErr) | 150 | + | mockMigrator.EXPECT().DetectChanges().Return(nil, expectedErr) |
| 150 | 151 | ||||
| 151 | uc := NewMigrateConfigUseCase(mockMigrator, mockFormatter) | 152 | uc := NewMigrateConfigUseCase(mockMigrator, mockFormatter) | ||
| 152 | ctx := context.Background() | 153 | ctx := context.Background() | ||
M internal/infrastructure/config/migrate.go +197 -21
| @@ -1,14 +1,17 @@ | |||
| 1 | 1 | package config | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "encoding/json" | |
| 4 | 5 | "fmt" | |
| 5 | 6 | "os" | |
| 7 | + | "path/filepath" | |
| 6 | 8 | "reflect" | |
| 7 | 9 | "sort" | |
| 8 | 10 | "strings" | |
| 9 | 11 | ||
| 10 | 12 | "github.com/pelletier/go-toml/v2" | |
| 11 | 13 | "github.com/spf13/viper" | |
| 14 | + | "gopkg.in/yaml.v3" | |
| 12 | 15 | ||
| 13 | 16 | "github.com/bnema/dumber/internal/application/port" | |
| 14 | 17 | ) | |
| @@ -252,6 +255,13 @@ const ( | |||
| 252 | 255 | typeNameString = "string" | |
| 253 | 256 | ) | |
| 254 | 257 | ||
| 258 | + | // Config format constants. | |
| 259 | + | const ( | |
| 260 | + | configFormatTOML = "toml" | |
| 261 | + | configFormatYAML = "yaml" | |
| 262 | + | configFormatJSON = "json" | |
| 263 | + | ) | |
| 264 | + | ||
| 255 | 265 | // typesAreCompatible checks if the default types for two keys are compatible. | |
| 256 | 266 | // This prevents matching int keys with string keys during rename detection. | |
| 257 | 267 | func (m *Migrator) typesAreCompatible(oldKey, newKey string) bool { | |
| @@ -339,7 +349,7 @@ func keysAreSimilar(oldKey, newKey string) bool { | |||
| 339 | 349 | return matches >= minTokens-1 && matches > 0 | |
| 340 | 350 | } | |
| 341 | 351 | ||
| 342 | - | // Migrate adds missing default keys to the user's config file. | |
| 352 | + | // Migrate adds missing default keys to the user's config file and removes deprecated keys. | |
| 343 | 353 | func (m *Migrator) Migrate() ([]string, error) { | |
| 344 | 354 | changes, err := m.DetectChanges() | |
| 345 | 355 | if err != nil { | |
| @@ -355,39 +365,57 @@ func (m *Migrator) Migrate() ([]string, error) { | |||
| 355 | 365 | return nil, fmt.Errorf("failed to get config file path: %w", err) | |
| 356 | 366 | } | |
| 357 | 367 | ||
| 358 | - | // Create a new Viper instance to read user config | |
| 359 | - | userViper := viper.New() | |
| 360 | - | userViper.SetConfigFile(configFile) | |
| 361 | - | userViper.SetConfigType("toml") | |
| 362 | - | ||
| 363 | - | // Set all defaults first | |
| 364 | - | mgr := &Manager{viper: userViper} | |
| 365 | - | mgr.setDefaults() | |
| 366 | - | ||
| 367 | - | // Read existing config (this merges with defaults) | |
| 368 | - | if err := userViper.ReadInConfig(); err != nil { | |
| 368 | + | // Read the raw config file into a nested map | |
| 369 | + | rawConfig, err := m.readRawConfig(configFile) | |
| 370 | + | if err != nil { | |
| 369 | 371 | return nil, fmt.Errorf("failed to read config file: %w", err) | |
| 370 | 372 | } | |
| 371 | 373 | ||
| 372 | - | // Apply changes | |
| 374 | + | // Build sets of keys to remove and renames to apply | |
| 375 | + | keysToRemove := make(map[string]bool) | |
| 376 | + | renames := make(map[string]string) // oldKey -> newKey | |
| 377 | + | ||
| 373 | 378 | var appliedKeys []string | |
| 374 | 379 | for _, change := range changes { | |
| 375 | 380 | switch change.Type { | |
| 376 | 381 | case port.KeyChangeRenamed: | |
| 377 | - | // Copy value from old key to new key | |
| 378 | - | if userViper.IsSet(change.OldKey) { | |
| 379 | - | userViper.Set(change.NewKey, userViper.Get(change.OldKey)) | |
| 380 | - | appliedKeys = append(appliedKeys, fmt.Sprintf("%s -> %s", change.OldKey, change.NewKey)) | |
| 381 | - | } | |
| 382 | + | renames[change.OldKey] = change.NewKey | |
| 383 | + | keysToRemove[change.OldKey] = true | |
| 384 | + | appliedKeys = append(appliedKeys, fmt.Sprintf("%s -> %s", change.OldKey, change.NewKey)) | |
| 382 | 385 | case port.KeyChangeAdded: | |
| 383 | - | // New key is already set via defaults, just track it | |
| 384 | 386 | appliedKeys = append(appliedKeys, change.NewKey) | |
| 385 | 387 | case port.KeyChangeRemoved: | |
| 386 | - | // Nothing to do - old key will be preserved but not used | |
| 387 | - | appliedKeys = append(appliedKeys, fmt.Sprintf("(deprecated: %s)", change.OldKey)) | |
| 388 | + | keysToRemove[change.OldKey] = true | |
| 389 | + | appliedKeys = append(appliedKeys, fmt.Sprintf("(removed: %s)", change.OldKey)) | |
| 390 | + | } | |
| 391 | + | } | |
| 392 | + | ||
| 393 | + | // Apply renames: copy value from old key to new key before removing old key | |
| 394 | + | for oldKey, newKey := range renames { | |
| 395 | + | if value := m.getNestedValue(rawConfig, oldKey); value != nil { | |
| 396 | + | m.setNestedValue(rawConfig, newKey, value) | |
| 388 | 397 | } | |
| 389 | 398 | } | |
| 390 | 399 | ||
| 400 | + | // Remove deprecated and renamed keys from the raw config | |
| 401 | + | for key := range keysToRemove { | |
| 402 | + | m.deleteNestedKey(rawConfig, key) | |
| 403 | + | } | |
| 404 | + | ||
| 405 | + | // Create a new Viper instance with defaults for added keys | |
| 406 | + | userViper := viper.New() | |
| 407 | + | userViper.SetConfigFile(configFile) | |
| 408 | + | userViper.SetConfigType(m.getConfigType(configFile)) | |
| 409 | + | ||
| 410 | + | // Set all defaults first | |
| 411 | + | mgr := &Manager{viper: userViper} | |
| 412 | + | mgr.setDefaults() | |
| 413 | + | ||
| 414 | + | // Merge the cleaned raw config on top of defaults | |
| 415 | + | if err := userViper.MergeConfigMap(rawConfig); err != nil { | |
| 416 | + | return nil, fmt.Errorf("failed to merge config: %w", err) | |
| 417 | + | } | |
| 418 | + | ||
| 391 | 419 | // Write the merged config back | |
| 392 | 420 | if err := userViper.WriteConfig(); err != nil { | |
| 393 | 421 | return nil, fmt.Errorf("failed to write config file: %w", err) | |
| @@ -396,6 +424,149 @@ func (m *Migrator) Migrate() ([]string, error) { | |||
| 396 | 424 | return appliedKeys, nil | |
| 397 | 425 | } | |
| 398 | 426 | ||
| 427 | + | // getConfigType returns the config type based on file extension. | |
| 428 | + | func (*Migrator) getConfigType(configFile string) string { | |
| 429 | + | ext := strings.TrimPrefix(filepath.Ext(configFile), ".") | |
| 430 | + | switch ext { | |
| 431 | + | case configFormatYAML, "yml": | |
| 432 | + | return configFormatYAML | |
| 433 | + | case configFormatJSON: | |
| 434 | + | return configFormatJSON | |
| 435 | + | case configFormatTOML: | |
| 436 | + | return configFormatTOML | |
| 437 | + | default: | |
| 438 | + | return configFormatTOML // default to TOML | |
| 439 | + | } | |
| 440 | + | } | |
| 441 | + | ||
| 442 | + | // readRawConfig reads the config file into a nested map without applying defaults. | |
| 443 | + | func (m *Migrator) readRawConfig(configFile string) (map[string]any, error) { | |
| 444 | + | data, err := os.ReadFile(configFile) | |
| 445 | + | if err != nil { | |
| 446 | + | return nil, err | |
| 447 | + | } | |
| 448 | + | ||
| 449 | + | var rawConfig map[string]any | |
| 450 | + | configType := m.getConfigType(configFile) | |
| 451 | + | ||
| 452 | + | switch configType { | |
| 453 | + | case configFormatTOML: | |
| 454 | + | if err := toml.Unmarshal(data, &rawConfig); err != nil { | |
| 455 | + | return nil, fmt.Errorf("failed to parse TOML: %w", err) | |
| 456 | + | } | |
| 457 | + | case configFormatYAML: | |
| 458 | + | if err := yaml.Unmarshal(data, &rawConfig); err != nil { | |
| 459 | + | return nil, fmt.Errorf("failed to parse YAML: %w", err) | |
| 460 | + | } | |
| 461 | + | case configFormatJSON: | |
| 462 | + | if err := json.Unmarshal(data, &rawConfig); err != nil { | |
| 463 | + | return nil, fmt.Errorf("failed to parse JSON: %w", err) | |
| 464 | + | } | |
| 465 | + | default: | |
| 466 | + | return nil, fmt.Errorf("unsupported config type: %s", configType) | |
| 467 | + | } | |
| 468 | + | ||
| 469 | + | return rawConfig, nil | |
| 470 | + | } | |
| 471 | + | ||
| 472 | + | // getNestedValue retrieves a value from a nested map using dot-notation key. | |
| 473 | + | func (*Migrator) getNestedValue(m map[string]any, key string) any { | |
| 474 | + | parts := strings.Split(key, ".") | |
| 475 | + | current := m | |
| 476 | + | ||
| 477 | + | for i, part := range parts { | |
| 478 | + | val, ok := current[part] | |
| 479 | + | if !ok { | |
| 480 | + | return nil | |
| 481 | + | } | |
| 482 | + | ||
| 483 | + | if i == len(parts)-1 { | |
| 484 | + | return val | |
| 485 | + | } | |
| 486 | + | ||
| 487 | + | nested, ok := val.(map[string]any) | |
| 488 | + | if !ok { | |
| 489 | + | return nil | |
| 490 | + | } | |
| 491 | + | current = nested | |
| 492 | + | } | |
| 493 | + | return nil | |
| 494 | + | } | |
| 495 | + | ||
| 496 | + | // setNestedValue sets a value in a nested map using dot-notation key. | |
| 497 | + | func (*Migrator) setNestedValue(m map[string]any, key string, value any) { | |
| 498 | + | parts := strings.Split(key, ".") | |
| 499 | + | current := m | |
| 500 | + | ||
| 501 | + | for i, part := range parts { | |
| 502 | + | if i == len(parts)-1 { | |
| 503 | + | current[part] = value | |
| 504 | + | return | |
| 505 | + | } | |
| 506 | + | ||
| 507 | + | val, ok := current[part] | |
| 508 | + | if !ok { | |
| 509 | + | // Create intermediate map | |
| 510 | + | newMap := make(map[string]any) | |
| 511 | + | current[part] = newMap | |
| 512 | + | current = newMap | |
| 513 | + | continue | |
| 514 | + | } | |
| 515 | + | ||
| 516 | + | nested, ok := val.(map[string]any) | |
| 517 | + | if !ok { | |
| 518 | + | // Overwrite non-map value with a map | |
| 519 | + | newMap := make(map[string]any) | |
| 520 | + | current[part] = newMap | |
| 521 | + | current = newMap | |
| 522 | + | continue | |
| 523 | + | } | |
| 524 | + | current = nested | |
| 525 | + | } | |
| 526 | + | } | |
| 527 | + | ||
| 528 | + | // deleteNestedKey removes a key from a nested map using dot-notation key. | |
| 529 | + | // It also cleans up empty parent maps. | |
| 530 | + | func (m *Migrator) deleteNestedKey(config map[string]any, key string) { | |
| 531 | + | parts := strings.Split(key, ".") | |
| 532 | + | m.deleteNestedKeyRecursive(config, parts) | |
| 533 | + | } | |
| 534 | + | ||
| 535 | + | // deleteNestedKeyRecursive recursively deletes a key and cleans up empty parents. | |
| 536 | + | func (m *Migrator) deleteNestedKeyRecursive(current map[string]any, parts []string) bool { | |
| 537 | + | if len(parts) == 0 { | |
| 538 | + | return false | |
| 539 | + | } | |
| 540 | + | ||
| 541 | + | part := parts[0] | |
| 542 | + | val, ok := current[part] | |
| 543 | + | if !ok { | |
| 544 | + | return false | |
| 545 | + | } | |
| 546 | + | ||
| 547 | + | if len(parts) == 1 { | |
| 548 | + | // This is the leaf key, delete it | |
| 549 | + | delete(current, part) | |
| 550 | + | return true | |
| 551 | + | } | |
| 552 | + | ||
| 553 | + | // Navigate deeper | |
| 554 | + | nested, ok := val.(map[string]any) | |
| 555 | + | if !ok { | |
| 556 | + | return false | |
| 557 | + | } | |
| 558 | + | ||
| 559 | + | // Recursively delete in the nested map | |
| 560 | + | deleted := m.deleteNestedKeyRecursive(nested, parts[1:]) | |
| 561 | + | ||
| 562 | + | // If the nested map is now empty, remove it too | |
| 563 | + | if deleted && len(nested) == 0 { | |
| 564 | + | delete(current, part) | |
| 565 | + | } | |
| 566 | + | ||
| 567 | + | return deleted | |
| 568 | + | } | |
| 569 | + | ||
| 399 | 570 | // GetKeyInfo returns detailed information about a config key. | |
| 400 | 571 | func (m *Migrator) GetKeyInfo(key string) port.KeyInfo { | |
| 401 | 572 | value := m.defaultViper.Get(key) | |
| @@ -414,6 +585,11 @@ func (m *Migrator) GetKeyInfo(key string) port.KeyInfo { | |||
| 414 | 585 | } | |
| 415 | 586 | } | |
| 416 | 587 | ||
| 588 | + | // GetConfigFile returns the path to the user's config file. | |
| 589 | + | func (*Migrator) GetConfigFile() (string, error) { | |
| 590 | + | return GetConfigFile() | |
| 591 | + | } | |
| 592 | + | ||
| 417 | 593 | // getAllDefaultKeys returns all keys from the default configuration. | |
| 418 | 594 | func (m *Migrator) getAllDefaultKeys() []string { | |
| 419 | 595 | keys := m.defaultViper.AllKeys() | |
| @@ -1,14 +1,17 @@ | |||||
| 1 | package config | 1 | package config | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "encoding/json" | |||
| 4 | "fmt" | 5 | "fmt" | ||
| 5 | "os" | 6 | "os" | ||
| 7 | + | "path/filepath" | |||
| 6 | "reflect" | 8 | "reflect" | ||
| 7 | "sort" | 9 | "sort" | ||
| 8 | "strings" | 10 | "strings" | ||
| 9 | 11 | ||||
| 10 | "github.com/pelletier/go-toml/v2" | 12 | "github.com/pelletier/go-toml/v2" | ||
| 11 | "github.com/spf13/viper" | 13 | "github.com/spf13/viper" | ||
| 14 | + | "gopkg.in/yaml.v3" | |||
| 12 | 15 | ||||
| 13 | "github.com/bnema/dumber/internal/application/port" | 16 | "github.com/bnema/dumber/internal/application/port" | ||
| 14 | ) | 17 | ) | ||
| @@ -252,6 +255,13 @@ const ( | |||||
| 252 | typeNameString = "string" | 255 | typeNameString = "string" | ||
| 253 | ) | 256 | ) | ||
| 254 | 257 | ||||
| 258 | + | // Config format constants. | |||
| 259 | + | const ( | |||
| 260 | + | configFormatTOML = "toml" | |||
| 261 | + | configFormatYAML = "yaml" | |||
| 262 | + | configFormatJSON = "json" | |||
| 263 | + | ) | |||
| 264 | + | ||||
| 255 | // typesAreCompatible checks if the default types for two keys are compatible. | 265 | // typesAreCompatible checks if the default types for two keys are compatible. | ||
| 256 | // This prevents matching int keys with string keys during rename detection. | 266 | // This prevents matching int keys with string keys during rename detection. | ||
| 257 | func (m *Migrator) typesAreCompatible(oldKey, newKey string) bool { | 267 | func (m *Migrator) typesAreCompatible(oldKey, newKey string) bool { | ||
| @@ -339,7 +349,7 @@ func keysAreSimilar(oldKey, newKey string) bool { | |||||
| 339 | return matches >= minTokens-1 && matches > 0 | 349 | return matches >= minTokens-1 && matches > 0 | ||
| 340 | } | 350 | } | ||
| 341 | 351 | ||||
| 342 | - | // Migrate adds missing default keys to the user's config file. | 352 | + | // Migrate adds missing default keys to the user's config file and removes deprecated keys. |
| 343 | func (m *Migrator) Migrate() ([]string, error) { | 353 | func (m *Migrator) Migrate() ([]string, error) { | ||
| 344 | changes, err := m.DetectChanges() | 354 | changes, err := m.DetectChanges() | ||
| 345 | if err != nil { | 355 | if err != nil { | ||
| @@ -355,39 +365,57 @@ func (m *Migrator) Migrate() ([]string, error) { | |||||
| 355 | return nil, fmt.Errorf("failed to get config file path: %w", err) | 365 | return nil, fmt.Errorf("failed to get config file path: %w", err) | ||
| 356 | } | 366 | } | ||
| 357 | 367 | ||||
| 358 | - | // Create a new Viper instance to read user config | 368 | + | // Read the raw config file into a nested map |
| 359 | - | userViper := viper.New() | 369 | + | rawConfig, err := m.readRawConfig(configFile) |
| 360 | - | userViper.SetConfigFile(configFile) | 370 | + | if err != nil { |
| 361 | - | userViper.SetConfigType("toml") | |||
| 362 | - | ||||
| 363 | - | // Set all defaults first | |||
| 364 | - | mgr := &Manager{viper: userViper} | |||
| 365 | - | mgr.setDefaults() | |||
| 366 | - | ||||
| 367 | - | // Read existing config (this merges with defaults) | |||
| 368 | - | if err := userViper.ReadInConfig(); err != nil { | |||
| 369 | return nil, fmt.Errorf("failed to read config file: %w", err) | 371 | return nil, fmt.Errorf("failed to read config file: %w", err) | ||
| 370 | } | 372 | } | ||
| 371 | 373 | ||||
| 372 | - | // Apply changes | 374 | + | // Build sets of keys to remove and renames to apply |
| 375 | + | keysToRemove := make(map[string]bool) | |||
| 376 | + | renames := make(map[string]string) // oldKey -> newKey | |||
| 377 | + | ||||
| 373 | var appliedKeys []string | 378 | var appliedKeys []string | ||
| 374 | for _, change := range changes { | 379 | for _, change := range changes { | ||
| 375 | switch change.Type { | 380 | switch change.Type { | ||
| 376 | case port.KeyChangeRenamed: | 381 | case port.KeyChangeRenamed: | ||
| 377 | - | // Copy value from old key to new key | 382 | + | renames[change.OldKey] = change.NewKey |
| 378 | - | if userViper.IsSet(change.OldKey) { | 383 | + | keysToRemove[change.OldKey] = true |
| 379 | - | userViper.Set(change.NewKey, userViper.Get(change.OldKey)) | 384 | + | appliedKeys = append(appliedKeys, fmt.Sprintf("%s -> %s", change.OldKey, change.NewKey)) |
| 380 | - | appliedKeys = append(appliedKeys, fmt.Sprintf("%s -> %s", change.OldKey, change.NewKey)) | |||
| 381 | - | } | |||
| 382 | case port.KeyChangeAdded: | 385 | case port.KeyChangeAdded: | ||
| 383 | - | // New key is already set via defaults, just track it | |||
| 384 | appliedKeys = append(appliedKeys, change.NewKey) | 386 | appliedKeys = append(appliedKeys, change.NewKey) | ||
| 385 | case port.KeyChangeRemoved: | 387 | case port.KeyChangeRemoved: | ||
| 386 | - | // Nothing to do - old key will be preserved but not used | 388 | + | keysToRemove[change.OldKey] = true |
| 387 | - | appliedKeys = append(appliedKeys, fmt.Sprintf("(deprecated: %s)", change.OldKey)) | 389 | + | appliedKeys = append(appliedKeys, fmt.Sprintf("(removed: %s)", change.OldKey)) |
| 390 | + | } | |||
| 391 | + | } | |||
| 392 | + | ||||
| 393 | + | // Apply renames: copy value from old key to new key before removing old key | |||
| 394 | + | for oldKey, newKey := range renames { | |||
| 395 | + | if value := m.getNestedValue(rawConfig, oldKey); value != nil { | |||
| 396 | + | m.setNestedValue(rawConfig, newKey, value) | |||
| 388 | } | 397 | } | ||
| 389 | } | 398 | } | ||
| 390 | 399 | ||||
| 400 | + | // Remove deprecated and renamed keys from the raw config | |||
| 401 | + | for key := range keysToRemove { | |||
| 402 | + | m.deleteNestedKey(rawConfig, key) | |||
| 403 | + | } | |||
| 404 | + | ||||
| 405 | + | // Create a new Viper instance with defaults for added keys | |||
| 406 | + | userViper := viper.New() | |||
| 407 | + | userViper.SetConfigFile(configFile) | |||
| 408 | + | userViper.SetConfigType(m.getConfigType(configFile)) | |||
| 409 | + | ||||
| 410 | + | // Set all defaults first | |||
| 411 | + | mgr := &Manager{viper: userViper} | |||
| 412 | + | mgr.setDefaults() | |||
| 413 | + | ||||
| 414 | + | // Merge the cleaned raw config on top of defaults | |||
| 415 | + | if err := userViper.MergeConfigMap(rawConfig); err != nil { | |||
| 416 | + | return nil, fmt.Errorf("failed to merge config: %w", err) | |||
| 417 | + | } | |||
| 418 | + | ||||
| 391 | // Write the merged config back | 419 | // Write the merged config back | ||
| 392 | if err := userViper.WriteConfig(); err != nil { | 420 | if err := userViper.WriteConfig(); err != nil { | ||
| 393 | return nil, fmt.Errorf("failed to write config file: %w", err) | 421 | return nil, fmt.Errorf("failed to write config file: %w", err) | ||
| @@ -396,6 +424,149 @@ func (m *Migrator) Migrate() ([]string, error) { | |||||
| 396 | return appliedKeys, nil | 424 | return appliedKeys, nil | ||
| 397 | } | 425 | } | ||
| 398 | 426 | ||||
| 427 | + | // getConfigType returns the config type based on file extension. | |||
| 428 | + | func (*Migrator) getConfigType(configFile string) string { | |||
| 429 | + | ext := strings.TrimPrefix(filepath.Ext(configFile), ".") | |||
| 430 | + | switch ext { | |||
| 431 | + | case configFormatYAML, "yml": | |||
| 432 | + | return configFormatYAML | |||
| 433 | + | case configFormatJSON: | |||
| 434 | + | return configFormatJSON | |||
| 435 | + | case configFormatTOML: | |||
| 436 | + | return configFormatTOML | |||
| 437 | + | default: | |||
| 438 | + | return configFormatTOML // default to TOML | |||
| 439 | + | } | |||
| 440 | + | } | |||
| 441 | + | ||||
| 442 | + | // readRawConfig reads the config file into a nested map without applying defaults. | |||
| 443 | + | func (m *Migrator) readRawConfig(configFile string) (map[string]any, error) { | |||
| 444 | + | data, err := os.ReadFile(configFile) | |||
| 445 | + | if err != nil { | |||
| 446 | + | return nil, err | |||
| 447 | + | } | |||
| 448 | + | ||||
| 449 | + | var rawConfig map[string]any | |||
| 450 | + | configType := m.getConfigType(configFile) | |||
| 451 | + | ||||
| 452 | + | switch configType { | |||
| 453 | + | case configFormatTOML: | |||
| 454 | + | if err := toml.Unmarshal(data, &rawConfig); err != nil { | |||
| 455 | + | return nil, fmt.Errorf("failed to parse TOML: %w", err) | |||
| 456 | + | } | |||
| 457 | + | case configFormatYAML: | |||
| 458 | + | if err := yaml.Unmarshal(data, &rawConfig); err != nil { | |||
| 459 | + | return nil, fmt.Errorf("failed to parse YAML: %w", err) | |||
| 460 | + | } | |||
| 461 | + | case configFormatJSON: | |||
| 462 | + | if err := json.Unmarshal(data, &rawConfig); err != nil { | |||
| 463 | + | return nil, fmt.Errorf("failed to parse JSON: %w", err) | |||
| 464 | + | } | |||
| 465 | + | default: | |||
| 466 | + | return nil, fmt.Errorf("unsupported config type: %s", configType) | |||
| 467 | + | } | |||
| 468 | + | ||||
| 469 | + | return rawConfig, nil | |||
| 470 | + | } | |||
| 471 | + | ||||
| 472 | + | // getNestedValue retrieves a value from a nested map using dot-notation key. | |||
| 473 | + | func (*Migrator) getNestedValue(m map[string]any, key string) any { | |||
| 474 | + | parts := strings.Split(key, ".") | |||
| 475 | + | current := m | |||
| 476 | + | ||||
| 477 | + | for i, part := range parts { | |||
| 478 | + | val, ok := current[part] | |||
| 479 | + | if !ok { | |||
| 480 | + | return nil | |||
| 481 | + | } | |||
| 482 | + | ||||
| 483 | + | if i == len(parts)-1 { | |||
| 484 | + | return val | |||
| 485 | + | } | |||
| 486 | + | ||||
| 487 | + | nested, ok := val.(map[string]any) | |||
| 488 | + | if !ok { | |||
| 489 | + | return nil | |||
| 490 | + | } | |||
| 491 | + | current = nested | |||
| 492 | + | } | |||
| 493 | + | return nil | |||
| 494 | + | } | |||
| 495 | + | ||||
| 496 | + | // setNestedValue sets a value in a nested map using dot-notation key. | |||
| 497 | + | func (*Migrator) setNestedValue(m map[string]any, key string, value any) { | |||
| 498 | + | parts := strings.Split(key, ".") | |||
| 499 | + | current := m | |||
| 500 | + | ||||
| 501 | + | for i, part := range parts { | |||
| 502 | + | if i == len(parts)-1 { | |||
| 503 | + | current[part] = value | |||
| 504 | + | return | |||
| 505 | + | } | |||
| 506 | + | ||||
| 507 | + | val, ok := current[part] | |||
| 508 | + | if !ok { | |||
| 509 | + | // Create intermediate map | |||
| 510 | + | newMap := make(map[string]any) | |||
| 511 | + | current[part] = newMap | |||
| 512 | + | current = newMap | |||
| 513 | + | continue | |||
| 514 | + | } | |||
| 515 | + | ||||
| 516 | + | nested, ok := val.(map[string]any) | |||
| 517 | + | if !ok { | |||
| 518 | + | // Overwrite non-map value with a map | |||
| 519 | + | newMap := make(map[string]any) | |||
| 520 | + | current[part] = newMap | |||
| 521 | + | current = newMap | |||
| 522 | + | continue | |||
| 523 | + | } | |||
| 524 | + | current = nested | |||
| 525 | + | } | |||
| 526 | + | } | |||
| 527 | + | ||||
| 528 | + | // deleteNestedKey removes a key from a nested map using dot-notation key. | |||
| 529 | + | // It also cleans up empty parent maps. | |||
| 530 | + | func (m *Migrator) deleteNestedKey(config map[string]any, key string) { | |||
| 531 | + | parts := strings.Split(key, ".") | |||
| 532 | + | m.deleteNestedKeyRecursive(config, parts) | |||
| 533 | + | } | |||
| 534 | + | ||||
| 535 | + | // deleteNestedKeyRecursive recursively deletes a key and cleans up empty parents. | |||
| 536 | + | func (m *Migrator) deleteNestedKeyRecursive(current map[string]any, parts []string) bool { | |||
| 537 | + | if len(parts) == 0 { | |||
| 538 | + | return false | |||
| 539 | + | } | |||
| 540 | + | ||||
| 541 | + | part := parts[0] | |||
| 542 | + | val, ok := current[part] | |||
| 543 | + | if !ok { | |||
| 544 | + | return false | |||
| 545 | + | } | |||
| 546 | + | ||||
| 547 | + | if len(parts) == 1 { | |||
| 548 | + | // This is the leaf key, delete it | |||
| 549 | + | delete(current, part) | |||
| 550 | + | return true | |||
| 551 | + | } | |||
| 552 | + | ||||
| 553 | + | // Navigate deeper | |||
| 554 | + | nested, ok := val.(map[string]any) | |||
| 555 | + | if !ok { | |||
| 556 | + | return false | |||
| 557 | + | } | |||
| 558 | + | ||||
| 559 | + | // Recursively delete in the nested map | |||
| 560 | + | deleted := m.deleteNestedKeyRecursive(nested, parts[1:]) | |||
| 561 | + | ||||
| 562 | + | // If the nested map is now empty, remove it too | |||
| 563 | + | if deleted && len(nested) == 0 { | |||
| 564 | + | delete(current, part) | |||
| 565 | + | } | |||
| 566 | + | ||||
| 567 | + | return deleted | |||
| 568 | + | } | |||
| 569 | + | ||||
| 399 | // GetKeyInfo returns detailed information about a config key. | 570 | // GetKeyInfo returns detailed information about a config key. | ||
| 400 | func (m *Migrator) GetKeyInfo(key string) port.KeyInfo { | 571 | func (m *Migrator) GetKeyInfo(key string) port.KeyInfo { | ||
| 401 | value := m.defaultViper.Get(key) | 572 | value := m.defaultViper.Get(key) | ||
| @@ -414,6 +585,11 @@ func (m *Migrator) GetKeyInfo(key string) port.KeyInfo { | |||||
| 414 | } | 585 | } | ||
| 415 | } | 586 | } | ||
| 416 | 587 | ||||
| 588 | + | // GetConfigFile returns the path to the user's config file. | |||
| 589 | + | func (*Migrator) GetConfigFile() (string, error) { | |||
| 590 | + | return GetConfigFile() | |||
| 591 | + | } | |||
| 592 | + | ||||
| 417 | // getAllDefaultKeys returns all keys from the default configuration. | 593 | // getAllDefaultKeys returns all keys from the default configuration. | ||
| 418 | func (m *Migrator) getAllDefaultKeys() []string { | 594 | func (m *Migrator) getAllDefaultKeys() []string { | ||
| 419 | keys := m.defaultViper.AllKeys() | 595 | keys := m.defaultViper.AllKeys() | ||
M internal/infrastructure/config/migrate_test.go +245 -0
| @@ -628,3 +628,248 @@ func TestMigrator_Migrate_NoConfigFile(t *testing.T) { | |||
| 628 | 628 | // which is difficult without refactoring. Skip for now. | |
| 629 | 629 | t.Skip("requires config path mocking") | |
| 630 | 630 | } | |
| 631 | + | ||
| 632 | + | func TestMigrator_GetConfigType(t *testing.T) { | |
| 633 | + | m := NewMigrator() | |
| 634 | + | ||
| 635 | + | tests := []struct { | |
| 636 | + | name string | |
| 637 | + | configFile string | |
| 638 | + | expected string | |
| 639 | + | }{ | |
| 640 | + | { | |
| 641 | + | name: "toml file", | |
| 642 | + | configFile: "/path/to/config.toml", | |
| 643 | + | expected: "toml", | |
| 644 | + | }, | |
| 645 | + | { | |
| 646 | + | name: "yaml file", | |
| 647 | + | configFile: "/path/to/config.yaml", | |
| 648 | + | expected: "yaml", | |
| 649 | + | }, | |
| 650 | + | { | |
| 651 | + | name: "yml file", | |
| 652 | + | configFile: "/path/to/config.yml", | |
| 653 | + | expected: "yaml", | |
| 654 | + | }, | |
| 655 | + | { | |
| 656 | + | name: "json file", | |
| 657 | + | configFile: "/path/to/config.json", | |
| 658 | + | expected: "json", | |
| 659 | + | }, | |
| 660 | + | { | |
| 661 | + | name: "unknown extension defaults to toml", | |
| 662 | + | configFile: "/path/to/config.unknown", | |
| 663 | + | expected: "toml", | |
| 664 | + | }, | |
| 665 | + | { | |
| 666 | + | name: "no extension defaults to toml", | |
| 667 | + | configFile: "/path/to/config", | |
| 668 | + | expected: "toml", | |
| 669 | + | }, | |
| 670 | + | } | |
| 671 | + | ||
| 672 | + | for _, tt := range tests { | |
| 673 | + | t.Run(tt.name, func(t *testing.T) { | |
| 674 | + | result := m.getConfigType(tt.configFile) | |
| 675 | + | assert.Equal(t, tt.expected, result) | |
| 676 | + | }) | |
| 677 | + | } | |
| 678 | + | } | |
| 679 | + | ||
| 680 | + | func TestMigrator_GetNestedValue(t *testing.T) { | |
| 681 | + | m := NewMigrator() | |
| 682 | + | ||
| 683 | + | config := map[string]any{ | |
| 684 | + | "level1": map[string]any{ | |
| 685 | + | "level2": map[string]any{ | |
| 686 | + | "level3": "deep_value", | |
| 687 | + | }, | |
| 688 | + | "simple": "simple_value", | |
| 689 | + | }, | |
| 690 | + | "top": "top_value", | |
| 691 | + | } | |
| 692 | + | ||
| 693 | + | tests := []struct { | |
| 694 | + | name string | |
| 695 | + | key string | |
| 696 | + | expected any | |
| 697 | + | }{ | |
| 698 | + | { | |
| 699 | + | name: "top level key", | |
| 700 | + | key: "top", | |
| 701 | + | expected: "top_value", | |
| 702 | + | }, | |
| 703 | + | { | |
| 704 | + | name: "nested key", | |
| 705 | + | key: "level1.simple", | |
| 706 | + | expected: "simple_value", | |
| 707 | + | }, | |
| 708 | + | { | |
| 709 | + | name: "deeply nested key", | |
| 710 | + | key: "level1.level2.level3", | |
| 711 | + | expected: "deep_value", | |
| 712 | + | }, | |
| 713 | + | { | |
| 714 | + | name: "non-existent key", | |
| 715 | + | key: "nonexistent", | |
| 716 | + | expected: nil, | |
| 717 | + | }, | |
| 718 | + | { | |
| 719 | + | name: "non-existent nested key", | |
| 720 | + | key: "level1.nonexistent", | |
| 721 | + | expected: nil, | |
| 722 | + | }, | |
| 723 | + | } | |
| 724 | + | ||
| 725 | + | for _, tt := range tests { | |
| 726 | + | t.Run(tt.name, func(t *testing.T) { | |
| 727 | + | result := m.getNestedValue(config, tt.key) | |
| 728 | + | assert.Equal(t, tt.expected, result) | |
| 729 | + | }) | |
| 730 | + | } | |
| 731 | + | } | |
| 732 | + | ||
| 733 | + | func TestMigrator_SetNestedValue(t *testing.T) { | |
| 734 | + | m := NewMigrator() | |
| 735 | + | ||
| 736 | + | t.Run("set top level value", func(t *testing.T) { | |
| 737 | + | config := make(map[string]any) | |
| 738 | + | m.setNestedValue(config, "top", "value") | |
| 739 | + | assert.Equal(t, "value", config["top"]) | |
| 740 | + | }) | |
| 741 | + | ||
| 742 | + | t.Run("set nested value with existing parent", func(t *testing.T) { | |
| 743 | + | config := map[string]any{ | |
| 744 | + | "parent": map[string]any{}, | |
| 745 | + | } | |
| 746 | + | m.setNestedValue(config, "parent.child", "value") | |
| 747 | + | parent := config["parent"].(map[string]any) | |
| 748 | + | assert.Equal(t, "value", parent["child"]) | |
| 749 | + | }) | |
| 750 | + | ||
| 751 | + | t.Run("set nested value creates intermediate maps", func(t *testing.T) { | |
| 752 | + | config := make(map[string]any) | |
| 753 | + | m.setNestedValue(config, "a.b.c", "deep_value") | |
| 754 | + | ||
| 755 | + | a := config["a"].(map[string]any) | |
| 756 | + | b := a["b"].(map[string]any) | |
| 757 | + | assert.Equal(t, "deep_value", b["c"]) | |
| 758 | + | }) | |
| 759 | + | } | |
| 760 | + | ||
| 761 | + | func TestMigrator_DeleteNestedKey(t *testing.T) { | |
| 762 | + | m := NewMigrator() | |
| 763 | + | ||
| 764 | + | t.Run("delete top level key", func(t *testing.T) { | |
| 765 | + | config := map[string]any{ | |
| 766 | + | "keep": "value", | |
| 767 | + | "delete": "value", | |
| 768 | + | } | |
| 769 | + | m.deleteNestedKey(config, "delete") | |
| 770 | + | assert.Contains(t, config, "keep") | |
| 771 | + | assert.NotContains(t, config, "delete") | |
| 772 | + | }) | |
| 773 | + | ||
| 774 | + | t.Run("delete nested key", func(t *testing.T) { | |
| 775 | + | config := map[string]any{ | |
| 776 | + | "parent": map[string]any{ | |
| 777 | + | "keep": "value", | |
| 778 | + | "delete": "value", | |
| 779 | + | }, | |
| 780 | + | } | |
| 781 | + | m.deleteNestedKey(config, "parent.delete") | |
| 782 | + | parent := config["parent"].(map[string]any) | |
| 783 | + | assert.Contains(t, parent, "keep") | |
| 784 | + | assert.NotContains(t, parent, "delete") | |
| 785 | + | }) | |
| 786 | + | ||
| 787 | + | t.Run("delete nested key cleans up empty parents", func(t *testing.T) { | |
| 788 | + | config := map[string]any{ | |
| 789 | + | "keep": "value", | |
| 790 | + | "parent": map[string]any{ | |
| 791 | + | "child": map[string]any{ | |
| 792 | + | "leaf": "value", | |
| 793 | + | }, | |
| 794 | + | }, | |
| 795 | + | } | |
| 796 | + | m.deleteNestedKey(config, "parent.child.leaf") | |
| 797 | + | // Both parent and child should be removed since they're empty | |
| 798 | + | assert.NotContains(t, config, "parent") | |
| 799 | + | assert.Contains(t, config, "keep") | |
| 800 | + | }) | |
| 801 | + | ||
| 802 | + | t.Run("delete non-existent key is no-op", func(t *testing.T) { | |
| 803 | + | config := map[string]any{ | |
| 804 | + | "keep": "value", | |
| 805 | + | } | |
| 806 | + | m.deleteNestedKey(config, "nonexistent") | |
| 807 | + | assert.Contains(t, config, "keep") | |
| 808 | + | }) | |
| 809 | + | } | |
| 810 | + | ||
| 811 | + | func TestMigrator_ReadRawConfig(t *testing.T) { | |
| 812 | + | m := NewMigrator() | |
| 813 | + | ||
| 814 | + | t.Run("read TOML config", func(t *testing.T) { | |
| 815 | + | tmpDir := t.TempDir() | |
| 816 | + | configFile := filepath.Join(tmpDir, "config.toml") | |
| 817 | + | ||
| 818 | + | content := ` | |
| 819 | + | [section] | |
| 820 | + | key = "value" | |
| 821 | + | number = 42 | |
| 822 | + | ` | |
| 823 | + | err := os.WriteFile(configFile, []byte(content), 0o644) | |
| 824 | + | require.NoError(t, err) | |
| 825 | + | ||
| 826 | + | config, err := m.readRawConfig(configFile) | |
| 827 | + | require.NoError(t, err) | |
| 828 | + | ||
| 829 | + | section := config["section"].(map[string]any) | |
| 830 | + | assert.Equal(t, "value", section["key"]) | |
| 831 | + | assert.Equal(t, int64(42), section["number"]) | |
| 832 | + | }) | |
| 833 | + | ||
| 834 | + | t.Run("read YAML config", func(t *testing.T) { | |
| 835 | + | tmpDir := t.TempDir() | |
| 836 | + | configFile := filepath.Join(tmpDir, "config.yaml") | |
| 837 | + | ||
| 838 | + | content := ` | |
| 839 | + | section: | |
| 840 | + | key: value | |
| 841 | + | number: 42 | |
| 842 | + | ` | |
| 843 | + | err := os.WriteFile(configFile, []byte(content), 0o644) | |
| 844 | + | require.NoError(t, err) | |
| 845 | + | ||
| 846 | + | config, err := m.readRawConfig(configFile) | |
| 847 | + | require.NoError(t, err) | |
| 848 | + | ||
| 849 | + | section := config["section"].(map[string]any) | |
| 850 | + | assert.Equal(t, "value", section["key"]) | |
| 851 | + | assert.Equal(t, 42, section["number"]) | |
| 852 | + | }) | |
| 853 | + | ||
| 854 | + | t.Run("read JSON config", func(t *testing.T) { | |
| 855 | + | tmpDir := t.TempDir() | |
| 856 | + | configFile := filepath.Join(tmpDir, "config.json") | |
| 857 | + | ||
| 858 | + | content := `{ | |
| 859 | + | "section": { | |
| 860 | + | "key": "value", | |
| 861 | + | "number": 42 | |
| 862 | + | } | |
| 863 | + | }` | |
| 864 | + | err := os.WriteFile(configFile, []byte(content), 0o644) | |
| 865 | + | require.NoError(t, err) | |
| 866 | + | ||
| 867 | + | config, err := m.readRawConfig(configFile) | |
| 868 | + | require.NoError(t, err) | |
| 869 | + | ||
| 870 | + | section := config["section"].(map[string]any) | |
| 871 | + | assert.Equal(t, "value", section["key"]) | |
| 872 | + | // JSON numbers are float64 | |
| 873 | + | assert.InDelta(t, 42.0, section["number"].(float64), 0.001) | |
| 874 | + | }) | |
| 875 | + | } | |
| @@ -628,3 +628,248 @@ func TestMigrator_Migrate_NoConfigFile(t *testing.T) { | |||||
| 628 | // which is difficult without refactoring. Skip for now. | 628 | // which is difficult without refactoring. Skip for now. | ||
| 629 | t.Skip("requires config path mocking") | 629 | t.Skip("requires config path mocking") | ||
| 630 | } | 630 | } | ||
| 631 | + | ||||
| 632 | + | func TestMigrator_GetConfigType(t *testing.T) { | |||
| 633 | + | m := NewMigrator() | |||
| 634 | + | ||||
| 635 | + | tests := []struct { | |||
| 636 | + | name string | |||
| 637 | + | configFile string | |||
| 638 | + | expected string | |||
| 639 | + | }{ | |||
| 640 | + | { | |||
| 641 | + | name: "toml file", | |||
| 642 | + | configFile: "/path/to/config.toml", | |||
| 643 | + | expected: "toml", | |||
| 644 | + | }, | |||
| 645 | + | { | |||
| 646 | + | name: "yaml file", | |||
| 647 | + | configFile: "/path/to/config.yaml", | |||
| 648 | + | expected: "yaml", | |||
| 649 | + | }, | |||
| 650 | + | { | |||
| 651 | + | name: "yml file", | |||
| 652 | + | configFile: "/path/to/config.yml", | |||
| 653 | + | expected: "yaml", | |||
| 654 | + | }, | |||
| 655 | + | { | |||
| 656 | + | name: "json file", | |||
| 657 | + | configFile: "/path/to/config.json", | |||
| 658 | + | expected: "json", | |||
| 659 | + | }, | |||
| 660 | + | { | |||
| 661 | + | name: "unknown extension defaults to toml", | |||
| 662 | + | configFile: "/path/to/config.unknown", | |||
| 663 | + | expected: "toml", | |||
| 664 | + | }, | |||
| 665 | + | { | |||
| 666 | + | name: "no extension defaults to toml", | |||
| 667 | + | configFile: "/path/to/config", | |||
| 668 | + | expected: "toml", | |||
| 669 | + | }, | |||
| 670 | + | } | |||
| 671 | + | ||||
| 672 | + | for _, tt := range tests { | |||
| 673 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 674 | + | result := m.getConfigType(tt.configFile) | |||
| 675 | + | assert.Equal(t, tt.expected, result) | |||
| 676 | + | }) | |||
| 677 | + | } | |||
| 678 | + | } | |||
| 679 | + | ||||
| 680 | + | func TestMigrator_GetNestedValue(t *testing.T) { | |||
| 681 | + | m := NewMigrator() | |||
| 682 | + | ||||
| 683 | + | config := map[string]any{ | |||
| 684 | + | "level1": map[string]any{ | |||
| 685 | + | "level2": map[string]any{ | |||
| 686 | + | "level3": "deep_value", | |||
| 687 | + | }, | |||
| 688 | + | "simple": "simple_value", | |||
| 689 | + | }, | |||
| 690 | + | "top": "top_value", | |||
| 691 | + | } | |||
| 692 | + | ||||
| 693 | + | tests := []struct { | |||
| 694 | + | name string | |||
| 695 | + | key string | |||
| 696 | + | expected any | |||
| 697 | + | }{ | |||
| 698 | + | { | |||
| 699 | + | name: "top level key", | |||
| 700 | + | key: "top", | |||
| 701 | + | expected: "top_value", | |||
| 702 | + | }, | |||
| 703 | + | { | |||
| 704 | + | name: "nested key", | |||
| 705 | + | key: "level1.simple", | |||
| 706 | + | expected: "simple_value", | |||
| 707 | + | }, | |||
| 708 | + | { | |||
| 709 | + | name: "deeply nested key", | |||
| 710 | + | key: "level1.level2.level3", | |||
| 711 | + | expected: "deep_value", | |||
| 712 | + | }, | |||
| 713 | + | { | |||
| 714 | + | name: "non-existent key", | |||
| 715 | + | key: "nonexistent", | |||
| 716 | + | expected: nil, | |||
| 717 | + | }, | |||
| 718 | + | { | |||
| 719 | + | name: "non-existent nested key", | |||
| 720 | + | key: "level1.nonexistent", | |||
| 721 | + | expected: nil, | |||
| 722 | + | }, | |||
| 723 | + | } | |||
| 724 | + | ||||
| 725 | + | for _, tt := range tests { | |||
| 726 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 727 | + | result := m.getNestedValue(config, tt.key) | |||
| 728 | + | assert.Equal(t, tt.expected, result) | |||
| 729 | + | }) | |||
| 730 | + | } | |||
| 731 | + | } | |||
| 732 | + | ||||
| 733 | + | func TestMigrator_SetNestedValue(t *testing.T) { | |||
| 734 | + | m := NewMigrator() | |||
| 735 | + | ||||
| 736 | + | t.Run("set top level value", func(t *testing.T) { | |||
| 737 | + | config := make(map[string]any) | |||
| 738 | + | m.setNestedValue(config, "top", "value") | |||
| 739 | + | assert.Equal(t, "value", config["top"]) | |||
| 740 | + | }) | |||
| 741 | + | ||||
| 742 | + | t.Run("set nested value with existing parent", func(t *testing.T) { | |||
| 743 | + | config := map[string]any{ | |||
| 744 | + | "parent": map[string]any{}, | |||
| 745 | + | } | |||
| 746 | + | m.setNestedValue(config, "parent.child", "value") | |||
| 747 | + | parent := config["parent"].(map[string]any) | |||
| 748 | + | assert.Equal(t, "value", parent["child"]) | |||
| 749 | + | }) | |||
| 750 | + | ||||
| 751 | + | t.Run("set nested value creates intermediate maps", func(t *testing.T) { | |||
| 752 | + | config := make(map[string]any) | |||
| 753 | + | m.setNestedValue(config, "a.b.c", "deep_value") | |||
| 754 | + | ||||
| 755 | + | a := config["a"].(map[string]any) | |||
| 756 | + | b := a["b"].(map[string]any) | |||
| 757 | + | assert.Equal(t, "deep_value", b["c"]) | |||
| 758 | + | }) | |||
| 759 | + | } | |||
| 760 | + | ||||
| 761 | + | func TestMigrator_DeleteNestedKey(t *testing.T) { | |||
| 762 | + | m := NewMigrator() | |||
| 763 | + | ||||
| 764 | + | t.Run("delete top level key", func(t *testing.T) { | |||
| 765 | + | config := map[string]any{ | |||
| 766 | + | "keep": "value", | |||
| 767 | + | "delete": "value", | |||
| 768 | + | } | |||
| 769 | + | m.deleteNestedKey(config, "delete") | |||
| 770 | + | assert.Contains(t, config, "keep") | |||
| 771 | + | assert.NotContains(t, config, "delete") | |||
| 772 | + | }) | |||
| 773 | + | ||||
| 774 | + | t.Run("delete nested key", func(t *testing.T) { | |||
| 775 | + | config := map[string]any{ | |||
| 776 | + | "parent": map[string]any{ | |||
| 777 | + | "keep": "value", | |||
| 778 | + | "delete": "value", | |||
| 779 | + | }, | |||
| 780 | + | } | |||
| 781 | + | m.deleteNestedKey(config, "parent.delete") | |||
| 782 | + | parent := config["parent"].(map[string]any) | |||
| 783 | + | assert.Contains(t, parent, "keep") | |||
| 784 | + | assert.NotContains(t, parent, "delete") | |||
| 785 | + | }) | |||
| 786 | + | ||||
| 787 | + | t.Run("delete nested key cleans up empty parents", func(t *testing.T) { | |||
| 788 | + | config := map[string]any{ | |||
| 789 | + | "keep": "value", | |||
| 790 | + | "parent": map[string]any{ | |||
| 791 | + | "child": map[string]any{ | |||
| 792 | + | "leaf": "value", | |||
| 793 | + | }, | |||
| 794 | + | }, | |||
| 795 | + | } | |||
| 796 | + | m.deleteNestedKey(config, "parent.child.leaf") | |||
| 797 | + | // Both parent and child should be removed since they're empty | |||
| 798 | + | assert.NotContains(t, config, "parent") | |||
| 799 | + | assert.Contains(t, config, "keep") | |||
| 800 | + | }) | |||
| 801 | + | ||||
| 802 | + | t.Run("delete non-existent key is no-op", func(t *testing.T) { | |||
| 803 | + | config := map[string]any{ | |||
| 804 | + | "keep": "value", | |||
| 805 | + | } | |||
| 806 | + | m.deleteNestedKey(config, "nonexistent") | |||
| 807 | + | assert.Contains(t, config, "keep") | |||
| 808 | + | }) | |||
| 809 | + | } | |||
| 810 | + | ||||
| 811 | + | func TestMigrator_ReadRawConfig(t *testing.T) { | |||
| 812 | + | m := NewMigrator() | |||
| 813 | + | ||||
| 814 | + | t.Run("read TOML config", func(t *testing.T) { | |||
| 815 | + | tmpDir := t.TempDir() | |||
| 816 | + | configFile := filepath.Join(tmpDir, "config.toml") | |||
| 817 | + | ||||
| 818 | + | content := ` | |||
| 819 | + | [section] | |||
| 820 | + | key = "value" | |||
| 821 | + | number = 42 | |||
| 822 | + | ` | |||
| 823 | + | err := os.WriteFile(configFile, []byte(content), 0o644) | |||
| 824 | + | require.NoError(t, err) | |||
| 825 | + | ||||
| 826 | + | config, err := m.readRawConfig(configFile) | |||
| 827 | + | require.NoError(t, err) | |||
| 828 | + | ||||
| 829 | + | section := config["section"].(map[string]any) | |||
| 830 | + | assert.Equal(t, "value", section["key"]) | |||
| 831 | + | assert.Equal(t, int64(42), section["number"]) | |||
| 832 | + | }) | |||
| 833 | + | ||||
| 834 | + | t.Run("read YAML config", func(t *testing.T) { | |||
| 835 | + | tmpDir := t.TempDir() | |||
| 836 | + | configFile := filepath.Join(tmpDir, "config.yaml") | |||
| 837 | + | ||||
| 838 | + | content := ` | |||
| 839 | + | section: | |||
| 840 | + | key: value | |||
| 841 | + | number: 42 | |||
| 842 | + | ` | |||
| 843 | + | err := os.WriteFile(configFile, []byte(content), 0o644) | |||
| 844 | + | require.NoError(t, err) | |||
| 845 | + | ||||
| 846 | + | config, err := m.readRawConfig(configFile) | |||
| 847 | + | require.NoError(t, err) | |||
| 848 | + | ||||
| 849 | + | section := config["section"].(map[string]any) | |||
| 850 | + | assert.Equal(t, "value", section["key"]) | |||
| 851 | + | assert.Equal(t, 42, section["number"]) | |||
| 852 | + | }) | |||
| 853 | + | ||||
| 854 | + | t.Run("read JSON config", func(t *testing.T) { | |||
| 855 | + | tmpDir := t.TempDir() | |||
| 856 | + | configFile := filepath.Join(tmpDir, "config.json") | |||
| 857 | + | ||||
| 858 | + | content := `{ | |||
| 859 | + | "section": { | |||
| 860 | + | "key": "value", | |||
| 861 | + | "number": 42 | |||
| 862 | + | } | |||
| 863 | + | }` | |||
| 864 | + | err := os.WriteFile(configFile, []byte(content), 0o644) | |||
| 865 | + | require.NoError(t, err) | |||
| 866 | + | ||||
| 867 | + | config, err := m.readRawConfig(configFile) | |||
| 868 | + | require.NoError(t, err) | |||
| 869 | + | ||||
| 870 | + | section := config["section"].(map[string]any) | |||
| 871 | + | assert.Equal(t, "value", section["key"]) | |||
| 872 | + | // JSON numbers are float64 | |||
| 873 | + | assert.InDelta(t, 42.0, section["number"].(float64), 0.001) | |||
| 874 | + | }) | |||
| 875 | + | } | |||