v7 validation and null properties about dynogels HOT 7 CLOSED

I think this may be an issue with the underlying database not accepting null values in previous versions (I'm guessing here). I've had a look and it does support NULL types which are in fact booleans! haha!

NULL — (Boolean)
An attribute of type Null. For example:
"NULL": true
http://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/DynamoDB.html#updateItem-property

I have no issues with you submitting a PR for supporting this in dynogels. We'd need to keep the ability to set undefined to act in the same way as it does now.

Cheers

from dynogels.

I think there are two aspects to this issue. The reason for the change is PR #54 for v7 enforces the schema. It prevents updating an item to be in violation of its schema, but I probably missed the case for .allow(null). I believe the previous behavior for setting an attribute to null was to delete that attribute. This makes sense because of the second aspect of this issue.

DynamoDB does not allow null values. If you look at the DynamoDB.putItem() page, it explicitly states this. If you look at the DynamoDB Attribute Values page it elaborates on the NULL data type, which is a boolean. I haven't used the NULL type, but it seems like if you checked the value it would be true or false, and not null.

Attribute values cannot be null. String and Binary type attributes must have lengths greater than zero. Set type attributes cannot be empty. Requests with empty values will be rejected with a ValidationException exception.

I think the desired behavior for dynogels should be that setting any attribute value to null is permitted and it deletes the attribute in DynamoDB. I am not sure, but I think requesting a non-existent attribute with DynamoDB.getItem() should return null.

from dynogels.

sorry for delay in responding (just had a baby). i forked and patched it to support null but it ended up breaking a lot of tests, which lead me to believe that a lot of old code might depend on the behavior.

perhaps a table option flag nullables t/f with default being false? that would give backwards compat.

DynamoDB does not allow null values

it does in typical aws fashion i.e. convoluted as F. DocumentClient translates this null data type to/from the DDB null type transparently. so using DocumentClient directly allows r/w null, which is what dynogels is using under the hood IIRC.

[...] but I think requesting a non-existent attribute with DynamoDB.getItem() should return null.

IMO getItem on a non-existent property should return undefined when the model schema is known and the property isn't defined.

A nullable boolean column type can have four states: undefined (non-existent), null, true, false. That type of thing can be useful in a pinch.

from dynogels.

So according to the documentation:

// setting an attribute to null will delete the attribute from DynamoDB
Account.update({email: '[email protected]', age: null}, function (err, acc) {
  console.log('update account', acc.get('age')); // prints null
});

I think this is because when vogels was created DynamoDB did not allow null values and now it does. It does seem that Dynogels should be updated to reflect this change but I'm not sure exactly how that should look. And like @cmawhorter mentioned, this will break a lot of other areas.

If null values were allowed then how would you delete a value?

from dynogels.

@dangerginger I agree, this is a big change. I'm not convinced in the JS world that a null type is useful. At no point while using dynamo db have I felt I needed it. I'm guessing AWS added it more for the strongly-typed languages? Right now, I have no idea how to make this work.

Also, thanks for going through all the issues! 👍

from dynogels.

So it seems that the only use case for this is when you have an attribute that is required, but that you can allow to be null. If the value is allowed to be null then it seems to me the attribute is not required and you can just remove it from the model. I am going to close this one. This would be a huge change and I don't think it is worth it.

from dynogels.

that's not the only use-case, but i understand adding support for this would be difficult to achieve in a backwards compat way.

i got around this by adding a special "$null" value and a translation step to my wrapper.

from dynogels.