fix(objectql): every engine verb folds filter → where — delete({filter}) no longer empties the object (#4346) - #4349
fix(objectql): every engine verb folds filter → where — delete({filter}) no longer empties the object (#4346)#4349os-zhuang wants to merge 2 commits into
filter → where — delete({filter}) no longer empties the object (#4346)#4349Conversation
…filter})` no longer empties the object (#4346) The driver AST understands `where` only, so an unfolded `filter` is not a narrower query — it is no query. `find()` folded it (with a comment saying exactly that, from the ADR-0057 over-grant); `findOne`/`count`/`aggregate`/ `update`/`delete` never did. Measured: findOne returned a non-matching row, count counted the whole object, and the write verbs reached updateMany / deleteMany with no predicate — rewriting every row, emptying the object. Reachable from the documented surface, not just internals: ScopedContext (`ctx.api.object('x')` in an L2 hook) forwards its bag verbatim with every param typed `any`, and the spec's own hook TSDoc taught `findOne({ filter: ... })` — corrected here. Each entry point now makes one call to the shared #3795 fold (foldQueryAliasSlots + ENGINE_FILTER_ALIAS_SLOTS) instead of five more copies of find's guard, and copies the caller's bag rather than mutating it. Same pass, the sixth alias pair: `top` is declared as the alias of `limit`, but protocol.ts overwrote `limit` with it unguarded while the engine kept `limit` — one query, two answers by path. Both read ENGINE_QUERY_ALIAS_SLOTS now; `top` stays a declared AST key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
📓 Docs Drift CheckThis PR changes 3 package(s): 110 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…t-alias-fold # Conflicts: # packages/metadata-protocol/src/protocol.ts
|
Superseded by #4356, which landed the same fix while this was in CI — closing rather than merging, since merging would leave two implementations of one fold. Verified theirs covers everything here, and then some: same six entry points, same #4181 conflict rule, same fold-before- Two small deltas from this branch, recorded here rather than lost — neither worth a follow-up PR on its own, both worth knowing if that code is touched again:
One observation on the spec side, not a defect: |
1 participant
Closes #4346. Started as the #3795 follow-up on the sixth alias pair (
top/limit) and turned up a P1 one layer down: thefilter→wherefold existed in one of six engine verbs.What was broken
ObjectQL.find()folds the alias, and its comment says why it must — the driver AST understandswhereonly, so an unfoldedfilteris not a narrower query, it is no query. The five siblings never got that fold.Measured against a real
ObjectQL+ a driver that appliesast.where(three rows:a: open,b: done,c: done), predicate passed asfilter:find({filter: done})[b, c]✅findOne({filter: done})a— a row that does not matchbcount({filter: done})aggregate({filter: done, …})update(data, {filter: done, multi})b,cdelete({filter: done, multi})b,cdeletedThe write verbs reached
driver.updateMany/deleteManywithast.where === undefined, which every driver reads as "no predicate". This is not a read over-grant like the ADR-0057 finding that put the fold infind()— it is an unbounded write.Why it was reachable
ScopedContext— the cross-object API handed to L2 hook bodies asctx.api.object('x')— forwards its argument bag verbatim, every parameter typedany:and the spec's own hook TSDoc taught the broken call —
packages/spec/src/data/hook.zod.ts:So a hook author following the documented example got an arbitrary user, and a cleanup hook doing
users.delete({ filter: …, multi: true })emptied the object. That example is corrected towherehere. The deprecatedDataEngine{Query,Update,Delete,Count}OptionsSchemaalso still declarefilterfor exactly these verbs, so "callers should not pass it" was never the contract.The fix
One call per entry point to the shared #3795 machinery (
foldQueryAliasSlots+ the newENGINE_FILTER_ALIAS_SLOTS), rather than five more copies offind's guard — the point of #3795 was to decide precedence once, and the engine is the third reader that sweep did not reach. Details that matter:withResolvedWhere(Flow node filters silently blank date macros: the template engine consumes{…}before the query engine sees it #3810 placeholder expansion), before the by-id fast path, and before the seeded AST the middleware chain scopes (Security: bulk (multi) update/delete skips OWD owner scoping on private objects — members can modify others' rows #2982). All of those readoptions.where; folding later would have fixed the symptom and left them blind.findOnefolds only thewhereslot — it forceslimit: 1itself, so a caller'stopis discarded as before; feeding it to the pagination slot would manufacture a conflict out of an unambiguous request.400 INVALID_REQUEST), the REST 列表：无法解析的filterJSON 被静默忽略 —— 返回未过滤整页（#4134/#4164 家族第三员） #4181 rule now applied at the engine layer too. The error carriesstatus/codeso a call arriving via REST surfaces as a 400, not a 500.The sixth pair, same pass
topis documented as "Alias for limit (OData compatibility)" on bothQuerySchemaandEngineQueryOptionsSchema, solimitis canonical — butprotocol.tsfolded it unguarded (options.limit = Number(options.top), the alias overwriting the canonical key) whileengine.findkeptlimit.{top: 1, limit: 3}answered 1 over HTTP and 3 through a direct engine call: #3795's exact divergence on the pair its scope note excluded. Both now readENGINE_QUERY_ALIAS_SLOTS.topdeliberately stays a declared AST key rather than joiningRPC_QUERY_ALIAS_SLOTS: those five are deprecated RPC keys the parse transform drops from its output, and removing a declared key from the parsed type is a breaking change that deserves its own decision. Giving it one precedence did not have to wait for that.Deliberately not in scope
The engine still does not fold
select/sort/skip/populate, which the deprecated query-options shape also declares. Those fail narrow (an ignored projection returns more columns, an ignored sort returns arbitrary order) rather than wide, andsort/populateneed value-shape lowering the engine has no equivalent of — doing that inside a P1 fix is how you ship a second bug. Worth its own pass; noted on #4346.Verification
engine-option-alias-fold.test.ts), including two that assert the write verbs touch only matched rows — the regression here is silent and destructive, and the suite had nothing covering it.top→limitfolding in the engine's direction,$topstill reaching the same slot, and the conflict rejection.objectql1411 /spec7156 /metadata-protocol136 green; fullpnpm test132/132 tasks green.check:generated8/8 green;api-surface.jsonregenerated (+2 exports).@objectstack/objectql+@objectstack/metadata-protocol, minor@objectstack/spec(new exports).Behaviour changes, all toward one answer: a predicate passed as
filternow actually applies on every verb, and two spellings of one slot with different values are refused instead of silently resolved. A single spelling, and redundant identical spellings, behave exactly as before.Refs #3795, #4181, #3713, ADR-0057.
🤖 Generated with Claude Code