dynamodb-toolbox/dynamodb-toolbox

`default` Functionality Should Be Extended to `undefined`

Closed this issue · 5 comments

Hello! I may be off base, but I have noticed that the default clause in each entity definition strictly kicks in if and only if the value is not passed at all to the entity by means of a .put . Even if, for example, the user sent an empty field in their request, leading to the intermediate value passed to the entity to be undefined but listed as a field, then default will simply not kick in.

For example:

const newEntry = {
    value1: req.val1, // val1 is defined with a value of 1
    value2: req.val2, // val2 is defined with a value of 2
    value3: req.val3 // the user left val3 undefined in req.body
}

Entry.put(newEntry)

If I run this code, instead of val3 triggering the default function in the Entity definition, it simply will not be sent to DynamoDB and that column will not exist. The only way I could get the trigger to work is with the following:

const newEntry = {
    value1: req.val1, // val1 is defined with a value of 1
    value2: req.val2, // val2 is defined with a value of 2
    // val3 is left not just undefined but completely unmentioned
}

Entry.put(newEntry)

The above code works as expected. However I can not use this implementation. I have optional variables that are being validated on the data transfer layer before they reach the DynamoDB request, so there is no way to know if I should or shouldn't define these values until runtime.

Is there a way around this? Am I missing something? I feel like if undefined is passed into an entity as a field value, it should trigger that field's default. Or at least have an option that allows for this functionality.

Help is appreciated. Thank you!

@jeremydaly @naorpeled Any ideas on this? I could just be missing some functionality here, but there isn't too much documentation on the role of default in the entity definitions.

After looking through the source, it seems like this code here is where this decision is made and it seems to be the case, if I am reading this correctly, that default is only processed for values not originally defined (not included in acc but is included in attrs, the schema definition). Again, may be misunderstanding that snippet.

Any clarification is very much appreciated. Thank you!

Hey @saidkharboutli,
thanks for opening this issue!

Taking a look 👀

@saidkharboutli,
after a bit of digging I can see that we're merging the defaults with the input item when preparing the put payload using Object.assign, which lets undefined take precedence.
Currently the only way to get around this is to remove all undefined fields manually before using the put operation. Not ideal but a quick solution.

Not sure whether we should add an option on the entity/table for this or just make it the default behavior because I'm not sure I'd expect undefined to make a field empty, unlike explicit nulls.
wdyt?

I'm not sure I'll have enough time to implement a solution within the library atm,
but if you want to open a PR for this I'd love to help you navigate through the codebase and so on.

Hello @naorpeled, thanks for taking time to investigate! In my opinion, given that null is an explicit value which developers do have strict control over (i.e. can choose to keep the null value or remove the field), I think it makes sense for undefined to trigger the default attribute as default behavior. I don't think making it an option is necessarily all that important.

While I would love to support this endeavor in the future, I am currently mid-way through a high prio project for a client (hence the ping/urgency, sorry about that!) so I am not certain I would be able to spend time on this myself, as I am already a little behind on that end.

However, for the sake of anyone out there who finds this thread in search of an answer, this one-liner solved my problem (execute before passing the object to the .put function):

Object.keys(obj).forEach(attr => obj[attr] === undefined && delete obj[attr]);

Executing that line, where obj is the object you created that you intend to feed to the .put function, will remove all the undefined values from the object and provide the expected functionality. Not sure if this "dirty" solution could be integrated on the library side, but it definitely solves the issue I ran into.

Hello @naorpeled, thanks for taking time to investigate! In my opinion, given that null is an explicit value which developers do have strict control over (i.e. can choose to keep the null value or remove the field), I think it makes sense for undefined to trigger the default attribute as default behavior. I don't think making it an option is necessarily all that important.

While I would love to support this endeavor in the future, I am currently mid-way through a high prio project for a client (hence the ping/urgency, sorry about that!) so I am not certain I would be able to spend time on this myself, as I am already a little behind on that end.

However, for the sake of anyone out there who finds this thread in search of an answer, this one-liner solved my problem (execute before passing the object to the .put function):

Object.keys(obj).forEach(attr => obj[attr] === undefined && delete obj[attr]);

Executing that line, where obj is the object you created that you intend to feed to the .put function, will remove all the undefined values from the object and provide the expected functionality. Not sure if this "dirty" solution could be integrated on the library side, but it definitely solves the issue I ran into.

No need to be sorry hehe,
all good bud ❤️

Good luck with your project!
I'll be closing this issue for now but feel free to ping me if you need anything :)