ag-admin/OpenSlides
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #2950 from ostcar/fix_2835

Browse Source 
Let CollectionElement fail early
This commit is contained in:
Emanuel Schütze 2017-02-13 13:45:14 +01:00 committed by GitHub
commit 802e253621
4 changed files with 61 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
openslides/utils/autoupdate.py
View File

@ -7,6 +7,7 @@ from channels import Channel, Group
from channels.asgi import get_channel_layer
from channels.auth import channel_session_user, channel_session_user_from_http
from django.apps import apps
from django.core.exceptions import ObjectDoesNotExist
from django.db import transaction
from ..core.config import config
@ -150,7 +151,11 @@ def send_data(message):
# Anonymous user
user = None
else:
user = CollectionElement.from_values('users/user', user_id)
try:
user = CollectionElement.from_values('users/user', user_id)
except ObjectDoesNotExist:
# The user does not exist. Skip him/her.
continue
output = collection_elements.as_autoupdate_for_user(user)
for channel_name in channel_names:
send_or_wait(Channel(channel_name).send, {'text': json.dumps(output)})

16
openslides/utils/collection.py
View File

@ -60,10 +60,13 @@ class CollectionElement:
if self.is_deleted():
# Delete the element from the cache, if self.is_deleted() is True:
self.delete_from_cache()
elif instance is not None:
# If this element is created via instance and the instance is not deleted
# then update the cache.
self.save_to_cache()
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
# then get_full_data() will save the object into the cache.
# This will also raise a DoesNotExist error, if the object does
# neither exist in the cache nor in the database.
self.get_full_data()
def __eq__(self, collection_element):
"""
@ -184,11 +187,14 @@ class CollectionElement:
"""
Returns the full_data of this collection_element from with all other
dics can be generated.
Raises a DoesNotExist error on the requested the coresponding model, if
the object does neither exist in the cache nor in the database.
"""
# 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 cach, get the content from the db and save
# 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.

27
tests/integration/utils/test_collection.py
View File

@ -27,11 +27,12 @@ class TestCollectionElementCache(TestCase):
Tests that the data is retrieved from the database.
"""
topic = Topic.objects.create(title='test topic')
collection_element = collection.CollectionElement.from_values('topics/topic', 1)
caches['locmem'].clear()
with self.assertNumQueries(3):
collection_element = collection.CollectionElement.from_values('topics/topic', 1)
instance = collection_element.get_full_data()
self.assertEqual(topic.title, instance['title'])
def test_with_cache(self):
@ -49,20 +50,32 @@ class TestCollectionElementCache(TestCase):
instance = collection_element.get_full_data()
self.assertEqual(topic.title, instance['title'])
def test_non_existing_instance(self):
collection_element = collection.CollectionElement.from_values('topics/topic', 1)
@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
not exist.
"""
with self.assertRaises(Topic.DoesNotExist):
collection_element.get_full_data()
collection.CollectionElement.from_values('topics/topic', 999)
class TestCollectionCache(TestCase):
def test_clean_cache(self):
"""
Tests that the instances are retrieved from the database.
Currently there are 10 queries needed. This can change in the future,
but it has to be more then zero.
"""
Topic.objects.create(title='test topic1')
Topic.objects.create(title='test topic2')

36
tests/unit/utils/test_collection.py
View File

@ -64,7 +64,8 @@ class TestGetModelFromCollectionString(TestCase):
class TestCollectionElement(TestCase):
def test_from_values(self):
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
with patch.object(collection.CollectionElement, 'get_full_data'):
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
self.assertEqual(collection_element.collection_string, 'testmodule/model')
self.assertEqual(collection_element.id, 42)
@ -84,7 +85,8 @@ class TestCollectionElement(TestCase):
mock_collection().delete_id_from_cache.assert_called_with(42)
def test_as_channel_message(self):
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
with patch.object(collection.CollectionElement, 'get_full_data'):
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
self.assertEqual(
collection_element.as_channels_message(),
@ -113,7 +115,8 @@ class TestCollectionElement(TestCase):
self.assertEqual(created_collection_element.information, {'some': 'information'})
def test_as_autoupdate_for_user(self):
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
with patch.object(collection.CollectionElement, 'get_full_data'):
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'
@ -128,7 +131,8 @@ class TestCollectionElement(TestCase):
collection_element.get_full_data.assert_called_once_with()
def test_as_autoupdate_for_user_no_permission(self):
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
with patch.object(collection.CollectionElement, 'get_full_data'):
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 = None
@ -171,7 +175,8 @@ class TestCollectionElement(TestCase):
'value': 'config_value'})
def test_get_instance(self):
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
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()
@ -184,7 +189,8 @@ class TestCollectionElement(TestCase):
Test that the cache and the self.get_instance() is not hit, when the
instance is already loaded.
"""
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
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()
@ -199,7 +205,8 @@ class TestCollectionElement(TestCase):
Test that the value from the cache is used not get_instance is not
called.
"""
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
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'
@ -215,7 +222,8 @@ class TestCollectionElement(TestCase):
"""
Test that the value from get_instance is used and saved to the cache
"""
collection_element = collection.CollectionElement.from_values('testmodule/model', 42)
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'
@ -230,7 +238,8 @@ class TestCollectionElement(TestCase):
mock_Collection.assert_called_once_with('testmodule/model')
mock_Collection().add_id_to_cache.assert_called_once_with(42)
def test_equal(self):
@patch.object(collection.CollectionElement, 'get_full_data')
def test_equal(self, mock_get_full_data):
self.assertEqual(
collection.CollectionElement.from_values('testmodule/model', 1),
collection.CollectionElement.from_values('testmodule/model', 1))
@ -244,7 +253,8 @@ class TestCollectionElement(TestCase):
collection.CollectionElement.from_values('testmodule/model', 1),
collection.CollectionElement.from_values('testmodule/other_model', 1))
def test_config_cache_key(self):
@patch.object(collection.CollectionElement, 'get_full_data')
def test_config_cache_key(self, mock_get_full_data):
"""
Test that collection elements for config values do always use the
config key as cache key.
@ -262,7 +272,8 @@ class TestCollectionElement(TestCase):
class TestcollectionElementList(TestCase):
def test_channel_message(self):
@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.
@ -276,7 +287,8 @@ class TestcollectionElementList(TestCase):
collection_elements,
collection.CollectionElementList.from_channels_message(collection_elements.as_channels_message()))
def test_as_autoupdate_for_user(self):
@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.