Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed $model->countRelated() failing when the relation is reusable #14444

Merged
merged 11 commits into from
Oct 13, 2019
2 changes: 2 additions & 0 deletions CHANGELOG-4.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
- Fixed `Phalcon\Firewall\Adapter\AdapterInterface::getRoleCallback` and `Phalcon\Firewall\Adapter\AbstractAdapter::setRoleCallback` to correctly accept and return a `Closure` [#14450](https://github.com/phalcon/cphalcon/issues/14450)
- Fixed `Phalcon\Events\Event::__constructor` to correctly accept an `object` as the `source` parameter [#14449](https://github.com/phalcon/cphalcon/issues/14449)
- Fixed `Phalcon\Cache::checkKey()` added `.` to key characters pattern [#14457](https://github.com/phalcon/cphalcon/pull/14457)
- Fixed `Phalcon\Mvc\Model\Manager` to store reusable related records correctly. [#14444](https://github.com/phalcon/cphalcon/pull/14444)
- Fixed `Phalcon\Mvc\Model::__call()` not to throw an exception when the return value is `null` for related records. [#14444](https://github.com/phalcon/cphalcon/pull/14444)

## Removed
- Removed `Phalcon\Application\AbstractApplication::handle()` as it does not serve any purpose and causing issues with type hinting. [#14407](https://github.com/phalcon/cphalcon/pull/14407)
Expand Down
27 changes: 16 additions & 11 deletions phalcon/Mvc/Model.zep
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,

let records = self::_invokeFinder(method, arguments);

if records !== null {
sergeyklay marked this conversation as resolved.
Show resolved Hide resolved
if records !== false {
return records;
}

Expand All @@ -199,7 +199,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
*/
let records = this->_getRelatedRecords(modelName, method, arguments);

if records !== null {
sergeyklay marked this conversation as resolved.
Show resolved Hide resolved
if records !== false {
return records;
}

Expand Down Expand Up @@ -4050,12 +4050,16 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
}

/**
* Returns related records defined relations depending on the method name
* Returns related records defined relations depending on the method name.
* Returns false if the relation is non-existent.
*
* @param array arguments
* @return mixed
* @param string modelName
* @param string method
* @param array arguments
*
* @return ResultsetInterface|ModelInterface|bool|null
*/
protected function _getRelatedRecords(string! modelName, string! method, var arguments)
protected function _getRelatedRecords(string! modelName, string! method, array! arguments)
{
var manager, relation, queryMethod, extraArgs, alias;

Expand All @@ -4077,10 +4081,11 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
);

/**
* Return if the relation was not found because getRelated() throws an exception if the relation is unknown
* Return if the relation was not found because getRelated()
* throws an exception if the relation is unknown
*/
if typeof relation != "object" {
return null;
return false;
}

return this->getRelated(alias, extraArgs);
Expand All @@ -4101,7 +4106,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
* If the relation was found perform the query via the models manager
*/
if typeof relation != "object" {
return null;
return false;
}

return manager->getRelationRecords(
Expand All @@ -4112,7 +4117,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
);
}

return null;
return false;
}

/**
Expand Down Expand Up @@ -4246,7 +4251,7 @@ abstract class Model extends AbstractInjectionAware implements EntityInterface,
let modelName = get_called_class();

if !extraMethod {
return null;
return false;
}

if unlikely !fetch value, arguments[0] {
Expand Down
6 changes: 3 additions & 3 deletions phalcon/Mvc/Model/Manager.zep
Original file line number Diff line number Diff line change
Expand Up @@ -1404,22 +1404,22 @@ class Manager implements ManagerInterface, InjectionAwareInterface, EventsAwareI
let retrieveMethod = method;
}

let arguments = [findParams];

/**
* Find first results could be reusable
*/
let reusable = (bool) relation->isReusable();

if reusable {
let uniqueKey = unique_key(referencedModel, arguments),
let uniqueKey = unique_key(referencedModel, [findParams, retrieveMethod]),
records = this->getReusableRecords(referencedModel, uniqueKey);

if typeof records == "array" || typeof records == "object" {
return records;
}
}

let arguments = [findParams];

/**
* Load the referenced model
* Call the function in the model
Expand Down
33 changes: 33 additions & 0 deletions tests/_data/fixtures/models/RobotsReusable.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/**
* This file is part of the Phalcon Framework.
*
* (c) Phalcon Team <[email protected]>
*
* For the full copyright and license information, please view the LICENSE.txt
* file that was distributed with this source code.
*/

namespace Phalcon\Test\Models;

use Phalcon\Mvc\Model;

class RobotsReusable extends Model
{
public function initialize()
{
$this->setSource('robots');

$this->hasMany(
'id',
RobotsParts::class,
'robots_id',
[
'alias' => 'robotsParts',
'foreignKey' => true,
'reusable' => true
]
);
}
}
124 changes: 121 additions & 3 deletions tests/integration/Mvc/Model/UnderscoreCallCest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,136 @@
namespace Phalcon\Test\Integration\Mvc\Model;

use IntegrationTester;
use Phalcon\Test\Fixtures\Traits\DiTrait;
use Phalcon\Test\Models;

class UnderscoreCallCest
{
use DiTrait;

public function _before(IntegrationTester $I)
{
$this->setNewFactoryDefault();
$this->setDiMysql();
}

public function _after(IntegrationTester $I)
{
$this->container['db']->close();
}

/**
* Tests Phalcon\Mvc\Model :: __call()
*
* @author Phalcon Team <[email protected]>
* @since 2018-11-13
* @author Balázs Németh <https://github.com/zsilbi>
* @since 2019-10-03
*/
public function mvcModelCall(IntegrationTester $I)
{
$I->wantToTest("Mvc\Model - __call()");
$I->skipTest('Need implementation');

/**
* Belongs-to relationship
*/
$robotPart = Models\RobotsParts::findFirst();

$part = $robotPart->getPart();

$I->assertInstanceOf(
Models\Parts::class,
$part
);

$nonExistentPart = $robotPart->getPart(
[
'id < 0',
'order' => 'id DESC',
]
);

$I->assertNull($nonExistentPart);

/**
* Testing has-one relationship
*/
$customer = Models\Customers::findFirst();

$user = $customer->getUser();

$I->assertInstanceOf(
Models\Users::class,
$user
);

$nonExistentUser = $customer->getUser(
[
'id < 0',
'order' => 'id DESC',
]
);

$I->assertNull($nonExistentUser);

/**
* Has-many relationship
*/
$robot = Models\Robots::findFirst();

$robotParts = $robot->getRobotsParts();

$I->assertInstanceOf(
\Phalcon\Mvc\Model\Resultset\Simple::class,
$robotParts
);

$countRobotParts = $robot->countRobotsParts();

$I->assertEquals(
$robotParts->count(),
$countRobotParts
);

$nonExistentRobotParts = $robot->getRobotsParts(
[
'id < 0',
'order' => 'id DESC',
]
);

$I->assertEquals(
0,
$nonExistentRobotParts->count()
);

/**
* Reusable has-many relationship
*/
$reusableRobot = Models\RobotsReusable::findFirst();

$reusableRobotParts = $reusableRobot->getRobotsParts();

$I->assertInstanceOf(
\Phalcon\Mvc\Model\Resultset\Simple::class,
$reusableRobotParts
);

$countReusableRobotParts = $reusableRobot->countRobotsParts();

$I->assertEquals(
$reusableRobotParts->count(),
$countReusableRobotParts
);

$nonExistentReusableRobotParts = $reusableRobot->getRobotsParts(
[
'id < 0',
'order' => 'id DESC',
]
);

$I->assertEquals(
0,
$nonExistentReusableRobotParts->count()
);
}
}