diff --git a/openslides/users/models.py b/openslides/users/models.py index 016537962..4f8833ae7 100644 --- a/openslides/users/models.py +++ b/openslides/users/models.py @@ -340,6 +340,12 @@ class PersonalNote(RESTModelMixin, models.Model): access_permissions = PersonalNoteAccessPermissions() + personalized_model = True + """ + Each model belongs to one user. This relation is set during creation and + will not be changed. + """ + objects = PersonalNoteManager() user = models.OneToOneField(User, on_delete=CASCADE_AND_AUTOUPDATE) diff --git a/openslides/utils/cache.py b/openslides/utils/cache.py index 422fc6f28..e97cf7388 100644 --- a/openslides/utils/cache.py +++ b/openslides/utils/cache.py @@ -308,16 +308,20 @@ class ElementCache: # the list(...) is important, because `changed_elements` will be # altered during iteration and restricting data for collection_string, elements in list(changed_elements.items()): - restricter = self.cachables[collection_string].restrict_elements - restricted_elements = await restricter(user_id, elements) - - # Add removed objects (through restricter) to deleted elements. - element_ids = set([element["id"] for element in elements]) - restricted_element_ids = set( - [element["id"] for element in restricted_elements] + cacheable = self.cachables[collection_string] + restricted_elements = await cacheable.restrict_elements( + user_id, elements ) - for id in element_ids - restricted_element_ids: - deleted_elements.append(get_element_id(collection_string, id)) + + # If the model is personalized, it must not be deleted for other users + if not cacheable.personalized_model: + # Add removed objects (through restricter) to deleted elements. + element_ids = set([element["id"] for element in elements]) + restricted_element_ids = set( + [element["id"] for element in restricted_elements] + ) + for id in element_ids - restricted_element_ids: + deleted_elements.append(get_element_id(collection_string, id)) if not restricted_elements: del changed_elements[collection_string] diff --git a/openslides/utils/cache_providers.py b/openslides/utils/cache_providers.py index 3677e4794..84adea402 100644 --- a/openslides/utils/cache_providers.py +++ b/openslides/utils/cache_providers.py @@ -623,6 +623,8 @@ class Cachable(Protocol): It needs at least the methods defined here. """ + personalized_model: bool + def get_collection_string(self) -> str: """ Returns the string representing the name of the cachable. diff --git a/openslides/utils/consumers.py b/openslides/utils/consumers.py index 49e97ce20..2ad665054 100644 --- a/openslides/utils/consumers.py +++ b/openslides/utils/consumers.py @@ -27,6 +27,8 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer): ID counter for assigning each instance of this class an unique id. """ + skipped_autoupdate_from_change_id: Optional[int] = None + def __init__(self, *args: Any, **kwargs: Any) -> None: self.projector_hash: Dict[int, int] = {} SiteConsumer.ID_COUNTER += 1 @@ -150,17 +152,31 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer): collection_string, id = split_element_id(element_id) deleted_elements[collection_string].append(id) - await self.send_json( - type="autoupdate", - content=AutoupdateFormat( - changed=changed_elements, - deleted=deleted_elements, - from_change_id=change_id, - to_change_id=max_change_id, - all_data=all_data, - ), - in_response=in_response, - ) + # Check, if the autoupdate has any data. + if not changed_elements and not deleted_element_ids: + # Set the current from_change_id, if it is the first skipped autoupdate + if not self.skipped_autoupdate_from_change_id: + self.skipped_autoupdate_from_change_id = change_id + else: + # Normal autoupdate with data + from_change_id = change_id + + # If there is at least one skipped autoupdate, take the saved from_change_id + if self.skipped_autoupdate_from_change_id: + from_change_id = self.skipped_autoupdate_from_change_id + self.skipped_autoupdate_from_change_id = None + + await self.send_json( + type="autoupdate", + content=AutoupdateFormat( + changed=changed_elements, + deleted=deleted_elements, + from_change_id=from_change_id, + to_change_id=max_change_id, + all_data=all_data, + ), + in_response=in_response, + ) async def send_data(self, event: Dict[str, Any]) -> None: """ diff --git a/openslides/utils/models.py b/openslides/utils/models.py index 2c82dc3bf..f9c9dea7b 100644 --- a/openslides/utils/models.py +++ b/openslides/utils/models.py @@ -38,6 +38,15 @@ class RESTModelMixin: access_permissions: Optional[BaseAccessPermissions] = None + personalized_model = False + """ + Flag, if the model is personalized on a per-user basis. + Requires the model to have a `user_id` which should be + a OneToOne relation to User. The relation must never change, + because it won't be deleted from it's former user when the relation + changes. + """ + def get_root_rest_element(self) -> models.Model: """ Returns the root rest instance. diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index b67bf203b..d961f9a73 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -14,6 +14,8 @@ class TConfig: Cachable, that fills the cache with the default values of the config variables. """ + personalized_model = False + def get_collection_string(self) -> str: return config.get_collection_string() @@ -38,6 +40,8 @@ class TUser: Cachable, that fills the cache with fake users. """ + personalized_model = False + def get_collection_string(self) -> str: return User.get_collection_string() @@ -75,6 +79,8 @@ class TProjector: Cachable, that mocks the projector. """ + personalized_model = False + def get_collection_string(self) -> str: return Projector.get_collection_string() diff --git a/tests/integration/utils/test_consumers.py b/tests/integration/utils/test_consumers.py index 6697073c8..3303a9ca6 100644 --- a/tests/integration/utils/test_consumers.py +++ b/tests/integration/utils/test_consumers.py @@ -1,6 +1,6 @@ import asyncio from importlib import import_module -from typing import Optional +from typing import Optional, Tuple from unittest.mock import patch import pytest @@ -21,7 +21,12 @@ from openslides.utils.websocket import ( WEBSOCKET_WRONG_FORMAT, ) -from ...unit.utils.cache_provider import Collection1, Collection2, get_cachable_provider +from ...unit.utils.cache_provider import ( + Collection1, + Collection2, + PersonalizedCollection, + get_cachable_provider, +) from ..helpers import TConfig, TProjector, TUser from ..websocket import WebsocketCommunicator @@ -36,7 +41,14 @@ async def prepare_element_cache(settings): await element_cache.cache_provider.clear_cache() orig_cachable_provider = element_cache.cachable_provider element_cache.cachable_provider = get_cachable_provider( - [Collection1(), Collection2(), TConfig(), TUser(), TProjector()] + [ + Collection1(), + Collection2(), + PersonalizedCollection(), + TConfig(), + TUser(), + TProjector(), + ] ) element_cache._cachables = None await element_cache.async_ensure_cache(default_change_id=2) @@ -51,11 +63,13 @@ async def prepare_element_cache(settings): async def get_communicator(): communicator: Optional[WebsocketCommunicator] = None - def get_communicator(query_string=""): + def get_communicator(query_string="", headers=None): nonlocal communicator # use the outer communicator variable if query_string: query_string = f"?{query_string}" - communicator = WebsocketCommunicator(application, f"/ws/{query_string}") + communicator = WebsocketCommunicator( + application, f"/ws/{query_string}", headers=headers + ) return communicator yield get_communicator @@ -117,6 +131,7 @@ async def test_connection_with_change_id(get_communicator, set_config): assert "to_change_id" in content assert Collection1().get_collection_string() in content["changed"] assert Collection2().get_collection_string() in content["changed"] + assert PersonalizedCollection().get_collection_string() in content["changed"] assert TConfig().get_collection_string() in content["changed"] assert TUser().get_collection_string() in content["changed"] @@ -179,26 +194,72 @@ async def test_anonymous_disabled(communicator): assert not connected -@pytest.mark.asyncio -async def test_with_user(): +async def create_user_session_cookie(user_id: int) -> Tuple[bytes, bytes]: # login user with id 1 engine = import_module(settings.SESSION_ENGINE) session = engine.SessionStore() # type: ignore - session[SESSION_KEY] = "1" + session[SESSION_KEY] = str(user_id) session[ HASH_SESSION_KEY ] = "362d4f2de1463293cb3aaba7727c967c35de43ee" # see helpers.TUser session[BACKEND_SESSION_KEY] = "django.contrib.auth.backends.ModelBackend" session.save() scn = settings.SESSION_COOKIE_NAME - cookies = (b"cookie", f"{scn}={session.session_key}".encode()) - communicator = WebsocketCommunicator(application, "/ws/", headers=[cookies]) + cookie_header = (b"cookie", f"{scn}={session.session_key}".encode()) + return cookie_header + + +@pytest.mark.asyncio +async def test_with_user(get_communicator): + cookie_header = await create_user_session_cookie(1) + communicator = get_communicator("autoupdate=on", headers=[cookie_header]) connected, __ = await communicator.connect() assert connected - await communicator.disconnect() + +@pytest.mark.asyncio +async def test_skipping_autoupdate(set_config, get_communicator): + cookie_header = await create_user_session_cookie(1) + communicator = get_communicator("autoupdate=on", headers=[cookie_header]) + + await communicator.connect() + + with patch("openslides.utils.autoupdate.save_history"): + await sync_to_async(inform_changed_elements)( + [ + Element( + id=2, + collection_string=PersonalizedCollection().get_collection_string(), + full_data={"id": 2, "value": "new value 1", "user_id": 2}, + disable_history=True, + ) + ] + ) + await sync_to_async(inform_changed_elements)( + [ + Element( + id=2, + collection_string=PersonalizedCollection().get_collection_string(), + full_data={"id": 2, "value": "new value 2", "user_id": 2}, + disable_history=True, + ) + ] + ) + assert await communicator.receive_nothing() + + # Trigger autoupdate + await set_config("general_event_name", "Test Event") + response = await communicator.receive_json_from() + content = response["content"] + + assert PersonalizedCollection().get_collection_string() not in content["deleted"] + assert PersonalizedCollection().get_collection_string() not in content["changed"] + assert config.get_collection_string() in content["changed"] + assert ( + content["to_change_id"] - content["from_change_id"] + ) == 2 # Skipped two autoupdates @pytest.mark.asyncio @@ -314,6 +375,7 @@ async def test_send_get_elements(communicator, set_config): assert "to_change_id" in content assert Collection1().get_collection_string() in content["changed"] assert Collection2().get_collection_string() in content["changed"] + assert PersonalizedCollection().get_collection_string() in content["changed"] assert TConfig().get_collection_string() in content["changed"] assert TUser().get_collection_string() in content["changed"] diff --git a/tests/unit/utils/cache_provider.py b/tests/unit/utils/cache_provider.py index 27387ef84..e26dbd788 100644 --- a/tests/unit/utils/cache_provider.py +++ b/tests/unit/utils/cache_provider.py @@ -21,6 +21,8 @@ def restrict_elements(elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: class Collection1: + personalized_model = False + def get_collection_string(self) -> str: return "app/collection1" @@ -34,6 +36,8 @@ class Collection1: class Collection2: + personalized_model = False + def get_collection_string(self) -> str: return "app/collection2" @@ -46,8 +50,26 @@ class Collection2: return restrict_elements(elements) +class PersonalizedCollection: + personalized_model = True + + def get_collection_string(self) -> str: + return "app/personalized-collection" + + def get_elements(self) -> List[Dict[str, Any]]: + return [ + {"id": 1, "key": "value1", "user_id": 1}, + {"id": 2, "key": "value2", "user_id": 2}, + ] + + async def restrict_elements( + self, user_id: int, elements: List[Dict[str, Any]] + ) -> List[Dict[str, Any]]: + return [element for element in elements if element["user_id"] == user_id] + + def get_cachable_provider( - cachables: List[Cachable] = [Collection1(), Collection2()] + cachables: List[Cachable] = [Collection1(), Collection2(), PersonalizedCollection()] ) -> Callable[[], List[Cachable]]: """ Returns a cachable_provider. @@ -59,6 +81,10 @@ def example_data(): return { "app/collection1": [{"id": 1, "value": "value1"}, {"id": 2, "value": "value2"}], "app/collection2": [{"id": 1, "key": "value1"}, {"id": 2, "key": "value2"}], + "app/personalized-collection": [ + {"id": 1, "key": "value1", "user_id": 1}, + {"id": 2, "key": "value2", "user_id": 2}, + ], } diff --git a/tests/unit/utils/test_cache.py b/tests/unit/utils/test_cache.py index 3da97ff4c..4a47816ce 100644 --- a/tests/unit/utils/test_cache.py +++ b/tests/unit/utils/test_cache.py @@ -79,6 +79,7 @@ async def test_change_elements_with_no_data_in_redis(element_cache): "app/collection1:2": {"id": 2, "value": "new"}, "app/collection2:1": {"id": 1, "key": "updated"}, "app/collection2:2": None, + "app/personalized-collection:2": None, } result = await element_cache.change_elements(input_data) @@ -89,6 +90,7 @@ async def test_change_elements_with_no_data_in_redis(element_cache): "app/collection1:1": '{"id": 1, "value": "updated"}', "app/collection1:2": '{"id": 2, "value": "new"}', "app/collection2:1": '{"id": 1, "key": "updated"}', + "app/personalized-collection:1": '{"id": 1, "key": "value1", "user_id": 1}', } ) assert element_cache.cache_provider.change_id_data == { @@ -97,6 +99,7 @@ async def test_change_elements_with_no_data_in_redis(element_cache): "app/collection1:2", "app/collection2:1", "app/collection2:2", + "app/personalized-collection:2", } } @@ -113,6 +116,8 @@ async def test_get_all_data_from_db(element_cache): "app/collection1:2": '{"id": 2, "value": "value2"}', "app/collection2:1": '{"id": 1, "key": "value1"}', "app/collection2:2": '{"id": 2, "key": "value2"}', + "app/personalized-collection:1": '{"id": 1, "key": "value1", "user_id": 1}', + "app/personalized-collection:2": '{"id": 2, "key": "value2", "user_id": 2}', } ) @@ -124,6 +129,8 @@ async def test_get_all_data_from_redis(element_cache): "app/collection1:2": '{"id": 2, "value": "value2"}', "app/collection2:1": '{"id": 1, "key": "value1"}', "app/collection2:2": '{"id": 2, "key": "value2"}', + "app/personalized-collection:1": '{"id": 1, "key": "value1", "user_id": 1}', + "app/personalized-collection:2": '{"id": 2, "key": "value2", "user_id": 2}', } result = await element_cache.get_all_data_list() @@ -139,6 +146,8 @@ async def test_get_data_since_change_id_0(element_cache): "app/collection1:2": '{"id": 2, "value": "value2"}', "app/collection2:1": '{"id": 1, "key": "value1"}', "app/collection2:2": '{"id": 2, "key": "value2"}', + "app/personalized-collection:1": '{"id": 1, "key": "value1", "user_id": 1}', + "app/personalized-collection:2": '{"id": 2, "key": "value2", "user_id": 2}', } result = await element_cache.get_data_since(None, 0) @@ -231,7 +240,9 @@ async def test_get_element_data_full_redis(element_cache): @pytest.mark.asyncio async def test_get_all_restricted_data(element_cache): - result = await element_cache.get_all_data_list(0) + result = await element_cache.get_all_data_list(1) + + # The output from redis has to be the same then the db_data assert sort_dict(result) == sort_dict( { @@ -243,13 +254,14 @@ async def test_get_all_restricted_data(element_cache): {"id": 1, "key": "restricted_value1"}, {"id": 2, "key": "restricted_value2"}, ], + "app/personalized-collection": [{"id": 1, "key": "value1", "user_id": 1}], } ) @pytest.mark.asyncio async def test_get_restricted_data_change_id_0(element_cache): - result = await element_cache.get_data_since(0, 0) + result = await element_cache.get_data_since(2, 0) assert sort_dict(result[0]) == sort_dict( { @@ -261,6 +273,7 @@ async def test_get_restricted_data_change_id_0(element_cache): {"id": 1, "key": "restricted_value1"}, {"id": 2, "key": "restricted_value2"}, ], + "app/personalized-collection": [{"id": 2, "key": "value2", "user_id": 2}], } ) @@ -279,6 +292,15 @@ async def test_get_restricted_data_2(element_cache): ) +@pytest.mark.asyncio +async def test_get_restricted_data_from_personalized_cacheable(element_cache): + element_cache.cache_provider.change_id_data = {1: {"app/personalized-collection:2"}} + + result = await element_cache.get_data_since(0, 1) + + assert result == ({}, []) + + @pytest.mark.asyncio async def test_get_restricted_data_change_id_lower_than_in_redis(element_cache): element_cache.cache_provider.default_change_id = 2