From 7cd70a568c11be7ea615400f4251d027e677af7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Norman=20J=C3=A4ckel?= Date: Fri, 30 Sep 2016 20:42:58 +0200 Subject: [PATCH] Added docstrings. Small changes. --- CHANGELOG | 1 + openslides/agenda/models.py | 5 ++++ openslides/assignments/models.py | 14 +++++++-- openslides/core/access_permissions.py | 12 ++++---- openslides/core/models.py | 2 +- openslides/core/views.py | 1 + openslides/motions/models.py | 25 +++++++++++++--- openslides/topics/models.py | 20 +++++++++---- openslides/users/models.py | 7 +++-- openslides/users/serializers.py | 2 +- openslides/utils/access_permissions.py | 8 ++--- openslides/utils/autoupdate.py | 26 ++++++++-------- openslides/utils/collection.py | 3 ++ openslides/utils/models.py | 36 ++++++++++++----------- tests/integration/agenda/test_viewsets.py | 2 +- 15 files changed, 108 insertions(+), 56 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 17146e2e6..3f14ccdc0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -42,6 +42,7 @@ Other: - Fixed bug, that the last change of a config value was not send via autoupdate. - Added template hooks for plugins (in item detail view and motion poll form). - Used Django Channels instead of Tornado. Refactoring of the autoupdate process. +- Added new caching system with support for Redis. Version 2.0 (2016-04-18) diff --git a/openslides/agenda/models.py b/openslides/agenda/models.py index 27e17645e..88b4d9579 100644 --- a/openslides/agenda/models.py +++ b/openslides/agenda/models.py @@ -24,6 +24,11 @@ class ItemManager(models.Manager): numbering. """ def get_full_queryset(self): + """ + Returns the normal queryset with all items. In the background all + speakers and related items (topics, motions, assignments) are + prefetched from the database. + """ return self.get_queryset().prefetch_related('speakers', 'content_object') def get_only_agenda_items(self): diff --git a/openslides/assignments/models.py b/openslides/assignments/models.py index 5986c4c8a..98e63d3e4 100644 --- a/openslides/assignments/models.py +++ b/openslides/assignments/models.py @@ -51,7 +51,15 @@ class AssignmentRelatedUser(RESTModelMixin, models.Model): class AssignmentManager(models.Manager): + """ + Customized model manager to support our get_full_queryset method. + """ def get_full_queryset(self): + """ + Returns the normal queryset with all assignments. In the background + all related users (candidates), the related agenda item and all + polls are prefetched from the database. + """ return self.get_queryset().prefetch_related( 'related_users', 'agenda_items', @@ -121,8 +129,8 @@ class Assignment(RESTModelMixin, models.Model): Tags for the assignment. """ - # In theory there could be one then more agenda_item. But support only one. - # See the property agenda_item. + # In theory there could be one then more agenda_item. But we support only + # one. See the property agenda_item. agenda_items = GenericRelation(Item, related_name='assignments') class Meta: @@ -304,6 +312,8 @@ class Assignment(RESTModelMixin, models.Model): """ Returns the related agenda item. """ + # We support only one agenda item so just return the first element of + # the queryset. return self.agenda_items.all()[0] @property diff --git a/openslides/core/access_permissions.py b/openslides/core/access_permissions.py index 623e6af80..0ffb49cba 100644 --- a/openslides/core/access_permissions.py +++ b/openslides/core/access_permissions.py @@ -84,11 +84,13 @@ class ConfigAccessPermissions(BaseAccessPermissions): Returns the serlialized config data. """ from .config import config + from .models import ConfigStore # Attention: The format of this response has to be the same as in # the retrieve method of ConfigViewSet. - if isinstance(instance, dict): - # It happens, that the caching system already sends the correct dict - # as instance. - return instance - return {'key': instance.key, 'value': config[instance.key]} + if isinstance(instance, ConfigStore): + result = {'key': instance.key, 'value': config[instance.key]} + else: + # It is possible, that the caching system already sends the correct data as "instance". + result = instance + return result diff --git a/openslides/core/models.py b/openslides/core/models.py index 4937b7bfa..c1dd6a509 100644 --- a/openslides/core/models.py +++ b/openslides/core/models.py @@ -116,7 +116,7 @@ class Projector(RESTModelMixin, models.Model): result[key]['error'] = str(e) return result - def get_all_requirements(self, on_slide=None): + def get_all_requirements(self, on_slide=None): #TODO: Refactor or rename this. """ Generator which returns all instances that are shown on this projector. """ diff --git a/openslides/core/views.py b/openslides/core/views.py index 83e396419..9982c5652 100644 --- a/openslides/core/views.py +++ b/openslides/core/views.py @@ -624,6 +624,7 @@ class ConfigViewSet(ViewSet): raise Http404 if content is None: # If content is None, the user has no permissions to see the item. + # See ConfigAccessPermissions or rather its parent class. self.permission_denied() return Response(content) diff --git a/openslides/motions/models.py b/openslides/motions/models.py index 5d4788eda..dae32ca75 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -29,8 +29,15 @@ from .exceptions import WorkflowError class MotionManager(models.Manager): + """ + Customized model manager to support our get_full_queryset method. + """ def get_full_queryset(self): - return (super().get_queryset() + """ + Returns the normal queryset with all motions. In the background we + join and prefetch all related models. + """ + return (self.get_queryset() .select_related('active_version') .prefetch_related( 'versions', @@ -45,7 +52,7 @@ class MotionManager(models.Manager): class Motion(RESTModelMixin, models.Model): """ - The Motion Class. + Model for motions. This class is the main entry point to all other classes related to a motion. """ @@ -151,8 +158,8 @@ class Motion(RESTModelMixin, models.Model): Configurable fields for comments. Contains a list of strings. """ - # In theory there could be one then more agenda_item. But support only one. - # See the property agenda_item. + # In theory there could be one then more agenda_item. But we support only + # one. See the property agenda_item. agenda_items = GenericRelation(Item, related_name='motions') class Meta: @@ -540,6 +547,8 @@ class Motion(RESTModelMixin, models.Model): """ Returns the related agenda item. """ + # We support only one agenda item so just return the first element of + # the queryset. return self.agenda_items.all()[0] @property @@ -961,7 +970,15 @@ class State(RESTModelMixin, models.Model): class WorkflowManager(models.Manager): + """ + Customized model manager to support our get_full_queryset method. + """ def get_full_queryset(self): + """ + Returns the normal queryset with all workflows. In the background + the first state is joined and all states and next states are + prefetched from the database. + """ return (self.get_queryset() .select_related('first_state') .prefetch_related('states', 'states__next_states')) diff --git a/openslides/topics/models.py b/openslides/topics/models.py index 51d65968e..d60e162e7 100644 --- a/openslides/topics/models.py +++ b/openslides/topics/models.py @@ -8,9 +8,16 @@ from .access_permissions import TopicAccessPermissions class TopicManager(models.Manager): - def get_queryset(self): - query = super().get_queryset().prefetch_related('attachments', 'agenda_items') - return query + """ + Customized model manager to support our get_full_queryset method. + """ + def get_full_queryset(self): + """ + Returns the normal queryset with all topics. In the background all + attachments and the related agenda item are prefetched from the + database. + """ + return self.get_queryset().prefetch_related('attachments', 'agenda_items') class Topic(RESTModelMixin, models.Model): @@ -18,14 +25,15 @@ class Topic(RESTModelMixin, models.Model): Model for slides with custom content. Used to be called custom slide. """ access_permissions = TopicAccessPermissions() + objects = TopicManager() title = models.CharField(max_length=256) text = models.TextField(blank=True) attachments = models.ManyToManyField(Mediafile, blank=True) - # In theory there could be one then more agenda_item. But support only one. - # See the property agenda_item. + # In theory there could be one then more agenda_item. But we support only + # one. See the property agenda_item. agenda_items = GenericRelation(Item, related_name='topics') class Meta: @@ -39,6 +47,8 @@ class Topic(RESTModelMixin, models.Model): """ Returns the related agenda item. """ + # We support only one agenda item so just return the first element of + # the queryset. return self.agenda_items.all()[0] @property diff --git a/openslides/users/models.py b/openslides/users/models.py index 76b7e775e..e7a8fcf03 100644 --- a/openslides/users/models.py +++ b/openslides/users/models.py @@ -21,10 +21,13 @@ from .access_permissions import UserAccessPermissions class UserManager(BaseUserManager): """ Customized manager that creates new users only with a password and a - username. + username. It also supports our get_full_queryset method. """ - def get_full_queryset(self): + """ + Returns the normal queryset with all users. In the background all + groups are prefetched from the database. + """ return self.get_queryset().prefetch_related('groups') def create_user(self, username, password, **kwargs): diff --git a/openslides/users/serializers.py b/openslides/users/serializers.py index e023fd087..fb5088543 100644 --- a/openslides/users/serializers.py +++ b/openslides/users/serializers.py @@ -21,12 +21,12 @@ USERCANSEESERIALIZER_FIELDS = ( 'number', 'about_me', 'groups', + 'is_present', 'is_committee', ) USERCANSEEEXTRASERIALIZER_FIELDS = USERCANSEESERIALIZER_FIELDS + ( - 'is_present', 'comment', 'is_active', ) diff --git a/openslides/utils/access_permissions.py b/openslides/utils/access_permissions.py index 1586fb6ae..3a1c4d6c7 100644 --- a/openslides/utils/access_permissions.py +++ b/openslides/utils/access_permissions.py @@ -68,7 +68,7 @@ class BaseAccessPermissions(object, metaclass=SignalConnectMetaClass): Hint: You should override this method if your get_serializer_class() method returns different serializers for different users or if you have access restrictions in your view or viewset in methods like - retrieve(), list() or check_object_permissions(). + retrieve() or list(). """ if self.check_permissions(user): data = full_data @@ -78,8 +78,8 @@ class BaseAccessPermissions(object, metaclass=SignalConnectMetaClass): def get_projector_data(self, full_data): """ - Returns the serialized data for the projector. Returns None if has no - access to this specific data. Returns reduced data if the user has - limited access. Default: Returns full data. + Returns the serialized data for the projector. Returns None if the + user has no access to this specific data. Returns reduced data if + the user has limited access. Default: Returns full data. """ return full_data diff --git a/openslides/utils/autoupdate.py b/openslides/utils/autoupdate.py index b9039ef4f..33517477b 100644 --- a/openslides/utils/autoupdate.py +++ b/openslides/utils/autoupdate.py @@ -23,7 +23,7 @@ def get_logged_in_users(): return User.objects.exclude(session=None).filter(session__expire_date__gte=timezone.now()).distinct() -def get_projector_element_data(projector, on_slide=None): +def get_projector_element_data(projector, on_slide=None): #TODO """ Returns a list of dicts that are required for a specific projector. @@ -81,7 +81,7 @@ def ws_add_projector(message, projector_id): Group('projector-{}'.format(projector_id)).add(message.reply_channel) # Send all elements that are on the projector. - output = get_projector_element_data(projector) + output = get_projector_element_data(projector) #TODO # Send all config elements. collection = Collection(config.get_collection_string()) @@ -102,7 +102,7 @@ def ws_disconnect_projector(message, projector_id): Group('projector-{}'.format(projector_id)).discard(message.reply_channel) -def send_data(message): +def send_data(message): #TODO """ Informs all users about changed data. """ @@ -174,9 +174,13 @@ def inform_changed_data(instance, information=None): 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)) + def inform_deleted_data(collection_string, id, information=None): """ Informs the autoupdate system and the caching system about the deletion of @@ -187,16 +191,10 @@ def inform_deleted_data(collection_string, id, information=None): id=id, deleted=True, information=information) - - # If currently there is an open database transaction, then the following - # function is only called, when the transaction is commited. If there - # is currently no transaction, then the function is called immediately. - def send_autoupdate(): - try: - Channel('autoupdate.send_data').send(collection_element.as_channels_message()) - except ChannelLayer.ChannelFull: - pass - + # 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)) diff --git a/openslides/utils/collection.py b/openslides/utils/collection.py index 7a0413700..1aa0f35b0 100644 --- a/openslides/utils/collection.py +++ b/openslides/utils/collection.py @@ -459,6 +459,9 @@ def get_collection_id_from_cache_key(cache_key): def use_redis_cache(): + """ + Returns True if Redis is used als caching backend. + """ try: from django_redis.cache import RedisCache except ImportError: diff --git a/openslides/utils/models.py b/openslides/utils/models.py index c2f1547fb..14b73deb0 100644 --- a/openslides/utils/models.py +++ b/openslides/utils/models.py @@ -57,38 +57,40 @@ class RESTModelMixin: def save(self, skip_autoupdate=False, information=None, *args, **kwargs): """ - Calls the django save-method and afterwards hits the autoupdate system. + Calls Django's save() method and afterwards hits the autoupdate system. If skip_autoupdate is set to True, then the autoupdate system is not informed about the model changed. This also means, that the model cache - is not updated. + is not updated. You have to do it manually. Like this: - The optional argument information can be an object that is given to the - autoupdate system. It should be a dict. + TODO HELP ME + + The optional argument information can be a dictionary that is given to + the autoupdate system. """ - # TODO: Fix circular imports + #TODO: Add example in docstring. + #TODO: Fix circular imports from .autoupdate import inform_changed_data return_value = super().save(*args, **kwargs) - inform_changed_data(self.get_root_rest_element(), information=information) + if not skip_autoupdate: + inform_changed_data(self.get_root_rest_element(), information=information) return return_value def delete(self, skip_autoupdate=False, information=None, *args, **kwargs): """ - Calls the django delete-method and afterwards hits the autoupdate system. + Calls Django's delete() method and afterwards hits the autoupdate system. See the save method above. """ - # TODO: Fix circular imports + #TODO: Fix circular imports from .autoupdate import inform_changed_data, inform_deleted_data - # Django sets the pk of the instance to None after deleting it. But - # we need the pk to tell the autoupdate system which element was deleted. instance_pk = self.pk return_value = super().delete(*args, **kwargs) - if self != self.get_root_rest_element(): - # The deletion of a included element is a change of the master - # element. - # TODO: Does this work in any case with self.pk = None? - inform_changed_data(self.get_root_rest_element(), information=information) - else: - inform_deleted_data(self, information=information) + if not skip_autoupdate: + if self != self.get_root_rest_element(): + # The deletion of a included element is a change of the root element. + #TODO: Does this work in any case with self.pk == None? + inform_changed_data(self.get_root_rest_element(), information=information) + else: + inform_deleted_data(self.get_collection_string(), instance_pk, information=information) return return_value diff --git a/tests/integration/agenda/test_viewsets.py b/tests/integration/agenda/test_viewsets.py index 4d581dae3..34287a271 100644 --- a/tests/integration/agenda/test_viewsets.py +++ b/tests/integration/agenda/test_viewsets.py @@ -40,7 +40,7 @@ class RetrieveItem(TestCase): permission = group.permissions.get(content_type__app_label=app_label, codename=codename) group.permissions.remove(permission) response = self.client.get(reverse('item-detail', args=[self.item.pk])) - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, status.HTTP_403_PERMISSION_DENIED) class TestDBQueries(TestCase):