From 6427320c7678caa65a18c78fabd9d58f33785dc3 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Tue, 24 Oct 2023 20:31:25 +0000 Subject: [PATCH] DynamoDB: update_item() now returns item for ConditionalCheckFailed-exceptions (#6950) --- moto/dynamodb/exceptions.py | 42 ++++++++++--- moto/dynamodb/models/__init__.py | 14 ++++- moto/dynamodb/responses.py | 4 ++ .../exceptions/test_dynamodb_exceptions.py | 51 +++++++++++++++ tests/test_dynamodb/test_dynamodb.py | 63 ++++++++----------- 5 files changed, 125 insertions(+), 49 deletions(-) diff --git a/moto/dynamodb/exceptions.py b/moto/dynamodb/exceptions.py index a5da4ecff..fa8de1535 100644 --- a/moto/dynamodb/exceptions.py +++ b/moto/dynamodb/exceptions.py @@ -1,5 +1,5 @@ import json -from typing import Any, List, Optional +from typing import Any, Dict, List, Optional from moto.core.exceptions import JsonRESTError from moto.dynamodb.limits import HASH_KEY_MAX_LENGTH, RANGE_KEY_MAX_LENGTH @@ -195,10 +195,31 @@ class IncorrectDataType(MockValidationException): class ConditionalCheckFailed(DynamodbException): error_type = ERROR_TYPE_PREFIX + "ConditionalCheckFailedException" - def __init__(self, msg: Optional[str] = None): - super().__init__( - ConditionalCheckFailed.error_type, msg or "The conditional request failed" - ) + def __init__( + self, msg: Optional[str] = None, item: Optional[Dict[str, Any]] = None + ): + _msg = msg or "The conditional request failed" + super().__init__(ConditionalCheckFailed.error_type, _msg) + if item: + self.description = json.dumps( + { + "__type": ConditionalCheckFailed.error_type, + # Note the uppercase Message + # This ensures the message is only part of the 'error': {'message': .., 'code': ..} + "Message": _msg, + "Item": item, + } + ) + else: + self.description = json.dumps( + { + "__type": ConditionalCheckFailed.error_type, + # Note that lowercase 'message' + # This ensures that 'message' is a top-level field in the response + # (in addition to being part of the 'error': {'message': .., 'code': ..} + "message": _msg, + } + ) class TransactionCanceledException(DynamodbException): @@ -212,10 +233,13 @@ class TransactionCanceledException(DynamodbException): super().__init__( error_type=TransactionCanceledException.error_type, message=msg ) - reasons = [ - {"Code": code, "Message": message, **item} if code else {"Code": "None"} - for code, message, item in errors - ] + reasons = [] + for code, message, item in errors: + r = {"Code": code, "Message": message} if code else {"Code": "None"} + if item: + r["Item"] = item + reasons.append(r) + self.description = json.dumps( { "__type": TransactionCanceledException.error_type, diff --git a/moto/dynamodb/models/__init__.py b/moto/dynamodb/models/__init__.py index 71123a97c..cf0318e3a 100644 --- a/moto/dynamodb/models/__init__.py +++ b/moto/dynamodb/models/__init__.py @@ -385,6 +385,7 @@ class DynamoDBBackend(BaseBackend): attribute_updates: Optional[Dict[str, Any]] = None, expected: Optional[Dict[str, Any]] = None, condition_expression: Optional[str] = None, + return_values_on_condition_check_failure: Optional[str] = None, ) -> Item: table = self.get_table(table_name) @@ -424,7 +425,13 @@ class DynamoDBBackend(BaseBackend): expression_attribute_values, ) if not condition_op.expr(item): - raise ConditionalCheckFailed + if ( + return_values_on_condition_check_failure == "ALL_OLD" + and item is not None + ): + raise ConditionalCheckFailed(item=item.to_json()["Attributes"]) + else: + raise ConditionalCheckFailed # Update does not fail on new items, so create one if item is None: @@ -538,6 +545,7 @@ class DynamoDBBackend(BaseBackend): Union[Tuple[str, str, Dict[str, Any]], Tuple[None, None, None]] ] = [] # [(Code, Message, Item), ..] for item in transact_items: + original_item: Optional[Dict[str, Any]] = None # check transact writes are not performing multiple operations # in the same item if len(list(item.keys())) > 1: @@ -586,7 +594,7 @@ class DynamoDBBackend(BaseBackend): return_values_on_condition_check_failure == "ALL_OLD" and current ): - item["Item"] = current.to_json()["Attributes"] + original_item = current.to_json()["Attributes"] self.put_item( table_name, @@ -643,7 +651,7 @@ class DynamoDBBackend(BaseBackend): self.tables = original_table_state raise MultipleTransactionsException() except Exception as e: # noqa: E722 Do not use bare except - errors.append((type(e).__name__, e.message, item)) # type: ignore[attr-defined] + errors.append((type(e).__name__, e.message, original_item)) # type: ignore if any([code is not None for code, _, _ in errors]): # Rollback to the original state, and reraise the errors self.tables = original_table_state diff --git a/moto/dynamodb/responses.py b/moto/dynamodb/responses.py index fd1ea51da..bcbad5883 100644 --- a/moto/dynamodb/responses.py +++ b/moto/dynamodb/responses.py @@ -851,6 +851,9 @@ class DynamoHandler(BaseResponse): raise MockValidationException( "Can not use both expression and non-expression parameters in the same request: Non-expression parameters: {AttributeUpdates} Expression parameters: {UpdateExpression}" ) + return_values_on_condition_check_failure = self.body.get( + "ReturnValuesOnConditionCheckFailure" + ) # We need to copy the item in order to avoid it being modified by the update_item operation existing_item = copy.deepcopy(self.dynamodb_backend.get_item(name, key)) if existing_item: @@ -887,6 +890,7 @@ class DynamoHandler(BaseResponse): expression_attribute_values=expression_attribute_values, expected=expected, condition_expression=condition_expression, + return_values_on_condition_check_failure=return_values_on_condition_check_failure, ) item_dict = item.to_json() diff --git a/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py b/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py index f7b7284a3..9cd1ec6c0 100644 --- a/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py +++ b/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py @@ -5,6 +5,7 @@ from boto3.dynamodb.conditions import Key from botocore.exceptions import ClientError from unittest import SkipTest from moto import mock_dynamodb, settings +from .. import dynamodb_aws_verified table_schema = { "KeySchema": [{"AttributeName": "partitionKey", "KeyType": "HASH"}], @@ -1113,3 +1114,53 @@ def test_query_with_missing_expression_attribute(): err["Message"] == "Invalid condition in KeyConditionExpression: Multiple attribute names used in one condition" ) + + +@pytest.mark.aws_verified +@dynamodb_aws_verified +def test_update_item_returns_old_item(table_name=None): + dynamodb = boto3.resource("dynamodb", region_name="us-east-1") + table = dynamodb.Table(table_name) + table.put_item(Item={"pk": "mark", "lock": {"acquired_at": 123}}) + + with pytest.raises(ClientError) as exc: + table.update_item( + Key={"pk": "mark"}, + UpdateExpression="set #lock = :lock", + ExpressionAttributeNames={ + "#lock": "lock", + "#acquired_at": "acquired_at", + }, + ExpressionAttributeValues={":lock": {"acquired_at": 124}}, + ConditionExpression="attribute_not_exists(#lock.#acquired_at)", + ) + resp = exc.value.response + assert resp["Error"] == { + "Message": "The conditional request failed", + "Code": "ConditionalCheckFailedException", + } + assert resp["message"] == "The conditional request failed" + assert "Item" not in resp + + with pytest.raises(ClientError) as exc: + table.update_item( + Key={"pk": "mark"}, + UpdateExpression="set #lock = :lock", + ExpressionAttributeNames={ + "#lock": "lock", + "#acquired_at": "acquired_at", + }, + ExpressionAttributeValues={":lock": {"acquired_at": 123}}, + ReturnValuesOnConditionCheckFailure="ALL_OLD", + ConditionExpression="attribute_not_exists(#lock.#acquired_at)", + ) + resp = exc.value.response + assert resp["Error"] == { + "Message": "The conditional request failed", + "Code": "ConditionalCheckFailedException", + } + assert "message" not in resp + assert resp["Item"] == { + "lock": {"M": {"acquired_at": {"N": "123"}}}, + "pk": {"S": "mark"}, + } diff --git a/tests/test_dynamodb/test_dynamodb.py b/tests/test_dynamodb/test_dynamodb.py index ec5f1982f..15f45e915 100644 --- a/tests/test_dynamodb/test_dynamodb.py +++ b/tests/test_dynamodb/test_dynamodb.py @@ -1,20 +1,20 @@ +import boto3 +import pytest import uuid +import re +from botocore.exceptions import ClientError from datetime import datetime from decimal import Decimal -import boto3 from boto3.dynamodb.conditions import Attr, Key from boto3.dynamodb.types import Binary -import re from moto import mock_dynamodb, settings from moto.core import DEFAULT_ACCOUNT_ID as ACCOUNT_ID from moto.dynamodb import dynamodb_backends -from botocore.exceptions import ClientError import moto.dynamodb.comparisons import moto.dynamodb.models - -import pytest +from . import dynamodb_aws_verified @mock_dynamodb @@ -3685,17 +3685,12 @@ def test_transact_write_items_put(): assert len(items) == 5 -@mock_dynamodb -def test_transact_write_items_put_conditional_expressions(): - table_schema = { - "KeySchema": [{"AttributeName": "id", "KeyType": "HASH"}], - "AttributeDefinitions": [{"AttributeName": "id", "AttributeType": "S"}], - } +@pytest.mark.aws_verified +@dynamodb_aws_verified +def test_transact_write_items_put_conditional_expressions(table_name=None): dynamodb = boto3.client("dynamodb", region_name="us-east-1") - dynamodb.create_table( - TableName="test-table", BillingMode="PAY_PER_REQUEST", **table_schema - ) - dynamodb.put_item(TableName="test-table", Item={"id": {"S": "foo2"}}) + + dynamodb.put_item(TableName=table_name, Item={"pk": {"S": "foo2"}}) # Put multiple items with pytest.raises(ClientError) as ex: dynamodb.transact_write_items( @@ -3703,12 +3698,12 @@ def test_transact_write_items_put_conditional_expressions(): { "Put": { "Item": { - "id": {"S": f"foo{i}"}, + "pk": {"S": f"foo{i}"}, "foo": {"S": "bar"}, }, - "TableName": "test-table", + "TableName": table_name, "ConditionExpression": "#i <> :i", - "ExpressionAttributeNames": {"#i": "id"}, + "ExpressionAttributeNames": {"#i": "pk"}, "ExpressionAttributeValues": { ":i": { "S": "foo2" @@ -3726,27 +3721,20 @@ def test_transact_write_items_put_conditional_expressions(): assert { "Code": "ConditionalCheckFailed", "Message": "The conditional request failed", - "Item": {"id": {"S": "foo2"}, "foo": {"S": "bar"}}, } in reasons assert {"Code": "None"} in reasons assert ex.value.response["ResponseMetadata"]["HTTPStatusCode"] == 400 # Assert all are present - items = dynamodb.scan(TableName="test-table")["Items"] + items = dynamodb.scan(TableName=table_name)["Items"] assert len(items) == 1 - assert items[0] == {"id": {"S": "foo2"}} + assert items[0] == {"pk": {"S": "foo2"}} -@mock_dynamodb -def test_transact_write_items_put_conditional_expressions_return_values_on_condition_check_failure_all_old(): - table_schema = { - "KeySchema": [{"AttributeName": "id", "KeyType": "HASH"}], - "AttributeDefinitions": [{"AttributeName": "id", "AttributeType": "S"}], - } +@pytest.mark.aws_verified +@dynamodb_aws_verified +def test_transact_write_items_failure__return_item(table_name=None): dynamodb = boto3.client("dynamodb", region_name="us-east-1") - dynamodb.create_table( - TableName="test-table", BillingMode="PAY_PER_REQUEST", **table_schema - ) - dynamodb.put_item(TableName="test-table", Item={"id": {"S": "foo2"}}) + dynamodb.put_item(TableName=table_name, Item={"pk": {"S": "foo2"}}) # Put multiple items with pytest.raises(ClientError) as ex: dynamodb.transact_write_items( @@ -3754,12 +3742,13 @@ def test_transact_write_items_put_conditional_expressions_return_values_on_condi { "Put": { "Item": { - "id": {"S": f"foo{i}"}, + "pk": {"S": f"foo{i}"}, "foo": {"S": "bar"}, }, - "TableName": "test-table", + "TableName": table_name, "ConditionExpression": "#i <> :i", - "ExpressionAttributeNames": {"#i": "id"}, + "ExpressionAttributeNames": {"#i": "pk"}, + # This man right here - should return item as part of error message "ReturnValuesOnConditionCheckFailure": "ALL_OLD", "ExpressionAttributeValues": { ":i": { @@ -3778,14 +3767,14 @@ def test_transact_write_items_put_conditional_expressions_return_values_on_condi assert { "Code": "ConditionalCheckFailed", "Message": "The conditional request failed", - "Item": {"id": {"S": "foo2"}}, + "Item": {"pk": {"S": "foo2"}}, } in reasons assert {"Code": "None"} in reasons assert ex.value.response["ResponseMetadata"]["HTTPStatusCode"] == 400 # Assert all are present - items = dynamodb.scan(TableName="test-table")["Items"] + items = dynamodb.scan(TableName=table_name)["Items"] assert len(items) == 1 - assert items[0] == {"id": {"S": "foo2"}} + assert items[0] == {"pk": {"S": "foo2"}} @mock_dynamodb