Cleaned up code

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)
+            op2 = int(six.next(token_iterator))
-            tokens2.append('BETWEEN')
+            tokens2.append(['between', field, op1, op2])
-            tokens2.append((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):
+    OPS = {'<': 5, '>': 5, '=': 5, '>=': 5, '<=': 5, '<>': 5, 'IN': 8, 'AND': 11, 'OR': 12, 'NOT': 10, 'BETWEEN': 9, '(': 100, ')': 100}
-        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, '(': 1, ')': 1}
+    def shunting_yard(token_list):
+        output = []
+        op_stack = []
 
-    output = []
+        # Basically takes in an infix notation calculation, converts it to a reverse polish notation where there is no
-    op_stack = []
+        # ambiguaty on which order operators are applied.
-    # Basically takes in an infix notation calculation, converts it to a reverse polish notation where there is no
+        while len(token_list) > 0:
-    # ambiguaty on which order operators are applied.
+            token = token_list.pop(0)
-    while len(tokens2) > 0:
-        token = tokens2.pop(0)
 
-        if token == '(':
+            if token == '(':
-            op_stack.append(token)
+                op_stack.append(token)
-        elif token == ')':
+            elif token == ')':
-            while len(op_stack) > 0 and op_stack[-1] != '(':
+                while len(op_stack) > 0 and op_stack[-1] != '(':
-                output.append(op_stack.pop())
+                    output.append(op_stack.pop())
-            if len(op_stack) == 0:
+                lbracket = op_stack.pop()
-                # No left paren on the stack, error
+                assert lbracket == '('
-                raise Exception('Missing left paren')
 
-            # Pop off the left paren
+            elif is_number(token):
-            op_stack.pop()
+                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):
+        return output
-            output.append(token)
+
-        else:
+    output = shunting_yard(tokens2)
-            # 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())
 
     # 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
 }

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,
+        try:
-                                                                          limit,
+            items, scanned_count, last_evaluated_key = dynamodb_backend2.scan(name, filters,
-                                                                          exclusive_start_key,
+                                                                              limit,
-                                                                          filter_expression,
+                                                                              exclusive_start_key,
-                                                                          expression_attribute_names,
+                                                                              filter_expression,
-                                                                          expression_attribute_values)
+                                                                              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')

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', {}, {})
+    # AND test
-    filter1.expr(row1).should.be(True)
+    filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id > 5 AND Subs < 7', {}, {})
-    filter1.expr(row2).should.be(False)
+    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)', {}, {})
+    # OR test
-    filter2.expr(row1).should.be(True)
+    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)', {}, {})
+    # BETWEEN test
-    filter3.expr(row1).should.be(True)
+    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)', {}, {})
+    # PAREN test
-    filter4.expr(row1).should.be(True)
+    filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id = 8 AND (Subs = 8 OR Subs = 5)', {}, {})
-    filter4.expr(row2).should.be(False)
+    filter_expr.expr(row1).should.be(True)
 
-    filter5 = moto.dynamodb2.comparisons.get_filter_expression('contains(KV, test1)', {}, {})
+    # IN test
-    filter5.expr(row1).should.be(True)
+    filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id IN (7,8, 9)', {}, {})
-    filter5.expr(row2).should.be(False)
+    filter_expr.expr(row1).should.be(True)
 
-    filter6 = moto.dynamodb2.comparisons.get_filter_expression('size(Desc) > size(KV)', {}, {})
+    # attribute function tests
-    filter6.expr(row1).should.be(True)
+    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