Bug: Builder (with field bool)

[dgavrilenko]

Critical.

Env:
new core '4.1.2'
old core '4.0.4'
db PostgreSql.
os: ubuntu 18.
php 7.4

Hello, bug in Builder:

new core:

id exist in the database

$model = new Model();
$model->save((['id' => 1, 'field_bool' => false]); (works with true)

will give an error. (ErrorException)

old core:

$builder->set($data, '', null);

new core:

// Must use the set() method to ensure to set the correct escape flag
(system/Model.php line 320)
foreach ($data as $key => $val)
{
$builder->set($key, $val, $escape[$key] ?? null);
}

because of the type. false is converted to empty

public function set($key, ?string $value = '', bool $escape = null)

and works for the old kernel, since the $key is an array

also the difference builder (maybe it's ok)

$model = new Model();
$entity = new Entity([
'id' => 1,
'text' => true/false, (bool in string field)
]);

update text - 1/0 (new core)
save text true/false (old core)

[dgavrilenko]

If you do not have functional tests for saving data to DB, please add to catch such errors (for different types of fields and for arrays of std classes and entities)

[dgavrilenko]

$model = new Model();
$model->save((['id' => 1, 'field_bool' => 0/1/true/false]);

new core bool in the bool field, assign false - error, 0/1/true - it works
old core - bool in the bool field, assign false/true work, 0/1 - does not work.

It seems like it worked correctly in the old kernel, but in the new one it falls to false.

there is no usual compatibility in the code, sadness (

as a result, the constructor is used, and the typed postgresql project with type bool will crash.

	// Must use the set() method to ensure to set the correct escape flag
	/*foreach ($data as $key => $val)
	{
		$builder->set($key, $val, $escape[$key] ?? null);
	}*/

	// compatibility issues old code  for insert/update - it works
    $builder->set($data, '', null);

[dgavrilenko]

namespace App\Integrations\Framework;

use App\Enum\UsersEnum;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\DatabaseTestTrait;
use App\Entities\Entity;
use App\Models\BaseModel;
use ErrorException;

class EntityTest extends Entity
{
protected $casts = [
'is_field' => 'bool',
];
protected function castAs($value, string $attribute, string $method = 'get')
{
if (array_key_exists($attribute, $this->casts)) {
$type = $this->casts[$attribute];

        if (strpos($type, '?') === 0) {
            if ($value === null) {
                return null;
            }
            $type = substr($type, 1);
        }

        switch($type) {
            case 'bool':
            case 'boolean':
                return redefine($value);  // Redefine  App\Models\BaseModel bool handling for postgresq  ('t' -> true, 'f' => false)
        }
    }

    return parent::castAs($value, $attribute, $method);
}

}

class BaseModelTest extends BaseModel
{
protected $useTimestamps = false;

protected $returnType = EntityTest::class;

protected $table = '_test_table_';
protected $allowedFields = [
    'is_field',
];

public static function query(): self
{
    return new static;
}

}

class ModelTest extends CIUnitTestCase
{
use DatabaseTestTrait;

protected $refresh = false;

private $table = '_test_table_';

public function setUp(): void
{
    parent::setUp();

    // login as system user
    user()->loginUsingId(UsersEnum::ADMIN_ID);

    $this->entity = new class extends Entity {
        protected $casts = [
            'is_field' => 'bool',
        ];
    };

    $sql = "CREATE TABLE IF NOT EXISTS {$this->table}(
            id SERIAL PRIMARY KEY,
            is_field BOOL DEFAULT NULL,
            int_id INTEGER DEFAULT NULL
        )
    ";

    $this->db->query($sql);
}

public function tearDown(): void
{
    parent::tearDown();

    $sql = "drop table {$this->table}";
    $this->db->query($sql);
}

public function get($id)
{
    return BaseModelTest::query()->find($id);
}

public function save($value, $field = 'is_field')
{
    return BaseModelTest::query()->insert([
        $field => $value,
    ]);
}

public function update($id, $value, $field = 'is_field')
{
    return BaseModelTest::query()->update($id, [$field => $value]);
}

public function testArrayForBoolFieldSaveAndUpdateCase1(): void
{
    // save bool true
    self::assertIsInt($id = $this->save(true));
    self::assertTrue($this->get($id)->is_field);
    // update false
    self::assertTrue($this->update($id,false));
    self::assertFalse($this->get($id)->is_field);
    // update bool null
    self::assertTrue($this->update($id,null));
    self::assertNull($this->get($id)->is_field);
    // update bool 1
    $this->expectException(ErrorException::class);
    $this->update($id,1); // expecting an ErrorException
}

public function testArrayForBoolFieldSaveAndUpdateCase2(): void
{
    // save bool false
    self::assertIsInt($id = $this->save(false));
    self::assertFalse($this->get($id)->is_field);
    // update bool true
    self::assertTrue($this->update($id, true));
    self::assertTrue($this->get($id)->is_field);

    $this->expectException(ErrorException::class);
    // update bool 0
    $this->update($id, 0); // expecting an ErrorException

    // bool null
    self::assertTrue($this->update($id,'')); // expecting an ErrorException          TODO we can lose the error
}

public function testSaveArrayWithFieldBoolFalseException(): void
{
    $this->expectException(ErrorException::class);
    // save bool 0
    $this->save(0); // // expecting an ErrorException
}

public function testSaveArrayWithFieldBoolTrueException(): void
{
    $this->expectException(ErrorException::class);
    // save bool 1
    $this->save(1); // // expecting an ErrorException
}

}

[paulbalandan]

Fixed in #4907