Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'add_version_condition' arg #1177

Merged
merged 1 commit into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 53 additions & 2 deletions docs/optimistic_locking.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
.. _optimistic_locking:

==================
Optimistic Locking
==================
Expand All @@ -18,7 +20,16 @@ See also:
Version Attribute
-----------------

To enable optimistic locking for a table simply add a ``VersionAttribute`` to your model definition.
To enable optimistic locking for a table, add a ``VersionAttribute`` to your model definition. The presence of this
attribute will change the model's behaviors:

* :meth:`~pynamodb.models.Model.save` and :meth:`~pynamodb.models.Model.update` would increment the version attribute
every time the model is persisted. This allows concurrent updates not to overwrite each other, at the expense
of the latter update failing.
* :meth:`~pynamodb.models.Model.save`, :meth:`~pynamodb.models.Model.update`
and :meth:`~pynamodb.models.Model.delete` would fail if they are the "latter update" (by adding to the update's
:ref:`conditions <conditional_operations>`). This behavior is optional since sometimes a more granular approach
can be desired (see :ref:`optimistic_locking_version_condition`).

.. code-block:: python

Expand Down Expand Up @@ -86,7 +97,7 @@ These operations will fail if the local object is out-of-date.
except TransactWriteError as e:
assert isinstance(e.cause, ClientError)
assert e.cause_response_code == "TransactionCanceledException"
assert "ConditionalCheckFailed" in e.cause_response_message
assert any(r.code == "ConditionalCheckFailed" for r in e.cancellation_reasons)
else:
raise AssertionError("The version attribute conditional check should have failed.")

Expand All @@ -107,6 +118,46 @@ These operations will fail if the local object is out-of-date.
with assert_condition_check_fails():
office.delete()


.. _optimistic_locking_version_condition:

Conditioning on the version
---------------------------

To have :meth:`~pynamodb.models.Model.save`, :meth:`~pynamodb.models.Model.update` or :meth:`~pynamodb.models.Model.delete`
execute even if the item was changed by someone else, pass the ``add_version_condition=False`` parameter.
In this mode, updates would perform unconditionally but would still increment the version:
in other words, you could make other updates fail, but your update will succeed.

Done indiscriminately, this would be unsafe, but can be useful in certain scenarios:

#. For ``save``, this is almost always unsafe and undesirable.
#. For ``update``, use it when updating attributes for which a "last write wins" approach is acceptable,
or if you're otherwise conditioning the update in a way that is more domain-specific.
#. For ``delete``, use it to delete the item regardless of its contents.

For example, if your ``save`` operation experiences frequent "ConditionalCheckFailedException" failures,
rewrite your code to call ``update`` with individual attributes while passing :code:`add_version_condition=False`.
By disabling the version condition, you could no longer rely on the checks you've done prior to the modification (due to
what is known as the "time-of-check to time-of-use" problem). Therefore, consider adding domain-specific conditions
to ensure the item in the table is in the expected state prior to the update.

For example, let's consider a hotel room-booking service with the conventional constraint that only one person
can book a room at a time. We can switch from a ``save`` to an ``update`` by specifying the individual attributes
and rewriting the `if` statement as a condition:

.. code-block:: diff

- if room.booked_by:
- raise Exception("Room is already booked")
- room.booked_by = user_id
- room.save()
+ room.update(
+ actions=[Room.booked_by.set(user_id)],
+ condition=Room.booked_by.does_not_exist(),
+ add_version_condition=False,
+ )

Transactions
------------

Expand Down
6 changes: 6 additions & 0 deletions docs/release_notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
Release Notes
=============

v5.5.0
----------
* :meth:`~pynamodb.models.Model.save`, :meth:`~pynamodb.models.Model.update`, :meth:`~pynamodb.models.Model.delete_item`,
and :meth:`~pynamodb.models.Model.delete` now accept a ``add_version_condition`` parameter.
See :ref:`optimistic_locking_version_condition` for more details.

v5.4.1
----------
* Use model's AWS credentials in threads (#1164)
Expand Down
2 changes: 1 addition & 1 deletion pynamodb/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
"""
__author__ = 'Jharrod LaFon'
__license__ = 'MIT'
__version__ = '5.4.1'
__version__ = '5.5.0'
2 changes: 1 addition & 1 deletion pynamodb/attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ def deserialize(self, value):

class VersionAttribute(NumberAttribute):
"""
A version attribute
A number attribute that implements :ref:`optimistic locking <optimistic_locking>`.
"""
null = True

Expand Down
38 changes: 27 additions & 11 deletions pynamodb/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,26 +402,35 @@ def __repr__(self) -> str:
msg = "{}<{}>".format(self.Meta.table_name, hash_key)
return msg

def delete(self, condition: Optional[Condition] = None, settings: OperationSettings = OperationSettings.default) -> Any:
def delete(self, condition: Optional[Condition] = None, settings: OperationSettings = OperationSettings.default,
*, add_version_condition: bool = True) -> Any:
"""
Deletes this object from dynamodb
Deletes this object from DynamoDB.

:param add_version_condition: For models which have a :class:`~pynamodb.attributes.VersionAttribute`,
specifies whether the item should only be deleted if its current version matches the expected one.
Set to `False` for a 'delete anyway' strategy.
:raises pynamodb.exceptions.DeleteError: If the record can not be deleted
"""
hk_value, rk_value = self._get_hash_range_key_serialized_values()

version_condition = self._handle_version_attribute()
if version_condition is not None:
if add_version_condition and version_condition is not None:
condition &= version_condition

return self._get_connection().delete_item(hk_value, range_key=rk_value, condition=condition, settings=settings)

def update(self, actions: List[Action], condition: Optional[Condition] = None, settings: OperationSettings = OperationSettings.default) -> Any:
def update(self, actions: List[Action], condition: Optional[Condition] = None, settings: OperationSettings = OperationSettings.default, *, add_version_condition: bool = True) -> Any:
"""
Updates an item using the UpdateItem operation.

:param actions: a list of Action updates to apply
:param condition: an optional Condition on which to update
:param settings: per-operation settings
:param add_version_condition: For models which have a :class:`~pynamodb.attributes.VersionAttribute`,
specifies whether only to update if the version matches the model that is currently loaded.
Set to `False` for a 'last write wins' strategy.
Regardless, the version will always be incremented to prevent "rollbacks" by concurrent :meth:`save` calls.
:raises ModelInstance.DoesNotExist: if the object to be updated does not exist
:raises pynamodb.exceptions.UpdateError: if the `condition` is not met
"""
Expand All @@ -430,7 +439,7 @@ def update(self, actions: List[Action], condition: Optional[Condition] = None, s

hk_value, rk_value = self._get_hash_range_key_serialized_values()
version_condition = self._handle_version_attribute(actions=actions)
if version_condition is not None:
if add_version_condition and version_condition is not None:
condition &= version_condition

data = self._get_connection().update_item(hk_value, range_key=rk_value, return_values=ALL_NEW, condition=condition, actions=actions, settings=settings)
Expand All @@ -441,11 +450,11 @@ def update(self, actions: List[Action], condition: Optional[Condition] = None, s
self.deserialize(item_data)
return data

def save(self, condition: Optional[Condition] = None, settings: OperationSettings = OperationSettings.default) -> Dict[str, Any]:
def save(self, condition: Optional[Condition] = None, settings: OperationSettings = OperationSettings.default, *, add_version_condition: bool = True) -> Dict[str, Any]:
"""
Save this object to dynamodb
"""
args, kwargs = self._get_save_args(condition=condition)
args, kwargs = self._get_save_args(condition=condition, add_version_condition=add_version_condition)
kwargs['settings'] = settings
data = self._get_connection().put_item(*args, **kwargs)
self.update_local_version_attribute()
Expand Down Expand Up @@ -474,11 +483,13 @@ def get_update_kwargs_from_instance(
actions: List[Action],
condition: Optional[Condition] = None,
return_values_on_condition_failure: Optional[str] = None,
*,
add_version_condition: bool = True,
) -> Dict[str, Any]:
hk_value, rk_value = self._get_hash_range_key_serialized_values()

version_condition = self._handle_version_attribute(actions=actions)
if version_condition is not None:
if add_version_condition and version_condition is not None:
condition &= version_condition

return self._get_connection().get_operation_kwargs(hk_value, range_key=rk_value, key=KEY, actions=actions, condition=condition, return_values_on_condition_failure=return_values_on_condition_failure)
Expand All @@ -487,11 +498,13 @@ def get_delete_kwargs_from_instance(
self,
condition: Optional[Condition] = None,
return_values_on_condition_failure: Optional[str] = None,
*,
add_version_condition: bool = True,
) -> Dict[str, Any]:
hk_value, rk_value = self._get_hash_range_key_serialized_values()

version_condition = self._handle_version_attribute()
if version_condition is not None:
if add_version_condition and version_condition is not None:
condition &= version_condition

return self._get_connection().get_operation_kwargs(hk_value, range_key=rk_value, key=KEY, condition=condition, return_values_on_condition_failure=return_values_on_condition_failure)
Expand Down Expand Up @@ -900,14 +913,17 @@ def _get_schema(cls) -> Dict[str, Any]:
})
return schema

def _get_save_args(self, null_check: bool = True, condition: Optional[Condition] = None) -> Tuple[Iterable[Any], Dict[str, Any]]:
def _get_save_args(self, null_check: bool = True, condition: Optional[Condition] = None, *, add_version_condition: bool = True) -> Tuple[Iterable[Any], Dict[str, Any]]:
"""
Gets the proper *args, **kwargs for saving and retrieving this object

This is used for serializing items to be saved, or for serializing just the keys.

:param null_check: If True, then attributes are checked for null.
:param condition: If set, a condition
:param add_version_condition: For models which have a :class:`~pynamodb.attributes.VersionAttribute`,
specifies whether the item should only be saved if its current version matches the expected one.
Set to `False` for a 'last-write-wins' strategy.
"""
attribute_values = self.serialize(null_check)
hash_key_attribute = self._hash_key_attribute()
Expand All @@ -921,7 +937,7 @@ def _get_save_args(self, null_check: bool = True, condition: Optional[Condition]
if range_key is not None:
kwargs['range_key'] = range_key
version_condition = self._handle_version_attribute(attributes=attribute_values)
if version_condition is not None:
if add_version_condition and version_condition is not None:
condition &= version_condition
kwargs['attributes'] = attribute_values
kwargs['condition'] = condition
Expand Down
15 changes: 11 additions & 4 deletions pynamodb/transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,11 @@ def condition_check(self, model_cls: Type[_M], hash_key: _KeyType, range_key: Op
)
self._condition_check_items.append(operation_kwargs)

def delete(self, model: _M, condition: Optional[Condition] = None) -> None:
operation_kwargs = model.get_delete_kwargs_from_instance(condition=condition)
def delete(self, model: _M, condition: Optional[Condition] = None, *, add_version_condition: bool = True) -> None:
operation_kwargs = model.get_delete_kwargs_from_instance(
condition=condition,
add_version_condition=add_version_condition,
)
self._delete_items.append(operation_kwargs)

def save(self, model: _M, condition: Optional[Condition] = None, return_values: Optional[str] = None) -> None:
Expand All @@ -112,11 +115,15 @@ def save(self, model: _M, condition: Optional[Condition] = None, return_values:
self._put_items.append(operation_kwargs)
self._models_for_version_attribute_update.append(model)

def update(self, model: _M, actions: List[Action], condition: Optional[Condition] = None, return_values: Optional[str] = None) -> None:
def update(self, model: _M, actions: List[Action], condition: Optional[Condition] = None,
return_values: Optional[str] = None,
*,
add_version_condition: bool = True) -> None:
operation_kwargs = model.get_update_kwargs_from_instance(
actions=actions,
condition=condition,
return_values_on_condition_failure=return_values
return_values_on_condition_failure=return_values,
add_version_condition=add_version_condition,
)
self._update_items.append(operation_kwargs)
self._models_for_version_attribute_update.append(model)
Expand Down
31 changes: 25 additions & 6 deletions tests/integration/test_transaction_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,11 @@ def test_transaction_write_with_version_attribute(connection):
foo3 = Foo(3)
foo3.save()

foo42 = Foo(42)
foo42.save()
foo42_dup = Foo.get(42)
foo42_dup.save() # increment version w/o letting foo4 "know"

with TransactWrite(connection=connection) as transaction:
transaction.condition_check(Foo, 1, condition=(Foo.bar.exists()))
transaction.delete(foo2)
Expand All @@ -337,13 +342,23 @@ def test_transaction_write_with_version_attribute(connection):
Foo.star.set('birdistheword'),
]
)
transaction.update(
foo42,
actions=[
Foo.star.set('last write wins'),
],
add_version_condition=False,
)

assert Foo.get(1).version == 1
with pytest.raises(DoesNotExist):
Foo.get(2)
# Local object's version attribute is updated automatically.
assert foo3.version == 2
assert Foo.get(4).version == 1
foo42 = Foo.get(42)
assert foo42.version == foo42_dup.version + 1 == 3 # ensure version is incremented
assert foo42.star == 'last write wins' # ensure last write wins


@pytest.mark.ddblocal
Expand Down Expand Up @@ -372,8 +387,10 @@ def test_transaction_write_with_version_attribute_condition_failure(connection):
with pytest.raises(TransactWriteError) as exc_info:
with TransactWrite(connection=connection) as transaction:
transaction.save(Foo(21))
assert get_error_code(exc_info.value) == TRANSACTION_CANCELLED
assert 'ConditionalCheckFailed' in get_error_message(exc_info.value)
assert exc_info.value.cause_response_code == TRANSACTION_CANCELLED
assert len(exc_info.value.cancellation_reasons) == 1
assert exc_info.value.cancellation_reasons[0].code == 'ConditionalCheckFailed'
assert isinstance(exc_info.value.cause, botocore.exceptions.ClientError)
assert Foo.Meta.table_name in exc_info.value.cause.MSG_TEMPLATE

with pytest.raises(TransactWriteError) as exc_info:
Expand All @@ -384,15 +401,17 @@ def test_transaction_write_with_version_attribute_condition_failure(connection):
Foo.star.set('birdistheword'),
]
)
assert get_error_code(exc_info.value) == TRANSACTION_CANCELLED
assert 'ConditionalCheckFailed' in get_error_message(exc_info.value)
assert exc_info.value.cause_response_code == TRANSACTION_CANCELLED
assert len(exc_info.value.cancellation_reasons) == 1
assert exc_info.value.cancellation_reasons[0].code == 'ConditionalCheckFailed'
assert Foo.Meta.table_name in exc_info.value.cause.MSG_TEMPLATE
# Version attribute is not updated on failure.
assert foo2.version is None

with pytest.raises(TransactWriteError) as exc_info:
with TransactWrite(connection=connection) as transaction:
transaction.delete(foo2)
assert get_error_code(exc_info.value) == TRANSACTION_CANCELLED
assert 'ConditionalCheckFailed' in get_error_message(exc_info.value)
assert exc_info.value.cause_response_code == TRANSACTION_CANCELLED
assert len(exc_info.value.cancellation_reasons) == 1
assert exc_info.value.cancellation_reasons[0].code == 'ConditionalCheckFailed'
assert Foo.Meta.table_name in exc_info.value.cause.MSG_TEMPLATE
Loading