Repository: bvanelli/actualpy
Pull request #65 (2024-09-04 16:35:59)

feat: Support reflecting models on download.
- Adds reflection when loading the database from Actual. This means that unknown columns will not be supported even when they are not present on the model. This should ensure that migrations will not break as often.
- Possibly dangerous migrations and the method `create_budget` now raise warnings due to the impossibility of running custom Javascript code on the Python wrapper.
- Support for Actual Server 24.9.0
- Reworked the parsing of the JS migrations to account for changes done in Actual 24.9.0

Thanks to @HamedHemati for the contributions on #hackergarten

Closes https://github.com/bvanelli/actualpy/issues/66 

Closes https://github.com/bvanelli/actualpy/issues/58

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/bvanelli/actualpy/pull/65?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 `98.43750%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 96.93%. Comparing base [(`846b09d`)](https://app.codecov.io/gh/bvanelli/actualpy/commit/846b09d13fff6772dc461c9f5d11707225be6e8e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli) to head [(`9552ab3`)](https://app.codecov.io/gh/bvanelli/actualpy/commit/9552ab37032238c0093b76dae460a395a5bd5da0?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/65?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/migrations.py](https://app.codecov.io/gh/bvanelli/actualpy/pull/65?src=pr&el=tree&filepath=actual%2Fmigrations.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brunno+Vanelli#diff-YWN0dWFsL21pZ3JhdGlvbnMucHk=) | 96.96% | [1 Missing :warning: ](https://app.codecov.io/gh/bvanelli/actualpy/pull/65?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      #65      +/-   ##
==========================================
- Coverage   96.95%   96.93%   -0.03%     
==========================================
  Files          13       14       +1     
  Lines        1842     1894      +52     
==========================================
+ Hits         1786     1836      +50     
- Misses         56       58       +2     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/bvanelli/actualpy/pull/65?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/__init__.py b/actual/__init__.py
index c7f01fa..d14fc1f 100644
--- a/actual/__init__.py
+++ b/actual/__init__.py
@@ -5,15 +5,16 @@ import datetime
 import io
 import json
 import pathlib
-import re
 import sqlite3
 import tempfile
 import uuid
+import warnings
 import zipfile
 from os import PathLike
 from typing import IO, Union
 
-from sqlmodel import Session, create_engine, select
+from sqlalchemy import insert, update
+from sqlmodel import MetaData, Session, create_engine, select
 
 from actual.api import ActualServer
 from actual.api.models import RemoteFileListDTO
@@ -22,11 +23,13 @@ from actual.database import (
     Accounts,
     MessagesClock,
     Transactions,
-    get_attribute_by_table_name,
-    get_class_by_table_name,
+    get_attribute_from_reflected_table_name,
+    get_class_from_reflected_table_name,
+    reflect_model,
     strong_reference_session,
 )
 from actual.exceptions import ActualError, InvalidZipFile, UnknownFileId
+from actual.migrations import js_migration_statements
 from actual.protobuf_models import HULC_Client, Message, SyncRequest
 from actual.queries import (
     get_account,
@@ -76,6 +79,7 @@ class Actual(ActualServer):
         self.engine = None
         self._session: Session | None = None
         self._client: HULC_Client | None = None
+        self._meta: MetaData | None = None  # stores the metadata loaded from remote
         # set the correct file
         if file:
             self.set_file(file)
@@ -139,7 +143,7 @@ class Actual(ActualServer):
             sql_statements = migration.decode()
             if file.endswith(".js"):
                 # there is one migration which is Javascript. All entries inside db.execQuery(`...`) must be executed
-                exec_entries = re.findall(r"db\.execQuery\(`([^`]*)`\)", sql_statements, re.DOTALL)
+                exec_entries = js_migration_statements(sql_statements)
                 sql_statements = "\n".join(exec_entries)
             conn.executescript(sql_statements)
             conn.execute(f"INSERT INTO __migrations__ (id) VALUES ({file_id});")
@@ -148,7 +152,10 @@ class Actual(ActualServer):
 
     def create_budget(self, budget_name: str):
         """Creates a budget using the remote server default database and migrations. If password is provided, the
-        budget will be encrypted."""
+        budget will be encrypted. It's important to note that `create_budget` depends on the migration files from the
+        Actual server, and those could be written in Javascript. Event though the library tries to execute all
+        statements in those files, is not an exact match. It is preferred to create budgets via frontend instead."""
+        warnings.warn("Creating budgets via actualpy is not recommended due to custom code migrations.")
         migration_files = self.data_file_index()
         # create folder for the files
         if not self._data_dir:
@@ -175,6 +182,8 @@ class Actual(ActualServer):
         self.engine = create_engine(f"sqlite:///{self._data_dir}/db.sqlite")
         if self._in_context:
             self._session = strong_reference_session(Session(self.engine, **self._sa_kwargs))
+        # reflect the session
+        self._meta = reflect_model(self.engine)
         # create a clock
         self.load_clock()
 
@@ -266,22 +275,21 @@ class Actual(ActualServer):
                     # write it to metadata.json instead
                     self.update_metadata({message.row: message.get_value()})
                     continue
-                table = get_class_by_table_name(message.dataset)
+                table = get_class_from_reflected_table_name(self._meta, message.dataset)
                 if table is None:
                     raise ActualError(
-                        f"Actual found a table not supported by the library: table '{message.dataset}' not found"
+                        f"Actual found a table not supported by the library: table '{message.dataset}' not found\n"
                     )
-                column = get_attribute_by_table_name(message.dataset, message.column)
+                column = get_attribute_from_reflected_table_name(self._meta, message.dataset, message.column)
                 if column is None:
                     raise ActualError(
                         f"Actual found a column not supported by the library: "
-                        f"column '{message.column}' at table '{message.dataset}' not found"
+                        f"column '{message.column}' at table '{message.dataset}' not found\n"
                     )
-                entry = s.get(table, message.row)
+                entry = s.exec(select(table).where(table.columns.id == message.row)).one_or_none()
                 if not entry:
-                    entry = table(id=message.row)
-                setattr(entry, column, message.get_value())
-                s.add(entry)
+                    s.exec(insert(table).values(id=message.row))
+                s.exec(update(table).values({column: message.get_value()}).where(table.columns.id == message.row))
                 # this seems to be required for sqlmodel, remove if not needed anymore when querying from cache
                 s.flush()
             s.commit()
@@ -341,6 +349,7 @@ class Actual(ActualServer):
         # this should extract 'db.sqlite' and 'metadata.json' to the folder
         zip_file.extractall(self._data_dir)
         self.engine = create_engine(f"sqlite:///{self._data_dir}/db.sqlite")
+        self._meta = reflect_model(self.engine)
         # load the client id
         self.load_clock()
 
diff --git a/actual/database.py b/actual/database.py
index a33d46e..07cd232 100644
--- a/actual/database.py
+++ b/actual/database.py
@@ -11,7 +11,7 @@ import datetime
 import decimal
 from typing import List, Optional, Union
 
-from sqlalchemy import event, inspect
+from sqlalchemy import MetaData, Table, engine, event, inspect
 from sqlalchemy.orm import class_mapper, object_session
 from sqlmodel import (
     Boolean,
@@ -51,6 +51,31 @@ that converts into the SQL equivalent `transactions.isParent`. In this case, we
 __TABLE_COLUMNS_MAP__ = dict()
 
 
+def reflect_model(eng: engine.Engine) -> MetaData:
+    """Reflects the current state of the database."""
+    local_meta = MetaData()
+    local_meta.reflect(bind=eng)
+    return local_meta
+
+
+def get_class_from_reflected_table_name(metadata: MetaData, table_name: str) -> Union[Table, None]:
+    """
+    Returns, based on the defined tables on the reflected model the corresponding SQLAlchemy table.
+    If not found, returns None.
+    """
+    return metadata.tables.get(table_name, None)
+
+
+def get_attribute_from_reflected_table_name(
+    metadata: MetaData, table_name: str, column_name: str
+) -> Union[Column, None]:
+    """
+    Returns, based, on the defined reflected model the corresponding and the SAColumn. If not found, returns None.
+    """
+    table = get_class_from_reflected_table_name(metadata, table_name)
+    return table.columns.get(column_name, None)
+
+
 def get_class_by_table_name(table_name: str) -> Union[SQLModel, None]:
     """
     Returns, based on the defined tables __tablename__ the corresponding SQLModel object. If not found, returns None.
@@ -340,6 +365,17 @@ class CustomReports(BaseModel, table=True):
     )
 
 
+class Dashboard(BaseModel, table=True):
+    id: Optional[str] = Field(default=None, sa_column=Column("id", Text, primary_key=True))
+    type: Optional[str] = Field(default=None, sa_column=Column("type", Text))
+    width: Optional[int] = Field(default=None, sa_column=Column("width", Integer))
+    height: Optional[int] = Field(default=None, sa_column=Column("height", Integer))
+    x: Optional[int] = Field(default=None, sa_column=Column("x", Integer))
+    y: Optional[int] = Field(default=None, sa_column=Column("y", Integer))
+    meta: Optional[str] = Field(default=None, sa_column=Column("meta", Text))
+    tombstone: Optional[int] = Field(default=None, sa_column=Column("tombstone", Integer, server_default=text("0")))
+
+
 class Kvcache(SQLModel, table=True):
     key: Optional[str] = Field(default=None, sa_column=Column("key", Text, primary_key=True))
     value: Optional[str] = Field(default=None, sa_column=Column("value", Text))
diff --git a/actual/migrations.py b/actual/migrations.py
new file mode 100644
index 0000000..c280583
--- /dev/null
+++ b/actual/migrations.py
@@ -0,0 +1,50 @@
+import re
+import uuid
+import warnings
+
+
+def js_migration_statements(js_file: str) -> list[str]:
+    queries = []
+    matches = re.finditer(r"db\.(execQuery|runQuery)", js_file)
+    for match in matches:
+        start_index, end_index = match.regs[0][1], match.regs[0][1]
+        # we now loop and find the first occasion where all parenthesis closed
+        parenthesis_count, can_return = 0, False
+        for i in range(start_index, len(js_file)):
+            if js_file[i] == "(":
+                can_return = True
+                parenthesis_count += 1
+            elif js_file[i] == ")":
+                parenthesis_count -= 1
+            if parenthesis_count == 0 and can_return:
+                end_index = i + 1
+                break
+        function_call = js_file[start_index:end_index]
+        # extract the query
+        next_tick = function_call.find("`")
+        next_quote = function_call.find("'")
+        string_character = "`" if next_tick > 0 and ((next_tick < next_quote) or next_quote < 0) else "'"
+        search = re.search(rf"{string_character}(.*?){string_character}", function_call, re.DOTALL)
+        if not search:
+            continue
+        query = search.group(1)
+        # skip empty queries
+        if not query:
+            continue
+        # skip select queries
+        if query.lower().startswith("select"):
+            continue
+        # if there are unknowns in the query, skip
+        if "?" in query:
+            warnings.warn(
+                f"Migration query from migrations cannot be executed due to custom code, it will be skipped. Query:\n\n"
+                f"{query}\n"
+            )
+            continue
+        # if there is an uuid generation, use it
+        while "${uuidv4()}" in query:
+            query = query.replace("${uuidv4()}", str(uuid.uuid4()), 1)
+        if not query.endswith(";"):
+            query = query + ";"
+        queries.append(query)
+    return queries
diff --git a/actual/version.py b/actual/version.py
index dad22a4..446b117 100644
--- a/actual/version.py
+++ b/actual/version.py
@@ -1,2 +1,2 @@
-__version_info__ = ("0", "3", "0")
+__version_info__ = ("0", "4", "0")
 __version__ = ".".join(__version_info__)
diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml
index e14f204..c59c568 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.8.0
+    image: docker.io/actualbudget/actual-server:24.9.0
     ports:
       - '5006:5006'
     volumes:


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index 4a879c5..3722aec 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -2,15 +2,17 @@ from unittest.mock import patch
 
 import pytest
 
-from actual import Actual
+from actual import Actual, reflect_model
 from actual.exceptions import ActualError, AuthorizationError, UnknownFileId
 from actual.protobuf_models import Message
 from tests.conftest import RequestsMock
 
 
-def test_api_apply(mocker):
-    actual = Actual.__new__(Actual)
-    actual.engine = mocker.MagicMock()
+def test_api_apply(mocker, session):
+    mocker.patch("actual.Actual.validate")
+    actual = Actual(token="foo")
+    actual.engine = session.bind
+    actual._meta = reflect_model(session.bind)
     # not found table
     m = Message(dict(dataset="foo", row="foobar", column="bar"))
     m.set_value("foobar")
@@ -21,8 +23,9 @@ def test_api_apply(mocker):
         actual.apply_changes([m])
 
 
-def test_rename_delete_budget_without_file():
-    actual = Actual.__new__(Actual)
+def test_rename_delete_budget_without_file(mocker):
+    mocker.patch("actual.Actual.validate")
+    actual = Actual(token="foo")
     actual._file = None
     with pytest.raises(UnknownFileId, match="No current file loaded"):
         actual.delete_budget()
@@ -31,8 +34,9 @@ def test_rename_delete_budget_without_file():
 
 
 @patch("requests.post", return_value=RequestsMock({"status": "error", "reason": "proxy-not-trusted"}))
-def test_api_login_unknown_error(_post):
-    actual = Actual.__new__(Actual)
+def test_api_login_unknown_error(_post, mocker):
+    mocker.patch("actual.Actual.validate")
+    actual = Actual(token="foo")
     actual.api_url = "localhost"
     actual.cert = False
     with pytest.raises(AuthorizationError, match="Something went wrong on login"):
diff --git a/tests/test_integration.py b/tests/test_integration.py
index 6c6ff7b..7aa4ea0 100644
--- a/tests/test_integration.py
+++ b/tests/test_integration.py
@@ -1,12 +1,11 @@
 import datetime
 
 import pytest
-import sqlalchemy.ext.declarative
 from testcontainers.core.container import DockerContainer
 from testcontainers.core.waiting_utils import wait_for_logs
 
-from actual import Actual
-from actual.database import __TABLE_COLUMNS_MAP__
+from actual import Actual, js_migration_statements
+from actual.database import __TABLE_COLUMNS_MAP__, reflect_model
 from actual.exceptions import ActualDecryptionError, ActualError, AuthorizationError
 from actual.queries import (
     create_transaction,
@@ -23,7 +22,7 @@ from actual.queries import (
 )
 
 
-@pytest.fixture(params=["24.8.0"])  # todo: support multiple versions at once
+@pytest.fixture(params=["24.8.0", "24.9.0"])  # 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:
@@ -131,9 +130,7 @@ def test_models(actual_server):
     with Actual(f"http://localhost:{port}", password="mypass", encryption_password="mypass", bootstrap=True) as actual:
         actual.create_budget("My Budget")
         # check if the models are matching
-        base = sqlalchemy.ext.declarative.declarative_base()
-        metadata = base.metadata
-        metadata.reflect(actual.engine)
+        metadata = reflect_model(actual.session.bind)
         # check first if all tables are present
         for table_name, table in metadata.tables.items():
             assert table_name in __TABLE_COLUMNS_MAP__, f"Missing table '{table_name}' on models."
@@ -157,3 +154,12 @@ def test_header_login():
         response_login = actual.login("mypass")
         response_header_login = actual.login("mypass", "header")
         assert response_login.data.token == response_header_login.data.token
+
+
+def test_empty_query_migrations():
+    # empty queries should not fail
+    assert js_migration_statements("await db.runQuery('');") == []
+    # malformed entries should not fail
+    assert js_migration_statements("await db.runQuery(") == []
+    # weird formats neither
+    assert js_migration_statements("db.runQuery\n('update 1')") == ["update 1;"]