From d2496f8b0b409d3d92f631c36cff278e57ef2761 Mon Sep 17 00:00:00 2001 From: Robin Timan Date: Mon, 19 Oct 2026 15:30:03 +0200 Subject: [PATCH 1/2] refactor(cli): add shared Metrics HTTP command foundation Co-Authored-By: Xirp Agent (gpt-6.1-sol) --- packages/cli/__tests__/commands/data.test.ts | 51 ++++ .../__tests__/features/data/dry-run.test.ts | 284 ++++++++++++++++++ .../cli/__tests__/network/metrics.test.ts | 194 ++++++++++++ packages/cli/bin/cli.ts | 2 + packages/cli/src/commands/data.ts | 16 + packages/cli/src/commands/index.ts | 1 + packages/cli/src/features/data/index.ts | 2 + packages/cli/src/features/data/output.ts | 26 ++ packages/cli/src/features/data/request.ts | 34 +++ packages/cli/src/network/index.ts | 2 + packages/cli/src/network/metrics-types.ts | 28 ++ packages/cli/src/network/metrics.ts | 30 ++ packages/cli/src/utils/api-error.ts | 11 + packages/cli/src/utils/index.ts | 1 + packages/cli/src/utils/telemetry.ts | 13 +- packages/core/__tests__/api/client.test.ts | 176 +++++++++++ packages/core/src/api/client.ts | 66 +++- packages/core/src/api/types.ts | 6 + 18 files changed, 927 insertions(+), 16 deletions(-) create mode 100644 packages/cli/__tests__/commands/data.test.ts create mode 100644 packages/cli/__tests__/features/data/dry-run.test.ts create mode 100644 packages/cli/__tests__/network/metrics.test.ts create mode 100644 packages/cli/src/commands/data.ts create mode 100644 packages/cli/src/features/data/index.ts create mode 100644 packages/cli/src/features/data/output.ts create mode 100644 packages/cli/src/features/data/request.ts create mode 100644 packages/cli/src/network/metrics-types.ts create mode 100644 packages/cli/src/network/metrics.ts create mode 100644 packages/cli/src/utils/api-error.ts diff --git a/packages/cli/__tests__/commands/data.test.ts b/packages/cli/__tests__/commands/data.test.ts new file mode 100644 index 00000000..1bf9cd8e --- /dev/null +++ b/packages/cli/__tests__/commands/data.test.ts @@ -0,0 +1,51 @@ +import { execFileSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import yargs from 'yargs'; +import { prepareAuthTokens } from '@spotify-confidence/testing/auth'; +import { dataCommand } from '@commands/index.js'; + +function run(args: string[]) { + let output = ''; + const sut = yargs(args).scriptName('confidence').command(dataCommand).help().exitProcess(false); + return sut + .parseAsync(args, (_error: Error | undefined, _argv: unknown, text: string) => { + output = text; + }) + .then(() => output); +} + +describe('data command group', () => { + it('is registered in the real root help alongside existing commands', () => { + using _auth = prepareAuthTokens('none'); + const sut = fileURLToPath(new URL('../../bin/cli.ts', import.meta.url)); + + const output = execFileSync(process.execPath, ['--import', 'tsx', sut, '--help'], { + encoding: 'utf8', + env: { ...process.env, CONFIDENCE_TELEMETRY: 'false' }, + }); + + expect(output).toContain('confidence data'); + expect(output).toContain('confidence warehouse'); + expect(output).toContain('confidence flags'); + }); + + it('shows help without authentication or placeholder API subcommands', async () => { + using _auth = prepareAuthTokens('none'); + const sut = run; + + const output = await sut(['data', '--help']); + + expect(output).toContain('confidence data '); + expect(output).not.toMatch(/onboard|discover|proposal|workflow/iu); + }); + + it('shows guidance when invoked without a subcommand', async () => { + const sut = run; + expect(await sut(['data'])).toContain('confidence data --help'); + }); + + it('rejects unimplemented subcommands', async () => { + const sut = run; + expect(await sut(['data', 'invented'])).toContain('Unknown argument: invented'); + }); +}); diff --git a/packages/cli/__tests__/features/data/dry-run.test.ts b/packages/cli/__tests__/features/data/dry-run.test.ts new file mode 100644 index 00000000..ad31223c --- /dev/null +++ b/packages/cli/__tests__/features/data/dry-run.test.ts @@ -0,0 +1,284 @@ +import { mkdtempSync, mkdirSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { http, HttpResponse, server } from '@spotify-confidence/testing'; +import { buildTestJwt, prepareAuthTokens } from '@spotify-confidence/testing/auth'; +import { resetTelemetry } from '@spotify-confidence/core'; +import { executeDataRequest, printData } from '@features/data/index.js'; +import { metricsRequest } from '@network/metrics.js'; +import { requireRecord, requireString } from '@network/metrics-types.js'; +import { flushTelemetry, telemetryMiddleware } from '@utils/telemetry.js'; +import { captureOutput } from '../../helpers/capture.js'; + +const TEST_URL = 'https://metrics.eu.confidence.dev/v1/testResources'; + +function inputFile(contents: string) { + const dir = mkdtempSync(join(tmpdir(), 'confidence-data-test-')); + const path = join(dir, 'input.json'); + writeFileSync(path, contents); + return { + path, + [Symbol.dispose]() { + rmSync(dir, { recursive: true, force: true }); + }, + }; +} + +// A feature-like caller validates real input before the shared execution helper. +async function runValidatedFile(argv: Record) { + const input = requireRecord( + JSON.parse(readFileSync(argv['from-file'] as string, 'utf8')), + 'Input', + ); + requireString(input.displayName, 'displayName'); + await executeDataRequest( + argv, + (token) => metricsRequest(token, { path: '/v1/testResources', method: 'POST', body: input }), + { method: 'POST', path: '/v1/testResources', body: input }, + ); +} + +afterEach(() => { + resetTelemetry(); + vi.unstubAllEnvs(); +}); + +describe('Data dry-runs', () => { + it('previews validated input before auth with no HTTP or file writes', async () => { + // Arrange + using _auth = prepareAuthTokens('none'); + using file = inputFile('{"displayName":"Example"}'); + using output = captureOutput(); + const configDir = process.env.CONFIDENCE_CONFIG_DIR!; + const before = readdirSync(configDir, { recursive: true }); + let calls = 0; + server.use( + http.all('*', () => { + calls++; + return HttpResponse.json({}); + }), + ); + const sut = runValidatedFile; + + // Act + await sut({ 'dry-run': true, 'from-file': file.path, json: true, profile: '../invalid' }); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ + data: { method: 'POST', path: '/v1/testResources', body: { displayName: 'Example' } }, + }); + expect(output.stderr).toBe(''); + expect(calls).toBe(0); + expect(readdirSync(configDir, { recursive: true })).toEqual(before); + expect(readFileSync(file.path, 'utf8')).toBe('{"displayName":"Example"}'); + }); + + it.each(['invalid JSON', '[]', '{"displayName":42}'])( + 'still rejects invalid local input %s before preview or auth', + async (contents) => { + using _auth = prepareAuthTokens('none'); + using file = inputFile(contents); + using output = captureOutput(); + const sut = runValidatedFile; + + await expect(sut({ 'dry-run': true, 'from-file': file.path, json: true })).rejects.toThrow(); + + expect(output.stdout).toBe(''); + expect(output.stderr).toBe(''); + }, + ); + + it('has a useful local preview when none is supplied', async () => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + let calls = 0; + const sut = executeDataRequest; + + await sut({ 'dry-run': true, json: true }, async () => { + calls++; + return { ok: true, data: {} }; + }); + + expect(JSON.parse(output.stdout)).toEqual({ data: { dryRun: true } }); + expect(calls).toBe(0); + }); +}); + +describe('Data request execution', () => { + it.each(['none', 'expired'] as const)( + 'fails without usable %s credentials before sending', + async (auth) => { + using _auth = prepareAuthTokens(auth); + using output = captureOutput(); + let calls = 0; + const sut = executeDataRequest; + + await sut({}, async () => { + calls++; + return { ok: true, data: {} }; + }); + + expect(calls).toBe(0); + expect(output.stderr).toMatch(/Not logged in|Token expired/u); + expect(process.exitCode).toBe(1); + }, + ); + + it('sends once and preserves state, errors, and pagination in the JSON envelope', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using file = inputFile('{"displayName":"Example"}'); + using output = captureOutput(); + let calls = 0; + const data = { state: 'FAILED', error: { message: 'Resource failed' }, nextPageToken: 'next' }; + server.use( + http.post(TEST_URL, () => { + calls++; + return HttpResponse.json(data); + }), + ); + const previousExitCode = process.exitCode; + const sut = runValidatedFile; + + // Act + await sut({ 'from-file': file.path, json: true }); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ data }); + expect(output.stderr).toBe(''); + expect(process.exitCode).toBe(previousExitCode); + expect(calls).toBe(1); + }); + + it('uses the requested auth profile', async () => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const profileDir = join(process.env.CONFIDENCE_CONFIG_DIR!, 'profiles', 'metrics'); + const token = buildTestJwt({ email: 'profile@example.com' }); + mkdirSync(profileDir, { recursive: true }); + writeFileSync(join(profileDir, 'credentials.json'), JSON.stringify({ accessToken: token })); + server.use( + http.get(TEST_URL, ({ request }) => + HttpResponse.json({ + authorized: request.headers.get('Authorization') === `Bearer ${token}`, + }), + ), + ); + const sut = executeDataRequest; + + // Act + await sut({ profile: 'metrics', json: true }, (value) => + metricsRequest(value, { path: '/v1/testResources' }), + ); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ data: { authorized: true } }); + }); + + it('prints a sanitized failure, omits arbitrary details, and exits nonzero without replay', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + let calls = 0; + server.use( + http.get(TEST_URL, () => { + calls++; + return HttpResponse.json( + { + code: 7, + message: 'Denied\n\u001b[31m Bearer private-credential', + details: [{ secret: 'private' }], + }, + { status: 403 }, + ); + }), + ); + const sut = executeDataRequest; + + // Act + await sut({ json: true }, (token) => metricsRequest(token, { path: '/v1/testResources' })); + + // Assert + expect(output.stdout).toBe(''); + expect(output.stderr).toContain('Denied'); + expect(output.stderr).not.toContain('private'); + expect(output.stderr).not.toContain('\u001b'); + expect(process.exitCode).toBe(1); + expect(calls).toBe(1); + }); + + it('reports thrown response validation errors as nonzero failures', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const sut = executeDataRequest; + + await sut({}, async () => { + throw new Error('Invalid resource response'); + }); + + expect(output.stderr).toContain('Invalid resource response'); + expect(process.exitCode).toBe(1); + }); + + it('renders nested resources as readable output when requested', () => { + using output = captureOutput(); + const sut = printData; + + sut({ name: 'testResources/a', status: { state: 'PENDING' } }, { output: 'table' }); + + expect(output.stdout).toContain('Field'); + expect(output.stdout).toContain('testResources/a'); + expect(output.stdout).toContain('PENDING'); + expect(output.stdout).not.toContain('[object Object]'); + }); +}); + +describe('dry-run telemetry suppression', () => { + it.each([ + ['data', 'entities', 'list'], + ['data', 'sql', 'preview'], + ['warehouse', 'list'], + ['warehouse', 'get'], + ])('sends no telemetry for %j even when telemetry is enabled', async (...command) => { + vi.stubEnv('CONFIDENCE_TELEMETRY', 'true'); + let calls = 0; + server.use( + http.all('*', () => { + calls++; + return HttpResponse.json({ clientSecret: 'test' }); + }), + ); + const sut = telemetryMiddleware; + + sut({ _: command, telemetry: true, 'dry-run': true }); + await flushTelemetry(); + + expect(calls).toBe(0); + }); + + it.each([ + { command: ['flags', 'list'], dryRun: true }, + { command: ['warehouse', 'create'], dryRun: true }, + { command: ['data', 'entities', 'list'], dryRun: false }, + ])('preserves existing telemetry behavior for %j', async ({ command, dryRun }) => { + vi.stubEnv('CONFIDENCE_TELEMETRY', 'true'); + const steps: string[] = []; + server.use( + http.post('https://onboarding.confidence.dev/v1/agentTelemetryKey:acquire', () => + HttpResponse.json({ clientSecret: 'test' }), + ), + http.post('https://events.eu.confidence.dev/v1/events:publish', async ({ request }) => { + const body = (await request.json()) as { events: { payload: { step: string } }[] }; + steps.push(body.events[0].payload.step); + return HttpResponse.json({}); + }), + ); + const sut = telemetryMiddleware; + + sut({ _: command, telemetry: true, 'dry-run': dryRun }); + await flushTelemetry(); + + expect(steps).toEqual([command.join('.')]); + }); +}); diff --git a/packages/cli/__tests__/network/metrics.test.ts b/packages/cli/__tests__/network/metrics.test.ts new file mode 100644 index 00000000..ea9cc088 --- /dev/null +++ b/packages/cli/__tests__/network/metrics.test.ts @@ -0,0 +1,194 @@ +import { http, HttpResponse, server } from '@spotify-confidence/testing'; +import { buildTestJwt } from '@spotify-confidence/testing/auth'; +import { metricsRequest } from '@network/metrics.js'; +import { isRecord, requireRecord, requireString, resourcePath } from '@network/metrics-types.js'; + +afterEach(() => vi.unstubAllEnvs()); + +describe('Metrics requests', () => { + it.each(['EU', 'US'])('routes to %s and authenticates the request', async (region) => { + const token = buildTestJwt({ 'https://confidence.dev/region': region }); + server.use( + http.get( + `https://metrics.${region.toLowerCase()}.confidence.dev/v1/testResources`, + ({ request }) => + HttpResponse.json({ + authorized: request.headers.get('Authorization') === `Bearer ${token}`, + }), + ), + ); + const sut = metricsRequest; + + const result = await sut(token, { path: '/v1/testResources' }); + + expect(result).toEqual({ ok: true, data: { authorized: true } }); + }); + + it('honors the API base override and sends body and explicit pagination parameters', async () => { + vi.stubEnv('CONFIDENCE_API_BASE_URL', 'http://localhost:9876'); + server.use( + http.post('http://localhost:9876/v1/testResources', async ({ request }) => + HttpResponse.json({ + body: await request.json(), + params: Object.fromEntries(new URL(request.url).searchParams), + }), + ), + ); + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { + path: '/v1/testResources', + method: 'POST', + body: { displayName: 'Example' }, + params: { page_size: '10', page_token: 'opaque?token/#' }, + }); + + expect(result).toEqual({ + ok: true, + data: { + body: { displayName: 'Example' }, + params: { page_size: '10', page_token: 'opaque?token/#' }, + }, + }); + }); + + it('preserves structured failures without retrying writes', async () => { + let calls = 0; + const error = { code: 3, message: 'Invalid input', details: [{ field: 'name' }] }; + server.use( + http.post('https://metrics.eu.confidence.dev/v1/testResources', () => { + calls++; + return HttpResponse.json(error, { status: 400 }); + }), + ); + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { path: '/v1/testResources', method: 'POST' }); + + expect(result).toEqual({ ok: false, status: 400, error }); + expect(calls).toBe(1); + }); + + it('does not replay a redirected write or send credentials to its location', async () => { + let redirectedCalls = 0; + server.use( + http.post( + 'https://metrics.eu.confidence.dev/v1/testResources', + () => + new HttpResponse(null, { + status: 307, + headers: { Location: 'https://metrics.eu.confidence.dev/v1/redirected' }, + }), + ), + http.post('https://metrics.eu.confidence.dev/v1/redirected', () => { + redirectedCalls++; + return HttpResponse.json({}); + }), + ); + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { + path: '/v1/testResources', + method: 'POST', + body: { displayName: 'Example' }, + }); + + expect(result).toMatchObject({ ok: false, status: 0 }); + expect(redirectedCalls).toBe(0); + }); + + it('passes cancellation through to the HTTP client', async () => { + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { + path: '/v1/testResources', + signal: AbortSignal.abort(), + timeoutMs: 100, + }); + + expect(result).toMatchObject({ ok: false, status: 0 }); + }); + + it.each([ + 'https://other.example/v1/testResources', + '//other.example/v1/testResources', + '/v1/../admin', + '/v1/%2e%2e/admin', + '/v1/testResources?secret=value', + '/v1/testResources#fragment', + '/v1/\\other.example', + ])('rejects unsafe API path %s before sending', async (path) => { + const sut = metricsRequest; + await expect(sut(buildTestJwt(), { path })).rejects.toThrow('local /v1/ API path'); + }); +}); + +describe('resource paths', () => { + it('builds a path from a qualified resource with exactly one ID', () => { + const sut = resourcePath; + expect(sut('testResources/abc-123_A', 'testResources')).toBe('/v1/testResources/abc-123_A'); + }); + + it.each([ + '', + 'abc', + '/testResources/abc', + 'https://evil.example/testResources/abc', + '//evil.example/testResources/abc', + 'other/abc', + 'testResources/', + 'testResources/.', + 'testResources/..', + 'testResources/../other', + 'testResources/abc/child', + 'testResources/%2e%2e', + 'testResources/%252fadmin', + 'testResources/a%2fb', + 'testResources/a?query=x', + 'testResources/a#fragment', + 'testResources/a\\b', + 'testResources/a b', + 'testResources/abc\n', + ])('rejects invalid resource name %j', (name) => { + const sut = resourcePath; + expect(() => sut(name, 'testResources')).toThrow('single valid ID'); + }); + + it.each(['../testResources', '/testResources', 'testResources/child', 'testResources?x=y'])( + 'rejects invalid collection %s', + (collection) => { + const sut = resourcePath; + expect(() => sut('testResources/abc', collection)).toThrow('Invalid resource collection'); + }, + ); +}); + +describe('response guards', () => { + it.each([null, [], 'text', 42, undefined].map((value) => ({ value })))( + 'rejects non-record value $value', + ({ value }) => { + const sut = requireRecord; + expect(isRecord(value)).toBe(false); + expect(() => sut(value, 'Response')).toThrow('Response must be an object'); + }, + ); + + it('preserves resource fields rather than discarding unknown ones', () => { + const resource = { name: 'testResources/a', state: 'FAILED', extra: { reason: 'example' } }; + const sut = requireRecord; + expect(sut(resource)).toBe(resource); + }); + + it.each(['', ' ', null, [], 42].map((value) => ({ value })))( + 'rejects empty or non-string value $value', + ({ value }) => { + const sut = requireString; + expect(() => sut(value, 'Name')).toThrow('Name must be a non-empty string'); + }, + ); + + it('does not silently rewrite strings', () => { + const sut = requireString; + expect(sut(' original ')).toBe(' original '); + }); +}); diff --git a/packages/cli/bin/cli.ts b/packages/cli/bin/cli.ts index 1e9f9157..f52a9689 100644 --- a/packages/cli/bin/cli.ts +++ b/packages/cli/bin/cli.ts @@ -20,6 +20,7 @@ import { pluginCommand, migrateCommand, warehouseCommand, + dataCommand, } from '../src/commands/index.js'; const cli = yargs(hideBin(process.argv)) @@ -88,6 +89,7 @@ const cli = yargs(hideBin(process.argv)) .command(pluginCommand) .command(migrateCommand) .command(warehouseCommand) + .command(dataCommand) .example('$0 login', 'Sign in to Confidence') .example('$0 flags setup', 'Set up feature flags in your project') .example('$0 docs search "feature flags"', 'Search the documentation') diff --git a/packages/cli/src/commands/data.ts b/packages/cli/src/commands/data.ts new file mode 100644 index 00000000..ec20479f --- /dev/null +++ b/packages/cli/src/commands/data.ts @@ -0,0 +1,16 @@ +import type { Argv } from 'yargs'; +import { noop } from '@spotify-confidence/shared-kernel'; + +/** Feature PRs register their subcommands in this group. */ +export const dataCommand = { + command: 'data', + describe: 'Work with Confidence data resources', + builder(yargs: Argv) { + return yargs + .usage('$0 data [options]') + .demandCommand(1, 'Run "confidence data --help" to see available commands.') + .strict() + .help(); + }, + handler: noop, +}; diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 84dd9b05..fbbee90a 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -13,4 +13,5 @@ export { pluginCommand } from './plugin.js'; export { updateCommand } from './update.js'; export { migrateCommand } from './migrate.js'; export { warehouseCommand } from './warehouse.js'; +export { dataCommand } from './data.js'; export type { GlobalFlags } from './types.js'; diff --git a/packages/cli/src/features/data/index.ts b/packages/cli/src/features/data/index.ts new file mode 100644 index 00000000..a4c9385e --- /dev/null +++ b/packages/cli/src/features/data/index.ts @@ -0,0 +1,2 @@ +export { printData } from './output.js'; +export { executeDataRequest } from './request.js'; diff --git a/packages/cli/src/features/data/output.ts b/packages/cli/src/features/data/output.ts new file mode 100644 index 00000000..06ba6ba0 --- /dev/null +++ b/packages/cli/src/features/data/output.ts @@ -0,0 +1,26 @@ +import { inspect } from 'node:util'; +import { isRecord } from '@network/metrics-types.js'; +import { extractFlags, formatJson, message, print, resolveFormat } from '@output/index.js'; + +/** Preserve the full response in JSON; render generic resources as key/value rows otherwise. */ +export function printData(data: unknown, argv: Record): void { + const flags = extractFlags(argv); + if (resolveFormat(flags) === 'json') { + message(formatJson(data)); + return; + } + + const fields = isRecord(data) ? data : { value: data }; + const rows = Object.entries(fields).map(([field, value]) => ({ + field, + value: typeof value === 'string' ? value : inspect(value, { depth: null, colors: false }), + })); + print({ + data: rows, + columns: [ + { key: 'field', header: 'Field' }, + { key: 'value', header: 'Value' }, + ], + flags, + }); +} diff --git a/packages/cli/src/features/data/request.ts b/packages/cli/src/features/data/request.ts new file mode 100644 index 00000000..24132bbd --- /dev/null +++ b/packages/cli/src/features/data/request.ts @@ -0,0 +1,34 @@ +import type { ApiResponse } from '@spotify-confidence/core'; +import { printApiError } from '@utils/api-error.js'; +import { requireAuth } from '@utils/auth.js'; +import { printData } from './output.js'; + +/** Callers must validate arguments and input before invoking this helper, including dry-runs. */ +export async function executeDataRequest( + argv: Record, + request: (token: string) => Promise>, + preview?: unknown, +): Promise { + if (argv['dry-run'] === true) { + printData(preview ?? { dryRun: true }, argv); + return; + } + + const token = requireAuth(argv.profile as string | undefined); + if (!token) return; + + try { + const result = await request(token); + if (!result.ok) { + printApiError(result); + return; + } + printData(result.data, argv); + } catch (err) { + printApiError({ + ok: false, + status: 0, + error: { code: 0, message: err instanceof Error ? err.message : 'Data request failed.' }, + }); + } +} diff --git a/packages/cli/src/network/index.ts b/packages/cli/src/network/index.ts index 609492b2..a63a9054 100644 --- a/packages/cli/src/network/index.ts +++ b/packages/cli/src/network/index.ts @@ -1,4 +1,6 @@ export { fetchLatestVersion } from './registry.js'; +export { metricsRequest } from './metrics.js'; +export { isRecord, requireRecord, requireString, resourcePath } from './metrics-types.js'; export { searchDocumentation, grepDocumentation, getFullSource } from './docs.js'; diff --git a/packages/cli/src/network/metrics-types.ts b/packages/cli/src/network/metrics-types.ts new file mode 100644 index 00000000..ac6f61dd --- /dev/null +++ b/packages/cli/src/network/metrics-types.ts @@ -0,0 +1,28 @@ +/** Small response/input guards; resource-specific validation belongs to each feature. */ +export function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +export function requireRecord(value: unknown, label = 'Value'): Record { + if (!isRecord(value)) throw new Error(`${label} must be an object.`); + return value; +} + +export function requireString(value: unknown, label = 'Value'): string { + if (typeof value !== 'string' || value.trim().length === 0) { + throw new Error(`${label} must be a non-empty string.`); + } + return value; +} + +/** A qualified resource name must contain exactly one unescaped resource ID. */ +export function resourcePath(name: string, collection: string): string { + if (!/^[A-Za-z][A-Za-z0-9]*$/u.test(collection)) { + throw new Error('Invalid resource collection.'); + } + const prefix = `${collection}/`; + if (!name.startsWith(prefix) || !/^[A-Za-z0-9][A-Za-z0-9_-]*$/u.test(name.slice(prefix.length))) { + throw new Error(`Resource name must be ${collection}/ with a single valid ID.`); + } + return `/v1/${name}`; +} diff --git a/packages/cli/src/network/metrics.ts b/packages/cli/src/network/metrics.ts new file mode 100644 index 00000000..30b32781 --- /dev/null +++ b/packages/cli/src/network/metrics.ts @@ -0,0 +1,30 @@ +import { + apiRequest, + extractRegion, + type ApiRequestOptions, + type ApiResponse, +} from '@spotify-confidence/core'; + +/** Send one authenticated request to the token's regional Metrics API. */ +export async function metricsRequest( + token: string, + options: Omit, +): Promise> { + // Accept API paths, never absolute URLs or response-supplied pagination URLs. + if ( + !options.path.startsWith('/v1/') || + /[\\%?#\s]/u.test(options.path) || + options.path.split('/').some((part) => part === '.' || part === '..') + ) { + throw new Error('Metrics requests require a local /v1/ API path.'); + } + + return apiRequest({ + ...options, + token, + region: extractRegion(token), + service: 'metrics', + // Redirects could replay writes or send credentials to an API-supplied URL. + redirect: 'error', + }); +} diff --git a/packages/cli/src/utils/api-error.ts b/packages/cli/src/utils/api-error.ts new file mode 100644 index 00000000..4022ce40 --- /dev/null +++ b/packages/cli/src/utils/api-error.ts @@ -0,0 +1,11 @@ +import type { ApiFailure } from '@spotify-confidence/core'; +import { fail } from '@output/index.js'; + +/** Render only the API message, not arbitrary response bodies or error details. */ +export function printApiError(failure: ApiFailure): void { + const message = failure.error.message + .replace(/Bearer\s+\S+/giu, 'Bearer [redacted]') + .replace(/[\p{Cc}\p{Cf}]/gu, ' ') + .trim(); + fail(message || `API request failed (HTTP ${failure.status}).`); +} diff --git a/packages/cli/src/utils/index.ts b/packages/cli/src/utils/index.ts index 248d517a..90f45a7e 100644 --- a/packages/cli/src/utils/index.ts +++ b/packages/cli/src/utils/index.ts @@ -1,4 +1,5 @@ export * from './auth.js'; +export * from './api-error.js'; export * from './mcp-error.js'; export * from './safely.js'; export * from './telemetry.js'; diff --git a/packages/cli/src/utils/telemetry.ts b/packages/cli/src/utils/telemetry.ts index 6733d36d..b0381671 100644 --- a/packages/cli/src/utils/telemetry.ts +++ b/packages/cli/src/utils/telemetry.ts @@ -2,7 +2,18 @@ import { initTelemetry, isTelemetryEnabled, getTelemetry, flush } from '@spotify export { flush as flushTelemetry }; -export function telemetryMiddleware(argv: { telemetry: boolean; _: (string | number)[] }) { +export function telemetryMiddleware(argv: { + telemetry: boolean; + _: (string | number)[]; + 'dry-run'?: boolean; +}) { + const [group, action] = argv._; + if ( + argv['dry-run'] === true && + (group === 'data' || (group === 'warehouse' && (action === 'list' || action === 'get'))) + ) { + return; + } if (!isTelemetryEnabled() || argv.telemetry === false) return; initTelemetry({ source: 'cli' }); diff --git a/packages/core/__tests__/api/client.test.ts b/packages/core/__tests__/api/client.test.ts index 981da382..77849826 100644 --- a/packages/core/__tests__/api/client.test.ts +++ b/packages/core/__tests__/api/client.test.ts @@ -1,7 +1,14 @@ import { http, HttpResponse, server } from '@spotify-confidence/testing'; import { resolveBaseUrl, apiRequest } from '@api/client.js'; +import type { ApiRequestOptions } from '@api/types.js'; const TEST_BASE = 'https://test-svc.eu.confidence.dev'; +const requestOptions: ApiRequestOptions = { + token: 'test-token', + region: 'EU', + service: 'test-svc', + path: '/v1/items', +}; beforeEach(() => { server.use( @@ -54,6 +61,175 @@ describe('resolveBaseUrl', () => { }); }); +describe('apiRequest response safety', () => { + it.each(['not JSON', '', 'private upstream error'])( + 'returns a controlled failure for malformed success body %j', + async (body) => { + server.use(http.get(`${TEST_BASE}/v1/items`, () => new HttpResponse(body))); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 200, + error: { code: 200, message: 'Invalid JSON response from API.' }, + }); + }, + ); + + it.each( + [null, [], 'error', { message: {} }, { code: 'bad', message: 'invalid' }].map((body) => ({ + body, + })), + )('normalizes malformed error response $body', async ({ body }) => { + server.use( + http.get(`${TEST_BASE}/v1/items`, () => + HttpResponse.json(body, { status: 503, statusText: 'Service Unavailable' }), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 503, + error: { code: 503, message: 'Service Unavailable' }, + }); + }); + + it('does not expose non-JSON error bodies', async () => { + server.use( + http.get( + `${TEST_BASE}/v1/items`, + () => new HttpResponse('upstream private details', { status: 502 }), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 502, + error: { code: 502, message: 'Bad Gateway' }, + }); + }); + + it('preserves structured API errors and details', async () => { + const error = { code: 3, message: 'Invalid input', details: [{ field: 'name' }] }; + server.use(http.get(`${TEST_BASE}/v1/items`, () => HttpResponse.json(error, { status: 400 }))); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ ok: false, status: 400, error }); + }); + + it('discards malformed error details', async () => { + server.use( + http.get(`${TEST_BASE}/v1/items`, () => + HttpResponse.json({ code: 3, message: 'Invalid input', details: 'bad' }, { status: 400 }), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 400, + error: { code: 3, message: 'Invalid input' }, + }); + }); + + it('returns a controlled failure if reading the body fails', async () => { + server.use( + http.get( + `${TEST_BASE}/v1/items`, + () => + new HttpResponse( + new ReadableStream({ + start(controller) { + controller.error(new Error('Connection closed')); + }, + }), + ), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + if (!result.ok) expect(result.error.message).toContain('Network error'); + }); +}); + +describe('apiRequest cancellation', () => { + it('honors an already cancelled caller signal', async () => { + const sut = apiRequest; + + const result = await sut({ ...requestOptions, signal: AbortSignal.abort() }); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + }); + + it.each(['timeout', 'caller'] as const)('cancels before headers via %s', async (mode) => { + const controller = new AbortController(); + server.use( + http.get(`${TEST_BASE}/v1/items`, async ({ request }) => { + if (mode === 'caller') controller.abort(); + await new Promise((resolve) => { + if (request.signal.aborted) resolve(); + else request.signal.addEventListener('abort', () => resolve(), { once: true }); + }); + return HttpResponse.json({}); + }), + ); + const sut = apiRequest; + + const result = await sut({ + ...requestOptions, + signal: controller.signal, + timeoutMs: mode === 'timeout' ? 50 : 1000, + }); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + }); + + it.each(['timeout', 'caller'] as const)('cancels body consumption via %s', async (mode) => { + const controller = new AbortController(); + server.use( + http.get( + `${TEST_BASE}/v1/items`, + ({ request }) => + new HttpResponse( + new ReadableStream({ + start(body) { + body.enqueue(new TextEncoder().encode('{')); + request.signal.addEventListener('abort', () => body.error(request.signal.reason), { + once: true, + }); + if (mode === 'caller') queueMicrotask(() => controller.abort()); + }, + }), + ), + ), + ); + const sut = apiRequest; + + const result = await sut({ + ...requestOptions, + signal: controller.signal, + timeoutMs: mode === 'timeout' ? 50 : 1000, + }); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + }); +}); + describe('apiRequest', () => { it('sends an authorized GET request', async () => { const result = await apiRequest<{ auth: string }>({ diff --git a/packages/core/src/api/client.ts b/packages/core/src/api/client.ts index a4501eee..71aecdbf 100644 --- a/packages/core/src/api/client.ts +++ b/packages/core/src/api/client.ts @@ -25,32 +25,68 @@ export async function apiRequest(opts: ApiRequestOptions): Promise; + /** Cancels both the request and response body consumption. */ + signal?: AbortSignal; + /** Redirect policy. Defaults to fetch's existing follow behavior. */ + redirect?: 'follow' | 'error'; + /** Overall deadline in milliseconds, including response body consumption. Defaults to 30 seconds. */ + timeoutMs?: number; }; From af051d0e96e04c03a33123c4f227fec8b3d5ab7b Mon Sep 17 00:00:00 2001 From: Robin Timan Date: Mon, 19 Oct 2026 17:45:29 +0200 Subject: [PATCH 2/2] feat(cli): add dimension table list get and create commands Co-Authored-By: Xirp Agent (gpt-6.1-sol) --- packages/cli/README.md | 72 ++ .../commands/dimension-tables.test.ts | 804 ++++++++++++++++++ .../features/data/dimension-input.test.ts | 314 +++++++ packages/cli/src/commands/data.ts | 3 +- .../cli/src/commands/data/dimension-tables.ts | 62 ++ .../cli/src/features/data/dimension-input.ts | 273 ++++++ .../cli/src/features/data/dimension-tables.ts | 45 + packages/cli/src/features/data/index.ts | 5 + .../src/network/metrics-dimension-tables.ts | 120 +++ 9 files changed, 1697 insertions(+), 1 deletion(-) create mode 100644 packages/cli/__tests__/commands/dimension-tables.test.ts create mode 100644 packages/cli/__tests__/features/data/dimension-input.test.ts create mode 100644 packages/cli/src/commands/data/dimension-tables.ts create mode 100644 packages/cli/src/features/data/dimension-input.ts create mode 100644 packages/cli/src/features/data/dimension-tables.ts create mode 100644 packages/cli/src/network/metrics-dimension-tables.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index fcfcb34b..38dce24a 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -91,6 +91,78 @@ confidence quickstart # Launch the interactive setup wizard | `confidence warehouse assignment-table create` | Create an assignment table for experiment analysis | | `confidence warehouse crypto-key create` | Create a crypto key (Snowflake) | +### Dimension Tables + +| Command | Description | +| --------------------------------------------------------------------------------- | ----------------------------------------- | +| `confidence data dimension-tables list` | List one page of dimension tables | +| `confidence data dimension-tables get ` | Get a table by ID or `dimensionTables/id` | +| `confidence data dimension-tables create --from-file [--id ]` | Create one dimension table | + +List options: `--page-size`, `--page-token`, `--filter` (Lucene query string), +`--order-by` (for example `displayName:asc`), `--show-deleted`, and `--all` to +follow every page, up to a hard limit of 1,000 pages. Without `--all`, JSON output +includes the page's `nextPageToken`. Repeated tokens, malformed responses, and +exceeding the page limit fail without returning an incomplete successful result; +narrow the filter or use single-page requests for larger result sets. + +Create reads **canonical camelCase DimensionTable resource JSON**, not a request +wrapper or CLI option names: + +```json +{ + "sql": "SELECT user_id, country FROM analytics.users", + "displayName": "User dimensions", + "entityColumnMapping": { + "column": { "name": "user_id" }, + "entity": "entities/user" + }, + "dimensions": [{ "name": "country" }], + "valueMappings": [ + { + "column": "country", + "mappings": [ + { "inputDimensionValue": "SE", "outputDimensionValue": "Sweden" }, + { "inputDimensionValue": null, "outputDimensionValue": "Unknown" } + ] + } + ] +} +``` + +Required fields are `sql`, `displayName`, `entityColumnMapping`, and a non-empty +`dimensions` array. Optional `description`, `labels`, `owner`, `timeConfig`, +`valueMappings`, `dimensionDataDeliveredUntilTime`, and +`dataDeliveredUntilUpdateStrategyConfig` are validated and preserved. A column +accepts `name` and optional boolean `repeated`, not output-only `type` or +annotations. `timeConfig` requires `timestampColumn` and a protobuf duration +`window` (for example `"86400s"`). Value-mapping inputs/outputs are strings or +null, not `{ "value": ... }` wrapper objects. Output-only fields such as `name`, +`state`, `error`, `schemaValidationJob`, and audit fields, as well as unknown +fields at any level, are rejected before authentication or HTTP. + +```bash +confidence data dimension-tables list --all --json +confidence data dimension-tables get dimensionTables/users --json +confidence data dimension-tables create --from-file dimensions.json --id users --dry-run --json +confidence data dimension-tables create --from-file dimensions.json --id users --json +``` + +`--dry-run` validates and previews requests without authentication or HTTP. +Create sends exactly one POST with the resource as its body and the optional ID +as the `dimensionTableId` query parameter. It does not request approval, retry, +or wait for schema validation. JSON output preserves the full returned resource, +including state, errors, and schema-validation job references. Get verifies that +the returned resource name matches the request. A successful get returns exit +status 0 even if the table is not ready; an HTTP, network, or input failure +returns nonzero. + +A create transport error, HTTP 5xx, or malformed successful response means the +**creation outcome is unknown**: the table may already exist. The diagnostic +preserves the original error, includes the requested `--id` when supplied, and +warns against automatic retries. Check `confidence data dimension-tables get ` +(or list existing tables if no ID was supplied) before deciding whether to retry. + ### SDK | Command | Description | diff --git a/packages/cli/__tests__/commands/dimension-tables.test.ts b/packages/cli/__tests__/commands/dimension-tables.test.ts new file mode 100644 index 00000000..8650fac6 --- /dev/null +++ b/packages/cli/__tests__/commands/dimension-tables.test.ts @@ -0,0 +1,804 @@ +import { writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { http, HttpResponse, server } from '@spotify-confidence/testing'; +import { prepareAuthTokens } from '@spotify-confidence/testing/auth'; +import { createProjectDir, writeJsonFile } from '@spotify-confidence/testing/scaffold'; +import { dataCommand } from '@commands/data.js'; +import { captureOutput } from '../helpers/capture.js'; +import { createRunner } from '../helpers/run-command.js'; + +const sut = createRunner(dataCommand); +const base = 'https://metrics.eu.confidence.dev/v1/dimensionTables'; +const resource = { + sql: 'SELECT user_id, country, event_time FROM user_dimensions', + displayName: 'User dimensions', + description: '', + entityColumnMapping: { column: { name: 'user_id' }, entity: 'entities/user' }, + dimensions: [{ name: 'country', repeated: false }], + timeConfig: { timestampColumn: { name: 'event_time' }, window: '86400s' }, + valueMappings: [ + { + column: 'country', + mappings: [ + { inputDimensionValue: 'SE', outputDimensionValue: 'Sweden' }, + { inputDimensionValue: null, outputDimensionValue: 'Unknown' }, + { inputDimensionValue: '', outputDimensionValue: null }, + {}, + ], + }, + ], + labels: { team: 'growth' }, + owner: 'identities/owner', + dimensionDataDeliveredUntilTime: '2026-07-01T00:00:00.123456789Z', + dataDeliveredUntilUpdateStrategyConfig: { + strategy: 'AUTOMATIC', + disableEmptyPartitions: true, + automaticUpdateConfig: { + commitDelay: '300s', + incrementDuration: '3600s', + changeThreshold: { value: '0.01' }, + }, + }, +}; + +function observeRequests(): string[] { + const requests: string[] = []; + server.use( + http.all('*', ({ request }) => { + requests.push(request.url); + return HttpResponse.json({ name: 'dimensionTables/unexpected' }); + }), + ); + return requests; +} + +describe('data dimension-tables create', () => { + it('writes exactly one resource body with camelCase fields and the transcoded ID query parameter', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', resource); + const requests: { body: unknown; query: string; authorization: string | null }[] = []; + const response = { + ...resource, + name: 'dimensionTables/users', + state: 'VALIDATING', + schemaValidationJob: 'sqlJobs/one', + }; + server.use( + http.post(base, async ({ request }) => { + requests.push({ + body: await request.json(), + query: new URL(request.url).search, + authorization: request.headers.get('authorization'), + }); + return HttpResponse.json(response); + }), + ); + + // Act + await sut([ + 'data', + 'dimension-tables', + 'create', + '--from-file', + file, + '--id', + 'users', + '--json', + ]); + + // Assert + expect(requests).toEqual([ + { + body: resource, + query: '?dimensionTableId=users', + authorization: expect.stringMatching(/^Bearer /u), + }, + ]); + expect(JSON.parse(output.stdout).data).toEqual(response); + expect(process.exitCode ?? 0).toBe(0); + }); + + it('omits the ID when the service should generate it', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', resource); + const queries: string[] = []; + server.use( + http.post(base, ({ request }) => { + queries.push(new URL(request.url).search); + return HttpResponse.json({ ...resource, name: 'dimensionTables/generated' }); + }), + ); + + // Act + await sut(['data', 'dimension-tables', 'create', '--from-file', file, '--json']); + + // Assert + expect(queries).toEqual(['']); + expect(JSON.parse(output.stdout).data.name).toBe('dimensionTables/generated'); + }); + + it('validates and previews the entire create request without authentication or HTTP', async () => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', resource); + const requests = observeRequests(); + + // Act + await sut([ + 'data', + 'dimension-tables', + 'create', + '--from-file', + file, + '--id', + 'users', + '--dry-run', + '--json', + ]); + + // Assert + expect(JSON.parse(output.stdout).data).toEqual({ + dryRun: true, + method: 'POST', + path: '/v1/dimensionTables', + params: { dimensionTableId: 'users' }, + body: resource, + }); + expect(output.stderr).toBe(''); + expect(requests).toEqual([]); + expect(process.exitCode ?? 0).toBe(0); + }); + + it.each([false, true])( + 'rejects invalid resource JSON before auth or HTTP (dry-run: %s)', + async (dryRun) => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', { ...resource, state: 'READY' }); + const requests = observeRequests(); + + // Act + await sut([ + 'data', + 'dimension-tables', + 'create', + '--from-file', + file, + ...(dryRun ? ['--dry-run'] : []), + ]); + + // Assert + expect(output.stderr).toContain('DimensionTable.state'); + expect(output.stderr).not.toContain('Not logged in'); + expect(output.stdout).toBe(''); + expect(requests).toEqual([]); + expect(process.exitCode).toBe(1); + }, + ); + + it.each([ + '../users', + 'dimensionTables/users', + 'users/child', + 'users?x=y', + 'users%2Fchild', + 'users#fragment', + ' users', + 'users\\escape', + ])('rejects invalid create ID %s before auth or HTTP', async (id) => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', resource); + const requests = observeRequests(); + + // Act + await sut(['data', 'dimension-tables', 'create', '--from-file', file, '--id', id, '--dry-run']); + + // Assert + expect(output.stderr).toContain('Resource name'); + expect(output.stderr).not.toContain('Not logged in'); + expect(requests).toEqual([]); + expect(process.exitCode).toBe(1); + }); + + it('reports malformed JSON before authentication', async () => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = join(project.path, 'broken.json'); + writeFileSync(file, '{'); + const requests = observeRequests(); + + // Act + await sut(['data', 'dimension-tables', 'create', '--from-file', file, '--dry-run']); + + // Assert + expect(output.stderr).toContain('Invalid JSON'); + expect(requests).toEqual([]); + expect(process.exitCode).toBe(1); + }); + + it('requires authentication for a valid non-preview create', async () => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', resource); + const requests = observeRequests(); + + // Act + await sut(['data', 'dimension-tables', 'create', '--from-file', file]); + + // Assert + expect(output.stderr).toContain('Not logged in'); + expect(requests).toEqual([]); + expect(process.exitCode).toBe(1); + }); + + it.each([400, 403, 409])('reports HTTP %s without retrying a create', async (status) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', resource); + let writes = 0; + server.use( + http.post(base, () => { + writes++; + return HttpResponse.json({ code: status, message: 'Creation refused' }, { status }); + }), + ); + + // Act + await sut(['data', 'dimension-tables', 'create', '--from-file', file, '--json']); + + // Assert + expect(writes).toBe(1); + expect(output.stderr).toContain('Creation refused'); + expect(output.stderr).not.toContain('outcome is unknown'); + expect(output.stdout).toBe(''); + expect(process.exitCode).toBe(1); + }); + + it.each([ + [500, undefined], + [502, 'users'], + [503, 'users'], + ])('reports HTTP %s as an unknown create outcome (ID: %s)', async (status, id) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', resource); + let writes = 0; + server.use( + http.post(base, () => { + writes++; + return HttpResponse.json({ code: status, message: 'Creation unavailable' }, { status }); + }), + ); + + // Act + await sut([ + 'data', + 'dimension-tables', + 'create', + '--from-file', + file, + ...(id === undefined ? [] : ['--id', id]), + '--json', + ]); + + // Assert + expect(writes).toBe(1); + expect(output.stderr).toContain('Creation unavailable'); + expect(output.stderr).toContain('outcome is unknown'); + expect(output.stderr).toContain('Do not automatically retry'); + expect(output.stderr).toContain( + id === undefined ? 'Check existing dimension tables' : 'Requested --id "users"', + ); + expect(output.stdout).toBe(''); + expect(process.exitCode).toBe(1); + }); + + it.each([ + { status: 200, body: '{', id: undefined, error: 'Invalid JSON response' }, + { status: 204, body: null, id: 'users', error: 'Invalid JSON response' }, + { status: 201, body: '{"state":"READY"}', id: 'users', error: 'response.name' }, + { status: 200, body: 'null', id: undefined, error: 'must be an object' }, + { + status: 200, + body: '{"name":"dimensionTables/other"}', + id: 'users', + error: 'does not match the requested resource', + }, + ])( + 'reports a malformed successful create as an unknown outcome: %j', + async ({ status, body, id, error }) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', resource); + let writes = 0; + server.use( + http.post(base, () => { + writes++; + return new HttpResponse(body, { status }); + }), + ); + + // Act + await sut([ + 'data', + 'dimension-tables', + 'create', + '--from-file', + file, + ...(id === undefined ? [] : ['--id', id]), + '--json', + ]); + + // Assert + expect(writes).toBe(1); + expect(output.stderr).toContain(error); + expect(output.stderr).toContain('outcome is unknown'); + expect(output.stderr).toContain('Do not automatically retry'); + expect(output.stderr).toContain( + id === undefined + ? 'Check existing dimension tables' + : 'confidence data dimension-tables get users', + ); + expect(output.stdout).toBe(''); + expect(process.exitCode).toBe(1); + }, + ); + + it.each([undefined, 'users'])( + 'does not retry a network failure and reports an unknown create outcome (ID: %s)', + async (id) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', resource); + let writes = 0; + server.use( + http.post(base, () => { + writes++; + return HttpResponse.error(); + }), + ); + + // Act + await sut([ + 'data', + 'dimension-tables', + 'create', + '--from-file', + file, + ...(id === undefined ? [] : ['--id', id]), + ]); + + // Assert + expect(writes).toBe(1); + expect(output.stderr).toContain('Network error'); + expect(output.stderr).toContain('outcome is unknown'); + expect(output.stderr).toContain('Do not automatically retry'); + expect(output.stderr).toContain( + id === undefined ? 'Check existing dimension tables' : 'Requested --id "users"', + ); + expect(output.stdout).toBe(''); + expect(process.exitCode).toBe(1); + }, + ); +}); + +describe('data dimension-tables get', () => { + it.each(['users', 'dimensionTables/users'])( + 'preserves the complete not-ready resource for %s with success exit status', + async (name) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const response = { + ...resource, + name: 'dimensionTables/users', + state: 'FAILED', + error: { message: 'Invalid SQL', details: [{ column: 'country' }] }, + schemaValidationJob: 'sqlJobs/failed', + creator: 'identities/creator', + createTime: '2026-07-01T00:00:00Z', + futureField: { preserved: true }, + }; + server.use(http.get(`${base}/users`, () => HttpResponse.json(response))); + + // Act + await sut(['data', 'dimension-tables', 'get', name, '--json']); + + // Assert + expect(JSON.parse(output.stdout).data).toEqual(response); + expect(output.stderr).toBe(''); + expect(process.exitCode ?? 0).toBe(0); + }, + ); + + it('prints nested state and errors in table output', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use( + http.get(`${base}/users`, () => + HttpResponse.json({ + name: 'dimensionTables/users', + state: 'VALIDATING', + error: { message: 'Schema pending' }, + }), + ), + ); + + await sut(['data', 'dimension-tables', 'get', 'users', '--output', 'table']); + + expect(output.stdout).toContain('VALIDATING'); + expect(output.stdout).toContain('Schema pending'); + expect(process.exitCode ?? 0).toBe(0); + }); + + it.each([ + '../users', + 'dimensionTables/a/b', + 'entities/users', + 'dimensionTables/%2e%2e', + 'users?x=y', + 'users#fragment', + 'users\\escape', + ])('rejects unsafe resource name %s before auth and HTTP', async (name) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = observeRequests(); + + await sut(['data', 'dimension-tables', 'get', name, '--dry-run']); + + expect(output.stderr).toContain('Resource name'); + expect(output.stderr).not.toContain('Not logged in'); + expect(requests).toEqual([]); + expect(process.exitCode).toBe(1); + }); + + it.each([null, [], { name: 'entities/wrong' }, { state: 'READY' }])( + 'reports malformed get response %j', + async (response) => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use(http.get(`${base}/users`, () => HttpResponse.json(response))); + + await sut(['data', 'dimension-tables', 'get', 'users', '--json']); + + expect(output.stdout).toBe(''); + expect(output.stderr).not.toBe(''); + expect(process.exitCode).toBe(1); + }, + ); + + it('rejects a valid resource with a different name than the requested table', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use( + http.get(`${base}/users`, () => + HttpResponse.json({ ...resource, name: 'dimensionTables/other' }), + ), + ); + + // Act + await sut(['data', 'dimension-tables', 'get', 'users', '--json']); + + // Assert + expect(output.stdout).toBe(''); + expect(output.stderr).toContain('does not match the requested resource'); + expect(process.exitCode).toBe(1); + }); + + it('reports a missing table', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use( + http.get(`${base}/missing`, () => + HttpResponse.json({ code: 404, message: 'Table not found' }, { status: 404 }), + ), + ); + + await sut(['data', 'dimension-tables', 'get', 'missing']); + + expect(output.stderr).toContain('Table not found'); + expect(process.exitCode).toBe(1); + }); +}); + +describe('data dimension-tables list', () => { + it('sends all list filters and preserves a single page with its continuation token', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const queries: Record[] = []; + const response = { + dimensionTables: [{ name: 'dimensionTables/users', state: 'VALIDATING' }], + nextPageToken: 'next+/=', + extra: { total: 20 }, + }; + server.use( + http.get(base, ({ request }) => { + queries.push(Object.fromEntries(new URL(request.url).searchParams)); + return HttpResponse.json(response); + }), + ); + + // Act + await sut([ + 'data', + 'dimension-tables', + 'list', + '--page-size', + '2', + '--page-token', + 'start+/=', + '--filter', + 'displayName:"User dimensions"', + '--order-by', + 'displayName:desc', + '--show-deleted', + '--json', + ]); + + // Assert + expect(queries).toEqual([ + { + pageSize: '2', + pageToken: 'start+/=', + filter: 'displayName:"User dimensions"', + orderBy: 'displayName:desc', + showDeleted: 'true', + }, + ]); + expect(JSON.parse(output.stdout).data).toEqual(response); + }); + + it('fetches all pages, retaining filters and treating continuation tokens only as query values', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const token = 'https://untrusted.example/next?token=abc'; + const queries: Record[] = []; + server.use( + http.get(base, ({ request }) => { + const params = Object.fromEntries(new URL(request.url).searchParams); + queries.push(params); + return HttpResponse.json( + params.pageToken === 'start' + ? { dimensionTables: [{ name: 'dimensionTables/one' }], nextPageToken: token } + : { dimensionTables: [{ name: 'dimensionTables/two' }], nextPageToken: '' }, + ); + }), + ); + + // Act + await sut([ + 'data', + 'dimension-tables', + 'list', + '--all', + '--page-token', + 'start', + '--page-size', + '1', + '--filter', + 'state:READY', + '--order-by', + 'displayName:asc', + '--show-deleted=false', + '--json', + ]); + + // Assert + expect(queries).toEqual( + ['start', token].map((pageToken) => ({ + pageToken, + pageSize: '1', + filter: 'state:READY', + orderBy: 'displayName:asc', + showDeleted: 'false', + })), + ); + expect(JSON.parse(output.stdout).data).toEqual({ + dimensionTables: [{ name: 'dimensionTables/one' }, { name: 'dimensionTables/two' }], + nextPageToken: '', + }); + }); + + it.each([{}, { dimensionTables: [] }])( + 'accepts an empty protobuf list response %j', + async (response) => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use(http.get(base, () => HttpResponse.json(response))); + + await sut(['data', 'dimension-tables', 'list', '--json']); + + expect(JSON.parse(output.stdout).data.dimensionTables).toEqual([]); + expect(process.exitCode ?? 0).toBe(0); + }, + ); + + it.each([false, true])( + 'stops repeated tokens including the initial token (initial token: %s)', + async (initialToken) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + let reads = 0; + server.use( + http.get(base, () => { + reads++; + return HttpResponse.json({ dimensionTables: [], nextPageToken: 'same' }); + }), + ); + + // Act + await sut([ + 'data', + 'dimension-tables', + 'list', + '--all', + ...(initialToken ? ['--page-token', 'same'] : []), + '--json', + ]); + + // Assert + expect(reads).toBe(initialToken ? 1 : 2); + expect(output.stderr).toContain('repeated a page token'); + expect(output.stdout).toBe(''); + expect(process.exitCode).toBe(1); + }, + ); + + it.each([ + null, + [], + { dimensionTables: {} }, + { dimensionTables: [null] }, + { dimensionTables: [{ name: 'entities/wrong' }] }, + { dimensionTables: [], nextPageToken: 123 }, + { dimensionTables: [], nextPageToken: {} }, + ])('rejects invalid list response %j', async (response) => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use(http.get(base, () => HttpResponse.json(response))); + + await sut(['data', 'dimension-tables', 'list', '--all', '--json']); + + expect(output.stdout).toBe(''); + expect(output.stderr).not.toBe(''); + expect(process.exitCode).toBe(1); + }); + + it.each([false, true])( + 'bounds unique pagination tokens at 1000 pages (last page is final: %s)', + async (lastPageIsFinal) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + let reads = 0; + server.use( + http.get(base, () => { + reads++; + return HttpResponse.json({ + dimensionTables: [{ name: `dimensionTables/table-${reads}` }], + nextPageToken: lastPageIsFinal && reads === 1000 ? '' : `cursor-${reads}`, + }); + }), + ); + + // Act + await sut(['data', 'dimension-tables', 'list', '--all', '--json']); + + // Assert + expect(reads).toBe(1000); + if (lastPageIsFinal) { + expect(JSON.parse(output.stdout).data.dimensionTables).toHaveLength(1000); + expect(output.stderr).toBe(''); + expect(process.exitCode ?? 0).toBe(0); + } else { + expect(output.stdout).toBe(''); + expect(output.stderr).toContain('exceeded the 1000-page limit'); + expect(output.stderr).toContain('No partial results returned'); + expect(process.exitCode).toBe(1); + } + }, + 30_000, + ); + + it('reports a later page error without returning incomplete success', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use( + http.get(base, ({ request }) => + new URL(request.url).searchParams.has('pageToken') + ? HttpResponse.json({ code: 403, message: 'No more access' }, { status: 403 }) + : HttpResponse.json({ + dimensionTables: [{ name: 'dimensionTables/one' }], + nextPageToken: 'two', + }), + ), + ); + + // Act + await sut(['data', 'dimension-tables', 'list', '--all', '--json']); + + // Assert + expect(output.stdout).toBe(''); + expect(output.stderr).toContain('No more access'); + expect(process.exitCode).toBe(1); + }); + + it.each(['0', '-1', '1.5', 'NaN', '2147483648'])( + 'rejects invalid page size %s before auth or HTTP', + async (size) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = observeRequests(); + + await sut(['data', 'dimension-tables', 'list', '--page-size', size, '--dry-run']); + + expect(output.stderr).toContain('--page-size'); + expect(output.stderr).not.toContain('Not logged in'); + expect(requests).toEqual([]); + expect(process.exitCode).toBe(1); + }, + ); +}); + +describe('data dimension-tables read boundaries', () => { + it.each([['list'], ['get', 'users']])( + 'requires authentication for %j without HTTP', + async (...action) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = observeRequests(); + + await sut(['data', 'dimension-tables', ...action, '--json']); + + expect(output.stderr).toContain('Not logged in'); + expect(requests).toEqual([]); + expect(process.exitCode).toBe(1); + }, + ); + + it.each([ + ['list', '--all'], + ['get', 'users'], + ])('previews %j without auth or HTTP', async (...action) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = observeRequests(); + + await sut(['data', 'dimension-tables', ...action, '--dry-run', '--json']); + + expect(JSON.parse(output.stdout).data).toMatchObject({ dryRun: true, method: 'GET' }); + expect(output.stderr).toBe(''); + expect(requests).toEqual([]); + expect(process.exitCode ?? 0).toBe(0); + }); +}); diff --git a/packages/cli/__tests__/features/data/dimension-input.test.ts b/packages/cli/__tests__/features/data/dimension-input.test.ts new file mode 100644 index 00000000..d7a6b2ec --- /dev/null +++ b/packages/cli/__tests__/features/data/dimension-input.test.ts @@ -0,0 +1,314 @@ +import { writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { createProjectDir, writeJsonFile } from '@spotify-confidence/testing/scaffold'; +import { readDimensionTable as sut } from '@features/data/dimension-input.js'; + +const valid = { + sql: 'SELECT id, country FROM users', + displayName: 'User dimensions', + entityColumnMapping: { column: { name: 'id' }, entity: 'entities/user' }, + dimensions: [{ name: 'country' }], +}; + +function readResource(body: unknown): Record { + using project = createProjectDir('empty'); + return sut(writeJsonFile(project.path, 'dimension.json', body)); +} + +describe('dimension table resource JSON input', () => { + it('reads canonical camelCase resource fields without converting them to CLI options', () => { + using project = createProjectDir('empty'); + const file = writeJsonFile(project.path, 'dimension.json', valid); + + const result = sut(file); + + expect(result).toEqual(valid); + expect(result).not.toHaveProperty('display-name'); + expect(result).not.toHaveProperty('entity-column-mapping'); + }); + + it.each([ + { description: '', owner: 'identities/owner', labels: { team: 'growth', empty: '' } }, + { description: '', owner: '', labels: {} }, + { + dimensions: [{ name: 'countries', repeated: true }], + entityColumnMapping: { column: { name: 'id', repeated: false }, entity: 'entities/user' }, + }, + { + timeConfig: { + timestampColumn: { name: 'time', repeated: false }, + window: '86400.123456789s', + }, + }, + { + valueMappings: [ + { + column: 'country', + mappings: [ + { inputDimensionValue: 'SE', outputDimensionValue: 'Sweden' }, + { outputDimensionValue: 'Unknown' }, + { inputDimensionValue: null, outputDimensionValue: null }, + { inputDimensionValue: '', outputDimensionValue: '' }, + {}, + ], + }, + ], + }, + { valueMappings: [] }, + { dimensionDataDeliveredUntilTime: '2026-01-01T12:30:00.123456789Z' }, + { dimensionDataDeliveredUntilTime: '2024-02-29T23:59:59.999999999+02:30' }, + { dimensionDataDeliveredUntilTime: '2000-02-29T00:00:00-01:00' }, + { dimensionDataDeliveredUntilTime: '0001-01-01T00:00:00Z' }, + { dimensionDataDeliveredUntilTime: '9999-12-31T23:59:59.999999999Z' }, + { dataDeliveredUntilUpdateStrategyConfig: { strategy: 0 } }, + { dataDeliveredUntilUpdateStrategyConfig: { strategy: 'STRATEGY_UNSPECIFIED' } }, + { + dataDeliveredUntilUpdateStrategyConfig: { strategy: 'MANUAL', disableEmptyPartitions: false }, + }, + { + dataDeliveredUntilUpdateStrategyConfig: { + strategy: 'AUTOMATIC', + automaticUpdateConfig: { + commitDelay: '300s', + incrementDuration: '3600s', + changeThreshold: { value: '0.01' }, + }, + }, + }, + { + dataDeliveredUntilUpdateStrategyConfig: { + strategy: 2, + dailyUpdateConfig: { maxZeroDuration: '86400s' }, + }, + }, + { + dataDeliveredUntilUpdateStrategyConfig: { + strategy: 'AUTOMATIC', + hourlyUpdateConfig: { maxZeroDuration: '3600s' }, + }, + }, + { dataDeliveredUntilUpdateStrategyConfig: { strategy: 1, dailyUpdateConfig: {} } }, + ...[{}, { value: '' }, { value: '0' }, { value: '-1.2e+3' }].map((changeThreshold) => ({ + dataDeliveredUntilUpdateStrategyConfig: { + strategy: 'AUTOMATIC', + automaticUpdateConfig: { commitDelay: '0s', changeThreshold }, + }, + })), + ...['315576000000.999999999s', '-315576000000.999999999s', '-0.000000001s'].map((window) => ({ + timeConfig: { timestampColumn: { name: 'time' }, window }, + })), + ])('preserves supported optional fields unchanged: %j', (fields) => { + const body = { ...valid, ...fields }; + + const result = readResource(body); + + expect(result).toEqual(body); + }); + + it.each(['sql', 'displayName', 'entityColumnMapping', 'dimensions'])( + 'rejects missing required %s', + (field) => { + const body: Record = { ...valid }; + delete body[field]; + + expect(() => readResource(body)).toThrow(field); + }, + ); + + it.each([null, [], 'text', 42, true])('rejects a non-object resource %j', (body) => { + expect(() => readResource(body)).toThrow('DimensionTable must be an object'); + }); + + it.each([ + ['sql', ''], + ['sql', ' '], + ['sql', 42], + ['displayName', null], + ['displayName', ''], + ['description', false], + ['owner', {}], + ['labels', []], + ['labels', null], + ['labels', { team: 42 }], + ['dimensions', []], + ['dimensions', 'country'], + ['dimensions', ['country']], + ['dimensions', [null]], + ['dimensions', [{ name: '' }]], + ['dimensions', [{ name: 'country', repeated: 'true' }]], + ['entityColumnMapping', {}], + ['entityColumnMapping', []], + ['entityColumnMapping', { column: 'id', entity: 'entities/user' }], + ['entityColumnMapping', { column: { name: 'id' } }], + ['entityColumnMapping', { column: { name: 'id' }, entity: 'user' }], + ['entityColumnMapping', { column: { name: 'id' }, entity: 'entities/a/b' }], + ['entityColumnMapping', { column: { name: 'id' }, entity: 'entities/../user' }], + ['entityColumnMapping', { column: { name: 'id' }, entity: 'entities/%2Fuser' }], + ['entityColumnMapping', { column: { name: 'id' }, entity: 'entities/user?other=value' }], + ['timeConfig', null], + ['timeConfig', {}], + ['timeConfig', { timestampColumn: 'time', window: '3600s' }], + ['timeConfig', { timestampColumn: { name: 'time' } }], + ['timeConfig', { timestampColumn: { name: 'time' }, window: { seconds: 3600 } }], + ['timeConfig', { timestampColumn: { name: 'time' }, window: '1h' }], + ['timeConfig', { timestampColumn: { name: 'time' }, window: '1.1234567890s' }], + ['timeConfig', { timestampColumn: { name: 'time' }, window: '315576000001s' }], + ['valueMappings', null], + ['valueMappings', {}], + ['valueMappings', [null]], + ['valueMappings', [{ column: 'country' }]], + ['valueMappings', [{ column: '', mappings: [] }]], + ['valueMappings', [{ column: 'country', mappings: {} }]], + ['valueMappings', [{ column: 'country', mappings: [null] }]], + ['valueMappings', [{ column: 'country', mappings: [{ inputDimensionValue: 1 }] }]], + ['valueMappings', [{ column: 'country', mappings: [{ outputDimensionValue: false }] }]], + [ + 'valueMappings', + [{ column: 'country', mappings: [{ inputDimensionValue: { value: 'SE' } }] }], + ], + ['dimensionDataDeliveredUntilTime', 42], + ['dimensionDataDeliveredUntilTime', 'not-a-time'], + ...[ + '2026-02-29T00:00:00Z', + '2026-02-30T00:00:00Z', + '1900-02-29T00:00:00Z', + '2026-04-31T00:00:00Z', + '2026-00-01T00:00:00Z', + '2026-13-01T00:00:00Z', + '2026-01-00T00:00:00Z', + '2026-01-01T24:00:00Z', + '2026-01-01T00:60:00Z', + '2026-01-01T00:00:60Z', + '2026-01-01T00:00:00+24:00', + '2026-01-01T00:00:00+00:60', + '2026-01-01T00:00:00.1234567890Z', + '0000-01-01T00:00:00Z', + '0001-01-01T00:00:00+00:01', + '9999-12-31T23:59:59-00:01', + ].map((value): [string, unknown] => ['dimensionDataDeliveredUntilTime', value]), + ['dataDeliveredUntilUpdateStrategyConfig', null], + ['dataDeliveredUntilUpdateStrategyConfig', {}], + ['dataDeliveredUntilUpdateStrategyConfig', { strategy: 'invented' }], + [ + 'dataDeliveredUntilUpdateStrategyConfig', + { strategy: 'AUTOMATIC', disableEmptyPartitions: 'true' }, + ], + [ + 'dataDeliveredUntilUpdateStrategyConfig', + { strategy: 'AUTOMATIC', automaticUpdateConfig: {} }, + ], + [ + 'dataDeliveredUntilUpdateStrategyConfig', + { strategy: 'AUTOMATIC', automaticUpdateConfig: { commitDelay: 300 } }, + ], + [ + 'dataDeliveredUntilUpdateStrategyConfig', + { + strategy: 'AUTOMATIC', + automaticUpdateConfig: { commitDelay: '300s', changeThreshold: '0.01' }, + }, + ], + [ + 'dataDeliveredUntilUpdateStrategyConfig', + { + strategy: 'AUTOMATIC', + automaticUpdateConfig: { commitDelay: '300s', changeThreshold: { value: 'NaN' } }, + }, + ], + [ + 'dataDeliveredUntilUpdateStrategyConfig', + { strategy: 'AUTOMATIC', dailyUpdateConfig: [], hourlyUpdateConfig: {} }, + ], + [ + 'dataDeliveredUntilUpdateStrategyConfig', + { strategy: 'AUTOMATIC', dailyUpdateConfig: { maxZeroDuration: 60 } }, + ], + ])('rejects invalid known field %s: %j', (field, value) => { + expect(() => readResource({ ...valid, [field as string]: value })).toThrow(); + }); + + it.each([ + 'name', + 'state', + 'error', + 'schemaValidationJob', + 'partitionedUpdateStrategyState', + 'creator', + 'updater', + 'createTime', + 'updateTime', + 'deleteTime', + 'systemCreated', + 'unknown', + 'display-name', + 'display_name', + 'dimensionTable', + 'dimensionTableId', + ])('rejects non-writable or unknown root field %s instead of silently discarding it', (field) => { + expect(() => readResource({ ...valid, [field]: 'unexpected' })).toThrow( + `DimensionTable.${field}`, + ); + }); + + it('rejects a request wrapper instead of reading a nested resource', () => { + expect(() => readResource({ dimensionTable: valid })).toThrow('DimensionTable.dimensionTable'); + }); + + it.each([ + { dimensions: [{ name: 'country', type: 'STRING' }] }, + { dimensions: [{ name: 'country', annotations: {} }] }, + { entityColumnMapping: { column: { name: 'id', type: 'STRING' }, entity: 'entities/user' } }, + { entityColumnMapping: { column: { name: 'id' }, entity: 'entities/user', other: true } }, + { timeConfig: { timestampColumn: { name: 'time', annotations: [] }, window: '86400s' } }, + { timeConfig: { timestampColumn: { name: 'time' }, window: '86400s', extra: true } }, + { valueMappings: [{ column: 'country', mappings: [], extra: true }] }, + { valueMappings: [{ column: 'country', mappings: [{ value: 'SE' }] }] }, + { dataDeliveredUntilUpdateStrategyConfig: { strategy: 'MANUAL', unknown: true } }, + { + dataDeliveredUntilUpdateStrategyConfig: { + strategy: 'AUTOMATIC', + automaticUpdateConfig: { commitDelay: '300s', unknown: true }, + }, + }, + { + dataDeliveredUntilUpdateStrategyConfig: { + strategy: 'AUTOMATIC', + hourlyUpdateConfig: { unknown: true }, + }, + }, + { + dataDeliveredUntilUpdateStrategyConfig: { + strategy: 'AUTOMATIC', + dailyUpdateConfig: { unknown: 1 }, + }, + }, + { + dataDeliveredUntilUpdateStrategyConfig: { + strategy: 'AUTOMATIC', + automaticUpdateConfig: { + commitDelay: '300s', + changeThreshold: { value: '0.1', unknown: 1 }, + }, + }, + }, + ])('rejects unknown or non-writable nested fields: %j', (fields) => { + expect(() => readResource({ ...valid, ...fields })).toThrow('Unknown or non-writable field'); + }); + + it('reports an unreadable file', () => { + using project = createProjectDir('empty'); + expect(() => sut(join(project.path, 'missing.json'))).toThrow('Could not read file'); + }); + + it('reports malformed JSON', () => { + using project = createProjectDir('empty'); + const file = join(project.path, 'broken.json'); + writeFileSync(file, '{'); + + expect(() => sut(file)).toThrow('Invalid JSON'); + }); + + it.each([undefined, null, 123, ''])('requires a file path: %j', (path) => { + expect(() => sut(path)).toThrow('--from-file must be a non-empty string'); + }); +}); diff --git a/packages/cli/src/commands/data.ts b/packages/cli/src/commands/data.ts index ec20479f..e4cbd754 100644 --- a/packages/cli/src/commands/data.ts +++ b/packages/cli/src/commands/data.ts @@ -1,13 +1,14 @@ import type { Argv } from 'yargs'; import { noop } from '@spotify-confidence/shared-kernel'; +import { dimensionTablesCommand } from './data/dimension-tables.js'; -/** Feature PRs register their subcommands in this group. */ export const dataCommand = { command: 'data', describe: 'Work with Confidence data resources', builder(yargs: Argv) { return yargs .usage('$0 data [options]') + .command(dimensionTablesCommand) .demandCommand(1, 'Run "confidence data --help" to see available commands.') .strict() .help(); diff --git a/packages/cli/src/commands/data/dimension-tables.ts b/packages/cli/src/commands/data/dimension-tables.ts new file mode 100644 index 00000000..8b6fb172 --- /dev/null +++ b/packages/cli/src/commands/data/dimension-tables.ts @@ -0,0 +1,62 @@ +import type { Argv } from 'yargs'; +import { noop } from '@spotify-confidence/shared-kernel'; +import { + createDimensionTableCmd, + getDimensionTableCmd, + listDimensionTablesCmd, +} from '@features/data/index.js'; +import { safely } from '@utils/index.js'; + +export const dimensionTablesCommand = { + command: 'dimension-tables', + describe: 'List, inspect, and create dimension tables', + builder(yargs: Argv) { + return yargs + .command( + 'list', + 'List dimension tables', + (y: Argv) => + y + .option('page-size', { type: 'number', describe: 'Maximum number of tables per page' }) + .option('page-token', { type: 'string', describe: 'Token for the next page' }) + .option('filter', { type: 'string', describe: 'Lucene query string filter' }) + .option('order-by', { type: 'string', describe: 'Sort fields, e.g. displayName:asc' }) + .option('show-deleted', { + type: 'boolean', + describe: 'Include deleted dimension tables', + }) + .option('all', { type: 'boolean', describe: 'Fetch all pages' }), + safely(listDimensionTablesCmd), + ) + .command( + 'get [name]', + 'Get a dimension table, including its current state and errors', + (y: Argv) => + y + .positional('name', { type: 'string', describe: 'ID or dimensionTables/' }) + .option('name', { + type: 'string', + demandOption: true, + describe: 'ID or dimensionTables/ (or supply it positionally)', + }), + safely(getDimensionTableCmd), + ) + .command( + 'create', + 'Create one dimension table from a canonical resource JSON file', + (y: Argv) => + y + .option('from-file', { + type: 'string', + demandOption: true, + describe: 'DimensionTable resource JSON file (not a request wrapper)', + }) + .option('id', { type: 'string', describe: 'Optional ID for the new dimension table' }), + safely(createDimensionTableCmd), + ) + .demandCommand(1, 'Available actions: list, get, create') + .strict() + .help(); + }, + handler: noop, +}; diff --git a/packages/cli/src/features/data/dimension-input.ts b/packages/cli/src/features/data/dimension-input.ts new file mode 100644 index 00000000..af4d4d03 --- /dev/null +++ b/packages/cli/src/features/data/dimension-input.ts @@ -0,0 +1,273 @@ +import { readFileSync } from 'node:fs'; +import { requireRecord, requireString, resourcePath } from '@network/metrics-types.js'; + +function writableFields(value: unknown, fields: string[], label: string): Record { + const record = requireRecord(value, label); + for (const field of Object.keys(record)) { + if (!fields.includes(field)) + throw new Error(`Unknown or non-writable field ${label}.${field}.`); + } + return record; +} + +function optionalString(value: unknown, label: string): void { + if (value !== undefined && typeof value !== 'string') + throw new Error(`${label} must be a string.`); +} + +function optionalBoolean(value: unknown, label: string): void { + if (value !== undefined && typeof value !== 'boolean') + throw new Error(`${label} must be a boolean.`); +} + +function requireArray(value: unknown, label: string): unknown[] { + if (!Array.isArray(value)) throw new Error(`${label} must be an array.`); + return value; +} + +function column(value: unknown, label: string): void { + const record = writableFields(value, ['name', 'repeated'], label); + requireString(record.name, `${label}.name`); + optionalBoolean(record.repeated, `${label}.repeated`); +} + +function duration(value: unknown, label: string): void { + const text = requireString(value, label); + const match = /^-?(0|[1-9]\d*)(?:\.\d{1,9})?s$/u.exec(text); + // The protobuf range applies to the seconds component; nanos are checked by the pattern. + if (match === null || Number(match[1]) > 315576000000) { + throw new Error(`${label} must be a protobuf duration string, for example "86400s".`); + } +} + +function timestamp(value: unknown, label: string): void { + const text = requireString(value, label); + const match = + /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})(?:\.\d{1,9})?(Z|[+-]\d{2}:\d{2})$/u.exec( + text, + ); + if (match === null) throw new Error(`${label} must be an RFC 3339 timestamp.`); + + const [, yearText, monthText, dayText, hourText, minuteText, secondText, zone] = match; + const [year, month, day, hour, minute, second] = [ + yearText, + monthText, + dayText, + hourText, + minuteText, + secondText, + ].map(Number); + const leapYear = year % 4 === 0 && (year % 100 !== 0 || year % 400 === 0); + const daysInMonth = + [31, leapYear ? 29 : 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31][month - 1] ?? 0; + const parsed = Date.parse(text); + // Date.parse normalizes invalid dates such as February 30 and accepts 24:00:00. + if ( + year < 1 || + day < 1 || + day > daysInMonth || + hour > 23 || + minute > 59 || + second > 59 || + (zone !== 'Z' && (Number(zone.slice(1, 3)) > 23 || Number(zone.slice(4, 6)) > 59)) || + !Number.isFinite(parsed) || + parsed < Date.parse('0001-01-01T00:00:00Z') || + parsed > Date.parse('9999-12-31T23:59:59.999Z') + ) { + throw new Error(`${label} must be an RFC 3339 timestamp within years 0001–9999.`); + } +} + +function updateStrategy(value: unknown): void { + const label = 'DimensionTable.dataDeliveredUntilUpdateStrategyConfig'; + const record = writableFields( + value, + [ + 'strategy', + 'disableEmptyPartitions', + 'automaticUpdateConfig', + 'dailyUpdateConfig', + 'hourlyUpdateConfig', + ], + label, + ); + if ( + ![0, 1, 2, 'STRATEGY_UNSPECIFIED', 'MANUAL', 'AUTOMATIC'].includes( + record.strategy as string | number, + ) + ) { + throw new Error( + `${label}.strategy must be STRATEGY_UNSPECIFIED, MANUAL, or AUTOMATIC (or 0, 1, 2).`, + ); + } + optionalBoolean(record.disableEmptyPartitions, `${label}.disableEmptyPartitions`); + const configs = ['automaticUpdateConfig', 'dailyUpdateConfig', 'hourlyUpdateConfig']; + if (configs.filter((key) => record[key] !== undefined).length > 1) { + throw new Error(`${label} accepts only one update config.`); + } + if (record.automaticUpdateConfig !== undefined) { + const configLabel = `${label}.automaticUpdateConfig`; + const config = writableFields( + record.automaticUpdateConfig, + ['incrementDuration', 'commitDelay', 'changeThreshold'], + configLabel, + ); + duration(config.commitDelay, `${configLabel}.commitDelay`); + if (config.incrementDuration !== undefined) + duration(config.incrementDuration, `${configLabel}.incrementDuration`); + if (config.changeThreshold !== undefined) { + const threshold = writableFields( + config.changeThreshold, + ['value'], + `${configLabel}.changeThreshold`, + ); + const value = threshold.value; + optionalString(value, `${configLabel}.changeThreshold.value`); + // Decimal's omitted or empty value represents zero; preserve it just like other defaults. + if ( + typeof value === 'string' && + value !== '' && + !/^[+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?$/u.test(value) + ) { + throw new Error(`${configLabel}.changeThreshold.value must be a decimal string.`); + } + } + } + for (const key of ['dailyUpdateConfig', 'hourlyUpdateConfig']) { + if (record[key] === undefined) continue; + const config = writableFields(record[key], ['maxZeroDuration'], `${label}.${key}`); + if (config.maxZeroDuration !== undefined) + duration(config.maxZeroDuration, `${label}.${key}.maxZeroDuration`); + } +} + +/** Validate canonical proto JSON without normalizing keys or dropping any input fields. */ +export function validateDimensionTable(value: unknown): Record { + const label = 'DimensionTable'; + const record = writableFields( + value, + [ + 'sql', + 'displayName', + 'description', + 'entityColumnMapping', + 'timeConfig', + 'dimensions', + 'valueMappings', + 'dimensionDataDeliveredUntilTime', + 'dataDeliveredUntilUpdateStrategyConfig', + 'labels', + 'owner', + ], + label, + ); + requireString(record.sql, `${label}.sql`); + requireString(record.displayName, `${label}.displayName`); + optionalString(record.description, `${label}.description`); + optionalString(record.owner, `${label}.owner`); + + const mapping = writableFields( + record.entityColumnMapping, + ['column', 'entity'], + `${label}.entityColumnMapping`, + ); + column(mapping.column, `${label}.entityColumnMapping.column`); + resourcePath(requireString(mapping.entity, `${label}.entityColumnMapping.entity`), 'entities'); + const dimensions = requireArray(record.dimensions, `${label}.dimensions`); + if (dimensions.length === 0) throw new Error(`${label}.dimensions must not be empty.`); + dimensions.forEach((value, index) => column(value, `${label}.dimensions[${index}]`)); + + if (record.timeConfig !== undefined) { + const config = writableFields( + record.timeConfig, + ['timestampColumn', 'window'], + `${label}.timeConfig`, + ); + column(config.timestampColumn, `${label}.timeConfig.timestampColumn`); + duration(config.window, `${label}.timeConfig.window`); + } + if (record.valueMappings !== undefined) { + requireArray(record.valueMappings, `${label}.valueMappings`).forEach((value, index) => { + const mappingLabel = `${label}.valueMappings[${index}]`; + const mapping = writableFields(value, ['column', 'mappings'], mappingLabel); + requireString(mapping.column, `${mappingLabel}.column`); + requireArray(mapping.mappings, `${mappingLabel}.mappings`).forEach((value, index) => { + const valueLabel = `${mappingLabel}.mappings[${index}]`; + const entry = writableFields( + value, + ['inputDimensionValue', 'outputDimensionValue'], + valueLabel, + ); + for (const key of ['inputDimensionValue', 'outputDimensionValue']) { + // StringValue uses strings (or null for absence), not { value: ... } in proto JSON. + if (entry[key] !== null) optionalString(entry[key], `${valueLabel}.${key}`); + } + }); + }); + } + if (record.labels !== undefined) { + for (const [key, value] of Object.entries(requireRecord(record.labels, `${label}.labels`))) { + if (typeof value !== 'string') throw new Error(`${label}.labels.${key} must be a string.`); + } + } + if (record.dimensionDataDeliveredUntilTime !== undefined) { + timestamp(record.dimensionDataDeliveredUntilTime, `${label}.dimensionDataDeliveredUntilTime`); + } + if (record.dataDeliveredUntilUpdateStrategyConfig !== undefined) + updateStrategy(record.dataDeliveredUntilUpdateStrategyConfig); + return record; +} + +export function readDimensionTable(file: unknown): Record { + const path = requireString(file, '--from-file'); + let contents: string; + try { + contents = readFileSync(path, 'utf8'); + } catch (cause) { + throw new Error(`Could not read file "${path}".`, { cause }); + } + let value: unknown; + try { + value = JSON.parse(contents); + } catch (cause) { + throw new Error(`Invalid JSON in "${path}".`, { cause }); + } + return validateDimensionTable(value); +} + +export function dimensionTablePath(value: unknown): string { + const name = requireString(value, 'Dimension table name'); + return resourcePath(name.includes('/') ? name : `dimensionTables/${name}`, 'dimensionTables'); +} + +export function dimensionTableId(value: unknown): string | undefined { + if (value === undefined) return undefined; + const id = requireString(value, '--id'); + resourcePath(`dimensionTables/${id}`, 'dimensionTables'); + return id; +} + +export function dimensionTableListOptions(argv: Record): { + params: Record; + all: boolean; +} { + const params: Record = {}; + if (argv['page-size'] !== undefined) { + const size = argv['page-size']; + if (typeof size !== 'number' || !Number.isInteger(size) || size < 1 || size > 2147483647) { + throw new Error('--page-size must be a positive 32-bit integer.'); + } + params.pageSize = String(size); + } + for (const [flag, field] of [ + ['page-token', 'pageToken'], + ['filter', 'filter'], + ['order-by', 'orderBy'], + ] as const) { + if (argv[flag] !== undefined) params[field] = requireString(argv[flag], `--${flag}`); + } + optionalBoolean(argv['show-deleted'], '--show-deleted'); + if (argv['show-deleted'] !== undefined) params.showDeleted = String(argv['show-deleted']); + optionalBoolean(argv.all, '--all'); + return { params, all: argv.all === true }; +} diff --git a/packages/cli/src/features/data/dimension-tables.ts b/packages/cli/src/features/data/dimension-tables.ts new file mode 100644 index 00000000..c8da47fc --- /dev/null +++ b/packages/cli/src/features/data/dimension-tables.ts @@ -0,0 +1,45 @@ +import { + createDimensionTable, + DIMENSION_TABLES_PATH, + getDimensionTable, + listDimensionTables, +} from '@network/metrics-dimension-tables.js'; +import { + dimensionTableId, + dimensionTableListOptions, + dimensionTablePath, + readDimensionTable, +} from './dimension-input.js'; +import { executeDataRequest } from './request.js'; + +export async function listDimensionTablesCmd(argv: Record): Promise { + const options = dimensionTableListOptions(argv); + await executeDataRequest(argv, (token) => listDimensionTables(token, options), { + dryRun: true, + method: 'GET', + path: DIMENSION_TABLES_PATH, + params: options.params, + all: options.all, + }); +} + +export async function getDimensionTableCmd(argv: Record): Promise { + const path = dimensionTablePath(argv.name); + await executeDataRequest(argv, (token) => getDimensionTable(token, path), { + dryRun: true, + method: 'GET', + path, + }); +} + +export async function createDimensionTableCmd(argv: Record): Promise { + const id = dimensionTableId(argv.id); + const body = readDimensionTable(argv['from-file']); + await executeDataRequest(argv, (token) => createDimensionTable(token, { body, id }), { + dryRun: true, + method: 'POST', + path: DIMENSION_TABLES_PATH, + params: id === undefined ? {} : { dimensionTableId: id }, + body, + }); +} diff --git a/packages/cli/src/features/data/index.ts b/packages/cli/src/features/data/index.ts index a4c9385e..c0ea904c 100644 --- a/packages/cli/src/features/data/index.ts +++ b/packages/cli/src/features/data/index.ts @@ -1,2 +1,7 @@ +export { + createDimensionTableCmd, + getDimensionTableCmd, + listDimensionTablesCmd, +} from './dimension-tables.js'; export { printData } from './output.js'; export { executeDataRequest } from './request.js'; diff --git a/packages/cli/src/network/metrics-dimension-tables.ts b/packages/cli/src/network/metrics-dimension-tables.ts new file mode 100644 index 00000000..343a3977 --- /dev/null +++ b/packages/cli/src/network/metrics-dimension-tables.ts @@ -0,0 +1,120 @@ +import type { ApiFailure, ApiResponse } from '@spotify-confidence/core'; +import { metricsRequest } from './metrics.js'; +import { requireRecord, requireString, resourcePath } from './metrics-types.js'; + +export const DIMENSION_TABLES_PATH = '/v1/dimensionTables'; +const MAX_DIMENSION_TABLE_PAGES = 1000; + +type Resource = Record; +type DimensionTablePage = Resource & { dimensionTables: Resource[]; nextPageToken?: string }; + +function dimensionTable(value: unknown, expectedPath?: string): Resource { + const resource = requireRecord(value, 'Dimension table response'); + const path = resourcePath( + requireString(resource.name, 'Dimension table response.name'), + 'dimensionTables', + ); + if (expectedPath !== undefined && path !== expectedPath) { + throw new Error('Dimension table response.name does not match the requested resource.'); + } + return resource; +} + +function dimensionTablePage(value: unknown): DimensionTablePage { + const page = requireRecord(value, 'Dimension table list response'); + // Protobuf JSON can omit an empty repeated field, but not replace it with null. + const tables = page.dimensionTables === undefined ? [] : page.dimensionTables; + if (!Array.isArray(tables)) { + throw new Error('Dimension table list response.dimensionTables must be an array.'); + } + if (page.nextPageToken !== undefined && typeof page.nextPageToken !== 'string') { + throw new Error('Dimension table list response.nextPageToken must be a string.'); + } + return { ...page, dimensionTables: tables.map((table) => dimensionTable(table)) }; +} + +export async function listDimensionTables( + token: string, + options: { params: Record; all: boolean }, +): Promise> { + const params = { ...options.params }; + const seen = new Set(); + if (params.pageToken) seen.add(params.pageToken); + const tables: Resource[] = []; + for (let pageCount = 0; pageCount < MAX_DIMENSION_TABLE_PAGES; pageCount++) { + const result = await metricsRequest(token, { path: DIMENSION_TABLES_PATH, params }); + if (!result.ok) return result; + const page = dimensionTablePage(result.data); + if (!options.all) return { ok: true, data: page }; + tables.push(...page.dimensionTables); + if (!page.nextPageToken) { + return { ok: true, data: { ...page, dimensionTables: tables, nextPageToken: '' } }; + } + if (seen.has(page.nextPageToken)) + throw new Error('Dimension table pagination repeated a page token.'); + seen.add(page.nextPageToken); + params.pageToken = page.nextPageToken; + } + throw new Error( + `Dimension table pagination exceeded the ${MAX_DIMENSION_TABLE_PAGES}-page limit. ` + + 'No partial results returned; narrow the filter or use single-page requests.', + ); +} + +export async function getDimensionTable( + token: string, + path: string, +): Promise> { + const result = await metricsRequest(token, { path }); + return result.ok ? { ok: true, data: dimensionTable(result.data, path) } : result; +} + +function unknownCreateOutcome(failure: ApiFailure, id?: string): ApiFailure { + const guidance = + id === undefined + ? 'Check existing dimension tables before retrying.' + : `Requested --id "${id}"; check with confidence data dimension-tables get ${id} before retrying.`; + return { + ...failure, + error: { + ...failure.error, + message: + `${failure.error.message} Creation outcome is unknown: the dimension table may have been created. ` + + `Do not automatically retry. ${guidance}`, + }, + }; +} + +export async function createDimensionTable( + token: string, + options: { body: Resource; id?: string }, +): Promise> { + const result = await metricsRequest(token, { + method: 'POST', + path: DIMENSION_TABLES_PATH, + body: options.body, + params: options.id === undefined ? undefined : { dimensionTableId: options.id }, + }); + if (!result.ok) { + const unknownOutcome = + result.status === 0 || result.status >= 500 || (result.status >= 200 && result.status < 300); + return unknownOutcome ? unknownCreateOutcome(result, options.id) : result; + } + try { + const expectedPath = + options.id === undefined ? undefined : `${DIMENSION_TABLES_PATH}/${options.id}`; + return { ok: true, data: dimensionTable(result.data, expectedPath) }; + } catch (err) { + return unknownCreateOutcome( + { + ok: false, + status: 0, + error: { + code: 0, + message: err instanceof Error ? err.message : 'Invalid dimension table response.', + }, + }, + options.id, + ); + } +}