Merge branch '3.next' into 4.x

src/Cache/Engine/RedisEngine.php

@@ -257,7 +257,7 @@ class RedisEngine extends CacheEngine
      * @param string $key Identifier for the data.
      * @param mixed $value Data to be cached.
      * @return bool True if the data was successfully cached, false on failure.
-     * @link https://github.com/phpredis/phpredis#setnx
+     * @link https://github.com/phpredis/phpredis#set
      */
     public function add(string $key, $value): bool
     {
@@ -265,9 +265,8 @@ class RedisEngine extends CacheEngine
         $key = $this->_key($key);
         $value = $this->serialize($value);
 
-        // setNx() doesn't have an expiry option, so follow up with an expiry
-        if ($this->_Redis->setNx($key, $value)) {
-            return $this->_Redis->expire($key, $duration);
+        if ($this->_Redis->set($key, $value, ['nx', 'ex' => $duration])) {
+            return true;
         }
 
         return false;

src/Database/Query.php

@@ -1536,7 +1536,7 @@ class Query implements ExpressionInterface, IteratorAggregate
      * ### Example
      *
      * ```
-     * $query->newExp()->lte('count', $query->identifier('total'));
+     * $query->newExpr()->lte('count', $query->identifier('total'));
      * ```
      *
      * @param string $identifier The identifier for an expression

src/Database/SqlDialectTrait.php

@@ -224,20 +224,30 @@ trait SqlDialectTrait
 
         $conditions = $query->clause('where');
         if ($conditions) {
-            $conditions->traverse(function ($condition) {
-                if (!($condition instanceof Comparison)) {
-                    return $condition;
-                }
+            $conditions->traverse(function ($expression) {
+                if ($expression instanceof Comparison) {
+                    $field = $expression->getField();
+                    if (is_string($field) &&
+                        strpos($field, '.') !== false
+                    ) {
+                        [, $unaliasedField] = explode('.', $field, 2);
+                        $expression->setField($unaliasedField);
+                    }
 
-                $field = $condition->getField();
-                if ($field instanceof ExpressionInterface || strpos($field, '.') === false) {
-                    return $condition;
+                    return $expression;
                 }
 
-                [, $field] = explode('.', $field);
-                $condition->setField($field);
+                if ($expression instanceof IdentifierExpression) {
+                    $identifier = $expression->getIdentifier();
+                    if (strpos($identifier, '.') !== false) {
+                        [, $unaliasedIdentifier] = explode('.', $identifier, 2);
+                        $expression->setIdentifier($unaliasedIdentifier);
+                    }
+
+                    return $expression;
+                }
 
-                return $condition;
+                return $expression;
             });
         }
 

src/Datasource/Paginator.php

@@ -183,7 +183,7 @@ class Paginator implements PaginatorInterface
 
         $pagingParams = $this->buildParams($data);
         $alias = $object->getAlias();
-        $this->_pagingParams = [$alias => $this->buildParams($data)];
+        $this->_pagingParams = [$alias => $pagingParams];
         if ($pagingParams['requestedPage'] > $pagingParams['page']) {
             throw new PageOutOfBoundsException([
                 'requestedPage' => $pagingParams['requestedPage'],
@@ -218,9 +218,9 @@ class Paginator implements PaginatorInterface
      *
      * @param \Cake\Datasource\QueryInterface $query Query instance.
      * @param array $data Pagination data.
-     * @return int
+     * @return int|null
      */
-    protected function getCount(QueryInterface $query, array $data): int
+    protected function getCount(QueryInterface $query, array $data): ?int
     {
         return $query->count();
     }
@@ -257,52 +257,123 @@ class Paginator implements PaginatorInterface
      */
     protected function buildParams(array $data): array
     {
-        $defaults = $data['defaults'];
-        $count = $data['count'];
-        $page = $data['options']['page'];
         $limit = $data['options']['limit'];
-        $pageCount = max((int)ceil($count / $limit), 1);
-        $requestedPage = $page;
-        $page = min($page, $pageCount);
 
+        $paging = [
+            'count' => $data['count'],
+            'current' => $data['numResults'],
+            'perPage' => $limit,
+            'page' => $data['options']['page'],
+            'requestedPage' => $data['options']['page'],
+        ];
+
+        $paging = $this->addPageCountParams($paging, $data);
+        $paging = $this->addStartEndParams($paging, $data);
+        $paging = $this->addPrevNextParams($paging, $data);
+        $paging = $this->addSortingParams($paging, $data);
+
+        $paging += [
+            'limit' => $data['defaults']['limit'] != $limit ? $limit : null,
+            'scope' => $data['options']['scope'],
+            'finder' => $data['finder'],
+        ];
+
+        return $paging;
+    }
+
+    /**
+     * Add "page" and "pageCount" params.
+     *
+     * @param array $params Paging params.
+     * @param array $data Paginator data.
+     * @return array Updated params.
+     */
+    protected function addPageCountParams(array $params, array $data)
+    {
+        $page = $params['page'];
+        $pageCount = 0;
+
+        if ($params['count'] !== null) {
+            $pageCount = max((int)ceil($params['count'] / $params['perPage']), 1);
+            $page = min($page, $pageCount);
+        } elseif ($params['current'] === 0 && $params['requestedPage'] > 1) {
+            $page = 1;
+        }
+
+        $params['page'] = $page;
+        $params['pageCount'] = $pageCount;
+
+        return $params;
+    }
+
+    /**
+     * Add "start" and "end" params.
+     *
+     * @param array $params Paging params.
+     * @param array $data Paginator data.
+     * @return array Updated params.
+     */
+    protected function addStartEndParams(array $params, array $data)
+    {
+        $start = $end = 0;
+
+        if ($params['current'] > 0) {
+            $start = (($params['page'] - 1) * $params['perPage']) + 1;
+            $end = $start + $params['current'] - 1;
+        }
+
+        $params['start'] = $start;
+        $params['end'] = $end;
+
+        return $params;
+    }
+
+    /**
+     * Add "prevPage" and "nextPage" params.
+     *
+     * @param array $params Paginator params.
+     * @param array $data Paging data.
+     * @return array Updated params.
+     */
+    protected function addPrevNextParams(array $params, array $data)
+    {
+        $params['prevPage'] = $params['page'] > 1;
+        if ($params['count'] === null) {
+            $params['nextPage'] = true;
+        } else {
+            $params['nextPage'] = $params['count'] > ($params['page'] * $params['perPage']);
+        }
+
+        return $params;
+    }
+
+    /**
+     * Add sorting / ordering params.
+     *
+     * @param array $params Paginator params.
+     * @param array $data Paging data.
+     * @return array Updated params.
+     */
+    protected function addSortingParams(array $params, array $data)
+    {
+        $defaults = $data['defaults'];
         $order = (array)$data['options']['order'];
         $sortDefault = $directionDefault = false;
+
         if (!empty($defaults['order']) && count($defaults['order']) === 1) {
             $sortDefault = key($defaults['order']);
             $directionDefault = current($defaults['order']);
         }
 
-        $start = 0;
-        if ($count >= 1) {
-            $start = (($page - 1) * $limit) + 1;
-        }
-        $end = $start + $limit - 1;
-        if ($count < $end) {
-            $end = $count;
-        }
-
-        $paging = [
-            'finder' => $data['finder'],
-            'requestedPage' => $requestedPage,
-            'page' => $page,
-            'current' => $data['numResults'],
-            'count' => $count,
-            'perPage' => $limit,
-            'start' => $start,
-            'end' => $end,
-            'prevPage' => $page > 1,
-            'nextPage' => $count > ($page * $limit),
-            'pageCount' => $pageCount,
+        $params += [
             'sort' => $data['options']['sort'],
             'direction' => isset($data['options']['sort']) ? current($order) : null,
-            'limit' => $defaults['limit'] !== $limit ? $limit : null,
             'sortDefault' => $sortDefault,
             'directionDefault' => $directionDefault,
-            'scope' => $data['options']['scope'],
             'completeSort' => $order,
         ];
 
-        return $paging;
+        return $params;
     }
 
     /**

src/Datasource/SimplePaginator.php

@@ -0,0 +1,38 @@
+<?php
+/**
+ * CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @link          http://cakephp.org CakePHP(tm) Project
+ * @since         3.9.0
+ * @license       http://www.opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Datasource;
+
+/**
+ * Simplified paginator which avoids potentially expensives queries
+ * to get the total count of records.
+ *
+ * When using a simple paginator you will not be able to generate page numbers.
+ * Instead use only the prev/next pagination controls, and handle 404 errors
+ * when pagination goes past the available result set.
+ */
+class SimplePaginator extends Paginator
+{
+    /**
+     * Simple pagination does not perform any count query, so this method returns `null`.
+     *
+     * @param \Cake\Datasource\QueryInterface $query Query instance.
+     * @param array $data Pagination data.
+     * @return int|null
+     */
+    protected function getCount(QueryInterface $query, array $data): ?int
+    {
+        return null;
+    }
+}

src/TestSuite/Fixture/FixtureManager.php

@@ -100,7 +100,6 @@ class FixtureManager
         if (empty($test->fixtures) || !empty($this->_processed[get_class($test)])) {
             return;
         }
-
         $this->_loadFixtures($test);
         $this->_processed[get_class($test)] = true;
     }

src/TestSuite/TestCase.php

@@ -164,6 +164,7 @@ abstract class TestCase extends BaseTestCase
         $this->getTableLocator()->clear();
         $this->_configure = [];
         $this->_tableLocator = null;
+        $this->fixtureManager = null;
     }
 
     /**

src/View/Helper/PaginatorHelper.php

@@ -715,7 +715,7 @@ class PaginatorHelper extends Helper
      * @return string Counter string.
      * @link https://book.cakephp.org/3.0/en/views/helpers/paginator.html#creating-a-page-counter
      */
-    public function counter(string $format, array $options = []): string
+    public function counter(string $format = 'pages', array $options = []): string
     {
         $options += [
             'model' => $this->defaultModel(),

tests/TestCase/Console/ConsoleIoTest.php

@@ -33,6 +33,21 @@ class ConsoleIoTest extends TestCase
     protected $io;
 
     /**
+     * @var \Cake\Console\ConsoleOutput|\PHPUnit\Framework\MockObject\MockObject
+     */
+    protected $out;
+
+    /**
+     * @var \Cake\Console\ConsoleOutput|\PHPUnit\Framework\MockObject\MockObject
+     */
+    protected $err;
+
+    /**
+     * @var \Cake\Console\ConsoleInput|\PHPUnit\Framework\MockObject\MockObject
+     */
+    protected $in;
+
+    /**
      * setUp method
      *
      * @return void
@@ -294,12 +309,16 @@ class ConsoleIoTest extends TestCase
             ->method('write')
             ->with('<error>Some error</error>', 1);
 
+        $this->expectException(StopException::class);
+        $this->expectExceptionCode(1);
+        $this->expectExceptionMessage('Some error');
+
         $this->io->abort('Some error');
     }
 
     /**
      * Tests abort() wrapper.
-
+     *
      * @return void
      */
     public function testAbortCustomCode()
@@ -312,6 +331,10 @@ class ConsoleIoTest extends TestCase
             ->method('write')
             ->with('<error>Some error</error>', 1);
 
+        $this->expectException(StopException::class);
+        $this->expectExceptionCode(99);
+        $this->expectExceptionMessage('Some error');
+
         $this->io->abort('Some error', 99);
     }
 

tests/TestCase/Database/QueryTest.php

@@ -2864,6 +2864,47 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Tests that aliases are stripped from delete query conditions
+     * where possible.
+     *
+     * @return void
+     */
+    public function testDeleteStripAliasesFromConditions()
+    {
+        $query = new Query($this->connection);
+
+        $query
+            ->delete('authors')
+            ->where([
+                'OR' => [
+                    'a.id' => 1,
+                    'a.name IS' => null,
+                    'a.email IS NOT' => null,
+                    'AND' => [
+                        'b.name NOT IN' => ['foo', 'bar'],
+                        'OR' => [
+                            $query->newExpr()->eq(new IdentifierExpression('c.name'), 'zap'),
+                            'd.name' => 'baz',
+                            (new Query($this->connection))->select(['e.name'])->where(['e.name' => 'oof'])
+                        ]
+                    ]
+                ],
+            ]);
+
+        $this->assertQuotedQuery(
+            'DELETE FROM <authors> WHERE \(' .
+                '<id> = :c0 OR \(<name>\) IS NULL OR \(<email>\) IS NOT NULL OR \(' .
+                    '<name> not in \(:c1,:c2\) AND \(' .
+                        '\(<name>\) = :c3 OR <name> = :c4 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c5\)' .
+                    '\)' .
+                '\)' .
+            '\)',
+            $query->sql(),
+            !$this->autoQuote
+        );
+    }
+
+    /**
      * Test setting select() & delete() modes.
      *
      * @return void
@@ -3076,6 +3117,8 @@ class QueryTest extends TestCase
             ->where([
                 'OR' => [
                     'a.id' => 1,
+                    'a.name IS' => null,
+                    'a.email IS NOT' => null,
                     'AND' => [
                         'b.name NOT IN' => ['foo', 'bar'],
                         'OR' => [
@@ -3089,9 +3132,9 @@ class QueryTest extends TestCase
 
         $this->assertQuotedQuery(
             'UPDATE <authors> SET <name> = :c0 WHERE \(' .
-                '<id> = :c1 OR \(' .
+                '<id> = :c1 OR \(<name>\) IS NULL OR \(<email>\) IS NOT NULL OR \(' .
                     '<name> not in \(:c2,:c3\) AND \(' .
-                        '\(<c>\.<name>\) = :c4 OR <name> = :c5 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c6\)' .
+                        '\(<name>\) = :c4 OR <name> = :c5 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c6\)' .
                     '\)' .
                 '\)' .
             '\)',
@@ -3838,6 +3881,14 @@ class QueryTest extends TestCase
         $query = new Query($this->connection);
         $sql = $query->select('*')->join(['foo' => $query->newExpr('bar')])->sql();
         $this->assertQuotedQuery('JOIN \(bar\) <foo>', $sql);
+
+        $query = new Query($this->connection);
+        $sql = $query->select('*')->join([
+            'alias' => 'orders',
+            'table' => 'Order',
+            'conditions' => ['1 = 1'],
+        ])->sql();
+        $this->assertQuotedQuery('JOIN <Order> <orders> ON 1 = 1', $sql);
     }
 
     /**

tests/TestCase/Datasource/SimplePaginatorTest.php

@@ -0,0 +1,177 @@
+<?php
+/**
+ * CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @link          http://cakephp.org CakePHP(tm) Project
+ * @since         3.9.0
+ * @license       http://www.opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Test\TestCase\Datasource;
+
+use Cake\Core\Configure;
+use Cake\Datasource\SimplePaginator;
+use Cake\ORM\Entity;
+
+class SimplePaginatorTest extends PaginatorTest
+{
+    /**
+     * fixtures property
+     *
+     * @var array
+     */
+    public $fixtures = [
+        'core.Posts', 'core.Articles', 'core.ArticlesTags',
+        'core.Authors', 'core.AuthorsTags', 'core.Tags'
+    ];
+
+    /**
+     * Don't load data for fixtures for all tests
+     *
+     * @var bool
+     */
+    public $autoFixtures = false;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        Configure::write('App.namespace', 'TestApp');
+
+        $this->Paginator = new SimplePaginator();
+
+        $this->Post = $this->getMockRepository();
+    }
+
+    /**
+     * test paginate() and custom find, to make sure the correct count is returned.
+     *
+     * @return void
+     */
+    public function testPaginateCustomFind()
+    {
+        $this->loadFixtures('Posts');
+        $titleExtractor = function ($result) {
+            $ids = [];
+            foreach ($result as $record) {
+                $ids[] = $record->title;
+            }
+
+            return $ids;
+        };
+
+        $table = $this->getTableLocator()->get('PaginatorPosts');
+        $data = ['author_id' => 3, 'title' => 'Fourth Post', 'body' => 'Article Body, unpublished', 'published' => 'N'];
+        $result = $table->save(new Entity($data));
+        $this->assertNotEmpty($result);
+
+        $result = $this->Paginator->paginate($table);
+        $this->assertCount(4, $result, '4 rows should come back');
+        $this->assertEquals(['First Post', 'Second Post', 'Third Post', 'Fourth Post'], $titleExtractor($result));
+
+        $pagingParams = $this->Paginator->getPagingParams();
+        $this->assertEquals(4, $pagingParams['PaginatorPosts']['current']);
+        $this->assertNull($pagingParams['PaginatorPosts']['count']);
+
+        $settings = ['finder' => 'published'];
+        $result = $this->Paginator->paginate($table, [], $settings);
+        $this->assertCount(3, $result, '3 rows should come back');
+        $this->assertEquals(['First Post', 'Second Post', 'Third Post'], $titleExtractor($result));
+
+        $pagingParams = $this->Paginator->getPagingParams();
+        $this->assertEquals(3, $pagingParams['PaginatorPosts']['current']);
+        $this->assertNull($pagingParams['PaginatorPosts']['count']);
+
+        $settings = ['finder' => 'published', 'limit' => 2, 'page' => 2];
+        $result = $this->Paginator->paginate($table, [], $settings);
+        $this->assertCount(1, $result, '1 rows should come back');
+        $this->assertEquals(['Third Post'], $titleExtractor($result));
+
+        $pagingParams = $this->Paginator->getPagingParams();
+        $this->assertEquals(1, $pagingParams['PaginatorPosts']['current']);
+        $this->assertNull($pagingParams['PaginatorPosts']['count']);
+        $this->assertSame(0, $pagingParams['PaginatorPosts']['pageCount']);
+
+        $settings = ['finder' => 'published', 'limit' => 2];
+        $result = $this->Paginator->paginate($table, [], $settings);
+        $this->assertCount(2, $result, '2 rows should come back');
+        $this->assertEquals(['First Post', 'Second Post'], $titleExtractor($result));
+
+        $pagingParams = $this->Paginator->getPagingParams();
+        $this->assertEquals(2, $pagingParams['PaginatorPosts']['current']);
+        $this->assertNull($pagingParams['PaginatorPosts']['count']);
+        $this->assertEquals(0, $pagingParams['PaginatorPosts']['pageCount']);
+        $this->assertTrue($pagingParams['PaginatorPosts']['nextPage']);
+        $this->assertFalse($pagingParams['PaginatorPosts']['prevPage']);
+        $this->assertEquals(2, $pagingParams['PaginatorPosts']['perPage']);
+        $this->assertNull($pagingParams['PaginatorPosts']['limit']);
+    }
+
+    /**
+     * test paginate() and custom find with fields array, to make sure the correct count is returned.
+     *
+     * @return void
+     */
+    public function testPaginateCustomFindFieldsArray()
+    {
+        $this->loadFixtures('Posts');
+        $table = $this->getTableLocator()->get('PaginatorPosts');
+        $data = ['author_id' => 3, 'title' => 'Fourth Article', 'body' => 'Article Body, unpublished', 'published' => 'N'];
+        $table->save(new Entity($data));
+
+        $settings = [
+            'finder' => 'list',
+            'conditions' => ['PaginatorPosts.published' => 'Y'],
+            'limit' => 2
+        ];
+        $results = $this->Paginator->paginate($table, [], $settings);
+
+        $result = $results->toArray();
+        $expected = [
+            1 => 'First Post',
+            2 => 'Second Post',
+        ];
+        $this->assertEquals($expected, $result);
+
+        $result = $this->Paginator->getPagingParams()['PaginatorPosts'];
+        $this->assertEquals(2, $result['current']);
+        $this->assertNull($result['count']);
+        $this->assertEquals(0, $result['pageCount']);
+        $this->assertTrue($result['nextPage']);
+        $this->assertFalse($result['prevPage']);
+    }
+
+    /**
+     * Test that special paginate types are called and that the type param doesn't leak out into defaults or options.
+     *
+     * @return void
+     */
+    public function testPaginateCustomFinder()
+    {
+        $settings = [
+            'PaginatorPosts' => [
+                'finder' => 'published',
+                'fields' => ['id', 'title'],
+                'maxLimit' => 10,
+            ]
+        ];
+
+        $this->loadFixtures('Posts');
+        $table = $this->getTableLocator()->get('PaginatorPosts');
+        $table->updateAll(['published' => 'N'], ['id' => 2]);
+
+        $this->Paginator->paginate($table, [], $settings);
+        $pagingParams = $this->Paginator->getPagingParams();
+        $this->assertSame('published', $pagingParams['PaginatorPosts']['finder']);
+
+        $this->assertSame(1, $pagingParams['PaginatorPosts']['start']);
+        $this->assertSame(2, $pagingParams['PaginatorPosts']['end']);
+        // nextPage will be always true for SimplePaginator
+        $this->assertTrue($pagingParams['PaginatorPosts']['nextPage']);
+    }
+}

tests/TestCase/View/Helper/PaginatorHelperTest.php

@@ -3222,4 +3222,58 @@ class PaginatorHelperTest extends TestCase
         ];
         $this->assertHtml($expected, $out);
     }
+
+    /**
+     * Test using paging params set by SimplePaginator which doesn't do count query.
+     *
+     * @return void
+     */
+    public function testMethodsWhenThereIsNoPageCount()
+    {
+        $request = new ServerRequest([
+            'url' => '/',
+            'params' => [
+                'paging' => [
+                    'Article' => [
+                        'page' => 1,
+                        'current' => 9,
+                        'count' => null,
+                        'prevPage' => false,
+                        'nextPage' => true,
+                        'pageCount' => 0,
+                        'start' => 1,
+                        'end' => 9,
+                        'sort' => null,
+                        'direction' => null,
+                        'limit' => null,
+                    ]
+                ]
+            ]
+        ]);
+
+        $view = new View($request);
+        $paginator = new PaginatorHelper($view);
+
+        $result = $paginator->first();
+        $this->assertFalse($result);
+
+        $result = $paginator->last();
+        $this->assertFalse($result);
+
+        // Using below methods when SimplePaginator is used makes no practical sense.
+        // The asserts are just to ensure they return a reasonable value.
+
+        $result = $paginator->numbers();
+        $this->assertFalse($result);
+
+        $result = $paginator->hasNext();
+        $this->assertTrue($result);
+
+        $result = $paginator->counter();
+        // counter() sets `pageCount` to 1 if empty.
+        $this->assertEquals('1 of 1', $result);
+
+        $result = $paginator->total();
+        $this->assertSame(0, $result);
+    }
 }