From e209dd6dfe1761ae58dedabe45b6d359f1b41ecd Mon Sep 17 00:00:00 2001 From: Oskar Hahn Date: Sat, 1 Oct 2016 15:26:00 +0200 Subject: [PATCH] Support to send many objects through the autoupdate system --- openslides/core/models.py | 1 - openslides/utils/autoupdate.py | 116 +++++++++++++------- openslides/utils/collection.py | 37 +++++++ tests/integration/utils/test_autoupdate.py | 120 ++++++++++++++++++++- tests/unit/utils/test_collection.py | 57 ++++++++++ 5 files changed, 288 insertions(+), 43 deletions(-) diff --git a/openslides/core/models.py b/openslides/core/models.py index 2409fca7e..d54f0b264 100644 --- a/openslides/core/models.py +++ b/openslides/core/models.py @@ -6,7 +6,6 @@ from jsonfield import JSONField from ..utils.collection import CollectionElement from ..utils.models import RESTModelMixin from ..utils.projector import ProjectorElement - from .access_permissions import ( ChatMessageAccessPermissions, ConfigAccessPermissions, diff --git a/openslides/utils/autoupdate.py b/openslides/utils/autoupdate.py index f45258207..613c2c596 100644 --- a/openslides/utils/autoupdate.py +++ b/openslides/utils/autoupdate.py @@ -1,5 +1,6 @@ import itertools import json +from collections import Iterable from asgiref.inmemory import ChannelLayer from channels import Channel, Group @@ -11,7 +12,7 @@ from ..core.config import config from ..core.models import Projector from ..users.auth import AnonymousUser from ..users.models import User -from .collection import Collection, CollectionElement +from .collection import Collection, CollectionElement, CollectionElementList def get_logged_in_users(): @@ -102,12 +103,12 @@ def send_data(message): """ Informs all site users and projector clients about changed data. """ - collection_element = CollectionElement.from_values(**message) + collection_elements = CollectionElementList.from_channels_message(message) # Loop over all logged in site users and the anonymous user and send changed data. for user in itertools.chain(get_logged_in_users(), [AnonymousUser()]): channel = Group('user-{}'.format(user.id)) - output = [collection_element.as_autoupdate_for_user(user)] + output = collection_elements.as_autoupdate_for_user(user) channel.send({'text': json.dumps(output)}) # Check whether broadcast is active at the moment and set the local @@ -119,11 +120,13 @@ def send_data(message): # Loop over all projectors and send data that they need. for projector in queryset: - if collection_element.is_deleted(): - output = [collection_element.as_autoupdate_for_projector()] - else: - collection_elements = projector.get_collection_elements_required_for_this(collection_element) - output = [collection_element.as_autoupdate_for_projector() for collection_element in collection_elements] + output = [] + for collection_element in collection_elements: + if collection_element.is_deleted(): + output.append(collection_element.as_autoupdate_for_projector()) + else: + for element in projector.get_collection_elements_required_for_this(collection_element): + output.append(collection_element.as_autoupdate_for_projector()) if output: if config['projector_broadcast'] > 0: Group('projector-all').send( @@ -133,50 +136,81 @@ def send_data(message): {'text': json.dumps(output)}) -def inform_changed_data(instance, information=None): +def inform_changed_data(instances, information=None): """ Informs the autoupdate system and the caching system about the creation or update of an element. - """ - try: - root_instance = instance.get_root_rest_element() - except AttributeError: - # Instance has no method get_root_rest_element. Just ignore it. - pass - else: - collection_element = CollectionElement.from_instance( - root_instance, - information=information) - # If currently there is an open database transaction, then the - # send_autoupdate function is only called, when the transaction is - # commited. If there is currently no transaction, then the function - # is called immediately. - transaction.on_commit(lambda: send_autoupdate(collection_element)) + The argument instances can be one instance or an interable over instances. + """ + root_instances = set() + if not isinstance(instances, Iterable): + # Make surce instance is an iterable + instances = (instances, ) + for instance in instances: + try: + root_instances.add(instance.get_root_rest_element()) + except AttributeError: + # Instance has no method get_root_rest_element. Just ignore it. + pass -def inform_deleted_data(collection_string, id, information=None): - """ - Informs the autoupdate system and the caching system about the deletion of - an element. - """ - collection_element = CollectionElement.from_values( - collection_string=collection_string, - id=id, - deleted=True, - information=information) + # Generates an collection element list for the root_instances. + collection_elements = CollectionElementList() + for root_instance in root_instances: + collection_elements.append( + CollectionElement.from_instance( + root_instance, + information=information)) # If currently there is an open database transaction, then the # send_autoupdate function is only called, when the transaction is # commited. If there is currently no transaction, then the function # is called immediately. - transaction.on_commit(lambda: send_autoupdate(collection_element)) + transaction.on_commit(lambda: send_autoupdate(collection_elements)) -def send_autoupdate(collection_element): +def inform_deleted_data(*args, information=None): """ - Helper function, that sends a collection_element through a channel to the + Informs the autoupdate system and the caching system about the deletion of + elements. + + The function has to be called with the attributes collection_string and id. + Multible elements can be used. For example: + + inform_deleted_data('motions/motion', 1, 'assignments/assignment', 5) + + The argument information is added to each collection element. + """ + if len(args) % 2 or not args: + raise ValueError( + "inform_deleted_data has to be called with the same number of " + "collection strings and ids. It has to be at least one collection " + "string and one id.") + + # Go through each pair of collection_string and id and generate a collection + # element from it. + collection_elements = CollectionElementList() + for index in range(0, len(args), 2): + collection_elements.append(CollectionElement.from_values( + collection_string=args[index], + id=args[index + 1], + deleted=True, + information=information)) + # If currently there is an open database transaction, then the + # send_autoupdate function is only called, when the transaction is + # commited. If there is currently no transaction, then the function + # is called immediately. + transaction.on_commit(lambda: send_autoupdate(collection_elements)) + + +def send_autoupdate(collection_elements): + """ + Helper function, that sends collection_elements through a channel to the autoupdate system. + + Does nothing if collection_elements is empty. """ - try: - Channel('autoupdate.send_data').send(collection_element.as_channels_message()) - except ChannelLayer.ChannelFull: - pass + if collection_elements: + try: + Channel('autoupdate.send_data').send(collection_elements.as_channels_message()) + except ChannelLayer.ChannelFull: + pass diff --git a/openslides/utils/collection.py b/openslides/utils/collection.py index eea1e71db..04fd5e027 100644 --- a/openslides/utils/collection.py +++ b/openslides/utils/collection.py @@ -225,6 +225,43 @@ class CollectionElement: Collection(self.collection_string).add_id_to_cache(self.id) +class CollectionElementList(list): + """ + List for collection elements that can hold collection elements from + different collections. + + It acts like a normal python list but with the following methods. + """ + + @classmethod + def from_channels_message(cls, message): + """ + Creates a collection element list from a channel message. + """ + self = cls() + for values in message['elements']: + self.append(CollectionElement.from_values(**values)) + return self + + def as_channels_message(self): + """ + Returns a list of dicts that can be send through the channel system. + """ + message = {'elements': []} + for element in self: + message['elements'].append(element.as_channels_message()) + return message + + def as_autoupdate_for_user(self, user): + """ + Returns a list of dicts, that can be send though the websocket to a user. + """ + result = [] + for element in self: + result.append(element.as_autoupdate_for_user(user)) + return result + + class Collection: """ Represents all elements of one collection. diff --git a/tests/integration/utils/test_autoupdate.py b/tests/integration/utils/test_autoupdate.py index 4644db167..126170548 100644 --- a/tests/integration/utils/test_autoupdate.py +++ b/tests/integration/utils/test_autoupdate.py @@ -1,10 +1,21 @@ from datetime import timedelta +from unittest.mock import patch +from channels import DEFAULT_CHANNEL_LAYER +from channels.asgi import channel_layers +from channels.tests import ChannelTestCase +from django.contrib.auth.models import Group from django.utils import timezone +from openslides.assignments.models import Assignment from openslides.core.models import Session +from openslides.topics.models import Topic from openslides.users.models import User -from openslides.utils.autoupdate import get_logged_in_users +from openslides.utils.autoupdate import ( + get_logged_in_users, + inform_changed_data, + inform_deleted_data, +) from openslides.utils.test import TestCase @@ -59,3 +70,110 @@ class TestGetLoggedInUsers(TestCase): session_key='2') self.assertEqual(list(get_logged_in_users()), [user1]) + + +@patch('openslides.utils.autoupdate.transaction.on_commit', lambda func: func()) +class TestsInformChangedData(ChannelTestCase): + # on_commit does not work with Djangos TestCase, see: + # https://docs.djangoproject.com/en/1.10/topics/db/transactions/#use-in-tests + # In this case it is also not possible to use a TransactionTestCase because + # we have to use the ChannelTestCase. + # The patch in the class decorator changes on_commit, so the given callable + # is called immediately. This is the same behavior as if there would be + # no transaction at all. + + def test_change_one_element(self): + topic = Topic.objects.create(title='test_topic') + channel_layers[DEFAULT_CHANNEL_LAYER].flush() + + inform_changed_data(topic) + + channel_message = self.get_next_message('autoupdate.send_data', require=True) + self.assertEqual(len(channel_message['elements']), 1) + self.assertEqual( + channel_message['elements'][0]['collection_string'], + 'topics/topic') + + def test_change_many_elements(self): + topics = ( + Topic.objects.create(title='test_topic1'), + Topic.objects.create(title='test_topic2'), + Topic.objects.create(title='test_topic3')) + channel_layers[DEFAULT_CHANNEL_LAYER].flush() + + inform_changed_data(topics) + + channel_message = self.get_next_message('autoupdate.send_data', require=True) + self.assertEqual(len(channel_message['elements']), 3) + + def test_change_with_non_root_rest_elements(self): + """ + Tests that if an root_rest_element is called together with one of its + child elements, then there is only the root_rest_element in the channel + message. + """ + assignment = Assignment.objects.create(title='test_assignment', open_posts=1) + poll = assignment.create_poll() + channel_layers[DEFAULT_CHANNEL_LAYER].flush() + + inform_changed_data((assignment, poll)) + + channel_message = self.get_next_message('autoupdate.send_data', require=True) + self.assertEqual(len(channel_message['elements']), 1) + + def test_change_only_non_root_rest_element(self): + """ + Tests that if only a non root_rest_element is called, then only the + root_rest_element is in the channel. + """ + assignment = Assignment.objects.create(title='test_assignment', open_posts=1) + poll = assignment.create_poll() + channel_layers[DEFAULT_CHANNEL_LAYER].flush() + + inform_changed_data(poll) + + channel_message = self.get_next_message('autoupdate.send_data', require=True) + self.assertEqual(len(channel_message['elements']), 1) + + def test_change_no_autoupdate_model(self): + """ + Tests that if inform_changed_data() is called with a model that does + not support autoupdate, nothing happens. + """ + group = Group.objects.create(name='test_group') + channel_layers[DEFAULT_CHANNEL_LAYER].flush() + + inform_changed_data(group) + + with self.assertRaises(AssertionError): + # self.get_next_message() with require=True raises a AssertionError + # if there is no message in the channel + self.get_next_message('autoupdate.send_data', require=True) + + def test_delete_one_element(self): + channel_layers[DEFAULT_CHANNEL_LAYER].flush() + + inform_deleted_data('topics/topic', 1) + + channel_message = self.get_next_message('autoupdate.send_data', require=True) + self.assertEqual(len(channel_message['elements']), 1) + self.assertTrue(channel_message['elements'][0]['deleted']) + + def test_delete_many_elements(self): + channel_layers[DEFAULT_CHANNEL_LAYER].flush() + + inform_deleted_data('topics/topic', 1, 'topics/topic', 2, 'testmodule/model', 1) + + channel_message = self.get_next_message('autoupdate.send_data', require=True) + self.assertEqual(len(channel_message['elements']), 3) + + def test_delete_no_element(self): + with self.assertRaises(ValueError): + inform_deleted_data() + + def test_delete_wrong_arguments(self): + with self.assertRaises(ValueError): + inform_deleted_data('testmodule/model') + + with self.assertRaises(ValueError): + inform_deleted_data('testmodule/model', 5, 'testmodule/model') diff --git a/tests/unit/utils/test_collection.py b/tests/unit/utils/test_collection.py index b5ae64f3a..462d167b3 100644 --- a/tests/unit/utils/test_collection.py +++ b/tests/unit/utils/test_collection.py @@ -92,6 +92,26 @@ class TestCollectionElement(TestCase): 'id': 42, 'deleted': False}) + def test_channel_message(self): + """ + Test that CollectionElement.from_values() works together with + collection_element.as_channels_message(). + """ + collection_element = collection.CollectionElement.from_values( + 'testmodule/model', + 42, + full_data={'data': 'value'}, + information={'some': 'information'}) + + created_collection_element = collection.CollectionElement.from_values( + **collection_element.as_channels_message()) + + self.assertEqual( + collection_element, + created_collection_element) + self.assertEqual(created_collection_element.full_data, {'data': 'value'}) + self.assertEqual(created_collection_element.information, {'some': 'information'}) + def test_as_autoupdate_for_user(self): collection_element = collection.CollectionElement.from_values('testmodule/model', 42) fake_user = MagicMock() @@ -241,6 +261,38 @@ class TestCollectionElement(TestCase): 'core/config:test_config_key') +class TestcollectionElementList(TestCase): + def test_channel_message(self): + """ + Test that a channel message from three collection elements can crate + the same collection element list. + """ + collection_elements = collection.CollectionElementList(( + collection.CollectionElement.from_values('testmodule/model', 1), + collection.CollectionElement.from_values('testmodule/model', 2), + collection.CollectionElement.from_values('testmodule/model2', 1))) + + self.assertEqual( + collection_elements, + collection.CollectionElementList.from_channels_message(collection_elements.as_channels_message())) + + def test_as_autoupdate_for_user(self): + """ + Test that as_autoupdate_for_user is a list of as_autoupdate_for_user + for each individual element in the list. + """ + fake_user = MagicMock() + collection_elements = collection.CollectionElementList(( + collection.CollectionElement.from_values('testmodule/model', 1), + collection.CollectionElement.from_values('testmodule/model', 2), + collection.CollectionElement.from_values('testmodule/model2', 1))) + + with patch.object(collection.CollectionElement, 'as_autoupdate_for_user', return_value='for_user'): + value = collection_elements.as_autoupdate_for_user(fake_user) + + self.assertEqual(value, ['for_user'] * 3) + + class TestCollection(TestCase): @patch('openslides.utils.collection.CollectionElement') @patch('openslides.utils.collection.cache') @@ -264,3 +316,8 @@ class TestCollection(TestCase): test_collection.get_model().objects.get_full_queryset().filter.assert_called_once_with(pk__in={3}) self.assertEqual(mock_CollectionElement.from_values.call_count, 2) self.assertEqual(mock_CollectionElement.from_instance.call_count, 1) + + def test_raw_cache_key(self): + test_collection = collection.Collection('testmodule/model') + + self.assertEqual(test_collection.get_cache_key(raw=True), ':1:testmodule/model')