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

Relations and find method #293

Open
sdrdis opened this issue Aug 30, 2013 · 8 comments
Open

Relations and find method #293

sdrdis opened this issue Aug 30, 2013 · 8 comments

Comments

@sdrdis
Copy link

sdrdis commented Aug 30, 2013

Lets say I have two models, Model_Test and Model_Tag, Model_Test having a many_to_many relation (named tags) with Model_Tag

$test = \Model_Test::find('first'); // lets say there is at least one Model_Test instance
$test->tags[] = \Model_Tag::forge(array('tag_label' => 'test'));
// <-- If I apply $test->save() here, it works well. $test->get_diff() returns an array with two non-empty elements
\Model_Test::find('first');
// <-- If I apply $test->save() here, the tag relation won't be saved. $test->get_diff() will return an array with two empty elements
@emlynwest
Copy link
Contributor

Going to need a little more information than that ;)

@sdrdis
Copy link
Author

sdrdis commented Aug 30, 2013

sorry, I pressed enter while correcting my title :)

@WanWizard
Copy link
Member

This is because the second find will not return a new object, it will return the same object. And the hydration after the find will reset the object to 'retrieved' state by syncing the data and original_data arrays.

Fixing this isn't easy:

  • You could ignore the cached object and return a new one (which means you have to objects that could become out of sync, which may be dangerous if you start modifying them)
  • You could return the cached object with it's changes (which is probably not what you want most of the time)
  • You could throw an exception if the object is in modified state (not very pleasant too)

Besides, get_diff() is an expensive operation, you really don't want to run that on the hydration of every record.

Apart from addressing this, we need to find an alternative solution for 2.0, maybe using a modified flag to track the state of the object.

@sdrdis
Copy link
Author

sdrdis commented Aug 30, 2013

Something strange is that if I do ;

$test = \Model_Test::find('first'); // lets say there is at least one Model_Test instance
$test->title = 'new title'; // old title was "old title"
\Model_Test::find('first'); // or $test = \Model_Test::find('first');
echo $test->title; // still prints "new title"

So the the object is not entirely reset...

@WanWizard
Copy link
Member

Checked the code, and is seems it treats relations different from properties. For properties it checks if the object is retrieved from cache, it doesn't do that for relations.

Either way this is inconsistent and needs to be fixed, the question is how (see my previous option list)...

@WanWizard
Copy link
Member

I found the solution for option 2 (merging the results in when the object is retrieved from cache), but I'm not happy with it. Sometimes you just want the original record again (without the changes you made before), sometimes you don't (like in your use-case).

Merging results back in also means you do retain additions, but you loose changed objects (the merge will overwrite them), and deleted relations will be restored, which is unwanted in most cases as well.

@sdrdis
Copy link
Author

sdrdis commented Sep 2, 2013

Can't the "from_cache" option be used when you want to specifically retrieve original object ?

@WanWizard
Copy link
Member

Yes, that is an option, if you disable the cache, a new object will be created with full hydration of the resultset, this object will not be cached.

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

No branches or pull requests

3 participants