diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f34ce7..7e53c44 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,12 @@ Updates should follow the [Keep a CHANGELOG](http://keepachangelog.com/) princip - Nothing --> +## [1.2.0] - 2020-02-25 + +### Added + +- Fix some errors with PaginableCriteria. + ## [1.1.1] - 2020-02-16 ### Added diff --git a/composer.json b/composer.json index 23941bf..4a24034 100755 --- a/composer.json +++ b/composer.json @@ -51,7 +51,7 @@ }, "extra": { "branch-alias": { - "dev-master": "1.1-dev" + "dev-master": "1.2-dev" } }, "config": { diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 664a46e..adc473c 100755 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -12,12 +12,6 @@ parameters: paths: - src/ ignoreErrors: - - - message: '/^Parameter \#1 \$function of function call_user_func_array expects callable/' - paths: - - src/AbstractCriteriaDecorator.php - - src/Expression/ExpressionFactory.php - - src/Expression/AbstractExpressionDecorator.php - '/^Method (.*) should return static\((.*)\) but returns (.*)\.$/' - message: '/^If condition is always false\.$/' diff --git a/src/AbstractCriteriaDecorator.php b/src/AbstractCriteriaDecorator.php index 56cd2f7..454a8b4 100755 --- a/src/AbstractCriteriaDecorator.php +++ b/src/AbstractCriteriaDecorator.php @@ -44,7 +44,15 @@ public function getInnerCriteria(bool $recursive = true): CriteriaInterface */ final protected function proxyCall(string $methodName, array $arguments = []) { - return call_user_func_array([$this->criteria, $methodName], $arguments); + $callable = [$this->criteria, $methodName]; + + if (!is_callable($callable)) { + throw new \BadMethodCallException( + sprintf('Call to an undefined method %s::%s()', get_class($this->criteria), $methodName) + ); + } + + return call_user_func_array($callable, $arguments); } /** diff --git a/src/Bridge/SpiralPagination/PaginableCriteria.php b/src/Bridge/SpiralPagination/PaginableCriteria.php index 6852560..7615e70 100755 --- a/src/Bridge/SpiralPagination/PaginableCriteria.php +++ b/src/Bridge/SpiralPagination/PaginableCriteria.php @@ -10,12 +10,11 @@ use Spiral\Pagination\PaginableInterface; use Spiral\Pagination\Paginator; use Spiral\Pagination\PaginatorInterface; -use Webmozart\Assert\Assert; -class PaginableCriteria extends AbstractCriteriaDecorator implements PaginableInterface +class PaginableCriteria extends AbstractCriteriaDecorator implements PaginableInterface, PaginatorInterface { /** - * @var PaginatorInterface|Paginator|null + * @var PaginatorInterface|Paginator */ private $paginator; @@ -28,43 +27,28 @@ public function __construct(?CriteriaInterface $criteria = null, ?PaginatorInter { parent::__construct($criteria ?? new Criteria()); - if ($paginator !== null) { + if ($paginator === null) { + $this->resetPaginator(); + } else { $this->paginator = $paginator; $paginator->paginate($this); - } else { - $this->resetPaginator(); } } /** - * Getter for `paginator` property - * @return PaginatorInterface|Paginator + * Clone criteria. */ - public function getPaginator(): PaginatorInterface + public function __clone() { - Assert::notNull($this->paginator); - return $this->paginator; + $this->paginator = clone $this->paginator; } /** - * @return PaginatorInterface|Paginator + * @inheritDoc */ - private function makePaginator(): PaginatorInterface - { - $paginator = $this->paginator ?? new Paginator(); - - Assert::isInstanceOf($paginator, Paginator::class); - - $limit = $this->getLimit() ?? 25; - $tmpCount = $limit + $this->getOffset(); - $page = (int)($this->getOffset() / $limit) + 1; - - return $paginator->withCount(max($tmpCount, $paginator->count()))->withLimit($limit)->withPage($page); - } - - private function resetPaginator(): void + public function getLimit(): int { - $this->paginator = $this->makePaginator(); + return parent::getLimit() ?? 25; } /** @@ -88,12 +72,29 @@ public function offset(?int $offset): CriteriaInterface } /** - * Clone criteria + * @inheritDoc */ - public function __clone() + public function paginate(PaginableInterface $target): PaginatorInterface { - if ($this->paginator !== null) { - $this->paginator = clone $this->paginator; - } + $this->paginator = $this->paginator->paginate($target); + + return $this; + } + + /** + * Getter for `paginator` property + * @return PaginatorInterface|Paginator + */ + public function getPaginator(): PaginatorInterface + { + return $this->paginator; + } + + private function resetPaginator(): void + { + $this->paginator = (new Paginator()) + ->withLimit($this->getLimit()) + ->withPage((int)($this->getOffset() / $this->getLimit()) + 1) + ->withCount($this->paginator instanceof \Countable ? $this->paginator->count() : 0); } } diff --git a/src/Expression/AbstractExpressionDecorator.php b/src/Expression/AbstractExpressionDecorator.php index 4bcc735..60cd87f 100755 --- a/src/Expression/AbstractExpressionDecorator.php +++ b/src/Expression/AbstractExpressionDecorator.php @@ -4,7 +4,6 @@ namespace spaceonfire\Criteria\Expression; -use BadMethodCallException; use spaceonfire\Criteria\Criteria; use Symfony\Component\PropertyAccess\PropertyPath; use Webmozart\Expression\Expression; @@ -188,25 +187,37 @@ public function orX(Expression $expr): Expression * @see ExpressionFactory */ public function __call(string $name, array $arguments = []) + { + if (null !== $expr = $this->magicLogicalExpression($name, $arguments)) { + return $expr; + } + + throw new \BadMethodCallException(sprintf('Call to an undefined method %s::%s()', static::class, $name)); + } + + private function magicLogicalExpression(string $name, array $arguments): ?Expression { $isAnd = strpos($name, 'and') === 0; $isOr = strpos($name, 'or') === 0; if (!$isAnd && !$isOr) { - throw new BadMethodCallException('Call to an undefined method ' . static::class . '::' . $name . '()'); + return null; } $factoryMethod = lcfirst(substr($name, $isAnd ? 3 : 2)); - try { - if (stripos($factoryMethod, 'and') !== 0 && stripos($factoryMethod, 'or') !== 0) { - $expr = call_user_func_array([Criteria::expr(), $factoryMethod], $arguments); - return $isAnd ? $this->andX($expr) : $this->orX($expr); - } - } catch (BadMethodCallException $e) { - // skip BadMethodCallException from ExpressionFactory + if (stripos($factoryMethod, 'and') === 0 || stripos($factoryMethod, 'or') === 0) { + return null; } - throw new BadMethodCallException('Call to an undefined method ' . static::class . '::' . $name . '()'); + $factory = [Criteria::expr(), $factoryMethod]; + + if (!is_callable($factory)) { + return null; + } + + $expr = call_user_func_array($factory, $arguments); + + return $isAnd ? $this->andX($expr) : $this->orX($expr); } } diff --git a/src/Expression/ExpressionFactory.php b/src/Expression/ExpressionFactory.php index 1a00f37..4bd9386 100755 --- a/src/Expression/ExpressionFactory.php +++ b/src/Expression/ExpressionFactory.php @@ -134,44 +134,59 @@ public function property($propertyName, Expression $expr): Selector */ public function __call(string $name, array $arguments = []) { - $proxyMethods = array_flip([ - 'all', - 'andX', - 'atLeast', - 'atMost', - 'contains', - 'count', - 'endsWith', - 'equals', - 'exactly', - 'false', - 'greaterThan', - 'greaterThanEqual', - 'in', - 'isEmpty', - 'isInstanceOf', - 'keyExists', - 'keyNotExists', - 'lessThan', - 'lessThanEqual', - 'matches', - 'method', - 'not', - 'notEmpty', - 'notEquals', - 'notNull', - 'notSame', - 'null', - 'orX', - 'same', - 'startsWith', - 'true', - ]); - - if (array_key_exists($name, $proxyMethods)) { - return call_user_func_array([Expr::class, $name], $arguments); + if (null !== $expr = $this->proxyCall($name, $arguments)) { + return $expr; } throw new BadMethodCallException('Call to an undefined method ' . static::class . '::' . $name . '()'); } + + private function proxyCall(string $name, array $arguments = []): ?Expression + { + $proxyMethods = [ + 'all' => true, + 'andX' => true, + 'atLeast' => true, + 'atMost' => true, + 'contains' => true, + 'count' => true, + 'endsWith' => true, + 'equals' => true, + 'exactly' => true, + 'false' => true, + 'greaterThan' => true, + 'greaterThanEqual' => true, + 'in' => true, + 'isEmpty' => true, + 'isInstanceOf' => true, + 'keyExists' => true, + 'keyNotExists' => true, + 'lessThan' => true, + 'lessThanEqual' => true, + 'matches' => true, + 'method' => true, + 'not' => true, + 'notEmpty' => true, + 'notEquals' => true, + 'notNull' => true, + 'notSame' => true, + 'null' => true, + 'orX' => true, + 'same' => true, + 'startsWith' => true, + 'true' => true, + ]; + + if (!array_key_exists($name, $proxyMethods)) { + return null; + } + + $factory = [Expr::class, $name]; + + if (!is_callable($factory)) { + return null; + } + + return call_user_func_array($factory, $arguments) ?: null; + } } diff --git a/tests/AbstractCriteriaDecoratorTest.php b/tests/AbstractCriteriaDecoratorTest.php index feaae8a..86c02f8 100755 --- a/tests/AbstractCriteriaDecoratorTest.php +++ b/tests/AbstractCriteriaDecoratorTest.php @@ -9,6 +9,10 @@ class AbstractCriteriaDecoratorTest extends AbstractCriteriaTest private function factory(?CriteriaInterface $criteria = null): AbstractCriteriaDecorator { return new class($criteria ?? new Criteria()) extends AbstractCriteriaDecorator { + public function proxyCallUnknown(): void + { + $this->proxyCall('unknownMethodName'); + } }; } @@ -26,4 +30,11 @@ public function testGetInnerCriteria(): void self::assertEquals($innerCriteria, $outerCriteriaAdapter->getInnerCriteria()); self::assertInstanceOf(AbstractCriteriaDecorator::class, $outerCriteriaAdapter->getInnerCriteria(false)); } + + public function testProxyCallUnknownMethod(): void + { + $this->expectException(\BadMethodCallException::class); + $criteria = $this->factory(); + $criteria->proxyCallUnknown(); + } } diff --git a/tests/Bridge/SpiralPagination/PaginableCriteriaTest.php b/tests/Bridge/SpiralPagination/PaginableCriteriaTest.php index cb65b8f..f565d21 100755 --- a/tests/Bridge/SpiralPagination/PaginableCriteriaTest.php +++ b/tests/Bridge/SpiralPagination/PaginableCriteriaTest.php @@ -6,6 +6,7 @@ use spaceonfire\Criteria\AbstractCriteriaTest; use spaceonfire\Criteria\CriteriaInterface; +use Spiral\Pagination\PaginableInterface; use Spiral\Pagination\Paginator; class PaginableCriteriaTest extends AbstractCriteriaTest @@ -36,7 +37,13 @@ public function testOffset(): void self::assertEquals(25, $this->criteria->getPaginator()->getLimit()); self::assertEquals(0, $this->criteria->getPaginator()->getOffset()); - parent::testOffset(); + self::assertEquals(0, $this->criteria->getOffset()); + $this->criteria->offset(25); + self::assertEquals(25, $this->criteria->getOffset()); + + $query = $this->makeQuery(75); + + $this->criteria->paginate($query); self::assertEquals(2, $this->criteria->getPaginator()->getPage()); self::assertEquals(25, $this->criteria->getPaginator()->getLimit()); @@ -49,10 +56,50 @@ public function testLimit(): void self::assertEquals(25, $this->criteria->getPaginator()->getLimit()); self::assertEquals(0, $this->criteria->getPaginator()->getOffset()); - parent::testLimit(); + self::assertEquals(25, $this->criteria->getLimit()); + $this->criteria->limit(50); + self::assertEquals(50, $this->criteria->getLimit()); self::assertEquals(1, $this->criteria->getPaginator()->getPage()); - self::assertEquals(25, $this->criteria->getPaginator()->getLimit()); + self::assertEquals(50, $this->criteria->getPaginator()->getLimit()); self::assertEquals(0, $this->criteria->getPaginator()->getOffset()); } + + public function testPaginate(): void + { + $query = $this->makeQuery(75); + + $this->criteria->paginate($query); + + self::assertSame(3, $this->criteria->getPaginator()->countPages()); + } + + private function makeQuery(int $count) + { + return new class($count) implements PaginableInterface, \Countable { + public $limit = 0; + public $offset = 0; + public $count; + + public function __construct(int $count) + { + $this->count = $count; + } + + public function count(): int + { + return $this->count; + } + + public function limit(int $limit): void + { + $this->limit = $limit; + } + + public function offset(int $offset): void + { + $this->offset = $offset; + } + }; + } }