-
Notifications
You must be signed in to change notification settings - Fork 12
feat(agent-bff): expose collection list and count with flat mapping #1742
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
843b80c
790611e
70d7b92
ab2f592
d7320f5
0d226e2
2d14997
d9c89e7
0661fa3
9ba5322
d22a4c2
9f72423
2b0ec1a
f962a17
aefa0cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import type { Logger } from '../ports/logger-port'; | ||
| import type { MetricTags, Metrics } from '../ports/metrics-port'; | ||
|
|
||
| import createConsoleLogger from './console-logger'; | ||
|
|
||
| export default function createConsoleMetrics(logger: Logger = createConsoleLogger()): Metrics { | ||
| return { | ||
| increment(name: string, tags?: MetricTags): void { | ||
| logger('Info', 'metric.increment', { metric: name, ...(tags ?? {}) }); | ||
| }, | ||
| gauge(name: string, value: number, tags?: MetricTags): void { | ||
| logger('Info', 'metric.gauge', { metric: name, value, ...(tags ?? {}) }); | ||
| }, | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import { HttpRequester } from '@forestadmin/agent-client'; | ||
|
|
||
| export interface AgentDataClientOptions { | ||
| agentUrl: string; | ||
| token: string; | ||
| } | ||
|
|
||
| export interface AgentDataClient { | ||
| list(collection: string, query: Record<string, unknown>): Promise<Record<string, unknown>[]>; | ||
| countRaw(collection: string, query: Record<string, unknown>): Promise<unknown>; | ||
| } | ||
|
|
||
| /** | ||
| * Thin data client bound to a request's agent token. Uses the raw `HttpRequester` (not `Collection`) | ||
| * so the BFF controls the query params — notably the resolved `timezone` — and can read the count | ||
| * endpoint's raw payload, which `collection.count()` coerces through `Number()` and loses. | ||
| */ | ||
| export default function createAgentDataClient({ | ||
| agentUrl, | ||
| token, | ||
| }: AgentDataClientOptions): AgentDataClient { | ||
| const requester = new HttpRequester(token, { url: agentUrl }); | ||
|
|
||
| return { | ||
| list: (collection, query) => | ||
| requester.query({ method: 'get', path: `/forest/${collection}`, query }), | ||
| countRaw: (collection, query) => | ||
| requester.query({ | ||
| method: 'get', | ||
| path: `/forest/${collection}/count`, | ||
| query, | ||
| skipDeserialization: true, | ||
| }), | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import { invalidRequest } from '../http/bff-local-errors'; | ||
|
|
||
| export interface BffSortClause { | ||
| field: string; | ||
| direction?: 'asc' | 'desc'; | ||
| } | ||
|
|
||
| export interface BffPage { | ||
| limit: number; | ||
| offset: number; | ||
| } | ||
|
|
||
| export interface ListRequestBody { | ||
| filter?: unknown; | ||
| projection?: string[]; | ||
| sort?: BffSortClause[]; | ||
| page?: BffPage; | ||
| } | ||
|
|
||
| export interface CountRequestBody { | ||
| filter?: unknown; | ||
| } | ||
|
|
||
| export type AgentQuery = Record<string, unknown> & { timezone: string }; | ||
|
|
||
| interface ConditionTreeBranch { | ||
| aggregator: string; | ||
| conditions: unknown[]; | ||
| } | ||
|
|
||
| interface ConditionTreeLeaf { | ||
| field: string; | ||
| } | ||
|
|
||
| function isBranch(node: unknown): node is ConditionTreeBranch { | ||
| return ( | ||
| typeof node === 'object' && | ||
| node !== null && | ||
| Array.isArray((node as { conditions?: unknown }).conditions) | ||
| ); | ||
| } | ||
|
|
||
| function isLeaf(node: unknown): node is ConditionTreeLeaf { | ||
| return ( | ||
| typeof node === 'object' && | ||
| node !== null && | ||
| typeof (node as { field?: unknown }).field === 'string' | ||
| ); | ||
| } | ||
|
|
||
| function collectFilterFields(filter: unknown, acc: string[]): void { | ||
| if (isBranch(filter)) { | ||
| filter.conditions.forEach(condition => collectFilterFields(condition, acc)); | ||
| } else if (isLeaf(filter)) { | ||
| acc.push(filter.field); | ||
| } | ||
| } | ||
|
|
||
| function serializeSort(sort: BffSortClause[]): string { | ||
| return sort.map(({ field, direction }) => (direction === 'desc' ? `-${field}` : field)).join(','); | ||
| } | ||
|
|
||
| function serializePage(page: BffPage): Record<string, number> { | ||
| const { limit, offset } = page; | ||
|
|
||
| if (!Number.isInteger(limit) || limit <= 0) { | ||
| throw invalidRequest(`Invalid page.limit: ${limit}`); | ||
| } | ||
|
|
||
| if (!Number.isInteger(offset) || offset < 0) { | ||
| throw invalidRequest(`Invalid page.offset: ${offset}`); | ||
| } | ||
|
|
||
| // The agent paginates by page number/size, so an arbitrary offset that is not a whole multiple | ||
| // of the limit cannot be expressed. Reject it rather than silently return a shifted window. | ||
| if (offset % limit !== 0) { | ||
| throw invalidRequest(`page.offset (${offset}) must be a multiple of page.limit (${limit})`); | ||
| } | ||
|
|
||
| return { 'page[size]': limit, 'page[number]': offset / limit + 1 }; | ||
| } | ||
|
|
||
| export function buildListAgentQuery( | ||
| collection: string, | ||
| timezone: string, | ||
| body: ListRequestBody, | ||
| ): AgentQuery { | ||
| const query: AgentQuery = { timezone }; | ||
|
|
||
| if (body.filter !== undefined) query.filters = JSON.stringify(body.filter); | ||
| if (body.projection?.length) query[`fields[${collection}]`] = body.projection.join(','); | ||
| if (body.sort?.length) query.sort = serializeSort(body.sort); | ||
| if (body.page) Object.assign(query, serializePage(body.page)); | ||
|
|
||
| return query; | ||
| } | ||
|
|
||
| export function buildCountAgentQuery(timezone: string, body: CountRequestBody): AgentQuery { | ||
| const query: AgentQuery = { timezone }; | ||
|
|
||
| if (body.filter !== undefined) query.filters = JSON.stringify(body.filter); | ||
|
|
||
| return query; | ||
| } | ||
|
|
||
| export function collectListFieldPaths(body: ListRequestBody): string[] { | ||
| const paths: string[] = [...(body.projection ?? [])]; | ||
| collectFilterFields(body.filter, paths); | ||
| (body.sort ?? []).forEach(({ field }) => paths.push(field)); | ||
|
|
||
| return paths; | ||
| } | ||
|
|
||
| export function collectCountFieldPaths(body: CountRequestBody): string[] { | ||
| const paths: string[] = []; | ||
| collectFilterFields(body.filter, paths); | ||
|
|
||
| return paths; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import type { AgentDataClient, AgentDataClientOptions } from './agent-data-client'; | ||
| import type { CountRequestBody, ListRequestBody } from './agent-query'; | ||
| import type { Logger } from '../ports/logger-port'; | ||
| import type ReadModelStore from '../read-model/read-model-store'; | ||
| import type { Middleware } from 'koa'; | ||
|
|
||
| import defaultCreateAgentDataClient from './agent-data-client'; | ||
| import { | ||
| buildCountAgentQuery, | ||
| buildListAgentQuery, | ||
| collectCountFieldPaths, | ||
| collectListFieldPaths, | ||
| } from './agent-query'; | ||
| import { mapCountResponse, mapListResponse } from './response-mappers'; | ||
| import { mapAgentError } from '../http/agent-error-mapper'; | ||
| import { schemaUnavailable, unknownCollection } from '../http/bff-local-errors'; | ||
| import SchemaUnavailableError from '../read-model/errors'; | ||
| import assertNoRelationFieldPaths from '../validation/relation-field-guard'; | ||
|
|
||
| const DATA_ROUTE = /^\/agent\/v1\/([^/]+)\/(list|count)$/; | ||
|
|
||
| export interface DataRoutesMiddlewareOptions { | ||
| store: ReadModelStore; | ||
| agentUrl: string; | ||
| logger: Logger; | ||
| createClient?: (options: AgentDataClientOptions) => AgentDataClient; | ||
| } | ||
|
|
||
| async function resolveReadModel(store: ReadModelStore) { | ||
| try { | ||
| return await store.getReadModel(); | ||
| } catch (error) { | ||
| if (error instanceof SchemaUnavailableError) throw schemaUnavailable(); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| export default function createDataRoutesMiddleware({ | ||
| store, | ||
| agentUrl, | ||
| logger, | ||
| createClient = defaultCreateAgentDataClient, | ||
| }: DataRoutesMiddlewareOptions): Middleware { | ||
| return async function dataRoutesMiddleware(ctx, next) { | ||
| const match = DATA_ROUTE.exec(ctx.path); | ||
|
|
||
| if (!match || ctx.method !== 'POST') { | ||
| await next(); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const collection = decodeURIComponent(match[1]); | ||
| const operation = match[2] as 'list' | 'count'; | ||
|
|
||
| const readModel = await resolveReadModel(store); | ||
|
|
||
| if (!readModel.isCollectionAllowed(collection)) { | ||
| // TODO(PRD-671): the read-model exposes a single allow-list (the exposed collections), so it | ||
| // cannot tell an unknown collection from a known-but-disallowed one. Every absent name maps | ||
| // to 404 here; `collection_not_allowed` (403) has no local trigger until a distinct exposure | ||
| // source exists. Escalated on the ticket — revisit when Anthony answers. | ||
| throw unknownCollection(`Unknown collection: ${collection}`); | ||
| } | ||
|
|
||
| const timezone = ctx.state.timezone as string; | ||
| const token = ctx.state.agentToken as string; | ||
| const client = createClient({ agentUrl, token }); | ||
| const body = (ctx.request.body ?? {}) as ListRequestBody & CountRequestBody; | ||
|
|
||
| if (operation === 'list') { | ||
| assertNoRelationFieldPaths(collectListFieldPaths(body)); | ||
|
|
||
| const query = buildListAgentQuery(collection, timezone, body); | ||
| let records: Record<string, unknown>[]; | ||
|
|
||
| try { | ||
| records = await client.list(collection, query); | ||
| } catch (error) { | ||
| throw mapAgentError(error, { logger }); | ||
| } | ||
|
|
||
| ctx.status = 200; | ||
| ctx.body = mapListResponse(collection, records, readModel.getPrimaryKeys(collection)); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| assertNoRelationFieldPaths(collectCountFieldPaths(body)); | ||
|
|
||
| const query = buildCountAgentQuery(timezone, body); | ||
| let raw: unknown; | ||
|
|
||
| try { | ||
| raw = await client.countRaw(collection, query); | ||
| } catch (error) { | ||
| throw mapAgentError(error, { logger }); | ||
| } | ||
|
|
||
| ctx.status = 200; | ||
| ctx.body = mapCountResponse(raw); | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||||||||||||||||||
| import type { PrimaryKeyField } from '../read-model/read-model'; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import { mappingError } from '../http/bff-local-errors'; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const PACKED_ID_SEPARATOR = '|'; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Rebuild the structured primary key of a record from its opaque packed id, mirroring the agent's | ||||||||||||||||||||||||||||||||||||||
| * `IdUtils.packId`/`unpackId` (`|`-joined values, `Number` columns cast back to numbers). Returns a | ||||||||||||||||||||||||||||||||||||||
| * `{ pkField: value }` map for `__forest.primaryKey`. Throws a mapping error rather than emitting a | ||||||||||||||||||||||||||||||||||||||
| * malformed key when the schema lacks key metadata or the packed id shape does not match it. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| export default function unpackPrimaryKey( | ||||||||||||||||||||||||||||||||||||||
| packedId: string, | ||||||||||||||||||||||||||||||||||||||
| primaryKeys: PrimaryKeyField[], | ||||||||||||||||||||||||||||||||||||||
| ): Record<string, string | number> { | ||||||||||||||||||||||||||||||||||||||
| if (primaryKeys.length === 0) { | ||||||||||||||||||||||||||||||||||||||
| throw mappingError('Cannot build primary key: the collection exposes no key metadata'); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const values = packedId.split(PACKED_ID_SEPARATOR); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (values.length !== primaryKeys.length) { | ||||||||||||||||||||||||||||||||||||||
| throw mappingError( | ||||||||||||||||||||||||||||||||||||||
| `Cannot build primary key: expected ${primaryKeys.length} values, found ${values.length}`, | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const result: Record<string, string | number> = {}; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| primaryKeys.forEach(({ name, type }, index) => { | ||||||||||||||||||||||||||||||||||||||
| const value = values[index]; | ||||||||||||||||||||||||||||||||||||||
| result[name] = type === 'Number' ? Number(value) : value; | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+31
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium
Suggested change
🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return result; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 High
src/cli-core.ts:191In OAuth mode, requests to
/agent/v1/:collection/list|countfail becausebuildDataRoutesMiddlewareis mounted for every authenticated/agentrequest but only the API-key branch populatesctx.state.agentToken. When the OAuth path runs,createAuthModeMiddlewaresets onlyctx.state.principal, leavingctx.state.agentTokenundefined, so the data routes call the agent withAuthorization: Bearer undefined. Either pass the OAuth bearer token intoctx.state.agentTokenfor the data routes or gatebuildDataRoutesMiddlewareso it only runs in API-key mode.🚀 Reply "fix it for me" or copy this AI Prompt for your agent: