From 9e4f82dcb6d54b59fdd80b5d07fb1f63f9e23b29 Mon Sep 17 00:00:00 2001 From: Markus Podar Date: Fri, 12 May 2023 16:52:31 +0200 Subject: [PATCH] pagination: enforce `nonNull` on the inner type as well as on the list --- CHANGELOG.md | 1 + README.md | 2 +- phpstan-baseline.neon | 40 ++++ src/Support/PaginationType.php | 10 +- src/Support/SelectFields.php | 2 +- src/Support/SimplePaginationType.php | 10 +- tests/Database/SelectFieldsTest.php | 172 ++++++++++++++++++ .../Queries/PostNonNullPaginationQuery.php | 31 ++++ .../PostNonNullSimplePaginationQuery.php | 31 ++++ 9 files changed, 287 insertions(+), 12 deletions(-) create mode 100644 tests/Support/Queries/PostNonNullPaginationQuery.php create mode 100644 tests/Support/Queries/PostNonNullSimplePaginationQuery.php diff --git a/CHANGELOG.md b/CHANGELOG.md index fb877698..0c3e893b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ CHANGELOG 'Mutation' and 'Subscription' - Return types were added to all methods of the commands [\#1005 / sforward](https://github.com/rebing/graphql-laravel/pull/1005) - Upgrade to laragraph/utils v2 [\#1032 / mfn](https://github.com/rebing/graphql-laravel/pull/1032) +- The `Pagination` and `SimplePagination` helper types no enforce `nonNull` on their data types ### Removed - Remove unused publish command [\#1004 / sforward](https://github.com/rebing/graphql-laravel/pull/1004) diff --git a/README.md b/README.md index 77fe4daa..16366a4b 100644 --- a/README.md +++ b/README.md @@ -1862,7 +1862,7 @@ class PostsQuery extends Query { public function type(): Type { - return Type::nonNull(GraphQL::simplePaginate('posts')); + return GraphQL::simplePaginate('posts'); } // ... diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 309f35ad..17f25a89 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -215,6 +215,11 @@ parameters: count: 1 path: src/Support/PaginationType.php + - + message: "#^Parameter \\#1 \\$underlyingType of method Rebing\\\\GraphQL\\\\Support\\\\PaginationType\\:\\:getPaginationFields\\(\\) expects GraphQL\\\\Type\\\\Definition\\\\ObjectType, GraphQL\\\\Type\\\\Definition\\\\Type given\\.$#" + count: 1 + path: src/Support/PaginationType.php + - message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\Privacy\\:\\:fire\\(\\) has parameter \\$args with no type specified\\.$#" count: 1 @@ -350,6 +355,11 @@ parameters: count: 1 path: src/Support/SelectFields.php + - + message: "#^Parameter \\#1 \\$underlyingType of method Rebing\\\\GraphQL\\\\Support\\\\SimplePaginationType\\:\\:getPaginationFields\\(\\) expects GraphQL\\\\Type\\\\Definition\\\\ObjectType, GraphQL\\\\Type\\\\Definition\\\\Type given\\.$#" + count: 1 + path: src/Support/SimplePaginationType.php + - message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\Type\\:\\:getFieldResolver\\(\\) has parameter \\$field with no value type specified in iterable type array\\.$#" count: 1 @@ -1005,6 +1015,36 @@ parameters: count: 1 path: tests/Support/Objects/ExamplesQuery.php + - + message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Queries\\\\PostNonNullPaginationQuery\\:\\:resolve\\(\\) has parameter \\$args with no type specified\\.$#" + count: 1 + path: tests/Support/Queries/PostNonNullPaginationQuery.php + + - + message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Queries\\\\PostNonNullPaginationQuery\\:\\:resolve\\(\\) has parameter \\$context with no type specified\\.$#" + count: 1 + path: tests/Support/Queries/PostNonNullPaginationQuery.php + + - + message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Queries\\\\PostNonNullPaginationQuery\\:\\:resolve\\(\\) has parameter \\$root with no type specified\\.$#" + count: 1 + path: tests/Support/Queries/PostNonNullPaginationQuery.php + + - + message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Queries\\\\PostNonNullSimplePaginationQuery\\:\\:resolve\\(\\) has parameter \\$args with no type specified\\.$#" + count: 1 + path: tests/Support/Queries/PostNonNullSimplePaginationQuery.php + + - + message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Queries\\\\PostNonNullSimplePaginationQuery\\:\\:resolve\\(\\) has parameter \\$context with no type specified\\.$#" + count: 1 + path: tests/Support/Queries/PostNonNullSimplePaginationQuery.php + + - + message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Queries\\\\PostNonNullSimplePaginationQuery\\:\\:resolve\\(\\) has parameter \\$root with no type specified\\.$#" + count: 1 + path: tests/Support/Queries/PostNonNullSimplePaginationQuery.php + - message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Queries\\\\PostNonNullWithSelectFieldsAndModelQuery\\:\\:resolve\\(\\) has no return type specified\\.$#" count: 1 diff --git a/src/Support/PaginationType.php b/src/Support/PaginationType.php index f5a54858..ff9cc710 100644 --- a/src/Support/PaginationType.php +++ b/src/Support/PaginationType.php @@ -15,13 +15,13 @@ public function __construct(string $typeName, string $customName = null) { $name = $customName ?: $typeName . 'Pagination'; + $underlyingType = GraphQL::type($typeName); + $config = [ 'name' => $name, - 'fields' => $this->getPaginationFields($typeName), + 'fields' => $this->getPaginationFields($underlyingType), ]; - $underlyingType = GraphQL::type($typeName); - if (isset($underlyingType->config['model'])) { $config['model'] = $underlyingType->config['model']; } @@ -29,11 +29,11 @@ public function __construct(string $typeName, string $customName = null) parent::__construct($config); } - protected function getPaginationFields(string $typeName): array + protected function getPaginationFields(ObjectType $underlyingType): array { return [ 'data' => [ - 'type' => GraphQLType::listOf(GraphQL::type($typeName)), + 'type' => GraphQLType::nonNull(GraphQLType::listOf(GraphQLType::nonNull($underlyingType))), 'description' => 'List of items on the current page', 'resolve' => function (LengthAwarePaginator $data): Collection { return $data->getCollection(); diff --git a/src/Support/SelectFields.php b/src/Support/SelectFields.php index 303a21fc..e4d025b8 100644 --- a/src/Support/SelectFields.php +++ b/src/Support/SelectFields.php @@ -185,7 +185,7 @@ protected static function handleFields( static::handleFields( $queryArgs, $field, - $fieldType->getWrappedType(), + $fieldType->getInnermostType(), $select, $with, $ctx diff --git a/src/Support/SimplePaginationType.php b/src/Support/SimplePaginationType.php index fe3ad337..9ad778ba 100644 --- a/src/Support/SimplePaginationType.php +++ b/src/Support/SimplePaginationType.php @@ -15,13 +15,13 @@ public function __construct(string $typeName, string $customName = null) { $name = $customName ?: $typeName . 'SimplePagination'; + $underlyingType = GraphQL::type($typeName); + $config = [ 'name' => $name, - 'fields' => $this->getPaginationFields($typeName), + 'fields' => $this->getPaginationFields($underlyingType), ]; - $underlyingType = GraphQL::type($typeName); - if (isset($underlyingType->config['model'])) { $config['model'] = $underlyingType->config['model']; } @@ -32,11 +32,11 @@ public function __construct(string $typeName, string $customName = null) /** * @return array> */ - protected function getPaginationFields(string $typeName): array + protected function getPaginationFields(ObjectType $underlyingType): array { return [ 'data' => [ - 'type' => GraphQLType::listOf(GraphQL::type($typeName)), + 'type' => GraphQLType::nonNull(GraphQLType::listOf(GraphQLType::nonNull($underlyingType))), 'description' => 'List of items on the current page', 'resolve' => function (Paginator $data): Collection { return $data->getCollection(); diff --git a/tests/Database/SelectFieldsTest.php b/tests/Database/SelectFieldsTest.php index 202cca70..eba2285d 100644 --- a/tests/Database/SelectFieldsTest.php +++ b/tests/Database/SelectFieldsTest.php @@ -3,9 +3,13 @@ declare(strict_types = 1); namespace Rebing\GraphQL\Tests\Database; +use GraphQL\Utils\SchemaPrinter; use Illuminate\Support\Carbon; +use Rebing\GraphQL\Support\Facades\GraphQL; use Rebing\GraphQL\Tests\Support\Models\Comment; use Rebing\GraphQL\Tests\Support\Models\Post; +use Rebing\GraphQL\Tests\Support\Queries\PostNonNullPaginationQuery; +use Rebing\GraphQL\Tests\Support\Queries\PostNonNullSimplePaginationQuery; use Rebing\GraphQL\Tests\Support\Queries\PostNonNullWithSelectFieldsAndModelQuery; use Rebing\GraphQL\Tests\Support\Queries\PostQuery; use Rebing\GraphQL\Tests\Support\Queries\PostQueryWithNonInjectableTypehintsQuery; @@ -35,6 +39,8 @@ protected function getEnvironmentSetUp($app): void $app['config']->set('graphql.schemas.default', [ 'query' => [ + PostNonNullPaginationQuery::class, + PostNonNullSimplePaginationQuery::class, PostNonNullWithSelectFieldsAndModelQuery::class, PostQuery::class, PostsListOfWithSelectFieldsAndModelQuery::class, @@ -614,4 +620,170 @@ public function testWithSelectFieldsNoModel(): void self::assertEquals(200, $response->getStatusCode()); self::assertEquals($expectedResult, $response->json()); } + public function testPostNonNullPaginationQuery(): void + { + /** @var Post $post */ + $post = factory(Post::class)->create([ + 'title' => 'Title of the post', + ]); + + $graphql = <<<'GRAQPHQL' +{ + postNonNullPaginationQuery { + data { + id, + title, + } + } +} +GRAQPHQL; + + $this->sqlCounterReset(); + + $response = $this->call('GET', '/graphql', [ + 'query' => $graphql, + ]); + + $this->assertSqlQueries( + <<<'SQL' +select count(*) as aggregate from "posts"; +select "posts"."id", "posts"."title" from "posts" limit 15 offset 0; +SQL + ); + + $expectedResult = [ + 'data' => [ + 'postNonNullPaginationQuery' => [ + 'data' => [ + [ + 'id' => "$post->id", + 'title' => 'Title of the post', + ], + ], + ], + ], + ]; + + self::assertEquals(200, $response->getStatusCode()); + self::assertEquals($expectedResult, $response->json()); + } + + public function testPostNonNullPaginationTypes(): void + { + $schema = GraphQL::buildSchemaFromConfig([ + 'query' => [ + 'postNonNullPaginationQuery' => PostNonNullPaginationQuery::class, + ], + ]); + + $gql = SchemaPrinter::doPrint($schema); + + $queryFragment = <<<'GQL' +type PostWithModelPagination { + "List of items on the current page" + data: [PostWithModel!]! + + "Number of total items selected by the query" + total: Int! + + "Number of items returned per page" + per_page: Int! + + "Current page of the cursor" + current_page: Int! + + "Number of the first item returned" + from: Int + + "Number of the last item returned" + to: Int + + "The last page (number of pages)" + last_page: Int! + + "Determines if cursor has more pages after the current page" + has_more_pages: Boolean! +} +GQL; + + self::assertStringContainsString($queryFragment, $gql); + } + + public function testPostNonNullSimplePaginationQuery(): void + { + /** @var Post $post */ + $post = factory(Post::class)->create([ + 'title' => 'Title of the post', + ]); + + $graphql = <<<'GRAQPHQL' +{ + postNonNullSimplePaginationQuery { + data { + id, + title, + } + } +} +GRAQPHQL; + + $this->sqlCounterReset(); + + $response = $this->call('GET', '/graphql', [ + 'query' => $graphql, + ]); + + $this->assertSqlQueries('select "posts"."id", "posts"."title" from "posts" limit 16 offset 0;'); + + $expectedResult = [ + 'data' => [ + 'postNonNullSimplePaginationQuery' => [ + 'data' => [ + [ + 'id' => "$post->id", + 'title' => 'Title of the post', + ], + ], + ], + ], + ]; + + self::assertEquals(200, $response->getStatusCode()); + self::assertEquals($expectedResult, $response->json()); + } + + public function testPostNonNullSimplePaginationTypes(): void + { + $schema = GraphQL::buildSchemaFromConfig([ + 'query' => [ + 'postNonNullSimplePaginationQuery' => PostNonNullSimplePaginationQuery::class, + ], + ]); + + $gql = SchemaPrinter::doPrint($schema); + + $queryFragment = <<<'GQL' +type PostWithModelSimplePagination { + "List of items on the current page" + data: [PostWithModel!]! + + "Number of items returned per page" + per_page: Int! + + "Current page of the cursor" + current_page: Int! + + "Number of the first item returned" + from: Int + + "Number of the last item returned" + to: Int + + "Determines if cursor has more pages after the current page" + has_more_pages: Boolean! +} +GQL; + + self::assertStringContainsString($queryFragment, $gql); + } } diff --git a/tests/Support/Queries/PostNonNullPaginationQuery.php b/tests/Support/Queries/PostNonNullPaginationQuery.php new file mode 100644 index 00000000..3dced839 --- /dev/null +++ b/tests/Support/Queries/PostNonNullPaginationQuery.php @@ -0,0 +1,31 @@ + 'postNonNullPaginationQuery', + ]; + + public function type(): Type + { + return GraphQL::paginate('PostWithModel'); + } + + public function resolve($root, $args, $context, ResolveInfo $resolveInfo, Closure $getSelectFields): LengthAwarePaginator + { + return Post::query() + ->select($getSelectFields()->getSelect()) + ->paginate(); + } +} diff --git a/tests/Support/Queries/PostNonNullSimplePaginationQuery.php b/tests/Support/Queries/PostNonNullSimplePaginationQuery.php new file mode 100644 index 00000000..8fc42753 --- /dev/null +++ b/tests/Support/Queries/PostNonNullSimplePaginationQuery.php @@ -0,0 +1,31 @@ + 'postNonNullSimplePaginationQuery', + ]; + + public function type(): Type + { + return GraphQL::simplePaginate('PostWithModel'); + } + + public function resolve($root, $args, $context, ResolveInfo $resolveInfo, Closure $getSelectFields): Paginator + { + return Post::query() + ->select($getSelectFields()->getSelect()) + ->simplePaginate(); + } +}