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

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.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 4
  • Comments: 18 (6 by maintainers)

Commits related to this issue

Most upvoted comments

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

Fixed in https://github.com/phalcon/cphalcon/pull/15195 and just released with 4.1

@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.

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

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

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.

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
    );
}