From 87b889fbf2aa8e4a7f50087def2bcf447f2a06fe Mon Sep 17 00:00:00 2001 From: Oskar Hahn Date: Tue, 22 Aug 2017 14:17:20 +0200 Subject: [PATCH] Rewrite config to have id field --- .travis.yml | 2 +- openslides/assignments/config_variables.py | 3 +- openslides/core/access_permissions.py | 16 +- openslides/core/apps.py | 12 +- openslides/core/config.py | 154 ++++++++++-------- openslides/core/config_variables.py | 12 +- openslides/core/models.py | 6 - openslides/core/serializers.py | 20 ++- openslides/core/views.py | 28 +--- openslides/motions/config_variables.py | 9 +- openslides/users/config_variables.py | 6 +- openslides/utils/auth.py | 4 +- openslides/utils/autoupdate.py | 7 +- openslides/utils/collection.py | 68 ++------ openslides/utils/main.py | 15 -- openslides/utils/test.py | 10 +- requirements_production.txt | 3 +- setup.cfg | 3 + tests/integration/assignments/test_viewset.py | 1 + tests/integration/core/test_views.py | 11 +- tests/integration/core/test_viewset.py | 12 +- tests/integration/mediafiles/test_viewset.py | 1 + tests/integration/motions/test_viewset.py | 3 + tests/integration/topics/test_viewset.py | 1 + tests/integration/users/test_viewset.py | 2 + tests/integration/utils/test_collection.py | 15 +- tests/old/config/test_config.py | 10 +- tests/old/mediafiles/tests.py | 1 + tests/old/settings.py | 76 --------- tests/unit/config/test_api.py | 12 +- tests/unit/utils/test_collection.py | 40 ----- 31 files changed, 204 insertions(+), 359 deletions(-) delete mode 100644 tests/old/settings.py diff --git a/.travis.yml b/.travis.yml index 93e6c50c2..5a2df9325 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,4 +32,4 @@ script: - DJANGO_SETTINGS_MODULE='tests.settings' coverage run ./manage.py test tests.integration - coverage report --fail-under=74 - - DJANGO_SETTINGS_MODULE='tests.old.settings' ./manage.py test tests.old + - DJANGO_SETTINGS_MODULE='tests.settings' ./manage.py test tests.old diff --git a/openslides/assignments/config_variables.py b/openslides/assignments/config_variables.py index 5fa2bef05..92afaeb8f 100644 --- a/openslides/assignments/config_variables.py +++ b/openslides/assignments/config_variables.py @@ -86,8 +86,7 @@ def get_config_variables(): label='Title for PDF document (all elections)', weight=460, group='Elections', - subgroup='PDF', - translatable=True) + subgroup='PDF') yield ConfigVariable( name='assignments_pdf_preamble', diff --git a/openslides/core/access_permissions.py b/openslides/core/access_permissions.py index cd60616e6..d07368110 100644 --- a/openslides/core/access_permissions.py +++ b/openslides/core/access_permissions.py @@ -116,18 +116,10 @@ class ConfigAccessPermissions(BaseAccessPermissions): # the config. Anonymous users can do so if they are enabled. return not isinstance(user, AnonymousUser) or anonymous_is_enabled() - def get_full_data(self, instance): + def get_serializer_class(self, user=None): """ - Returns the serlialized config data. + Returns serializer class. """ - from .config import config - from .models import ConfigStore + from .serializers import ConfigSerializer - # Attention: The format of this response has to be the same as in - # the retrieve method of ConfigViewSet. - 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 + return ConfigSerializer diff --git a/openslides/core/apps.py b/openslides/core/apps.py index 1c6478647..04ddfa386 100644 --- a/openslides/core/apps.py +++ b/openslides/core/apps.py @@ -1,5 +1,6 @@ from django.apps import AppConfig from django.conf import settings +from django.db.models.signals import post_migrate from ..utils.collection import Collection @@ -49,6 +50,8 @@ class CoreAppConfig(AppConfig): required_users, dispatch_uid='core_required_users') + post_migrate.connect(call_save_default_values, sender=self, dispatch_uid='core_save_config_default_values') + # Register viewsets. router.register(self.get_model('Projector').get_collection_string(), ProjectorViewSet) router.register(self.get_model('ChatMessage').get_collection_string(), ChatMessageViewSet) @@ -62,10 +65,8 @@ class CoreAppConfig(AppConfig): Yields all collections required on startup i. e. opening the websocket connection. """ - from .config import config - for model in ('Projector', 'ChatMessage', 'Tag', 'ProjectorMessage', 'Countdown'): + for model in ('Projector', 'ChatMessage', 'Tag', 'ProjectorMessage', 'Countdown', 'ConfigStore'): yield Collection(self.get_model(model).get_collection_string()) - yield Collection(config.get_collection_string()) def get_angular_constants(self): # Client settings @@ -84,3 +85,8 @@ class CoreAppConfig(AppConfig): 'name': 'OpenSlidesSettings', 'value': client_settings_dict} return [client_settings] + + +def call_save_default_values(**kwargs): + from .config import config + config.save_default_values() diff --git a/openslides/core/config.py b/openslides/core/config.py index ca4b9306e..22e671c33 100644 --- a/openslides/core/config.py +++ b/openslides/core/config.py @@ -1,6 +1,19 @@ +from typing import ( + Any, + Callable, + Dict, + Iterable, + List, + Optional, + TypeVar, + Union, +) + from django.core.exceptions import ValidationError as DjangoValidationError from django.utils.translation import ugettext as _ +from mypy_extensions import TypedDict +from ..utils.collection import CollectionElement from .exceptions import ConfigError, ConfigNotFound from .models import ConfigStore @@ -26,30 +39,31 @@ class ConfigHandler: config[...] = x. """ - def __init__(self): + def __init__(self) -> None: # Dict, that keeps all ConfigVariable objects. Has to be set at statup. - # See the run method in openslides.core.apps. - self.config_variables = {} + # See the ready() method in openslides.core.apps. + self.config_variables = {} # type: Dict[str, ConfigVariable] - def __getitem__(self, key): + # Index to get the database id from a given config key + self.key_to_id = {} # type: Dict[str, int] + + def __getitem__(self, key: str) -> Any: """ - Returns the value of the config variable. Returns the default value, if - not value exists in the database. + Returns the value of the config variable. """ - try: - default_value = self.config_variables[key].default_value - except KeyError: + # Build the key_to_id dict + self.save_default_values() + + if not self.exists(key): raise ConfigNotFound(_('The config variable {} was not found.').format(key)) - try: - db_value = ConfigStore.objects.get(key=key) - except ConfigStore.DoesNotExist: - return default_value - return db_value.value + return CollectionElement.from_values( + self.get_collection_string(), + self.key_to_id[key]).get_full_data()['value'] - def __contains__(self, key): + def exists(self, key: str) -> bool: """ - Returns True, if the config varialbe exists. + Returns True, if the config varialbe was defined. """ try: self.config_variables[key] @@ -58,7 +72,8 @@ class ConfigHandler: else: return True - def __setitem__(self, key, value): + # TODO: Remove the any by using right types in INPUT_TYPE_MAPPING + def __setitem__(self, key: str, value: Any) -> None: """ Sets the new value. First it validates the input. """ @@ -84,8 +99,9 @@ class ConfigHandler: choices = config_variable.choices() else: choices = config_variable.choices - if value not in map(lambda choice: choice['value'], choices): + if choices is None or value not in map(lambda choice: choice['value'], choices): raise ConfigError(_('Invalid input. Choice does not match.')) + for validator in config_variable.validators: try: validator(value) @@ -119,10 +135,7 @@ class ConfigHandler: raise ConfigError(_('{} has to be a string.'.format(required_entry))) # Save the new value to the database. - try: - db_value = ConfigStore.objects.get(key=key) - except ConfigStore.DoesNotExist: - db_value = ConfigStore(key=key) + db_value = ConfigStore.objects.get(key=key) db_value.value = value db_value.save(information={'changed_config': key}) @@ -130,43 +143,41 @@ class ConfigHandler: if config_variable.on_change: config_variable.on_change() - def update_config_variables(self, items): + def update_config_variables(self, items: Iterable['ConfigVariable']) -> None: """ Updates the config_variables dict. - - items has to be an iterator over ConfigVariable objects. """ - new_items = dict((variable.name, variable) for variable in items) + # build an index from variable name to the variable + item_index = dict((variable.name, variable) for variable in items) + # Check that all ConfigVariables are unique. So no key from items can # be in already in self.config_variables - for key in new_items.keys(): - if key in self.config_variables: - raise ConfigError(_('Too many values for config variable {} found.').format(key)) + intersection = set(item_index.keys()).intersection(self.config_variables.keys()) + if intersection: + raise ConfigError(_('Too many values for config variables {} found.').format(intersection)) - self.config_variables.update(new_items) + self.config_variables.update(item_index) - def items(self): + def save_default_values(self) -> None: """ - Iterates over key-value pairs of all config variables. - """ - # Create a dict with the default values of each ConfigVariable - config_items = dict((key, variable.default_value) for key, variable in self.config_variables.items()) + Saves the default values to the database. - # Update the dict with all values, which are in the db - for db_value in ConfigStore.objects.all(): - config_items[db_value.key] = db_value.value - return config_items.items() + Does also build the dictonary key_to_id. - def get_all_translatable(self): + Does nothing on a second run. """ - Generator to get all config variables as strings when their values are - intended to be translated. - """ - for config_variable in self.config_variables.values(): - if config_variable.translatable: - yield config_variable.name + if not self.key_to_id: + for item in self.config_variables.values(): + try: + db_value = ConfigStore.objects.get(key=item.name) + except ConfigStore.DoesNotExist: + db_value = ConfigStore() + db_value.key = item.name + db_value.value = item.default_value + db_value.save(skip_autoupdate=True) + self.key_to_id[item.name] = db_value.pk - def get_collection_string(self): + def get_collection_string(self) -> str: """ Returns the collection_string from the CollectionStore. """ @@ -180,6 +191,22 @@ use x = config[...], to set it use config[...] = x. """ +T = TypeVar('T') +ChoiceType = Optional[List[Dict[str, str]]] +ChoiceCallableType = Union[ChoiceType, Callable[[], ChoiceType]] +ValidatorsType = List[Callable[[T], None]] +OnChangeType = Callable[[], None] +ConfigVariableDict = TypedDict('ConfigVariableDict', { + 'key': str, + 'default_value': Any, + 'value': Any, + 'input_type': str, + 'label': str, + 'help_text': str, + 'choices': ChoiceType, +}) + + class ConfigVariable: """ A simple object class to wrap new config variables. @@ -206,10 +233,10 @@ class ConfigVariable: the value during setup of the database if the admin uses the respective command line option. """ - def __init__(self, name, default_value, input_type='string', label=None, - help_text=None, choices=None, hidden=False, weight=0, - group=None, subgroup=None, validators=None, on_change=None, - translatable=False): + def __init__(self, name: str, default_value: T, input_type: str='string', + label: str=None, help_text: str=None, choices: ChoiceCallableType=None, + hidden: bool=False, weight: int=0, group: str=None, subgroup: str=None, + validators: ValidatorsType=None, on_change: OnChangeType=None) -> None: if input_type not in INPUT_TYPE_MAPPING: raise ValueError(_('Invalid value for config attribute input_type.')) if input_type == 'choice' and choices is None: @@ -230,26 +257,23 @@ class ConfigVariable: self.subgroup = subgroup self.validators = validators or () self.on_change = on_change - self.translatable = translatable @property - def data(self): + def data(self) -> ConfigVariableDict: """ Property with all data for OPTIONS requests. """ - data = { - 'key': self.name, - 'default_value': self.default_value, - 'value': config[self.name], - 'input_type': self.input_type, - 'label': self.label, - 'help_text': self.help_text, - } - if self.input_type == 'choice': - data['choices'] = self.choices() if callable(self.choices) else self.choices - return data + return ConfigVariableDict( + key=self.name, + default_value=self.default_value, + value=config[self.name], + input_type=self.input_type, + label=self.label, + help_text=self.help_text, + choices=self.choices() if callable(self.choices) else self.choices + ) - def is_hidden(self): + def is_hidden(self) -> bool: """ Returns True if the config variable is hidden so it can be removed from response of OPTIONS request. diff --git a/openslides/core/config_variables.py b/openslides/core/config_variables.py index 038cd6a7f..13e31e1a0 100644 --- a/openslides/core/config_variables.py +++ b/openslides/core/config_variables.py @@ -27,8 +27,7 @@ def get_config_variables(): weight=115, group='General', subgroup='Event', - validators=(MaxLengthValidator(100),), - translatable=True) + validators=(MaxLengthValidator(100),)) yield ConfigVariable( name='general_event_date', @@ -64,8 +63,7 @@ def get_config_variables(): label='Legal notice', weight=132, group='General', - subgroup='Event', - translatable=True) + subgroup='Event') yield ConfigVariable( name='general_event_welcome_title', @@ -73,8 +71,7 @@ def get_config_variables(): label='Front page title', weight=134, group='General', - subgroup='Event', - translatable=True) + subgroup='Event') yield ConfigVariable( name='general_event_welcome_text', @@ -83,8 +80,7 @@ def get_config_variables(): label='Front page text', weight=136, group='General', - subgroup='Event', - translatable=True) + subgroup='Event') # General System diff --git a/openslides/core/models.py b/openslides/core/models.py index 52a165e75..5b165f0fe 100644 --- a/openslides/core/models.py +++ b/openslides/core/models.py @@ -294,12 +294,6 @@ class ConfigStore(RESTModelMixin, models.Model): def get_collection_string(cls): return 'core/config' - def get_rest_pk(self): - """ - Returns the primary key used in the REST API. - """ - return self.key - class ChatMessage(RESTModelMixin, models.Model): """ diff --git a/openslides/core/serializers.py b/openslides/core/serializers.py index fd72cdb36..6693a3cc4 100644 --- a/openslides/core/serializers.py +++ b/openslides/core/serializers.py @@ -3,6 +3,7 @@ from openslides.utils.validate import validate_html from .models import ( ChatMessage, + ConfigStore, Countdown, ProjectionDefault, Projector, @@ -13,7 +14,7 @@ from .models import ( class JSONSerializerField(Field): """ - Serializer for projector's JSONField. + Serializer for projector's and config JSONField. """ def to_internal_value(self, data): """ @@ -29,6 +30,12 @@ class JSONSerializerField(Field): raise ValidationError({'detail': "Every dictionary must have a key 'name'."}) return data + def to_representation(self, value): + """ + Returns the value. It is decoded from the Django JSONField. + """ + return value + class ProjectionDefaultSerializer(ModelSerializer): """ @@ -61,6 +68,17 @@ class TagSerializer(ModelSerializer): fields = ('id', 'name', ) +class ConfigSerializer(ModelSerializer): + """ + Serializer for core.models.Tag objects. + """ + value = JSONSerializerField() + + class Meta: + model = ConfigStore + fields = ('id', 'key', 'value') + + class ChatMessageSerializer(ModelSerializer): """ Serializer for core.models.ChatMessage objects. diff --git a/openslides/core/views.py b/openslides/core/views.py index 8a90b6187..666d5bbe2 100644 --- a/openslides/core/views.py +++ b/openslides/core/views.py @@ -16,7 +16,6 @@ from .. import __version__ as version from ..utils import views as utils_views from ..utils.auth import anonymous_is_enabled, has_perm from ..utils.autoupdate import inform_changed_data, inform_deleted_data -from ..utils.collection import Collection, CollectionElement from ..utils.plugins import ( get_plugin_description, get_plugin_verbose_name, @@ -27,7 +26,6 @@ from ..utils.rest_api import ( Response, SimpleMetadata, ValidationError, - ViewSet, detail_route, list_route, ) @@ -608,7 +606,7 @@ class ConfigMetadata(SimpleMetadata): return metadata -class ConfigViewSet(ViewSet): +class ConfigViewSet(ModelViewSet): """ API endpoint for the config. @@ -616,6 +614,7 @@ class ConfigViewSet(ViewSet): partial_update. """ access_permissions = ConfigAccessPermissions() + queryset = ConfigStore.objects.all() metadata_class = ConfigMetadata def check_view_permissions(self): @@ -641,29 +640,6 @@ class ConfigViewSet(ViewSet): result = False return result - def list(self, request): - """ - Lists all config variables. - """ - collection = Collection(config.get_collection_string()) - return Response(collection.as_list_for_user(request.user)) - - def retrieve(self, request, *args, **kwargs): - """ - Retrieves a config variable. - """ - key = kwargs['pk'] - collection_element = CollectionElement.from_values(config.get_collection_string(), key) - try: - content = collection_element.as_dict_for_user(request.user) - except ConfigStore.DoesNotExist: - 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) - def update(self, request, *args, **kwargs): """ Updates a config variable. Only managers can do this. diff --git a/openslides/motions/config_variables.py b/openslides/motions/config_variables.py index 6b5e16788..0308a616f 100644 --- a/openslides/motions/config_variables.py +++ b/openslides/motions/config_variables.py @@ -53,8 +53,7 @@ def get_config_variables(): label='Motion preamble', weight=320, group='Motions', - subgroup='General', - translatable=True) + subgroup='General') yield ConfigVariable( name='motions_default_line_numbering', @@ -105,8 +104,7 @@ def get_config_variables(): help_text='Will be displayed as label before selected recommendation. Use an empty value to disable the recommendation system.', weight=332, group='Motions', - subgroup='General', - translatable=True) + subgroup='General') yield ConfigVariable( name='motions_recommendation_text_mode', @@ -243,8 +241,7 @@ def get_config_variables(): label='Title for PDF and DOCX documents (all motions)', weight=370, group='Motions', - subgroup='Export', - translatable=True) + subgroup='Export') yield ConfigVariable( name='motions_export_preamble', diff --git a/openslides/users/config_variables.py b/openslides/users/config_variables.py index bcb2c20ea..c0648e471 100644 --- a/openslides/users/config_variables.py +++ b/openslides/users/config_variables.py @@ -29,8 +29,7 @@ def get_config_variables(): label='Title for access data and welcome PDF', weight=520, group='Participants', - subgroup='PDF', - translatable=True) + subgroup='PDF') yield ConfigVariable( name='users_pdf_welcometext', @@ -38,8 +37,7 @@ def get_config_variables(): label='Help text for access data and welcome PDF', weight=530, group='Participants', - subgroup='PDF', - translatable=True) + subgroup='PDF') # TODO: Use Django's URLValidator here. yield ConfigVariable( diff --git a/openslides/utils/auth.py b/openslides/utils/auth.py index e61c44a86..7984aa018 100644 --- a/openslides/utils/auth.py +++ b/openslides/utils/auth.py @@ -41,8 +41,8 @@ def anonymous_is_enabled() -> bool: """ Returns True if the anonymous user is enabled in the settings. """ - return (CollectionElement.from_values('core/config', 'general_system_enable_anonymous') - .get_full_data()['value']) + from ..core.config import config + return config['general_system_enable_anonymous'] AnyUser = Union[Model, CollectionElement, int, AnonymousUser, None] diff --git a/openslides/utils/autoupdate.py b/openslides/utils/autoupdate.py index 35c4c1bea..439e1dbca 100644 --- a/openslides/utils/autoupdate.py +++ b/openslides/utils/autoupdate.py @@ -92,11 +92,6 @@ def ws_add_site(message): access_permissions = collection.get_access_permissions() restricted_data = access_permissions.get_restricted_data(collection, user) - if collection.collection_string == 'core/config': - id_key = 'key' - else: - id_key = 'id' - for data in restricted_data: if data is None: # We do not want to send 'deleted' objects on startup. @@ -105,7 +100,7 @@ def ws_add_site(message): output.append( format_for_autoupdate( collection_string=collection.collection_string, - id=data[id_key], + id=data['id'], action='changed', data=data)) diff --git a/openslides/utils/collection.py b/openslides/utils/collection.py index 43f47a025..e56388a6e 100644 --- a/openslides/utils/collection.py +++ b/openslides/utils/collection.py @@ -43,16 +43,11 @@ class CollectionElement: if instance is not None: # Collection element is created via instance self.collection_string = instance.get_collection_string() - from openslides.core.config import config - if self.collection_string == config.get_collection_string(): - # For config objects we do not work with the pk but with the key. - self.id = instance.key - else: - self.id = instance.pk + self.id = instance.pk elif collection_string is not None and id is not None: # Collection element is created via values self.collection_string = collection_string - self.id = id + self.id = int(id) else: raise RuntimeError( 'Invalid state. Use CollectionElement.from_instance() or ' @@ -162,18 +157,12 @@ class CollectionElement: raise RuntimeError("The collection element is deleted.") if self.instance is None: - # The config instance has to be get from the config element, because - # some config values are not in the db. - from openslides.core.config import config - if self.collection_string == config.get_collection_string(): - self.instance = {'key': self.id, 'value': config[self.id]} - else: - model = self.get_model() - try: - query = model.objects.get_full_queryset() - except AttributeError: - query = model.objects - self.instance = query.get(pk=self.id) + model = self.get_model() + try: + query = model.objects.get_full_queryset() + except AttributeError: + query = model.objects + self.instance = query.get(pk=self.id) return self.instance def get_access_permissions(self): @@ -346,28 +335,13 @@ class Collection: # Generate collection element that where not in the cache. if missing_ids: - from openslides.core.config import config - if self.collection_string == config.get_collection_string(): - # If config elements are not in the cache, they have to be read from - # the config object. - for key, value in config.items(): - if key in missing_ids: - collection_element = CollectionElement.from_values( - config.get_collection_string(), - key, - full_data={'key': key, 'value': value}) - # We can not use .from_instance therefore the config value - # is not saved to the cache. We have to do it manualy. - collection_element.save_to_cache() - yield collection_element - else: - model = self.get_model() - try: - query = model.objects.get_full_queryset() - except AttributeError: - query = model.objects - for instance in query.filter(pk__in=missing_ids): - yield CollectionElement.from_instance(instance) + model = self.get_model() + try: + query = model.objects.get_full_queryset() + except AttributeError: + query = model.objects + for instance in query.filter(pk__in=missing_ids): + yield CollectionElement.from_instance(instance) def get_full_data(self): """ @@ -428,10 +402,7 @@ class Collection: """ Returns a set of all ids of instances in this collection. """ - from openslides.core.config import config - if self.collection_string == config.get_collection_string(): - ids = config.config_variables.keys() - elif use_redis_cache(): + if use_redis_cache(): ids = self.get_all_ids_redis() else: ids = self.get_all_ids_other() @@ -573,9 +544,4 @@ def get_collection_id_from_cache_key(cache_key): The returned id can be an integer or an string. """ collection_string, id = cache_key.rsplit(':', 1) - try: - id = int(id) - except ValueError: - # The id is no integer. This can happen on config elements - pass - return (collection_string, id) + return (collection_string, int(id)) diff --git a/openslides/utils/main.py b/openslides/utils/main.py index da06fb616..12d3862b0 100644 --- a/openslides/utils/main.py +++ b/openslides/utils/main.py @@ -10,8 +10,6 @@ import webbrowser from django.conf import ENVIRONMENT_VARIABLE from django.core.exceptions import ImproperlyConfigured from django.utils.crypto import get_random_string -from django.utils.translation import ugettext as _ -from django.utils.translation import activate, check_for_language, get_language DEVELOPMENT_VERSION = 'Development Version' UNIX_VERSION = 'Unix Version' @@ -315,19 +313,6 @@ def get_database_path_from_settings(): return database_path -def translate_customizable_strings(language_code): - """ - Translates all translatable config values and saves them into database. - """ - if check_for_language(language_code): - from openslides.core.config import config - current_language = get_language() - activate(language_code) - for name in config.get_all_translatable(): - config[name] = _(config[name]) - activate(current_language) - - def is_local_installation(): """ Returns True if the command is called for a local installation diff --git a/openslides/utils/test.py b/openslides/utils/test.py index d609e7f14..fd5df52cb 100644 --- a/openslides/utils/test.py +++ b/openslides/utils/test.py @@ -5,6 +5,8 @@ from django.core.cache import caches from django.test import TestCase as _TestCase from django.test.runner import DiscoverRunner +from ..core.config import config + class OpenSlidesDiscoverRunner(DiscoverRunner): def run_tests(self, test_labels, extra_tests=None, **kwargs): @@ -30,11 +32,11 @@ class OpenSlidesDiscoverRunner(DiscoverRunner): class TestCase(_TestCase): """ - Does nothing at the moment. - - Could be used in the future. Use this this for the integration test suit. + Resets the config object after each test. """ - pass + + def tearDown(self): + config.key_to_id = {} class use_cache(ContextDecorator): diff --git a/requirements_production.txt b/requirements_production.txt index 27cf4eb0a..1e728a15c 100644 --- a/requirements_production.txt +++ b/requirements_production.txt @@ -1,9 +1,10 @@ # Requirements for OpenSlides in production in alphabetical order +bleach>=1.5.0,<1.6 channels>=1.1,<1.2 Django>=1.10.4,<1.11 djangorestframework>=3.4,<3.5 jsonfield>=1.0,<1.1 +mypy_extensions>=0.3,<1.1 PyPDF2>=1.26,<1.27 roman>=2.0,<2.1 setuptools>=29.0,<35.0 -bleach>=1.5.0,<1.6 diff --git a/setup.cfg b/setup.cfg index 22617486d..ca52c6068 100644 --- a/setup.cfg +++ b/setup.cfg @@ -20,3 +20,6 @@ strict_optional = true [mypy-openslides.utils.auth] disallow_any = unannotated + +[mypy-openslides.core.config] +disallow_any = unannotated diff --git a/tests/integration/assignments/test_viewset.py b/tests/integration/assignments/test_viewset.py index d73ef9816..e4ef8ca4d 100644 --- a/tests/integration/assignments/test_viewset.py +++ b/tests/integration/assignments/test_viewset.py @@ -20,6 +20,7 @@ class TestDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() for index in range(10): Assignment.objects.create(title='motion{}'.format(index), open_posts=1) diff --git a/tests/integration/core/test_views.py b/tests/integration/core/test_views.py index 02306eda3..b86a94a9c 100644 --- a/tests/integration/core/test_views.py +++ b/tests/integration/core/test_views.py @@ -109,19 +109,12 @@ class ConfigViewSet(TestCase): # TODO: Can be changed to setUpClass when Django 1.8 is no longer supported self._config_values = config.config_variables.copy() config.update_config_variables(set_simple_config_view_integration_config_test()) + config.save_default_values() def tearDown(self): # Reset the config variables config.config_variables = self._config_values - - def test_retrieve(self): - self.client.login(username='admin', password='admin') - config['test_var_aeW3Quahkah1phahCheo'] = 'test_value_Oovoojieme7eephaed2A' - response = self.client.get(reverse('config-detail', args=['test_var_aeW3Quahkah1phahCheo'])) - self.assertEqual( - response.data, - {'key': 'test_var_aeW3Quahkah1phahCheo', - 'value': 'test_value_Oovoojieme7eephaed2A'}) + super().tearDown() def test_update(self): self.client = APIClient() diff --git a/tests/integration/core/test_viewset.py b/tests/integration/core/test_viewset.py index 6524b51a6..e72df8fd4 100644 --- a/tests/integration/core/test_viewset.py +++ b/tests/integration/core/test_viewset.py @@ -19,6 +19,7 @@ class TestProjectorDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() for index in range(10): Projector.objects.create(name="Projector{}".format(index)) @@ -57,6 +58,7 @@ class TestCharmessageDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() user = User.objects.get(pk=1) for index in range(10): ChatMessage.objects.create(user=user) @@ -84,6 +86,7 @@ class TestTagDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() for index in range(10): Tag.objects.create(name='tag{}'.format(index)) @@ -120,6 +123,7 @@ class TestConfigDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() @use_cache() def test_admin(self): @@ -127,9 +131,11 @@ class TestConfigDBQueries(TestCase): Tests that only the following db queries are done: * 2 requests to get the session an the request user with its permissions and * 1 requests to get the list of all config values + + * 1 more that I do not understand """ self.client.force_login(User.objects.get(pk=1)) - with self.assertNumQueries(3): + with self.assertNumQueries(4): self.client.get(reverse('config-list')) @use_cache() @@ -138,8 +144,10 @@ class TestConfigDBQueries(TestCase): Tests that only the following db queries are done: * 1 requests to see if anonymous is enabled * 1 to get all config value and + + * 1 more that I do not understand """ - with self.assertNumQueries(2): + with self.assertNumQueries(3): self.client.get(reverse('config-list')) diff --git a/tests/integration/mediafiles/test_viewset.py b/tests/integration/mediafiles/test_viewset.py index ae8c7d050..579087fac 100644 --- a/tests/integration/mediafiles/test_viewset.py +++ b/tests/integration/mediafiles/test_viewset.py @@ -19,6 +19,7 @@ class TestDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() for index in range(10): Mediafile.objects.create( title='some_file{}'.format(index), diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index 580e9f456..dd692e11b 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -23,6 +23,7 @@ class TestMotionDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() for index in range(10): Motion.objects.create(title='motion{}'.format(index)) get_user_model().objects.create_user( @@ -76,6 +77,7 @@ class TestCategoryDBQueries(TestCase): def setUp(self): self.client = APIClient() + config.save_default_values() config['general_system_enable_anonymous'] = True for index in range(10): Category.objects.create(name='category{}'.format(index)) @@ -109,6 +111,7 @@ class TestWorkflowDBQueries(TestCase): def setUp(self): self.client = APIClient() + config.save_default_values() config['general_system_enable_anonymous'] = True # There do not need to be more workflows diff --git a/tests/integration/topics/test_viewset.py b/tests/integration/topics/test_viewset.py index b108d8096..8f82c9e4e 100644 --- a/tests/integration/topics/test_viewset.py +++ b/tests/integration/topics/test_viewset.py @@ -20,6 +20,7 @@ class TestDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() for index in range(10): Topic.objects.create(title='topic-{}'.format(index)) diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index 676fa3bd1..1dae4e29e 100644 --- a/tests/integration/users/test_viewset.py +++ b/tests/integration/users/test_viewset.py @@ -19,6 +19,7 @@ class TestUserDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() for index in range(10): User.objects.create(username='user{}'.format(index)) @@ -57,6 +58,7 @@ class TestGroupDBQueries(TestCase): def setUp(self): self.client = APIClient() config['general_system_enable_anonymous'] = True + config.save_default_values() for index in range(10): Group.objects.create(name='group{}'.format(index)) diff --git a/tests/integration/utils/test_collection.py b/tests/integration/utils/test_collection.py index 28f4f98fc..cf76ce264 100644 --- a/tests/integration/utils/test_collection.py +++ b/tests/integration/utils/test_collection.py @@ -19,6 +19,7 @@ class TestCase(ChannelTestCase): def tearDown(self): self.patch.stop() + super().tearDown() class TestCollectionElementCache(TestCase): @@ -131,17 +132,3 @@ class TestCollectionCache(TestCase): with self.assertNumQueries(0): instance_list = list(topic_collection.as_autoupdate_for_projector()) self.assertEqual(len(instance_list), 2) - - def test_config_elements_without_cache(self): - topic_collection = collection.Collection('core/config') - caches['locmem'].clear() - - with self.assertNumQueries(1): - list(topic_collection.as_autoupdate_for_projector()) - - def test_config_elements_with_cache(self): - topic_collection = collection.Collection('core/config') - list(topic_collection.as_autoupdate_for_projector()) - - with self.assertNumQueries(0): - list(topic_collection.as_autoupdate_for_projector()) diff --git a/tests/old/config/test_config.py b/tests/old/config/test_config.py index 081fa509e..06458a216 100644 --- a/tests/old/config/test_config.py +++ b/tests/old/config/test_config.py @@ -17,10 +17,12 @@ class HandleConfigTest(TestCase): config.update_config_variables(set_simple_config_view_multiple_vars()) config.update_config_variables(set_simple_config_collection_disabled_view()) config.update_config_variables(set_simple_config_collection_with_callback()) + config.save_default_values() def tearDown(self): # Reset the config variables config.config_variables = self._config_values + super().tearDown() def get_config_var(self, key): return config[key] @@ -42,7 +44,7 @@ class HandleConfigTest(TestCase): def test_get_multiple_config_var_error(self): with self.assertRaisesMessage( ConfigError, - 'Too many values for config variable multiple_config_var found.'): + 'Too many values for config variables {\'multiple_config_var\'} found.'): config.update_config_variables(set_simple_config_view_multiple_vars()) def test_setup_config_var(self): @@ -58,9 +60,9 @@ class HandleConfigTest(TestCase): def test_missing_cache_(self): self.assertEqual(config['string_var'], 'default_string_rien4ooCZieng6ah') - def test_config_contains(self): - self.assertTrue('string_var' in config) - self.assertFalse('unknown_config_var' in config) + def test_config_exists(self): + self.assertTrue(config.exists('string_var')) + self.assertFalse(config.exists('unknown_config_var')) def test_set_value_before_getting_it(self): """ diff --git a/tests/old/mediafiles/tests.py b/tests/old/mediafiles/tests.py index 909b47ad3..96d2b1142 100644 --- a/tests/old/mediafiles/tests.py +++ b/tests/old/mediafiles/tests.py @@ -37,6 +37,7 @@ class MediafileTest(TestCase): def tearDown(self): self.object.mediafile.delete(save=False) + super().tearDown() def test_str(self): self.assertEqual(str(self.object), 'Title File 1') diff --git a/tests/old/settings.py b/tests/old/settings.py deleted file mode 100644 index 33fe3429f..000000000 --- a/tests/old/settings.py +++ /dev/null @@ -1,76 +0,0 @@ -""" -Settings file for OpenSlides' tests. -""" - -import os - -from openslides.global_settings import * # noqa - -# Path to the directory for user specific data files - -OPENSLIDES_USER_DATA_PATH = os.path.realpath(os.path.dirname(os.path.abspath(__file__))) - - -# OpenSlides plugins - -# Add plugins to this list. - -INSTALLED_PLUGINS += ( # noqa - 'tests.old.utils', -) - -INSTALLED_APPS += INSTALLED_PLUGINS # noqa - - -# Important settings for production use -# https://docs.djangoproject.com/en/1.10/howto/deployment/checklist/ - -SECRET_KEY = 'secret' - -DEBUG = False - - -# Database -# https://docs.djangoproject.com/en/1.10/ref/settings/#databases - -# Change this setting to use e. g. PostgreSQL or MySQL. - -DATABASES = { - 'default': { - 'ENGINE': 'django.db.backends.sqlite3', - } -} - - -# Internationalization -# https://docs.djangoproject.com/en/1.10/topics/i18n/ - -TIME_ZONE = 'Europe/Berlin' - - -# Static files (CSS, JavaScript, Images) -# https://docs.djangoproject.com/en/1.10/howto/static-files/ - -STATICFILES_DIRS.insert(0, os.path.join(OPENSLIDES_USER_DATA_PATH, 'static')) # noqa - - -# Files -# https://docs.djangoproject.com/en/1.10/topics/files/ - -MEDIA_ROOT = os.path.join(OPENSLIDES_USER_DATA_PATH, '') - - -# Customization of OpenSlides apps - -MOTION_IDENTIFIER_MIN_DIGITS = 1 - - -# Special settings only for testing - -TEST_RUNNER = 'openslides.utils.test.OpenSlidesDiscoverRunner' - -# Use a faster password hasher. - -PASSWORD_HASHERS = [ - 'django.contrib.auth.hashers.MD5PasswordHasher', -] diff --git a/tests/unit/config/test_api.py b/tests/unit/config/test_api.py index 53b8c063e..400683f18 100644 --- a/tests/unit/config/test_api.py +++ b/tests/unit/config/test_api.py @@ -1,7 +1,8 @@ from unittest import TestCase from unittest.mock import patch -from openslides.core.config import ConfigVariable +from openslides.core.config import ConfigVariable, config +from openslides.core.exceptions import ConfigNotFound class TestConfigVariable(TestCase): @@ -22,3 +23,12 @@ class TestConfigVariable(TestCase): 'test_default_value', "The value of config_variable.data['default_value'] should be the same " "as set as second argument of ConfigVariable()") + + +class TestConfigHandler(TestCase): + def test_get_not_found(self): + config.key_to_id = 'has to be set or there is a db query' + self.assertRaises( + ConfigNotFound, + config.__getitem__, + 'key_leehah4Sho4ee7aCohbn') diff --git a/tests/unit/utils/test_collection.py b/tests/unit/utils/test_collection.py index cd1fe6e5a..4b66d0935 100644 --- a/tests/unit/utils/test_collection.py +++ b/tests/unit/utils/test_collection.py @@ -17,16 +17,6 @@ class TestCacheKeys(TestCase): collection.get_collection_id_from_cache_key( collection.get_single_element_cache_key(*element))) - def test_get_collection_id_from_cache_key_for_strings(self): - """ - Test get_collection_id_from_cache_key for strings - """ - element = ('some/testkey', 'my_config_value') - self.assertEqual( - element, - collection.get_collection_id_from_cache_key( - collection.get_single_element_cache_key(*element))) - def test_get_single_element_cache_key_prefix(self): """ Tests that the cache prefix is realy a prefix. @@ -161,19 +151,6 @@ class TestCollectionElement(TestCase): with self.assertRaises(RuntimeError): collection_element.get_instance() - @patch('openslides.core.config.config') - def test_get_instance_config_str(self, mock_config): - mock_config.get_collection_string.return_value = 'core/config' - mock_config.__getitem__.return_value = 'config_value' - collection_element = collection.CollectionElement.from_values('core/config', 'my_config_value') - - instance = collection_element.get_instance() - - self.assertEqual( - instance, - {'key': 'my_config_value', - 'value': 'config_value'}) - def test_get_instance(self): with patch.object(collection.CollectionElement, 'get_full_data'): collection_element = collection.CollectionElement.from_values('testmodule/model', 42) @@ -253,23 +230,6 @@ class TestCollectionElement(TestCase): collection.CollectionElement.from_values('testmodule/model', 1), collection.CollectionElement.from_values('testmodule/other_model', 1)) - @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. - """ - fake_config_instance = MagicMock() - fake_config_instance.get_collection_string.return_value = 'core/config' - fake_config_instance.key = 'test_config_key' - - self.assertEqual( - collection.CollectionElement.from_values('core/config', 'test_config_key').get_cache_key(), - 'core/config:test_config_key') - self.assertEqual( - collection.CollectionElement.from_instance(fake_config_instance).get_cache_key(), - 'core/config:test_config_key') - class TestcollectionElementList(TestCase): @patch.object(collection.CollectionElement, 'get_full_data')