Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change ObjectBuilder to reload related objects when collection is partia... #389

Merged
merged 5 commits into from Jul 13, 2012

Conversation

CyExy
Copy link
Contributor

@CyExy CyExy commented Jun 20, 2012

We are migrating from Propel 1.4 to 1.6 and have came up to some BC issues.

Given that Author (A1) has 2 Books: B1, B2

<?php

$c = new Criteria();
$c->add(BookPeer::ID, B1);

$books = BookPeer::doSelectJoinAuthor($c);

// poisoned cache issue, returns only the book B1
$books[0]->getAuthor()->getBooks();

When instance pooling has been turned on it becomes even bigger issue, because code down the line would get the object with poisoned cache from the pool

<?php
// ...
// because of the query above, this object will returns only the book B1
AuthorPeer::retrieveByPK(A1)->getBooks();

There are other circumstances where this would happen, like when saving an Book object that has the Author object loaded, for instance.

If you find any issues with this fix please let me know, I'll try to address them.

@travisbot
Copy link

This pull request fails (merged abc7fc8 into da1e6ce).

@travisbot
Copy link

This pull request passes (merged 3a3ad2c into da1e6ce).

@willdurand
Copy link
Contributor

Heya, thanks for this fix. It seems ok, but it's really weird that nobody reported it before.. Will try to investigate a bit more before to merge.

@CyExy
Copy link
Contributor Author

CyExy commented Jun 23, 2012

Hi, I found a ticket from the old Propel trac that talks about the same issue http://trac.propelorm.org/ticket/1449

@CyExy
Copy link
Contributor Author

CyExy commented Jun 26, 2012

There is another BC issue. When fetching related objects with criteria, then any change to the related objects will not be saved when parent object is saved.

$author = AuthorPeer::retrieveByPK(A1);

$c = new Criteria();
$c->add(BookPeer::ID, B1);

$books = $author->getBooks($c);
$books[0]->setTitle('Update to a book');

$author->save();

// will return true, because author didn't save the object
$books[0]->isModified();

@travisbot
Copy link

This pull request passes (merged f371602 into da1e6ce).

@travisbot
Copy link

This pull request passes (merged 9731032 into da1e6ce).

@CyExy
Copy link
Contributor Author

CyExy commented Jun 28, 2012

@willdurand I feel that I should squash f371602 and 9731032 but I'm not sure is it ok if its in a pull request.

@willdurand
Copy link
Contributor

@CyExy na, it's ok

willdurand added a commit that referenced this pull request Jul 13, 2012
Change ObjectBuilder to reload related objects when collection is partia...
@willdurand willdurand merged commit 889dfc9 into propelorm:master Jul 13, 2012
@willdurand
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants