Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
{
"$schema": "https://databricks.github.io/appkit/schemas/metric-source.schema.json",
"metricViews": {
"revenue": {
"source": "appkit_demo.public.revenue_metrics"
Expand Down
6 changes: 3 additions & 3 deletions docs/docs/development/type-generation.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,13 +86,13 @@ In blocking mode the generator starts a stopped warehouse, waits (bounded) for i

## Metric-view types

`generate-types` (and the Vite plugin) emit metric-view types **additively** — there is no separate command. When a `config/queries/metric-views.json` file is present, the same run that generates your query types also DESCRIBEs each declared [UC Metric View](../plugins/analytics.md) and writes `metric-views.d.ts` into `shared/appkit-types/`:
`generate-types` (and the Vite plugin) emit metric-view types **additively** — there is no separate command. When a `config/metric-views/definitions.json` file is present, the same run that generates your query types also DESCRIBEs each declared [UC Metric View](../plugins/analytics.md) and writes `metric-views.d.ts` into `shared/appkit-types/`:

- `metric-views.d.ts` — augments the `MetricRegistry` interface so `useMetricView('<key>', …)` is autocompleted and type-checked. Each view's measures, dimensions, and their semantic metadata (SQL type, display name, format, time grains) are encoded at the type level.

If `metric-views.json` is absent the metric path stays dormant (nothing is emitted). When present it follows the **same** warehouse-readiness contract as query types: in the default non-blocking run a view that can't be described yet — a cold warehouse, or a bad/unreachable source — is written with permissive types and a warning, while under `--wait` that same situation fails the build so CI never ships incomplete metric types. A malformed `metric-views.json` (invalid JSON, or a source that isn't a three-part UC FQN) fails fast in every mode.
If `config/metric-views/definitions.json` is absent the metric path stays dormant (nothing is emitted). When present it follows the **same** warehouse-readiness contract as query types: in the default non-blocking run a view that can't be described yet — a cold warehouse, or a bad/unreachable source — is written with permissive types and a warning, while under `--wait` that same situation fails the build so CI never ships incomplete metric types. A malformed `definitions.json` (invalid JSON, or a source that isn't a three-part UC FQN) fails fast in every mode.

`metric-views.json` is keyed by metric key; each entry names the three-part UC FQN of the view and, optionally, the executor it runs as (`app_service_principal`, the default, or `user`):
`definitions.json` is keyed by metric key; each entry names the three-part UC FQN of the view and, optionally, the executor it runs as (`app_service_principal`, the default, or `user`):

```json
{
Expand Down
14 changes: 7 additions & 7 deletions docs/docs/plugins/analytics.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -113,9 +113,9 @@ The analytics plugin exposes these endpoints (mounted under `/api/analytics`):

## 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.
`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/metric-views/definitions.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.
The route is **dormant until `config/metric-views/definitions.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

Expand All@@ -133,7 +133,7 @@ Content-Type: application/json
}
```

`:key` is a metric key from `metric-views.json`. The body fields:
`:key` is a metric key from `definitions.json`. The body fields:

| Field | Type | Required | Description |
| --------------- | ---------- | -------- | ------------------------------------------------------------------------------------------------------------ |
Expand DownExpand Up@@ -208,7 +208,7 @@ Filters are bounded to keep hostile input from exhausting the server: nesting de

### 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:
Each entry in `definitions.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 |
| --------------------------------- | ---------------------------------- | ----------------------- |
Expand DownExpand Up@@ -241,11 +241,11 @@ On failure it emits an `error` event instead.

| 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). |
| `404` | `{ "error": "Metric not found" }` | `:key` is not declared in `definitions.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. |
| `503` | `{ "error": "Metric registry not available", "code": "METRIC_REGISTRY_LOAD_FAILED" }` | `definitions.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.
Editing `definitions.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

Expand Down
2 changes: 1 addition & 1 deletion docs/static/schemas/metric-source.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

88 changes: 75 additions & 13 deletions packages/appkit/src/app/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -29,17 +29,32 @@ interface FileSystemAdapter {

export class AppManager {
private readonly _queriesDir: string;
private readonly _metricViewsDir: string;

constructor(
queriesDir: string = path.resolve(process.cwd(), "config/queries"),
// Metric-view declarations live in a sibling `config/metric-views/`
// directory (next to `config/queries/` and `config/agents/`), NOT inside
// the queries folder. Default it as a sibling of `queriesDir` so a
// single-arg `new AppManager(dir)` (and every test that overrides only the
// queries dir) still resolves the metric-views dir consistently.
metricViewsDir: string = path.resolve(
path.dirname(queriesDir),
"metric-views",
),
) {
this._queriesDir = queriesDir;
this._metricViewsDir = metricViewsDir;
}

get queriesDir(): string {
return this._queriesDir;
}

get metricViewsDir(): string {
return this._metricViewsDir;
}

/**
* Whether `req` is a dev-tunnel (`?dev`) request. Internal `?dev` predicate
* shared by {@link createFsAdapter} (tunnel-vs-`fs` branch) and
Expand All@@ -50,15 +65,19 @@ export class AppManager {
}

/**
* Validates that a file path is within the queries directory
* Validates that a file path is within the given base directory. The
* `baseDir` is passed explicitly (rather than pinned to `queriesDir`) so the
* same traversal guard protects reads from any config directory —
* `config/queries/` for `.sql` files and `config/metric-views/` for the
* metric-view definitions.
*/
private validatePath(fileName: string): string | null {
const queryFilePath = path.join(this.queriesDir, fileName);
const resolvedPath = path.resolve(queryFilePath);
const resolvedQueriesDir = path.resolve(this.queriesDir);
private validatePath(fileName: string, baseDir: string): string | null {
const filePath = path.join(baseDir, fileName);
const resolvedPath = path.resolve(filePath);
const resolvedBaseDir = path.resolve(baseDir);

if (!resolvedPath.startsWith(resolvedQueriesDir)) {
logger.error("Invalid query path: path traversal detected");
if (!resolvedPath.startsWith(resolvedBaseDir)) {
logger.error("Invalid config path: path traversal detected");
Comment on lines +76 to +80
return null;
}

Expand DownExpand Up@@ -160,7 +179,7 @@ export class AppManager {
}

// Validate and resolve the file path
const resolvedPath = this.validatePath(queryFileName);
const resolvedPath = this.validatePath(queryFileName, this._queriesDir);
if (!resolvedPath) {
return null;
}
Expand All@@ -176,20 +195,24 @@ export class AppManager {
}

/**
* Read a single config file from the queries directory, dev-tunnel-aware.
* Read a single config file from a given base directory, dev-tunnel-aware.
* Shared core behind {@link readConfigFile} (queries dir) and
* {@link readMetricViewsConfig} (metric-views dir).
*
* @param fileName - File name (or relative path) within the queries directory.
* @param baseDir - The config directory the file must resolve within.
* @param fileName - File name (or relative path) within `baseDir`.
* @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(
private async readFileFromDir(
baseDir: string,
fileName: string,
req?: RequestLike,
devFileReader?: DevFileReader,
): Promise<string | null> {
// Traversal guard: refuse to read outside the queries directory.
const resolvedPath = this.validatePath(fileName);
// Traversal guard: refuse to read outside the base directory.
const resolvedPath = this.validatePath(fileName, baseDir);
if (!resolvedPath) {
return null;
}
Expand All@@ -208,6 +231,45 @@ export class AppManager {
}
}

/**
* 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<string | null> {
return this.readFileFromDir(this._queriesDir, fileName, req, devFileReader);
}

/**
* Read a single config file from the metric-views directory
* (`config/metric-views/`), dev-tunnel-aware. Same absent-file/traversal
* semantics as {@link readConfigFile}, but rooted at {@link metricViewsDir}.
*
* @param fileName - File name (or relative path) within the metric-views 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 readMetricViewsConfig(
fileName: string,
req?: RequestLike,
devFileReader?: DevFileReader,
): Promise<string | null> {
return this.readFileFromDir(
this._metricViewsDir,
fileName,
req,
devFileReader,
);
}

private isNotFoundError(error: unknown, req?: RequestLike): boolean {
if ((error as NodeJS.ErrnoException)?.code === "ENOENT") {
return true;
Expand Down
112 changes: 103 additions & 9 deletions packages/appkit/src/app/tests/read-config-file.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,17 +34,38 @@ describe("AppManager.readConfigFile", () => {
path.resolve(process.cwd(), "config/queries"),
);
});

test("metricViewsDir getter exposes the overridden directory", () => {
const mvDir = path.join(tmpDir, "metric-views");
const manager = new AppManager(tmpDir, mvDir);
expect(manager.metricViewsDir).toBe(mvDir);
});

test("metricViewsDir defaults to a sibling of queriesDir", () => {
// Single-arg construction: metric-views sits next to the queries dir.
const manager = new AppManager(tmpDir);
expect(manager.metricViewsDir).toBe(
path.resolve(path.dirname(tmpDir), "metric-views"),
);
});

test("metricViewsDir defaults to <cwd>/config/metric-views with no override", () => {
const defaultManager = new AppManager();
expect(defaultManager.metricViewsDir).toBe(
path.resolve(process.cwd(), "config/metric-views"),
);
});
});

describe("production mode (direct fs)", () => {
test("returns file contents for an existing file", async () => {
await fs.writeFile(
path.join(tmpDir, "metric-views.json"),
path.join(tmpDir, "sample-config.json"),
'{"hello":"world"}',
"utf8",
);

const result = await appManager.readConfigFile("metric-views.json");
const result = await appManager.readConfigFile("sample-config.json");
expect(result).toBe('{"hello":"world"}');
});

Expand All@@ -68,13 +89,13 @@ describe("AppManager.readConfigFile", () => {
vi.spyOn(fs, "readFile").mockRejectedValueOnce(err);

await expect(
appManager.readConfigFile("metric-views.json"),
appManager.readConfigFile("sample-config.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"),
path.join(tmpDir, "sample-config.json"),
"prod-contents",
"utf8",
);
Expand All@@ -84,7 +105,7 @@ describe("AppManager.readConfigFile", () => {
};

const result = await appManager.readConfigFile(
"metric-views.json",
"sample-config.json",
{ query: {}, headers: {} },
devFileReader,
);
Expand DownExpand Up@@ -114,14 +135,14 @@ describe("AppManager.readConfigFile", () => {
};

const result = await appManager.readConfigFile(
"metric-views.json",
"sample-config.json",
devReq,
devFileReader,
);

expect(result).toBe("dev-contents");
expect(devFileReader.readFile).toHaveBeenCalledWith(
expect.stringContaining("metric-views.json"),
expect.stringContaining("sample-config.json"),
devReq,
);
});
Expand All@@ -135,7 +156,7 @@ describe("AppManager.readConfigFile", () => {
};

const result = await appManager.readConfigFile(
"metric-views.json",
"sample-config.json",
devReq,
devFileReader,
);
Expand All@@ -150,8 +171,81 @@ describe("AppManager.readConfigFile", () => {
};

await expect(
appManager.readConfigFile("metric-views.json", devReq, devFileReader),
appManager.readConfigFile("sample-config.json", devReq, devFileReader),
).rejects.toThrow("tunnel disconnected");
});
});

describe("readMetricViewsConfig (metric-views dir)", () => {
let mvDir: string;
let manager: AppManager;

beforeEach(async () => {
// Give the metric-views dir its own explicit path so it is independent of
// the queries dir under test above.
mvDir = await fs.mkdtemp(path.join(os.tmpdir(), "appmgr-mv-"));
manager = new AppManager(tmpDir, mvDir);
});

afterEach(async () => {
await fs.rm(mvDir, { recursive: true, force: true });
});

test("reads definitions.json from the metric-views dir", async () => {
await fs.writeFile(
path.join(mvDir, "definitions.json"),
'{"metricViews":{}}',
"utf8",
);

const result = await manager.readMetricViewsConfig("definitions.json");
expect(result).toBe('{"metricViews":{}}');
});

test("does NOT read the file from the queries dir", async () => {
// A definitions.json in the queries dir must not resolve — the reader is
// rooted at the metric-views dir only.
await fs.writeFile(
path.join(tmpDir, "definitions.json"),
'{"metricViews":{"stray":{}}}',
"utf8",
);

const result = await manager.readMetricViewsConfig("definitions.json");
expect(result).toBeNull();
});

test("returns null for a genuine not-found (ENOENT)", async () => {
const result = await manager.readMetricViewsConfig("definitions.json");
expect(result).toBeNull();
});

test("returns null and does not read outside the metric-views dir", async () => {
const readSpy = vi.spyOn(fs, "readFile");
const result = await manager.readMetricViewsConfig("../../etc/passwd");

expect(result).toBeNull();
expect(readSpy).not.toHaveBeenCalled();
});

test("reads via devFileReader in dev mode", async () => {
const devReq = { query: { dev: "true" }, headers: {} };
const devFileReader: DevFileReader = {
readdir: vi.fn(),
readFile: vi.fn().mockResolvedValue("dev-mv-contents"),
};

const result = await manager.readMetricViewsConfig(
"definitions.json",
devReq,
devFileReader,
);

expect(result).toBe("dev-mv-contents");
expect(devFileReader.readFile).toHaveBeenCalledWith(
expect.stringContaining("definitions.json"),
devReq,
);
});
});
});
Loading