refactor(settings): extract shared internal/core/settings store - #1529
Open
perber wants to merge 1 commit into
Open
refactor(settings): extract shared internal/core/settings store#1529perber wants to merge 1 commit into
perber wants to merge 1 commit into
Conversation
branding.json and public-access.json each carried their own copy of the data-dir file path, atomic load/save, RWMutex-guarded cache, Reload() (for the restore path), and — for public-access — the env-managed vs settings-managed split. Add internal/core/settings: - Store[T]: atomic (shared.WriteFileAtomic), RWMutex-cached JSON file in the data dir. Missing file => the zero value passed to New, not an error. An onLoad(*T) hook re-applies non-persisted (json:"-") fields after every load/reload/put, replacing branding's hand-written constraint injection. - Reloadable: what internal/restore iterates instead of enumerating each service by hand. - Value[T] / Managed[T] / Fixed[T]: the env-managed split. Managed wraps a *Store[T]; Fixed is a boot-pinned value with no file whose Put returns ErrEnvManaged and whose Reload is a no-op, so it sits harmlessly in restore's []Reloadable. Migrate internal/branding and internal/publicaccess onto it (publicaccess loses its (enabled, envManaged, store) triple, its local mutex, the per-method mode branch, and the Service-level/Store-level double cache; SetEnabled maps settings.ErrEnvManaged onto the existing ErrCodeEnvManaged LocalizedError so the HTTP 409 mapping is unchanged). Rewire internal/restore: Config.BrandingService / Config.PublicAccess become Reloadables []settings.Reloadable; manager gains reloadAll() which accumulates every failure via errors.Join, matching reopenAllStores. internal/backup keeps its own encrypted ConfigStore (AES-GCM SecretBox) and is deliberately not migrated — Store[T] has no codec hook. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Extracts the persistence machinery shared by
branding.jsonandpublic-access.jsoninto a newinternal/core/settingspackage, so the nextsettings-managed JSON config (Hosted Onboarding Wizard's
onboarding.json)can be built on it from day one instead of as a fourth hand-rolled copy.
Each of the two subsystems carried its own copy of: data-dir file path, atomic
load/save,
RWMutex-guarded in-memory cache,Reload()(called byinternal/restoreafter a snapshot swap), and — forpublic-access— theenv-managed vs settings-managed split.
New package
internal/core/settingsStore[T]— atomic (shared.WriteFileAtomic),RWMutex-cached JSONfile in the data dir.
New/Get/Put/Reload. Missing file ⇒ thezero value passed to
New, not an error.onLoad func(*T)hook re-appliesnon-persisted (
json:"-") fields after every load/reload/put — replacesbranding's hand-written constraint injection.
Reloadable(Reload() error) — whatinternal/restoreiteratesinstead of enumerating each service by hand.
Value[T]/Managed[T]/Fixed[T]— the env-managed split.Managed[T]{Store: st}delegates to a*Store[T];Fixed[T]is aboot-pinned value with no file —
Putreturns theErrEnvManagedsentinel,
Reloadis a no-op, so aFixedentry is harmless inrestore's[]Reloadable.Migrations
internal/branding—BrandingStoreembeds*settings.Store[BrandingConfig];load/save/mutex/reload/constraint-injection deleted.
internal/publicaccess—store.godeleted;Servicenow carries asingle
settings.Value[fileConfig]. The(enabled, envManaged, store)triple, the local
sync.RWMutex, the per-method mode branch, and theService-level/Store-level double cache are gone.
SetEnabledmapssettings.ErrEnvManaged→ the existingErrCodeEnvManaged*LocalizedError, so the HTTP 409 behaviour is unchanged — every existingservice_test.gocase passes untouched.internal/restore—Config.BrandingService/Config.PublicAccessbecome
Reloadables []settings.Reloadable;manager.gogainsreloadAll()which accumulates every failure via
errors.Join(matchingreopenAllStores).cmd/leafwiki/main.gowires the slice.Deliberately out of scope
internal/backupkeeps its ownConfigStore(AES-GCM SecretBox, HKDF keyfrom the JWT secret).
Store[T]has no codec hook; adding an opt-inStore.WithCodec(enc, dec)later is the path if backup ever needs to sharethe primitive.
Tests
store_test.go,value_test.go(round-trip, missing-file, perms,onLoad,
Fixed/Managedsemantics,Valuein a[]Reloadable,-race).go test ./...green;go vet+golangci-lintv2 clean onthe changed packages.
🤖 Generated with Claude Code