Skip to content

Commit

Permalink
Merge branch 'pull-requests/66' into develop
Browse files Browse the repository at this point in the history
* pull-requests/66:
  update CHANGES
  tox.ini: do not install postgis if not  required
  update docs
  - Document new setting  - Fix loaddata test to pass without disabling concurrency  - Remove loaddata test that disable concurrency (now redundant)
  - fixes #36
  • Loading branch information
saxix committed Jul 15, 2016
2 parents a2350e8 + f8875aa commit 9a17194
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 25 deletions.
4 changes: 3 additions & 1 deletion CHANGES
@@ -1,8 +1,10 @@
Release 1.3 (dev)
-----------------
* drop support for Python < 3.3
* add support Django>1.10
* add support Django>=1.10
* change license
* fixes :issue:`36`. (thanks claytondaley)
* new :setting:`IGNORE_DEFAULT` to ignore default version number


Release 1.2 (05 Apr 2016)
Expand Down
15 changes: 15 additions & 0 deletions docs/settings.rst
Expand Up @@ -92,6 +92,21 @@ Default: ``CONCURRENCY_LIST_EDITABLE_POLICY_SILENT``

.. _list_editable_policies:

.. setting:: CONCURRENCY_IGNORE_DEFAULT

IGNORE_DEFAULT
--------------
.. versionadded:: >1.2

Default: ``True``

Determines whether a default version number is ignored or used in a concurrency check. While this
configuration defaults to True for backwards compatibility, this setting can cause omitted version
numbers to pass concurrency checks. New implementations are recommended to set this to ``False``.

.. note:: For security reasons, starting from version 1.5, default value will be ``False``.


``CONCURRENCY_LIST_EDITABLE_POLICY_SILENT``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Used by admin's integrations to handle ``list_editable`` conflicts.
Expand Down
4 changes: 3 additions & 1 deletion src/concurrency/config.py
Expand Up @@ -50,7 +50,9 @@ class AppSettings(object):
'FIELD_SIGNER': 'concurrency.forms.VersionFieldSigner',
'POLICY': CONCURRENCY_LIST_EDITABLE_POLICY_SILENT,
'CALLBACK': 'concurrency.views.callback',
'HANDLER409': 'concurrency.views.conflict'}
'HANDLER409': 'concurrency.views.conflict',
'IGNORE_DEFAULT': True,
}

def __init__(self, prefix):
"""
Expand Down
18 changes: 14 additions & 4 deletions src/concurrency/fields.py
Expand Up @@ -184,11 +184,23 @@ def _do_update(model_instance, base_qs, using, pk_val, values, update_fields, fo
# else:
# new_version = old_version
break
if values:

# This provides a default if either (1) no values were provided or (2) we reached this code as part of a
# create. We don't need to worry about a race condition because a competing create should produce an
# error anyway.
updated = base_qs.filter(pk=pk_val).exists()

# This second situation can occur because `Model.save_base` calls `Model._save_parent` without relaying
# the `force_insert` flag that marks the process as a create. Eventually, `Model._save_table` will call
# this function as-if it were in the middle of an update. The update is expected to fail because there
# is no object to update and the caller will fall back on the create logic instead. We need to ensure
# the update fails (but does not raise an exception) under this circumstance by skipping the concurrency
# logic.
if values and updated:
if (model_instance._concurrencymeta.enabled and
conf.ENABLED and
not getattr(model_instance, '_concurrency_disabled', False) and
old_version):
(old_version or not conf.IGNORE_DEFAULT)):
filter_kwargs = {'pk': pk_val, version_field.attname: old_version}
updated = base_qs.filter(**filter_kwargs)._update(values) >= 1
if not updated:
Expand All @@ -197,8 +209,6 @@ def _do_update(model_instance, base_qs, using, pk_val, values, update_fields, fo
else:
filter_kwargs = {'pk': pk_val}
updated = base_qs.filter(**filter_kwargs)._update(values) >= 1
else:
updated = base_qs.filter(pk=pk_val).exists()

return updated

Expand Down
18 changes: 18 additions & 0 deletions tests/test_base.py
@@ -1,4 +1,6 @@
import pytest
from django.test import override_settings

from demo.util import concurrent_model, unique_id, with_all_models, with_std_models

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


@pytest.mark.django_db(transaction=True)
@with_std_models
def test_conflict_no_version_and_no_skip_flag(model_class):
"""When IGNORE_DEFAULT is disabled, attempting to update a record with a default version number should fail."""
with override_settings(CONCURRENCY_IGNORE_DEFAULT=False):
id = next(unique_id)
instance, __ = model_class.objects.get_or_create(pk=id)
instance.save()

copy = refetch(instance)
copy.version = 0

with pytest.raises(RecordModifiedError):
copy.save()


@with_std_models
@pytest.mark.django_db(transaction=False)
def test_update_fields(model_class):
Expand Down
18 changes: 1 addition & 17 deletions tests/test_loaddata_dumpdata.py
Expand Up @@ -32,22 +32,6 @@ def test_loaddata_fail():
data = json.load(open(datafile, 'r'))
pk = data[0]['pk']

with pytest.raises(RecordModifiedError):
call_command('loaddata', datafile, stdout=StringIO())

assert not SimpleConcurrentModel.objects.filter(id=pk).exists()


@pytest.mark.django_db(transaction=False)
def test_loaddata():
datafile = os.path.join(os.path.dirname(__file__), 'dumpdata.json')
data = json.load(open(datafile, 'r'))
pk = data[0]['pk']

# with pytest.raises(RecordModifiedError):
# call_command('loaddata', datafile, stdout=StringIO())

with disable_concurrency():
call_command('loaddata', datafile, stdout=StringIO())
call_command('loaddata', datafile, stdout=StringIO())

assert SimpleConcurrentModel.objects.get(id=pk).username == 'loaded'
4 changes: 2 additions & 2 deletions tox.ini
Expand Up @@ -38,10 +38,10 @@ setenv =
sqlite: DBENGINE = sqlite

deps=
py{27,33,34,35}: psycopg2>=2.6.1
py{27,33,34,35}-{pg}: psycopg2>=2.6.1
pypy-d{18,19}-{pg}: psycopg2cffi

mysql: MySQL-python
mysql: mysqlclient

d18: django>=1.8,<1.9
d18: django-reversion==1.9.3
Expand Down

0 comments on commit 9a17194

Please sign in to comment.