Skip autoupdates on foreign personal notes

This commit is contained in:
FinnStutzenstein 2019-09-02 13:57:12 +02:00
parent ee0b82b93b
commit 5baae14156
9 changed files with 187 additions and 34 deletions

View File

@ -340,6 +340,12 @@ class PersonalNote(RESTModelMixin, models.Model):
access_permissions = PersonalNoteAccessPermissions() 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() objects = PersonalNoteManager()
user = models.OneToOneField(User, on_delete=CASCADE_AND_AUTOUPDATE) user = models.OneToOneField(User, on_delete=CASCADE_AND_AUTOUPDATE)

View File

@ -308,9 +308,13 @@ class ElementCache:
# the list(...) is important, because `changed_elements` will be # the list(...) is important, because `changed_elements` will be
# altered during iteration and restricting data # altered during iteration and restricting data
for collection_string, elements in list(changed_elements.items()): 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
)
# 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. # Add removed objects (through restricter) to deleted elements.
element_ids = set([element["id"] for element in elements]) element_ids = set([element["id"] for element in elements])
restricted_element_ids = set( restricted_element_ids = set(

View File

@ -623,6 +623,8 @@ class Cachable(Protocol):
It needs at least the methods defined here. It needs at least the methods defined here.
""" """
personalized_model: bool
def get_collection_string(self) -> str: def get_collection_string(self) -> str:
""" """
Returns the string representing the name of the cachable. Returns the string representing the name of the cachable.

View File

@ -27,6 +27,8 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer):
ID counter for assigning each instance of this class an unique id. 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: def __init__(self, *args: Any, **kwargs: Any) -> None:
self.projector_hash: Dict[int, int] = {} self.projector_hash: Dict[int, int] = {}
SiteConsumer.ID_COUNTER += 1 SiteConsumer.ID_COUNTER += 1
@ -150,12 +152,26 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer):
collection_string, id = split_element_id(element_id) collection_string, id = split_element_id(element_id)
deleted_elements[collection_string].append(id) deleted_elements[collection_string].append(id)
# 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( await self.send_json(
type="autoupdate", type="autoupdate",
content=AutoupdateFormat( content=AutoupdateFormat(
changed=changed_elements, changed=changed_elements,
deleted=deleted_elements, deleted=deleted_elements,
from_change_id=change_id, from_change_id=from_change_id,
to_change_id=max_change_id, to_change_id=max_change_id,
all_data=all_data, all_data=all_data,
), ),

View File

@ -38,6 +38,15 @@ class RESTModelMixin:
access_permissions: Optional[BaseAccessPermissions] = None 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: def get_root_rest_element(self) -> models.Model:
""" """
Returns the root rest instance. Returns the root rest instance.

View File

@ -14,6 +14,8 @@ class TConfig:
Cachable, that fills the cache with the default values of the config variables. Cachable, that fills the cache with the default values of the config variables.
""" """
personalized_model = False
def get_collection_string(self) -> str: def get_collection_string(self) -> str:
return config.get_collection_string() return config.get_collection_string()
@ -38,6 +40,8 @@ class TUser:
Cachable, that fills the cache with fake users. Cachable, that fills the cache with fake users.
""" """
personalized_model = False
def get_collection_string(self) -> str: def get_collection_string(self) -> str:
return User.get_collection_string() return User.get_collection_string()
@ -75,6 +79,8 @@ class TProjector:
Cachable, that mocks the projector. Cachable, that mocks the projector.
""" """
personalized_model = False
def get_collection_string(self) -> str: def get_collection_string(self) -> str:
return Projector.get_collection_string() return Projector.get_collection_string()

View File

@ -1,6 +1,6 @@
import asyncio import asyncio
from importlib import import_module from importlib import import_module
from typing import Optional from typing import Optional, Tuple
from unittest.mock import patch from unittest.mock import patch
import pytest import pytest
@ -21,7 +21,12 @@ from openslides.utils.websocket import (
WEBSOCKET_WRONG_FORMAT, 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 ..helpers import TConfig, TProjector, TUser
from ..websocket import WebsocketCommunicator from ..websocket import WebsocketCommunicator
@ -36,7 +41,14 @@ async def prepare_element_cache(settings):
await element_cache.cache_provider.clear_cache() await element_cache.cache_provider.clear_cache()
orig_cachable_provider = element_cache.cachable_provider orig_cachable_provider = element_cache.cachable_provider
element_cache.cachable_provider = get_cachable_provider( element_cache.cachable_provider = get_cachable_provider(
[Collection1(), Collection2(), TConfig(), TUser(), TProjector()] [
Collection1(),
Collection2(),
PersonalizedCollection(),
TConfig(),
TUser(),
TProjector(),
]
) )
element_cache._cachables = None element_cache._cachables = None
await element_cache.async_ensure_cache(default_change_id=2) await element_cache.async_ensure_cache(default_change_id=2)
@ -51,11 +63,13 @@ async def prepare_element_cache(settings):
async def get_communicator(): async def get_communicator():
communicator: Optional[WebsocketCommunicator] = None communicator: Optional[WebsocketCommunicator] = None
def get_communicator(query_string=""): def get_communicator(query_string="", headers=None):
nonlocal communicator # use the outer communicator variable nonlocal communicator # use the outer communicator variable
if query_string: if query_string:
query_string = f"?{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 return communicator
yield get_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 "to_change_id" in content
assert Collection1().get_collection_string() in content["changed"] assert Collection1().get_collection_string() in content["changed"]
assert Collection2().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 TConfig().get_collection_string() in content["changed"]
assert TUser().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 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 # login user with id 1
engine = import_module(settings.SESSION_ENGINE) engine = import_module(settings.SESSION_ENGINE)
session = engine.SessionStore() # type: ignore session = engine.SessionStore() # type: ignore
session[SESSION_KEY] = "1" session[SESSION_KEY] = str(user_id)
session[ session[
HASH_SESSION_KEY HASH_SESSION_KEY
] = "362d4f2de1463293cb3aaba7727c967c35de43ee" # see helpers.TUser ] = "362d4f2de1463293cb3aaba7727c967c35de43ee" # see helpers.TUser
session[BACKEND_SESSION_KEY] = "django.contrib.auth.backends.ModelBackend" session[BACKEND_SESSION_KEY] = "django.contrib.auth.backends.ModelBackend"
session.save() session.save()
scn = settings.SESSION_COOKIE_NAME 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() connected, __ = await communicator.connect()
assert connected 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 @pytest.mark.asyncio
@ -314,6 +375,7 @@ async def test_send_get_elements(communicator, set_config):
assert "to_change_id" in content assert "to_change_id" in content
assert Collection1().get_collection_string() in content["changed"] assert Collection1().get_collection_string() in content["changed"]
assert Collection2().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 TConfig().get_collection_string() in content["changed"]
assert TUser().get_collection_string() in content["changed"] assert TUser().get_collection_string() in content["changed"]

View File

@ -21,6 +21,8 @@ def restrict_elements(elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
class Collection1: class Collection1:
personalized_model = False
def get_collection_string(self) -> str: def get_collection_string(self) -> str:
return "app/collection1" return "app/collection1"
@ -34,6 +36,8 @@ class Collection1:
class Collection2: class Collection2:
personalized_model = False
def get_collection_string(self) -> str: def get_collection_string(self) -> str:
return "app/collection2" return "app/collection2"
@ -46,8 +50,26 @@ class Collection2:
return restrict_elements(elements) 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( def get_cachable_provider(
cachables: List[Cachable] = [Collection1(), Collection2()] cachables: List[Cachable] = [Collection1(), Collection2(), PersonalizedCollection()]
) -> Callable[[], List[Cachable]]: ) -> Callable[[], List[Cachable]]:
""" """
Returns a cachable_provider. Returns a cachable_provider.
@ -59,6 +81,10 @@ def example_data():
return { return {
"app/collection1": [{"id": 1, "value": "value1"}, {"id": 2, "value": "value2"}], "app/collection1": [{"id": 1, "value": "value1"}, {"id": 2, "value": "value2"}],
"app/collection2": [{"id": 1, "key": "value1"}, {"id": 2, "key": "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},
],
} }

View File

@ -79,6 +79,7 @@ async def test_change_elements_with_no_data_in_redis(element_cache):
"app/collection1:2": {"id": 2, "value": "new"}, "app/collection1:2": {"id": 2, "value": "new"},
"app/collection2:1": {"id": 1, "key": "updated"}, "app/collection2:1": {"id": 1, "key": "updated"},
"app/collection2:2": None, "app/collection2:2": None,
"app/personalized-collection:2": None,
} }
result = await element_cache.change_elements(input_data) 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:1": '{"id": 1, "value": "updated"}',
"app/collection1:2": '{"id": 2, "value": "new"}', "app/collection1:2": '{"id": 2, "value": "new"}',
"app/collection2:1": '{"id": 1, "key": "updated"}', "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 == { 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/collection1:2",
"app/collection2:1", "app/collection2:1",
"app/collection2:2", "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/collection1:2": '{"id": 2, "value": "value2"}',
"app/collection2:1": '{"id": 1, "key": "value1"}', "app/collection2:1": '{"id": 1, "key": "value1"}',
"app/collection2:2": '{"id": 2, "key": "value2"}', "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/collection1:2": '{"id": 2, "value": "value2"}',
"app/collection2:1": '{"id": 1, "key": "value1"}', "app/collection2:1": '{"id": 1, "key": "value1"}',
"app/collection2:2": '{"id": 2, "key": "value2"}', "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() 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/collection1:2": '{"id": 2, "value": "value2"}',
"app/collection2:1": '{"id": 1, "key": "value1"}', "app/collection2:1": '{"id": 1, "key": "value1"}',
"app/collection2:2": '{"id": 2, "key": "value2"}', "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) 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 @pytest.mark.asyncio
async def test_get_all_restricted_data(element_cache): 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( 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": 1, "key": "restricted_value1"},
{"id": 2, "key": "restricted_value2"}, {"id": 2, "key": "restricted_value2"},
], ],
"app/personalized-collection": [{"id": 1, "key": "value1", "user_id": 1}],
} }
) )
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_get_restricted_data_change_id_0(element_cache): 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( 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": 1, "key": "restricted_value1"},
{"id": 2, "key": "restricted_value2"}, {"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 @pytest.mark.asyncio
async def test_get_restricted_data_change_id_lower_than_in_redis(element_cache): async def test_get_restricted_data_change_id_lower_than_in_redis(element_cache):
element_cache.cache_provider.default_change_id = 2 element_cache.cache_provider.default_change_id = 2