Skip to content

Commit 1103fe0

Browse files
committed
fix: Validate required method params
1 parent 7968051 commit 1103fe0

7 files changed

Lines changed: 117 additions & 8 deletions

File tree

codegen/layouts/partials/route-class-endpoint-export.hbs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
export type {{parametersTypeName}} = {{> request-object parameters=parameters}}
1+
export type {{parametersTypeName}} = {{#if requiresAtLeastOneParameter}}RequireAtLeastOne<{{/if}}{{> request-object parameters=parameters}}{{#if requiresAtLeastOneParameter}}>{{/if}}
22

33
/**
44
* @deprecated Use {{requestTypeName}} instead.

codegen/layouts/partials/route-class-endpoint.hbs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
{{methodName}}
33
{{> endpont-method-signature }}
44
{
5+
assertValidRequestParameters(parameters, '{{path}}', {{hasRequiredParameters}})
6+
57
return new SeamHttpRequest(this, {
68
pathname: '{{path}}',
79
method: '{{method}}',

codegen/layouts/route.hbs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55

66
{{> route-imports }}
77

8+
{{#if endpoints}}
9+
import { assertValidRequestParameters } from 'lib/request-parameters.js'
10+
{{/if}}
11+
{{#if needsRequireAtLeastOneImport}}
12+
import type { RequireAtLeastOne } from 'lib/request-parameters.js'
13+
{{/if}}
14+
815
export class {{className}} {
916
{{> route-class-methods }}
1017

codegen/lib/layouts/route.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export interface RouteLayoutContext {
1010
subroutes: SubrouteLayoutContext[]
1111
skipClientSessionImport: boolean
1212
needsActionAttemptsImport: boolean
13+
needsRequireAtLeastOneImport: boolean
1314
resourceTypeImports: ResourceTypeImport[]
1415
}
1516

@@ -35,6 +36,8 @@ export interface EndpointLayoutContext {
3536
returnsActionAttempt: boolean
3637
returnsVoid: boolean
3738
isOptionalParamsOk: boolean
39+
hasRequiredParameters: boolean
40+
requiresAtLeastOneParameter: boolean
3841
parameters: Parameter[]
3942
responseIsList: boolean
4043
responseResourceTypeName: string
@@ -64,6 +67,10 @@ export const setRouteLayoutContext = (
6467
node.path !== '/action_attempts' &&
6568
'endpoints' in node &&
6669
node.endpoints.some(isActionAttemptEndpoint)
70+
file.needsRequireAtLeastOneImport =
71+
node != null &&
72+
'endpoints' in node &&
73+
node.endpoints.some(requiresAtLeastOneParameter)
6774
file.resourceTypeImports =
6875
node != null && 'endpoints' in node
6976
? [
@@ -140,9 +147,9 @@ export const getEndpointLayoutContext = (
140147
responseTypeName: `${prefix}Response`,
141148
optionsTypeName: `${prefix}Options`,
142149
requestTypeName: `${prefix}Request`,
143-
isOptionalParamsOk: endpoint.request.parameters.every(
144-
(parameter) => !parameter.isRequired,
145-
),
150+
isOptionalParamsOk: !endpoint.request.hasRequiredParameters,
151+
hasRequiredParameters: endpoint.request.hasRequiredParameters,
152+
requiresAtLeastOneParameter: requiresAtLeastOneParameter(endpoint),
146153
parameters: endpoint.request.parameters,
147154
responseIsList: endpoint.response.responseType === 'resource_list',
148155
responseResourceTypeName:
@@ -155,6 +162,10 @@ export const getEndpointLayoutContext = (
155162
}
156163
}
157164

165+
const requiresAtLeastOneParameter = (endpoint: Endpoint): boolean =>
166+
endpoint.request.hasRequiredParameters &&
167+
endpoint.request.parameters.every(({ isRequired }) => !isRequired)
168+
158169
const isActionAttemptEndpoint = (endpoint: Endpoint): boolean =>
159170
endpoint.response.responseType === 'resource' &&
160171
endpoint.response.resourceType === 'action_attempt'

src/lib/request-parameters.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
export type RequireAtLeastOne<
2+
T,
3+
Keys extends keyof T = keyof T,
4+
> = Keys extends keyof T
5+
? Required<Pick<T, Keys>> & Partial<Omit<T, Keys>>
6+
: never
7+
8+
export const assertValidRequestParameters = (
9+
parameters: unknown,
10+
path: string,
11+
hasRequiredParameters: boolean,
12+
): void => {
13+
if (parameters === undefined) {
14+
if (hasRequiredParameters) {
15+
throw new TypeError(`Parameters are required for ${path}`)
16+
}
17+
return
18+
}
19+
20+
if (
21+
parameters === null ||
22+
typeof parameters !== 'object' ||
23+
Array.isArray(parameters)
24+
) {
25+
throw new TypeError(`Parameters for ${path} must be an object`)
26+
}
27+
28+
if (hasRequiredParameters && Object.keys(parameters).length === 0) {
29+
throw new TypeError(
30+
`Parameters for ${path} must contain at least one property`,
31+
)
32+
}
33+
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import test from 'ava'
2+
3+
import { SeamHttp } from '@seamapi/http/connect'
4+
5+
const seam = SeamHttp.fromApiKey('seam_apikey1_token')
6+
7+
test('endpoint rejects missing required parameters', (t) => {
8+
t.throws(
9+
() => {
10+
// @ts-expect-error Verify requiredness in the generated method signature.
11+
seam.devices.get()
12+
},
13+
{
14+
instanceOf: TypeError,
15+
message: 'Parameters are required for /devices/get',
16+
},
17+
)
18+
})
19+
20+
test('endpoint rejects an empty required parameters object', (t) => {
21+
t.throws(
22+
() => {
23+
// @ts-expect-error Verify RequireAtLeastOne in the generated parameter type.
24+
seam.devices.get({})
25+
},
26+
{
27+
instanceOf: TypeError,
28+
message: 'Parameters for /devices/get must contain at least one property',
29+
},
30+
)
31+
})
32+
33+
test('endpoint rejects non-object parameters', (t) => {
34+
t.throws(
35+
() => {
36+
// @ts-expect-error Verify the generated parameter type rejects primitives.
37+
seam.devices.list('invalid')
38+
},
39+
{
40+
instanceOf: TypeError,
41+
message: 'Parameters for /devices/list must be an object',
42+
},
43+
)
44+
})
45+
46+
test('endpoint accepts omitted optional parameters', (t) => {
47+
t.notThrows(() => seam.devices.list())
48+
})
49+
50+
test('endpoint accepts required parameters', (t) => {
51+
t.notThrows(() => seam.devices.get({ device_id: 'device-id' }))
52+
t.notThrows(() => seam.devices.get({ name: 'Front Door' }))
53+
})

test/seam/connect/seam-paginator.test.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,13 @@ test('SeamPaginator: cannot paginate a request with an empty response', async (t
1414
const { seed, endpoint } = await getTestServer(t)
1515
const seam = SeamHttp.fromApiKey(seed.seam_apikey1_token, { endpoint })
1616

17-
// @ts-expect-error Testing validation
18-
t.throws(() => seam.createPaginator(seam.devices.update()), {
19-
message: /does not support pagination/,
20-
})
17+
t.throws(
18+
() =>
19+
seam.createPaginator(
20+
seam.devices.update({ device_id: 'test-device-id' }),
21+
),
22+
{ message: /does not support pagination/ },
23+
)
2124
})
2225

2326
// TODO: Validate the request supports pagination by extending SeamHttpRequest with this knowledge via codegen.

0 commit comments

Comments
 (0)