Merge pull request #65 from unicef/feature/241796-add-cache-on-projects-for-kobo

AB#241796: Add cache for Kobo data

Author: sergey-misuk-valor

src/country_workspace/config/fragments/constance.py

@@ -51,6 +51,7 @@
     "KOBO_MASTER_API_TOKEN": (KOBO_MASTER_API_TOKEN, "Kobo API Master Access Token", "write_only_input"),
     "KOBO_PROJECT_VIEW_ID": (KOBO_PROJECT_VIEW_ID, "Kobo Project View ID", str),
     "KOBO_KF_URL": (KOBO_KF_URL, "Kobo Server address", str),
+    "KOBO_CACHE_TTL": (86400, "Kobo data cache TTL", int),
     "CACHE_TIMEOUT": (86400, "Cache Redis TTL", int),
     "CACHE_BY_VERSION": (False, "Invalidate Cache on CW version change", bool),
     "CONCURRENCY_GUARD": (
@@ -74,6 +75,7 @@
         "KOBO_MASTER_API_TOKEN",
         "KOBO_PROJECT_VIEW_ID",
         "KOBO_KF_URL",
+        "KOBO_CACHE_TTL",
     ),
     "Data consistency": ("CONCURRENCY_GUARD",),
 }

src/country_workspace/contrib/kobo/api/client/main.py

@@ -1,24 +1,20 @@
 from collections.abc import Generator
-from functools import partial
-from typing import Final
 
-from requests import Session
-
-from country_workspace.contrib.kobo.api.client.auth import Auth
 from country_workspace.contrib.kobo.api.client.helpers import DataGetter, get_asset_list, get_asset_list_url
 from country_workspace.contrib.kobo.api.data.asset import Asset
 
-ACCEPT_JSON_HEADERS: Final[dict[str, str]] = {"Accept": "application/json"}
-
 
 class Client:
     def __init__(
-        self, *, base_url: str, token: str, country_code: str | None = None, project_view_id: str | None = None
+        self,
+        *,
+        data_getter: DataGetter,
+        base_url: str,
+        country_code: str | None = None,
+        project_view_id: str | None = None,
     ) -> None:
         self.url = get_asset_list_url(base_url, project_view_id, country_code)
-        session = Session()
-        session.auth = Auth(token)
-        self.data_getter: DataGetter = partial(session.get, headers=ACCEPT_JSON_HEADERS)
+        self.data_getter = data_getter
 
     @property
     def assets(self) -> Generator[Asset, None, None]:

src/country_workspace/contrib/kobo/api/common.py

@@ -1,5 +1,77 @@
 from collections.abc import Callable
+from typing import Any, TypedDict
 
-from requests import Response
+from django.core.cache import cache
+from requests import Response, Session, HTTPError
 
-DataGetter = Callable[[str], Response]
+UrlPredicate = Callable[[str], bool]
+
+
+class ResponseDict(TypedDict):
+    """Dictionary for response data."""
+
+    json: dict[str, Any]
+    status_code: int
+
+
+class CachedResponse:
+    """Wrapper resembling requests.Response."""
+
+    def __init__(self, response_dict: ResponseDict) -> None:
+        self._response_dict = response_dict
+
+    def json(self) -> dict[str, Any]:
+        return self._response_dict["json"]
+
+    @property
+    def status_code(self) -> int:
+        return self._response_dict["status_code"]
+
+    def raise_for_status(self) -> None:
+        pass
+
+
+def data_getter_cache_key(url: str) -> str:
+    return f"dg:{url}"
+
+
+class DataGetter:
+    """
+    An abstraction over HTTP GET request.
+
+    A class used to abstract HTTP GET request, so authentication, caching,
+    etc. can be configured without touching the rest of the code.
+    """
+
+    def __init__(
+        self,
+        session: Session,
+        cache_ttl: int,
+        headers: dict[str, str] | None = None,
+        do_not_use_cache_if: UrlPredicate | None = None,
+    ) -> None:
+        self._session = session
+        self._headers = headers
+        self._cache_ttl = cache_ttl
+        self._do_not_use_cache_if = do_not_use_cache_if
+
+    def __call__(self, url: str) -> Response | CachedResponse:
+        if self._do_not_use_cache_if and self._do_not_use_cache_if(url):
+            return self._session.get(url, headers=self._headers)
+
+        cache_key = data_getter_cache_key(url)
+
+        if cached_value := cache.get(cache_key):
+            return CachedResponse(cached_value)
+
+        response = self._session.get(url, headers=self._headers)
+        try:
+            response.raise_for_status()
+        except HTTPError:
+            # don't cache failed response
+            return response
+
+        value: ResponseDict = {"json": response.json(), "status_code": response.status_code}
+        cache.set(cache_key, value, self._cache_ttl)
+
+        return response

src/country_workspace/contrib/kobo/sync.py

@@ -1,9 +1,13 @@
-from typing import Any, TypedDict, cast
+import re
+from typing import Any, TypedDict, cast, Final
 
 from constance import config as constance_config
 from django.core.cache import cache
+from requests import Session
 
+from country_workspace.contrib.kobo.api.client.auth import Auth
 from country_workspace.contrib.kobo.api.client.main import Client
+from country_workspace.contrib.kobo.api.common import DataGetter
 from country_workspace.contrib.kobo.api.data.asset import Asset
 from country_workspace.contrib.kobo.api.data.submission import Submission
 from country_workspace.contrib.kobo.models import KoboSubmission
@@ -17,12 +21,30 @@ class Config(BatchNameConfig, FailIfAlienConfig):
     individual_records_field: str
 
 
+ACCEPT_JSON_HEADERS: Final[dict[str, str]] = {"Accept": "application/json"}
+
+SUBMISSION_URL_RE = re.compile(".+/assets/[^/]+/data/.*")
+
+
+def is_submission_data_url(url: str) -> bool:
+    return bool(SUBMISSION_URL_RE.match(url))
+
+
 def make_client(country_code: str | None) -> Client:
+    session = Session()
     token = constance_config.KOBO_MASTER_API_TOKEN or constance_config.KOBO_API_TOKEN
+    session.auth = Auth(token)
+    data_getter = DataGetter(
+        session=session,
+        cache_ttl=constance_config.KOBO_CACHE_TTL,
+        headers=ACCEPT_JSON_HEADERS,
+        do_not_use_cache_if=is_submission_data_url,
+    )
     project_view_id = constance_config.KOBO_PROJECT_VIEW_ID if constance_config.KOBO_MASTER_API_TOKEN else None
+
     return Client(
+        data_getter=data_getter,
         base_url=constance_config.KOBO_KF_URL,
-        token=token,
         country_code=country_code,
         project_view_id=project_view_id,
     )

tests/contrib/kobo/api/client/test_client_main.py

@@ -1,32 +1,25 @@
+from unittest.mock import Mock
+
 from pytest_mock import MockerFixture
 
-from country_workspace.contrib.kobo.api.client.main import ACCEPT_JSON_HEADERS, Client
+from country_workspace.contrib.kobo.api.client.main import Client
 
 
 def test_client(mocker: MockerFixture) -> None:
-    session_class = mocker.patch("country_workspace.contrib.kobo.api.client.main.Session")
-    session = session_class.return_value
-    auth_class = mocker.patch("country_workspace.contrib.kobo.api.client.main.Auth")
-    auth = auth_class.return_value
-    partial = mocker.patch("country_workspace.contrib.kobo.api.client.main.partial")
-    data_getter = partial.return_value
+    data_getter_mock = Mock()
     get_asset_list_url = mocker.patch("country_workspace.contrib.kobo.api.client.main.get_asset_list_url")
     url = get_asset_list_url.return_value
     get_asset_list = mocker.patch("country_workspace.contrib.kobo.api.client.main.get_asset_list")
     get_asset_list.return_value = []
 
     tuple(
         Client(
+            data_getter=data_getter_mock,
             base_url=(base_url := "https://test.org"),
-            token=(token := "test-token"),
             country_code=(country_code := "CNT"),
             project_view_id=(project_view_id := "project-view-id"),
         ).assets
     )
 
     get_asset_list_url.assert_called_once_with(base_url, project_view_id, country_code)
-    session_class.assert_called_once()
-    auth_class.assert_called_once_with(token)
-    assert session.auth == auth
-    partial.assert_called_once_with(session.get, headers=ACCEPT_JSON_HEADERS)
-    get_asset_list.assert_called_with(data_getter, url)
+    get_asset_list.assert_called_with(data_getter_mock, url)

tests/contrib/kobo/api/common/test_cached_response.py

@@ -0,0 +1,14 @@
+from country_workspace.contrib.kobo.api.common import CachedResponse
+
+
+def test_cached_response() -> None:
+    cached_response = CachedResponse(
+        {
+            "json": (expected_json := {"foo": "bar"}),
+            "status_code": (expected_status_code := 42),
+        }
+    )
+    assert cached_response.json() == expected_json
+    assert cached_response.status_code == expected_status_code
+    # we should not get an exception here
+    cached_response.raise_for_status()

tests/contrib/kobo/api/common/test_data_getter.py

@@ -0,0 +1,101 @@
+from collections.abc import Generator
+from unittest.mock import Mock, MagicMock
+
+import pytest
+from pytest_mock import MockFixture
+from requests import HTTPError
+
+from country_workspace.contrib.kobo.api.common import DataGetter
+
+
+URL = "https://test.org"
+CACHE_TTL = 42
+
+
+@pytest.fixture
+def cache_mock(mocker: MockFixture) -> Mock:
+    return mocker.patch("country_workspace.contrib.kobo.api.common.cache")
+
+
+@pytest.fixture
+def session_mock() -> Generator[Mock, None, None]:
+    return MagicMock(name="Session()")
+
+
+@pytest.fixture
+def cached_response_class_mock(mocker: MockFixture) -> Mock:
+    return mocker.patch("country_workspace.contrib.kobo.api.common.CachedResponse")
+
+
+@pytest.fixture
+def data_getter_cache_key_mock(mocker: MockFixture) -> Mock:
+    return mocker.patch("country_workspace.contrib.kobo.api.common.data_getter_cache_key")
+
+
+def test_cache_can_be_skipped(cache_mock: Mock, session_mock: Mock) -> None:
+    function = MagicMock()
+    function.return_value = True
+
+    data_getter = DataGetter(
+        session=session_mock,
+        cache_ttl=CACHE_TTL,
+        do_not_use_cache_if=function,
+    )
+    response = data_getter(URL)
+
+    assert response == session_mock.get.return_value
+    session_mock.get.assert_called_with(URL, headers=None)
+    function.assert_called_once_with(URL)
+    cache_mock.assert_not_called()
+
+
+def test_cached_value_is_returned(
+    cache_mock: Mock, session_mock: Mock, cached_response_class_mock: Mock, data_getter_cache_key_mock: Mock
+) -> None:
+    data_getter = DataGetter(
+        session=session_mock,
+        cache_ttl=CACHE_TTL,
+    )
+    response = data_getter(URL)
+
+    assert response == cached_response_class_mock.return_value
+    cached_response_class_mock.assert_called_once_with(cache_mock.get.return_value)
+    session_mock.get.assert_not_called()
+    cache_mock.get.assert_called_once_with(data_getter_cache_key_mock.return_value)
+    data_getter_cache_key_mock.assert_called_once_with(URL)
+
+
+def test_failing_response_is_not_cached(cache_mock: Mock, session_mock: Mock) -> None:
+    cache_mock.get.return_value = None
+    session_mock.get.return_value.raise_for_status.side_effect = HTTPError()
+
+    data_getter = DataGetter(
+        session=session_mock,
+        cache_ttl=CACHE_TTL,
+    )
+    response = data_getter(URL)
+
+    assert response == session_mock.get.return_value
+    cache_mock.set.assert_not_called()
+
+
+def test_response_is_cached(
+    cache_mock: Mock, session_mock: Mock, cached_response_class_mock: Mock, data_getter_cache_key_mock: Mock
+) -> None:
+    cache_mock.get.return_value = None
+
+    data_getter = DataGetter(
+        session=session_mock,
+        cache_ttl=CACHE_TTL,
+    )
+    response = data_getter(URL)
+
+    assert response == session_mock.get.return_value
+    cache_mock.set.assert_called_once_with(
+        data_getter_cache_key_mock.return_value,
+        {
+            "json": session_mock.get.return_value.json.return_value,
+            "status_code": session_mock.get.return_value.status_code,
+        },
+        CACHE_TTL,
+    )

tests/contrib/kobo/api/common/test_is_submission_data_url.py

@@ -0,0 +1,15 @@
+import pytest
+
+from country_workspace.contrib.kobo.sync import is_submission_data_url
+
+
+@pytest.mark.parametrize(
+    ("url", "expected"),
+    [
+        ("", False),
+        ("https://example.com", False),
+        ("https://example.com/api/v2/assets/abc42/data/", True),
+    ],
+)
+def test_is_submission_data_url(url: str, expected: bool) -> None:
+    assert is_submission_data_url(url) is expected