Fix #20226: Revert all PR for "Data providers perform unnecessary COUNT queries that negatively affect performance"

2025-08-26 06:15:19 +08:00 · 2024-07-12 01:48:58 -04:00
parent 5f5ef64892
commit a6257d8f46
10 changed files with 86 additions and 158 deletions
--- a/composer.lock
+++ b/composer.lock
@ -4,7 +4,7 @@
        "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
        "This file is @generated automatically"
    ],
-    "content-hash": "fe4cba7b97bdf43a740c7f6f9d224ae8",
+    "content-hash": "67d678d7fc90991fe6c1967c41f6264d",
    "packages": [
        {
            "name": "bower-asset/inputmask",
@ -565,16 +565,16 @@
        },
        {
            "name": "myclabs/deep-copy",
-            "version": "1.11.1",
+            "version": "1.12.0",
            "source": {
                "type": "git",
                "url": "https://github.com/myclabs/DeepCopy.git",
-                "reference": "7284c22080590fb39f2ffa3e9057f10a4ddd0e0c"
+                "reference": "3a6b9a42cd8f8771bd4295d13e1423fa7f3d942c"
            },
            "dist": {
                "type": "zip",
-                "url": "https://api.github.com/repos/myclabs/DeepCopy/zipball/7284c22080590fb39f2ffa3e9057f10a4ddd0e0c",
-                "reference": "7284c22080590fb39f2ffa3e9057f10a4ddd0e0c",
+                "url": "https://api.github.com/repos/myclabs/DeepCopy/zipball/3a6b9a42cd8f8771bd4295d13e1423fa7f3d942c",
+                "reference": "3a6b9a42cd8f8771bd4295d13e1423fa7f3d942c",
                "shasum": ""
            },
            "require": {
@ -582,11 +582,12 @@
            },
            "conflict": {
                "doctrine/collections": "<1.6.8",
-                "doctrine/common": "<2.13.3 || >=3,<3.2.2"
+                "doctrine/common": "<2.13.3 || >=3 <3.2.2"
            },
            "require-dev": {
                "doctrine/collections": "^1.6.8",
                "doctrine/common": "^2.13.3 || ^3.2.2",
+                "phpspec/prophecy": "^1.10",
                "phpunit/phpunit": "^7.5.20 || ^8.5.23 || ^9.5.13"
            },
            "type": "library",
@ -612,7 +613,7 @@
            ],
            "support": {
                "issues": "https://github.com/myclabs/DeepCopy/issues",
-                "source": "https://github.com/myclabs/DeepCopy/tree/1.11.1"
+                "source": "https://github.com/myclabs/DeepCopy/tree/1.12.0"
            },
            "funding": [
                {
@ -620,20 +621,20 @@
                    "type": "tidelift"
                }
            ],
-            "time": "2023-03-08T13:26:56+00:00"
+            "time": "2024-06-12T14:39:25+00:00"
        },
        {
            "name": "nikic/php-parser",
-            "version": "v5.0.2",
+            "version": "v5.1.0",
            "source": {
                "type": "git",
                "url": "https://github.com/nikic/PHP-Parser.git",
-                "reference": "139676794dc1e9231bf7bcd123cfc0c99182cb13"
+                "reference": "683130c2ff8c2739f4822ff7ac5c873ec529abd1"
            },
            "dist": {
                "type": "zip",
-                "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/139676794dc1e9231bf7bcd123cfc0c99182cb13",
-                "reference": "139676794dc1e9231bf7bcd123cfc0c99182cb13",
+                "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/683130c2ff8c2739f4822ff7ac5c873ec529abd1",
+                "reference": "683130c2ff8c2739f4822ff7ac5c873ec529abd1",
                "shasum": ""
            },
            "require": {
@ -644,7 +645,7 @@
            },
            "require-dev": {
                "ircmaxell/php-yacc": "^0.0.7",
-                "phpunit/phpunit": "^7.0 || ^8.0 || ^9.0"
+                "phpunit/phpunit": "^9.0"
            },
            "bin": [
                "bin/php-parse"
@ -676,9 +677,9 @@
            ],
            "support": {
                "issues": "https://github.com/nikic/PHP-Parser/issues",
-                "source": "https://github.com/nikic/PHP-Parser/tree/v5.0.2"
+                "source": "https://github.com/nikic/PHP-Parser/tree/v5.1.0"
            },
-            "time": "2024-03-05T20:51:40+00:00"
+            "time": "2024-07-01T20:03:41+00:00"
        },
        {
            "name": "phar-io/manifest",
@ -1119,16 +1120,16 @@
        },
        {
            "name": "phpunit/phpunit",
-            "version": "9.6.0",
+            "version": "9.6.19",
            "source": {
                "type": "git",
                "url": "https://github.com/sebastianbergmann/phpunit.git",
-                "reference": "70fc8be1d0b9fad56a199a4df5f9cfabfc246f84"
+                "reference": "a1a54a473501ef4cdeaae4e06891674114d79db8"
            },
            "dist": {
                "type": "zip",
-                "url": "https://api.github.com/repos/sebastianbergmann/phpunit/zipball/70fc8be1d0b9fad56a199a4df5f9cfabfc246f84",
-                "reference": "70fc8be1d0b9fad56a199a4df5f9cfabfc246f84",
+                "url": "https://api.github.com/repos/sebastianbergmann/phpunit/zipball/a1a54a473501ef4cdeaae4e06891674114d79db8",
+                "reference": "a1a54a473501ef4cdeaae4e06891674114d79db8",
                "shasum": ""
            },
            "require": {
@ -1143,7 +1144,7 @@
                "phar-io/manifest": "^2.0.3",
                "phar-io/version": "^3.0.2",
                "php": ">=7.3",
-                "phpunit/php-code-coverage": "^9.2.13",
+                "phpunit/php-code-coverage": "^9.2.28",
                "phpunit/php-file-iterator": "^3.0.5",
                "phpunit/php-invoker": "^3.1.1",
                "phpunit/php-text-template": "^2.0.3",
@ -1161,8 +1162,8 @@
                "sebastian/version": "^3.0.2"
            },
            "suggest": {
-                "ext-soap": "*",
-                "ext-xdebug": "*"
+                "ext-soap": "To be able to generate mocks based on WSDL files",
+                "ext-xdebug": "PHP extension that provides line coverage as well as branch and path coverage"
            },
            "bin": [
                "phpunit"
@ -1201,7 +1202,8 @@
            ],
            "support": {
                "issues": "https://github.com/sebastianbergmann/phpunit/issues",
-                "source": "https://github.com/sebastianbergmann/phpunit/tree/9.6.0"
+                "security": "https://github.com/sebastianbergmann/phpunit/security/policy",
+                "source": "https://github.com/sebastianbergmann/phpunit/tree/9.6.19"
            },
            "funding": [
                {
@ -1217,7 +1219,7 @@
                    "type": "tidelift"
                }
            ],
-            "time": "2023-02-03T07:32:24+00:00"
+            "time": "2024-04-05T04:35:58+00:00"
        },
        {
            "name": "sebastian/cli-parser",
@ -2184,16 +2186,16 @@
        },
        {
            "name": "squizlabs/php_codesniffer",
-            "version": "3.9.0",
+            "version": "3.10.1",
            "source": {
                "type": "git",
                "url": "https://github.com/PHPCSStandards/PHP_CodeSniffer.git",
-                "reference": "d63cee4890a8afaf86a22e51ad4d97c91dd4579b"
+                "reference": "8f90f7a53ce271935282967f53d0894f8f1ff877"
            },
            "dist": {
                "type": "zip",
-                "url": "https://api.github.com/repos/PHPCSStandards/PHP_CodeSniffer/zipball/d63cee4890a8afaf86a22e51ad4d97c91dd4579b",
-                "reference": "d63cee4890a8afaf86a22e51ad4d97c91dd4579b",
+                "url": "https://api.github.com/repos/PHPCSStandards/PHP_CodeSniffer/zipball/8f90f7a53ce271935282967f53d0894f8f1ff877",
+                "reference": "8f90f7a53ce271935282967f53d0894f8f1ff877",
                "shasum": ""
            },
            "require": {
@ -2260,7 +2262,7 @@
                    "type": "open_collective"
                }
            ],
-            "time": "2024-02-16T15:06:51+00:00"
+            "time": "2024-05-22T21:24:41+00:00"
        },
        {
            "name": "theseer/tokenizer",
@ -2314,16 +2316,16 @@
        },
        {
            "name": "yiisoft/yii2-coding-standards",
-            "version": "3.0.0",
+            "version": "3.0.1",
            "source": {
                "type": "git",
                "url": "https://github.com/yiisoft/yii2-coding-standards.git",
-                "reference": "8bc39acaae848aec1ad52b2af4cf380e3f0b104e"
+                "reference": "842ffdf6c31f46bb6f4b3f3c7dda4f570321ace7"
            },
            "dist": {
                "type": "zip",
-                "url": "https://api.github.com/repos/yiisoft/yii2-coding-standards/zipball/8bc39acaae848aec1ad52b2af4cf380e3f0b104e",
-                "reference": "8bc39acaae848aec1ad52b2af4cf380e3f0b104e",
+                "url": "https://api.github.com/repos/yiisoft/yii2-coding-standards/zipball/842ffdf6c31f46bb6f4b3f3c7dda4f570321ace7",
+                "reference": "842ffdf6c31f46bb6f4b3f3c7dda4f570321ace7",
                "shasum": ""
            },
            "require": {
@ -2396,7 +2398,7 @@
                    "type": "open_collective"
                }
            ],
-            "time": "2024-03-15T12:57:48+00:00"
+            "time": "2024-06-12T13:50:40+00:00"
        }
    ],
    "aliases": [],
--- a/framework/CHANGELOG.md
+++ b/framework/CHANGELOG.md
@ -7,8 +7,8 @@ Yii Framework 2 Change Log
 - Bug #20195: Do not set non abstract values into `ColumnSchema->type` on MSSQL version less then 2017 (axeltomasson)
 - Bug #16116: Codeception: oci does not support enabling/disabling integrity check (@terabytesoftw)
 - Bug #20191: Fix `ActiveRecord::getDirtyAttributes()` for JSON columns with multi-dimensional array values (brandonkelly)
- Bug #20175: Fix bad result for pagination when used with GridView (@lav45)
 - Bug #20211: Add acceptable parameters to `MaskedInput::init()` method (alxlnk)
+- Bug #20226: Revert all PR for "Data providers perform unnecessary COUNT queries that negatively affect performance" (@terabytesoftw)


 2.0.50 May 30, 2024
--- a/framework/data/ActiveDataProvider.php
+++ b/framework/data/ActiveDataProvider.php
@ -102,6 +102,7 @@ class ActiveDataProvider extends BaseDataProvider
        }
        $query = clone $this->query;
        if (($pagination = $this->getPagination()) !== false) {
+            $pagination->totalCount = $this->getTotalCount();
            if ($pagination->totalCount === 0) {
                return [];
            }
@ -110,6 +111,7 @@ class ActiveDataProvider extends BaseDataProvider
        if (($sort = $this->getSort()) !== false) {
            $query->addOrderBy($sort->getOrders());
        }
+
        return $query->all($this->db);
    }

@ -127,6 +129,7 @@ class ActiveDataProvider extends BaseDataProvider
                    $keys[] = call_user_func($this->key, $model);
                }
            }
+
            return $keys;
        } elseif ($this->query instanceof ActiveQueryInterface) {
            /* @var $class \yii\db\ActiveRecordInterface */
@ -146,8 +149,10 @@ class ActiveDataProvider extends BaseDataProvider
                    $keys[] = $kk;
                }
            }
+
            return $keys;
        }
+
        return array_keys($models);
    }

@ -192,6 +197,7 @@ class ActiveDataProvider extends BaseDataProvider
        if (is_object($this->query)) {
            $this->query = clone $this->query;
        }
+
        parent::__clone();
    }
 }
--- a/framework/data/ArrayDataProvider.php
+++ b/framework/data/ArrayDataProvider.php
@ -86,10 +86,14 @@ class ArrayDataProvider extends BaseDataProvider
            $models = $this->sortModels($models, $sort);
        }

-        $pagination = $this->getPagination();
-        if ($pagination !== false && $pagination->getPageSize() > 0) {
+        if (($pagination = $this->getPagination()) !== false) {
+            $pagination->totalCount = $this->getTotalCount();
+
+            if ($pagination->getPageSize() > 0) {
                $models = array_slice($models, $pagination->getOffset(), $pagination->getLimit(), true);
            }
+        }
+
        return $models;
    }

--- a/framework/data/BaseDataProvider.php
+++ b/framework/data/BaseDataProvider.php
@ -164,13 +164,12 @@ abstract class BaseDataProvider extends Component implements DataProviderInterfa
     */
    public function getTotalCount()
    {
-        if ($this->_pagination === false) {
+        if ($this->getPagination() === false) {
            return $this->getCount();
+        } elseif ($this->_totalCount === null) {
+            $this->_totalCount = $this->prepareTotalCount();
        }
-        if ($this->_totalCount !== null) {
-            return (int)$this->_totalCount;
-        }
-        return $this->prepareTotalCount();
+        return $this->_totalCount;
    }

    /**
@ -193,6 +192,7 @@ abstract class BaseDataProvider extends Component implements DataProviderInterfa
        if ($this->_pagination === null) {
            $this->setPagination([]);
        }
+
        return $this->_pagination;
    }

@ -216,15 +216,9 @@ abstract class BaseDataProvider extends Component implements DataProviderInterfa
                $config['pageParam'] = $this->id . '-page';
                $config['pageSizeParam'] = $this->id . '-per-page';
            }
-            $value = Yii::createObject(array_merge($config, $value));
-        }
-        if ($value instanceof Pagination) {
-            $value->setTotalCount(function () {
-                return $this->getTotalCount();
-            });
+            $this->_pagination = Yii::createObject(array_merge($config, $value));
+        } elseif ($value instanceof Pagination || $value === false) {
            $this->_pagination = $value;
-        } elseif ($value === false) {
-            $this->_pagination = false;
        } else {
            throw new InvalidArgumentException('Only Pagination instance, configuration array or false is allowed.');
        }
--- a/framework/data/Pagination.php
+++ b/framework/data/Pagination.php
@ -7,7 +7,6 @@

 namespace yii\data;

-use Closure;
 use Yii;
 use yii\base\BaseObject;
 use yii\web\Link;
@ -70,7 +69,6 @@ use yii\web\Request;
 * @property-read int $pageCount Number of pages.
 * @property int $pageSize The number of items per page. If it is less than 1, it means the page size is
 * infinite, and thus a single page contains all items.
- * @property int $totalCount total number of items.
 *
 * @author Qiang Xue <qiang.xue@gmail.com>
 * @since 2.0
@ -125,6 +123,10 @@ class Pagination extends BaseObject implements Linkable
     * number validation. By doing so, [[page]] will return the value indexed by [[pageParam]] in [[params]].
     */
    public $validatePage = true;
+    /**
+     * @var int total number of items.
+     */
+    public $totalCount = 0;
    /**
     * @var int the default page size. This property will be returned by [[pageSize]] when page size
     * cannot be determined by [[pageSizeParam]] from [[params]].
@ -141,10 +143,6 @@ class Pagination extends BaseObject implements Linkable
     * If it is less than 1, it means the page size is infinite, and thus a single page contains all items.
     */
    private $_pageSize;
-    /**
-     * @var Closure|int total number of items or closure returning it.
-     */
-    private $_totalCount = 0;


    /**
@ -153,11 +151,13 @@ class Pagination extends BaseObject implements Linkable
    public function getPageCount()
    {
        $pageSize = $this->getPageSize();
-        $totalCount = $this->getTotalCount();
        if ($pageSize < 1) {
-            return $totalCount > 0 ? 1 : 0;
+            return $this->totalCount > 0 ? 1 : 0;
        }
-        return (int) ((max($totalCount, 0) + $pageSize - 1) / $pageSize);
+
+        $totalCount = $this->totalCount < 0 ? 0 : (int) $this->totalCount;
+
+        return (int) (($totalCount + $pageSize - 1) / $pageSize);
    }

    private $_page;
@ -173,6 +173,7 @@ class Pagination extends BaseObject implements Linkable
            $page = (int) $this->getQueryParam($this->pageParam, 1) - 1;
            $this->setPage($page, true);
        }
+
        return $this->_page;
    }

@ -220,6 +221,7 @@ class Pagination extends BaseObject implements Linkable
                $this->setPageSize($pageSize, true);
            }
        }
+
        return $this->_pageSize;
    }

@ -262,7 +264,7 @@ class Pagination extends BaseObject implements Linkable
            $request = Yii::$app->getRequest();
            $params = $request instanceof Request ? $request->getQueryParams() : [];
        }
-        if ($page > 0 || ($page === 0 && $this->forcePageParam)) {
+        if ($page > 0 || $page == 0 && $this->forcePageParam) {
            $params[$this->pageParam] = $page + 1;
        } else {
            unset($params[$this->pageParam]);
@ -280,6 +282,7 @@ class Pagination extends BaseObject implements Linkable
        if ($absolute) {
            return $urlManager->createAbsoluteUrl($params);
        }
+
        return $urlManager->createUrl($params);
    }

@ -290,6 +293,7 @@ class Pagination extends BaseObject implements Linkable
    public function getOffset()
    {
        $pageSize = $this->getPageSize();
+
        return $pageSize < 1 ? 0 : $this->getPage() * $pageSize;
    }

@ -301,6 +305,7 @@ class Pagination extends BaseObject implements Linkable
    public function getLimit()
    {
        $pageSize = $this->getPageSize();
+
        return $pageSize < 1 ? -1 : $pageSize;
    }

@ -326,6 +331,7 @@ class Pagination extends BaseObject implements Linkable
                $links[self::LINK_NEXT] = $this->createUrl($currentPage + 1, null, $absolute);
            }
        }
+
        return $links;
    }

@ -342,25 +348,7 @@ class Pagination extends BaseObject implements Linkable
            $request = Yii::$app->getRequest();
            $params = $request instanceof Request ? $request->getQueryParams() : [];
        }
+
        return isset($params[$name]) && is_scalar($params[$name]) ? $params[$name] : $defaultValue;
    }
-
-    /**
-     * @return int total number of items.
-     */
-    public function getTotalCount()
-    {
-        if (is_numeric($this->_totalCount)) {
-            return (int)$this->_totalCount;
-        }
-        return (int)call_user_func($this->_totalCount);
-    }
-
-    /**
-     * @param Closure|int $count
-     */
-    public function setTotalCount($count)
-    {
-        $this->_totalCount = $count;
-    }
 }
--- a/framework/data/SqlDataProvider.php
+++ b/framework/data/SqlDataProvider.php
@ -126,6 +126,7 @@ class SqlDataProvider extends BaseDataProvider
        }

        if ($pagination !== false) {
+            $pagination->totalCount = $this->getTotalCount();
            $limit = $pagination->getLimit();
            $offset = $pagination->getOffset();
        }
@ -149,8 +150,10 @@ class SqlDataProvider extends BaseDataProvider
                    $keys[] = call_user_func($this->key, $model);
                }
            }
+
            return $keys;
        }
+
        return array_keys($models);
    }

--- a/framework/rest/Serializer.php
+++ b/framework/rest/Serializer.php
@ -188,12 +188,6 @@ class Serializer extends Component
     */
    protected function serializeDataProvider($dataProvider)
    {
-        if (($pagination = $dataProvider->getPagination()) !== false) {
-            $this->addPaginationHeaders($pagination);
-        }
-        if ($this->request->getIsHead()) {
-            return null;
-        }
        if ($this->preserveKeys) {
            $models = $dataProvider->getModels();
        } else {
@ -201,7 +195,13 @@ class Serializer extends Component
        }
        $models = $this->serializeModels($models);

-        if ($this->collectionEnvelope === null) {
+        if (($pagination = $dataProvider->getPagination()) !== false) {
+            $this->addPaginationHeaders($pagination);
+        }
+
+        if ($this->request->getIsHead()) {
+            return null;
+        } elseif ($this->collectionEnvelope === null) {
            return $models;
        }

--- a/tests/framework/data/ActiveDataProviderTest.php
+++ b/tests/framework/data/ActiveDataProviderTest.php
@ -170,8 +170,9 @@ abstract class ActiveDataProviderTest extends DatabaseTestCase
            'query' => $query->from('order')->orderBy('id'),
        ]);
        $pagination = $provider->getPagination();
-        $this->assertEquals(1, $pagination->getPageCount());
+        $this->assertEquals(0, $pagination->getPageCount());
        $this->assertCount(3, $provider->getModels());
+        $this->assertEquals(1, $pagination->getPageCount());

        $provider->getPagination()->pageSize = 2;
        $this->assertCount(3, $provider->getModels());
@ -197,23 +198,4 @@ abstract class ActiveDataProviderTest extends DatabaseTestCase

        $this->assertEquals(0, $pagination->getPageCount());
    }
-
-    public function testTotalCountAfterSearch()
-    {
-        $query = Order::find();
-        $provider = new ActiveDataProvider([
-            'query' => $query,
-            'pagination' => [
-                'pageSize' => 2,
-            ],
-        ]);
-
-        $pagination = $provider->getPagination();
-        $this->assertEquals(2, $pagination->getPageCount());
-        $this->assertEquals(3, $pagination->getTotalCount());
-
-        $query->andWhere(['customer_id' => 2]);
-        $this->assertEquals(1, $pagination->getPageCount());
-        $this->assertEquals(2, $pagination->getTotalCount());
-    }
 }
--- a/tests/framework/rest/SerializerTest.php
+++ b/tests/framework/rest/SerializerTest.php
@ -10,7 +10,6 @@ namespace yiiunit\framework\rest;
 use yii\base\Model;
 use yii\data\ArrayDataProvider;
 use yii\rest\Serializer;
-use yii\web\Request;
 use yiiunit\TestCase;

 /**
@ -416,56 +415,6 @@ class SerializerTest extends TestCase
        $this->assertEquals($expectedResult, $serializer->serialize($dataProvider));
    }

-    /**
-     * @dataProvider dataProviderSerializeDataProvider
-     *
-     * @param \yii\data\DataProviderInterface $dataProvider
-     * @param array $expectedResult
-     * @param bool $saveKeys
-     */
-    public function testHeadSerializeDataProvider($dataProvider, $expectedResult, $saveKeys = false)
-    {
-        $serializer = new Serializer();
-        $serializer->preserveKeys = $saveKeys;
-        $serializer->collectionEnvelope = 'data';
-
-        $this->assertEquals($expectedResult, $serializer->serialize($dataProvider)['data']);
-
-        $_SERVER['REQUEST_METHOD'] = 'HEAD';
-        $request = new Request();
-        $_POST[$request->methodParam] = 'HEAD';
-        $serializer = new Serializer([
-            'request' => $request
-        ]);
-        $serializer->preserveKeys = $saveKeys;
-        $this->assertEmpty($serializer->serialize($dataProvider));
-        $this->assertNotEmpty($serializer->response->getHeaders()->get($serializer->totalCountHeader));
-
-        $arrayDataProviderMock = $this->getMockBuilder(ArrayDataProvider::className())
-            ->disableOriginalConstructor()
-            ->getMock();
-
-        // stub getModels to prevent empty
-        $arrayDataProviderMock
-            ->method('getModels')
-            ->willReturn($expectedResult);
-
-        // stub getPagination for header
-        $arrayDataProviderMock
-            ->method('getPagination')
-            ->willReturn($dataProvider->getPagination());
-
-        // assert normal HEAD is empty response
-        $this->assertEmpty($serializer->serialize($arrayDataProviderMock));
-
-        // Test #20002:  Set up the expectation for the getModels method
-        $arrayDataProviderMock->expects($this->never())
-            ->method('getModels');
-
-        // reset Method
-        unset($_POST[$request->methodParam], $_SERVER['REQUEST_METHOD']);
-    }
-
    /**
     * @see https://github.com/yiisoft/yii2/issues/16334
     */