Invalidate XForm list cache when form is published/replaced (#2730)

* invalidate xform list cache when form is published/replaced

* refactor test

* enhance test

* suppress lint warning import-outside-toplevel

* fix cyclic dependency

* refactor to use safe cache operations

onadata/apps/api/tests/test_tools.py

@@ -1,16 +1,17 @@
 """Tests for module onadata.apps.api.tools"""
 
 from django.contrib.auth import get_user_model
+from django.core.cache import cache
 
 from onadata.apps.api.models.organization_profile import (
     OrganizationProfile,
     Team,
     get_organization_members_team,
 )
-from onadata.apps.api.tools import add_user_to_organization
+from onadata.apps.api.tools import add_user_to_organization, invalidate_xform_list_cache
 from onadata.apps.logger.models.project import Project
 from onadata.apps.main.tests.test_base import TestBase
-from onadata.libs.permissions import DataEntryRole, ManagerRole, OwnerRole
+from onadata.libs.permissions import ROLES, DataEntryRole, ManagerRole, OwnerRole
 
 User = get_user_model()
 
@@ -96,3 +97,52 @@ def test_role_none(self):
             members_team.user_set.filter(username=self.user.username).exists()
         )
         self.assertTrue(DataEntryRole.user_has_role(self.user, self.project))
+
+
+class InvalidateXFormListCacheTestCase(TestBase):
+    """Tests for invalidate_xform_list_cache"""
+
+    def setUp(self):
+        super().setUp()
+
+        self._publish_transportation_form()
+        self.cache_keys = [
+            f"xfm-list-{self.xform.pk}-XForm-anon",
+            f"xfm-list-{self.xform.project.pk}-Project-anon",
+        ]
+
+        # Simulate cached data
+        for role in ROLES:
+            self.cache_keys.extend(
+                [
+                    f"xfm-list-{self.xform.pk}-XForm-{role}",
+                    f"xfm-list-{self.xform.project.pk}-Project-{role}",
+                ]
+            )
+
+        for key in self.cache_keys:
+            cache.set(key, "data")
+
+    def test_cache_invalidated(self):
+        """Cache invalidated for xform and project"""
+        self.assertIsNotNone(cache.get(f"xfm-list-{self.xform.pk}-XForm-anon"))
+        self.assertIsNotNone(
+            cache.get(f"xfm-list-{self.xform.project.pk}-Project-anon")
+        )
+
+        for role in ROLES:
+            self.assertIsNotNone(cache.get(f"xfm-list-{self.xform.pk}-XForm-{role}"))
+            self.assertIsNotNone(
+                cache.get(f"xfm-list-{self.xform.project.pk}-Project-{role}")
+            )
+
+        invalidate_xform_list_cache(self.xform)
+
+        self.assertIsNone(cache.get(f"xfm-list-{self.xform.pk}-XForm-anon"))
+        self.assertIsNone(cache.get(f"xfm-list-{self.xform.project.pk}-Project-anon"))
+
+        for role in ROLES:
+            self.assertIsNone(cache.get(f"xfm-list-{self.xform.pk}-XForm-{role}"))
+            self.assertIsNone(
+                cache.get(f"xfm-list-{self.xform.project.pk}-Project-{role}")
+            )

onadata/apps/api/tools.py

@@ -2,6 +2,7 @@
 """
 API utility functions.
 """
+
 import importlib
 import os
 import tempfile
@@ -37,13 +38,7 @@
     get_organization_members_team,
 )
 from onadata.apps.api.models.team import Team
-from onadata.apps.logger.models import (
-    DataView,
-    Instance,
-    Project,
-    XForm,
-    EntityList,
-)
+from onadata.apps.logger.models import DataView, EntityList, Instance, Project, XForm
 from onadata.apps.main.forms import QuickConverter
 from onadata.apps.main.models.meta_data import MetaData
 from onadata.apps.main.models.user_profile import UserProfile
@@ -61,22 +56,22 @@
     OwnerRole,
     get_role,
     get_role_in_org,
-    is_organization,
     get_team_project_default_permissions,
+    is_organization,
 )
 from onadata.libs.serializers.project_serializer import ProjectSerializer
 from onadata.libs.utils.api_export_tools import (
     custom_response_handler,
-    get_metadata_format,
     get_entity_list_export_response,
+    get_metadata_format,
 )
 from onadata.libs.utils.cache_tools import (
+    ORG_PROFILE_CACHE,
     PROJ_BASE_FORMS_CACHE,
     PROJ_FORMS_CACHE,
     PROJ_NUM_DATASET_CACHE,
     PROJ_OWNER_CACHE,
     PROJ_SUB_DATE_CACHE,
-    ORG_PROFILE_CACHE,
     XFORM_LIST_CACHE,
     reset_project_cache,
     safe_delete,
@@ -96,7 +91,6 @@
     check_and_set_form_by_id_string,
 )
 
-
 DECIMAL_PRECISION = 2
 
 # pylint: disable=invalid-name
@@ -918,18 +912,24 @@ def invalidate_organization_cache(org_username):
     safe_delete(f"{ORG_PROFILE_CACHE}{org_username}-anon")
 
 
+def _get_xform_list_cache_key_prefix(xform_or_project):
+    """Get the cache key prefix for the XForm list by user's role
+
+    :param xform_or_project: XForm or Project being accessed
+    :return: cache key prefix based on role assigned to form/project
+    """
+    object_type = type(xform_or_project).__name__
+    return f"{XFORM_LIST_CACHE}{xform_or_project.id}-{object_type}"
+
+
 def get_xform_list_cache_key(user, xform_or_project):
     """Get the cache key for the XForm list by user's role
 
-    Args:
-        user: User making request
-        xform_or_project: XForm or Project being accessed
-
-    Returns:
-        str: cache key based on role assigned to form/project
+    :param user: User making request
+    :param xform_or_project: XForm or Project being accessed
+    :return: cache key based on role assigned to form/project
     """
-    object_type = type(xform_or_project).__name__
-    cache_key_prefix = f"{XFORM_LIST_CACHE}{xform_or_project.id}-{object_type}"
+    cache_key_prefix = _get_xform_list_cache_key_prefix(xform_or_project)
     anonymous_user_key = f"{cache_key_prefix}-anon"
 
     if user.is_anonymous:
@@ -942,3 +942,20 @@ def get_xform_list_cache_key(user, xform_or_project):
         return anonymous_user_key
 
     return f"{cache_key_prefix}-{user_role}"
+
+
+def invalidate_xform_list_cache(xform):
+    """Invalidate the cache for the XForm list by user's role
+
+    :param xform: XForm instance
+    :return: None
+    """
+    xform_cache_key_prefix = _get_xform_list_cache_key_prefix(xform)
+    project_cache_key_prefix = _get_xform_list_cache_key_prefix(xform.project)
+
+    for role in ROLES_ORDERED:
+        safe_delete(f"{xform_cache_key_prefix}-{role.name}")
+        safe_delete(f"{project_cache_key_prefix}-{role.name}")
+
+    safe_delete(f"{xform_cache_key_prefix}-anon")
+    safe_delete(f"{project_cache_key_prefix}-anon")

onadata/apps/api/viewsets/xform_list_viewset.py

@@ -2,8 +2,8 @@
 """
 OpenRosa Form List API - https://docs.getodk.org/openrosa-form-list/
 """
+
 from django.conf import settings
-from django.core.cache import cache
 from django.http import Http404, StreamingHttpResponse
 from django.shortcuts import get_object_or_404
 
@@ -217,7 +217,7 @@ def manifest(self, request, *args, **kwargs):
         # pylint: disable=attribute-defined-outside-init
         xform = self.get_object()
         cache_key = f"{XFORM_MANIFEST_CACHE}{xform.pk}"
-        cached_manifest: str | None = cache.get(cache_key)
+        cached_manifest: str | None = safe_cache_get(cache_key)
         # Ensure a previous stream has completed updating the cache by
         # confirm the last tag </manifest> exists
         if cached_manifest is not None and cached_manifest.endswith("</manifest>"):

onadata/apps/viewer/models/data_dictionary.py

@@ -2,14 +2,16 @@
 """
 DataDictionary model.
 """
+
+import importlib
 import json
 import os
 from io import BytesIO, StringIO
 
 from django.contrib.contenttypes.models import ContentType
 from django.core.files.uploadedfile import InMemoryUploadedFile
-from django.db.models.signals import post_save, pre_save
 from django.db import transaction
+from django.db.models.signals import post_save, pre_save
 from django.utils import timezone
 from django.utils.translation import gettext as _
 
@@ -23,8 +25,8 @@
 from pyxform.xls2json_backends import xlsx_value_to_str
 
 from onadata.apps.logger.models.entity_list import EntityList
-from onadata.apps.logger.models.registration_form import RegistrationForm
 from onadata.apps.logger.models.follow_up_form import FollowUpForm
+from onadata.apps.logger.models.registration_form import RegistrationForm
 from onadata.apps.logger.models.xform import XForm, check_version_set, check_xform_uuid
 from onadata.apps.logger.xform_instance_parser import XLSFormError
 from onadata.apps.main.models.meta_data import MetaData
@@ -205,11 +207,6 @@ def set_object_permissions(sender, instance=None, created=False, **kwargs):
     Apply the relevant object permissions for the form to all users who should
     have access to it.
     """
-    if instance.project:
-        # clear cache
-        safe_delete(f"{PROJ_FORMS_CACHE}{instance.project.pk}")
-        safe_delete(f"{PROJ_BASE_FORMS_CACHE}{instance.project.pk}")
-
     # seems the super is not called, have to get xform from here
     xform = XForm.objects.get(pk=instance.pk)
 
@@ -223,9 +220,9 @@ def set_object_permissions(sender, instance=None, created=False, **kwargs):
             OwnerRole.add(instance.created_by, xform)
 
         # pylint: disable=import-outside-toplevel
-        from onadata.libs.utils.project_utils import (
+        from onadata.libs.utils.project_utils import (  # noqa
             set_project_perms_to_xform_async,
-        )  # noqa
+        )
 
         try:
             transaction.on_commit(
@@ -235,9 +232,9 @@ def set_object_permissions(sender, instance=None, created=False, **kwargs):
             )
         except OperationalError:
             # pylint: disable=import-outside-toplevel
-            from onadata.libs.utils.project_utils import (
+            from onadata.libs.utils.project_utils import (  # noqa
                 set_project_perms_to_xform,
-            )  # noqa
+            )
 
             set_project_perms_to_xform(xform, instance.project)
 
@@ -418,3 +415,22 @@ def disable_registration_form(sender, instance=None, created=False, **kwargs):
     sender=DataDictionary,
     dispatch_uid="disable_registration_form_datadictionary",
 )
+
+
+def invalidate_caches(sender, instance=None, created=False, **kwargs):
+    """Invalidate caches"""
+    # Avoid cyclic dependency errors
+    api_tools = importlib.import_module("onadata.apps.api.tools")
+
+    xform = XForm.objects.get(pk=instance.pk)
+
+    safe_delete(f"{PROJ_FORMS_CACHE}{instance.project.pk}")
+    safe_delete(f"{PROJ_BASE_FORMS_CACHE}{instance.project.pk}")
+    api_tools.invalidate_xform_list_cache(xform)
+
+
+post_save.connect(
+    invalidate_caches,
+    sender=DataDictionary,
+    dispatch_uid="xform_invalidate_caches",
+)

onadata/apps/viewer/models/tests/test_data_dictionary.py

@@ -2,12 +2,15 @@
 
 import json
 
-from onadata.apps.main.tests.test_base import TestBase
+from django.core.cache import cache
+
 from onadata.apps.logger.models.entity_list import EntityList
-from onadata.apps.logger.models.xform import XForm
 from onadata.apps.logger.models.follow_up_form import FollowUpForm
 from onadata.apps.logger.models.registration_form import RegistrationForm
+from onadata.apps.logger.models.xform import XForm
 from onadata.apps.main.models.meta_data import MetaData
+from onadata.apps.main.tests.test_base import TestBase
+from onadata.libs.permissions import ROLES
 from onadata.libs.utils.user_auth import get_user_default_project
 
 
@@ -269,3 +272,38 @@ def test_reactivate_followup_form(self):
         self._replace_form(self.follow_up_form, data_dict)
         form.refresh_from_db()
         self.assertTrue(form.is_active)
+
+    def test_cache_invalidated(self):
+        """Various caches are invalidated when form is created/replaced"""
+        cache_keys = [
+            f"ps-project_forms-{self.project.pk}",
+            f"ps-project_base_forms-{self.project.pk}",
+            f"xfm-list-{self.project.pk}-Project-anon",
+        ]
+
+        for role in ROLES:
+            cache_keys.append(f"xfm-list-{self.project.pk}-Project-{role}")
+
+        for key in cache_keys:
+            cache.set(key, "data")
+
+        data_dict = self._publish_markdown(self.registration_form, self.user)
+
+        for key in cache_keys:
+            self.assertIsNone(cache.get(key))
+
+        # Reset caches
+        xform = XForm.objects.get(pk=data_dict.pk)
+        cache_keys.append(f"xfm-list-{xform.pk}-XForm-anon")
+
+        for role in ROLES:
+            cache_keys.append(f"xfm-list-{xform.pk}-XForm-{role}")
+
+        for key in cache_keys:
+            cache.set(key, "data")
+
+        # Replace form
+        self._replace_form(self.follow_up_form, data_dict)
+
+        for key in cache_keys:
+            self.assertIsNone(cache.get(key))