Skip to content

Conversation

@drboyer
Copy link

@drboyer drboyer commented Nov 1, 2017

I've run into a situation a couple times recently where I end up with some code sprawl trying to update a row in Dynamo because I may only have certain fields available to update - maybe I have one case where I have an object to update with a foo property defined, but it's missing/undefined in another. When defining the parameters for an update call, you have to outline all of the keys and values to update. Dynamo will throw an error if any are missing because the value is undefined.

This PR adds a new method to dyno called dynamicUpdate that takes an object that represents all of the keys and values you want a row to contain after it is updated and dynamically builds part of the update parameters. It will iterate over each property in your object and build up the ExpressionAttributeNames, ExpressionAttributeValues and UpdateExpression properties. You provide any other update parameters needed (like the Key).

@drboyer drboyer requested a review from l-r November 1, 2017 14:33
Copy link

@l-r l-r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, made a small suggestion.

var updateExpressionParts = [];

Object.keys(newObject).forEach(function (key) {
if (Object.keys(updateParams.Key).indexOf(key) !== -1) return;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I suggest we rename key to something else? Right now we're checking whether key is in the updateParams.Key's key list, it might be clearer if it was more explicit, i.e. newObjectKey or similar.

@rclark
Copy link
Contributor

rclark commented Nov 1, 2017

My sense is that a slightly different API might be better -- one that avoids mutating a passed-in argument and then making a request with the result on your behalf.

The important part here is your code that maps a fully-formed new object into an expression object with the intended statements. To me a better flow would be:

const newState = { how: 'I want things', to: 'be' };
const params = dyno.updateExpression(newState);
params.Key = 'abc';
params.WhateverOtherThingsIneed = ...

dyno.update(params, callback);

I also want to point out that converting a JS object into an update expression is only helpful when you want to SET a bunch of values. It breaks down pretty quickly if you have lists or sets or want to add / subtract from some value.

@drboyer
Copy link
Author

drboyer commented Nov 1, 2017

Thanks for the input @rclark, yeah that's exactly the other option I was considering. It does probably make more sense, so that the user has complete control over running the update. I debated whether something that just built those expression objects was better served in Dyno or even just in its own small module. Still open to that latter option.

I do realize there are a lot more complicated updates you can perform in dynamo, this mostly just came about because I've run into the situation of "SET a bunch of [simple] values" a handful of times in different projects in the past couple weeks and figured it might be useful to abstract that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants