Skip to content

fix(storage): mirror bulk upsert validation#60

Merged
aryasaatvik merged 1 commit into
devfrom
fix/bulk-upsert-upstream-parity
Jun 24, 2026
Merged

fix(storage): mirror bulk upsert validation#60
aryasaatvik merged 1 commit into
devfrom
fix/bulk-upsert-upstream-parity

Conversation

@aryasaatvik

Copy link
Copy Markdown
Owner

Summary

Validation

  • bunx vitest run src/query/table-policy.test.ts from packages/core/fumadb
  • bunx oxlint --no-ignore --deny-warnings src/query/orm/index.ts src/adapters/drizzle/query.ts src/adapters/memory/index.ts src/query/table-policy.test.ts from packages/core/fumadb
  • git diff --check

Notes

bun run --cwd packages/core/fumadb typecheck is currently blocked in this checkout by the existing missing @libsql/client import in src/adapters/drizzle/runtime-ensure.test.ts. Root oxlint intentionally ignores packages/core/fumadb/, so the file-level lint was run from the package with --no-ignore.

@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR hardens upsertMany validation by rejecting empty target or update arrays at multiple defensive layers: the public ORM query boundary, the Drizzle adapter, and the memory adapter. It also promotes the update.length === 0 check from adapters-only up to the ORM layer, closing the gap where a caller could hit an unguarded path.

  • orm/index.ts: Both target and update are now validated before any column resolution or policy evaluation runs, so callers get a clear error message rather than a downstream crash or silent misbehavior.
  • Drizzle and memory adapters: Each gains a target.length === 0 guard as defense-in-depth, symmetric with the pre-existing update.length === 0 check.
  • table-policy.test.ts: A new Effect Vitest regression test covers both rejection paths through the public ORM surface.

Confidence Score: 5/5

Safe to merge. All changes are additive validation guards with no behavioral change for valid inputs.

The change adds early-throw guards for previously unvalidated empty-array inputs across three consistent locations. Valid callers are unaffected. The new test exercises both rejection cases through the public ORM surface, matching the style and harness used throughout the suite. No existing behavior is altered.

No files require special attention. The memory adapter's pre-existing absence of a values.length === 0 early-return (unlike the Drizzle adapter) is harmless since iterating an empty array is a no-op, and is unrelated to this PR.

Important Files Changed

Filename Overview
packages/core/fumadb/src/query/orm/index.ts Adds both target.length === 0 and update.length === 0 guards at the public query boundary, before any column resolution or policy evaluation; mirrors previously adapter-only validation.
packages/core/fumadb/src/adapters/drizzle/query.ts Adds the missing v.target.length === 0 guard (defense-in-depth, symmetric with the pre-existing v.update.length === 0 check); placed correctly after the v.values.length === 0 early-return.
packages/core/fumadb/src/adapters/memory/index.ts Adds v.target.length === 0 guard before the pre-existing v.update.length === 0 check; consistent with the Drizzle adapter ordering.
packages/core/fumadb/src/query/table-policy.test.ts Adds a regression test covering both empty-target and empty-update rejection paths through the public ORM boundary; uses correct Effect Vitest pattern consistent with the surrounding suite.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["caller: upsertMany(name, {target, update, values})"] --> B{values.length === 0?}
    B -- yes --> C[return early]
    B -- no --> D{target.length === 0?}
    D -- yes --> E["throw: at least one target column required"]
    D -- no --> F{update.length === 0?}
    F -- yes --> G["throw: at least one update column required"]
    F -- no --> H["resolve columns, apply policies"]
    H --> I{adapter.upsertMany available?}
    I -- yes --> J["adapter.upsertMany()"]
    I -- no --> K["fallback: loop upsert()"]
    J --> L["Drizzle guard: target.length === 0? throw"]
    J --> M["Drizzle guard: update.length === 0? throw"]
    L --> N["execute SQL upsert"]
    M --> N

    style E fill:#f66,color:#fff
    style G fill:#f66,color:#fff
    style L fill:#ffa,color:#000
    style M fill:#ffa,color:#000
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["caller: upsertMany(name, {target, update, values})"] --> B{values.length === 0?}
    B -- yes --> C[return early]
    B -- no --> D{target.length === 0?}
    D -- yes --> E["throw: at least one target column required"]
    D -- no --> F{update.length === 0?}
    F -- yes --> G["throw: at least one update column required"]
    F -- no --> H["resolve columns, apply policies"]
    H --> I{adapter.upsertMany available?}
    I -- yes --> J["adapter.upsertMany()"]
    I -- no --> K["fallback: loop upsert()"]
    J --> L["Drizzle guard: target.length === 0? throw"]
    J --> M["Drizzle guard: update.length === 0? throw"]
    L --> N["execute SQL upsert"]
    M --> N

    style E fill:#f66,color:#fff
    style G fill:#f66,color:#fff
    style L fill:#ffa,color:#000
    style M fill:#ffa,color:#000
Loading

Reviews (1): Last reviewed commit: "fix(storage): mirror bulk upsert validat..." | Re-trigger Greptile

@aryasaatvik aryasaatvik merged commit 7784c0f into dev Jun 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant