cakephp-mongodb icon indicating copy to clipboard operation
cakephp-mongodb copied to clipboard

Setting driver wide collection options for safe, fsync and timeout.

Open elricho opened this issue 12 years ago • 8 comments

Presently there is no way to set safe, fsync and timeout collection options for creates and updates. This patch implements a driver field 'collection_options' => array(...) The array can take 'safe' => n, 'fsync' => true/false, 'timeout' => n See http://www.php.net/manual/en/mongocollection.save.php for more informaiton on field settings.

elricho avatar Sep 02 '11 12:09 elricho

Perfect. I was about to do that myself (make update operations accept extra parameters other than 'multiple' => true). But thanks for getting ahead. I still changed something related to updates; see here: https://github.com/ichikaway/cakephp-mongodb/pull/31

YOMorales avatar Sep 05 '11 20:09 YOMorales

Cool. :) FYI - There was a bug in my insert, which is fixed now. :)

I was also thinking of extending it a little further with a mechanism that lets colection_options be set for one request only (I dont always need safe mode set for everything). I'll model it on http://book.cakephp.org/view/1045/Creating-and-Destroying-Associations-on-the-Fly#!/view/1045/Creating-and-Destroying-Associations-on-the-Fly - Like binding the options for one request only.

elricho avatar Sep 05 '11 21:09 elricho

That is a better approach.

At first I thought of passing a third parameter in updateAll so that collection options could be set on demand like you say (something like: Mode->updateAll($fields, $conditions, $mongodb_options). But that would need some override of the Model class' updateAll method, and IMO this is something out of the 'scope' of this plugin.

But your suggestion is better. Something like binding. It won't even need using a new method the way 'Binding Associations' does, but the simplest way is to set a class attribute. Something like:

$this->Model->collectionOptions = array('upsert' => true, 'safe' => true. etc.); Then in Datasource... if (!empty($this>collectionOptions)) { $mongoCollectionObj->update($cond, $data, $this>collectionOptions);

YOMorales avatar Sep 06 '11 02:09 YOMorales

It is not appropriate to use a db-wide options array on multiple collections - and on function calls that are expecting different options to be used.

  • batchInsert has only safe and fsync
  • insert has safe, fsync and timeout
  • save has safe, fsync and timeout
  • update has safe, fsync, timeout, upsert and multiple

It's likely that if you want to modify these settings, you want them different for specific collections and probably specific operations. By which I mean for example: inserts on [collection] are safe, but updating some counter/timestamp field is not a safe update. Therefore, while these changes may scratch your current itch it doesn't really solve "the problem".

I'd suggest that where currently you reference $this->config['collection_options'] it should effectively reference $Model->mongoOptions[$methodname] or similar. Unless it's possible to easily change these options at least on a per collection basis I don't see a benefit in changing hardcoded options for inflexible options.

AD7six avatar Sep 07 '11 07:09 AD7six

Hi Andy,

Thanks for the feedback and fair point.

To expand on your suggestion, should the options in $Model->mongoOptions[$methodname] be set by $Model->setMongoOptions(array('save' => array('safe'), 'insert' => array('safe','fsync','upsert','timeout' => n), ...)); (ie getter/setter style) or just set it directly in the model like YOMorales' suggestion ?

Cheers r

elricho avatar Sep 07 '11 09:09 elricho

elricho - personally I'd go with simple - if (!empty($Model->...)) - but Ichi might have a different idea, and it's his code. However, if it works (by any means) it would be simple to make it work better before/after merging.

AD7six avatar Sep 07 '11 09:09 AD7six

sorry too late reply. I agree with using model property. I will merge it,then change to use model property. Now the patch fails test cases, I will fix it.

Thanks great idea and discussion.

ichikaway avatar Nov 27 '11 11:11 ichikaway

Hi Ichi, Not sure if you fixed the failed test cases... I found that if I made these changes to the patch I could get all tests to pass:

line 112: 'insert' => array('safe' => true),

line 742: $params = array_merge($this->collectionOptions['update'], array("multiple" => false));

redthor avatar Feb 10 '12 04:02 redthor