From 53871e5a8a8f3ffd95d2300f5589b3577b622e72 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Fri, 28 Nov 2014 14:53:33 +0100 Subject: [PATCH] Detecting concurrent edition and failing properly if needed --- taiga/projects/occ/mixins.py | 12 +- .../test_issues_resources.py | 55 +++++----- .../test_tasks_resources.py | 53 +++++---- .../test_userstories_resources.py | 53 +++++---- .../test_wiki_resources.py | 103 ++++++++++-------- tests/integration/test_occ.py | 8 +- tests/integration/test_userstories.py | 2 +- 7 files changed, 156 insertions(+), 130 deletions(-) diff --git a/taiga/projects/occ/mixins.py b/taiga/projects/occ/mixins.py index e7d3fe00..e4ef0244 100644 --- a/taiga/projects/occ/mixins.py +++ b/taiga/projects/occ/mixins.py @@ -26,16 +26,20 @@ class OCCResourceMixin(object): Rest Framework resource mixin for resources that need to have concurrent accesses and editions controlled. """ - def pre_save(self, obj): - current_version = obj.version - param_version = self.request.DATA.get('version', None) + def _validate_and_update_version(self, obj): + current_version = None + if obj.id: + current_version = type(obj).objects.model.objects.get(id=obj.id).version - if obj.id is not None and current_version != param_version: + param_version = self.request.DATA.get('version', None) + if current_version != param_version: raise exc.WrongArguments({"version": "The version doesn't match with the current one"}) if obj.id: obj.version = models.F('version') + 1 + def pre_save(self, obj): + self._validate_and_update_version(obj) super().pre_save(obj) def post_save(self, obj, created=False): diff --git a/tests/integration/resources_permissions/test_issues_resources.py b/tests/integration/resources_permissions/test_issues_resources.py index 0e32b5e0..d4895443 100644 --- a/tests/integration/resources_permissions/test_issues_resources.py +++ b/tests/integration/resources_permissions/test_issues_resources.py @@ -7,6 +7,9 @@ from taiga.base.utils import json from tests import factories as f from tests.utils import helper_test_http_method, disconnect_signals, reconnect_signals from taiga.projects.votes.services import add_vote +from taiga.projects.occ import OCCResourceMixin + +from unittest import mock import pytest pytestmark = pytest.mark.django_db @@ -132,23 +135,24 @@ def test_issue_update(client, data): data.project_owner ] - issue_data = IssueSerializer(data.public_issue).data - issue_data["subject"] = "test" - issue_data = json.dumps(issue_data) - results = helper_test_http_method(client, 'put', public_url, issue_data, users) - assert results == [401, 403, 403, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + issue_data = IssueSerializer(data.public_issue).data + issue_data["subject"] = "test" + issue_data = json.dumps(issue_data) + results = helper_test_http_method(client, 'put', public_url, issue_data, users) + assert results == [401, 403, 403, 200, 200] - issue_data = IssueSerializer(data.private_issue1).data - issue_data["subject"] = "test" - issue_data = json.dumps(issue_data) - results = helper_test_http_method(client, 'put', private_url1, issue_data, users) - assert results == [401, 403, 403, 200, 200] + issue_data = IssueSerializer(data.private_issue1).data + issue_data["subject"] = "test" + issue_data = json.dumps(issue_data) + results = helper_test_http_method(client, 'put', private_url1, issue_data, users) + assert results == [401, 403, 403, 200, 200] - issue_data = IssueSerializer(data.private_issue2).data - issue_data["subject"] = "test" - issue_data = json.dumps(issue_data) - results = helper_test_http_method(client, 'put', private_url2, issue_data, users) - assert results == [401, 403, 403, 200, 200] + issue_data = IssueSerializer(data.private_issue2).data + issue_data["subject"] = "test" + issue_data = json.dumps(issue_data) + results = helper_test_http_method(client, 'put', private_url2, issue_data, users) + assert results == [401, 403, 403, 200, 200] def test_issue_delete(client, data): @@ -162,6 +166,7 @@ def test_issue_delete(client, data): data.project_member_without_perms, data.project_member_with_perms, ] + results = helper_test_http_method(client, 'delete', public_url, None, users) assert results == [401, 403, 403, 204] results = helper_test_http_method(client, 'delete', private_url1, None, users) @@ -261,17 +266,18 @@ def test_issue_patch(client, data): data.project_owner ] - patch_data = json.dumps({"subject": "test", "version": data.public_issue.version}) - results = helper_test_http_method(client, 'patch', public_url, patch_data, users) - assert results == [401, 403, 403, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + patch_data = json.dumps({"subject": "test", "version": data.public_issue.version}) + results = helper_test_http_method(client, 'patch', public_url, patch_data, users) + assert results == [401, 403, 403, 200, 200] - patch_data = json.dumps({"subject": "test", "version": data.private_issue1.version}) - results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) - assert results == [401, 403, 403, 200, 200] + patch_data = json.dumps({"subject": "test", "version": data.private_issue1.version}) + results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) + assert results == [401, 403, 403, 200, 200] - patch_data = json.dumps({"subject": "test", "version": data.private_issue2.version}) - results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) - assert results == [401, 403, 403, 200, 200] + patch_data = json.dumps({"subject": "test", "version": data.private_issue2.version}) + results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) + assert results == [401, 403, 403, 200, 200] def test_issue_bulk_create(client, data): @@ -303,7 +309,6 @@ def test_issue_bulk_create(client, data): data.project_owner ] - bulk_data = json.dumps({"bulk_issues": "test1\ntest2", "project_id": data.public_issue.project.pk}) results = helper_test_http_method(client, 'post', url, bulk_data, users) diff --git a/tests/integration/resources_permissions/test_tasks_resources.py b/tests/integration/resources_permissions/test_tasks_resources.py index 680164a3..7d63db6a 100644 --- a/tests/integration/resources_permissions/test_tasks_resources.py +++ b/tests/integration/resources_permissions/test_tasks_resources.py @@ -3,10 +3,13 @@ from django.core.urlresolvers import reverse from taiga.base.utils import json from taiga.projects.tasks.serializers import TaskSerializer from taiga.permissions.permissions import MEMBERS_PERMISSIONS, ANON_PERMISSIONS, USER_PERMISSIONS +from taiga.projects.occ import OCCResourceMixin from tests import factories as f from tests.utils import helper_test_http_method, disconnect_signals, reconnect_signals +from unittest import mock + import pytest pytestmark = pytest.mark.django_db @@ -132,23 +135,24 @@ def test_task_update(client, data): data.project_owner ] - task_data = TaskSerializer(data.public_task).data - task_data["subject"] = "test" - task_data = json.dumps(task_data) - results = helper_test_http_method(client, 'put', public_url, task_data, users) - assert results == [401, 403, 403, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + task_data = TaskSerializer(data.public_task).data + task_data["subject"] = "test" + task_data = json.dumps(task_data) + results = helper_test_http_method(client, 'put', public_url, task_data, users) + assert results == [401, 403, 403, 200, 200] - task_data = TaskSerializer(data.private_task1).data - task_data["subject"] = "test" - task_data = json.dumps(task_data) - results = helper_test_http_method(client, 'put', private_url1, task_data, users) - assert results == [401, 403, 403, 200, 200] + task_data = TaskSerializer(data.private_task1).data + task_data["subject"] = "test" + task_data = json.dumps(task_data) + results = helper_test_http_method(client, 'put', private_url1, task_data, users) + assert results == [401, 403, 403, 200, 200] - task_data = TaskSerializer(data.private_task2).data - task_data["subject"] = "test" - task_data = json.dumps(task_data) - results = helper_test_http_method(client, 'put', private_url2, task_data, users) - assert results == [401, 403, 403, 200, 200] + task_data = TaskSerializer(data.private_task2).data + task_data["subject"] = "test" + task_data = json.dumps(task_data) + results = helper_test_http_method(client, 'put', private_url2, task_data, users) + assert results == [401, 403, 403, 200, 200] def test_task_delete(client, data): @@ -252,17 +256,18 @@ def test_task_patch(client, data): data.project_owner ] - patch_data = json.dumps({"subject": "test", "version": data.public_task.version}) - results = helper_test_http_method(client, 'patch', public_url, patch_data, users) - assert results == [401, 403, 403, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + patch_data = json.dumps({"subject": "test", "version": data.public_task.version}) + results = helper_test_http_method(client, 'patch', public_url, patch_data, users) + assert results == [401, 403, 403, 200, 200] - patch_data = json.dumps({"subject": "test", "version": data.private_task1.version}) - results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) - assert results == [401, 403, 403, 200, 200] + patch_data = json.dumps({"subject": "test", "version": data.private_task1.version}) + results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) + assert results == [401, 403, 403, 200, 200] - patch_data = json.dumps({"subject": "test", "version": data.private_task2.version}) - results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) - assert results == [401, 403, 403, 200, 200] + patch_data = json.dumps({"subject": "test", "version": data.private_task2.version}) + results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) + assert results == [401, 403, 403, 200, 200] def test_task_action_bulk_create(client, data): diff --git a/tests/integration/resources_permissions/test_userstories_resources.py b/tests/integration/resources_permissions/test_userstories_resources.py index fe03d969..1b006935 100644 --- a/tests/integration/resources_permissions/test_userstories_resources.py +++ b/tests/integration/resources_permissions/test_userstories_resources.py @@ -3,10 +3,13 @@ from django.core.urlresolvers import reverse from taiga.base.utils import json from taiga.projects.userstories.serializers import UserStorySerializer from taiga.permissions.permissions import MEMBERS_PERMISSIONS, ANON_PERMISSIONS, USER_PERMISSIONS +from taiga.projects.occ import OCCResourceMixin from tests import factories as f from tests.utils import helper_test_http_method, disconnect_signals, reconnect_signals +from unittest import mock + import pytest pytestmark = pytest.mark.django_db @@ -130,23 +133,24 @@ def test_user_story_update(client, data): data.project_owner ] - user_story_data = UserStorySerializer(data.public_user_story).data - user_story_data["subject"] = "test" - user_story_data = json.dumps(user_story_data) - results = helper_test_http_method(client, 'put', public_url, user_story_data, users) - assert results == [401, 403, 403, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + user_story_data = UserStorySerializer(data.public_user_story).data + user_story_data["subject"] = "test" + user_story_data = json.dumps(user_story_data) + results = helper_test_http_method(client, 'put', public_url, user_story_data, users) + assert results == [401, 403, 403, 200, 200] - user_story_data = UserStorySerializer(data.private_user_story1).data - user_story_data["subject"] = "test" - user_story_data = json.dumps(user_story_data) - results = helper_test_http_method(client, 'put', private_url1, user_story_data, users) - assert results == [401, 403, 403, 200, 200] + user_story_data = UserStorySerializer(data.private_user_story1).data + user_story_data["subject"] = "test" + user_story_data = json.dumps(user_story_data) + results = helper_test_http_method(client, 'put', private_url1, user_story_data, users) + assert results == [401, 403, 403, 200, 200] - user_story_data = UserStorySerializer(data.private_user_story2).data - user_story_data["subject"] = "test" - user_story_data = json.dumps(user_story_data) - results = helper_test_http_method(client, 'put', private_url2, user_story_data, users) - assert results == [401, 403, 403, 200, 200] + user_story_data = UserStorySerializer(data.private_user_story2).data + user_story_data["subject"] = "test" + user_story_data = json.dumps(user_story_data) + results = helper_test_http_method(client, 'put', private_url2, user_story_data, users) + assert results == [401, 403, 403, 200, 200] def test_user_story_delete(client, data): @@ -235,17 +239,18 @@ def test_user_story_patch(client, data): data.project_owner ] - patch_data = json.dumps({"subject": "test", "version": data.public_user_story.version}) - results = helper_test_http_method(client, 'patch', public_url, patch_data, users) - assert results == [401, 403, 403, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + patch_data = json.dumps({"subject": "test", "version": data.public_user_story.version}) + results = helper_test_http_method(client, 'patch', public_url, patch_data, users) + assert results == [401, 403, 403, 200, 200] - patch_data = json.dumps({"subject": "test", "version": data.private_user_story1.version}) - results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) - assert results == [401, 403, 403, 200, 200] + patch_data = json.dumps({"subject": "test", "version": data.private_user_story1.version}) + results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) + assert results == [401, 403, 403, 200, 200] - patch_data = json.dumps({"subject": "test", "version": data.private_user_story2.version}) - results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) - assert results == [401, 403, 403, 200, 200] + patch_data = json.dumps({"subject": "test", "version": data.private_user_story2.version}) + results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) + assert results == [401, 403, 403, 200, 200] def test_user_story_action_bulk_create(client, data): diff --git a/tests/integration/resources_permissions/test_wiki_resources.py b/tests/integration/resources_permissions/test_wiki_resources.py index 800a6f4a..8d0ff278 100644 --- a/tests/integration/resources_permissions/test_wiki_resources.py +++ b/tests/integration/resources_permissions/test_wiki_resources.py @@ -4,10 +4,13 @@ from taiga.base.utils import json from taiga.projects.wiki.serializers import WikiPageSerializer, WikiLinkSerializer from taiga.projects.wiki.models import WikiPage, WikiLink from taiga.permissions.permissions import MEMBERS_PERMISSIONS, ANON_PERMISSIONS, USER_PERMISSIONS +from taiga.projects.occ import OCCResourceMixin from tests import factories as f from tests.utils import helper_test_http_method, disconnect_signals, reconnect_signals +from unittest import mock + import pytest pytestmark = pytest.mark.django_db @@ -121,23 +124,24 @@ def test_wiki_page_update(client, data): data.project_owner ] - wiki_page_data = WikiPageSerializer(data.public_wiki_page).data - wiki_page_data["content"] = "test" - wiki_page_data = json.dumps(wiki_page_data) - results = helper_test_http_method(client, 'put', public_url, wiki_page_data, users) - assert results == [401, 200, 200, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + wiki_page_data = WikiPageSerializer(data.public_wiki_page).data + wiki_page_data["content"] = "test" + wiki_page_data = json.dumps(wiki_page_data) + results = helper_test_http_method(client, 'put', public_url, wiki_page_data, users) + assert results == [401, 200, 200, 200, 200] - wiki_page_data = WikiPageSerializer(data.private_wiki_page1).data - wiki_page_data["content"] = "test" - wiki_page_data = json.dumps(wiki_page_data) - results = helper_test_http_method(client, 'put', private_url1, wiki_page_data, users) - assert results == [401, 200, 200, 200, 200] + wiki_page_data = WikiPageSerializer(data.private_wiki_page1).data + wiki_page_data["content"] = "test" + wiki_page_data = json.dumps(wiki_page_data) + results = helper_test_http_method(client, 'put', private_url1, wiki_page_data, users) + assert results == [401, 200, 200, 200, 200] - wiki_page_data = WikiPageSerializer(data.private_wiki_page2).data - wiki_page_data["content"] = "test" - wiki_page_data = json.dumps(wiki_page_data) - results = helper_test_http_method(client, 'put', private_url2, wiki_page_data, users) - assert results == [401, 403, 403, 200, 200] + wiki_page_data = WikiPageSerializer(data.private_wiki_page2).data + wiki_page_data["content"] = "test" + wiki_page_data = json.dumps(wiki_page_data) + results = helper_test_http_method(client, 'put', private_url2, wiki_page_data, users) + assert results == [401, 403, 403, 200, 200] def test_wiki_page_delete(client, data): @@ -238,17 +242,18 @@ def test_wiki_page_patch(client, data): data.project_owner ] - patch_data = json.dumps({"content": "test", "version": data.public_wiki_page.version}) - results = helper_test_http_method(client, 'patch', public_url, patch_data, users) - assert results == [401, 200, 200, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + patch_data = json.dumps({"content": "test", "version": data.public_wiki_page.version}) + results = helper_test_http_method(client, 'patch', public_url, patch_data, users) + assert results == [401, 200, 200, 200, 200] - patch_data = json.dumps({"content": "test", "version": data.private_wiki_page2.version}) - results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) - assert results == [401, 200, 200, 200, 200] + patch_data = json.dumps({"content": "test", "version": data.private_wiki_page2.version}) + results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) + assert results == [401, 200, 200, 200, 200] - patch_data = json.dumps({"content": "test", "version": data.private_wiki_page2.version}) - results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) - assert results == [401, 403, 403, 200, 200] + patch_data = json.dumps({"content": "test", "version": data.private_wiki_page2.version}) + results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) + assert results == [401, 403, 403, 200, 200] def test_wiki_page_action_render(client, data): url = reverse('wiki-render') @@ -300,23 +305,24 @@ def test_wiki_link_update(client, data): data.project_owner ] - wiki_link_data = WikiLinkSerializer(data.public_wiki_link).data - wiki_link_data["title"] = "test" - wiki_link_data = json.dumps(wiki_link_data) - results = helper_test_http_method(client, 'put', public_url, wiki_link_data, users) - assert results == [401, 200, 200, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + wiki_link_data = WikiLinkSerializer(data.public_wiki_link).data + wiki_link_data["title"] = "test" + wiki_link_data = json.dumps(wiki_link_data) + results = helper_test_http_method(client, 'put', public_url, wiki_link_data, users) + assert results == [401, 200, 200, 200, 200] - wiki_link_data = WikiLinkSerializer(data.private_wiki_link1).data - wiki_link_data["title"] = "test" - wiki_link_data = json.dumps(wiki_link_data) - results = helper_test_http_method(client, 'put', private_url1, wiki_link_data, users) - assert results == [401, 200, 200, 200, 200] + wiki_link_data = WikiLinkSerializer(data.private_wiki_link1).data + wiki_link_data["title"] = "test" + wiki_link_data = json.dumps(wiki_link_data) + results = helper_test_http_method(client, 'put', private_url1, wiki_link_data, users) + assert results == [401, 200, 200, 200, 200] - wiki_link_data = WikiLinkSerializer(data.private_wiki_link2).data - wiki_link_data["title"] = "test" - wiki_link_data = json.dumps(wiki_link_data) - results = helper_test_http_method(client, 'put', private_url2, wiki_link_data, users) - assert results == [401, 403, 403, 200, 200] + wiki_link_data = WikiLinkSerializer(data.private_wiki_link2).data + wiki_link_data["title"] = "test" + wiki_link_data = json.dumps(wiki_link_data) + results = helper_test_http_method(client, 'put', private_url2, wiki_link_data, users) + assert results == [401, 403, 403, 200, 200] def test_wiki_link_delete(client, data): @@ -417,14 +423,15 @@ def test_wiki_link_patch(client, data): data.project_owner ] - patch_data = json.dumps({"title": "test"}) - results = helper_test_http_method(client, 'patch', public_url, patch_data, users) - assert results == [401, 200, 200, 200, 200] + with mock.patch.object(OCCResourceMixin, "_validate_and_update_version") as _validate_and_update_version_mock: + patch_data = json.dumps({"title": "test"}) + results = helper_test_http_method(client, 'patch', public_url, patch_data, users) + assert results == [401, 200, 200, 200, 200] - patch_data = json.dumps({"title": "test"}) - results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) - assert results == [401, 200, 200, 200, 200] + patch_data = json.dumps({"title": "test"}) + results = helper_test_http_method(client, 'patch', private_url1, patch_data, users) + assert results == [401, 200, 200, 200, 200] - patch_data = json.dumps({"title": "test"}) - results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) - assert results == [401, 403, 403, 200, 200] + patch_data = json.dumps({"title": "test"}) + results = helper_test_http_method(client, 'patch', private_url2, patch_data, users) + assert results == [401, 403, 403, 200, 200] diff --git a/tests/integration/test_occ.py b/tests/integration/test_occ.py index 88597b12..1f0eae79 100644 --- a/tests/integration/test_occ.py +++ b/tests/integration/test_occ.py @@ -41,7 +41,7 @@ def test_invalid_concurrent_save_for_issue(client): mock_path = "taiga.projects.issues.api.IssueViewSet.pre_conditions_on_save" with patch(mock_path) as m: url = reverse("issues-detail", args=(issue.id,)) - data = {} + data = {"version":9} response = client.patch(url, json.dumps(data), content_type="application/json") assert response.status_code == 400 @@ -71,7 +71,7 @@ def test_invalid_concurrent_save_for_wiki_page(client): client.login(user) url = reverse("wiki-detail", args=(wiki_page.id,)) - data = {} + data = {"version":9} response = client.patch(url, json.dumps(data), content_type="application/json") assert response.status_code == 400 @@ -98,7 +98,7 @@ def test_invalid_concurrent_save_for_us(client): client.login(user) url = reverse("userstories-detail", args=(userstory.id,)) - data = {} + data = {"version":9} response = client.patch(url, json.dumps(data), content_type="application/json") assert response.status_code == 400 @@ -143,7 +143,7 @@ def test_invalid_concurrent_save_for_task(client): mock_path = "taiga.projects.tasks.api.TaskViewSet.pre_conditions_on_save" with patch(mock_path) as m: url = reverse("tasks-detail", args=(task.id,)) - data = {} + data = {"version":9} response = client.patch(url, json.dumps(data), content_type="application/json") assert response.status_code == 400 diff --git a/tests/integration/test_userstories.py b/tests/integration/test_userstories.py index d554c050..caad1435 100644 --- a/tests/integration/test_userstories.py +++ b/tests/integration/test_userstories.py @@ -149,7 +149,7 @@ def test_update_userstory_points(client): # Api should save successful data = {} - data["version"] = usdata["version"] + data["version"] = usdata["version"] + 1 data["points"] = copy.copy(usdata["points"]) data["points"].update({str(role1.pk):points3.pk})