-
Notifications
You must be signed in to change notification settings - Fork 2
Description
Background
Currently, the transformation we do on JSON API responses at runtime is to parse date values and camel-case all keys. Importantly, that processing does not rely on anything other than the response itself. For the keys, we camel-case all of them, and for the dates, we're looking at a) whether the key is prefixed with time_ and b) whether the value successfully parses as a date. We do not make reference to anything from the OpenAPI definition at runtime.
Lines 22 to 59 in 6358641
| /** | |
| * Recursively map (k, v) pairs using Object.entries | |
| * | |
| * Note that value transform function takes both k and v so we can use the key | |
| * to decide whether to transform the value. | |
| * | |
| * @param kf maps key to key | |
| * @param vf maps key + value to value | |
| */ | |
| export const mapObj = | |
| ( | |
| kf: (k: string) => string, | |
| vf: (k: string | undefined, v: unknown) => unknown = (k, v) => v | |
| ) => | |
| (o: unknown): unknown => { | |
| if (!isObjectOrArray(o)) return o; | |
| if (Array.isArray(o)) return o.map(mapObj(kf, vf)); | |
| const newObj: Record<string, unknown> = {}; | |
| for (const [k, v] of Object.entries(o as Record<string, unknown>)) { | |
| newObj[kf(k)] = isObjectOrArray(v) ? mapObj(kf, vf)(v) : vf(k, v); | |
| } | |
| return newObj; | |
| }; | |
| export const parseIfDate = (k: string | undefined, v: unknown) => { | |
| if (typeof v === "string" && (k?.startsWith("time_") || k === "timestamp")) { | |
| const d = new Date(v); | |
| if (isNaN(d.getTime())) return v; | |
| return d; | |
| } | |
| return v; | |
| }; | |
| export const snakeify = mapObj(camelToSnake); | |
| export const processResponseBody = mapObj(snakeToCamel, parseIfDate); |
Problem
oxidecomputer/omicron#5258 is introducing a new type: "string", format: "uint128" type to the Nexus OpenAPI definition, and we need to modify the generated client to run those values through BigInt at runtime. The problem is that it's not obvious how to recognize them from the response body alone. There are some ways we could do it
- Doesn't work: transform any string that matches
/^\d+$/. Doesn't work because anyone can set the description of a project to555. Blocklisting keys likedescriptionto always skip is way too fragile. - Add a special prefix or suffix analogous to
time_to the keys (in this caseallocatedandcapacityto indicate BigInt. This can be made to work, but it's ugly — it's a hack on the API that's way too specific to this generator, and all it does is let us keep being bad.allocated_bigint?allocated_n? What if someone sees that and thinks we just do that for numbers? Too fragile.
The information we need is right there in the schema — format: "uint128".
Another way: smarter runtime response transformations
In the OpenAPI definition, we of course have the real format data:
"time_created": {
"description": "timestamp when this resource was created",
"type": "string",
"format": "date-time"
},
"time_modified": {
"description": "timestamp when this resource was last modified",
"type": "string",
"format": "date-time"
}And we are using that when we generate the types:
oxide.ts/generator/schema/base.ts
Lines 40 to 42 in 6358641
| .with({ type: "string", format: "date-time" }, (s) => | |
| handlers.date(s, io) | |
| ) |
oxide.ts/generator/schema/types.ts
Lines 32 to 34 in 6358641
| date(_, { w0 }) { | |
| w0(`Date`); | |
| }, |
Lines 1807 to 1818 in 6358641
| export type Project = { | |
| /** human-readable free-form text about a resource */ | |
| description: string; | |
| /** unique, immutable, system-controlled identifier for each resource */ | |
| id: string; | |
| /** unique, mutable, user-controlled identifier for each resource */ | |
| name: Name; | |
| /** timestamp when this resource was created */ | |
| timeCreated: Date; | |
| /** timestamp when this resource was last modified */ | |
| timeModified: Date; | |
| }; |
Ideally we should be able to generate some additional code as part of each request handler that represents the transformation we would like to do on the response body.
Options
This is the method we generate for the utilization endpoint added in the omicron PR. One way to do the above would be to add a parameter to request that is a function doing the transformation we want.
/**
* Fetch IP pool utilization
*/
ipPoolUtilizationView: (
{ path }: { path: IpPoolUtilizationViewPathParams },
params: FetchParams = {}
) => {
return this.request<IpPoolUtilization>({
path: `/v1/system/ip-pools/${path.pool}/utilization`,
method: 'GET',
...params,
})
},
So in this case it would look something like
transformResponse: (resp) => ({
...resp,
allocated: BigInt(resp.allocated),
capacity: BigInt(resp.capacity),
})I have some other idea, need to think about it more. I'm also looking at the reviver param on JSON.parse.