Merge pull request #4455 from ostcar/disable_restricted_data_future_lock

Disable the future-lock when updating the restircted data cache

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