From ebabc291c57c9c72c7331ff91c08db32e2278e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Mon, 1 May 2017 23:12:42 +0200 Subject: [PATCH] Refactoring of data parsing for startup and autoupdate. --- CHANGELOG | 2 + openslides/agenda/access_permissions.py | 51 +++++++++--- openslides/agenda/signals.py | 7 +- openslides/assignments/access_permissions.py | 35 +++++--- openslides/assignments/signals.py | 15 ++-- openslides/core/signals.py | 12 +-- openslides/mediafiles/access_permissions.py | 30 ++++--- openslides/mediafiles/signals.py | 16 ++-- openslides/motions/access_permissions.py | 84 +++++++++++++------- openslides/motions/signals.py | 15 ++-- openslides/users/access_permissions.py | 76 +++++++++++------- openslides/utils/access_permissions.py | 25 +++--- openslides/utils/autoupdate.py | 58 +++++++++----- openslides/utils/cache.py | 29 +++---- openslides/utils/collection.py | 68 +++++++++------- tests/unit/utils/test_collection.py | 4 +- 16 files changed, 327 insertions(+), 200 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e44134ff0..8b94e1b2f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -42,6 +42,8 @@ Core: pdf [#3184, #3207, #3208]. - Fixing error when clearing empty chat [#3199]. - Added notify system [#3212]. +- Enhanced performance esp. for server restart and first connection of all + clients by refactorting autoupdate, Collection and AccessPermission [#3223]. General: - Switched from npm to Yarn [#3188]. diff --git a/openslides/agenda/access_permissions.py b/openslides/agenda/access_permissions.py index 60277fb6a..c6f34ce3e 100644 --- a/openslides/agenda/access_permissions.py +++ b/openslides/agenda/access_permissions.py @@ -1,5 +1,6 @@ from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import has_perm +from ..utils.collection import Collection class ItemAccessPermissions(BaseAccessPermissions): @@ -22,10 +23,13 @@ class ItemAccessPermissions(BaseAccessPermissions): # TODO: In the following method we use full_data['is_hidden'] but this can be out of date. - def get_restricted_data(self, full_data, user): + def get_restricted_data(self, container, user): """ Returns the restricted serialized data for the instance prepared for the user. + + We remove comments for non admins/managers and a lot of fields of + hidden items for users without permission to see hidden items. """ def filtered_data(full_data, blocked_keys): """ @@ -34,35 +38,56 @@ class ItemAccessPermissions(BaseAccessPermissions): whitelist = full_data.keys() - blocked_keys return {key: full_data[key] for key in whitelist} - # many_items is True, when there are more then one items in full_data. - many_items = not isinstance(full_data, dict) - full_data = full_data if many_items else [full_data] + # Expand full_data to a list if it is not one. + full_data = container.get_full_data() if isinstance(container, Collection) else [container.get_full_data()] + # Parse data. if has_perm(user, 'agenda.can_see'): - if has_perm(user, 'agenda.can_manage'): + if has_perm(user, 'agenda.can_manage') and has_perm(user, 'agenda.can_see_hidden_items'): + # Managers with special permission can see everything. data = full_data elif has_perm(user, 'agenda.can_see_hidden_items'): + # Non managers with special permission can see everything but comments. blocked_keys = ('comment',) data = [filtered_data(full, blocked_keys) for full in full_data] else: - data = [] - filtered_blocked_keys = full_data[0].keys() - ( + # Users without special permissin for hidden items. + + # In hidden case managers and non managers see only some fields + # so that list of speakers is provided regardless. + blocked_keys_hidden_case = full_data[0].keys() - ( 'id', 'title', 'speakers', 'speaker_list_closed', 'content_object') - not_filtered_blocked_keys = ('comment',) + + # In non hidden case managers see everything and non managers see + # everything but comments. + if has_perm(user, 'agenda.can_manage'): + blocked_keys_non_hidden_case = [] + else: + blocked_keys_non_hidden_case = ('comment',) + + data = [] for full in full_data: if full['is_hidden']: - blocked_keys = filtered_blocked_keys + data.append(filtered_data(full, blocked_keys_hidden_case)) else: - blocked_keys = not_filtered_blocked_keys - data.append(filtered_data(full, blocked_keys)) + data.append(filtered_data(full, blocked_keys_non_hidden_case)) else: - data = None + data = [] - return data if many_items else data[0] + # Reduce result to a single item or None if it was not a collection at + # the beginning of the method. + if isinstance(container, Collection): + restricted_data = data + elif data: + restricted_data = data[0] + else: + restricted_data = None + + return restricted_data def get_projector_data(self, full_data): """ diff --git a/openslides/agenda/signals.py b/openslides/agenda/signals.py index 9aaecc8ae..f71abe7a4 100644 --- a/openslides/agenda/signals.py +++ b/openslides/agenda/signals.py @@ -54,10 +54,11 @@ def get_permission_change_data(sender, permissions, **kwargs): break -def is_user_data_required(sender, request_user, user_data, **kwargs): +def is_user_data_required(sender, request_user, **kwargs): """ - If request_user can see the agenda, then returns all user ids that are - speakers in some agenda items. Else, it returns an empty set. + Returns all user ids that are displayed as speakers in any agenda item + if request_user can see the agenda. This function may return an empty + set. """ speakers = set() if has_perm(request_user, 'agenda.can_see'): diff --git a/openslides/assignments/access_permissions.py b/openslides/assignments/access_permissions.py index 8df50b07a..74bec1a7c 100644 --- a/openslides/assignments/access_permissions.py +++ b/openslides/assignments/access_permissions.py @@ -1,5 +1,6 @@ from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import has_perm +from ..utils.collection import Collection class AssignmentAccessPermissions(BaseAccessPermissions): @@ -24,26 +25,38 @@ class AssignmentAccessPermissions(BaseAccessPermissions): serializer_class = AssignmentShortSerializer return serializer_class - def get_restricted_data(self, full_data, user): + def get_restricted_data(self, container, user): """ 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. """ + # Expand full_data to a list if it is not one. + full_data = container.get_full_data() if isinstance(container, Collection) else [container.get_full_data()] + + # Parse data. if has_perm(user, 'assignments.can_see') and has_perm(user, 'assignments.can_manage'): data = full_data elif has_perm(user, 'assignments.can_see'): - many_items = not isinstance(full_data, dict) - full_data_list = full_data if many_items else [full_data] - out = [] - for full_data in full_data_list: - data = full_data.copy() - data['polls'] = [poll for poll in data['polls'] if data['published']] - out.append(data) - data = out if many_items else out[0] + # Exclude unpublished polls. + data = [] + for full in full_data: + full_copy = full.copy() + full_copy['polls'] = [poll for poll in full['polls'] if poll['published']] + data.append(full_copy) else: - data = None - return data + data = [] + + # Reduce result to a single item or None if it was not a collection at + # the beginning of the method. + if isinstance(container, Collection): + restricted_data = data + elif data: + restricted_data = data[0] + else: + restricted_data = None + + return restricted_data def get_projector_data(self, full_data): """ diff --git a/openslides/assignments/signals.py b/openslides/assignments/signals.py index fdd65eb2e..0fef119cf 100644 --- a/openslides/assignments/signals.py +++ b/openslides/assignments/signals.py @@ -16,16 +16,17 @@ def get_permission_change_data(sender, permissions=None, **kwargs): yield from assignments_app.get_startup_elements() -def is_user_data_required(sender, request_user, user_data, **kwargs): +def is_user_data_required(sender, request_user, **kwargs): """ - If request_user can see assignments, then returns all user ids that are - displayed as candidates (including poll options). Else, it returns an empty set. + Returns all user ids that are displayed as candidates (including poll + options) in any assignment if request_user can see assignments. This + function may return an empty set. """ - user_ids = set() + candidates = set() if has_perm(request_user, 'assignments.can_see'): for assignment_collection_element in Collection(Assignment.get_collection_string()).element_generator(): full_data = assignment_collection_element.get_full_data() - user_ids.update(related_user['user_id'] for related_user in full_data['assignment_related_users']) + candidates.update(related_user['user_id'] for related_user in full_data['assignment_related_users']) for poll in full_data['polls']: - user_ids.update(option['candidate_id'] for option in poll['options']) - return user_ids + candidates.update(option['candidate_id'] for option in poll['options']) + return candidates diff --git a/openslides/core/signals.py b/openslides/core/signals.py index 744efb87f..b68e419f9 100644 --- a/openslides/core/signals.py +++ b/openslides/core/signals.py @@ -52,14 +52,14 @@ def get_permission_change_data(sender, permissions, **kwargs): yield Collection(core_app.get_model('ChatMessage').get_collection_string()) -def is_user_data_required(sender, request_user, user_data, **kwargs): +def is_user_data_required(sender, request_user, **kwargs): """ - If request_user can use chat, then returns all user ids that are - displayed as chatter. Else, it returns an empty set. + Returns all user ids that are displayed as chatters if request_user can + use the chat. This function may return an empty set. """ - user_ids = set() + chatters = set() if has_perm(request_user, 'core.can_use_chat'): for chat_message_collection_element in Collection(ChatMessage.get_collection_string()).element_generator(): full_data = chat_message_collection_element.get_full_data() - user_ids.add(full_data['user_id']) - return user_ids + chatters.add(full_data['user_id']) + return chatters diff --git a/openslides/mediafiles/access_permissions.py b/openslides/mediafiles/access_permissions.py index 9946c74f4..455c867ce 100644 --- a/openslides/mediafiles/access_permissions.py +++ b/openslides/mediafiles/access_permissions.py @@ -1,5 +1,6 @@ from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import has_perm +from ..utils.collection import Collection class MediafileAccessPermissions(BaseAccessPermissions): @@ -20,19 +21,30 @@ class MediafileAccessPermissions(BaseAccessPermissions): return MediafileSerializer - def get_restricted_data(self, full_data, user): + def get_restricted_data(self, container, user): """ Returns the restricted serialized data for the instance prepared - for the user. + for the user. Removes hidden mediafiles for some users. """ - data = None + # Expand full_data to a list if it is not one. + full_data = container.get_full_data() if isinstance(container, Collection) else [container.get_full_data()] + + # Parse data. if has_perm(user, 'mediafiles.can_see') and has_perm(user, 'mediafiles.can_see_hidden'): data = full_data elif has_perm(user, 'mediafiles.can_see'): - many_items = not isinstance(full_data, dict) - full_data_list = full_data if many_items else [full_data] - data = [full_data for full_data in full_data_list if not full_data['hidden']] - data = data if many_items else data[0] + # Exclude hidden mediafiles. + data = [full for full in full_data if not full['hidden']] else: - data = None - return data + data = [] + + # Reduce result to a single item or None if it was not a collection at + # the beginning of the method. + if isinstance(container, Collection): + restricted_data = data + elif data: + restricted_data = data[0] + else: + restricted_data = None + + return restricted_data diff --git a/openslides/mediafiles/signals.py b/openslides/mediafiles/signals.py index 44e593bdc..a9a06a7a6 100644 --- a/openslides/mediafiles/signals.py +++ b/openslides/mediafiles/signals.py @@ -16,17 +16,15 @@ def get_permission_change_data(sender, permissions=None, **kwargs): yield from mediafiles_app.get_startup_elements() -def is_user_data_required(sender, request_user, user_data, **kwargs): +def is_user_data_required(sender, request_user, **kwargs): """ - Returns True if request user can see mediafiles and user_data is required - to be displayed as uploader. - - If request_user can see mediafiles, then returns all user ids that are - displayed as uploader. Else, it returns an empty set. + Returns all user ids that are displayed as uploaders in any mediafile + if request_user can see mediafiles. This function may return an empty + set. """ - user_ids = set() + uploaders = set() if has_perm(request_user, 'mediafiles.can_see'): for mediafile_collection_element in Collection(Mediafile.get_collection_string()).element_generator(): full_data = mediafile_collection_element.get_full_data() - user_ids.add(full_data['uploader_id']) - return user_ids + uploaders.add(full_data['uploader_id']) + return uploaders diff --git a/openslides/motions/access_permissions.py b/openslides/motions/access_permissions.py index 7c13359ec..473133b0f 100644 --- a/openslides/motions/access_permissions.py +++ b/openslides/motions/access_permissions.py @@ -3,7 +3,7 @@ from copy import deepcopy from ..core.config import config from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import has_perm -from ..utils.collection import CollectionElement +from ..utils.collection import Collection, CollectionElement class MotionAccessPermissions(BaseAccessPermissions): @@ -24,51 +24,77 @@ class MotionAccessPermissions(BaseAccessPermissions): return MotionSerializer - def get_restricted_data(self, full_data, user): + def get_restricted_data(self, container, user): """ Returns the restricted serialized data for the instance prepared for the user. Removes motion if the user has not the permission to see the motion in this state. Removes non public comment fields for - some unauthorized users. + some unauthorized users. Ensures that a user can only see his own + personal notes. """ - many_items = not isinstance(full_data, dict) - full_data_list = full_data if many_items else [full_data] - out = [] - for full_data in full_data_list: - if isinstance(user, CollectionElement): - is_submitter = user.get_full_data()['id'] in full_data.get('submitters_id', []) - else: - # Anonymous users can not be submitters - is_submitter = False + # Expand full_data to a list if it is not one. + full_data = container.get_full_data() if isinstance(container, Collection) else [container.get_full_data()] - required_permission_to_see = full_data['state_required_permission_to_see'] - data = None - if has_perm(user, 'motions.can_see'): - if (not required_permission_to_see or - has_perm(user, required_permission_to_see) or - has_perm(user, 'motions.can_manage') or - is_submitter): - if has_perm(user, 'motions.can_see_and_manage_comments') or not full_data.get('comments'): - data = full_data + # Parse data. + if has_perm(user, '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 full.get('submitters_id', []) + else: + # Anonymous users can not be submitters. + is_submitter = False + + # Check see permission for this motion. + required_permission_to_see = full['state_required_permission_to_see'] + permission = ( + not required_permission_to_see or + has_perm(user, required_permission_to_see) or + has_perm(user, 'motions.can_manage') or + is_submitter) + + # Parse single motion. + if permission: + if has_perm(user, 'motions.can_see_and_manage_comments') or not full.get('comments'): + # Provide access to all fields. + motion = full else: - data = deepcopy(full_data) + # Set private comment fields to None. + full_copy = deepcopy(full) for i, field in enumerate(config['motions_comments']): if not field.get('public'): try: - data['comments'][i] = None + full_copy['comments'][i] = None except IndexError: # No data in range. Just do nothing. pass + motion = full_copy + # Now filter personal notes. - data = data.copy() - data['personal_notes'] = [] + motion = motion.copy() + motion['personal_notes'] = [] if user is not None: - for personal_note in full_data.get('personal_notes', []): + for personal_note in full.get('personal_notes', []): if personal_note.get('user_id') == user.id: - data['personal_notes'].append(personal_note) + motion['personal_notes'].append(personal_note) break - out.append(data) - return out if many_items else out[0] + + data.append(motion) + else: + data = [] + + # Reduce result to a single item or None if it was not a collection at + # the beginning of the method. + if isinstance(container, Collection): + restricted_data = data + elif data: + restricted_data = data[0] + else: + restricted_data = None + + return restricted_data def get_projector_data(self, full_data): """ diff --git a/openslides/motions/signals.py b/openslides/motions/signals.py index 8b2db11fa..5b0a37016 100644 --- a/openslides/motions/signals.py +++ b/openslides/motions/signals.py @@ -118,15 +118,16 @@ def get_permission_change_data(sender, permissions, **kwargs): yield from motions_app.get_startup_elements() -def is_user_data_required(sender, request_user, user_data, **kwargs): +def is_user_data_required(sender, request_user, **kwargs): """ - If request_user can see motions, then returns all user ids that are - displayed as submitter or supporter. Else, it returns an empty set. + Returns all user ids that are displayed as as submitter or supporter in + any motion if request_user can see motions. This function may return an + empty set. """ - user_ids = set() + submitters_supporters = set() if has_perm(request_user, 'motions.can_see'): for motion_collection_element in Collection(Motion.get_collection_string()).element_generator(): full_data = motion_collection_element.get_full_data() - user_ids.update(full_data['submitters_id']) - user_ids.update(full_data['supporters_id']) - return user_ids + submitters_supporters.update(full_data['submitters_id']) + submitters_supporters.update(full_data['supporters_id']) + return submitters_supporters diff --git a/openslides/users/access_permissions.py b/openslides/users/access_permissions.py index 618d53ebb..246aceb28 100644 --- a/openslides/users/access_permissions.py +++ b/openslides/users/access_permissions.py @@ -3,6 +3,7 @@ from django.contrib.auth.models import AnonymousUser from ..core.signals import user_data_required from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import anonymous_is_enabled, has_perm +from ..utils.collection import Collection class UserAccessPermissions(BaseAccessPermissions): @@ -23,7 +24,7 @@ class UserAccessPermissions(BaseAccessPermissions): return UserFullSerializer - def get_restricted_data(self, full_data, user): + def get_restricted_data(self, container, user): """ Returns the restricted serialized data for the instance prepared for the user. Removes several fields for non admins so that they do @@ -31,22 +32,28 @@ class UserAccessPermissions(BaseAccessPermissions): """ from .serializers import USERCANSEESERIALIZER_FIELDS, USERCANSEEEXTRASERIALIZER_FIELDS - def filtered_data(full_data, only_keys): + def filtered_data(full_data, whitelist): """ - Returns a new dict like full_data but with all blocked_keys removed. + Returns a new dict like full_data but only with whitelisted keys. """ - return {key: full_data[key] for key in only_keys} + return {key: full_data[key] for key in whitelist} - # many_items is True, when there are more then one items in full_data. - many_items = not isinstance(full_data, dict) - full_data = full_data if many_items else [full_data] + # Expand full_data to a list if it is not one. + full_data = container.get_full_data() if isinstance(container, Collection) else [container.get_full_data()] - many_fields = set(USERCANSEEEXTRASERIALIZER_FIELDS) - little_fields = set(USERCANSEESERIALIZER_FIELDS) - many_fields.add('groups_id') - many_fields.discard('groups') - little_fields.add('groups_id') - little_fields.discard('groups') + # We have four sets of data to be sent: + # * full data i. e. all fields, + # * many data i. e. all fields but not the default password, + # * little data i. e. all fields but not the default password, comments and active status, + # * no data. + + # Prepare field set for users with "many" data and with "little" data. + many_data_fields = set(USERCANSEEEXTRASERIALIZER_FIELDS) + many_data_fields.add('groups_id') + many_data_fields.discard('groups') + litte_data_fields = set(USERCANSEESERIALIZER_FIELDS) + litte_data_fields.add('groups_id') + litte_data_fields.discard('groups') # Check user permissions. if has_perm(user, 'users.can_see_name'): @@ -54,33 +61,46 @@ class UserAccessPermissions(BaseAccessPermissions): if has_perm(user, 'users.can_manage'): data = full_data else: - data = [filtered_data(full, many_fields) for full in full_data] + data = [filtered_data(full, many_data_fields) for full in full_data] else: - data = [filtered_data(full, little_fields) for full in full_data] + data = [filtered_data(full, litte_data_fields) for full in full_data] else: - # Build a list of users, that can be seen without permissions. - no_perm_users = set() - if user is not None: - no_perm_users.add(user.id) + # Build a list of users, that can be seen without any permissions (with little fields). - # Get a list of all users, that are needed by another app + user_ids = set() + + # Everybody can see himself. Also everybody can see every user + # that is required e. g. as speaker, motion submitter or + # assignment candidate. + + # Add oneself. + if user is not None: + user_ids.add(user.id) + + # Get a list of all users, that are required by another app. receiver_responses = user_data_required.send( sender=self.__class__, - request_user=user, - user_data=full_data) + request_user=user) for receiver, response in receiver_responses: - no_perm_users.update(response) + user_ids.update(response) + # Parse data. data = [ - filtered_data(full, little_fields) + filtered_data(full, litte_data_fields) for full in full_data - if full['id'] in no_perm_users] + if full['id'] in user_ids] - # Set data to [None] if data is empty - data = data or [None] + # Reduce result to a single item or None if it was not a collection at + # the beginning of the method. + if isinstance(container, Collection): + restricted_data = data + elif data: + restricted_data = data[0] + else: + restricted_data = None - return data if many_items else data[0] + return restricted_data def get_projector_data(self, full_data): """ diff --git a/openslides/utils/access_permissions.py b/openslides/utils/access_permissions.py index a325e5ec1..9c5b17dd0 100644 --- a/openslides/utils/access_permissions.py +++ b/openslides/utils/access_permissions.py @@ -1,5 +1,6 @@ from django.dispatch import Signal +from .collection import Collection from .dispatch import SignalConnectMetaClass @@ -56,31 +57,29 @@ class BaseAccessPermissions(object, metaclass=SignalConnectMetaClass): """ return self.get_serializer_class(user=None)(instance).data - def get_restricted_data(self, full_data, user): + def get_restricted_data(self, container, user): """ Returns the restricted serialized data for the instance prepared for the user. - Returns None or 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 container should be a CollectionElement or a + Collection. The type of the return value is a dictionary or a list + according to the given type (or None). Returns None or 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(). - - full_data can be a list or one single item. If it is a list, then the - retun value is also a list of restricted data. - - When the user can not access """ if self.check_permissions(user): - data = full_data - elif isinstance(full_data, dict): - data = None - else: + data = container.get_full_data() + elif isinstance(container, Collection): data = [] + else: + data = None return data def get_projector_data(self, full_data): diff --git a/openslides/utils/autoupdate.py b/openslides/utils/autoupdate.py index 712279e56..ea5cb5131 100644 --- a/openslides/utils/autoupdate.py +++ b/openslides/utils/autoupdate.py @@ -12,14 +12,8 @@ from django.db import transaction from ..core.config import config from ..core.models import Projector from .auth import has_perm, user_to_collection_user -from .cache import start_up_cache, websocket_user_cache -from .collection import ( - Collection, - CollectionElement, - CollectionElementList, - format_for_autoupdate, - get_model_from_collection_string, -) +from .cache import startup_cache, websocket_user_cache +from .collection import Collection, CollectionElement, CollectionElementList def send_or_wait(send_func, *args, **kwargs): @@ -47,6 +41,28 @@ def send_or_wait(send_func, *args, **kwargs): ) +def format_for_autoupdate(collection_string, id, action, data=None): + """ + Returns a dict that can be used for autoupdate. + """ + if not data: + # If the data is None or is empty, then the action has to be deleted, + # even when it says diffrently. This can happen when the object is not + # deleted, but the user has no permission to see it. + action = 'deleted' + + output = { + 'collection': collection_string, + 'id': id, + 'action': action, + } + + if action != 'deleted': + output['data'] = data + + return output + + @channel_session_user_from_http def ws_add_site(message): """ @@ -65,32 +81,31 @@ def ws_add_site(message): send_or_wait(message.reply_channel.send, {'accept': True}) # Collect all elements that shoud be send to the client when the websocket - # connection is established + # connection is established. user = user_to_collection_user(message.user.id) output = [] - for collection_string, full_data in start_up_cache.get_collection_elements().items(): - access_permission = get_model_from_collection_string(collection_string).get_access_permissions() - if collection_string == 'core/config': + for collection in startup_cache.get_collections(): + access_permissions = collection.get_access_permissions() + restricted_data = access_permissions.get_restricted_data(collection, user) + + if collection.collection_string == 'core/config': id_key = 'key' else: id_key = 'id' - restricted_data = access_permission.get_restricted_data(full_data, user) - if restricted_data is None: - continue - for data in restricted_data: if data is None: + # We do not want to send 'deleted' objects on startup. + # That's why we skip such data. continue - output.append( format_for_autoupdate( - collection_string=collection_string, + collection_string=collection.collection_string, id=data[id_key], action='changed', data=data)) - # Send all data. If there is no data, then only accept the connection + # Send all data. if output: send_or_wait(message.reply_channel.send, {'text': json.dumps(output)}) @@ -354,10 +369,13 @@ def send_autoupdate(collection_elements): Helper function, that sends collection_elements through a channel to the autoupdate system. + Before sending the startup_cache is cleared because it is possibly out of + date. + Does nothing if collection_elements is empty. """ if collection_elements: - start_up_cache.clear() + startup_cache.clear() send_or_wait( Channel('autoupdate.send_data').send, collection_elements.as_channels_message()) diff --git a/openslides/utils/cache.py b/openslides/utils/cache.py index 3cf380632..7ea2d92f2 100644 --- a/openslides/utils/cache.py +++ b/openslides/utils/cache.py @@ -187,16 +187,15 @@ class DjangoCacheWebsocketUserCache(BaseWebsocketUserCache): class StartupCache: """ - Cache of all data that are needed when a clients connects via websocket. + Cache of all data that are required when a client connects via websocket. """ - - cache_key = "full_data_cache" + cache_key = "full_data_startup_cache" def build(self): """ - Generate the cache by going though all apps. - Returns a dict where the key is the collection string and the value a - list of the full_data from the collection_elements. + Generate the cache by going through all apps. Returns a dict where the + key is the collection string and the value a list of the full_data from + the collection elements. """ cache_data = {} for app in apps.get_app_configs(): @@ -205,7 +204,7 @@ class StartupCache: # This method has to return an iterable of Collection objects. get_startup_elements = app.get_startup_elements except AttributeError: - # Skip apps that do not implement get_startup_elements + # Skip apps that do not implement get_startup_elements. continue for collection in get_startup_elements(): @@ -223,25 +222,23 @@ class StartupCache: """ cache.delete(self.cache_key) - def get_collection_elements(self): + def get_collections(self): """ - Returns a dict of all collection_elements, that are needed at startup. + Generator that returns all cached Collections. - The key is the collection_string and the value a list of CollectionElement - objects. Each list has at least one value. - - The data is read from the cache if it exists. It builds the cache, if it + The data is read from the cache if it exists. It builds the cache if it does not exists. """ + from .collection import Collection data = cache.get(self.cache_key) if data is None: # The cache does not exist. data = self.build() - - return data + for collection_string, full_data in data.items(): + yield Collection(collection_string, full_data) -start_up_cache = StartupCache() +startup_cache = StartupCache() def use_redis_cache(): diff --git a/openslides/utils/collection.py b/openslides/utils/collection.py index f4ff455b5..da373be74 100644 --- a/openslides/utils/collection.py +++ b/openslides/utils/collection.py @@ -100,9 +100,18 @@ class CollectionElement: Only for internal use. Do not use it directly. Use as_autoupdate_for_user() or as_autoupdate_for_projector(). """ + from .autoupdate import format_for_autoupdate + + # TODO: Revert this after get_projector_data is also enhanced like get_restricted_data. + if method == 'get_restricted_data': + container = self + else: + container = self.get_full_data() + # End of hack + if not self.is_deleted(): data = getattr(self.get_access_permissions(), method)( - self.get_full_data(), + container, *args) else: data = None @@ -140,9 +149,7 @@ class CollectionElement: The argument `user` can be anything, that is allowd as argument for utils.auth.has_perm(). """ - return self.get_access_permissions().get_restricted_data( - self.get_full_data(), - user) + return self.get_access_permissions().get_restricted_data(self, user) def get_model(self): """ @@ -283,8 +290,15 @@ class Collection: Represents all elements of one collection. """ - def __init__(self, collection_string): + def __init__(self, collection_string, full_data=None): + """ + Initiates a Collection. A collection_string has to be given. If + full_data (a list of dictionaries) is not given the method + get_full_data() exposes all data by iterating over all + CollectionElements. + """ self.collection_string = collection_string + self.full_data = full_data def get_cache_key(self, raw=False): """ @@ -304,10 +318,18 @@ class Collection: """ return get_model_from_collection_string(self.collection_string) + def get_access_permissions(self): + """ + Returns the get_access_permissions object for the this collection. + """ + return self.get_model().get_access_permissions() + def element_generator(self): """ Generator that yields all collection_elements of this collection. """ + # TODO: This method should use self.full_data if it already exists. + # Get all cache keys. ids = self.get_all_ids() cache_keys = [ @@ -352,10 +374,23 @@ class Collection: for instance in query.filter(pk__in=missing_ids): yield CollectionElement.from_instance(instance) + def get_full_data(self): + """ + Returns a list of dictionaries with full_data of all collection + elements. + """ + if self.full_data is None: + self.full_data = [ + collection_element.get_full_data() + for collection_element + in self.element_generator()] + return self.full_data + def as_autoupdate_for_projector(self): """ Returns a list of dictonaries to send them to the projector. """ + # TODO: This method is only used in one case. Remove it. output = [] for collection_element in self.element_generator(): content = collection_element.as_autoupdate_for_projector() @@ -371,6 +406,7 @@ class Collection: The argument `user` can be anything, that is allowd as argument for utils.auth.has_perm(). """ + # TODO: This method is not used. Remove it. output = [] for collection_element in self.element_generator(): content = collection_element.as_autoupdate_for_user(user) @@ -548,25 +584,3 @@ def get_collection_id_from_cache_key(cache_key): # The id is no integer. This can happen on config elements pass return (collection_string, id) - - -def format_for_autoupdate(collection_string, id, action, data=None): - """ - Returns a dict that can be used for autoupdate. - """ - if not data: - # If the data is None or is empty, then the action has to be deleted, - # even when it says diffrently. This can happen when the object is not - # deleted, but the user has no permission to see it. - action = 'deleted' - - output = { - 'collection': collection_string, - 'id': id, - 'action': action, - } - - if action != 'deleted': - output['data'] = data - - return output diff --git a/tests/unit/utils/test_collection.py b/tests/unit/utils/test_collection.py index 9bb292e9b..cd1fe6e5a 100644 --- a/tests/unit/utils/test_collection.py +++ b/tests/unit/utils/test_collection.py @@ -128,7 +128,7 @@ class TestCollectionElement(TestCase): 'id': 42, 'action': 'changed', 'data': 'restricted_data'}) - collection_element.get_full_data.assert_called_once_with() + collection_element.get_full_data.assert_not_called() def test_as_autoupdate_for_user_no_permission(self): with patch.object(collection.CollectionElement, 'get_full_data'): @@ -143,7 +143,7 @@ class TestCollectionElement(TestCase): {'collection': 'testmodule/model', 'id': 42, 'action': 'deleted'}) - collection_element.get_full_data.assert_called_once_with() + collection_element.get_full_data.assert_not_called() def test_as_autoupdate_for_user_deleted(self): collection_element = collection.CollectionElement.from_values('testmodule/model', 42, deleted=True)