Skip to content

Commit 9154ba8

Browse files
claytondaleysaxix
authored andcommitted
- fixes #36
1 parent a2350e8 commit 9154ba8

File tree

3 files changed

+35
-5
lines changed

3 files changed

+35
-5
lines changed

src/concurrency/config.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ class AppSettings(object):
5050
'FIELD_SIGNER': 'concurrency.forms.VersionFieldSigner',
5151
'POLICY': CONCURRENCY_LIST_EDITABLE_POLICY_SILENT,
5252
'CALLBACK': 'concurrency.views.callback',
53-
'HANDLER409': 'concurrency.views.conflict'}
53+
'HANDLER409': 'concurrency.views.conflict',
54+
'IGNORE_DEFAULT': True,
55+
}
5456

5557
def __init__(self, prefix):
5658
"""

src/concurrency/fields.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,23 @@ def _do_update(model_instance, base_qs, using, pk_val, values, update_fields, fo
184184
# else:
185185
# new_version = old_version
186186
break
187-
if values:
187+
188+
# This provides a default if either (1) no values were provided or (2) we reached this code as part of a
189+
# create. We don't need to worry about a race condition because a competing create should produce an
190+
# error anyway.
191+
updated = base_qs.filter(pk=pk_val).exists()
192+
193+
# This second situation can occur because `Model.save_base` calls `Model._save_parent` without relaying
194+
# the `force_insert` flag that marks the process as a create. Eventually, `Model._save_table` will call
195+
# this function as-if it were in the middle of an update. The update is expected to fail because there
196+
# is no object to update and the caller will fall back on the create logic instead. We need to ensure
197+
# the update fails (but does not raise an exception) under this circumstance by skipping the concurrency
198+
# logic.
199+
if values and updated:
188200
if (model_instance._concurrencymeta.enabled and
189201
conf.ENABLED and
190202
not getattr(model_instance, '_concurrency_disabled', False) and
191-
old_version):
203+
(old_version or not conf.IGNORE_DEFAULT)):
192204
filter_kwargs = {'pk': pk_val, version_field.attname: old_version}
193205
updated = base_qs.filter(**filter_kwargs)._update(values) >= 1
194206
if not updated:
@@ -197,8 +209,6 @@ def _do_update(model_instance, base_qs, using, pk_val, values, update_fields, fo
197209
else:
198210
filter_kwargs = {'pk': pk_val}
199211
updated = base_qs.filter(**filter_kwargs)._update(values) >= 1
200-
else:
201-
updated = base_qs.filter(pk=pk_val).exists()
202212

203213
return updated
204214

tests/test_base.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import pytest
2+
from django.test import override_settings
3+
24
from demo.util import concurrent_model, unique_id, with_all_models, with_std_models
35

46
from concurrency.core import _set_version
@@ -54,6 +56,22 @@ def test_do_not_check_if_no_version(model_class):
5456
assert instance.get_concurrency_version() != copy.get_concurrency_version()
5557

5658

59+
@pytest.mark.django_db(transaction=True)
60+
@with_std_models
61+
def test_conflict_no_version_and_no_skip_flag(model_class):
62+
"""When IGNORE_DEFAULT is disabled, attempting to update a record with a default version number should fail."""
63+
with override_settings(CONCURRENCY_IGNORE_DEFAULT=False):
64+
id = next(unique_id)
65+
instance, __ = model_class.objects.get_or_create(pk=id)
66+
instance.save()
67+
68+
copy = refetch(instance)
69+
copy.version = 0
70+
71+
with pytest.raises(RecordModifiedError):
72+
copy.save()
73+
74+
5775
@with_std_models
5876
@pytest.mark.django_db(transaction=False)
5977
def test_update_fields(model_class):

0 commit comments

Comments
 (0)