You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
src/webserver/database/sqlpage_functions/functions.rs is a 1161-line god-file with no internal boundaries. It is one of the most-churned files in the DB module, and it mixes the registry of every built-in SQL function with ~40 unrelated implementations: HTTP fetch, password hashing, file persistence, regex, HMAC, OIDC claim extraction, subprocess exec, and the recursive run_sql engine entry point. Navigation is by scrolling, security-sensitive code sits next to unrelated helpers, and unit tests are interleaved with implementations.
This splits out the refactor item from #1249 into a concrete, actionable issue.
Why this matters
Unrelated and security-sensitive concerns are interleaved in a single file, which raises cognitive load and regression risk on a file that changes often:
Subprocess exec sits a few lines from the HTTP request builders, run_sql (the recursive engine entry point) is in the same flat file as hash_password and hmac.
The OIDC user_info claim resolver is a long hand-maintained 30+ arm match over claim names that has no business living next to subprocess and SQL execution code.
Proposed change
Split the implementations into topical submodules under sqlpage_functions/.
At compile time, iterate of files in the sqlpage_functions folder and generate one function per file. Use convention over configuration, adding a new file in the folder is all that should be needed to create a new sqlpage function.
The short readme in sqlpage_functions should explain the mechanism.
Risks and mitigations
Low risk; this is a mechanical move with no behavior change.
The macro registry keeps wiring centralized, so the public function surface does not change.
Move implementations file-by-file, one module at a time, each a self-contained reviewable commit.
Existing function tests (the interleaved test_* plus the end-to-end SQL function tests) pin behavior across the move.
Expected win
Better navigability and reviewability of a frequently-edited file, security-sensitive exec/run_sql isolated and easy to audit, and removing the fetch/fetch_with_meta duplication eliminates a drift hazard.
Split
functions.rsinto one file per functionsrc/webserver/database/sqlpage_functions/functions.rsis a 1161-line god-file with no internal boundaries. It is one of the most-churned files in the DB module, and it mixes the registry of every built-in SQL function with ~40 unrelated implementations: HTTP fetch, password hashing, file persistence, regex, HMAC, OIDC claim extraction, subprocessexec, and the recursiverun_sqlengine entry point. Navigation is by scrolling, security-sensitive code sits next to unrelated helpers, and unit tests are interleaved with implementations.This splits out the refactor item from #1249 into a concrete, actionable issue.
Why this matters
Unrelated and security-sensitive concerns are interleaved in a single file, which raises cognitive load and regression risk on a file that changes often:
execsits a few lines from the HTTP request builders,run_sql(the recursive engine entry point) is in the same flat file ashash_passwordandhmac.test_random_string,test_hash_password,test_hmac.fetchandfetch_with_metarepeat the same tracing span setup, client creation, request build, and body preparation. Two copies of this pipeline is a drift hazard (a fix to one, like the body-log redaction in Codebase review: high-priority security, reliability, maintainability, and bloat findings #1249, can silently miss the other).user_infoclaim resolver is a long hand-maintained 30+ armmatchover claim names that has no business living next to subprocess and SQL execution code.Proposed change
Split the implementations into topical submodules under
sqlpage_functions/.At compile time, iterate of files in the sqlpage_functions folder and generate one function per file. Use convention over configuration, adding a new file in the folder is all that should be needed to create a new sqlpage function.
The short readme in sqlpage_functions should explain the mechanism.
Risks and mitigations
Low risk; this is a mechanical move with no behavior change.
test_*plus the end-to-end SQL function tests) pin behavior across the move.Expected win
Better navigability and reviewability of a frequently-edited file, security-sensitive
exec/run_sqlisolated and easy to audit, and removing thefetch/fetch_with_metaduplication eliminates a drift hazard.Refs #1249 (item 6).