ensure Model.update accounts for REMOVE expressions on in memory object

[conjmurph]

If an UpdateItem removes an attribute on an item in the db, this update is not reflected on the in memory object after the request succeeds.

We request RETURN_VALUES as ALL_NEW during an update. DynamoDB will return the current state of all attributes as Attributes in the response.

Before this PR, we iterate over all of the returned attributes and assign them to the in memory copy of the item. This fails to account for an attribute that was removed via the UpdateItem request e.g.

>>> item = SomeModel.get('foo')
>>> print(item.some_attribute)
'attribute_value'
>>> item.update(actions=[SomeModel.some_attribute.remove()]
>>> print(item.some_attribute)
'attribute_value'

In this PR, we leverage Model._deserialize to iterate over the model's attributes, and pull each attribute out of the response Attributes. We expect the above example to execute as:

>>> item = SomeModel.get('foo')
>>> print(item.some_attribute)
'attribute_value'
>>> item.update(actions=[SomeModel.some_attribute.remove()]
>>> print(item.some_attribute)
None

[conjmurph]

Note that this assertion always passed since views was never set.

[conjmurph]

This is also a breaking change for anyone relying on the attribute still being set even after the db update. Any guidance on what version we'd need to introduce this?

[hallie]

Does ALL_NEW include values that weren't updated at all? For example, if you have a model SimpleUserModel with attributes user_name, email, is_active, views, and you call:

foop = SimpleUserModel(
    user_name='foop',
    email='foop@geep.com',
    is_active=True,
    views=5,
)
foop.save()
foop.update(
    actions=[
        SimpleUserModel.is_active.set(False),
        SimpleUserModel.views.remove(),
    ]
)

will user_name and email be in the returned data/set when _deserialized is called?

[conjmurph]

Does ALL_NEW include values that weren't updated at all? For example, if you have a model SimpleUserModel with attributes user_name, email, is_active, views, and you call:

foop = SimpleUserModel(
    user_name='foop',
    email='foop@geep.com',
    is_active=True,
    views=5,
)
foop.save()
foop.update(
    actions=[
        SimpleUserModel.is_active.set(False),
        SimpleUserModel.views.remove(),
    ]
)

will user_name and email be in the returned data/set when _deserialized is called?

Yes, ALL_NEW returns all of the attributes of the item, as they appear after the UpdateItem operation. UPDATED_NEW returns only the updated attributes, as they appear after the UpdateItem operation. https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_UpdateItem.html#DDB-UpdateItem-request-ReturnValues. Seems like UPDATED_NEW would only return the updated values.

[ikonst]

@jpinner-lyft Maybe I'm remembering wrong but I feel we had another issue with deserialization being guided by the server's response rather than the model's attributes.

[garrettheel]

Spent some time testing this and generally seems good. If we do allow customization of RETURN_VALUES to be changed in future then we'll need to tweak, but that should be fine.

One odd thing, which is an existing issue, is that you can unset an attribute which omits null=True. With the changes in this PR, the update would still succeed but you'd get a ValueError: Attribute 'foo' cannot be None on a subsequent save (currently it would transparently re-set the old value). I do think that the error is more desirable, so happy to go with that for now.

@ikonst the main thing I could think of was the handling of default in cases like these, where you may expect to see whatever default you set rather than None (especially if null=False). Any thoughts on whether the current behavior is reasonable for that? I couldn't think of any other issues that might arise from server-led deserialization, but curious if you can think of any.

[ikonst]

I don't see any problem with this behavior.

In general I'd like what we call "default" today to start behaving as "default_for_new" (defaults for when the object is constructed by the user), rather the way it's today - fallbacks for missing values, whether from user or from network.