gabordemooij / redbean

ORM layer that creates models, config and database on the fly
https://www.redbeanphp.com
2.3k stars 280 forks source link

Unexpected results in xown #921

Closed marios88 closed 1 year ago

marios88 commented 1 year ago

Please see the attached code snippet as explaining the issue is a bit hard.

<?php

require_once __DIR__ . '/vendor/autoload.php';

use \RedBeanPHP\R as R;

R::setup();
R::fancyDebug(true);
R::nuke();

$person = R::dispense('person');
$person->name = 'John';
$person->surname = 'Doe';

for($i = 1; $i <= 3; $i++){
    $phone = R::dispense('phonenumber');
    $phone->number = $i;
    $phone->type = 'mobile';
    $person->xownPhonenumberList[] = $phone;
}

R::store($person);

$person = R::load('person', 1);
$phone1 = R::load('phonenumber', 1);
$phone1->number = 11;
$person->xownPhonenumberList[$phone1->id] = $phone1;
R::store($person);

foreach ($person->xownPhonenumberList as $phone){
    echo $phone->number.PHP_EOL;
}

// resulting
// DELETE FROM `phonenumber`  WHERE ( `id` IN ( 1 ) )
// ...
// UPDATE `phonenumber` SET  `number` = 11 , `type` = 'mobile' , `person_id` = 1  WHERE id = 1
// Expected:
// UPDATE `phonenumber` SET `number` = 11 WHERE ( `id` IN ( 1 ) )
gabordemooij commented 1 year ago

Thanks for the clear example.

This happens because RedBeanPHP uses a fast 1-pass array_diff() operation to determine which beans to delete, update and add. With non-exclusive lists, this would probably not be a problem. This is a limitation of RedBeanPHP. A workaround might be: $person->xownPhonenumberList[$phone1->id]->number = 11;

Not sure if I can do anything about it in the short term, there is no simple fix for this (without breaking a lot of stuff).

marios88 commented 1 year ago

own works but the queries are funny

UPDATE `phonenumber` SET  `number` = 1 , `type` = 'mobile' , `person_id` = NULL  WHERE id = 1 
UPDATE `phonenumber` SET  `number` = 11 , `type` = 'mobile' , `person_id` = 1  WHERE id = 1 

To make sure we are on the same page, the method responsible for this is

https://github.com/gabordemooij/redbean/blob/b6ab15f85edacf98b40ff8e1a287fcaf56e024c2/RedBeanPHP/Repository.php#L150

The workaround you provided works as expected but unfortunately does not fit my current requirements. For now i wrap it up a transaction like this, without touching the xOwn list

R::begin();
...
R::store($phone1);
unset($person[$phone2->id]); // or R::trash();
R::store($person);
...
//run validation
...
if($validation === true){
  R::commit();
}else{
  R::rollback();
}
gabordemooij commented 1 year ago

Why not simply modify the bean in-place? That would be the RedBean-way I guess...

marios88 commented 1 year ago

Thanks, i will look into it and reopen if needed!