diff --git a/AGENTS.md b/AGENTS.md index 6448e0b28..953898163 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,147 +1,45 @@ -# SQLPage architecture - -SQLPage is an SQL-only web application builder and web server. An application is primarily a set of `.sql` -files: SQLPage routes an HTTP request to a file, executes its statements against a database, interprets rows -whose `component` column names a UI component, and streams the resulting HTML (or another response) to the -client. It is intended for fast, data-centric applications while still allowing custom HTML, CSS, and -JavaScript where needed. - -## Features and repository layout - -- **Application entry point and configuration** (`src/main.rs`, `src/lib.rs`, `src/app_config.rs`, `src/cli/`). - The executable starts the server; application state, configuration, environment variables, and command-line - handling are defined here. `configuration.md` documents the user-facing settings. -- **SQL semantics and execution** (`src/webserver/database/`). SQLPage uses the database's SQL for selects, joins, aggregation, inserts, updates, - deletes, transactions, JSON processing, and database-specific features. It parses SQL, recognizes SQLPage - extensions, binds request values safely, and sends ordinary SQL to the selected database. SQL files contain - sequential statements; result sets become component invocations in response order. `SET` assigns a value - to a mutable SQLPage variable and is useful for reusing query results or controlling later statements. -- **Request variables** (`src/webserver/request_variables.rs`, `src/webserver/http_request_info.rs`, - `src/webserver/database/syntax_tree.rs`). `?name` refers to a URL/GET parameter, `:name` explicitly refers to a form/POST - value, and `$name` is the compatibility shorthand that uses a POST value when present and otherwise a GET - value (a SET variable takes precedence where applicable). Values are passed as parameters, not interpolated - into SQL. GET and POST variables are request inputs; SET variables are mutable during request execution. - `sqlpage.variables()` exposes them as JSON, with SET > POST > GET precedence. -- **SQLPage functions** (`src/webserver/database/sqlpage_functions/`). Calls such as `sqlpage.fetch`, `sqlpage.run_sql`, `sqlpage.set_variable`, file - readers, hashing/HMAC helpers, request metadata, uploads, headers/cookies, URL helpers, OIDC user info, - and HTTP fetch are registered in `src/webserver/database/sqlpage_functions/functions.rs`. Functions can - return values, alter response/request state, include another SQL file, or raise an error. `sqlpage.exec` - is deliberately disabled by default because it runs server processes. -- **Database support and pooling** (`src/webserver/database/connect.rs`, `execute_queries.rs`, `migrations.rs`). - Native drivers support SQLite, PostgreSQL, MySQL, and Microsoft SQL - Server; the ODBC driver provides access to other ODBC-compatible databases. SQLPage uses `sqlx` and a - reusable connection pool, with configurable maximum connections, idle/lifetime timeouts, acquire timeout, - retries, and optional `on_connect.sql`/`on_reset.sql` hooks. Database-specific SQL should be isolated or - covered by the relevant database tests. -- **Rendering and components** (`src/render.rs`, `src/templates.rs`, `src/dynamic_component.rs`, - `src/template_helpers.rs`, `sqlpage/templates/`, `frontend/src/`). Built-in components live in `sqlpage/templates/*.handlebars` and cover shells, text, - tables, lists, cards, charts, forms, navigation, modals, downloads, maps, and more. Query columns map to - component properties; nested/dynamic components and `sqlpage.run_sql` support composition and lazy loading. - Custom Handlebars components can be placed in the configured `sqlpage/templates` directory. Raw HTML and - custom assets are possible through the HTML/shell components. Rendering is streamed so the response can - start while later query results are still being processed. -- **Control flow and errors** (`src/webserver/error.rs`, `error_with_status.rs`, `routing.rs`, - `src/default_404.sql`). SQL remains declarative: use predicates, `CASE`, `SET`, component rows, and - the `redirect` component to conditionally continue, redirect, or implement guards/error pages. There is no - general SQLPage `IF` statement. Parse, database, function, component, and response errors are converted to - contextual HTTP errors; `default_404.sql` handles missing routes. Do not hide errors by changing unrelated - error handling or tests. -- **HTTP server and client** (`src/webserver/http.rs`, `http_client.rs`, `response_writer.rs`, `static_content.rs`, - `https.rs`, `content_security_policy.rs`, `server_timing.rs`). The server is built on Actix Web, supports normal HTTP request/response - handling, streaming, uploads, static assets, HTTP/2, HTTPS, and optional Unix sockets/serverless adapters. - The shared outbound client is used by HTTP-fetch and OIDC integrations and honors configured/native TLS - certificates and timeouts. Content-security-policy and response/header helpers are part of the request - pipeline. -- **OIDC** (`src/webserver/oidc.rs`, `src/webserver/database/sqlpage_functions/functions/user_info.rs`). - Optional OpenID Connect middleware protects configured path prefixes, performs provider discovery, - login/callback/logout, validates tokens, maintains an authenticated cookie, and exposes identity claims to - SQLPage functions. Configuration is in `AppConfig`/`configuration.md`; public paths can be excluded. -- **Caching and files** (`src/filesystem.rs`, `src/file_cache.rs`, `src/telemetry*.rs`). Parsed SQL files are cached. Files may come from the web root/filesystem or from the - database-backed `sqlpage_files` store, and templates/migrations are loaded from the configuration directory. - Telemetry, request timing, and debug logging help diagnose query, pool, and rendering performance. - -- **Examples, tests, and project operations** (`examples/`, `tests/`, `configuration.md`, `CONTRIBUTING.md`, - `README.md`, `.github/workflows/ci.yml`). Examples include the official documentation site and its migrations; - tests cover SQL fixtures, database variants, uploads, OIDC, and server timing. The contribution guide and CI - workflow define the development and validation conventions. -- **Deployment and local infrastructure** (`Dockerfile`, `lambda.Dockerfile`, `docker-compose.yml`, - `sqlpage.service`). These provide container, serverless, local database-testing, and service deployment support. - -## Documentation and release notes - -The official documentation site is itself an SQLPage application in `examples/official-site/`. Its database -schema and documentation content are created by the SQL migrations in -`examples/official-site/sqlpage/migrations/`; the site is recreated from scratch during deployment. Existing -official-site migrations are editable source files: update the migration that already documents a component, -function, configuration option, or feature in place. Do not create a new migration merely to update existing -documentation. Add a new migration only for genuinely new documentation content when that is the established -pattern for the relevant area. - -- Add or update a component's row, parameters, and examples in the component documentation migrations when - changing `sqlpage/templates/` or component behavior. -- Add or update a function's description, parameters, examples, and caveats in the function documentation - migrations when changing `sqlpage_functions` or any `sqlpage.*` function behavior. -- Update the relevant configuration documentation when adding or changing `AppConfig` settings, environment - variables, defaults, authentication behavior, HTTP/TLS behavior, database settings, or custom components. -- Document OIDC changes in the authentication/OIDC migrations, including configuration requirements, exposed - claims/functions, login/logout behavior, and security implications. -- Document other user-visible behavior—SQL syntax extensions, variables, control flow, errors, uploads, - rendering, HTTP endpoints, performance, or deployment—in the corresponding official-site SQL page or - migration. Follow nearby migrations and keep examples executable and database-portable where possible. -- Update `CHANGELOG.md` for user-visible changes only (new features, bug fixes, breaking changes, deprecations). Keep the entry concise and not too technical, focusing on the impact for users. Don't update the entry for a version that was already released (tagged). - -## Validation - -### When working on rust code -Mandatory formatting (rust): `cargo fmt --all` -Mandatory linting: `cargo clippy --all-targets --all-features -- -D warnings` - -### When working on css or js -Frontend formatting: `npm run format` -Mandatory frontend validation: `npm test` - -More about testing: see [github actions](./.github/workflows/ci.yml). -Contributor setup and validation: see [CONTRIBUTING.md](./CONTRIBUTING.md). Module overview: see [src/lib.rs](./src/lib.rs); architecture diagram: [docs/architecture-detailed.png](./docs/architecture-detailed.png). - -NEVER reformat/lint/touch files unrelated to your task. Always run tests/lints/format before stopping when you changed code. - -### Testing - -``` -cargo test # tests with in-memory SQLite by default -``` - -For other databases, see [docker testing setup](./docker-compose.yml) - -``` -docker compose up --wait mssql # or postgres, mysql, mariadb, oracle -DATABASE_URL='mssql://root:Password123!@localhost/sqlpage' cargo test -``` - -ODBC tests require the database-specific ODBC driver on the host; starting the container is not sufficient. See the Oracle and DuckDB matrix entries in [CI](./.github/workflows/ci.yml) for driver setup and connection strings. On Linux and macOS, `cargo test --features odbc-static` matches CI's static unixODBC linking. - -For dynamic frontend changes, run the Playwright tests under `tests/end-to-end/` as described in [CONTRIBUTING.md](./CONTRIBUTING.md). Component browser tests belong in `tests/end-to-end/fixtures//{index.sql,test.ts}` and must import the shared `fixture.ts` harness. Exercise components through SQL fixtures and normal page initialization; do not inject synthetic component DOM or call private initialization functions. Prefer Playwright locators and web-first assertions. For examples containing `test.hurl`, run `scripts/test-examples-hurl.sh `. - -### Documentation - -Components and functions are documented in [official-site migrations](./examples/official-site/sqlpage/migrations/). Edit the existing migration for an existing entity; add an appropriately ordered migration for a new entity. The official-site database is recreated from migrations on each deployment. - -official documentation website sql tables: - - `parameter_type(name)` -- the allowed values of `parameter.type` - - `component(name,description,icon,introduced_in_version)` -- icon name from tabler icon - - `parameter(top_level BOOLEAN, name, component REFERENCES component(name), description, description_md, type REFERENCES parameter_type(name), optional BOOLEAN)` parameter types: BOOLEAN, COLOR, HTML, ICON, INTEGER, JSON, REAL, TEXT, TIMESTAMP, URL. Set exactly one of `description` (plain text) and `description_md` (markdown). - - `example(component REFERENCES component(name), description, properties JSON)` - - `sqlpage_functions(name,icon,description_md,return_type,introduced_in_version)` - - `sqlpage_function_parameters(function,index,name,description_md,type)` - - `blog_posts(title,description,icon,external_url,content,created_at)` -- release announcements and long-form guides - - `example_cards(title,folder,db_engine,description)` - -#### Project Conventions - -- Built-in UI component templates: `sqlpage/templates/*.handlebars`; header/control components: `src/render.rs`. -- SQLPage functions: one `async fn` module under `src/webserver/database/sqlpage_functions/functions/`, declared with `mod` and registered with `sqlpage_functions!` in `functions.rs`. See its [README](./src/webserver/database/sqlpage_functions/README.md). -- [Configuration](./configuration.md): see [AppConfig](./src/app_config.rs) -- Routing: file-based in `src/webserver/routing.rs`. Missing paths use the nearest ancestor `404.sql`; without one, HTML uses `src/default_404.sql` and other formats receive a plain-text 404. -- Playwright component suites: `tests/end-to-end/fixtures//{index.sql,test.ts}` using `tests/end-to-end/fixture.ts`; official-site smoke tests remain in `tests/end-to-end/*.spec.ts`. -- Follow patterns from similar modules before introducing new abstractions. -- frontend: see [frontend/src](./frontend/src/), bundled by `npm run build` +# Working on SQLPage + +## Keep guidance small + +This file records durable architectural constraints and instructions for agents. +[CONTRIBUTING.md](./CONTRIBUTING.md) covers contributor setup and validation; +[CI](./.github/workflows/ci.yml) defines the supported test matrix. Discover module +layout, APIs, schemas, and detailed behavior from the code and nearby tests. + +Edit and prune these documents rather than appending after every task. Add guidance +only when it expresses a lasting rule that cannot be made clear in code. Put local +implementation caveats beside the implementation or regression test. Keep user-facing +reference material in the official documentation site. Do not duplicate it here. + +## Architectural contracts + +- SQL files execute statements sequentially and stream component rows in response + order. Preserve streaming, cancellation, transaction rollback, and contextual + errors; do not hide failures through unrelated error-handling changes. +- Request inputs are bound parameters, never interpolated into SQL. Preserve the + distinction between request inputs and mutable SET variables, including explicit + NULL and child execution isolation. +- Database behavior must remain portable across supported drivers. Isolate + engine-specific SQL and verify execution, binding, and decoding changes against + the affected CI matrix entries. +- Nested SQL execution must release an active fetch stream before reusing its + connection. Preserve one-connection regression coverage, output column ordering, + and exclusion of private function inputs from responses. +- Extend existing module and fixture patterns before introducing abstractions. + Browser component tests must exercise SQL fixtures and normal page initialization + through the shared Playwright harness. + +## Changes and validation + +Keep changes scoped to the task, including formatting. Reuse existing tests and +parameterize repeated setup without removing assertions or scenarios. Run the +relevant tests and the formatting/lint checks in CONTRIBUTING.md before stopping. +Report unavailable checks and failures accurately; a test summary followed by a +hung process is a failure. + +The official site in `examples/official-site/` is rebuilt from its SQL migrations +on deployment. Update the existing page or documentation migration for changed +behavior; add a migration only for new documentation content. Document user-visible +changes there and in configuration.md when applicable. Add a concise CHANGELOG.md +entry only for user-visible changes, and never edit an already tagged release. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0d46ee99b..9ffaf4ebb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,261 +1,84 @@ # Contributing to SQLPage -Thank you for your interest in contributing to SQLPage! This document will guide you through the contribution process. +## Setup and build -## Development Setup - -1. Install Rust and Cargo (latest stable version): https://www.rust-lang.org/tools/install -2. Install Node.js: https://nodejs.org/en/download/ -3. Clone the repository - -```bash -git clone https://github.com/sqlpage/sqlpage -cd sqlpage -``` - -## Building the project - -The first time you build the project, Cargo and npm download their -dependencies, so you will need internet access, and the build may take a while. - -Run this command from the root of a checkout to install the locked npm dependencies, build the browser assets, and build SQLPage in development mode: +Install stable Rust and Node.js (see [CI](./.github/actions/install-frontend-dependencies/action.yml) +for the Node version), then run this command from a checkout: ```bash npm run build:rust ``` -After the browser assets have been built, you can run `cargo build` directly for Rust-only changes. Run `npm run build` again after changing frontend sources. Builds from the published crate use its included browser assets and do not need Node.js. +This installs locked npm dependencies, builds browser assets, and creates +`target/debug/sqlpage`. Afterwards, use `cargo build` for Rust changes and +`npm run build` for frontend changes. Add `-- --release` to `npm run build:rust` +for a release build. -The resulting executable will be in `target/debug/sqlpage`. +Default builds require a system ODBC driver manager. On Linux and macOS, +`--features odbc-static` bundles unixODBC, but database-specific drivers must still +be installed separately. Windows uses its system ODBC driver manager. Published +crates include browser assets and do not require Node.js. -### Release mode +## Validation -To build the project in release mode: +For Rust changes: ```bash -npm run build:rust -- --release -``` - -The resulting executable will be in `target/release/sqlpage`. - -### ODBC build modes - -SQLPage can either be built with an integrated odbc driver manager (static linking), -or depend on having one already installed on the system where it is running (dynamic linking). - -- Dynamic ODBC (default): `npm run build:rust` -- Static ODBC (Linux and MacOS only): `npm run build:rust -- --features odbc-static` - -Windows comes with ODBC pre-installed; SQLPage cannot statically link to the unixODBC driver manager on windows. - -## Code Style and Linting - -### Rust - -- Use `cargo fmt --all` to format your Rust code -- Run `cargo clippy` to catch common mistakes and improve code quality -- All code must pass the following checks: - -```bash -cargo fmt --all -- --check +cargo fmt --all cargo clippy --all-targets --all-features -- -D warnings -``` - -### Frontend - -We use Biome for linting and formatting of the frontend code, and TypeScript -to typecheck it. - -```bash -npm install # once -npm run format # apply formatting -npm test # the check CI runs: biome, typecheck, and the frontend unit tests -``` - -`npm test` checks the entire frontend codebase (html, css, js, ts). -Biome also rejects promise-valued conditions and synchronous callbacks that discard -promises (`noMisusedPromises`), awaiting synchronous values (`useAwaitThenable`), -and runtime import cycles (`noImportCycles`). Type-only imports may form cycles. -Keep async Playwright assertions and actions awaited or returned; -`noPlaywrightMissingAwait` checks these even when TypeScript inference cannot -follow the shared fixture's types. The promise and Playwright rules are in -Biome's nursery group, so review their diagnostics when upgrading Biome. - -## Testing - -### Rust Tests - -Run the backend tests: - -```bash cargo test ``` -By default, the tests are run against an SQLite in-memory database. - -If you want to run them against another database, -start a database server with `docker compose up database_name` (mssql, mysql, mariadb, or postgres) -and run the tests with the `DATABASE_URL` environment variable pointing to the database: +For frontend changes: ```bash -docker compose up mssql # or mysql, mariadb, postgres -export DATABASE_URL=mssql://root:Password123!@localhost/sqlpage -cargo test +npm run format +npm test +npm run build ``` -Use `common::response_for(path)` for ordinary integration requests and -`common::response_with(path, config)` for custom configuration. Custom requests use -`common::response_from(request)`; tests sharing database or cache state use `common::send_request`. -These helpers use the production `create_app` routes and middleware through an Actix test service, -whose destructor drains its request pool. -Keep `#[actix_web::test]`; standalone `TestRequest::to_srv_request()` retains application state. -When testing Oracle locally, use `cargo test -- --test-threads=2` to avoid overwhelming the listener. - -### End-to-End Tests - -We use Playwright for end-to-end testing of dynamic frontend features. -Tests are located in [`tests/end-to-end/`](./tests/end-to-end/). Key areas covered include: - -Component tests use deterministic SQL applications under `tests/end-to-end/fixtures//`. -Each suite contains an `index.sql` page and a `test.ts` file. Import `test` and `expect` from -`../../fixture`; the shared fixture opens the matching SQL page before every test and waits for -all SQLPage components to initialize. Prefer role, label, and text locators over CSS selectors -when the assertion does not specifically concern generated markup. - -Keep official-site smoke and integration tests in root-level `*.spec.ts` files. Component behavior -tests should render real components through their SQL fixture; do not inject component markup or -invoke SQLPage's JavaScript initialization functions directly. Parameterized fixtures may accept -request variables when several tests need the same component with different data. - -#### Run the tests +Keep formatting scoped to changed files. `npm test` checks formatting, lint, +types, and frontend unit tests. Dynamic frontend changes also need Playwright: ```bash -npm install cd tests/end-to-end -npx playwright install chromium -npm run test -``` - -Playwright starts both servers itself on a free port. Set `SQLPAGE_BINARY` to run the servers from an already compiled binary instead of `cargo run`. - -## Documentation - -### Component Documentation - -When adding new components, comprehensive documentation is required. Example from a component documentation: - -```sql -INSERT INTO component(name, icon, description, introduced_in_version) VALUES - ('component_name', 'icon_name', 'Description of the component', 'version'); - --- Document all parameters -INSERT INTO parameter(component, name, description, type, top_level, optional) -VALUES ('component_name', 'param_name', 'param_description', 'TEXT|BOOLEAN|INTEGER|JSON|ICON|COLOR|HTML|REAL|TIMESTAMP|URL', false, true); - --- Use description_md instead of description when the text contains markdown -INSERT INTO parameter(component, name, description_md, type, top_level, optional) -VALUES ('component_name', 'other_param', 'Set to `true` to see [the docs](/documentation.sql).', 'BOOLEAN', true, true); - --- Include usage examples -INSERT INTO example(component, description, properties) VALUES - ('component_name', 'Example description in markdown', JSON('[ -{"component": "new_component_name", "top_level_property_1": "value1", "top_level_property_2": "value2"}, -{"row_level_property_1": "value1", "row_level_property_2": "value2"} -]')); -``` - -Component documentation is stored in [`./examples/official-site/sqlpage/migrations/`](./examples/official-site/sqlpage/migrations/). - -If you are editing an existing component, edit the existing sql documentation file directly. -If you are adding a new component, add a new sql file in the folder, and add the appropriate insert statements above. - -### SQLPage Function Documentation - -When adding new SQLPage functions, document them using a SQL migrations. Example structure: - -```sql --- Function Definition -INSERT INTO sqlpage_functions ( - "name", - "introduced_in_version", - "icon", - "description_md" -) -VALUES ( - 'your_function_name', - '1.0.0', - 'function-icon-name', - 'Description of what the function does. - -### Example - - select ''text'' as component, sqlpage.your_function_name(''parameter'') as result; - -Additional markdown documentation, usage notes, and examples go here. -'); - --- Function Parameters -INSERT INTO sqlpage_function_parameters ( - "function", - "index", - "name", - "description_md", - "type" -) -VALUES ( - 'your_function_name', - 1, - 'parameter_name', - 'Description of what this parameter does and how to use it.', - 'TEXT|BOOLEAN|INTEGER|JSON' -); +npx playwright install --with-deps --only-shell chromium +npm test ``` -Key elements to include in function documentation: - -- Clear description of the function's purpose -- Version number where the function was introduced -- Appropriate icon -- Markdown-formatted documentation with examples -- All parameters documented with clear descriptions and types -- Security considerations if applicable -- Example usage scenarios +Playwright starts its servers on free ports. Set `SQLPAGE_BINARY` to use an already +built executable. Component suites use `fixtures//{index.sql,test.ts}` and +import the shared `fixture.ts` harness; root-level `*.spec.ts` files cover the +official site. For examples with `test.hurl`, run +`scripts/test-examples-hurl.sh ''` from the repository root. -## Pull Request Process +Integration tests use the shared request helpers in [tests/common](./tests/common/mod.rs) +to exercise production routes and middleware and drain request pools. -1. Create a new branch for your feature/fix: +Rust tests default to in-memory SQLite; `DATABASE_URL` selects another database. +For example: ```bash -git checkout -b feature/your-feature-name +docker compose up --wait mssql +DATABASE_URL='mssql://root:Password123!@localhost/sqlpage' cargo test ``` -2. Make your changes, ensuring: - -- All tests pass -- Code is properly formatted -- New features are documented -- tests cover new functionality -- `CHANGELOG.md` has an entry for any user-visible change - -3. Push your changes and create a Pull Request - -4. CI Checks - Our CI pipeline will automatically: - - Run Rust formatting and clippy checks - - Execute all tests across multiple platforms (Linux, Windows) - - Build Docker images for multiple architectures - - Run frontend linting, typechecking and unit tests (`npm test`) - - Test against multiple databases (SQLite, PostgreSQL, MySQL, MSSQL, Oracle, and ODBC) - -## Release Process - -Releases are automated when pushing tags that match the pattern `v*` (e.g., `v1.0.0`). The CI pipeline will: - -- Build and test the code -- Create Docker images for multiple architectures -- Push images to Docker Hub -- Create GitHub releases - -## Questions? - -If you have any questions, feel free to open an issue or discussion on GitHub. +Use the [CI matrix](./.github/workflows/ci.yml) for supported backends, connection +strings, and driver setup. On Linux and macOS, `cargo test --features odbc-static` +matches CI's driver-manager linking. Oracle and DuckDB need host ODBC drivers even +when the database runs in a container. For local Oracle runs, add +`-- --test-threads=2` to avoid overwhelming the listener. Check affected matrix results for SQL +execution changes; SQLite alone cannot establish portability. + +## Submitting a change + +Follow nearby code and tests, add regression coverage, and describe the resulting +behavior and validation in the pull request. Keep changes and formatting focused. +Architectural constraints and documentation maintenance rules are in +[AGENTS.md](./AGENTS.md). + +User documentation lives in [the official-site SQL pages and migrations](./examples/official-site/). +Edit existing documentation in place: deployment recreates the site database. +Use nearby entries as examples rather than copying schemas into this guide. Update +[configuration.md](./configuration.md) for configuration changes and +[CHANGELOG.md](./CHANGELOG.md) for user-visible changes in an unreleased version. diff --git a/src/webserver/database/execute_queries.rs b/src/webserver/database/execute_queries.rs index 810cba2d0..475380d44 100644 --- a/src/webserver/database/execute_queries.rs +++ b/src/webserver/database/execute_queries.rs @@ -10,10 +10,9 @@ use tracing::Instrument; use super::csv_import::run_csv_import; use super::error_highlighting::{display_stmt_db_error, display_stmt_error, is_positioned_error}; use super::sql::{ - DatabaseQuery, FileStatement, OutputColumn, Query, QueryBody, SourceSpan, SqlFile, - StaticSimpleSelect, + DatabaseQuery, FileStatement, Query, QueryBody, SourceSpan, SqlFile, StaticSimpleSelect, }; -use super::sqlpage_expr::{NoInputs, RowExpr, RowInputs}; +use super::sqlpage_expr::{NoInputs, RowInputs}; use crate::dynamic_component::parse_dynamic_rows; use crate::utils::add_value_to_map; use crate::webserver::ErrorWithStatus; @@ -35,10 +34,24 @@ use sqlx::value::ValueRef; pub type DbConn = Option>; -/// One database result together with private values reserved for computed -/// columns and therefore omitted from the user-visible row. -struct QueryResult { - item: DbItem, +/// Database events before computed columns and dynamic components are evaluated. +enum QueryEvent { + Row(QueryRow), + FinishedQuery, +} + +impl QueryEvent { + fn into_db_item(self) -> DbItem { + match self { + Self::Row(row) => DbItem::Row(row.value), + Self::FinishedQuery => DbItem::FinishedQuery, + } + } +} + +/// A decoded row together with private values reserved for computed columns. +struct QueryRow { + value: Value, inputs: RowInputs, output_column_count: usize, } @@ -224,32 +237,29 @@ pub fn stream_query_results_with_conn<'a>( let connection = take_connection(&request.app_state.db, db_connection, request).await?; let mut stream = connection.fetch_many(query); loop { - let start_next = std::time::Instant::now(); - let next_elem = stream.next().instrument(query_span.clone()).await; - query_metrics.add_duration(start_next.elapsed()); - let Some(elem) = next_elem else { break; }; - - let mut query_result = parse_single_sql_result(source_file, stmt, statement.source_span, elem); - if let DbItem::Error(e) = query_result.item { - error = Some(e); - break; - } - if matches!(query_result.item, DbItem::Row(_)) { - returned_rows += 1; - } - apply_json_columns(&mut query_result.item, &stmt.json_columns); + let Some(result) = next_query_event( + &mut stream, &mut query_metrics, &mut returned_rows, + source_file, stmt, statement.source_span, + ).await else { break; }; + let mut query_result = match result { + Ok(event) => event, + Err(e) => { + error = Some(e); + break; + } + }; if buffer_rows { deferred_query_results.push(query_result); } else { let mut computed_connection = None; - if let Err(err) = evaluate_computed_columns(request, &stmt.computed_columns, &mut query_result, &mut computed_connection) + if let Err(err) = prepare_query_event(request, stmt, &mut query_result, &mut computed_connection) .instrument(query_span.clone()) .await { error = Some(err); break; } - for db_item in parse_dynamic_rows(query_result.item) { + for db_item in parse_dynamic_rows(query_result.into_db_item()) { yield db_item; } } @@ -258,9 +268,9 @@ pub fn stream_query_results_with_conn<'a>( } if error.is_none() && buffer_rows { for mut query_result in deferred_query_results { - if let Err(err) = evaluate_computed_columns( + if let Err(err) = prepare_query_event( request, - &stmt.computed_columns, + stmt, &mut query_result, db_connection, ) @@ -270,7 +280,7 @@ pub fn stream_query_results_with_conn<'a>( error = Some(err); break; } - for db_item in parse_dynamic_rows(query_result.item) { + for db_item in parse_dynamic_rows(query_result.into_db_item()) { yield db_item; } } @@ -402,7 +412,7 @@ async fn execute_scalar_query<'a>( }; ensure_scalar_column_count(static_select.columns.len())?; let row = execute_static_simple_select(static_select, request, db_connection).await?; - return scalar_value_from_row(DbItem::Row(row)); + return scalar_value_from_row(row); }; let query = bind_query(database_query, request, db_connection).await?; log::debug!("Executing scalar query: {:?}", query.sql); @@ -422,17 +432,20 @@ async fn execute_scalar_query<'a>( let connection = take_connection(&request.app_state.db, db_connection, request).await?; let mut stream = connection.fetch_many(query); loop { - let start_next = std::time::Instant::now(); - let next_elem = stream.next().instrument(query_span.clone()).await; - query_metrics.add_duration(start_next.elapsed()); - let Some(elem) = next_elem else { break }; - - let result = - parse_single_sql_result(source_file, database_query, statement.source_span, elem); - let output_column_count = result.output_column_count; - match result.item { - row @ DbItem::Row(_) => { - returned_rows += 1; + let Some(result) = next_query_event( + &mut stream, + &mut query_metrics, + &mut returned_rows, + source_file, + database_query, + statement.source_span, + ) + .await + else { + break; + }; + match result { + Ok(QueryEvent::Row(row)) => { if scalar_row.is_some() { debug_assert_eq!( scalar_subquery_behavior, @@ -443,17 +456,13 @@ async fn execute_scalar_query<'a>( )); break; } - scalar_row = Some(QueryResult { - item: row, - inputs: result.inputs, - output_column_count, - }); + scalar_row = Some(row); if scalar_subquery_behavior == ScalarSubqueryBehavior::FirstRow { break; } } - DbItem::FinishedQuery => {} - DbItem::Error(err) => { + Ok(QueryEvent::FinishedQuery) => {} + Err(err) => { error = Some(err); break; } @@ -469,15 +478,9 @@ async fn execute_scalar_query<'a>( ); } else if let Some(mut row) = scalar_row { ensure_scalar_column_count(row.output_column_count)?; - apply_json_columns(&mut row.item, &database_query.json_columns); - if let Err(error) = evaluate_computed_columns( - request, - &database_query.computed_columns, - &mut row, - db_connection, - ) - .instrument(query_span.clone()) - .await + if let Err(error) = prepare_query_row(request, database_query, &mut row, db_connection) + .instrument(query_span.clone()) + .await { let connection = take_connection(&request.app_state.db, db_connection, request).await?; return Err(record_error_and_rollback( @@ -488,7 +491,7 @@ async fn execute_scalar_query<'a>( ) .await); } - scalar_value_from_row(row.item)? + scalar_value_from_row(row.value)? } else { None }; @@ -508,8 +511,8 @@ async fn record_error_and_rollback( error } -fn scalar_value_from_row(item: DbItem) -> anyhow::Result> { - let DbItem::Row(Value::Object(row)) = item else { +fn scalar_value_from_row(value: Value) -> anyhow::Result> { + let Value::Object(row) = value else { anyhow::bail!("SET scalar query did not return a row object"); }; ensure_scalar_column_count(row.len())?; @@ -609,51 +612,57 @@ async fn set_trace_context(connection: &mut AnyConnection, db: &Database) { } } +/// Poll and decode one physical result while measuring only database fetch time. +async fn next_query_event( + stream: &mut (impl Stream>> + Unpin), + metrics: &mut DbQueryMetricsContext<'_>, + returned_rows: &mut i64, + source_file: &Path, + query: &DatabaseQuery, + source_span: SourceSpan, +) -> Option> { + let start_next = std::time::Instant::now(); + let next = stream.next().instrument(metrics.span.clone()).await; + metrics.add_duration(start_next.elapsed()); + let result = parse_single_sql_result(source_file, query, source_span, next?); + if matches!(result, Ok(QueryEvent::Row(_))) { + *returned_rows += 1; + } + Some(result) +} + #[inline] fn parse_single_sql_result( source_file: &Path, query: &DatabaseQuery, source_span: SourceSpan, res: sqlx::error::Result>, -) -> QueryResult { +) -> anyhow::Result { match res { Ok(Either::Right(r)) => { if log::log_enabled!(log::Level::Trace) { debug_row(&r); } - match super::sql_to_json::row_to_json_with_inputs(&r, query.row_input_json.len()) { - Ok((row, mut inputs)) => { - decode_json_values(&mut inputs, &query.row_input_json); - QueryResult { - item: DbItem::Row(row), - inputs: RowInputs::new(inputs), - output_column_count: r.columns().len() - query.row_input_json.len() - + query.computed_columns.len(), - } - } - Err(error) => QueryResult { - item: DbItem::Error(error), - inputs: RowInputs::new(Vec::new()), - output_column_count: 0, - }, - } + let (row, mut inputs) = + super::sql_to_json::row_to_json_with_inputs(&r, query.row_input_json.len())?; + decode_json_values(&mut inputs, &query.row_input_json); + Ok(QueryEvent::Row(QueryRow { + value: row, + inputs: RowInputs::new(inputs), + output_column_count: r.columns().len() - query.row_input_json.len() + + query.computed_columns.len(), + })) } Ok(Either::Left(res)) => { log::debug!("Finished query with result: {res:?}"); - QueryResult { - item: DbItem::FinishedQuery, - inputs: RowInputs::new(Vec::new()), - output_column_count: 0, - } - } - Err(err) => { - let nice_err = display_stmt_db_error(source_file, &query.sql, source_span, err); - QueryResult { - item: DbItem::Error(nice_err), - inputs: RowInputs::new(Vec::new()), - output_column_count: 0, - } + Ok(QueryEvent::FinishedQuery) } + Err(err) => Err(display_stmt_db_error( + source_file, + &query.sql, + source_span, + err, + )), } } @@ -725,17 +734,30 @@ async fn bind_query<'a>( }) } -async fn evaluate_computed_columns( +async fn prepare_query_event( request: &ExecutionContext, - columns: &[OutputColumn], - result: &mut QueryResult, + query: &DatabaseQuery, + event: &mut QueryEvent, db_connection: &mut DbConn, ) -> anyhow::Result<()> { - if let DbItem::Row(Value::Object(results)) = &mut result.item { - for column in columns { + match event { + QueryEvent::Row(row) => prepare_query_row(request, query, row, db_connection).await, + QueryEvent::FinishedQuery => Ok(()), + } +} + +async fn prepare_query_row( + request: &ExecutionContext, + query: &DatabaseQuery, + row: &mut QueryRow, + db_connection: &mut DbConn, +) -> anyhow::Result<()> { + apply_json_columns(&mut row.value, &query.json_columns); + if let Value::Object(results) = &mut row.value { + for column in &query.computed_columns { let value = column .value - .evaluate(request, db_connection, &mut result.inputs) + .evaluate(request, db_connection, &mut row.inputs) .await? .into_json(); let old_results = std::mem::take(results); @@ -745,8 +767,8 @@ async fn evaluate_computed_columns( Ok(()) } -fn apply_json_columns(item: &mut DbItem, json_columns: &[String]) { - if let DbItem::Row(Value::Object(row)) = item { +fn apply_json_columns(value: &mut Value, json_columns: &[String]) { + if let Value::Object(row) = value { for column in json_columns { if let Some(value) = row.get_mut(column) { if let Value::String(json_str) = value { @@ -831,13 +853,9 @@ mod tests { use tracing_subscriber::prelude::*; use tracing_subscriber::registry::LookupSpan; - fn create_row_item(value: Value) -> DbItem { - DbItem::Row(value) - } - - fn assert_json_value(item: &DbItem, key: &str, expected: Value) { - let DbItem::Row(Value::Object(row)) = item else { - panic!("Expected DbItem::Row"); + fn assert_json_value(item: &Value, key: &str, expected: Value) { + let Value::Object(row) = item else { + panic!("Expected a row object"); }; assert_eq!(row[key], expected); drop(expected); @@ -845,10 +863,10 @@ mod tests { #[test] fn test_basic_json_string_conversion() { - let mut item = create_row_item(json!({ + let mut item = json!({ "json_col": "{\"key\": \"value\"}", "normal_col": "text" - })); + }); apply_json_columns(&mut item, &["json_col".to_string()]); assert_json_value(&item, "json_col", json!({"key": "value"})); assert_json_value(&item, "normal_col", json!("text")); @@ -856,10 +874,10 @@ mod tests { #[test] fn test_json_array_conversion() { - let mut item = create_row_item(json!({ + let mut item = json!({ "array_col": ["{\"a\": 1}", "{\"b\": 2}"], "normal_array": ["text"] - })); + }); apply_json_columns(&mut item, &["array_col".to_string()]); assert_json_value(&item, "array_col", json!([{"a": 1}, {"b": 2}])); assert_json_value(&item, "normal_array", json!(["text"])); @@ -867,10 +885,10 @@ mod tests { #[test] fn test_invalid_json_handling() { - let mut item = create_row_item(json!({ + let mut item = json!({ "invalid_json": "{not valid json}", "normal_col": "text" - })); + }); apply_json_columns(&mut item, &["invalid_json".to_string()]); assert_json_value(&item, "invalid_json", json!("{not valid json}")); assert_json_value(&item, "normal_col", json!("text")); @@ -878,31 +896,81 @@ mod tests { #[test] fn test_missing_column_handling() { - let mut item = create_row_item(json!({ + let mut item = json!({ "existing_col": "text" - })); + }); apply_json_columns(&mut item, &["missing_col".to_string()]); assert_json_value(&item, "existing_col", json!("text")); } #[test] - fn test_non_row_dbitem_handling() { - let mut item = DbItem::FinishedQuery; + fn test_non_object_json_handling() { + let mut item = Value::Null; apply_json_columns(&mut item, &["json_col".to_string()]); - assert!(matches!(item, DbItem::FinishedQuery)); + assert!(item.is_null()); } #[test] fn test_duplicate_json_column_names() { - let mut item = create_row_item(json!({ + let mut item = json!({ "json_col": "{\"key\": \"value\"}", "normal_col": "text" - })); + }); apply_json_columns(&mut item, &["json_col".to_string(), "json_col".to_string()]); assert_json_value(&item, "json_col", json!({"key": "value"})); assert_json_value(&item, "normal_col", json!("text")); } + #[actix_web::test] + async fn dropping_stream_stops_later_rows_and_releases_connection_borrow() { + let mut config: crate::app_config::AppConfig = serde_json::from_str("{}").unwrap(); + config.database_url = crate::app_config::test_database_url(); + config.max_database_pool_connections = Some(1); + config.database_connection_acquire_timeout_seconds = 1.0; + let state = Arc::new(crate::AppState::init(&config).await.unwrap()); + let mut request = actix_web::test::TestRequest::get().to_srv_request(); + let request = crate::webserver::http_request_info::extract_request_info( + &mut request, + state, + crate::webserver::server_timing::ServerTiming::default(), + ) + .await + .unwrap(); + let file = SqlFile::new( + &request.app_state.db, + // Text markers keep this ordering test independent of driver numeric representations. + "SELECT n, sqlpage.fetch(url) AS value + FROM (SELECT 'row1' AS n, NULL AS url UNION ALL SELECT 'row2' AS n, 'invalid URL' AS url) source_rows + ORDER BY n; + SET after_query = 'unexpected'", + Path::new("execution-test.sql"), + ); + let mut connection = None; + { + let stream = stream_query_results_with_conn(&file, &request, &mut connection); + futures_util::pin_mut!(stream); + let Some(DbItem::Row(row)) = stream.next().await else { + panic!("Expected the first row before the later function error"); + }; + assert_eq!(row, json!({"n": "row1", "value": null})); + } + assert!(!request.set_variables.borrow().contains_key("after_query")); + let file = SqlFile::new( + &request.app_state.db, + "SELECT n FROM (SELECT 'reused' AS n) source_rows", + Path::new("execution-test.sql"), + ); + let stream = stream_query_results_with_conn(&file, &request, &mut connection); + futures_util::pin_mut!(stream); + let Some(DbItem::Row(row)) = stream.next().await else { + panic!("Expected a row from the reused connection"); + }; + assert_eq!(row, json!({"n": "reused"})); + while let Some(item) = stream.next().await { + assert!(matches!(item, DbItem::FinishedQuery)); + } + } + #[derive(Default)] struct RecordedFields(HashMap<&'static str, String>); diff --git a/src/webserver/database/sql.rs b/src/webserver/database/sql.rs index 9cefc8787..02f4c28d0 100644 --- a/src/webserver/database/sql.rs +++ b/src/webserver/database/sql.rs @@ -41,13 +41,12 @@ mod dialect; mod rewrite; mod statement; -#[cfg(test)] -pub(super) use statement::SourceLocation; pub use statement::SqlFile; pub(super) use statement::{ - DatabaseQuery, FileStatement, OutputColumn, Query, QueryBody, SourceSpan, StaticSimpleSelect, - VariableName, + DatabaseQuery, FileStatement, Query, QueryBody, SourceSpan, StaticSimpleSelect, VariableName, }; +#[cfg(test)] +pub(super) use statement::{OutputColumn, SourceLocation}; impl SqlFile { #[must_use] diff --git a/tests/sql_test_files/column_validation/error_set_scalar_query_returned_more_than_one_column.sql b/tests/sql_test_files/column_validation/error_set_scalar_query_returned_more_than_one_column.sql new file mode 100644 index 000000000..e3086c7ff --- /dev/null +++ b/tests/sql_test_files/column_validation/error_set_scalar_query_returned_more_than_one_column.sql @@ -0,0 +1,3 @@ +set invalid_value = ( + select n as duplicate, n as duplicate from (select 1 as n) source_rows +); diff --git a/tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql b/tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql index 61a8c3b17..f9a774a3d 100644 --- a/tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql +++ b/tests/sql_test_files/data/database-specific/sqlite/set_multiple_rows.sql @@ -5,3 +5,6 @@ set actual = ( ); select 'first' as expected, $actual as actual; + +select '[{"values_json":[[1],[2]],"encoded":"a%20b"}]' as expected, + sqlpage.run_sql('tests/sql_test_files/query_events/sqlite_json.sql') as actual; diff --git a/tests/sql_test_files/data/query_event_rows.sql b/tests/sql_test_files/data/query_event_rows.sql new file mode 100644 index 000000000..2815ecdda --- /dev/null +++ b/tests/sql_test_files/data/query_event_rows.sql @@ -0,0 +1,9 @@ +-- Compare the complete rows, including order and absence of private input columns. +set included = json_array(json_object('nested', '[{"value":"nested"}]')); +set expected = json_array( + json_object('n', 'row1', 'included', $included), + json_object('n', 'row2', 'included', $included), + json_object('scalar_included', $included), + json_object('values_json', json_array('[1]', '[2]'), 'encoded', json_array('a b', 'a%20b')) +); +select $expected as expected, sqlpage.run_sql('tests/sql_test_files/query_events/rows.sql') as actual; diff --git a/tests/sql_test_files/data/set_variable_to_sqlpage_function.sql b/tests/sql_test_files/data/set_variable_to_sqlpage_function.sql index 7fc0c5157..869236c8a 100644 --- a/tests/sql_test_files/data/set_variable_to_sqlpage_function.sql +++ b/tests/sql_test_files/data/set_variable_to_sqlpage_function.sql @@ -1,3 +1,6 @@ set my_var = sqlpage.url_encode(' '); select '%20' as expected, - $my_var as actual; \ No newline at end of file + $my_var as actual; + +set encoded = (select sqlpage.url_encode(name) from (select 'a b' as name) source_rows); +select 'a%20b' as expected, $encoded as actual; diff --git a/tests/sql_test_files/mod.rs b/tests/sql_test_files/mod.rs index be7eb7b76..4062d5465 100644 --- a/tests/sql_test_files/mod.rs +++ b/tests/sql_test_files/mod.rs @@ -78,6 +78,10 @@ fn get_sql_test_cases() -> Vec { "tests/sql_test_files/data", SqlTestFormat::Json, )); + tests.extend(read_sql_tests_in_dir( + "tests/sql_test_files/column_validation", + SqlTestFormat::Html, + )); tests } diff --git a/tests/sql_test_files/query_events/child.sql b/tests/sql_test_files/query_events/child.sql new file mode 100644 index 000000000..ce01ab0d8 --- /dev/null +++ b/tests/sql_test_files/query_events/child.sql @@ -0,0 +1 @@ +select sqlpage.run_sql('tests/sql_test_files/query_events/grandchild.sql') as nested from (select 1 as n) source_rows; diff --git a/tests/sql_test_files/query_events/grandchild.sql b/tests/sql_test_files/query_events/grandchild.sql new file mode 100644 index 000000000..4fd4f80d8 --- /dev/null +++ b/tests/sql_test_files/query_events/grandchild.sql @@ -0,0 +1 @@ +select 'nested' as value from (select 1 as n) source_rows; diff --git a/tests/sql_test_files/query_events/rows.sql b/tests/sql_test_files/query_events/rows.sql new file mode 100644 index 000000000..b1a09919b --- /dev/null +++ b/tests/sql_test_files/query_events/rows.sql @@ -0,0 +1,9 @@ +select n, sqlpage.run_sql(path) as included +from (select 'row1' as n, 'tests/sql_test_files/query_events/child.sql' as path + union all select 'row2' as n, 'tests/sql_test_files/query_events/child.sql' as path) source_rows +order by n; +set scalar_included = (select sqlpage.run_sql(path) from (select 'tests/sql_test_files/query_events/child.sql' as path) source_rows); +select $scalar_included as scalar_included; +select '[1]' as values_json, '[2]' as values_json, + sqlpage.url_encode(name) as encoded, name as encoded +from (select 'a b' as name) source_rows; diff --git a/tests/sql_test_files/query_events/sqlite_json.sql b/tests/sql_test_files/query_events/sqlite_json.sql new file mode 100644 index 000000000..6d6cfa014 --- /dev/null +++ b/tests/sql_test_files/query_events/sqlite_json.sql @@ -0,0 +1,3 @@ +select json('[1]') as values_json, json('[2]') as values_json, + sqlpage.url_encode(name) as encoded +from (select 'a b' as name) source_rows; diff --git a/tests/transactions/failed_computed_column.sql b/tests/transactions/failed_computed_column.sql new file mode 100644 index 000000000..09593a61e --- /dev/null +++ b/tests/transactions/failed_computed_column.sql @@ -0,0 +1,4 @@ +create temporary table query_event_rollback(value integer); +begin; +insert into query_event_rollback values (1); +select sqlpage.fetch(url) as value from (select 'invalid URL' as url) source_rows; diff --git a/tests/transactions/mod.rs b/tests/transactions/mod.rs index e0e3cf324..8795972b7 100644 --- a/tests/transactions/mod.rs +++ b/tests/transactions/mod.rs @@ -1,5 +1,6 @@ use actix_web::http::StatusCode; use sqlpage::webserver::database::SupportedDatabase; +use sqlx::row::Row as _; use crate::common::{make_app_data, multipart_request, response_with_data, send_request}; @@ -22,6 +23,20 @@ async fn test_transaction_error() -> actix_web::Result<()> { body_str.contains("error") && body_str.contains("null"), "{body_str}\nexpected to contain: constraint failed" ); + if data.db.info.database_type == SupportedDatabase::Sqlite { + let resp = response_with_data( + "/tests/transactions/failed_computed_column.sql", + data.clone(), + ) + .await?; + let body_str = crate::common::read_body_string(resp).await; + assert!(body_str.contains("invalid URL")); + let row = sqlx::query::query("SELECT COUNT(*) AS count FROM query_event_rollback") + .fetch_one(&data.db.connection) + .await + .unwrap(); + assert_eq!(row.try_get::("count").unwrap(), 0); + } // Now query again, with ?x=1447 let path_with_param = path.to_string() + "?x=1447"; let resp = response_with_data(&path_with_param, data.clone()).await?;