diff --git a/openslides/agenda/access_permissions.py b/openslides/agenda/access_permissions.py index 940e5a3b0..8b602f935 100644 --- a/openslides/agenda/access_permissions.py +++ b/openslides/agenda/access_permissions.py @@ -1,8 +1,7 @@ -from typing import Any, Dict, Iterable, List, Optional +from typing import Any, Dict, Iterable, List from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import async_has_perm -from ..utils.collection import CollectionElement class ItemAccessPermissions(BaseAccessPermissions): @@ -25,7 +24,7 @@ class ItemAccessPermissions(BaseAccessPermissions): async def get_restricted_data( self, full_data: List[Dict[str, Any]], - user: Optional[CollectionElement]) -> List[Dict[str, Any]]: + user_id: int) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. @@ -43,11 +42,11 @@ class ItemAccessPermissions(BaseAccessPermissions): return {key: full_data[key] for key in whitelist} # Parse data. - if full_data and await async_has_perm(user, 'agenda.can_see'): - if await async_has_perm(user, 'agenda.can_manage') and await async_has_perm(user, 'agenda.can_see_internal_items'): + if full_data and await async_has_perm(user_id, 'agenda.can_see'): + if await async_has_perm(user_id, 'agenda.can_manage') and await async_has_perm(user_id, 'agenda.can_see_internal_items'): # Managers with special permission can see everything. data = full_data - elif await async_has_perm(user, 'agenda.can_see_internal_items'): + elif await async_has_perm(user_id, 'agenda.can_see_internal_items'): # Non managers with special permission can see everything but # comments and hidden items. data = [full for full in full_data if not full['is_hidden']] # filter hidden items @@ -68,7 +67,7 @@ class ItemAccessPermissions(BaseAccessPermissions): # In non internal case managers see everything and non managers see # everything but comments. - if await async_has_perm(user, 'agenda.can_manage'): + if await async_has_perm(user_id, 'agenda.can_manage'): blocked_keys_non_internal_hidden_case: Iterable[str] = [] can_see_hidden = True else: diff --git a/openslides/assignments/access_permissions.py b/openslides/assignments/access_permissions.py index e8d70e778..05814a25e 100644 --- a/openslides/assignments/access_permissions.py +++ b/openslides/assignments/access_permissions.py @@ -1,8 +1,7 @@ -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import async_has_perm, has_perm -from ..utils.collection import CollectionElement class AssignmentAccessPermissions(BaseAccessPermissions): @@ -26,16 +25,16 @@ class AssignmentAccessPermissions(BaseAccessPermissions): async def get_restricted_data( self, full_data: List[Dict[str, Any]], - user: Optional[CollectionElement]) -> List[Dict[str, Any]]: + user_id: int) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. Removes unpublished polls for non admins so that they only get a result like the AssignmentShortSerializer would give them. """ # Parse data. - if await async_has_perm(user, 'assignments.can_see') and await async_has_perm(user, 'assignments.can_manage'): + if await async_has_perm(user_id, 'assignments.can_see') and await async_has_perm(user_id, 'assignments.can_manage'): data = full_data - elif await async_has_perm(user, 'assignments.can_see'): + elif await async_has_perm(user_id, 'assignments.can_see'): # Exclude unpublished poll votes. data = [] for full in full_data: diff --git a/openslides/core/config.py b/openslides/core/config.py index 9185a5e5f..f4ae2796f 100644 --- a/openslides/core/config.py +++ b/openslides/core/config.py @@ -16,7 +16,6 @@ from django.utils.translation import ugettext as _ from mypy_extensions import TypedDict from ..utils.cache import element_cache -from ..utils.collection import CollectionElement from .exceptions import ConfigError, ConfigNotFound from .models import ConfigStore @@ -58,9 +57,7 @@ class ConfigHandler: if not self.exists(key): raise ConfigNotFound(_('The config variable {} was not found.').format(key)) - return CollectionElement.from_values( - self.get_collection_string(), - self.get_key_to_id()[key]).get_full_data()['value'] + return async_to_sync(element_cache.get_element_full_data)(self.get_collection_string(), self.get_key_to_id()[key])['value'] def get_key_to_id(self) -> Dict[str, int]: """ diff --git a/openslides/core/websocket.py b/openslides/core/websocket.py index d747e1611..d316ff5c9 100644 --- a/openslides/core/websocket.py +++ b/openslides/core/websocket.py @@ -45,7 +45,7 @@ class NotifyWebsocketClientMessage(BaseWebsocketClientMessage): "type": "send_notify", "incomming": content, "senderReplyChannelName": consumer.channel_name, - "senderUserId": consumer.scope['user'].id if consumer.scope['user'] else 0, + "senderUserId": consumer.scope['user']['id'], }, ) @@ -83,7 +83,7 @@ class GetElementsWebsocketClientMessage(BaseWebsocketClientMessage): async def receive_content(self, consumer: "ProtocollAsyncJsonWebsocketConsumer", content: Any, id: str) -> None: requested_change_id = content.get('change_id', 0) try: - element_data = await get_element_data(consumer.scope['user'], requested_change_id) + element_data = await get_element_data(consumer.scope['user']['id'], requested_change_id) except ValueError as error: await consumer.send_json(type='error', content=str(error), in_response=id) else: diff --git a/openslides/mediafiles/access_permissions.py b/openslides/mediafiles/access_permissions.py index 461e75774..2b438e1de 100644 --- a/openslides/mediafiles/access_permissions.py +++ b/openslides/mediafiles/access_permissions.py @@ -1,8 +1,7 @@ -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import async_has_perm -from ..utils.collection import CollectionElement class MediafileAccessPermissions(BaseAccessPermissions): @@ -22,15 +21,15 @@ class MediafileAccessPermissions(BaseAccessPermissions): async def get_restricted_data( self, full_data: List[Dict[str, Any]], - user: Optional[CollectionElement]) -> List[Dict[str, Any]]: + user_id: int) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared - for the user. Removes hidden mediafiles for some users. + for the user. Removes hidden mediafiles for some users. """ # Parse data. - if await async_has_perm(user, 'mediafiles.can_see') and await async_has_perm(user, 'mediafiles.can_see_hidden'): + if await async_has_perm(user_id, 'mediafiles.can_see') and await async_has_perm(user_id, 'mediafiles.can_see_hidden'): data = full_data - elif await async_has_perm(user, 'mediafiles.can_see'): + elif await async_has_perm(user_id, 'mediafiles.can_see'): # Exclude hidden mediafiles. data = [full for full in full_data if not full['hidden']] else: diff --git a/openslides/motions/access_permissions.py b/openslides/motions/access_permissions.py index 069cadadc..f3603778d 100644 --- a/openslides/motions/access_permissions.py +++ b/openslides/motions/access_permissions.py @@ -1,9 +1,8 @@ from copy import deepcopy -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import async_has_perm, async_in_some_groups -from ..utils.collection import CollectionElement class MotionAccessPermissions(BaseAccessPermissions): @@ -23,7 +22,7 @@ class MotionAccessPermissions(BaseAccessPermissions): async def get_restricted_data( self, full_data: List[Dict[str, Any]], - user: Optional[CollectionElement]) -> List[Dict[str, Any]]: + user_id: int) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. Removes motion if the user has not the permission to see @@ -32,13 +31,13 @@ class MotionAccessPermissions(BaseAccessPermissions): personal notes. """ # Parse data. - if await async_has_perm(user, 'motions.can_see'): + if await async_has_perm(user_id, 'motions.can_see'): # TODO: Refactor this after personal_notes system is refactored. data = [] for full in full_data: # Check if user is submitter of this motion. - if isinstance(user, CollectionElement): - is_submitter = user.get_full_data()['id'] in [ + if user_id: + is_submitter = user_id in [ submitter['user_id'] for submitter in full.get('submitters', [])] else: # Anonymous users can not be submitters. @@ -48,8 +47,8 @@ class MotionAccessPermissions(BaseAccessPermissions): required_permission_to_see = full['state_required_permission_to_see'] permission = ( not required_permission_to_see or - await async_has_perm(user, required_permission_to_see) or - await async_has_perm(user, 'motions.can_manage') or + await async_has_perm(user_id, required_permission_to_see) or + await async_has_perm(user_id, 'motions.can_manage') or is_submitter) # Parse single motion. @@ -57,7 +56,7 @@ class MotionAccessPermissions(BaseAccessPermissions): full_copy = deepcopy(full) full_copy['comments'] = [] for comment in full['comments']: - if await async_in_some_groups(user, comment['read_groups_id']): + if await async_in_some_groups(user_id, comment['read_groups_id']): full_copy['comments'].append(comment) data.append(full_copy) else: @@ -83,15 +82,15 @@ class MotionChangeRecommendationAccessPermissions(BaseAccessPermissions): async def get_restricted_data( self, full_data: List[Dict[str, Any]], - user: Optional[CollectionElement]) -> List[Dict[str, Any]]: + user_id: int) -> List[Dict[str, Any]]: """ Removes change recommendations if they are internal and the user has not the can_manage permission. To see change recommendation the user needs the can_see permission. """ # Parse data. - if await async_has_perm(user, 'motions.can_see'): - has_manage_perms = await async_has_perm(user, 'motion.can_manage') + if await async_has_perm(user_id, 'motions.can_see'): + has_manage_perms = await async_has_perm(user_id, 'motion.can_manage') data = [] for full in full_data: if not full['internal'] or has_manage_perms: @@ -119,18 +118,18 @@ class MotionCommentSectionAccessPermissions(BaseAccessPermissions): async def get_restricted_data( self, full_data: List[Dict[str, Any]], - user: Optional[CollectionElement]) -> List[Dict[str, Any]]: + user_id: int) -> List[Dict[str, Any]]: """ If the user has manage rights, he can see all sections. If not all sections will be removed, when the user is not in at least one of the read_groups. """ data: List[Dict[str, Any]] = [] - if await async_has_perm(user, 'motions.can_manage'): + if await async_has_perm(user_id, 'motions.can_manage'): data = full_data else: for full in full_data: read_groups = full.get('read_groups_id', []) - if await async_in_some_groups(user, read_groups): + if await async_in_some_groups(user_id, read_groups): data.append(full) return data diff --git a/openslides/motions/views.py b/openslides/motions/views.py index 2ce47613b..c387f74e5 100644 --- a/openslides/motions/views.py +++ b/openslides/motions/views.py @@ -1,5 +1,5 @@ import re -from typing import List, Optional +from typing import List from django.conf import settings from django.contrib.auth import get_user_model @@ -13,7 +13,6 @@ from rest_framework import status from ..core.config import config from ..utils.auth import has_perm, in_some_groups from ..utils.autoupdate import inform_changed_data -from ..utils.collection import CollectionElement from ..utils.exceptions import OpenSlidesError from ..utils.rest_api import ( CreateModelMixin, @@ -117,9 +116,7 @@ class MotionViewSet(ModelViewSet): # Check if parent motion exists. if request.data.get('parent_id') is not None: try: - parent_motion: Optional[CollectionElement] = CollectionElement.from_values( - Motion.get_collection_string(), - request.data['parent_id']) + parent_motion = Motion.objects.get(pk=request.data['parent_id']) except Motion.DoesNotExist: raise ValidationError({'detail': _('The parent motion does not exist.')}) else: @@ -143,8 +140,8 @@ class MotionViewSet(ModelViewSet): 'category_id', # This will be set to the matching 'motion_block_id', # values from parent_motion. ]) - request.data['category_id'] = parent_motion.get_full_data().get('category_id') - request.data['motion_block_id'] = parent_motion.get_full_data().get('motion_block_id') + request.data['category_id'] = parent_motion.category_id + request.data['motion_block_id'] = parent_motion.motion_block_id for key in keys: if key not in whitelist: del request.data[key] diff --git a/openslides/users/access_permissions.py b/openslides/users/access_permissions.py index b6fc27848..a8b6297e3 100644 --- a/openslides/users/access_permissions.py +++ b/openslides/users/access_permissions.py @@ -1,11 +1,8 @@ -from typing import Any, Dict, List, Optional, Set +from typing import Any, Dict, List, Set from ..utils.access_permissions import BaseAccessPermissions, required_user from ..utils.auth import async_has_perm -from ..utils.collection import ( - CollectionElement, - get_model_from_collection_string, -) +from ..utils.utils import get_model_from_collection_string class UserAccessPermissions(BaseAccessPermissions): @@ -24,7 +21,7 @@ class UserAccessPermissions(BaseAccessPermissions): async def get_restricted_data( self, full_data: List[Dict[str, Any]], - user: Optional[CollectionElement]) -> List[Dict[str, Any]]: + user_id: int) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. Removes several fields for non admins so that they do @@ -57,9 +54,9 @@ class UserAccessPermissions(BaseAccessPermissions): litte_data_fields.discard('groups') # Check user permissions. - if await async_has_perm(user, 'users.can_see_name'): - if await async_has_perm(user, 'users.can_see_extra_data'): - if await async_has_perm(user, 'users.can_manage'): + if await async_has_perm(user_id, 'users.can_see_name'): + if await async_has_perm(user_id, 'users.can_see_extra_data'): + if await async_has_perm(user_id, 'users.can_manage'): data = [filtered_data(full, all_data_fields) for full in full_data] else: data = [filtered_data(full, many_data_fields) for full in full_data] @@ -74,14 +71,14 @@ class UserAccessPermissions(BaseAccessPermissions): can_see_collection_strings: Set[str] = set() for collection_string in required_user.get_collection_strings(): - if await async_has_perm(user, get_model_from_collection_string(collection_string).can_see_permission): + if await async_has_perm(user_id, get_model_from_collection_string(collection_string).can_see_permission): can_see_collection_strings.add(collection_string) user_ids = await required_user.get_required_users(can_see_collection_strings) # Add oneself. - if user is not None: - user_ids.add(user.id) + if user_id: + user_ids.add(user_id) # Parse data. data = [ @@ -124,17 +121,17 @@ class PersonalNoteAccessPermissions(BaseAccessPermissions): async def get_restricted_data( self, full_data: List[Dict[str, Any]], - user: Optional[CollectionElement]) -> List[Dict[str, Any]]: + user_id: int) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. Everybody gets only his own personal notes. """ # Parse data. - if user is None: + if not user_id: data: List[Dict[str, Any]] = [] else: for full in full_data: - if full['user_id'] == user.id: + if full['user_id'] == user_id: data = [full] break else: diff --git a/openslides/users/models.py b/openslides/users/models.py index b7e68eaf7..311364bbd 100644 --- a/openslides/users/models.py +++ b/openslides/users/models.py @@ -20,7 +20,6 @@ from jsonfield import JSONField from ..core.config import config from ..core.models import Projector from ..utils.auth import GROUP_ADMIN_PK -from ..utils.collection import CollectionElement from ..utils.models import RESTModelMixin from .access_permissions import ( GroupAccessPermissions, @@ -211,7 +210,6 @@ class User(RESTModelMixin, PermissionsMixin, AbstractBaseUser): """ if kwargs.get('update_fields') == ['last_login']: kwargs['skip_autoupdate'] = True - CollectionElement.from_instance(self) return super().save(*args, **kwargs) def delete(self, skip_autoupdate=False, *args, **kwargs): diff --git a/openslides/users/views.py b/openslides/users/views.py index bdf4fe6bc..cf7537a01 100644 --- a/openslides/users/views.py +++ b/openslides/users/views.py @@ -23,14 +23,13 @@ from ..utils.auth import ( GROUP_DEFAULT_PK, anonymous_is_enabled, has_perm, - user_to_collection_user, ) from ..utils.autoupdate import ( + Element, inform_changed_data, - inform_data_collection_element_list, + inform_changed_elements, ) from ..utils.cache import element_cache -from ..utils.collection import CollectionElement from ..utils.rest_api import ( ModelViewSet, Response, @@ -112,7 +111,7 @@ class UserViewSet(ModelViewSet): del request.data[key] response = super().update(request, *args, **kwargs) # Maybe some group assignments have changed. Better delete the restricted user cache - async_to_sync(element_cache.del_user)(user_to_collection_user(user)) + async_to_sync(element_cache.del_user)(user.pk) return response def destroy(self, request, *args, **kwargs): @@ -312,7 +311,7 @@ class GroupViewSet(ModelViewSet): # Delete the user chaches of all affected users for user in group.user_set.all(): - async_to_sync(element_cache.del_user)(user_to_collection_user(user)) + async_to_sync(element_cache.del_user)(user.pk) def diff(full, part): """ @@ -327,14 +326,14 @@ class GroupViewSet(ModelViewSet): # Some permissions are added. if len(new_permissions) > 0: - collection_elements: List[CollectionElement] = [] + elements: List[Element] = [] signal_results = permission_change.send(None, permissions=new_permissions, action='added') all_full_data = async_to_sync(element_cache.get_all_full_data)() for receiver, signal_collections in signal_results: for cachable in signal_collections: - for element in all_full_data.get(cachable.get_collection_string(), {}): - collection_elements.append(CollectionElement.from_values(cachable.get_collection_string(), element['id'])) - inform_data_collection_element_list(collection_elements) + for full_data in all_full_data.get(cachable.get_collection_string(), {}): + elements.append(Element(id=full_data['id'], collection_string=cachable.get_collection_string(), full_data=full_data)) + inform_changed_elements(elements) # TODO: Some permissions are deleted. @@ -465,7 +464,7 @@ class UserLoginView(APIView): # self.request.method == 'POST' context['user_id'] = self.user.pk context['user'] = async_to_sync(element_cache.get_element_restricted_data)( - CollectionElement.from_instance(self.user), + self.user.pk or 0, self.user.get_collection_string(), self.user.pk) return super().get_context_data(**context) @@ -496,16 +495,16 @@ class WhoAmIView(APIView): user. Appends also a flag if guest users are enabled in the config. Appends also the serialized user if available. """ - user_id = self.request.user.pk - if user_id is not None: + user_id = self.request.user.pk or 0 + if user_id: user_data = async_to_sync(element_cache.get_element_restricted_data)( - user_to_collection_user(self.request.user), + user_id, self.request.user.get_collection_string(), user_id) else: user_data = None return super().get_context_data( - user_id=user_id, + user_id=user_id or None, guest_enabled=anonymous_is_enabled(), user=user_data, **context) diff --git a/openslides/utils/access_permissions.py b/openslides/utils/access_permissions.py index 14e49dae5..840315315 100644 --- a/openslides/utils/access_permissions.py +++ b/openslides/utils/access_permissions.py @@ -1,16 +1,11 @@ -from typing import Any, Callable, Dict, List, Optional, Set +from typing import Any, Callable, Dict, List, Set from asgiref.sync import async_to_sync from django.db.models import Model from rest_framework.serializers import Serializer -from .auth import ( - async_anonymous_is_enabled, - async_has_perm, - user_to_collection_user, -) +from .auth import async_anonymous_is_enabled, async_has_perm, user_to_user_id from .cache import element_cache -from .collection import CollectionElement class BaseAccessPermissions: @@ -28,31 +23,33 @@ class BaseAccessPermissions: If this string is empty, all users can see it. """ - def check_permissions(self, user: Optional[CollectionElement]) -> bool: + def check_permissions(self, user_id: int) -> bool: """ Returns True if the user has read access to model instances. """ # Convert user to right type # TODO: Remove this and make sure, that user has always the right type - user = user_to_collection_user(user) - return async_to_sync(self.async_check_permissions)(user) + user_id = user_to_user_id(user_id) + return async_to_sync(self.async_check_permissions)(user_id) - async def async_check_permissions(self, user: Optional[CollectionElement]) -> bool: + async def async_check_permissions(self, user_id: int) -> bool: """ Returns True if the user has read access to model instances. """ if self.base_permission: - return await async_has_perm(user, self.base_permission) + return await async_has_perm(user_id, self.base_permission) else: - return user is not None or await async_anonymous_is_enabled() + return bool(user_id) or await async_anonymous_is_enabled() - def get_serializer_class(self, user: CollectionElement = None) -> Serializer: + def get_serializer_class(self, user_id: int = 0) -> Serializer: """ Returns different serializer classes according to users permissions. This should return the serializer for full data access if user is None. See get_full_data(). """ + # TODO: Rewrite me by using an serializer_class attribute and removing + # the user_id argument. raise NotImplementedError( "You have to add the method 'get_serializer_class' to your " "access permissions class.".format(self)) @@ -61,27 +58,26 @@ class BaseAccessPermissions: """ Returns all possible serialized data for the given instance. """ - return self.get_serializer_class(user=None)(instance).data + return self.get_serializer_class()(instance).data async def get_restricted_data( self, full_data: List[Dict[str, Any]], - user: Optional[CollectionElement]) -> List[Dict[str, Any]]: + user_id: int) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. - The argument full_data has to be a list of full_data dicts as they are - created with CollectionElement.get_full_data(). The type of the return - is the same. Returns an empty list if the user has no read access. - Returns reduced data if the user has limited access. - Default: Returns full data if the user has read access to model instances. + The argument full_data has to be a list of full_data dicts. The type of + the return is the same. Returns an empty list if the user has no read + access. Returns reduced data if the user has limited access. Default: + Returns full data if the user has read access to model instances. Hint: You should override this method if your get_serializer_class() method returns different serializers for different users or if you have access restrictions in your view or viewset in methods like retrieve() or list(). """ - return full_data if await self.async_check_permissions(user) else [] + return full_data if await self.async_check_permissions(user_id) else [] class RequiredUsers: diff --git a/openslides/utils/auth.py b/openslides/utils/auth.py index 5cb5e73b9..7cfc4cd0c 100644 --- a/openslides/utils/auth.py +++ b/openslides/utils/auth.py @@ -1,4 +1,4 @@ -from typing import Dict, List, Optional, Union, cast +from typing import Dict, List, Union, cast from asgiref.sync import async_to_sync from django.apps import apps @@ -9,12 +9,15 @@ from django.core.exceptions import ImproperlyConfigured from django.db.models import Model from .cache import element_cache -from .collection import CollectionElement GROUP_DEFAULT_PK = 1 # This is the hard coded pk for the default group. GROUP_ADMIN_PK = 2 # This is the hard coded pk for the admin group. +# Hard coded collection string for users and groups +group_collection_string = 'users/group' +user_collection_string = 'users/user' + def get_group_model() -> Model: """ @@ -30,61 +33,63 @@ def get_group_model() -> Model: ) -def has_perm(user: Optional[CollectionElement], perm: str) -> bool: +def has_perm(user_id: int, perm: str) -> bool: """ Checks that user has a specific permission. - User can be a CollectionElement of a user or None. + user_id 0 means anonymous user. """ # Convert user to right type # TODO: Remove this and make use, that user has always the right type - user = user_to_collection_user(user) - return async_to_sync(async_has_perm)(user, perm) + user_id = user_to_user_id(user_id) + return async_to_sync(async_has_perm)(user_id, perm) -async def async_has_perm(user: Optional[CollectionElement], perm: str) -> bool: +async def async_has_perm(user_id: int, perm: str) -> bool: """ Checks that user has a specific permission. - User can be a CollectionElement of a user or None. + user_id 0 means anonymous user. """ - group_collection_string = 'users/group' # This is the hard coded collection string for openslides.users.models.Group - - if user is None and not await async_anonymous_is_enabled(): + if not user_id and not await async_anonymous_is_enabled(): has_perm = False - elif user is None: + elif not user_id: # Use the permissions from the default group. default_group = await element_cache.get_element_full_data(group_collection_string, GROUP_DEFAULT_PK) if default_group is None: raise RuntimeError('Default Group does not exist.') has_perm = perm in default_group['permissions'] - elif GROUP_ADMIN_PK in user.get_full_data()['groups_id']: - # User in admin group (pk 2) grants all permissions. - has_perm = True else: - # Get all groups of the user and then see, if one group has the required - # permission. If the user has no groups, then use the default group. - group_ids = user.get_full_data()['groups_id'] or [GROUP_DEFAULT_PK] - for group_id in group_ids: - group = await element_cache.get_element_full_data(group_collection_string, group_id) - if group is None: - raise RuntimeError('User is in non existing group with id {}.'.format(group_id)) - - if perm in group['permissions']: - has_perm = True - break + user_data = await element_cache.get_element_full_data(user_collection_string, user_id) + if user_data is None: + raise RuntimeError('User with id {} does not exist.'.format(user_id)) + if GROUP_ADMIN_PK in user_data['groups_id']: + # User in admin group (pk 2) grants all permissions. + has_perm = True else: - has_perm = False + # Get all groups of the user and then see, if one group has the required + # permission. If the user has no groups, then use the default group. + group_ids = user_data['groups_id'] or [GROUP_DEFAULT_PK] + for group_id in group_ids: + group = await element_cache.get_element_full_data(group_collection_string, group_id) + if group is None: + raise RuntimeError('User is in non existing group with id {}.'.format(group_id)) + + if perm in group['permissions']: + has_perm = True + break + else: + has_perm = False return has_perm -def in_some_groups(user: Optional[CollectionElement], groups: List[int]) -> bool: +def in_some_groups(user_id: int, groups: List[int]) -> bool: """ Checks that user is in at least one given group. Groups can be given as a list of ids or group instances. If the user is in the admin group (pk = 2) the result is always true. - User can be a CollectionElement of a user or None. + user_id 0 means anonymous user. """ if len(groups) == 0: @@ -92,40 +97,44 @@ def in_some_groups(user: Optional[CollectionElement], groups: List[int]) -> bool # Convert user to right type # TODO: Remove this and make use, that user has always the right type - user = user_to_collection_user(user) - return async_to_sync(async_in_some_groups)(user, groups) + user_id = user_to_user_id(user_id) + return async_to_sync(async_in_some_groups)(user_id, groups) -async def async_in_some_groups(user: Optional[CollectionElement], groups: List[int]) -> bool: +async def async_in_some_groups(user_id: int, groups: List[int]) -> bool: """ Checks that user is in at least one given group. Groups can be given as a list of ids. If the user is in the admin group (pk = 2) the result is always true. - User can be a CollectionElement of a user or None. + user_id 0 means anonymous user. """ if not len(groups): return False # early end here, if no groups are given. - if user is None and not await async_anonymous_is_enabled(): + if not user_id and not await async_anonymous_is_enabled(): in_some_groups = False - elif user is None: + elif not user_id: # Use the permissions from the default group. in_some_groups = GROUP_DEFAULT_PK in groups - elif GROUP_ADMIN_PK in user.get_full_data()['groups_id']: - # User in admin group (pk 2) grants all permissions. - in_some_groups = True else: - # Get all groups of the user and then see, if one group has the required - # permission. If the user has no groups, then use the default group. - group_ids = user.get_full_data()['groups_id'] or [GROUP_DEFAULT_PK] - for group_id in group_ids: - if group_id in groups: - in_some_groups = True - break + user_data = await element_cache.get_element_full_data(user_collection_string, user_id) + if user_data is None: + raise RuntimeError('User with id {} does not exist.'.format(user_id)) + if GROUP_ADMIN_PK in user_data['groups_id']: + # User in admin group (pk 2) grants all permissions. + in_some_groups = True else: - in_some_groups = False + # Get all groups of the user and then see, if one group has the required + # permission. If the user has no groups, then use the default group. + group_ids = user_data['groups_id'] or [GROUP_DEFAULT_PK] + for group_id in group_ids: + if group_id in groups: + in_some_groups = True + break + else: + in_some_groups = False return in_some_groups @@ -149,17 +158,17 @@ async def async_anonymous_is_enabled() -> bool: return False if element is None else element['value'] -AnyUser = Union[Model, CollectionElement, int, AnonymousUser, None] +AnyUser = Union[Model, int, AnonymousUser, None] -def user_to_collection_user(user: AnyUser) -> Optional[CollectionElement]: +def user_to_user_id(user: AnyUser) -> int: """ - Takes an object, that represents a user and converts it to a CollectionElement - or to None, if it is an anonymous user. + Takes an object, that represents a user returns its user_id. + + user_id 0 means anonymous user. User can be * an user object, - * a CollectionElement of an user, * an user id or * an anonymous user. @@ -168,30 +177,15 @@ def user_to_collection_user(user: AnyUser) -> Optional[CollectionElement]: User = get_user_model() if user is None: - # Nothing to do - pass - elif isinstance(user, CollectionElement) and user.collection_string == User.get_collection_string(): - # Nothing to do - pass - elif isinstance(user, CollectionElement): - raise TypeError( - "Unsupported type for user. Only CollectionElements for users can be" - "used. Not {}".format(user.collection_string)) + user_id = 0 elif isinstance(user, int): - # user 0 means anonymous - if user == 0: - user = None - else: - user = CollectionElement.from_values(User.get_collection_string(), user) + # Nothing to do + user_id = user elif isinstance(user, AnonymousUser): - user = None + user_id = 0 elif isinstance(user, User): - # Converts a user object to a collection element. - # from_instance can not be used because the user serializer loads - # the group from the db. So each call to from_instance(user) costs - # one db query. - user = CollectionElement.from_values(User.get_collection_string(), user.id) + user_id = user.pk else: raise TypeError( "Unsupported type for user. User {} has type {}.".format(user, type(user))) - return user + return user_id diff --git a/openslides/utils/autoupdate.py b/openslides/utils/autoupdate.py index c7df5ff5c..cfd359014 100644 --- a/openslides/utils/autoupdate.py +++ b/openslides/utils/autoupdate.py @@ -4,9 +4,30 @@ from typing import Any, Dict, Iterable, List, Optional, Tuple, Union from asgiref.sync import async_to_sync from channels.layers import get_channel_layer from django.db.models import Model +from mypy_extensions import TypedDict from .cache import element_cache, get_element_id -from .collection import CollectionElement + + +Element = TypedDict( + 'Element', + { + 'id': int, + 'collection_string': str, + 'full_data': Optional[Dict[str, Any]], + } +) + +AutoupdateFormat = TypedDict( + 'AutoupdateFormat', + { + 'changed': Dict[str, List[Dict[str, Any]]], + 'deleted': Dict[str, List[int]], + 'from_change_id': int, + 'to_change_id': int, + 'all_data': bool, + }, +) def inform_changed_data(instances: Union[Iterable[Model], Model]) -> None: @@ -27,53 +48,51 @@ def inform_changed_data(instances: Union[Iterable[Model], Model]) -> None: # Instance has no method get_root_rest_element. Just ignore it. pass - collection_elements = {} + elements: Dict[str, Element] = {} for root_instance in root_instances: - collection_element = CollectionElement.from_instance(root_instance) key = root_instance.get_collection_string() + str(root_instance.get_rest_pk()) - collection_elements[key] = collection_element + elements[key] = Element( + id=root_instance.get_rest_pk(), + collection_string=root_instance.get_collection_string(), + full_data=root_instance.get_full_data()) bundle = autoupdate_bundle.get(threading.get_ident()) if bundle is not None: - # Put all collection elements into the autoupdate_bundle. - bundle.update(collection_elements) + # Put all elements into the autoupdate_bundle. + bundle.update(elements) else: # Send autoupdate directly - handle_collection_elements(collection_elements.values()) + handle_changed_elements(elements.values()) -def inform_deleted_data(elements: Iterable[Tuple[str, int]]) -> None: +def inform_deleted_data(deleted_elements: Iterable[Tuple[str, int]]) -> None: """ Informs the autoupdate system and the caching system about the deletion of elements. """ - collection_elements: Dict[str, Any] = {} - for element in elements: - collection_element = CollectionElement.from_values( - collection_string=element[0], - id=element[1], - deleted=True) - key = element[0] + str(element[1]) - collection_elements[key] = collection_element + elements: Dict[str, Element] = {} + for deleted_element in deleted_elements: + key = deleted_element[0] + str(deleted_element[1]) + elements[key] = Element(id=deleted_element[1], collection_string=deleted_element[0], full_data=None) bundle = autoupdate_bundle.get(threading.get_ident()) if bundle is not None: - # Put all collection elements into the autoupdate_bundle. - bundle.update(collection_elements) + # Put all elements into the autoupdate_bundle. + bundle.update(elements) else: # Send autoupdate directly - handle_collection_elements(collection_elements.values()) + handle_changed_elements(elements.values()) -def inform_data_collection_element_list(collection_elements: List[CollectionElement]) -> None: +def inform_changed_elements(changed_elements: Iterable[Element]) -> None: """ Informs the autoupdate system about some collection elements. This is used just to send some data to all users. """ elements = {} - for collection_element in collection_elements: - key = collection_element.collection_string + str(collection_element.id) - elements[key] = collection_element + for changed_element in changed_elements: + key = changed_element['collection_string'] + str(changed_element['id']) + elements[key] = changed_element bundle = autoupdate_bundle.get(threading.get_ident()) if bundle is not None: @@ -81,13 +100,13 @@ def inform_data_collection_element_list(collection_elements: List[CollectionElem bundle.update(elements) else: # Send autoupdate directly - handle_collection_elements(elements.values()) + handle_changed_elements(elements.values()) """ Global container for autoupdate bundles """ -autoupdate_bundle: Dict[int, Dict[str, CollectionElement]] = {} +autoupdate_bundle: Dict[int, Dict[str, Element]] = {} class AutoupdateBundleMiddleware: @@ -104,39 +123,36 @@ class AutoupdateBundleMiddleware: response = self.get_response(request) - bundle: Dict[str, CollectionElement] = autoupdate_bundle.pop(thread_id) - handle_collection_elements(bundle.values()) + bundle: Dict[str, Element] = autoupdate_bundle.pop(thread_id) + handle_changed_elements(bundle.values()) return response -def handle_collection_elements(collection_elements: Iterable[CollectionElement]) -> None: +def handle_changed_elements(elements: Iterable[Element]) -> None: """ - Helper function, that sends collection_elements through a channel to the + Helper function, that sends elements through a channel to the autoupdate system and updates the cache. - Does nothing if collection_elements is empty. + Does nothing if elements is empty. """ - async def update_cache(collection_elements: Iterable[CollectionElement]) -> int: + async def update_cache() -> int: """ Async helper function to update the cache. Returns the change_id """ cache_elements: Dict[str, Optional[Dict[str, Any]]] = {} - for element in collection_elements: - element_id = get_element_id(element.collection_string, element.id) - if element.is_deleted(): - cache_elements[element_id] = None - else: - cache_elements[element_id] = element.get_full_data() + for element in elements: + element_id = get_element_id(element['collection_string'], element['id']) + cache_elements[element_id] = element['full_data'] return await element_cache.change_elements(cache_elements) - async def async_handle_collection_elements(collection_elements: Iterable[CollectionElement]) -> None: + async def async_handle_collection_elements() -> None: """ Async helper function to update cache and send autoupdate. """ # Update cache - change_id = await update_cache(collection_elements) + change_id = await update_cache() # Send autoupdate channel_layer = get_channel_layer() @@ -148,8 +164,8 @@ def handle_collection_elements(collection_elements: Iterable[CollectionElement]) }, ) - if collection_elements: + if elements: # TODO: Save histroy here using sync code # Update cache and send autoupdate - async_to_sync(async_handle_collection_elements)(collection_elements) + async_to_sync(async_handle_collection_elements)() diff --git a/openslides/utils/cache.py b/openslides/utils/cache.py index 0e216334b..ebc91ed57 100644 --- a/openslides/utils/cache.py +++ b/openslides/utils/cache.py @@ -3,16 +3,7 @@ import json from collections import defaultdict from datetime import datetime from time import sleep -from typing import ( - TYPE_CHECKING, - Any, - Callable, - Dict, - List, - Optional, - Tuple, - Type, -) +from typing import Any, Callable, Dict, List, Optional, Tuple, Type from asgiref.sync import async_to_sync from django.conf import settings @@ -25,17 +16,12 @@ from .cache_providers import ( get_all_cachables, ) from .redis import use_redis -from .utils import get_element_id, get_user_id, split_element_id - - -if TYPE_CHECKING: - # Dummy import Collection for mypy, can be fixed with python 3.7 - from .collection import CollectionElement # noqa +from .utils import get_element_id, split_element_id class ElementCache: """ - Cache for the CollectionElements. + Cache for the elements. Saves the full_data and if enabled the restricted data. @@ -217,23 +203,22 @@ class ElementCache: return None return json.loads(element.decode()) - async def exists_restricted_data(self, user: Optional['CollectionElement']) -> bool: + async def exists_restricted_data(self, user_id: int) -> bool: """ Returns True, if the restricted_data exists for the user. """ if not self.use_restricted_data_cache: return False - return await self.cache_provider.data_exists(get_user_id(user)) + return await self.cache_provider.data_exists(user_id) - async def del_user(self, user: Optional['CollectionElement']) -> None: + async def del_user(self, user_id: int) -> None: """ Removes one user from the resticted_data_cache. """ - await self.cache_provider.del_restricted_data(get_user_id(user)) + await self.cache_provider.del_restricted_data(user_id) - async def update_restricted_data( - self, user: Optional['CollectionElement']) -> None: + async def update_restricted_data(self, user_id: int) -> None: """ Updates the restricted data for an user from the full_data_cache. """ @@ -248,12 +233,12 @@ class ElementCache: # Try to write a special key. # If this succeeds, there is noone else currently updating the cache. # TODO: Make a timeout. Else this could block forever - lock_name = "restricted_data_{}".format(get_user_id(user)) + lock_name = "restricted_data_{}".format(user_id) if await self.cache_provider.set_lock(lock_name): future: asyncio.Future = asyncio.Future() - self.restricted_data_cache_updater[get_user_id(user)] = future + self.restricted_data_cache_updater[user_id] = future # Get change_id for this user - value = await self.cache_provider.get_change_id_user(get_user_id(user)) + value = await self.cache_provider.get_change_id_user(user_id) # If the change id is not in the cache yet, use -1 to get all data since 0 user_change_id = int(value) if value else -1 change_id = await self.get_current_change_id() @@ -264,35 +249,35 @@ class ElementCache: # The user_change_id is lower then the lowest change_id in the cache. # The whole restricted_data for that user has to be recreated. full_data_elements = await self.get_all_full_data() - await self.cache_provider.del_restricted_data(get_user_id(user)) + await self.cache_provider.del_restricted_data(user_id) else: # Remove deleted elements if deleted_elements: - await self.cache_provider.del_elements(deleted_elements, get_user_id(user)) + await self.cache_provider.del_elements(deleted_elements, user_id) mapping = {} for collection_string, full_data in full_data_elements.items(): restricter = self.cachables[collection_string].restrict_elements - elements = await restricter(user, full_data) + elements = await restricter(user_id, full_data) for element in elements: mapping.update( {get_element_id(collection_string, element['id']): json.dumps(element)}) mapping['_config:change_id'] = str(change_id) - await self.cache_provider.update_restricted_data(get_user_id(user), mapping) + await self.cache_provider.update_restricted_data(user_id, mapping) # Unset the lock await self.cache_provider.del_lock(lock_name) future.set_result(1) else: # Wait until the update if finshed - if get_user_id(user) in self.restricted_data_cache_updater: + if user_id in self.restricted_data_cache_updater: # The active worker is on the same asgi server, we can use the future - await self.restricted_data_cache_updater[get_user_id(user)] + await self.restricted_data_cache_updater[user_id] else: while await self.cache_provider.get_lock(lock_name): await asyncio.sleep(0.01) - async def get_all_restricted_data(self, user: Optional['CollectionElement']) -> Dict[str, List[Dict[str, Any]]]: + async def get_all_restricted_data(self, user_id: int) -> Dict[str, List[Dict[str, Any]]]: """ Like get_all_full_data but with restricted_data for an user. """ @@ -300,14 +285,14 @@ class ElementCache: all_restricted_data = {} for collection_string, full_data in (await self.get_all_full_data()).items(): restricter = self.cachables[collection_string].restrict_elements - elements = await restricter(user, full_data) + elements = await restricter(user_id, full_data) all_restricted_data[collection_string] = elements return all_restricted_data - await self.update_restricted_data(user) + await self.update_restricted_data(user_id) out: Dict[str, List[Dict[str, Any]]] = defaultdict(list) - restricted_data = await self.cache_provider.get_all_data(get_user_id(user)) + restricted_data = await self.cache_provider.get_all_data(user_id) for element_id, data in restricted_data.items(): if element_id.decode().startswith('_config'): continue @@ -317,7 +302,7 @@ class ElementCache: async def get_restricted_data( self, - user: Optional['CollectionElement'], + user_id: int, change_id: int = 0, max_change_id: int = -1) -> Tuple[Dict[str, List[Dict[str, Any]]], List[str]]: """ @@ -325,14 +310,14 @@ class ElementCache: """ if change_id == 0: # Return all data - return (await self.get_all_restricted_data(user), []) + return (await self.get_all_restricted_data(user_id), []) if not self.use_restricted_data_cache: changed_elements, deleted_elements = await self.get_full_data(change_id, max_change_id) restricted_data = {} for collection_string, full_data in changed_elements.items(): restricter = self.cachables[collection_string].restrict_elements - elements = await restricter(user, full_data) + elements = await restricter(user_id, full_data) restricted_data[collection_string] = elements return restricted_data, deleted_elements @@ -347,15 +332,15 @@ class ElementCache: # If another coroutine or another daphne server also updates the restricted # data, this waits until it is done. - await self.update_restricted_data(user) + await self.update_restricted_data(user_id) - raw_changed_elements, deleted_elements = await self.cache_provider.get_data_since(change_id, get_user_id(user), max_change_id) + raw_changed_elements, deleted_elements = await self.cache_provider.get_data_since(change_id, user_id, max_change_id) return ( {collection_string: [json.loads(value.decode()) for value in value_list] for collection_string, value_list in raw_changed_elements.items()}, deleted_elements) - async def get_element_restricted_data(self, user: Optional['CollectionElement'], collection_string: str, id: int) -> Optional[Dict[str, Any]]: + async def get_element_restricted_data(self, user_id: int, collection_string: str, id: int) -> Optional[Dict[str, Any]]: """ Returns the restricted_data of one element. @@ -366,12 +351,12 @@ class ElementCache: if full_data is None: return None restricter = self.cachables[collection_string].restrict_elements - restricted_data = await restricter(user, [full_data]) + restricted_data = await restricter(user_id, [full_data]) return restricted_data[0] if restricted_data else None - await self.update_restricted_data(user) + await self.update_restricted_data(user_id) - out = await self.cache_provider.get_element(get_element_id(collection_string, id), get_user_id(user)) + out = await self.cache_provider.get_element(get_element_id(collection_string, id), user_id) return json.loads(out.decode()) if out else None async def get_current_change_id(self) -> int: diff --git a/openslides/utils/cache_providers.py b/openslides/utils/cache_providers.py index 919e13722..991cb82dc 100644 --- a/openslides/utils/cache_providers.py +++ b/openslides/utils/cache_providers.py @@ -1,14 +1,5 @@ from collections import defaultdict -from typing import ( - TYPE_CHECKING, - Any, - Dict, - Iterable, - List, - Optional, - Set, - Tuple, -) +from typing import Any, Dict, Iterable, List, Optional, Set, Tuple from django.apps import apps from typing_extensions import Protocol @@ -21,11 +12,6 @@ if use_redis: from .redis import get_connection, aioredis -if TYPE_CHECKING: - # Dummy import Collection for mypy, can be fixed with python 3.7 - from .collection import CollectionElement # noqa - - class ElementCacheProvider(Protocol): """ Base class for cache provider. @@ -506,7 +492,7 @@ class Cachable(Protocol): async def restrict_elements( self, - user: Optional['CollectionElement'], + user_id: int, elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """ Converts full_data to restricted_data. diff --git a/openslides/utils/collection.py b/openslides/utils/collection.py deleted file mode 100644 index 917572dc0..000000000 --- a/openslides/utils/collection.py +++ /dev/null @@ -1,160 +0,0 @@ -from typing import TYPE_CHECKING, Any, Dict, Generator, List, Optional, Type - -from asgiref.sync import async_to_sync -from django.apps import apps -from django.db.models import Model -from mypy_extensions import TypedDict - -from .cache import element_cache - - -if TYPE_CHECKING: - from .access_permissions import BaseAccessPermissions - - -AutoupdateFormat = TypedDict( - 'AutoupdateFormat', - { - 'changed': Dict[str, List[Dict[str, Any]]], - 'deleted': Dict[str, List[int]], - 'from_change_id': int, - 'to_change_id': int, - 'all_data': bool, - }, -) - - -class CollectionElement: - def __init__(self, instance: Model = None, deleted: bool = False, collection_string: str = None, - id: int = None, full_data: Dict[str, Any] = None) -> None: - """ - Do not use this. Use the methods from_instance() or from_values(). - """ - self.instance = instance - self.deleted = deleted - self.full_data = full_data - if instance is not None: - # Collection element is created via instance - self.collection_string = instance.get_collection_string() - self.id = instance.pk - elif collection_string is not None and id is not None: - # Collection element is created via values - self.collection_string = collection_string - self.id = id - else: - raise RuntimeError( - 'Invalid state. Use CollectionElement.from_instance() or ' - 'CollectionElement.from_values() but not CollectionElement() ' - 'directly.') - - if not self.deleted: - self.get_full_data() # This raises DoesNotExist, if the element does not exist. - - @classmethod - def from_instance( - cls, instance: Model, deleted: bool = False) -> 'CollectionElement': - """ - Returns a collection element from a database instance. - - This will also update the instance in the cache. - - If deleted is set to True, the element is deleted from the cache. - """ - return cls(instance=instance, deleted=deleted) - - @classmethod - def from_values(cls, collection_string: str, id: int, deleted: bool = False, - full_data: Dict[str, Any] = None) -> 'CollectionElement': - """ - Returns a collection element from a collection_string and an id. - - If deleted is set to True, the element is deleted from the cache. - - With the argument full_data, the content of the CollectionElement can be set. - It has to be a dict in the format that is used be access_permission.get_full_data(). - """ - return cls(collection_string=collection_string, id=id, deleted=deleted, full_data=full_data) - - def __eq__(self, collection_element: 'CollectionElement') -> bool: # type: ignore - """ - Compares two collection_elements. - - Two collection elements are equal, if they have the same collection_string - and id. - """ - return (self.collection_string == collection_element.collection_string and - self.id == collection_element.id) - - def get_model(self) -> Type[Model]: - """ - Returns the django model that is used for this collection. - """ - return get_model_from_collection_string(self.collection_string) - - def get_access_permissions(self) -> 'BaseAccessPermissions': - """ - Returns the get_access_permissions object for the this collection element. - """ - return self.get_model().get_access_permissions() - - def get_full_data(self) -> Dict[str, Any]: - """ - Returns the full_data of this collection_element from with all other - dics can be generated. - - Raises a DoesNotExist error on the requested the coresponding model, if - the object does neither exist in the cache nor in the database. - """ - # If the full_data is already loaded, return it - # If there is a db_instance, use it to get the full_data - # else: use the cache. - if self.full_data is None: - if self.instance is None: - # The type of data has to be set for mypy - data: Optional[Dict[str, Any]] = None - data = async_to_sync(element_cache.get_element_full_data)(self.collection_string, self.id) - if data is None: - raise self.get_model().DoesNotExist( - "Collection {} with id {} does not exist".format(self.collection_string, self.id)) - self.full_data = data - else: - self.full_data = self.get_access_permissions().get_full_data(self.instance) - return self.full_data - - def is_deleted(self) -> bool: - """ - Returns Ture if the item is marked as deleted. - """ - return self.deleted - - -_models_to_collection_string: Dict[str, Type[Model]] = {} - - -def get_model_from_collection_string(collection_string: str) -> Type[Model]: - """ - Returns a model class which belongs to the argument collection_string. - """ - def model_generator() -> Generator[Type[Model], None, None]: - """ - Yields all models of all apps. - """ - for app_config in apps.get_app_configs(): - for model in app_config.get_models(): - yield model - - # On the first run, generate the dict. It can not change at runtime. - if not _models_to_collection_string: - for model in model_generator(): - try: - get_collection_string = model.get_collection_string - except AttributeError: - # Skip models which do not have the method get_collection_string. - pass - else: - _models_to_collection_string[get_collection_string()] = model - try: - model = _models_to_collection_string[collection_string] - except KeyError: - raise ValueError('Invalid message. A valid collection_string is missing. Got {}'.format(collection_string)) - return model diff --git a/openslides/utils/consumers.py b/openslides/utils/consumers.py index e05184549..6e532ca12 100644 --- a/openslides/utils/consumers.py +++ b/openslides/utils/consumers.py @@ -2,9 +2,9 @@ from collections import defaultdict from typing import Any, Dict, List from urllib.parse import parse_qs -from .auth import async_anonymous_is_enabled, user_to_collection_user +from .auth import async_anonymous_is_enabled +from .autoupdate import AutoupdateFormat from .cache import element_cache, split_element_id -from .collection import AutoupdateFormat from .websocket import ProtocollAsyncJsonWebsocketConsumer, get_element_data @@ -23,12 +23,10 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer): Sends the startup data to the user. """ - # If the user is the anonymous user, change the value to None - if self.scope['user'].id is None: - self.scope['user'] = None - + # self.scope['user'] is the full_data dict of the user. For an + # anonymous user is it the dict {'id': 0} change_id = None - if not await async_anonymous_is_enabled() and self.scope['user'] is None: + if not await async_anonymous_is_enabled() and not self.scope['user']['id']: await self.close() return @@ -48,7 +46,7 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer): if change_id is not None: try: - data = await get_element_data(user_to_collection_user(self.scope['user']), change_id) + data = await get_element_data(self.scope['user']['id'], change_id) except ValueError: # When the change_id is to big, do nothing pass @@ -65,7 +63,7 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer): """ Send a notify message to the user. """ - user_id = self.scope['user'].id if self.scope['user'] else 0 + user_id = self.scope['user']['id'] out = [] for item in event['incomming']: @@ -87,7 +85,7 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer): """ change_id = event['change_id'] changed_elements, deleted_elements_ids = await element_cache.get_restricted_data( - user_to_collection_user(self.scope['user']), + self.scope['user']['id'], change_id, max_change_id=change_id) diff --git a/openslides/utils/middleware.py b/openslides/utils/middleware.py index 152ffae40..1642f103e 100644 --- a/openslides/utils/middleware.py +++ b/openslides/utils/middleware.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, Union +from typing import Any, Dict, Optional from channels.auth import ( AuthMiddleware, @@ -8,37 +8,42 @@ from channels.auth import ( ) from django.conf import settings from django.contrib.auth import BACKEND_SESSION_KEY, HASH_SESSION_KEY -from django.contrib.auth.models import AnonymousUser from django.utils.crypto import constant_time_compare from .cache import element_cache -from .collection import CollectionElement class CollectionAuthMiddleware(AuthMiddleware): """ - Like the channels AuthMiddleware but returns a CollectionElement instead of + Like the channels AuthMiddleware but returns a user dict id instead of a django Model as user. """ + def populate_scope(self, scope: Dict[str, Any]) -> None: + # Make sure we have a session + if "session" not in scope: + raise ValueError( + "AuthMiddleware cannot find session in scope. SessionMiddleware must be above it." + ) + # Add it to the scope if it's not there already + if "user" not in scope: + scope["user"] = {} async def resolve_scope(self, scope: Dict[str, Any]) -> None: - scope["user"]._wrapped = await get_user(scope) + scope["user"].update(await get_user(scope)) -async def get_user(scope: Dict[str, Any]) -> Union[CollectionElement, AnonymousUser]: +async def get_user(scope: Dict[str, Any]) -> Dict[str, Any]: """ - Returns a User-CollectionElement from a channels-scope-session. + Returns a user id from a channels-scope-session. - If no user is retrieved, return AnonymousUser. + If no user is retrieved, return {'id': 0}. """ - # This can not return None because a LazyObject can not become None - # This code is basicly from channels.auth: # https://github.com/django/channels/blob/d5e81a78e96770127da79248349808b6ee6ec2a7/channels/auth.py#L16 if "session" not in scope: raise ValueError("Cannot find session in scope. You should wrap your consumer in SessionMiddleware.") session = scope["session"] - user = None + user: Optional[Dict[str, Any]] = None try: user_id = _get_user_session_key(session) backend_path = session[BACKEND_SESSION_KEY] @@ -47,7 +52,7 @@ async def get_user(scope: Dict[str, Any]) -> Union[CollectionElement, AnonymousU else: if backend_path in settings.AUTHENTICATION_BACKENDS: user = await element_cache.get_element_full_data("users/user", user_id) - if user is not None: + if user: # Verify the session session_hash = session.get(HASH_SESSION_KEY) session_hash_verified = session_hash and constant_time_compare( @@ -56,7 +61,7 @@ async def get_user(scope: Dict[str, Any]) -> Union[CollectionElement, AnonymousU if not session_hash_verified: session.flush() user = None - return CollectionElement.from_values("users/user", user_id, full_data=user) if user else AnonymousUser() + return user or {'id': 0} # Handy shortcut for applying all three layers at once diff --git a/openslides/utils/models.py b/openslides/utils/models.py index 74389a5b3..d8f556e34 100644 --- a/openslides/utils/models.py +++ b/openslides/utils/models.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING, Any, Dict, List, Optional +from typing import Any, Dict, List, Optional from django.core.exceptions import ImproperlyConfigured from django.db import models @@ -7,11 +7,6 @@ from .access_permissions import BaseAccessPermissions from .utils import convert_camel_case_to_pseudo_snake_case -if TYPE_CHECKING: - # Dummy import Collection for mypy, can be fixed with python 3.7 - from .collection import CollectionElement # noqa - - class MinMaxIntegerField(models.IntegerField): """ IntegerField with options to set a min- and a max-value. @@ -78,10 +73,8 @@ class RESTModelMixin: If skip_autoupdate is set to True, then the autoupdate system is not informed about the model changed. This also means, that the model cache - is not updated. You have to do this manually, by creating a collection - element from the instance: - - CollectionElement.from_instance(instance) + is not updated. You have to do this manually by calling + inform_changed_data(). """ # We don't know how to fix this circular import from .autoupdate import inform_changed_data @@ -96,14 +89,8 @@ class RESTModelMixin: If skip_autoupdate is set to True, then the autoupdate system is not informed about the model changed. This also means, that the model cache - is not updated. You have to do this manually, by creating a collection - element from the instance: - - CollectionElement.from_instance(instance, deleted=True) - - or - - CollectionElement.from_values(collection_string, id, deleted=True) + is not updated. You have to do this manually by calling + inform_deleted_data(). """ # We don't know how to fix this circular import from .autoupdate import inform_changed_data, inform_deleted_data @@ -131,14 +118,20 @@ class RESTModelMixin: query = cls.objects # type: ignore # Build a dict from the instance id to the full_data - return [cls.get_access_permissions().get_full_data(instance) for instance in query.all()] + return [instance.get_full_data() for instance in query.all()] @classmethod async def restrict_elements( cls, - user: Optional['CollectionElement'], + user_id: int, elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """ Converts a list of elements from full_data to restricted_data. """ - return await cls.get_access_permissions().get_restricted_data(elements, user) + return await cls.get_access_permissions().get_restricted_data(elements, user_id) + + def get_full_data(self) -> Dict[str, Any]: + """ + Returns the full_data of the instance. + """ + return self.get_access_permissions().get_full_data(self) diff --git a/openslides/utils/rest_api.py b/openslides/utils/rest_api.py index cbd1578e8..ffb6e3b7f 100644 --- a/openslides/utils/rest_api.py +++ b/openslides/utils/rest_api.py @@ -42,7 +42,6 @@ from rest_framework.viewsets import ( ) from .access_permissions import BaseAccessPermissions -from .auth import user_to_collection_user from .cache import element_cache @@ -197,7 +196,7 @@ class ListModelMixin(_ListModelMixin): # The corresponding queryset does not support caching. response = super().list(request, *args, **kwargs) else: - all_restricted_data = async_to_sync(element_cache.get_all_restricted_data)(user_to_collection_user(request.user)) + all_restricted_data = async_to_sync(element_cache.get_all_restricted_data)(request.user.pk or 0) response = Response(all_restricted_data.get(collection_string, [])) return response @@ -215,8 +214,8 @@ class RetrieveModelMixin(_RetrieveModelMixin): response = super().retrieve(request, *args, **kwargs) else: lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field - user = user_to_collection_user(request.user) - content = async_to_sync(element_cache.get_element_restricted_data)(user, collection_string, self.kwargs[lookup_url_kwarg]) + user_id = request.user.pk or 0 + content = async_to_sync(element_cache.get_element_restricted_data)(user_id, collection_string, self.kwargs[lookup_url_kwarg]) if content is None: raise Http404 response = Response(content) diff --git a/openslides/utils/test.py b/openslides/utils/test.py index abc47ca12..b5d321d4d 100644 --- a/openslides/utils/test.py +++ b/openslides/utils/test.py @@ -1,12 +1,7 @@ from django.test import TestCase as _TestCase -from ..core.config import config - class TestCase(_TestCase): """ - Resets the config object after each test. + Does currently nothing. """ - - def tearDown(self) -> None: - config.save_default_values() diff --git a/openslides/utils/utils.py b/openslides/utils/utils.py index 1c64c4ec5..0e12298a9 100644 --- a/openslides/utils/utils.py +++ b/openslides/utils/utils.py @@ -1,13 +1,11 @@ import re -from typing import TYPE_CHECKING, Dict, Optional, Tuple, Union +from typing import Dict, Generator, Tuple, Type, Union import roman +from django.apps import apps +from django.db.models import Model -if TYPE_CHECKING: - # Dummy import Collection for mypy, can be fixed with python 3.7 - from .collection import CollectionElement # noqa - CAMEL_CASE_TO_PSEUDO_SNAKE_CASE_CONVERSION_REGEX_1 = re.compile('(.)([A-Z][a-z]+)') CAMEL_CASE_TO_PSEUDO_SNAKE_CASE_CONVERSION_REGEX_2 = re.compile('([a-z0-9])([A-Z])') @@ -54,19 +52,6 @@ def split_element_id(element_id: Union[str, bytes]) -> Tuple[str, int]: return (collection_str, int(id)) -def get_user_id(user: Optional['CollectionElement']) -> int: - """ - Returns the user id for an CollectionElement user. - - Returns 0 for anonymous. - """ - if user is None: - user_id = 0 - else: - user_id = user.id - return user_id - - def str_dict_to_bytes(str_dict: Dict[str, str]) -> Dict[bytes, bytes]: """ Converts the key and the value of a dict from str to bytes. @@ -75,3 +60,35 @@ def str_dict_to_bytes(str_dict: Dict[str, str]) -> Dict[bytes, bytes]: for key, value in str_dict.items(): out[key.encode()] = value.encode() return out + + +_models_to_collection_string: Dict[str, Type[Model]] = {} + + +def get_model_from_collection_string(collection_string: str) -> Type[Model]: + """ + Returns a model class which belongs to the argument collection_string. + """ + def model_generator() -> Generator[Type[Model], None, None]: + """ + Yields all models of all apps. + """ + for app_config in apps.get_app_configs(): + for model in app_config.get_models(): + yield model + + # On the first run, generate the dict. It can not change at runtime. + if not _models_to_collection_string: + for model in model_generator(): + try: + get_collection_string = model.get_collection_string + except AttributeError: + # Skip models which do not have the method get_collection_string. + pass + else: + _models_to_collection_string[get_collection_string()] = model + try: + model = _models_to_collection_string[collection_string] + except KeyError: + raise ValueError('Invalid message. A valid collection_string is missing. Got {}'.format(collection_string)) + return model diff --git a/openslides/utils/websocket.py b/openslides/utils/websocket.py index 3c8248f22..19e24d1fe 100644 --- a/openslides/utils/websocket.py +++ b/openslides/utils/websocket.py @@ -4,8 +4,8 @@ from typing import Any, Dict, List, Optional import jsonschema from channels.generic.websocket import AsyncJsonWebsocketConsumer +from .autoupdate import AutoupdateFormat from .cache import element_cache -from .collection import AutoupdateFormat, CollectionElement from .utils import split_element_id @@ -126,7 +126,7 @@ def register_client_message(websocket_client_message: BaseWebsocketClientMessage schema['anyOf'].append(message_schema) -async def get_element_data(user: Optional[CollectionElement], change_id: int = 0) -> AutoupdateFormat: +async def get_element_data(user_id: int, change_id: int = 0) -> AutoupdateFormat: """ Returns all element data since a change_id. """ @@ -134,10 +134,10 @@ async def get_element_data(user: Optional[CollectionElement], change_id: int = 0 if change_id > current_change_id: raise ValueError("Requested change_id is higher this highest change_id.") try: - changed_elements, deleted_element_ids = await element_cache.get_restricted_data(user, change_id, current_change_id) + changed_elements, deleted_element_ids = await element_cache.get_restricted_data(user_id, change_id, current_change_id) except RuntimeError: # The change_id is lower the the lowerst change_id in redis. Return all data - changed_elements = await element_cache.get_all_restricted_data(user) + changed_elements = await element_cache.get_all_restricted_data(user_id) all_data = True deleted_elements: Dict[str, List[int]] = {} else: diff --git a/tests/integration/agenda/test_viewset.py b/tests/integration/agenda/test_viewset.py index c1aa10067..89a615f1e 100644 --- a/tests/integration/agenda/test_viewset.py +++ b/tests/integration/agenda/test_viewset.py @@ -14,7 +14,6 @@ from openslides.motions.models import Motion from openslides.topics.models import Topic from openslides.users.models import Group from openslides.utils.autoupdate import inform_changed_data -from openslides.utils.collection import CollectionElement from openslides.utils.test import TestCase from ..helpers import count_queries @@ -199,7 +198,6 @@ class ManageSpeaker(TestCase): admin.groups.add(group_delegates) admin.groups.remove(group_admin) inform_changed_data(admin) - CollectionElement.from_instance(admin) response = self.client.post( reverse('item-manage-speaker', args=[self.item.pk]), diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 38b6c9f46..5083cd195 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List from asgiref.sync import sync_to_async from django.db import DEFAULT_DB_ALIAS, connections @@ -6,9 +6,7 @@ from django.test.utils import CaptureQueriesContext from openslides.core.config import config from openslides.users.models import User -from openslides.utils.autoupdate import inform_data_collection_element_list -from openslides.utils.cache import element_cache, get_element_id -from openslides.utils.collection import CollectionElement +from openslides.utils.autoupdate import Element, inform_changed_elements class TConfig: @@ -29,7 +27,7 @@ class TConfig: async def restrict_elements( self, - user: Optional['CollectionElement'], + user_id: int, elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: return elements @@ -52,7 +50,7 @@ class TUser: async def restrict_elements( self, - user: Optional['CollectionElement'], + user_id: int, elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: return elements @@ -64,9 +62,8 @@ async def set_config(key, value): collection_string = config.get_collection_string() config_id = config.key_to_id[key] # type: ignore full_data = {'id': config_id, 'key': key, 'value': value} - await element_cache.change_elements({get_element_id(collection_string, config_id): full_data}) - await sync_to_async(inform_data_collection_element_list)([ - CollectionElement.from_values(collection_string, config_id, full_data=full_data)]) + await sync_to_async(inform_changed_elements)([ + Element(id=config_id, collection_string=collection_string, full_data=full_data)]) def count_queries(func, *args, **kwargs) -> int: diff --git a/tests/integration/utils/test_collection.py b/tests/integration/utils/test_collection.py deleted file mode 100644 index d2c7f8d2f..000000000 --- a/tests/integration/utils/test_collection.py +++ /dev/null @@ -1,28 +0,0 @@ -from openslides.topics.models import Topic -from openslides.utils import collection -from openslides.utils.test import TestCase - - -class TestCollectionElementCache(TestCase): - def test_with_cache(self): - """ - Tests that no db query is used when the valie is in the cache. - - The value is added to the test when .create(...) is called. This hits - the autoupdate system, which fills the cache. - """ - topic = Topic.objects.create(title='test topic') - collection_element = collection.CollectionElement.from_values('topics/topic', 1) - - with self.assertNumQueries(0): - collection_element = collection.CollectionElement.from_values('topics/topic', 1) - instance = collection_element.get_full_data() - self.assertEqual(topic.title, instance['title']) - - def test_fail_early(self): - """ - Tests that a CollectionElement.from_values fails, if the object does - not exist. - """ - with self.assertRaises(Topic.DoesNotExist): - collection.CollectionElement.from_values('topics/topic', 999) diff --git a/tests/unit/utils/cache_provider.py b/tests/unit/utils/cache_provider.py index 7e57e5767..13a8c3aa8 100644 --- a/tests/unit/utils/cache_provider.py +++ b/tests/unit/utils/cache_provider.py @@ -1,13 +1,10 @@ import asyncio -from typing import Any, Callable, Dict, List, Optional +from typing import Any, Callable, Dict, List from openslides.utils.cache_providers import Cachable, MemmoryCacheProvider -from openslides.utils.collection import CollectionElement -def restrict_elements( - user: Optional[CollectionElement], - elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: +def restrict_elements(elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """ Adds the prefix 'restricted_' to all values except id. """ @@ -32,8 +29,8 @@ class Collection1: {'id': 1, 'value': 'value1'}, {'id': 2, 'value': 'value2'}] - async def restrict_elements(self, user: Optional[CollectionElement], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - return restrict_elements(user, elements) + async def restrict_elements(self, user_id: int, elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + return restrict_elements(elements) class Collection2: @@ -45,8 +42,8 @@ class Collection2: {'id': 1, 'key': 'value1'}, {'id': 2, 'key': 'value2'}] - async def restrict_elements(self, user: Optional[CollectionElement], elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - return restrict_elements(user, elements) + async def restrict_elements(self, user_id: int, elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + return restrict_elements(elements) def get_cachable_provider(cachables: List[Cachable] = [Collection1(), Collection2()]) -> Callable[[], List[Cachable]]: diff --git a/tests/unit/utils/test_cache.py b/tests/unit/utils/test_cache.py index 43b505390..a96bdb8fe 100644 --- a/tests/unit/utils/test_cache.py +++ b/tests/unit/utils/test_cache.py @@ -212,7 +212,7 @@ async def test_exists_restricted_data(element_cache): 'app/collection2:1': '{"id": 1, "key": "value1"}', 'app/collection2:2': '{"id": 2, "key": "value2"}'}} - result = await element_cache.exists_restricted_data(None) + result = await element_cache.exists_restricted_data(0) assert result @@ -226,7 +226,7 @@ async def test_exists_restricted_data_do_not_use_restricted_data(element_cache): 'app/collection2:1': '{"id": 1, "key": "value1"}', 'app/collection2:2': '{"id": 2, "key": "value2"}'}} - result = await element_cache.exists_restricted_data(None) + result = await element_cache.exists_restricted_data(0) assert not result @@ -240,7 +240,7 @@ async def test_del_user(element_cache): 'app/collection2:1': '{"id": 1, "key": "value1"}', 'app/collection2:2': '{"id": 2, "key": "value2"}'}} - await element_cache.del_user(None) + await element_cache.del_user(0) assert not element_cache.cache_provider.restricted_data @@ -249,7 +249,7 @@ async def test_del_user(element_cache): async def test_del_user_for_empty_user(element_cache): element_cache.use_restricted_data_cache = True - await element_cache.del_user(None) + await element_cache.del_user(0) assert not element_cache.cache_provider.restricted_data @@ -258,7 +258,7 @@ async def test_del_user_for_empty_user(element_cache): async def test_update_restricted_data(element_cache): element_cache.use_restricted_data_cache = True - await element_cache.update_restricted_data(None) + await element_cache.update_restricted_data(0) assert decode_dict(element_cache.cache_provider.restricted_data[0]) == decode_dict({ 'app/collection1:1': '{"id": 1, "value": "restricted_value1"}', @@ -276,7 +276,7 @@ async def test_update_restricted_data(element_cache): async def test_update_restricted_data_disabled_restricted_data(element_cache): element_cache.use_restricted_data_cache = False - await element_cache.update_restricted_data(None) + await element_cache.update_restricted_data(0) assert not element_cache.cache_provider.restricted_data @@ -289,7 +289,7 @@ async def test_update_restricted_data_to_low_change_id(element_cache): element_cache.cache_provider.change_id_data = { 3: {'app/collection1:1'}} - await element_cache.update_restricted_data(None) + await element_cache.update_restricted_data(0) assert decode_dict(element_cache.cache_provider.restricted_data[0]) == decode_dict({ 'app/collection1:1': '{"id": 1, "value": "restricted_value1"}', @@ -307,7 +307,7 @@ async def test_update_restricted_data_with_same_id(element_cache): element_cache.cache_provider.change_id_data = { 1: {'app/collection1:1'}} - await element_cache.update_restricted_data(None) + await element_cache.update_restricted_data(0) # Same id means, there is nothing to do assert element_cache.cache_provider.restricted_data[0] == { @@ -323,7 +323,7 @@ async def test_update_restricted_data_with_deleted_elements(element_cache): element_cache.cache_provider.change_id_data = { 2: {'app/collection1:3'}} - await element_cache.update_restricted_data(None) + await element_cache.update_restricted_data(0) assert element_cache.cache_provider.restricted_data[0] == { '_config:change_id': '2'} @@ -341,7 +341,7 @@ async def test_update_restricted_data_second_worker_on_different_server(element_ await element_cache.cache_provider.set_lock("restricted_data_0") await element_cache.cache_provider.del_lock_after_wait("restricted_data_0") - await element_cache.update_restricted_data(None) + await element_cache.update_restricted_data(0) # Restricted_data_should not be set on second worker assert element_cache.cache_provider.restricted_data == {0: {}} @@ -361,7 +361,7 @@ async def test_update_restricted_data_second_worker_on_same_server(element_cache await element_cache.cache_provider.set_lock("restricted_data_0") await element_cache.cache_provider.del_lock_after_wait("restricted_data_0", future) - await element_cache.update_restricted_data(None) + await element_cache.update_restricted_data(0) # Restricted_data_should not be set on second worker assert element_cache.cache_provider.restricted_data == {0: {}} @@ -371,7 +371,7 @@ async def test_update_restricted_data_second_worker_on_same_server(element_cache async def test_get_all_restricted_data(element_cache): element_cache.use_restricted_data_cache = True - result = await element_cache.get_all_restricted_data(None) + result = await element_cache.get_all_restricted_data(0) assert sort_dict(result) == sort_dict({ 'app/collection1': [{"id": 1, "value": "restricted_value1"}, {"id": 2, "value": "restricted_value2"}], @@ -381,7 +381,7 @@ async def test_get_all_restricted_data(element_cache): @pytest.mark.asyncio async def test_get_all_restricted_data_disabled_restricted_data_cache(element_cache): element_cache.use_restricted_data_cache = False - result = await element_cache.get_all_restricted_data(None) + result = await element_cache.get_all_restricted_data(0) assert sort_dict(result) == sort_dict({ 'app/collection1': [{"id": 1, "value": "restricted_value1"}, {"id": 2, "value": "restricted_value2"}], @@ -392,7 +392,7 @@ async def test_get_all_restricted_data_disabled_restricted_data_cache(element_ca async def test_get_restricted_data_change_id_0(element_cache): element_cache.use_restricted_data_cache = True - result = await element_cache.get_restricted_data(None, 0) + result = await element_cache.get_restricted_data(0, 0) assert sort_dict(result[0]) == sort_dict({ 'app/collection1': [{"id": 1, "value": "restricted_value1"}, {"id": 2, "value": "restricted_value2"}], @@ -404,7 +404,7 @@ async def test_get_restricted_data_disabled_restricted_data_cache(element_cache) element_cache.use_restricted_data_cache = False element_cache.cache_provider.change_id_data = {1: {'app/collection1:1', 'app/collection1:3'}} - result = await element_cache.get_restricted_data(None, 1) + result = await element_cache.get_restricted_data(0, 1) assert result == ( {'app/collection1': [{"id": 1, "value": "restricted_value1"}]}, @@ -417,7 +417,7 @@ async def test_get_restricted_data_change_id_lower_then_in_redis(element_cache): element_cache.cache_provider.change_id_data = {2: {'app/collection1:1'}} with pytest.raises(RuntimeError): - await element_cache.get_restricted_data(None, 1) + await element_cache.get_restricted_data(0, 1) @pytest.mark.asyncio @@ -425,7 +425,7 @@ async def test_get_restricted_data_change_with_id(element_cache): element_cache.use_restricted_data_cache = True element_cache.cache_provider.change_id_data = {2: {'app/collection1:1'}} - result = await element_cache.get_restricted_data(None, 2) + result = await element_cache.get_restricted_data(0, 2) assert result == ({'app/collection1': [{"id": 1, "value": "restricted_value1"}]}, []) diff --git a/tests/unit/utils/test_collection.py b/tests/unit/utils/test_collection.py deleted file mode 100644 index 0485f170b..000000000 --- a/tests/unit/utils/test_collection.py +++ /dev/null @@ -1,40 +0,0 @@ -from unittest import TestCase -from unittest.mock import patch - -from openslides.core.models import Projector -from openslides.utils import collection - - -class TestGetModelFromCollectionString(TestCase): - def test_known_app(self): - projector_model = collection.get_model_from_collection_string('core/projector') - - self.assertEqual(projector_model, Projector) - - def test_unknown_app(self): - with self.assertRaises(ValueError): - collection.get_model_from_collection_string('invalid/model') - - -class TestCollectionElement(TestCase): - def test_from_values(self): - with patch.object(collection.CollectionElement, 'get_full_data'): - collection_element = collection.CollectionElement.from_values('testmodule/model', 42) - - self.assertEqual(collection_element.collection_string, 'testmodule/model') - self.assertEqual(collection_element.id, 42) - - @patch.object(collection.CollectionElement, 'get_full_data') - def test_equal(self, mock_get_full_data): - self.assertEqual( - collection.CollectionElement.from_values('testmodule/model', 1), - collection.CollectionElement.from_values('testmodule/model', 1)) - self.assertEqual( - collection.CollectionElement.from_values('testmodule/model', 1), - collection.CollectionElement.from_values('testmodule/model', 1, deleted=True)) - self.assertNotEqual( - collection.CollectionElement.from_values('testmodule/model', 1), - collection.CollectionElement.from_values('testmodule/model', 2)) - self.assertNotEqual( - collection.CollectionElement.from_values('testmodule/model', 1), - collection.CollectionElement.from_values('testmodule/other_model', 1)) diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index 6d244ddba..3af9622d8 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -1,11 +1,22 @@ -from unittest import TestCase +import pytest +from openslides.core.models import Projector from openslides.utils import utils -class ToRomanTest(TestCase): - def test_to_roman_result(self): - self.assertEqual(utils.to_roman(3), 'III') +def test_to_roman_result(): + assert utils.to_roman(3) == 'III' - def test_to_roman_none(self): - self.assertEqual(utils.to_roman(-3), '-3') + +def test_to_roman_none(): + assert utils.to_roman(-3) == '-3' + + +def test_get_model_from_collection_string_known_app(): + projector_model = utils.get_model_from_collection_string('core/projector') + assert projector_model == Projector + + +def test_get_model_from_collection_string_unknown_app(): + with pytest.raises(ValueError): + utils.get_model_from_collection_string('invalid/model')