From c405b4b323de33b2cbe380cc904382a69c7bc892 Mon Sep 17 00:00:00 2001 From: Oskar Hahn Date: Sun, 28 Oct 2018 10:04:52 +0100 Subject: [PATCH] Use Protocol instead of ABC in cache_provicer --- .travis.yml | 4 +- openslides/utils/cache.py | 4 +- openslides/utils/cache_providers.py | 96 +++++++------------ openslides/utils/collection.py | 3 +- requirements/production.txt | 1 + .../commands/create-example-data.py | 5 +- tests/integration/helpers.py | 19 +++- tests/unit/users/test_access_permissions.py | 2 +- tests/unit/users/test_serializers.py | 2 +- tests/unit/utils/cache_provider.py | 4 +- 10 files changed, 65 insertions(+), 75 deletions(-) diff --git a/.travis.yml b/.travis.yml index 74cd05e18..fab09b228 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,7 @@ matrix: script: - flake8 openslides tests - isort --check-only --diff --recursive openslides tests - - python -m mypy openslides/ + - python -m mypy openslides/ tests/ - python -W ignore -m pytest --cov --cov-fail-under=70 - language: python @@ -34,7 +34,7 @@ matrix: script: - flake8 openslides tests - isort --check-only --diff --recursive openslides tests - - python -m mypy openslides/ + - python -m mypy openslides/ tests/ - python -W ignore -m pytest --cov --cov-fail-under=70 - language: node_js diff --git a/openslides/utils/cache.py b/openslides/utils/cache.py index 7fd4124ad..46f4df3cf 100644 --- a/openslides/utils/cache.py +++ b/openslides/utils/cache.py @@ -18,8 +18,8 @@ from asgiref.sync import async_to_sync, sync_to_async from django.conf import settings from .cache_providers import ( - BaseCacheProvider, Cachable, + ElementCacheProvider, MemmoryCacheProvider, RedisCacheProvider, get_all_cachables, @@ -62,7 +62,7 @@ class ElementCache: self, redis: str, use_restricted_data_cache: bool = False, - cache_provider_class: Type[BaseCacheProvider] = RedisCacheProvider, + cache_provider_class: Type[ElementCacheProvider] = RedisCacheProvider, cachable_provider: Callable[[], List[Cachable]] = get_all_cachables, start_time: int = None) -> None: """ diff --git a/openslides/utils/cache_providers.py b/openslides/utils/cache_providers.py index c82a60675..f5962bd55 100644 --- a/openslides/utils/cache_providers.py +++ b/openslides/utils/cache_providers.py @@ -11,6 +11,7 @@ from typing import ( ) from django.apps import apps +from typing_extensions import Protocol from .utils import split_element_id, str_dict_to_bytes @@ -27,83 +28,52 @@ else: no_redis_dependency = False -class BaseCacheProvider: +class ElementCacheProvider(Protocol): """ Base class for cache provider. See RedisCacheProvider as reverence implementation. """ - full_data_cache_key = 'full_data' - restricted_user_cache_key = 'restricted_data:{user_id}' - change_id_cache_key = 'change_id' - prefix = 'element_cache_' - def __init__(self, *args: Any) -> None: - pass + def __init__(self, *args: Any) -> None: ... - def get_full_data_cache_key(self) -> str: - return "".join((self.prefix, self.full_data_cache_key)) + async def clear_cache(self) -> None: ... - def get_restricted_data_cache_key(self, user_id: int) -> str: - return "".join((self.prefix, self.restricted_user_cache_key.format(user_id=user_id))) + async def reset_full_cache(self, data: Dict[str, str]) -> None: ... - def get_change_id_cache_key(self) -> str: - return "".join((self.prefix, self.change_id_cache_key)) + async def data_exists(self, user_id: Optional[int] = None) -> bool: ... - async def clear_cache(self) -> None: - raise NotImplementedError("CacheProvider has to implement the method clear_cache().") + async def add_elements(self, elements: List[str]) -> None: ... - async def reset_full_cache(self, data: Dict[str, str]) -> None: - raise NotImplementedError("CacheProvider has to implement the method reset_full_cache().") + async def del_elements(self, elements: List[str], user_id: Optional[int] = None) -> None: ... - async def data_exists(self, user_id: Optional[int] = None) -> bool: - raise NotImplementedError("CacheProvider has to implement the method exists_full_data().") + async def add_changed_elements(self, default_change_id: int, element_ids: Iterable[str]) -> int: ... - async def add_elements(self, elements: List[str]) -> None: - raise NotImplementedError("CacheProvider has to implement the method add_elements().") - - async def del_elements(self, elements: List[str], user_id: Optional[int] = None) -> None: - raise NotImplementedError("CacheProvider has to implement the method del_elements().") - - async def add_changed_elements(self, default_change_id: int, element_ids: Iterable[str]) -> int: - raise NotImplementedError("CacheProvider has to implement the method add_changed_elements().") - - async def get_all_data(self, user_id: Optional[int] = None) -> Dict[bytes, bytes]: - raise NotImplementedError("CacheProvider has to implement the method get_all_data().") + async def get_all_data(self, user_id: Optional[int] = None) -> Dict[bytes, bytes]: ... async def get_data_since( self, change_id: int, user_id: Optional[int] = None, - max_change_id: int = -1) -> Tuple[Dict[str, List[bytes]], List[str]]: - raise NotImplementedError("CacheProvider has to implement the method get_data_since().") + max_change_id: int = -1) -> Tuple[Dict[str, List[bytes]], List[str]]: ... - async def get_element(self, element_id: str) -> Optional[bytes]: - raise NotImplementedError("CacheProvider has to implement the method get_element().") + async def get_element(self, element_id: str) -> Optional[bytes]: ... - async def del_restricted_data(self, user_id: int) -> None: - raise NotImplementedError("CacheProvider has to implement the method del_restricted_data().") + async def del_restricted_data(self, user_id: int) -> None: ... - async def set_lock(self, lock_name: str) -> bool: - raise NotImplementedError("CacheProvider has to implement the method set_lock().") + async def set_lock(self, lock_name: str) -> bool: ... - async def get_lock(self, lock_name: str) -> bool: - raise NotImplementedError("CacheProvider has to implement the method get_lock().") + async def get_lock(self, lock_name: str) -> bool: ... - async def del_lock(self, lock_name: str) -> None: - raise NotImplementedError("CacheProvider has to implement the method del_lock().") + async def del_lock(self, lock_name: str) -> None: ... - async def get_change_id_user(self, user_id: int) -> Optional[int]: - raise NotImplementedError("CacheProvider has to implement the method get_change_id_user().") + async def get_change_id_user(self, user_id: int) -> Optional[int]: ... - async def update_restricted_data(self, user_id: int, data: Dict[str, str]) -> None: - raise NotImplementedError("CacheProvider has to implement the method update_restricted_data().") + async def update_restricted_data(self, user_id: int, data: Dict[str, str]) -> None: ... - async def get_current_change_id(self) -> List[Tuple[str, int]]: - raise NotImplementedError("CacheProvider has to implement the method get_current_change_id().") + async def get_current_change_id(self) -> List[Tuple[str, int]]: ... - async def get_lowest_change_id(self) -> Optional[int]: - raise NotImplementedError("CacheProvider has to implement the method get_lowest_change_id().") + async def get_lowest_change_id(self) -> Optional[int]: ... class RedisConnectionContextManager: @@ -123,11 +93,15 @@ class RedisConnectionContextManager: self.conn.close() -class RedisCacheProvider(BaseCacheProvider): +class RedisCacheProvider: """ Cache provider that loads and saves the data to redis. """ redis_pool: Optional[aioredis.RedisConnection] = None + full_data_cache_key: str = 'full_data' + restricted_user_cache_key: str = 'restricted_data:{user_id}' + change_id_cache_key: str = 'change_id' + prefix: str = 'element_cache_' def __init__(self, redis: str) -> None: self.redis_address = redis @@ -138,6 +112,15 @@ class RedisCacheProvider(BaseCacheProvider): """ return RedisConnectionContextManager(self.redis_address) + def get_full_data_cache_key(self) -> str: + return "".join((self.prefix, self.full_data_cache_key)) + + def get_restricted_data_cache_key(self, user_id: int) -> str: + return "".join((self.prefix, self.restricted_user_cache_key.format(user_id=user_id))) + + def get_change_id_cache_key(self) -> str: + return "".join((self.prefix, self.change_id_cache_key)) + async def clear_cache(self) -> None: """ Deleted all cache entries created with this element cache. @@ -371,7 +354,7 @@ class RedisCacheProvider(BaseCacheProvider): '_config:lowest_change_id') -class MemmoryCacheProvider(BaseCacheProvider): +class MemmoryCacheProvider: """ CacheProvider for the ElementCache that uses only the memory. @@ -516,7 +499,7 @@ class MemmoryCacheProvider(BaseCacheProvider): return None -class Cachable: +class Cachable(Protocol): """ A Cachable is an object that returns elements that can be cached. @@ -527,13 +510,11 @@ class Cachable: """ Returns the string representing the name of the cachable. """ - raise NotImplementedError("Cachable has to implement the method get_collection_string().") def get_elements(self) -> List[Dict[str, Any]]: """ Returns all elements of the cachable. """ - raise NotImplementedError("Cachable has to implement the method get_collection_string().") def restrict_elements( self, @@ -544,10 +525,7 @@ class Cachable: elements can be an empty list, a list with some elements of the cachable or with all elements of the cachable. - - The default implementation returns the full_data. """ - return elements def get_all_cachables() -> List[Cachable]: @@ -558,7 +536,7 @@ def get_all_cachables() -> List[Cachable]: for app in apps.get_app_configs(): try: # Get the method get_startup_elements() from an app. - # This method has to return an iterable of Collection objects. + # This method has to return an iterable of Cachable objects. get_startup_elements = app.get_startup_elements except AttributeError: # Skip apps that do not implement get_startup_elements. diff --git a/openslides/utils/collection.py b/openslides/utils/collection.py index 8d329cc9d..362ecbc1d 100644 --- a/openslides/utils/collection.py +++ b/openslides/utils/collection.py @@ -16,7 +16,6 @@ from django.db.models import Model from mypy_extensions import TypedDict from .cache import element_cache -from .cache_providers import Cachable if TYPE_CHECKING: @@ -211,7 +210,7 @@ class CollectionElement: return self.deleted -class Collection(Cachable): +class Collection: """ Represents all elements of one collection. """ diff --git a/requirements/production.txt b/requirements/production.txt index 92e44734d..956983e3a 100644 --- a/requirements/production.txt +++ b/requirements/production.txt @@ -10,3 +10,4 @@ mypy_extensions>=0.4,<0.5 PyPDF2>=1.26,<1.27 roman>=2.0,<3.1 setuptools>=29.0,<41.0 +typing_extensions>=3.6.6,<3.7 diff --git a/tests/example_data_generator/management/commands/create-example-data.py b/tests/example_data_generator/management/commands/create-example-data.py index 060b765cd..837c316ba 100644 --- a/tests/example_data_generator/management/commands/create-example-data.py +++ b/tests/example_data_generator/management/commands/create-example-data.py @@ -1,4 +1,5 @@ from textwrap import dedent +from typing import Optional from django.contrib.auth.hashers import make_password from django.core.management.base import BaseCommand, CommandError @@ -183,7 +184,7 @@ class Command(BaseCommand): @transaction.atomic def create_staff_users(self, options): if options['users'] is None and not options['only']: - staff_users = DEFAULT_NUMBER + staff_users: Optional[int] = DEFAULT_NUMBER elif options['users'] is None: staff_users = None else: @@ -214,7 +215,7 @@ class Command(BaseCommand): @transaction.atomic def create_default_users(self, options): if options['users'] is None and not options['only']: - default_users = DEFAULT_NUMBER + default_users: Optional[int] = DEFAULT_NUMBER elif options['users'] is None: default_users = None else: diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 2edfb4567..439decd29 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, List +from typing import Any, Dict, List, Optional from asgiref.sync import sync_to_async from django.db import DEFAULT_DB_ALIAS, connections @@ -8,11 +8,10 @@ from openslides.core.config import config from openslides.users.models import User from openslides.utils.autoupdate import inform_data_collection_element_list from openslides.utils.cache import element_cache, get_element_id -from openslides.utils.cache_providers import Cachable from openslides.utils.collection import CollectionElement -class TConfig(Cachable): +class TConfig: """ Cachable, that fills the cache with the default values of the config variables. """ @@ -28,8 +27,14 @@ class TConfig(Cachable): config.key_to_id[item.name] = id+1 return elements + def restrict_elements( + self, + user: Optional['CollectionElement'], + elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + return elements -class TUser(Cachable): + +class TUser: """ Cachable, that fills the cache with the default values of the config variables. """ @@ -45,6 +50,12 @@ class TUser(Cachable): 'last_email_send': None, 'comment': '', 'is_active': True, 'default_password': 'admin', 'session_auth_hash': '362d4f2de1463293cb3aaba7727c967c35de43ee'}] + def restrict_elements( + self, + user: Optional['CollectionElement'], + elements: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + return elements + async def set_config(key, value): """ diff --git a/tests/unit/users/test_access_permissions.py b/tests/unit/users/test_access_permissions.py index 4c7d63bff..c96ed1f7e 100644 --- a/tests/unit/users/test_access_permissions.py +++ b/tests/unit/users/test_access_permissions.py @@ -57,6 +57,6 @@ class TestPersonalNoteAccessPermissions(TestCase): [CollectionElement.from_values( 'users/personal_note', 1, - full_data={'user_id': 1})], + full_data={'user_id': 1}).get_full_data()], None) self.assertEqual(rd, []) diff --git a/tests/unit/users/test_serializers.py b/tests/unit/users/test_serializers.py index 9a9857f23..1b2655a6e 100644 --- a/tests/unit/users/test_serializers.py +++ b/tests/unit/users/test_serializers.py @@ -11,7 +11,7 @@ class UserCreateUpdateSerializerTest(TestCase): Tests, that the validator raises a ValidationError, if not data is given. """ serializer = UserFullSerializer() - data = {} + data: object = {} with self.assertRaises(ValidationError): serializer.validate(data) diff --git a/tests/unit/utils/cache_provider.py b/tests/unit/utils/cache_provider.py index 4530d042a..4c0ead6fd 100644 --- a/tests/unit/utils/cache_provider.py +++ b/tests/unit/utils/cache_provider.py @@ -23,7 +23,7 @@ def restrict_elements( return out -class Collection1(Cachable): +class Collection1: def get_collection_string(self) -> str: return 'app/collection1' @@ -36,7 +36,7 @@ class Collection1(Cachable): return restrict_elements(user, elements) -class Collection2(Cachable): +class Collection2: def get_collection_string(self) -> str: return 'app/collection2'