Enhancement: Add check for reserved keywords in condition expression attributes [dynamodb2] (#4778)
This commit is contained in:
parent
ebe74d2eb0
commit
6d160303a4
@ -2,6 +2,9 @@ import re
|
|||||||
from collections import deque
|
from collections import deque
|
||||||
from collections import namedtuple
|
from collections import namedtuple
|
||||||
|
|
||||||
|
from moto.dynamodb2.exceptions import ConditionAttributeIsReservedKeyword
|
||||||
|
from moto.dynamodb2.parsing.reserved_keywords import ReservedKeywords
|
||||||
|
|
||||||
|
|
||||||
def get_filter_expression(expr, names, values):
|
def get_filter_expression(expr, names, values):
|
||||||
"""
|
"""
|
||||||
@ -238,6 +241,11 @@ class ConditionExpressionParser:
|
|||||||
|
|
||||||
Node = namedtuple("Node", ["nonterminal", "kind", "text", "value", "children"])
|
Node = namedtuple("Node", ["nonterminal", "kind", "text", "value", "children"])
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def raise_exception_if_keyword(cls, attribute):
|
||||||
|
if attribute.upper() in ReservedKeywords.get_reserved_keywords():
|
||||||
|
raise ConditionAttributeIsReservedKeyword(attribute)
|
||||||
|
|
||||||
def _lex_condition_expression(self):
|
def _lex_condition_expression(self):
|
||||||
nodes = deque()
|
nodes = deque()
|
||||||
remaining_expression = self.condition_expression
|
remaining_expression = self.condition_expression
|
||||||
@ -403,6 +411,7 @@ class ConditionExpressionParser:
|
|||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
# e.g. ItemId
|
# e.g. ItemId
|
||||||
|
self.raise_exception_if_keyword(name)
|
||||||
return self.Node(
|
return self.Node(
|
||||||
nonterminal=self.Nonterminal.IDENTIFIER,
|
nonterminal=self.Nonterminal.IDENTIFIER,
|
||||||
kind=self.Kind.LITERAL,
|
kind=self.Kind.LITERAL,
|
||||||
|
@ -31,6 +31,30 @@ class InvalidUpdateExpression(MockValidationException):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class InvalidConditionExpression(MockValidationException):
|
||||||
|
invalid_condition_expr_msg = (
|
||||||
|
"Invalid ConditionExpression: {condition_expression_error}"
|
||||||
|
)
|
||||||
|
|
||||||
|
def __init__(self, condition_expression_error):
|
||||||
|
self.condition_expression_error = condition_expression_error
|
||||||
|
super().__init__(
|
||||||
|
self.invalid_condition_expr_msg.format(
|
||||||
|
condition_expression_error=condition_expression_error
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class ConditionAttributeIsReservedKeyword(InvalidConditionExpression):
|
||||||
|
attribute_is_keyword_msg = (
|
||||||
|
"Attribute name is a reserved keyword; reserved keyword: {keyword}"
|
||||||
|
)
|
||||||
|
|
||||||
|
def __init__(self, keyword):
|
||||||
|
self.keyword = keyword
|
||||||
|
super().__init__(self.attribute_is_keyword_msg.format(keyword=keyword))
|
||||||
|
|
||||||
|
|
||||||
class AttributeDoesNotExist(MockValidationException):
|
class AttributeDoesNotExist(MockValidationException):
|
||||||
attr_does_not_exist_msg = (
|
attr_does_not_exist_msg = (
|
||||||
"The provided expression refers to an attribute that does not exist in the item"
|
"The provided expression refers to an attribute that does not exist in the item"
|
||||||
|
@ -1173,7 +1173,7 @@ def test_filter_expression():
|
|||||||
attrs={
|
attrs={
|
||||||
"Id": {"N": "8"},
|
"Id": {"N": "8"},
|
||||||
"Subs": {"N": "5"},
|
"Subs": {"N": "5"},
|
||||||
"Desc": {"S": "Some description"},
|
"Des": {"S": "Some description"},
|
||||||
"KV": {"SS": ["test1", "test2"]},
|
"KV": {"SS": ["test1", "test2"]},
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
@ -1183,7 +1183,7 @@ def test_filter_expression():
|
|||||||
attrs={
|
attrs={
|
||||||
"Id": {"N": "8"},
|
"Id": {"N": "8"},
|
||||||
"Subs": {"N": "10"},
|
"Subs": {"N": "10"},
|
||||||
"Desc": {"S": "A description"},
|
"Des": {"S": "A description"},
|
||||||
"KV": {"SS": ["test3", "test4"]},
|
"KV": {"SS": ["test3", "test4"]},
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
@ -1250,7 +1250,7 @@ def test_filter_expression():
|
|||||||
|
|
||||||
# attribute function tests (with extra spaces)
|
# attribute function tests (with extra spaces)
|
||||||
filter_expr = moto.dynamodb2.comparisons.get_filter_expression(
|
filter_expr = moto.dynamodb2.comparisons.get_filter_expression(
|
||||||
"attribute_exists(Id) AND attribute_not_exists (User)", {}, {}
|
"attribute_exists(Id) AND attribute_not_exists (UnknownAttribute)", {}, {}
|
||||||
)
|
)
|
||||||
filter_expr.expr(row1).should.be(True)
|
filter_expr.expr(row1).should.be(True)
|
||||||
|
|
||||||
@ -1261,7 +1261,7 @@ def test_filter_expression():
|
|||||||
|
|
||||||
# beginswith function test
|
# beginswith function test
|
||||||
filter_expr = moto.dynamodb2.comparisons.get_filter_expression(
|
filter_expr = moto.dynamodb2.comparisons.get_filter_expression(
|
||||||
"begins_with(Desc, :v0)", {}, {":v0": {"S": "Some"}}
|
"begins_with(Des, :v0)", {}, {":v0": {"S": "Some"}}
|
||||||
)
|
)
|
||||||
filter_expr.expr(row1).should.be(True)
|
filter_expr.expr(row1).should.be(True)
|
||||||
filter_expr.expr(row2).should.be(False)
|
filter_expr.expr(row2).should.be(False)
|
||||||
@ -1275,7 +1275,7 @@ def test_filter_expression():
|
|||||||
|
|
||||||
# size function test
|
# size function test
|
||||||
filter_expr = moto.dynamodb2.comparisons.get_filter_expression(
|
filter_expr = moto.dynamodb2.comparisons.get_filter_expression(
|
||||||
"size(Desc) > size(KV)", {}, {}
|
"size(Des) > size(KV)", {}, {}
|
||||||
)
|
)
|
||||||
filter_expr.expr(row1).should.be(True)
|
filter_expr.expr(row1).should.be(True)
|
||||||
|
|
||||||
@ -1288,7 +1288,7 @@ def test_filter_expression():
|
|||||||
filter_expr.expr(row1).should.be(True)
|
filter_expr.expr(row1).should.be(True)
|
||||||
# Expression from to check contains on string value
|
# Expression from to check contains on string value
|
||||||
filter_expr = moto.dynamodb2.comparisons.get_filter_expression(
|
filter_expr = moto.dynamodb2.comparisons.get_filter_expression(
|
||||||
"contains(#n0, :v0)", {"#n0": "Desc"}, {":v0": {"S": "Some"}}
|
"contains(#n0, :v0)", {"#n0": "Des"}, {":v0": {"S": "Some"}}
|
||||||
)
|
)
|
||||||
filter_expr.expr(row1).should.be(True)
|
filter_expr.expr(row1).should.be(True)
|
||||||
filter_expr.expr(row2).should.be(False)
|
filter_expr.expr(row2).should.be(False)
|
||||||
|
@ -1,8 +1,9 @@
|
|||||||
|
from decimal import Decimal
|
||||||
|
import re
|
||||||
|
|
||||||
import boto3
|
import boto3
|
||||||
import pytest
|
import pytest
|
||||||
import sure # noqa # pylint: disable=unused-import
|
import sure # noqa # pylint: disable=unused-import
|
||||||
|
|
||||||
from decimal import Decimal
|
|
||||||
from moto import mock_dynamodb2
|
from moto import mock_dynamodb2
|
||||||
|
|
||||||
|
|
||||||
@ -365,3 +366,67 @@ def test_condition_expression__and_order():
|
|||||||
ExpressionAttributeValues={":ttl": {"N": "6"}, ":old_ttl": {"N": "5"}},
|
ExpressionAttributeValues={":ttl": {"N": "6"}, ":old_ttl": {"N": "5"}},
|
||||||
)
|
)
|
||||||
_assert_conditional_check_failed_exception(exc)
|
_assert_conditional_check_failed_exception(exc)
|
||||||
|
|
||||||
|
|
||||||
|
@mock_dynamodb2
|
||||||
|
def test_condition_expression_with_reserved_keyword_as_attr_name():
|
||||||
|
dynamodb = boto3.resource("dynamodb", region_name="us-east-2")
|
||||||
|
table_name = "Test"
|
||||||
|
dynamodb.create_table(
|
||||||
|
TableName=table_name,
|
||||||
|
KeySchema=[{"AttributeName": "id", "KeyType": "HASH"}],
|
||||||
|
AttributeDefinitions=[{"AttributeName": "id", "AttributeType": "S"}],
|
||||||
|
)
|
||||||
|
table = dynamodb.Table(table_name)
|
||||||
|
|
||||||
|
email_like_str = "test@foo.com"
|
||||||
|
record = {
|
||||||
|
"id": "key-0",
|
||||||
|
"first": {email_like_str: {"end": {"VALUE"}},},
|
||||||
|
}
|
||||||
|
table.put_item(Item=record)
|
||||||
|
|
||||||
|
expected_error_message = re.escape(
|
||||||
|
"An error occurred (ValidationException) when "
|
||||||
|
"calling the UpdateItem operation: Invalid ConditionExpression: Attribute name "
|
||||||
|
"is a reserved keyword; reserved keyword: end"
|
||||||
|
)
|
||||||
|
with pytest.raises(
|
||||||
|
dynamodb.meta.client.exceptions.ClientError, match=expected_error_message
|
||||||
|
):
|
||||||
|
table.update_item(
|
||||||
|
Key={"id": "key-0"},
|
||||||
|
UpdateExpression="REMOVE #first.#second, #other",
|
||||||
|
ExpressionAttributeNames={
|
||||||
|
"#first": "first",
|
||||||
|
"#second": email_like_str,
|
||||||
|
"#other": "other",
|
||||||
|
},
|
||||||
|
ExpressionAttributeValues={":value": "VALUE", ":one": 1},
|
||||||
|
ConditionExpression="size(#first.#second.end) = :one AND contains(#first.#second.end, :value)",
|
||||||
|
ReturnValues="ALL_NEW",
|
||||||
|
)
|
||||||
|
|
||||||
|
# table is unchanged
|
||||||
|
item = table.get_item(Key={"id": "key-0"})["Item"]
|
||||||
|
item.should.equal(record)
|
||||||
|
|
||||||
|
# using attribute names solves the issue
|
||||||
|
table.update_item(
|
||||||
|
Key={"id": "key-0"},
|
||||||
|
UpdateExpression="REMOVE #first.#second, #other",
|
||||||
|
ExpressionAttributeNames={
|
||||||
|
"#first": "first",
|
||||||
|
"#second": email_like_str,
|
||||||
|
"#other": "other",
|
||||||
|
"#end": "end",
|
||||||
|
},
|
||||||
|
ExpressionAttributeValues={":value": "VALUE", ":one": 1},
|
||||||
|
ConditionExpression="size(#first.#second.#end) = :one AND contains(#first.#second.#end, :value)",
|
||||||
|
ReturnValues="ALL_NEW",
|
||||||
|
)
|
||||||
|
|
||||||
|
item = table.get_item(Key={"id": "key-0"})["Item"]
|
||||||
|
item.should.equal(
|
||||||
|
{"id": "key-0", "first": {},}
|
||||||
|
)
|
||||||
|
Loading…
Reference in New Issue
Block a user