Merge pull request #3956 from ostcar/use_typing_protocol

Use Protocol instead of ABC in cache_provicer

.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

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:
         """

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.

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.
     """

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

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:

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):
     """

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, [])

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)

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'