search

clarkie / dynogels

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

[WIP] .update() enforces schema #54

Closed jkav77 closed 7 years ago

jkav77 commented 7 years ago

I created a function internals.validateItemFragment() to check that attribute modifications requested with .update() conform to the defined schema. This is more involved than just using Joi.validate() because .update() is not guaranteed to receive all required attributes. For example, modifying one attribute of an existing item would only require specifying the Hash Key, Range Key (if there is one), and the attribute to modify. This works as I mentioned, but I would be interested in any feedback on the below. @Si1kIfY?

A few issues/questions:

  1. This method will not check the item has all .isRequired() attributes if you create a new item with .update(). Rather it will ensure the fields specified match the schema. @deedw, any opinion on this?

  2. I didn't add anything to handle nested objects. The schema in the README does not cover nested objects or using the Joi.object() type. I am not really sure if this is possible or if people are doing that.

  3. Right now .update() validates the attributes it receives automatically. This would create problems if anyone was relying on .update() to create items that don't match their schema.

fixes #39

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 98.659% when pulling 534099b676323da319b5a7394268b1f03f8c918d on dangerginger:update-enforces-schema into 2f4293c6daa9356d5251c2fdbf7ffe87738e80a5 on clarkie:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 98.659% when pulling 534099b676323da319b5a7394268b1f03f8c918d on dangerginger:update-enforces-schema into 2f4293c6daa9356d5251c2fdbf7ffe87738e80a5 on clarkie:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 98.659% when pulling 534099b676323da319b5a7394268b1f03f8c918d on dangerginger:update-enforces-schema into 2f4293c6daa9356d5251c2fdbf7ffe87738e80a5 on clarkie:master.

csi-lk commented 7 years ago

The code looks great to me and the tests seem to validate this would fix the issue posted

Thanks for putting the time into this 👍

clarkie commented 7 years ago

Hi, thanks for your PR. I'm really sorry I missed this. I'll hopefully have chance today to have a look.