From 5c7e0b56afaeeafda053f9f15022d4520f9b2e2e Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Wed, 8 Apr 2020 13:53:53 +0100 Subject: [PATCH 01/22] #2877 - Ensure NetworkInterfaces are assigned to the default Subnet --- moto/ec2/models.py | 9 +++++- tests/test_ec2/test_instances.py | 2 +- tests/test_ec2/test_subnets.py | 47 ++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index be39bab28..2611c2f1a 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -775,7 +775,14 @@ class Instance(TaggedEC2Resource, BotoInstance): if "SubnetId" in nic: subnet = self.ec2_backend.get_subnet(nic["SubnetId"]) else: - subnet = None + # Get default Subnet + subnet = [ + subnet + for subnet in self.ec2_backend.get_all_subnets( + filters={"availabilityZone": self._placement.zone} + ) + if subnet.default_for_az + ][0] group_id = nic.get("SecurityGroupId") group_ids = [group_id] if group_id else [] diff --git a/tests/test_ec2/test_instances.py b/tests/test_ec2/test_instances.py index 85ba0fe01..fe1631223 100644 --- a/tests/test_ec2/test_instances.py +++ b/tests/test_ec2/test_instances.py @@ -71,7 +71,7 @@ def test_instance_launch_and_terminate(): instance.id.should.equal(instance.id) instance.state.should.equal("running") instance.launch_time.should.equal("2014-01-01T05:00:00.000Z") - instance.vpc_id.should.equal(None) + instance.vpc_id.shouldnt.equal(None) instance.placement.should.equal("us-east-1a") root_device_name = instance.root_device_name diff --git a/tests/test_ec2/test_subnets.py b/tests/test_ec2/test_subnets.py index 7bb57aab4..a16693d5f 100644 --- a/tests/test_ec2/test_subnets.py +++ b/tests/test_ec2/test_subnets.py @@ -599,3 +599,50 @@ def validate_subnet_details_after_creating_eni( for eni in enis_created: client.delete_network_interface(NetworkInterfaceId=eni["NetworkInterfaceId"]) client.delete_subnet(SubnetId=subnet["SubnetId"]) + + +@mock_ec2 +def test_run_instances_should_attach_to_default_subnet(): + ec2 = boto3.resource("ec2", region_name="us-west-1") + client = boto3.client("ec2", region_name="us-west-1") + ec2.create_security_group(GroupName="sg01", Description="Test security group sg01") + # run_instances + instances = client.run_instances( + MinCount=1, + MaxCount=1, + SecurityGroups=["sg01"], + TagSpecifications=[ + { + "ResourceType": "instance", + "Tags": [{"Key": "Name", "Value": "test-01"},], + } + ], + ) + default_subnet_id = client.describe_subnets()["Subnets"][0]["SubnetId"] + instances["Instances"][0]["NetworkInterfaces"][0]["SubnetId"].should.equal( + default_subnet_id + ) + + +@mock_ec2 +def test_describe_subnets_where_network_interface_has_no_subnets_attached(): + # https://github.com/spulec/moto/issues/2877 + # create security groups + ec2 = boto3.resource("ec2", region_name="us-west-1") + client = boto3.client("ec2", region_name="us-west-1") + ec2.create_security_group(GroupName="sg01", Description="Test security group sg01") + # run_instances + client.run_instances( + MinCount=1, + MaxCount=1, + SecurityGroups=["sg01"], + TagSpecifications=[ + { + "ResourceType": "instance", + "Tags": [{"Key": "Name", "Value": "test-01"},], + } + ], + ) + # describe_subnets + subnets = client.describe_subnets()["Subnets"] + subnets[0]["AvailableIpAddressCount"].should.equal(4090) From 8475804a8b37ea11fcdd5acde02dec1f6ca31b9b Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Wed, 8 Apr 2020 14:02:35 +0100 Subject: [PATCH 02/22] Simplify tests --- tests/test_ec2/test_subnets.py | 40 +++++----------------------------- 1 file changed, 5 insertions(+), 35 deletions(-) diff --git a/tests/test_ec2/test_subnets.py b/tests/test_ec2/test_subnets.py index a16693d5f..eae0bc468 100644 --- a/tests/test_ec2/test_subnets.py +++ b/tests/test_ec2/test_subnets.py @@ -603,46 +603,16 @@ def validate_subnet_details_after_creating_eni( @mock_ec2 def test_run_instances_should_attach_to_default_subnet(): + # https://github.com/spulec/moto/issues/2877 ec2 = boto3.resource("ec2", region_name="us-west-1") client = boto3.client("ec2", region_name="us-west-1") ec2.create_security_group(GroupName="sg01", Description="Test security group sg01") # run_instances - instances = client.run_instances( - MinCount=1, - MaxCount=1, - SecurityGroups=["sg01"], - TagSpecifications=[ - { - "ResourceType": "instance", - "Tags": [{"Key": "Name", "Value": "test-01"},], - } - ], - ) - default_subnet_id = client.describe_subnets()["Subnets"][0]["SubnetId"] + instances = client.run_instances(MinCount=1, MaxCount=1, SecurityGroups=["sg01"],) + # Assert subnet is created appropriately + subnets = client.describe_subnets()["Subnets"] + default_subnet_id = subnets[0]["SubnetId"] instances["Instances"][0]["NetworkInterfaces"][0]["SubnetId"].should.equal( default_subnet_id ) - - -@mock_ec2 -def test_describe_subnets_where_network_interface_has_no_subnets_attached(): - # https://github.com/spulec/moto/issues/2877 - # create security groups - ec2 = boto3.resource("ec2", region_name="us-west-1") - client = boto3.client("ec2", region_name="us-west-1") - ec2.create_security_group(GroupName="sg01", Description="Test security group sg01") - # run_instances - client.run_instances( - MinCount=1, - MaxCount=1, - SecurityGroups=["sg01"], - TagSpecifications=[ - { - "ResourceType": "instance", - "Tags": [{"Key": "Name", "Value": "test-01"},], - } - ], - ) - # describe_subnets - subnets = client.describe_subnets()["Subnets"] subnets[0]["AvailableIpAddressCount"].should.equal(4090) From 414fcf7bbd0ac261b83928f5eec9166ef4748aa3 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Wed, 8 Apr 2020 15:14:39 +0100 Subject: [PATCH 03/22] Fix AvailibilityZones in CF tests --- .../test_cloudformation_stack_integration.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_cloudformation/test_cloudformation_stack_integration.py b/tests/test_cloudformation/test_cloudformation_stack_integration.py index e50179660..67ef0af9b 100644 --- a/tests/test_cloudformation/test_cloudformation_stack_integration.py +++ b/tests/test_cloudformation/test_cloudformation_stack_integration.py @@ -495,7 +495,7 @@ def test_autoscaling_group_with_elb(): "my-as-group": { "Type": "AWS::AutoScaling::AutoScalingGroup", "Properties": { - "AvailabilityZones": ["us-east1"], + "AvailabilityZones": ["us-east-1a"], "LaunchConfigurationName": {"Ref": "my-launch-config"}, "MinSize": "2", "MaxSize": "2", @@ -522,7 +522,7 @@ def test_autoscaling_group_with_elb(): "my-elb": { "Type": "AWS::ElasticLoadBalancing::LoadBalancer", "Properties": { - "AvailabilityZones": ["us-east1"], + "AvailabilityZones": ["us-east-1a"], "Listeners": [ { "LoadBalancerPort": "80", @@ -545,10 +545,10 @@ def test_autoscaling_group_with_elb(): web_setup_template_json = json.dumps(web_setup_template) - conn = boto.cloudformation.connect_to_region("us-west-1") + conn = boto.cloudformation.connect_to_region("us-east-1") conn.create_stack("web_stack", template_body=web_setup_template_json) - autoscale_conn = boto.ec2.autoscale.connect_to_region("us-west-1") + autoscale_conn = boto.ec2.autoscale.connect_to_region("us-east-1") autoscale_group = autoscale_conn.get_all_groups()[0] autoscale_group.launch_config_name.should.contain("my-launch-config") autoscale_group.load_balancers[0].should.equal("my-elb") @@ -557,7 +557,7 @@ def test_autoscaling_group_with_elb(): autoscale_conn.get_all_launch_configurations().should.have.length_of(1) # Confirm the ELB was actually created - elb_conn = boto.ec2.elb.connect_to_region("us-west-1") + elb_conn = boto.ec2.elb.connect_to_region("us-east-1") elb_conn.get_all_load_balancers().should.have.length_of(1) stack = conn.describe_stacks()[0] @@ -584,7 +584,7 @@ def test_autoscaling_group_with_elb(): elb_resource.physical_resource_id.should.contain("my-elb") # confirm the instances were created with the right tags - ec2_conn = boto.ec2.connect_to_region("us-west-1") + ec2_conn = boto.ec2.connect_to_region("us-east-1") reservations = ec2_conn.get_all_reservations() len(reservations).should.equal(1) reservation = reservations[0] @@ -604,7 +604,7 @@ def test_autoscaling_group_update(): "my-as-group": { "Type": "AWS::AutoScaling::AutoScalingGroup", "Properties": { - "AvailabilityZones": ["us-west-1"], + "AvailabilityZones": ["us-west-1a"], "LaunchConfigurationName": {"Ref": "my-launch-config"}, "MinSize": "2", "MaxSize": "2", From 92bbc3fbacbbe65fc6d9e134d15e78c97d3e256c Mon Sep 17 00:00:00 2001 From: Tim Date: Thu, 16 Apr 2020 08:20:43 -0700 Subject: [PATCH 04/22] Adds initial support for secretsmanager update_secret The support in this patch is preliminary and may or may not be feature complete. It provides the basic support for update_secret so that future work can build on it as needed. --- moto/secretsmanager/models.py | 28 +++++++ moto/secretsmanager/responses.py | 10 +++ .../test_secretsmanager.py | 76 +++++++++++++++++++ 3 files changed, 114 insertions(+) diff --git a/moto/secretsmanager/models.py b/moto/secretsmanager/models.py index 294a6401e..11a024be6 100644 --- a/moto/secretsmanager/models.py +++ b/moto/secretsmanager/models.py @@ -107,6 +107,34 @@ class SecretsManagerBackend(BaseBackend): return response + def update_secret( + self, secret_id, secret_string=None, secret_binary=None, **kwargs + ): + + # error if secret does not exist + if secret_id not in self.secrets.keys(): + raise SecretNotFoundException() + + if "deleted_date" in self.secrets[secret_id]: + raise InvalidRequestException( + "An error occurred (InvalidRequestException) when calling the UpdateSecret operation: " + "You can't perform this operation on the secret because it was marked for deletion." + ) + + version_id = self._add_secret( + secret_id, secret_string=secret_string, secret_binary=secret_binary + ) + + response = json.dumps( + { + "ARN": secret_arn(self.region, secret_id), + "Name": secret_id, + "VersionId": version_id, + } + ) + + return response + def create_secret( self, name, secret_string=None, secret_binary=None, tags=[], **kwargs ): diff --git a/moto/secretsmanager/responses.py b/moto/secretsmanager/responses.py index 28af7b91d..757b888a3 100644 --- a/moto/secretsmanager/responses.py +++ b/moto/secretsmanager/responses.py @@ -29,6 +29,16 @@ class SecretsManagerResponse(BaseResponse): tags=tags, ) + def update_secret(self): + secret_id = self._get_param("SecretId") + secret_string = self._get_param("SecretString") + secret_binary = self._get_param("SecretBinary") + return secretsmanager_backends[self.region].update_secret( + secret_id=secret_id, + secret_string=secret_string, + secret_binary=secret_binary, + ) + def get_random_password(self): password_length = self._get_param("PasswordLength", if_none=32) exclude_characters = self._get_param("ExcludeCharacters", if_none="") diff --git a/tests/test_secretsmanager/test_secretsmanager.py b/tests/test_secretsmanager/test_secretsmanager.py index 3b8c74e81..49d1dc925 100644 --- a/tests/test_secretsmanager/test_secretsmanager.py +++ b/tests/test_secretsmanager/test_secretsmanager.py @@ -711,3 +711,79 @@ def test_can_list_secret_version_ids(): returned_version_ids = [v["VersionId"] for v in versions_list["Versions"]] assert [first_version_id, second_version_id].sort() == returned_version_ids.sort() + + +@mock_secretsmanager +def test_update_secret(): + conn = boto3.client("secretsmanager", region_name="us-west-2") + + created_secret = conn.create_secret(Name="test-secret", SecretString="foosecret") + + assert created_secret["ARN"] + assert created_secret["Name"] == "test-secret" + assert created_secret["VersionId"] != "" + + secret = conn.get_secret_value(SecretId="test-secret") + assert secret["SecretString"] == "foosecret" + + updated_secret = conn.update_secret( + SecretId="test-secret", SecretString="barsecret" + ) + + assert updated_secret["ARN"] + assert updated_secret["Name"] == "test-secret" + assert updated_secret["VersionId"] != "" + + secret = conn.get_secret_value(SecretId="test-secret") + assert secret["SecretString"] == "barsecret" + assert created_secret["VersionId"] != updated_secret["VersionId"] + + +@mock_secretsmanager +def test_update_secret_which_does_not_exit(): + conn = boto3.client("secretsmanager", region_name="us-west-2") + + with assert_raises(ClientError) as cm: + updated_secret = conn.update_secret( + SecretId="test-secret", SecretString="barsecret" + ) + + assert_equal( + "Secrets Manager can't find the specified secret.", + cm.exception.response["Error"]["Message"], + ) + + +@mock_secretsmanager +def test_update_secret_marked_as_deleted(): + conn = boto3.client("secretsmanager", region_name="us-west-2") + + created_secret = conn.create_secret(Name="test-secret", SecretString="foosecret") + deleted_secret = conn.delete_secret(SecretId="test-secret") + + with assert_raises(ClientError) as cm: + updated_secret = conn.update_secret( + SecretId="test-secret", SecretString="barsecret" + ) + + assert ( + "because it was marked for deletion." + in cm.exception.response["Error"]["Message"] + ) + + +@mock_secretsmanager +def test_update_secret_marked_as_deleted_after_restoring(): + conn = boto3.client("secretsmanager", region_name="us-west-2") + + created_secret = conn.create_secret(Name="test-secret", SecretString="foosecret") + deleted_secret = conn.delete_secret(SecretId="test-secret") + restored_secret = conn.restore_secret(SecretId="test-secret") + + updated_secret = conn.update_secret( + SecretId="test-secret", SecretString="barsecret" + ) + + assert updated_secret["ARN"] + assert updated_secret["Name"] == "test-secret" + assert updated_secret["VersionId"] != "" From 4dc46a697d21eed1c22edcb2f8ffafbaa9e5445a Mon Sep 17 00:00:00 2001 From: Hugo Lopes Tavares Date: Thu, 16 Apr 2020 15:14:37 -0400 Subject: [PATCH 05/22] Bugfix: Allow stop_db_instance for compatible engines From the RDS documentation: You can stop and start a DB instance whether it is configured for a single Availability Zone or for Multi-AZ, for database engines that support Multi-AZ deployments. You can't stop an Amazon RDS for SQL Server DB instance in a Multi-AZ configuration. https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_StopInstance.html#USER_StopInstance.Limitations --- moto/rds2/models.py | 5 ++++- tests/test_rds2/test_rds2.py | 31 +++++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/moto/rds2/models.py b/moto/rds2/models.py index 963af1c63..722d7d4fd 100644 --- a/moto/rds2/models.py +++ b/moto/rds2/models.py @@ -865,7 +865,10 @@ class RDS2Backend(BaseBackend): def stop_database(self, db_instance_identifier, db_snapshot_identifier=None): database = self.describe_databases(db_instance_identifier)[0] # todo: certain rds types not allowed to be stopped at this time. - if database.is_replica or database.multi_az: + # https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_StopInstance.html#USER_StopInstance.Limitations + if database.is_replica or ( + database.multi_az and database.engine.lower().startswith("sqlserver") + ): # todo: more db types not supported by stop/start instance api raise InvalidDBClusterStateFaultError(db_instance_identifier) if database.status != "available": diff --git a/tests/test_rds2/test_rds2.py b/tests/test_rds2/test_rds2.py index e93ff43e9..13e35549a 100644 --- a/tests/test_rds2/test_rds2.py +++ b/tests/test_rds2/test_rds2.py @@ -183,12 +183,12 @@ def test_start_database(): @mock_rds2 -def test_fail_to_stop_multi_az(): +def test_fail_to_stop_multi_az_and_sqlserver(): conn = boto3.client("rds", region_name="us-west-2") database = conn.create_db_instance( DBInstanceIdentifier="db-master-1", AllocatedStorage=10, - Engine="postgres", + Engine="sqlserver-ee", DBName="staging-postgres", DBInstanceClass="db.m1.small", LicenseModel="license-included", @@ -213,6 +213,33 @@ def test_fail_to_stop_multi_az(): ).should.throw(ClientError) +@mock_rds2 +def test_stop_multi_az_postgres(): + conn = boto3.client("rds", region_name="us-west-2") + database = conn.create_db_instance( + DBInstanceIdentifier="db-master-1", + AllocatedStorage=10, + Engine="postgres", + DBName="staging-postgres", + DBInstanceClass="db.m1.small", + LicenseModel="license-included", + MasterUsername="root", + MasterUserPassword="hunter2", + Port=1234, + DBSecurityGroups=["my_sg"], + MultiAZ=True, + ) + + mydb = conn.describe_db_instances( + DBInstanceIdentifier=database["DBInstance"]["DBInstanceIdentifier"] + )["DBInstances"][0] + mydb["DBInstanceStatus"].should.equal("available") + + response = conn.stop_db_instance(DBInstanceIdentifier=mydb["DBInstanceIdentifier"]) + response["ResponseMetadata"]["HTTPStatusCode"].should.equal(200) + response["DBInstance"]["DBInstanceStatus"].should.equal("stopped") + + @mock_rds2 def test_fail_to_stop_readreplica(): conn = boto3.client("rds", region_name="us-west-2") From 76a249c0ecbc616588b5ccf26224ad9efa9a05c9 Mon Sep 17 00:00:00 2001 From: Andrey Kislyuk Date: Thu, 16 Apr 2020 21:28:27 -0700 Subject: [PATCH 06/22] awslambda: Do not assume X-Amz-Invocation-Type is set --- moto/awslambda/responses.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/moto/awslambda/responses.py b/moto/awslambda/responses.py index ce6c93f16..28b0e74fd 100644 --- a/moto/awslambda/responses.py +++ b/moto/awslambda/responses.py @@ -184,9 +184,9 @@ class LambdaResponse(BaseResponse): function_name, qualifier, self.body, self.headers, response_headers ) if payload: - if request.headers["X-Amz-Invocation-Type"] == "Event": + if request.headers.get("X-Amz-Invocation-Type") == "Event": status_code = 202 - elif request.headers["X-Amz-Invocation-Type"] == "DryRun": + elif request.headers.get("X-Amz-Invocation-Type") == "DryRun": status_code = 204 else: status_code = 200 From 7ea419dd54f475660f4e927ad39d62d226a43513 Mon Sep 17 00:00:00 2001 From: pvbouwel Date: Sat, 11 Apr 2020 11:07:22 +0100 Subject: [PATCH 07/22] Better DDB expressions support1: TokenizationDDB Currently the mock for DynamoDB has adhoc code to implement its updateExpression functionality. This series will transform the logic such that Update Expressions are processed as follows: 1) Expression gets parsed into a tokenlist (tokenized) -> This commit 2) Tokenlist get transformed to expression tree (AST) 3) The AST gets validated (full semantic correctness) 4) AST gets processed to perform the update This alows for a more realistic mocking. It will throw exceptions much more aggressively avoiding situations where a test passes against the mock but fails with an exception when running against AWS. Introduction of step 3 also allows to have the update expression as an atomic unit of work. So updates at the start of the expression cannot be performed if there is an error further down the expression. This specific commit will tokenize expressions but the tokenlist is not yet used. It is purely to keep clear boundaries. It does do a minor refactoring of the exceptions to allow more re-use and to ease testing. This series of changes is to aid providing a long-term solution for https://github.com/spulec/moto/issues/2806. --- moto/dynamodb2/exceptions.py | 58 +++- moto/dynamodb2/parsing/__init__.py | 0 moto/dynamodb2/parsing/tokens.py | 210 ++++++++++++++ moto/dynamodb2/responses.py | 14 +- .../test_dynamodb_expression_tokenizer.py | 259 ++++++++++++++++++ 5 files changed, 527 insertions(+), 14 deletions(-) create mode 100644 moto/dynamodb2/parsing/__init__.py create mode 100644 moto/dynamodb2/parsing/tokens.py create mode 100644 tests/test_dynamodb2/test_dynamodb_expression_tokenizer.py diff --git a/moto/dynamodb2/exceptions.py b/moto/dynamodb2/exceptions.py index 1f3b5f974..4c5dfd447 100644 --- a/moto/dynamodb2/exceptions.py +++ b/moto/dynamodb2/exceptions.py @@ -2,9 +2,59 @@ class InvalidIndexNameError(ValueError): pass -class InvalidUpdateExpression(ValueError): - pass +class MockValidationException(ValueError): + def __init__(self, message): + self.exception_msg = message -class ItemSizeTooLarge(Exception): - message = "Item size has exceeded the maximum allowed size" +class InvalidUpdateExpression(MockValidationException): + invalid_update_expression_msg = ( + "The document path provided in the update expression is invalid for update" + ) + + def __init__(self): + super(InvalidUpdateExpression, self).__init__( + self.invalid_update_expression_msg + ) + + +class UpdateExprSyntaxError(MockValidationException): + update_expr_syntax_error_msg = ( + "Invalid UpdateExpression: Syntax error; {error_detail}" + ) + + def __init__(self, error_detail): + self.error_detail = error_detail + super(UpdateExprSyntaxError, self).__init__( + self.update_expr_syntax_error_msg.format(error_detail=error_detail) + ) + + +class InvalidTokenException(UpdateExprSyntaxError): + token_detail_msg = 'token: "{token}", near: "{near}"' + + def __init__(self, token, near): + self.token = token + self.near = near + super(InvalidTokenException, self).__init__( + self.token_detail_msg.format(token=token, near=near) + ) + + +class InvalidExpressionAttributeNameKey(MockValidationException): + invalid_expr_attr_name_msg = ( + 'ExpressionAttributeNames contains invalid key: Syntax error; key: "{key}"' + ) + + def __init__(self, key): + self.key = key + super(InvalidExpressionAttributeNameKey, self).__init__( + self.invalid_expr_attr_name_msg.format(key=key) + ) + + +class ItemSizeTooLarge(MockValidationException): + item_size_too_large_msg = "Item size has exceeded the maximum allowed size" + + def __init__(self): + super(ItemSizeTooLarge, self).__init__(self.item_size_too_large_msg) diff --git a/moto/dynamodb2/parsing/__init__.py b/moto/dynamodb2/parsing/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/moto/dynamodb2/parsing/tokens.py b/moto/dynamodb2/parsing/tokens.py new file mode 100644 index 000000000..07d65ae64 --- /dev/null +++ b/moto/dynamodb2/parsing/tokens.py @@ -0,0 +1,210 @@ +import re + +from moto.dynamodb2.exceptions import ( + InvalidTokenException, + InvalidExpressionAttributeNameKey, +) + + +class Token(object): + _TOKEN_INSTANCE = None + MINUS_SIGN = "-" + PLUS_SIGN = "+" + SPACE_SIGN = " " + EQUAL_SIGN = "=" + OPEN_ROUND_BRACKET = "(" + CLOSE_ROUND_BRACKET = ")" + COMMA = "," + SPACE = " " + DOT = "." + OPEN_SQUARE_BRACKET = "[" + CLOSE_SQUARE_BRACKET = "]" + + SPECIAL_CHARACTERS = [ + MINUS_SIGN, + PLUS_SIGN, + SPACE_SIGN, + EQUAL_SIGN, + OPEN_ROUND_BRACKET, + CLOSE_ROUND_BRACKET, + COMMA, + SPACE, + DOT, + OPEN_SQUARE_BRACKET, + CLOSE_SQUARE_BRACKET, + ] + + # Attribute: an identifier that is an attribute + ATTRIBUTE = 0 + # Place holder for attribute name + ATTRIBUTE_NAME = 1 + # Placeholder for attribute value starts with : + ATTRIBUTE_VALUE = 2 + # WhiteSpace shall be grouped together + WHITESPACE = 3 + # Placeholder for a number + NUMBER = 4 + + PLACEHOLDER_NAMES = { + ATTRIBUTE: "Attribute", + ATTRIBUTE_NAME: "AttributeName", + ATTRIBUTE_VALUE: "AttributeValue", + WHITESPACE: "Whitespace", + NUMBER: "Number", + } + + def __init__(self, token_type, value): + assert ( + token_type in self.SPECIAL_CHARACTERS + or token_type in self.PLACEHOLDER_NAMES + ) + self.type = token_type + self.value = value + + def __repr__(self): + if isinstance(self.type, int): + return 'Token("{tt}", "{tv}")'.format( + tt=self.PLACEHOLDER_NAMES[self.type], tv=self.value + ) + else: + return 'Token("{tt}", "{tv}")'.format(tt=self.type, tv=self.value) + + def __eq__(self, other): + return self.type == other.type and self.value == other.value + + +class ExpressionTokenizer(object): + """ + Takes a string and returns a list of tokens. While attribute names in DynamoDB must be between 1 and 255 characters + long there are no other restrictions for attribute names. For expressions however there are additional rules. If an + attribute name does not adhere then it must be passed via an ExpressionAttributeName. This tokenizer is aware of the + rules of Expression attributes. + + We consider a Token as a tuple which has the tokenType + + From https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.ExpressionAttributeNames.html + 1) If an attribute name begins with a number or contains a space, a special character, or a reserved word, you + must use an expression attribute name to replace that attribute's name in the expression. + => So spaces,+,- or other special characters do identify tokens in update expressions + + 2) When using a dot (.) in an attribute name you must use expression-attribute-names. A dot in an expression + will be interpreted as a separator in a document path + + 3) For a nested structure if you want to use expression_attribute_names you must specify one per part of the + path. Since for members of expression_attribute_names the . is part of the name + + """ + + @classmethod + def is_simple_token_character(cls, character): + return character.isalnum() or character in ("_", ":", "#") + + @classmethod + def is_possible_token_boundary(cls, character): + return ( + character in Token.SPECIAL_CHARACTERS + or not cls.is_simple_token_character(character) + ) + + @classmethod + def is_expression_attribute(cls, input_string): + return re.compile("^[a-zA-Z][a-zA-Z0-9_]*$").match(input_string) is not None + + @classmethod + def is_expression_attribute_name(cls, input_string): + """ + https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.ExpressionAttributeNames.html + An expression attribute name must begin with a pound sign (#), and be followed by one or more alphanumeric + characters. + """ + return input_string.startswith("#") and cls.is_expression_attribute( + input_string[1:] + ) + + @classmethod + def is_expression_attribute_value(cls, input_string): + return re.compile("^:[a-zA-Z0-9_]*$").match(input_string) is not None + + def raise_unexpected_token(self): + """If during parsing an unexpected token is encountered""" + if len(self.token_list) == 0: + near = "" + else: + if len(self.token_list) == 1: + near = self.token_list[-1].value + else: + if self.token_list[-1].type == Token.WHITESPACE: + # Last token was whitespace take 2nd last token value as well to help User orientate + near = self.token_list[-2].value + self.token_list[-1].value + else: + near = self.token_list[-1].value + + problematic_token = self.staged_characters[0] + raise InvalidTokenException(problematic_token, near + self.staged_characters) + + def __init__(self, input_expression_str): + self.input_expression_str = input_expression_str + self.token_list = [] + self.staged_characters = "" + + @classmethod + def make_list(cls, input_expression_str): + assert isinstance(input_expression_str, str) + return ExpressionTokenizer(input_expression_str)._make_list() + + def add_token(self, token_type, token_value): + self.token_list.append(Token(token_type, token_value)) + + def add_token_from_stage(self, token_type): + self.add_token(token_type, self.staged_characters) + self.staged_characters = "" + + def process_staged_characters(self): + if len(self.staged_characters) == 0: + return + if self.staged_characters.startswith("#"): + if self.is_expression_attribute_name(self.staged_characters): + self.add_token_from_stage(Token.ATTRIBUTE_NAME) + else: + raise InvalidExpressionAttributeNameKey(self.staged_characters) + elif self.staged_characters.isnumeric(): + self.add_token_from_stage(Token.NUMBER) + elif self.is_expression_attribute(self.staged_characters): + self.add_token_from_stage(Token.ATTRIBUTE) + elif self.is_expression_attribute_value(self.staged_characters): + self.add_token_from_stage(Token.ATTRIBUTE_VALUE) + else: + self.raise_unexpected_token() + + def _make_list(self): + """ + Just go through characters if a character is not a token boundary stage it for adding it as a grouped token + later if it is a tokenboundary process staged characters and then process the token boundary as well. + """ + for character in self.input_expression_str: + if not self.is_possible_token_boundary(character): + self.staged_characters += character + else: + self.process_staged_characters() + + if character == Token.SPACE: + if ( + len(self.token_list) > 0 + and self.token_list[-1].type == Token.WHITESPACE + ): + self.token_list[-1].value = ( + self.token_list[-1].value + character + ) + else: + self.add_token(Token.WHITESPACE, character) + elif character in Token.SPECIAL_CHARACTERS: + self.add_token(character, character) + elif not self.is_simple_token_character(character): + self.staged_characters += character + self.raise_unexpected_token() + else: + raise NotImplementedError( + "Encountered character which was not implemented : " + character + ) + self.process_staged_characters() + return self.token_list diff --git a/moto/dynamodb2/responses.py b/moto/dynamodb2/responses.py index 65484aa08..d21d1d756 100644 --- a/moto/dynamodb2/responses.py +++ b/moto/dynamodb2/responses.py @@ -9,7 +9,7 @@ import six from moto.core.responses import BaseResponse from moto.core.utils import camelcase_to_underscores, amzn_request_id -from .exceptions import InvalidIndexNameError, InvalidUpdateExpression, ItemSizeTooLarge +from .exceptions import InvalidIndexNameError, InvalidUpdateExpression, ItemSizeTooLarge, MockValidationException from moto.dynamodb2.models import dynamodb_backends, dynamo_json_dump @@ -298,7 +298,7 @@ class DynamoHandler(BaseResponse): ) except ItemSizeTooLarge: er = "com.amazonaws.dynamodb.v20111205#ValidationException" - return self.error(er, ItemSizeTooLarge.message) + return self.error(er, ItemSizeTooLarge.item_size_too_large_msg) except KeyError as ke: er = "com.amazonaws.dynamodb.v20111205#ValidationException" return self.error(er, ke.args[0]) @@ -764,15 +764,9 @@ class DynamoHandler(BaseResponse): expected, condition_expression, ) - except InvalidUpdateExpression: + except MockValidationException as mve: er = "com.amazonaws.dynamodb.v20111205#ValidationException" - return self.error( - er, - "The document path provided in the update expression is invalid for update", - ) - except ItemSizeTooLarge: - er = "com.amazonaws.dynamodb.v20111205#ValidationException" - return self.error(er, ItemSizeTooLarge.message) + return self.error(er, mve.exception_msg) except ValueError: er = "com.amazonaws.dynamodb.v20111205#ConditionalCheckFailedException" return self.error( diff --git a/tests/test_dynamodb2/test_dynamodb_expression_tokenizer.py b/tests/test_dynamodb2/test_dynamodb_expression_tokenizer.py new file mode 100644 index 000000000..3330d431e --- /dev/null +++ b/tests/test_dynamodb2/test_dynamodb_expression_tokenizer.py @@ -0,0 +1,259 @@ +from moto.dynamodb2.exceptions import ( + InvalidTokenException, + InvalidExpressionAttributeNameKey, +) +from moto.dynamodb2.parsing.tokens import ExpressionTokenizer, Token + + +def test_expression_tokenizer_single_set_action(): + set_action = "SET attrName = :attrValue" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "attrName"), + Token(Token.WHITESPACE, " "), + Token(Token.EQUAL_SIGN, "="), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_VALUE, ":attrValue"), + ] + + +def test_expression_tokenizer_single_set_action_leading_space(): + set_action = "Set attrName = :attrValue" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "Set"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "attrName"), + Token(Token.WHITESPACE, " "), + Token(Token.EQUAL_SIGN, "="), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_VALUE, ":attrValue"), + ] + + +def test_expression_tokenizer_single_set_action_attribute_name_leading_space(): + set_action = "SET #a = :attrValue" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_NAME, "#a"), + Token(Token.WHITESPACE, " "), + Token(Token.EQUAL_SIGN, "="), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_VALUE, ":attrValue"), + ] + + +def test_expression_tokenizer_single_set_action_trailing_space(): + set_action = "SET attrName = :attrValue " + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "attrName"), + Token(Token.WHITESPACE, " "), + Token(Token.EQUAL_SIGN, "="), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_VALUE, ":attrValue"), + Token(Token.WHITESPACE, " "), + ] + + +def test_expression_tokenizer_single_set_action_multi_spaces(): + set_action = "SET attrName = :attrValue " + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "attrName"), + Token(Token.WHITESPACE, " "), + Token(Token.EQUAL_SIGN, "="), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_VALUE, ":attrValue"), + Token(Token.WHITESPACE, " "), + ] + + +def test_expression_tokenizer_single_set_action_with_numbers_in_identifiers(): + set_action = "SET attrName3 = :attr3Value" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "attrName3"), + Token(Token.WHITESPACE, " "), + Token(Token.EQUAL_SIGN, "="), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_VALUE, ":attr3Value"), + ] + + +def test_expression_tokenizer_single_set_action_with_underscore_in_identifier(): + set_action = "SET attr_Name = :attr_Value" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "attr_Name"), + Token(Token.WHITESPACE, " "), + Token(Token.EQUAL_SIGN, "="), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_VALUE, ":attr_Value"), + ] + + +def test_expression_tokenizer_leading_underscore_in_attribute_name_expression(): + """Leading underscore is not allowed for an attribute name""" + set_action = "SET attrName = _idid" + try: + ExpressionTokenizer.make_list(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "_" + assert te.near == "= _idid" + + +def test_expression_tokenizer_leading_underscore_in_attribute_value_expression(): + """Leading underscore is allowed in an attribute value""" + set_action = "SET attrName = :_attrValue" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "attrName"), + Token(Token.WHITESPACE, " "), + Token(Token.EQUAL_SIGN, "="), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_VALUE, ":_attrValue"), + ] + + +def test_expression_tokenizer_single_set_action_nested_attribute(): + set_action = "SET attrName.elem = :attrValue" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "attrName"), + Token(Token.DOT, "."), + Token(Token.ATTRIBUTE, "elem"), + Token(Token.WHITESPACE, " "), + Token(Token.EQUAL_SIGN, "="), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE_VALUE, ":attrValue"), + ] + + +def test_expression_tokenizer_list_index_with_sub_attribute(): + set_action = "SET itemmap.itemlist[1].foos=:Item" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "itemmap"), + Token(Token.DOT, "."), + Token(Token.ATTRIBUTE, "itemlist"), + Token(Token.OPEN_SQUARE_BRACKET, "["), + Token(Token.NUMBER, "1"), + Token(Token.CLOSE_SQUARE_BRACKET, "]"), + Token(Token.DOT, "."), + Token(Token.ATTRIBUTE, "foos"), + Token(Token.EQUAL_SIGN, "="), + Token(Token.ATTRIBUTE_VALUE, ":Item"), + ] + + +def test_expression_tokenizer_list_index_surrounded_with_whitespace(): + set_action = "SET itemlist[ 1 ]=:Item" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "itemlist"), + Token(Token.OPEN_SQUARE_BRACKET, "["), + Token(Token.WHITESPACE, " "), + Token(Token.NUMBER, "1"), + Token(Token.WHITESPACE, " "), + Token(Token.CLOSE_SQUARE_BRACKET, "]"), + Token(Token.EQUAL_SIGN, "="), + Token(Token.ATTRIBUTE_VALUE, ":Item"), + ] + + +def test_expression_tokenizer_single_set_action_attribute_name_invalid_key(): + """ + ExpressionAttributeNames contains invalid key: Syntax error; key: "#va#l2" + """ + set_action = "SET #va#l2 = 3" + try: + ExpressionTokenizer.make_list(set_action) + assert False, "Exception not raised correctly" + except InvalidExpressionAttributeNameKey as e: + assert e.key == "#va#l2" + + +def test_expression_tokenizer_single_set_action_attribute_name_invalid_key_double_hash(): + """ + ExpressionAttributeNames contains invalid key: Syntax error; key: "#va#l" + """ + set_action = "SET #va#l = 3" + try: + ExpressionTokenizer.make_list(set_action) + assert False, "Exception not raised correctly" + except InvalidExpressionAttributeNameKey as e: + assert e.key == "#va#l" + + +def test_expression_tokenizer_single_set_action_attribute_name_valid_key(): + set_action = "SET attr=#val2" + token_list = ExpressionTokenizer.make_list(set_action) + assert token_list == [ + Token(Token.ATTRIBUTE, "SET"), + Token(Token.WHITESPACE, " "), + Token(Token.ATTRIBUTE, "attr"), + Token(Token.EQUAL_SIGN, "="), + Token(Token.ATTRIBUTE_NAME, "#val2"), + ] + + +def test_expression_tokenizer_just_a_pipe(): + set_action = "|" + try: + ExpressionTokenizer.make_list(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "|" + assert te.near == "|" + + +def test_expression_tokenizer_just_a_pipe_with_leading_white_spaces(): + set_action = " |" + try: + ExpressionTokenizer.make_list(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "|" + assert te.near == " |" + + +def test_expression_tokenizer_just_a_pipe_for_set_expression(): + set_action = "SET|" + try: + ExpressionTokenizer.make_list(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "|" + assert te.near == "SET|" + + +def test_expression_tokenizer_just_an_attribute_and_a_pipe_for_set_expression(): + set_action = "SET a|" + try: + ExpressionTokenizer.make_list(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "|" + assert te.near == "a|" From 9ed613e197e1d6e85f9631f7a15a2b8ce2f19b2e Mon Sep 17 00:00:00 2001 From: pvbouwel Date: Sat, 11 Apr 2020 21:17:16 +0100 Subject: [PATCH 08/22] Better DDB expressions support2: ExpressionTree Part of structured approach for UpdateExpressions: 1) Expression gets parsed into a tokenlist (tokenized) 2) Tokenlist get transformed to expression tree (AST) -> This commit 3) The AST gets validated (full semantic correctness) 4) AST gets processed to perform the update This commit uses the tokenlist to build an expression tree. This tree is not yet used. Still it allows to raise additional Validation Exceptions which previously were missed silently therefore it allows tests to catch these type of ValidationException. For that reason DDB UpdateExpressions will be parsed already. It also makes sure we won't break existing tests. One of the existing tests had to be changed in order to still pass: - test_dynamodb_table_with_range_key.test_update_item_with_expression This test passed in a numeric literal which is not supported by DynamoDB and with the current tokenization it would get the same error as in AWS DynamoDB. --- moto/dynamodb2/models/__init__.py | 12 +- moto/dynamodb2/parsing/ast_nodes.py | 205 ++++ moto/dynamodb2/parsing/expressions.py | 1010 +++++++++++++++++ moto/dynamodb2/parsing/reserved_keywords.py | 29 + moto/dynamodb2/parsing/reserved_keywords.txt | 573 ++++++++++ moto/dynamodb2/parsing/tokens.py | 17 +- moto/dynamodb2/responses.py | 5 - tests/test_dynamodb2/test_dynamodb.py | 68 ++ .../test_dynamodb_expressions.py | 395 +++++++ .../test_dynamodb_table_with_range_key.py | 16 +- 10 files changed, 2317 insertions(+), 13 deletions(-) create mode 100644 moto/dynamodb2/parsing/ast_nodes.py create mode 100644 moto/dynamodb2/parsing/expressions.py create mode 100644 moto/dynamodb2/parsing/reserved_keywords.py create mode 100644 moto/dynamodb2/parsing/reserved_keywords.txt create mode 100644 tests/test_dynamodb2/test_dynamodb_expressions.py diff --git a/moto/dynamodb2/models/__init__.py b/moto/dynamodb2/models/__init__.py index 29713d211..1f448f288 100644 --- a/moto/dynamodb2/models/__init__.py +++ b/moto/dynamodb2/models/__init__.py @@ -14,10 +14,11 @@ from moto.core import BaseBackend, BaseModel from moto.core.utils import unix_time from moto.core.exceptions import JsonRESTError from moto.dynamodb2.comparisons import get_filter_expression -from moto.dynamodb2.comparisons import get_expected -from moto.dynamodb2.exceptions import InvalidIndexNameError, ItemSizeTooLarge +from moto.dynamodb2.comparisons import get_expected, get_comparison_func +from moto.dynamodb2.exceptions import InvalidIndexNameError, ItemSizeTooLarge, InvalidUpdateExpression from moto.dynamodb2.models.utilities import bytesize, attribute_is_list from moto.dynamodb2.models.dynamo_type import DynamoType +from moto.dynamodb2.parsing.expressions import UpdateExpressionParser class DynamoJsonEncoder(json.JSONEncoder): @@ -1197,6 +1198,13 @@ class DynamoDBBackend(BaseBackend): ): table = self.get_table(table_name) + # Support spaces between operators in an update expression + # E.g. `a = b + c` -> `a=b+c` + if update_expression: + # Parse expression to get validation errors + UpdateExpressionParser.make(update_expression) + update_expression = re.sub(r"\s*([=\+-])\s*", "\\1", update_expression) + if all([table.hash_key_attr in key, table.range_key_attr in key]): # Covers cases where table has hash and range keys, ``key`` param # will be a dict diff --git a/moto/dynamodb2/parsing/ast_nodes.py b/moto/dynamodb2/parsing/ast_nodes.py new file mode 100644 index 000000000..78c7b6b2b --- /dev/null +++ b/moto/dynamodb2/parsing/ast_nodes.py @@ -0,0 +1,205 @@ +import abc +import six + + +@six.add_metaclass(abc.ABCMeta) +class Node: + def __init__(self, children=None): + self.type = self.__class__.__name__ + assert children is None or isinstance(children, list) + self.children = children + self.parent = None + + if isinstance(children, list): + for child in children: + if isinstance(child, Node): + child.set_parent(self) + + def set_parent(self, parent_node): + self.parent = parent_node + + +class LeafNode(Node): + """A LeafNode is a Node where none of the children are Nodes themselves.""" + + def __init__(self, children=None): + super(LeafNode, self).__init__(children) + + +@six.add_metaclass(abc.ABCMeta) +class Expression(Node): + """ + Abstract Syntax Tree representing the expression + + For the Grammar start here and jump down into the classes at the righ-hand side to look further. Nodes marked with + a star are abstract and won't appear in the final AST. + + Expression* => UpdateExpression + Expression* => ConditionExpression + """ + + +class UpdateExpression(Expression): + """ + UpdateExpression => UpdateExpressionClause* + UpdateExpression => UpdateExpressionClause* UpdateExpression + """ + + +@six.add_metaclass(abc.ABCMeta) +class UpdateExpressionClause(UpdateExpression): + """ + UpdateExpressionClause* => UpdateExpressionSetClause + UpdateExpressionClause* => UpdateExpressionRemoveClause + UpdateExpressionClause* => UpdateExpressionAddClause + UpdateExpressionClause* => UpdateExpressionDeleteClause + """ + + +class UpdateExpressionSetClause(UpdateExpressionClause): + """ + UpdateExpressionSetClause => SET SetActions + """ + + +class UpdateExpressionSetActions(UpdateExpressionClause): + """ + UpdateExpressionSetClause => SET SetActions + + SetActions => SetAction + SetActions => SetAction , SetActions + + """ + + +class UpdateExpressionSetAction(UpdateExpressionClause): + """ + SetAction => Path = Value + """ + + +class UpdateExpressionRemoveActions(UpdateExpressionClause): + """ + UpdateExpressionSetClause => REMOVE RemoveActions + + RemoveActions => RemoveAction + RemoveActions => RemoveAction , RemoveActions + """ + + +class UpdateExpressionRemoveAction(UpdateExpressionClause): + """ + RemoveAction => Path + """ + + +class UpdateExpressionAddActions(UpdateExpressionClause): + """ + UpdateExpressionAddClause => ADD RemoveActions + + AddActions => AddAction + AddActions => AddAction , AddActions + """ + + +class UpdateExpressionAddAction(UpdateExpressionClause): + """ + AddAction => Path Value + """ + + +class UpdateExpressionDeleteActions(UpdateExpressionClause): + """ + UpdateExpressionDeleteClause => DELETE RemoveActions + + DeleteActions => DeleteAction + DeleteActions => DeleteAction , DeleteActions + """ + + +class UpdateExpressionDeleteAction(UpdateExpressionClause): + """ + DeleteAction => Path Value + """ + + +class UpdateExpressionPath(UpdateExpressionClause): + pass + + +class UpdateExpressionValue(UpdateExpressionClause): + """ + Value => Operand + Value => Operand + Value + Value => Operand - Value + """ + + +class UpdateExpressionGroupedValue(UpdateExpressionClause): + """ + GroupedValue => ( Value ) + """ + + +class UpdateExpressionRemoveClause(UpdateExpressionClause): + """ + UpdateExpressionRemoveClause => REMOVE RemoveActions + """ + + +class UpdateExpressionAddClause(UpdateExpressionClause): + """ + UpdateExpressionAddClause => ADD AddActions + """ + + +class UpdateExpressionDeleteClause(UpdateExpressionClause): + """ + UpdateExpressionDeleteClause => DELETE DeleteActions + """ + + +class ExpressionPathDescender(Node): + """Node identifying descender into nested structure (.) in expression""" + + +class ExpressionSelector(LeafNode): + """Node identifying selector [selection_index] in expresion""" + + def __init__(self, selection_index): + super(ExpressionSelector, self).__init__(children=[selection_index]) + + +class ExpressionAttribute(LeafNode): + """An attribute identifier as used in the DDB item""" + + def __init__(self, attribute): + super(ExpressionAttribute, self).__init__(children=[attribute]) + + +class ExpressionAttributeName(LeafNode): + """An ExpressionAttributeName is an alias for an attribute identifier""" + + def __init__(self, attribute_name): + super(ExpressionAttributeName, self).__init__(children=[attribute_name]) + + +class ExpressionAttributeValue(LeafNode): + """An ExpressionAttributeValue is an alias for an value""" + + def __init__(self, value): + super(ExpressionAttributeValue, self).__init__(children=[value]) + + +class ExpressionValueOperator(LeafNode): + """An ExpressionValueOperator is an operation that works on 2 values""" + + def __init__(self, value): + super(ExpressionValueOperator, self).__init__(children=[value]) + + +class UpdateExpressionFunction(Node): + """ + A Node representing a function of an Update Expression. The first child is the function name the others are the + arguments. + """ diff --git a/moto/dynamodb2/parsing/expressions.py b/moto/dynamodb2/parsing/expressions.py new file mode 100644 index 000000000..e418bb47e --- /dev/null +++ b/moto/dynamodb2/parsing/expressions.py @@ -0,0 +1,1010 @@ +import logging +from abc import abstractmethod +import abc +import six +from collections import deque + +from moto.dynamodb2.parsing.ast_nodes import ( + UpdateExpression, + UpdateExpressionSetClause, + UpdateExpressionSetActions, + UpdateExpressionSetAction, + UpdateExpressionRemoveActions, + UpdateExpressionRemoveAction, + UpdateExpressionPath, + UpdateExpressionValue, + UpdateExpressionGroupedValue, + UpdateExpressionRemoveClause, + ExpressionPathDescender, + ExpressionSelector, + ExpressionAttribute, + ExpressionAttributeName, + ExpressionAttributeValue, + ExpressionValueOperator, + UpdateExpressionFunction, + UpdateExpressionAddClause, + UpdateExpressionAddActions, + UpdateExpressionAddAction, + UpdateExpressionDeleteAction, + UpdateExpressionDeleteActions, + UpdateExpressionDeleteClause, +) +from moto.dynamodb2.exceptions import InvalidTokenException +from moto.dynamodb2.parsing.tokens import Token, ExpressionTokenizer + + +class NestableExpressionParserMixin(object): + """ + For nodes that can be nested in themselves (recursive). Take for example UpdateExpression's grammar: + + UpdateExpression => UpdateExpressionClause* + UpdateExpression => UpdateExpressionClause* UpdateExpression + + If we consider it of structure + NestableExpression => TargetClause* + NestableExpression => TargetClause* NestableExpression + + This pattern comes back multiple times. This Mixin adds re-usability for that type of pattern. + + This approach is taken since it allows to remain the ordering of the Nodes as how the corresponding tokens where + in the originating expression. + """ + + def __init__(self, *args, **kwargs): + self.target_clauses = deque() + + def _parse_target_clause(self, factory_class): + """ + + Args: + factory_class: The factory for the target clause e.g. UpdateExpressionSetClauseParser + + Returns: + + """ + logging.debug( + "Move token pos {pos} to continue parsing with specific factory class {fc}".format( + pos=self.token_pos, fc=factory_class.__class__.__name__ + ) + ) + # noinspection PyProtectedMember + ast, token_pos = factory_class(**self._initializer_args())._parse_with_pos() + self.target_clauses.append(ast) + logging.debug( + "Continue where previous parsing ended {token_pos}".format( + token_pos=token_pos + ) + ) + self.token_pos = token_pos + + @abstractmethod + def _initializer_args(self): + """ + Get the arguments of the initializer. This is implemented by the calling class. See ExpressionParser for an + example. + + Returns: + dict: A dictionary of the initializer arguments + """ + + @classmethod + @abstractmethod + def _nestable_class(cls): + """ + Get the class of the Node that will be created that would be nested. For the example in the docstring this would + be UpdateExpression + + Returns: + class: The class of the Nodes that will be created. + """ + + def _create_node(self): + """ + target_clauses has the nodes in order of encountering. Go through them backwards and build the tree bottom up. + + This way left-deep-descending traversal will process nodes in order. + + Continuing the example of an UpdateExpression: + For example SET a=3 REMOVE b + UpdateExpression + / \ + SET a=3 UpdateExpression + | + REMOVE b + + self.target_clauses looks like: ( SET a=3 >> REMOVE b ) + Returns: + moto.dynamodb2.ast_nodes.Node: Node of an AST representing the Expression as produced by the factory. + """ + assert len(self.target_clauses) > 0, "No nodes for {cn}".format( + cn=self.__class__.__name__ + ) + target_node = self._nestable_class()(children=[self.target_clauses.pop()]) + while len(self.target_clauses) > 0: + target_node = self._nestable_class()( + children=[self.target_clauses.pop(), target_node] + ) + return target_node + + +@six.add_metaclass(abc.ABCMeta) +class ExpressionParser: + """Abstract class""" + + def __init__(self, expression_token_list, token_pos=0): + """ + + Args: + expression_token_list: + token_pos(int): Location where parsing is + """ + self.token_list = expression_token_list + self.token_pos = token_pos + + def _initializer_args(self): + return {"expression_token_list": self.token_list, "token_pos": self.token_pos} + + @abstractmethod + def _parse(self): + """ + Start parsing the token_list from token_pos for the factory type. + + Returns: + moto.dynamodb2.ast_nodes.Node: AST which is root node of resulting abstract syntax tree + """ + + @classmethod + def is_possible_start(cls, token): + return token is not None and cls._is_possible_start(token) + + @classmethod + @abstractmethod + def _is_possible_start(cls, token): + """ + + Args: + token(moto.dynamodb2.tokens.Token): + + Returns: + bool: True if token is a possible start for entries processed by `cls` + """ + + def _parse_with_pos(self): + """ + Start parsing the token_list from token_pos for the factory type and also return the resulting token_pos. + + Returns: + (ast, token_pos): tuple of AST which is root node of resulting abstract syntax tree and token_pos is the + position in the tokenlist. + """ + return self._parse(), self.token_pos + + def parse(self): + return self._parse() + + def get_next_token_type(self): + """ + Get the type of the next token to be processed + + Returns: + str: Token type or None if no more next token + """ + try: + return self.get_next_token().type + except AttributeError: + return None + + def get_next_token(self): + """ + Get the next token to be processed + + Returns: + moto.dynamodb2.tokens.Token: or None if no more next token + """ + try: + return self.token_list[self.token_pos] + except IndexError: + return None + + def get_next_token_value(self): + """ + Get the value of the next token to be processed + + Returns: + str: value or None if no more next token + """ + try: + return self.get_next_token().value + except AttributeError: + return None + + def is_at_end(self): + """Return boolean indicating whether we are at end of the parsing""" + return self.token_pos == len(self.token_list) + + def is_at_start(self): + """Return boolean indicating whether we are at start of the parsing""" + return self.token_pos == 0 + + def get_last_token_value(self): + """Get the last token that was correctly parsed or return empty string""" + if self.token_pos > 0: + return self.token_list[self.token_pos - 1].value + else: + return "" + + def get_last_token_type(self): + """Get the last token type that was correctly parsed or return None""" + if self.token_pos > 0: + return self.token_list[self.token_pos - 1].type + else: + return None + + def get_2nd_last_token_value_if_last_was_whitespace(self): + """Get the 2nd last token that was correctly parsed if last one was whitespace or return empty string""" + if self.token_pos > 1 and self.get_last_token_type() == Token.WHITESPACE: + return self.token_list[self.token_pos - 2].value + else: + return "" + + def get_following_token_value(self): + """Get the token value after the one that is being parsed or empty string if non existent.""" + try: + return self.token_list[self.token_pos + 1].value + except IndexError: + return "" + + def get_following_token_type(self): + """Get the token type after the one that is being parsed or None if non existent.""" + try: + return self.token_list[self.token_pos + 1].type + except IndexError: + return None + + def get_2nd_following_token_value_if_following_was_whitespace(self): + """Get the 2nd following token that was correctly parsed if 1st one was whitespace or return empty string""" + if self.get_following_token_type() == Token.WHITESPACE: + try: + return self.token_list[self.token_pos + 2].value + except IndexError: + return "" + else: + return "" + + def skip_white_space(self): + try: + while self.get_next_token_type() == Token.WHITESPACE: + self.token_pos += 1 + except IndexError: + assert self.token_pos > 0, "We should always have positive indexes" + logging.debug("We are out of range so end is reached") + + def process_token_of_type(self, token_type): + """ + Maker sure the next token is of type `token_type` if not raise unexpected token + Args: + token_type: A token type + + Returns: + str: The value if the token is of type `token_type` + """ + if self.get_next_token_type() == token_type: + token_value = self.get_next_token_value() + self.goto_next_significant_token() + return token_value + else: + self.raise_unexpected_token() + + def goto_next_significant_token(self): + """Continue past current token and skip all whitespaces""" + self.token_pos += 1 + self.skip_white_space() + + def raise_unexpected_token(self): + if self.is_at_end(): + problematic_token = "" + problematic_token_in_near = "" + else: + problematic_token_in_near = problematic_token = self.get_next_token_value() + + near = "".join( + [ + self.get_2nd_last_token_value_if_last_was_whitespace(), + self.get_last_token_value(), + problematic_token_in_near, + self.get_following_token_value(), + self.get_2nd_following_token_value_if_following_was_whitespace(), + ] + ) + + raise InvalidTokenException(problematic_token, near) + + +class NestableBinExpressionParser(ExpressionParser): + """ + For nodes that can be nested in themselves (recursive) but with an operation. Take for example + UpdateExpressionValue's grammar: + + Value => Operand* + Value => Operand* + Value + Value => Operand* - Value + + If we consider it of structure + NestableBinExpression => TargetClause* + NestableBinExpression => TargetClause* BinOp NestableBinExpression + + This pattern comes back multiple times. This Mixin adds re-usability for that type of pattern. + + This approach is taken since it allows to remain the ordering of the Nodes as how the corresponding tokens where + in the originating expression. + """ + + def __init__(self, *args, **kwargs): + super(NestableBinExpressionParser, self).__init__(*args, **kwargs) + self.target_nodes = deque() + + def _parse_target_clause(self, factory_class): + """ + + Args: + factory_class: The factory for the target clause e.g. UpdateExpressionSetClauseParser + + Returns: + + """ + # noinspection PyProtectedMember + ast, self.token_pos = factory_class( + **self._initializer_args() + )._parse_with_pos() + self.target_nodes.append(ast) + logging.debug( + "Continue where previous parsing ended {token_pos}".format( + token_pos=self.token_pos + ) + ) + + def _parse(self): + self._parse_target_clause(self._operand_factory_class()) + while self._binop_factory_class().is_possible_start(self.get_next_token()): + self._parse_target_clause(self._binop_factory_class()) + if self._operand_factory_class().is_possible_start(self.get_next_token()): + self._parse_target_clause(self._operand_factory_class()) + else: + self.raise_unexpected_token() + + @abstractmethod + def _operand_factory_class(self): + """ + Get the Parser class of the Operands for the Binary operations/actions. + + Returns: + class: + """ + + @abstractmethod + def _binop_factory_class(self): + """ + Get a factory that gets the possible binary operation. + + Returns: + class: A class extending ExpressionParser + """ + + def _create_node(self): + """ + target_clauses has the nodes in order of encountering. Go through them forward and build the tree bottom up. + For simplicity docstring will use Operand Node rather than the specific node + + This way left-deep-descending traversal will process nodes in order. + + Continuing the example of an UpdateExpressionValue: + For example value => a + :val - :val2 + UpdateExpressionValue + / | \ + UpdateExpressionValue BinOp Operand + / | | | | + UpdateExpressionValue BinOp Operand - :val2 + / | | + Operand + :val + | + a + + self.target_nodes looks like: ( a >> + >> :val >> - >> :val2 ) + Returns: + moto.dynamodb2.ast_nodes.Node: Node of an AST representing the Expression as produced by the factory. + """ + if len(self.target_nodes) == 1: + return UpdateExpressionValue(children=[self.target_nodes.popleft()]) + else: + target_node = UpdateExpressionValue( + children=[ + self.target_nodes.popleft(), + self.target_nodes.popleft(), + self.target_nodes.popleft(), + ] + ) + while len(self.target_nodes) >= 2: + target_node = UpdateExpressionValue( + children=[ + target_node, + self.target_nodes.popleft(), + self.target_nodes.popleft(), + ] + ) + assert len(self.target_nodes) == 0 + return target_node + + +class UpdateExpressionParser(ExpressionParser, NestableExpressionParserMixin): + """ + Parser to create update expressions + """ + + @classmethod + def _sub_factories(cls): + return [ + UpdateExpressionSetClauseParser, + UpdateExpressionAddClauseParser, + UpdateExpressionDeleteClauseParser, + UpdateExpressionRemoveClauseParser, + ] + + @classmethod + def _is_possible_start(cls, token): + pass + + def __init__(self, *args, **kwargs): + super(UpdateExpressionParser, self).__init__(*args, **kwargs) + NestableExpressionParserMixin.__init__(self) + + @classmethod + def _nestable_class(cls): + return UpdateExpression + + def _parse_expression_clause(self, factory_class): + return self._parse_target_clause(factory_class) + + def _parse_by_a_subfactory(self): + for sub_factory in self._sub_factories(): + if sub_factory.is_possible_start(self.get_next_token()): + self._parse_expression_clause(sub_factory) + return True + return False + + def _parse(self): + """ + Update Expression is the top-most node therefore it is expected to end up at the end of the expression. + """ + while True: + self.skip_white_space() + if self.is_at_end(): + logging.debug("End reached") + break + elif self._parse_by_a_subfactory(): + continue + else: + self.raise_unexpected_token() + + return self._create_node(), self.token_pos + + @classmethod + def make(cls, expression_str): + token_list = ExpressionTokenizer.make_list(expression_str) + return cls(token_list).parse() + + +class UpdateExpressionSetClauseParser(ExpressionParser): + """ + UpdateExpressionSetClause => SET SetActions + """ + + @classmethod + def _is_possible_start(cls, token): + return token.type == Token.ATTRIBUTE and token.value.upper() == "SET" + + def _parse(self): + assert self.is_possible_start(self.get_next_token()) + self.goto_next_significant_token() + ast, self.token_pos = UpdateExpressionSetActionsParser( + **self._initializer_args() + )._parse_with_pos() + # noinspection PyProtectedMember + return UpdateExpressionSetClause(children=[ast]) + + +class UpdateExpressionActionsParser(ExpressionParser, NestableExpressionParserMixin): + """ + UpdateExpressionSetActions + """ + + def __init__(self, *args, **kwargs): + super(UpdateExpressionActionsParser, self).__init__(*args, **kwargs) + NestableExpressionParserMixin.__init__(self) + + @classmethod + def _is_possible_start(cls, token): + raise RuntimeError( + "{class_name} cannot be identified by the next token.".format( + class_name=cls._nestable_class().__name__ + ) + ) + + @classmethod + @abstractmethod + def _nestable_class(cls): + return UpdateExpressionSetActions + + @classmethod + @abstractmethod + def _nested_expression_parser_class(cls): + """Returns the parser for the query part that creates the nested nodes""" + + def _parse(self): + """ + UpdateExpressionSetActions is inside the expression so it can be followed by others. Process SetActions one by + one until no more SetAction. + """ + self.skip_white_space() + + while self._nested_expression_parser_class().is_possible_start( + self.get_next_token() + ): + self._parse_target_clause(self._nested_expression_parser_class()) + self.skip_white_space() + if self.get_next_token_type() == Token.COMMA: + self.goto_next_significant_token() + else: + break + + if len(self.target_clauses) == 0: + logging.debug( + "Didn't encounter a single {nc} in {nepc}.".format( + nc=self._nestable_class().__name__, + nepc=self._nested_expression_parser_class().__name__, + ) + ) + self.raise_unexpected_token() + + return self._create_node() + + +class UpdateExpressionSetActionsParser(UpdateExpressionActionsParser): + """ + UpdateExpressionSetActions + """ + + @classmethod + def _nested_expression_parser_class(cls): + return UpdateExpressionSetActionParser + + @classmethod + def _nestable_class(cls): + return UpdateExpressionSetActions + + +class UpdateExpressionSetActionParser(ExpressionParser): + """ + SetAction => Path = Value + + So we create an UpdateExpressionSetAction Node that has 2 children. Left child Path and right child Value. + """ + + @classmethod + def _is_possible_start(cls, token): + return UpdateExpressionPathParser.is_possible_start(token) + + def _parse(self): + """ + UpdateExpressionSetActionParser only gets called when expecting a SetAction. So we should be aggressive on + raising invalid Tokens. We can thus do the following: + 1) Process path + 2) skip whitespace if there are any + 3) Process equal-sign token + 4) skip whitespace if there are any + 3) Process value + + """ + path, self.token_pos = UpdateExpressionPathParser( + **self._initializer_args() + )._parse_with_pos() + self.skip_white_space() + self.process_token_of_type(Token.EQUAL_SIGN) + self.skip_white_space() + value, self.token_pos = UpdateExpressionValueParser( + **self._initializer_args() + )._parse_with_pos() + return UpdateExpressionSetAction(children=[path, value]) + + +class UpdateExpressionPathParser(ExpressionParser): + """ + Paths are selectors within items to specify a part within an Item. DynamoDB does not impose much restrictions on the + data it stores but it does store more strict restrictions on how they are represented in UpdateExpression's. + + """ + + def __init__(self, *args, **kwargs): + super(UpdateExpressionPathParser, self).__init__(*args, **kwargs) + self.path_nodes = [] + + @classmethod + def _is_possible_start(cls, token): + """ + Args: + token(Token): the token to be checked + + Returns: + bool: Whether the token could be the start of an UpdateExpressionPath + """ + if token.type == Token.ATTRIBUTE_NAME: + return True + elif token.type == Token.ATTRIBUTE and token.value.upper() != "REMOVE": + """We have to make sure remove is not passed""" + return True + return False + + def _parse(self): + return self.process_path() + + def process_path(self): + self.parse_path() + return UpdateExpressionPath(children=self.path_nodes) + + def parse_path(self): + """ + A path is comprised of: + - Attribute: the name of an attribute as how it is stored which has no special characters + - ATTRIBUTE_NAME: A placeholder that has no special characters except leading # to refer to attributes that + have a name that is not allowed in an UpdateExpression) + - DOT's: These are used to decent in a nested structure. When a DOT is in a path expression it is never part + of an attribute name but always means to descent into a MAP. We will call each descend a patch + chain + - SELECTORs: E.g.: [1] These are used to select an element in ordered datatypes like a list. + + Whitespaces can be between all these elements that build a path. For SELECTORs it is also allowed to have + whitespaces between brackets and numbers but the number cannot be split up with spaces + + Attributes and attribute_names must be separated with DOT's. + Returns: + UpdateExpressionPath: + """ + self.parse_path_chain() + while self.is_next_token_start_of_patch_chain(): + self.process_dot() + self.parse_path_chain() + + def is_next_token_start_of_patch_chain(self): + return self.get_next_token_type() == Token.DOT + + def process_dot(self): + self.path_nodes.append(ExpressionPathDescender()) + self.goto_next_significant_token() + + def parse_path_chain(self): + self.process_attribute_identifying_token() + self.skip_white_space() + while self.is_next_token_start_of_selector(): + self.process_selector() + self.skip_white_space() + + def process_attribute_identifying_token(self): + if self.get_next_token_type() == Token.ATTRIBUTE: + self.path_nodes.append(ExpressionAttribute(self.get_next_token_value())) + elif self.get_next_token_type() == Token.ATTRIBUTE_NAME: + self.path_nodes.append(ExpressionAttributeName(self.get_next_token_value())) + else: + self.raise_unexpected_token() + + self.goto_next_significant_token() + + def is_next_token_start_of_selector(self): + return self.get_next_token_type() == Token.OPEN_SQUARE_BRACKET + + def process_selector(self): + """ + Process the selector is only called when a selector must be processed. So do the following actions: + - skip opening bracket + - skip optional spaces + - read numeric literal + - skip optional spaces + - pass closing bracket + """ + self.process_token_of_type(Token.OPEN_SQUARE_BRACKET) + selector_value = self.process_token_of_type(Token.NUMBER) + self.process_token_of_type(Token.CLOSE_SQUARE_BRACKET) + self.path_nodes.append(ExpressionSelector(selector_value)) + + +class UpdateExpressionValueParser(NestableBinExpressionParser): + @classmethod + def _is_possible_start(cls, token): + return UpdateExpressionOperandParser.is_possible_start(token) + + def _operand_factory_class(self): + return UpdateExpressionOperandParser + + def _binop_factory_class(self): + return UpdateExpressionValueOperatorParser + + +class UpdateExpressionGroupedValueParser(ExpressionParser): + """ + A grouped value is an Update Expression value clause that is surrounded by round brackets. Each Operand can be + a grouped value by itself. + """ + + def _parse(self): + self.process_token_of_type(Token.OPEN_ROUND_BRACKET) + value, self.token_pos = UpdateExpressionValueParser( + **self._initializer_args() + )._parse_with_pos() + self.process_token_of_type(Token.CLOSE_ROUND_BRACKET) + return UpdateExpressionGroupedValue(children=value) + + @classmethod + def _is_possible_start(cls, token): + return token.type == Token.OPEN_ROUND_BRACKET + + +class UpdateExpressionValueOperatorParser(ExpressionParser): + OPERATION_TOKENS = [Token.PLUS_SIGN, Token.MINUS_SIGN] + + @classmethod + def _is_possible_start(cls, token): + return token.type in cls.OPERATION_TOKENS + + def _parse(self): + operation_value = self.get_next_token_value() + assert operation_value in self.OPERATION_TOKENS + self.goto_next_significant_token() + return ExpressionValueOperator(operation_value) + + +class UpdateExpressionOperandParser(ExpressionParser): + """ + Grammar + Operand* => AttributeValue + Operand* => UpdateExpressionFunction + Operand* => Path + Operand* => GroupedValue + """ + + @classmethod + def _sub_factories(cls): + return [ + UpdateExpressionAttributeValueParser, + UpdateExpressionFunctionParser, + UpdateExpressionPathParser, + UpdateExpressionGroupedValueParser, + ] + + @classmethod + def _is_possible_start(cls, token): + return any(parser.is_possible_start(token) for parser in cls._sub_factories()) + + def _parse(self): + for factory in self._sub_factories(): + if factory.is_possible_start(self.get_next_token()): + node, self.token_pos = factory( + **self._initializer_args() + )._parse_with_pos() + return node + self.raise_unexpected_token() + + +class UpdateExpressionAttributeValueParser(ExpressionParser): + def _parse(self): + attr_value = ExpressionAttributeValue( + self.process_token_of_type(Token.ATTRIBUTE_VALUE) + ) + return attr_value + + @classmethod + def _is_possible_start(cls, token): + return token.type == Token.ATTRIBUTE_VALUE + + +class UpdateExpressionFunctionParser(ExpressionParser): + """ + A helper to process a function of an Update Expression + """ + + # TODO(pbbouwel): Function names are supposedly case sensitive according to doc add tests + # Map function to the factories for its elements + FUNCTIONS = { + "if_not_exists": [UpdateExpressionPathParser, UpdateExpressionValueParser], + "list_append": [UpdateExpressionOperandParser, UpdateExpressionOperandParser], + } + + @classmethod + def _is_possible_start(cls, token): + """ + Check whether a token is supposed to be a function + Args: + token(Token): the token to check + + Returns: + bool: True if token is the start of a function. + """ + if token.type == Token.ATTRIBUTE: + return token.value in cls.FUNCTIONS.keys() + else: + return False + + def _parse(self): + function_name = self.get_next_token_value() + self.goto_next_significant_token() + self.process_token_of_type(Token.OPEN_ROUND_BRACKET) + function_elements = [function_name] + function_arguments = self.FUNCTIONS[function_name] + for i, func_elem_factory in enumerate(function_arguments): + func_elem, self.token_pos = func_elem_factory( + **self._initializer_args() + )._parse_with_pos() + function_elements.append(func_elem) + if i + 1 < len(function_arguments): + self.skip_white_space() + self.process_token_of_type(Token.COMMA) + self.process_token_of_type(Token.CLOSE_ROUND_BRACKET) + return UpdateExpressionFunction(children=function_elements) + + +class UpdateExpressionRemoveClauseParser(ExpressionParser): + """ + UpdateExpressionRemoveClause => REMOVE RemoveActions + """ + + def _parse(self): + assert self.is_possible_start(self.get_next_token()) + self.goto_next_significant_token() + ast, self.token_pos = UpdateExpressionRemoveActionsParser( + **self._initializer_args() + )._parse_with_pos() + # noinspection PyProtectedMember + return UpdateExpressionRemoveClause(children=[ast]) + + @classmethod + def _is_possible_start(cls, token): + """REMOVE is not a keyword""" + return token.type == Token.ATTRIBUTE and token.value.upper() == "REMOVE" + + +class UpdateExpressionRemoveActionsParser(UpdateExpressionActionsParser): + """ + UpdateExpressionSetActions + """ + + @classmethod + def _nested_expression_parser_class(cls): + return UpdateExpressionRemoveActionParser + + @classmethod + def _nestable_class(cls): + return UpdateExpressionRemoveActions + + +class UpdateExpressionRemoveActionParser(ExpressionParser): + """ + RemoveAction => Path = Value + + So we create an UpdateExpressionSetAction Node that has 2 children. Left child Path and right child Value. + """ + + @classmethod + def _is_possible_start(cls, token): + return UpdateExpressionPathParser.is_possible_start(token) + + def _parse(self): + """ + UpdateExpressionRemoveActionParser only gets called when expecting a RemoveAction. So we should be aggressive on + raising invalid Tokens. We can thus do the following: + 1) Process path + 2) skip whitespace if there are any + + """ + path, self.token_pos = UpdateExpressionPathParser( + **self._initializer_args() + )._parse_with_pos() + self.skip_white_space() + return UpdateExpressionRemoveAction(children=[path]) + + +class UpdateExpressionAddClauseParser(ExpressionParser): + def _parse(self): + assert self.is_possible_start(self.get_next_token()) + self.goto_next_significant_token() + ast, self.token_pos = UpdateExpressionAddActionsParser( + **self._initializer_args() + )._parse_with_pos() + # noinspection PyProtectedMember + return UpdateExpressionAddClause(children=[ast]) + + @classmethod + def _is_possible_start(cls, token): + return token.type == Token.ATTRIBUTE and token.value.upper() == "ADD" + + +class UpdateExpressionAddActionsParser(UpdateExpressionActionsParser): + """ + UpdateExpressionSetActions + """ + + @classmethod + def _nested_expression_parser_class(cls): + return UpdateExpressionAddActionParser + + @classmethod + def _nestable_class(cls): + return UpdateExpressionAddActions + + +@six.add_metaclass(abc.ABCMeta) +class UpdateExpressionPathValueParser(ExpressionParser): + def _parse_path_and_value(self): + """ + UpdateExpressionAddActionParser only gets called when expecting an AddAction. So we should be aggressive on + raising invalid Tokens. We can thus do the following: + 1) Process path + 2) skip whitespace if there are any + 3) Process a value + 4) skip whitespace if there are any + + Returns: + [path, value]: A list containing the Path node and the AttributeValue nodes + """ + path, self.token_pos = UpdateExpressionPathParser( + **self._initializer_args() + )._parse_with_pos() + self.skip_white_space() + value, self.token_pos = UpdateExpressionAttributeValueParser( + **self._initializer_args() + )._parse_with_pos() + self.skip_white_space() + return [path, value] + + +class UpdateExpressionAddActionParser(UpdateExpressionPathValueParser): + @classmethod + def _is_possible_start(cls, token): + return UpdateExpressionPathParser.is_possible_start(token) + + def _parse(self): + return UpdateExpressionAddAction(children=self._parse_path_and_value()) + + +class UpdateExpressionDeleteClauseParser(ExpressionParser): + def _parse(self): + assert self.is_possible_start(self.get_next_token()) + self.goto_next_significant_token() + ast, self.token_pos = UpdateExpressionDeleteActionsParser( + **self._initializer_args() + )._parse_with_pos() + # noinspection PyProtectedMember + return UpdateExpressionDeleteClause(children=[ast]) + + @classmethod + def _is_possible_start(cls, token): + return token.type == Token.ATTRIBUTE and token.value.upper() == "DELETE" + + +class UpdateExpressionDeleteActionsParser(UpdateExpressionActionsParser): + """ + UpdateExpressionSetActions + """ + + @classmethod + def _nested_expression_parser_class(cls): + return UpdateExpressionDeleteActionParser + + @classmethod + def _nestable_class(cls): + return UpdateExpressionDeleteActions + + +class UpdateExpressionDeleteActionParser(UpdateExpressionPathValueParser): + @classmethod + def _is_possible_start(cls, token): + return UpdateExpressionPathParser.is_possible_start(token) + + def _parse(self): + return UpdateExpressionDeleteAction(children=self._parse_path_and_value()) diff --git a/moto/dynamodb2/parsing/reserved_keywords.py b/moto/dynamodb2/parsing/reserved_keywords.py new file mode 100644 index 000000000..d82b16e98 --- /dev/null +++ b/moto/dynamodb2/parsing/reserved_keywords.py @@ -0,0 +1,29 @@ +class ReservedKeywords(list): + """ + DynamoDB has an extensive list of keywords. Keywords are considered when validating the expression Tree. + Not earlier since an update expression like "SET path = VALUE 1" fails with: + 'Invalid UpdateExpression: Syntax error; token: "1", near: "VALUE 1"' + """ + + KEYWORDS = None + + @classmethod + def get_reserved_keywords(cls): + if cls.KEYWORDS is None: + cls.KEYWORDS = cls._get_reserved_keywords() + return cls.KEYWORDS + + @classmethod + def _get_reserved_keywords(cls): + """ + Get a list of reserved keywords of DynamoDB + """ + try: + import importlib.resources as pkg_resources + except ImportError: + import importlib_resources as pkg_resources + + reserved_keywords = pkg_resources.read_text( + "moto.dynamodb2.parsing", "reserved_keywords.txt" + ) + return reserved_keywords.split() diff --git a/moto/dynamodb2/parsing/reserved_keywords.txt b/moto/dynamodb2/parsing/reserved_keywords.txt new file mode 100644 index 000000000..7c0106127 --- /dev/null +++ b/moto/dynamodb2/parsing/reserved_keywords.txt @@ -0,0 +1,573 @@ +ABORT +ABSOLUTE +ACTION +ADD +AFTER +AGENT +AGGREGATE +ALL +ALLOCATE +ALTER +ANALYZE +AND +ANY +ARCHIVE +ARE +ARRAY +AS +ASC +ASCII +ASENSITIVE +ASSERTION +ASYMMETRIC +AT +ATOMIC +ATTACH +ATTRIBUTE +AUTH +AUTHORIZATION +AUTHORIZE +AUTO +AVG +BACK +BACKUP +BASE +BATCH +BEFORE +BEGIN +BETWEEN +BIGINT +BINARY +BIT +BLOB +BLOCK +BOOLEAN +BOTH +BREADTH +BUCKET +BULK +BY +BYTE +CALL +CALLED +CALLING +CAPACITY +CASCADE +CASCADED +CASE +CAST +CATALOG +CHAR +CHARACTER +CHECK +CLASS +CLOB +CLOSE +CLUSTER +CLUSTERED +CLUSTERING +CLUSTERS +COALESCE +COLLATE +COLLATION +COLLECTION +COLUMN +COLUMNS +COMBINE +COMMENT +COMMIT +COMPACT +COMPILE +COMPRESS +CONDITION +CONFLICT +CONNECT +CONNECTION +CONSISTENCY +CONSISTENT +CONSTRAINT +CONSTRAINTS +CONSTRUCTOR +CONSUMED +CONTINUE +CONVERT +COPY +CORRESPONDING +COUNT +COUNTER +CREATE +CROSS +CUBE +CURRENT +CURSOR +CYCLE +DATA +DATABASE +DATE +DATETIME +DAY +DEALLOCATE +DEC +DECIMAL +DECLARE +DEFAULT +DEFERRABLE +DEFERRED +DEFINE +DEFINED +DEFINITION +DELETE +DELIMITED +DEPTH +DEREF +DESC +DESCRIBE +DESCRIPTOR +DETACH +DETERMINISTIC +DIAGNOSTICS +DIRECTORIES +DISABLE +DISCONNECT +DISTINCT +DISTRIBUTE +DO +DOMAIN +DOUBLE +DROP +DUMP +DURATION +DYNAMIC +EACH +ELEMENT +ELSE +ELSEIF +EMPTY +ENABLE +END +EQUAL +EQUALS +ERROR +ESCAPE +ESCAPED +EVAL +EVALUATE +EXCEEDED +EXCEPT +EXCEPTION +EXCEPTIONS +EXCLUSIVE +EXEC +EXECUTE +EXISTS +EXIT +EXPLAIN +EXPLODE +EXPORT +EXPRESSION +EXTENDED +EXTERNAL +EXTRACT +FAIL +FALSE +FAMILY +FETCH +FIELDS +FILE +FILTER +FILTERING +FINAL +FINISH +FIRST +FIXED +FLATTERN +FLOAT +FOR +FORCE +FOREIGN +FORMAT +FORWARD +FOUND +FREE +FROM +FULL +FUNCTION +FUNCTIONS +GENERAL +GENERATE +GET +GLOB +GLOBAL +GO +GOTO +GRANT +GREATER +GROUP +GROUPING +HANDLER +HASH +HAVE +HAVING +HEAP +HIDDEN +HOLD +HOUR +IDENTIFIED +IDENTITY +IF +IGNORE +IMMEDIATE +IMPORT +IN +INCLUDING +INCLUSIVE +INCREMENT +INCREMENTAL +INDEX +INDEXED +INDEXES +INDICATOR +INFINITE +INITIALLY +INLINE +INNER +INNTER +INOUT +INPUT +INSENSITIVE +INSERT +INSTEAD +INT +INTEGER +INTERSECT +INTERVAL +INTO +INVALIDATE +IS +ISOLATION +ITEM +ITEMS +ITERATE +JOIN +KEY +KEYS +LAG +LANGUAGE +LARGE +LAST +LATERAL +LEAD +LEADING +LEAVE +LEFT +LENGTH +LESS +LEVEL +LIKE +LIMIT +LIMITED +LINES +LIST +LOAD +LOCAL +LOCALTIME +LOCALTIMESTAMP +LOCATION +LOCATOR +LOCK +LOCKS +LOG +LOGED +LONG +LOOP +LOWER +MAP +MATCH +MATERIALIZED +MAX +MAXLEN +MEMBER +MERGE +METHOD +METRICS +MIN +MINUS +MINUTE +MISSING +MOD +MODE +MODIFIES +MODIFY +MODULE +MONTH +MULTI +MULTISET +NAME +NAMES +NATIONAL +NATURAL +NCHAR +NCLOB +NEW +NEXT +NO +NONE +NOT +NULL +NULLIF +NUMBER +NUMERIC +OBJECT +OF +OFFLINE +OFFSET +OLD +ON +ONLINE +ONLY +OPAQUE +OPEN +OPERATOR +OPTION +OR +ORDER +ORDINALITY +OTHER +OTHERS +OUT +OUTER +OUTPUT +OVER +OVERLAPS +OVERRIDE +OWNER +PAD +PARALLEL +PARAMETER +PARAMETERS +PARTIAL +PARTITION +PARTITIONED +PARTITIONS +PATH +PERCENT +PERCENTILE +PERMISSION +PERMISSIONS +PIPE +PIPELINED +PLAN +POOL +POSITION +PRECISION +PREPARE +PRESERVE +PRIMARY +PRIOR +PRIVATE +PRIVILEGES +PROCEDURE +PROCESSED +PROJECT +PROJECTION +PROPERTY +PROVISIONING +PUBLIC +PUT +QUERY +QUIT +QUORUM +RAISE +RANDOM +RANGE +RANK +RAW +READ +READS +REAL +REBUILD +RECORD +RECURSIVE +REDUCE +REF +REFERENCE +REFERENCES +REFERENCING +REGEXP +REGION +REINDEX +RELATIVE +RELEASE +REMAINDER +RENAME +REPEAT +REPLACE +REQUEST +RESET +RESIGNAL +RESOURCE +RESPONSE +RESTORE +RESTRICT +RESULT +RETURN +RETURNING +RETURNS +REVERSE +REVOKE +RIGHT +ROLE +ROLES +ROLLBACK +ROLLUP +ROUTINE +ROW +ROWS +RULE +RULES +SAMPLE +SATISFIES +SAVE +SAVEPOINT +SCAN +SCHEMA +SCOPE +SCROLL +SEARCH +SECOND +SECTION +SEGMENT +SEGMENTS +SELECT +SELF +SEMI +SENSITIVE +SEPARATE +SEQUENCE +SERIALIZABLE +SESSION +SET +SETS +SHARD +SHARE +SHARED +SHORT +SHOW +SIGNAL +SIMILAR +SIZE +SKEWED +SMALLINT +SNAPSHOT +SOME +SOURCE +SPACE +SPACES +SPARSE +SPECIFIC +SPECIFICTYPE +SPLIT +SQL +SQLCODE +SQLERROR +SQLEXCEPTION +SQLSTATE +SQLWARNING +START +STATE +STATIC +STATUS +STORAGE +STORE +STORED +STREAM +STRING +STRUCT +STYLE +SUB +SUBMULTISET +SUBPARTITION +SUBSTRING +SUBTYPE +SUM +SUPER +SYMMETRIC +SYNONYM +SYSTEM +TABLE +TABLESAMPLE +TEMP +TEMPORARY +TERMINATED +TEXT +THAN +THEN +THROUGHPUT +TIME +TIMESTAMP +TIMEZONE +TINYINT +TO +TOKEN +TOTAL +TOUCH +TRAILING +TRANSACTION +TRANSFORM +TRANSLATE +TRANSLATION +TREAT +TRIGGER +TRIM +TRUE +TRUNCATE +TTL +TUPLE +TYPE +UNDER +UNDO +UNION +UNIQUE +UNIT +UNKNOWN +UNLOGGED +UNNEST +UNPROCESSED +UNSIGNED +UNTIL +UPDATE +UPPER +URL +USAGE +USE +USER +USERS +USING +UUID +VACUUM +VALUE +VALUED +VALUES +VARCHAR +VARIABLE +VARIANCE +VARINT +VARYING +VIEW +VIEWS +VIRTUAL +VOID +WAIT +WHEN +WHENEVER +WHERE +WHILE +WINDOW +WITH +WITHIN +WITHOUT +WORK +WRAPPED +WRITE +YEAR +ZONE diff --git a/moto/dynamodb2/parsing/tokens.py b/moto/dynamodb2/parsing/tokens.py index 07d65ae64..4fbb7883a 100644 --- a/moto/dynamodb2/parsing/tokens.py +++ b/moto/dynamodb2/parsing/tokens.py @@ -1,4 +1,5 @@ import re +import sys from moto.dynamodb2.exceptions import ( InvalidTokenException, @@ -147,9 +148,17 @@ class ExpressionTokenizer(object): self.token_list = [] self.staged_characters = "" + @classmethod + def is_py2(cls): + return sys.version_info[0] == 2 + @classmethod def make_list(cls, input_expression_str): - assert isinstance(input_expression_str, str) + if cls.is_py2(): + pass + else: + assert isinstance(input_expression_str, str) + return ExpressionTokenizer(input_expression_str)._make_list() def add_token(self, token_type, token_value): @@ -159,6 +168,10 @@ class ExpressionTokenizer(object): self.add_token(token_type, self.staged_characters) self.staged_characters = "" + @classmethod + def is_numeric(cls, input_str): + return re.compile("[0-9]+").match(input_str) is not None + def process_staged_characters(self): if len(self.staged_characters) == 0: return @@ -167,7 +180,7 @@ class ExpressionTokenizer(object): self.add_token_from_stage(Token.ATTRIBUTE_NAME) else: raise InvalidExpressionAttributeNameKey(self.staged_characters) - elif self.staged_characters.isnumeric(): + elif self.is_numeric(self.staged_characters): self.add_token_from_stage(Token.NUMBER) elif self.is_expression_attribute(self.staged_characters): self.add_token_from_stage(Token.ATTRIBUTE) diff --git a/moto/dynamodb2/responses.py b/moto/dynamodb2/responses.py index d21d1d756..a5aeeac70 100644 --- a/moto/dynamodb2/responses.py +++ b/moto/dynamodb2/responses.py @@ -748,11 +748,6 @@ class DynamoHandler(BaseResponse): expression_attribute_names = self.body.get("ExpressionAttributeNames", {}) expression_attribute_values = self.body.get("ExpressionAttributeValues", {}) - # Support spaces between operators in an update expression - # E.g. `a = b + c` -> `a=b+c` - if update_expression: - update_expression = re.sub(r"\s*([=\+-])\s*", "\\1", update_expression) - try: item = self.dynamodb_backend.update_item( name, diff --git a/tests/test_dynamodb2/test_dynamodb.py b/tests/test_dynamodb2/test_dynamodb.py index bec24c966..09401d562 100644 --- a/tests/test_dynamodb2/test_dynamodb.py +++ b/tests/test_dynamodb2/test_dynamodb.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals, print_function +import re from decimal import Decimal import six @@ -4177,3 +4178,70 @@ def test_gsi_verify_negative_number_order(): [float(item["gsiK1SortKey"]) for item in resp["Items"]].should.equal( [-0.7, -0.6, 0.7] ) + + +def assert_raise_syntax_error(client_error, token, near): + """ + Assert whether a client_error is as expected Syntax error. Syntax error looks like: `syntax_error_template` + + Args: + client_error(ClientError): The ClientError exception that was raised + token(str): The token that ws unexpected + near(str): The part in the expression that shows where the error occurs it generally has the preceding token the + optional separation and the problematic token. + """ + syntax_error_template = ( + 'Invalid UpdateExpression: Syntax error; token: "{token}", near: "{near}"' + ) + expected_syntax_error = syntax_error_template.format(token=token, near=near) + assert client_error.response["Error"]["Code"] == "ValidationException" + assert expected_syntax_error == client_error.response["Error"]["Message"] + + +@mock_dynamodb2 +def test_update_expression_with_numeric_literal_instead_of_value(): + """ + DynamoDB requires literals to be passed in as values. If they are put literally in the expression a token error will + be raised + """ + dynamodb = boto3.client("dynamodb", region_name="eu-west-1") + + dynamodb.create_table( + TableName="moto-test", + KeySchema=[{"AttributeName": "id", "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": "id", "AttributeType": "S"}], + ) + + try: + dynamodb.update_item( + TableName="moto-test", + Key={"id": {"S": "1"}}, + UpdateExpression="SET MyStr = myNum + 1", + ) + assert False, "Validation exception not thrown" + except dynamodb.exceptions.ClientError as e: + assert_raise_syntax_error(e, "1", "+ 1") + + +@mock_dynamodb2 +def test_update_expression_with_multiple_set_clauses_must_be_comma_separated(): + """ + An UpdateExpression can have multiple set clauses but if they are passed in without the separating comma. + """ + dynamodb = boto3.client("dynamodb", region_name="eu-west-1") + + dynamodb.create_table( + TableName="moto-test", + KeySchema=[{"AttributeName": "id", "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": "id", "AttributeType": "S"}], + ) + + try: + dynamodb.update_item( + TableName="moto-test", + Key={"id": {"S": "1"}}, + UpdateExpression="SET MyStr = myNum Mystr2 myNum2", + ) + assert False, "Validation exception not thrown" + except dynamodb.exceptions.ClientError as e: + assert_raise_syntax_error(e, "Mystr2", "myNum Mystr2 myNum2") diff --git a/tests/test_dynamodb2/test_dynamodb_expressions.py b/tests/test_dynamodb2/test_dynamodb_expressions.py new file mode 100644 index 000000000..1066231af --- /dev/null +++ b/tests/test_dynamodb2/test_dynamodb_expressions.py @@ -0,0 +1,395 @@ +from moto.dynamodb2.exceptions import InvalidTokenException +from moto.dynamodb2.parsing.expressions import UpdateExpressionParser +from moto.dynamodb2.parsing.reserved_keywords import ReservedKeywords + + +def test_get_reserved_keywords(): + reserved_keywords = ReservedKeywords.get_reserved_keywords() + assert "SET" in reserved_keywords + assert "DELETE" in reserved_keywords + assert "ADD" in reserved_keywords + # REMOVE is not part of the list of reserved keywords. + assert "REMOVE" not in reserved_keywords + + +def test_update_expression_numeric_literal_in_expression(): + set_action = "SET attrName = 3" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "3" + assert te.near == "= 3" + + +def test_expression_tokenizer_multi_number_numeric_literal_in_expression(): + set_action = "SET attrName = 34" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "34" + assert te.near == "= 34" + + +def test_expression_tokenizer_numeric_literal_unclosed_square_bracket(): + set_action = "SET MyStr[ 3" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "" + assert te.near == "3" + + +def test_expression_tokenizer_wrong_closing_bracket_with_space(): + set_action = "SET MyStr[3 )" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == ")" + assert te.near == "3 )" + + +def test_expression_tokenizer_wrong_closing_bracket(): + set_action = "SET MyStr[3)" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == ")" + assert te.near == "3)" + + +def test_expression_tokenizer_only_numeric_literal_for_set(): + set_action = "SET 2" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "2" + assert te.near == "SET 2" + + +def test_expression_tokenizer_only_numeric_literal(): + set_action = "2" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "2" + assert te.near == "2" + + +def test_expression_tokenizer_set_closing_round_bracket(): + set_action = "SET )" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == ")" + assert te.near == "SET )" + + +def test_expression_tokenizer_set_closing_followed_by_numeric_literal(): + set_action = "SET ) 3" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == ")" + assert te.near == "SET ) 3" + + +def test_expression_tokenizer_numeric_literal_unclosed_square_bracket_trailing_space(): + set_action = "SET MyStr[ 3 " + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "" + assert te.near == "3 " + + +def test_expression_tokenizer_unbalanced_round_brackets_only_opening(): + set_action = "SET MyStr = (:_val" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "" + assert te.near == ":_val" + + +def test_expression_tokenizer_unbalanced_round_brackets_only_opening_trailing_space(): + set_action = "SET MyStr = (:_val " + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "" + assert te.near == ":_val " + + +def test_expression_tokenizer_unbalanced_square_brackets_only_opening(): + set_action = "SET MyStr = [:_val" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "[" + assert te.near == "= [:_val" + + +def test_expression_tokenizer_unbalanced_square_brackets_only_opening_trailing_spaces(): + set_action = "SET MyStr = [:_val " + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "[" + assert te.near == "= [:_val" + + +def test_expression_tokenizer_unbalanced_round_brackets_multiple_opening(): + set_action = "SET MyStr = (:_val + (:val2" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "" + assert te.near == ":val2" + + +def test_expression_tokenizer_unbalanced_round_brackets_only_closing(): + set_action = "SET MyStr = ):_val" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == ")" + assert te.near == "= ):_val" + + +def test_expression_tokenizer_unbalanced_square_brackets_only_closing(): + set_action = "SET MyStr = ]:_val" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "]" + assert te.near == "= ]:_val" + + +def test_expression_tokenizer_unbalanced_round_brackets_only_closing_followed_by_other_parts(): + set_action = "SET MyStr = ):_val + :val2" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == ")" + assert te.near == "= ):_val" + + +def test_update_expression_starts_with_keyword_reset_followed_by_identifier(): + update_expression = "RESET NonExistent" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "RESET" + assert te.near == "RESET NonExistent" + + +def test_update_expression_starts_with_keyword_reset_followed_by_identifier_and_value(): + update_expression = "RESET NonExistent value" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "RESET" + assert te.near == "RESET NonExistent" + + +def test_update_expression_starts_with_leading_spaces_and_keyword_reset_followed_by_identifier_and_value(): + update_expression = " RESET NonExistent value" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "RESET" + assert te.near == " RESET NonExistent" + + +def test_update_expression_with_only_keyword_reset(): + update_expression = "RESET" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "RESET" + assert te.near == "RESET" + + +def test_update_nested_expression_with_selector_just_should_fail_parsing_at_numeric_literal_value(): + update_expression = "SET a[0].b = 5" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "5" + assert te.near == "= 5" + + +def test_update_nested_expression_with_selector_and_spaces_should_only_fail_parsing_at_numeric_literal_value(): + update_expression = "SET a [ 2 ]. b = 5" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "5" + assert te.near == "= 5" + + +def test_update_nested_expression_with_double_selector_and_spaces_should_only_fail_parsing_at_numeric_literal_value(): + update_expression = "SET a [2][ 3 ]. b = 5" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "5" + assert te.near == "= 5" + + +def test_update_nested_expression_should_only_fail_parsing_at_numeric_literal_value(): + update_expression = "SET a . b = 5" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "5" + assert te.near == "= 5" + + +def test_nested_selectors_in_update_expression_should_fail_at_nesting(): + update_expression = "SET a [ [2] ]. b = 5" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "[" + assert te.near == "[ [2" + + +def test_update_expression_number_in_selector_cannot_be_splite(): + update_expression = "SET a [2 1]. b = 5" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "1" + assert te.near == "2 1]" + + +def test_update_expression_cannot_have_successive_attributes(): + update_expression = "SET #a a = 5" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "a" + assert te.near == "#a a =" + + +def test_update_expression_path_with_both_attribute_and_attribute_name_should_only_fail_at_numeric_value(): + update_expression = "SET #a.a = 5" + try: + UpdateExpressionParser.make(update_expression) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "5" + assert te.near == "= 5" + + +def test_expression_tokenizer_2_same_operators_back_to_back(): + set_action = "SET MyStr = NoExist + + :_val " + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "+" + assert te.near == "+ + :_val" + + +def test_expression_tokenizer_2_different_operators_back_to_back(): + set_action = "SET MyStr = NoExist + - :_val " + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "-" + assert te.near == "+ - :_val" + + +def test_update_expression_remove_does_not_allow_operations(): + remove_action = "REMOVE NoExist + " + try: + UpdateExpressionParser.make(remove_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "+" + assert te.near == "NoExist + " + + +def test_update_expression_add_does_not_allow_attribute_after_path(): + """value here is not really a value since a value starts with a colon (:)""" + add_expr = "ADD attr val foobar" + try: + UpdateExpressionParser.make(add_expr) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "val" + assert te.near == "attr val foobar" + + +def test_update_expression_add_does_not_allow_attribute_foobar_after_value(): + add_expr = "ADD attr :val foobar" + try: + UpdateExpressionParser.make(add_expr) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "foobar" + assert te.near == ":val foobar" + + +def test_update_expression_delete_does_not_allow_attribute_after_path(): + """value here is not really a value since a value starts with a colon (:)""" + delete_expr = "DELETE attr val" + try: + UpdateExpressionParser.make(delete_expr) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "val" + assert te.near == "attr val" + + +def test_update_expression_delete_does_not_allow_attribute_foobar_after_value(): + delete_expr = "DELETE attr :val foobar" + try: + UpdateExpressionParser.make(delete_expr) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "foobar" + assert te.near == ":val foobar" + + +def test_update_expression_parsing_is_not_keyword_aware(): + """path and VALUE are keywords. Yet a token error will be thrown for the numeric literal 1.""" + delete_expr = "SET path = VALUE 1" + try: + UpdateExpressionParser.make(delete_expr) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "1" + assert te.near == "VALUE 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 c433a3a31..1aa2175c1 100644 --- a/tests/test_dynamodb2/test_dynamodb_table_with_range_key.py +++ b/tests/test_dynamodb2/test_dynamodb_table_with_range_key.py @@ -1254,14 +1254,22 @@ def test_update_item_with_expression(): item_key = {"forum_name": "the-key", "subject": "123"} - table.update_item(Key=item_key, UpdateExpression="SET field=2") + table.update_item( + Key=item_key, + UpdateExpression="SET field = :field_value", + ExpressionAttributeValues={":field_value": 2}, + ) dict(table.get_item(Key=item_key)["Item"]).should.equal( - {"field": "2", "forum_name": "the-key", "subject": "123"} + {"field": Decimal("2"), "forum_name": "the-key", "subject": "123"} ) - table.update_item(Key=item_key, UpdateExpression="SET field = 3") + table.update_item( + Key=item_key, + UpdateExpression="SET field = :field_value", + ExpressionAttributeValues={":field_value": 3}, + ) dict(table.get_item(Key=item_key)["Item"]).should.equal( - {"field": "3", "forum_name": "the-key", "subject": "123"} + {"field": Decimal("3"), "forum_name": "the-key", "subject": "123"} ) From 891801d5697f80ab44afe8a20e5896e8807237b6 Mon Sep 17 00:00:00 2001 From: Bob Wombat Hogg Date: Sat, 18 Apr 2020 07:46:28 -0400 Subject: [PATCH 09/22] Use ISO 8601 format for ELB DescribeLoadBalancers --- moto/elb/models.py | 5 ++++- moto/elb/responses.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/moto/elb/models.py b/moto/elb/models.py index f77811623..4991b0754 100644 --- a/moto/elb/models.py +++ b/moto/elb/models.py @@ -1,6 +1,9 @@ from __future__ import unicode_literals import datetime + +import pytz + from boto.ec2.elb.attributes import ( LbAttributes, ConnectionSettingAttribute, @@ -83,7 +86,7 @@ class FakeLoadBalancer(BaseModel): self.zones = zones self.listeners = [] self.backends = [] - self.created_time = datetime.datetime.now() + self.created_time = datetime.datetime.now(pytz.utc) self.scheme = scheme self.attributes = FakeLoadBalancer.get_default_attributes() self.policies = Policies() diff --git a/moto/elb/responses.py b/moto/elb/responses.py index de21f23e7..79db5a788 100644 --- a/moto/elb/responses.py +++ b/moto/elb/responses.py @@ -442,7 +442,7 @@ DESCRIBE_LOAD_BALANCERS_TEMPLATE = """= 1 + return self.children[n] + + +class DDBTypedValue(Node): + """ + A node representing a DDBTyped value. This can be any structure as supported by DyanmoDB. The node only has 1 child + which is the value of type `DynamoType`. + """ + + def __init__(self, value): + assert isinstance(value, DynamoType), "DDBTypedValue must be of DynamoType" + super(DDBTypedValue, self).__init__(children=[value]) + + def get_value(self): + return self.children[0] + + +class NoneExistingPath(LeafNode): + """A placeholder for Paths that did not exist in the Item.""" + + def __init__(self, creatable=False): + super(NoneExistingPath, self).__init__(children=[creatable]) + + def is_creatable(self): + """Can this path be created if need be. For example path creating element in a dictionary or creating a new + attribute under root level of an item.""" + return self.children[0] + + +class DepthFirstTraverser(object): + """ + Helper class that allows depth first traversal and to implement custom processing for certain AST nodes. The + processor of a node must return the new resulting node. This node will be placed in the tree. Processing of a + node using this traverser should therefore only transform child nodes. The returned node will get the same parent + as the node before processing had. + """ + + @abstractmethod + def _processing_map(self): + """ + A map providing a processing function per node class type to a function that takes in a Node object and + processes it. A Node can only be processed by a single function and they are considered in order. Therefore if + multiple classes from a single class hierarchy strain are used the more specific classes have to be put before + the less specific ones. That requires overriding `nodes_to_be_processed`. If no multiple classes form a single + class hierarchy strain are used the default implementation of `nodes_to_be_processed` should be OK. + Returns: + dict: Mapping a Node Class to a processing function. + """ + pass + + def nodes_to_be_processed(self): + """Cached accessor for getting Node types that need to be processed.""" + return tuple(k for k in self._processing_map().keys()) + + def process(self, node): + """Process a Node""" + for class_key, processor in self._processing_map().items(): + if isinstance(node, class_key): + return processor(node) + + def pre_processing_of_child(self, parent_node, child_id): + """Hook that is called pre-processing of the child at position `child_id`""" + pass + + def traverse_node_recursively(self, node, child_id=-1): + """ + Traverse nodes depth first processing nodes bottom up (if root node is considered the top). + + Args: + node(Node): The node which is the last node to be processed but which allows to identify all the + work (which is in the children) + child_id(int): The index in the list of children from the parent that this node corresponds to + + Returns: + Node: The node of the new processed AST + """ + if isinstance(node, Node): + parent_node = node.parent + if node.children is not None: + for i, child_node in enumerate(node.children): + self.pre_processing_of_child(node, i) + self.traverse_node_recursively(child_node, i) + # noinspection PyTypeChecker + if isinstance(node, self.nodes_to_be_processed()): + node = self.process(node) + node.parent = parent_node + parent_node.children[child_id] = node + return node + + def traverse(self, node): + return self.traverse_node_recursively(node) + + +class NodeDepthLeftTypeFetcher(object): + """Helper class to fetch a node of a specific type. Depth left-first traversal""" + + def __init__(self, node_type, root_node): + assert issubclass(node_type, Node) + self.node_type = node_type + self.root_node = root_node + self.queue = deque() + self.add_nodes_left_to_right_depth_first(self.root_node) + + def add_nodes_left_to_right_depth_first(self, node): + if isinstance(node, Node) and node.children is not None: + for child_node in node.children: + self.add_nodes_left_to_right_depth_first(child_node) + self.queue.append(child_node) + self.queue.append(node) + + def __iter__(self): + return self + + def next(self): + return self.__next__() + + def __next__(self): + while len(self.queue) > 0: + candidate = self.queue.popleft() + if isinstance(candidate, self.node_type): + return candidate + else: + raise StopIteration diff --git a/moto/dynamodb2/parsing/expressions.py b/moto/dynamodb2/parsing/expressions.py index e418bb47e..4c1d42a55 100644 --- a/moto/dynamodb2/parsing/expressions.py +++ b/moto/dynamodb2/parsing/expressions.py @@ -29,7 +29,7 @@ from moto.dynamodb2.parsing.ast_nodes import ( UpdateExpressionDeleteActions, UpdateExpressionDeleteClause, ) -from moto.dynamodb2.exceptions import InvalidTokenException +from moto.dynamodb2.exceptions import InvalidTokenException, InvalidUpdateExpression from moto.dynamodb2.parsing.tokens import Token, ExpressionTokenizer @@ -371,6 +371,7 @@ class NestableBinExpressionParser(ExpressionParser): self._parse_target_clause(self._operand_factory_class()) else: self.raise_unexpected_token() + return self._create_node() @abstractmethod def _operand_factory_class(self): @@ -485,7 +486,7 @@ class UpdateExpressionParser(ExpressionParser, NestableExpressionParserMixin): else: self.raise_unexpected_token() - return self._create_node(), self.token_pos + return self._create_node() @classmethod def make(cls, expression_str): @@ -804,15 +805,41 @@ class UpdateExpressionAttributeValueParser(ExpressionParser): return token.type == Token.ATTRIBUTE_VALUE +class UpdateExpressionAttributeValueOrPathParser(ExpressionParser): + def _parse(self): + if UpdateExpressionAttributeValueParser.is_possible_start( + self.get_next_token() + ): + token, self.token_pos = UpdateExpressionAttributeValueParser( + **self._initializer_args() + )._parse_with_pos() + else: + token, self.token_pos = UpdateExpressionPathParser( + **self._initializer_args() + )._parse_with_pos() + return token + + @classmethod + def _is_possible_start(cls, token): + return any( + [ + UpdateExpressionAttributeValueParser.is_possible_start(token), + UpdateExpressionPathParser.is_possible_start(token), + ] + ) + + class UpdateExpressionFunctionParser(ExpressionParser): """ A helper to process a function of an Update Expression """ - # TODO(pbbouwel): Function names are supposedly case sensitive according to doc add tests # Map function to the factories for its elements FUNCTIONS = { - "if_not_exists": [UpdateExpressionPathParser, UpdateExpressionValueParser], + "if_not_exists": [ + UpdateExpressionPathParser, + UpdateExpressionAttributeValueOrPathParser, + ], "list_append": [UpdateExpressionOperandParser, UpdateExpressionOperandParser], } @@ -833,6 +860,9 @@ class UpdateExpressionFunctionParser(ExpressionParser): def _parse(self): function_name = self.get_next_token_value() + if function_name not in self.FUNCTIONS.keys(): + # Function names are case sensitive + raise InvalidUpdateExpression(function_name) self.goto_next_significant_token() self.process_token_of_type(Token.OPEN_ROUND_BRACKET) function_elements = [function_name] diff --git a/moto/dynamodb2/parsing/validators.py b/moto/dynamodb2/parsing/validators.py new file mode 100644 index 000000000..180c7a874 --- /dev/null +++ b/moto/dynamodb2/parsing/validators.py @@ -0,0 +1,341 @@ +""" +See docstring class Validator below for more details on validation +""" +from abc import abstractmethod +from copy import deepcopy + +from moto.dynamodb2.exceptions import ( + AttributeIsReservedKeyword, + ExpressionAttributeValueNotDefined, + AttributeDoesNotExist, + ExpressionAttributeNameNotDefined, + IncorrectOperandType, + InvalidUpdateExpressionInvalidDocumentPath, +) +from moto.dynamodb2.models import DynamoType +from moto.dynamodb2.parsing.ast_nodes import ( + ExpressionAttribute, + UpdateExpressionPath, + UpdateExpressionSetAction, + UpdateExpressionAddAction, + UpdateExpressionDeleteAction, + UpdateExpressionRemoveAction, + DDBTypedValue, + ExpressionAttributeValue, + ExpressionAttributeName, + DepthFirstTraverser, + NoneExistingPath, + UpdateExpressionFunction, + ExpressionPathDescender, + UpdateExpressionValue, + ExpressionValueOperator, + ExpressionSelector, +) +from moto.dynamodb2.parsing.reserved_keywords import ReservedKeywords + + +class ExpressionAttributeValueProcessor(DepthFirstTraverser): + def __init__(self, expression_attribute_values): + self.expression_attribute_values = expression_attribute_values + + def _processing_map(self): + return { + ExpressionAttributeValue: self.replace_expression_attribute_value_with_value + } + + def replace_expression_attribute_value_with_value(self, node): + """A node representing an Expression Attribute Value. Resolve and replace value""" + assert isinstance(node, ExpressionAttributeValue) + attribute_value_name = node.get_value_name() + try: + target = self.expression_attribute_values[attribute_value_name] + except KeyError: + raise ExpressionAttributeValueNotDefined( + attribute_value=attribute_value_name + ) + return DDBTypedValue(DynamoType(target)) + + +class ExpressionAttributeResolvingProcessor(DepthFirstTraverser): + def _processing_map(self): + return { + UpdateExpressionSetAction: self.disable_resolving, + UpdateExpressionPath: self.process_expression_path_node, + } + + def __init__(self, expression_attribute_names, item): + self.expression_attribute_names = expression_attribute_names + self.item = item + self.resolving = False + + def pre_processing_of_child(self, parent_node, child_id): + """ + We have to enable resolving if we are processing a child of UpdateExpressionSetAction that is not first. + Because first argument is path to be set, 2nd argument would be the value. + """ + if isinstance( + parent_node, + ( + UpdateExpressionSetAction, + UpdateExpressionRemoveAction, + UpdateExpressionDeleteAction, + UpdateExpressionAddAction, + ), + ): + if child_id == 0: + self.resolving = False + else: + self.resolving = True + + def disable_resolving(self, node=None): + self.resolving = False + return node + + def process_expression_path_node(self, node): + """Resolve ExpressionAttribute if not part of a path and resolving is enabled.""" + if self.resolving: + return self.resolve_expression_path(node) + else: + # Still resolve but return original note to make sure path is correct Just make sure nodes are creatable. + result_node = self.resolve_expression_path(node) + if ( + isinstance(result_node, NoneExistingPath) + and not result_node.is_creatable() + ): + raise InvalidUpdateExpressionInvalidDocumentPath() + + return node + + def resolve_expression_path(self, node): + assert isinstance(node, UpdateExpressionPath) + + target = deepcopy(self.item.attrs) + for child in node.children: + # First replace placeholder with attribute_name + attr_name = None + if isinstance(child, ExpressionAttributeName): + attr_placeholder = child.get_attribute_name_placeholder() + try: + attr_name = self.expression_attribute_names[attr_placeholder] + except KeyError: + raise ExpressionAttributeNameNotDefined(attr_placeholder) + elif isinstance(child, ExpressionAttribute): + attr_name = child.get_attribute_name() + self.raise_exception_if_keyword(attr_name) + if attr_name is not None: + # Resolv attribute_name + try: + target = target[attr_name] + except (KeyError, TypeError): + if child == node.children[-1]: + return NoneExistingPath(creatable=True) + return NoneExistingPath() + else: + if isinstance(child, ExpressionPathDescender): + continue + elif isinstance(child, ExpressionSelector): + index = child.get_index() + if target.is_list(): + try: + target = target[index] + except IndexError: + # When a list goes out of bounds when assigning that is no problem when at the assignment + # side. It will just append to the list. + if child == node.children[-1]: + return NoneExistingPath(creatable=True) + return NoneExistingPath() + else: + raise InvalidUpdateExpressionInvalidDocumentPath + else: + raise NotImplementedError( + "Path resolution for {t}".format(t=type(child)) + ) + return DDBTypedValue(DynamoType(target)) + + @classmethod + def raise_exception_if_keyword(cls, attribute): + if attribute.upper() in ReservedKeywords.get_reserved_keywords(): + raise AttributeIsReservedKeyword(attribute) + + +class UpdateExpressionFunctionEvaluator(DepthFirstTraverser): + """ + At time of writing there are only 2 functions for DDB UpdateExpressions. They both are specific to the SET + expression as per the official AWS docs: + https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/ + Expressions.UpdateExpressions.html#Expressions.UpdateExpressions.SET + """ + + def _processing_map(self): + return {UpdateExpressionFunction: self.process_function} + + def process_function(self, node): + assert isinstance(node, UpdateExpressionFunction) + function_name = node.get_function_name() + first_arg = node.get_nth_argument(1) + second_arg = node.get_nth_argument(2) + + if function_name == "if_not_exists": + if isinstance(first_arg, NoneExistingPath): + result = second_arg + else: + result = first_arg + assert isinstance(result, (DDBTypedValue, NoneExistingPath)) + return result + elif function_name == "list_append": + first_arg = self.get_list_from_ddb_typed_value(first_arg, function_name) + second_arg = self.get_list_from_ddb_typed_value(second_arg, function_name) + for list_element in second_arg.value: + first_arg.value.append(list_element) + return DDBTypedValue(first_arg) + else: + raise NotImplementedError( + "Unsupported function for moto {name}".format(name=function_name) + ) + + @classmethod + def get_list_from_ddb_typed_value(cls, node, function_name): + assert isinstance(node, DDBTypedValue) + dynamo_value = node.get_value() + assert isinstance(dynamo_value, DynamoType) + if not dynamo_value.is_list(): + raise IncorrectOperandType(function_name, dynamo_value.type) + return dynamo_value + + +class NoneExistingPathChecker(DepthFirstTraverser): + """ + Pass through the AST and make sure there are no none-existing paths. + """ + + def _processing_map(self): + return {NoneExistingPath: self.raise_none_existing_path} + + def raise_none_existing_path(self, node): + raise AttributeDoesNotExist + + +class ExecuteOperations(DepthFirstTraverser): + def _processing_map(self): + return {UpdateExpressionValue: self.process_update_expression_value} + + def process_update_expression_value(self, node): + """ + If an UpdateExpressionValue only has a single child the node will be replaced with the childe. + Otherwise it has 3 children and the middle one is an ExpressionValueOperator which details how to combine them + Args: + node(Node): + + Returns: + Node: The resulting node of the operation if present or the child. + """ + assert isinstance(node, UpdateExpressionValue) + if len(node.children) == 1: + return node.children[0] + elif len(node.children) == 3: + operator_node = node.children[1] + assert isinstance(operator_node, ExpressionValueOperator) + operator = operator_node.get_operator() + left_operand = self.get_dynamo_value_from_ddb_typed_value(node.children[0]) + right_operand = self.get_dynamo_value_from_ddb_typed_value(node.children[2]) + if operator == "+": + return self.get_sum(left_operand, right_operand) + elif operator == "-": + return self.get_subtraction(left_operand, right_operand) + else: + raise NotImplementedError( + "Moto does not support operator {operator}".format( + operator=operator + ) + ) + else: + raise NotImplementedError( + "UpdateExpressionValue only has implementations for 1 or 3 children." + ) + + @classmethod + def get_dynamo_value_from_ddb_typed_value(cls, node): + assert isinstance(node, DDBTypedValue) + dynamo_value = node.get_value() + assert isinstance(dynamo_value, DynamoType) + return dynamo_value + + @classmethod + def get_sum(cls, left_operand, right_operand): + """ + Args: + left_operand(DynamoType): + right_operand(DynamoType): + + Returns: + DDBTypedValue: + """ + try: + return DDBTypedValue(left_operand + right_operand) + except TypeError: + raise IncorrectOperandType("+", left_operand.type) + + @classmethod + def get_subtraction(cls, left_operand, right_operand): + """ + Args: + left_operand(DynamoType): + right_operand(DynamoType): + + Returns: + DDBTypedValue: + """ + try: + return DDBTypedValue(left_operand - right_operand) + except TypeError: + raise IncorrectOperandType("-", left_operand.type) + + +class Validator(object): + """ + A validator is used to validate expressions which are passed in as an AST. + """ + + def __init__( + self, expression, expression_attribute_names, expression_attribute_values, item + ): + """ + Besides validation the Validator should also replace referenced parts of an item which is cheapest upon + validation. + + Args: + expression(Node): The root node of the AST representing the expression to be validated + expression_attribute_names(ExpressionAttributeNames): + expression_attribute_values(ExpressionAttributeValues): + item(Item): The item which will be updated (pointed to by Key of update_item) + """ + self.expression_attribute_names = expression_attribute_names + self.expression_attribute_values = expression_attribute_values + self.item = item + self.processors = self.get_ast_processors() + self.node_to_validate = deepcopy(expression) + + @abstractmethod + def get_ast_processors(self): + """Get the different processors that go through the AST tree and processes the nodes.""" + + def validate(self): + n = self.node_to_validate + for processor in self.processors: + n = processor.traverse(n) + return n + + +class UpdateExpressionValidator(Validator): + def get_ast_processors(self): + """Get the different processors that go through the AST tree and processes the nodes.""" + processors = [ + ExpressionAttributeValueProcessor(self.expression_attribute_values), + ExpressionAttributeResolvingProcessor( + self.expression_attribute_names, self.item + ), + UpdateExpressionFunctionEvaluator(), + NoneExistingPathChecker(), + ExecuteOperations(), + ] + return processors diff --git a/moto/dynamodb2/responses.py b/moto/dynamodb2/responses.py index a5aeeac70..d14a54873 100644 --- a/moto/dynamodb2/responses.py +++ b/moto/dynamodb2/responses.py @@ -9,7 +9,7 @@ import six from moto.core.responses import BaseResponse from moto.core.utils import camelcase_to_underscores, amzn_request_id -from .exceptions import InvalidIndexNameError, InvalidUpdateExpression, ItemSizeTooLarge, MockValidationException +from .exceptions import InvalidIndexNameError, ItemSizeTooLarge, MockValidationException from moto.dynamodb2.models import dynamodb_backends, dynamo_json_dump diff --git a/tests/test_dynamodb2/test_dynamodb_expressions.py b/tests/test_dynamodb2/test_dynamodb_expressions.py index 1066231af..2c82d8bc4 100644 --- a/tests/test_dynamodb2/test_dynamodb_expressions.py +++ b/tests/test_dynamodb2/test_dynamodb_expressions.py @@ -393,3 +393,13 @@ def test_update_expression_parsing_is_not_keyword_aware(): except InvalidTokenException as te: assert te.token == "1" assert te.near == "VALUE 1" + + +def test_expression_if_not_exists_is_not_valid_in_remove_statement(): + set_action = "REMOVE if_not_exists(a,b)" + try: + UpdateExpressionParser.make(set_action) + assert False, "Exception not raised correctly" + except InvalidTokenException as te: + assert te.token == "(" + assert te.near == "if_not_exists(a" diff --git a/tests/test_dynamodb2/test_dynamodb_validation.py b/tests/test_dynamodb2/test_dynamodb_validation.py new file mode 100644 index 000000000..d60dd48f6 --- /dev/null +++ b/tests/test_dynamodb2/test_dynamodb_validation.py @@ -0,0 +1,464 @@ +from moto.dynamodb2.exceptions import ( + AttributeIsReservedKeyword, + ExpressionAttributeValueNotDefined, + AttributeDoesNotExist, + ExpressionAttributeNameNotDefined, + IncorrectOperandType, + InvalidUpdateExpressionInvalidDocumentPath, +) +from moto.dynamodb2.models import Item, DynamoType +from moto.dynamodb2.parsing.ast_nodes import ( + NodeDepthLeftTypeFetcher, + UpdateExpressionSetAction, + UpdateExpressionValue, + DDBTypedValue, +) +from moto.dynamodb2.parsing.expressions import UpdateExpressionParser +from moto.dynamodb2.parsing.validators import UpdateExpressionValidator +from parameterized import parameterized + + +def test_validation_of_update_expression_with_keyword(): + try: + update_expression = "SET myNum = path + :val" + update_expression_values = {":val": {"N": "3"}} + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "path": {"N": "3"}}, + ) + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=update_expression_values, + item=item, + ).validate() + assert False, "No exception raised" + except AttributeIsReservedKeyword as e: + assert e.keyword == "path" + + +@parameterized( + ["SET a = #b + :val2", "SET a = :val2 + #b",] +) +def test_validation_of_a_set_statement_with_incorrect_passed_value(update_expression): + """ + By running permutations it shows that values are replaced prior to resolving attributes. + + An error occurred (ValidationException) when calling the UpdateItem operation: Invalid UpdateExpression: + An expression attribute value used in expression is not defined; attribute value: :val2 + """ + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "b": {"N": "3"}}, + ) + try: + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names={"#b": "ok"}, + expression_attribute_values={":val": {"N": "3"}}, + item=item, + ).validate() + except ExpressionAttributeValueNotDefined as e: + assert e.attribute_value == ":val2" + + +def test_validation_of_update_expression_with_attribute_that_does_not_exist_in_item(): + """ + When an update expression tries to get an attribute that does not exist it must throw the appropriate exception. + + An error occurred (ValidationException) when calling the UpdateItem operation: + The provided expression refers to an attribute that does not exist in the item + """ + try: + update_expression = "SET a = nonexistent" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "path": {"N": "3"}}, + ) + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=None, + item=item, + ).validate() + assert False, "No exception raised" + except AttributeDoesNotExist: + assert True + + +@parameterized( + ["SET a = #c", "SET a = #c + #d",] +) +def test_validation_of_update_expression_with_attribute_name_that_is_not_defined( + update_expression, +): + """ + When an update expression tries to get an attribute name that is not provided it must throw an exception. + + An error occurred (ValidationException) when calling the UpdateItem operation: Invalid UpdateExpression: + An expression attribute name used in the document path is not defined; attribute name: #c + """ + try: + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "path": {"N": "3"}}, + ) + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names={"#b": "ok"}, + expression_attribute_values=None, + item=item, + ).validate() + assert False, "No exception raised" + except ExpressionAttributeNameNotDefined as e: + assert e.not_defined_attribute_name == "#c" + + +def test_validation_of_if_not_exists_not_existing_invalid_replace_value(): + try: + update_expression = "SET a = if_not_exists(b, a.c)" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "a": {"S": "A"}}, + ) + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=None, + item=item, + ).validate() + assert False, "No exception raised" + except AttributeDoesNotExist: + assert True + + +def get_first_node_of_type(ast, node_type): + return next(NodeDepthLeftTypeFetcher(node_type, ast)) + + +def get_set_action_value(ast): + """ + Helper that takes an AST and gets the first UpdateExpressionSetAction and retrieves the value of that action. + This should only be called on validated expressions. + Args: + ast(Node): + + Returns: + DynamoType: The DynamoType object representing the Dynamo value. + """ + set_action = get_first_node_of_type(ast, UpdateExpressionSetAction) + typed_value = set_action.children[1] + assert isinstance(typed_value, DDBTypedValue) + dynamo_value = typed_value.children[0] + assert isinstance(dynamo_value, DynamoType) + return dynamo_value + + +def test_validation_of_if_not_exists_not_existing_value(): + update_expression = "SET a = if_not_exists(b, a)" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "a": {"S": "A"}}, + ) + validated_ast = UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=None, + item=item, + ).validate() + dynamo_value = get_set_action_value(validated_ast) + assert dynamo_value == DynamoType({"S": "A"}) + + +def test_validation_of_if_not_exists_with_existing_attribute_should_return_attribute(): + update_expression = "SET a = if_not_exists(b, a)" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "a": {"S": "A"}, "b": {"S": "B"}}, + ) + validated_ast = UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=None, + item=item, + ).validate() + dynamo_value = get_set_action_value(validated_ast) + assert dynamo_value == DynamoType({"S": "B"}) + + +def test_validation_of_if_not_exists_with_existing_attribute_should_return_value(): + update_expression = "SET a = if_not_exists(b, :val)" + update_expression_values = {":val": {"N": "4"}} + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "b": {"N": "3"}}, + ) + validated_ast = UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=update_expression_values, + item=item, + ).validate() + dynamo_value = get_set_action_value(validated_ast) + assert dynamo_value == DynamoType({"N": "3"}) + + +def test_validation_of_if_not_exists_with_non_existing_attribute_should_return_value(): + update_expression = "SET a = if_not_exists(b, :val)" + update_expression_values = {":val": {"N": "4"}} + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}}, + ) + validated_ast = UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=update_expression_values, + item=item, + ).validate() + dynamo_value = get_set_action_value(validated_ast) + assert dynamo_value == DynamoType({"N": "4"}) + + +def test_validation_of_sum_operation(): + update_expression = "SET a = a + b" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "a": {"N": "3"}, "b": {"N": "4"}}, + ) + validated_ast = UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=None, + item=item, + ).validate() + dynamo_value = get_set_action_value(validated_ast) + assert dynamo_value == DynamoType({"N": "7"}) + + +def test_validation_homogeneous_list_append_function(): + update_expression = "SET ri = list_append(ri, :vals)" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "ri": {"L": [{"S": "i1"}, {"S": "i2"}]}}, + ) + validated_ast = UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values={":vals": {"L": [{"S": "i3"}, {"S": "i4"}]}}, + item=item, + ).validate() + dynamo_value = get_set_action_value(validated_ast) + assert dynamo_value == DynamoType( + {"L": [{"S": "i1"}, {"S": "i2"}, {"S": "i3"}, {"S": "i4"}]} + ) + + +def test_validation_hetereogenous_list_append_function(): + update_expression = "SET ri = list_append(ri, :vals)" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "ri": {"L": [{"S": "i1"}, {"S": "i2"}]}}, + ) + validated_ast = UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values={":vals": {"L": [{"N": "3"}]}}, + item=item, + ).validate() + dynamo_value = get_set_action_value(validated_ast) + assert dynamo_value == DynamoType({"L": [{"S": "i1"}, {"S": "i2"}, {"N": "3"}]}) + + +def test_validation_list_append_function_with_non_list_arg(): + """ + Must error out: + Invalid UpdateExpression: Incorrect operand type for operator or function; + operator or function: list_append, operand type: S' + Returns: + + """ + try: + update_expression = "SET ri = list_append(ri, :vals)" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "ri": {"L": [{"S": "i1"}, {"S": "i2"}]}}, + ) + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values={":vals": {"S": "N"}}, + item=item, + ).validate() + except IncorrectOperandType as e: + assert e.operand_type == "S" + assert e.operator_or_function == "list_append" + + +def test_sum_with_incompatible_types(): + """ + Must error out: + Invalid UpdateExpression: Incorrect operand type for operator or function; operator or function: +, operand type: S' + Returns: + + """ + try: + update_expression = "SET ri = :val + :val2" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "ri": {"L": [{"S": "i1"}, {"S": "i2"}]}}, + ) + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values={":val": {"S": "N"}, ":val2": {"N": "3"}}, + item=item, + ).validate() + except IncorrectOperandType as e: + assert e.operand_type == "S" + assert e.operator_or_function == "+" + + +def test_validation_of_subraction_operation(): + update_expression = "SET ri = :val - :val2" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "1"}, "a": {"N": "3"}, "b": {"N": "4"}}, + ) + validated_ast = UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values={":val": {"N": "1"}, ":val2": {"N": "3"}}, + item=item, + ).validate() + dynamo_value = get_set_action_value(validated_ast) + assert dynamo_value == DynamoType({"N": "-2"}) + + +def test_cannot_index_into_a_string(): + """ + Must error out: + The document path provided in the update expression is invalid for update' + """ + try: + update_expression = "set itemstr[1]=:Item" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "foo2"}, "itemstr": {"S": "somestring"}}, + ) + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values={":Item": {"S": "string_update"}}, + item=item, + ).validate() + assert False, "Must raise exception" + except InvalidUpdateExpressionInvalidDocumentPath: + assert True + + +def test_validation_set_path_does_not_need_to_be_resolvable_when_setting_a_new_attribute(): + """If this step just passes we are happy enough""" + update_expression = "set d=a" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "foo2"}, "a": {"N": "3"}}, + ) + validated_ast = UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=None, + item=item, + ).validate() + dynamo_value = get_set_action_value(validated_ast) + assert dynamo_value == DynamoType({"N": "3"}) + + +def test_validation_set_path_does_not_need_to_be_resolvable_but_must_be_creatable_when_setting_a_new_attribute(): + try: + update_expression = "set d.e=a" + update_expression_ast = UpdateExpressionParser.make(update_expression) + item = Item( + hash_key=DynamoType({"S": "id"}), + hash_key_type="TYPE", + range_key=None, + range_key_type=None, + attrs={"id": {"S": "foo2"}, "a": {"N": "3"}}, + ) + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=None, + expression_attribute_values=None, + item=item, + ).validate() + assert False, "Must raise exception" + except InvalidUpdateExpressionInvalidDocumentPath: + assert True From e6b51a28ee884697cba89a68e5f9948880c25199 Mon Sep 17 00:00:00 2001 From: pvbouwel Date: Sun, 19 Apr 2020 16:50:53 +0100 Subject: [PATCH 11/22] Enable AST Validation This commit puts AST validation on the execution path. This means updates get validated prior to being executed. There were quite a few tests that were not working against Amazon DDB. These tests I considered broken and as such this commit adapts them such that they pass against Amazon DDB. test_update_item_on_map() => One of the SET actions would try to set a nested element by specifying the nesting on the path rather than by putting a map as a value for a non-existent key. This got changed. test_item_size_is_under_400KB => Used the keyword "item" which DDB doesn't like. Change to cont in order to keep the same sizings. => Secondly the size error messages differs a bit depending whether it is part of the update or part of a put_item. For an update it should be: Item size to update has exceeded the maximum allowed size otherwise it is Item size has exceeded the maximum allowed size' test_remove_top_level_attribute => Used a keyword item. Use ExpressionAttributeNames test_update_item_double_nested_remove => Used keywords name & first. Migrated to non-deprecated API and use ExpressionAttributeNames test_update_item_set & test_boto3_update_item_conditions_pass & test_boto3_update_item_conditions_pass_because_expect_not_exists & test_boto3_update_item_conditions_pass_because_expect_not_exists_by_compare_to_null & test_boto3_update_item_conditions_pass_because_expect_exists_by_compare_to_not_null & test_boto3_update_item_conditions_fail & test_boto3_update_item_conditions_fail_because_expect_not_exists & test_boto3_update_item_conditions_fail_because_expect_not_exists_by_compare_to_null => Were broken tests which had string literal instead of value placeholder --- moto/dynamodb2/exceptions.py | 11 ++++ moto/dynamodb2/models/__init__.py | 22 +++++-- tests/test_dynamodb2/test_dynamodb.py | 54 ++++++++++++---- .../test_dynamodb_table_without_range_key.py | 63 +++++++++++++------ 4 files changed, 117 insertions(+), 33 deletions(-) diff --git a/moto/dynamodb2/exceptions.py b/moto/dynamodb2/exceptions.py index a6acae071..5dd87ef6b 100644 --- a/moto/dynamodb2/exceptions.py +++ b/moto/dynamodb2/exceptions.py @@ -111,6 +111,17 @@ class ItemSizeTooLarge(MockValidationException): super(ItemSizeTooLarge, self).__init__(self.item_size_too_large_msg) +class ItemSizeToUpdateTooLarge(MockValidationException): + item_size_to_update_too_large_msg = ( + "Item size to update has exceeded the maximum allowed size" + ) + + def __init__(self): + super(ItemSizeToUpdateTooLarge, self).__init__( + self.item_size_to_update_too_large_msg + ) + + class IncorrectOperandType(InvalidUpdateExpression): inv_operand_msg = "Incorrect operand type for operator or function; operator or function: {f}, operand type: {t}" diff --git a/moto/dynamodb2/models/__init__.py b/moto/dynamodb2/models/__init__.py index 1f448f288..00825e06a 100644 --- a/moto/dynamodb2/models/__init__.py +++ b/moto/dynamodb2/models/__init__.py @@ -14,11 +14,16 @@ from moto.core import BaseBackend, BaseModel from moto.core.utils import unix_time from moto.core.exceptions import JsonRESTError from moto.dynamodb2.comparisons import get_filter_expression -from moto.dynamodb2.comparisons import get_expected, get_comparison_func -from moto.dynamodb2.exceptions import InvalidIndexNameError, ItemSizeTooLarge, InvalidUpdateExpression +from moto.dynamodb2.comparisons import get_expected +from moto.dynamodb2.exceptions import ( + InvalidIndexNameError, + ItemSizeTooLarge, + ItemSizeToUpdateTooLarge, +) from moto.dynamodb2.models.utilities import bytesize, attribute_is_list from moto.dynamodb2.models.dynamo_type import DynamoType from moto.dynamodb2.parsing.expressions import UpdateExpressionParser +from moto.dynamodb2.parsing.validators import UpdateExpressionValidator class DynamoJsonEncoder(json.JSONEncoder): @@ -151,7 +156,10 @@ class Item(BaseModel): if "." in key and attr not in self.attrs: raise ValueError # Setting nested attr not allowed if first attr does not exist yet elif attr not in self.attrs: - self.attrs[attr] = dyn_value # set new top-level attribute + try: + self.attrs[attr] = dyn_value # set new top-level attribute + except ItemSizeTooLarge: + raise ItemSizeToUpdateTooLarge() else: self.attrs[attr].set( ".".join(key.split(".")[1:]), dyn_value, list_index @@ -1202,7 +1210,7 @@ class DynamoDBBackend(BaseBackend): # E.g. `a = b + c` -> `a=b+c` if update_expression: # Parse expression to get validation errors - UpdateExpressionParser.make(update_expression) + update_expression_ast = UpdateExpressionParser.make(update_expression) update_expression = re.sub(r"\s*([=\+-])\s*", "\\1", update_expression) if all([table.hash_key_attr in key, table.range_key_attr in key]): @@ -1247,6 +1255,12 @@ class DynamoDBBackend(BaseBackend): item = table.get_item(hash_value, range_value) if update_expression: + UpdateExpressionValidator( + update_expression_ast, + expression_attribute_names=expression_attribute_names, + expression_attribute_values=expression_attribute_values, + item=item, + ).validate() item.update( update_expression, expression_attribute_names, diff --git a/tests/test_dynamodb2/test_dynamodb.py b/tests/test_dynamodb2/test_dynamodb.py index 09401d562..0004001bc 100644 --- a/tests/test_dynamodb2/test_dynamodb.py +++ b/tests/test_dynamodb2/test_dynamodb.py @@ -2147,13 +2147,33 @@ def test_update_item_on_map(): # Nonexistent nested attributes are supported for existing top-level attributes. table.update_item( Key={"forum_name": "the-key", "subject": "123"}, - UpdateExpression="SET body.#nested.#data = :tb, body.nested.#nonexistentnested.#data = :tb2", + UpdateExpression="SET body.#nested.#data = :tb", + ExpressionAttributeNames={"#nested": "nested", "#data": "data",}, + ExpressionAttributeValues={":tb": "new_value"}, + ) + # Running this against AWS DDB gives an exception so make sure it also fails.: + with assert_raises(client.exceptions.ClientError): + # botocore.exceptions.ClientError: An error occurred (ValidationException) when calling the UpdateItem + # operation: The document path provided in the update expression is invalid for update + table.update_item( + Key={"forum_name": "the-key", "subject": "123"}, + UpdateExpression="SET body.#nested.#nonexistentnested.#data = :tb2", + ExpressionAttributeNames={ + "#nested": "nested", + "#nonexistentnested": "nonexistentnested", + "#data": "data", + }, + ExpressionAttributeValues={":tb2": "other_value"}, + ) + + table.update_item( + Key={"forum_name": "the-key", "subject": "123"}, + UpdateExpression="SET body.#nested.#nonexistentnested = :tb2", ExpressionAttributeNames={ "#nested": "nested", "#nonexistentnested": "nonexistentnested", - "#data": "data", }, - ExpressionAttributeValues={":tb": "new_value", ":tb2": "other_value"}, + ExpressionAttributeValues={":tb2": {"data": "other_value"}}, ) resp = table.scan() @@ -2161,8 +2181,8 @@ def test_update_item_on_map(): {"nested": {"data": "new_value", "nonexistentnested": {"data": "other_value"}}} ) - # Test nested value for a nonexistent attribute. - with assert_raises(client.exceptions.ConditionalCheckFailedException): + # Test nested value for a nonexistent attribute throws a ClientError. + with assert_raises(client.exceptions.ClientError): table.update_item( Key={"forum_name": "the-key", "subject": "123"}, UpdateExpression="SET nonexistent.#nested = :tb", @@ -3184,7 +3204,10 @@ def test_remove_top_level_attribute(): TableName=table_name, Item={"id": {"S": "foo"}, "item": {"S": "bar"}} ) client.update_item( - TableName=table_name, Key={"id": {"S": "foo"}}, UpdateExpression="REMOVE item" + TableName=table_name, + Key={"id": {"S": "foo"}}, + UpdateExpression="REMOVE #i", + ExpressionAttributeNames={"#i": "item"}, ) # result = client.get_item(TableName=table_name, Key={"id": {"S": "foo"}})["Item"] @@ -3359,21 +3382,21 @@ def test_item_size_is_under_400KB(): assert_failure_due_to_item_size( func=client.put_item, TableName="moto-test", - Item={"id": {"S": "foo"}, "item": {"S": large_item}}, + Item={"id": {"S": "foo"}, "cont": {"S": large_item}}, ) assert_failure_due_to_item_size( - func=table.put_item, Item={"id": "bar", "item": large_item} + func=table.put_item, Item={"id": "bar", "cont": large_item} ) - assert_failure_due_to_item_size( + assert_failure_due_to_item_size_to_update( func=client.update_item, TableName="moto-test", Key={"id": {"S": "foo2"}}, - UpdateExpression="set item=:Item", + UpdateExpression="set cont=:Item", ExpressionAttributeValues={":Item": {"S": large_item}}, ) # Assert op fails when updating a nested item assert_failure_due_to_item_size( - func=table.put_item, Item={"id": "bar", "itemlist": [{"item": large_item}]} + func=table.put_item, Item={"id": "bar", "itemlist": [{"cont": large_item}]} ) assert_failure_due_to_item_size( func=client.put_item, @@ -3394,6 +3417,15 @@ def assert_failure_due_to_item_size(func, **kwargs): ) +def assert_failure_due_to_item_size_to_update(func, **kwargs): + with assert_raises(ClientError) as ex: + func(**kwargs) + ex.exception.response["Error"]["Code"].should.equal("ValidationException") + ex.exception.response["Error"]["Message"].should.equal( + "Item size to update has exceeded the maximum allowed size" + ) + + @mock_dynamodb2 # https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_Query.html#DDB-Query-request-KeyConditionExpression def test_hash_key_cannot_use_begins_with_operations(): diff --git a/tests/test_dynamodb2/test_dynamodb_table_without_range_key.py b/tests/test_dynamodb2/test_dynamodb_table_without_range_key.py index 08d7724f8..b5cc01c84 100644 --- a/tests/test_dynamodb2/test_dynamodb_table_without_range_key.py +++ b/tests/test_dynamodb2/test_dynamodb_table_without_range_key.py @@ -443,23 +443,40 @@ def test_update_item_nested_remove(): dict(returned_item).should.equal({"username": "steve", "Meta": {}}) -@mock_dynamodb2_deprecated +@mock_dynamodb2 def test_update_item_double_nested_remove(): - conn = boto.dynamodb2.connect_to_region("us-east-1") - table = Table.create("messages", schema=[HashKey("username")]) + conn = boto3.client("dynamodb", region_name="us-east-1") + conn.create_table( + TableName="messages", + KeySchema=[{"AttributeName": "username", "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": "username", "AttributeType": "S"}], + BillingMode="PAY_PER_REQUEST", + ) - data = {"username": "steve", "Meta": {"Name": {"First": "Steve", "Last": "Urkel"}}} - table.put_item(data=data) + item = { + "username": {"S": "steve"}, + "Meta": { + "M": {"Name": {"M": {"First": {"S": "Steve"}, "Last": {"S": "Urkel"}}}} + }, + } + conn.put_item(TableName="messages", Item=item) key_map = {"username": {"S": "steve"}} # Then remove the Meta.FullName field - conn.update_item("messages", key_map, update_expression="REMOVE Meta.Name.First") - - returned_item = table.get_item(username="steve") - dict(returned_item).should.equal( - {"username": "steve", "Meta": {"Name": {"Last": "Urkel"}}} + conn.update_item( + TableName="messages", + Key=key_map, + UpdateExpression="REMOVE Meta.#N.#F", + ExpressionAttributeNames={"#N": "Name", "#F": "First"}, ) + returned_item = conn.get_item(TableName="messages", Key=key_map) + expected_item = { + "username": {"S": "steve"}, + "Meta": {"M": {"Name": {"M": {"Last": {"S": "Urkel"}}}}}, + } + dict(returned_item["Item"]).should.equal(expected_item) + @mock_dynamodb2_deprecated def test_update_item_set(): @@ -471,7 +488,10 @@ def test_update_item_set(): key_map = {"username": {"S": "steve"}} conn.update_item( - "messages", key_map, update_expression="SET foo=bar, blah=baz REMOVE SentBy" + "messages", + key_map, + update_expression="SET foo=:bar, blah=:baz REMOVE SentBy", + expression_attribute_values={":bar": {"S": "bar"}, ":baz": {"S": "baz"}}, ) returned_item = table.get_item(username="steve") @@ -616,8 +636,9 @@ def test_boto3_update_item_conditions_fail(): table.put_item(Item={"username": "johndoe", "foo": "baz"}) table.update_item.when.called_with( Key={"username": "johndoe"}, - UpdateExpression="SET foo=bar", + UpdateExpression="SET foo=:bar", Expected={"foo": {"Value": "bar"}}, + ExpressionAttributeValues={":bar": "bar"}, ).should.throw(botocore.client.ClientError) @@ -627,8 +648,9 @@ def test_boto3_update_item_conditions_fail_because_expect_not_exists(): table.put_item(Item={"username": "johndoe", "foo": "baz"}) table.update_item.when.called_with( Key={"username": "johndoe"}, - UpdateExpression="SET foo=bar", + UpdateExpression="SET foo=:bar", Expected={"foo": {"Exists": False}}, + ExpressionAttributeValues={":bar": "bar"}, ).should.throw(botocore.client.ClientError) @@ -638,8 +660,9 @@ def test_boto3_update_item_conditions_fail_because_expect_not_exists_by_compare_ table.put_item(Item={"username": "johndoe", "foo": "baz"}) table.update_item.when.called_with( Key={"username": "johndoe"}, - UpdateExpression="SET foo=bar", + UpdateExpression="SET foo=:bar", Expected={"foo": {"ComparisonOperator": "NULL"}}, + ExpressionAttributeValues={":bar": "bar"}, ).should.throw(botocore.client.ClientError) @@ -649,8 +672,9 @@ def test_boto3_update_item_conditions_pass(): table.put_item(Item={"username": "johndoe", "foo": "bar"}) table.update_item( Key={"username": "johndoe"}, - UpdateExpression="SET foo=baz", + UpdateExpression="SET foo=:baz", Expected={"foo": {"Value": "bar"}}, + ExpressionAttributeValues={":baz": "baz"}, ) returned_item = table.get_item(Key={"username": "johndoe"}) assert dict(returned_item)["Item"]["foo"].should.equal("baz") @@ -662,8 +686,9 @@ def test_boto3_update_item_conditions_pass_because_expect_not_exists(): table.put_item(Item={"username": "johndoe", "foo": "bar"}) table.update_item( Key={"username": "johndoe"}, - UpdateExpression="SET foo=baz", + UpdateExpression="SET foo=:baz", Expected={"whatever": {"Exists": False}}, + ExpressionAttributeValues={":baz": "baz"}, ) returned_item = table.get_item(Key={"username": "johndoe"}) assert dict(returned_item)["Item"]["foo"].should.equal("baz") @@ -675,8 +700,9 @@ def test_boto3_update_item_conditions_pass_because_expect_not_exists_by_compare_ table.put_item(Item={"username": "johndoe", "foo": "bar"}) table.update_item( Key={"username": "johndoe"}, - UpdateExpression="SET foo=baz", + UpdateExpression="SET foo=:baz", Expected={"whatever": {"ComparisonOperator": "NULL"}}, + ExpressionAttributeValues={":baz": "baz"}, ) returned_item = table.get_item(Key={"username": "johndoe"}) assert dict(returned_item)["Item"]["foo"].should.equal("baz") @@ -688,8 +714,9 @@ def test_boto3_update_item_conditions_pass_because_expect_exists_by_compare_to_n table.put_item(Item={"username": "johndoe", "foo": "bar"}) table.update_item( Key={"username": "johndoe"}, - UpdateExpression="SET foo=baz", + UpdateExpression="SET foo=:baz", Expected={"foo": {"ComparisonOperator": "NOT_NULL"}}, + ExpressionAttributeValues={":baz": "baz"}, ) returned_item = table.get_item(Key={"username": "johndoe"}) assert dict(returned_item)["Item"]["foo"].should.equal("baz") From 3a774ed0e0b22a42cf206533a9c4e6952089937f Mon Sep 17 00:00:00 2001 From: pvbouwel Date: Sun, 19 Apr 2020 17:55:00 +0100 Subject: [PATCH 12/22] Make sure reserved_keywords.txt is packaged with the library. --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 79b9875ee..b142f3203 100755 --- a/setup.py +++ b/setup.py @@ -100,4 +100,5 @@ setup( project_urls={ "Documentation": "http://docs.getmoto.org/en/latest/", }, + data_files=[('', ['moto/dynamodb2/parsing/reserved_keywords.txt'])], ) From 0d04306861f82cb7489b1d6d261dbaf3d6c745dd Mon Sep 17 00:00:00 2001 From: Asher Foa <1268088+asherf@users.noreply.github.com> Date: Sun, 19 Apr 2020 19:12:40 -0700 Subject: [PATCH 13/22] Fix deprecation warning. --- moto/ec2/urls.py | 2 +- moto/ssm/models.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/moto/ec2/urls.py b/moto/ec2/urls.py index 4d85b2f56..78f234320 100644 --- a/moto/ec2/urls.py +++ b/moto/ec2/urls.py @@ -2,6 +2,6 @@ from __future__ import unicode_literals from .responses import EC2Response -url_bases = ["https?://ec2\.(.+)\.amazonaws\.com(|\.cn)"] +url_bases = [r"https?://ec2\.(.+)\.amazonaws\.com(|\.cn)"] url_paths = {"{0}/": EC2Response.dispatch} diff --git a/moto/ssm/models.py b/moto/ssm/models.py index 201f43c5a..3ce3b3a22 100644 --- a/moto/ssm/models.py +++ b/moto/ssm/models.py @@ -651,7 +651,7 @@ class SimpleSystemManagerBackend(BaseBackend): label.startswith("aws") or label.startswith("ssm") or label[:1].isdigit() - or not re.match("^[a-zA-z0-9_\.\-]*$", label) + or not re.match(r"^[a-zA-z0-9_\.\-]*$", label) ): invalid_labels.append(label) continue From ad0805de0ec3546767f5c13141e3d072b8c6f496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20K=C3=A4ufl?= Date: Mon, 20 Apr 2020 09:19:24 +0200 Subject: [PATCH 14/22] Add Python 3.8 to trove classifiers --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 79b9875ee..684c0dcea 100755 --- a/setup.py +++ b/setup.py @@ -94,6 +94,7 @@ setup( "Programming Language :: Python :: 3.5", "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", + "Programming Language :: Python :: 3.8", "License :: OSI Approved :: Apache Software License", "Topic :: Software Development :: Testing", ], From b6789a2cc7d7ab9c2bd3fdd5b95d00e6fa20758d Mon Sep 17 00:00:00 2001 From: Tomoya Iwata Date: Tue, 21 Apr 2020 14:11:53 +0900 Subject: [PATCH 15/22] Added existence check of target thing to IoT ListThingPrincipals fix #2910 --- moto/iot/exceptions.py | 4 ++-- moto/iot/models.py | 8 ++++++++ tests/test_iot/test_iot.py | 7 +++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/moto/iot/exceptions.py b/moto/iot/exceptions.py index d114a12ad..7a578c221 100644 --- a/moto/iot/exceptions.py +++ b/moto/iot/exceptions.py @@ -7,10 +7,10 @@ class IoTClientError(JsonRESTError): class ResourceNotFoundException(IoTClientError): - def __init__(self): + def __init__(self, msg=None): self.code = 404 super(ResourceNotFoundException, self).__init__( - "ResourceNotFoundException", "The specified resource does not exist" + "ResourceNotFoundException", msg or "The specified resource does not exist" ) diff --git a/moto/iot/models.py b/moto/iot/models.py index de4383b96..51a23b6c6 100644 --- a/moto/iot/models.py +++ b/moto/iot/models.py @@ -805,6 +805,14 @@ class IoTBackend(BaseBackend): return thing_names def list_thing_principals(self, thing_name): + + things = [_ for _ in self.things.values() if _.thing_name == thing_name] + if len(things) == 0: + raise ResourceNotFoundException( + "Failed to list principals for thing %s because the thing does not exist in your account" + % thing_name + ) + principals = [ k[0] for k, v in self.principal_things.items() if k[1] == thing_name ] diff --git a/tests/test_iot/test_iot.py b/tests/test_iot/test_iot.py index f8c4f579c..f3c151714 100644 --- a/tests/test_iot/test_iot.py +++ b/tests/test_iot/test_iot.py @@ -728,6 +728,13 @@ def test_principal_thing(): res = client.list_thing_principals(thingName=thing_name) res.should.have.key("principals").which.should.have.length_of(0) + with assert_raises(ClientError) as e: + client.list_thing_principals(thingName='xxx') + + e.exception.response["Error"]["Code"].should.equal("ResourceNotFoundException") + e.exception.response["Error"]["Message"].should.equal( + "Failed to list principals for thing xxx because the thing does not exist in your account" + ) @mock_iot def test_delete_principal_thing(): From d9b782be0a6944426347378345b2289732a2c7d9 Mon Sep 17 00:00:00 2001 From: Tomoya Iwata Date: Tue, 21 Apr 2020 14:43:04 +0900 Subject: [PATCH 16/22] fix lint --- tests/test_iot/test_iot.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_iot/test_iot.py b/tests/test_iot/test_iot.py index f3c151714..2f43de5b9 100644 --- a/tests/test_iot/test_iot.py +++ b/tests/test_iot/test_iot.py @@ -729,13 +729,14 @@ def test_principal_thing(): res.should.have.key("principals").which.should.have.length_of(0) with assert_raises(ClientError) as e: - client.list_thing_principals(thingName='xxx') + client.list_thing_principals(thingName="xxx") e.exception.response["Error"]["Code"].should.equal("ResourceNotFoundException") e.exception.response["Error"]["Message"].should.equal( "Failed to list principals for thing xxx because the thing does not exist in your account" ) + @mock_iot def test_delete_principal_thing(): client = boto3.client("iot", region_name="ap-northeast-1") From 12669400b715ba9a3eb7759407fbf61f1283874c Mon Sep 17 00:00:00 2001 From: thatguysimon Date: Tue, 21 Apr 2020 16:53:22 +0300 Subject: [PATCH 17/22] Mark sts>get_caller_identity as implemented Seems like it's implemented but not marked --- IMPLEMENTATION_COVERAGE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/IMPLEMENTATION_COVERAGE.md b/IMPLEMENTATION_COVERAGE.md index 705618524..36caec175 100644 --- a/IMPLEMENTATION_COVERAGE.md +++ b/IMPLEMENTATION_COVERAGE.md @@ -7210,13 +7210,13 @@ - [ ] update_vtl_device_type ## sts -50% implemented +62% implemented - [X] assume_role - [ ] assume_role_with_saml - [X] assume_role_with_web_identity - [ ] decode_authorization_message - [ ] get_access_key_info -- [ ] get_caller_identity +- [X] get_caller_identity - [X] get_federation_token - [X] get_session_token From 156ba56fdc94414e45ff83763d04fca91b111513 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Mon, 15 Apr 2019 19:57:42 -0500 Subject: [PATCH 18/22] set default status for s3 posts and add support for success_action_redirect. --- moto/s3/responses.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/moto/s3/responses.py b/moto/s3/responses.py index 22cd45c08..2f52e0d4a 100644 --- a/moto/s3/responses.py +++ b/moto/s3/responses.py @@ -776,8 +776,9 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): template = self.response_template(S3_DELETE_BUCKET_WITH_ITEMS_ERROR) return 409, {}, template.render(bucket=removed_bucket) - def _bucket_response_post(self, request, body, bucket_name): - if not request.headers.get("Content-Length"): + def _bucket_response_post(self, request, body, bucket_name, headers): + response_headers = {} + if not request.headers.get('Content-Length'): return 411, {}, "Content-Length required" path = self._get_path(request) @@ -810,13 +811,21 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): else: f = request.files["file"].stream.read() + if 'success_action_redirect' in form: + response_headers['Location'] = form['success_action_redirect'] + + if 'success_action_status' in form: + status_code = form['success_action_status'] + else: + status_code = 204 + new_key = self.backend.set_key(bucket_name, key, f) # Metadata metadata = metadata_from_headers(form) new_key.set_metadata(metadata) - return 200, {}, "" + return status_code, response_headers, "" @staticmethod def _get_path(request): From b3f6e5ab2fed73cfc9f66de92b16cfa52e3602bc Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Wed, 29 May 2019 15:22:29 -0500 Subject: [PATCH 19/22] add test --- moto/s3/responses.py | 2 ++ tests/test_s3/test_s3.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/moto/s3/responses.py b/moto/s3/responses.py index 2f52e0d4a..5526646a3 100644 --- a/moto/s3/responses.py +++ b/moto/s3/responses.py @@ -816,6 +816,8 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): if 'success_action_status' in form: status_code = form['success_action_status'] + elif 'success_action_redirect' in form: + status_code = 303 else: status_code = 204 diff --git a/tests/test_s3/test_s3.py b/tests/test_s3/test_s3.py index 303ed523d..f7040e006 100644 --- a/tests/test_s3/test_s3.py +++ b/tests/test_s3/test_s3.py @@ -14,6 +14,7 @@ from io import BytesIO import mimetypes import zlib import pickle +import uuid import json import boto @@ -4428,3 +4429,34 @@ def test_s3_config_dict(): assert not logging_bucket["supplementaryConfiguration"].get( "BucketTaggingConfiguration" ) + + +@mock_s3 +def test_creating_presigned_post(): + bucket = 'presigned-test' + s3 = boto3.client('s3', region_name='us-east-1') + s3.create_bucket(Bucket=bucket) + success_url = 'http://localhost/completed' + fdata = b'test data\n' + file_uid = uuid.uuid4() + conditions = [ + {"Content-Type": 'text/plain'}, + {"x-amz-server-side-encryption": "AES256"}, + {'success_action_redirect': success_url}, + ] + conditions.append(["content-length-range", 1, 30]) + data = s3.generate_presigned_post( + Bucket=bucket, + Key='{file_uid}.txt'.format(file_uid=file_uid), + Fields={ + 'content-type': 'text/plain', + 'success_action_redirect': success_url, + 'x-amz-server-side-encryption': 'AES256' + }, + Conditions=conditions, + ExpiresIn=1000, + ) + resp = requests.post(data['url'], data=data['fields'], files={'file': fdata}, allow_redirects=False) + assert resp.headers['Location'] == url + assert resp.status_code == 303 + assert s3.get_object(Bucket=bucket, Key='{file_uuid}.txt'.format(file_uid=file_uid))['Body'].read() == fdata From 49b056563a2396727e17253d09e6924ce24ef09e Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Tue, 21 Apr 2020 19:51:48 -0500 Subject: [PATCH 20/22] process multipart form --- moto/s3/responses.py | 49 +++++++++++++++++++++++++++++----------- tests/test_s3/test_s3.py | 4 ++-- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/moto/s3/responses.py b/moto/s3/responses.py index 5526646a3..92a82e4ff 100644 --- a/moto/s3/responses.py +++ b/moto/s3/responses.py @@ -7,7 +7,7 @@ import six from botocore.awsrequest import AWSPreparedRequest from moto.core.utils import str_to_rfc_1123_datetime, py2_strip_unicode_keys -from six.moves.urllib.parse import parse_qs, urlparse, unquote +from six.moves.urllib.parse import parse_qs, urlparse, unquote, parse_qsl import xmltodict @@ -143,6 +143,31 @@ def is_delete_keys(request, path, bucket_name): ) +def _process_multipart_formdata(request): + """ + When not using the live server, the request does not pass through flask, so it is not processed. + This will only be used in places where we end up with a requests PreparedRequest. + """ + form = {} + boundkey = request.headers['Content-Type'][len('multipart/form-data; boundary='):] + boundary = f'--{boundkey}' + data = request.body.decode().split(boundary) + fields = [field.split('\r\n\r\n') for field in data][1:-1] + for key, value in fields: + key, value = key.replace('\r\n', ''), value.replace('\r\n', '') + key = key.split('; ') + if len(key) == 2: + disposition, name = key + filename = None + else: + disposition, name, filename = key + name = name[len('name='):].strip('"') + if disposition.endswith('form-data'): + form[name] = value + import code; code.interact(local=locals()) + return form + + class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): def __init__(self, backend): super(ResponseObject, self).__init__() @@ -776,9 +801,9 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): template = self.response_template(S3_DELETE_BUCKET_WITH_ITEMS_ERROR) return 409, {}, template.render(bucket=removed_bucket) - def _bucket_response_post(self, request, body, bucket_name, headers): + def _bucket_response_post(self, request, body, bucket_name): response_headers = {} - if not request.headers.get('Content-Length'): + if not request.headers.get("Content-Length"): return 411, {}, "Content-Length required" path = self._get_path(request) @@ -796,14 +821,12 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): if hasattr(request, "form"): # Not HTTPretty form = request.form + elif request.headers.get('Content-Type').startswith('multipart/form-data'): + form = _process_multipart_formdata(request) else: # HTTPretty, build new form object body = body.decode() - - form = {} - for kv in body.split("&"): - k, v = kv.split("=") - form[k] = v + form = dict(parse_qsl(body)) key = form["key"] if "file" in form: @@ -811,12 +834,12 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): else: f = request.files["file"].stream.read() - if 'success_action_redirect' in form: - response_headers['Location'] = form['success_action_redirect'] + if "success_action_redirect" in form: + response_headers["Location"] = form["success_action_redirect"] - if 'success_action_status' in form: - status_code = form['success_action_status'] - elif 'success_action_redirect' in form: + if "success_action_status" in form: + status_code = form["success_action_status"] + elif "success_action_redirect" in form: status_code = 303 else: status_code = 204 diff --git a/tests/test_s3/test_s3.py b/tests/test_s3/test_s3.py index f7040e006..c226a7b3b 100644 --- a/tests/test_s3/test_s3.py +++ b/tests/test_s3/test_s3.py @@ -4457,6 +4457,6 @@ def test_creating_presigned_post(): ExpiresIn=1000, ) resp = requests.post(data['url'], data=data['fields'], files={'file': fdata}, allow_redirects=False) - assert resp.headers['Location'] == url + assert resp.headers['Location'] == success_url assert resp.status_code == 303 - assert s3.get_object(Bucket=bucket, Key='{file_uuid}.txt'.format(file_uid=file_uid))['Body'].read() == fdata + assert s3.get_object(Bucket=bucket, Key='{file_uid}.txt'.format(file_uid=file_uid))['Body'].read() == fdata From 4b0ba7320433b4b66488fc851c828f2ec1b56836 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Tue, 21 Apr 2020 20:13:53 -0500 Subject: [PATCH 21/22] use werkzeug hooray, thanks pallets discord! --- moto/s3/responses.py | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/moto/s3/responses.py b/moto/s3/responses.py index 92a82e4ff..965d15f57 100644 --- a/moto/s3/responses.py +++ b/moto/s3/responses.py @@ -5,6 +5,7 @@ import sys import six from botocore.awsrequest import AWSPreparedRequest +from werkzeug.wrappers import Request from moto.core.utils import str_to_rfc_1123_datetime, py2_strip_unicode_keys from six.moves.urllib.parse import parse_qs, urlparse, unquote, parse_qsl @@ -143,31 +144,6 @@ def is_delete_keys(request, path, bucket_name): ) -def _process_multipart_formdata(request): - """ - When not using the live server, the request does not pass through flask, so it is not processed. - This will only be used in places where we end up with a requests PreparedRequest. - """ - form = {} - boundkey = request.headers['Content-Type'][len('multipart/form-data; boundary='):] - boundary = f'--{boundkey}' - data = request.body.decode().split(boundary) - fields = [field.split('\r\n\r\n') for field in data][1:-1] - for key, value in fields: - key, value = key.replace('\r\n', ''), value.replace('\r\n', '') - key = key.split('; ') - if len(key) == 2: - disposition, name = key - filename = None - else: - disposition, name, filename = key - name = name[len('name='):].strip('"') - if disposition.endswith('form-data'): - form[name] = value - import code; code.interact(local=locals()) - return form - - class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): def __init__(self, backend): super(ResponseObject, self).__init__() @@ -822,7 +798,13 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): # Not HTTPretty form = request.form elif request.headers.get('Content-Type').startswith('multipart/form-data'): - form = _process_multipart_formdata(request) + request = Request.from_values( + input_stream=six.BytesIO(request.body), + content_length=request.headers['Content-Length'], + content_type=request.headers['Content-Type'], + method='POST', + ) + form = request.form else: # HTTPretty, build new form object body = body.decode() From 80b27a6b93d0c52d8f9f5349ec87efd036a66247 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Tue, 21 Apr 2020 21:43:32 -0500 Subject: [PATCH 22/22] blacken --- moto/s3/responses.py | 8 ++++---- tests/test_s3/test_s3.py | 33 ++++++++++++++++++++------------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/moto/s3/responses.py b/moto/s3/responses.py index 965d15f57..6ac139a14 100644 --- a/moto/s3/responses.py +++ b/moto/s3/responses.py @@ -797,12 +797,12 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): if hasattr(request, "form"): # Not HTTPretty form = request.form - elif request.headers.get('Content-Type').startswith('multipart/form-data'): + elif request.headers.get("Content-Type").startswith("multipart/form-data"): request = Request.from_values( input_stream=six.BytesIO(request.body), - content_length=request.headers['Content-Length'], - content_type=request.headers['Content-Type'], - method='POST', + content_length=request.headers["Content-Length"], + content_type=request.headers["Content-Type"], + method="POST", ) form = request.form else: diff --git a/tests/test_s3/test_s3.py b/tests/test_s3/test_s3.py index c226a7b3b..ffbd73966 100644 --- a/tests/test_s3/test_s3.py +++ b/tests/test_s3/test_s3.py @@ -4433,30 +4433,37 @@ def test_s3_config_dict(): @mock_s3 def test_creating_presigned_post(): - bucket = 'presigned-test' - s3 = boto3.client('s3', region_name='us-east-1') + bucket = "presigned-test" + s3 = boto3.client("s3", region_name="us-east-1") s3.create_bucket(Bucket=bucket) - success_url = 'http://localhost/completed' - fdata = b'test data\n' + success_url = "http://localhost/completed" + fdata = b"test data\n" file_uid = uuid.uuid4() conditions = [ - {"Content-Type": 'text/plain'}, + {"Content-Type": "text/plain"}, {"x-amz-server-side-encryption": "AES256"}, - {'success_action_redirect': success_url}, + {"success_action_redirect": success_url}, ] conditions.append(["content-length-range", 1, 30]) data = s3.generate_presigned_post( Bucket=bucket, - Key='{file_uid}.txt'.format(file_uid=file_uid), + Key="{file_uid}.txt".format(file_uid=file_uid), Fields={ - 'content-type': 'text/plain', - 'success_action_redirect': success_url, - 'x-amz-server-side-encryption': 'AES256' + "content-type": "text/plain", + "success_action_redirect": success_url, + "x-amz-server-side-encryption": "AES256", }, Conditions=conditions, ExpiresIn=1000, ) - resp = requests.post(data['url'], data=data['fields'], files={'file': fdata}, allow_redirects=False) - assert resp.headers['Location'] == success_url + resp = requests.post( + data["url"], data=data["fields"], files={"file": fdata}, allow_redirects=False + ) + assert resp.headers["Location"] == success_url assert resp.status_code == 303 - assert s3.get_object(Bucket=bucket, Key='{file_uid}.txt'.format(file_uid=file_uid))['Body'].read() == fdata + assert ( + s3.get_object(Bucket=bucket, Key="{file_uid}.txt".format(file_uid=file_uid))[ + "Body" + ].read() + == fdata + )