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] 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