From 77fcafca18bd95764d6e0f727dc3122517e06491 Mon Sep 17 00:00:00 2001 From: Terry Cain Date: Sun, 8 Oct 2017 04:18:25 +0100 Subject: [PATCH] Cleaned up code --- moto/dynamodb2/comparisons.py | 130 +++++++++++-------- moto/dynamodb2/responses.py | 21 +++- tests/test_dynamodb2/test_dynamodb.py | 175 +++++++++++++++++++++++--- 3 files changed, 255 insertions(+), 71 deletions(-) diff --git a/moto/dynamodb2/comparisons.py b/moto/dynamodb2/comparisons.py index 57bfe6a39..faaaaf638 100644 --- a/moto/dynamodb2/comparisons.py +++ b/moto/dynamodb2/comparisons.py @@ -69,7 +69,8 @@ def get_filter_expression(expr, names, values): # Remove all spaces, tbf we could just skip them in the next step. # The number of known options is really small so we can do a fair bit of cheating - expr = list(re.sub('\s', '', expr)) # 'Id>5ANDattribute_exists(test)ORNOTlength<6' + #expr = list(re.sub('\s', '', expr)) # 'Id>5ANDattribute_exists(test)ORNOTlength<6' + expr = list(expr) # DodgyTokenisation stage 1 def is_value(val): @@ -90,7 +91,11 @@ def get_filter_expression(expr, names, values): while len(expr) > 0: current_char = expr.pop(0) - if current_char == ',': # Split params , + if current_char == ' ': + if len(stack) > 0: + tokens.append(stack) + stack = '' + elif current_char == ',': # Split params , if len(stack) > 0: tokens.append(stack) stack = '' @@ -113,6 +118,9 @@ def get_filter_expression(expr, names, values): if len(stack) > 0: tokens.append(stack) + def is_op(val): + return val in ('<', '>', '=', '>=', '<=', '<>', 'BETWEEN', 'IN', 'AND', 'OR', 'NOT') + # DodgyTokenisation stage 2, it groups together some elements to make RPN'ing it later easier. tokens2 = [] token_iterator = iter(tokens) @@ -122,17 +130,30 @@ def get_filter_expression(expr, names, values): next_token = six.next(token_iterator) while next_token != ')': + try: + next_token = int(next_token) + except ValueError: + try: + next_token = float(next_token) + except ValueError: + pass tuple_list.append(next_token) next_token = six.next(token_iterator) - tokens2.append(tuple(tuple_list)) + # Sigh, we only want to group a tuple if it doesnt contain operators + if any([is_op(item) for item in tuple_list]): + tokens2.append('(') + tokens2.extend(tuple_list) + tokens2.append(')') + else: + tokens2.append(tuple(tuple_list)) elif token == 'BETWEEN': - op1 = six.next(token_iterator) + field = tokens2.pop() + op1 = int(six.next(token_iterator)) and_op = six.next(token_iterator) assert and_op == 'AND' - op2 = six.next(token_iterator) - tokens2.append('BETWEEN') - tokens2.append((op1, op2)) + op2 = int(six.next(token_iterator)) + tokens2.append(['between', field, op1, op2]) elif is_function(token): function_list = [token] @@ -161,39 +182,38 @@ def get_filter_expression(expr, names, values): def is_number(val): return val not in ('<', '>', '=', '>=', '<=', '<>', 'BETWEEN', 'IN', 'AND', 'OR', 'NOT') - def is_op(val): - return val in ('<', '>', '=', '>=', '<=', '<>', 'BETWEEN', 'IN', 'AND', 'OR', 'NOT') + OPS = {'<': 5, '>': 5, '=': 5, '>=': 5, '<=': 5, '<>': 5, 'IN': 8, 'AND': 11, 'OR': 12, 'NOT': 10, 'BETWEEN': 9, '(': 100, ')': 100} - OPS = {'<': 5, '>': 5, '=': 5, '>=': 5, '<=': 5, '<>': 5, 'IN': 8, 'AND': 11, 'OR': 12, 'NOT': 10, 'BETWEEN': 9, '(': 1, ')': 1} + def shunting_yard(token_list): + output = [] + op_stack = [] - output = [] - op_stack = [] - # Basically takes in an infix notation calculation, converts it to a reverse polish notation where there is no - # ambiguaty on which order operators are applied. - while len(tokens2) > 0: - token = tokens2.pop(0) + # Basically takes in an infix notation calculation, converts it to a reverse polish notation where there is no + # ambiguaty on which order operators are applied. + while len(token_list) > 0: + token = token_list.pop(0) - if token == '(': - op_stack.append(token) - elif token == ')': - while len(op_stack) > 0 and op_stack[-1] != '(': - output.append(op_stack.pop()) - if len(op_stack) == 0: - # No left paren on the stack, error - raise Exception('Missing left paren') + if token == '(': + op_stack.append(token) + elif token == ')': + while len(op_stack) > 0 and op_stack[-1] != '(': + output.append(op_stack.pop()) + lbracket = op_stack.pop() + assert lbracket == '(' - # Pop off the left paren - op_stack.pop() + elif is_number(token): + output.append(token) + else: + # Must be operator kw + while len(op_stack) > 0 and OPS[op_stack[-1]] <= OPS[token]: + output.append(op_stack.pop()) + op_stack.append(token) + while len(op_stack) > 0: + output.append(op_stack.pop()) - elif is_number(token): - output.append(token) - else: - # Must be operator kw - while len(op_stack) > 0 and OPS[op_stack[-1]] <= OPS[token]: - output.append(op_stack.pop()) - op_stack.append(token) - while len(op_stack) > 0: - output.append(op_stack.pop()) + return output + + output = shunting_yard(tokens2) # Hacky funcition to convert dynamo functions (which are represented as lists) to their Class equivelent def to_func(val): @@ -217,7 +237,11 @@ def get_filter_expression(expr, names, values): else: stack.append(to_func(token)) - return stack[0] + result = stack.pop(0) + if len(stack) > 0: + raise ValueError('Malformed filter expression') + + return result class Op(object): @@ -249,7 +273,7 @@ class Op(object): rhs = self.rhs if isinstance(self.rhs, (Op, Func)): rhs = self.rhs.expr(item) - elif isinstance(self.lhs, six.string_types): + elif isinstance(self.rhs, six.string_types): try: rhs = item.attrs[self.rhs].cast_value except Exception: @@ -357,15 +381,6 @@ class OpIn(Op): return lhs in rhs -class OpBetween(Op): - OP = 'BETWEEN' - - def expr(self, item): - lhs = self._lhs(item) - rhs = self._rhs(item) - return rhs[0] <= lhs <= rhs[1] - - class FuncAttrExists(Func): FUNC = 'attribute_exists' @@ -432,18 +447,32 @@ class FuncSize(Func): def expr(self, item): if self.attr not in item.attrs: - raise ValueError('Invalid option') + raise ValueError('Invalid attribute name {0}'.format(self.attr)) if item.attrs[self.attr].type in ('S', 'SS', 'NS', 'B', 'BS', 'L', 'M'): return len(item.attrs[self.attr].value) - raise ValueError('Invalid option') + raise ValueError('Invalid filter expression') + + +class FuncBetween(Func): + FUNC = 'between' + + def __init__(self, attribute, start, end): + self.attr = attribute + self.start = start + self.end = end + + def expr(self, item): + if self.attr not in item.attrs: + raise ValueError('Invalid attribute name {0}'.format(self.attr)) + + return self.start <= item.attrs[self.attr].cast_value <= self.end OP_CLASS = { 'AND': OpAnd, 'OR': OpOr, 'IN': OpIn, - 'BETWEEN': OpBetween, '<': OpLessThan, '>': OpGreaterThan, '<=': OpLessThanOrEqual, @@ -458,5 +487,6 @@ FUNC_CLASS = { 'attribute_type': FuncAttrType, 'begins_with': FuncBeginsWith, 'contains': FuncContains, - 'size': FuncSize + 'size': FuncSize, + 'between': FuncBetween } diff --git a/moto/dynamodb2/responses.py b/moto/dynamodb2/responses.py index 32af47df1..75e625c73 100644 --- a/moto/dynamodb2/responses.py +++ b/moto/dynamodb2/responses.py @@ -439,13 +439,22 @@ class DynamoHandler(BaseResponse): exclusive_start_key = self.body.get('ExclusiveStartKey') limit = self.body.get("Limit") - items, scanned_count, last_evaluated_key = dynamodb_backend2.scan(name, filters, - limit, - exclusive_start_key, - filter_expression, - expression_attribute_names, - expression_attribute_values) + try: + items, scanned_count, last_evaluated_key = dynamodb_backend2.scan(name, filters, + limit, + exclusive_start_key, + filter_expression, + expression_attribute_names, + expression_attribute_values) + except ValueError as err: + er = 'com.amazonaws.dynamodb.v20111205#ValidationError' + return self.error(er, 'Bad Filter Expression: {0}'.format(err)) + except Exception as err: + er = 'com.amazonaws.dynamodb.v20111205#InternalFailure' + return self.error(er, 'Internal error. {0}'.format(err)) + # Items should be a list, at least an empty one. Is None if table does not exist. + # Should really check this at the beginning if items is None: er = 'com.amazonaws.dynamodb.v20111205#ResourceNotFoundException' return self.error(er, 'Requested resource not found') diff --git a/tests/test_dynamodb2/test_dynamodb.py b/tests/test_dynamodb2/test_dynamodb.py index 994c64e7c..85d8feb34 100644 --- a/tests/test_dynamodb2/test_dynamodb.py +++ b/tests/test_dynamodb2/test_dynamodb.py @@ -576,29 +576,51 @@ def test_get_item_returns_consumed_capacity(): def test_filter_expression(): + # TODO NOT not yet supported row1 = moto.dynamodb2.models.Item(None, None, None, None, {'Id': {'N': '8'}, 'Subs': {'N': '5'}, 'Desc': {'S': 'Some description'}, 'KV': {'SS': ['test1', 'test2']}}) row2 = moto.dynamodb2.models.Item(None, None, None, None, {'Id': {'N': '8'}, 'Subs': {'N': '10'}, 'Desc': {'S': 'A description'}, 'KV': {'SS': ['test3', 'test4']}}) - filter1 = moto.dynamodb2.comparisons.get_filter_expression('Id > 5 AND Subs < 7', {}, {}) - filter1.expr(row1).should.be(True) - filter1.expr(row2).should.be(False) + # AND test + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id > 5 AND Subs < 7', {}, {}) + filter_expr.expr(row1).should.be(True) + filter_expr.expr(row2).should.be(False) - filter2 = moto.dynamodb2.comparisons.get_filter_expression('attribute_exists(Id) AND attribute_not_exists(User)', {}, {}) - filter2.expr(row1).should.be(True) + # OR test + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id = 5 OR Id=8', {}, {}) + filter_expr.expr(row1).should.be(True) - filter3 = moto.dynamodb2.comparisons.get_filter_expression('attribute_type(Id, N)', {}, {}) - filter3.expr(row1).should.be(True) + # BETWEEN test + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id BETWEEN 5 AND 10', {}, {}) + filter_expr.expr(row1).should.be(True) - filter4 = moto.dynamodb2.comparisons.get_filter_expression('begins_with(Desc, Some)', {}, {}) - filter4.expr(row1).should.be(True) - filter4.expr(row2).should.be(False) + # PAREN test + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id = 8 AND (Subs = 8 OR Subs = 5)', {}, {}) + filter_expr.expr(row1).should.be(True) - filter5 = moto.dynamodb2.comparisons.get_filter_expression('contains(KV, test1)', {}, {}) - filter5.expr(row1).should.be(True) - filter5.expr(row2).should.be(False) + # IN test + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id IN (7,8, 9)', {}, {}) + filter_expr.expr(row1).should.be(True) - filter6 = moto.dynamodb2.comparisons.get_filter_expression('size(Desc) > size(KV)', {}, {}) - filter6.expr(row1).should.be(True) + # attribute function tests + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('attribute_exists(Id) AND attribute_not_exists(User)', {}, {}) + filter_expr.expr(row1).should.be(True) + + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('attribute_type(Id, N)', {}, {}) + filter_expr.expr(row1).should.be(True) + + # beginswith function test + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('begins_with(Desc, Some)', {}, {}) + filter_expr.expr(row1).should.be(True) + filter_expr.expr(row2).should.be(False) + + # contains function test + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('contains(KV, test1)', {}, {}) + filter_expr.expr(row1).should.be(True) + filter_expr.expr(row2).should.be(False) + + # size function test + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('size(Desc) > size(KV)', {}, {}) + filter_expr.expr(row1).should.be(True) @mock_dynamodb2 @@ -631,3 +653,126 @@ def test_scan_filter(): FilterExpression=Attr('app').eq('app1') ) assert response['Count'] == 1 + + +@mock_dynamodb2 +def test_bad_scan_filter(): + client = boto3.client('dynamodb', region_name='us-east-1') + dynamodb = boto3.resource('dynamodb', region_name='us-east-1') + + # Create the DynamoDB table. + client.create_table( + TableName='test1', + AttributeDefinitions=[{'AttributeName': 'client', 'AttributeType': 'S'}, {'AttributeName': 'app', 'AttributeType': 'S'}], + KeySchema=[{'AttributeName': 'client', 'KeyType': 'HASH'}, {'AttributeName': 'app', 'KeyType': 'RANGE'}], + ProvisionedThroughput={'ReadCapacityUnits': 123, 'WriteCapacityUnits': 123} + ) + table = dynamodb.Table('test1') + + # Bad expression + try: + table.scan( + FilterExpression='client test' + ) + except ClientError as err: + err.response['Error']['Code'].should.equal('ValidationError') + else: + raise RuntimeError('Should of raised ResourceInUseException') + + + +@mock_dynamodb2 +def test_duplicate_create(): + client = boto3.client('dynamodb', region_name='us-east-1') + + # Create the DynamoDB table. + client.create_table( + TableName='test1', + AttributeDefinitions=[{'AttributeName': 'client', 'AttributeType': 'S'}, {'AttributeName': 'app', 'AttributeType': 'S'}], + KeySchema=[{'AttributeName': 'client', 'KeyType': 'HASH'}, {'AttributeName': 'app', 'KeyType': 'RANGE'}], + ProvisionedThroughput={'ReadCapacityUnits': 123, 'WriteCapacityUnits': 123} + ) + + try: + client.create_table( + TableName='test1', + AttributeDefinitions=[{'AttributeName': 'client', 'AttributeType': 'S'}, {'AttributeName': 'app', 'AttributeType': 'S'}], + KeySchema=[{'AttributeName': 'client', 'KeyType': 'HASH'}, {'AttributeName': 'app', 'KeyType': 'RANGE'}], + ProvisionedThroughput={'ReadCapacityUnits': 123, 'WriteCapacityUnits': 123} + ) + except ClientError as err: + err.response['Error']['Code'].should.equal('ResourceInUseException') + else: + raise RuntimeError('Should of raised ResourceInUseException') + + +@mock_dynamodb2 +def test_delete_table(): + client = boto3.client('dynamodb', region_name='us-east-1') + + # Create the DynamoDB table. + client.create_table( + TableName='test1', + AttributeDefinitions=[{'AttributeName': 'client', 'AttributeType': 'S'}, {'AttributeName': 'app', 'AttributeType': 'S'}], + KeySchema=[{'AttributeName': 'client', 'KeyType': 'HASH'}, {'AttributeName': 'app', 'KeyType': 'RANGE'}], + ProvisionedThroughput={'ReadCapacityUnits': 123, 'WriteCapacityUnits': 123} + ) + + client.delete_table(TableName='test1') + + resp = client.list_tables() + len(resp['TableNames']).should.equal(0) + + try: + client.delete_table(TableName='test1') + except ClientError as err: + err.response['Error']['Code'].should.equal('ResourceNotFoundException') + else: + raise RuntimeError('Should of raised ResourceNotFoundException') + + +@mock_dynamodb2 +def test_delete_item(): + client = boto3.client('dynamodb', region_name='us-east-1') + dynamodb = boto3.resource('dynamodb', region_name='us-east-1') + + # Create the DynamoDB table. + client.create_table( + TableName='test1', + AttributeDefinitions=[{'AttributeName': 'client', 'AttributeType': 'S'}, {'AttributeName': 'app', 'AttributeType': 'S'}], + KeySchema=[{'AttributeName': 'client', 'KeyType': 'HASH'}, {'AttributeName': 'app', 'KeyType': 'RANGE'}], + ProvisionedThroughput={'ReadCapacityUnits': 123, 'WriteCapacityUnits': 123} + ) + client.put_item( + TableName='test1', + Item={ + 'client': {'S': 'client1'}, + 'app': {'S': 'app1'} + } + ) + client.put_item( + TableName='test1', + Item={ + 'client': {'S': 'client1'}, + 'app': {'S': 'app2'} + } + ) + + table = dynamodb.Table('test1') + response = table.scan() + assert response['Count'] == 2 + + # Test deletion and returning old value + response = table.delete_item(Key={'client': 'client1', 'app': 'app1'}, ReturnValues='ALL_OLD') + response['Attributes'].should.contain('client') + response['Attributes'].should.contain('app') + + response = table.scan() + assert response['Count'] == 1 + + # Test deletion returning nothing + response = table.delete_item(Key={'client': 'client1', 'app': 'app2'}) + len(response['Attributes']).should.equal(0) + + response = table.scan() + assert response['Count'] == 0