-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor database tracking and safety proxy architecture #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
cfaae0b
Refactor database tracking and safety proxy architecture
digitalmio 53a22c1
Fix async error propagation and track nested relations in safety proxy
digitalmio 27b7d6c
Support Drizzle SQL templates in raw database tracking
digitalmio 925379e
Pass query type to trackExec for cascade isolation
digitalmio 8152008
Fix .withoutWhere() to not double-call method
digitalmio d75cdba
Refactor table assignment logic for clarity
digitalmio File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { getTableConfig } from "drizzle-orm/sqlite-core"; | ||
|
|
||
| export function buildPkMap(schema: Record<string, unknown>): Map<string, string[]> { | ||
| const map = new Map<string, string[]>(); | ||
| for (const key in schema) { | ||
| const table = schema[key]; | ||
| if (!table || !(table as any)[Symbol.for("drizzle:Name")]) continue; | ||
| const config = getTableConfig(table as any); | ||
| const pkCols = config.columns.filter((c: any) => c.primary).map((c: any) => c.name); | ||
| map.set(config.name, pkCols); | ||
| } | ||
| return map; | ||
| } | ||
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| import type { TrackContext } from "./createSafetyProxy"; | ||
| import { trackExec, warnRowLimit } from "./tracking"; | ||
|
|
||
| const STATE = Symbol("edgepod_builder_state"); | ||
| const EXEC = ["then", "run", "all", "get", "values", "execute"]; | ||
| const MAX_LIMIT = 1000; | ||
| const MAX_BULK_INSERT = 1000; | ||
|
|
||
| type BuilderConfig = { | ||
| type: "select" | "insert" | "update" | "delete"; | ||
| tableName?: string; | ||
| }; | ||
|
|
||
| export function createBuilderProxy(builder: any, ctx: TrackContext, config: BuilderConfig): any { | ||
| if (!builder[STATE]) { | ||
| builder[STATE] = { limitSet: false, whereSet: false, withoutWhereSet: false }; | ||
| } | ||
|
|
||
| return new Proxy(builder, { | ||
| get(target: any, prop: string) { | ||
| const state: { limitSet: boolean; whereSet: boolean; withoutWhereSet: boolean } = target[ | ||
| STATE | ||
| ] || { limitSet: false, whereSet: false, withoutWhereSet: false }; | ||
|
|
||
| // Safety: limit clamping for SELECT | ||
| if (prop === "limit" && config.type === "select") | ||
| return (n: number) => { | ||
| const clamped = Math.max(0, Math.min(n, MAX_LIMIT)); | ||
| if (n > MAX_LIMIT) ctx.warnings.push(`Query limit of ${n} overridden to ${MAX_LIMIT}.`); | ||
| return wrap(target.limit(clamped), ctx, config, { ...state, limitSet: true }); | ||
| }; | ||
|
|
||
| // Safety: WHERE enforcement for UPDATE/DELETE | ||
| if (prop === "where" && (config.type === "update" || config.type === "delete")) | ||
| return (...args: unknown[]) => | ||
| wrap(target.where(...args), ctx, config, { ...state, whereSet: true }); | ||
| if (prop === "withoutWhere" && (config.type === "update" || config.type === "delete")) | ||
| return () => { | ||
| ctx.warnings.push(`[EdgePod] Unfiltered ${config.type} executed via .withoutWhere().`); | ||
| return wrap(target, ctx, config, { ...state, withoutWhereSet: true }); | ||
| }; | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
|
|
||
| // Safety: bulk insert limit | ||
| if (prop === "values" && config.type === "insert") | ||
| return (...args: unknown[]) => { | ||
| const rows = args[0]; | ||
| if (Array.isArray(rows) && rows.length > MAX_BULK_INSERT) | ||
| throw new Error( | ||
| `[EdgePod] Bulk insert blocked: ${rows.length} rows > ${MAX_BULK_INSERT}. Split into smaller batches.`, | ||
| ); | ||
| return wrap(target.values(...args), ctx, config, state); | ||
| }; | ||
|
|
||
| // Safety: prepare | ||
| if (prop === "prepare" && config.type !== "select") | ||
| return () => { | ||
| throw new Error(`[EdgePod] .prepare() is not supported for ${config.type}s.`); | ||
| }; | ||
| if (prop === "prepare") | ||
| return (...args: unknown[]) => { | ||
| const b = state.limitSet ? target : target.limit(MAX_LIMIT); | ||
| return (b as any).prepare(...args); | ||
| }; | ||
|
|
||
| // Execution: safety check → track → execute | ||
| if (EXEC.includes(prop)) | ||
| return (...args: unknown[]) => { | ||
| if ( | ||
| !state.whereSet && | ||
| !state.withoutWhereSet && | ||
| (config.type === "update" || config.type === "delete") | ||
| ) | ||
| throw new Error( | ||
| `[EdgePod] ${config.type.toUpperCase()} without WHERE is blocked. If intentional, chain .withoutWhere().`, | ||
| ); | ||
|
|
||
| let b = target; | ||
| if (config.type === "select" && !state.limitSet) b = target.limit(MAX_LIMIT); | ||
|
|
||
| trackExec(b, ctx, config.tableName, config.type); | ||
|
|
||
| if (prop === "then") { | ||
| const [resolve, reject] = args as [(v: unknown) => void, (e: unknown) => void]; | ||
| return b.then((res: unknown) => { | ||
| warnRowLimit(res, ctx.warnings); | ||
| resolve(res); | ||
| }, reject); | ||
| } | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
|
|
||
| const method = b[prop] as Function; | ||
| const result = method.apply(b, args); | ||
| warnRowLimit(result, ctx.warnings); | ||
| return result; | ||
| }; | ||
|
|
||
| // Generic pass-through: wrap if result is a builder | ||
| return passThrough(target, prop, ctx, config, state); | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| function wrap( | ||
| result: any, | ||
| ctx: TrackContext, | ||
| config: BuilderConfig, | ||
| state: { limitSet: boolean; whereSet: boolean; withoutWhereSet: boolean }, | ||
| ): any { | ||
| if (isBuilder(result)) { | ||
| result[STATE] = state; | ||
| return createBuilderProxy(result, ctx, config); | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| function passThrough( | ||
| target: any, | ||
| prop: string, | ||
| ctx: TrackContext, | ||
| config: BuilderConfig, | ||
| state: { limitSet: boolean; whereSet: boolean; withoutWhereSet: boolean }, | ||
| ): any { | ||
| const raw = target[prop]; | ||
| if (typeof raw === "function") | ||
| return (...args: unknown[]) => { | ||
| const result = raw.apply(target, args); | ||
| return isBuilder(result) | ||
| ? createBuilderProxy(Object.assign(result, { [STATE]: state }), ctx, config) | ||
| : result; | ||
| }; | ||
| if (isBuilder(raw)) { | ||
| raw[STATE] = state; | ||
| return createBuilderProxy(raw, ctx, config); | ||
| } | ||
| return raw; | ||
| } | ||
|
|
||
| function isBuilder(v: unknown): boolean { | ||
| return typeof v === "object" && v !== null && !Array.isArray(v); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.primarymay not be the correct Drizzle column propertyconfig.columns.filter((c) => c.primary)uses the.primaryfield to detect primary-key columns. In the Drizzle ORM SQLite adapter the canonical property exposed bygetTableConfigisc.primaryKey(a boolean), notc.primary. If the property name is wrong, everypkColsarray will be empty andpkMapwill be populated but useless for future consumers. Worth verifying against the installed Drizzle version beforepkMapis wired into invalidation logic.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proof that .primary is correct and .primaryKey is wrong:
this.primary = config.primaryKey;
The constructor maps config.primaryKey (config-time property) to this.primary (instance property).
const config = getTableConfig(users);
for (const col of config.columns) {
console.log(col.name, '| primary:', col.primary, '| primaryKey:', col.primaryKey);
}
Output:
id | primary: true | primaryKey: undefined
name | primary: false | primaryKey: undefined
Greptile confused the config-time property name (primaryKey in the builder API: integer("id").primaryKey()) with the runtime instance property name (primary on the column object returned by getTableConfig). The code correctly uses the runtime property .primary.