Disable the future-lock when updating the restircted data cache

Before this commit, there where two different locks when updating the restricted
data cache. A future lock, what is faster but only works in the same thread. The
other lock is in redis, it is not so fast, but also works in many threads.

The future lock was buggy, because on a second call of update_restricted_data
the same future was reused. So on the second run, the future was already done.

I don't see any way to delete. The last client would have to delete it, but there
is no way to find out which client the last one is.

openslides/agenda/models.py

@@ -15,8 +15,8 @@ from openslides.utils.exceptions import OpenSlidesError
 from openslides.utils.models import RESTModelMixin
 from openslides.utils.utils import to_roman
 
-from ..utils.models import CASCADE_AND_AUTOUODATE, SET_NULL_AND_AUTOUPDATE
 from .access_permissions import ItemAccessPermissions
+from ..utils.models import CASCADE_AND_AUTOUODATE, SET_NULL_AND_AUTOUPDATE
 
 
 class ItemManager(models.Manager):

openslides/agenda/signals.py

@@ -1,8 +1,8 @@
 from django.apps import apps
 from django.contrib.contenttypes.models import ContentType
 
-from ..utils.autoupdate import inform_changed_data
 from .models import Item
+from ..utils.autoupdate import inform_changed_data
 
 
 def listen_to_related_object_post_save(sender, instance, created, **kwargs):

openslides/agenda/views.py

@@ -16,9 +16,9 @@ from openslides.utils.rest_api import (
     list_route,
 )
 
-from ..utils.auth import has_perm
 from .access_permissions import ItemAccessPermissions
 from .models import Item, Speaker
+from ..utils.auth import has_perm
 
 
 # Viewsets for the REST API

openslides/assignments/models.py

@@ -21,8 +21,8 @@ from openslides.utils.autoupdate import inform_changed_data
 from openslides.utils.exceptions import OpenSlidesError
 from openslides.utils.models import RESTModelMixin
 
-from ..utils.models import CASCADE_AND_AUTOUODATE, SET_NULL_AND_AUTOUPDATE
 from .access_permissions import AssignmentAccessPermissions
+from ..utils.models import CASCADE_AND_AUTOUODATE, SET_NULL_AND_AUTOUPDATE
 
 
 class AssignmentRelatedUser(RESTModelMixin, models.Model):

openslides/assignments/serializers.py

@@ -12,7 +12,6 @@ from openslides.utils.rest_api import (
     ValidationError,
 )
 
-from ..utils.validate import validate_html
 from .models import (
     Assignment,
     AssignmentOption,
@@ -21,6 +20,7 @@ from .models import (
     AssignmentVote,
     models,
 )
+from ..utils.validate import validate_html
 
 
 def posts_validator(data):

openslides/assignments/views.py

@@ -12,10 +12,10 @@ from openslides.utils.rest_api import (
     detail_route,
 )
 
-from ..utils.auth import has_perm
 from .access_permissions import AssignmentAccessPermissions
 from .models import Assignment, AssignmentPoll, AssignmentRelatedUser
 from .serializers import AssignmentAllPollSerializer
+from ..utils.auth import has_perm
 
 
 # Viewsets for the REST API

openslides/core/config.py

@@ -5,9 +5,9 @@ from django.apps import apps
 from django.core.exceptions import ValidationError as DjangoValidationError
 from mypy_extensions import TypedDict
 
-from ..utils.cache import element_cache
 from .exceptions import ConfigError, ConfigNotFound
 from .models import ConfigStore
+from ..utils.cache import element_cache
 
 
 INPUT_TYPE_MAPPING = {

openslides/core/models.py

@@ -4,13 +4,6 @@ from django.db import models, transaction
 from django.utils.timezone import now
 from jsonfield import JSONField
 
-from ..utils.autoupdate import Element
-from ..utils.cache import element_cache, get_element_id
-from ..utils.models import (
-    CASCADE_AND_AUTOUODATE,
-    SET_NULL_AND_AUTOUPDATE,
-    RESTModelMixin,
-)
 from .access_permissions import (
     ChatMessageAccessPermissions,
     ConfigAccessPermissions,
@@ -20,6 +13,13 @@ from .access_permissions import (
     ProjectorMessageAccessPermissions,
     TagAccessPermissions,
 )
+from ..utils.autoupdate import Element
+from ..utils.cache import element_cache, get_element_id
+from ..utils.models import (
+    CASCADE_AND_AUTOUODATE,
+    SET_NULL_AND_AUTOUPDATE,
+    RESTModelMixin,
+)
 
 
 class ProjectorManager(models.Manager):

openslides/core/serializers.py

@@ -1,8 +1,5 @@
 from typing import Any
 
-from ..utils.projector import projector_slides
-from ..utils.rest_api import Field, IntegerField, ModelSerializer, ValidationError
-from ..utils.validate import validate_html
 from .models import (
     ChatMessage,
     ConfigStore,
@@ -13,6 +10,9 @@ from .models import (
     ProjectorMessage,
     Tag,
 )
+from ..utils.projector import projector_slides
+from ..utils.rest_api import Field, IntegerField, ModelSerializer, ValidationError
+from ..utils.validate import validate_html
 
 
 class JSONSerializerField(Field):

openslides/core/views.py

@@ -11,6 +11,28 @@ from django.utils.timezone import now
 from django.views import static
 from django.views.generic.base import View
 
+from .access_permissions import (
+    ChatMessageAccessPermissions,
+    ConfigAccessPermissions,
+    CountdownAccessPermissions,
+    HistoryAccessPermissions,
+    ProjectorAccessPermissions,
+    ProjectorMessageAccessPermissions,
+    TagAccessPermissions,
+)
+from .config import config
+from .exceptions import ConfigError, ConfigNotFound
+from .models import (
+    ChatMessage,
+    ConfigStore,
+    Countdown,
+    History,
+    HistoryData,
+    ProjectionDefault,
+    Projector,
+    ProjectorMessage,
+    Tag,
+)
 from .. import __license__ as license, __url__ as url, __version__ as version
 from ..users.models import User
 from ..utils import views as utils_views
@@ -34,28 +56,6 @@ from ..utils.rest_api import (
     detail_route,
     list_route,
 )
-from .access_permissions import (
-    ChatMessageAccessPermissions,
-    ConfigAccessPermissions,
-    CountdownAccessPermissions,
-    HistoryAccessPermissions,
-    ProjectorAccessPermissions,
-    ProjectorMessageAccessPermissions,
-    TagAccessPermissions,
-)
-from .config import config
-from .exceptions import ConfigError, ConfigNotFound
-from .models import (
-    ChatMessage,
-    ConfigStore,
-    Countdown,
-    History,
-    HistoryData,
-    ProjectionDefault,
-    Projector,
-    ProjectorMessage,
-    Tag,
-)
 
 
 # Special Django views

openslides/mediafiles/models.py

@@ -1,10 +1,10 @@
 from django.conf import settings
 from django.db import models
 
+from .access_permissions import MediafileAccessPermissions
 from ..core.config import config
 from ..utils.autoupdate import inform_changed_data
 from ..utils.models import SET_NULL_AND_AUTOUPDATE, RESTModelMixin
-from .access_permissions import MediafileAccessPermissions
 
 
 class Mediafile(RESTModelMixin, models.Model):

openslides/mediafiles/serializers.py

@@ -5,8 +5,8 @@ from django.db import models as dbmodels
 from PyPDF2 import PdfFileReader
 from PyPDF2.utils import PdfReadError
 
-from ..utils.rest_api import FileField, ModelSerializer, SerializerMethodField
 from .models import Mediafile
+from ..utils.rest_api import FileField, ModelSerializer, SerializerMethodField
 
 
 class AngularCompatibleFileField(FileField):

openslides/mediafiles/views.py

@@ -1,11 +1,11 @@
 from django.http import HttpResponseForbidden, HttpResponseNotFound
 from django.views.static import serve
 
+from .access_permissions import MediafileAccessPermissions
+from .models import Mediafile
 from ..core.config import config
 from ..utils.auth import has_perm
 from ..utils.rest_api import ModelViewSet, ValidationError
-from .access_permissions import MediafileAccessPermissions
-from .models import Mediafile
 
 
 # Viewsets for the REST API

openslides/motions/models.py

@@ -23,7 +23,6 @@ from openslides.utils.autoupdate import inform_changed_data
 from openslides.utils.exceptions import OpenSlidesError
 from openslides.utils.models import RESTModelMixin
 
-from ..utils.models import CASCADE_AND_AUTOUODATE, SET_NULL_AND_AUTOUPDATE
 from .access_permissions import (
     CategoryAccessPermissions,
     MotionAccessPermissions,
@@ -34,6 +33,7 @@ from .access_permissions import (
     WorkflowAccessPermissions,
 )
 from .exceptions import WorkflowError
+from ..utils.models import CASCADE_AND_AUTOUODATE, SET_NULL_AND_AUTOUPDATE
 
 
 class StatuteParagraph(RESTModelMixin, models.Model):

openslides/motions/serializers.py

@@ -2,6 +2,20 @@ from typing import Dict, Optional
 
 from django.db import transaction
 
+from .models import (
+    Category,
+    Motion,
+    MotionBlock,
+    MotionChangeRecommendation,
+    MotionComment,
+    MotionCommentSection,
+    MotionLog,
+    MotionPoll,
+    State,
+    StatuteParagraph,
+    Submitter,
+    Workflow,
+)
 from ..core.config import config
 from ..poll.serializers import default_votes_validator
 from ..utils.auth import get_group_model
@@ -18,20 +32,6 @@ from ..utils.rest_api import (
     ValidationError,
 )
 from ..utils.validate import validate_html
-from .models import (
-    Category,
-    Motion,
-    MotionBlock,
-    MotionChangeRecommendation,
-    MotionComment,
-    MotionCommentSection,
-    MotionLog,
-    MotionPoll,
-    State,
-    StatuteParagraph,
-    Submitter,
-    Workflow,
-)
 
 
 def validate_workflow_field(value):

openslides/motions/views.py

@@ -10,22 +10,6 @@ from django.db.models.deletion import ProtectedError
 from django.http.request import QueryDict
 from rest_framework import status
 
-from ..core.config import config
-from ..core.models import Tag
-from ..utils.auth import has_perm, in_some_groups
-from ..utils.autoupdate import inform_changed_data, inform_deleted_data
-from ..utils.rest_api import (
-    CreateModelMixin,
-    DestroyModelMixin,
-    GenericViewSet,
-    ModelViewSet,
-    Response,
-    ReturnDict,
-    UpdateModelMixin,
-    ValidationError,
-    detail_route,
-    list_route,
-)
 from .access_permissions import (
     CategoryAccessPermissions,
     MotionAccessPermissions,
@@ -50,6 +34,22 @@ from .models import (
     Workflow,
 )
 from .serializers import MotionPollSerializer, StateSerializer
+from ..core.config import config
+from ..core.models import Tag
+from ..utils.auth import has_perm, in_some_groups
+from ..utils.autoupdate import inform_changed_data, inform_deleted_data
+from ..utils.rest_api import (
+    CreateModelMixin,
+    DestroyModelMixin,
+    GenericViewSet,
+    ModelViewSet,
+    Response,
+    ReturnDict,
+    UpdateModelMixin,
+    ValidationError,
+    detail_route,
+    list_route,
+)
 
 
 # Viewsets for the REST API

openslides/topics/models.py

@@ -3,10 +3,10 @@ from typing import Any, Dict
 from django.contrib.contenttypes.fields import GenericRelation
 from django.db import models
 
+from .access_permissions import TopicAccessPermissions
 from ..agenda.models import Item
 from ..mediafiles.models import Mediafile
 from ..utils.models import RESTModelMixin
-from .access_permissions import TopicAccessPermissions
 
 
 class TopicManager(models.Manager):

openslides/topics/views.py

@@ -1,8 +1,8 @@
 from openslides.utils.rest_api import ModelViewSet
 
-from ..utils.auth import has_perm
 from .access_permissions import TopicAccessPermissions
 from .models import Topic
+from ..utils.auth import has_perm
 
 
 class TopicViewSet(ModelViewSet):

openslides/users/models.py

@@ -17,14 +17,14 @@ from django.db.models import Prefetch
 from django.utils import timezone
 from jsonfield import JSONField
 
-from ..core.config import config
-from ..utils.auth import GROUP_ADMIN_PK
-from ..utils.models import CASCADE_AND_AUTOUODATE, RESTModelMixin
 from .access_permissions import (
     GroupAccessPermissions,
     PersonalNoteAccessPermissions,
     UserAccessPermissions,
 )
+from ..core.config import config
+from ..utils.auth import GROUP_ADMIN_PK
+from ..utils.models import CASCADE_AND_AUTOUODATE, RESTModelMixin
 
 
 class UserManager(BaseUserManager):

openslides/users/serializers.py

@@ -1,6 +1,7 @@
 from django.contrib.auth.hashers import make_password
 from django.contrib.auth.models import Permission
 
+from .models import Group, PersonalNote, User
 from ..utils.autoupdate import inform_changed_data
 from ..utils.rest_api import (
     IdPrimaryKeyRelatedField,
@@ -9,7 +10,6 @@ from ..utils.rest_api import (
     RelatedField,
     ValidationError,
 )
-from .models import Group, PersonalNote, User
 
 
 USERCANSEESERIALIZER_FIELDS = (

openslides/users/signals.py

@@ -2,8 +2,8 @@ from django.apps import apps
 from django.contrib.auth.models import Permission
 from django.db.models import Q
 
-from ..utils.auth import GROUP_ADMIN_PK, GROUP_DEFAULT_PK
 from .models import Group, User
+from ..utils.auth import GROUP_ADMIN_PK, GROUP_DEFAULT_PK
 
 
 def get_permission_change_data(sender, permissions=None, **kwargs):

openslides/users/views.py

@@ -20,6 +20,13 @@ from django.http.request import QueryDict
 from django.utils.encoding import force_bytes, force_text
 from django.utils.http import urlsafe_base64_decode, urlsafe_base64_encode
 
+from .access_permissions import (
+    GroupAccessPermissions,
+    PersonalNoteAccessPermissions,
+    UserAccessPermissions,
+)
+from .models import Group, PersonalNote, User
+from .serializers import GroupSerializer, PermissionRelatedField
 from ..core.config import config
 from ..core.signals import permission_change
 from ..utils.auth import (
@@ -40,13 +47,6 @@ from ..utils.rest_api import (
     status,
 )
 from ..utils.views import APIView
-from .access_permissions import (
-    GroupAccessPermissions,
-    PersonalNoteAccessPermissions,
-    UserAccessPermissions,
-)
-from .models import Group, PersonalNote, User
-from .serializers import GroupSerializer, PermissionRelatedField
 
 
 # Viewsets for the REST API

openslides/utils/cache.py

@@ -69,9 +69,6 @@ class ElementCache:
             )
         self.start_time = start_time
 
-        # Contains Futures to controll, that only one client updates the restricted_data.
-        self.restricted_data_cache_updater: Dict[int, asyncio.Future] = {}
-
         # Tells if self.ensure_cache was called.
         self.ensured = False
 
@@ -280,8 +277,6 @@ class ElementCache:
         # TODO: Make a timeout. Else this could block forever
         lock_name = f"restricted_data_{user_id}"
         if await self.cache_provider.set_lock(lock_name):
-            future: asyncio.Future = asyncio.Future()
-            self.restricted_data_cache_updater[user_id] = future
             # Get change_id for this user
             value = await self.cache_provider.get_change_id_user(user_id)
             # If the change id is not in the cache yet, use -1 to get all data since 0
@@ -330,15 +325,10 @@ class ElementCache:
                     await self.cache_provider.del_elements(deleted_elements, user_id)
             # Unset the lock
             await self.cache_provider.del_lock(lock_name)
-            future.set_result(1)
         else:
             # Wait until the update if finshed
-            if user_id in self.restricted_data_cache_updater:
+            while await self.cache_provider.get_lock(lock_name):
-                # The active worker is on the same asgi server, we can use the future
+                await asyncio.sleep(0.01)
-                await self.restricted_data_cache_updater[user_id]
-            else:
-                while await self.cache_provider.get_lock(lock_name):
-                    await asyncio.sleep(0.01)
 
     async def get_all_restricted_data(
         self, user_id: int

tests/integration/utils/test_consumers.py

@@ -17,8 +17,8 @@ from openslides.utils.autoupdate import (
 )
 from openslides.utils.cache import element_cache
 
-from ...unit.utils.cache_provider import Collection1, Collection2, get_cachable_provider
 from ..helpers import TConfig, TProjector, TUser
+from ...unit.utils.cache_provider import Collection1, Collection2, get_cachable_provider
 
 
 @pytest.fixture(autouse=True)

tests/unit/utils/cache_provider.py

@@ -71,12 +71,7 @@ class TTestCacheProvider(MemmoryCacheProvider):
     async def del_lock_after_wait(
         self, lock_name: str, future: asyncio.Future = None
     ) -> None:
-        if future is None:
+        async def set_future() -> None:
-            asyncio.ensure_future(self.del_lock(lock_name))
+            await self.del_lock(lock_name)
-        else:
 
-            async def set_future() -> None:
+        asyncio.ensure_future(set_future())
-                await self.del_lock(lock_name)
-                future.set_result(1)  # type: ignore
-
-            asyncio.ensure_future(set_future())

tests/unit/utils/test_cache.py

@@ -1,4 +1,3 @@
-import asyncio
 import json
 from typing import Any, Dict, List
 from unittest.mock import patch
@@ -306,8 +305,6 @@ async def test_update_restricted_data(element_cache):
     )
     # Make sure the lock is deleted
     assert not await element_cache.cache_provider.get_lock("restricted_data_0")
-    # And the future is done
-    assert element_cache.restricted_data_cache_updater[0].done()
 
 
 @pytest.mark.asyncio
@@ -330,8 +327,6 @@ async def test_update_restricted_data_full_restricted_elements(element_cache):
     )
     # Make sure the lock is deleted
     assert not await element_cache.cache_provider.get_lock("restricted_data_0")
-    # And the future is done
-    assert element_cache.restricted_data_cache_updater[0].done()
 
 
 @pytest.mark.asyncio
@@ -389,7 +384,7 @@ async def test_update_restricted_data_with_deleted_elements(element_cache):
 
 
 @pytest.mark.asyncio
-async def test_update_restricted_data_second_worker_on_different_server(element_cache):
+async def test_update_restricted_data_second_worker(element_cache):
     """
     Test, that if another worker is updating the data, noting is done.
 
@@ -406,26 +401,6 @@ async def test_update_restricted_data_second_worker_on_different_server(element_
     assert element_cache.cache_provider.restricted_data == {0: {}}
 
 
-@pytest.mark.asyncio
-async def test_update_restricted_data_second_worker_on_same_server(element_cache):
-    """
-    Test, that if another worker is updating the data, noting is done.
-
-    This tests makes use of the future as it would on the same daphne server.
-    """
-    element_cache.use_restricted_data_cache = True
-    element_cache.cache_provider.restricted_data = {0: {}}
-    future: asyncio.Future = asyncio.Future()
-    element_cache.restricted_data_cache_updater[0] = future
-    await element_cache.cache_provider.set_lock("restricted_data_0")
-    await element_cache.cache_provider.del_lock_after_wait("restricted_data_0", future)
-
-    await element_cache.update_restricted_data(0)
-
-    # Restricted_data_should not be set on second worker
-    assert element_cache.cache_provider.restricted_data == {0: {}}
-
-
 @pytest.mark.asyncio
 async def test_get_all_restricted_data(element_cache):
     element_cache.use_restricted_data_cache = True