Cphalcon: Phalcon\Mvc\Model::__isset doesn't account for the PHP `empty` failover

[CameronHall]

Expected and Actual Behavior

When checking empty on a protected model property (out of scope) you would expect to get a definitive result by invoking the getter (if it exists, if not trigger a warning??). Instead, it'll always failover to the Phalcon\Mvc\Model::__isset which checks if a relationship exists.

class MyModel extends \Phalcon\Mvc\Model
{
    protected $secretValue;

    public function setSecretValue($value)
    {
        $this->secretValue = $value;
    }

    public function getSecretValue()
    {
        return $this->secretValue;
    }
}

$model = new MyModel();
$model->secretValue = 123;
var_dump(empty($model->secretValue)); // false

Details

• Phalcon version: 3.4.1
• PHP Version: (php -v)
• Operating System: Windows
• Installation type: DLL Download
• Server: Apache
  Other related info (Database, table schema):

[JABirchall]

This is expected behavour by the PHP engine. The same as __set() and __get() allows you to access private/protected properties of a class if designed that way.

[CameronHall]

I know it says it on the documentation. What I'm saying is the expected behavior would be that __isset would handle this functionality. However, it looks like it was designed to only check if relationships exist. I believe it should be updated to check the getters of protected properties as well.

[JABirchall]

I get where you are coming from. It seems that they already have this functionallity in place in the __set

if !manager->isVisibleModelProperty(this, property) {
    throw new Exception("Property '" . property . "' does not have a setter.");
}

But do not use it in __get or __isset. Prolly have to wait for niden or klay to respond about that decision.

[niden]

I think it is a good addition to the framework (both for __get and __isset)

However I don't want this in 3.4.x series since it could very well have unexpected results to existing applications. We should get this in 4.x

@phalcon/core-team Thoughts?

[JABirchall]

@niden I would also consider changing the message. Since it is refering to a property that isnt visable the error should be simular if not the same as PHPs error for accessing private/protected properties

Fatal error: Cannot access private property SpecialException::$_type in C:\path\to\exceptions.php on line 74

So changing it to
throw new Exception("Cannot access property '" . property . "' is not public");
or something of the sorts. The current error is somewhat ambiguous.

[niden]

Resolved in https://github.com/phalcon/cphalcon/pull/14659