propelorm / Propel3

High performance data-mapper ORM with optional active-record traits for RAD and modern PHP 7.2+
MIT License
251 stars 35 forks source link

Related objects persistance #56

Closed cristianoc72 closed 7 years ago

cristianoc72 commented 7 years ago

Related objects of a given entity are not sincronized with the database (this issue is reproduced along Propel\Tests\Generator\Builder\Om\GeneratedObjectRelTest test case).

Get a look at the following code:

$author = new Author();
$author->setName('Oscar Wilde');
$author->save();

$book1 = new Book();
$book1->setTitle('The picture of Dorian Gray');
$book1->setAuthor($author);

$book2 =new Book();
$book2->setTitle('The importance of being Ernest');
$author->addBook($book2);
$author->save();

Everything is ok: 1 author related with 2 books, both in memory and in db. If I add a book:

$book3 = new Book();
$book3->setTitle('The Duchess of Padua');
$book3->setAuthor($author);
$author->save();

Everything is ok: 1 author related with 3 books, both in memory and in db. But if I remove a book:

$author->removeBook($book1);
$author->save();

Now I have: 1 author and 2 books in memory and 1 Author and 3 books in the db. If I add another book, then I find 4 books in the db.

Possible solutions:

  1. when removing related objects, perform a query to also delete the entity from the database. This piece of code should be written in removeXXX method.
  2. implement a way to mark the entity as 'deleted' and let the persister do the deletion work, via presistDependencies method
  3. implement a preSave hook which cleanup the database before persistence
  4. (suggestions?)

Suggestions are wellcome!

marcj commented 7 years ago

Shouldn't you have still book3 in memory? removeBook should only break the relation, not delete the book3 entity from the database in my opinion.

cristianoc72 commented 7 years ago

Ok! Anyway, removeBook should update the database, because if I break the relation, the removed object is no more a dependency and it's not persisted with the Author entity, isn't it?

marcj commented 7 years ago

Yes, it should update the database. It actually is still related, as the db state indicates still that it is related. Once it goes through the change detection it needs to check whether it sets the foreign key from Book to Author accordingly to null.

marcj commented 7 years ago

Yes it should set the foreign key to null. But I thought this works already😱

Sent from my iPhone

On 14. May 2017, at 18:42, Cristiano Cinotti notifications@github.com wrote:

Ok! Anyway, removeBook should update the database, because if I break the relation, the removed object is no more a dependency and it's not persisted with the Author entity, isn't it?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

cristianoc72 commented 7 years ago

Found! The in-memory process works correctly (as @marcj told before) it seems that the modification isn't persisted. Anyway, I misunderstood a couple of test cases :grimacing: ..... I'm closing this issue. Thank you!