Skip to content
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

Return empty results for pagination first: 0 #2480

Merged
merged 7 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ You can find and compare releases at the [GitHub release page](https://github.co

## Unreleased

## v6.29.0

### Fixed

- Return empty results for pagination `first: 0`

## v6.28.0

### Added
Expand Down
4 changes: 2 additions & 2 deletions src/Execution/ModelsLoader/PaginatedModelsLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Pagination\LengthAwarePaginator;
use Nuwave\Lighthouse\Pagination\PaginationArgs;
use Nuwave\Lighthouse\Pagination\ZeroPageLengthAwarePaginator;
use Nuwave\Lighthouse\Pagination\ZeroPerPageLengthAwarePaginator;
use Nuwave\Lighthouse\Support\Utils;

class PaginatedModelsLoader implements ModelsLoader
Expand Down Expand Up @@ -185,7 +185,7 @@ protected function convertRelationToPaginator(EloquentCollection $parents): void
$total = CountModelsLoader::extractCount($model, $this->relation);

$paginator = $first === 0
? new ZeroPageLengthAwarePaginator($total, $page)
? new ZeroPerPageLengthAwarePaginator($total, $page)
: new LengthAwarePaginator($model->getRelation($this->relation), $total, $first, $page);

$model->setRelation($this->relation, $paginator);
Expand Down
54 changes: 12 additions & 42 deletions src/Pagination/PaginateDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,17 @@
use GraphQL\Language\AST\FieldDefinitionNode;
use GraphQL\Language\AST\InterfaceTypeDefinitionNode;
use GraphQL\Language\AST\ObjectTypeDefinitionNode;
use Illuminate\Contracts\Pagination\LengthAwarePaginator;
use Illuminate\Contracts\Pagination\Paginator;
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Database\Query\Builder as QueryBuilder;
use Laravel\Scout\Builder as ScoutBuilder;
use Nuwave\Lighthouse\Cache\CacheDirective;
use Nuwave\Lighthouse\Execution\ResolveInfo;
use Nuwave\Lighthouse\Schema\AST\DocumentAST;
use Nuwave\Lighthouse\Schema\Directives\BaseDirective;
use Nuwave\Lighthouse\Schema\Values\FieldValue;
use Nuwave\Lighthouse\Support\Contracts\ComplexityResolverDirective;
use Nuwave\Lighthouse\Support\Contracts\Directive;
use Nuwave\Lighthouse\Support\Contracts\FieldManipulator;
use Nuwave\Lighthouse\Support\Contracts\FieldResolver;
use Nuwave\Lighthouse\Support\Contracts\GraphQLContext;
Expand Down Expand Up @@ -138,25 +137,28 @@ public function manipulateFieldDefinition(DocumentAST &$documentAST, FieldDefini
public function resolveField(FieldValue $fieldValue): callable
{
return function (mixed $root, array $args, GraphQLContext $context, ResolveInfo $resolveInfo): Paginator {
if ($this->directiveHasArgument('resolver')) {
// This is done only for validation
PaginationArgs::extractArgs($args, $this->paginationType(), $this->paginateMaxCount());
$paginationArgs = PaginationArgs::extractArgs($args, $resolveInfo, $this->paginationType(), $this->paginateMaxCount());

if ($this->directiveHasArgument('resolver')) {
$paginator = $this->getResolverFromArgument('resolver')($root, $args, $context, $resolveInfo);

assert(
$paginator instanceof Paginator,
"The method referenced by the resolver argument of the @{$this->name()} directive on {$this->nodeName()} must return a Paginator.",
);

if ($paginationArgs->first === 0) {
if ($paginator instanceof LengthAwarePaginator) {
return new ZeroPerPageLengthAwarePaginator($paginator->total(), $paginationArgs->page);
}

return new ZeroPerPagePaginator($paginationArgs->page);
}

return $paginator;
}

if ($this->directiveHasArgument('builder')) {
$builderResolver = $this->getResolverFromArgument('builder');

$query = $builderResolver($root, $args, $context, $resolveInfo);

$query = $this->getResolverFromArgument('builder')($root, $args, $context, $resolveInfo);
assert(
$query instanceof QueryBuilder || $query instanceof EloquentBuilder || $query instanceof ScoutBuilder || $query instanceof Relation,
"The method referenced by the builder argument of the @{$this->name()} directive on {$this->nodeName()} must return a Builder or Relation.",
Expand All @@ -174,42 +176,10 @@ public function resolveField(FieldValue $fieldValue): callable
$resolveInfo,
);

$paginationArgs = PaginationArgs::extractArgs($args, $this->paginationType(), $this->paginateMaxCount());

$paginationArgs->type = $this->optimalPaginationType($resolveInfo);

return $paginationArgs->applyToBuilder($query);
};
}

protected function optimalPaginationType(ResolveInfo $resolveInfo): PaginationType
{
$type = $this->paginationType();

// Already the most optimal type.
if ($type->isSimple()) {
return $type;
}

// If the result may be used in a cache, we always want to retrieve and store the full pagination data.
// Even though the query that initially creates the cache may not need additional information such as
// the total counts, following queries may need them - and use the same cached value.
$hasCacheDirective = $resolveInfo->argumentSet
->directives
->contains(static fn (Directive $directive): bool => $directive instanceof CacheDirective);
if ($hasCacheDirective) {
return $type;
}

// If the page info is not requested, we can save a database query by using the simple paginator.
// In contrast to the full pagination, it does not query total counts.
if (! isset($resolveInfo->getFieldSelection()[$type->infoFieldName()])) {
return new PaginationType(PaginationType::SIMPLE);
}

return $type;
}

protected function paginationType(): PaginationType
{
return new PaginationType(
Expand Down
50 changes: 47 additions & 3 deletions src/Pagination/PaginationArgs.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
use Illuminate\Database\Query\Builder as QueryBuilder;
use Illuminate\Support\Arr;
use Laravel\Scout\Builder as ScoutBuilder;
use Nuwave\Lighthouse\Cache\CacheDirective;
use Nuwave\Lighthouse\Execution\ResolveInfo;
use Nuwave\Lighthouse\Support\Contracts\Directive;

class PaginationArgs
{
Expand All @@ -23,10 +26,11 @@ public function __construct(
*
* @param array<string, mixed> $args
*/
public static function extractArgs(array $args, PaginationType $paginationType, ?int $paginateMaxCount): self
public static function extractArgs(array $args, ResolveInfo $resolveInfo, PaginationType $proposedPaginationType, ?int $paginateMaxCount): self
{
$first = $args['first'];
$page = $paginationType->isConnection()

$page = $proposedPaginationType->isConnection()
? self::calculateCurrentPage(
$first,
Cursor::decode($args),
Expand All @@ -50,7 +54,9 @@ public static function extractArgs(array $args, PaginationType $paginationType,
);
}

return new static($page, $first, $paginationType);
$optimalPaginationType = self::optimalPaginationType($proposedPaginationType, $resolveInfo);

return new static($page, $first, $optimalPaginationType);
}

public static function requestedLessThanZeroItems(int $amount): string
Expand All @@ -71,6 +77,32 @@ protected static function calculateCurrentPage(int $first, int $after, int $defa
: $defaultPage;
}

protected static function optimalPaginationType(PaginationType $proposedType, ResolveInfo $resolveInfo): PaginationType
{
// Already the most optimal type.
if ($proposedType->isSimple()) {
return $proposedType;
}

// If the result may be used in a cache, we always want to retrieve and store the full pagination data.
// Even though the query that initially creates the cache may not need additional information such as
// the total counts, following queries may need them - and use the same cached value.
$hasCacheDirective = $resolveInfo->argumentSet
->directives
->contains(static fn (Directive $directive): bool => $directive instanceof CacheDirective);
if ($hasCacheDirective) {
return $proposedType;
}

// If the page info is not requested, we can save a database query by using the simple paginator.
// In contrast to the full pagination, it does not query total counts.
if (! isset($resolveInfo->getFieldSelection()[$proposedType->infoFieldName()])) {
return new PaginationType(PaginationType::SIMPLE);
}

return $proposedType;
}

/**
* Apply the args to a builder, constructing a paginator.
*
Expand All @@ -82,6 +114,18 @@ protected static function calculateCurrentPage(int $first, int $after, int $defa
*/
public function applyToBuilder(QueryBuilder|ScoutBuilder|EloquentBuilder|Relation $builder): Paginator
{
if ($this->first === 0) {
if ($this->type->isSimple()) {
return new ZeroPerPagePaginator($this->page);
}

$total = $builder instanceof ScoutBuilder
? 0 // Laravel\Scout\Builder exposes no method to get the total count
: $builder->count(); // @phpstan-ignore-line see Illuminate\Database\Query\Builder::count(), available as a mixin in the other classes

return new ZeroPerPageLengthAwarePaginator($total, $this->page);
}

$methodName = $this->type->isSimple()
? 'simplePaginate'
: 'paginate';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@
use Illuminate\Pagination\LengthAwarePaginator;
use Illuminate\Support\Collection;

/**
* @extends \Illuminate\Pagination\LengthAwarePaginator<mixed>
*/
class ZeroPageLengthAwarePaginator extends LengthAwarePaginator
/** @extends \Illuminate\Pagination\LengthAwarePaginator<mixed> */
class ZeroPerPageLengthAwarePaginator extends LengthAwarePaginator
{
public function __construct(int $total, int $page)
{
Expand Down
17 changes: 17 additions & 0 deletions src/Pagination/ZeroPerPagePaginator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php declare(strict_types=1);

namespace Nuwave\Lighthouse\Pagination;

use Illuminate\Pagination\Paginator;
use Illuminate\Support\Collection;

/** @extends \Illuminate\Pagination\Paginator<mixed> */
class ZeroPerPagePaginator extends Paginator
{
public function __construct(int $page)
{
$this->perPage = 0;
$this->currentPage = $page;
$this->items = new Collection();
}
}
6 changes: 3 additions & 3 deletions src/Schema/Directives/RelationDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function resolveField(FieldValue $fieldValue): callable

return function (Model $parent, array $args, GraphQLContext $context, ResolveInfo $resolveInfo) use ($relationName) {
$decorateBuilder = $this->makeBuilderDecorator($parent, $args, $context, $resolveInfo);
$paginationArgs = $this->paginationArgs($args);
$paginationArgs = $this->paginationArgs($args, $resolveInfo);

$relation = $parent->{$relationName}();
assert($relation instanceof Relation);
Expand Down Expand Up @@ -140,12 +140,12 @@ protected function edgeType(DocumentAST $documentAST): ?ObjectTypeDefinitionNode
}

/** @param array<string, mixed> $args */
protected function paginationArgs(array $args): ?PaginationArgs
protected function paginationArgs(array $args, ResolveInfo $resolveInfo): ?PaginationArgs
{
$paginationType = $this->paginationType();

return $paginationType !== null
? PaginationArgs::extractArgs($args, $paginationType, $this->paginationMaxCount())
? PaginationArgs::extractArgs($args, $resolveInfo, $paginationType, $this->paginationMaxCount())
: null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Tracing/FederatedTracing/Proto/FieldStat.php

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Tracing/FederatedTracing/Proto/QueryLatencyStats.php

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions src/Tracing/FederatedTracing/Proto/Trace.php

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading