[BUG]: Model virtual FK query not saving new value after execution

[svdigital-development]

Describe the bug

Related to: #15625, in particular this comment: #15625 (comment)

When using a Model with a "belongsTo()" relationship it is possible to use the "navigation magic method" using the alias name given to belongsTo()'s options to obtain the related FK Model instance from the first Model's instance.

If I use that magic method before changing the value of that FK on the first Model's instance, after saving the model the FK value is still the previous one.

To Reproduce

I will show an example using model "Promotion", which contains a FK to another model "Person".

Model Promotion:

public function initialize() {
	parent::initialize();

	$this->setSource( 'promotion' );

	$this->belongsTo( 'PersonId', Person::class, 'Id', [
		'alias' => 'Person',
		'foreignKey' => [
			'allowNulls' => true,
		],
	] );
}

NOT working example:

/* Obtain the instance of an existing Promotion */
$promotion = Promotion::findFirstById( 1 );

var_dump( $promotion->getPersonId() ); // returns "222"

$person = $promotion->getPerson(); // returns the instance of Person with "Id = 222"

/* Change the related Person record with another one */
$promotion->setPersonId( 333 );

$promotion->save();

var_dump( $promotion->getPersonId() ); // returns "222" instead of "333"

Working example:

/* Obtain the instance of an existing Promotion */
$promotion = Promotion::findFirstById( 1 );

var_dump( $promotion->getPersonId() ); // returns "222"

/* I am NOT using the FK magic method */
/* $person = $promotion->getPerson(); */

/* Change the related Person record with another one */
$promotion->setPersonId( 333 );

$promotion->save();

var_dump( $promotion->getPersonId() ); // returns "333": CORRECT

PHP version: 7.4.22
Phalcon version: 4.1.2
MySql 5.7 (Percona), with InnoDB tables - FKs are handled by MySql
Debian GNU/Linux 10

I am using dynamic updates and snapshots on all Models, but disabling them does not change anything.

I tried to explicitly specify 'reusable' => false in belongsTo()'s options but it didn't change anything.

I tried to disable FOREIGN_KEY_CHECKS on MySql before exetuing the test code, but it didn't change anything.

I tried to setup models at the beginning with: 'virtualForeignKeys' => false,, but it didn't change anything.

I'm not using any cache inside my Models, at least AFAIK: I have not explicitly specified any cache settings in any of my models. I'm using Phalcon's Cache but not inside Models, just to cache some data to reuse it, but nothing related to Models.

Expected behavior

The new FK value is saved with or without using its magic FK method before changing it.

I am upgrading from Phalcon 3.4.4 to Phalcon 4.1.2.

Everything works correctly except for this behaviour.

The code didn't change during the upgrade and the exact same code is working on:

PHP version: 7.3.27
Phalcon version: 3.4.4

and it's NOT working on:

PHP version: 7.4.22
Phalcon version: 4.1.2

Details

phalcon

Phalcon is a full stack PHP framework, delivered as a PHP extension, offering lower resource consumption and high performance.
phalcon => enabled
Author => Phalcon Team and contributors
Version => 4.1.2
Build Date => May  1 2021 19:01:51
Powered by Zephir => Version 0.12.21-8a412a1

Directive => Local Value => Master Value
phalcon.db.escape_identifiers => On => On
phalcon.db.force_casting => Off => Off
phalcon.orm.case_insensitive_column_map => Off => Off
phalcon.orm.cast_last_insert_id_to_int => Off => Off
phalcon.orm.cast_on_hydrate => Off => Off
phalcon.orm.column_renaming => On => On
phalcon.orm.disable_assign_setters => Off => Off
phalcon.orm.enable_implicit_joins => On => On
phalcon.orm.enable_literals => On => On
phalcon.orm.events => On => On
phalcon.orm.exception_on_failed_save => Off => Off
phalcon.orm.exception_on_failed_metadata_save => On => On
phalcon.orm.ignore_unknown_columns => Off => Off
phalcon.orm.late_state_binding => Off => Off
phalcon.orm.not_null_validations => On => On
phalcon.orm.update_snapshot_on_save => On => On
phalcon.orm.virtual_foreign_keys => On => On
phalcon.warning.enable => On => On

• Phalcon version: 4.2.1
• PHP Version: (php -v)

PHP 7.4.22 (cli) (built: Jul 30 2021 13:09:18) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.22, Copyright (c), by Zend Technologies

• Operating System: Linux Debian
• Installation type: installing via package manager
• Server: Nginx
• Other related info (Database, table schema): MySql 5.7.30-33 (Percona Server (GPL), Release '33', Revision '6517692')

[Jeckerson]

/cc @niden

[svdigital-development]

I can suggest a complete test case along with DB tables. Everything is very "basic": I use \Phalcon\Mvc\Model class directly to extend the Models and I don't extend any other method.

1. Create table zzz_test_a:

CREATE TABLE `zzz_test_a` (
`zzz_test_a_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
`description` varchar(255) DEFAULT NULL,
PRIMARY KEY (`zzz_test_a_id`)
) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=utf8;

2. Create table zzz_test_b:

CREATE TABLE `zzz_test_b` (
`zzz_test_b_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
`zzz_test_a_id` int(10) unsigned NOT NULL,
`table_name` varchar(45) DEFAULT NULL,
PRIMARY KEY (`zzz_test_b_id`),
KEY `zzz_test_b_zzz_test_a_idx` (`zzz_test_a_id`),
CONSTRAINT `zzz_test_b_zzz_test_a` FOREIGN KEY (`zzz_test_a_id`) REFERENCES `zzz_test_a` (`zzz_test_a_id`) ON DELETE NO ACTION ON UPDATE CASCADE
) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8;

3. Models:

class ZzzTestA extends \Phalcon\Mvc\Model {
	// Protected properties for fields with public getters and setters
	
	public function initialize() {
		$this->setConnectionService( 'db' );
		$this->setSchema( 'db_name' );
		
		$this->useDynamicUpdate( true );
		$this->keepSnapshots( true );
		
		$this->setSource( 'zzz_test_a' );
		
		$this->hasMany( 'Id', ZzzTestB::class, 'ZzzTestAId', [ 'alias' => 'ZzzTestBs' ] );
	}
	
	public function columnMap() {
		return [
			'zzz_test_a_id' => 'Id',
			'description' => 'Description',
		];
	}
}


class ZzzTestB extends \Phalcon\Mvc\Model {
	// Protected properties for fields with public getters and setters
	
	public function initialize() {
		$this->setConnectionService( 'db' );
		$this->setSchema( 'db_name' );
		
		$this->useDynamicUpdate( true );
		$this->keepSnapshots( true );
		
		$this->setSource( 'zzz_test_a' );
		
		$this->belongsTo( 'ZzzTestAId', ZzzTestA::class, 'Id', [ 'alias' => 'ZzzTestA' ] );
	}
	
	public function columnMap() {
		return [
			'zzz_test_b_id' => 'Id',
			'zzz_test_a_id' => 'ZzzTestAId',
			'table_name' => 'TableName',
		];
	}
}

Test case

$zzzA = new ZzzTestA();
$zzzA->setDescription( 'Test A 100' );
$zzzA->save();

$id1 = $zzzA->getId();

$zzzA = new ZzzTestA();
$zzzA->setDescription( 'Test A 200' );
$zzzA->save();

$id2 = $zzzA->getId();

$this->assertNotEquals( $id1, $id2 );

$zzzB = new ZzzTestB();
$zzzB->setZzzTestAId( $id1 );
$zzzB->setTableName( 'Test B 100' );
$zzzB->save();

$idB = $zzzB->getId();

$testB = ZzzTestB::findFirst( $idB );

$this->assertEquals( $id1, (int)$testB->getZzzTestAId(), 'FIRST CHECK' );

$testB->getZzzTestA();

$testB->setZzzTestAId( $id2 );
$result = $testB->save();

$this->assertTrue( $result );

$this->assertEquals( $id2, (int)$testB->getZzzTestAId(), 'SECOND CHECK' );

Test case result:

SECOND CHECK
Failed asserting that 6 matches expected '7'.
Expected :7
Actual   :6

[svdigital-development]

I tried to work on "related" and "dirtyRelated" arrays inside save().

In my previous example (#15649 (comment)), inside class ZzzTestB extends \Phalcon\Mvc\Model model class I extend save() method this way:

public function save(): bool {
	$this->related['ZzzTestA'] = ZzzTestA::findFirstById( $this->ZzzTestAId );
	
	return parent::save();
}

The test passes.

Inside extended save() method, $this->ZzzTestAId is correctly updated. Its value changes somewhere inside parent::save(). Before calling parent::save() I force $this->related to have the "new" record of ZzzTestA, which $this->ZzzTestAId correctly has at that point.

This "fixes" the problem. This is obviously not an adequate fix, but a clue as to what could be causing the problem.

[svdigital-development]

I think the possible bug is here:

Model::getRelated()

        if arguments === null {
            /**
             * If the related records are already in cache and the relation is reusable,
             * we return the cached records.
             */
            if relation->isReusable() && this->isRelationshipLoaded(lowerAlias) {
                let result = this->related[lowerAlias];
            } else {
                /**
                 * Call the 'getRelationRecords' in the models manager.
                 */
                let result = manager->getRelationRecords(relation, this, arguments);

                /**
                 * We store relationship objects in the related cache if there were no arguments.
                 */
                let this->related[lowerAlias] = result;
            }
        } else {
            /**
             * Individually queried related records are handled by Manager.
             * The Manager also checks and stores reusable records.
             */
            let result = manager->getRelationRecords(relation, this, arguments);
        }

I think this is not correct:

                /**
                 * We store relationship objects in the related cache if there were no arguments.
                 */
                let this->related[lowerAlias] = result;

because this->related contains the "old" related record. After changing the "id" FK value on ZzzTestB model instance and then invoking save(), this->related still contains the old record, becuase ZzzTestB::setZzzTestAId() is a real method, defined inside ZzzTestB.

During save() execution, Model::collectRelatedToSave() is executed (see: https://github.com/phalcon/cphalcon/blob/v4.1.2/phalcon/Mvc/Model.zep#L977); the "for name, record in related" cycle executes let dirtyRelated[name] = record; (line 1001), but in that assignment record is the old related record coming from this->related which is never updated.

If I extend the method inside my ZzzTestB model like this:

public function getRelated( string $alias, $arguments = null ) {
	if( is_null( $arguments ) ) {
		$arguments = [];
	}
	
	return parent::getRelated( $alias, $arguments );
}

The test passes.

I force $arguments to be not null, avoiding this->related assignment. I'm sure this is NOT a correct solution as well, but could be another clue to the problem.

[svdigital-development]

Sorry, in my previous comment I said "I think this is not correct:" regarding this line: let this->related[lowerAlias] = result; - Actually that is correct, the problem is that let this->related is not updated anymore, even when updating its FK keys inside the model instance

[rtiagom]

same issue here

[niden]

Addressed in #15702