diff --git a/apps/dev-playground/shared/appkit-types/metric-views.d.ts b/apps/dev-playground/shared/appkit-types/metric-views.d.ts index e0ada3749..1c7fb87a9 100644 --- a/apps/dev-playground/shared/appkit-types/metric-views.d.ts +++ b/apps/dev-playground/shared/appkit-types/metric-views.d.ts @@ -30,23 +30,31 @@ declare module "@databricks/appkit-ui/react" { measures: { "active_accounts": { type: "bigint"; + display_name: "Active Accounts"; + format: "#,##0"; }; "churn_rate": { type: "decimal"; + display_name: "Churn Rate"; }; "avg_ltv": { type: "double"; + display_name: "Average LTV"; + format: "$#,##0.00"; }; }; dimensions: { "segment": { type: "string"; + display_name: "Customer Segment"; }; "region": { type: "string"; + display_name: "Region"; }; "csm_email": { type: "string"; + display_name: "CSM Email"; }; }; }; @@ -80,27 +88,38 @@ declare module "@databricks/appkit-ui/react" { measures: { "mrr": { type: "double"; + display_name: "Monthly Recurring Revenue"; + format: "$#,##0.00"; }; "arr": { type: "double"; + display_name: "Annual Recurring Revenue"; + format: "$#,##0.00"; description: "Annualized contract value across all active subscriptions"; }; "new_arr": { type: "double"; + display_name: "New ARR"; + format: "$#,##0.00"; }; "churned_arr": { type: "double"; + display_name: "Churned ARR"; + format: "$#,##0.00"; }; }; dimensions: { "region": { type: "string"; + display_name: "Region"; }; "segment": { type: "string"; + display_name: "Customer Segment"; }; "created_at": { type: "timestamp_ltz"; + display_name: "Subscription Start"; time_grain: readonly ["day", "hour", "minute", "month", "quarter", "week", "year"]; }; }; diff --git a/docs/docs/plugins/analytics.md b/docs/docs/plugins/analytics.md index 1ed102107..0458fedff 100644 --- a/docs/docs/plugins/analytics.md +++ b/docs/docs/plugins/analytics.md @@ -7,6 +7,7 @@ sidebar_position: 3 Enables SQL query execution against Databricks SQL Warehouses. **Key features:** + - File-based SQL queries with automatic type generation - Parameterized queries with type-safe [SQL helpers](../api/appkit/Variable.sql.md) - JSON and Arrow format support @@ -35,7 +36,6 @@ await createApp({ The execution context is determined by the SQL file name, not by the hook call. - ## SQL parameters Use `:paramName` placeholders and optionally annotate parameter types using SQL comments: @@ -56,6 +56,7 @@ Annotate with `INT`, or use `sql.number()` (auto-infers `INT` for values in at the call site. **Supported `-- @param` types** (case-insensitive): + - `STRING`, `BOOLEAN`, `DATE`, `TIMESTAMP`, `BINARY` - `INT`, `BIGINT`, `TINYINT`, `SMALLINT` — bind via `sql.int()` / `sql.bigint()` - `FLOAT`, `DOUBLE` — bind via `sql.float()` / `sql.double()` @@ -103,12 +104,149 @@ The analytics plugin exposes these endpoints (mounted under `/api/analytics`): - `POST /api/analytics/query/:query_key` - `GET /api/analytics/arrow-result/:jobId` +- `POST /api/analytics/metric/:key` — measure a Unity Catalog Metric View (see [Metric views](#metric-views)) ## Format options - `format: "JSON"` (default) returns JSON rows - `format: "ARROW"` returns an Arrow "statement_id" payload over SSE, then the client fetches binary Arrow from `/api/analytics/arrow-result/:jobId` +## Metric views + +`POST /api/analytics/metric/:key` measures a [Unity Catalog Metric View](https://docs.databricks.com/en/metric-views/index.html) that you declared in `config/queries/metric-views.json`. Instead of writing SQL, the caller sends a structured request — which measures to aggregate, which dimensions to group by, and an optional filter — and the plugin builds and runs the `SELECT MEASURE(...) ... GROUP BY ALL` for you against the view. + +The route is **dormant until `metric-views.json` exists**: with no config file, every metric key returns `404`. Declaring the file (and generating types) is covered in [Metric-view types](../development/type-generation.md#metric-view-types); this section documents the runtime endpoint that config activates. + +### Request body + +``` +POST /api/analytics/metric/:key +Content-Type: application/json + +{ + "measures": ["arr", "revenue"], + "dimensions": ["region", "order_date"], + "timeGrain": "month", + "timeDimension": "order_date", + "filter": { "member": "region", "operator": "in", "values": ["EMEA", "APAC"] }, + "limit": 100 +} +``` + +`:key` is a metric key from `metric-views.json`. The body fields: + +| Field | Type | Required | Description | +| --------------- | ---------- | -------- | ------------------------------------------------------------------------------------------------------------ | +| `measures` | `string[]` | yes | Measures to aggregate. At least 1, at most 50. Each becomes `MEASURE() AS `. | +| `dimensions` | `string[]` | no | Dimensions to group by (max 20). Selected verbatim and grouped via `GROUP BY ALL`. | +| `filter` | object | no | Structured predicate tree translated into a parameterized `WHERE` clause (see [Filters](#filters)). | +| `timeGrain` | `string` | no | Bucket a time dimension via `date_trunc('', …)` — e.g. `day`, `month`. Requires `timeDimension`. | +| `timeDimension` | `string` | no | The single dimension `timeGrain` buckets. Must be one of `dimensions`. Required whenever `timeGrain` is set. | +| `limit` | `number` | no | Positive integer row cap (max 100000). | +| `format` | `string` | no | `JSON_ARRAY` (default). `JSON` is accepted as a deprecated alias for it; Arrow formats (`ARROW`, `ARROW_STREAM`) are rejected on this route. | + +Measures and dimensions must be unique across both lists — a name cannot repeat, nor appear as both a measure and a dimension. + +### How the request becomes SQL + +Given a view registered as `catalog.schema.revenue_metrics`, the request above produces (measures and dimensions are sorted for a deterministic SELECT list): + +```sql +SELECT MEASURE(`arr`) AS `arr`, MEASURE(`revenue`) AS `revenue`, + date_trunc('month', `order_date`) AS `order_date`, `region` +FROM `catalog`.`schema`.`revenue_metrics` +WHERE `region` IN (:f_0, :f_1) +GROUP BY ALL +LIMIT 100 +``` + +The metric view's FQN and every measure/dimension identifier are backtick-quoted; filter values are bound as parameters (`:f_0`, `:f_1`, …), never interpolated into the SQL string. + +### Filters + +`filter` is a recursive tree. A leaf is a single predicate: + +```json +{ "member": "region", "operator": "equals", "values": ["EMEA"] } +``` + +Predicates combine with `and` / `or` groups, which can nest: + +```json +{ + "and": [ + { "member": "region", "operator": "in", "values": ["EMEA", "APAC"] }, + { + "or": [ + { "member": "segment", "operator": "equals", "values": ["Enterprise"] }, + { "member": "deal_size", "operator": "gt", "values": [50000] } + ] + } + ] +} +``` + +The operator vocabulary: + +| Operator | SQL | Values | +| --------------------------- | ----------------------- | ------------------ | +| `equals` | `=` | exactly one | +| `notEquals` | `<>` | exactly one | +| `in` | `IN (…)` | one or more | +| `notIn` | `NOT IN (…)` | one or more | +| `gt` / `gte` / `lt` / `lte` | `>` / `>=` / `<` / `<=` | exactly one | +| `contains` | `LIKE :param` | exactly one string | +| `notContains` | `NOT LIKE :param` | exactly one string | +| `set` | `IS NOT NULL` | none | +| `notSet` | `IS NULL` | none | + +For `contains` / `notContains`, the `%…%` wildcards are applied to the *bound parameter value* (`%value%`), not written into the SQL text — so the value is never interpolated, consistent with every other operator. + +Empty groups of either kind (`{ "or": [] }`, `{ "and": [] }`) are rejected with `400` — every `and` / `or` group must contain at least one predicate. To send no filter, omit the `filter` field entirely rather than passing an empty group. + +Filters are bounded to keep hostile input from exhausting the server: nesting depth ≤ 8, ≤ 100 children per `and` / `or` group, and ≤ 1000 values per predicate. A request that exceeds a cap is rejected with `400`. + +### Executors (cache scope) + +Each entry in `metric-views.json` names the executor the query runs as, which also sets the cache scope. This is fixed by config, not the request: + +| `executor` | Runs as | Cache | +| --------------------------------- | ---------------------------------- | ----------------------- | +| `app_service_principal` (default) | The app service principal | Shared across all users | +| `user` | The requesting user (on-behalf-of) | Per user | + +This mirrors the `.sql` vs `.obo.sql` distinction for [file-based queries](#execution-context). + +### Response + +The response is the same SSE stream as `POST /api/analytics/query/:query_key`. If the SQL warehouse is cold it first emits `warehouse_status` events (see [Warehouse readiness](#warehouse-readiness)), then a single `result` event with the rows as objects: + +```json +{ + "type": "result", + "data": [ + { + "region": "EMEA", + "order_date": "2025-01-01", + "arr": 1200000, + "revenue": 340000 + } + ] +} +``` + +On failure it emits an `error` event instead. + +### Errors and behavior + +| Status | Body | When | +| ------ | ------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | +| `404` | `{ "error": "Metric not found" }` | `:key` is not declared in `metric-views.json` (also the response for every key when the file is absent). | +| `400` | `{ "error": "Invalid metric request body (fields: …)", "code": … }` | The request body fails validation. The message names only the offending field paths, never the submitted values. | +| `503` | `{ "error": "Metric registry not available", "code": "METRIC_REGISTRY_LOAD_FAILED" }` | `metric-views.json` is present but malformed or unreadable. | + +Editing `metric-views.json` is picked up on the next request — no server restart is needed. A previously malformed file that you fix likewise starts working on the next request. + ## Frontend usage ### useAnalyticsQuery @@ -118,15 +256,19 @@ React hook that subscribes to an analytics query over SSE and returns its latest ```ts import { useAnalyticsQuery } from "@databricks/appkit-ui/react"; -const { data, loading, error } = useAnalyticsQuery(queryKey, parameters, options); +const { data, loading, error } = useAnalyticsQuery( + queryKey, + parameters, + options, +); ``` **Return type:** ```ts { - data: T | null; // query result (typed array for JSON, TypedArrowTable for ARROW) - loading: boolean; // true while the query is executing + data: T | null; // query result (typed array for JSON, TypedArrowTable for ARROW) + loading: boolean; // true while the query is executing error: string | null; // error message, or null on success warehouseStatus: WarehouseStatus | null; // see "Warehouse readiness" below } @@ -134,11 +276,11 @@ const { data, loading, error } = useAnalyticsQuery(queryKey, parameters, options **Options:** -| Option | Type | Default | Description | -|--------|------|---------|-------------| -| `format` | `"JSON" \| "ARROW"` | `"JSON"` | Response format | -| `maxParametersSize` | `number` | `102400` | Max serialized parameters size in bytes | -| `autoStart` | `boolean` | `true` | Start query on mount | +| Option | Type | Default | Description | +| ------------------- | ------------------- | -------- | --------------------------------------- | +| `format` | `"JSON" \| "ARROW"` | `"JSON"` | Response format | +| `maxParametersSize` | `number` | `102400` | Max serialized parameters size in bytes | +| `autoStart` | `boolean` | `true` | Start query on mount | ### Warehouse readiness @@ -154,8 +296,10 @@ This means a cold start no longer freezes the UI on a stalled spinner. Render th import { useAnalyticsQuery } from "@databricks/appkit-ui/react"; function SpendTable() { - const { data, loading, error, warehouseStatus } = - useAnalyticsQuery("spend_summary", params); + const { data, loading, error, warehouseStatus } = useAnalyticsQuery( + "spend_summary", + params, + ); if (warehouseStatus && warehouseStatus.state !== "RUNNING") { return
Warehouse is {warehouseStatus.state.toLowerCase()}…
; @@ -197,10 +341,7 @@ export function AppShell({ children }) { If you already render your own `` for unrelated app toasts, drop the indicator and call `useResourceStatusToaster()` instead so resource-status toasts share that single Toaster: ```tsx -import { - useResourceStatusToaster, - Toaster, -} from "@databricks/appkit-ui/react"; +import { useResourceStatusToaster, Toaster } from "@databricks/appkit-ui/react"; function App() { useResourceStatusToaster(); @@ -219,7 +360,8 @@ For a fully custom toast body, pass `render` (rendered through `toast.custom`): (
- {agg.worst?.kind} {agg.worst?.state.toLowerCase()} ({agg.activeCount} waiting) + {agg.worst?.kind} {agg.worst?.state.toLowerCase()} ({agg.activeCount}{" "} + waiting)
)} /> @@ -232,8 +374,7 @@ To override copy for a specific kind without rewriting the whole UI, pass `rende renderers={{ warehouse: { title: () => "Spinning up your data", - description: (_s, agg) => - `${agg.affectedLabels.length} chart(s) waiting`, + description: (_s, agg) => `${agg.affectedLabels.length} chart(s) waiting`, }, }} /> @@ -263,11 +404,9 @@ import { useEffect, useId } from "react"; function useLakebaseReadiness() { const id = useId(); - const { publish, unpublish } = useResourceStatusPublisher( - id, - "lakebase", - { kindHint: "lakebase" }, - ); + const { publish, unpublish } = useResourceStatusPublisher(id, "lakebase", { + kindHint: "lakebase", + }); useEffect(() => { publish({ @@ -283,10 +422,10 @@ function useLakebaseReadiness() { **Server config (in `analytics({...})`):** -| Option | Type | Default | Description | -|--------|------|---------|-------------| -| `warehouseStartupTimeoutMs` | `number` | `300000` (5 min) | Maximum time to wait for the warehouse to reach `RUNNING` before failing the request | -| `autoStartWarehouse` | `boolean` | `true` | When `true`, a `STOPPED` warehouse is auto-started on the first request. Set to `false` for cost-controlled deployments where billable warehouse starts must not be triggered by user requests; in that case `STOPPED` surfaces as a `ConfigurationError` | +| Option | Type | Default | Description | +| --------------------------- | --------- | ---------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `warehouseStartupTimeoutMs` | `number` | `300000` (5 min) | Maximum time to wait for the warehouse to reach `RUNNING` before failing the request | +| `autoStartWarehouse` | `boolean` | `true` | When `true`, a `STOPPED` warehouse is auto-started on the first request. Set to `false` for cost-controlled deployments where billable warehouse starts must not be triggered by user requests; in that case `STOPPED` surfaces as a `ConfigurationError` | **Example with loading/error/empty handling:** @@ -296,21 +435,27 @@ import { sql } from "@databricks/appkit-ui/js"; import { Skeleton } from "@databricks/appkit-ui"; function SpendTable() { - const params = useMemo(() => ({ - startDate: sql.date("2025-01-01"), - endDate: sql.date("2025-12-31"), - }), []); + const params = useMemo( + () => ({ + startDate: sql.date("2025-01-01"), + endDate: sql.date("2025-12-31"), + }), + [], + ); const { data, loading, error } = useAnalyticsQuery("spend_summary", params); if (loading) return ; if (error) return
{error}
; - if (!data?.length) return
No results
; + if (!data?.length) + return
No results
; return (
    {data.map((row) => ( -
  • {row.name}: ${row.cost_usd}
  • +
  • + {row.name}: ${row.cost_usd} +
  • ))}
); diff --git a/docs/static/schemas/metric-source.schema.json b/docs/static/schemas/metric-source.schema.json index 1976ea7ce..e606b92da 100644 --- a/docs/static/schemas/metric-source.schema.json +++ b/docs/static/schemas/metric-source.schema.json @@ -21,6 +21,7 @@ "properties": { "source": { "type": "string", + "maxLength": 767, "pattern": "^[^\\x00-\\x20\\x7f./]+\\.[^\\x00-\\x20\\x7f./]+\\.[^\\x00-\\x20\\x7f./]+$", "description": "Three-part Unity Catalog FQN of the metric view: ..", "examples": [ diff --git a/packages/appkit/src/app/index.ts b/packages/appkit/src/app/index.ts index 1b8428f1e..ec2260536 100644 --- a/packages/appkit/src/app/index.ts +++ b/packages/appkit/src/app/index.ts @@ -28,7 +28,26 @@ interface FileSystemAdapter { } export class AppManager { - private readonly queriesDir = path.resolve(process.cwd(), "config/queries"); + private readonly _queriesDir: string; + + constructor( + queriesDir: string = path.resolve(process.cwd(), "config/queries"), + ) { + this._queriesDir = queriesDir; + } + + get queriesDir(): string { + return this._queriesDir; + } + + /** + * Whether `req` is a dev-tunnel (`?dev`) request. Internal `?dev` predicate + * shared by {@link createFsAdapter} (tunnel-vs-`fs` branch) and + * {@link isNotFoundError} (dev's message-match not-found classification). + */ + private isDevRequest(req?: RequestLike): boolean { + return req?.query?.dev !== undefined; + } /** * Validates that a file path is within the queries directory @@ -53,7 +72,7 @@ export class AppManager { req?: RequestLike, devFileReader?: DevFileReader, ): FileSystemAdapter { - const isDevMode = req?.query?.dev !== undefined; + const isDevMode = this.isDevRequest(req); if (isDevMode && devFileReader && req) { // Dev mode: use WebSocket tunnel to read from local filesystem @@ -155,6 +174,50 @@ export class AppManager { return null; } } + + /** + * Read a single config file from the queries directory, dev-tunnel-aware. + * + * @param fileName - File name (or relative path) within the queries directory. + * @param req - Optional request object to detect dev mode. + * @param devFileReader - Optional DevFileReader to read via the WebSocket tunnel. + * @returns The raw file contents, or `null` when the file is absent / the path is rejected. + */ + async readConfigFile( + fileName: string, + req?: RequestLike, + devFileReader?: DevFileReader, + ): Promise { + // Traversal guard: refuse to read outside the queries directory. + const resolvedPath = this.validatePath(fileName); + if (!resolvedPath) { + return null; + } + + const fsAdapter = this.createFsAdapter(req, devFileReader); + + try { + return await fsAdapter.readFile(resolvedPath); + } catch (error) { + if (this.isNotFoundError(error, req)) { + return null; + } + // Any other error (permission / IO / tunnel) is fatal: propagate so the + // caller can distinguish it from a dormant (absent) config. + throw error; + } + } + + private isNotFoundError(error: unknown, req?: RequestLike): boolean { + if ((error as NodeJS.ErrnoException)?.code === "ENOENT") { + return true; + } + if (this.isDevRequest(req)) { + const message = (error as Error)?.message ?? ""; + return /ENOENT|no such file/i.test(message); + } + return false; + } } -export type { DevFileReader }; +export type { DevFileReader, RequestLike }; diff --git a/packages/appkit/src/app/tests/read-config-file.test.ts b/packages/appkit/src/app/tests/read-config-file.test.ts new file mode 100644 index 000000000..0a44e192c --- /dev/null +++ b/packages/appkit/src/app/tests/read-config-file.test.ts @@ -0,0 +1,157 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import type { DevFileReader } from "../index"; +import { AppManager } from "../index"; + +// NOTE: unlike the sibling `app.test.ts`, this spec deliberately does NOT mock +// `node:fs/promises` — `readConfigFile` is exercised against real temp files so +// the not-found-vs-throw distinction is driven by genuine errno codes. + +describe("AppManager.readConfigFile", () => { + let tmpDir: string; + let appManager: AppManager; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "appmgr-config-")); + appManager = new AppManager(tmpDir); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + describe("dir override + getter", () => { + test("queriesDir getter exposes the overridden directory", () => { + expect(appManager.queriesDir).toBe(tmpDir); + }); + + test("defaults to /config/queries when no override is given", () => { + const defaultManager = new AppManager(); + expect(defaultManager.queriesDir).toBe( + path.resolve(process.cwd(), "config/queries"), + ); + }); + }); + + describe("production mode (direct fs)", () => { + test("returns file contents for an existing file", async () => { + await fs.writeFile( + path.join(tmpDir, "metric-views.json"), + '{"hello":"world"}', + "utf8", + ); + + const result = await appManager.readConfigFile("metric-views.json"); + expect(result).toBe('{"hello":"world"}'); + }); + + test("returns null for a genuine not-found (ENOENT)", async () => { + const result = await appManager.readConfigFile("does-not-exist.json"); + expect(result).toBeNull(); + }); + + test("throws (does NOT return null) for a non-ENOENT error", async () => { + // Reading a directory as a file rejects with EISDIR — a real, non-ENOENT + // error that must propagate rather than be swallowed to null. + await fs.mkdir(path.join(tmpDir, "a-directory")); + + await expect(appManager.readConfigFile("a-directory")).rejects.toThrow(); + }); + + test("throws for a simulated EACCES (permission) error", async () => { + const err = Object.assign(new Error("permission denied"), { + code: "EACCES", + }); + vi.spyOn(fs, "readFile").mockRejectedValueOnce(err); + + await expect( + appManager.readConfigFile("metric-views.json"), + ).rejects.toThrow("permission denied"); + }); + + test("reads via direct fs (not the dev reader) without ?dev", async () => { + await fs.writeFile( + path.join(tmpDir, "metric-views.json"), + "prod-contents", + "utf8", + ); + const devFileReader: DevFileReader = { + readdir: vi.fn(), + readFile: vi.fn(), + }; + + const result = await appManager.readConfigFile( + "metric-views.json", + { query: {}, headers: {} }, + devFileReader, + ); + + expect(result).toBe("prod-contents"); + expect(devFileReader.readFile).not.toHaveBeenCalled(); + }); + }); + + describe("path traversal protection", () => { + test("returns null and does not read outside the queries dir", async () => { + const readSpy = vi.spyOn(fs, "readFile"); + const result = await appManager.readConfigFile("../../etc/passwd"); + + expect(result).toBeNull(); + expect(readSpy).not.toHaveBeenCalled(); + }); + }); + + describe("dev mode (WebSocket tunnel)", () => { + const devReq = { query: { dev: "true" }, headers: {} }; + + test("reads via devFileReader in dev mode", async () => { + const devFileReader: DevFileReader = { + readdir: vi.fn(), + readFile: vi.fn().mockResolvedValue("dev-contents"), + }; + + const result = await appManager.readConfigFile( + "metric-views.json", + devReq, + devFileReader, + ); + + expect(result).toBe("dev-contents"); + expect(devFileReader.readFile).toHaveBeenCalledWith( + expect.stringContaining("metric-views.json"), + devReq, + ); + }); + + test("returns null for the dev tunnel's not-found signal (best-effort)", async () => { + const devFileReader: DevFileReader = { + readdir: vi.fn(), + readFile: vi + .fn() + .mockRejectedValue(new Error("ENOENT: no such file or directory")), + }; + + const result = await appManager.readConfigFile( + "metric-views.json", + devReq, + devFileReader, + ); + + expect(result).toBeNull(); + }); + + test("throws for a non-not-found dev tunnel error", async () => { + const devFileReader: DevFileReader = { + readdir: vi.fn(), + readFile: vi.fn().mockRejectedValue(new Error("tunnel disconnected")), + }; + + await expect( + appManager.readConfigFile("metric-views.json", devReq, devFileReader), + ).rejects.toThrow("tunnel disconnected"); + }); + }); +}); diff --git a/packages/appkit/src/plugins/analytics/analytics.ts b/packages/appkit/src/plugins/analytics/analytics.ts index 4ad05cc37..7515fd140 100644 --- a/packages/appkit/src/plugins/analytics/analytics.ts +++ b/packages/appkit/src/plugins/analytics/analytics.ts @@ -29,6 +29,13 @@ import { Plugin, toPlugin } from "../../plugin"; import type { PluginManifest } from "../../registry"; import { queryDefaults } from "./defaults"; import manifest from "./manifest.json"; +import { + buildMetricSql, + composeMetricCacheKey, + deriveMetricExecutorKey, + loadMetricRegistry, + validateMetricRequest, +} from "./metric"; import { QueryProcessor } from "./query"; import { type ArrowCapability, @@ -41,6 +48,7 @@ import { type AnalyticsStreamMessage, type IAnalyticsConfig, type IAnalyticsQueryRequest, + type MetricRegistration, normalizeAnalyticsFormat, type WarehouseStatus, } from "./types"; @@ -137,10 +145,20 @@ export class AnalyticsPlugin extends Plugin implements ToolProvider { }, }); + // Metric-view route. Registered parallel to `/query` + // measures a registered UC Metric View over the same SSE envelope. + this.route(router, { + name: "metric", + method: "post", + path: "/metric/:key", + handler: async (req: express.Request, res: express.Response) => { + await this._handleMetricRoute(req, res); + }, + }); + // Column-names fallback for very wide Arrow schemas whose names don't fit - // in the `X-Appkit-Arrow-Columns` response header (see - // `_setArrowColumnsHeader`). The client hits this with the statement id - // from `X-Appkit-Arrow-Columns-Ref`. + // in the `X-Appkit-Arrow-Columns` response header. + // The client hits this with the statement id from `X-Appkit-Arrow-Columns-Ref`. this.route(router, { name: "arrow-columns", method: "get", @@ -426,6 +444,264 @@ export class AnalyticsPlugin extends Plugin implements ToolProvider { ); } + /** + * Handle metric-view execution requests (`POST /api/analytics/metric/:key`). + * + * Mirrors {@link _handleQueryRoute}'s JSON SSE path: the outer + * `executeStream` disables cache/retry and streams warehouse-readiness + * (`warehouse_status`) events, then the inner `execute` builds the metric SQL + * and delivers rows through {@link deliverJsonResult} as a `result` message. + * The `originalError` re-throw discipline preserves each error's structured + * `errorCode`/`clientMessage` for the SSE error payload. + * + * Lane dispatch is driven by the registration: an SP-lane metric runs as the + * app service principal (shared cache); an OBO-lane metric runs + * on-behalf-of the requesting user via `asUser(req)` (per-user cache keyed by + * a hash of the user identity). + */ + async _handleMetricRoute( + req: express.Request, + res: express.Response, + ): Promise { + const { key } = req.params; + + logger.debug(req, "Executing metric: %s", key); + + const event = logger.event(req); + event?.setComponent("analytics", "executeMetric").setContext("analytics", { + metric_key: key, + plugin: this.name, + }); + + if (!key) { + res.status(400).json({ error: "metric key is required" }); + return; + } + + // Resolve the registry from disk: read + parse `metric-views.json` once per + // request (no memoization). Reads through the plugin's shared `this.app` + // (the base `Plugin`'s `AppManager`, rooted at `config/queries/` under the + // process cwd), so this is dev-tunnel aware and inherits the traversal + // guard — same as the sibling `.sql` query path. + let registry: Record; + try { + registry = await loadMetricRegistry(this.app, req, this.devFileReader); + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + logger.warn(req, "Failed to load metric registry: %s", reason); + event?.setContext("analytics", { + metric_registry_load_error: reason, + }); + res.status(503).json({ + error: "Metric registry not available", + code: "METRIC_REGISTRY_LOAD_FAILED", + }); + return; + } + + // Own-property lookup: never resolve `key` to an inherited `Object.prototype` member. + const registration = Object.hasOwn(registry, key) + ? registry[key] + : undefined; + if (!registration) { + // Don't echo the user-supplied `key` back in the public response. + event?.setContext("analytics", { unknown_metric_key: key }); + res.status(404).json({ error: "Metric not found" }); + return; + } + + // Validate the body on the canonical error path. + // `validateMetricRequest` throws a `ValidationError` (400) whose message names only field paths, never raw values. + let request: ReturnType; + try { + request = validateMetricRequest(req.body ?? {}); + } catch (err) { + if (err instanceof AppKitError) { + res.status(err.statusCode).json({ error: err.message, code: err.code }); + return; + } + event?.setContext("analytics", { + unexpected_error: err instanceof Error ? err.message : String(err), + metric_key: key, + }); + logger.warn( + req, + "Unexpected throw during metric request validation for %s: %s", + key, + err instanceof Error ? err.message : String(err), + ); + res.status(400).json({ error: "Invalid request body" }); + return; + } + + // Lane dispatch. The lane comes from the registration (the entry's + // `executor` in metric-views.json), NOT a URL segment or `.obo.sql` + // filename: an OBO-lane metric runs on-behalf-of the requesting user + // (per-user cache via `asUser(req)`), an SP-lane metric as the app service + // principal (shared cache). + let executor: AnalyticsPlugin; + let executorKey: string; + try { + const isObo = registration.lane === "obo"; + executor = isObo ? this.asUser(req) : this; + executorKey = deriveMetricExecutorKey({ + lane: registration.lane, + userIdentity: isObo ? this.resolveUserId(req) : undefined, + }); + } catch (err) { + if (err instanceof AppKitError) { + res.status(err.statusCode).json({ error: err.message, code: err.code }); + return; + } + throw err; + } + + // Cache key. Composed over the canonicalized args (sorted measures/ + // dimensions, stable-sorted predicates, grain, timeDimension, limit) plus + // the `executorKey` — `"sp"` shares the cache across all users, a per-user + // identity hash isolates OBO callers. + const cacheConfig = { + ...queryDefaults.cache, + cacheKey: composeMetricCacheKey({ + metricKey: key, + source: registration.source, + measures: request.measures, + dimensions: request.dimensions, + timeGrain: request.timeGrain, + timeDimension: request.timeDimension, + filter: request.filter, + format: "JSON_ARRAY", + executorKey, + limit: request.limit, + }), + }; + + // Cache/retry/timeout scoped to the SQL execution itself (inner `execute`) + // so the warehouse-readiness phase isn't retried and the generator value + // never leaks into the cache. + const sqlConfig: PluginExecuteConfig = { + ...queryDefaults, + cache: cacheConfig, + }; + + // Outer stream: no cache/retry — `executeStream` would otherwise wrap the + // generator factory and cache the generator object itself. Telemetry + + // trace context still apply. + const streamExecutionSettings: StreamExecutionSettings = { + default: { + cache: { enabled: false }, + retry: { enabled: false }, + }, + }; + + const startupTimeoutMs = + this.config.warehouseStartupTimeoutMs ?? + DEFAULT_WAREHOUSE_STARTUP_TIMEOUT_MS; + const autoStartWarehouse = this.config.autoStartWarehouse ?? true; + + const self = this; + + await executor.executeStream( + res, + async function* ( + signal, + ): AsyncGenerator { + const workspaceClient = getWorkspaceClient(); + const warehouseId = await getWarehouseId(); + + // Stream warehouse-readiness updates as SSE events, then run SQL. + const readinessUpdates = streamCallbacks( + (emit) => + self.SQLClient.ensureWarehouseRunning( + workspaceClient, + warehouseId, + { + signal, + timeoutMs: startupTimeoutMs, + autoStart: autoStartWarehouse, + onStatus: emit, + }, + ), + ); + for await (const update of readinessUpdates) { + yield { + type: "warehouse_status", + status: { + state: update.state as WarehouseStatus["state"], + elapsedMs: update.elapsedMs, + }, + }; + } + + // `execute()` reduces a thrown error to `{ status, message }`, + // dropping the rich fields (`errorCode`, `clientMessage`). Capture the + // original here so we can re-throw it intact — the SSE error path + // (`StreamManager`) reads `errorCode`/`clientMessage` off it. + let originalError: unknown; + const sqlResult = await executor.execute( + async (sig) => { + try { + const { statement, parameters } = buildMetricSql( + registration, + request, + ); + const processedParams = + await self.queryProcessor.processQueryParams( + statement, + Object.keys(parameters).length > 0 ? parameters : undefined, + ); + // Reuse the query route's JSON delivery: INLINE JSON_ARRAY with + // an ARROW_STREAM-inline fallback, returning plain rows in a + // `result` message — byte-identical envelope to `/query`. + return await self._executeJsonArrayPath( + executor, + statement, + processedParams, + sig, + ); + } catch (err) { + originalError = err; + throw err; + } + }, + { default: sqlConfig }, + executorKey, + ); + + if (!sqlResult.ok) { + const msg = sqlResult.message; + const lower = msg.toLowerCase(); + if ( + lower.includes("operation was aborted") || + lower.includes("the request was aborted") || + lower.includes("statement was canceled") + ) { + const err = new DOMException( + lower.includes("canceled") ? msg : "The operation was aborted.", + "AbortError", + ); + throw err; + } + // Re-throw the original error so its structured `errorCode` and + // sanitized `clientMessage` survive to the SSE error payload. Fall + // back to a generic statement failure only if it wasn't an + // AppKitError. + if (originalError instanceof AppKitError) { + throw originalError; + } + const inner = msg.startsWith("Statement failed: ") + ? msg.slice("Statement failed: ".length) + : msg; + throw ExecutionError.statementFailed(inner); + } + + yield sqlResult.data as AnalyticsStreamMessage; + }, + streamExecutionSettings, + executorKey, + ); + } + /** * JSON_ARRAY SSE path. Delegates the disposition/format fallback to * {@link deliverJsonResult} (INLINE JSON_ARRAY → on `needs-arrow-inline`, diff --git a/packages/appkit/src/plugins/analytics/metric.ts b/packages/appkit/src/plugins/analytics/metric.ts new file mode 100644 index 000000000..3e8a6f47b --- /dev/null +++ b/packages/appkit/src/plugins/analytics/metric.ts @@ -0,0 +1 @@ +export * from "./mv"; diff --git a/packages/appkit/src/plugins/analytics/mv/cache.ts b/packages/appkit/src/plugins/analytics/mv/cache.ts new file mode 100644 index 000000000..0eb2ae8c5 --- /dev/null +++ b/packages/appkit/src/plugins/analytics/mv/cache.ts @@ -0,0 +1,75 @@ +import { createHash } from "node:crypto"; +import { AuthenticationError } from "../../../errors"; +import type { MetricFilter, MetricLane, MetricPredicate } from "../types"; +import { sortFilterChildren } from "./formatters"; +import type { MetricCacheKeyInput } from "./types"; + +export function composeMetricCacheKey(input: MetricCacheKeyInput): string[] { + const sortedMeasures = [...input.measures].sort(); + const sortedDimensions = [...(input.dimensions ?? [])].sort(); + const filterFingerprint = + input.filter !== undefined ? canonicalizeFilter(input.filter) : "_"; + // `timeDimension` only changes the SQL when `timeGrain` is set (see renderDimensionClause) + const timeDimensionPart = + input.timeGrain != null ? (input.timeDimension ?? "_") : "_"; + return [ + "metric", + input.metricKey, + input.source, + input.format, + // JSON-encode (not raw `.join(",")`): a comma is a legal identifier + // character (`isValidColumnName` rejects only control chars / newlines), so + // joining on `,` would collapse `["a,b"]` and `["a","b"]` to the same key + // element despite rendering different SQL. JSON quoting keeps the key + // one-to-one with the generated SQL — the same encoding `canonicalizeFilter` + // uses for predicate members below. + JSON.stringify(sortedMeasures), + JSON.stringify(sortedDimensions), + input.timeGrain ?? "_", + timeDimensionPart, + filterFingerprint, + typeof input.limit === "number" ? String(input.limit) : "_", + input.executorKey, + ]; +} + +export function deriveMetricExecutorKey(input: { + lane: MetricLane; + userIdentity?: string | null; +}): string { + if (input.lane === "sp") { + return "sp"; + } + const identity = input.userIdentity?.trim(); + if (!identity) { + throw AuthenticationError.missingUserId(); + } + return createHash("sha256").update(identity).digest("hex"); +} + +function canonicalizeFilter(node: MetricFilter): string { + if (node === null || typeof node !== "object") { + return "_"; + } + + if ("and" in node || "or" in node) { + const groupKey = "and" in node ? "and" : "or"; + const children = ( + node as { and?: ReadonlyArray } & { + or?: ReadonlyArray; + } + )[groupKey]; + + if (!Array.isArray(children) || children.length === 0) { + return `${groupKey}()`; + } + + const sorted = sortFilterChildren(children); + const childFingerprints = sorted.map(canonicalizeFilter); + return `${groupKey}(${childFingerprints.join(",")})`; + } + + const p = node as MetricPredicate; + const valuesPart = (p.values ?? []).map((v) => [typeof v, v]); + return `p(${JSON.stringify([p.member, p.operator, valuesPart])})`; +} diff --git a/packages/appkit/src/plugins/analytics/mv/constants.ts b/packages/appkit/src/plugins/analytics/mv/constants.ts new file mode 100644 index 000000000..abd210db7 --- /dev/null +++ b/packages/appkit/src/plugins/analytics/mv/constants.ts @@ -0,0 +1,89 @@ +import type { MetricFilterOperatorName, MetricLane } from "../types"; + +export const METRIC_CONFIG_FILE = "metric-views.json"; + +/** + * Measure, dimension, and filter-member names are **column identifiers**: they + * are validated by the shared {@link isValidColumnName} (rejects only control + * characters / newlines) and backtick-quoted via {@link quoteIdentifier} at + * every interpolation point. + * + * Time-grain token shape. Unlike the column identifiers above, the grain is + * interpolated as a single-quoted `date_trunc` unit LITERAL in {@link renderDimensionClause}, + * so it keeps a narrow keyword-shaped gate — that pattern is what keeps a hostile token out + * of the quoted-literal position. + */ +export const TIME_GRAIN_PATTERN = /^[a-z][a-z_]*$/; + +/** + * The depth count is the number of nested `{ and }` / `{ or }` wrappers + * encountered while descending; leaf predicates do not count toward depth. + * Prevents stack-overflowing the recursive validator or translator. + */ +export const METRIC_FILTER_MAX_DEPTH = 8; + +/** + * Cardinality caps on user-controlled arrays. + * Prevents stack-overflowing the recursive validator or translator. + */ +export const METRIC_MEASURES_MAX = 50; +export const METRIC_DIMENSIONS_MAX = 20; +export const METRIC_FILTER_VALUES_MAX = 1000; +export const METRIC_LIMIT_MAX = 100_000; + +/** + * Maximum number of children per AND/OR group node. + * Prevents stack-overflowing the recursive validator or translator. + */ +export const METRIC_FILTER_GROUP_MAX = 100; + +/** Operators that require at least one value. */ +export const LIST_VALUE_OPERATORS = new Set([ + "in", + "notIn", +]); + +/** Operators that reject `values` entirely. */ +export const NULL_OPERATORS = new Set([ + "set", + "notSet", +]); + +/** Operators that emit `LIKE` / `NOT LIKE` and require a string value. */ +export const STRING_OPERATORS = new Set([ + "contains", + "notContains", +]); + +/** Operators that require exactly one value. */ +export const SINGLE_VALUE_OPERATORS = new Set([ + "equals", + "notEquals", + "gt", + "gte", + "lt", + "lte", + ...STRING_OPERATORS, +]); + +/** + * Map an entry's declared `executor` to the internal execution lane: + * - `"user"` → `"obo"` (per-user cache, on-behalf-of) + * - `"app_service_principal"` (default) → `"sp"` (shared cache) + */ +export function laneFromExecutor( + executor: "app_service_principal" | "user", +): MetricLane { + return executor === "user" ? "obo" : "sp"; +} + +/** + * The exact twelve filter operators allowed at v1. The runtime tuple is the + * server-side source of truth; the client-side type union + * `MetricFilterOperatorName` mirrors these names statically. + */ +export const METRIC_FILTER_OPERATORS = [ + ...SINGLE_VALUE_OPERATORS, + ...LIST_VALUE_OPERATORS, + ...NULL_OPERATORS, +] as const satisfies readonly MetricFilterOperatorName[]; diff --git a/packages/appkit/src/plugins/analytics/mv/formatters.ts b/packages/appkit/src/plugins/analytics/mv/formatters.ts new file mode 100644 index 000000000..376e01055 --- /dev/null +++ b/packages/appkit/src/plugins/analytics/mv/formatters.ts @@ -0,0 +1,324 @@ +import { type SQLTypeMarker, sql as sqlHelpers } from "shared"; +import { + isValidColumnName, + isValidFqn, + quoteFqnForSql, + quoteIdentifier, +} from "../../../../../shared/src/schemas/metric-fqn"; +import type { + IAnalyticsMetricRequest, + MetricFilter, + MetricFilterOperatorName, + MetricPredicate, + MetricRegistration, +} from "../types"; +import { + METRIC_FILTER_MAX_DEPTH, + METRIC_FILTER_OPERATORS, + TIME_GRAIN_PATTERN, +} from "./constants"; +import type { FilterRenderState } from "./types"; + +function quoteSafeFqn(fqn: string): string { + if (!isValidFqn(fqn)) { + throw new Error( + `Refusing to build SQL: "${fqn}" is not a valid three-part UC FQN.`, + ); + } + return quoteFqnForSql(fqn); +} + +export function buildMetricSql( + registration: MetricRegistration, + request: IAnalyticsMetricRequest, +): { + statement: string; + parameters: Record; +} { + const quotedSource = quoteSafeFqn(registration.source); + + if (request.measures.length === 0) { + throw new Error("buildMetricSql requires at least one measure."); + } + + for (const m of request.measures) { + if (!isValidColumnName(m)) { + throw new Error( + `Refusing to build SQL: measure "${m}" is not a valid identifier.`, + ); + } + } + + const dimensions = request.dimensions ?? []; + for (const d of dimensions) { + if (!isValidColumnName(d)) { + throw new Error( + `Refusing to build SQL: dimension "${d}" is not a valid identifier.`, + ); + } + } + + const measureClauses = [...request.measures] + .sort() + .map((m) => `MEASURE(${quoteIdentifier(m)}) AS ${quoteIdentifier(m)}`); + + const dimensionClauses = [...dimensions] + .sort() + .map((d) => + renderDimensionClause(d, request.timeGrain, request.timeDimension), + ); + + const selectList = [...measureClauses, ...dimensionClauses].join(", "); + const groupByClause = dimensions.length > 0 ? " GROUP BY ALL" : ""; + + const limitClause = + typeof request.limit === "number" && request.limit > 0 + ? ` LIMIT ${Math.floor(request.limit)}` + : ""; + + const parameters: Record = {}; + let whereClause = ""; + if (request.filter !== undefined) { + const fragment = renderFilter(request.filter, parameters, { + counter: 0, + depth: 0, + }); + if (fragment !== null && fragment.length > 0) { + whereClause = ` WHERE ${fragment}`; + } + } + + const statement = `SELECT ${selectList} FROM ${quotedSource}${whereClause}${groupByClause}${limitClause}`; + return { statement, parameters }; +} + +function renderFilter( + node: MetricFilter, + params: Record, + state: FilterRenderState, +): string | null { + if (node === null || typeof node !== "object") { + throw new Error( + "Refusing to build SQL: filter node must be an object Predicate or { and } / { or } group.", + ); + } + + if ("and" in node || "or" in node) { + const groupKey = "and" in node ? "and" : "or"; + if (state.depth + 1 > METRIC_FILTER_MAX_DEPTH) { + throw new Error( + `Refusing to build SQL: filter AND/OR nesting exceeds the maximum depth of ${METRIC_FILTER_MAX_DEPTH}.`, + ); + } + + const children = ( + node as { and?: ReadonlyArray } & { + or?: ReadonlyArray; + } + )[groupKey]; + + if (!Array.isArray(children) || children.length === 0) { + // Empty group → the Boolean identity element for the operator, so the + // result is correct in ANY position (including nested inside the other + // operator). An empty OR is vacuously false (`1 = 0`); an empty AND is + // vacuously true (`1 = 1`). Returning `null` for empty-AND (dropped by the + // parent) would only be correct at the top level — nested in an OR it + // would silently vanish and turn `TRUE OR P` into `P`, under-returning + // rows. (The validator rejects empty groups outright; this is the + // defense-in-depth fallback and must be semantically correct too.) + return groupKey === "or" ? "1 = 0" : "1 = 1"; + } + + const sortedChildren = sortFilterChildren(children); + + const fragments: string[] = []; + const childState: FilterRenderState = { + counter: state.counter, + depth: state.depth + 1, + }; + for (const child of sortedChildren) { + const rendered = renderFilter(child, params, childState); + if (rendered != null && rendered.length > 0) { + fragments.push(rendered); + } + } + state.counter = childState.counter; + + if (fragments.length === 0) return null; + if (fragments.length === 1) return fragments[0]; + const joiner = groupKey === "and" ? " AND " : " OR "; + return `(${fragments.join(joiner)})`; + } + + const predicate = node as MetricPredicate; + + if (!isValidColumnName(predicate.member)) { + throw new Error( + `Refusing to build SQL: filter member "${predicate.member}" is not a valid identifier.`, + ); + } + if ( + !METRIC_FILTER_OPERATORS.includes( + predicate.operator as MetricFilterOperatorName, + ) + ) { + throw new Error( + `Refusing to build SQL: unknown filter operator "${predicate.operator}".`, + ); + } + + return renderPredicate(predicate, params, state); +} + +export function sortFilterChildren( + children: ReadonlyArray, +): MetricFilter[] { + const indexed = children.map((child, idx) => { + let key: string; + let isPredicate: boolean; + if ( + child !== null && + typeof child === "object" && + !("and" in child) && + !("or" in child) + ) { + const p = child as MetricPredicate; + key = JSON.stringify([p.member, p.operator]); + isPredicate = true; + } else { + key = ""; + isPredicate = false; + } + return { child, idx, key, isPredicate }; + }); + + indexed.sort((a, b) => { + if (a.isPredicate && !b.isPredicate) return -1; + if (!a.isPredicate && b.isPredicate) return 1; + if (a.isPredicate && b.isPredicate) { + if (a.key < b.key) return -1; + if (a.key > b.key) return 1; + } + return a.idx - b.idx; + }); + + return indexed.map((entry) => entry.child); +} + +function renderPredicate( + predicate: MetricPredicate, + params: Record, + state: FilterRenderState, +): string { + const col = quoteIdentifier(predicate.member); + const op = predicate.operator; + const values = predicate.values ?? []; + + switch (op) { + case "equals": + return `${col} = ${bindValue(values[0], params, state)}`; + case "notEquals": + return `${col} <> ${bindValue(values[0], params, state)}`; + case "gt": + return `${col} > ${bindValue(values[0], params, state)}`; + case "gte": + return `${col} >= ${bindValue(values[0], params, state)}`; + case "lt": + return `${col} < ${bindValue(values[0], params, state)}`; + case "lte": + return `${col} <= ${bindValue(values[0], params, state)}`; + case "in": { + const placeholders = values.map((v) => bindValue(v, params, state)); + return `${col} IN (${placeholders.join(", ")})`; + } + case "notIn": { + const placeholders = values.map((v) => bindValue(v, params, state)); + return `${col} NOT IN (${placeholders.join(", ")})`; + } + case "contains": { + const raw = values[0]; + if (typeof raw !== "string") { + throw new Error( + `Refusing to build SQL: filter operator "contains" requires a string value (got ${typeof raw}).`, + ); + } + return `${col} LIKE ${bindLikeValue(raw, params, state)}`; + } + case "notContains": { + const raw = values[0]; + if (typeof raw !== "string") { + throw new Error( + `Refusing to build SQL: filter operator "notContains" requires a string value (got ${typeof raw}).`, + ); + } + return `${col} NOT LIKE ${bindLikeValue(raw, params, state)}`; + } + case "set": + return `${col} IS NOT NULL`; + case "notSet": + return `${col} IS NULL`; + default: { + const _exhaustive: never = op; + throw new Error( + `Refusing to build SQL: unhandled filter operator "${_exhaustive as string}".`, + ); + } + } +} + +function bindValue( + value: string | number | undefined, + params: Record, + state: FilterRenderState, +): string { + if (value === undefined) { + throw new Error( + "Refusing to build SQL: filter predicate is missing a required value.", + ); + } + const name = `f_${state.counter}`; + state.counter += 1; + if (typeof value === "number") { + params[name] = sqlHelpers.number(value); + } else if (typeof value === "string") { + params[name] = sqlHelpers.string(value); + } else { + throw new Error( + `Refusing to build SQL: filter value must be a string or number (got ${typeof value}).`, + ); + } + return `:${name}`; +} + +function bindLikeValue( + value: string, + params: Record, + state: FilterRenderState, +): string { + const name = `f_${state.counter}`; + state.counter += 1; + params[name] = sqlHelpers.string(`%${value}%`); + return `:${name}`; +} + +function renderDimensionClause( + dim: string, + timeGrain: string | undefined, + timeDimension: string | undefined, +): string { + if (timeGrain != null && dim === timeDimension) { + if (!isValidColumnName(dim)) { + throw new Error( + `Refusing to build SQL: timeDimension "${dim}" is not a valid identifier.`, + ); + } + if (!TIME_GRAIN_PATTERN.test(timeGrain)) { + throw new Error( + `Refusing to build SQL: timeGrain "${timeGrain}" is not a valid grain token.`, + ); + } + const quoted = quoteIdentifier(dim); + return `date_trunc('${timeGrain}', ${quoted}) AS ${quoted}`; + } + return quoteIdentifier(dim); +} diff --git a/packages/appkit/src/plugins/analytics/mv/index.ts b/packages/appkit/src/plugins/analytics/mv/index.ts new file mode 100644 index 000000000..17c97793e --- /dev/null +++ b/packages/appkit/src/plugins/analytics/mv/index.ts @@ -0,0 +1,4 @@ +export { composeMetricCacheKey, deriveMetricExecutorKey } from "./cache"; +export { buildMetricSql } from "./formatters"; +export { loadMetricRegistry } from "./registry"; +export { validateMetricRequest } from "./schemas"; diff --git a/packages/appkit/src/plugins/analytics/mv/registry.ts b/packages/appkit/src/plugins/analytics/mv/registry.ts new file mode 100644 index 000000000..fa3c13174 --- /dev/null +++ b/packages/appkit/src/plugins/analytics/mv/registry.ts @@ -0,0 +1,85 @@ +import path from "node:path"; +// Canonical metric-source schema — the single source of truth for +// `metric-views.json`. Imported from the shared source directly (matching the +// type-generator's runtime, which pulls the zod-free `metric-fqn.ts` from the +// same tree) so the runtime and the generated JSON schema validate identically. +import { metricSourceSchema } from "../../../../../shared/src/schemas/metric-source"; +import type { AppManager, DevFileReader, RequestLike } from "../../../app"; +import { createLogger } from "../../../logging/logger"; +import type { MetricRegistration } from "../types"; +import { laneFromExecutor, METRIC_CONFIG_FILE } from "./constants"; + +const logger = createLogger("analytics:metric-views"); + +/** + * Read and validate `config/queries/metric-views.json` into a metric registry. + * + * Async and stateless — registration is a pure config parse with no warehouse + * round-trip, no `DESCRIBE`, and no build-time metadata bundle. + * + * The file is read **through {@link AppManager.readConfigFile}** rather than + * `node:fs` directly, so this path is dev-tunnel-aware (a `?dev` request reads + * the developer's local file over the WebSocket tunnel) and inherits the + * traversal guard. In production that's a plain `fs.readFile` under the hood, + * so the semantics below are unchanged. + * + * Absent file (or a rejected traversal path) -> empty registry (`null` from + * `readConfigFile`). + * Malformed file -> 503 (throws). + * + * @param app - The {@link AppManager} that resolves + reads the config file. + * @param req - Optional request object, used to detect dev mode. + * @param devFileReader - Optional dev tunnel reader. + */ +export async function loadMetricRegistry( + app: AppManager, + req?: RequestLike, + devFileReader?: DevFileReader, +): Promise> { + const metricPath = path.join(app.queriesDir, METRIC_CONFIG_FILE); + + const raw = await app.readConfigFile(METRIC_CONFIG_FILE, req, devFileReader); + if (raw === null) { + // Absent file (ENOENT in prod / dev-tunnel not-found) or a rejected + // traversal path → dormant. Same as the old ENOENT branch. + return Object.create(null); + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new Error( + `Failed to parse metric-views.json at ${metricPath}: ${(err as Error).message}`, + ); + } + + const result = metricSourceSchema.safeParse(parsed); + if (!result.success) { + const issues = result.error.issues + .map((i) => `${i.path.join(".")}: ${i.message}`) + .join("; "); + throw new Error(`Invalid metric-views.json at ${metricPath}: ${issues}`); + } + + // Null-prototype map so a metric key that collides with an inherited + // `Object.prototype` member (`__proto__`, `constructor`, `toString`, …) + // cannot resolve to a truthy non-registration at the `registry[key]` read + // site and slip past the unknown-key 404. + // Keys are still grammar-gated by `metricKeySchema` (identifier shape), + // but the null prototype removes the whole class of inherited-property lookups as a boundary. + const registry: Record = Object.create(null); + for (const [key, entry] of Object.entries(result.data.metricViews ?? {})) { + registry[key] = { + key, + source: entry.source, + lane: laneFromExecutor(entry.executor), + }; + } + + logger.debug( + "Loaded metric registry: %d entry(ies)", + Object.keys(registry).length, + ); + return registry; +} diff --git a/packages/appkit/src/plugins/analytics/mv/schemas.ts b/packages/appkit/src/plugins/analytics/mv/schemas.ts new file mode 100644 index 000000000..92fbed0f0 --- /dev/null +++ b/packages/appkit/src/plugins/analytics/mv/schemas.ts @@ -0,0 +1,362 @@ +import { z } from "zod"; +import { isValidColumnName } from "../../../../../shared/src/schemas/metric-fqn"; +import { ValidationError } from "../../../errors"; +import type { + IAnalyticsMetricRequest, + MetricFilter, + MetricFilterOperatorName, + MetricPredicate, +} from "../types"; +import { normalizeAnalyticsFormat } from "../types"; +import { + LIST_VALUE_OPERATORS, + METRIC_DIMENSIONS_MAX, + METRIC_FILTER_GROUP_MAX, + METRIC_FILTER_MAX_DEPTH, + METRIC_FILTER_OPERATORS, + METRIC_FILTER_VALUES_MAX, + METRIC_LIMIT_MAX, + METRIC_MEASURES_MAX, + NULL_OPERATORS, + SINGLE_VALUE_OPERATORS, + STRING_OPERATORS, + TIME_GRAIN_PATTERN, +} from "./constants"; + +/** A leaf predicate: `{ member, operator, values? }`, no extra keys. */ +const filterPredicateSchema: z.ZodType = z + .object({ + member: z + .string() + .min(1, { message: "filter predicate 'member' cannot be empty" }) + .refine(isValidColumnName, { + message: + "filter predicate 'member' contains a character that cannot be used in a SQL identifier (control character or newline)", + }), + operator: z.string().min(1, { + message: "filter predicate 'operator' cannot be empty", + }) as z.ZodType, + values: z + .array(z.union([z.string(), z.number()])) + .max(METRIC_FILTER_VALUES_MAX, { + message: `filter predicate 'values' length exceeds the maximum of ${METRIC_FILTER_VALUES_MAX}`, + }) + .optional(), + }) + .strict(); + +/** Recursive filter: a predicate leaf or an `{ and }` / `{ or }` group. */ +const filterSchema: z.ZodType = z.lazy(() => + z.union([ + filterPredicateSchema, + z + .object({ + and: z.array(filterSchema).max(METRIC_FILTER_GROUP_MAX, { + message: `filter 'and' group exceeds the maximum of ${METRIC_FILTER_GROUP_MAX} children`, + }), + }) + .strict(), + z + .object({ + or: z.array(filterSchema).max(METRIC_FILTER_GROUP_MAX, { + message: `filter 'or' group exceeds the maximum of ${METRIC_FILTER_GROUP_MAX} children`, + }), + }) + .strict(), + ]), +); + +const metricRequestSchema = z + .object({ + measures: z + .array( + z + .string() + .min(1, "measure name cannot be empty") + .refine(isValidColumnName, { + message: + "measure name contains a character that cannot be used in a SQL identifier (control character or newline)", + }), + ) + .min(1, "at least one measure is required") + .max(METRIC_MEASURES_MAX, { + message: `measures length exceeds the maximum of ${METRIC_MEASURES_MAX}`, + }), + dimensions: z + .array( + z + .string() + .min(1, "dimension name cannot be empty") + .refine(isValidColumnName, { + message: + "dimension name contains a character that cannot be used in a SQL identifier (control character or newline)", + }), + ) + .max(METRIC_DIMENSIONS_MAX, { + message: `dimensions length exceeds the maximum of ${METRIC_DIMENSIONS_MAX}`, + }) + .optional(), + filter: filterSchema.optional(), + // Grammar-shaped bucketing grain, applied to `timeDimension` via + // `date_trunc`. The token is validated for safety here; the grain literal + // is interpolated (single-quoted, never a bind param) in + // `renderDimensionClause`, so this pattern gate is the security boundary. + timeGrain: z + .string() + .min(1, { message: "timeGrain cannot be empty" }) + .regex(TIME_GRAIN_PATTERN, { + message: "timeGrain must match /^[a-z][a-z_]*$/", + }) + .optional(), + // The single dimension `timeGrain` applies to via `date_trunc`. A column + // identifier (backtick-quoted at interpolation), so it accepts the full + // delimited-identifier grammar. Cross-field rules in `superRefine`: + // required when `timeGrain` is set, and must be one of `dimensions`. + timeDimension: z + .string() + .min(1, { message: "timeDimension cannot be empty" }) + .refine(isValidColumnName, { + message: + "timeDimension contains a character that cannot be used in a SQL identifier (control character or newline)", + }) + .optional(), + limit: z + .number() + .int({ message: "limit must be an integer" }) + .positive({ message: "limit must be positive" }) + .max(METRIC_LIMIT_MAX, { + message: `limit exceeds the maximum of ${METRIC_LIMIT_MAX}`, + }) + .optional(), + format: z.enum(["JSON_ARRAY", "ARROW_STREAM", "JSON", "ARROW"]).optional(), + }) + .strict() + .superRefine((value, ctx) => { + if (value.filter != null) { + validateFilterTree(value.filter, ctx, ["filter"], 0); + } + + const seen = new Set(); + const collided = new Set(); + for (const name of [...value.measures, ...(value.dimensions ?? [])]) { + if (seen.has(name)) { + collided.add(name); + } + seen.add(name); + } + if (collided.size > 0) { + ctx.addIssue({ + code: "custom", + message: + "measures and dimensions must be unique across both lists (a name cannot repeat, nor appear as both a measure and a dimension)", + path: ["measures"], + }); + } + + if ( + value.format != null && + normalizeAnalyticsFormat(value.format) !== "JSON_ARRAY" + ) { + ctx.addIssue({ + code: "custom", + message: + "format: only JSON_ARRAY is supported on the metric route at v1 (ARROW_STREAM is not yet implemented)", + path: ["format"], + }); + } + + if (value.timeGrain != null && value.timeDimension == null) { + ctx.addIssue({ + code: "custom", + message: "timeDimension is required when timeGrain is set", + path: ["timeDimension"], + }); + } + if ( + value.timeDimension != null && + !(value.dimensions ?? []).includes(value.timeDimension) + ) { + ctx.addIssue({ + code: "custom", + message: "timeDimension must be one of dimensions", + path: ["timeDimension"], + }); + } + }) as z.ZodType; + +function validateFilterTree( + node: MetricFilter, + ctx: z.RefinementCtx, + path: Array, + depth: number, +): void { + if (node === null || typeof node !== "object") { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path, + message: "filter node must be a Predicate or { and } / { or } group", + }); + return; + } + + if ("and" in node || "or" in node) { + if (depth + 1 > METRIC_FILTER_MAX_DEPTH) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path, + message: `filter AND/OR nesting exceeds the maximum depth of ${METRIC_FILTER_MAX_DEPTH}`, + }); + return; + } + + const groupKey = "and" in node ? "and" : "or"; + const children = ( + node as { and?: ReadonlyArray } & { + or?: ReadonlyArray; + } + )[groupKey]; + + if (!Array.isArray(children)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [...path, groupKey], + message: `filter ${groupKey} group must be an array of predicates or nested groups`, + }); + return; + } + + if (children.length === 0) { + // Reject empty groups of either kind. An empty `or` is vacuously false; + // an empty `and` contributes no constraint and renders to no WHERE + // clause — identical SQL to omitting `filter` entirely, but it would + // canonicalize to a distinct cache key (`and()` vs `_`), needlessly + // splitting the cache across semantically identical requests. Requiring + // at least one child keeps request shape ↔ cache key one-to-one. + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [...path, groupKey], + message: `filter '${groupKey}' group must contain at least one predicate`, + }); + return; + } + + children.forEach((child, idx) => { + validateFilterTree(child, ctx, [...path, groupKey, idx], depth + 1); + }); + return; + } + + const predicate = node as MetricPredicate; + + if ( + !METRIC_FILTER_OPERATORS.includes( + predicate.operator as MetricFilterOperatorName, + ) + ) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [...path, "operator"], + message: `filter operator "${predicate.operator}" is not one of: ${METRIC_FILTER_OPERATORS.join(", ")}`, + }); + return; + } + + const op = predicate.operator; + const values = predicate.values; + const valuesLen = values?.length ?? 0; + + if (NULL_OPERATORS.has(op)) { + if (values != null && valuesLen > 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [...path, "values"], + message: `filter operator "${op}" must not carry values`, + }); + } + } else if (SINGLE_VALUE_OPERATORS.has(op)) { + if (valuesLen !== 1) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [...path, "values"], + message: `filter operator "${op}" requires exactly one value (got ${valuesLen})`, + }); + } + } else if (LIST_VALUE_OPERATORS.has(op)) { + if (valuesLen < 1) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [...path, "values"], + message: `filter operator "${op}" requires at least one value`, + }); + } + } + + if (STRING_OPERATORS.has(op) && valuesLen > 0) { + const v = predicate.values?.[0]; + if (typeof v !== "string") { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [...path, "values"], + message: `filter operator "${op}" requires a string value (got ${typeof v})`, + }); + } + } +} + +function preCheckFilterDepth(filter: unknown): void { + if (filter == null || typeof filter !== "object") return; + const stack: Array<[unknown, number]> = [[filter, 0]]; + while (stack.length > 0) { + const popped = stack.pop(); + if (popped === undefined) continue; + const [node, depth] = popped; + if (node == null || typeof node !== "object") continue; + const obj = node as Record; + for (const groupKey of ["and", "or"] as const) { + const children = obj[groupKey]; + if (!Array.isArray(children)) continue; + if (children.length > METRIC_FILTER_GROUP_MAX) { + throw new ValidationError( + "Invalid metric request body (fields: filter)", + { + context: { + reason: `filter ${groupKey} group has ${children.length} children; the maximum is ${METRIC_FILTER_GROUP_MAX}`, + }, + }, + ); + } + if (depth + 1 > METRIC_FILTER_MAX_DEPTH) { + throw new ValidationError( + "Invalid metric request body (fields: filter)", + { + context: { + reason: `filter AND/OR nesting exceeds the maximum depth of ${METRIC_FILTER_MAX_DEPTH}`, + }, + }, + ); + } + for (const child of children) { + stack.push([child, depth + 1]); + } + } + } +} + +export function validateMetricRequest(body: unknown): IAnalyticsMetricRequest { + if (body != null && typeof body === "object") { + preCheckFilterDepth((body as { filter?: unknown }).filter); + } + const result = metricRequestSchema.safeParse(body); + if (!result.success) { + const fieldPaths = result.error.issues + .map((i) => i.path.join(".") || "(root)") + .join(", "); + throw new ValidationError( + fieldPaths.length > 0 + ? `Invalid metric request body (fields: ${fieldPaths})` + : "Invalid metric request body", + { context: { issues: result.error.issues } }, + ); + } + return result.data; +} diff --git a/packages/appkit/src/plugins/analytics/mv/types.ts b/packages/appkit/src/plugins/analytics/mv/types.ts new file mode 100644 index 000000000..791435d17 --- /dev/null +++ b/packages/appkit/src/plugins/analytics/mv/types.ts @@ -0,0 +1,19 @@ +import type { MetricFilter } from "../types"; + +export interface FilterRenderState { + counter: number; + depth: number; +} + +export interface MetricCacheKeyInput { + metricKey: string; + source: string; + measures: string[]; + dimensions?: string[]; + timeGrain?: string; + timeDimension?: string; + filter?: MetricFilter; + format: string; + executorKey: string; + limit?: number; +} diff --git a/packages/appkit/src/plugins/analytics/tests/analytics.test.ts b/packages/appkit/src/plugins/analytics/tests/analytics.test.ts index 356777922..aff246145 100644 --- a/packages/appkit/src/plugins/analytics/tests/analytics.test.ts +++ b/packages/appkit/src/plugins/analytics/tests/analytics.test.ts @@ -90,18 +90,23 @@ describe("Analytics Plugin", () => { }); describe("injectRoutes", () => { - test("should register single POST route for queries", () => { + test("should register the query and metric POST routes", () => { const plugin = new AnalyticsPlugin(config); const { router } = createMockRouter(); plugin.injectRoutes(router); - // Only 1 POST route - asUser is determined by .obo.sql file convention - expect(router.post).toHaveBeenCalledTimes(1); + // Two POST routes: the SQL query route (asUser determined by the + // .obo.sql file convention) and the metric-view route. + expect(router.post).toHaveBeenCalledTimes(2); expect(router.post).toHaveBeenCalledWith( "/query/:query_key", expect.any(Function), ); + expect(router.post).toHaveBeenCalledWith( + "/metric/:key", + expect.any(Function), + ); }); test("/query/:query_key should return 400 when query_key is missing", async () => { diff --git a/packages/appkit/src/plugins/analytics/tests/metric.test.ts b/packages/appkit/src/plugins/analytics/tests/metric.test.ts new file mode 100644 index 000000000..41b78079c --- /dev/null +++ b/packages/appkit/src/plugins/analytics/tests/metric.test.ts @@ -0,0 +1,2230 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { + createMockRequest, + createMockResponse, + createMockRouter, + mockServiceContext, + setupDatabricksEnv, +} from "@tools/test-helpers"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { AppManager } from "../../../app"; +import { ServiceContext } from "../../../context/service-context"; +import { AuthenticationError } from "../../../errors"; +import { AnalyticsPlugin } from "../analytics"; +import { + buildMetricSql, + composeMetricCacheKey, + deriveMetricExecutorKey, + loadMetricRegistry, + validateMetricRequest, +} from "../metric"; +import type { + IAnalyticsConfig, + MetricFilter, + MetricRegistration, +} from "../types"; + +// Mirror the analytics.test.ts CacheManager mock so the inner `execute`'s +// cache interceptor is a no-op pass-through (each request re-executes). +const { mockCacheStore, mockCacheInstance } = vi.hoisted(() => { + const store = new Map(); + const generateKey = (parts: unknown[], userKey: string): string => { + const { createHash } = require("node:crypto"); + const serialized = JSON.stringify([userKey, ...parts]); + return createHash("sha256").update(serialized).digest("hex"); + }; + const instance = { + get: vi.fn(), + set: vi.fn(), + delete: vi.fn(), + getOrExecute: vi.fn( + async (key: unknown[], fn: () => Promise, userKey: string) => { + const cacheKey = generateKey(key, userKey); + if (store.has(cacheKey)) return store.get(cacheKey); + const result = await fn(); + store.set(cacheKey, result); + return result; + }, + ), + generateKey: vi.fn((parts: unknown[], userKey: string) => + generateKey(parts, userKey), + ), + }; + return { mockCacheStore: store, mockCacheInstance: instance }; +}); + +vi.mock("../../../cache", () => ({ + CacheManager: { + getInstanceSync: vi.fn(() => mockCacheInstance), + }, +})); + +// Temp dirs created by `registryDir` / `writeRegistry`, cleaned up after each +// test. Using real files (pointing the plugin's `AppManager` at the dir, see +// `pluginForDir`) exercises the actual read → parse path in +// `loadMetricRegistry` rather than poking private plugin state. +const tempRegistryDirs: string[] = []; + +/** + * Construct an `AnalyticsPlugin` whose metric-registry gateway is pointed at + * `dir`. + * + * The plugin reads the registry through the base `Plugin`'s shared `this.app` + * (an `AppManager` rooted at `config/queries/` under the cwd). There is no + * config field to relocate that directory, so a test points the plugin at a + * fixture dir by overriding the `AppManager` with one constructed over `dir`. + * `app` is `protected` on the base `Plugin`, hence the deliberate test-only + * cast — the single seam every route-handler test threads through. + */ +function pluginForDir(config: IAnalyticsConfig, dir: string): AnalyticsPlugin { + const plugin = new AnalyticsPlugin(config); + (plugin as any).app = new AppManager(dir); + return plugin; +} + +/** + * Write a `metric-views.json` into a fresh temp dir and return the dir, for use + * with `pluginForDir(config, dir)`. Accepts the internal `MetricRegistration` + * shape (matching the old `setRegistry` helper) and maps each entry's `lane` + * back to the config's `executor` field. + */ +function registryDir(registry: Record): string { + const dir = mkdtempSync(path.join(tmpdir(), "mv-route-")); + tempRegistryDirs.push(dir); + const metricViews: Record = {}; + for (const [key, reg] of Object.entries(registry)) { + metricViews[key] = { + source: reg.source, + executor: reg.lane === "obo" ? "user" : "app_service_principal", + }; + } + writeFileSync( + path.join(dir, "metric-views.json"), + JSON.stringify({ metricViews }), + ); + return dir; +} + +/** + * Overwrite the `metric-views.json` in an existing temp dir (for hot-reload / + * self-heal tests). `raw` lets a test write deliberately malformed content. + */ +function writeRegistry( + dir: string, + content: Record | string, +): void { + const body = + typeof content === "string" + ? content + : JSON.stringify({ + metricViews: Object.fromEntries( + Object.entries(content).map(([key, reg]) => [ + key, + { + source: reg.source, + executor: reg.lane === "obo" ? "user" : "app_service_principal", + }, + ]), + ), + }); + writeFileSync(path.join(dir, "metric-views.json"), body); +} + +describe("analytics metric route (Phase 1)", () => { + let config: IAnalyticsConfig; + let serviceContextMock: Awaited>; + + beforeEach(async () => { + config = { timeout: 5000 }; + setupDatabricksEnv(); + mockCacheStore.clear(); + ServiceContext.reset(); + serviceContextMock = await mockServiceContext(); + }); + + afterEach(() => { + serviceContextMock?.restore(); + while (tempRegistryDirs.length > 0) { + const dir = tempRegistryDirs.pop(); + if (dir) rmSync(dir, { recursive: true, force: true }); + } + }); + + describe("injectRoutes", () => { + test("registers POST /metric/:key alongside /query", () => { + const plugin = new AnalyticsPlugin(config); + const { router } = createMockRouter(); + + plugin.injectRoutes(router); + + expect(router.post).toHaveBeenCalledWith( + "/metric/:key", + expect.any(Function), + ); + }); + }); + + // ── Identifier safety — the primary security test. Measure/dimension names + // are backtick-quoted (not narrow-gated) at interpolation, so an injection- + // shaped name is NEUTRALIZED by quoting rather than rejected: it becomes an + // inert (if nonexistent) column the warehouse resolves away. Only a name + // that cannot be safely quoted at all — one containing a control character + // or newline — is refused. + describe("buildMetricSql identifier safety (quoting)", () => { + const registration: MetricRegistration = { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }; + + test("neutralizes an injection-shaped measure by quoting (no breakout)", () => { + // `arr; DROP TABLE users` has no control chars, so it is a valid (if + // nonexistent) column name: quoted whole, the `;` and `DROP` are inert + // inside the backtick-delimited identifier — not separate statements. + const { statement } = buildMetricSql(registration, { + measures: ["arr; DROP TABLE users"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr; DROP TABLE users`) AS `arr; DROP TABLE users` FROM `cat`.`sch`.`revenue_metrics`", + ); + }); + + test("neutralizes a backtick in a measure by doubling it", () => { + // The one real breakout char is the backtick; quoteIdentifier doubles it + // so it cannot close the identifier early. + const { statement } = buildMetricSql(registration, { + measures: ["arr`"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr```) AS `arr``` FROM `cat`.`sch`.`revenue_metrics`", + ); + }); + + test("throws for a measure containing a control character (cannot be quoted)", () => { + expect(() => + buildMetricSql(registration, { measures: ["arr\ndrop"] }), + ).toThrow(/not a valid identifier|control character/); + }); + + test("throws when no measures are supplied", () => { + expect(() => buildMetricSql(registration, { measures: [] })).toThrow( + /at least one measure/, + ); + }); + + test("throws for a non-three-part source FQN", () => { + expect(() => + buildMetricSql( + { key: "x", source: "cat.sch", lane: "sp" }, + { measures: ["arr"] }, + ), + ).toThrow(/not a valid three-part UC FQN/); + }); + + test("accepts a hyphenated UC-legal FQN and backtick-quotes it", () => { + // Regression for the grammar divergence: `prod-data` is a legal UC + // object name (hyphens are allowed in quoted identifiers) and passes the + // shared schema + typegen, but the old narrow runtime pattern + // [a-zA-Z0-9_-] anchored per segment REJECTED it — a latent prod break on + // a documented-legal name. The builder now accepts it and quotes every + // segment, so it reaches the warehouse as valid SQL rather than throwing. + const { statement } = buildMetricSql( + { key: "x", source: "prod-data.analytics.revenue", lane: "sp" }, + { measures: ["arr"] }, + ); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr` FROM `prod-data`.`analytics`.`revenue`", + ); + }); + + test("backtick-quoting neutralizes an injection-shaped FQN segment", () => { + // A source carrying a backtick would break out of the quoted identifier + // if interpolated raw; quoteFqnForSql doubles it. (isValidFqn rejects + // most such names first, but the quoting is the actual injection + // boundary — this asserts it, not just the grammar gate.) + const { statement } = buildMetricSql( + { key: "x", source: "cat.sch.re`v", lane: "sp" }, + { measures: ["arr"] }, + ); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr` FROM `cat`.`sch`.`re``v`", + ); + }); + }); + + // ── Measures-only SQL shape. + describe("buildMetricSql SQL shape", () => { + const registration: MetricRegistration = { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }; + + test("single measure → SELECT MEASURE(`m`) AS `m` FROM ", () => { + const { statement, parameters } = buildMetricSql(registration, { + measures: ["arr"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr` FROM `cat`.`sch`.`revenue_metrics`", + ); + expect(parameters).toEqual({}); + }); + + test("multiple measures are sorted for a deterministic SELECT list", () => { + const { statement } = buildMetricSql(registration, { + measures: ["revenue", "arr"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, MEASURE(`revenue`) AS `revenue` FROM `cat`.`sch`.`revenue_metrics`", + ); + }); + + test("positive limit appends a floored LIMIT clause", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + limit: 10.9, + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr` FROM `cat`.`sch`.`revenue_metrics` LIMIT 10", + ); + }); + }); + + // ── Phase 2: dimensions + GROUP BY ALL. Bare dimensions here; date_trunc + // grain application (via timeDimension) is covered in its own block below. + describe("buildMetricSql dimensions + GROUP BY", () => { + const registration: MetricRegistration = { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }; + + test("measures + dimensions → SELECT MEASURE(...), FROM GROUP BY ALL", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["region"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, `region` FROM `cat`.`sch`.`revenue_metrics` GROUP BY ALL", + ); + }); + + test("dimensions are sorted for a deterministic SELECT list", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["segment", "region"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, `region`, `segment` FROM `cat`.`sch`.`revenue_metrics` GROUP BY ALL", + ); + }); + + test("measures sort before dimensions in the SELECT list", () => { + const { statement } = buildMetricSql(registration, { + measures: ["revenue", "arr"], + dimensions: ["region"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, MEASURE(`revenue`) AS `revenue`, `region` FROM `cat`.`sch`.`revenue_metrics` GROUP BY ALL", + ); + }); + + test("empty dimensions array → no GROUP BY (ungrouped)", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + dimensions: [], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr` FROM `cat`.`sch`.`revenue_metrics`", + ); + }); + + test("dimensions + limit compose GROUP BY ALL then LIMIT", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["region"], + limit: 100, + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, `region` FROM `cat`.`sch`.`revenue_metrics` GROUP BY ALL LIMIT 100", + ); + }); + + test("no timeGrain → all dimensions render bare (no date_trunc)", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["order_date", "region"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, `order_date`, `region` FROM `cat`.`sch`.`revenue_metrics` GROUP BY ALL", + ); + expect(statement).not.toContain("date_trunc"); + }); + }); + + // ── Phase 2a: timeGrain + timeDimension → date_trunc on the named column. + // The grain is a grammar-gated single-quoted literal; the column keeps its + // plain alias; other dimensions render bare; GROUP BY ALL is present. + describe("buildMetricSql timeGrain + timeDimension (date_trunc)", () => { + const registration: MetricRegistration = { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }; + + test("buckets only the timeDimension via date_trunc; other dimensions bare", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["order_date", "region"], + timeGrain: "month", + timeDimension: "order_date", + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, date_trunc('month', `order_date`) AS `order_date`, `region` FROM `cat`.`sch`.`revenue_metrics` GROUP BY ALL", + ); + expect(statement).toContain( + "date_trunc('month', `order_date`) AS `order_date`", + ); + expect(statement).toContain(" GROUP BY ALL"); + }); + + test("the grain literal is threaded through (day) for the timeDimension", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["order_date"], + timeGrain: "day", + timeDimension: "order_date", + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, date_trunc('day', `order_date`) AS `order_date` FROM `cat`.`sch`.`revenue_metrics` GROUP BY ALL", + ); + }); + + test("a grammar-invalid timeGrain throws in the builder (defense-in-depth, even if validation is bypassed)", () => { + // buildMetricSql is exported and may be reached on a path that skips + // validateMetricRequest, so the grain — interpolated into a single-quoted + // date_trunc literal — is re-gated at the interpolation point. A quote- + // breakout payload must be refused by the builder itself. + expect(() => + buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["order_date"], + timeGrain: "month'); DROP TABLE t;--", + timeDimension: "order_date", + }), + ).toThrow(/not a valid grain token/); + }); + }); + + // ── Phase 2: dimension identifier safety. A dimension is backtick-quoted at + // interpolation, so an injection-shaped name is neutralized (inert quoted + // column), and only an unquotable (control-char) name throws. + describe("buildMetricSql dimension identifier safety (quoting)", () => { + const registration: MetricRegistration = { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }; + + test("neutralizes an injection-shaped dimension by quoting", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["region; DROP TABLE users"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, `region; DROP TABLE users` FROM `cat`.`sch`.`revenue_metrics` GROUP BY ALL", + ); + }); + + test("neutralizes a backtick in a dimension by doubling it", () => { + const { statement } = buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["region`"], + }); + expect(statement).toBe( + "SELECT MEASURE(`arr`) AS `arr`, `region``` FROM `cat`.`sch`.`revenue_metrics` GROUP BY ALL", + ); + }); + + test("throws for a dimension containing a control character", () => { + expect(() => + buildMetricSql(registration, { + measures: ["arr"], + dimensions: ["region\tbad"], + }), + ).toThrow(/not a valid identifier|control character/); + }); + }); + + // ── Envelope parity — streams warehouse_status* then a `result` message, + // byte-identical to the /query route's JSON SSE path. + describe("_handleMetricRoute SSE envelope", () => { + test("streams warehouse_status then a result message with aliased rows", async () => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + + const executeMock = vi.fn().mockResolvedValue({ + result: { data: [{ arr: 1234 }] }, + }); + (plugin as any).SQLClient.executeStatement = executeMock; + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + // The measures-only SQL reaches the warehouse. + expect(executeMock).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + statement: + "SELECT MEASURE(`arr`) AS `arr` FROM `cat`.`sch`.`revenue_metrics`", + warehouse_id: "test-warehouse-id", + }), + expect.any(AbortSignal), + ); + + // Same SSE envelope as /query: a `result` event carrying the rows. + expect(mockRes.write).toHaveBeenCalledWith("event: result\n"); + expect(mockRes.write).toHaveBeenCalledWith( + expect.stringContaining('"data":[{"arr":1234}]'), + ); + expect(mockRes.end).toHaveBeenCalled(); + }); + + test("emits warehouse_status before result for a STARTING warehouse", async () => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + + const executeMock = vi.fn().mockResolvedValue({ + result: { data: [{ arr: 1 }] }, + }); + (plugin as any).SQLClient.executeStatement = executeMock; + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + + const warehouseGet = vi + .fn() + .mockResolvedValueOnce({ state: "STARTING" }) + .mockResolvedValueOnce({ state: "RUNNING" }); + const mockReq = createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + }); + mockReq.serviceWorkspaceClient.warehouses.get = warehouseGet; + mockReq.userWorkspaceClient.warehouses.get = warehouseGet; + const mockRes = createMockResponse(); + + vi.useFakeTimers(); + const handlerPromise = handler(mockReq, mockRes); + await vi.runAllTimersAsync(); + await handlerPromise; + vi.useRealTimers(); + + const eventLines = (mockRes.write as any).mock.calls + .map((call: any[]) => call[0] as string) + .filter((s: string) => s.startsWith("event: ")); + const warehouseIdx = eventLines.findIndex( + (s: string) => s === "event: warehouse_status\n", + ); + const resultIdx = eventLines.findIndex( + (s: string) => s === "event: result\n", + ); + expect(warehouseIdx).toBeGreaterThanOrEqual(0); + expect(resultIdx).toBeGreaterThanOrEqual(0); + expect(warehouseIdx).toBeLessThan(resultIdx); + }); + + test("returns 400 when the body fails structural validation", async () => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: "revenue" }, + body: { measures: [] }, // empty → fails .min(1) + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(400); + }); + }); + + // ── 503-vs-404 latching + dormancy. + describe("registry latching and dormancy", () => { + test("unknown key against a valid registry → 404 (generic body)", async () => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: "nope" }, + body: { measures: ["arr"] }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(404); + expect(mockRes.json).toHaveBeenCalledWith({ error: "Metric not found" }); + }); + + test.each(["__proto__", "constructor", "toString", "hasOwnProperty"])( + "inherited Object.prototype key %j → 404, no execution (own-property lookup)", + async (dangerousKey) => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + // A real (own) entry so the registry is populated but does NOT contain + // the dangerous key. Without the own-property guard, `registry[key]` + // would resolve the inherited prototype member and slip past the 404. + + const executeMock = vi.fn(); + (plugin as any).SQLClient.executeStatement = executeMock; + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: dangerousKey }, + body: { measures: ["arr"] }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(404); + expect(mockRes.json).toHaveBeenCalledWith({ + error: "Metric not found", + }); + expect(executeMock).not.toHaveBeenCalled(); + }, + ); + + test("malformed registry → 503 METRIC_REGISTRY_LOAD_FAILED", async () => { + const dir = mkdtempSync(path.join(tmpdir(), "mv-route-")); + tempRegistryDirs.push(dir); + // A real malformed config on disk (invalid JSON) → the loader throws → + // the route surfaces a 503, exercising the actual load path. + writeRegistry(dir, "{ not valid json"); + const plugin = pluginForDir(config, dir); + const { router, getHandler } = createMockRouter(); + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(503); + expect(mockRes.json).toHaveBeenCalledWith({ + error: "Metric registry not available", + code: "METRIC_REGISTRY_LOAD_FAILED", + }); + }); + + test("self-heal: a fixed config is picked up on the next request (no restart)", async () => { + const dir = mkdtempSync(path.join(tmpdir(), "mv-route-")); + tempRegistryDirs.push(dir); + writeRegistry(dir, "{ not valid json"); + const plugin = pluginForDir(config, dir); + const { router, getHandler } = createMockRouter(); + const executeMock = vi + .fn() + .mockResolvedValue({ result: { data: [{ arr: 1 }] } }); + (plugin as any).SQLClient.executeStatement = executeMock; + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + + // First request: malformed → 503, and the failure is NOT cached. + const res1 = createMockResponse(); + await handler( + createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + }), + res1, + ); + expect(res1.status).toHaveBeenCalledWith(503); + + // Fix the file. Each request re-reads + re-parses the config, so the + // next request serves it — no server restart, no latched 503. + writeRegistry(dir, { + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + }); + const res2 = createMockResponse(); + await handler( + createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + }), + res2, + ); + expect(res2.status).not.toHaveBeenCalledWith(503); + expect(res2.status).not.toHaveBeenCalledWith(404); + expect(executeMock).toHaveBeenCalled(); + }); + + test("hot-reload: a new key added to a working config is visible next request", async () => { + const dir = mkdtempSync(path.join(tmpdir(), "mv-route-")); + tempRegistryDirs.push(dir); + writeRegistry(dir, { + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + }); + const plugin = pluginForDir(config, dir); + const { router, getHandler } = createMockRouter(); + const executeMock = vi + .fn() + .mockResolvedValue({ result: { data: [{ arr: 1 }] } }); + (plugin as any).SQLClient.executeStatement = executeMock; + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + + // `orders` is not registered yet → 404. + const res1 = createMockResponse(); + await handler( + createMockRequest({ + params: { key: "orders" }, + body: { measures: ["cnt"] }, + }), + res1, + ); + expect(res1.status).toHaveBeenCalledWith(404); + + // Add `orders` to the config. Each request re-reads the config, so the + // next request sees it without a restart. + writeRegistry(dir, { + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + orders: { key: "orders", source: "cat.sch.order_metrics", lane: "sp" }, + }); + const res2 = createMockResponse(); + await handler( + createMockRequest({ + params: { key: "orders" }, + body: { measures: ["cnt"] }, + }), + res2, + ); + expect(res2.status).not.toHaveBeenCalledWith(404); + expect(executeMock).toHaveBeenCalled(); + }); + + test("no metric-views.json present → registry empty, unknown key 404, nothing executes", async () => { + const plugin = new AnalyticsPlugin(config); + const { router, getHandler } = createMockRouter(); + + const executeMock = vi.fn(); + (plugin as any).SQLClient.executeStatement = executeMock; + // Registry lazily loads from cwd; no config/queries/metric-views.json in + // the test cwd → empty registry (dormant). + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(404); + expect(executeMock).not.toHaveBeenCalled(); + }); + }); +}); + +// ── loadMetricRegistry: config parse against the landed metricSourceSchema. +// The loader reads the config file THROUGH an `AppManager` (Phase 2), so each +// test points an `AppManager` at its temp dir instead of passing a bare +// directory string. The loader is stateless — it reads + parses on every call +// (no memoization), so there is no cache to reset between tests. +describe("loadMetricRegistry", () => { + let dir: string; + let app: AppManager; + + beforeEach(() => { + dir = mkdtempSync(path.join(tmpdir(), "mv-registry-")); + app = new AppManager(dir); + }); + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + test("absent metric-views.json → empty registry (dormancy)", async () => { + expect(await loadMetricRegistry(app)).toEqual({}); + }); + + test("derives lane from executor (default sp, user → obo)", async () => { + writeFileSync( + path.join(dir, "metric-views.json"), + JSON.stringify({ + metricViews: { + revenue: { source: "cat.sch.revenue_metrics" }, + customers: { source: "cat.sch.customer_metrics", executor: "user" }, + }, + }), + ); + + const registry = await loadMetricRegistry(app); + expect(registry.revenue).toEqual({ + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }); + expect(registry.customers).toEqual({ + key: "customers", + source: "cat.sch.customer_metrics", + lane: "obo", + }); + }); + + test("registry has a null prototype (no inherited-property lookups)", async () => { + writeFileSync( + path.join(dir, "metric-views.json"), + JSON.stringify({ + metricViews: { revenue: { source: "cat.sch.revenue_metrics" } }, + }), + ); + + const registry = await loadMetricRegistry(app); + expect(Object.getPrototypeOf(registry)).toBeNull(); + // A key that would resolve to a truthy inherited member on a plain object + // resolves to undefined here. + expect((registry as Record).toString).toBeUndefined(); + expect((registry as Record).__proto__).toBeUndefined(); + }); + + test("absent file yields a null-prototype (dormant) registry too", async () => { + // The ENOENT dormancy path must also be prototype-free — a metric key + // colliding with an inherited member can't 200 against an empty registry. + const registry = await loadMetricRegistry(app); + expect(Object.getPrototypeOf(registry)).toBeNull(); + }); + + test("malformed JSON throws", async () => { + writeFileSync(path.join(dir, "metric-views.json"), "{ not json"); + await expect(loadMetricRegistry(app)).rejects.toThrow(/Failed to parse/); + }); + + test("schema-invalid config throws", async () => { + writeFileSync( + path.join(dir, "metric-views.json"), + JSON.stringify({ + metricViews: { revenue: { source: "not-a-three-part-fqn" } }, + }), + ); + await expect(loadMetricRegistry(app)).rejects.toThrow( + /Invalid metric-views.json/, + ); + }); + + test("rejects more than 200 metric views (runtime parity with typegen cap)", async () => { + // A config that fails type generation (MAX_METRIC_VIEWS) must not silently + // pass at runtime. z.record has no `.max` in zod 4, so this is enforced by + // the schema's superRefine. + const metricViews: Record = {}; + for (let i = 0; i < 201; i++) { + metricViews[`m_${i}`] = { source: `cat.sch.view_${i}` }; + } + writeFileSync( + path.join(dir, "metric-views.json"), + JSON.stringify({ metricViews }), + ); + await expect(loadMetricRegistry(app)).rejects.toThrow( + /Invalid metric-views.json/, + ); + }); + + test("rejects an FQN segment longer than 255 chars (per-segment cap)", async () => { + // The per-segment length cap is not expressible as a whole-string + // maxLength; the schema's superRefine enforces it so runtime matches the + // typegen resolver. + const longSegment = "a".repeat(256); + writeFileSync( + path.join(dir, "metric-views.json"), + JSON.stringify({ + metricViews: { revenue: { source: `cat.sch.${longSegment}` } }, + }), + ); + await expect(loadMetricRegistry(app)).rejects.toThrow( + /Invalid metric-views.json/, + ); + }); + + test("accepts exactly 200 views and a 255-char segment (boundary)", async () => { + const metricViews: Record = {}; + for (let i = 0; i < 200; i++) { + metricViews[`m_${i}`] = { source: `cat.sch.view_${i}` }; + } + // One entry with a segment at exactly the 255 limit — must pass. + metricViews.m_0 = { source: `cat.sch.${"a".repeat(255)}` }; + writeFileSync( + path.join(dir, "metric-views.json"), + JSON.stringify({ metricViews }), + ); + await expect(loadMetricRegistry(app)).resolves.toBeDefined(); + }); +}); + +// ── Phase 2: the structured filter engine (translator + validator). +// Registry-free: names are grammar-gated, values are parameterized. No +// allowlist, no op⇄dimension-type check. +describe("metric — filter translator", () => { + const registration: MetricRegistration = { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }; + + // Render a filter via buildMetricSql and return the WHERE fragment + params. + function render(filter: unknown) { + const { statement, parameters } = buildMetricSql(registration, { + measures: ["arr"], + filter: filter as never, + }); + const match = statement.match(/ WHERE (.+?)( GROUP BY| LIMIT|$)/); + const where = match ? match[1] : null; + return { statement, where, parameters }; + } + + describe("operators (12 unit tests)", () => { + test("equals → ` = :f_0`", () => { + const { where, parameters } = render({ + member: "region", + operator: "equals", + values: ["EMEA"], + }); + expect(where).toBe("`region` = :f_0"); + expect(parameters).toEqual({ + f_0: { __sql_type: "STRING", value: "EMEA" }, + }); + }); + + test("notEquals → ` <> :f_0`", () => { + const { where, parameters } = render({ + member: "region", + operator: "notEquals", + values: ["EMEA"], + }); + expect(where).toBe("`region` <> :f_0"); + expect(parameters.f_0).toEqual({ __sql_type: "STRING", value: "EMEA" }); + }); + + test("in → ` IN (:f_0, :f_1, ...)`", () => { + const { where, parameters } = render({ + member: "region", + operator: "in", + values: ["EMEA", "APAC", "AMER"], + }); + expect(where).toBe("`region` IN (:f_0, :f_1, :f_2)"); + expect(parameters.f_0).toEqual({ __sql_type: "STRING", value: "EMEA" }); + expect(parameters.f_1).toEqual({ __sql_type: "STRING", value: "APAC" }); + expect(parameters.f_2).toEqual({ __sql_type: "STRING", value: "AMER" }); + }); + + test("notIn → ` NOT IN (:f_0, :f_1, ...)`", () => { + const { where, parameters } = render({ + member: "region", + operator: "notIn", + values: ["EMEA", "APAC"], + }); + expect(where).toBe("`region` NOT IN (:f_0, :f_1)"); + expect(Object.keys(parameters)).toHaveLength(2); + }); + + test("gt → ` > :f_0` (numeric value bound as INT)", () => { + const { where, parameters } = render({ + member: "deal_size", + operator: "gt", + values: [10000], + }); + expect(where).toBe("`deal_size` > :f_0"); + expect(parameters.f_0).toEqual({ __sql_type: "INT", value: "10000" }); + }); + + test("gte → ` >= :f_0`", () => { + const { where } = render({ + member: "deal_size", + operator: "gte", + values: [5000], + }); + expect(where).toBe("`deal_size` >= :f_0"); + }); + + test("lt → ` < :f_0`", () => { + const { where } = render({ + member: "deal_size", + operator: "lt", + values: [100], + }); + expect(where).toBe("`deal_size` < :f_0"); + }); + + test("lte → ` <= :f_0`", () => { + const { where } = render({ + member: "deal_size", + operator: "lte", + values: [50000], + }); + expect(where).toBe("`deal_size` <= :f_0"); + }); + + test("contains → ` LIKE :f_0` (value wrapped in %...%)", () => { + const { where, parameters } = render({ + member: "region", + operator: "contains", + values: ["MEA"], + }); + expect(where).toBe("`region` LIKE :f_0"); + expect(parameters.f_0).toEqual({ __sql_type: "STRING", value: "%MEA%" }); + }); + + test("notContains → ` NOT LIKE :f_0`", () => { + const { where, parameters } = render({ + member: "region", + operator: "notContains", + values: ["test"], + }); + expect(where).toBe("`region` NOT LIKE :f_0"); + expect(parameters.f_0).toEqual({ + __sql_type: "STRING", + value: "%test%", + }); + }); + + test("set → ` IS NOT NULL` (no bind)", () => { + const { where, parameters } = render({ + member: "region", + operator: "set", + }); + expect(where).toBe("`region` IS NOT NULL"); + expect(parameters).toEqual({}); + }); + + test("notSet → ` IS NULL` (no bind)", () => { + const { where, parameters } = render({ + member: "region", + operator: "notSet", + }); + expect(where).toBe("`region` IS NULL"); + expect(parameters).toEqual({}); + }); + }); + + describe("AND/OR composition", () => { + test("flat AND group renders predicates joined by AND", () => { + const { where, parameters } = render({ + and: [ + { member: "region", operator: "equals", values: ["EMEA"] }, + { member: "segment", operator: "equals", values: ["Enterprise"] }, + ], + }); + expect(where).toBe("(`region` = :f_0 AND `segment` = :f_1)"); + expect(parameters.f_0.value).toBe("EMEA"); + expect(parameters.f_1.value).toBe("Enterprise"); + }); + + test("flat OR group renders predicates joined by OR", () => { + const { where } = render({ + or: [ + { member: "region", operator: "equals", values: ["EMEA"] }, + { member: "region", operator: "equals", values: ["APAC"] }, + ], + }); + expect(where).toBe("(`region` = :f_0 OR `region` = :f_1)"); + }); + + test("AND-of-OR composes nested groups", () => { + const { where } = render({ + and: [ + { member: "region", operator: "in", values: ["EMEA", "APAC"] }, + { + or: [ + { member: "segment", operator: "equals", values: ["Enterprise"] }, + { member: "deal_size", operator: "gt", values: [50000] }, + ], + }, + ], + }); + expect(where).toContain("(`region` IN ("); + expect(where).toContain(" AND "); + expect(where).toContain("OR"); + }); + + test("OR-of-AND composes nested groups", () => { + const { where } = render({ + or: [ + { + and: [ + { member: "region", operator: "equals", values: ["EMEA"] }, + { member: "segment", operator: "equals", values: ["Enterprise"] }, + ], + }, + { member: "region", operator: "equals", values: ["APAC"] }, + ], + }); + expect(where).toMatch(/^\(.+ OR .+\)$/); + expect(where).toContain(" AND "); + }); + + test("empty `and: []` renders 1 = 1 (defense in depth past the validator)", () => { + // The validator rejects `and: []` (see cardinality tests), but if a + // bypass ever reaches the renderer, an empty conjunction must render the + // AND identity element (vacuous-true) so it is correct in any position — + // returning `null` (dropped) would only be right at the top level, not + // nested inside an OR (see the OR-of-empty-AND test below). No bind + // parameters are emitted for the tautology. + const { where, parameters } = render({ and: [] }); + expect(where).toBe("1 = 1"); + expect(parameters).toEqual({}); + }); + + test("empty `or: []` renders 1 = 0 (defense in depth past the validator)", () => { + // The validator rejects `or: []` (see cardinality tests), but if a bypass + // ever reaches the renderer, an empty disjunction must render vacuous- + // false — never drop the predicate (which would mean "match everything"). + const { where } = render({ or: [] }); + expect(where).toBe("1 = 0"); + }); + + test("empty `and` nested in `or` stays vacuous-true (does not under-return)", () => { + // `TRUE OR P` is all rows. If empty-AND returned `null` (dropped by the + // parent OR), this would collapse to just `P` and under-return. The + // identity element `1 = 1` keeps the disjunction matching everything. + // `sortFilterChildren` orders the predicate before the empty-`and` group, + // so the tautology lands on the right; the disjunction still matches all rows. + const { where } = render({ + or: [ + { and: [] }, + { member: "region", operator: "equals", values: ["EMEA"] }, + ], + }); + expect(where).toBe("(`region` = :f_0 OR 1 = 1)"); + }); + }); + + describe("depth cap", () => { + test("rejects 9 levels of AND nesting at preCheckFilterDepth (before Zod)", () => { + let node: unknown = { + member: "region", + operator: "equals", + values: ["EMEA"], + }; + for (let i = 0; i < 9; i += 1) { + node = { and: [node] }; + } + expect(() => + validateMetricRequest({ measures: ["arr"], filter: node }), + ).toThrowError(/fields:.*filter/); + }); + + test("accepts exactly 8 levels of AND nesting (validator)", () => { + let node: unknown = { + member: "region", + operator: "equals", + values: ["EMEA"], + }; + for (let i = 0; i < 8; i += 1) { + node = { and: [node] }; + } + expect(() => + validateMetricRequest({ measures: ["arr"], filter: node }), + ).not.toThrow(); + }); + + test("renderFilter independently rejects nesting past the depth cap", () => { + // Second, independent depth enforcement: even if a payload bypasses the + // pre-Zod pre-check and Zod's superRefine, the SQL renderer re-checks the + // depth as defense in depth. Build 9-deep and call buildMetricSql + // directly (skipping validateMetricRequest). + let node: unknown = { + member: "region", + operator: "equals", + values: ["EMEA"], + }; + for (let i = 0; i < 9; i += 1) { + node = { and: [node] }; + } + expect(() => + buildMetricSql(registration, { + measures: ["arr"], + filter: node as never, + }), + ).toThrow(/nesting exceeds the maximum depth/); + }); + + test("rejects pathologically deep filter without stack-overflow (pre-parse cap)", () => { + let node: unknown = { + member: "region", + operator: "equals", + values: ["EMEA"], + }; + for (let i = 0; i < 10_000; i += 1) { + node = { and: [node] }; + } + expect(() => + validateMetricRequest({ measures: ["arr"], filter: node }), + ).toThrowError(/fields:.*filter/); + }); + + test("rejects deep `or` even when paired with empty `and` (else-if bypass guard)", () => { + let deepOr: unknown = { + member: "region", + operator: "equals", + values: ["EMEA"], + }; + for (let i = 0; i < 10_000; i += 1) { + deepOr = { or: [deepOr] }; + } + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { and: [], or: [deepOr] } as never, + }), + ).toThrowError(/fields:.*filter/); + }); + + test("rejects breadth-DoS: a single group with too many children", () => { + const wide = Array.from({ length: 1000 }, () => ({ + member: "region", + operator: "equals" as const, + values: ["EMEA"], + })); + expect(() => + validateMetricRequest({ measures: ["arr"], filter: { and: wide } }), + ).toThrowError(/fields:.*filter/); + }); + }); + + describe("parameterization safety (no values in rendered SQL)", () => { + test("string values do not appear verbatim in the SQL string", () => { + const sneaky = "EMEA' OR '1'='1"; + const { statement } = render({ + member: "region", + operator: "equals", + values: [sneaky], + }); + expect(statement).not.toContain(sneaky); + expect(statement).toContain(":f_0"); + }); + + test("numeric values do not appear verbatim in the SQL string", () => { + const { statement } = render({ + member: "deal_size", + operator: "gt", + values: [987654321], + }); + expect(statement).not.toContain("987654321"); + expect(statement).toContain(":f_0"); + }); + + test("LIKE wildcard is the only value transformation; the original string is not in SQL", () => { + const { statement } = render({ + member: "region", + operator: "contains", + values: ["dangerous%"], + }); + expect(statement).not.toContain("dangerous"); + expect(statement).toContain(":f_0"); + }); + + test("IN values are individually bound (not concatenated)", () => { + const { statement, parameters } = render({ + member: "region", + operator: "in", + values: ["A", "B", "C"], + }); + expect(statement).not.toMatch(/region IN \([^:]/); + expect(Object.keys(parameters)).toHaveLength(3); + }); + + test("an injection-shaped filter member is neutralized by quoting", () => { + // No control char → a valid (if nonexistent) column name: quoted whole, + // the `;`/`DROP`/`--` are inert inside the backtick-delimited identifier. + const { where } = render({ + member: "region; DROP TABLE foo --", + operator: "equals", + values: ["x"], + }); + expect(where).toBe("`region; DROP TABLE foo --` = :f_0"); + }); + + test("a filter member with a control character throws before SQL is built", () => { + expect(() => + render({ + member: "region\ndrop", + operator: "equals", + values: ["x"], + }), + ).toThrowError(/not a valid identifier|control character/); + }); + }); + + describe("validator — operator + cardinality (registry-free)", () => { + test("rejects an unknown operator", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { + member: "region", + operator: "startsWith" as never, + values: ["E"], + }, + }), + ).toThrowError(/fields:.*filter\.operator/); + }); + + test("rejects equals with zero values (cardinality)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { member: "region", operator: "equals", values: [] }, + }), + ).toThrowError(/fields:.*filter\.values/); + }); + + test("rejects equals with multiple values (cardinality)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { member: "region", operator: "equals", values: ["A", "B"] }, + }), + ).toThrowError(/fields:.*filter\.values/); + }); + + test("rejects in with empty values (cardinality)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { member: "region", operator: "in", values: [] }, + }), + ).toThrowError(/fields:.*filter\.values/); + }); + + test("rejects set with values (cardinality — must be absent)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { member: "region", operator: "set", values: ["EMEA"] }, + }), + ).toThrowError(/fields:.*filter\.values/); + }); + + test("accepts set with no values", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { member: "region", operator: "set" }, + }), + ).not.toThrow(); + }); + + test("accepts notSet with an empty values array", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { member: "region", operator: "notSet", values: [] }, + }), + ).not.toThrow(); + }); + + test("rejects `contains` with a non-string value", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { + member: "region", + operator: "contains", + values: [42 as unknown as string], + }, + }), + ).toThrowError(/fields:.*filter\.values/); + }); + + test("rejects a filter predicate with too many values (DoS guard)", () => { + const big = Array.from({ length: 2000 }, (_, i) => `v${i}`); + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { member: "region", operator: "in", values: big }, + }), + ).toThrowError(/fields:.*filter\.values/); + }); + + test("rejects a bare-array filter (not a Predicate or { and }/{ or } group)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: [ + { member: "region", operator: "in", values: ["EMEA"] }, + ] as never, + }), + ).toThrow(); + }); + + test("rejects empty `or` group (empty disjunction is vacuously false)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { or: [] }, + }), + ).toThrowError(/fields:.*filter\.or/); + }); + + test("rejects empty `and` group (renders to no WHERE, would split the cache)", () => { + // An empty `and` contributes no constraint and renders to no WHERE + // clause — identical SQL to omitting `filter` — but canonicalizes to a + // distinct cache key (`and()` vs `_`). Reject it so request shape and + // cache key stay one-to-one. + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { and: [] }, + }), + ).toThrowError(/fields:.*filter\.and/); + }); + + test("rejects an unknown operator at depth (nested filter)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { + and: [ + { member: "region", operator: "equals", values: ["EMEA"] }, + { + member: "segment", + operator: "matches" as never, + values: ["X"], + }, + ], + }, + }), + ).toThrowError(/fields:.*filter\.and\.1\.operator/); + }); + + test("well-formed-but-unknown member is NOT rejected locally (no allowlist)", () => { + // The dropped-allowlist posture: a grammar-valid member that isn't a + // real dimension passes validation AND SQL construction — it reaches the + // warehouse, which is the authority on whether the column exists. + expect(() => + validateMetricRequest({ + measures: ["arr"], + filter: { member: "ghost_column", operator: "equals", values: ["x"] }, + }), + ).not.toThrow(); + const { where } = render({ + member: "ghost_column", + operator: "equals", + values: ["x"], + }); + expect(where).toBe("`ghost_column` = :f_0"); + }); + }); + + describe("validator — measure/dimension uniqueness", () => { + // Measures and dimensions become SELECT columns aliased to their own name; + // a repeated name collapses to a single row-object key and silently drops + // a value during row materialization, so uniqueness is enforced up front. + test("rejects a duplicate measure", () => { + expect(() => + validateMetricRequest({ measures: ["arr", "arr"] }), + ).toThrowError(/fields:.*measures/); + }); + + test("rejects a duplicate dimension", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + dimensions: ["region", "region"], + }), + ).toThrowError(/fields:.*measures/); + }); + + test("rejects a name that is BOTH a measure and a dimension", () => { + // The corruption case: `SELECT MEASURE(`x`) AS `x`, x ... GROUP BY ALL` + // materializes two `x` columns and the second overwrites the first. + expect(() => + validateMetricRequest({ + measures: ["revenue"], + dimensions: ["revenue"], + }), + ).toThrowError(/fields:.*measures/); + }); + + test("accepts distinct measures and dimensions (no false positive)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr", "mrr"], + dimensions: ["region", "segment"], + }), + ).not.toThrow(); + }); + }); + + describe("validator — format (JSON-only at v1)", () => { + test("accepts JSON_ARRAY", () => { + expect(() => + validateMetricRequest({ measures: ["arr"], format: "JSON_ARRAY" }), + ).not.toThrow(); + }); + + test("accepts the legacy JSON alias (normalizes to JSON_ARRAY)", () => { + expect(() => + validateMetricRequest({ measures: ["arr"], format: "JSON" }), + ).not.toThrow(); + }); + + test("rejects ARROW_STREAM (not implemented on the metric route)", () => { + // Fail loud rather than silently deliver JSON for an Arrow request. + expect(() => + validateMetricRequest({ measures: ["arr"], format: "ARROW_STREAM" }), + ).toThrowError(/fields:.*format/); + }); + + test("rejects the legacy ARROW alias (normalizes to ARROW_STREAM)", () => { + expect(() => + validateMetricRequest({ measures: ["arr"], format: "ARROW" }), + ).toThrowError(/fields:.*format/); + }); + }); + + describe("timeGrain + timeDimension (validator)", () => { + test("a grammar-valid timeGrain + timeDimension (in dimensions) is accepted", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + dimensions: ["order_date"], + timeGrain: "month", + timeDimension: "order_date", + }), + ).not.toThrow(); + }); + + test("a timeGrain failing TIME_GRAIN_PATTERN is rejected", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + dimensions: ["order_date"], + timeGrain: "MONTH; DROP TABLE t", + timeDimension: "order_date", + }), + ).toThrowError(/fields:.*timeGrain/); + }); + + test("a capitalized grain (Month) is rejected by the grammar gate", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + dimensions: ["order_date"], + timeGrain: "Month", + timeDimension: "order_date", + }), + ).toThrowError(/fields:.*timeGrain/); + }); + + test("a timeDimension failing DIMENSION_NAME_PATTERN is rejected", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + dimensions: ["order_date"], + timeGrain: "month", + timeDimension: "order_date; DROP TABLE t", + }), + ).toThrowError(/fields:.*timeDimension/); + }); + + test("timeGrain without timeDimension → 400 (grain requires a target)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + dimensions: ["order_date"], + timeGrain: "day", + }), + ).toThrowError(/fields:.*timeDimension/); + }); + + test("timeDimension not in dimensions → 400 (must be selectable + in GROUP BY ALL)", () => { + expect(() => + validateMetricRequest({ + measures: ["arr"], + dimensions: ["region"], + timeGrain: "month", + timeDimension: "order_date", + }), + ).toThrowError(/fields:.*timeDimension/); + }); + + test("timeDimension not in dimensions → 400 even without timeGrain", () => { + // The dimensions-membership rule holds independent of timeGrain: a + // timeDimension that isn't selected can never be bucketed or grouped. + expect(() => + validateMetricRequest({ + measures: ["arr"], + dimensions: ["region"], + timeDimension: "order_date", + }), + ).toThrowError(/fields:.*timeDimension/); + }); + }); + + describe("sort-before-hash (predicate ordering inside groups)", () => { + test("predicate order does not affect the rendered SQL within an AND group", () => { + const a = render({ + and: [ + { member: "region", operator: "equals", values: ["EMEA"] }, + { member: "segment", operator: "equals", values: ["Ent"] }, + ], + }); + const b = render({ + and: [ + { member: "segment", operator: "equals", values: ["Ent"] }, + { member: "region", operator: "equals", values: ["EMEA"] }, + ], + }); + expect(a.where).toBe(b.where); + }); + }); + + // The warehouse-authoritative unknown-name parity test (sanitized + // clientMessage/errorCode envelope) lands here because the Phase 1 harness + // can drive the metric route end-to-end and assert on the SSE error bytes. + describe("warehouse-authoritative unknown-name parity", () => { + let config: IAnalyticsConfig; + let serviceContextMock: Awaited>; + + beforeEach(async () => { + config = { timeout: 5000 }; + setupDatabricksEnv(); + mockCacheStore.clear(); + ServiceContext.reset(); + serviceContextMock = await mockServiceContext(); + }); + + afterEach(() => { + serviceContextMock?.restore(); + }); + + test("a well-formed-but-unknown measure reaches the warehouse and surfaces a sanitized error envelope", async () => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + + // The warehouse rejects the unknown column. The raw text carries the + // offending name; the connector wraps it in an ExecutionError whose + // `.message` is server-log-only and whose `clientMessage` is sanitized. + const { ExecutionError } = await import("../../../errors/execution"); + const rawWarehouseText = + "[UNRESOLVED_COLUMN] A column with name `ghost_measure` cannot be resolved"; + const executeMock = vi + .fn() + .mockRejectedValue( + ExecutionError.statementFailed(rawWarehouseText, "UNRESOLVED_COLUMN"), + ); + (plugin as any).SQLClient.executeStatement = executeMock; + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: "revenue" }, + // grammar-valid, but not a real measure — NOT rejected locally. + body: { measures: ["ghost_measure"] }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + // The well-formed-unknown measure was NOT rejected locally — it reached + // the warehouse (parity with the raw `.sql` flow). + expect(executeMock).toHaveBeenCalled(); + expect(executeMock.mock.calls[0][1]).toEqual( + expect.objectContaining({ + statement: + "SELECT MEASURE(`ghost_measure`) AS `ghost_measure` FROM `cat`.`sch`.`revenue_metrics`", + }), + ); + + // The SSE error envelope carries a sanitized clientMessage + structured + // errorCode — the raw warehouse text (with the offending column name) + // never reaches the client payload (#329 scrubbing, inherited). + const errorWrites = (mockRes.write as any).mock.calls + .map((call: any[]) => call[0] as string) + .filter((s: string) => s.startsWith("data: ")); + const errorPayload = errorWrites.find((s: string) => + s.includes('"errorCode"'), + ); + expect(errorPayload).toBeTruthy(); + expect(errorPayload).toContain('"errorCode":"UNRESOLVED_COLUMN"'); + expect(errorPayload).toContain('"error":"Query execution failed"'); + expect(errorPayload).not.toContain("ghost_measure"); + expect(errorPayload).not.toContain("UNRESOLVED_COLUMN]"); + }); + }); +}); + +// ── Phase 3: cache-key composition. `composeMetricCacheKey` produces the +// array `CacheManager.generateKey` concatenates + sha256s; the invariants +// below are what make the cache both correct (semantically equal calls collapse) +// and safe (distinct args / executors never collide). +describe("composeMetricCacheKey", () => { + const base: { + metricKey: string; + source: string; + measures: string[]; + format: string; + executorKey: string; + } = { + metricKey: "revenue", + source: "cat.sch.revenue_metrics", + measures: ["arr"], + format: "JSON_ARRAY", + executorKey: "sp", + }; + + test("same key but different source → different keys (config repoint is not stale-served)", () => { + const a = composeMetricCacheKey({ ...base, source: "cat.sch.old_view" }); + const b = composeMetricCacheKey({ ...base, source: "cat.sch.new_view" }); + expect(a).not.toEqual(b); + }); + + test("timeDimension does NOT fork the key when timeGrain is absent (no SQL effect)", () => { + // Without a grain, renderDimensionClause emits the bare column, so + // timeDimension has no effect on the SQL — two such calls must cache-hit. + const withTd = composeMetricCacheKey({ + ...base, + dimensions: ["order_date"], + timeDimension: "order_date", + }); + const withoutTd = composeMetricCacheKey({ + ...base, + dimensions: ["order_date"], + }); + expect(withTd).toEqual(withoutTd); + }); + + test("timeDimension DOES fork the key when timeGrain is set (changes the SQL)", () => { + const a = composeMetricCacheKey({ + ...base, + dimensions: ["order_date", "region"], + timeGrain: "month", + timeDimension: "order_date", + }); + const b = composeMetricCacheKey({ + ...base, + dimensions: ["order_date", "region"], + timeGrain: "month", + timeDimension: "region", + }); + expect(a).not.toEqual(b); + }); + + test("measure ORDER does not affect the key (sorted before hashing)", () => { + const a = composeMetricCacheKey({ + ...base, + measures: ["arr", "revenue"], + }); + const b = composeMetricCacheKey({ + ...base, + measures: ["revenue", "arr"], + }); + expect(a).toEqual(b); + }); + + test("dimension ORDER does not affect the key (sorted before hashing)", () => { + const a = composeMetricCacheKey({ + ...base, + dimensions: ["region", "segment"], + }); + const b = composeMetricCacheKey({ + ...base, + dimensions: ["segment", "region"], + }); + expect(a).toEqual(b); + }); + + test("different measures → different keys", () => { + const a = composeMetricCacheKey({ ...base, measures: ["arr"] }); + const b = composeMetricCacheKey({ ...base, measures: ["mrr"] }); + expect(a).not.toEqual(b); + }); + + test("different dimensions → different keys", () => { + const a = composeMetricCacheKey({ ...base, dimensions: ["region"] }); + const b = composeMetricCacheKey({ ...base, dimensions: ["segment"] }); + expect(a).not.toEqual(b); + }); + + // A comma is a legal identifier character (`isValidColumnName` rejects only + // control chars / newlines), so a raw `.join(",")` would collapse a single + // comma-containing name and a two-name list to the same key element while the + // generated SQL differs — serving wrong cached rows. JSON encoding keeps the + // key one-to-one with the SQL. + test("measures with a comma in a name do NOT collide with a two-measure list", () => { + const a = composeMetricCacheKey({ ...base, measures: ["a,b"] }); + const b = composeMetricCacheKey({ ...base, measures: ["a", "b"] }); + expect(a).not.toEqual(b); + }); + + test("dimensions with a comma in a name do NOT collide with a two-dimension list", () => { + const a = composeMetricCacheKey({ ...base, dimensions: ["a,b"] }); + const b = composeMetricCacheKey({ ...base, dimensions: ["a", "b"] }); + expect(a).not.toEqual(b); + }); + + test("different timeGrain → different keys", () => { + const a = composeMetricCacheKey({ + ...base, + dimensions: ["order_date"], + timeGrain: "day", + timeDimension: "order_date", + }); + const b = composeMetricCacheKey({ + ...base, + dimensions: ["order_date"], + timeGrain: "month", + timeDimension: "order_date", + }); + expect(a).not.toEqual(b); + }); + + test("different timeDimension → different keys (new field salts the key)", () => { + const a = composeMetricCacheKey({ + ...base, + dimensions: ["order_date", "ship_date"], + timeGrain: "day", + timeDimension: "order_date", + }); + const b = composeMetricCacheKey({ + ...base, + dimensions: ["order_date", "ship_date"], + timeGrain: "day", + timeDimension: "ship_date", + }); + expect(a).not.toEqual(b); + }); + + test("different limit → different keys", () => { + const a = composeMetricCacheKey({ ...base, limit: 10 }); + const b = composeMetricCacheKey({ ...base, limit: 20 }); + expect(a).not.toEqual(b); + }); + + test("predicate ORDER inside a group does not affect the key (canonicalizeFilter)", () => { + const a = composeMetricCacheKey({ + ...base, + filter: { + and: [ + { member: "region", operator: "equals", values: ["EMEA"] }, + { member: "segment", operator: "equals", values: ["Ent"] }, + ], + } as MetricFilter, + }); + const b = composeMetricCacheKey({ + ...base, + filter: { + and: [ + { member: "segment", operator: "equals", values: ["Ent"] }, + { member: "region", operator: "equals", values: ["EMEA"] }, + ], + } as MetricFilter, + }); + expect(a).toEqual(b); + }); + + test("predicate ORDER is stable for two ops on the SAME member (sort-key delimiter)", () => { + // Same member, different operators — the sort key is `${member}/${operator}`, + // so the "/" delimiter (not a raw concatenation) is what disambiguates the + // pair and keeps the ordering — and thus the key — independent of input + // order. Two range bounds on one column is the everyday shape of this. + const a = composeMetricCacheKey({ + ...base, + filter: { + and: [ + { member: "revenue", operator: "gt", values: [100] }, + { member: "revenue", operator: "lt", values: [200] }, + ], + } as MetricFilter, + }); + const b = composeMetricCacheKey({ + ...base, + filter: { + and: [ + { member: "revenue", operator: "lt", values: [200] }, + { member: "revenue", operator: "gt", values: [100] }, + ], + } as MetricFilter, + }); + expect(a).toEqual(b); + }); + + test("distinct filter VALUES → distinct keys", () => { + const a = composeMetricCacheKey({ + ...base, + filter: { + member: "region", + operator: "equals", + values: ["EMEA"], + } as MetricFilter, + }); + const b = composeMetricCacheKey({ + ...base, + filter: { + member: "region", + operator: "equals", + values: ["APAC"], + } as MetricFilter, + }); + expect(a).not.toEqual(b); + }); + + test("a filter present vs absent → different keys", () => { + const withFilter = composeMetricCacheKey({ + ...base, + filter: { + member: "region", + operator: "set", + } as MetricFilter, + }); + const withoutFilter = composeMetricCacheKey({ ...base }); + expect(withFilter).not.toEqual(withoutFilter); + }); + + test("SP vs OBO (same args) → different keys via executorKey", () => { + const sp = composeMetricCacheKey({ ...base, executorKey: "sp" }); + const obo = composeMetricCacheKey({ + ...base, + executorKey: deriveMetricExecutorKey({ + lane: "obo", + userIdentity: "alice", + }), + }); + expect(sp).not.toEqual(obo); + }); +}); + +// ── Phase 3: executor-key isolation. The key is what scopes the cache — `"sp"` +// shares it across all callers, a per-user hash isolates OBO callers. The raw +// identity must never enter the key verbatim (privacy: cache keys are logged +// and persisted). +describe("deriveMetricExecutorKey", () => { + test("SP lane → literal 'sp' (shared cache)", () => { + expect(deriveMetricExecutorKey({ lane: "sp" })).toBe("sp"); + }); + + test("SP lane ignores any supplied identity", () => { + expect(deriveMetricExecutorKey({ lane: "sp", userIdentity: "alice" })).toBe( + "sp", + ); + }); + + test("OBO lane hashes the identity — raw identity never appears in the key", () => { + const key = deriveMetricExecutorKey({ + lane: "obo", + userIdentity: "alice@example.com", + }); + expect(key).toMatch(/^[a-f0-9]{64}$/); + expect(key).not.toContain("alice"); + }); + + test("OBO same identity → stable key (cache hits work)", () => { + const a = deriveMetricExecutorKey({ lane: "obo", userIdentity: "alice" }); + const b = deriveMetricExecutorKey({ lane: "obo", userIdentity: "alice" }); + expect(a).toBe(b); + }); + + test("OBO identity is trimmed before hashing (whitespace does not fork the scope)", () => { + const padded = deriveMetricExecutorKey({ + lane: "obo", + userIdentity: " alice ", + }); + const bare = deriveMetricExecutorKey({ + lane: "obo", + userIdentity: "alice", + }); + expect(padded).toBe(bare); + }); + + test("OBO different identities → different keys (isolation holds)", () => { + const alice = deriveMetricExecutorKey({ + lane: "obo", + userIdentity: "alice", + }); + const bob = deriveMetricExecutorKey({ lane: "obo", userIdentity: "bob" }); + expect(alice).not.toBe(bob); + }); + + test("OBO empty identity → throws AuthenticationError (no shared 'anonymous' scope)", () => { + expect(() => + deriveMetricExecutorKey({ lane: "obo", userIdentity: "" }), + ).toThrow(AuthenticationError); + }); + + test("OBO whitespace-only identity → throws AuthenticationError", () => { + expect(() => + deriveMetricExecutorKey({ lane: "obo", userIdentity: " " }), + ).toThrow(AuthenticationError); + }); + + test("OBO missing (undefined/null) identity → throws AuthenticationError", () => { + expect(() => deriveMetricExecutorKey({ lane: "obo" })).toThrow( + AuthenticationError, + ); + expect(() => + deriveMetricExecutorKey({ lane: "obo", userIdentity: null }), + ).toThrow(AuthenticationError); + }); +}); + +// ── Phase 3: lane dispatch at the handler level. The lane comes from the +// registration (the entry's `executor` in metric-views.json), NOT the URL: +// OBO-lane routes through `asUser(req)`, SP-lane through the default executor. +// A missing/whitespace OBO identity must land on the canonical 401 envelope, +// never an out-of-envelope 500. +describe("metric route — lane dispatch (Phase 3)", () => { + let config: IAnalyticsConfig; + let serviceContextMock: Awaited>; + + beforeEach(async () => { + config = { timeout: 5000 }; + setupDatabricksEnv(); + mockCacheStore.clear(); + ServiceContext.reset(); + serviceContextMock = await mockServiceContext(); + }); + + afterEach(() => { + serviceContextMock?.restore(); + }); + + test("OBO-lane registration routes through asUser(req)", async () => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "obo", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + + const asUserSpy = vi.spyOn(plugin, "asUser"); + const executeMock = vi + .fn() + .mockResolvedValue({ result: { data: [{ arr: 1 }] } }); + (plugin as any).SQLClient.executeStatement = executeMock; + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + headers: { + "x-forwarded-access-token": "user-token", + "x-forwarded-user": "alice", + }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + // The OBO lane dispatched through the user-scoped executor… + expect(asUserSpy).toHaveBeenCalledWith(mockReq); + // …and the SQL still reached the warehouse and streamed a result. + expect(executeMock).toHaveBeenCalled(); + expect(mockRes.write).toHaveBeenCalledWith("event: result\n"); + }); + + test("SP-lane registration uses the default executor (asUser not called)", async () => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "sp", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + + const asUserSpy = vi.spyOn(plugin, "asUser"); + const executeMock = vi + .fn() + .mockResolvedValue({ result: { data: [{ arr: 1 }] } }); + (plugin as any).SQLClient.executeStatement = executeMock; + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + // Even with user headers present, an SP-lane metric must NOT impersonate. + headers: { + "x-forwarded-access-token": "user-token", + "x-forwarded-user": "alice", + }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + expect(asUserSpy).not.toHaveBeenCalled(); + expect(executeMock).toHaveBeenCalled(); + expect(mockRes.write).toHaveBeenCalledWith("event: result\n"); + }); + + test("OBO metric with no user identity → canonical 401, no SQL executed", async () => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "obo", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + + const executeMock = vi.fn(); + (plugin as any).SQLClient.executeStatement = executeMock; + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + // No x-forwarded-* headers at all → the OBO dispatch throws an + // AuthenticationError, caught and returned as a canonical 401 envelope. + const mockReq = createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(401); + expect(mockRes.json).toHaveBeenCalledWith( + expect.objectContaining({ code: "AUTHENTICATION_ERROR" }), + ); + // Landed on the 401 path before any SQL and without opening the SSE stream. + expect(executeMock).not.toHaveBeenCalled(); + expect(mockRes.write).not.toHaveBeenCalled(); + }); + + test("OBO metric with whitespace-only user identity → canonical 401", async () => { + const plugin = pluginForDir( + config, + registryDir({ + revenue: { + key: "revenue", + source: "cat.sch.revenue_metrics", + lane: "obo", + }, + }), + ); + const { router, getHandler } = createMockRouter(); + + const executeMock = vi.fn(); + (plugin as any).SQLClient.executeStatement = executeMock; + + plugin.injectRoutes(router); + const handler = getHandler("POST", "/metric/:key"); + const mockReq = createMockRequest({ + params: { key: "revenue" }, + body: { measures: ["arr"] }, + headers: { + "x-forwarded-access-token": "user-token", + "x-forwarded-user": " ", + }, + }); + const mockRes = createMockResponse(); + + await handler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(401); + expect(mockRes.json).toHaveBeenCalledWith( + expect.objectContaining({ code: "AUTHENTICATION_ERROR" }), + ); + expect(executeMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/appkit/src/plugins/analytics/types.ts b/packages/appkit/src/plugins/analytics/types.ts index 95c987d14..e91a361f8 100644 --- a/packages/appkit/src/plugins/analytics/types.ts +++ b/packages/appkit/src/plugins/analytics/types.ts @@ -112,3 +112,99 @@ export interface AnalyticsQueryResponse { row_count: number; data: any[]; } + +// ──────────────────────────────────────────────────────────────────────────── +// Metric views — POST /api/analytics/metric/:key +// ──────────────────────────────────────────────────────────────────────────── + +/** + * Execution lane for a registered metric view, derived from the entry's + * `executor` in `metric-views.json`: + * - `"sp"` ← `executor: "app_service_principal"` — queried as the app + * service principal (cache shared across all users). + * - `"obo"` ← `executor: "user"` — queried on-behalf-of the requesting + * user (per-user cache). OBO dispatch is wired in a later phase. + */ +export type MetricLane = "sp" | "obo"; + +/** + * A single registered metric view, loaded from `config/queries/metric-views.json`. + * + * The registration carries only what the runtime needs to build and dispatch + * SQL: the metric `key`, the three-part UC FQN `source`, and the `lane`. There + * is intentionally NO build-time measure/dimension metadata here — the security + * boundary is the grammar gate plus parameterized values, not a name allowlist, + * so the runtime never enumerates known measures/dimensions. + */ +export interface MetricRegistration { + key: string; + source: string; + lane: MetricLane; +} + +/** + * v1 filter operator vocabulary — exactly twelve names. The runtime tuple + * `METRIC_FILTER_OPERATORS` (next to the validator in `metric.ts`) is the + * server-side source of truth; this union mirrors it statically. + */ +export type MetricFilterOperatorName = + | "equals" + | "notEquals" + | "in" + | "notIn" + | "gt" + | "gte" + | "lt" + | "lte" + | "contains" + | "notContains" + | "set" + | "notSet"; + +/** + * A single filter predicate — the leaf node of the recursive + * {@link MetricFilter} tree. `member` is a dimension name (grammar-gated, not + * allowlisted); `values` is bound through parameterized `:f_` bind vars + * and never interpolated into the SQL string. + */ +export interface MetricPredicate { + member: string; + operator: MetricFilterOperatorName; + values?: ReadonlyArray; +} + +/** + * Recursive filter expression for the metric-view request body: a leaf + * {@link MetricPredicate} or an `{ and: [...] }` / `{ or: [...] }` group. The + * shape is intentionally non-generic server-side — per-metric narrowing (if + * any) lives client-side. + */ +export type MetricFilter = + | MetricPredicate + | { and: ReadonlyArray } + | { or: ReadonlyArray }; + +/** + * Validated request body for `POST /api/analytics/metric/:key`. + * + * `measures` is required. `dimensions` drive `GROUP BY ALL`; `filter` is the + * recursive structured predicate tree translated into a parameterized `WHERE` + * clause. `timeGrain` buckets the single dimension named by `timeDimension` + * via `date_trunc`; it requires `timeDimension`, and `timeDimension` must be + * one of `dimensions` so it is selected and in `GROUP BY ALL`. Both tokens are + * grammar-gated before they reach SQL. + */ +export interface IAnalyticsMetricRequest { + measures: string[]; + dimensions?: string[]; + filter?: MetricFilter; + timeGrain?: string; + /** + * The single dimension that `timeGrain` buckets via `date_trunc`. Must be + * one of `dimensions` (so it is selected and in `GROUP BY ALL`) and is + * required whenever `timeGrain` is set. Grammar-gated as a SQL identifier. + */ + timeDimension?: string; + limit?: number; + format?: AnalyticsFormat; +} diff --git a/packages/appkit/src/type-generator/mv-registry/config.ts b/packages/appkit/src/type-generator/mv-registry/config.ts index 1995f0723..6d4926470 100644 --- a/packages/appkit/src/type-generator/mv-registry/config.ts +++ b/packages/appkit/src/type-generator/mv-registry/config.ts @@ -94,26 +94,9 @@ function isValidMetricKey(key: string): boolean { return /^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key); } -/** - * Total predicate: is `fqn` a well-formed three-part UC metric view FQN? - * - * Well-formed = exactly three non-empty, dot-separated segments, each a valid - * Unity Catalog object name per the shared {@link UC_FQN_PATTERN} (the single - * source of truth, also used by the canonical Zod schema). Used as the - * defense-in-depth re-check at the describe fetcher seam; {@link resolveMetricConfig} - * runs the same checks but with specific, staged error messages. - * - * @note Segment length ({@link MAX_FQN_SEGMENT_LENGTH}) is NOT checked here — - * an over-long but otherwise legal name is still "valid shape". The length cap - * is a separate concern enforced (with its own message) in resolveMetricConfig. - */ -export function isValidFqn(fqn: string): boolean { - const segments = fqn.split("."); - if (segments.length !== FQN_SEGMENT_COUNT) { - return false; - } - return segments.every((segment) => UC_FQN_PATTERN.test(segment)); -} +// `resolveMetricConfig` re-derives the three-part UC FQN checks inline (rather +// than calling a shared predicate) so it can emit specific, staged error +// messages: arity, per-segment charset, per-segment length. /** * Field allowlists enforced by {@link resolveMetricConfig}. diff --git a/packages/appkit/src/type-generator/mv-registry/describe.ts b/packages/appkit/src/type-generator/mv-registry/describe.ts index db3231d67..a77caf75b 100644 --- a/packages/appkit/src/type-generator/mv-registry/describe.ts +++ b/packages/appkit/src/type-generator/mv-registry/describe.ts @@ -1,7 +1,13 @@ import type { WorkspaceClient } from "@databricks/sdk-experimental"; +// Grammar + SQL-quoting for metric-view FQNs live together in the shared, +// zod-free leaf so the type-generator and the analytics runtime validate and +// escape against one source of truth (see the module doc in metric-fqn.ts). +import { + isValidFqn, + quoteFqnForSql, +} from "../../../../shared/src/schemas/metric-fqn"; import { type DescribeFormatMemo, describeAdaptive } from "../statement-result"; import type { DatabricksStatementExecutionResponse } from "../types"; -import { isValidFqn } from "./config"; import type { DescribeFetcher, MetricColumnMetadata } from "./types"; /** @@ -284,49 +290,6 @@ function inferTimeGrains(type: string): string[] | undefined { return undefined; } -/** - * Quote a dot-separated FQN for safe interpolation into a Spark/Databricks SQL - * statement. - * - * Each dot-split segment is wrapped in backtick-quoted-identifier syntax. The - * one character that can break out of a backtick-quoted identifier is the - * backtick itself, escaped by doubling (`` ` `` → `` `` ``) — so every backtick - * inside a segment is doubled before the segment is wrapped. Control characters - * and newlines have no valid escape inside a quoted identifier, so a segment - * containing one is rejected outright. - * - * This is a pure, standalone escaper: it is intentionally independent of FQN - * naming validation ({@link isValidFqn}). Naming validation decides whether an - * FQN is an acceptable metric source; this function only guarantees that - * whatever it is handed cannot break out of the quoted identifier it produces. - * - * An ordinary identifier is unchanged apart from the wrapping backticks: - * `catalog.schema.view` → `` `catalog`.`schema`.`view` ``. - * - * @param fqn - Dot-separated identifier (e.g. `catalog.schema.view`). - * @returns The backtick-quoted, escaped identifier ready for interpolation. - * @throws If any segment contains a control character or newline. - */ -export function quoteFqnForSql(fqn: string): string { - // Reject anything that cannot be represented inside a backtick-quoted - // identifier. \p{Cc} is the Unicode "control" category, which covers C0 - // (incl. \n, \r, \t), DEL, and C1 — i.e. every control character/newline. - const CONTROL_OR_NEWLINE = /\p{Cc}/u; - return fqn - .split(".") - .map((segment) => { - if (CONTROL_OR_NEWLINE.test(segment)) { - throw new Error( - `Cannot quote FQN segment "${segment}" for SQL: it contains a control character or newline, which has no valid escape inside a backtick-quoted identifier.`, - ); - } - // Double every backtick — the only break-out from a backtick-quoted - // identifier — then wrap the whole segment in backticks. - return `\`${segment.replace(/`/g, "``")}\``; - }) - .join("."); -} - /** * Build a DescribeFetcher from a real WorkspaceClient + warehouseId. */ diff --git a/packages/appkit/src/type-generator/tests/mv-registry.test.ts b/packages/appkit/src/type-generator/tests/mv-registry.test.ts index 068861405..8a06aca68 100644 --- a/packages/appkit/src/type-generator/tests/mv-registry.test.ts +++ b/packages/appkit/src/type-generator/tests/mv-registry.test.ts @@ -3,13 +3,16 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, test } from "vitest"; +// `quoteFqnForSql` now lives in the shared zod-free leaf alongside the FQN +// grammar (moved so the analytics runtime can reuse it); the describe seam +// imports it from there. +import { quoteFqnForSql } from "../../../../shared/src/schemas/metric-fqn"; import { metricSourceSchema } from "../../../../shared/src/schemas/metric-source"; import { readMetricConfig, resolveMetricConfig } from "../mv-registry/config"; import { createWorkspaceDescribeFetcher, extractMetricColumns, parseDescribeTableExtendedJson, - quoteFqnForSql, } from "../mv-registry/describe"; import { generateMetricTypeDeclarations } from "../mv-registry/render-types"; import { syncMetrics } from "../mv-registry/sync"; diff --git a/packages/shared/src/schemas/metric-fqn.ts b/packages/shared/src/schemas/metric-fqn.ts index b958cd1a8..2a9a8647a 100644 --- a/packages/shared/src/schemas/metric-fqn.ts +++ b/packages/shared/src/schemas/metric-fqn.ts @@ -1,9 +1,8 @@ /** - * Unity Catalog object-name grammar - the single source of truth for metric - * view FQN naming validation. + * Unity Catalog object-name grammar + * the single source of truth for metric view FQN (Fully Qualified Name). * - * This module is deliberately **zod-free**. A metric view's `source` FQN is - * validated in two places that must agree: + * A metric view's `source` FQN is validated in two places that must agree: * * 1. The canonical Zod schema (`./metric-source.ts`), which composes the * three-part FQN regex from {@link UC_FQN_PATTERN} for IDE/CI and the @@ -12,18 +11,8 @@ * which imports {@link UC_FQN_PATTERN} as a plain value to validate each * dot-split segment. * - * The type-generator's runtime path must NOT pull the shared Zod schema package - * in (locked dependency-graph ruling - see the comment in - * `packages/appkit/src/type-generator/cache.ts`). Keeping the pattern in this - * zod-free module lets the runtime import the regex without dragging zod into - * its bundle, while still single-sourcing the grammar. * - * -- UC delimited (quoted) object-name rules ------------------------------ - * The metric view FQN is always backtick-quoted before interpolation into SQL - * (see `quoteFqnForSql` in the type-generator), so the **delimited identifier** - * grammar is the one that applies - not the narrower unquoted-identifier rule. - * - * Per the Databricks SQL names reference, a Unity Catalog object name: + * A Unity Catalog object name: * - cannot exceed 255 characters ({@link MAX_UC_OBJECT_NAME_LENGTH}); and * - cannot contain any of these characters: * - period (`.`) @@ -34,43 +23,14 @@ * * Every other character is permitted in a quoted name, including non-ASCII * letters (the docs demonstrate Chinese/Russian/Portuguese names) and hyphens. - * This is intentionally broader than the old hand-rolled allowlist - * (`[a-zA-Z0-9_-]`), which was flagged in PR #433 review (pkosiec: "more - * restrictive than UC"): the goal is to accept what UC accepts as a quoted - * name and reject only what UC rejects. - * - * Verified against the Databricks docs on 2026-06-19: - * https://docs.databricks.com/aws/en/sql/language-manual/sql-ref-names - * (the link cited in the PR #433 review). If the published rules change, - * re-confirm against that page. * - * @note The period is excluded here because it is the FQN segment separator - - * a name containing a literal dot cannot be expressed in the dotted `source` - * string at all. The dotted-source arity (exactly three segments) and the - * 255-char-per-segment cap are enforced structurally by the callers; this - * pattern only encodes the per-segment allowed character set. + * @note the regex is the per-segment character set; structure (3 parts) and length are intentionally left to the callers. */ -/** - * Maximum length, in characters, of a single Unity Catalog object name - * (catalog, schema, or metric view). UC rejects names longer than this. - */ export const MAX_UC_OBJECT_NAME_LENGTH = 255; /** - * Matches a single, non-empty Unity Catalog object name as it may appear in a - * backtick-quoted (delimited) identifier - one segment of a metric view FQN. - * - * Accepts any non-empty run of characters EXCEPT the UC-prohibited set: - * period, space, forward slash, ASCII control characters (U+0000-U+001F), and - * DELETE (U+007F). Length is NOT bounded here - callers enforce - * {@link MAX_UC_OBJECT_NAME_LENGTH} separately so they can emit a precise - * "segment too long" message distinct from a charset violation. - * - * The negated character class encodes the prohibited set as one contiguous - * range plus singletons: U+0000-U+0020 (every ASCII control character plus the - * space, which sits at U+0020 immediately after the control range), U+007F - * (DELETE), `.` (period - also the FQN segment separator), and `/` (slash). + * Matches a single, non-empty Unity Catalog object name as it may appear in a backtick-quoted (delimited) identifier. * * @example * UC_FQN_PATTERN.test("revenue_metrics"); // true @@ -81,3 +41,79 @@ export const MAX_UC_OBJECT_NAME_LENGTH = 255; */ // biome-ignore lint/suspicious/noControlCharactersInRegex: UC explicitly prohibits ASCII control characters in object names; this negated class encodes that rule. export const UC_FQN_PATTERN = /^[^\x00-\x20\x7f./]+$/; + +/** A metric view FQN is exactly three segments: catalog.schema.metric_view. */ +const FQN_SEGMENT_COUNT = 3; + +/** + * Total predicate: is `fqn` a well-formed three-part UC metric view FQN? + * Well-formed = exactly three non-empty, dot-separated segments, each a valid Unity Catalog object name per {@link UC_FQN_PATTERN}. + * @example + * isValidFqn("main.analytics.revenue"); // true + * isValidFqn("prod-data.analytics.rev"); // true (hyphens are UC-legal) + * isValidFqn("main.analytics"); // false (only two segments) + */ +export function isValidFqn(fqn: string): boolean { + const segments = fqn.split("."); + if (segments.length !== FQN_SEGMENT_COUNT) { + return false; + } + return segments.every((segment) => UC_FQN_PATTERN.test(segment)); +} + +/** + * Quote a dot-separated FQN for safe interpolation into a Spark/Databricks SQL + * statement. + * + * Each dot-split segment is wrapped in backtick-quoted-identifier syntax. The + * one character that can break out of a backtick-quoted identifier is the + * backtick itself, escaped by doubling (`` ` `` → `` `` ``) — so every backtick + * inside a segment is doubled before the segment is wrapped. Control characters + * and newlines have no valid escape inside a quoted identifier, so a segment + * containing one is rejected outright. + * + * This is a pure, standalone escaper: it is intentionally independent of FQN + * naming validation ({@link isValidFqn}). Naming validation decides whether an + * FQN is an acceptable metric source; this function only guarantees that + * whatever it is handed cannot break out of the quoted identifier it produces. + * Grammar and quoting live together here so a metric source is validated and + * escaped against one shared source of truth. + * + * An ordinary identifier is unchanged apart from the wrapping backticks: + * `catalog.schema.view` → `` `catalog`.`schema`.`view` ``. + * + * @param fqn - Dot-separated identifier (e.g. `catalog.schema.view`). + * @returns The backtick-quoted, escaped identifier ready for interpolation. + * @throws If any segment contains a control character or newline. + */ +export function quoteFqnForSql(fqn: string): string { + return fqn.split(".").map(quoteIdentifier).join("."); +} + +/** + * The Unicode "control" category (`\p{Cc}`): C0 (incl. `\n`, `\r`, `\t`), DEL, + * and C1 — every control character/newline. These have no valid escape inside + * a backtick-quoted identifier, so a name containing one cannot be safely + * quoted and is rejected. + */ +const CONTROL_OR_NEWLINE = /\p{Cc}/u; + +export function isValidColumnName(name: string): boolean { + return name.length > 0 && !CONTROL_OR_NEWLINE.test(name); +} + +/** + * Quote a single identifier (one column/measure/dimension name, or one FQN + * segment) as a backtick-delimited identifier for safe SQL interpolation. + * + * @throws If `name` contains a control character or newline. + */ +export function quoteIdentifier(name: string): string { + if (CONTROL_OR_NEWLINE.test(name)) { + throw new Error( + `Cannot quote identifier "${name}" for SQL: it contains a control character or newline, which has no valid escape inside a backtick-quoted identifier.`, + ); + } + // Double every backtick, then wrap in backticks. + return `\`${name.replace(/`/g, "``")}\``; +} diff --git a/packages/shared/src/schemas/metric-source.ts b/packages/shared/src/schemas/metric-source.ts index 2e4e1dfd4..4d64458a7 100644 --- a/packages/shared/src/schemas/metric-source.ts +++ b/packages/shared/src/schemas/metric-source.ts @@ -18,7 +18,25 @@ */ import { z } from "zod"; -import { UC_FQN_PATTERN } from "./metric-fqn"; +import { MAX_UC_OBJECT_NAME_LENGTH, UC_FQN_PATTERN } from "./metric-fqn"; + +/** + * Safety cap on the number of declared metric views — a typo / DoS guard, NOT + * a Unity Catalog limit. Mirrors the type-generator's `MAX_METRIC_VIEWS` so + * runtime config validation and type generation accept exactly the same + * configs (a config that fails generation must not silently pass at runtime). + */ +const MAX_METRIC_VIEWS = 200; + +/** + * Whole-FQN length cap: three max-length UC object names plus the two dots. + * The per-segment cap ({@link MAX_UC_OBJECT_NAME_LENGTH}) is enforced in the + * `superRefine` below; this whole-string bound is the declarative half (it + * serializes to a JSON-schema `maxLength`, whereas the per-segment check — like + * the entry-count cap — cannot be expressed declaratively and lives in the + * refinement, so it is a runtime/type-generator gate only). + */ +const MAX_FQN_LENGTH = MAX_UC_OBJECT_NAME_LENGTH * 3 + 2; /** * Three-part Unity Catalog FQN matcher, composed from the single-segment @@ -62,6 +80,7 @@ export const metricEntrySchema = z source: z .string() .regex(UC_THREE_PART_FQN_PATTERN) + .max(MAX_FQN_LENGTH) .describe( "Three-part Unity Catalog FQN of the metric view: ..", ) @@ -94,7 +113,39 @@ export const metricSourceSchema = z .strict() .describe( "Schema for AppKit metric-views.json — declares Unity Catalog Metric View sources for the analytics plugin's metric-view path. Each entry under 'metricViews' binds a metric key to a UC metric view FQN and an executor ('app_service_principal' shared cache, or 'user' per-user cache). Object form (rather than bare string) at v1 enables future per-entry option growth without breaking changes.", - ); + ) + // Caps that cannot be expressed declaratively (zod 4's `z.record` has no + // `.max`, and a per-dot-segment length bound isn't a whole-string + // `maxLength`). Enforced here so runtime config validation matches the + // type-generator's `resolveMetricConfig` exactly — a config that fails type + // generation must not silently pass at runtime. These refinements are + // invisible to `z.toJSONSchema`, so the generated JSON schema carries only + // the declarative `maxLength` on `source`; runtime + type-generator remain + // the authoritative gates for the entry-count and per-segment caps. + .superRefine((value, ctx) => { + const entries = value.metricViews ? Object.entries(value.metricViews) : []; + + if (entries.length > MAX_METRIC_VIEWS) { + ctx.addIssue({ + code: "custom", + message: `too many metric views: ${entries.length} declared, exceeding the maximum of ${MAX_METRIC_VIEWS}`, + path: ["metricViews"], + }); + } + + for (const [key, entry] of entries) { + const segments = entry.source.split("."); + for (let i = 0; i < segments.length; i++) { + if (segments[i].length > MAX_UC_OBJECT_NAME_LENGTH) { + ctx.addIssue({ + code: "custom", + message: `metric source segment ${i + 1} is ${segments[i].length} characters, exceeding the per-segment maximum of ${MAX_UC_OBJECT_NAME_LENGTH}`, + path: ["metricViews", key, "source"], + }); + } + } + } + }); export type MetricKey = z.infer; export type MetricExecutor = z.infer;