From d7db7145620d05a762ec5fa872ca3cccf4b349ce Mon Sep 17 00:00:00 2001 From: Oskar Hahn Date: Mon, 4 Sep 2017 00:25:45 +0200 Subject: [PATCH] CollectionElement and Autoupdate cleanups * change get_restricted_data and get_projector_data to always use a list * Add typings to all get_restricted_data and get_projector_data methods * Replace CollectionElementList with a real list * Fixed arguments of inform_deleted_data * Moved CollectionElementCache to cache.py and refactored it * Run tests with cache enabled (using fakeredis) --- CHANGELOG | 1 + openslides/agenda/access_permissions.py | 44 +- openslides/assignments/access_permissions.py | 44 +- openslides/core/views.py | 5 +- openslides/mediafiles/access_permissions.py | 28 +- openslides/motions/access_permissions.py | 43 +- openslides/users/access_permissions.py | 63 +-- openslides/users/views.py | 6 +- openslides/utils/access_permissions.py | 38 +- openslides/utils/autoupdate.py | 94 ++-- openslides/utils/cache.py | 156 +++++- openslides/utils/collection.py | 444 +++++------------- openslides/utils/models.py | 2 +- openslides/utils/rest_api.py | 4 +- openslides/utils/test.py | 26 +- requirements.txt | 3 +- setup.cfg | 3 + tests/integration/agenda/test_viewset.py | 22 +- tests/integration/assignments/test_viewset.py | 22 +- tests/integration/core/test_views.py | 1 + tests/integration/core/test_viewset.py | 56 +-- tests/integration/mediafiles/test_viewset.py | 17 +- tests/integration/motions/test_viewset.py | 58 ++- tests/integration/topics/test_viewset.py | 17 +- tests/integration/users/test_viewset.py | 23 +- tests/integration/utils/test_autoupdate.py | 15 +- tests/integration/utils/test_collection.py | 64 +-- tests/settings.py | 17 +- tests/unit/users/test_access_permissions.py | 22 +- tests/unit/utils/test_collection.py | 205 +------- 30 files changed, 548 insertions(+), 995 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 19b2bdeba..c25e63c84 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -96,6 +96,7 @@ General: to pdfmake 0.1.30) [#3278, #3285]. - Bugfixes for PDF creation [#3227, #3251, #3279, #3286, #3346, #3347, #3342]. - Improvements for plugin integration [#3330]. +- Cleanups for the collection and autoupdate system [#3390] Version 2.1.1 (2017-04-05) diff --git a/openslides/agenda/access_permissions.py b/openslides/agenda/access_permissions.py index 9dbcf0491..83cfb6906 100644 --- a/openslides/agenda/access_permissions.py +++ b/openslides/agenda/access_permissions.py @@ -1,11 +1,8 @@ -from typing import Iterable # noqa +from typing import Any, Dict, Iterable, List, Optional # noqa -from ..utils.access_permissions import ( # noqa - BaseAccessPermissions, - RestrictedData, -) +from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import has_perm -from ..utils.collection import Collection +from ..utils.collection import CollectionElement class ItemAccessPermissions(BaseAccessPermissions): @@ -28,7 +25,10 @@ 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, container, user): + def get_restricted_data( + self, + full_data: List[Dict[str, Any]], + user: Optional[CollectionElement]) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. @@ -43,9 +43,6 @@ class ItemAccessPermissions(BaseAccessPermissions): whitelist = full_data.keys() - blocked_keys return {key: full_data[key] for key in whitelist} - # 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') and has_perm(user, 'agenda.can_see_hidden_items'): @@ -83,18 +80,9 @@ class ItemAccessPermissions(BaseAccessPermissions): 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 # type: RestrictedData - elif data: - restricted_data = data[0] - else: - restricted_data = None + return data - return restricted_data - - def get_projector_data(self, container): + def get_projector_data(self, full_data: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the projector. Removes field 'comment'. @@ -106,20 +94,8 @@ class ItemAccessPermissions(BaseAccessPermissions): whitelist = full_data.keys() - blocked_keys return {key: full_data[key] for key in whitelist} - # 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. blocked_keys = ('comment',) data = [filtered_data(full, blocked_keys) for full in full_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): - projector_data = data # type: RestrictedData - elif data: - projector_data = data[0] - else: - projector_data = None - - return projector_data + return data diff --git a/openslides/assignments/access_permissions.py b/openslides/assignments/access_permissions.py index 8cd760ef5..65a38bcf7 100644 --- a/openslides/assignments/access_permissions.py +++ b/openslides/assignments/access_permissions.py @@ -1,9 +1,8 @@ -from ..utils.access_permissions import ( # noqa - BaseAccessPermissions, - RestrictedData, -) +from typing import Any, Dict, List, Optional + +from ..utils.access_permissions import BaseAccessPermissions # noqa from ..utils.auth import has_perm -from ..utils.collection import Collection +from ..utils.collection import CollectionElement class AssignmentAccessPermissions(BaseAccessPermissions): @@ -28,15 +27,15 @@ class AssignmentAccessPermissions(BaseAccessPermissions): serializer_class = AssignmentShortSerializer return serializer_class - def get_restricted_data(self, container, user): + def get_restricted_data( + self, + full_data: List[Dict[str, Any]], + user: Optional[CollectionElement]) -> 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. """ - # 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 @@ -50,25 +49,13 @@ class AssignmentAccessPermissions(BaseAccessPermissions): 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 # type: RestrictedData - elif data: - restricted_data = data[0] - else: - restricted_data = None + return data - return restricted_data - - def get_projector_data(self, container): + def get_projector_data(self, full_data: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the projector. Removes unpublished polls. """ - # 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. Exclude unpublished polls. data = [] for full in full_data: @@ -76,13 +63,4 @@ class AssignmentAccessPermissions(BaseAccessPermissions): full_copy['polls'] = [poll for poll in full['polls'] if poll['published']] data.append(full_copy) - # Reduce result to a single item or None if it was not a collection at - # the beginning of the method. - if isinstance(container, Collection): - projector_data = data # type: RestrictedData - elif data: - projector_data = data[0] - else: - projector_data = None - - return projector_data + return data diff --git a/openslides/core/views.py b/openslides/core/views.py index 9a1115978..42436f935 100644 --- a/openslides/core/views.py +++ b/openslides/core/views.py @@ -714,12 +714,11 @@ class ChatMessageViewSet(ModelViewSet): chatmessages = ChatMessage.objects.all() args = [] for chatmessage in chatmessages: - args.append(chatmessage.get_collection_string()) - args.append(chatmessage.pk) + args.append((chatmessage.get_collection_string(), chatmessage.pk)) chatmessages.delete() # Trigger autoupdate and setup response. if len(args) > 0: - inform_deleted_data(*args) + inform_deleted_data(args) return Response({'detail': _('All chat messages deleted successfully.')}) diff --git a/openslides/mediafiles/access_permissions.py b/openslides/mediafiles/access_permissions.py index aaf331f3c..ab23be913 100644 --- a/openslides/mediafiles/access_permissions.py +++ b/openslides/mediafiles/access_permissions.py @@ -1,9 +1,8 @@ -from ..utils.access_permissions import ( # noqa - BaseAccessPermissions, - RestrictedData, -) +from typing import Any, Dict, List, Optional + +from ..utils.access_permissions import BaseAccessPermissions # noqa from ..utils.auth import has_perm -from ..utils.collection import Collection +from ..utils.collection import CollectionElement class MediafileAccessPermissions(BaseAccessPermissions): @@ -24,14 +23,14 @@ class MediafileAccessPermissions(BaseAccessPermissions): return MediafileSerializer - def get_restricted_data(self, container, user): + def get_restricted_data( + self, + full_data: List[Dict[str, Any]], + user: Optional[CollectionElement]) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. Removes hidden mediafiles for some users. """ - # 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 @@ -41,13 +40,4 @@ class MediafileAccessPermissions(BaseAccessPermissions): 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 # type: RestrictedData - elif data: - restricted_data = data[0] - else: - restricted_data = None - - return restricted_data + return data diff --git a/openslides/motions/access_permissions.py b/openslides/motions/access_permissions.py index b60b64d06..19a9f4171 100644 --- a/openslides/motions/access_permissions.py +++ b/openslides/motions/access_permissions.py @@ -1,12 +1,10 @@ from copy import deepcopy +from typing import Any, Dict, List, Optional from ..core.config import config -from ..utils.access_permissions import ( # noqa - BaseAccessPermissions, - RestrictedData, -) +from ..utils.access_permissions import BaseAccessPermissions # noqa from ..utils.auth import has_perm -from ..utils.collection import Collection, CollectionElement +from ..utils.collection import CollectionElement class MotionAccessPermissions(BaseAccessPermissions): @@ -27,7 +25,10 @@ class MotionAccessPermissions(BaseAccessPermissions): return MotionSerializer - def get_restricted_data(self, container, user): + def get_restricted_data( + self, + full_data: List[Dict[str, Any]], + user: Optional[CollectionElement]) -> 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 @@ -35,9 +36,6 @@ class MotionAccessPermissions(BaseAccessPermissions): some unauthorized users. Ensures that a user can only see his own personal notes. """ - # 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, 'motions.can_see'): # TODO: Refactor this after personal_notes system is refactored. @@ -78,25 +76,13 @@ class MotionAccessPermissions(BaseAccessPermissions): 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 # type: RestrictedData - elif data: - restricted_data = data[0] - else: - restricted_data = None + return data - return restricted_data - - def get_projector_data(self, container): + def get_projector_data(self, full_data: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the projector. Removes several comment fields. """ - # 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. data = [] for full in full_data: @@ -114,16 +100,7 @@ class MotionAccessPermissions(BaseAccessPermissions): else: data.append(full) - # Reduce result to a single item or None if it was not a collection at - # the beginning of the method. - if isinstance(container, Collection): - projector_data = data # type: RestrictedData - elif data: - projector_data = data[0] - else: - projector_data = None - - return projector_data + return data class MotionChangeRecommendationAccessPermissions(BaseAccessPermissions): diff --git a/openslides/users/access_permissions.py b/openslides/users/access_permissions.py index aa59be509..c1a5ec904 100644 --- a/openslides/users/access_permissions.py +++ b/openslides/users/access_permissions.py @@ -1,14 +1,11 @@ -from typing import Any, Dict, List # noqa +from typing import Any, Dict, List, Optional from django.contrib.auth.models import AnonymousUser from ..core.signals import user_data_required -from ..utils.access_permissions import ( # noqa - BaseAccessPermissions, - RestrictedData, -) +from ..utils.access_permissions import BaseAccessPermissions # noqa from ..utils.auth import anonymous_is_enabled, has_perm -from ..utils.collection import Collection +from ..utils.collection import CollectionElement class UserAccessPermissions(BaseAccessPermissions): @@ -29,7 +26,10 @@ class UserAccessPermissions(BaseAccessPermissions): return UserFullSerializer - def get_restricted_data(self, container, user): + def get_restricted_data( + self, + full_data: List[Dict[str, Any]], + user: Optional[CollectionElement]) -> 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 @@ -43,9 +43,6 @@ class UserAccessPermissions(BaseAccessPermissions): """ return {key: full_data[key] for key in whitelist} - # 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()] - # 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, @@ -96,18 +93,9 @@ class UserAccessPermissions(BaseAccessPermissions): in full_data if full['id'] in user_ids] - # 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 # type: RestrictedData - elif data: - restricted_data = data[0] - else: - restricted_data = None + return data - return restricted_data - - def get_projector_data(self, container): + def get_projector_data(self, full_data: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the projector. Removes several fields. @@ -120,25 +108,13 @@ class UserAccessPermissions(BaseAccessPermissions): """ return {key: full_data[key] for key in whitelist} - # 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. litte_data_fields = set(USERCANSEESERIALIZER_FIELDS) litte_data_fields.add('groups_id') litte_data_fields.discard('groups') data = [filtered_data(full, litte_data_fields) for full in full_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): - projector_data = data # type: RestrictedData - elif data: - projector_data = data[0] - else: - projector_data = None - - return projector_data + return data class GroupAccessPermissions(BaseAccessPermissions): @@ -182,14 +158,14 @@ class PersonalNoteAccessPermissions(BaseAccessPermissions): return PersonalNoteSerializer - def get_restricted_data(self, container, user): + def get_restricted_data( + self, + full_data: List[Dict[str, Any]], + user: Optional[CollectionElement]) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. Everybody gets only his own personal notes. """ - # 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 user is None: data = [] # type: List[Dict[str, Any]] @@ -201,13 +177,4 @@ class PersonalNoteAccessPermissions(BaseAccessPermissions): 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 # type: RestrictedData - elif data: - restricted_data = data[0] - else: - restricted_data = None - - return restricted_data + return data diff --git a/openslides/users/views.py b/openslides/users/views.py index 5169fa457..926de14d1 100644 --- a/openslides/users/views.py +++ b/openslides/users/views.py @@ -1,3 +1,5 @@ +from typing import List # noqa + from django.contrib.auth import login as auth_login from django.contrib.auth import logout as auth_logout from django.contrib.auth import update_session_auth_hash @@ -15,7 +17,7 @@ from ..utils.autoupdate import ( inform_changed_data, inform_data_collection_element_list, ) -from ..utils.collection import CollectionElement, CollectionElementList +from ..utils.collection import CollectionElement from ..utils.rest_api import ( ModelViewSet, Response, @@ -250,7 +252,7 @@ class GroupViewSet(ModelViewSet): # Some permissions are added. if len(new_permissions) > 0: - collection_elements = CollectionElementList() + collection_elements = [] # type: List[CollectionElement] signal_results = permission_change.send(None, permissions=new_permissions, action='added') for receiver, signal_collections in signal_results: for collection in signal_collections: diff --git a/openslides/utils/access_permissions.py b/openslides/utils/access_permissions.py index 1bd1d8395..02ae09663 100644 --- a/openslides/utils/access_permissions.py +++ b/openslides/utils/access_permissions.py @@ -1,12 +1,9 @@ -from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, Optional from django.db.models import Model from rest_framework.serializers import Serializer -from .collection import Collection, CollectionElement - -Container = Union[CollectionElement, Collection] -RestrictedData = Union[List[Dict[str, Any]], Dict[str, Any], None] +from .collection import CollectionElement class BaseAccessPermissions: @@ -40,35 +37,30 @@ class BaseAccessPermissions: """ return self.get_serializer_class(user=None)(instance).data - def get_restricted_data(self, container: Container, user: Optional[CollectionElement]) -> RestrictedData: + def get_restricted_data( + self, full_data: List[Dict[str, Any]], + user: Optional[CollectionElement]) -> List[Dict[str, Any]]: """ Returns the restricted serialized data for the instance prepared for the user. - 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. + 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. 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(). """ - if self.check_permissions(user): - data = container.get_full_data() - elif isinstance(container, Collection): - data = [] - else: - data = None - return data + return full_data if self.check_permissions(user) else [] - def get_projector_data(self, container: Container) -> RestrictedData: + def get_projector_data(self, full_data: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """ - Returns the serialized data for the projector. Returns None if the - user has no access to this specific data. Returns reduced data if + Returns the serialized data for the projector. Returns an empty list if + the user has no access to this specific data. Returns reduced data if the user has limited access. Default: Returns full data. """ - return container.get_full_data() + return full_data diff --git a/openslides/utils/autoupdate.py b/openslides/utils/autoupdate.py index a1923bff1..c87564110 100644 --- a/openslides/utils/autoupdate.py +++ b/openslides/utils/autoupdate.py @@ -2,7 +2,7 @@ import json import time import warnings from collections import defaultdict -from typing import Any, Dict, Generator, Iterable, List, Union, cast +from typing import Any, Dict, Generator, Iterable, List, Tuple, Union from channels import Channel, Group from channels.asgi import get_channel_layer @@ -16,7 +16,15 @@ from ..core.config import config from ..core.models import Projector from .auth import anonymous_is_enabled, has_perm, user_to_collection_user from .cache import restricted_data_cache, websocket_user_cache -from .collection import Collection, CollectionElement, CollectionElementList +from .collection import AutoupdateFormat # noqa +from .collection import ( + ChannelMessageFormat, + Collection, + CollectionElement, + format_for_autoupdate, + from_channel_message, + to_channel_message, +) def send_or_wait(send_func: Any, *args: Any, **kwargs: Any) -> None: @@ -44,28 +52,6 @@ def send_or_wait(send_func: Any, *args: Any, **kwargs: Any) -> None: ) -def format_for_autoupdate(collection_string: str, id: int, action: str, data: Dict[str, Any]=None) -> Dict[str, Any]: - """ - 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: Any) -> None: """ @@ -97,10 +83,7 @@ def ws_add_site(message: Any) -> None: output = [] for collection in get_startup_collections(): access_permissions = collection.get_access_permissions() - restricted_data = access_permissions.get_restricted_data(collection, user) - - # At this point restricted_data has to be a list. So we have to tell it mypy - restricted_data = cast(List[Dict[str, Any]], restricted_data) + restricted_data = access_permissions.get_restricted_data(collection.get_full_data(), user) for data in restricted_data: if data is None: @@ -109,10 +92,10 @@ def ws_add_site(message: Any) -> None: continue formatted_data = format_for_autoupdate( - collection_string=collection.collection_string, - id=data['id'], - action='changed', - data=data) + collection_string=collection.collection_string, + id=data['id'], + action='changed', + data=data) output.append(formatted_data) # Cache restricted data for user @@ -231,14 +214,21 @@ def ws_add_projector(message: Any, projector_id: int) -> None: projector = Projector.objects.get(pk=config['projector_broadcast']) # Collect all elements that are on the projector. - output = [] + output = [] # type: List[AutoupdateFormat] for requirement in projector.get_all_requirements(): required_collection_element = CollectionElement.from_instance(requirement) output.append(required_collection_element.as_autoupdate_for_projector()) # Collect all config elements. - collection = Collection(config.get_collection_string()) - output.extend(collection.as_autoupdate_for_projector()) + config_collection = Collection(config.get_collection_string()) + projector_data = (config_collection.get_access_permissions() + .get_projector_data(config_collection.get_full_data())) + for data in projector_data: + output.append(format_for_autoupdate( + config_collection.collection_string, + data['id'], + 'changed', + data)) # Collect the projector instance. collection_element = CollectionElement.from_instance(projector) @@ -255,11 +245,11 @@ def ws_disconnect_projector(message: Any, projector_id: int) -> None: Group('projector-{}'.format(projector_id)).discard(message.reply_channel) -def send_data(message: Any) -> None: +def send_data(message: ChannelMessageFormat) -> None: """ Informs all site users and projector clients about changed data. """ - collection_elements = CollectionElementList.from_channels_message(message) + collection_elements = from_channel_message(message) # Send data to site users. for user_id, channel_names in websocket_user_cache.get_all().items(): @@ -338,7 +328,7 @@ def inform_changed_data(instances: Union[Iterable[Model], Model], information: D pass # Generates an collection element list for the root_instances. - collection_elements = CollectionElementList() + collection_elements = [] # type: List[CollectionElement] for root_instance in root_instances: collection_elements.append( CollectionElement.from_instance( @@ -351,32 +341,20 @@ def inform_changed_data(instances: Union[Iterable[Model], Model], information: D transaction.on_commit(lambda: send_autoupdate(collection_elements)) -# TODO: Change the input argument to tuples -def inform_deleted_data(*args: Any, information: Dict[str, Any]=None) -> None: +def inform_deleted_data(elements: Iterable[Tuple[str, int]], information: Dict[str, Any]=None) -> None: """ Informs the autoupdate system and the caching system about the deletion of elements. - The function has to be called with the attributes collection_string and id. - Multible elements can be used. For example: - - inform_deleted_data('motions/motion', 1, 'assignments/assignment', 5) - The argument information is added to each collection element. """ - if len(args) % 2 or not args: - raise ValueError( - "inform_deleted_data has to be called with the same number of " - "collection strings and ids. It has to be at least one collection " - "string and one id.") - # Go through each pair of collection_string and id and generate a collection # element from it. - collection_elements = CollectionElementList() - for index in range(0, len(args), 2): + collection_elements = [] # type: List[CollectionElement] + for element in elements: collection_elements.append(CollectionElement.from_values( - collection_string=args[index], - id=args[index + 1], + collection_string=element[0], + id=element[1], deleted=True, information=information)) # If currently there is an open database transaction, then the @@ -386,7 +364,7 @@ def inform_deleted_data(*args: Any, information: Dict[str, Any]=None) -> None: transaction.on_commit(lambda: send_autoupdate(collection_elements)) -def inform_data_collection_element_list(collection_elements: CollectionElementList, +def inform_data_collection_element_list(collection_elements: List[CollectionElement], information: Dict[str, Any]=None) -> None: """ Informs the autoupdate system about some collection elements. This is @@ -399,7 +377,7 @@ def inform_data_collection_element_list(collection_elements: CollectionElementLi transaction.on_commit(lambda: send_autoupdate(collection_elements)) -def send_autoupdate(collection_elements: CollectionElementList) -> None: +def send_autoupdate(collection_elements: List[CollectionElement]) -> None: """ Helper function, that sends collection_elements through a channel to the autoupdate system. @@ -409,7 +387,7 @@ def send_autoupdate(collection_elements: CollectionElementList) -> None: if collection_elements: send_or_wait( Channel('autoupdate.send_data').send, - collection_elements.as_channels_message()) + to_channel_message(collection_elements)) def get_startup_collections() -> Generator[Collection, None, None]: diff --git a/openslides/utils/cache.py b/openslides/utils/cache.py index 518da51f2..b9f6ae6eb 100644 --- a/openslides/utils/cache.py +++ b/openslides/utils/cache.py @@ -10,6 +10,7 @@ from typing import ( # noqa List, Optional, Set, + Type, Union, ) @@ -204,9 +205,158 @@ class DjangoCacheWebsocketUserCache(BaseWebsocketUserCache): cache.set(self.get_cache_key(), data) +class FullDataCache: + """ + Caches all data as full data. + + Helps to get all data from one collection. + """ + + base_cache_key = 'full_data_cache' + + def build_for_collection(self, collection_string: str) -> None: + """ + Build the cache for collection from a django model. + + Rebuilds the cache for that collection, if it already exists. + """ + redis = get_redis_connection() + pipe = redis.pipeline() + + # Clear the cache for collection + pipe.delete(self.get_cache_key(collection_string)) + + # Save all elements + from .collection import get_model_from_collection_string + model = get_model_from_collection_string(collection_string) + try: + query = model.objects.get_full_queryset() + except AttributeError: + # If the model des not have to method get_full_queryset(), then use + # the default queryset from django. + query = model.objects + + # Build a dict from the instance id to the full_data + mapping = {instance.pk: json.dumps(model.get_access_permissions().get_full_data(instance)) + for instance in query.all()} + + if mapping: + # Save the dict into a redis map, if there is at least one value + pipe.hmset( + self.get_cache_key(collection_string), + mapping) + + pipe.execute() + + def add_element(self, collection_string: str, id: int, data: Dict[str, Any]) -> None: + """ + Adds one element to the cache. If the cache does not exists for the collection, + it is created. + """ + redis = get_redis_connection() + + # If the cache does not exist for the collection, then create it first. + if not self.exists_for_collection(collection_string): + self.build_for_collection(collection_string) + + redis.hset( + self.get_cache_key(collection_string), + id, + json.dumps(data)) + + def del_element(self, collection_string: str, id: int) -> None: + """ + Removes one element from the cache. + + Does nothing if the cache does not exist. + """ + redis = get_redis_connection() + redis.hdel( + self.get_cache_key(collection_string), + id) + + def exists_for_collection(self, collection_string: str) -> bool: + """ + Returns True if the cache for the collection exists, else False. + """ + redis = get_redis_connection() + return redis.exists(self.get_cache_key(collection_string)) + + def get_data(self, collection_string: str) -> List[Dict[str, Any]]: + """ + Returns all data for the collection. + """ + redis = get_redis_connection() + return [json.loads(element.decode()) for element in redis.hvals(self.get_cache_key(collection_string))] + + def get_element(self, collection_string: str, id: int) -> Dict[str, Any]: + """ + Returns one element from the collection. + + Raises model.DoesNotExist if the element is not in the cache. + """ + redis = get_redis_connection() + element = redis.hget(self.get_cache_key(collection_string), id) + if element is None: + from .collection import get_model_from_collection_string + model = get_model_from_collection_string(collection_string) + raise model.DoesNotExist(collection_string, id) + return json.loads(element.decode()) + + def get_cache_key(self, collection_string: str) -> str: + """ + Returns the cache key for a collection. + """ + return cache.make_key('{}:{}'.format(self.base_cache_key, collection_string)) + + +class DummyFullDataCache: + """ + Dummy FullDataCache that does nothing. + """ + def build_for_collection(self, collection_string: str) -> None: + pass + + def add_element(self, collection_string: str, id: int, data: Dict[str, Any]) -> None: + pass + + def del_element(self, collection_string: str, id: int) -> None: + pass + + def exists_for_collection(self, collection_string: str) -> bool: + return False + + def get_data(self, collection_string: str) -> List[Dict[str, Any]]: + from .collection import get_model_from_collection_string + model = get_model_from_collection_string(collection_string) + try: + query = model.objects.get_full_queryset() + except AttributeError: + # If the model des not have to method get_full_queryset(), then use + # the default queryset from django. + query = model.objects + + return [model.get_access_permissions().get_full_data(instance) + for instance in query.all()] + + def get_element(self, collection_string: str, id: int) -> Dict[str, Any]: + from .collection import get_model_from_collection_string + model = get_model_from_collection_string(collection_string) + try: + query = model.objects.get_full_queryset() + except AttributeError: + # If the model des not have to method get_full_queryset(), then use + # the default queryset from django. + query = model.objects + + return model.get_access_permissions().get_full_data(query.get(pk=id)) + + class RestrictedDataCache: """ - Caches all Data for a specific users. + Caches all data for a specific users. + + Helps to get all data from all collections for a specific user. The cached values are expected to be formatted for outout via websocket. """ @@ -249,7 +399,7 @@ class RestrictedDataCache: The returned value is a list of the elements. """ redis = get_redis_connection() - return [json.loads(element) for element in redis.hvals(self.get_cache_key(user_id))] + return [json.loads(element.decode()) for element in redis.hvals(self.get_cache_key(user_id))] def get_cache_key(self, user_id: int) -> str: """ @@ -301,6 +451,8 @@ if use_redis_cache(): restricted_data_cache = DummyRestrictedDataCache() # type: Union[RestrictedDataCache, DummyRestrictedDataCache] else: restricted_data_cache = RestrictedDataCache() + full_data_cache = FullDataCache() # type: Union[FullDataCache, DummyFullDataCache] else: websocket_user_cache = DjangoCacheWebsocketUserCache() restricted_data_cache = DummyRestrictedDataCache() + full_data_cache = DummyFullDataCache() diff --git a/openslides/utils/collection.py b/openslides/utils/collection.py index 2b3432d55..a7515748e 100644 --- a/openslides/utils/collection.py +++ b/openslides/utils/collection.py @@ -1,4 +1,3 @@ -from typing import Mapping # noqa from typing import ( TYPE_CHECKING, Any, @@ -6,23 +5,48 @@ from typing import ( Generator, List, Optional, - Set, - Tuple, Type, - Union, + cast, ) from django.apps import apps -from django.core.cache import cache from django.db.models import Model +from mypy_extensions import TypedDict -from .cache import get_redis_connection, use_redis_cache +from .cache import full_data_cache if TYPE_CHECKING: from .access_permissions import BaseAccessPermissions # noqa -# TODO: Try to import this type from access_permission -RestrictedData = Union[List[Dict[str, Any]], Dict[str, Any], None] + +AutoupdateFormat = TypedDict( + 'AutoupdateFormat', + { + 'collection': str, + 'id': int, + 'action': 'str', + 'data': Dict[str, Any], + }, + total=False, +) + +InnerChannelMessageFormat = TypedDict( + 'InnerChannelMessageFormat', + { + 'collection_string': str, + 'id': int, + 'deleted': bool, + 'information': Dict[str, Any], + 'full_data': Optional[Dict[str, Any]], + } +) + +ChannelMessageFormat = TypedDict( + 'ChannelMessageFormat', + { + 'elements': List[InnerChannelMessageFormat], + } +) class CollectionElement: @@ -51,7 +75,7 @@ class CollectionElement: if self.is_deleted(): # Delete the element from the cache, if self.is_deleted() is True: - self.delete_from_cache() + full_data_cache.del_element(self.collection_string, self.id) else: # The call to get_full_data() has some sideeffects. When the object # was created with from_instance() or the object is not in the cache @@ -95,34 +119,14 @@ class CollectionElement: return (self.collection_string == collection_element.collection_string and self.id == collection_element.id) - def as_channels_message(self) -> Dict[str, Any]: + def as_autoupdate_for_user(self, user: Optional['CollectionElement']) -> AutoupdateFormat: """ - Returns a dictonary that can be used to send the object through the - channels system. + Returns a dict that can be sent through the autoupdate system for a site + user. """ - channel_message = { - 'collection_string': self.collection_string, - 'id': self.id, - 'deleted': self.is_deleted()} - if self.information: - channel_message['information'] = self.information - if self.full_data: - # Do not use the method get_full_data but the attribute, so the - # full_data is not generated. - channel_message['full_data'] = self.full_data - return channel_message - - def as_autoupdate(self, method: str, *args: Any) -> Dict[str, Any]: - """ - 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 - if not self.is_deleted(): - data = getattr(self.get_access_permissions(), method)( - self, - *args) + restricted_data = self.get_access_permissions().get_restricted_data([self.get_full_data()], user) + data = restricted_data[0] if restricted_data else None else: data = None @@ -132,28 +136,31 @@ class CollectionElement: action='deleted' if self.is_deleted() else 'changed', data=data) - def as_autoupdate_for_user(self, user: Optional['CollectionElement']) -> Dict[str, Any]: - """ - Returns a dict that can be sent through the autoupdate system for a site - user. - """ - return self.as_autoupdate( - 'get_restricted_data', - user) - - def as_autoupdate_for_projector(self) -> Dict[str, Any]: + def as_autoupdate_for_projector(self) -> AutoupdateFormat: """ Returns a dict that can be sent through the autoupdate system for the projector. """ - return self.as_autoupdate( - 'get_projector_data') + if not self.is_deleted(): + restricted_data = self.get_access_permissions().get_projector_data([self.get_full_data()]) + data = restricted_data[0] if restricted_data else None + else: + data = None - def as_dict_for_user(self, user: Optional['CollectionElement']) -> 'RestrictedData': + return format_for_autoupdate( + collection_string=self.collection_string, + id=self.id, + action='deleted' if self.is_deleted() else 'changed', + data=data) + + def as_dict_for_user(self, user: Optional['CollectionElement']) -> Optional[Dict[str, Any]]: """ Returns a dict with the data for a user. Can be used for the rest api. + + Returns None if the user does not has the permission to see the element. """ - return self.get_access_permissions().get_restricted_data(self, user) + restricted_data = self.get_access_permissions().get_restricted_data([self.get_full_data()], user) + return restricted_data[0] if restricted_data else None def get_model(self) -> Type[Model]: """ @@ -161,31 +168,13 @@ class CollectionElement: """ return get_model_from_collection_string(self.collection_string) - def get_instance(self) -> Model: - """ - Returns the instance as django object. - - May raise a DoesNotExist exception. - """ - if self.is_deleted(): - raise RuntimeError("The collection element is deleted.") - - if self.instance is None: - model = self.get_model() - try: - query = model.objects.get_full_queryset() - except AttributeError: - query = model.objects - self.instance = query.get(pk=self.id) - return self.instance - 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) -> Any: + def get_full_data(self) -> Dict[str, Any]: """ Returns the full_data of this collection_element from with all other dics can be generated. @@ -195,17 +184,17 @@ class CollectionElement: """ # If the full_data is already loaded, return it # If there is a db_instance, use it to get the full_data - # else: try to use the cache. - # If there is no value in the cache, get the content from the db and save - # it to the cache. - if self.full_data is None and self.instance is None: - # Use the cache version if self.instance is not set. - # After this line full_data can be None, if the element is not in the cache. - self.full_data = cache.get(self.get_cache_key()) - + # else: use the cache. if self.full_data is None: - self.full_data = self.get_access_permissions().get_full_data(self.get_instance()) - self.save_to_cache() + if self.instance is None: + # Make sure the cache exists + if not full_data_cache.exists_for_collection(self.collection_string): + # Build the cache if it does not exists. + full_data_cache.build_for_collection(self.collection_string) + self.full_data = full_data_cache.get_element(self.collection_string, self.id) + else: + self.full_data = self.get_access_permissions().get_full_data(self.instance) + full_data_cache.add_element(self.collection_string, self.id, self.full_data) return self.full_data def is_deleted(self) -> bool: @@ -214,74 +203,6 @@ class CollectionElement: """ return self.deleted - def get_cache_key(self) -> str: - """ - Returns a string that is used as cache key for a single instance. - """ - return get_single_element_cache_key(self.collection_string, self.id) - - def delete_from_cache(self) -> None: - """ - Delets the element from the cache. - - Does nothing if the element is not in the cache. - """ - # Deletes the element from the cache. - cache.delete(self.get_cache_key()) - - # Delete the id of the instance of the instance list - Collection(self.collection_string).delete_id_from_cache(self.id) - - def save_to_cache(self) -> None: - """ - Add or update the element to the cache. - """ - # Set the element to the cache. - cache.set(self.get_cache_key(), self.get_full_data()) - - # Add the id of the element to the collection - Collection(self.collection_string).add_id_to_cache(self.id) - - -class CollectionElementList(list): - """ - List for collection elements that can hold collection elements from - different collections. - - It acts like a normal python list but with the following methods. - """ - - @classmethod - def from_channels_message(cls, message: Dict[str, Any]) -> 'CollectionElementList': - """ - Creates a collection element list from a channel message. - """ - self = cls() - for values in message['elements']: - self.append(CollectionElement.from_values(**values)) - return self - - def as_channels_message(self) -> Dict[str, Any]: - """ - Returns a list of dicts that can be send through the channel system. - """ - message = {'elements': []} # type: Dict[str, Any] - for element in self: - message['elements'].append(element.as_channels_message()) - return message - - def as_autoupdate_for_user(self, user: Optional[CollectionElement]) -> List[Dict[str, Any]]: - """ - Returns a list of dicts, that can be send though the websocket to a user. - - The argument `user` can be anything, that is allowd as argument for - utils.auth.has_perm(). - """ - result = [] - for element in self: - result.append(element.as_autoupdate_for_user(user)) - return result - class Collection: """ @@ -298,18 +219,6 @@ class Collection: self.collection_string = collection_string self.full_data = full_data - def get_cache_key(self, raw: bool=False) -> str: - """ - Returns a string that is used as cache key for a collection. - - Django adds a prefix to the cache key when using the django cache api. - In other cases use raw=True to add the same cache key. - """ - key = get_element_list_cache_key(self.collection_string) - if raw: - key = cache.make_key(key) - return key - def get_model(self) -> Type[Model]: """ Returns the django model that is used for this collection. @@ -326,38 +235,11 @@ class Collection: """ 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 = [ - get_single_element_cache_key(self.collection_string, id) - for id in ids] - cached_full_data_dict = cache.get_many(cache_keys) - - # Get all ids that are missing. - missing_cache_keys = set(cache_keys).difference(cached_full_data_dict.keys()) - missing_ids = set( - get_collection_id_from_cache_key(cache_key)[1] - for cache_key in missing_cache_keys) - - # Generate collection elements that where in the cache. - for cache_key, cached_full_data in cached_full_data_dict.items(): - collection_string, id = get_collection_id_from_cache_key(cache_key) + for full_data in self.get_full_data(): yield CollectionElement.from_values( - collection_string, - id, - full_data=cached_full_data) - - # Generate collection element that where not in the cache. - if missing_ids: - model = self.get_model() - try: - query = model.objects.get_full_queryset() - except AttributeError: - query = model.objects - for instance in query.filter(pk__in=missing_ids): - yield CollectionElement.from_instance(instance) + self.collection_string, + full_data['id'], + full_data=full_data) def get_full_data(self) -> List[Dict[str, Any]]: """ @@ -365,129 +247,19 @@ class Collection: elements. """ if self.full_data is None: - self.full_data = [ - collection_element.get_full_data() - for collection_element - in self.element_generator()] + # Build the cache, if it does not exists. + if not full_data_cache.exists_for_collection(self.collection_string): + full_data_cache.build_for_collection(self.collection_string) + + self.full_data = full_data_cache.get_data(self.collection_string) return self.full_data - def as_autoupdate_for_projector(self) -> List[Dict[str, Any]]: - """ - 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() - # Content can not be None. If the projector can not see an element, - # then it is marked as deleted. - output.append(content) - return output - - def as_autoupdate_for_user(self, user: Optional[CollectionElement]) -> List[Dict[str, Any]]: - """ - Returns a list of dicts, that can be send though the websocket to a user. - """ - # TODO: This method is not used. Remove it. - output = [] - for collection_element in self.element_generator(): - content = collection_element.as_autoupdate_for_user(user) - if content is not None: - output.append(content) - return output - - def as_list_for_user(self, user: Optional[CollectionElement]) -> List['RestrictedData']: + def as_list_for_user(self, user: Optional[CollectionElement]) -> List[Dict[str, Any]]: """ Returns a list of dictonaries to send them to a user, for example over the rest api. """ - output = [] # type: List[RestrictedData] - for collection_element in self.element_generator(): - content = collection_element.as_dict_for_user(user) # type: RestrictedData - if content is not None: - output.append(content) - return output - - def get_all_ids(self) -> Set[int]: - """ - Returns a set of all ids of instances in this collection. - """ - if use_redis_cache(): - ids = self.get_all_ids_redis() - else: - ids = self.get_all_ids_other() - return ids - - def get_all_ids_redis(self) -> Set[int]: - redis = get_redis_connection() - ids = redis.smembers(self.get_cache_key(raw=True)) - if not ids: - ids = set(self.get_model().objects.values_list('pk', flat=True)) - if ids: - redis.sadd(self.get_cache_key(raw=True), *ids) - # Redis returns the ids as string. - ids = set(int(id) for id in ids) - return ids - - def get_all_ids_other(self) -> Set[int]: - ids = cache.get(self.get_cache_key()) - if ids is None: - # If it is not in the cache then get it from the database. - ids = set(self.get_model().objects.values_list('pk', flat=True)) - cache.set(self.get_cache_key(), ids) - return ids - - def delete_id_from_cache(self, id: int) -> None: - """ - Delets a id from the cache. - """ - if use_redis_cache(): - self.delete_id_from_cache_redis(id) - else: - self.delete_id_from_cache_other(id) - - def delete_id_from_cache_redis(self, id: int) -> None: - redis = get_redis_connection() - redis.srem(self.get_cache_key(raw=True), id) - - def delete_id_from_cache_other(self, id: int) -> None: - ids = cache.get(self.get_cache_key()) - if ids is not None: - ids = set(ids) - try: - ids.remove(id) - except KeyError: - # The id is not part of id list - pass - else: - if ids: - cache.set(self.get_cache_key(), ids) - else: - # Delete the key, if there are not ids left - cache.delete(self.get_cache_key()) - - def add_id_to_cache(self, id: int) -> None: - """ - Adds a collection id to the list of collection ids in the cache. - """ - if use_redis_cache(): - self.add_id_to_cache_redis(id) - else: - self.add_id_to_cache_other(id) - - def add_id_to_cache_redis(self, id: int) -> None: - redis = get_redis_connection() - if redis.exists(self.get_cache_key(raw=True)): - # Only add the value if it is in the cache. - redis.sadd(self.get_cache_key(raw=True), id) - - def add_id_to_cache_other(self, id: int) -> None: - ids = cache.get(self.get_cache_key()) - if ids is not None: - # Only change the value if it is in the cache. - ids = set(ids) - ids.add(id) - cache.set(self.get_cache_key(), ids) + return self.get_access_permissions().get_restricted_data(self.get_full_data(), user) _models_to_collection_string = {} # type: Dict[str, Type[Model]] @@ -522,36 +294,52 @@ def get_model_from_collection_string(collection_string: str) -> Type[Model]: return model -def get_single_element_cache_key(collection_string: str, id: int) -> str: +def format_for_autoupdate(collection_string: str, id: int, action: str, data: Dict[str, Any]=None) -> AutoupdateFormat: """ - Returns a string that is used as cache key for a single instance. + Returns a dict that can be used for autoupdate. """ - return "{prefix}{id}".format( - prefix=get_single_element_cache_key_prefix(collection_string), - id=id) + if data is None: + # If the data is None 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 = AutoupdateFormat( + collection=collection_string, + id=id, + action=action, + ) + + if action != 'deleted': + data = cast(Dict[str, Any], data) # In this case data can not be None + output['data'] = data + + return output -def get_single_element_cache_key_prefix(collection_string: str) -> str: +def to_channel_message(elements: List[CollectionElement]) -> ChannelMessageFormat: """ - Returns the first part of the cache key for single elements, which is the - same for all cache keys of the same collection. + Converts a list of collection elements to a dict, that can be send to the + channels system. """ - return "{collection}:".format(collection=collection_string) + output = [] + for element in elements: + output.append(InnerChannelMessageFormat( + collection_string=element.collection_string, + id=element.id, + deleted=element.is_deleted(), + information=element.information, + full_data=element.full_data, + )) + return ChannelMessageFormat(elements=output) -def get_element_list_cache_key(collection_string: str) -> str: +def from_channel_message(message: ChannelMessageFormat) -> List[CollectionElement]: """ - Returns a string that is used as cache key for a collection. + Converts a list of collection elements back from a dict, that was created + via to_channel_message. """ - return "{collection}".format(collection=collection_string) - - -def get_collection_id_from_cache_key(cache_key: str) -> Tuple[str, int]: - """ - Returns a tuble of the collection string and the id from a cache_key - created with get_instance_cache_key. - - The returned id can be an integer or an string. - """ - collection_string, id = cache_key.rsplit(':', 1) - return (collection_string, int(id)) + elements = [] + for value in message['elements']: + elements.append(CollectionElement.from_values(**value)) + return elements diff --git a/openslides/utils/models.py b/openslides/utils/models.py index cc0dfc33a..9cbd82e41 100644 --- a/openslides/utils/models.py +++ b/openslides/utils/models.py @@ -115,5 +115,5 @@ class RESTModelMixin: # The deletion of a included element is a change of the root element. inform_changed_data(self.get_root_rest_element(), information=information) else: - inform_deleted_data(self.get_collection_string(), instance_pk, information=information) + inform_deleted_data([(self.get_collection_string(), instance_pk)], information=information) return return_value diff --git a/openslides/utils/rest_api.py b/openslides/utils/rest_api.py index 190f325f5..a9d509428 100644 --- a/openslides/utils/rest_api.py +++ b/openslides/utils/rest_api.py @@ -32,7 +32,7 @@ from rest_framework.viewsets import GenericViewSet as _GenericViewSet # noqa from rest_framework.viewsets import ModelViewSet as _ModelViewSet # noqa from rest_framework.viewsets import ViewSet as _ViewSet # noqa -from .access_permissions import BaseAccessPermissions, RestrictedData # noqa +from .access_permissions import BaseAccessPermissions from .auth import user_to_collection_user from .collection import Collection, CollectionElement @@ -214,7 +214,7 @@ class RetrieveModelMixin(_RetrieveModelMixin): collection_string, self.kwargs[lookup_url_kwarg]) user = user_to_collection_user(request.user) try: - content = collection_element.as_dict_for_user(user) # type: RestrictedData + content = collection_element.as_dict_for_user(user) except collection_element.get_model().DoesNotExist: raise Http404 if content is None: diff --git a/openslides/utils/test.py b/openslides/utils/test.py index dd9faee8d..5c714b611 100644 --- a/openslides/utils/test.py +++ b/openslides/utils/test.py @@ -1,8 +1,3 @@ -from contextlib import ContextDecorator -from typing import Any -from unittest.mock import patch - -from django.core.cache import caches from django.test import TestCase as _TestCase from django.test.runner import DiscoverRunner @@ -37,23 +32,6 @@ class TestCase(_TestCase): """ def tearDown(self) -> None: + from django_redis import get_redis_connection config.key_to_id = {} - - -class use_cache(ContextDecorator): - """ - Contextmanager that changes the code to use the local memory cache. - - Can also be used as decorator for a function. - - The code inside the contextmananger starts with an empty cache. - """ - - def __enter__(self) -> None: - cache = caches['locmem'] - cache.clear() - self.patch = patch('openslides.utils.collection.cache', cache) - self.patch.start() - - def __exit__(self, *exc: Any) -> None: - self.patch.stop() + get_redis_connection("default").flushall() diff --git a/requirements.txt b/requirements.txt index e458bce41..d9f99809d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,9 @@ # Requirements for OpenSlides in production --r requirements_production.txt +-r requirements_big_mode.txt # Requirements for development and tests in alphabetical order coverage flake8 isort==4.2.5 mypy +fakeredis diff --git a/setup.cfg b/setup.cfg index 82060da0e..a07430750 100644 --- a/setup.cfg +++ b/setup.cfg @@ -24,3 +24,6 @@ disallow_any = unannotated [mypy-openslides.core.config] disallow_any = unannotated + +[mypy-tests.*] +ignore_errors = true diff --git a/tests/integration/agenda/test_viewset.py b/tests/integration/agenda/test_viewset.py index 6144b6d22..5927a6fbb 100644 --- a/tests/integration/agenda/test_viewset.py +++ b/tests/integration/agenda/test_viewset.py @@ -1,6 +1,7 @@ from django.contrib.auth import get_user_model from django.core.urlresolvers import reverse from django.utils.translation import ugettext +from django_redis import get_redis_connection from rest_framework import status from rest_framework.test import APIClient @@ -11,7 +12,8 @@ from openslides.core.models import Countdown from openslides.motions.models import Motion from openslides.topics.models import Topic from openslides.users.models import User -from openslides.utils.test import TestCase, use_cache +from openslides.utils.autoupdate import inform_changed_data +from openslides.utils.test import TestCase class RetrieveItem(TestCase): @@ -91,12 +93,11 @@ class TestDBQueries(TestCase): Motion.objects.create(title='motion2') Assignment.objects.create(title='assignment', open_posts=5) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions, - * 2 requests to get the list of all agenda items, + * 7 requests to get the session an the request user with its permissions, + * 1 requests to get the list of all agenda items, * 1 request to get all speakers, * 3 requests to get the assignments, motions and topics and @@ -105,15 +106,15 @@ class TestDBQueries(TestCase): TODO: The last two request for the motionsversions are a bug. """ self.client.force_login(User.objects.get(pk=1)) - with self.assertNumQueries(12): + get_redis_connection("default").flushall() + with self.assertNumQueries(14): self.client.get(reverse('item-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 3 requests to get the permission for anonymous, - * 2 requests to get the list of all agenda items, + * 1 requests to get the list of all agenda items, * 1 request to get all speakers, * 3 requests to get the assignments, motions and topics and @@ -121,7 +122,8 @@ class TestDBQueries(TestCase): TODO: The last two request for the motionsversions are a bug. """ - with self.assertNumQueries(11): + get_redis_connection("default").flushall() + with self.assertNumQueries(10): self.client.get(reverse('item-list')) @@ -205,6 +207,8 @@ class ManageSpeaker(TestCase): group_delegates = type(group_staff).objects.get(name='Delegates') admin.groups.add(group_delegates) admin.groups.remove(group_staff) + inform_changed_data(admin) + response = self.client.post( reverse('item-manage-speaker', args=[self.item.pk]), {'user': self.user.pk}) @@ -240,7 +244,9 @@ class ManageSpeaker(TestCase): group_delegates = type(group_staff).objects.get(name='Delegates') admin.groups.add(group_delegates) admin.groups.remove(group_staff) + inform_changed_data(admin) speaker = Speaker.objects.add(self.user, self.item) + response = self.client.delete( reverse('item-manage-speaker', args=[self.item.pk]), {'speaker': speaker.pk}) diff --git a/tests/integration/assignments/test_viewset.py b/tests/integration/assignments/test_viewset.py index e4ef8ca4d..01cd20b1e 100644 --- a/tests/integration/assignments/test_viewset.py +++ b/tests/integration/assignments/test_viewset.py @@ -1,12 +1,14 @@ from django.contrib.auth import get_user_model from django.core.urlresolvers import reverse +from django_redis import get_redis_connection from rest_framework import status from rest_framework.test import APIClient from openslides.assignments.models import Assignment from openslides.core.config import config from openslides.users.models import User -from openslides.utils.test import TestCase, use_cache +from openslides.utils.autoupdate import inform_changed_data +from openslides.utils.test import TestCase class TestDBQueries(TestCase): @@ -24,12 +26,11 @@ class TestDBQueries(TestCase): for index in range(10): Assignment.objects.create(title='motion{}'.format(index), open_posts=1) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions, - * 2 requests to get the list of all assignments, + * 7 requests to get the session an the request user with its permissions, + * 1 requests to get the list of all assignments, * 1 request to get all related users, * 1 request to get the agenda item, * 1 request to get the polls, @@ -40,15 +41,15 @@ class TestDBQueries(TestCase): TODO: The last request are a bug. """ self.client.force_login(User.objects.get(pk=1)) - with self.assertNumQueries(20): + get_redis_connection("default").flushall() + with self.assertNumQueries(22): self.client.get(reverse('assignment-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 3 requests to get the permission for anonymous, - * 2 requests to get the list of all assignments, + * 1 requests to get the list of all assignments, * 1 request to get all related users, * 1 request to get the agenda item, * 1 request to get the polls, @@ -58,7 +59,8 @@ class TestDBQueries(TestCase): TODO: The last 10 requests are an bug. """ - with self.assertNumQueries(19): + get_redis_connection("default").flushall() + with self.assertNumQueries(18): self.client.get(reverse('assignment-list')) @@ -109,6 +111,7 @@ class CanidatureSelf(TestCase): group_delegates = type(group_staff).objects.get(name='Delegates') admin.groups.add(group_delegates) admin.groups.remove(group_staff) + inform_changed_data(admin) response = self.client.post(reverse('assignment-candidature-self', args=[self.assignment.pk])) @@ -155,6 +158,7 @@ class CanidatureSelf(TestCase): group_delegates = type(group_staff).objects.get(name='Delegates') admin.groups.add(group_delegates) admin.groups.remove(group_staff) + inform_changed_data(admin) response = self.client.delete(reverse('assignment-candidature-self', args=[self.assignment.pk])) @@ -235,6 +239,7 @@ class CandidatureOther(TestCase): group_delegates = type(group_staff).objects.get(name='Delegates') admin.groups.add(group_delegates) admin.groups.remove(group_staff) + inform_changed_data(admin) response = self.client.post( reverse('assignment-candidature-other', args=[self.assignment.pk]), @@ -290,6 +295,7 @@ class CandidatureOther(TestCase): group_delegates = type(group_staff).objects.get(name='Delegates') admin.groups.add(group_delegates) admin.groups.remove(group_staff) + inform_changed_data(admin) response = self.client.delete( reverse('assignment-candidature-other', args=[self.assignment.pk]), diff --git a/tests/integration/core/test_views.py b/tests/integration/core/test_views.py index b86a94a9c..02e5eecff 100644 --- a/tests/integration/core/test_views.py +++ b/tests/integration/core/test_views.py @@ -108,6 +108,7 @@ class ConfigViewSet(TestCase): # Save the old value of the config object and add the test values # TODO: Can be changed to setUpClass when Django 1.8 is no longer supported self._config_values = config.config_variables.copy() + config.key_to_id = {} config.update_config_variables(set_simple_config_view_integration_config_test()) config.save_default_values() diff --git a/tests/integration/core/test_viewset.py b/tests/integration/core/test_viewset.py index e72df8fd4..0143ee5b0 100644 --- a/tests/integration/core/test_viewset.py +++ b/tests/integration/core/test_viewset.py @@ -1,11 +1,12 @@ from django.core.urlresolvers import reverse +from django_redis import get_redis_connection from rest_framework import status from rest_framework.test import APIClient from openslides.core.config import config from openslides.core.models import ChatMessage, Projector, Tag from openslides.users.models import User -from openslides.utils.test import TestCase, use_cache +from openslides.utils.test import TestCase class TestProjectorDBQueries(TestCase): @@ -23,27 +24,27 @@ class TestProjectorDBQueries(TestCase): for index in range(10): Projector.objects.create(name="Projector{}".format(index)) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions, - * 2 requests to get the list of all projectors, + * 7 requests to get the session an the request user with its permissions, + * 1 requests to get the list of all projectors, * 1 request to get the list of the projector defaults. """ self.client.force_login(User.objects.get(pk=1)) - with self.assertNumQueries(7): + get_redis_connection("default").flushall() + with self.assertNumQueries(9): self.client.get(reverse('projector-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 3 requests to get the permission for anonymous, - * 2 requests to get the list of all projectors, + * 1 requests to get the list of all projectors, * 1 request to get the list of the projector defaults and """ - with self.assertNumQueries(6): + get_redis_connection("default").flushall() + with self.assertNumQueries(5): self.client.get(reverse('projector-list')) @@ -63,15 +64,15 @@ class TestCharmessageDBQueries(TestCase): for index in range(10): ChatMessage.objects.create(user=user) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions, - * 2 requests to get the list of all chatmessages, + * 7 requests to get the session an the request user with its permissions, + * 1 requests to get the list of all chatmessages, """ self.client.force_login(User.objects.get(pk=1)) - with self.assertNumQueries(6): + get_redis_connection("default").flushall() + with self.assertNumQueries(8): self.client.get(reverse('chatmessage-list')) @@ -90,25 +91,25 @@ class TestTagDBQueries(TestCase): for index in range(10): Tag.objects.create(name='tag{}'.format(index)) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 2 requests to get the session an the request user with its permissions, - * 2 requests to get the list of all tags, + * 5 requests to get the session an the request user with its permissions, + * 1 requests to get the list of all tags, """ self.client.force_login(User.objects.get(pk=1)) - with self.assertNumQueries(4): + get_redis_connection("default").flushall() + with self.assertNumQueries(6): self.client.get(reverse('tag-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 1 requests to see if anonyomus is enabled - * 2 requests to get the list of all projectors, + * 1 requests to get the list of all projectors, """ - with self.assertNumQueries(3): + get_redis_connection("default").flushall() + with self.assertNumQueries(2): self.client.get(reverse('tag-list')) @@ -125,29 +126,24 @@ class TestConfigDBQueries(TestCase): config['general_system_enable_anonymous'] = True config.save_default_values() - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 2 requests to get the session an the request user with its permissions and + * 5 requests to get the session an the request user with its permissions and * 1 requests to get the list of all config values - - * 1 more that I do not understand """ self.client.force_login(User.objects.get(pk=1)) - with self.assertNumQueries(4): + get_redis_connection("default").flushall() + with self.assertNumQueries(6): self.client.get(reverse('config-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: - * 1 requests to see if anonymous is enabled - * 1 to get all config value and - - * 1 more that I do not understand + * 1 requests to see if anonymous is enabled and get all config values """ - with self.assertNumQueries(3): + get_redis_connection("default").flushall() + with self.assertNumQueries(1): self.client.get(reverse('config-list')) diff --git a/tests/integration/mediafiles/test_viewset.py b/tests/integration/mediafiles/test_viewset.py index 579087fac..ddcb9bf8e 100644 --- a/tests/integration/mediafiles/test_viewset.py +++ b/tests/integration/mediafiles/test_viewset.py @@ -1,11 +1,12 @@ from django.core.files.uploadedfile import SimpleUploadedFile from django.core.urlresolvers import reverse +from django_redis import get_redis_connection from rest_framework.test import APIClient from openslides.core.config import config from openslides.mediafiles.models import Mediafile from openslides.users.models import User -from openslides.utils.test import TestCase, use_cache +from openslides.utils.test import TestCase class TestDBQueries(TestCase): @@ -27,23 +28,23 @@ class TestDBQueries(TestCase): 'some_file{}'.format(index), b'some content.')) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions and - * 2 requests to get the list of all files. + * 7 requests to get the session an the request user with its permissions and + * 1 requests to get the list of all files. """ self.client.force_login(User.objects.get(pk=1)) - with self.assertNumQueries(6): + get_redis_connection('default').flushall() + with self.assertNumQueries(8): self.client.get(reverse('mediafile-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 3 requests to get the permission for anonymous and - * 2 requests to get the list of all projectors. + * 1 requests to get the list of all projectors. """ - with self.assertNumQueries(5): + get_redis_connection('default').flushall() + with self.assertNumQueries(4): self.client.get(reverse('mediafile-list')) diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index 4529626e7..bc827313d 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -3,13 +3,16 @@ import json from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission from django.core.urlresolvers import reverse +from django_redis import get_redis_connection from rest_framework import status from rest_framework.test import APIClient from openslides.core.config import config from openslides.core.models import Tag from openslides.motions.models import Category, Motion, MotionBlock, State -from openslides.utils.test import TestCase, use_cache +from openslides.users.models import Group +from openslides.utils.autoupdate import inform_changed_data +from openslides.utils.test import TestCase class TestMotionDBQueries(TestCase): @@ -31,12 +34,11 @@ class TestMotionDBQueries(TestCase): password='password') # TODO: Create some polls etc. - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions, - * 2 requests to get the list of all motions, + * 7 requests to get the session an the request user with its permissions, + * 1 requests to get the list of all motions, * 1 request to get the motion versions, * 1 request to get the agenda item, * 1 request to get the motion log, @@ -46,15 +48,15 @@ class TestMotionDBQueries(TestCase): * 2 requests to get the submitters and supporters. """ self.client.force_login(get_user_model().objects.get(pk=1)) - with self.assertNumQueries(14): + get_redis_connection('default').flushall() + with self.assertNumQueries(16): self.client.get(reverse('motion-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 3 requests to get the permission for anonymous, - * 2 requests to get the list of all motions, + * 1 requests to get the list of all motions, * 1 request to get the motion versions, * 1 request to get the agenda item, * 1 request to get the motion log, @@ -63,7 +65,8 @@ class TestMotionDBQueries(TestCase): * 1 request to get the tags, * 2 requests to get the submitters and supporters. """ - with self.assertNumQueries(13): + get_redis_connection('default').flushall() + with self.assertNumQueries(12): self.client.get(reverse('motion-list')) @@ -82,25 +85,25 @@ class TestCategoryDBQueries(TestCase): for index in range(10): Category.objects.create(name='category{}'.format(index)) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions and - * 2 requests to get the list of all categories. + * 7 requests to get the session an the request user with its permissions and + * 1 requests to get the list of all categories. """ self.client.force_login(get_user_model().objects.get(pk=1)) - with self.assertNumQueries(6): + get_redis_connection('default').flushall() + with self.assertNumQueries(8): self.client.get(reverse('category-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 3 requests to get the permission for anonymous (config and permissions) - * 2 requests to get the list of all motions and + * 1 requests to get the list of all motions and """ - with self.assertNumQueries(5): + get_redis_connection('default').flushall() + with self.assertNumQueries(4): self.client.get(reverse('category-list')) @@ -115,29 +118,29 @@ class TestWorkflowDBQueries(TestCase): config['general_system_enable_anonymous'] = True # There do not need to be more workflows - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions, - * 2 requests to get the list of all workflows, + * 7 requests to get the session an the request user with its permissions, + * 1 requests to get the list of all workflows, * 1 request to get all states and * 1 request to get the next states of all states. """ self.client.force_login(get_user_model().objects.get(pk=1)) - with self.assertNumQueries(8): + get_redis_connection('default').flushall() + with self.assertNumQueries(10): self.client.get(reverse('workflow-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 3 requests to get the permission for anonymous, - * 2 requests to get the list of all workflows, + * 1 requests to get the list of all workflows, * 1 request to get all states and * 1 request to get the next states of all states. """ - with self.assertNumQueries(7): + get_redis_connection('default').flushall() + with self.assertNumQueries(6): self.client.get(reverse('workflow-list')) @@ -372,6 +375,7 @@ class CreateMotion(TestCase): self.admin = get_user_model().objects.get(username='admin') self.admin.groups.add(2) self.admin.groups.remove(3) + inform_changed_data(self.admin) response = self.client.post( reverse('motion-list'), @@ -412,7 +416,6 @@ class RetrieveMotion(TestCase): username='user_{}'.format(index), password='password') - @use_cache() def test_number_of_queries(self): """ Tests that only the following db queries are done: @@ -427,6 +430,7 @@ class RetrieveMotion(TestCase): * 2 requests to get the submitters and supporters. TODO: Fix all bugs. """ + get_redis_connection('default').flushall() with self.assertNumQueries(18): self.client.get(reverse('motion-detail', args=[self.motion.pk])) @@ -436,6 +440,9 @@ class RetrieveMotion(TestCase): state = self.motion.state state.required_permission_to_see = 'permission_that_the_user_does_not_have_leeceiz9hi7iuta4ahY2' state.save() + # The cache has to be cleared, see: + # https://github.com/OpenSlides/OpenSlides/issues/3396 + get_redis_connection("default").flushall() response = guest_client.get(reverse('motion-detail', args=[self.motion.pk])) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) @@ -461,11 +468,13 @@ class RetrieveMotion(TestCase): def test_user_without_can_see_user_permission_to_see_motion_and_submitter_data(self): self.motion.submitters.add(get_user_model().objects.get(username='admin')) - group = get_user_model().groups.field.related_model.objects.get(pk=1) # Group with pk 1 is for anonymous and default users. + inform_changed_data(self.motion) + group = Group.objects.get(pk=1) # Group with pk 1 is for anonymous and default users. permission_string = 'users.can_see_name' app_label, codename = permission_string.split('.') permission = group.permissions.get(content_type__app_label=app_label, codename=codename) group.permissions.remove(permission) + inform_changed_data(group) config['general_system_enable_anonymous'] = True guest_client = APIClient() @@ -549,6 +558,7 @@ class UpdateMotion(TestCase): admin = get_user_model().objects.get(username='admin') group_staff = admin.groups.get(name='Staff') admin.groups.remove(group_staff) + inform_changed_data(admin) self.motion.submitters.add(admin) supporter = get_user_model().objects.create_user( username='test_username_ahshi4oZin0OoSh9chee', diff --git a/tests/integration/topics/test_viewset.py b/tests/integration/topics/test_viewset.py index 8f82c9e4e..5548b1f9b 100644 --- a/tests/integration/topics/test_viewset.py +++ b/tests/integration/topics/test_viewset.py @@ -1,12 +1,13 @@ from django.contrib.auth import get_user_model from django.core.urlresolvers import reverse +from django_redis import get_redis_connection from rest_framework import status from rest_framework.test import APIClient from openslides.agenda.models import Item from openslides.core.config import config from openslides.topics.models import Topic -from openslides.utils.test import TestCase, use_cache +from openslides.utils.test import TestCase class TestDBQueries(TestCase): @@ -24,29 +25,29 @@ class TestDBQueries(TestCase): for index in range(10): Topic.objects.create(title='topic-{}'.format(index)) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions, - * 2 requests to get the list of all topics, + * 7 requests to get the session an the request user with its permissions, + * 1 requests to get the list of all topics, * 1 request to get attachments, * 1 request to get the agenda item """ self.client.force_login(get_user_model().objects.get(pk=1)) - with self.assertNumQueries(8): + get_redis_connection('default').flushall() + with self.assertNumQueries(10): self.client.get(reverse('topic-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 3 requests to get the permission for anonymous, - * 2 requests to get the list of all topics, + * 1 requests to get the list of all topics, * 1 request to get attachments, * 1 request to get the agenda item, """ - with self.assertNumQueries(7): + get_redis_connection('default').flushall() + with self.assertNumQueries(6): self.client.get(reverse('topic-list')) diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index 1dae4e29e..caaa76fdf 100644 --- a/tests/integration/users/test_viewset.py +++ b/tests/integration/users/test_viewset.py @@ -1,11 +1,12 @@ from django.core.urlresolvers import reverse +from django_redis import get_redis_connection from rest_framework import status from rest_framework.test import APIClient from openslides.core.config import config from openslides.users.models import Group, PersonalNote, User from openslides.users.serializers import UserFullSerializer -from openslides.utils.test import TestCase, use_cache +from openslides.utils.test import TestCase class TestUserDBQueries(TestCase): @@ -23,7 +24,6 @@ class TestUserDBQueries(TestCase): for index in range(10): User.objects.create(username='user{}'.format(index)) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: @@ -32,18 +32,19 @@ class TestUserDBQueries(TestCase): * 1 requests to get the list of all groups. """ self.client.force_login(User.objects.get(pk=1)) + get_redis_connection('default').flushall() with self.assertNumQueries(7): self.client.get(reverse('user-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 3 requests to get the permission for anonymous, - * 2 requests to get the list of all users and + * 1 requests to get the list of all users and * 2 request to get all groups (needed by the user serializer). """ - with self.assertNumQueries(7): + get_redis_connection('default').flushall() + with self.assertNumQueries(6): self.client.get(reverse('user-list')) @@ -62,28 +63,28 @@ class TestGroupDBQueries(TestCase): for index in range(10): Group.objects.create(name='group{}'.format(index)) - @use_cache() def test_admin(self): """ Tests that only the following db queries are done: - * 4 requests to get the session an the request user with its permissions and + * 6 requests to get the session an the request user with its permissions and * 1 request to get the list of all groups. The data of the groups where loaded when the admin was authenticated. So only the list of all groups has be fetched from the db. """ self.client.force_login(User.objects.get(pk=1)) - with self.assertNumQueries(5): + get_redis_connection('default').flushall() + with self.assertNumQueries(7): self.client.get(reverse('group-list')) - @use_cache() def test_anonymous(self): """ Tests that only the following db queries are done: * 1 requests to find out if anonymous is enabled - * 3 request to get the list of all groups and + * 2 request to get the list of all groups and """ - with self.assertNumQueries(4): + get_redis_connection('default').flushall() + with self.assertNumQueries(3): self.client.get(reverse('group-list')) diff --git a/tests/integration/utils/test_autoupdate.py b/tests/integration/utils/test_autoupdate.py index 91efe3829..8e5aef7f3 100644 --- a/tests/integration/utils/test_autoupdate.py +++ b/tests/integration/utils/test_autoupdate.py @@ -95,7 +95,7 @@ class TestsInformChangedData(ChannelTestCase): def test_delete_one_element(self): channel_layers[DEFAULT_CHANNEL_LAYER].flush() - inform_deleted_data('topics/topic', 1) + inform_deleted_data([('topics/topic', 1)]) channel_message = self.get_next_message('autoupdate.send_data', require=True) self.assertEqual(len(channel_message['elements']), 1) @@ -104,18 +104,7 @@ class TestsInformChangedData(ChannelTestCase): def test_delete_many_elements(self): channel_layers[DEFAULT_CHANNEL_LAYER].flush() - inform_deleted_data('topics/topic', 1, 'topics/topic', 2, 'testmodule/model', 1) + inform_deleted_data([('topics/topic', 1), ('topics/topic', 2), ('testmodule/model', 1)]) channel_message = self.get_next_message('autoupdate.send_data', require=True) self.assertEqual(len(channel_message['elements']), 3) - - def test_delete_no_element(self): - with self.assertRaises(ValueError): - inform_deleted_data() - - def test_delete_wrong_arguments(self): - with self.assertRaises(ValueError): - inform_deleted_data('testmodule/model') - - with self.assertRaises(ValueError): - inform_deleted_data('testmodule/model', 5, 'testmodule/model') diff --git a/tests/integration/utils/test_collection.py b/tests/integration/utils/test_collection.py index cf76ce264..53e9af150 100644 --- a/tests/integration/utils/test_collection.py +++ b/tests/integration/utils/test_collection.py @@ -1,34 +1,17 @@ -from unittest.mock import patch - -from channels.tests import ChannelTestCase -from django.core.cache import caches +from channels.tests import ChannelTestCase as TestCase +from django_redis import get_redis_connection from openslides.topics.models import Topic from openslides.utils import collection -class TestCase(ChannelTestCase): - """ - Testcase that uses the local mem cache and clears the cache after each test. - """ - def setUp(self): - cache = caches['locmem'] - cache.clear() - self.patch = patch('openslides.utils.collection.cache', cache) - self.patch.start() - - def tearDown(self): - self.patch.stop() - super().tearDown() - - class TestCollectionElementCache(TestCase): def test_clean_cache(self): """ Tests that the data is retrieved from the database. """ topic = Topic.objects.create(title='test topic') - caches['locmem'].clear() + get_redis_connection("default").flushall() with self.assertNumQueries(3): collection_element = collection.CollectionElement.from_values('topics/topic', 1) @@ -51,19 +34,6 @@ class TestCollectionElementCache(TestCase): instance = collection_element.get_full_data() self.assertEqual(topic.title, instance['title']) - @patch('openslides.utils.collection.cache') - def test_save_to_cache_called_once(self, mock_cache): - """ - Makes sure, that save_to_cache ins called (only) once, if CollectionElement - is created with "from_instance()". - """ - topic = Topic.objects.create(title='test topic') - mock_cache.set.reset_mock() - collection.CollectionElement.from_instance(topic) - - # cache.set() is called two times. Once for the object and once for the collection. - self.assertEqual(mock_cache.set.call_count, 2) - def test_fail_early(self): """ Tests that a CollectionElement.from_values fails, if the object does @@ -82,10 +52,10 @@ class TestCollectionCache(TestCase): Topic.objects.create(title='test topic2') Topic.objects.create(title='test topic3') topic_collection = collection.Collection('topics/topic') - caches['locmem'].clear() + get_redis_connection("default").flushall() - with self.assertNumQueries(4): - instance_list = list(topic_collection.as_autoupdate_for_projector()) + with self.assertNumQueries(3): + instance_list = list(topic_collection.get_full_data()) self.assertEqual(len(instance_list), 3) def test_with_cache(self): @@ -96,24 +66,10 @@ class TestCollectionCache(TestCase): Topic.objects.create(title='test topic2') Topic.objects.create(title='test topic3') topic_collection = collection.Collection('topics/topic') - list(topic_collection.as_autoupdate_for_projector()) + list(topic_collection.get_full_data()) with self.assertNumQueries(0): - instance_list = list(topic_collection.as_autoupdate_for_projector()) - self.assertEqual(len(instance_list), 3) - - def test_with_some_objects_in_the_cache(self): - """ - One element (topic3) is in the cache and two are not. - """ - Topic.objects.create(title='test topic1') - Topic.objects.create(title='test topic2') - caches['locmem'].clear() - Topic.objects.create(title='test topic3') - topic_collection = collection.Collection('topics/topic') - - with self.assertNumQueries(4): - instance_list = list(topic_collection.as_autoupdate_for_projector()) + instance_list = list(topic_collection.get_full_data()) self.assertEqual(len(instance_list), 3) def test_deletion(self): @@ -125,10 +81,10 @@ class TestCollectionCache(TestCase): Topic.objects.create(title='test topic2') topic3 = Topic.objects.create(title='test topic3') topic_collection = collection.Collection('topics/topic') - list(topic_collection.as_autoupdate_for_projector()) + list(topic_collection.get_full_data()) topic3.delete() with self.assertNumQueries(0): - instance_list = list(topic_collection.as_autoupdate_for_projector()) + instance_list = list(collection.Collection('topics/topic').get_full_data()) self.assertEqual(len(instance_list), 2) diff --git a/tests/settings.py b/tests/settings.py index 4c771882d..2b5f2c588 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -41,6 +41,10 @@ DATABASES = { } } +# When use_redis is True, the restricted data cache caches the data individuel +# for each user. This requires a lot of memory if there are a lot of active +# users. If use_redis is False, this setting has no effect. +DISABLE_USER_CACHE = False # Internationalization # https://docs.djangoproject.com/en/1.10/topics/i18n/ @@ -75,13 +79,12 @@ PASSWORD_HASHERS = [ 'django.contrib.auth.hashers.MD5PasswordHasher', ] - -# Use the dummy cache that does not cache anything CACHES = { - 'default': { - 'BACKEND': 'django.core.cache.backends.dummy.DummyCache' - }, - 'locmem': { - 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache' + "default": { + "BACKEND": "django_redis.cache.RedisCache", + "LOCATION": "redis://127.0.0.1:6379/0", + "OPTIONS": { + "REDIS_CLIENT_CLASS": "fakeredis.FakeStrictRedis", + } } } diff --git a/tests/unit/users/test_access_permissions.py b/tests/unit/users/test_access_permissions.py index e6929b2da..4c7d63bff 100644 --- a/tests/unit/users/test_access_permissions.py +++ b/tests/unit/users/test_access_permissions.py @@ -12,7 +12,7 @@ class UserGetProjectorDataTest(TestCase): """ This test ensures that comment field is removed. """ - container = CollectionElement.from_values('users/user', 42, full_data={ + full_data = { 'id': 42, 'username': 'username_ai3Oofu7eit0eeyu1sie', 'title': '', @@ -25,9 +25,10 @@ class UserGetProjectorDataTest(TestCase): 'is_present': False, 'is_committee': False, 'comment': 'comment_gah7aipeJohv9xethoku', - }) - data = UserAccessPermissions().get_projector_data(container) - self.assertEqual(data, { + } + + data = UserAccessPermissions().get_projector_data([full_data]) + self.assertEqual(data[0], { 'id': 42, 'username': 'username_ai3Oofu7eit0eeyu1sie', 'title': '', @@ -46,19 +47,16 @@ class TestPersonalNoteAccessPermissions(TestCase): def test_get_restricted_data(self): ap = PersonalNoteAccessPermissions() rd = ap.get_restricted_data( - CollectionElement.from_values( - 'users/personal_note', - 1, - full_data={'user_id': 1}), + [{'user_id': 1}], CollectionElement.from_values('users/user', 5, full_data={})) - self.assertEqual(rd, None) + self.assertEqual(rd, []) def test_get_restricted_data_for_anonymous(self): ap = PersonalNoteAccessPermissions() rd = ap.get_restricted_data( - CollectionElement.from_values( + [CollectionElement.from_values( 'users/personal_note', 1, - full_data={'user_id': 1}), + full_data={'user_id': 1})], None) - self.assertEqual(rd, None) + self.assertEqual(rd, []) diff --git a/tests/unit/utils/test_collection.py b/tests/unit/utils/test_collection.py index 4b66d0935..0c9ba4f1c 100644 --- a/tests/unit/utils/test_collection.py +++ b/tests/unit/utils/test_collection.py @@ -5,42 +5,6 @@ from openslides.core.models import Projector from openslides.utils import collection -class TestCacheKeys(TestCase): - def test_get_collection_id_from_cache_key(self): - """ - Test that get_collection_id_from_cache_key works together with - get_single_element_cache_key. - """ - element = ('some/testkey', 42) - self.assertEqual( - element, - collection.get_collection_id_from_cache_key( - collection.get_single_element_cache_key(*element))) - - def test_get_single_element_cache_key_prefix(self): - """ - Tests that the cache prefix is realy a prefix. - """ - element = ('some/testkey', 42) - - cache_key = collection.get_single_element_cache_key(*element) - prefix = collection.get_single_element_cache_key_prefix(element[0]) - - self.assertTrue(cache_key.startswith(prefix)) - - def test_prefix_different_then_list(self): - """ - Test that the return value of get_single_element_cache_key_prefix is - something different then get_element_list_cache_key. - """ - collection_string = 'some/testkey' - - prefix = collection.get_single_element_cache_key_prefix(collection_string) - list_cache_key = collection.get_element_list_cache_key(collection_string) - - self.assertNotEqual(prefix, list_cache_key) - - class TestGetModelFromCollectionString(TestCase): def test_known_app(self): projector_model = collection.get_model_from_collection_string('core/projector') @@ -60,34 +24,9 @@ class TestCollectionElement(TestCase): self.assertEqual(collection_element.collection_string, 'testmodule/model') self.assertEqual(collection_element.id, 42) - @patch('openslides.utils.collection.Collection') - @patch('openslides.utils.collection.cache') - def test_from_values_deleted(self, mock_cache, mock_collection): - """ - Tests that when createing a CollectionElement with deleted=True the element - is deleted from the cache. - """ - collection_element = collection.CollectionElement.from_values('testmodule/model', 42, deleted=True) - - self.assertTrue(collection_element.is_deleted()) - mock_cache.delete.assert_called_with('testmodule/model:42') - mock_collection.assert_called_with('testmodule/model') - mock_collection().delete_id_from_cache.assert_called_with(42) - - def test_as_channel_message(self): - with patch.object(collection.CollectionElement, 'get_full_data'): - collection_element = collection.CollectionElement.from_values('testmodule/model', 42) - - self.assertEqual( - collection_element.as_channels_message(), - {'collection_string': 'testmodule/model', - 'id': 42, - 'deleted': False}) - def test_channel_message(self): """ - Test that CollectionElement.from_values() works together with - collection_element.as_channels_message(). + Test that to_channel_message works together with from_channel_message. """ collection_element = collection.CollectionElement.from_values( 'testmodule/model', @@ -95,8 +34,8 @@ class TestCollectionElement(TestCase): full_data={'data': 'value'}, information={'some': 'information'}) - created_collection_element = collection.CollectionElement.from_values( - **collection_element.as_channels_message()) + created_collection_element = collection.from_channel_message( + collection.to_channel_message([collection_element]))[0] self.assertEqual( collection_element, @@ -109,7 +48,7 @@ class TestCollectionElement(TestCase): collection_element = collection.CollectionElement.from_values('testmodule/model', 42) fake_user = MagicMock() collection_element.get_access_permissions = MagicMock() - collection_element.get_access_permissions().get_restricted_data.return_value = 'restricted_data' + collection_element.get_access_permissions().get_restricted_data.return_value = ['restricted_data'] collection_element.get_full_data = MagicMock() self.assertEqual( @@ -118,7 +57,6 @@ class TestCollectionElement(TestCase): 'id': 42, 'action': 'changed', 'data': 'restricted_data'}) - 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'): @@ -133,7 +71,6 @@ class TestCollectionElement(TestCase): {'collection': 'testmodule/model', 'id': 42, 'action': 'deleted'}) - 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) @@ -145,76 +82,6 @@ class TestCollectionElement(TestCase): 'id': 42, 'action': 'deleted'}) - def test_get_instance_deleted(self): - collection_element = collection.CollectionElement.from_values('testmodule/model', 42, deleted=True) - - with self.assertRaises(RuntimeError): - collection_element.get_instance() - - def test_get_instance(self): - with patch.object(collection.CollectionElement, 'get_full_data'): - collection_element = collection.CollectionElement.from_values('testmodule/model', 42) - collection_element.get_model = MagicMock() - - collection_element.get_instance() - - collection_element.get_model().objects.get_full_queryset().get.assert_called_once_with(pk=42) - - @patch('openslides.utils.collection.cache') - def test_get_full_data_already_loaded(self, mock_cache): - """ - Test that the cache and the self.get_instance() is not hit, when the - instance is already loaded. - """ - with patch.object(collection.CollectionElement, 'get_full_data'): - collection_element = collection.CollectionElement.from_values('testmodule/model', 42) - collection_element.full_data = 'my_full_data' - collection_element.get_instance = MagicMock() - - collection_element.get_full_data() - - mock_cache.get.assert_not_called() - collection_element.get_instance.assert_not_called() - - @patch('openslides.utils.collection.cache') - def test_get_full_data_from_cache(self, mock_cache): - """ - Test that the value from the cache is used not get_instance is not - called. - """ - with patch.object(collection.CollectionElement, 'get_full_data'): - collection_element = collection.CollectionElement.from_values('testmodule/model', 42) - collection_element.get_instance = MagicMock() - mock_cache.get.return_value = 'cache_value' - - instance = collection_element.get_full_data() - - self.assertEqual(instance, 'cache_value') - mock_cache.get.assert_called_once_with('testmodule/model:42') - collection_element.get_instance.assert_not_called - - @patch('openslides.utils.collection.Collection') - @patch('openslides.utils.collection.cache') - def test_get_full_data_from_get_instance(self, mock_cache, mock_Collection): - """ - Test that the value from get_instance is used and saved to the cache - """ - with patch.object(collection.CollectionElement, 'get_full_data'): - collection_element = collection.CollectionElement.from_values('testmodule/model', 42) - collection_element.get_instance = MagicMock() - collection_element.get_access_permissions = MagicMock() - collection_element.get_access_permissions().get_full_data.return_value = 'get_instance_value' - mock_cache.get.return_value = None - - instance = collection_element.get_full_data() - - self.assertEqual(instance, 'get_instance_value') - mock_cache.get.assert_called_once_with('testmodule/model:42') - collection_element.get_instance.assert_called_once_with() - mock_cache.set.assert_called_once_with('testmodule/model:42', 'get_instance_value') - mock_Collection.assert_called_once_with('testmodule/model') - mock_Collection().add_id_to_cache.assert_called_once_with(42) - @patch.object(collection.CollectionElement, 'get_full_data') def test_equal(self, mock_get_full_data): self.assertEqual( @@ -229,67 +96,3 @@ class TestCollectionElement(TestCase): self.assertNotEqual( collection.CollectionElement.from_values('testmodule/model', 1), collection.CollectionElement.from_values('testmodule/other_model', 1)) - - -class TestcollectionElementList(TestCase): - @patch.object(collection.CollectionElement, 'get_full_data') - def test_channel_message(self, mock_get_full_data): - """ - Test that a channel message from three collection elements can crate - the same collection element list. - """ - collection_elements = collection.CollectionElementList(( - collection.CollectionElement.from_values('testmodule/model', 1), - collection.CollectionElement.from_values('testmodule/model', 2), - collection.CollectionElement.from_values('testmodule/model2', 1))) - - self.assertEqual( - collection_elements, - collection.CollectionElementList.from_channels_message(collection_elements.as_channels_message())) - - @patch.object(collection.CollectionElement, 'get_full_data') - def test_as_autoupdate_for_user(self, mock_get_full_data): - """ - Test that as_autoupdate_for_user is a list of as_autoupdate_for_user - for each individual element in the list. - """ - fake_user = MagicMock() - collection_elements = collection.CollectionElementList(( - collection.CollectionElement.from_values('testmodule/model', 1), - collection.CollectionElement.from_values('testmodule/model', 2), - collection.CollectionElement.from_values('testmodule/model2', 1))) - - with patch.object(collection.CollectionElement, 'as_autoupdate_for_user', return_value='for_user'): - value = collection_elements.as_autoupdate_for_user(fake_user) - - self.assertEqual(value, ['for_user'] * 3) - - -class TestCollection(TestCase): - @patch('openslides.utils.collection.CollectionElement') - @patch('openslides.utils.collection.cache') - def test_element_generator(self, mock_cache, mock_CollectionElement): - """ - Test with the following scenario: The collection has three elements. Two - are in the cache and one is not. - """ - test_collection = collection.Collection('testmodule/model') - test_collection.get_all_ids = MagicMock(return_value=set([1, 2, 3])) - test_collection.get_model = MagicMock() - test_collection.get_model().objects.get_full_queryset().filter.return_value = ['my_instance'] - mock_cache.get_many.return_value = { - 'testmodule/model:1': 'element1', - 'testmodule/model:2': 'element2'} - - list(test_collection.element_generator()) - - mock_cache.get_many.assert_called_once_with( - ['testmodule/model:1', 'testmodule/model:2', 'testmodule/model:3']) - test_collection.get_model().objects.get_full_queryset().filter.assert_called_once_with(pk__in={3}) - self.assertEqual(mock_CollectionElement.from_values.call_count, 2) - self.assertEqual(mock_CollectionElement.from_instance.call_count, 1) - - def test_raw_cache_key(self): - test_collection = collection.Collection('testmodule/model') - - self.assertEqual(test_collection.get_cache_key(raw=True), ':1:testmodule/model')