[BUG]: When updating including a relation, the relation destination is not updated

[fagai]

Describe the bug

The following updates will not be updated.

$user = User::find(1);
$user->userCard->token = $token;
$user->save(); // not update(4.0), update(3.4)

You can reinsert the object changed in this way,

$user = User::find(1);
$userCard = $user->userCard;
$userCard->token = $token;
$user->userCard = $userCard;
$user->save(); // update!

It works if you run update directly at the relation destination.

$user = User::find(1);
$user->userCard->token = $token;
$user->userCard->save(); // update!

I think this is because it has not been possible to detect that the data of the relation destination has been dirty.
This worked until v3.4.

Details

• Phalcon version: 4.0.6
• PHP Version: 7.4.9
• Operating System: debian(docker)
• Installation type: Compiling from source
• Zephir version (if any):
• Server: Apache
• Other related info (Database, table schema):

Additional context
Add any other context about the problem here.

[zsilbi]

I couldn't find anything in the docs (https://docs.phalcon.io/4.0/en/db-models-relationships) about this use case.
The fixes I made in #14035 affected this issue because I was not aware of this possible functionality.

I've always used it like your second example for updates:

$user = User::find(1);
$userCard = $user->userCard;
$userCard->token = $token;
$user->userCard = $userCard;
$user->save(); // update!

@phalcon/core-team, what's your opinion?

[gponcon]

I have the same problem:

# (Address BelongsTo User)
$address = Address::find(1);
$user = new User();
$user->address = $address;

# 3.4: _preSaveRelatedRecords() triggered, _preSave() triggered
# 4.0: _preSaveRelatedRecords() triggered, _preSave() triggered
$user->save();

$user->address->city = 'Paris';

# 3.4: _preSaveRelatedRecords() triggered, _preSave() triggered (saved)
# 4.0: _preSaveRelatedRecords() NOT triggered, _preSave() triggered (NOT saved)
$user->save();

However, reinserting the related record does not work for me either:

# ...
$address = $user->address;
$address->city = 'Paris';
$user->address = $address;

# 3.4: _preSaveRelatedRecords() triggered, _preSave() triggered (saved)
# 4.0: _preSaveRelatedRecords() NOT triggered, _preSave() triggered (NOT saved)
$user->save();

Setting keepSnapshots to true or call explicitly _preSaveRelatedRecords() in _preSave() has no effect.

I did not find an adequate option in the documentation, I'm just Imagine that it should work using ->save() (https://docs.phalcon.io/4.0/en/db-models-relationships#save).

• Phalcon 4.0.6
• PHP 7.4.11
• Alpine Linux on Docker

Any idea?

[zsilbi]

We are going to fix and document this before the next release.

[zsilbi]

Fixed in #15195 and just released with 4.1

[gponcon]

Thank you very much for your great work @zsilbi.

Your fix works for me, but maybe only partially.

With hasOne / hasMany relationships (as in your test), it's perfect. However, if you replace hasOne() by belongsTo() in the Invoices.php model, values are not updated, I don't know why:

1) SaveCest: Test Mvc\Model - save() with related records property
 Test tests/database/Mvc/Model/SaveCest.php:mvcModelSaveWithRelatedRecordsProperty
 Step Assert equals "new_firstName", "test_firstName_1
 Fail Failed asserting that two strings are equal.
- Expected | + Actual
@@ @@
-'new_firstName
+'test_firstName_1'.

Scenario Steps:

 5. $I->assertEquals("new_firstName", "test_firstName_1") at tests/database/Mvc/Model/SaveCest.php:240

But maybe it's a problem of understanding on my part...

[Jeckerson]

@gponcon Seems that in your case belongsTo() must be not inside Invoices model, but inside another, inside hasOne() declaration.

class Invoice
   hasOne('id', InvoiceDocument::class, 'invoice_id')

class InvoiceDocument
  belongsTo('invoice_id', Invoice::class, 'id')

[gponcon]

Thank you @Jeckerson for this clarification.

I asked this question because we have a lot of hasMany() <-> belongsTo() relations that work with Phalcon 3.4 and not with Phalcon 4, for example:

class Customer
   hasMany('id', Invoice::class, 'invoice_id')

class Invoice
   belongsTo('customer_id', Customer::class, 'id')

Invoice is saved before Customer when doing this:

$invoice->customer->name = 'baz';
$invoice->save();

With Phalcon 4, Customer is no longer saved.

In the unit tests, we have a hasMany() <-> hasOne() relationship as in the documentation.

[zsilbi]

Let's reopen this until I add some more tests to verify this.

[gponcon]

You can try with CustomersKeepSnapshots for example:

public function mvcModelSaveWithRelatedManyAndBelongsRecordsProperty(DatabaseTester $I)
{
    $I->wantToTest('Mvc\Model - save() with related records property (relation many - belongs)');

    /** @var \PDO $connection */
    $connection = $I->getConnection();

    $invoicesMigration = new InvoicesMigration($connection);
    $invoicesMigration->clear();
    $invoicesMigration->insert(77, 1, 0, uniqid('inv-', true));

    $customersMigration = new CustomersMigration($connection);
    $customersMigration->clear();
    $customersMigration->insert(1, 1, 'test_firstName_1', 'test_lastName_1');

    /**
     * @var Invoices $invoice
     */
    $invoice = InvoicesKeepSnapshots::findFirst(77);

    $I->assertEquals(
        1,
        $invoice->customer->id
    );

    $invoice->customer->cst_name_first  = 'new_firstName';
    $invoice->customer->cst_status_flag = 0;

    $I->assertTrue(
        $invoice->save()
    );

    /**
     * @var Customers $customer
     */
    $customer = Customers::findFirst(1);

    $I->assertEquals(
        'new_firstName',
        $customer->cst_name_first
    );

    $I->assertEquals(
        0,
        $customer->cst_status_flag
    );
}

[fagai]

I have not yet verified if this issue has been resolved.
To confirm this issue correctly

• Enable keepSnapshots
• Enable reusable options

It is necessary to set the above two and verify whether it works.

[gponcon]

For both InvoicesKeepSnapshots and CustomersKeepSnapshots models, keepSnapshots and reusable seem to be activated in their initialize():

// ...
public function initialize()
{
    $this->keepSnapshots(true);
    $this->setSource('co_customers');

    $this->hasMany(
        cst_id',
        InvoicesKeepSnapshots::class,
        inv_cst_id',
        [
            alias' => 'invoices',
            reusable' => true,
        ]
    );
}
// ...

(but InvoicesKeepSnapshots is linked to Customers...)

I'm trying to investigate.

[gponcon]

I don't know if I've done it right, but if I make these changes it seems to work:

https://github.com/phalcon/cphalcon/pull/15203/files

Note that I am not yet familiar with Zephir and the algorithm behind the MVC models.

At your disposal.

[bouzikas]

Hello,
After a lot of trial and error dealing with this issue, I realized that it's working only when you set reusable to true.

$invoice = Invoice::findFirstById(1);
$invoice->user->total_invoices = 10;
$invoice->save(); // doens't work
$invoice->user->save(); // work

Invoice Model:

$this->belongsTo(
    'user_id',
    'User',
    'id',
    [
        'alias' => 'user',
        'reusable' => true
    ]
);

Setting reusable to false $invoice->user->save(); doesn't work either.

• Phalcon version: 4.1.0
• PHP Version: 7.3.24
• Operating System: Centos7 (docker)
• Installation type: yum install php-phalcon4
• Server: Apache 2.4
• mysql image: mysql:5.5.62

[fagai]

@bouzikas

Thank you.
But this issue is about $invoice->save();.

$invoice = Invoice::findFirstById(1);
$invoice->user->total_invoices = 10;
$invoice->save(); // doens't work ← this issue
$invoice->user->save(); // work

[mikacalvo]

I have a similar problem but with an even simpler reproduction.

/**
 * @property Toolbox Toolbox
 * @method Toolbox getToolbox($parameters = null)
 * @method static WidgetConfig[]|Resultset find($parameters = null)
 * @method static WidgetConfig|ModelInterface findFirst($parameters = null)
 */
class WidgetConfig extends Phalcon\Mvc\Model
{
    /**
     * @var integer
     * @Primary
     * @Identity
     * @Column(column="id", type="integer", length=11, nullable=false)
     */
    protected $id;

    /**
     * @var integer
     * @Column(column="toolbox_id", type="integer", length=11, nullable=true)
     */
    protected $toolboxId;

    /**
     * @param integer $id
     * @return $this
     */
    public function setId($id)
    {
        $this->id = $id;

        return $this;
    }

    /**
     * @param integer $toolboxId
     * @return $this
     */
    public function setToolboxId($toolboxId)
    {
        $this->toolboxId = $toolboxId;

        return $this;
    }

    /**
     * Initialize method for model.
     */
    public function initialize()
    {
        $this->belongsTo(
            static::TOOLBOX_ID,
            Toolbox::class,
            static::ID,
            [static::ALIAS => 'Toolbox']
        );
    }
}

class WidgetConfigTest extends Codeception\Test\Unit
{

    /**
     * @throws Exception
     * @throws \Phalcon\Exception
     */
    public function testRelationships()
    {
        $toolbox = new Toolbox();
        $toolbox
            ->setId(99)
            ->setTranslationKey('test');

        $w = new \Models\Widget();
        $w
            ->setName('test')
            ->setWidgetTypeId(5)
            ->save();

        $wc = new \Models\WidgetConfig();

        // first, set it to null
        $wc->Toolbox = null;
        static::assertNull($wc->Toolbox); // success
        static::assertNull($wc->getToolbox()); // success

        $wc->Toolbox = $toolbox;
        static::assertNotNull($wc->Toolbox); // success
        static::assertNotNull($wc->getToolbox()); // fail

        $wc
            ->setWidgetId($w->getId())
            ->save();

        static::assertNotNull($wc->Toolbox); // success
        static::assertNotNull($wc->getToolbox()); // fail
        static::assertNotNull($wc->getToolboxId()); // fail

        $wc = WidgetConfig::findFirst($wc->getId());

        static::assertNotNull($wc->Toolbox); // fail
        static::assertNotNull($wc->getToolbox()); // fail
    }

}

If I don't set my toolbox to null in the first place, every test is a success. It seems we can't instantiate two times that same alias ?

PS : I'm still on 4.0.3, I will test it after I migrate to the latest
PS2 : 'reusable' => true, didn't change anything

[niden]

@gponcon here: #15203

Resolved in #15838

[kirill-voronov]

@zsilbi @niden
Hello, we are trying to migrate from 4.0 to 5.0 and got unexpected behaviour because now related records are updating always even if they haven't changed.

Here is an example:

        $user = User::findFirstById(1);
        foreach ($user->getInvoices() as $item) {
            // Just check something here without any changes
        }

        foreach ($user->getSomeAnotherRelatedModels() as $item) {
            // Just check something here without any changes
        }

        foreach ($user->getSomeThirdTypeRelatedModels() as $item) {
            // Just check something here without any changes
        }

        $user->someProp = 'some value';
        $user->save();

This will trigger saving user and all related models. If we have 100 related invoices + 100 someAnotherRelatedModels + 100 someThirdTypeRelatedModels, there will be 301 update query sent into DB and 300 of those queries are parasite, because there wasn't actual changes in related models.

This happens even if reusable option is enabled or not because code block here was commented (and all queried relations now always put into this->related) and here related and dirtyRelated are join together.

Cascade updates like

$model = SomeModel::findFirst();
$model2 = $model->getSomeRelatedModel(); // No changes was made to $model2
$items = $model2->getSomeModels(); // No changes was made to $items
$model3 = $model2->getSomeRelatedModel(); // No changes was made to $model3
$items2 = $model3->getSomeModels(); // No changes was made to $items2
$model->someValue = 'value';
$model->save(); // Updates all of the models above

slow down our application, we need an approach to turn off this behaviour.

[larsverp]

@kirill-voronov We're having the exact same issue in our application after updating to Phalcon 5. Any fix found yet? :)