[Backport] Improving performance when updating objects

remotes/origin/issue/4795/notification_even_they_are_disabled
Alejandro Alonso 2016-05-11 12:41:16 +02:00 committed by David Barragán Merino
parent e8bcc84b35
commit 1dc8b193cf
10 changed files with 105 additions and 73 deletions

View File

@ -20,11 +20,18 @@ from .permissions import ADMINS_PERMISSIONS, MEMBERS_PERMISSIONS, ANON_PERMISSIO
from django.apps import apps
def _get_user_project_membership(user, project):
def _get_user_project_membership(user, project, cache="user"):
"""
cache param determines how memberships are calculated trying to reuse the existing data
in cache
"""
if user.is_anonymous():
return None
return user.cached_membership_for_project(project)
if cache == "user":
return user.cached_membership_for_project(project)
return project.cached_memberships_for_user(user)
def _get_object_project(obj):
@ -63,13 +70,17 @@ def is_project_admin(user, obj):
return False
def user_has_perm(user, perm, obj=None):
def user_has_perm(user, perm, obj=None, cache="user"):
"""
cache param determines how memberships are calculated trying to reuse the existing data
in cache
"""
project = _get_object_project(obj)
if not project:
return False
return perm in get_user_project_permissions(user, project)
return perm in get_user_project_permissions(user, project, cache=cache)
def role_has_perm(role, perm):
@ -82,8 +93,12 @@ def _get_membership_permissions(membership):
return []
def get_user_project_permissions(user, project):
membership = _get_user_project_membership(user, project)
def get_user_project_permissions(user, project, cache="user"):
"""
cache param determines how memberships are calculated trying to reuse the existing data
in cache
"""
membership = _get_user_project_membership(user, project, cache=cache)
if user.is_superuser:
admins_permissions = list(map(lambda perm: perm[0], ADMINS_PERMISSIONS))
members_permissions = list(map(lambda perm: perm[0], MEMBERS_PERMISSIONS))

View File

@ -44,7 +44,6 @@ from taiga.permissions.permissions import ANON_PERMISSIONS, MEMBERS_PERMISSIONS
from taiga.projects.notifications.choices import NotifyLevel
from taiga.projects.notifications.services import (
get_notify_policy,
set_notify_policy_level,
set_notify_policy_level_to_ignore,
create_notify_policy_if_not_exists)
@ -345,6 +344,33 @@ class Project(ProjectDefaults, TaggedMixin, models.Model):
def cached_user_stories(self):
return list(self.user_stories.all())
@cached_property
def cached_notify_policies(self):
return {np.user.id: np for np in self.notify_policies.select_related("user", "project")}
def cached_notify_policy_for_user(self, user):
"""
Get notification level for specified project and user.
"""
policy = self.cached_notify_policies.get(user.id, None)
if policy is None:
model_cls = apps.get_model("notifications", "NotifyPolicy")
policy = model_cls.objects.create(
project=self,
user=user,
notify_level= NotifyLevel.involved)
del self.cached_notify_policies
return policy
@cached_property
def cached_memberships(self):
return {m.user.id: m for m in self.memberships.exclude(user__isnull=True).select_related("user", "project", "role")}
def cached_memberships_for_user(self, user):
return self.cached_memberships.get(user.id, None)
def get_roles(self):
return self.roles.all()
@ -426,7 +452,7 @@ class Project(ProjectDefaults, TaggedMixin, models.Model):
set_notify_policy_level(notify_policy, notify_level)
def remove_watcher(self, user):
notify_policy = get_notify_policy(self, user)
notify_policy = self.cached_notify_policy_for_user(user)
set_notify_policy_level_to_ignore(notify_policy)
def delete_related_content(self):

View File

@ -78,16 +78,6 @@ def create_notify_policy_if_not_exists(project, user, level=NotifyLevel.involved
raise exc.IntegrityError(_("Notify exists for specified user and project")) from e
def get_notify_policy(project, user):
"""
Get notification level for specified project and user.
"""
model_cls = apps.get_model("notifications", "NotifyPolicy")
instance, _ = model_cls.objects.get_or_create(project=project, user=user,
defaults={"notify_level": NotifyLevel.involved})
return instance
def analize_object_for_watchers(obj:object, comment:str, user:object):
"""
Generic implementation for analize model objects and
@ -124,13 +114,13 @@ def _filter_by_permissions(obj, user):
WikiPage = apps.get_model("wiki", "WikiPage")
if isinstance(obj, UserStory):
return user_has_perm(user, "view_us", obj)
return user_has_perm(user, "view_us", obj, cache="project")
elif isinstance(obj, Issue):
return user_has_perm(user, "view_issues", obj)
return user_has_perm(user, "view_issues", obj, cache="project")
elif isinstance(obj, Task):
return user_has_perm(user, "view_tasks", obj)
return user_has_perm(user, "view_tasks", obj, cache="project")
elif isinstance(obj, WikiPage):
return user_has_perm(user, "view_wiki_pages", obj)
return user_has_perm(user, "view_wiki_pages", obj, cache="project")
return False
@ -149,7 +139,7 @@ def get_users_to_notify(obj, *, discard_users=None) -> list:
project = obj.get_project()
def _check_level(project:object, user:object, levels:tuple) -> bool:
policy = get_notify_policy(project, user)
policy = project.cached_notify_policy_for_user(user)
return policy.notify_level in levels
_can_notify_hard = partial(_check_level, project,
@ -226,8 +216,7 @@ def send_notifications(obj, *, history):
# Get a complete list of notifiable users for current
# object and send the change notification to them.
notify_users = get_users_to_notify(obj, discard_users=[notification.owner])
for notify_user in notify_users:
notification.notify_users.add(notify_user)
notification.notify_users.add(*notify_users)
# If we are the min interval is 0 it just work in a synchronous and spamming way
if settings.CHANGE_NOTIFICATIONS_MIN_INTERVAL == 0:

View File

@ -110,6 +110,3 @@ models.signals.post_save.connect(attach_sequence, sender=UserStory, dispatch_uid
models.signals.post_save.connect(attach_sequence, sender=Issue, dispatch_uid="refissue")
models.signals.post_save.connect(attach_sequence, sender=Task, dispatch_uid="reftask")
models.signals.post_delete.connect(delete_sequence, sender=Project, dispatch_uid="refprojdel")

View File

@ -54,7 +54,7 @@ def update_project_tags_colors_handler(instance):
new_color = _get_new_color(tag, settings.TAGS_PREDEFINED_COLORS,
exclude=used_colors)
instance.project.tags_colors.append([tag, new_color])
remove_unused_tags(instance.project)
if not isinstance(instance, Project):

View File

@ -78,8 +78,7 @@ def _add_to_objects_timeline(objects, instance:object, event_type:str, created_d
_add_to_object_timeline(obj, instance, event_type, created_datetime, namespace, extra_data)
@app.task
def push_to_timeline(objects, instance:object, event_type:str, created_datetime:object, namespace:str="default", extra_data:dict={}):
def _push_to_timeline(objects, instance:object, event_type:str, created_datetime:object, namespace:str="default", extra_data:dict={}):
if isinstance(objects, Model):
_add_to_object_timeline(objects, instance, event_type, created_datetime, namespace, extra_data)
elif isinstance(objects, QuerySet) or isinstance(objects, list):
@ -88,6 +87,32 @@ def push_to_timeline(objects, instance:object, event_type:str, created_datetime:
raise Exception("Invalid objects parameter")
@app.task
def push_to_timelines(project, user, obj, event_type, created_datetime, extra_data={}):
if project is not None:
# Actions related with a project
## Project timeline
_push_to_timeline(project, obj, event_type, created_datetime,
namespace=build_project_namespace(project),
extra_data=extra_data)
project.refresh_totals()
if hasattr(obj, "get_related_people"):
related_people = obj.get_related_people()
_push_to_timeline(related_people, obj, event_type, created_datetime,
namespace=build_user_namespace(user),
extra_data=extra_data)
else:
# Actions not related with a project
## - Me
_push_to_timeline(user, obj, event_type, created_datetime,
namespace=build_user_namespace(user),
extra_data=extra_data)
def get_timeline(obj, namespace=None):
assert isinstance(obj, Model), "obj must be a instance of Model"
from .models import Timeline

View File

@ -22,42 +22,17 @@ from django.utils.translation import ugettext as _
from taiga.projects.history import services as history_services
from taiga.projects.history.choices import HistoryType
from taiga.timeline.service import (push_to_timeline,
from taiga.timeline.service import (push_to_timelines,
build_user_namespace,
build_project_namespace,
extract_user_info)
def _push_to_timeline(*args, **kwargs):
def _push_to_timelines(*args, **kwargs):
if settings.CELERY_ENABLED:
push_to_timeline.delay(*args, **kwargs)
push_to_timelines.delay(*args, **kwargs)
else:
push_to_timeline(*args, **kwargs)
def _push_to_timelines(project, user, obj, event_type, created_datetime, extra_data={}):
if project is not None:
# Actions related with a project
## Project timeline
_push_to_timeline(project, obj, event_type, created_datetime,
namespace=build_project_namespace(project),
extra_data=extra_data)
project.refresh_totals()
if hasattr(obj, "get_related_people"):
related_people = obj.get_related_people()
_push_to_timeline(related_people, obj, event_type, created_datetime,
namespace=build_user_namespace(user),
extra_data=extra_data)
else:
# Actions not related with a project
## - Me
_push_to_timeline(user, obj, event_type, created_datetime,
namespace=build_user_namespace(user),
extra_data=extra_data)
push_to_timelines(*args, **kwargs)
def _clean_description_fields(values_diff):

View File

@ -27,8 +27,6 @@ def connect_webhooks_signals():
dispatch_uid="webhooks")
def disconnect_webhooks_signals():
signals.post_save.disconnect(sender=apps.get_model("history", "HistoryEntry"), dispatch_uid="webhooks")

View File

@ -60,7 +60,7 @@ def test_create_retrieve_notify_policy():
current_number = policy_model_cls.objects.all().count()
assert current_number == 0
policy = services.get_notify_policy(project, project.owner)
policy = project.cached_notify_policy_for_user(project.owner)
current_number = policy_model_cls.objects.all().count()
assert current_number == 1
@ -182,6 +182,7 @@ def test_users_to_notify():
policy_member1.notify_level = NotifyLevel.all
policy_member1.save()
del project.cached_notify_policies
users = services.get_users_to_notify(issue)
assert len(users) == 2
assert users == {member1.user, issue.get_owner()}
@ -190,6 +191,8 @@ def test_users_to_notify():
issue.add_watcher(member3.user)
policy_member3.notify_level = NotifyLevel.all
policy_member3.save()
del project.cached_notify_policies
users = services.get_users_to_notify(issue)
assert len(users) == 3
assert users == {member1.user, member3.user, issue.get_owner()}
@ -199,12 +202,14 @@ def test_users_to_notify():
policy_member3.save()
issue.add_watcher(member3.user)
del project.cached_notify_policies
users = services.get_users_to_notify(issue)
assert len(users) == 2
assert users == {member1.user, issue.get_owner()}
# Test with watchers without permissions
issue.add_watcher(member5.user)
del project.cached_notify_policies
users = services.get_users_to_notify(issue)
assert len(users) == 2
assert users == {member1.user, issue.get_owner()}
@ -231,7 +236,7 @@ def test_watching_users_to_notify_on_issue_modification_1():
issue = f.IssueFactory.create(project=project)
watching_user = f.UserFactory()
issue.add_watcher(watching_user)
watching_user_policy = services.get_notify_policy(project, watching_user)
watching_user_policy = project.cached_notify_policy_for_user(watching_user)
issue.description = "test1"
issue.save()
watching_user_policy.notify_level = NotifyLevel.all
@ -250,7 +255,7 @@ def test_watching_users_to_notify_on_issue_modification_2():
issue = f.IssueFactory.create(project=project)
watching_user = f.UserFactory()
issue.add_watcher(watching_user)
watching_user_policy = services.get_notify_policy(project, watching_user)
watching_user_policy = project.cached_notify_policy_for_user(watching_user)
issue.description = "test1"
issue.save()
watching_user_policy.notify_level = NotifyLevel.involved
@ -269,7 +274,7 @@ def test_watching_users_to_notify_on_issue_modification_3():
issue = f.IssueFactory.create(project=project)
watching_user = f.UserFactory()
issue.add_watcher(watching_user)
watching_user_policy = services.get_notify_policy(project, watching_user)
watching_user_policy = project.cached_notify_policy_for_user(watching_user)
issue.description = "test1"
issue.save()
watching_user_policy.notify_level = NotifyLevel.none
@ -289,7 +294,7 @@ def test_watching_users_to_notify_on_issue_modification_4():
issue = f.IssueFactory.create(project=project)
watching_user = f.UserFactory()
project.add_watcher(watching_user)
watching_user_policy = services.get_notify_policy(project, watching_user)
watching_user_policy = project.cached_notify_policy_for_user(watching_user)
issue.description = "test1"
issue.save()
watching_user_policy.notify_level = NotifyLevel.none
@ -309,7 +314,7 @@ def test_watching_users_to_notify_on_issue_modification_5():
issue = f.IssueFactory.create(project=project)
watching_user = f.UserFactory()
project.add_watcher(watching_user)
watching_user_policy = services.get_notify_policy(project, watching_user)
watching_user_policy = project.cached_notify_policy_for_user(watching_user)
issue.description = "test1"
issue.save()
watching_user_policy.notify_level = NotifyLevel.all
@ -329,7 +334,7 @@ def test_watching_users_to_notify_on_issue_modification_6():
issue = f.IssueFactory.create(project=project)
watching_user = f.UserFactory()
project.add_watcher(watching_user)
watching_user_policy = services.get_notify_policy(project, watching_user)
watching_user_policy = project.cached_notify_policy_for_user(watching_user)
issue.description = "test1"
issue.save()
watching_user_policy.notify_level = NotifyLevel.involved
@ -902,7 +907,7 @@ def test_watchers_assignation_for_us(client):
def test_retrieve_notify_policies_by_anonymous_user(client):
project = f.ProjectFactory.create()
policy = services.get_notify_policy(project, project.owner)
policy = project.cached_notify_policy_for_user(project.owner)
url = reverse("notifications-detail", args=[policy.pk])
response = client.get(url, content_type="application/json")

View File

@ -26,12 +26,14 @@ from taiga.projects.models import Project
import pytest
pytestmark = pytest.mark.django_db
def test_push_to_timeline_many_objects():
with patch("taiga.timeline.service._add_to_object_timeline") as mock:
users = [get_user_model(), get_user_model(), get_user_model()]
owner = get_user_model()
project = Project()
service.push_to_timeline(users, project, "test", project.created_date)
service._push_to_timeline(users, project, "test", project.created_date)
assert mock.call_count == 3
assert mock.mock_calls == [
call(users[0], project, "test", project.created_date, "default", {}),
@ -39,7 +41,7 @@ def test_push_to_timeline_many_objects():
call(users[2], project, "test", project.created_date, "default", {}),
]
with pytest.raises(Exception):
service.push_to_timeline(None, project, "test")
service._push_to_timeline(None, project, "test")
def test_add_to_objects_timeline():
@ -54,7 +56,7 @@ def test_add_to_objects_timeline():
call(users[2], project, "test", project.created_date, "default", {}),
]
with pytest.raises(Exception):
service.push_to_timeline(None, project, "test")
service._push_to_timeline(None, project, "test")
def test_get_impl_key_from_model():