Merge pull request #23 from xoeye/feature/no-single-unchanged-writes

CHANGES.md

@@ -1,3 +1,19 @@
+## 1.15.0
+
+Changes to `versioned_transact_write_items`:
+
+1. Any write to an item that leaves it in its pre-existing state
+   (`==`) will perform only a ConditionCheck and not an actual
+   Put/Delete on that item. This avoids having incrementing
+   `item_version` when nothing was actually changed.
+2. Single-item `versioned_transact_write_items` where the item is
+   unchanged (`==`) performs no ConditionCheck and no spurious
+   write. If you're not operating on multiple items, then there is no
+   meaningful 'transactionality' to knowing whether the item was
+   changed _after_ your transaction started, because transactions
+   can't assert anything about the future - only about a conjunction
+   between multiple items at a point in time.
+
 ## 1.14.0
 
 `StackContext` and `OnCallDefault` utilities for providing new ways of

tests/xoto3/dynamodb/write_versioned/ddb_api_test.py

@@ -2,11 +2,13 @@
 import pytest
 from botocore.exceptions import ClientError
 
+import xoto3.dynamodb.write_versioned as wv
 from xoto3.dynamodb.write_versioned import delete
 from xoto3.dynamodb.write_versioned.ddb_api import (
     built_transaction_to_transact_write_items_args,
     is_cancelled_and_retryable,
     known_key_schema,
+    make_transact_multiple_but_optimize_single,
 )
 from xoto3.dynamodb.write_versioned.errors import TableSchemaUnknownError, VersionedTransaction
 from xoto3.dynamodb.write_versioned.prepare import items_and_keys_to_clean_table_data
@@ -87,3 +89,24 @@ def test_built_transaction_includes_unmodified():
             },
         ]
     } == args
+
+
+def test_built_transaction_does_not_write_deep_equal_items():
+    tx = VersionedTransaction(dict())
+    table = wv.ItemTable("Foo")
+    item = dict(id="steve", val=3)
+    tx = table.presume(dict(id="steve"), item)(tx)
+    tx = table.put(item)(tx)
+
+    args = built_transaction_to_transact_write_items_args(tx, "adatetimestring")
+    effects = args["TransactItems"]
+    assert len(effects) == 1
+    assert set(effects[0]) == {"ConditionCheck"}
+
+
+def test_dont_call_the_client_if_theres_nothing_to_do():
+    no_client_transact = make_transact_multiple_but_optimize_single(None)
+    with pytest.raises(AttributeError):
+        no_client_transact([dict(Put=dict(some=1))])
+    no_client_transact([dict(ConditionCheck=dict(what=3))])
+    no_client_transact([])

tests/xoto3/dynamodb/write_versioned/run_test.py

@@ -281,3 +281,17 @@ def put_after_get(tx):
     )
 
     assert 9 == require(res, integration_test_id_table.name, dict(id=test_id_to_put))["val"]
+
+
+def test_dont_write_single_unchanged(integration_test_id_table_put, integration_test_id_table):
+    test_key = dict(id="versioned-transact-assert-item-version-unchanged")
+
+    integration_test_id_table_put(dict(test_key, val=4, item_version=3))
+
+    def put_after_get(tx):
+        a = require(tx, integration_test_id_table.name, test_key)
+        return put(tx, integration_test_id_table.name, a)
+
+    res = versioned_transact_write_items(put_after_get)
+
+    assert require(res, integration_test_id_table.name, test_key)["item_version"] == 3

xoto3/__about__.py

@@ -1,4 +1,4 @@
 """xoto3"""
-__version__ = "1.14.0"
+__version__ = "1.15.0"
 __author__ = "Peter Gaultney"
 __author_email__ = "pgaultney@xoi.io"

xoto3/dynamodb/write_versioned/ddb_api.py

@@ -123,8 +123,16 @@ def _ddb_batch_get_item(
 
 def make_transact_multiple_but_optimize_single(ddb_client):
     def boto3_transact_multiple_but_optimize_single(TransactItems: List[dict], **kwargs) -> Any:
+        if len(TransactItems) == 0:
+            _log.debug("Nothing to transact - returning")
+            return
+        # ClientRequestToken, if provided, indicates a desire to use
+        # certain idempotency guarantees provided only by
+        # TransactWriteItems. I'm not sure if it's even relevant for a
+        # single-item operation, but it's an issue of expense, not
+        # correctness, to leave it in.
         if len(TransactItems) == 1 and "ClientRequestToken" not in kwargs:
-            # attempt single put or delete to halve the cost
+            # attempt simple condition-checked put or delete to halve the cost
             command = TransactItems[0]
             item_args = tuple(command.values())[0]  # first and only value is a dict of arguments
             if set(command) == {"Put"}:
@@ -133,7 +141,12 @@ def boto3_transact_multiple_but_optimize_single(TransactItems: List[dict], **kwa
             if set(command) == {"Delete"}:
                 ddb_client.delete_item(**{**item_args, **kwargs})
                 return
-            # we don't (yet) support single writee optimization for things other than Put or Delete
+            if set(command) == {"ConditionCheck"}:
+                _log.debug(
+                    "Item was not modified and is solitary - no need to interact with the table"
+                )
+                return
+            # we don't (yet) support single write optimization for things other than Put or Delete
         ddb_client.transact_write_items(TransactItems=TransactItems, **kwargs)
 
     return boto3_transact_multiple_but_optimize_single
@@ -164,14 +177,9 @@ def _serialize_versioned_expr(expr: dict) -> dict:
 def built_transaction_to_transact_write_items_args(
     transaction: VersionedTransaction,
     last_written_at_str: str,
-    ClientRequestToken: str = "",
     item_version_attribute: str = "item_version",
     last_written_attribute: str = "last_written_at",
 ) -> dict:
-
-    args: Dict[str, Any] = dict(ClientRequestToken=ClientRequestToken)
-    if not ClientRequestToken:
-        args.pop("ClientRequestToken")
     transact_items = list()
     for table_name, tbl_data in transaction.tables.items():
         items, effects, key_attributes = tbl_data
@@ -185,7 +193,7 @@ def put_or_delete_item(item_hashable_key: HashableItemKey, effect: Optional[Item
             keys_of_items_to_be_modified.add(item_hashable_key)
             item = get_existing_item(item_hashable_key)
             expected_version = item.get(item_version_attribute, 0)
-            expression_ensuring_unmodifiedness = _serialize_versioned_expr(
+            expression_expecting_item_version = _serialize_versioned_expr(
                 versioned_item_expression(
                     expected_version,
                     item_version_key=item_version_attribute,
@@ -200,7 +208,7 @@ def put_or_delete_item(item_hashable_key: HashableItemKey, effect: Optional[Item
                         Key=serialize_item(
                             dict(hashable_key_to_key(key_attributes, item_hashable_key))
                         ),
-                        **expression_ensuring_unmodifiedness,
+                        **expression_expecting_item_version,
                     )
                 )
 
@@ -217,7 +225,7 @@ def put_or_delete_item(item_hashable_key: HashableItemKey, effect: Optional[Item
                             },
                         )
                     ),
-                    **expression_ensuring_unmodifiedness,
+                    **expression_expecting_item_version,
                 )
             )
 
@@ -232,7 +240,7 @@ def item_remains_unmodified(
             item_hashable_key: HashableItemKey, item: Optional[Item]
         ) -> dict:
             """This will also check that the item still does not exist if it previously did not"""
-            expression_ensuring_unmodifiedness = _serialize_versioned_expr(
+            expression_expecting_item_version = _serialize_versioned_expr(
                 versioned_item_expression(
                     get_existing_item(item_hashable_key).get(item_version_attribute, 0),
                     item_version_key=item_version_attribute,
@@ -245,7 +253,7 @@ def item_remains_unmodified(
                     Key=serialize_item(
                         dict(hashable_key_to_key(key_attributes, item_hashable_key))
                     ),
-                    **expression_ensuring_unmodifiedness,
+                    **expression_expecting_item_version,
                 )
             )
 
@@ -257,5 +265,4 @@ def item_remains_unmodified(
             ]
         )
 
-    args["TransactItems"] = transact_items
-    return args
+    return dict(TransactItems=transact_items)

xoto3/dynamodb/write_versioned/modify.py

@@ -71,14 +71,17 @@ def _write(
     )
 
     hashable_item_key = hashable_key(item_key)
+    if hashable_item_key in items and items[hashable_item_key] == item_or_none:
+        """You've asked us to write an effect that would have no effect, and we are dropping it"""
+        new_effects = {k: v for k, v in effects.items() if k != hashable_item_key}
+    else:
+        new_effects = {**effects, hashable_item_key: item_or_none}
 
     return VersionedTransaction(
         tables={
             **transaction.tables,
             table_name: _TableData(
-                items=items,
-                effects={**effects, hashable_item_key: item_or_none},
-                key_attributes=key_attributes,
+                items=items, effects=new_effects, key_attributes=key_attributes,
             ),
         }
     )

xoto3/dynamodb/write_versioned/run.py

@@ -60,6 +60,10 @@ def versioned_transact_write_items(
     moment of the successful transaction completion, including items
     you did not need to modify.
 
+    An exception to the above rule is that if you interact with only
+    one item and you do not change it, we will not call
+    TransactWriteItems at all.
+
     Returns the completed transaction, which contains the resulting
     items as written to the table(s) at the completion of the
     transaction.
@@ -71,6 +75,7 @@ def versioned_transact_write_items(
     The implementation for transact_write_items will also optimize
     your DynamoDB costs by reverting to a simple (but still versioned)
     Put or Delete if you only operate on a single item.
+
     """
     batch_get_item, transact_write_items = boto3_impl_defaults(
         batch_get_item, transact_write_items,
@@ -90,7 +95,6 @@ def versioned_transact_write_items(
                 **built_transaction_to_transact_write_items_args(
                     built_transaction,
                     iso8601strict(datetime.utcnow()),
-                    "",
                     item_version_attribute,
                     last_written_attribute,
                 )