From 573ca2e0e348a7fe573a3e8fbc29a6588ece8c4e Mon Sep 17 00:00:00 2001 From: Korvin Szanto Date: Fri, 14 Feb 2025 13:33:14 -0800 Subject: [PATCH] Fix deprecated errors in tests (#575) * Fix deprecated errors in tests * Safely count all possible types * Support iterable type fully in newer pagerfanta versions * Use psalm 4.30 --- .github/workflows/run-tests.yml | 2 +- .github/workflows/static.yml | 2 +- .gitignore | 3 +- composer.json | 8 +- phpstan.neon.dist | 2 + psalm.xml | 1 + src/Manager.php | 4 +- src/Pagination/DoctrinePaginatorAdapter.php | 6 +- src/Pagination/LaminasPaginatorAdapter.php | 95 +++++++++++++++++++ src/Pagination/PagerfantaPaginatorAdapter.php | 4 +- src/Pagination/PaginatorCountTrait.php | 42 ++++++++ .../DoctrinePaginatorAdapterTest.php | 20 +++- .../LaminasFrameworkPaginatorAdapterTest.php | 53 +++++++++++ test/Pagination/PaginatorCountTraitTest.php | 60 ++++++++++++ .../ZendFrameworkPaginatorAdapterTest.php | 6 ++ test/Serializer/DataArraySerializerTest.php | 9 -- test/Stub/SimpleTraversable.php | 43 +++++++++ test/phpstan.php | 4 + 18 files changed, 340 insertions(+), 24 deletions(-) create mode 100644 src/Pagination/LaminasPaginatorAdapter.php create mode 100644 src/Pagination/PaginatorCountTrait.php create mode 100644 test/Pagination/LaminasFrameworkPaginatorAdapterTest.php create mode 100644 test/Pagination/PaginatorCountTraitTest.php create mode 100644 test/Stub/SimpleTraversable.php create mode 100644 test/phpstan.php diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index f01fdbe3..bc1d5106 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -9,7 +9,7 @@ jobs: fail-fast: true matrix: os: [ubuntu-20.04] - php: [7.4, 8.0, 8.1] + php: [7.4, 8.0, 8.1, 8.2, 8.3, 8.4] name: League - PHP ${{ matrix.php }} on ${{ matrix.os }} diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml index 328a6c8f..6d5655ac 100644 --- a/.github/workflows/static.yml +++ b/.github/workflows/static.yml @@ -37,7 +37,7 @@ jobs: php-version: 8.1 extensions: apcu, redis coverage: none - tools: vimeo/psalm:4.22.0 + tools: vimeo/psalm:4.30.0 - name: Download dependencies uses: ramsey/composer-install@v1 diff --git a/.gitignore b/.gitignore index 1cfd6e80..199a062b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ composer.lock build vendor -.phpunit.result.cache \ No newline at end of file +.phpunit.result.cache +.idea diff --git a/composer.json b/composer.json index 540a7c92..9a449cd2 100644 --- a/composer.json +++ b/composer.json @@ -26,18 +26,18 @@ "require-dev": { "doctrine/orm": "^2.5", "illuminate/contracts": "~5.0", + "laminas/laminas-paginator": "~2.12", "mockery/mockery": "^1.3", - "pagerfanta/pagerfanta": "~1.0.0", + "pagerfanta/pagerfanta": "~1.0.0|~4.0.0", "phpstan/phpstan": "^1.4", "phpunit/phpunit": "^9.5", "squizlabs/php_codesniffer": "~3.4", - "vimeo/psalm": "^4.22", - "zendframework/zend-paginator": "~2.3" + "vimeo/psalm": "^4.30" }, "suggest": { "illuminate/pagination": "The Illuminate Pagination component.", "pagerfanta/pagerfanta": "Pagerfanta Paginator", - "zendframework/zend-paginator": "Zend Framework Paginator" + "laminas/laminas-paginator": "Laminas Framework Paginator" }, "autoload": { "psr-4": { diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 1b347546..4a6d98d5 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -6,3 +6,5 @@ parameters: reportUnmatchedIgnoredErrors: false paths: - src + bootstrapFiles: + - test/phpstan.php diff --git a/psalm.xml b/psalm.xml index f03e94f8..1647ee9e 100644 --- a/psalm.xml +++ b/psalm.xml @@ -8,6 +8,7 @@ errorBaseline="psalm.baseline.xml" > + diff --git a/src/Manager.php b/src/Manager.php index 0320bb1e..8ccff24a 100644 --- a/src/Manager.php +++ b/src/Manager.php @@ -61,7 +61,7 @@ class Manager */ private ScopeFactoryInterface $scopeFactory; - public function __construct(ScopeFactoryInterface $scopeFactory = null) + public function __construct(?ScopeFactoryInterface $scopeFactory = null) { $this->scopeFactory = $scopeFactory ?: new ScopeFactory(); } @@ -72,7 +72,7 @@ public function __construct(ScopeFactoryInterface $scopeFactory = null) public function createData( ResourceInterface $resource, ?string $scopeIdentifier = null, - Scope $parentScopeInstance = null + ?Scope $parentScopeInstance = null ): Scope { if ($parentScopeInstance !== null) { return $this->scopeFactory->createChildScopeFor($this, $parentScopeInstance, $resource, $scopeIdentifier); diff --git a/src/Pagination/DoctrinePaginatorAdapter.php b/src/Pagination/DoctrinePaginatorAdapter.php index bcab596a..30eed478 100644 --- a/src/Pagination/DoctrinePaginatorAdapter.php +++ b/src/Pagination/DoctrinePaginatorAdapter.php @@ -19,6 +19,8 @@ */ class DoctrinePaginatorAdapter implements PaginatorInterface { + use PaginatorCountTrait; + /** * The paginator instance. * @var Paginator @@ -73,7 +75,7 @@ public function getTotal(): int */ public function getCount(): int { - return $this->paginator->getIterator()->count(); + return $this->getTraversableCount($this->paginator->getIterator()); } /** @@ -93,7 +95,7 @@ public function getUrl(int $page): string } /** - * Get the the route generator. + * Get the route generator. */ private function getRouteGenerator(): callable { diff --git a/src/Pagination/LaminasPaginatorAdapter.php b/src/Pagination/LaminasPaginatorAdapter.php new file mode 100644 index 00000000..bc427681 --- /dev/null +++ b/src/Pagination/LaminasPaginatorAdapter.php @@ -0,0 +1,95 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace League\Fractal\Pagination; + +use Laminas\Paginator\Paginator; + +/** + * A paginator adapter for laminas/laminas-paginator. + * + * @author Abdul Malik Ikhsan + */ +class LaminasPaginatorAdapter implements PaginatorInterface +{ + protected Paginator $paginator; + + /** + * The route generator. + * + * @var callable + */ + protected $routeGenerator; + + public function __construct(Paginator $paginator, callable $routeGenerator) + { + $this->paginator = $paginator; + $this->routeGenerator = $routeGenerator; + } + + /** + * {@inheritDoc} + */ + public function getCurrentPage(): int + { + return $this->paginator->getCurrentPageNumber(); + } + + /** + * {@inheritDoc} + */ + public function getLastPage(): int + { + return $this->paginator->count(); + } + + /** + * {@inheritDoc} + */ + public function getTotal(): int + { + return $this->paginator->getTotalItemCount(); + } + + /** + * {@inheritDoc} + */ + public function getCount(): int + { + return $this->paginator->getCurrentItemCount(); + } + + /** + * {@inheritDoc} + */ + public function getPerPage(): int + { + return $this->paginator->getItemCountPerPage(); + } + + /** + * {@inheritDoc} + */ + public function getUrl(int $page): string + { + return call_user_func($this->routeGenerator, $page); + } + + public function getPaginator(): Paginator + { + return $this->paginator; + } + + public function getRouteGenerator(): callable + { + return $this->routeGenerator; + } +} diff --git a/src/Pagination/PagerfantaPaginatorAdapter.php b/src/Pagination/PagerfantaPaginatorAdapter.php index 9f593c33..9b7a2380 100644 --- a/src/Pagination/PagerfantaPaginatorAdapter.php +++ b/src/Pagination/PagerfantaPaginatorAdapter.php @@ -20,6 +20,8 @@ */ class PagerfantaPaginatorAdapter implements PaginatorInterface { + use PaginatorCountTrait; + protected Pagerfanta $paginator; /** @@ -64,7 +66,7 @@ public function getTotal(): int */ public function getCount(): int { - return count($this->paginator->getCurrentPageResults()); + return $this->getIterableCount($this->paginator->getCurrentPageResults()); } /** diff --git a/src/Pagination/PaginatorCountTrait.php b/src/Pagination/PaginatorCountTrait.php new file mode 100644 index 00000000..384ae4b2 --- /dev/null +++ b/src/Pagination/PaginatorCountTrait.php @@ -0,0 +1,42 @@ +getTraversableCount($iterable); + } + + return count($iterable); + } + + /** + * Safely get the count from a traversable + */ + private function getTraversableCount(\Traversable $traversable): int + { + if ($traversable instanceof \Countable) { + return count($traversable); + } + + // Call the "count" method if it exists + if (method_exists($traversable, 'count')) { + return $traversable->count(); + } + + // If not, fall back to iterator_count and rewind if possible + $count = iterator_count($traversable); + if ($traversable instanceof \Iterator || $traversable instanceof \IteratorAggregate) { + $traversable->rewind(); + } + + return $count; + } + +} diff --git a/test/Pagination/DoctrinePaginatorAdapterTest.php b/test/Pagination/DoctrinePaginatorAdapterTest.php index 26ba291e..91202672 100644 --- a/test/Pagination/DoctrinePaginatorAdapterTest.php +++ b/test/Pagination/DoctrinePaginatorAdapterTest.php @@ -2,7 +2,9 @@ namespace League\Fractal\Test\Pagination; use Doctrine\ORM\Query; +use Doctrine\ORM\Tools\Pagination\Paginator; use League\Fractal\Pagination\DoctrinePaginatorAdapter; +use League\Fractal\Test\Stub\SimpleTraversable; use Mockery; use PHPUnit\Framework\TestCase; @@ -28,11 +30,9 @@ public function testPaginationAdapter() $paginator->shouldReceive('getQuery')->andReturn($query); //Mock the iterator of the paginator - $iterator = Mockery::mock('IteratorAggregate'); - $iterator->shouldReceive('count')->andReturn($count); + $iterator = new \ArrayIterator(range(1, $count)); $paginator->shouldReceive('getIterator')->andReturn($iterator); - $adapter = new DoctrinePaginatorAdapter($paginator, function ($page) { return 'http://example.com/foo?page='.$page; }); @@ -57,6 +57,20 @@ public function testPaginationAdapter() ); } + public function testCountingTraversables() + { + $traversable = new SimpleTraversable(range(1, 100)); + $adapter = Mockery::mock('Doctrine\ORM\Tools\Pagination\Paginator'); + $adapter->shouldReceive('getIterator')->andReturn($traversable); + $adapter = new DoctrinePaginatorAdapter($adapter, function ($page) { + return (string) $page; + }); + + $this->assertEquals($traversable->key(), 0); + $this->assertEquals($adapter->getCount(), 100); + $this->assertEquals($traversable->key(), 0); + } + public function tearDown(): void { Mockery::close(); diff --git a/test/Pagination/LaminasFrameworkPaginatorAdapterTest.php b/test/Pagination/LaminasFrameworkPaginatorAdapterTest.php new file mode 100644 index 00000000..62d3de5d --- /dev/null +++ b/test/Pagination/LaminasFrameworkPaginatorAdapterTest.php @@ -0,0 +1,53 @@ +makePartial(); + + $total = 50; + $count = 10; + $perPage = 10; + $currentPage = 2; + $lastPage = 5; + + $paginator = Mockery::mock('Laminas\Paginator\Paginator', [$adapter])->makePartial(); + + $paginator->shouldReceive('getCurrentPageNumber')->andReturn($currentPage); + $paginator->shouldReceive('count')->andReturn($lastPage); + $paginator->shouldReceive('getItemCountPerPage')->andReturn($perPage); + + $adapter = new LaminasPaginatorAdapter($paginator, function ($page) { + return 'http://example.com/foo?page='.$page; + }); + + $this->assertInstanceOf('League\Fractal\Pagination\PaginatorInterface', $adapter); + + $this->assertSame($currentPage, $adapter->getCurrentPage()); + $this->assertSame($lastPage, $adapter->getLastPage()); + $this->assertSame($count, $adapter->getCount()); + $this->assertSame($total, $adapter->getTotal()); + $this->assertSame($perPage, $adapter->getPerPage()); + $this->assertSame('http://example.com/foo?page=1', $adapter->getUrl(1)); + $this->assertSame('http://example.com/foo?page=3', $adapter->getUrl(3)); + } + + public function tearDown(): void + { + Mockery::close(); + } +} diff --git a/test/Pagination/PaginatorCountTraitTest.php b/test/Pagination/PaginatorCountTraitTest.php new file mode 100644 index 00000000..53fa9e55 --- /dev/null +++ b/test/Pagination/PaginatorCountTraitTest.php @@ -0,0 +1,60 @@ +instance = new class() { + use PaginatorCountTrait; + + public function getIterableCountPublic(iterable $iterable) + { + return $this->getIterableCount($iterable); + } + + public function getTraversableCountPublic(\Traversable $traversable) + { + return $this->getTraversableCount($traversable); + } + }; + } + + /** + * @dataProvider arrayProvider + */ + public function testSupportsIterables(array $data) + { + $count = count($data); + $this->assertEquals($count, $this->instance->getIterableCountPublic($data)); + $this->assertEquals($count, $this->instance->getIterableCountPublic(new \ArrayIterator($data))); + $this->assertEquals($count, $this->instance->getIterableCountPublic(new SimpleTraversable($data))); + } + + /** + * @dataProvider arrayProvider + */ + public function testSupportsTraversables(array $data) + { + $count = count($data); + $this->assertEquals($count, $this->instance->getTraversableCountPublic(new \ArrayIterator($data))); + $this->assertEquals($count, $this->instance->getTraversableCountPublic(new SimpleTraversable($data))); + } + + public function arrayProvider() + { + return [ + [[]], + [[1, 2, 3]], + [range(1, 100)], + ]; + } +} diff --git a/test/Pagination/ZendFrameworkPaginatorAdapterTest.php b/test/Pagination/ZendFrameworkPaginatorAdapterTest.php index f3574d67..872d666b 100644 --- a/test/Pagination/ZendFrameworkPaginatorAdapterTest.php +++ b/test/Pagination/ZendFrameworkPaginatorAdapterTest.php @@ -7,6 +7,12 @@ class ZendFrameworkPaginatorAdapterTest extends TestCase { + public static function setUpBeforeClass(): void + { + class_alias(\Laminas\Paginator\Adapter\ArrayAdapter::class, \Zend\Paginator\Adapter\ArrayAdapter::class); + class_alias(\Laminas\Paginator\Paginator::class, \Zend\Paginator\Paginator::class); + } + public function testPaginationAdapter() { $items = [ diff --git a/test/Serializer/DataArraySerializerTest.php b/test/Serializer/DataArraySerializerTest.php index 8e9eb88e..9293589f 100644 --- a/test/Serializer/DataArraySerializerTest.php +++ b/test/Serializer/DataArraySerializerTest.php @@ -357,15 +357,6 @@ public function testSerializingNullResource() $this->assertSame($expected, $scope->toArray()); } - public function testCanPassNullValueToSerializer() - { - $testClass = new \stdClass(); - $testClass->name = 'test'; - $testClass->email = 'test@test.com'; - - - } - public function tearDown(): void { Mockery::close(); diff --git a/test/Stub/SimpleTraversable.php b/test/Stub/SimpleTraversable.php new file mode 100644 index 00000000..edefe2e1 --- /dev/null +++ b/test/Stub/SimpleTraversable.php @@ -0,0 +1,43 @@ +list = array_values($list); + $this->keys = array_keys($list); + } + + #[\ReturnTypeWillChange] + public function current() + { + return $this->list[$this->cursor]; + } + + public function next(): void + { + $this->cursor++; + } + + #[\ReturnTypeWillChange] + public function key() + { + return $this->keys[$this->cursor]; + } + + public function valid(): bool + { + return isset($this->list[$this->cursor]); + } + + public function rewind(): void + { + $this->cursor = 0; + } +} diff --git a/test/phpstan.php b/test/phpstan.php new file mode 100644 index 00000000..e0c4e65c --- /dev/null +++ b/test/phpstan.php @@ -0,0 +1,4 @@ +