From 3271782381ee87b944f4bca1113489d797c30b02 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Mon, 4 May 2020 22:16:23 +0300 Subject: [PATCH] Fixed return type for CriteriaInterface::getLimit See: https://github.com/phalcon/cphalcon/issues/15004 --- CHANGELOG-4.0.md | 2 + ext/phalcon/mvc/model/criteria.zep.c | 60 +++++++++++-------- ext/phalcon/mvc/model/criteria.zep.h | 9 +-- ext/phalcon/mvc/model/criteriainterface.zep.c | 12 ++-- ext/phalcon/mvc/model/criteriainterface.zep.h | 9 +-- phalcon/Mvc/Model/Criteria.zep | 24 ++++---- phalcon/Mvc/Model/CriteriaInterface.zep | 14 +++-- .../database/Mvc/Model/Criteria/LimitCest.php | 44 ++++++++------ 8 files changed, 93 insertions(+), 81 deletions(-) diff --git a/CHANGELOG-4.0.md b/CHANGELOG-4.0.md index 4d8a498211b..f015214c2ce 100644 --- a/CHANGELOG-4.0.md +++ b/CHANGELOG-4.0.md @@ -8,6 +8,7 @@ - Changed the way `Phalcon\Http\Response::__construct` checks `content` data type. Now a `TypeError` will be thrown if incompatible data type was passed [#14983](https://github.com/phalcon/cphalcon/issues/14983) - Changed return type hints of the following `Phalcon\Flash\FlashInterface`'s methods: `error`, `message`, `notice`, `success` and `warning` [#14994](https://github.com/phalcon/cphalcon/issues/14994) - Changed return type hint for `Phalcon\Mvc\ModelInterface::sum` [#15000](https://github.com/phalcon/cphalcon/issues/15000) +- Changed return type for `Phalcon\Mvc\Model\Criteria::getLimit` so that integer, NULL or array will be returned [#15004](https://github.com/phalcon/cphalcon/issues/15004) ## Fixed - Fixed `Phalcon\Mvc\Model\Query\Builder::getPhql` to add single quote between string value on a simple condition [#14874](https://github.com/phalcon/cphalcon/issues/14874) @@ -43,6 +44,7 @@ - Fixed return type hints of the following `Phalcon\Flash\AbstractFlash`'s methods: `error`, `notice`, `success` and `warning` [#14994](https://github.com/phalcon/cphalcon/issues/14994) - Fixed return type hint for `Phalcon\Translate\InterpolatorFactory::newInstance` [#14996](https://github.com/phalcon/cphalcon/issues/14996) - Fixed return type hint for `Phalcon\Mvc\Model::sum` [#15000](https://github.com/phalcon/cphalcon/issues/15000) +- Fixed return type hint for `Phalcon\Mvc\Model\CriteriaInterface::getLimit` and `Phalcon\Mvc\Model\Criteria::getLimit` to follow documentation and original purpose [#15004](https://github.com/phalcon/cphalcon/issues/15004) [#14987](https://github.com/phalcon/cphalcon/issues/14987) diff --git a/ext/phalcon/mvc/model/criteria.zep.c b/ext/phalcon/mvc/model/criteria.zep.c index 35ed3808f31..386d3929ffd 100644 --- a/ext/phalcon/mvc/model/criteria.zep.c +++ b/ext/phalcon/mvc/model/criteria.zep.c @@ -832,11 +832,11 @@ PHP_METHOD(Phalcon_Mvc_Model_Criteria, getHaving) { } /** - * Returns the limit parameter in the criteria, which will be an integer if - * limit was set without an offset, an array with 'number' and 'offset' keys - * if an offset was set with the limit, or null if limit has not been set. + * Returns the limit parameter in the criteria, which will be * - * @return string|null + * - An integer if 'limit' was set without an 'offset' + * - An array with 'number' and 'offset' keys if an offset was set with the limit + * - NULL if limit has not been set */ PHP_METHOD(Phalcon_Mvc_Model_Criteria, getLimit) { @@ -1092,7 +1092,7 @@ PHP_METHOD(Phalcon_Mvc_Model_Criteria, inWhere) { array_init(&bindParams); ZEPHIR_INIT_VAR(&bindKeys); array_init(&bindKeys); - zephir_is_iterable(&values, 0, "phalcon/Mvc/Model/Criteria.zep", 544); + zephir_is_iterable(&values, 0, "phalcon/Mvc/Model/Criteria.zep", 545); if (Z_TYPE_P(&values) == IS_ARRAY) { ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(&values), _1) { @@ -1103,7 +1103,7 @@ PHP_METHOD(Phalcon_Mvc_Model_Criteria, inWhere) { zephir_get_strval(&key, &_3$$4); ZEPHIR_INIT_NVAR(&queryKey); ZEPHIR_CONCAT_SVS(&queryKey, ":", &key, ":"); - zephir_array_append(&bindKeys, &queryKey, PH_SEPARATE, "phalcon/Mvc/Model/Criteria.zep", 534); + zephir_array_append(&bindKeys, &queryKey, PH_SEPARATE, "phalcon/Mvc/Model/Criteria.zep", 535); zephir_array_update_zval(&bindParams, &key, &value, PH_COPY | PH_SEPARATE); ZEPHIR_SEPARATE(&hiddenParam); zephir_increment(&hiddenParam); @@ -1124,7 +1124,7 @@ PHP_METHOD(Phalcon_Mvc_Model_Criteria, inWhere) { zephir_get_strval(&key, &_4$$5); ZEPHIR_INIT_NVAR(&queryKey); ZEPHIR_CONCAT_SVS(&queryKey, ":", &key, ":"); - zephir_array_append(&bindKeys, &queryKey, PH_SEPARATE, "phalcon/Mvc/Model/Criteria.zep", 534); + zephir_array_append(&bindKeys, &queryKey, PH_SEPARATE, "phalcon/Mvc/Model/Criteria.zep", 535); zephir_array_update_zval(&bindParams, &key, &value, PH_COPY | PH_SEPARATE); ZEPHIR_SEPARATE(&hiddenParam); zephir_increment(&hiddenParam); @@ -1321,17 +1321,19 @@ PHP_METHOD(Phalcon_Mvc_Model_Criteria, leftJoin) { */ PHP_METHOD(Phalcon_Mvc_Model_Criteria, limit) { - zval _3; + zval _5$$5; zephir_method_globals *ZEPHIR_METHOD_GLOBALS_PTR = NULL; - zval *limit_param = NULL, *offset_param = NULL, _0, _1, _2, _4; + zval *limit_param = NULL, *offset_param = NULL, _0, _1, _2, _3$$4, _4$$4, _6$$5; zend_long limit, offset, ZEPHIR_LAST_CALL_STATUS; zval *this_ptr = getThis(); ZVAL_UNDEF(&_0); ZVAL_UNDEF(&_1); ZVAL_UNDEF(&_2); - ZVAL_UNDEF(&_4); - ZVAL_UNDEF(&_3); + ZVAL_UNDEF(&_3$$4); + ZVAL_UNDEF(&_4$$4); + ZVAL_UNDEF(&_6$$5); + ZVAL_UNDEF(&_5$$5); ZEPHIR_MM_GROW(); zephir_fetch_params(1, 1, 1, &limit_param, &offset_param); @@ -1355,17 +1357,25 @@ PHP_METHOD(Phalcon_Mvc_Model_Criteria, limit) { if (UNEXPECTED(limit == 0)) { RETURN_THIS(); } - ZEPHIR_INIT_VAR(&_3); - zephir_create_array(&_3, 2, 0); - ZEPHIR_INIT_VAR(&_4); - ZVAL_LONG(&_4, limit); - zephir_array_update_string(&_3, SL("number"), &_4, PH_COPY | PH_SEPARATE); - ZEPHIR_INIT_NVAR(&_4); - ZVAL_LONG(&_4, offset); - zephir_array_update_string(&_3, SL("offset"), &_4, PH_COPY | PH_SEPARATE); - ZEPHIR_INIT_NVAR(&_4); - ZVAL_STRING(&_4, "limit"); - zephir_update_property_array(this_ptr, SL("params"), &_4, &_3); + if (offset == 0) { + ZEPHIR_INIT_VAR(&_3$$4); + ZVAL_STRING(&_3$$4, "limit"); + ZEPHIR_INIT_VAR(&_4$$4); + ZVAL_LONG(&_4$$4, limit); + zephir_update_property_array(this_ptr, SL("params"), &_3$$4, &_4$$4); + } else { + ZEPHIR_INIT_VAR(&_5$$5); + zephir_create_array(&_5$$5, 2, 0); + ZEPHIR_INIT_VAR(&_6$$5); + ZVAL_LONG(&_6$$5, limit); + zephir_array_update_string(&_5$$5, SL("number"), &_6$$5, PH_COPY | PH_SEPARATE); + ZEPHIR_INIT_NVAR(&_6$$5); + ZVAL_LONG(&_6$$5, offset); + zephir_array_update_string(&_5$$5, SL("offset"), &_6$$5, PH_COPY | PH_SEPARATE); + ZEPHIR_INIT_NVAR(&_6$$5); + ZVAL_STRING(&_6$$5, "limit"); + zephir_update_property_array(this_ptr, SL("params"), &_6$$5, &_5$$5); + } RETURN_THIS(); } @@ -1491,7 +1501,7 @@ PHP_METHOD(Phalcon_Mvc_Model_Criteria, notInWhere) { array_init(&bindParams); ZEPHIR_INIT_VAR(&bindKeys); array_init(&bindKeys); - zephir_is_iterable(&values, 0, "phalcon/Mvc/Model/Criteria.zep", 723); + zephir_is_iterable(&values, 0, "phalcon/Mvc/Model/Criteria.zep", 728); if (Z_TYPE_P(&values) == IS_ARRAY) { ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(&values), _0) { @@ -1502,7 +1512,7 @@ PHP_METHOD(Phalcon_Mvc_Model_Criteria, notInWhere) { zephir_get_strval(&key, &_2$$3); ZEPHIR_INIT_NVAR(&_3$$3); ZEPHIR_CONCAT_SVS(&_3$$3, ":", &key, ":"); - zephir_array_append(&bindKeys, &_3$$3, PH_SEPARATE, "phalcon/Mvc/Model/Criteria.zep", 713); + zephir_array_append(&bindKeys, &_3$$3, PH_SEPARATE, "phalcon/Mvc/Model/Criteria.zep", 718); zephir_array_update_zval(&bindParams, &key, &value, PH_COPY | PH_SEPARATE); ZEPHIR_SEPARATE(&hiddenParam); zephir_increment(&hiddenParam); @@ -1523,7 +1533,7 @@ PHP_METHOD(Phalcon_Mvc_Model_Criteria, notInWhere) { zephir_get_strval(&key, &_4$$4); ZEPHIR_INIT_NVAR(&_5$$4); ZEPHIR_CONCAT_SVS(&_5$$4, ":", &key, ":"); - zephir_array_append(&bindKeys, &_5$$4, PH_SEPARATE, "phalcon/Mvc/Model/Criteria.zep", 713); + zephir_array_append(&bindKeys, &_5$$4, PH_SEPARATE, "phalcon/Mvc/Model/Criteria.zep", 718); zephir_array_update_zval(&bindParams, &key, &value, PH_COPY | PH_SEPARATE); ZEPHIR_SEPARATE(&hiddenParam); zephir_increment(&hiddenParam); diff --git a/ext/phalcon/mvc/model/criteria.zep.h b/ext/phalcon/mvc/model/criteria.zep.h index 299d6a502b6..d3c3d044c76 100644 --- a/ext/phalcon/mvc/model/criteria.zep.h +++ b/ext/phalcon/mvc/model/criteria.zep.h @@ -194,13 +194,6 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_phalcon_mvc_model_criteria_getdi #endif ZEND_END_ARG_INFO() -#if PHP_VERSION_ID >= 70200 -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_phalcon_mvc_model_criteria_getlimit, 0, 0, IS_STRING, 1) -#else -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_phalcon_mvc_model_criteria_getlimit, 0, 0, IS_STRING, NULL, 1) -#endif -ZEND_END_ARG_INFO() - #if PHP_VERSION_ID >= 70200 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_phalcon_mvc_model_criteria_getmodelname, 0, 0, IS_STRING, 0) #else @@ -455,7 +448,7 @@ ZEPHIR_INIT_FUNCS(phalcon_mvc_model_criteria_method_entry) { PHP_ME(Phalcon_Mvc_Model_Criteria, getDI, arginfo_phalcon_mvc_model_criteria_getdi, ZEND_ACC_PUBLIC) PHP_ME(Phalcon_Mvc_Model_Criteria, getGroupBy, NULL, ZEND_ACC_PUBLIC) PHP_ME(Phalcon_Mvc_Model_Criteria, getHaving, NULL, ZEND_ACC_PUBLIC) - PHP_ME(Phalcon_Mvc_Model_Criteria, getLimit, arginfo_phalcon_mvc_model_criteria_getlimit, ZEND_ACC_PUBLIC) + PHP_ME(Phalcon_Mvc_Model_Criteria, getLimit, NULL, ZEND_ACC_PUBLIC) PHP_ME(Phalcon_Mvc_Model_Criteria, getModelName, arginfo_phalcon_mvc_model_criteria_getmodelname, ZEND_ACC_PUBLIC) PHP_ME(Phalcon_Mvc_Model_Criteria, getOrderBy, arginfo_phalcon_mvc_model_criteria_getorderby, ZEND_ACC_PUBLIC) PHP_ME(Phalcon_Mvc_Model_Criteria, getParams, arginfo_phalcon_mvc_model_criteria_getparams, ZEND_ACC_PUBLIC) diff --git a/ext/phalcon/mvc/model/criteriainterface.zep.c b/ext/phalcon/mvc/model/criteriainterface.zep.c index 6af54ef2d17..3fb62263a3e 100644 --- a/ext/phalcon/mvc/model/criteriainterface.zep.c +++ b/ext/phalcon/mvc/model/criteriainterface.zep.c @@ -17,8 +17,8 @@ * * (c) Phalcon Team * - * For the full copyright and license information, please view the LICENSE.txt - * file that was distributed with this source code. + * For the full copyright and license information, please view the + * LICENSE.txt file that was distributed with this source code. */ /** * Phalcon\Mvc\Model\CriteriaInterface @@ -112,9 +112,11 @@ ZEPHIR_DOC_METHOD(Phalcon_Mvc_Model_CriteriaInterface, getGroupBy); ZEPHIR_DOC_METHOD(Phalcon_Mvc_Model_CriteriaInterface, getHaving); /** - * Returns the limit parameter in the criteria, which will be an integer if - * limit was set without an offset, an array with 'number' and 'offset' keys - * if an offset was set with the limit, or null if limit has not been set. + * Returns the limit parameter in the criteria, which will be + * + * - An integer if 'limit' was set without an 'offset' + * - An array with 'number' and 'offset' keys if an offset was set with the limit + * - NULL if limit has not been set */ ZEPHIR_DOC_METHOD(Phalcon_Mvc_Model_CriteriaInterface, getLimit); diff --git a/ext/phalcon/mvc/model/criteriainterface.zep.h b/ext/phalcon/mvc/model/criteriainterface.zep.h index 90add113ccd..f69295c07a8 100644 --- a/ext/phalcon/mvc/model/criteriainterface.zep.h +++ b/ext/phalcon/mvc/model/criteriainterface.zep.h @@ -108,13 +108,6 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_phalcon_mvc_model_criteriainterf #endif ZEND_END_ARG_INFO() -#if PHP_VERSION_ID >= 70200 -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_phalcon_mvc_model_criteriainterface_getlimit, 0, 0, IS_STRING, 1) -#else -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_phalcon_mvc_model_criteriainterface_getlimit, 0, 0, IS_STRING, NULL, 1) -#endif -ZEND_END_ARG_INFO() - #if PHP_VERSION_ID >= 70200 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_phalcon_mvc_model_criteriainterface_getmodelname, 0, 0, IS_STRING, 0) #else @@ -334,7 +327,7 @@ ZEPHIR_INIT_FUNCS(phalcon_mvc_model_criteriainterface_method_entry) { PHP_ABSTRACT_ME(Phalcon_Mvc_Model_CriteriaInterface, getConditions, arginfo_phalcon_mvc_model_criteriainterface_getconditions) PHP_ABSTRACT_ME(Phalcon_Mvc_Model_CriteriaInterface, getGroupBy, NULL) PHP_ABSTRACT_ME(Phalcon_Mvc_Model_CriteriaInterface, getHaving, NULL) - PHP_ABSTRACT_ME(Phalcon_Mvc_Model_CriteriaInterface, getLimit, arginfo_phalcon_mvc_model_criteriainterface_getlimit) + PHP_ABSTRACT_ME(Phalcon_Mvc_Model_CriteriaInterface, getLimit, NULL) PHP_ABSTRACT_ME(Phalcon_Mvc_Model_CriteriaInterface, getModelName, arginfo_phalcon_mvc_model_criteriainterface_getmodelname) PHP_ABSTRACT_ME(Phalcon_Mvc_Model_CriteriaInterface, getOrderBy, arginfo_phalcon_mvc_model_criteriainterface_getorderby) PHP_ABSTRACT_ME(Phalcon_Mvc_Model_CriteriaInterface, getParams, arginfo_phalcon_mvc_model_criteriainterface_getparams) diff --git a/phalcon/Mvc/Model/Criteria.zep b/phalcon/Mvc/Model/Criteria.zep index 34c40ae790d..5e805d7f217 100644 --- a/phalcon/Mvc/Model/Criteria.zep +++ b/phalcon/Mvc/Model/Criteria.zep @@ -393,15 +393,15 @@ class Criteria implements CriteriaInterface, InjectionAwareInterface } /** - * Returns the limit parameter in the criteria, which will be an integer if - * limit was set without an offset, an array with 'number' and 'offset' keys - * if an offset was set with the limit, or null if limit has not been set. + * Returns the limit parameter in the criteria, which will be * - * @return string|null + * - An integer if 'limit' was set without an 'offset' + * - An array with 'number' and 'offset' keys if an offset was set with the limit + * - NULL if limit has not been set */ - public function getLimit() -> string | null + public function getLimit() -> int | array | null { - var limit; + var limit, offset; if !fetch limit, this->params["limit"] { return null; @@ -634,10 +634,14 @@ class Criteria implements CriteriaInterface, InjectionAwareInterface return this; } - let this->params["limit"] = [ - "number": limit, - "offset": offset - ]; + if offset == 0 { + let this->params["limit"] = limit; + } else { + let this->params["limit"] = [ + "number": limit, + "offset": offset + ]; + } return this; } diff --git a/phalcon/Mvc/Model/CriteriaInterface.zep b/phalcon/Mvc/Model/CriteriaInterface.zep index 2b9a7acfdfc..0cddfa11e6d 100644 --- a/phalcon/Mvc/Model/CriteriaInterface.zep +++ b/phalcon/Mvc/Model/CriteriaInterface.zep @@ -4,8 +4,8 @@ * * (c) Phalcon Team * - * For the full copyright and license information, please view the LICENSE.txt - * file that was distributed with this source code. + * For the full copyright and license information, please view the + * LICENSE.txt file that was distributed with this source code. */ namespace Phalcon\Mvc\Model; @@ -98,11 +98,13 @@ interface CriteriaInterface public function getHaving(); /** - * Returns the limit parameter in the criteria, which will be an integer if - * limit was set without an offset, an array with 'number' and 'offset' keys - * if an offset was set with the limit, or null if limit has not been set. + * Returns the limit parameter in the criteria, which will be + * + * - An integer if 'limit' was set without an 'offset' + * - An array with 'number' and 'offset' keys if an offset was set with the limit + * - NULL if limit has not been set */ - public function getLimit() -> string | null; + public function getLimit() -> int | array | null; /** * Returns an internal model name on which the criteria will be applied diff --git a/tests/database/Mvc/Model/Criteria/LimitCest.php b/tests/database/Mvc/Model/Criteria/LimitCest.php index c42754321bd..f472673eef6 100644 --- a/tests/database/Mvc/Model/Criteria/LimitCest.php +++ b/tests/database/Mvc/Model/Criteria/LimitCest.php @@ -5,8 +5,8 @@ * * (c) Phalcon Team * - * For the full copyright and license information, please view the LICENSE.txt - * file that was distributed with this source code. + * For the full copyright and license information, please view the + * LICENSE.txt file that was distributed with this source code. */ declare(strict_types=1); @@ -16,24 +16,34 @@ use DatabaseTester; use Phalcon\Mvc\Model\Criteria; use Phalcon\Mvc\Model\Query\Builder; +use Phalcon\Storage\Exception; use Phalcon\Test\Fixtures\Traits\DiTrait; use Phalcon\Test\Models\Invoices; -/** - * Class LimitCest - */ class LimitCest { use DiTrait; - public function _before(DatabaseTester $I) + /** + * Executed before each test + * + * @param DatabaseTester $I + * @return void + */ + public function _before(DatabaseTester $I): void { - $this->setNewFactoryDefault(); + try { + $this->setNewFactoryDefault(); + } catch (Exception $e) { + $I->fail($e->getMessage()); + } } /** * Tests Phalcon\Mvc\Model\Criteria :: limit() * + * @param DatabaseTester $I + * * @author Phalcon Team * @since 2020-02-01 * @@ -58,20 +68,16 @@ public function mvcModelCriteriaLimit(DatabaseTester $I) $expected = 'SELECT [Phalcon\Test\Models\Invoices].* ' . 'FROM [Phalcon\Test\Models\Invoices] ' . 'LIMIT :APL0:'; - $actual = $builder->getPhql(); - $I->assertEquals($expected, $actual); - $expected = [ - 'number' => 10, - 'offset' => 0, - ]; - $actual = $criteria->getLimit(); - $I->assertEquals($expected, $actual); + $I->assertEquals($expected, $builder->getPhql()); + $I->assertEquals(10, $criteria->getLimit()); } /** * Tests Phalcon\Mvc\Model\Criteria :: limit() - offset * + * @param DatabaseTester $I + * * @author Phalcon Team * @since 2020-02-01 * @@ -96,14 +102,14 @@ public function mvcModelCriteriaLimitOffset(DatabaseTester $I) $expected = 'SELECT [Phalcon\Test\Models\Invoices].* ' . 'FROM [Phalcon\Test\Models\Invoices] ' . 'LIMIT :APL0: OFFSET :APL1:'; - $actual = $builder->getPhql(); - $I->assertEquals($expected, $actual); + + $I->assertEquals($expected, $builder->getPhql()); $expected = [ 'number' => 10, 'offset' => 15, ]; - $actual = $criteria->getLimit(); - $I->assertEquals($expected, $actual); + + $I->assertEquals($expected, $criteria->getLimit()); } }