Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ical feed generation of meetings not slow #3321

Merged
merged 2 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lego/apps/events/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
log = get_logger()


class EventPermissionHandler(PermissionHandler):
class EventPermissionHandler(PermissionHandler["Event"]):
perms_without_object = [CREATE, "administrate"]

def event_type_keyword_permissions(self, event_type, perm):
Expand Down
3 changes: 2 additions & 1 deletion lego/apps/ical/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ def add_meeting_to_ical_feed(feed, meeting, user):
start_datetime=meeting.start_time,
end_datetime=meeting.end_time,
location=meeting.location,
transparency="OPAQUE" if user in meeting.participants else "TRANSPARENT",
# This uses an annotation on the queryset for performance reasons
transparency="OPAQUE" if meeting.user_participating else "TRANSPARENT",
)


Expand Down
23 changes: 18 additions & 5 deletions lego/apps/ical/viewsets.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from datetime import timedelta

from django.db.models import Q
from django.utils import timezone
from rest_framework import decorators, permissions, viewsets
from rest_framework.permissions import IsAuthenticated
Expand All @@ -11,7 +12,8 @@
from lego.apps.ical.authentication import ICalTokenAuthentication
from lego.apps.ical.models import ICalToken
from lego.apps.ical.serializers import ICalTokenSerializer
from lego.apps.meetings.models import Meeting
from lego.apps.meetings import constants as meeting_constants
from lego.apps.meetings.models import Meeting, MeetingInvitation
from lego.apps.permissions.utils import get_permission_handler


Expand Down Expand Up @@ -86,13 +88,24 @@ def personal(self, request):
calendar_type = constants.TYPE_PERSONAL
feed = utils.generate_ical_feed(request, calendar_type)

permission_handler = get_permission_handler(Event)
following_events = permission_handler.filter_queryset(
event_permission_handler = get_permission_handler(Event)
following_events = event_permission_handler.filter_queryset(
request.user, Event.objects.filter(followers__follower_id=request.user.id)
)

permission_handler = get_permission_handler(Meeting)
meetings = permission_handler.filter_queryset(request.user, Meeting.objects)
meeting_permission_handler = get_permission_handler(Meeting)

attending_invites = MeetingInvitation.objects.filter(
status=meeting_constants.ATTENDING, user=request.user
)
meetings = meeting_permission_handler.filter_queryset(
request.user,
Meeting.objects.annotate(
# Annotate the queryset with participating status for use in the feed generation.
# Otherwise lookups are very expensive
user_participating=Q(invitations__in=attending_invites)
),
)

utils.add_events_to_ical_feed(feed, following_events)
utils.add_meetings_to_ical_feed(feed, meetings, request.user)
Expand Down
67 changes: 48 additions & 19 deletions lego/apps/permissions/permissions.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,25 @@
from django.db.models import Q
from __future__ import annotations

from typing import TYPE_CHECKING, Generic, Iterable, Sequence, Type, TypeVar

from django.db.models import Q, QuerySet

from lego.apps.permissions.constants import CREATE, LIST, VIEW
from lego.apps.permissions.models import ObjectPermissionsModel
from lego.utils.models import BasisModel

if TYPE_CHECKING:
from lego.apps.users.models import User

Check warning on line 12 in lego/apps/permissions/permissions.py

View check run for this annotation

Codecov / codecov/patch

lego/apps/permissions/permissions.py#L12

Added line #L12 was not covered by tests

def _check_intersection(first, second):

def _check_intersection(first, second) -> bool:
return len(set(first).intersection(set(second))) > 0


class PermissionHandler:
T = TypeVar("T", bound=BasisModel)


class PermissionHandler(Generic[T]):
"""
The permission handler defines how permissions should be handled on a model

Expand Down Expand Up @@ -45,13 +56,19 @@

perms_without_object = [CREATE]

def permissions_grant(self, permissions, user, obj=None, queryset=None):
def permissions_grant(
self,
permissions: Sequence[str],
user: User,
obj: T | None = None,
queryset: QuerySet[T] | None = None,
) -> list[str]:
"""
Lookup possible permissions the user has access to.
"""
permissions = list(set(permissions))

filtered_permissions = []
filtered_permissions: Iterable[str] = []
if obj is not None:
filtered_permissions = filter(
lambda perm: user.has_perm(perm, obj), permissions
Expand All @@ -63,7 +80,13 @@

return list(filtered_permissions)

def has_object_level_permissions(self, user, perm, obj=None, queryset=None):
def has_object_level_permissions(
self,
user: User,
perm: str,
obj: T | None = None,
queryset: QuerySet[T] | None = None,
) -> bool:
"""
Check whether the queryset or object requires a permission check at object level.
This function should only be used by the api permission backend.
Expand Down Expand Up @@ -107,13 +130,13 @@

def has_perm(
self,
user,
perm,
obj=None,
queryset=None,
user: User,
perm: str,
obj: T | None = None,
queryset: QuerySet[T] | None = None,
check_keyword_permissions=True,
**kwargs
):
**kwargs,
) -> bool:
"""
Check permission on a object.
"""
Expand Down Expand Up @@ -161,7 +184,9 @@

return False

def filter_queryset(self, user, queryset, **kwargs):
def filter_queryset(
self, user: User, queryset: QuerySet[T], **kwargs
) -> QuerySet[T]:
"""
Filter queryset based on object-level permissions.
We currently only supports queryset filtering on ObjectPermissionsModels.
Expand All @@ -184,7 +209,9 @@

return queryset

def require_auth(self, perm, obj=None, queryset=None):
def require_auth(
self, perm: str, obj: T | None = None, queryset: QuerySet[T] | None = None
) -> bool:
require_auth = self.authentication_map.get(perm, self.default_require_auth)
if not require_auth:
return False
Expand All @@ -197,10 +224,10 @@

return True

def is_authenticated(self, user):
def is_authenticated(self, user: User) -> bool:
return user and user.is_authenticated

def keyword_permission(self, model, perm):
def keyword_permission(self, model: Type[BasisModel], perm: str) -> str:
"""
Create default permission string based on the model class, action and default_permission
format. This is used when no permission is provided for a action in the permission_map.
Expand All @@ -212,7 +239,9 @@
}
return self.default_keyword_permission.format(**kwargs)

def required_keyword_permissions(self, model, perm):
def required_keyword_permissions(
self, model: Type[BasisModel], perm: str
) -> list[str]:
"""
Get required keyword permissions based on the action and model class.
Override the permission_map to create custom permissions, the default_keyword_permission
Expand All @@ -221,11 +250,11 @@

return self.permission_map.get(perm, [self.keyword_permission(model, perm)])

def created_by(self, user, obj):
def created_by(self, user: User, obj: T) -> bool:
created_by = getattr(obj, "created_by_id", None)
return created_by == user.id

def has_object_permissions(self, user, perm, obj):
def has_object_permissions(self, user: User, perm: str, obj: T) -> bool:
if perm in self.safe_methods:
# Check can_view
if _check_intersection(
Expand Down
7 changes: 6 additions & 1 deletion lego/apps/permissions/utils.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
from typing import TypeVar

from lego.apps.permissions.permissions import PermissionHandler

# isort:skip

default_permission_handler = PermissionHandler()


def get_permission_handler(model):
T = TypeVar("T")


def get_permission_handler(model: T) -> PermissionHandler[T]:
"""
Try to get the permission handler used by the model or use the default handler.
"""
Expand Down
33 changes: 20 additions & 13 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ coreapi = "2.3.3"
[tool.poetry.group.dev.dependencies]
django-debug-toolbar = "4.0.0"
tox = "4.5.0"
tblib = "^1.7.0"

[tool.poetry.group.coverage.dependencies]
coverage = "7.2.3"
Expand Down