Skip to content

Commit

Permalink
Locking the element on take_snapshot to avoid racing condition issues
Browse files Browse the repository at this point in the history
  • Loading branch information
superalex authored and bameda committed Feb 11, 2015
1 parent 985838c commit 50fd9b6
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 50 deletions.
1 change: 1 addition & 0 deletions requirements.txt
Expand Up @@ -30,6 +30,7 @@ django-ipware==0.1.0
premailer==2.8.1
django-transactional-cleanup==0.1.13
lxml==3.4.1
git+https://github.com/Xof/django-pglocks.git@dbb8d7375066859f897604132bd437832d2014ea

# Comment it if you are using python >= 3.4
enum34==1.0
103 changes: 53 additions & 50 deletions taiga/projects/history/services.py
Expand Up @@ -37,6 +37,7 @@ def create(request):
from django.core.paginator import Paginator, InvalidPage
from django.apps import apps
from django.db import transaction as tx
from django_pglocks import advisory_lock

from taiga.mdrender.service import render as mdrender
from taiga.base.utils.db import get_typename_for_model_class
Expand Down Expand Up @@ -269,6 +270,7 @@ def get_modified_fields(obj:object, last_modifications):

return modified_fields


@tx.atomic
def take_snapshot(obj:object, *, comment:str="", user=None, delete:bool=False):
"""
Expand All @@ -280,56 +282,57 @@ def take_snapshot(obj:object, *, comment:str="", user=None, delete:bool=False):
"""

key = make_key_from_model_object(obj)
typename = get_typename_for_model_class(obj.__class__)

new_fobj = freeze_model_instance(obj)
old_fobj, need_real_snapshot = get_last_snapshot_for_key(key)

entry_model = apps.get_model("history", "HistoryEntry")
user_id = None if user is None else user.id
user_name = "" if user is None else user.get_full_name()

# Determine history type
if delete:
entry_type = HistoryType.delete
elif new_fobj and not old_fobj:
entry_type = HistoryType.create
elif new_fobj and old_fobj:
entry_type = HistoryType.change
else:
raise RuntimeError("Unexpected condition")

fdiff = make_diff(old_fobj, new_fobj)

# If diff and comment are empty, do
# not create empty history entry
if (not fdiff.diff and not comment
and old_fobj is not None
and entry_type != HistoryType.delete):

return None

fvals = make_diff_values(typename, fdiff)

if len(comment) > 0:
is_hidden = False
else:
is_hidden = is_hidden_snapshot(fdiff)

kwargs = {
"user": {"pk": user_id, "name": user_name},
"key": key,
"type": entry_type,
"snapshot": fdiff.snapshot if need_real_snapshot else None,
"diff": fdiff.diff,
"values": fvals,
"comment": comment,
"comment_html": mdrender(obj.project, comment),
"is_hidden": is_hidden,
"is_snapshot": need_real_snapshot,
}

return entry_model.objects.create(**kwargs)
with advisory_lock(key) as acquired_key_lock:
typename = get_typename_for_model_class(obj.__class__)

new_fobj = freeze_model_instance(obj)
old_fobj, need_real_snapshot = get_last_snapshot_for_key(key)

entry_model = apps.get_model("history", "HistoryEntry")
user_id = None if user is None else user.id
user_name = "" if user is None else user.get_full_name()

# Determine history type
if delete:
entry_type = HistoryType.delete
elif new_fobj and not old_fobj:
entry_type = HistoryType.create
elif new_fobj and old_fobj:
entry_type = HistoryType.change
else:
raise RuntimeError("Unexpected condition")

fdiff = make_diff(old_fobj, new_fobj)

# If diff and comment are empty, do
# not create empty history entry
if (not fdiff.diff and not comment
and old_fobj is not None
and entry_type != HistoryType.delete):

return None

fvals = make_diff_values(typename, fdiff)

if len(comment) > 0:
is_hidden = False
else:
is_hidden = is_hidden_snapshot(fdiff)

kwargs = {
"user": {"pk": user_id, "name": user_name},
"key": key,
"type": entry_type,
"snapshot": fdiff.snapshot if need_real_snapshot else None,
"diff": fdiff.diff,
"values": fvals,
"comment": comment,
"comment_html": mdrender(obj.project, comment),
"is_hidden": is_hidden,
"is_snapshot": need_real_snapshot,
}

return entry_model.objects.create(**kwargs)


# High level query api
Expand Down

0 comments on commit 50fd9b6

Please sign in to comment.