Fixed bug sending always all data to users, who get restricted data

Due to changes in an iterator, a RuntimeError was thrown but interpred al a too low change id. Fixed the bug and make a custom exception for this.
This commit is contained in:
FinnStutzenstein 2019-09-02 08:50:22 +02:00
parent 93fae8ef3b
commit 366777ebfd
3 changed files with 15 additions and 12 deletions

View File

@ -22,6 +22,10 @@ from .utils import get_element_id, split_element_id
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
class ChangeIdTooLowError(Exception):
pass
def get_all_cachables() -> List[Cachable]: def get_all_cachables() -> List[Cachable]:
""" """
Returns all element of OpenSlides. Returns all element of OpenSlides.
@ -274,7 +278,7 @@ class ElementCache:
Only returns elements with the change_id or newer. When change_id is 0, Only returns elements with the change_id or newer. When change_id is 0,
all elements are returned. all elements are returned.
Raises a RuntimeError when the lowest change_id in redis is higher then Raises a ChangeIdTooLowError when the lowest change_id in redis is higher then
the requested change_id. In this case the method has to be rerun with the requested change_id. In this case the method has to be rerun with
change_id=0. This is importend because there could be deleted elements change_id=0. This is importend because there could be deleted elements
that the cache does not know about. that the cache does not know about.
@ -288,9 +292,8 @@ class ElementCache:
if change_id < lowest_change_id: if change_id < lowest_change_id:
# When change_id is lower then the lowest change_id in redis, we can # When change_id is lower then the lowest change_id in redis, we can
# not inform the user about deleted elements. # not inform the user about deleted elements.
raise RuntimeError( raise ChangeIdTooLowError(
f"change_id {change_id} is lower then the lowest change_id in redis {lowest_change_id}. " f"change_id {change_id} is lower then the lowest change_id in redis {lowest_change_id}."
"Catch this exception and rerun the method with change_id=0."
) )
raw_changed_elements, deleted_elements = await self.cache_provider.get_data_since( raw_changed_elements, deleted_elements = await self.cache_provider.get_data_since(
@ -302,7 +305,9 @@ class ElementCache:
} }
if user_id is not None: if user_id is not None:
for collection_string, elements in changed_elements.items(): # 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 restricter = self.cachables[collection_string].restrict_elements
restricted_elements = await restricter(user_id, elements) restricted_elements = await restricter(user_id, elements)
@ -332,8 +337,6 @@ class ElementCache:
async def get_lowest_change_id(self) -> int: async def get_lowest_change_id(self) -> int:
""" """
Returns the lowest change id. Returns the lowest change id.
Raises a RuntimeError if there is no change_id.
""" """
return await self.cache_provider.get_lowest_change_id() return await self.cache_provider.get_lowest_change_id()

View File

@ -7,7 +7,7 @@ from ..utils.websocket import WEBSOCKET_CHANGE_ID_TOO_HIGH
from . import logging from . import logging
from .auth import async_anonymous_is_enabled from .auth import async_anonymous_is_enabled
from .autoupdate import AutoupdateFormat from .autoupdate import AutoupdateFormat
from .cache import element_cache, split_element_id from .cache import ChangeIdTooLowError, element_cache, split_element_id
from .utils import get_worker_id from .utils import get_worker_id
from .websocket import ProtocollAsyncJsonWebsocketConsumer from .websocket import ProtocollAsyncJsonWebsocketConsumer
@ -138,7 +138,7 @@ class SiteConsumer(ProtocollAsyncJsonWebsocketConsumer):
changed_elements, deleted_element_ids = await element_cache.get_data_since( changed_elements, deleted_element_ids = await element_cache.get_data_since(
user_id, change_id, max_change_id user_id, change_id, max_change_id
) )
except RuntimeError: except ChangeIdTooLowError:
# The change_id is lower the the lowerst change_id in redis. Return all data # The change_id is lower the the lowerst change_id in redis. Return all data
changed_elements = await element_cache.get_all_data_list(user_id) changed_elements = await element_cache.get_all_data_list(user_id)
all_data = True all_data = True

View File

@ -3,7 +3,7 @@ from typing import Any, Dict, List
import pytest import pytest
from openslides.utils.cache import ElementCache from openslides.utils.cache import ChangeIdTooLowError, ElementCache
from .cache_provider import TTestCacheProvider, example_data, get_cachable_provider from .cache_provider import TTestCacheProvider, example_data, get_cachable_provider
@ -156,7 +156,7 @@ async def test_get_data_since_change_id_lower_than_in_redis(element_cache):
} }
element_cache.cache_provider.default_change_id = 2 element_cache.cache_provider.default_change_id = 2
element_cache.cache_provider.change_id_data = {2: {"app/collection1:1"}} element_cache.cache_provider.change_id_data = {2: {"app/collection1:1"}}
with pytest.raises(RuntimeError): with pytest.raises(ChangeIdTooLowError):
await element_cache.get_data_since(None, 1) await element_cache.get_data_since(None, 1)
@ -283,7 +283,7 @@ async def test_get_restricted_data_2(element_cache):
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
with pytest.raises(RuntimeError): with pytest.raises(ChangeIdTooLowError):
await element_cache.get_data_since(0, 1) await element_cache.get_data_since(0, 1)