JackAdams / meteor-transactions

App level transactions for Meteor + Mongo
http://transactions.taonova.com/
MIT License
113 stars 16 forks source link

Enable operator modifier commands to be saved and retrieved from tx.Transactions #16

Closed rjsmith closed 9 years ago

rjsmith commented 9 years ago

This partially addresses #14 by enabling update commands like:

      fooCollection.update(
        {_id: insertedFooDoc._id},
        {
          $addToSet: {
            foo: {$each: newBars}
          }
        },
        {tx: true});

to be successfully saved into the tx.Transactions collection.

NB: Need further changes to support correct inverse commands for $addToSet before this issue can be completed.

rjsmith commented 9 years ago

Related tests added to my bug repo:

https://github.com/rjsmith/meteor-transactions-bug-repo

JackAdams commented 9 years ago

Cool. Just merged this. Do you want me to release a new version of the package or should we wait until the inverse operation is ready?

rjsmith commented 9 years ago

Yeah , I would wait until the inverse is ready, suspect that is going to be tougher.

So, you are thinking of trying to mirror Mongo's $addToSet behaviour (only adding items if not already in array) in creating a inverse operation (remove potentially multiple items, but only if they are not already in the array?) ie. a fine-grained approach? I can see that working if the undo / redo stack strictly prevents skipping undo steps (ie have to also unwind any intervening changes).

JackAdams commented 9 years ago

Ah... forgot about that. $pull will remove duplicates, won't it? I would like to use a fine-grained approach if at all possible but, like you say, that's not going to be easy.

rjsmith commented 9 years ago

Yes, I believe that is the case:

http://docs.mongodb.org/manual/reference/operator/update/pull/

The $pull operator removes from an existing array all instances of a value or values that match a specified query.

rjsmith commented 9 years ago

Hence my idea to a simple brute force approach: (in the switch (inverseCommand) { block:

          case '$pull' :
            // Brute force approach to ensure previous array is restored on undo
            // even if $addToSet uses sub-modifiers like $each / $slice
            _.each(_.keys(updateMap), function (keyName) {
               var formerVal = self._drillDown(existingDoc,keyName);
               if (typeof formerVal !== 'undefined') {
                inverseCommand = '$set';
                formerValues[keyName] = formerVal;
               } else {
                inverseCommand = '$unset';
                formerValues[keyName] = '';
               }
            })
            // formerValues = updateMap;
            break;
JackAdams commented 9 years ago

Yeah, I'm willing to go with this for now. I guess if the need for something more fine-grained ever arises, I can beat my head against the problem them.

Want me to release a new version before I go to bed? (It's 2.40am in China.)

rjsmith commented 9 years ago

If 25 years of experience in the software industry teaches me anything, NEVER commit anything at 2:40am in the morning :-)

JackAdams commented 9 years ago

Hahaha... awesome. Then I'll bid you a good night.

p.s. That code should apply to both the $pull and $addToSet operators, right?

rjsmith commented 9 years ago

Please don;t commit the code snippet above. Found an edge case where it breaks. If there are multiple fields in the $addToSet list, and one or more of them do not already exist in the existing doc, the inverseCommand is set to $unset. But inverseCommand is applied to all of the fields. Need a list of inverse commands, allowing a mixture of $set and $unset as necessary.