dynamodb-data-types
dynamodb-data-types copied to clipboard
kayomarz
string Attributes are not allowed to be empty
Yes - API doesn't allow empty strings. http://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_PutItem.html We've made a fix in our fork, testing it on our system at the moment.
On 6 April 2016 at 14:30, Frankie Bagnardi [email protected] wrote:
Oh, I see what you mean, it shouldn't allow empty strings.
— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/kayomarz/dynamodb-data-types/issues/7#issuecomment-206373866
Bryan Crotaz Managing Director Silver Curve
The library avoids doing data validations. Invalid data in a DynamoDB request results in an error response thereby indicating the invalid data. Could you make use of the error response to capture invalid data?
Including data checks/validations in this library would help detect invalid data earlier - i.e. before making an HTTP API request. However in that case the library would need to validate data exactly as DynamoDB does - else the feature might not be worth having.
Does your use case need data validation to be done before making a DynamoDB API call?
ps: There was a line in the docs mentioning that this library doesn't perform such checks. This line currently seems to be missing in the docs.
@BryanCrotaz thanks for the pull request and test case.
As you mentioned, this library detects an empty string as { S: '' }. I would like to keep this behaviour intact.
The idea is to let this library do nothing more than help represent your data. If the data is invalid it would be better to let the application handle it.
Nonetheless, to attempt to do what you require, you can now make use of a hook. The hook reconcileFn gives an opportunity to the application to reconcile data so that it is valid for DynamoDb.
Suppose your application needs an empty string to be detected as {NULL: true} it can now be done as follows:
var dynamoDdDataTypes = require('dynamodb-data-types');
dynamoDdDataTypes.setGlobalOption({reconcileFn: dynamoDdDataTypes.reconcile})
See examples/05-reconcile-data.js for a full example.
The reconcile function gives the application an opportunity to reconcile data. A custom reconcile function may be used instead of lib.reconcile
This change is only in git, not yet moved to the NPM repo.
Let me know what you think. I hope this is useful for your use case.
@brigand would be great if you have any suggestions or alternate way to do this.
Thanks.
The thing is that all applications need this - Dynamo will never accept {S: ''} - we're putting a burden on developers that doesn't need to be there
I'd be happy with a compromise of making lib.reconcile the default behaviour - developers should not need to specify this every time.
If setting the global var seems an unnecessary step, another solution would be to keep existing functions wrap() and wrap1() as they are and create two new functions to address this discussion. Say vWrap() and vWrap1() to make valid and then wrap; or any other appropriate names, maybe cWrap() andcWrap1() for coerce and wrap.
This way, both approaches are supported. On 10-Apr-2016 6:52 pm, "Bryan" [email protected] wrote:
I'd be happy with a compromise of making lib.reconcile the default behaviour - developers should not need to specify this every time.
— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/kayomarz/dynamodb-data-types/issues/7#issuecomment-207983805
I'm mostly just concerned that this should be a library that "just works". The default out of the box behaviour should enable the user to load and save data to and from Dynamo without having to think about it.
My point is to maintain separation of concerns. This library should only be concerned with data representation.
To me, some reasons not to convert an Empty string into null are the following:
- as a developer, if dynamoDB doesn't allow empty strings I would like to get an error and know about it.
- if data contains an Empty string, an application might expect to read back the same data (an empty string)/as far as possible. The fact that an empty string is illegal is something better handled by a prior step to writing data.
- it might help to think about why dynamoDB does not silently convert an Empty string into null.
Maybe its better to let the application decide what to do if an empty string appears.
Ideally if you want to convert an Empty string into null before writing it to DynamoDB, I would do it as:
Coerce(my data); // do stuff such as covert empty string to null Wrap(coerced data); // what wrap() currently does
This is just my approach.
Maybe having vWrap() separate from wrap() is a way to let both approaches co-exist.
Besides, changing the behaviour of wrap() might break applications using this lib because its difficult presume how applications might have used it. On 11-Apr-2016 10:39 pm, "Bryan" [email protected] wrote:
When as a developer would you choose not to use vwrap / cwrap?
— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/kayomarz/dynamodb-data-types/issues/7#issuecomment-208452636
The problem is that unwrap(wrap(x)) should return a clone of x. That said, it probably makes the most sense to not create invalid wrapped data.
Another option would be to create a custom structure for representing empty strings. For example, you could convert '' to {S: '6b8587a0-0ee5-4759-b4a5-600f30d785f8'} (a static uuid). Then unwrap can check for that exact uuid and convert it to an empty string. It seems pretty weird to do this, and you'd need to handle it specially if accessing dynamodb without this library, so I'm not sure if it's a good solution. It would preserve unwrap(wrap(x)) giving x. This would be a good candidate for an alternative method name.
Your uuid approach is interesting. Using it to overcome restrictions would be a plus to have. On 12-Apr-2016 12:26 am, "Frankie Bagnardi" [email protected] wrote:
The problem is that unwrap(wrap(x)) should return a clone of x. That said, it probably makes the most sense to not create invalid wrapped data.
Another option would be to create a custom structure for representing empty strings. For example, you could convert '' to {S: '6b8587a0-0ee5-4759-b4a5-600f30d785f8}' (a static uuid). Then unwrap can check for that exact uuid and convert it to an empty string. It seems pretty weird to do this, and you'd need to handle it specially if accessing dynamodb without this library, so I'm not sure if it's a good solution. It would preserve unwrap(wrap(x)) giving x. This would be a good candidate for an alternative method name.
— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/kayomarz/dynamodb-data-types/issues/7#issuecomment-208498427
Just ran into this too and saw the reconcile change was reverted. I ended up writing some code to remove empty strings from my objects before putting them into .wrap(). Just thought I'd leave it for the next passer by.
const clean = (value) =>
Object.getOwnPropertyNames(value)
.reduce((o, k) => {
const v = value[k];
if (typeof (v) !== 'string' || v !== '') {
if (typeof (v) === 'object') {
o[k] = clean(v);
} else {
o[k] = v;
}
}
return o;
}, {});
@bfsmith this is late response, but you can use hapi joi to validate use inputs. https://hapi.dev/family/joi/?v=16.1.7
