Moto: DynamoDB#update_item() - AttributeUpdates should have action=PUT by default

[gsamaras]

Boto provides a parameter with a default, but moto doesn't seem to follow this, which lead to an error when trying to mock DynamoDB in a Unit Test.

Quoting boto3.DynamoDB.Client.update_item() documentation(emphasis mine):

Action (string) --Specifies how to perform the update. Valid values are PUT (default), ...

So in my code to-be-tested I was doing:

def my_upload_record_to_dynamodb(...):
    ...
    dataclass_attributes[current_field.name] = {'Value': attribute_value}
    record_key = {'record_id': {'S': record_id}}
    self._dynamodb_client.update_item(TableName=table_name, Key=record_key, AttributeUpdates=dataclass_attributes)

and my unit test looked like this:

import unittest
import boto3

from moto import mock_dynamodb2
from unittest.mock import MagicMock, patch

class TestElasticSearch(unittest.TestCase):
    def setUp(self):
        self.es_record = ...

    @mock_dynamodb2
    @patch('boto3.client')
    def test_upload_record_to_dynamodb(self, mock_boto3_sts_client):
        table_name = 'my_table'
        dynamodb = boto3.resource('dynamodb', region_name='eu-west-1')

        table = dynamodb.create_table(
            TableName=table_name,
            KeySchema=[
                {
                    'AttributeName': 'record_id',
                    'KeyType': 'HASH'
                },
            ],
            AttributeDefinitions=[
                {
                    'AttributeName': 'record_id',
                    'AttributeType': 'S'
                },

            ]
        )
        # Wait until the table exists.
        table.meta.client.get_waiter('table_exists').wait(TableName=table_name)

        my_upload_record_to_dynamodb(table_name, self.es_record)

which was failing with:

[CPython37-test:stdout]         es_reporter = ElasticSearchReporter(self.role_arn, None)
[CPython37-test:stdout]         es_reporter.my_upload_record_to_dynamodb(
[CPython37-test:stdout]             ElasticsearchIndices.ANALYSIS_BACKEND_SESSIONS_ALPHA,
[CPython37-test:stdout] >           self.es_record
[CPython37-test:stdout]         )
[CPython37-test:stdout] 
...
[CPython37-test:stdout] self = Item: {'Attributes': {'record_id': {'S': '6266046e-95c5-4448-aa2d-8d4959b6e306'}}}
[CPython37-test:stdout] attribute_updates = {'aggregator_results_s3_path': {'Value': {'S': 's3:/my_bucket/2020-11-06T110210251207_81_my_session/inference-results/...:02:10.390290'}}, 'audio_s3_path': {'Value': {'S': 's3:/my_bucket/2020-11-06T110210251207_81_my_session/audio/'}}, ...}
[CPython37-test:stdout] 
[CPython37-test:stdout]     def update_with_attribute_updates(self, attribute_updates):
[CPython37-test:stdout]         for attribute_name, update_action in attribute_updates.items():
[CPython37-test:stdout] >           action = update_action['Action']
[CPython37-test:stdout] E           KeyError: 'Action'
[CPython37-test:stdout] 
[CPython37-test:stdout] /home/gsamaras/pkg-cache/packages/Python-moto/Python-moto-1.x.67811.0/AL2012/DEV.STD.PTHREAD/build/lib/python3.7/site-packages/moto/dynamodb2/models.py:293: KeyError
[CPython37-test:stdout] ----------------------------- Captured stderr call -----------------------------
[CPython37-test:stdout] 2020-11-06 15:55:29,175 INFO [botocore.credentials] Found credentials in environment variables.
[CPython37-test:stdout] 2020-11-06 15:55:29 INFO [elastic_search] Record about to be sent to DynamoDB (record ID: 6266046e-95c5-4448-aa2d-8d4959b6e306): {'stage': {<and so on..>

because there was no Action specified explicitly by me.

In github.com/spulec/moto/blob/master/moto/dynamodb2/models/__init__.py#L112 one can see the code that caused the error:

def update_with_attribute_updates(self, attribute_updates):
    for attribute_name, update_action in attribute_updates.items():
        action = update_action["Action"]

So, I propose to change it to:

def update_with_attribute_updates(self, attribute_updates):
    for attribute_name, update_action in attribute_updates.items():
        # If no explicit "Action" passed, then use default value as in Boto3 documentation:
        # https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/dynamodb.html#DynamoDB.Client.update_item
        action = update_action["Action"] if "Action" in update_action else "PUT"

I would be more than happy to do a PR with this change (or a better one if you can suggest), so let me know please about your opinion and if should go for the code change, so that the future user won't have the same (customer) experience as me. :)

[micrictor]

A more succinct way of having a default value might be:

action = update_action.get("Action", "PUT")

[bblommers]

Hi @gsamaras, thanks for raising this! PR's are always welcome.

The solution by @micrictor is most readable IMO. In terms of testing, it looks like you can simply extend/replicate this test to verify that it works as expected.

[gsamaras]

@bblommers absolutely. I think replicating the unit test you linked with items that don't have the Action explicit passed to them is the way to go here IMHO. I am going to create a PR within the upcoming week, thank you so much for coming back to me and @micrictor for improving my suggested fix. :)

[gsamaras]

All checks have passed, no branch conflict, coverage remained the same.

[bblommers]

This is now part of moto >= 1.3.16.dev119