mirror of
https://github.com/DeBrosOfficial/orama.git
synced 2026-06-16 21:54:14 +00:00
fix(gateway): update rqlite consistency level and improve column mapping
- Change RQLite consistency level from `none` to `weak` to ensure reads route to the leader and prevent stale data reads (fixes #235) - Add `normalizeColumnKey` to allow snake_case SQL columns to map to CamelCase Go struct fields automatically (fixes #65) - Add comprehensive unit tests for DSN generation and column mapping
This commit is contained in:
parent
6d9822dc35
commit
5ccacb91d6
@ -173,11 +173,7 @@ func initializeRQLite(logger *logging.ColoredLogger, cfg *Config, deps *Dependen
|
||||
// Inject basic auth credentials into DSN if available
|
||||
dsn = injectRQLiteAuth(dsn, cfg.RQLiteUsername, cfg.RQLitePassword)
|
||||
|
||||
if strings.Contains(dsn, "?") {
|
||||
dsn += "&disableClusterDiscovery=true&level=none"
|
||||
} else {
|
||||
dsn += "?disableClusterDiscovery=true&level=none"
|
||||
}
|
||||
dsn = appendRQLiteQueryParams(dsn)
|
||||
db, err := sql.Open("rqlite", dsn)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to open rqlite sql db: %w", err)
|
||||
@ -824,6 +820,28 @@ func injectRQLiteAuth(dsn, username, password string) string {
|
||||
return dsn
|
||||
}
|
||||
|
||||
// appendRQLiteQueryParams adds the standard query parameters to a RQLite DSN:
|
||||
//
|
||||
// - `disableClusterDiscovery=true` — gorqlite's discovery /nodes call is
|
||||
// unreliable when peers are unreachable; we manage topology ourselves.
|
||||
// - `level=weak` — Bug #235. Reads route to the leader (the only node
|
||||
// guaranteed to have all committed writes), so a SELECT after an UPDATE
|
||||
// in the same serverless invocation sees the new state. Previously
|
||||
// `level=none`, which read from the local follower's possibly-stale
|
||||
// snapshot. gorqlite's upstream default is `weak`; we were overriding
|
||||
// to `none` and that hid this bug.
|
||||
//
|
||||
// The cost of `weak` over `none` is one HTTP hop to the leader (~1-2ms over
|
||||
// the WireGuard mesh) and applies only to reads. Writes are unaffected
|
||||
// because rqlite always redirects them to the leader regardless of `level`.
|
||||
func appendRQLiteQueryParams(dsn string) string {
|
||||
const params = "disableClusterDiscovery=true&level=weak"
|
||||
if strings.Contains(dsn, "?") {
|
||||
return dsn + "&" + params
|
||||
}
|
||||
return dsn + "?" + params
|
||||
}
|
||||
|
||||
// buildPushDispatcher constructs the push subsystem.
|
||||
//
|
||||
// As of bug #220 follow-up, push always initializes when ClusterSecret is
|
||||
|
||||
59
core/pkg/gateway/dependencies_dsn_test.go
Normal file
59
core/pkg/gateway/dependencies_dsn_test.go
Normal file
@ -0,0 +1,59 @@
|
||||
package gateway
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestAppendRQLiteQueryParams_consistencyLevelWeak is the regression guard
|
||||
// for bug #235. The DSN passed to gorqlite MUST encode `level=weak` so reads
|
||||
// route to the leader and see all committed writes from earlier in the same
|
||||
// serverless invocation. `level=none` (the previous default) read from the
|
||||
// local follower's possibly-stale state and broke `INSERT → UPDATE → SELECT`
|
||||
// patterns inside host functions.
|
||||
func TestAppendRQLiteQueryParams_consistencyLevelWeak(t *testing.T) {
|
||||
got := appendRQLiteQueryParams("http://localhost:5001")
|
||||
if !strings.Contains(got, "level=weak") {
|
||||
t.Errorf("DSN missing level=weak (bug #235 regression):\n%s", got)
|
||||
}
|
||||
if strings.Contains(got, "level=none") {
|
||||
t.Errorf("DSN must NOT carry level=none (bug #235):\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "disableClusterDiscovery=true") {
|
||||
t.Errorf("DSN missing disableClusterDiscovery=true:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAppendRQLiteQueryParams_existingQueryString — when the inbound DSN
|
||||
// already has a `?param=value` segment (e.g. authentication appended
|
||||
// upstream), the new params must be `&`-joined, not start a fresh `?`.
|
||||
func TestAppendRQLiteQueryParams_existingQueryString(t *testing.T) {
|
||||
got := appendRQLiteQueryParams("http://localhost:5001?foo=bar")
|
||||
if strings.Count(got, "?") != 1 {
|
||||
t.Errorf("expected exactly one '?' in DSN, got: %s", got)
|
||||
}
|
||||
if !strings.Contains(got, "?foo=bar&disableClusterDiscovery=true&level=weak") {
|
||||
t.Errorf("DSN didn't append params with '&' join:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAppendRQLiteQueryParams_noExistingQueryString — when no `?` is present,
|
||||
// the params must be introduced with a `?` not an `&`.
|
||||
func TestAppendRQLiteQueryParams_noExistingQueryString(t *testing.T) {
|
||||
got := appendRQLiteQueryParams("http://localhost:5001")
|
||||
if !strings.HasSuffix(got, "?disableClusterDiscovery=true&level=weak") {
|
||||
t.Errorf("DSN didn't introduce query string with '?':\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAppendRQLiteQueryParams_preservesAuthCredentials — credentials injected
|
||||
// upstream by injectRQLiteAuth must survive the param append unchanged.
|
||||
func TestAppendRQLiteQueryParams_preservesAuthCredentials(t *testing.T) {
|
||||
got := appendRQLiteQueryParams("http://orama:secret@localhost:5001")
|
||||
if !strings.Contains(got, "orama:secret@localhost:5001") {
|
||||
t.Errorf("auth credentials lost:\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "level=weak") {
|
||||
t.Errorf("level=weak missing after auth-injected DSN:\n%s", got)
|
||||
}
|
||||
}
|
||||
@ -14,14 +14,33 @@ type RQLiteAdapter struct {
|
||||
db *sql.DB
|
||||
}
|
||||
|
||||
// NewRQLiteAdapter creates a new adapter that provides sql.DB interface for RQLite
|
||||
func NewRQLiteAdapter(manager *RQLiteManager) (*RQLiteAdapter, error) {
|
||||
// Build DSN with optional basic auth credentials
|
||||
dsn := fmt.Sprintf("http://localhost:%d?disableClusterDiscovery=true&level=none", manager.config.RQLitePort)
|
||||
if manager.config.RQLiteUsername != "" && manager.config.RQLitePassword != "" {
|
||||
dsn = fmt.Sprintf("http://%s:%s@localhost:%d?disableClusterDiscovery=true&level=none",
|
||||
manager.config.RQLiteUsername, manager.config.RQLitePassword, manager.config.RQLitePort)
|
||||
// adapterReadConsistencyLevel is the rqlite consistency level used for
|
||||
// gateway-internal SQL reads. Set to `weak` (matches gorqlite's own upstream
|
||||
// default). MUST NOT be `none` — see bug #235: with `none`, reads serve from
|
||||
// the local SQLite of whichever node the client is connected to, including
|
||||
// followers that haven't replayed the most-recent Raft commits. Serverless
|
||||
// functions running an `INSERT → UPDATE → SELECT` pattern in a single
|
||||
// invocation saw the pre-write snapshot. `weak` routes reads to the leader,
|
||||
// which always has the committed state, at a cost of ~1-2ms LAN hop over
|
||||
// the WireGuard mesh.
|
||||
const adapterReadConsistencyLevel = "weak"
|
||||
|
||||
// buildRQLiteDSN composes the DSN URL passed to gorqlite's stdlib driver.
|
||||
// Pulled out for unit testing — the URL must encode `level=weak` (bug #235)
|
||||
// in addition to `disableClusterDiscovery=true`.
|
||||
func buildRQLiteDSN(host string, port int, username, password string) string {
|
||||
if username != "" && password != "" {
|
||||
return fmt.Sprintf("http://%s:%s@%s:%d?disableClusterDiscovery=true&level=%s",
|
||||
username, password, host, port, adapterReadConsistencyLevel)
|
||||
}
|
||||
return fmt.Sprintf("http://%s:%d?disableClusterDiscovery=true&level=%s",
|
||||
host, port, adapterReadConsistencyLevel)
|
||||
}
|
||||
|
||||
// NewRQLiteAdapter creates a new adapter that provides sql.DB interface for RQLite.
|
||||
func NewRQLiteAdapter(manager *RQLiteManager) (*RQLiteAdapter, error) {
|
||||
dsn := buildRQLiteDSN("localhost", manager.config.RQLitePort,
|
||||
manager.config.RQLiteUsername, manager.config.RQLitePassword)
|
||||
db, err := sql.Open("rqlite", dsn)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to open RQLite SQL connection: %w", err)
|
||||
|
||||
@ -1,12 +1,48 @@
|
||||
package rqlite
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// TestBuildRQLiteDSN_consistencyLevelWeak is the regression guard for bug
|
||||
// #235. The DSN MUST encode `level=weak` so reads route to the leader and
|
||||
// see all committed writes. `level=none` (the previous default) caused
|
||||
// serverless `INSERT → UPDATE → SELECT` patterns to return stale snapshots
|
||||
// when the local node was a follower lagging on Raft replay.
|
||||
func TestBuildRQLiteDSN_consistencyLevelWeak(t *testing.T) {
|
||||
got := buildRQLiteDSN("localhost", 5001, "", "")
|
||||
if !strings.Contains(got, "level=weak") {
|
||||
t.Errorf("DSN missing level=weak (bug #235 regression):\n%s", got)
|
||||
}
|
||||
if strings.Contains(got, "level=none") {
|
||||
t.Errorf("DSN must NOT carry level=none (bug #235):\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "disableClusterDiscovery=true") {
|
||||
t.Errorf("DSN missing disableClusterDiscovery=true:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildRQLiteDSN_withAuthCredentials(t *testing.T) {
|
||||
got := buildRQLiteDSN("rqlite-host", 5001, "orama", "secret123")
|
||||
if !strings.Contains(got, "orama:secret123@rqlite-host:5001") {
|
||||
t.Errorf("DSN missing inline credentials:\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "level=weak") {
|
||||
t.Errorf("DSN with auth still missing level=weak:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildRQLiteDSN_noAuthOmitsCredentials(t *testing.T) {
|
||||
got := buildRQLiteDSN("localhost", 5001, "", "")
|
||||
if strings.Contains(got, "@localhost") {
|
||||
t.Errorf("DSN should not include credentials when both empty:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdapterPoolConstants verifies the connection pool configuration values
|
||||
// used in NewRQLiteAdapter match the expected tuning parameters.
|
||||
// These values are critical for RQLite performance and stale connection eviction.
|
||||
|
||||
@ -108,7 +108,12 @@ func scanCurrentRowIntoStruct(rows *sql.Rows, cols []string, destStruct reflect.
|
||||
}
|
||||
fieldIndex := buildFieldIndex(destStruct.Type())
|
||||
for i, c := range cols {
|
||||
if idx, ok := fieldIndex[strings.ToLower(c)]; ok {
|
||||
// normalizeColumnKey strips underscores so snake_case SQL columns
|
||||
// match CamelCase struct field names (bug found via feature #65:
|
||||
// `trigger_id` column vs `TriggerID` field silently failed to map,
|
||||
// leaving rows with empty IDs/expressions and a cron scheduler
|
||||
// stuck in a tight retry loop on phantom triggers).
|
||||
if idx, ok := fieldIndex[normalizeColumnKey(c)]; ok {
|
||||
field := destStruct.Field(idx)
|
||||
if field.CanSet() {
|
||||
if err := setReflectValue(field, raw[i]); err != nil {
|
||||
@ -130,7 +135,17 @@ func normalizeSQLValue(v any) any {
|
||||
}
|
||||
}
|
||||
|
||||
// buildFieldIndex creates a map of lowercase column names to field indices.
|
||||
// buildFieldIndex creates a map of normalized column keys to field indices.
|
||||
//
|
||||
// Column keys are normalized (lowercase + underscores stripped) so a
|
||||
// SQL column named `trigger_id` matches a struct field named `TriggerID`
|
||||
// without requiring an explicit `db:"trigger_id"` tag. Explicit tags still
|
||||
// take precedence over the field name when present.
|
||||
//
|
||||
// Without this normalization, snake_case ↔ CamelCase silently failed to
|
||||
// map and the scanner returned rows with zero-valued fields — exactly the
|
||||
// failure mode that broke the cron scheduler in feature #65 (DB had valid
|
||||
// rows, scheduler saw empty `trigger_id` / `cron_expression` every tick).
|
||||
func buildFieldIndex(t reflect.Type) map[string]int {
|
||||
m := make(map[string]int)
|
||||
for i := 0; i < t.NumField(); i++ {
|
||||
@ -146,11 +161,25 @@ func buildFieldIndex(t reflect.Type) map[string]int {
|
||||
if col == "" {
|
||||
col = f.Name
|
||||
}
|
||||
m[strings.ToLower(col)] = i
|
||||
m[normalizeColumnKey(col)] = i
|
||||
}
|
||||
return m
|
||||
}
|
||||
|
||||
// normalizeColumnKey lowercases its input and strips underscores so
|
||||
// scanner lookups work across naming conventions:
|
||||
//
|
||||
// column "trigger_id" -> "triggerid"
|
||||
// column "TriggerID" -> "triggerid"
|
||||
// column "triggerID" -> "triggerid"
|
||||
// column "id" -> "id"
|
||||
//
|
||||
// This is the central rule that lets snake_case SQL columns map onto
|
||||
// CamelCase Go struct fields without explicit `db:` tags.
|
||||
func normalizeColumnKey(s string) string {
|
||||
return strings.ReplaceAll(strings.ToLower(s), "_", "")
|
||||
}
|
||||
|
||||
// setReflectValue sets a reflect.Value from a raw SQL value.
|
||||
func setReflectValue(field reflect.Value, raw any) error {
|
||||
if raw == nil {
|
||||
|
||||
104
core/pkg/rqlite/scanner_snake_case_test.go
Normal file
104
core/pkg/rqlite/scanner_snake_case_test.go
Normal file
@ -0,0 +1,104 @@
|
||||
package rqlite
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
_ "github.com/mattn/go-sqlite3"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestScanIntoDest_snakeCaseColumnsCamelCaseFields is the end-to-end
|
||||
// regression guard for the feature #65 cron-scheduler bug. The scanner MUST
|
||||
// populate CamelCase struct fields from snake_case SQL columns even when
|
||||
// no `db:` tags are present on the struct.
|
||||
//
|
||||
// Pre-fix, rqlite returned valid `function_cron_triggers` rows with
|
||||
// non-empty `id` / `cron_expression` / `next_run_at`, but the cron
|
||||
// scheduler scanned them into a `CronDueRow` struct (no db tags) and got
|
||||
// back zero values: `TriggerID == ""`, `CronExpression == ""`. The
|
||||
// scheduler then logged "bad expression" every poll tick and never fired
|
||||
// any function — even though everything from the DB to the goroutine to
|
||||
// the HTTP API was working.
|
||||
//
|
||||
// This test reproduces that exact wiring against an in-memory SQLite db
|
||||
// so any regression on the snake/Camel mapping fails CI loudly.
|
||||
func TestScanIntoDest_snakeCaseColumnsCamelCaseFields(t *testing.T) {
|
||||
db, err := sql.Open("sqlite3", ":memory:")
|
||||
require.NoError(t, err)
|
||||
defer db.Close()
|
||||
|
||||
_, err = db.Exec(`
|
||||
CREATE TABLE function_cron_triggers (
|
||||
id TEXT PRIMARY KEY,
|
||||
function_id TEXT NOT NULL,
|
||||
cron_expression TEXT NOT NULL,
|
||||
next_run_at TIMESTAMP
|
||||
);
|
||||
INSERT INTO function_cron_triggers VALUES
|
||||
('10a70f79-35b3-4bc6-b173-aec3012b570d',
|
||||
'dfe55b75-9c39-45ce-b60e-c72b131350b2',
|
||||
'*/30 * * * * *',
|
||||
'2026-05-09T05:50:00Z');
|
||||
`)
|
||||
require.NoError(t, err)
|
||||
|
||||
rows, err := db.Query(`
|
||||
SELECT id AS trigger_id, function_id, cron_expression, next_run_at
|
||||
FROM function_cron_triggers
|
||||
`)
|
||||
require.NoError(t, err)
|
||||
defer rows.Close()
|
||||
|
||||
// Mirror of pkg/serverless/triggers.CronDueRow — CamelCase fields,
|
||||
// NO db tags, snake_case SQL columns. Pre-fix the fields stayed at
|
||||
// zero values; post-fix they populate.
|
||||
type cronDueRowLike struct {
|
||||
TriggerID string
|
||||
FunctionID string
|
||||
CronExpression string
|
||||
NextRunAt time.Time
|
||||
}
|
||||
|
||||
var dst []cronDueRowLike
|
||||
require.NoError(t, scanIntoDest(rows, &dst))
|
||||
require.Len(t, dst, 1)
|
||||
|
||||
got := dst[0]
|
||||
assert.Equal(t, "10a70f79-35b3-4bc6-b173-aec3012b570d", got.TriggerID,
|
||||
"TriggerID populated from `trigger_id` column (regression guard for feature #65)")
|
||||
assert.Equal(t, "dfe55b75-9c39-45ce-b60e-c72b131350b2", got.FunctionID,
|
||||
"FunctionID populated from `function_id` column")
|
||||
assert.Equal(t, "*/30 * * * * *", got.CronExpression,
|
||||
"CronExpression populated from `cron_expression` column")
|
||||
assert.False(t, got.NextRunAt.IsZero(),
|
||||
"NextRunAt populated from `next_run_at` column")
|
||||
assert.Equal(t, 2026, got.NextRunAt.Year())
|
||||
}
|
||||
|
||||
// TestScanIntoDest_explicitDBTagStillTakesPrecedence guarantees that adding
|
||||
// the snake-case fix didn't break callers that already supplied explicit
|
||||
// `db:` tags. Tag wins; no double-mapping ambiguity.
|
||||
func TestScanIntoDest_explicitDBTagStillTakesPrecedence(t *testing.T) {
|
||||
db, err := sql.Open("sqlite3", ":memory:")
|
||||
require.NoError(t, err)
|
||||
defer db.Close()
|
||||
|
||||
_, err = db.Exec(`CREATE TABLE t (custom_col TEXT); INSERT INTO t VALUES ('hello');`)
|
||||
require.NoError(t, err)
|
||||
|
||||
rows, err := db.Query(`SELECT custom_col FROM t`)
|
||||
require.NoError(t, err)
|
||||
defer rows.Close()
|
||||
|
||||
type tagged struct {
|
||||
// Field name has nothing to do with the column; only the tag binds it.
|
||||
Whatever string `db:"custom_col"`
|
||||
}
|
||||
var dst []tagged
|
||||
require.NoError(t, scanIntoDest(rows, &dst))
|
||||
require.Len(t, dst, 1)
|
||||
assert.Equal(t, "hello", dst[0].Whatever)
|
||||
}
|
||||
@ -80,10 +80,15 @@ type structWithEmbedded struct {
|
||||
func TestBuildFieldIndex(t *testing.T) {
|
||||
t.Run("tagged struct", func(t *testing.T) {
|
||||
idx := buildFieldIndex(reflect.TypeOf(taggedStruct{}))
|
||||
// Keys are normalized via normalizeColumnKey: lowercased and
|
||||
// underscores stripped. So `user_name` and `UserName` both
|
||||
// resolve to `username` and the scanner can match snake_case SQL
|
||||
// columns onto CamelCase struct fields without explicit `db:` tags
|
||||
// (feature #65 cron-scheduler bug fix).
|
||||
assert.Equal(t, 0, idx["id"])
|
||||
assert.Equal(t, 1, idx["user_name"])
|
||||
assert.Equal(t, 2, idx["email_addr"])
|
||||
assert.Equal(t, 3, idx["created_at"])
|
||||
assert.Equal(t, 1, idx["username"])
|
||||
assert.Equal(t, 2, idx["emailaddr"])
|
||||
assert.Equal(t, 3, idx["createdat"])
|
||||
assert.Len(t, idx, 4)
|
||||
})
|
||||
|
||||
@ -99,7 +104,8 @@ func TestBuildFieldIndex(t *testing.T) {
|
||||
idx := buildFieldIndex(reflect.TypeOf(mixedStruct{}))
|
||||
assert.Equal(t, 0, idx["id"])
|
||||
assert.Equal(t, 1, idx["name"])
|
||||
assert.Equal(t, 3, idx["is_active"])
|
||||
// `is_active` normalizes to `isactive`.
|
||||
assert.Equal(t, 3, idx["isactive"])
|
||||
// "-" tag means the first part of the tag is "-", so it maps with key "-"
|
||||
// The actual behavior: tag="-" → col="-" → stored as "-"
|
||||
// Let's verify what actually happens
|
||||
@ -157,6 +163,59 @@ func TestBuildFieldIndex(t *testing.T) {
|
||||
_, hasLowerID := idx["id"]
|
||||
assert.True(t, hasLowerID)
|
||||
})
|
||||
|
||||
// REGRESSION (feature #65 cron-scheduler bug): a struct with CamelCase
|
||||
// fields and NO `db:` tags must map onto snake_case SQL columns.
|
||||
// Pre-fix, the cron scheduler read DB rows with valid `trigger_id` and
|
||||
// `cron_expression` columns into a `CronDueRow` struct with `TriggerID`
|
||||
// and `CronExpression` fields — and got back zero values for both,
|
||||
// because `triggerid` (the lowercased struct field name) didn't match
|
||||
// `trigger_id` (the lowercased column name) in the index lookup.
|
||||
t.Run("camelCase field maps to snake_case column without db tag", func(t *testing.T) {
|
||||
type cronDueRowLike struct {
|
||||
TriggerID string
|
||||
FunctionID string
|
||||
FunctionName string
|
||||
Namespace string
|
||||
CronExpression string
|
||||
}
|
||||
idx := buildFieldIndex(reflect.TypeOf(cronDueRowLike{}))
|
||||
|
||||
// Both spellings of the same key must resolve to the same field
|
||||
// because of normalizeColumnKey.
|
||||
for _, sqlCol := range []string{"trigger_id", "TriggerID", "triggerid"} {
|
||||
pos, ok := idx[normalizeColumnKey(sqlCol)]
|
||||
assert.True(t, ok, "column %q should map to TriggerID", sqlCol)
|
||||
assert.Equal(t, 0, pos)
|
||||
}
|
||||
for _, sqlCol := range []string{"cron_expression", "CronExpression"} {
|
||||
pos, ok := idx[normalizeColumnKey(sqlCol)]
|
||||
assert.True(t, ok, "column %q should map to CronExpression", sqlCol)
|
||||
assert.Equal(t, 4, pos)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestNormalizeColumnKey exercises the central rule that snake_case ↔
|
||||
// CamelCase map to the same scanner key. Pre-fix, the absence of this
|
||||
// normalization silently broke struct scanning for any multi-word column
|
||||
// name (the cron scheduler in feature #65 was the canary).
|
||||
func TestNormalizeColumnKey(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"id": "id",
|
||||
"ID": "id",
|
||||
"trigger_id": "triggerid",
|
||||
"TriggerID": "triggerid",
|
||||
"triggerID": "triggerid",
|
||||
"cron_expression": "cronexpression",
|
||||
"CronExpression": "cronexpression",
|
||||
"": "",
|
||||
"_": "",
|
||||
"___": "",
|
||||
}
|
||||
for in, want := range cases {
|
||||
assert.Equal(t, want, normalizeColumnKey(in), "normalizeColumnKey(%q)", in)
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@debros/orama",
|
||||
"version": "1.0.0",
|
||||
"version": "1.0.1",
|
||||
"description": "TypeScript SDK for Orama Network - Database, PubSub, Cache, Storage, Vault, and more",
|
||||
"type": "module",
|
||||
"main": "./dist/index.js",
|
||||
|
||||
@ -53,13 +53,50 @@ export class AuthClient {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Exchange a stored refresh token for a fresh access token.
|
||||
*
|
||||
* Pulls the refresh token (and the namespace it was issued for) out of
|
||||
* storage — both are persisted by `verify()` after a successful wallet
|
||||
* sign-in. The gateway returns a new access token and may rotate the
|
||||
* refresh token; we persist the rotated one if present.
|
||||
*
|
||||
* Bug #239: previously this method (a) sent no body and (b) read the
|
||||
* wrong response field, so the call always 400-ed AND silently wrote
|
||||
* `undefined` as the in-memory JWT. Both issues fixed.
|
||||
*/
|
||||
async refresh(): Promise<string> {
|
||||
const response = await this.httpClient.post<{ token: string }>(
|
||||
"/v1/auth/refresh"
|
||||
);
|
||||
const token = response.token;
|
||||
this.setJwt(token);
|
||||
return token;
|
||||
const refreshToken = await this.storage.get("refreshToken");
|
||||
if (!refreshToken) {
|
||||
throw new Error(
|
||||
"refresh failed: no refresh token in storage — call verify() first"
|
||||
);
|
||||
}
|
||||
const namespace = (await this.storage.get("namespace")) ?? "default";
|
||||
|
||||
const response = await this.httpClient.post<{
|
||||
access_token: string;
|
||||
refresh_token?: string;
|
||||
expires_in?: number;
|
||||
subject?: string;
|
||||
namespace?: string;
|
||||
token_type?: string;
|
||||
}>("/v1/auth/refresh", { refresh_token: refreshToken, namespace });
|
||||
|
||||
if (!response?.access_token) {
|
||||
throw new Error("refresh failed: server returned no access_token");
|
||||
}
|
||||
|
||||
this.setJwt(response.access_token);
|
||||
|
||||
// Rotate the stored refresh token if the server returned a new one
|
||||
// (rqlite-side gateway currently echoes the same token; future versions
|
||||
// may rotate, so handle both shapes).
|
||||
if (response.refresh_token && response.refresh_token !== refreshToken) {
|
||||
await this.storage.set("refreshToken", response.refresh_token);
|
||||
}
|
||||
|
||||
return response.access_token;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -198,6 +235,14 @@ export class AuthClient {
|
||||
await this.storage.set("refreshToken", (response as any).refresh_token);
|
||||
}
|
||||
|
||||
// Persist the namespace this JWT was issued for so refresh() can
|
||||
// include it in the refresh request body (the gateway scopes refresh
|
||||
// tokens to the issuing namespace). Bug #239 — without this, refresh
|
||||
// would default to "default" and fail for namespace-scoped sessions.
|
||||
const issuedNamespace =
|
||||
(response as any).namespace || params.namespace || "default";
|
||||
await this.storage.set("namespace", issuedNamespace);
|
||||
|
||||
return response as any;
|
||||
}
|
||||
|
||||
|
||||
154
sdk/tests/unit/auth/refresh-bug-239.test.ts
Normal file
154
sdk/tests/unit/auth/refresh-bug-239.test.ts
Normal file
@ -0,0 +1,154 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { AuthClient } from "../../../src/auth/client";
|
||||
import { HttpClient } from "../../../src/core/http";
|
||||
import type { StorageAdapter } from "../../../src/auth/types";
|
||||
|
||||
/**
|
||||
* Bug #239 regression guard. Two pre-existing defects in
|
||||
* AuthClient.refresh() were demonstrated by this file in its pre-fix form
|
||||
* (the call carried no body and read the wrong response field, silently
|
||||
* corrupting the in-memory JWT to undefined). Both have been fixed and
|
||||
* the assertions below now lock in the correct behavior so the bug can't
|
||||
* silently come back.
|
||||
*/
|
||||
describe("Bug #239 — AuthClient.refresh() regression guard", () => {
|
||||
let fetchSpy: ReturnType<typeof vi.fn>;
|
||||
let storage: StorageAdapter;
|
||||
let memStore: Map<string, string>;
|
||||
|
||||
function setupGoodResponse() {
|
||||
fetchSpy = vi.fn(async (_input: any, _init?: RequestInit) => {
|
||||
return new Response(
|
||||
// Server response shape matches
|
||||
// core/pkg/gateway/handlers/auth/jwt_handler.go:106-113.
|
||||
JSON.stringify({
|
||||
access_token: "NEW-JWT-FROM-SERVER",
|
||||
token_type: "Bearer",
|
||||
expires_in: 900,
|
||||
refresh_token: "rotated-refresh",
|
||||
subject: "0xabc",
|
||||
namespace: "anchat-test",
|
||||
}),
|
||||
{ status: 200, headers: { "Content-Type": "application/json" } }
|
||||
);
|
||||
});
|
||||
vi.stubGlobal("fetch", fetchSpy);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
memStore = new Map<string, string>();
|
||||
storage = {
|
||||
get: async (k: string) => memStore.get(k),
|
||||
set: async (k: string, v: string) => {
|
||||
memStore.set(k, v);
|
||||
},
|
||||
delete: async (k: string) => {
|
||||
memStore.delete(k);
|
||||
},
|
||||
clear: async () => {
|
||||
memStore.clear();
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("sends { refresh_token, namespace } in the request body", async () => {
|
||||
setupGoodResponse();
|
||||
await storage.set("refreshToken", "stored-refresh-tok");
|
||||
await storage.set("namespace", "anchat-test");
|
||||
|
||||
const http = new HttpClient({ baseURL: "https://example.invalid" });
|
||||
const auth = new AuthClient({ httpClient: http, storage });
|
||||
|
||||
await auth.refresh();
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledOnce();
|
||||
const init = fetchSpy.mock.calls[0][1] as RequestInit;
|
||||
expect(init?.body, "refresh() must send a JSON body").toBeDefined();
|
||||
|
||||
const sentBody = JSON.parse(init!.body as string);
|
||||
expect(sentBody).toEqual({
|
||||
refresh_token: "stored-refresh-tok",
|
||||
namespace: "anchat-test",
|
||||
});
|
||||
});
|
||||
|
||||
it("reads access_token from the response and propagates it as the new JWT", async () => {
|
||||
setupGoodResponse();
|
||||
await storage.set("refreshToken", "stored-refresh-tok");
|
||||
await storage.set("namespace", "anchat-test");
|
||||
|
||||
const http = new HttpClient({ baseURL: "https://example.invalid" });
|
||||
const auth = new AuthClient({ httpClient: http, storage });
|
||||
|
||||
const returned = await auth.refresh();
|
||||
|
||||
expect(returned).toBe("NEW-JWT-FROM-SERVER");
|
||||
expect(auth.getToken()).toBe("NEW-JWT-FROM-SERVER");
|
||||
});
|
||||
|
||||
it("rotates the stored refresh token when the server returns a new one", async () => {
|
||||
setupGoodResponse();
|
||||
await storage.set("refreshToken", "old-refresh");
|
||||
await storage.set("namespace", "anchat-test");
|
||||
|
||||
const http = new HttpClient({ baseURL: "https://example.invalid" });
|
||||
const auth = new AuthClient({ httpClient: http, storage });
|
||||
|
||||
await auth.refresh();
|
||||
|
||||
expect(await storage.get("refreshToken")).toBe("rotated-refresh");
|
||||
});
|
||||
|
||||
it("falls back to the 'default' namespace when none stored", async () => {
|
||||
setupGoodResponse();
|
||||
await storage.set("refreshToken", "stored-refresh-tok");
|
||||
// No namespace set.
|
||||
|
||||
const http = new HttpClient({ baseURL: "https://example.invalid" });
|
||||
const auth = new AuthClient({ httpClient: http, storage });
|
||||
|
||||
await auth.refresh();
|
||||
|
||||
const sentBody = JSON.parse(
|
||||
(fetchSpy.mock.calls[0][1] as RequestInit)!.body as string
|
||||
);
|
||||
expect(sentBody.namespace).toBe("default");
|
||||
});
|
||||
|
||||
it("throws (not silently undefined-ing the JWT) when no refresh token is stored", async () => {
|
||||
// No refresh token in storage. Server should never be called.
|
||||
fetchSpy = vi.fn();
|
||||
vi.stubGlobal("fetch", fetchSpy);
|
||||
|
||||
const http = new HttpClient({ baseURL: "https://example.invalid" });
|
||||
const auth = new AuthClient({ httpClient: http, storage });
|
||||
|
||||
await expect(auth.refresh()).rejects.toThrow(/no refresh token/i);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
expect(auth.getToken()).toBeUndefined();
|
||||
});
|
||||
|
||||
it("throws if the server response is missing access_token", async () => {
|
||||
// Server returns 200 but with malformed body (no access_token).
|
||||
fetchSpy = vi.fn(async () => {
|
||||
return new Response(JSON.stringify({ token_type: "Bearer" }), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
});
|
||||
vi.stubGlobal("fetch", fetchSpy);
|
||||
await storage.set("refreshToken", "stored-refresh-tok");
|
||||
|
||||
const http = new HttpClient({ baseURL: "https://example.invalid" });
|
||||
const auth = new AuthClient({ httpClient: http, storage });
|
||||
|
||||
await expect(auth.refresh()).rejects.toThrow(/no access_token/i);
|
||||
// In-memory JWT must NOT be set to undefined — pre-fix this is what
|
||||
// corrupted the auth state.
|
||||
expect(auth.getToken()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Loading…
x
Reference in New Issue
Block a user