clarkie / dynogels

DynamoDB data mapper for node.js. Originally forked from https://github.com/ryanfitz/vogels
Other
490 stars 110 forks source link

Don't mutate options parameter when updating table #69

Closed randyxli closed 7 years ago

randyxli commented 7 years ago

Thanks for the awesome package!

I noticed that table#update was unexpectedly mutating the options parameter, which can cause silent bugs. For example, if we want to log all the times where an item we want to update does not exist, we might write something like:

let options = { expected: { id: { Exists: true }}};
for (let model in modelsToUpdate) {
  model.update(updateObject, options, function(err, updatedModel) {
    if (err) {
      logger.warn('Model not found');
    }
  });
}

This code does not log as intended because the expected key is deleted from the options object after the first update. In this PR I propose making use of lodash to avoid mutating the options parameter and simplify the code slightly. Thanks for looking!

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.008%) to 98.654% when pulling 7f85915408d017e07fefc169587f75152803a60e on randyxli:options-mutation into d327665b1ad475d1b2aba4ae4dd3a86db8b34685 on clarkie:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.008%) to 98.654% when pulling 7f85915408d017e07fefc169587f75152803a60e on randyxli:options-mutation into d327665b1ad475d1b2aba4ae4dd3a86db8b34685 on clarkie:master.