Repository: getmoto/moto
Issue category: Feature Request

Issue:
DynamoDB should raise a ValidationError when ADD and DELETE statements include the same document path
DynamoDB does not support ADD and DELETE actions on the same string set in the same UpdateItem call. Here's a test to demonstrate:

```python
from unittest import TestCase

import boto3
from botocore.exceptions import ClientError
from moto import mock_aws


@mock_aws
class TestEmptyStringSet(TestCase):
    """
    DynamoDB will return a ValidationError, if you try to ADD and DELETE to the same String Set. This can be confirmed
    by running this test against a real AWS account, without the @mock_aws decorator.
    """

    def test_validation_error_on_delete_and_add(self):
        self._table.put_item(Item={'pk': 'foo', 'string_set': {'a', 'b'}})

        with self.assertRaises(ClientError) as e:
            self._table.update_item(
                Key={'pk': 'foo'},
                UpdateExpression='ADD #stringSet :addSet  DELETE #stringSet :deleteSet',
                ExpressionAttributeNames={'#stringSet': 'string_set'},
                ExpressionAttributeValues={':addSet': {'c'}, ':deleteSet': {'a'}},
            )
        self.assertEqual('ValidationException', e.exception.response['Error']['Code'])
        self.assertTrue(
            e.exception.response['Error']['Message'].startswith(
                'Invalid UpdateExpression: Two document paths overlap with each other;'
            )
        )

    def setUp(self):
        self._table_name = 'example-test-table'
        self._client = boto3.client('dynamodb')
        self._table = boto3.resource('dynamodb').create_table(
            AttributeDefinitions=[{'AttributeName': 'pk', 'AttributeType': 'S'}],
            TableName=self._table_name,
            KeySchema=[{'AttributeName': 'pk', 'KeyType': 'HASH'}],
            BillingMode='PAY_PER_REQUEST',
        )
        waiter = self._client.get_waiter('table_exists')
        waiter.wait(TableName=self._table_name)

    def tearDown(self):
        self._table.delete()
        waiter = self._client.get_waiter('table_not_exists')
        waiter.wait(TableName=self._table_name)
```


Hints:
Thanks for raising this @jusdino - I'll mark it as an enhancement to add validation for this scenario.

Patch:
diff --git a/moto/dynamodb/parsing/ast_nodes.py b/moto/dynamodb/parsing/ast_nodes.py
index 75971f8ac61a..a64898711c60 100644
--- a/moto/dynamodb/parsing/ast_nodes.py
+++ b/moto/dynamodb/parsing/ast_nodes.py
@@ -30,15 +30,29 @@ def set_parent(self, parent_node):
 
     def validate(self, limit_set_actions: bool = False) -> None:
         if self.type == "UpdateExpression":
-            if len(self.find_clauses([UpdateExpressionAddClause])) > 1:
+            add_clauses = self.find_clauses([UpdateExpressionAddClause])
+            remove_clauses = self.find_clauses([UpdateExpressionRemoveClause])
+
+            # Only allow single ADD/REMOVE clauses
+            if len(add_clauses) > 1:
                 raise TooManyClauses("ADD")
-            if len(self.find_clauses([UpdateExpressionRemoveClause])) > 1:
+            if len(remove_clauses) > 1:
                 raise TooManyClauses("REMOVE")
+
+            add_actions = self.find_clauses([UpdateExpressionAddAction])
+            delete_actions = self.find_clauses([UpdateExpressionDeleteAction])
             set_actions = self.find_clauses([UpdateExpressionSetAction])
-            # set_attributes = ["attr", "map.attr", attr.list[2], ..]
-            set_attributes = [s.children[0].to_str() for s in set_actions]
+
+            # Cannot ADD/DELETE to the same path
+            add_paths = [a.get_value() for a in add_actions]
+            delete_paths = [a.get_value() for a in delete_actions]
+            if set(add_paths).intersection(delete_paths):
+                raise DuplicateUpdateExpression(names=[*add_paths, *delete_paths])
+
+            # Ensure SET has no duplicates
             # We currently only check for duplicates
             # We should also check for partial duplicates, i.e. [attr, attr.sub] is also invalid
+            set_attributes = [s.children[0].to_str() for s in set_actions]
             duplicates = extract_duplicates(set_attributes)
             if duplicates:
                 # There might be more than one attribute duplicated:
@@ -160,6 +174,13 @@ class UpdateExpressionSetAction(UpdateExpressionClause):
     """
 
 
+class UpdateExpressionAction(UpdateExpressionClause):
+    def get_value(self):
+        expression_path = self.children[0]
+        expression_selector = expression_path.children[-1]
+        return expression_selector.children[0]
+
+
 class UpdateExpressionRemoveActions(UpdateExpressionClause):
     """
     UpdateExpressionSetClause => REMOVE RemoveActions
@@ -169,19 +190,14 @@ class UpdateExpressionRemoveActions(UpdateExpressionClause):
     """
 
 
-class UpdateExpressionRemoveAction(UpdateExpressionClause):
+class UpdateExpressionRemoveAction(UpdateExpressionAction):
     """
     RemoveAction => Path
     """
 
-    def _get_value(self):
-        expression_path = self.children[0]
-        expression_selector = expression_path.children[-1]
-        return expression_selector.children[0]
-
     def __lt__(self, other):
-        self_value = self._get_value()
-        other_value = other._get_value()
+        self_value = self.get_value()
+        other_value = other.get_value()
         if isinstance(self_value, int) and isinstance(other_value, int):
             return self_value < other_value
         else:
@@ -197,7 +213,7 @@ class UpdateExpressionAddActions(UpdateExpressionClause):
     """
 
 
-class UpdateExpressionAddAction(UpdateExpressionClause):
+class UpdateExpressionAddAction(UpdateExpressionAction):
     """
     AddAction => Path Value
     """
@@ -212,7 +228,7 @@ class UpdateExpressionDeleteActions(UpdateExpressionClause):
     """
 
 
-class UpdateExpressionDeleteAction(UpdateExpressionClause):
+class UpdateExpressionDeleteAction(UpdateExpressionAction):
     """
     DeleteAction => Path Value
     """


Test patch:
diff --git a/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py b/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py
index 8c662f5f5669..e0bb4c38d105 100644
--- a/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py
+++ b/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py
@@ -204,27 +204,57 @@ def test_empty_expressionattributenames_with_projection():
     assert err["Message"] == "ExpressionAttributeNames must not be empty"
 
 
-@mock_aws
-def test_update_item_range_key_set():
-    ddb = boto3.resource("dynamodb", region_name="us-east-1")
+@pytest.mark.aws_verified
+class TestUpdateExpressionClausesWithClashingExpressions(BaseTest):
+    def test_multiple_add_clauses(self):
+        with pytest.raises(ClientError) as exc:
+            self.table.update_item(
+                Key={"pk": "the-key"},
+                UpdateExpression="ADD x :one SET a = :a ADD y :one",
+                ExpressionAttributeValues={":one": 1, ":a": "lore ipsum"},
+            )
+        err = exc.value.response["Error"]
+        assert err["Code"] == "ValidationException"
+        assert (
+            err["Message"]
+            == 'Invalid UpdateExpression: The "ADD" section can only be used once in an update expression;'
+        )
 
-    # Create the DynamoDB table.
-    table = ddb.create_table(
-        TableName="test-table", BillingMode="PAY_PER_REQUEST", **table_schema
-    )
+    def test_multiple_remove_clauses(self):
+        with pytest.raises(ClientError) as exc:
+            self.table.update_item(
+                Key={"pk": "the-key"},
+                UpdateExpression="REMOVE #ulist[0] SET current_user = :current_user REMOVE some_param",
+                ExpressionAttributeNames={"#ulist": "user_list"},
+                ExpressionAttributeValues={":current_user": {"name": "Jane"}},
+            )
+        err = exc.value.response["Error"]
+        assert err["Code"] == "ValidationException"
+        assert (
+            err["Message"]
+            == 'Invalid UpdateExpression: The "REMOVE" section can only be used once in an update expression;'
+        )
 
-    with pytest.raises(ClientError) as exc:
-        table.update_item(
-            Key={"partitionKey": "the-key"},
-            UpdateExpression="ADD x :one SET a = :a ADD y :one",
-            ExpressionAttributeValues={":one": 1, ":a": "lore ipsum"},
+    def test_delete_and_add_on_same_set(self):
+        with pytest.raises(ClientError) as exc:
+            self.table.update_item(
+                Key={"pk": "foo"},
+                UpdateExpression="ADD #stringSet :addSet DELETE #stringSet :deleteSet",
+                ExpressionAttributeNames={"#stringSet": "string_set"},
+                ExpressionAttributeValues={":addSet": {"c"}, ":deleteSet": {"a"}},
+            )
+        assert exc.value.response["Error"]["Code"] == "ValidationException"
+        assert exc.value.response["Error"]["Message"].startswith(
+            "Invalid UpdateExpression: Two document paths overlap with each other;"
+        )
+
+    def test_delete_and_add_on_different_set(self):
+        self.table.update_item(
+            Key={"pk": "the-key"},
+            UpdateExpression="ADD #stringSet1 :addSet DELETE #stringSet2 :deleteSet",
+            ExpressionAttributeNames={"#stringSet1": "ss1", "#stringSet2": "ss2"},
+            ExpressionAttributeValues={":addSet": {"c"}, ":deleteSet": {"a"}},
         )
-    err = exc.value.response["Error"]
-    assert err["Code"] == "ValidationException"
-    assert (
-        err["Message"]
-        == 'Invalid UpdateExpression: The "ADD" section can only be used once in an update expression;'
-    )
 
 
 @pytest.mark.aws_verified
diff --git a/tests/test_dynamodb/test_dynamodb_executor.py b/tests/test_dynamodb/test_dynamodb_executor.py
index e176c51bb5a9..2939b41d5b2e 100644
--- a/tests/test_dynamodb/test_dynamodb_executor.py
+++ b/tests/test_dynamodb/test_dynamodb_executor.py
@@ -493,10 +493,10 @@ def test_normalize_with_multiple_actions__order_is_preserved(table):
     assert isinstance(validated_ast.children[0], UpdateExpressionAddAction)
     # Removal actions in reverse order
     assert isinstance(validated_ast.children[1], UpdateExpressionRemoveAction)
-    assert validated_ast.children[1]._get_value() == 3
+    assert validated_ast.children[1].get_value() == 3
     assert isinstance(validated_ast.children[2], UpdateExpressionRemoveAction)
-    assert validated_ast.children[2]._get_value() == 2
+    assert validated_ast.children[2].get_value() == 2
     assert isinstance(validated_ast.children[3], UpdateExpressionRemoveAction)
-    assert validated_ast.children[3]._get_value() == 1
+    assert validated_ast.children[3].get_value() == 1
     # Set action last, as per insertion order
     assert isinstance(validated_ast.children[4], UpdateExpressionSetAction)
diff --git a/tests/test_dynamodb/test_dynamodb_update_expressions.py b/tests/test_dynamodb/test_dynamodb_update_expressions.py
index a579c75c7a44..1d6283b77000 100644
--- a/tests/test_dynamodb/test_dynamodb_update_expressions.py
+++ b/tests/test_dynamodb/test_dynamodb_update_expressions.py
@@ -243,38 +243,6 @@ def test_update_item_with_empty_expression(table_name=None):
     )
 
 
-@pytest.mark.aws_verified
-@dynamodb_aws_verified()
-def test_update_expression_with_multiple_remove_clauses(table_name=None):
-    ddb_client = boto3.client("dynamodb", "us-east-1")
-    payload = {
-        "pk": {"S": "primary_key"},
-        "current_user": {"M": {"name": {"S": "John"}, "surname": {"S": "Doe"}}},
-        "user_list": {
-            "L": [
-                {"M": {"name": {"S": "John"}, "surname": {"S": "Doe"}}},
-                {"M": {"name": {"S": "Jane"}, "surname": {"S": "Smith"}}},
-            ]
-        },
-        "some_param": {"NULL": True},
-    }
-    ddb_client.put_item(TableName=table_name, Item=payload)
-    with pytest.raises(ClientError) as exc:
-        ddb_client.update_item(
-            TableName=table_name,
-            Key={"pk": {"S": "primary_key"}},
-            UpdateExpression="REMOVE #ulist[0] SET current_user = :current_user REMOVE some_param",
-            ExpressionAttributeNames={"#ulist": "user_list"},
-            ExpressionAttributeValues={":current_user": {"M": {"name": {"S": "Jane"}}}},
-        )
-    err = exc.value.response["Error"]
-    assert err["Code"] == "ValidationException"
-    assert (
-        err["Message"]
-        == 'Invalid UpdateExpression: The "REMOVE" section can only be used once in an update expression;'
-    )
-
-
 @pytest.mark.aws_verified
 @dynamodb_aws_verified()
 def test_update_expression_remove_list_and_attribute(table_name=None):