Merge pull request #4970 from FinnStutzenstein/noAutoupdateForForeignPersonalNotes

Skip autoupdates on foreign personal notes

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)

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
+                cacheable = self.cachables[collection_string]
-                restricted_elements = await restricter(user_id, elements)
+                restricted_elements = await cacheable.restrict_elements(
-
+                    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]
                 )
-                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]

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.

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(
+        # Check, if the autoupdate has any data.
-            type="autoupdate",
+        if not changed_elements and not deleted_element_ids:
-            content=AutoupdateFormat(
+            # Set the current from_change_id, if it is the first skipped autoupdate
-                changed=changed_elements,
+            if not self.skipped_autoupdate_from_change_id:
-                deleted=deleted_elements,
+                self.skipped_autoupdate_from_change_id = change_id
-                from_change_id=change_id,
+        else:
-                to_change_id=max_change_id,
+            # Normal autoupdate with data
-                all_data=all_data,
+            from_change_id = change_id
-            ),
+
-            in_response=in_response,
+            # 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:
         """

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.

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()
 

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 create_user_session_cookie(user_id: int) -> Tuple[bytes, bytes]:
-async def test_with_user():
     # 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())
+    cookie_header = (b"cookie", f"{scn}={session.session_key}".encode())
-    communicator = WebsocketCommunicator(application, "/ws/", headers=[cookies])
+    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"]
 

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},
+        ],
     }
 
 

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