Skip to content

Commit 02309cd

Browse files
chrisbbreuerclaude
andcommitted
fix(orm): read and write attributes under their declared name
Attributes are stored under their column name, but models declare them in camelCase and every other surface already accepts that spelling: `create({ pollIntervalMinutes })` writes it, `where('pollIntervalMinutes')` queries it, and `ModelRow` types the property as present. Property reads were the one surface that disagreed, resolving only the column name. So `row.pollIntervalMinutes` was undefined on every multi-word attribute while typechecking clean. That is the worst shape a bug can take: the type says the field is there, the read silently yields nothing, and the value usually disappears into a `|| default` instead of failing. It cost a real feature a poll interval, a failure counter, and a stored API key before anyone noticed. The instance proxy now falls back to the column name on get, set, and `in`. The fallback resolves last, so it cannot shadow an attribute, relation, or trait method — it only answers where the read was already undefined. Writes route to the existing column instead of inventing a camelCase one that save() would then fail to persist. `ownKeys` still reports column names only, so spreads, `Object.keys`, and JSON responses keep exactly the shape they had. This adds a way to read an attribute; it does not change the wire format. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent f927cda commit 02309cd

2 files changed

Lines changed: 160 additions & 0 deletions

File tree

storage/framework/core/orm/src/define-model.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,24 @@ function wrapModelInstance<T extends object>(
635635
: (...args: any[]) => fn((target as any).id, ...args)
636636
}
637637
}
638+
639+
// Attributes are stored under their column name, but models declare
640+
// them in camelCase and every other surface accepts that spelling:
641+
// `create({ pollIntervalMinutes })` writes, `where('pollIntervalMinutes')`
642+
// queries, and `ModelRow` types the property as present. Only property
643+
// reads disagreed, returning undefined for every multi-word attribute
644+
// while typechecking clean — so the mistake was invisible and the value
645+
// usually vanished into a `|| default`.
646+
//
647+
// Resolved last, so it can never shadow a real attribute, relation, or
648+
// trait method: it only answers where the read was already undefined.
649+
// `ownKeys` deliberately still reports column names only, which keeps
650+
// spreads, `Object.keys`, and JSON responses byte-identical.
651+
if (a) {
652+
const column = snakeCase(prop)
653+
if (column !== prop && Object.prototype.hasOwnProperty.call(a, column))
654+
return a[column]
655+
}
638656
}
639657
const v = Reflect.get(target, prop, target)
640658
return typeof v === 'function' ? v.bind(target) : v
@@ -659,6 +677,18 @@ function wrapModelInstance<T extends object>(
659677
else a[prop] = value
660678
return true
661679
}
680+
// `inst.pollIntervalMinutes = 5` must land on `poll_interval_minutes`.
681+
// Falling through to the new-attribute branch below would add a second
682+
// key under the camelCase name, which save() then tries to write as a
683+
// column that does not exist.
684+
if (a) {
685+
const column = snakeCase(prop)
686+
if (column !== prop && Object.prototype.hasOwnProperty.call(a, column)) {
687+
if (typeof setter === 'function') setter.call(target, column, value)
688+
else a[column] = value
689+
return true
690+
}
691+
}
662692
// New attribute key not yet in _attributes — still write through
663693
// so `inst.newField = x` followed by save() works.
664694
if (a && !(prop in (target as object))) {
@@ -675,6 +705,11 @@ function wrapModelInstance<T extends object>(
675705
if (a && Object.prototype.hasOwnProperty.call(a, prop)) return true
676706
const rels = (target as any)._relations
677707
if (rels && Object.prototype.hasOwnProperty.call(rels, prop)) return true
708+
// Keep `in` agreeing with what `get` will actually resolve.
709+
if (a) {
710+
const column = snakeCase(prop)
711+
if (column !== prop && Object.prototype.hasOwnProperty.call(a, column)) return true
712+
}
678713
}
679714
return Reflect.has(target, prop)
680715
},
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/**
2+
* Multi-word attributes must be readable under the name the model declared.
3+
*
4+
* Attributes are stored under their column name. Every other surface already
5+
* accepts the declared camelCase spelling — `create({ pollIntervalMinutes })`
6+
* writes it, `where('pollIntervalMinutes', 30)` queries it, and `ModelRow`
7+
* types the property as present — but property reads resolved only the column
8+
* name. `row.pollIntervalMinutes` was therefore `undefined` while typechecking
9+
* clean, so the mistake was invisible and the value usually disappeared into a
10+
* `|| default`.
11+
*
12+
* Serialization deliberately does NOT change: `ownKeys` still reports column
13+
* names only, so spreads, `Object.keys`, and JSON responses keep the exact
14+
* shape they had.
15+
*/
16+
import { afterAll, beforeAll, describe, expect, it } from 'bun:test'
17+
import { Database } from 'bun:sqlite'
18+
import { configureOrm, getDatabase } from 'bun-query-builder'
19+
import { acquireDbConfigLock } from '@stacksjs/database'
20+
import { defineModel } from '../src/define-model'
21+
22+
describe('camelCase attribute accessors', () => {
23+
let db: Database
24+
let releaseDbConfigLock: () => void
25+
26+
beforeAll(async () => {
27+
releaseDbConfigLock = await acquireDbConfigLock()
28+
configureOrm({ database: ':memory:' })
29+
db = getDatabase()
30+
db.run(`CREATE TABLE cc_profiles (
31+
id INTEGER PRIMARY KEY AUTOINCREMENT,
32+
display_name TEXT,
33+
poll_interval_minutes INTEGER,
34+
last_mention_at TEXT,
35+
status TEXT
36+
)`)
37+
})
38+
39+
afterAll(() => {
40+
releaseDbConfigLock()
41+
})
42+
43+
const Profile = defineModel({
44+
name: 'CcProfile',
45+
table: 'cc_profiles',
46+
primaryKey: 'id',
47+
autoIncrement: true,
48+
attributes: {
49+
displayName: { type: 'string', fillable: true },
50+
pollIntervalMinutes: { type: 'number', fillable: true },
51+
lastMentionAt: { type: 'string', fillable: true, required: false },
52+
status: { type: 'string', fillable: true },
53+
},
54+
} as const)
55+
56+
async function seed() {
57+
return await (Profile as any).create({
58+
displayName: 'Riverside', pollIntervalMinutes: 30,
59+
lastMentionAt: '2026-08-17T10:00:00.000Z', status: 'active',
60+
})
61+
}
62+
63+
it('reads a multi-word attribute under its declared name', async () => {
64+
const created = await seed()
65+
const row = await (Profile as any).find(Number(created.id))
66+
67+
expect(row.pollIntervalMinutes).toBe(30)
68+
expect(row.displayName).toBe('Riverside')
69+
expect(row.lastMentionAt).toBe('2026-08-17T10:00:00.000Z')
70+
})
71+
72+
it('still reads the column name, so existing call sites keep working', async () => {
73+
const created = await seed()
74+
const row = await (Profile as any).find(Number(created.id))
75+
76+
expect(row.poll_interval_minutes).toBe(30)
77+
expect(row.display_name).toBe('Riverside')
78+
})
79+
80+
it('resolves through a query chain, not just find()', async () => {
81+
await seed()
82+
const row = await (Profile as any).where('status', 'active').first()
83+
expect(row.pollIntervalMinutes).toBe(30)
84+
})
85+
86+
it('reports the declared name from the `in` operator', async () => {
87+
const created = await seed()
88+
const row = await (Profile as any).find(Number(created.id))
89+
90+
expect('pollIntervalMinutes' in row).toBe(true)
91+
expect('poll_interval_minutes' in row).toBe(true)
92+
expect('notAnAttribute' in row).toBe(false)
93+
})
94+
95+
it('leaves serialization on column names', async () => {
96+
const created = await seed()
97+
const row = await (Profile as any).find(Number(created.id))
98+
99+
// The wire shape must not change: no duplicated camelCase fields in an
100+
// API response, no surprise growth in payload size.
101+
expect(Object.keys(row)).toContain('poll_interval_minutes')
102+
expect(Object.keys(row)).not.toContain('pollIntervalMinutes')
103+
expect(Object.keys({ ...row })).not.toContain('pollIntervalMinutes')
104+
expect(JSON.stringify(row)).not.toContain('pollIntervalMinutes')
105+
})
106+
107+
it('writes through the declared name to the real column', async () => {
108+
const created = await seed()
109+
const row = await (Profile as any).find(Number(created.id))
110+
111+
row.pollIntervalMinutes = 90
112+
await row.save()
113+
114+
const stored = db.query('SELECT poll_interval_minutes FROM cc_profiles WHERE id = ?').get(Number(created.id)) as any
115+
expect(stored.poll_interval_minutes).toBe(90)
116+
// and no stray camelCase column was invented
117+
expect(Object.keys(stored)).not.toContain('pollIntervalMinutes')
118+
})
119+
120+
it('returns undefined for a name that is not an attribute', async () => {
121+
const created = await seed()
122+
const row = await (Profile as any).find(Number(created.id))
123+
expect(row.somethingElseEntirely).toBeUndefined()
124+
})
125+
})

0 commit comments

Comments
 (0)