Cache xform list results (#2692)

* cache xform list results by user role

* remove ipdb statement

* enhance test

* enhance test

* add test

* update test docstring

* update method name

* rename method

* update docstring

* revert changes

* revert changes

* update docstring

onadata/apps/api/tasks.py

@@ -20,14 +20,14 @@
 from onadata.apps.api import tools
 from onadata.apps.api.models.organization_profile import OrganizationProfile
 from onadata.apps.logger.models import Instance, ProjectInvitation, XForm, Project
+from onadata.apps.api.tools import invalidate_organization_cache
 from onadata.celeryapp import app
 from onadata.libs.utils.email import send_generic_email
 from onadata.libs.utils.model_tools import queryset_iterator
 from onadata.libs.utils.cache_tools import (
     safe_delete,
     XFORM_REGENERATE_INSTANCE_JSON_TASK,
 )
-from onadata.libs.utils.logger_tools import invalidate_organization_cache
 from onadata.libs.models.share_project import ShareProject
 from onadata.libs.utils.email import ProjectInvitationEmail
 

onadata/apps/api/tests/viewsets/test_xform_list_viewset.py

@@ -1335,3 +1335,120 @@ def test_retrieve_forms_in_project(self):
         response = self.view(request, project_pk=self.project.pk)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 3)
+
+    def test_xform_list_cache_set(self):
+        """XForm list cache is set if xform_pk or project_pk kwargs present"""
+        # `xform_pk` anonymous user
+        request = self.factory.get("/")
+        response = self.view(request, xform_pk=self.xform.pk)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            cache.get(f"xfm-list-{self.xform.pk}-XForm-anon"),
+            [
+                {
+                    "formID": "transportation_2011_07_25",
+                    "name": "transportation_2011_07_25",
+                    "version": "2014111",
+                    "hash": self.xform.hash,
+                    "descriptionText": "",
+                    "downloadUrl": f"http://testserver/bob/forms/{self.xform.pk}/form.xml",
+                    "manifestUrl": None,
+                }
+            ],
+        )
+
+        # `xform_pk` authenticated user
+        cache.clear()
+        request = self.factory.get("/", **self.extra)
+        response = self.view(request, xform_pk=self.xform.pk)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            cache.get(f"xfm-list-{self.xform.pk}-XForm-owner"),
+            [
+                {
+                    "formID": "transportation_2011_07_25",
+                    "name": "transportation_2011_07_25",
+                    "version": "2014111",
+                    "hash": self.xform.hash,
+                    "descriptionText": "",
+                    "downloadUrl": f"http://testserver/bob/forms/{self.xform.pk}/form.xml",
+                    "manifestUrl": None,
+                }
+            ],
+        )
+
+        # `project_pk` anonymous user
+        cache.clear()
+        request = self.factory.get("/")
+        response = self.view(request, project_pk=self.project.pk)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            cache.get(f"xfm-list-{self.project.pk}-Project-anon"),
+            [
+                {
+                    "formID": "transportation_2011_07_25",
+                    "name": "transportation_2011_07_25",
+                    "version": "2014111",
+                    "hash": self.xform.hash,
+                    "descriptionText": "",
+                    "downloadUrl": f"http://testserver/bob/forms/{self.xform.pk}/form.xml",
+                    "manifestUrl": None,
+                }
+            ],
+        )
+
+        # `project_pk` authenticated user
+        cache.clear()
+        request = self.factory.get("/", **self.extra)
+        response = self.view(request, project_pk=self.project.pk)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            cache.get(f"xfm-list-{self.project.pk}-Project-owner"),
+            [
+                {
+                    "formID": "transportation_2011_07_25",
+                    "name": "transportation_2011_07_25",
+                    "version": "2014111",
+                    "hash": self.xform.hash,
+                    "descriptionText": "",
+                    "downloadUrl": f"http://testserver/bob/forms/{self.xform.pk}/form.xml",
+                    "manifestUrl": None,
+                }
+            ],
+        )
+
+    def test_xform_list_cache_hit(self):
+        """XForm list results returned from cache if available"""
+        cache.set(
+            f"xfm-list-{self.xform.pk}-XForm-anon",
+            [
+                {
+                    "formID": "transportation_2011_07_25",
+                    "name": "transportation_2011_07_25",
+                    "version": "2014111",
+                    "hash": self.xform.hash,
+                    "descriptionText": "",
+                    "downloadUrl": f"http://testserver/bob/forms/{self.xform.pk}/form.xml",
+                    "manifestUrl": None,
+                }
+            ],
+        )
+
+        with patch.object(cache, "set") as mock_cache_set:
+            request = self.factory.get("/")
+            response = self.view(request, xform_pk=self.xform.pk)
+            self.assertEqual(response.status_code, 200)
+            # Cache set not called because results were returned from cache
+            mock_cache_set.assert_not_called()
+
+        path = os.path.join(os.path.dirname(__file__), "..", "fixtures", "formList.xml")
+
+        with open(path, encoding="utf-8") as f:
+            form_list_xml = f.read().strip()
+            data = {"hash": self.xform.hash, "pk": self.xform.pk}
+            content = response.render().content.decode("utf-8")
+            self.assertEqual(content, form_list_xml % data)
+            self.assertTrue(response.has_header("X-OpenRosa-Version"))
+            self.assertTrue(response.has_header("X-OpenRosa-Accept-Content-Length"))
+            self.assertTrue(response.has_header("Date"))
+            self.assertEqual(response["Content-Type"], "text/xml; charset=utf-8")

onadata/apps/api/tools.py

@@ -51,6 +51,7 @@
 from onadata.libs.models.share_project import ShareProject
 from onadata.libs.permissions import (
     ROLES,
+    ROLES_ORDERED,
     DataEntryMinorRole,
     DataEntryOnlyRole,
     DataEntryRole,
@@ -75,6 +76,8 @@
     PROJ_NUM_DATASET_CACHE,
     PROJ_OWNER_CACHE,
     PROJ_SUB_DATE_CACHE,
+    ORG_PROFILE_CACHE,
+    XFORM_LIST_CACHE,
     reset_project_cache,
     safe_delete,
 )
@@ -93,6 +96,7 @@
     check_and_set_form_by_id_string,
 )
 
+
 DECIMAL_PRECISION = 2
 
 # pylint: disable=invalid-name
@@ -873,3 +877,50 @@ def share(project, role):
             else:
                 project_role = get_team_project_default_permissions(team, project)
                 share(project, project_role)
+
+
+def get_org_profile_cache_key(user, organization):
+    """Return cache key given user and organization profile"""
+    org_username = organization.user.username
+
+    if user.is_anonymous:
+        return f"{ORG_PROFILE_CACHE}{org_username}-anon"
+
+    user_role = get_role_in_org(user, organization)
+
+    return f"{ORG_PROFILE_CACHE}{org_username}-{user_role}"
+
+
+def invalidate_organization_cache(org_username):
+    """Set organization cache to none for all roles"""
+    for role in ROLES_ORDERED:
+        key = f"{ORG_PROFILE_CACHE}{org_username}-{role.name}"
+        safe_delete(key)
+
+    safe_delete(f"{ORG_PROFILE_CACHE}{org_username}-anon")
+
+
+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
+    """
+    object_type = type(xform_or_project).__name__
+    cache_key_prefix = f"{XFORM_LIST_CACHE}{xform_or_project.id}-{object_type}"
+    anonymous_user_key = f"{cache_key_prefix}-anon"
+
+    if user.is_anonymous:
+        return anonymous_user_key
+
+    perms = get_perms(user, xform_or_project)
+    user_role = get_role(perms, xform_or_project)
+
+    if user_role is None:
+        return anonymous_user_key
+
+    return f"{cache_key_prefix}-{user_role}"

onadata/apps/api/viewsets/organization_profile_viewset.py

@@ -17,7 +17,7 @@
 
 from onadata.apps.api import permissions
 from onadata.apps.api.models.organization_profile import OrganizationProfile
-from onadata.apps.api.tools import get_baseviewset_class
+from onadata.apps.api.tools import get_baseviewset_class, get_org_profile_cache_key
 from onadata.libs.filters import (
     OrganizationPermissionFilter,
     OrganizationsSharedWithUserFilter,
@@ -32,7 +32,6 @@
 from onadata.libs.serializers.organization_serializer import OrganizationSerializer
 from onadata.libs.utils.cache_tools import safe_delete
 from onadata.libs.utils.common_tools import merge_dicts
-from onadata.libs.utils.logger_tools import get_org_profile_cache_key
 
 BaseViewset = get_baseviewset_class()
 
@@ -121,14 +120,11 @@ def members(self, request, *args, **kwargs):
 
         serializer.save()
 
-        data = self.serializer_class(
-            organization, context={"request": request}
-        ).data
+        data = self.serializer_class(organization, context={"request": request}).data
         # pylint: disable=attribute-defined-outside-init
         self.etag_data = json.dumps(data)
         resp_status = (
-            status.HTTP_201_CREATED if request.method == "POST"
-            else status.HTTP_200_OK
+            status.HTTP_201_CREATED if request.method == "POST" else status.HTTP_200_OK
         )
 
         return Response(status=resp_status, data=serializer.data)

onadata/apps/api/viewsets/xform_list_viewset.py

@@ -13,7 +13,12 @@
 from rest_framework.decorators import action
 from rest_framework.response import Response
 
-from onadata.apps.api.tools import get_baseviewset_class, get_media_file_response
+from onadata.apps.api.tools import (
+    get_baseviewset_class,
+    get_media_file_response,
+    get_xform_list_cache_key,
+)
+from onadata.apps.logger.models.project import Project
 from onadata.apps.logger.models.xform import XForm, get_forms_shared_with_user
 from onadata.apps.main.models.meta_data import MetaData
 from onadata.apps.main.models.user_profile import UserProfile
@@ -30,7 +35,12 @@
     XFormListSerializer,
     XFormManifestSerializer,
 )
-from onadata.libs.utils.cache_tools import XFORM_MANIFEST_CACHE
+from onadata.libs.utils.cache_tools import (
+    XFORM_MANIFEST_CACHE,
+    XFROM_LIST_CACHE_TTL,
+    safe_cache_get,
+    safe_cache_set,
+)
 from onadata.libs.utils.common_tags import GROUP_DELIMETER_TAG, REPEAT_INDEX_TAGS
 from onadata.libs.utils.export_builder import ExportBuilder
 
@@ -153,15 +163,42 @@ def filter_queryset(self, queryset):
 
         return queryset
 
-    def list(self, request, *args, **kwargs):
-        # pylint: disable=attribute-defined-outside-init
-        self.object_list = self.filter_queryset(self.get_queryset())
+    def _get_xform_list_cache_key(self):
+        xform_pk = self.kwargs.get("xform_pk")
+        project_pk = self.kwargs.get("project_pk")
+        cache_key = None
 
+        if xform_pk:
+            xform = get_object_or_404(XForm, pk=xform_pk)
+            cache_key = get_xform_list_cache_key(self.request.user, xform)
+
+        elif project_pk:
+            project = get_object_or_404(Project, pk=project_pk)
+            cache_key = get_xform_list_cache_key(self.request.user, project)
+
+        return cache_key
+
+    def list(self, request, *args, **kwargs):
         headers = get_openrosa_headers(request, location=False)
-        serializer = self.get_serializer(self.object_list, many=True)
+
         if request.method in ["HEAD"]:
             return Response("", headers=headers, status=204)
 
+        cache_key = self._get_xform_list_cache_key()
+
+        if cache_key is not None:
+            cached_result = safe_cache_get(cache_key)
+
+            if cached_result is not None:
+                return Response(cached_result, headers=headers)
+
+        # pylint: disable=attribute-defined-outside-init
+        self.object_list = self.filter_queryset(self.get_queryset())
+        serializer = self.get_serializer(self.object_list, many=True)
+
+        if cache_key is not None:
+            safe_cache_set(cache_key, serializer.data, XFROM_LIST_CACHE_TTL)
+
         return Response(serializer.data, headers=headers)
 
     def retrieve(self, request, *args, **kwargs):