From 0a900cc84dd7035a75afd7a770d43a6a88fd54b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 26 Jul 2016 13:23:27 +0200 Subject: [PATCH 1/4] Fixed race condition on multiple attachment deletion --- taiga/projects/attachments/api.py | 2 +- taiga/projects/history/mixins.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/taiga/projects/attachments/api.py b/taiga/projects/attachments/api.py index 27f7ebe1..986c8f19 100644 --- a/taiga/projects/attachments/api.py +++ b/taiga/projects/attachments/api.py @@ -74,7 +74,7 @@ class BaseAttachmentViewSet(HistoryResourceMixin, WatchedResourceMixin, # NOTE: When destroy an attachment, the content_object change # after and not before self.persist_history_snapshot(obj, delete=True) - super().pre_delete(obj) + super().post_delete(obj) def get_object_for_snapshot(self, obj): return obj.content_object diff --git a/taiga/projects/history/mixins.py b/taiga/projects/history/mixins.py index 0a70366d..14a0f44d 100644 --- a/taiga/projects/history/mixins.py +++ b/taiga/projects/history/mixins.py @@ -62,7 +62,7 @@ class HistoryResourceMixin(object): obj = self.get_object() sobj = self.get_object_for_snapshot(obj) - if sobj != obj and delete: + if sobj != obj: delete = False notifications_services.analize_object_for_watchers(obj, comment, user) From 5214717856a95328c6cec569a03421625af4ffb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Wed, 27 Jul 2016 11:56:41 +0200 Subject: [PATCH 2/4] Fix some of the deadlocks on take_snapshot --- taiga/projects/history/services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taiga/projects/history/services.py b/taiga/projects/history/services.py index 764cca39..b9526b08 100644 --- a/taiga/projects/history/services.py +++ b/taiga/projects/history/services.py @@ -312,7 +312,7 @@ def take_snapshot(obj: object, *, comment: str="", user=None, delete: bool=False """ key = make_key_from_model_object(obj) - with advisory_lock(key): + with advisory_lock("history-"+key): typename = get_typename_for_model_class(obj.__class__) new_fobj = freeze_model_instance(obj) From 5eb6bfe1e09c0dcf30cecc1f91642dca224feaf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Wed, 27 Jul 2016 12:44:02 +0200 Subject: [PATCH 3/4] Only update tasks milestone when needed --- taiga/projects/userstories/signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taiga/projects/userstories/signals.py b/taiga/projects/userstories/signals.py index 11638595..fc1fdacc 100644 --- a/taiga/projects/userstories/signals.py +++ b/taiga/projects/userstories/signals.py @@ -59,7 +59,7 @@ def update_role_points_when_create_or_edit_us(sender, instance, **kwargs): def update_milestone_of_tasks_when_edit_us(sender, instance, created, **kwargs): if not created: - instance.tasks.update(milestone=instance.milestone) + instance.tasks.exclude(milestone=instance.milestone).update(milestone=instance.milestone) for task in instance.tasks.all(): take_snapshot(task) From 3e9bfd7523bcd83416a879e1c4bca347a576821a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Wed, 27 Jul 2016 13:16:36 +0200 Subject: [PATCH 4/4] Fixing slug duplications on race conditions --- taiga/base/decorators.py | 4 +-- taiga/projects/api.py | 24 ++++++++++-------- taiga/projects/milestones/models.py | 13 +++++----- taiga/projects/models.py | 38 ++++++++++++++--------------- taiga/projects/wiki/models.py | 12 ++++++--- taiga/users/models.py | 30 ++++++++++++----------- 6 files changed, 65 insertions(+), 56 deletions(-) diff --git a/taiga/base/decorators.py b/taiga/base/decorators.py index 5700e75b..46b80b24 100644 --- a/taiga/base/decorators.py +++ b/taiga/base/decorators.py @@ -18,6 +18,7 @@ from django_pglocks import advisory_lock + def detail_route(methods=['get'], **kwargs): """ Used to mark a method on a ViewSet that should be routed for detail requests. @@ -51,12 +52,11 @@ def model_pk_lock(func): """ def decorator(self, *args, **kwargs): from taiga.base.utils.db import get_typename_for_model_class - lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field pk = self.kwargs.get(self.pk_url_kwarg, None) tn = get_typename_for_model_class(self.get_queryset().model) key = "{0}:{1}".format(tn, pk) - with advisory_lock(key) as acquired_key_lock: + with advisory_lock(key): return func(self, *args, **kwargs) return decorator diff --git a/taiga/projects/api.py b/taiga/projects/api.py index 6445c17f..89fdffe2 100644 --- a/taiga/projects/api.py +++ b/taiga/projects/api.py @@ -26,6 +26,8 @@ from django.http import Http404 from django.utils.translation import ugettext as _ from django.utils import timezone +from django_pglocks import advisory_lock + from taiga.base import filters from taiga.base import exceptions as exc from taiga.base import response @@ -214,20 +216,22 @@ class ProjectViewSet(LikedResourceMixin, HistoryResourceMixin, if not template_description: raise response.BadRequest(_("Not valid template description")) - template_slug = slugify_uniquely(template_name, models.ProjectTemplate) + with advisory_lock("create-project-template") as acquired_key_lock: + template_slug = slugify_uniquely(template_name, models.ProjectTemplate) - project = self.get_object() + project = self.get_object() - self.check_permissions(request, 'create_template', project) + self.check_permissions(request, 'create_template', project) - template = models.ProjectTemplate( - name=template_name, - slug=template_slug, - description=template_description, - ) + template = models.ProjectTemplate( + name=template_name, + slug=template_slug, + description=template_description, + ) - template.load_data_from_project(project) - template.save() + template.load_data_from_project(project) + + template.save() return response.Created(serializers.ProjectTemplateSerializer(template).data) @detail_route(methods=['POST']) diff --git a/taiga/projects/milestones/models.py b/taiga/projects/milestones/models.py index 21d85b14..4488d178 100644 --- a/taiga/projects/milestones/models.py +++ b/taiga/projects/milestones/models.py @@ -16,9 +16,8 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -from django.apps import apps from django.db import models -from django.db.models import Prefetch, Count +from django.db.models import Count from django.conf import settings from django.utils.translation import ugettext_lazy as _ from django.utils import timezone @@ -28,7 +27,7 @@ from django.utils.functional import cached_property from taiga.base.utils.slug import slugify_uniquely from taiga.base.utils.dicts import dict_sum from taiga.projects.notifications.mixins import WatchedModelMixin -from taiga.projects.userstories.models import UserStory +from django_pglocks import advisory_lock import itertools import datetime @@ -84,9 +83,11 @@ class Milestone(WatchedModelMixin, models.Model): if not self._importing or not self.modified_date: self.modified_date = timezone.now() if not self.slug: - self.slug = slugify_uniquely(self.name, self.__class__) - - super().save(*args, **kwargs) + with advisory_lock("milestone-creation-{}".format(self.project_id)): + self.slug = slugify_uniquely(self.name, self.__class__) + super().save(*args, **kwargs) + else: + super().save(*args, **kwargs) @cached_property def cached_user_stories(self): diff --git a/taiga/projects/models.py b/taiga/projects/models.py index 83e36e54..2c0f4027 100644 --- a/taiga/projects/models.py +++ b/taiga/projects/models.py @@ -16,27 +16,23 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -import itertools -import uuid - from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.postgres.fields import ArrayField from django.core.exceptions import ValidationError from django.db import models -from django.db.models import signals, Q +from django.db.models import Q from django.apps import apps -from django.conf import settings -from django.dispatch import receiver from django.utils.translation import ugettext_lazy as _ from django.utils import timezone from django.utils.functional import cached_property +from django_pglocks import advisory_lock + from django_pgjson.fields import JsonField from taiga.projects.tagging.models import TaggedMixin from taiga.projects.tagging.models import TagsColorsdMixin -from taiga.base.utils.dicts import dict_sum from taiga.base.utils.files import get_file_path from taiga.base.utils.sequence import arithmetic_progression from taiga.base.utils.slug import slugify_uniquely @@ -270,16 +266,6 @@ class Project(ProjectDefaults, TaggedMixin, TagsColorsdMixin, models.Model): if not self._importing or not self.modified_date: self.modified_date = timezone.now() - if not self.slug: - base_name = "{}-{}".format(self.owner.username, self.name) - base_slug = slugify_uniquely(base_name, self.__class__) - slug = base_slug - for i in arithmetic_progression(): - if not type(self).objects.filter(slug=slug).exists() or i > 100: - break - slug = "{}-{}".format(base_slug, i) - self.slug = slug - if not self.is_backlog_activated: self.total_milestones = None self.total_story_points = None @@ -290,13 +276,25 @@ class Project(ProjectDefaults, TaggedMixin, TagsColorsdMixin, models.Model): if not self.is_looking_for_people: self.looking_for_people_note = "" - if self.anon_permissions == None: + if self.anon_permissions is None: self.anon_permissions = [] - if self.public_permissions == None: + if self.public_permissions is None: self.public_permissions = [] - super().save(*args, **kwargs) + if not self.slug: + with advisory_lock("project-creation"): + base_name = "{}-{}".format(self.owner.username, self.name) + base_slug = slugify_uniquely(base_name, self.__class__) + slug = base_slug + for i in arithmetic_progression(): + if not type(self).objects.filter(slug=slug).exists() or i > 100: + break + slug = "{}-{}".format(base_slug, i) + self.slug = slug + super().save(*args, **kwargs) + else: + super().save(*args, **kwargs) def refresh_totals(self, save=True): now = timezone.now() diff --git a/taiga/projects/wiki/models.py b/taiga/projects/wiki/models.py index 19cd6b25..5a4b3485 100644 --- a/taiga/projects/wiki/models.py +++ b/taiga/projects/wiki/models.py @@ -21,6 +21,8 @@ from django.contrib.contenttypes.fields import GenericRelation from django.conf import settings from django.utils.translation import ugettext_lazy as _ from django.utils import timezone +from django_pglocks import advisory_lock + from taiga.base.utils.slug import slugify_uniquely_for_queryset from taiga.projects.notifications.mixins import WatchedModelMixin from taiga.projects.occ import OCCModelMixin @@ -84,7 +86,9 @@ class WikiLink(models.Model): def save(self, *args, **kwargs): if not self.href: - wl_qs = self.project.wiki_links.all() - self.href = slugify_uniquely_for_queryset(self.title, wl_qs, slugfield="href") - - super().save(*args, **kwargs) + with advisory_lock("wiki-page-creation-{}".format(self.project_id)): + wl_qs = self.project.wiki_links.all() + self.href = slugify_uniquely_for_queryset(self.title, wl_qs, slugfield="href") + super().save(*args, **kwargs) + else: + super().save(*args, **kwargs) diff --git a/taiga/users/models.py b/taiga/users/models.py index b9c60e4b..98a71dc8 100644 --- a/taiga/users/models.py +++ b/taiga/users/models.py @@ -35,6 +35,7 @@ from django.utils import timezone from django.utils.translation import ugettext_lazy as _ from django_pgjson.fields import JsonField +from django_pglocks import advisory_lock from taiga.auth.tokens import get_token_for_user from taiga.base.utils.slug import slugify_uniquely @@ -265,20 +266,21 @@ class User(AbstractBaseUser, PermissionsMixin): super().save(*args, **kwargs) def cancel(self): - self.username = slugify_uniquely("deleted-user", User, slugfield="username") - self.email = "{}@taiga.io".format(self.username) - self.is_active = False - self.full_name = "Deleted user" - self.color = "" - self.bio = "" - self.lang = "" - self.theme = "" - self.timezone = "" - self.colorize_tags = True - self.token = None - self.set_unusable_password() - self.photo = None - self.save() + with advisory_lock("delete-user"): + self.username = slugify_uniquely("deleted-user", User, slugfield="username") + self.email = "{}@taiga.io".format(self.username) + self.is_active = False + self.full_name = "Deleted user" + self.color = "" + self.bio = "" + self.lang = "" + self.theme = "" + self.timezone = "" + self.colorize_tags = True + self.token = None + self.set_unusable_password() + self.photo = None + self.save() self.auth_data.all().delete() # Blocking all owned projects