Meteor-Community-Packages / meteor-collection-hooks

Meteor Collection Hooks
https://atmospherejs.com/matb33/collection-hooks
MIT License
657 stars 92 forks source link

Using a different mongo operation in a hook called by a hook throws an error. #142

Closed evolross closed 9 years ago

evolross commented 9 years ago

Using Meteor 1.1.0.2 and the matb33:collection-hooks package I'm running into an error trying to add a $set to the modifier object in a before.update hook that was called by an $inc update.

I have an after.update hook on an Invoices Collection so when the Invoices instance is approved it increments a totalPurchases counter on a Products Collection instance like so:

if(Meteor.isServer) {
   Invoices.after.update(function(userId, doc, fieldNames, modifier, options) {

      if(modifier.$set && modifier.$set.approved === true) {
          //  Increment product.totalPurchases
          Products.update(doc.productId, {$inc: {totalPurchases: 1}});
      }
   });
}

And then a second hook on a Products collection that checks if the totalPurchases counter has exceeded an inventory maximum and if so, it sets a flag on the Products instance modifier object so when the update occurs on the Products instance the inventoryMaxReached flag gets updated too:

if(Meteor.isServer) {
   Products.before.update(function(userId, doc, fieldNames, modifier, options) {

      if(modifier.$inc && modifier.$inc.totalPurchases) {
          //  Check if total purchases is beyond max
          var newTotalPurchases = doc.totalPurchases + $modifier.$inc.totalPurchases;
          if(newTotalPurchases > TOTAL_INVENTORY_MAX)
             modifier.$set.inventoryMaxReached = true;
          }
      }
   });
}

The problem is the modifier.$set.inventoryMaxReached = true returns an error because I'm adding a $set to the modifier object. I figured out that if I make the first hook a $set instead of an $inc then the second hook works fine. The modifier object is only happy when all the operations are the same. So to do that I changed the line in the first hook:

Products.update(doc.productId, {$inc: {totalPurchases: 1}});

to:

var product = Products.findOne(doc.productId);
Products.update(product.productId, {$set: {totalPurchases: product.totalPurchases + 1}});

I thought this solved my issue but I discovered this creates a second problem. The totalPurchases counter will not get incremented correctly if a ton of Invoices come in really fast because of the way the hooks resolve asynchronously. product.totalPurchases + 1 will resolve to the same value and not actually increment. I discovered this when populating my database with initial data in a bunch of for loops that executed very quickly.

However if I use an $inc like in the first version of the Invoices hook, the totalPurchases counter updates exactly right. The problem is that I just can't do a $set in any hooks triggered by the $inc update, which I need to do.

To try to solve this I actually tried to do an update inside the update hook. I changed:

modifier.$set.inventoryMaxReached = true;

to

Products.update(doc.id, {$set: inventoryMaxReached: true}});

But that causes what seems like infinite loops in the browser (which I'm not sure why since I have the if(modifier...) line to check what kind of update it is. Regardless if it works, it's hacky to need/try to call and update on an instance inside of it's own update hook - which is probably why the browser freaks out. That's what the modifier object is for.

EDIT: I'm thinking this may be a bug as it's perfectly acceptable to call a Mongo update like this mixing operations:

Gadgets.update(gadgetId, {$inc: {totalStock: -1}, $set: {inventoryMaxReached: true}});
matb33 commented 9 years ago

I assume that the $ you have in front of modifier was introduced by accident? And finally, you need to initialize the $set in your Products.before.update. In full:

if(Meteor.isServer) {
   Products.before.update(function(userId, doc, fieldNames, modifier, options) {

      if(modifier.$inc && modifier.$inc.totalPurchases) {
          //  Check if total purchases is beyond max
          var newTotalPurchases = doc.totalPurchases + modifier.$inc.totalPurchases;    // Removed extra $ in front of modifier
          if(newTotalPurchases > TOTAL_INVENTORY_MAX)
             if (!modifier.$set) modifier.$set = {};    // You ran an $inc only, there's no $set so the next line wouldn't have worked
             modifier.$set.inventoryMaxReached = true;
          }
      }
   });
}
evolross commented 9 years ago

Thanks, that was it. Since modifier was already an object, I didn't realize you had to initialize and object within an object. I thought you could just add/append to it using Object.whatever Now I know! #JavascriptNewbie Sincere thanks for your help.