Nested save

[pnomolos]

Did some work on the plane today. Probably needs some more cleaning up and more test cases, but wanted to get your thoughts on where this is going.

[vlucas]

Looking good so far. Really nice work here. Before I merge this, just one question: I see delete called in a few places. What circumstances would cause the deletion of entity records, and is that expected by the user?

[vlucas]

Also - it would be sweet if save could automatically figure this out and use the saveNested method internally so the API would be consistent for the end user. How much additional work would that be?

[pnomolos]

Re: delete - if something fails to save then I roll everything back, including deleting the original object. It's a bit of a chicken-and-egg problem. Nested models aren't valid until they have a parent object ID and you can't get that until you actually persist the parent object to the store. I didn't really see a better way - I'm sure I could do some partial validation to make it happen not as much, but ultimately there will need to be cases where things are rolled back.

For this reason it's probably best if saveNested isn't automatically called from save due to how it behaves. I felt that it has a specific use case and should probably not be automatically called with respect to how it interacts.

[vlucas]

How about wrapping everything in a transaction? It's the perfect use-case for this. Examples: https://github.com/vlucas/Spot/blob/master/tests/Test/Transactions.php

[pnomolos]

I thought about that - are they fully available across both adapters?

[vlucas]

No - MongoDB does not support transactions.

[vlucas]

I guess the delete is fine as a rolback as long as all the originally set data is set back on the object (if it was modified), so that it's state doesn't change on a failed transaction. On a successful transaction, the relations should be loaded on the entity so they will work as normal immediately after save.

[pnomolos]

OK. At this point saveNested doesn't play with existing objects so I don't have the first concern to worry about (yet). I'm going to work on that, though. I guess this also invalidates the second point because the base object isn't returned, just the primary key.

[vlucas]

It may not be returned, but objects are passed by reference by default in PHP, so when you modify them it modifies the original object instance as well. All you should have to do is call $this->loadRelations($entity); for it to work. It does that in get, insert, etc. and should do it in this case, because the nested array data needs to be transformed into working relation proxies so the relations can be retrieved and worked with as expected (this will discard the nested arrays, but that is okay since they were all saved okay).

[vlucas]

This will enable you to do (and we should test for this):

$mapper->saveNested($post);
$this->assertEquals(2, $post->comments->count());

[pnomolos]

Ah, I see what you're getting at. We can't do that - saveNested only accepts an array, not an Entity object. Passing in an Entity object wouldn't work because there's no way to assign values to the relationships, unless we do something like Rails/ActiveRecord does with the *_attributes proxies.

[vlucas]

I think passing in the Entity is the best way to do this. We don't have to pre-load all the relationship objects - all we're doing is restoring the normal way to retrieve relations through the Spot\Relation proxy object, and that can be done using the loadRelations method.

[marvelade]

No - MongoDB does not support transactions.

Neither does MyISAM if I'm not mistaken ?

On Wed, Dec 5, 2012 at 9:28 PM, Vance Lucas notifications@github.comwrote:

I think passing in the Entity is the best way to do this. We don't have to pre-load all the relationship objects - all we're doing is restoring the normal way to retrieve relations through the Spot\Relation proxy object, and that can be done using the loadRelations method.

— Reply to this email directly or view it on GitHubhttps://github.com/vlucas/Spot/pull/25#issuecomment-11059007.

[vlucas]

No, MyISAM does not support transaction either, but Spot uses InnoDB be default for MySQL.