From 7f912b7a5b96b8bd466ab1c2831629ff1ce27af7 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Sat, 9 Oct 2021 21:02:53 +0000 Subject: [PATCH] DynamoDB - Throw exception when updating key using SET operation (#4245) --- moto/dynamodb2/exceptions.py | 15 +++++- moto/dynamodb2/parsing/validators.py | 19 +++++++- tests/test_dynamodb2/test_dynamodb.py | 48 +++++++++++++++++++ .../test_dynamodb_table_with_range_key.py | 46 ++++++++++++++++++ .../test_dynamodb_validation.py | 25 +--------- 5 files changed, 127 insertions(+), 26 deletions(-) diff --git a/moto/dynamodb2/exceptions.py b/moto/dynamodb2/exceptions.py index bc6615499..7d517d8c1 100644 --- a/moto/dynamodb2/exceptions.py +++ b/moto/dynamodb2/exceptions.py @@ -190,6 +190,17 @@ class TransactionCanceledException(ValueError): class EmptyKeyAttributeException(MockValidationException): empty_str_msg = "One or more parameter values were invalid: An AttributeValue may not contain an empty string" + # AWS has a different message for empty index keys + empty_index_msg = "One or more parameter values are not valid. The update expression attempted to update a secondary index key to a value that is not supported. The AttributeValue for a key attribute cannot contain an empty string value." - def __init__(self): - super(EmptyKeyAttributeException, self).__init__(self.empty_str_msg) + def __init__(self, key_in_index=False): + super(EmptyKeyAttributeException, self).__init__( + self.empty_index_msg if key_in_index else self.empty_str_msg + ) + + +class UpdateHashRangeKeyException(MockValidationException): + msg = "One or more parameter values were invalid: Cannot update attribute {}. This attribute is part of the key" + + def __init__(self, key_name): + super(UpdateHashRangeKeyException, self).__init__(self.msg.format(key_name)) diff --git a/moto/dynamodb2/parsing/validators.py b/moto/dynamodb2/parsing/validators.py index 839c75299..3d958b917 100644 --- a/moto/dynamodb2/parsing/validators.py +++ b/moto/dynamodb2/parsing/validators.py @@ -13,6 +13,7 @@ from moto.dynamodb2.exceptions import ( InvalidUpdateExpressionInvalidDocumentPath, ProvidedKeyDoesNotExist, EmptyKeyAttributeException, + UpdateHashRangeKeyException, ) from moto.dynamodb2.models import DynamoType from moto.dynamodb2.parsing.ast_nodes import ( @@ -337,7 +338,22 @@ class EmptyStringKeyValueValidator(DepthFirstTraverser): and val_node.type in ["S", "B"] and key in self.key_attributes ): - raise EmptyKeyAttributeException + raise EmptyKeyAttributeException(key_in_index=True) + return node + + +class UpdateHashRangeKeyValidator(DepthFirstTraverser): + def __init__(self, table_key_attributes): + self.table_key_attributes = table_key_attributes + + def _processing_map(self): + return {UpdateExpressionPath: self.check_for_hash_or_range_key} + + def check_for_hash_or_range_key(self, node): + """Check that hash and range keys are not updated""" + key_to_update = node.children[0].children[0] + if key_to_update in self.table_key_attributes: + raise UpdateHashRangeKeyException(key_to_update) return node @@ -386,6 +402,7 @@ class UpdateExpressionValidator(Validator): def get_ast_processors(self): """Get the different processors that go through the AST tree and processes the nodes.""" processors = [ + UpdateHashRangeKeyValidator(self.table.table_key_attrs), ExpressionAttributeValueProcessor(self.expression_attribute_values), ExpressionAttributeResolvingProcessor( self.expression_attribute_names, self.item diff --git a/tests/test_dynamodb2/test_dynamodb.py b/tests/test_dynamodb2/test_dynamodb.py index 48f647cb8..39c264c05 100644 --- a/tests/test_dynamodb2/test_dynamodb.py +++ b/tests/test_dynamodb2/test_dynamodb.py @@ -5527,6 +5527,54 @@ def test_gsi_key_can_be_updated(): item["main_key"].should.equal({"S": "testkey1"}) +@mock_dynamodb2 +def test_gsi_key_cannot_be_empty(): + name = "TestTable" + conn = boto3.client("dynamodb", region_name="eu-west-2") + conn.create_table( + TableName=name, + KeySchema=[{"AttributeName": "main_key", "KeyType": "HASH"}], + AttributeDefinitions=[ + {"AttributeName": "main_key", "AttributeType": "S"}, + {"AttributeName": "index_key", "AttributeType": "S"}, + ], + ProvisionedThroughput={"ReadCapacityUnits": 5, "WriteCapacityUnits": 5}, + GlobalSecondaryIndexes=[ + { + "IndexName": "test_index", + "KeySchema": [{"AttributeName": "index_key", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL",}, + "ProvisionedThroughput": { + "ReadCapacityUnits": 1, + "WriteCapacityUnits": 1, + }, + } + ], + ) + + conn.put_item( + TableName=name, + Item={ + "main_key": {"S": "testkey1"}, + "extra_data": {"S": "testdata"}, + "index_key": {"S": "indexkey1"}, + }, + ) + + with pytest.raises(ClientError) as ex: + conn.update_item( + TableName=name, + Key={"main_key": {"S": "testkey1"}}, + UpdateExpression="set index_key=:new_index_key", + ExpressionAttributeValues={":new_index_key": {"S": ""}}, + ) + err = ex.value.response["Error"] + err["Code"].should.equal("ValidationException") + err["Message"].should.equal( + "One or more parameter values are not valid. The update expression attempted to update a secondary index key to a value that is not supported. The AttributeValue for a key attribute cannot contain an empty string value." + ) + + @mock_dynamodb2 def test_create_backup_for_non_existent_table_raises_error(): client = boto3.client("dynamodb", "us-east-1") diff --git a/tests/test_dynamodb2/test_dynamodb_table_with_range_key.py b/tests/test_dynamodb2/test_dynamodb_table_with_range_key.py index 11cbf2c63..1f93695c5 100644 --- a/tests/test_dynamodb2/test_dynamodb_table_with_range_key.py +++ b/tests/test_dynamodb2/test_dynamodb_table_with_range_key.py @@ -14,6 +14,7 @@ import pytest from moto import mock_dynamodb2, mock_dynamodb2_deprecated from boto.exception import JSONResponseError from tests.helpers import requires_boto_gte +from uuid import uuid4 try: from boto.dynamodb2.fields import GlobalAllIndex, HashKey, RangeKey, AllIndex @@ -2283,3 +2284,48 @@ def test_scan_by_index(): assert last_eval_key["id"]["S"] == "1" assert last_eval_key["range_key"]["S"] == "1" assert last_eval_key["lsi_range_key"]["S"] == "1" + + +@mock_dynamodb2 +@pytest.mark.parametrize("create_item_first", [False, True]) +@pytest.mark.parametrize( + "expression", ["set h=:New", "set r=:New", "set x=:New, r=:New"] +) +def test_update_item_throws_exception_when_updating_hash_or_range_key( + create_item_first, expression +): + client = boto3.client("dynamodb", region_name="ap-northeast-3") + table_name = "testtable_3877" + + client.create_table( + TableName=table_name, + KeySchema=[ + {"AttributeName": "h", "KeyType": "HASH"}, + {"AttributeName": "r", "KeyType": "RANGE"}, + ], + AttributeDefinitions=[ + {"AttributeName": "h", "AttributeType": "S"}, + {"AttributeName": "r", "AttributeType": "S"}, + ], + ) + + initial_val = str(uuid4()) + + if create_item_first: + client.put_item( + TableName=table_name, Item={"h": {"S": initial_val}, "r": {"S": "1"}}, + ) + + # Updating the HASH key should fail + with pytest.raises(ClientError) as ex: + client.update_item( + TableName=table_name, + Key={"h": {"S": initial_val}, "r": {"S": "1"}}, + UpdateExpression=expression, + ExpressionAttributeValues={":New": {"S": "2"}}, + ) + err = ex.value.response["Error"] + err["Code"].should.equal("ValidationException") + err["Message"].should.match( + r"One or more parameter values were invalid: Cannot update attribute (r|h). This attribute is part of the key" + ) diff --git a/tests/test_dynamodb2/test_dynamodb_validation.py b/tests/test_dynamodb2/test_dynamodb_validation.py index 90b5ca6fb..eb33ae5c9 100644 --- a/tests/test_dynamodb2/test_dynamodb_validation.py +++ b/tests/test_dynamodb2/test_dynamodb_validation.py @@ -20,9 +20,9 @@ from moto.dynamodb2.parsing.validators import UpdateExpressionValidator def test_valid_update_expression(table): - update_expression = "set forum_name=:NewName, forum_type=:NewType" + update_expression = "set forum_desc=:Desc, forum_type=:NewType" update_expression_values = { - ":NewName": {"S": "AmazingForum"}, + ":Desc": {"S": "AmazingForum"}, ":NewType": {"S": "BASIC"}, } update_expression_ast = UpdateExpressionParser.make(update_expression) @@ -42,27 +42,6 @@ def test_valid_update_expression(table): ).validate() -def test_validation_of_empty_string_key_val(table): - with pytest.raises(EmptyKeyAttributeException): - update_expression = "set forum_name=:NewName" - update_expression_values = {":NewName": {"S": ""}} - update_expression_ast = UpdateExpressionParser.make(update_expression) - item = Item( - hash_key=DynamoType({"S": "forum_name"}), - hash_key_type="TYPE", - range_key=None, - range_key_type=None, - attrs={"forum_name": {"S": "hello"}}, - ) - UpdateExpressionValidator( - update_expression_ast, - expression_attribute_names=None, - expression_attribute_values=update_expression_values, - item=item, - table=table, - ).validate() - - def test_validation_of_update_expression_with_keyword(table): try: update_expression = "SET myNum = path + :val"