refactor(toggl): pull out JsonFileCache class

Author: thekaveman

compiler_admin/services/files.py

@@ -1,4 +1,6 @@
 import json
+import os
+from pathlib import Path
 
 import pandas as pd
 
@@ -23,3 +25,26 @@ def write_json(file_path: str, data):
     """Write a python object as JSON to the given path."""
     with open(file_path, "w") as f:
         json.dump(data, f, indent=2)
+
+
+class JsonFileCache:
+    """Very basic in-memory cache of a JSON file."""
+
+    def __init__(self, env_file_path=None):
+        self._cache = {}
+        self._path = None
+
+        if env_file_path:
+            p = os.environ.get(env_file_path)
+            self._path = Path(p) if p else None
+        if self._path and self._path.exists():
+            self._cache.update(read_json(self._path))
+
+    def __getitem__(self, key):
+        return self._cache.get(key)
+
+    def __setitem__(self, key, value):
+        self._cache[key] = value
+
+    def get(self, key, default=None):
+        return self._cache.get(key, default)

compiler_admin/services/toggl.py

@@ -12,11 +12,8 @@
 from compiler_admin.services.google import user_info as google_user_info
 import compiler_admin.services.files as files
 
-# cache of previously seen project information, keyed on Toggl project name
-PROJECT_INFO = {}
-
 # cache of previously seen user information, keyed on email
-USER_INFO = {}
+USER_INFO = files.JsonFileCache("TOGGL_USER_INFO")
 NOT_FOUND = "NOT FOUND"
 
 # input CSV columns needed for conversion
@@ -130,29 +127,9 @@ def detailed_time_entries(self, start_date: datetime, end_date: datetime, **kwar
         return response
 
 
-def _get_info(obj: dict, key: str, env_key: str):
-    """Read key from obj, populating obj once from a file path at env_key."""
-    if obj == {}:
-        file_path = os.environ.get(env_key)
-        if file_path:
-            file_info = files.read_json(file_path)
-            obj.update(file_info)
-    return obj.get(key)
-
-
-def _toggl_project_info(project: str):
-    """Return the cached project for the given project key."""
-    return _get_info(PROJECT_INFO, project, "TOGGL_PROJECT_INFO")
-
-
-def _toggl_user_info(email: str):
-    """Return the cached user for the given email."""
-    return _get_info(USER_INFO, email, "TOGGL_USER_INFO")
-
-
 def _get_first_name(email: str) -> str:
     """Get cached first name or derive from email."""
-    user = _toggl_user_info(email)
+    user = USER_INFO.get(email)
     first_name = user.get("First Name") if user else None
     if first_name is None:
         parts = email.split("@")
@@ -167,7 +144,7 @@ def _get_first_name(email: str) -> str:
 
 def _get_last_name(email: str):
     """Get cached last name or query from Google."""
-    user = _toggl_user_info(email)
+    user = USER_INFO.get(email)
     last_name = user.get("Last Name") if user else None
     if last_name is None:
         user = google_user_info(email)
@@ -179,7 +156,7 @@ def _get_last_name(email: str):
     return last_name
 
 
-def _str_timedelta(td):
+def _str_timedelta(td: str):
     """Convert a string formatted duration (e.g. 01:30) to a timedelta."""
     return pd.to_timedelta(pd.to_datetime(td, format="%H:%M:%S").strftime("%H:%M:%S"))
 
@@ -220,8 +197,9 @@ def convert_to_harvest(
     source["Client"] = client_name
     source["Task"] = "Project Consulting"
 
-    # get cached project name if any
-    source["Project"] = source["Project"].apply(lambda x: _toggl_project_info(x) or x)
+    # get cached project name if any, keyed on Toggl project name
+    project_info = files.JsonFileCache("TOGGL_PROJECT_INFO")
+    source["Project"] = source["Project"].apply(lambda x: project_info.get(key=x, default=x))
 
     # assign First and Last name
     source["First name"] = source["Email"].apply(_get_first_name)

tests/services/test_files.py

@@ -3,7 +3,7 @@
 import pytest
 
 import compiler_admin.services.files
-from compiler_admin.services.files import pd, read_csv, read_json, write_csv, write_json
+from compiler_admin.services.files import JsonFileCache, pd, read_csv, read_json, write_csv, write_json
 
 
 @pytest.fixture
@@ -81,3 +81,30 @@ def test_write_json(sample_data, temp_file):
 
     with open(temp_file.name, "rt") as f:
         assert json.load(f) == sample_data
+
+
+def test_JsonFileCache(monkeypatch):
+    with NamedTemporaryFile("w") as temp:
+        monkeypatch.setenv("INFO_FILE", temp.name)
+        temp.write('{"key": "value"}')
+        temp.seek(0)
+
+        cache = JsonFileCache("INFO_FILE")
+
+        assert cache._path.exists()
+        assert cache.get("key") == "value"
+        assert cache["key"] == "value"
+        assert cache.get("other") is None
+        assert cache["other"] is None
+
+        cache["key"] = "other"
+        assert cache.get("key") == "other"
+        assert cache["key"] == "other"
+
+
+def test_JsonFileCache_no_file():
+    cache = JsonFileCache("INFO_FILE")
+
+    assert cache._cache == {}
+    assert cache._path is None
+    assert cache.get("key") is None

tests/services/test_toggl.py

@@ -14,12 +14,7 @@
     files,
     INPUT_COLUMNS,
     OUTPUT_COLUMNS,
-    PROJECT_INFO,
-    USER_INFO,
     Toggl,
-    _get_info,
-    _toggl_project_info,
-    _toggl_user_info,
     _get_first_name,
     _get_last_name,
     _str_timedelta,
@@ -35,19 +30,14 @@ def mock_environment(monkeypatch):
     monkeypatch.setenv("TOGGL_USER_INFO", "notebooks/data/toggl-user-info-sample.json")
 
 
-@pytest.fixture(autouse=True)
-def reset_USER_INFO():
-    USER_INFO.clear()
-
-
 @pytest.fixture
 def spy_files(mocker):
     return mocker.patch.object(compiler_admin.services.toggl, "files", wraps=files)
 
 
-@pytest.fixture
-def mock_get_info(mocker):
-    return mocker.patch(f"{MODULE}._get_info")
+@pytest.fixture(autouse=True)
+def mock_USER_INFO(mocker):
+    return mocker.patch(f"{MODULE}.USER_INFO", new={})
 
 
 @pytest.fixture
@@ -163,103 +153,68 @@ def test_toggl_detailed_time_entries_dynamic_timeout(mock_requests, toggl):
     assert mock_requests.post.call_args.kwargs["timeout"] == 30
 
 
-def test_get_info(monkeypatch):
-    with NamedTemporaryFile("w") as temp:
-        monkeypatch.setenv("INFO_FILE", temp.name)
-        temp.write('{"key": "value"}')
-        temp.seek(0)
-
-        obj = {}
-        result = _get_info(obj, "key", "INFO_FILE")
-
-        assert result == "value"
-        assert obj["key"] == "value"
-
-
-def test_get_info_no_file():
-    obj = {}
-    result = _get_info(obj, "key", "INFO_FILE")
-
-    assert result is None
-    assert "key" not in obj
-
-
-def test_toggl_project_info(mock_get_info):
-    _toggl_project_info("project")
-
-    mock_get_info.assert_called_once_with(PROJECT_INFO, "project", "TOGGL_PROJECT_INFO")
-
-
-def test_toggl_user_info(mock_get_info):
-    _toggl_user_info("user")
-
-    mock_get_info.assert_called_once_with(USER_INFO, "user", "TOGGL_USER_INFO")
-
-
-def test_get_first_name_matching(mock_get_info):
-    mock_get_info.return_value = {"First Name": "User"}
+def test_get_first_name_matching(mock_USER_INFO):
+    mock_USER_INFO["email"] = {"First Name": "User"}
 
     result = _get_first_name("email")
 
     assert result == "User"
 
 
-def test_get_first_name_calcuated_with_record(mock_get_info):
+def test_get_first_name_calcuated_with_record(mock_USER_INFO):
     email = "user@email.com"
-    mock_get_info.return_value = {}
-    USER_INFO[email] = {"Data": 1234}
+    mock_USER_INFO[email] = {"Data": 1234}
 
     result = _get_first_name(email)
 
     assert result == "User"
-    assert USER_INFO[email]["First Name"] == "User"
-    assert USER_INFO[email]["Data"] == 1234
+    assert mock_USER_INFO[email]["First Name"] == "User"
+    assert mock_USER_INFO[email]["Data"] == 1234
 
 
-def test_get_first_name_calcuated_without_record(mock_get_info):
+def test_get_first_name_calcuated_without_record(mock_USER_INFO):
     email = "user@email.com"
-    mock_get_info.return_value = {}
+    mock_USER_INFO[email] = {}
 
     result = _get_first_name(email)
 
     assert result == "User"
-    assert USER_INFO[email]["First Name"] == "User"
-    assert list(USER_INFO[email].keys()) == ["First Name"]
+    assert mock_USER_INFO[email]["First Name"] == "User"
+    assert list(mock_USER_INFO[email].keys()) == ["First Name"]
 
 
-def test_get_last_name_matching(mock_get_info, mock_google_user_info):
-    mock_get_info.return_value = {"Last Name": "User"}
+def test_get_last_name_matching(mock_USER_INFO, mock_google_user_info):
+    mock_USER_INFO["email"] = {"Last Name": "User"}
 
     result = _get_last_name("email")
 
     assert result == "User"
     mock_google_user_info.assert_not_called()
 
 
-def test_get_last_name_lookup_with_record(mock_get_info, mock_google_user_info):
+def test_get_last_name_lookup_with_record(mock_USER_INFO, mock_google_user_info):
     email = "user@email.com"
-    mock_get_info.return_value = {}
-    USER_INFO[email] = {"Data": 1234}
+    mock_USER_INFO[email] = {"Data": 1234}
     mock_google_user_info.return_value = {"Last Name": "User"}
 
     result = _get_last_name(email)
 
     assert result == "User"
-    assert USER_INFO[email]["Last Name"] == "User"
-    assert USER_INFO[email]["Data"] == 1234
+    assert mock_USER_INFO[email]["Last Name"] == "User"
+    assert mock_USER_INFO[email]["Data"] == 1234
     mock_google_user_info.assert_called_once_with(email)
 
 
-def test_get_last_name_lookup_without_record(mock_get_info, mock_google_user_info):
+def test_get_last_name_lookup_without_record(mock_USER_INFO, mock_google_user_info):
     email = "user@email.com"
-    mock_get_info.return_value = {}
+    mock_USER_INFO[email] = {}
     mock_google_user_info.return_value = {"Last Name": "User"}
 
     result = _get_last_name(email)
 
     assert result == "User"
-    assert USER_INFO[email]["Last Name"] == "User"
-    assert list(USER_INFO[email].keys()) == ["Last Name"]
+    assert mock_USER_INFO[email]["Last Name"] == "User"
+    assert list(mock_USER_INFO[email].keys()) == ["Last Name"]
     mock_google_user_info.assert_called_once_with(email)