git.iko.soy/crush
on main help sign in
fix(config): sort SetConfigFields keys, remove redundant MkdirAll, rename test
5b5f7b6Kieran Klukas
internal/config/store.go+11−5
compare with main
unifiedsplit
@@ -172,9 +172,6 @@ func (s *ConfigStore) atomicWrite(scope Scope, fn func(current []byte) ([]byte,
172 return err
172 return err
173 }
173 }
174
174
175− if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
176− return fmt.Errorf("create config directory: %w", err)
177− }
178 return atomicWriteFile(path, newData, 0o600)
175 return atomicWriteFile(path, newData, 0o600)
179}
176}
180
177
@@ -221,11 +218,20 @@ func (s *ConfigStore) SetConfigField(scope Scope, key string, value any) error {
221// The write is protected by an in-process mutex and a cross-process flock
218// The write is protected by an in-process mutex and a cross-process flock
222// to prevent races between concurrent writers in different processes.
219// to prevent races between concurrent writers in different processes.
223func (s *ConfigStore) SetConfigFields(scope Scope, kv map[string]any) error {
220func (s *ConfigStore) SetConfigFields(scope Scope, kv map[string]any) error {
221+ // Sort keys for deterministic output regardless of map iteration
222+ // order. This also ensures consistent results when callers pass
223+ // overlapping JSONPath keys (e.g. "a" and "a.b").
224+ keys := make([]string, 0, len(kv))
225+ for k := range kv {
226+ keys = append(keys, k)
227+ }
228+ slices.Sort(keys)
229+
224 err := s.atomicWrite(scope, func(data []byte) ([]byte, error) {
230 err := s.atomicWrite(scope, func(data []byte) ([]byte, error) {
225 v := string(data)
231 v := string(data)
226− for key, value := range kv {
232+ for _, key := range keys {
227 var sErr error
233 var sErr error
228− v, sErr = sjson.Set(v, key, value)
234+ v, sErr = sjson.Set(v, key, kv[key])
229 if sErr != nil {
235 if sErr != nil {
230 return nil, fmt.Errorf("failed to set config field %s: %w", key, sErr)
236 return nil, fmt.Errorf("failed to set config field %s: %w", key, sErr)
231 }
237 }
internal/config/store_test.go+5−3
@@ -732,9 +732,11 @@ func TestRefreshOAuthToken_UsesDiskTokenWhenDifferent(t *testing.T) {
732 require.Equal(t, "refresh-abc", updatedConfig.OAuthToken.RefreshToken)
732 require.Equal(t, "refresh-abc", updatedConfig.OAuthToken.RefreshToken)
733}
733}
734
734
735−// TestConfigStore_SetConfigFields_concurrent verifies that concurrent writes do
735+// TestConfigStore_SetConfigFields_concurrentInProcess verifies that
736−// not lose data when protected by the in-process mutex and cross-process flock.
736+// concurrent in-process writes do not lose data when serialized by the
737−func TestConfigStore_SetConfigFields_concurrent(t *testing.T) {
737+// s.mu mutex. This does not exercise the cross-process flock; testing
738+// that would require spawning a separate OS process.
739+func TestConfigStore_SetConfigFields_concurrentInProcess(t *testing.T) {
738 t.Parallel()
740 t.Parallel()
739
741
740 dir := t.TempDir()
742 dir := t.TempDir()