diff --git a/codegen/layouts/partials/route-method.hbs b/codegen/layouts/partials/route-method.hbs index 0229b4dc..0552974d 100644 --- a/codegen/layouts/partials/route-method.hbs +++ b/codegen/layouts/partials/route-method.hbs @@ -1,7 +1,7 @@ {{{methodPhpDoc this}}} public function {{methodName}}({{{signatureParams}}}): {{returnType}} { {{#if requiresAtLeastOneParameter}} - if ({{#each parameters}}{{#unless @first}} && {{/unless}}${{name}} === null{{/each}}) { + if ({{#each atLeastOneParameterNames}}{{#unless @first}} && {{/unless}}${{this}} === null{{/each}}) { throw new \InvalidArgumentException("At least one parameter is required for {{path}}"); } {{/if}} diff --git a/codegen/lib/layouts/route.ts b/codegen/lib/layouts/route.ts index 8407fc4a..f58c5ef9 100644 --- a/codegen/lib/layouts/route.ts +++ b/codegen/lib/layouts/route.ts @@ -40,6 +40,7 @@ export interface MethodLayoutContext { returnType: string hasParams: boolean requiresAtLeastOneParameter: boolean + atLeastOneParameterNames: string[] signatureParams: string usesActionAttempt: boolean usesOnResponse: boolean @@ -60,6 +61,8 @@ export interface RouteLayoutContext extends ClientLayoutContext { useStatements: string[] } +const paginationParameters = new Set(['limit', 'page_cursor']) + const waitForActionAttemptParameter = { name: 'wait_for_action_attempt', type: 'bool|array|null', @@ -124,6 +127,10 @@ const getMethodLayoutContext = ( .concat(usesOnResponse ? ['?callable $on_response = null'] : []) .join(', ') + const atLeastOneParameterNames = sortedParameters + .map(({ name }) => name) + .filter((name) => !paginationParameters.has(name)) + const endpointParameters = sortedParameters.map( ({ name, type, phpDocType, description, isOptional, isNullable }) => ({ name, @@ -164,7 +171,9 @@ const getMethodLayoutContext = ( path, returnType, hasParams: parameters.length > 0, - requiresAtLeastOneParameter: method.requiresAtLeastOneParameter, + requiresAtLeastOneParameter: + method.requiresAtLeastOneParameter && atLeastOneParameterNames.length > 0, + atLeastOneParameterNames, signatureParams, usesActionAttempt, usesOnResponse, diff --git a/src/Routes/AccessCodesClient.php b/src/Routes/AccessCodesClient.php index c5211607..9fb239cb 100644 --- a/src/Routes/AccessCodesClient.php +++ b/src/Routes/AccessCodesClient.php @@ -386,8 +386,6 @@ public function list( $access_method_id === null && $customer_key === null && $device_id === null && - $limit === null && - $page_cursor === null && $search === null && $user_identifier_key === null ) { diff --git a/src/Routes/AccessMethodsClient.php b/src/Routes/AccessMethodsClient.php index 41c22195..2e3210f6 100644 --- a/src/Routes/AccessMethodsClient.php +++ b/src/Routes/AccessMethodsClient.php @@ -228,8 +228,6 @@ public function list( $access_grant_key === null && $acs_entrance_id === null && $device_id === null && - $limit === null && - $page_cursor === null && $space_id === null ) { throw new \InvalidArgumentException( diff --git a/src/Routes/EventsClient.php b/src/Routes/EventsClient.php index f2c8177c..671745be 100644 --- a/src/Routes/EventsClient.php +++ b/src/Routes/EventsClient.php @@ -149,7 +149,6 @@ public function list( $event_ids === null && $event_type === null && $event_types === null && - $limit === null && $since === null && $space_id === null && $space_ids === null && diff --git a/tests/RequiredParametersTest.php b/tests/RequiredParametersTest.php new file mode 100644 index 00000000..e3c5d584 --- /dev/null +++ b/tests/RequiredParametersTest.php @@ -0,0 +1,165 @@ + [], "events" => []]), + ]); + + return [ + Seam::from_api_key( + "seam_apikey_token", + endpoint: "https://example.com", + guzzle_options: $recorder->guzzle_options(), + retries: 0, + ), + $recorder, + ]; + } + + private function assertRejected(callable $call, string $path): void + { + [$seam, $recorder] = $this->recorded(); + + try { + $call($seam); + $this->fail("Expected InvalidArgumentException for $path"); + } catch (\InvalidArgumentException $error) { + $this->assertSame( + "At least one parameter is required for $path", + $error->getMessage(), + ); + } + + $this->assertSame(0, $recorder->request_count()); + } + + public function testRejectsACallThatNamesNothing(): void + { + $this->assertRejected( + fn(Seam $seam) => $seam->access_codes->list(), + "/access_codes/list", + ); + } + + /** + * @dataProvider paginationOnlyCalls + */ + public function testPaginationParamsAloneDoNotSatisfyTheGuard( + callable $call, + string $path, + ): void { + $this->assertRejected($call, $path); + } + + public static function paginationOnlyCalls(): array + { + return [ + "limit" => [ + fn(Seam $seam) => $seam->access_codes->list(limit: 20), + "/access_codes/list", + ], + "page cursor" => [ + fn(Seam $seam) => $seam->access_codes->list( + page_cursor: "cursor", + ), + "/access_codes/list", + ], + "limit and page cursor" => [ + fn(Seam $seam) => $seam->access_codes->list( + limit: 20, + page_cursor: "cursor", + ), + "/access_codes/list", + ], + "limit on an unpaginated list" => [ + fn(Seam $seam) => $seam->events->list(limit: 20), + "/events/list", + ], + ]; + } + + /** + * @dataProvider filteredCalls + */ + public function testAcceptsACallThatNamesAFilter( + callable $call, + string $expected_query, + ): void { + [$seam, $recorder] = $this->recorded(); + + $call($seam); + + $this->assertSame(1, $recorder->request_count()); + $this->assertStringContainsString( + $expected_query, + $recorder->request()->getUri()->getQuery(), + ); + } + + public static function filteredCalls(): array + { + return [ + "a filter" => [ + fn(Seam $seam) => $seam->access_codes->list( + device_id: "device-1", + ), + "device_id=device-1", + ], + "a filter alongside pagination" => [ + fn(Seam $seam) => $seam->access_codes->list( + device_id: "device-1", + limit: 20, + ), + "device_id=device-1", + ], + "a filter on an unpaginated list" => [ + fn(Seam $seam) => $seam->events->list( + event_type: "device.connected", + ), + "event_type=device.connected", + ], + ]; + } + + public function testAPaginatorOverAnUnfilteredListIsRejectedThroughout(): void + { + [$seam] = $this->recorded(); + + $pages = $seam->createPaginator( + fn($params) => $seam->access_codes->list(...$params), + ); + + foreach ( + [ + fn() => $pages->firstPage(), + fn() => $pages->flattenToArray(), + fn() => iterator_to_array($pages->flatten()), + ] + as $call + ) { + try { + $call(); + $this->fail("Expected InvalidArgumentException"); + } catch (\InvalidArgumentException $error) { + $this->assertSame( + "At least one parameter is required for /access_codes/list", + $error->getMessage(), + ); + } + } + } +}