nette/database

ActiveRow: Isset on return false

miranovy opened this issue · 4 comments

  • bug report? yes
  • feature request? no
  • version: 2.4.5

Description

The use of the function isset return false. We used it to check relation over sevetal tables. This examle is for easy for ease of understanding simplified.

$row = $context->table('book')->get(2);
if (isset($row->author->name)) { ... }

This condition returns false under php 7.0 and 7.1. In php 5.6 it returns the expected value true. Call var_dump($row->author->name) will write the correct author name under all php versions.

Steps To Reproduce

I prepare test:

<?php
/**
 * @dataProvider? ../databases.ini
 */
use Tester\Assert;
require __DIR__ . '/../../connect.inc.php'; // create $connection
Nette\Database\Helpers::loadFromFile($connection, __DIR__ . "/../../files/{$driverName}-nette_test1.sql");
test(function () use ($context) {
	$row = $context->table('book')->get(2);
	Assert::same('Jakub Vrana', $row->author->name);
	Assert::true(isset($row->author->name));
});

On php 5.6 is corrent. On php 7 or greate break with fail. The difference is how php internally interprets the isset function.

Testing example:

<?php

class Foo
{
    private $bar;

    public function __construct()
    {
        unset($this->bar);
    }

    public function __isset($name)
    {
        var_dump(__METHOD__ . '('. $name .')');
        return true;
    }
    
    public function __get($name)
    {
        var_dump(__METHOD__ .'('. $name .')');
        return $this;
    }
}

$instance = new Foo();
var_dump(isset($instance->rel1->rel2->property));

In php 5.6 returns the result:

php test.php 
string(19) "Foo::__get(surname)"
string(16) "Foo::__get(name)"
string(17) "Foo::__isset(foo)"
bool(true)

and in php 7.0 or great returns:

 php test.php 
string(18) "Foo::__isset(rel1)"
string(16) "Foo::__get(rel1)"
string(18) "Foo::__isset(rel2)"
string(16) "Foo::__get(rel2)"
string(22) "Foo::__isset(property)"
bool(true)

The solution is add to function __isset check the existence of the relation:

	public function __isset($key)
	{
		if ($this->accessColumn($key)) {
			return isset($this->data[$key]);
		}

		$referenced = $this->table->getReferencedTable($this, $key);
		if ($referenced !== false) {
			$this->accessColumn($key, false);
			return true;
		}

		$this->removeAccessColumn($key);
		return false;
	}
dg commented

Fixed #196

Sorry for breaking the test Database/Table/Table.discoveredReflection.phpt. During my testing, I was skipped because I had a bad Database.ini.

How to combine these two things?
Assert::true(isset($row->author->name)) and Assert::false(isset($row->author))

dg commented

I will modify the tests because behavior has simply changed.

There is commit with fix the test miranovy/database@023527d