gabordemooij / redbean

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

Preloading yields errors when referenced beans are not in DB #307

Closed rr- closed 11 years ago

rr- commented 11 years ago

Using R::preload($books, 'author') when some $book references author that isn't in DB anymore yields errors:

Undefined offset: 0 at (...)/lib/redbean/RedBean/Preloader.php:428

That's because Preloader assigns beans by their ids and the id of non-existent tainted author bean is equal to 0. Since it isn't available in $map variable, user gets undefined offset: 0 error.

Expected behaviour would be to get problematic $book->author to contain the tainted bean.

gabordemooij commented 11 years ago

Not sure I fully understand what's going on here...

Can I see the usage scenario in code that leads to this error? Are you trying to preload beans that have not been saved yet?

And what do you mean by contain the 'tainted' bean?

rr- commented 11 years ago
<?php
require_once 'redbean/RedBean/redbean.inc.php';

file_exists('test.sqlite') and unlink('test.sqlite');
R::setup('sqlite:test.sqlite');

$users = R::dispense('user', 2);
$users[0]->name = 'john';
$users[1]->name = 'dave';
R::store($users[0]);
R::store($users[1]);

$posts = R::dispense('post', 2);
$posts[0]->author = $users[0];
$posts[0]->text = 'great post 1';
$posts[1]->author = $users[1];
$posts[1]->text = 'great post 2';
R::store($posts[0]);
R::store($posts[1]);

//needed to have orphaned foreign keys so "ON DELETE CASCADE" won't kick in
R::exec('PRAGMA foreign_keys = OFF');

R::trash($users[0]);

$posts = R::find('post');
R::preload($posts, ['author' => 'user']);

foreach ($posts as $post)
{
    echo 'Post text: ' . $post->text . PHP_EOL;
    echo 'User: ' . $post->author_id . PHP_EOL;
    echo 'User name: ' . $post->author->name . PHP_EOL;
}

This code yields:

PHP Notice:  Undefined offset: 0 in /home/rr-/redbean/RedBean/Preloader.php on line 428
PHP Warning:  Invalid argument supplied for foreach() in /home/rr-/redbean/RedBean/Preloader.php on line 428
Post text: great post 1
User: 1
User name:
Post text: great post 2
User: 2
User name: dave

By "tainted" bean I mean one that is created on the fly and has $bean->__info['tainted'] = true (var_dump($parent); in Preloader::preloadParentBeans($type, $field, $ids, $map) yields):

object(RedBean_OODBBean)#19 (9) {
  ["properties":"RedBean_OODBBean":private]=>
  array(1) {
    ["id"]=>
    int(0)
  }
  ["__info":"RedBean_OODBBean":private]=>
  array(4) {
    ["type"]=>
    string(4) "user"
    ["sys.id"]=>
    string(2) "id"
    ["sys.orig"]=>
    array(1) {
      ["id"]=>
      int(0)
    }
    ["tainted"]=>
    bool(true)
  }
  ["beanHelper":"RedBean_OODBBean":private]=>
  object(RedBean_BeanHelper_Facade)#5 (0) {
  }
  ["fetchType":"RedBean_OODBBean":private]=>
  NULL
  ["withSql":"RedBean_OODBBean":private]=>
  string(0) ""
  ["withParams":"RedBean_OODBBean":private]=>
  array(0) {
  }
  ["aliasName":"RedBean_OODBBean":private]=>
  NULL
  ["via":"RedBean_OODBBean":private]=>
  NULL
}
gabordemooij commented 11 years ago

It seems that:

PRAGMA foreign_keys = OFF

will mess up database integrity, you end up with a broken key in post 1.

you're example works just fine if I comment this line out. RedBeanPHP will not perform a cascaded delete unless you have told it to do so (if you have added a constraint by accident you have to remove it in your database client manually).

rr- commented 11 years ago

Yep, that's exactly my point - when integrity is broken, R produces PHP warnings. It's uncool. :crying_cat_face:

Also, during development of my app I never did anything like PRAGMA foreign_keys = OFF and yet I ended up with broken integrity... somehow. (I don't know how. I just went to inspect the DB only after I discovered the warnings.)

gabordemooij commented 11 years ago

That should not be possible. By default the key has to be set to NULL. Anyway that would be a different bug. Given broken integrity everything collapses, there is no 'solution' for this, hiding the problem would make it even worse.

rr- commented 11 years ago

That's right, however a friendly warning or even an exception would be more helpful I guess. Just saying.

gabordemooij commented 11 years ago

Okay, that could be arranged.

gabordemooij commented 11 years ago

Tried to implement this but it's actually almost untestable.