Repository: bvanelli/actualpy
Pull request #84 (2024-10-11 16:26:47)

perf: Improve performance of API queries by using the same session for all
Due to Actual rate limiting, the requests are throttled if coming from "different clients". This means that using the same session for all the requests make them faster.

Here is a benchmark before and after changes, for exporting a bit budget with many changes:

- Before changes: 35.13s
- After changes: 10.90s

This should only affect larger budgets that are remote.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/bvanelli/actualpy/pull/84?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli) Report
Attention: Patch coverage is `92.59259%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 97.48%. Comparing base [(`0e866b5`)](https://app.codecov.io/gh/bvanelli/actualpy/commit/0e866b5e828ad49fb91175f8fc03950a90b0c55b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli) to head [(`4c4c268`)](https://app.codecov.io/gh/bvanelli/actualpy/commit/4c4c26899049f17b4a68f8d5e75da15ff3a32a35?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli).

| [Files with missing lines](https://app.codecov.io/gh/bvanelli/actualpy/pull/84?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli) | Patch % | Lines |
|---|---|---|
| [actual/api/\_\_init\_\_.py](https://app.codecov.io/gh/bvanelli/actualpy/pull/84?src=pr&el=tree&filepath=actual%2Fapi%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli#diff-YWN0dWFsL2FwaS9fX2luaXRfXy5weQ==) | 92.59% | [2 Missing :warning: ](https://app.codecov.io/gh/bvanelli/actualpy/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main      #84      +/-   ##
==========================================
- Coverage   97.52%   97.48%   -0.05%     
==========================================
  Files          16       16              
  Lines        2184     2187       +3     
==========================================
+ Hits         2130     2132       +2     
- Misses         54       55       +1     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/bvanelli/actualpy/pull/84?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli).


Patch:
diff --git a/actual/api/__init__.py b/actual/api/__init__.py
index 9f03605..c67f410 100644
--- a/actual/api/__init__.py
+++ b/actual/api/__init__.py
@@ -51,16 +51,20 @@ class ActualServer:
         :param cert: if a custom certificate should be used (i.e. self-signed certificate), it's path can be provided
                      as a string. Set to `False` for no certificate check.
         """
-        self.api_url = base_url
-        self._token = token
-        self.cert = cert
+        self.api_url: str = base_url
+        self._token: str | None = token
+        self._requests_session: requests.Session = requests.Session()
+        if cert is not None:
+            self._requests_session.verify = cert
         if token is None and password is None:
             raise ValueError("Either provide a valid token or a password.")
-        # already try to login if password was provided
+        # already try to log-in if password was provided
         if password and bootstrap and not self.needs_bootstrap().data.bootstrapped:
             self.bootstrap(password)
         elif password:
             self.login(password)
+        # set default headers for the connection
+        self._requests_session.headers = self.headers()
         # finally call validate
         self.validate()
 
@@ -72,17 +76,17 @@ class ActualServer:
         :param password: password of the Actual server.
         :param method: the method used to authenticate with the server. Check the [official auth header documentation](
         https://actualbudget.org/docs/advanced/http-header-auth/) for information.
+        :raises AuthorizationError: if the token is invalid.
         """
         if not password:
             raise AuthorizationError("Trying to login but not password was provided.")
         if method == "password":
-            response = requests.post(f"{self.api_url}/{Endpoints.LOGIN}", json={"password": password}, verify=self.cert)
+            response = self._requests_session.post(f"{self.api_url}/{Endpoints.LOGIN}", json={"password": password})
         else:
-            response = requests.post(
+            response = self._requests_session.post(
                 f"{self.api_url}/{Endpoints.LOGIN}",
                 json={"loginMethod": method},
                 headers={"X-ACTUAL-PASSWORD": password},
-                verify=self.cert,
             )
         response_dict = response.json()
         if response.status_code == 400 and "invalid-password" in response.text:
@@ -101,7 +105,9 @@ class ActualServer:
         return login_response
 
     def headers(self, file_id: str = None, extra_headers: dict = None) -> dict:
-        """Generates headers by retrieving a token, if one is not provided, and auto-filling the file id."""
+        """Generates a header based on the stored token for the connection. If a `file_id` is provided, it would be
+        used as the `X-ACTUAL-FILE-ID` header. Extra headers will be included as they are provided on the final
+        dictionary."""
         if not self._token:
             raise AuthorizationError("Token not available for requests. Use the login() method or provide a token.")
         headers = {"X-ACTUAL-TOKEN": self._token}
@@ -113,26 +119,24 @@ class ActualServer:
 
     def info(self) -> InfoDTO:
         """Gets the information from the Actual server, like the name and version."""
-        response = requests.get(f"{self.api_url}/{Endpoints.INFO}", verify=self.cert)
+        response = self._requests_session.get(f"{self.api_url}/{Endpoints.INFO}")
         response.raise_for_status()
         return InfoDTO.model_validate(response.json())
 
     def validate(self) -> ValidateDTO:
         """Validates if the user is valid and logged in, and if the token is also valid and bound to a session."""
-        response = requests.get(
-            f"{self.api_url}/{Endpoints.ACCOUNT_VALIDATE}", headers=self.headers(), verify=self.cert
-        )
+        response = self._requests_session.get(f"{self.api_url}/{Endpoints.ACCOUNT_VALIDATE}")
         response.raise_for_status()
         return ValidateDTO.model_validate(response.json())
 
     def needs_bootstrap(self) -> BootstrapInfoDTO:
         """Checks if the Actual needs bootstrap, in other words, if it needs a master password for the server."""
-        response = requests.get(f"{self.api_url}/{Endpoints.NEEDS_BOOTSTRAP}", verify=self.cert)
+        response = self._requests_session.get(f"{self.api_url}/{Endpoints.NEEDS_BOOTSTRAP}")
         response.raise_for_status()
         return BootstrapInfoDTO.model_validate(response.json())
 
     def bootstrap(self, password: str) -> LoginDTO:
-        response = requests.post(f"{self.api_url}/{Endpoints.BOOTSTRAP}", json={"password": password}, verify=self.cert)
+        response = self._requests_session.post(f"{self.api_url}/{Endpoints.BOOTSTRAP}", json={"password": password})
         response.raise_for_status()
         login_response = LoginDTO.model_validate(response.json())
         self._token = login_response.data.token
@@ -140,13 +144,13 @@ class ActualServer:
 
     def data_file_index(self) -> List[str]:
         """Gets all the migration file references for the actual server."""
-        response = requests.get(f"{self.api_url}/{Endpoints.DATA_FILE_INDEX}", verify=self.cert)
+        response = self._requests_session.get(f"{self.api_url}/{Endpoints.DATA_FILE_INDEX}")
         response.raise_for_status()
         return response.content.decode().splitlines()
 
     def data_file(self, file_path: str) -> bytes:
         """Gets the content of the individual migration file from server."""
-        response = requests.get(f"{self.api_url}/data/{file_path}", verify=self.cert)
+        response = self._requests_session.get(f"{self.api_url}/data/{file_path}")
         response.raise_for_status()
         return response.content
 
@@ -155,10 +159,8 @@ class ActualServer:
         the upload_user_file() method."""
         if file_id is None:
             raise UnknownFileId("Could not reset the file without a valid 'file_id'")
-        request = requests.post(
-            f"{self.api_url}/{Endpoints.RESET_USER_FILE}",
-            json={"fileId": file_id, "token": self._token},
-            verify=self.cert,
+        request = self._requests_session.post(
+            f"{self.api_url}/{Endpoints.RESET_USER_FILE}", json={"fileId": file_id, "token": self._token}
         )
         request.raise_for_status()
         return StatusDTO.model_validate(request.json())
@@ -167,9 +169,7 @@ class ActualServer:
         """Downloads the user file based on the file_id provided. Returns the `bytes` from the response, which is a
         zipped folder of the database `db.sqlite` and the `metadata.json`. If the database is encrypted, the key id
         has to be retrieved additionally using user_get_key()."""
-        db = requests.get(
-            f"{self.api_url}/{Endpoints.DOWNLOAD_USER_FILE}", headers=self.headers(file_id), verify=self.cert
-        )
+        db = self._requests_session.get(f"{self.api_url}/{Endpoints.DOWNLOAD_USER_FILE}", headers=self.headers(file_id))
         db.raise_for_status()
         return db.content
 
@@ -187,11 +187,10 @@ class ActualServer:
         }
         if encryption_meta:
             base_headers["X-ACTUAL-ENCRYPT-META"] = json.dumps(encryption_meta)
-        request = requests.post(
+        request = self._requests_session.post(
             f"{self.api_url}/{Endpoints.UPLOAD_USER_FILE}",
             data=binary_data,
             headers=self.headers(extra_headers=base_headers),
-            verify=self.cert,
         )
         request.raise_for_status()
         return UploadUserFileDTO.model_validate(request.json())
@@ -199,49 +198,43 @@ class ActualServer:
     def list_user_files(self) -> ListUserFilesDTO:
         """Lists the user files. If the response item contains `encrypt_key_id` different from `None`, then the
         file must be decrypted on retrieval."""
-        response = requests.get(f"{self.api_url}/{Endpoints.LIST_USER_FILES}", headers=self.headers(), verify=self.cert)
+        response = self._requests_session.get(f"{self.api_url}/{Endpoints.LIST_USER_FILES}")
         response.raise_for_status()
         return ListUserFilesDTO.model_validate(response.json())
 
     def get_user_file_info(self, file_id: str) -> GetUserFileInfoDTO:
         """Gets the user file information, including the encryption metadata."""
-        response = requests.get(
-            f"{self.api_url}/{Endpoints.GET_USER_FILE_INFO}", headers=self.headers(file_id), verify=self.cert
+        response = self._requests_session.get(
+            f"{self.api_url}/{Endpoints.GET_USER_FILE_INFO}", headers=self.headers(file_id)
         )
         response.raise_for_status()
         return GetUserFileInfoDTO.model_validate(response.json())
 
     def update_user_file_name(self, file_id: str, file_name: str) -> StatusDTO:
         """Updates the file name for the budget on the remote server."""
-        response = requests.post(
+        response = self._requests_session.post(
             f"{self.api_url}/{Endpoints.UPDATE_USER_FILE_NAME}",
             json={"fileId": file_id, "name": file_name, "token": self._token},
-            headers=self.headers(),
-            verify=self.cert,
         )
         response.raise_for_status()
         return StatusDTO.model_validate(response.json())
 
     def delete_user_file(self, file_id: str):
         """Deletes the user file that is loaded from the remote server."""
-        response = requests.post(
-            f"{self.api_url}/{Endpoints.DELETE_USER_FILE}",
-            json={"fileId": file_id, "token": self._token},
-            headers=self.headers(),
-            verify=self.cert,
+        response = self._requests_session.post(
+            f"{self.api_url}/{Endpoints.DELETE_USER_FILE}", json={"fileId": file_id, "token": self._token}
         )
         return StatusDTO.model_validate(response.json())
 
     def user_get_key(self, file_id: str) -> UserGetKeyDTO:
         """Gets the key information associated with a user file, including the algorithm, key, salt and iv."""
-        response = requests.post(
+        response = self._requests_session.post(
             f"{self.api_url}/{Endpoints.USER_GET_KEY}",
             json={
                 "fileId": file_id,
                 "token": self._token,
             },
             headers=self.headers(file_id),
-            verify=self.cert,
         )
         response.raise_for_status()
         return UserGetKeyDTO.model_validate(response.json())
@@ -251,9 +244,8 @@ class ActualServer:
         still needs to be uploaded."""
         key = create_key_buffer(password, key_salt)
         test_content = make_test_message(key_id, key)
-        response = requests.post(
+        response = self._requests_session.post(
             f"{self.api_url}/{Endpoints.USER_CREATE_KEY}",
-            headers=self.headers(),
             json={
                 "fileId": file_id,
                 "keyId": key_id,
@@ -261,7 +253,6 @@ class ActualServer:
                 "testContent": json.dumps(test_content),
                 "token": self._token,
             },
-            verify=self.cert,
         )
         return StatusDTO.model_validate(response.json())
 
@@ -270,11 +261,10 @@ class ActualServer:
         protobuf models. The request and response are not standard REST, but rather protobuf binary serialized data.
         The server stores this serialized data to allow the user to replay all changes to the database and construct
         a local copy."""
-        response = requests.post(
+        response = self._requests_session.post(
             f"{self.api_url}/{Endpoints.SYNC}",
             headers=self.headers(request.fileId, extra_headers={"Content-Type": "application/actual-sync"}),
             data=SyncRequest.serialize(request),
-            verify=self.cert,
         )
         response.raise_for_status()
         parsed_response = SyncResponse.deserialize(response.content)
@@ -282,13 +272,13 @@ class ActualServer:
 
     def bank_sync_status(self, bank_sync: Literal["gocardless", "simplefin"] | str) -> BankSyncStatusDTO:
         endpoint = Endpoints.BANK_SYNC_STATUS.value.format(bank_sync=bank_sync)
-        response = requests.post(f"{self.api_url}/{endpoint}", headers=self.headers(), json={}, verify=self.cert)
+        response = self._requests_session.post(f"{self.api_url}/{endpoint}", json={})
         return BankSyncStatusDTO.model_validate(response.json())
 
     def bank_sync_accounts(self, bank_sync: Literal["gocardless", "simplefin"]) -> BankSyncAccountResponseDTO:
         endpoint = Endpoints.BANK_SYNC_ACCOUNTS.value.format(bank_sync=bank_sync)
-        response = requests.post(f"{self.api_url}/{endpoint}", headers=self.headers(), json={}, verify=self.cert)
-        return BankSyncAccountResponseDTO.model_validate(response.json())
+        response = self._requests_session.post(f"{self.api_url}/{endpoint}", json={})
+        return BankSyncAccountResponseDTO.validate_python(response.json())
 
     def bank_sync_transactions(
         self,
@@ -303,5 +293,5 @@ class ActualServer:
         payload = {"accountId": account_id, "startDate": start_date.strftime("%Y-%m-%d")}
         if requisition_id:
             payload["requisitionId"] = requisition_id
-        response = requests.post(f"{self.api_url}/{endpoint}", headers=self.headers(), json=payload, verify=self.cert)
+        response = self._requests_session.post(f"{self.api_url}/{endpoint}", json=payload)
         return BankSyncResponseDTO.validate_python(response.json())
diff --git a/actual/api/models.py b/actual/api/models.py
index 8be9ebd..480de3c 100644
--- a/actual/api/models.py
+++ b/actual/api/models.py
@@ -156,7 +156,7 @@ class BankSyncStatusDTO(StatusDTO):
     data: IsConfiguredDTO
 
 
-class BankSyncAccountResponseDTO(StatusDTO):
+class BankSyncAccountDTO(StatusDTO):
     data: BankSyncAccountData
 
 
@@ -168,4 +168,5 @@ class BankSyncErrorDTO(StatusDTO):
     data: BankSyncErrorData
 
 
+BankSyncAccountResponseDTO = TypeAdapter(Union[BankSyncErrorDTO, BankSyncAccountDTO])
 BankSyncResponseDTO = TypeAdapter(Union[BankSyncErrorDTO, BankSyncTransactionResponseDTO])
diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml
index 92c0c57..4d22a7d 100644
--- a/docker/docker-compose.yml
+++ b/docker/docker-compose.yml
@@ -2,7 +2,7 @@
 services:
   actual:
     container_name: actual
-    image: docker.io/actualbudget/actual-server:24.10.0
+    image: docker.io/actualbudget/actual-server:24.10.1
     ports:
       - '5006:5006'
     volumes:


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index 3722aec..bedba20 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -1,6 +1,7 @@
 from unittest.mock import patch
 
 import pytest
+from requests import Session
 
 from actual import Actual, reflect_model
 from actual.exceptions import ActualError, AuthorizationError, UnknownFileId
@@ -33,7 +34,7 @@ def test_rename_delete_budget_without_file(mocker):
         actual.rename_budget("foo")
 
 
-@patch("requests.post", return_value=RequestsMock({"status": "error", "reason": "proxy-not-trusted"}))
+@patch.object(Session, "post", return_value=RequestsMock({"status": "error", "reason": "proxy-not-trusted"}))
 def test_api_login_unknown_error(_post, mocker):
     mocker.patch("actual.Actual.validate")
     actual = Actual(token="foo")
@@ -41,3 +42,9 @@ def test_api_login_unknown_error(_post, mocker):
     actual.cert = False
     with pytest.raises(AuthorizationError, match="Something went wrong on login"):
         actual.login("foo")
+
+
+def test_no_certificate(mocker):
+    mocker.patch("actual.Actual.validate")
+    actual = Actual(token="foo", cert=False)
+    assert actual._requests_session.verify is False
diff --git a/tests/test_bank_sync.py b/tests/test_bank_sync.py
index 0e9424f..094c5d8 100644
--- a/tests/test_bank_sync.py
+++ b/tests/test_bank_sync.py
@@ -3,6 +3,7 @@ import datetime
 import decimal
 
 import pytest
+from requests import Session
 
 from actual import Actual, ActualBankSyncError
 from actual.database import Banks
@@ -76,8 +77,8 @@ def set_mocks(mocker):
     # call for validate
     response_empty = copy.deepcopy(response)
     response_empty["transactions"]["all"] = []
-    mocker.patch("requests.get").return_value = RequestsMock({"status": "ok", "data": {"validated": True}})
-    main_mock = mocker.patch("requests.post")
+    mocker.patch.object(Session, "get").return_value = RequestsMock({"status": "ok", "data": {"validated": True}})
+    main_mock = mocker.patch.object(Session, "post")
     main_mock.side_effect = [
         RequestsMock({"status": "ok", "data": {"configured": True}}),
         RequestsMock({"status": "ok", "data": response}),
@@ -138,8 +139,8 @@ def test_full_bank_sync_go_simplefin(session, set_mocks):
 
 
 def test_bank_sync_unconfigured(mocker, session):
-    mocker.patch("requests.get").return_value = RequestsMock({"status": "ok", "data": {"validated": True}})
-    main_mock = mocker.patch("requests.post")
+    mocker.patch.object(Session, "get").return_value = RequestsMock({"status": "ok", "data": {"validated": True}})
+    main_mock = mocker.patch.object(Session, "post")
     main_mock.return_value = RequestsMock({"status": "ok", "data": {"configured": False}})
 
     with Actual(token="foo") as actual:
@@ -149,8 +150,8 @@ def test_bank_sync_unconfigured(mocker, session):
 
 
 def test_bank_sync_exception(session, mocker):
-    mocker.patch("requests.get").return_value = RequestsMock({"status": "ok", "data": {"validated": True}})
-    main_mock = mocker.patch("requests.post")
+    mocker.patch.object(Session, "get").return_value = RequestsMock({"status": "ok", "data": {"validated": True}})
+    main_mock = mocker.patch.object(Session, "post")
     main_mock.side_effect = [
         RequestsMock({"status": "ok", "data": {"configured": True}}),
         RequestsMock({"status": "ok", "data": fail_response}),
diff --git a/tests/test_integration.py b/tests/test_integration.py
index c41052c..cd62a4c 100644
--- a/tests/test_integration.py
+++ b/tests/test_integration.py
@@ -23,7 +23,7 @@ from actual.queries import (
 )
 
 
-@pytest.fixture(params=["24.8.0", "24.9.0", "24.10.0"])  # todo: support multiple versions at once
+@pytest.fixture(params=["24.8.0", "24.9.0", "24.10.0", "24.10.1"])  # todo: support multiple versions at once
 def actual_server(request):
     # we test integration with the 5 latest versions of actual server
     with DockerContainer(f"actualbudget/actual-server:{request.param}").with_exposed_ports(5006) as container:
@@ -54,7 +54,10 @@ def test_create_user_file(actual_server):
         # run rules
         actual.run_rules()
         # run bank sync
-        actual.run_bank_sync()
+        assert actual.run_bank_sync() == []
+        # check also bank sync accounts, should fail because of no token
+        assert actual.bank_sync_accounts("simplefin").data.error_type == "INVALID_ACCESS_TOKEN"
+        # same test with goCardless returns 404 for some reason, so we don't do that
 
     # make sure a new instance can now retrieve the budget info
     with Actual(f"http://localhost:{port}", password="mypass", file="My Budget"):