From 6261ea2afba167d31e23f6cedb9ebb3e63324cd7 Mon Sep 17 00:00:00 2001 From: Virgil Date: Mon, 30 Mar 2026 17:45:39 +0000 Subject: [PATCH] refactor(store): clarify AX terminology in code and docs Co-Authored-By: Virgil --- CLAUDE.md | 2 +- coverage_test.go | 4 ++-- docs/architecture.md | 8 ++++---- docs/history.md | 20 ++++++++++---------- events.go | 7 ++++--- store.go | 26 +++++++++++++------------- store_test.go | 4 ++-- 7 files changed, 36 insertions(+), 35 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1443a8a..989e5b8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -131,7 +131,7 @@ func main() { ## Adding a New Method 1. Implement on `*Store` in `store.go` -2. If mutating, call `s.notify(Event{...})` after successful DB write +2. If mutating, call `s.notify(Event{...})` after successful database write 3. Add delegation method on `ScopedStore` in `scope.go` (prefix the group) 4. Update `checkQuota` in `scope.go` if it affects key/group counts 5. Write `Test__` tests diff --git a/coverage_test.go b/coverage_test.go index 840a6f3..840d9e3 100644 --- a/coverage_test.go +++ b/coverage_test.go @@ -107,7 +107,7 @@ func TestCoverage_GetAll_Bad_RowsError(t *testing.T) { for i := range garbage { garbage[i] = 0xFF } - require.Greater(t, len(data), len(garbage)*2, "DB should be large enough to corrupt") + require.Greater(t, len(data), len(garbage)*2, "database file should be large enough to corrupt") offset := len(data) * 3 / 4 maxOffset := len(data) - (len(garbage) * 2) if offset > maxOffset { @@ -194,7 +194,7 @@ func TestCoverage_Render_Bad_RowsError(t *testing.T) { for i := range garbage { garbage[i] = 0xFF } - require.Greater(t, len(data), len(garbage)*2, "DB should be large enough to corrupt") + require.Greater(t, len(data), len(garbage)*2, "database file should be large enough to corrupt") offset := len(data) * 3 / 4 maxOffset := len(data) - (len(garbage) * 2) if offset > maxOffset { diff --git a/docs/architecture.md b/docs/architecture.md index 45e2a22..6b4bba8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -40,7 +40,7 @@ CREATE TABLE IF NOT EXISTS entries ( The compound primary key `(group_name, entry_key)` enforces uniqueness per group-key pair and provides efficient indexed lookups. The `expires_at` column stores a Unix millisecond timestamp (nullable); a `NULL` value means the key never expires. -**Schema migration.** Databases created before the AX schema rename used a legacy `kv` table. On `New()`, go-store migrates that legacy table into `entries`, preserving rows and copying the expiry data when present. Databases that already have `entries` but lack `expires_at` still receive an additive `ALTER TABLE entries ADD COLUMN expires_at INTEGER` migration; if the column already exists, SQLite returns a "duplicate column" error which is silently ignored. +**Schema migration.** Databases created before the AX schema rename used a legacy key-value table. On `New()`, go-store migrates that legacy table into `entries`, preserving rows and copying the expiry data when present. Databases that already have `entries` but lack `expires_at` still receive an additive `ALTER TABLE entries ADD COLUMN expires_at INTEGER` migration; if the column already exists, SQLite returns a "duplicate column" error which is silently ignored. ## Group/Key Model @@ -169,7 +169,7 @@ for event := range watcher.Events { ### OnChange Callbacks -`OnChange(fn func(Event))` registers a synchronous callback that fires on every mutation. The callback runs in the goroutine that performed the write. Returns an idempotent unregister function. +`OnChange(callback func(Event))` registers a synchronous callback that fires on every mutation. The callback runs in the goroutine that performed the write. Returns an idempotent unregister function. This is the designed integration point for consumers such as go-ws: @@ -233,8 +233,8 @@ Exceeding a limit returns `QuotaExceededError`. All SQLite access is serialised through a single connection (`SetMaxOpenConns(1)`). The store's event registry uses two separate `sync.RWMutex` instances: `watchersLock` for watcher registration and dispatch, and `callbacksLock` for callback registration and dispatch. These locks do not interact: -- DB writes acquire no application-level lock. -- `notify()` acquires `watchersLock` (read) after the DB write completes, then `callbacksLock` (read) to snapshot callbacks. +- Database writes acquire no application-level lock. +- `notify()` acquires `watchersLock` (read) after the database write completes, then `callbacksLock` (read) to snapshot callbacks. - `Watch`/`Unwatch` acquire `watchersLock` (write) to modify watcher registrations. - `OnChange` acquires `callbacksLock` (write) to modify callback registrations. diff --git a/docs/history.md b/docs/history.md index bb118bc..5aaa8c5 100644 --- a/docs/history.md +++ b/docs/history.md @@ -63,14 +63,14 @@ Added optional time-to-live for keys. ### Changes -- `expires_at INTEGER` nullable column added to the `kv` schema. +- `expires_at INTEGER` nullable column added to the key-value schema. - `SetWithTTL(group, key, value string, ttl time.Duration)` stores the current time plus TTL as a Unix millisecond timestamp in `expires_at`. - `Get()` performs lazy deletion: if a key is found with an `expires_at` in the past, it is deleted and `NotFoundError` is returned. - `Count()`, `GetAll()`, and `Render()` include `(expires_at IS NULL OR expires_at > ?)` in all queries, excluding expired keys from results. - `PurgeExpired()` public method deletes all physically stored expired rows and returns the count removed. - Background goroutine calls `PurgeExpired()` every 60 seconds, controlled by a `context.WithCancel` that is cancelled on `Close()`. - `Set()` clears any existing TTL when overwriting a key (sets `expires_at = NULL`). -- Schema migration: `ALTER TABLE kv ADD COLUMN expires_at INTEGER` runs on `New()`. The "duplicate column" error on already-upgraded databases is silently ignored. +- Schema migration: `ALTER TABLE entries ADD COLUMN expires_at INTEGER` runs on `New()`. The "duplicate column" error on already-upgraded databases is silently ignored. ### Tests added @@ -117,14 +117,14 @@ Added a reactive notification system for store mutations. ### Changes -- `events.go` introduced with `EventType` (`EventSet`, `EventDelete`, `EventDeleteGroup`), `Event` struct, `Watcher` struct, `callbackEntry` struct. -- `watcherBufferSize = 16` constant. +- `events.go` introduced with `EventType` (`EventSet`, `EventDelete`, `EventDeleteGroup`), `Event` struct, `Watcher` struct, `changeCallbackRegistration` struct. +- `watcherEventBufferCapacity = 16` constant. - `Watch(group, key string) *Watcher`: creates a buffered channel watcher. Wildcard `"*"` supported for both group and key. Uses `atomic.AddUint64` for monotonic watcher IDs. -- `Unwatch(w *Watcher)`: removes watcher from the registry and closes its channel. Idempotent. -- `OnChange(fn func(Event)) func()`: registers a synchronous callback. Returns an idempotent unregister function using `sync.Once`. -- `notify(e Event)`: internal dispatch. Acquires read-lock on `s.mu`; non-blocking send to each matching watcher channel (drop-on-full); calls each callback synchronously. Separate `watcherMatches` helper handles wildcard logic. +- `Unwatch(watcher *Watcher)`: removes watcher from the registry and closes its channel. Idempotent. +- `OnChange(callback func(Event)) func()`: registers a synchronous callback. Returns an idempotent unregister function using `sync.Once`. +- `notify(event Event)`: internal dispatch. Acquires read-lock on `watchersLock`; non-blocking send to each matching watcher channel (drop-on-full); calls each callback synchronously. Separate `watcherMatches` helper handles wildcard logic. - `Set()`, `SetWithTTL()`, `Delete()`, `DeleteGroup()` each call `notify()` after the successful database write. -- `Store` struct extended with `watchers []*Watcher`, `callbacks []callbackEntry`, `mu sync.RWMutex`, `nextID uint64`. +- `Store` struct extended with `watchers []*Watcher`, `callbacks []changeCallbackRegistration`, `watchersLock sync.RWMutex`, `callbacksLock sync.RWMutex`, `nextWatcherID uint64`, `nextCallbackID uint64`. - ScopedStore mutations automatically emit events with the full prefixed group name — no extra implementation required. ### Tests added @@ -175,9 +175,9 @@ Renamed the internal SQLite schema to use descriptive names that are easier for ### Changes -- Replaced the abbreviated `kv` table with the descriptive `entries` table. +- Replaced the abbreviated key-value table with the descriptive `entries` table. - Renamed the `grp`, `key`, and `value` schema columns to `group_name`, `entry_key`, and `entry_value`. -- Added a startup migration that copies legacy `kv` databases into the new schema and preserves TTL data when present. +- Added a startup migration that copies legacy key-value databases into the new schema and preserves TTL data when present. - Kept the public Go API unchanged; the migration only affects the internal storage layout. --- diff --git a/events.go b/events.go index 7a718cc..8bdb73d 100644 --- a/events.go +++ b/events.go @@ -61,8 +61,8 @@ type Watcher struct { id uint64 } -// changeCallbackRegistration{id: 7, callback: fn} keeps one OnChange callback -// so unregister can remove the exact entry later. +// changeCallbackRegistration{id: 7, callback: handleConfigChange} keeps one +// OnChange callback so unregister can remove the exact entry later. type changeCallbackRegistration struct { id uint64 callback func(Event) @@ -159,7 +159,8 @@ func (storeInstance *Store) notify(event Event) { } } -// watcherMatches reports whether a watcher's filter matches the given event. +// watcherMatches reports whether Watch("config", "*") should receive +// Event{Group: "config", Key: "theme"}. func watcherMatches(watcher *Watcher, event Event) bool { if watcher.group != "*" && watcher.group != event.Group { return false diff --git a/store.go b/store.go index 9cc2d24..aae85ad 100644 --- a/store.go +++ b/store.go @@ -20,11 +20,11 @@ var NotFoundError = core.E("store", "not found", nil) var QuotaExceededError = core.E("store", "quota exceeded", nil) const ( - entriesTableName = "entries" - legacyEntriesTableName = "kv" - entryGroupColumn = "group_name" - entryKeyColumn = "entry_key" - entryValueColumn = "entry_value" + entriesTableName = "entries" + legacyKeyValueTableName = "kv" + entryGroupColumn = "group_name" + entryKeyColumn = "entry_key" + entryValueColumn = "entry_value" ) // Usage example: `storeInstance, err := store.New(":memory:"); if err != nil { return }; if err := storeInstance.Set("config", "theme", "dark"); err != nil { return }` @@ -428,15 +428,15 @@ const createEntriesTableSQL = `CREATE TABLE IF NOT EXISTS entries ( PRIMARY KEY (group_name, entry_key) )` -// ensureSchema creates the current entries table and migrates the legacy kv -// table when present. +// ensureSchema creates the current entries table and migrates the legacy +// key-value table when present. func ensureSchema(database *sql.DB) error { entriesTableExists, err := tableExists(database, entriesTableName) if err != nil { return core.E("store.New", "schema", err) } - legacyEntriesTableExists, err := tableExists(database, legacyEntriesTableName) + legacyEntriesTableExists, err := tableExists(database, legacyKeyValueTableName) if err != nil { return core.E("store.New", "schema", err) } @@ -484,7 +484,7 @@ func ensureExpiryColumn(database schemaDatabase) error { return nil } -// migrateLegacyEntriesTable copies rows from the old kv table into the +// migrateLegacyEntriesTable copies rows from the old key-value table into the // descriptive entries schema and then removes the legacy table. func migrateLegacyEntriesTable(database *sql.DB) error { transaction, err := database.Begin() @@ -511,19 +511,19 @@ func migrateLegacyEntriesTable(database *sql.DB) error { } } - legacyHasExpiryColumn, err := tableHasColumn(transaction, legacyEntriesTableName, "expires_at") + legacyHasExpiryColumn, err := tableHasColumn(transaction, legacyKeyValueTableName, "expires_at") if err != nil { return err } - insertSQL := "INSERT OR IGNORE INTO " + entriesTableName + " (" + entryGroupColumn + ", " + entryKeyColumn + ", " + entryValueColumn + ", expires_at) SELECT grp, key, value, NULL FROM " + legacyEntriesTableName + insertSQL := "INSERT OR IGNORE INTO " + entriesTableName + " (" + entryGroupColumn + ", " + entryKeyColumn + ", " + entryValueColumn + ", expires_at) SELECT grp, key, value, NULL FROM " + legacyKeyValueTableName if legacyHasExpiryColumn { - insertSQL = "INSERT OR IGNORE INTO " + entriesTableName + " (" + entryGroupColumn + ", " + entryKeyColumn + ", " + entryValueColumn + ", expires_at) SELECT grp, key, value, expires_at FROM " + legacyEntriesTableName + insertSQL = "INSERT OR IGNORE INTO " + entriesTableName + " (" + entryGroupColumn + ", " + entryKeyColumn + ", " + entryValueColumn + ", expires_at) SELECT grp, key, value, expires_at FROM " + legacyKeyValueTableName } if _, err := transaction.Exec(insertSQL); err != nil { return err } - if _, err := transaction.Exec("DROP TABLE " + legacyEntriesTableName); err != nil { + if _, err := transaction.Exec("DROP TABLE " + legacyKeyValueTableName); err != nil { return err } if err := transaction.Commit(); err != nil { diff --git a/store_test.go b/store_test.go index 9f78df2..a6555e5 100644 --- a/store_test.go +++ b/store_test.go @@ -68,7 +68,7 @@ func TestStore_New_Bad_ReadOnlyDir(t *testing.T) { dir := t.TempDir() dbPath := core.Path(dir, "readonly.db") - // Create a valid DB first, then make the directory read-only. + // Create a valid database first, then make the directory read-only. s, err := New(dbPath) require.NoError(t, err) require.NoError(t, s.Close()) @@ -1262,7 +1262,7 @@ func TestStore_SchemaUpgrade_Good_LegacyAndCurrentTables(t *testing.T) { func TestStore_SchemaUpgrade_Good_PreTTLDatabase(t *testing.T) { // Simulate a database created before the AX schema rename and TTL support. - // The legacy kv table has no expires_at column yet. + // The legacy key-value table has no expires_at column yet. dbPath := testPath(t, "pre-ttl.db") db, err := sql.Open("sqlite", dbPath) require.NoError(t, err)