From 5754f899f010babbb4dd1dcbe15b216ea12d4e08 Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Wed, 12 Aug 2026 14:56:52 +0200 Subject: [PATCH 01/12] feat: add read schema compatibility profiles --- src/moneywiz_api/database_accessor.py | 18 +++++++ src/moneywiz_api/managers/record_manager.py | 38 +++++++++++--- src/moneywiz_api/model/transaction.py | 1 - src/moneywiz_api/schema/raw_data_handler.py | 29 +++++++++-- src/moneywiz_api/schema_profile.py | 55 +++++++++++++++++++++ tests/unit/test_schema_profiles.py | 47 ++++++++++++++++++ 6 files changed, 175 insertions(+), 13 deletions(-) create mode 100644 src/moneywiz_api/schema_profile.py create mode 100644 tests/unit/test_schema_profiles.py diff --git a/src/moneywiz_api/database_accessor.py b/src/moneywiz_api/database_accessor.py index e951d2b..0c79b65 100644 --- a/src/moneywiz_api/database_accessor.py +++ b/src/moneywiz_api/database_accessor.py @@ -6,6 +6,7 @@ from moneywiz_api.schema.raw_data_handler import RawDataHandler as RDH from moneywiz_api.model.record import Record +from moneywiz_api.schema_profile import SchemaProfile, detect_schema_profile from moneywiz_api.types import ENT_ID, GID, ID from moneywiz_api.schema.schema_profile import SchemaProfile @@ -26,6 +27,7 @@ def dict_factory(cursor, row): return record self._con.row_factory = dict_factory + self._schema_profile = detect_schema_profile(self._con) self._ent_to_typename: Dict[ENT_ID, str] = self._load_primarykey() self._typename_to_ent: Dict[str, ENT_ID] = { @@ -49,6 +51,11 @@ def __repr__(self): f"{key}: {value}" for key, value in self._ent_to_typename.items() ) + @property + def schema_profile(self) -> SchemaProfile: + """Return the physical-column compatibility profile for this store.""" + return self._schema_profile + def typename_for(self, ent_id: ENT_ID) -> str: typename = self._ent_to_typename.get(ent_id) assert typename is not None, f"Unknown ent_id {ent_id}" @@ -59,6 +66,13 @@ def ent_for(self, typename: str) -> ENT_ID: assert ent_id is not None, f"Unknown typename {typename}" return ent_id + def _table_exists(self, table_name: str) -> bool: + row = self._con.execute( + "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = ?", + (table_name,), + ).fetchone() + return row is not None + def query_objects(self, typenames: List[str]) -> List[Any]: cur = self._con.cursor() res = cur.execute( @@ -96,6 +110,8 @@ def get_record_by_gid(self, gid: GID, constructor: type[Record] = Record): def get_category_assignment(self) -> Dict[ID, List[Tuple[ID, Decimal]]]: transaction_map: Dict[ID, List[Tuple[ID, Decimal]]] = defaultdict(list) + if not self._table_exists("ZCATEGORYASSIGMENT"): + return transaction_map cur = self._con.cursor() res = cur.execute( """ @@ -111,6 +127,8 @@ def get_category_assignment(self) -> Dict[ID, List[Tuple[ID, Decimal]]]: def get_refund_maps(self) -> Dict[ID, ID]: refund_to_withdraw: Dict[ID, ID] = {} + if not self._table_exists("ZWITHDRAWREFUNDTRANSACTIONLINK"): + return refund_to_withdraw cur = self._con.cursor() res = cur.execute( """ diff --git a/src/moneywiz_api/managers/record_manager.py b/src/moneywiz_api/managers/record_manager.py index 99049cc..2319614 100644 --- a/src/moneywiz_api/managers/record_manager.py +++ b/src/moneywiz_api/managers/record_manager.py @@ -1,17 +1,20 @@ from abc import ABC, abstractmethod -from typing import Dict, Generic, TypeVar +import logging +from typing import Callable, Dict, Generic, TypeVar from moneywiz_api.database_accessor import DatabaseAccessor from moneywiz_api.model.record import Record from moneywiz_api.types import GID, ID T = TypeVar("T", bound=Record) +logger = logging.getLogger(__name__) class RecordManager(ABC, Generic[T]): def __init__(self) -> None: self._records: Dict[ID, T] = {} self._gid_to_id: Dict[GID, ID] = {} + self._load_errors: list[tuple[ID, str, str]] = [] @property @abstractmethod @@ -23,14 +26,28 @@ def load(self, db_accessor: DatabaseAccessor) -> None: for record in records: typename = db_accessor.typename_for(record["Z_ENT"]) - assert typename in self.ents, ( - f"Unknown typename {typename} for record {record}" - ) + if typename in self.ents: + try: + obj = self.construct_record(self.ents[typename], record, db_accessor) + obj.validate() + except (AssertionError, KeyError, ValueError) as exc: + record_id = record.get("Z_PK") + detail = type(exc).__name__ + self._load_errors.append((record_id, typename, detail)) + logger.debug( + "Skipping unreadable %s record %s: %s", + typename, + record_id, + detail, + ) + continue + self.add(obj) - model_cls = self.ents[typename] - obj = db_accessor.schema_profile.create_record(record, model_cls) - obj.validate() - self.add(obj) + def construct_record( + self, constructor: Callable, record, db_accessor: DatabaseAccessor + ): + """Construct a record; subclasses can supply schema-specific context.""" + return db_accessor.schema_profile.create_record(record, constructor) def add(self, record: T) -> None: self._records[record.id] = record @@ -53,5 +70,10 @@ def get_by_gid(self, gid: GID) -> T | None: def records(self) -> Dict[ID, T]: return self._records + @property + def load_errors(self) -> list[tuple[ID, str, str]]: + """Return records skipped during best-effort read parsing.""" + return list(self._load_errors) + def __repr__(self): return "\n".join(f"{key}: {value}" for key, value in self.records().items()) diff --git a/src/moneywiz_api/model/transaction.py b/src/moneywiz_api/model/transaction.py index 2d70231..405d9cf 100644 --- a/src/moneywiz_api/model/transaction.py +++ b/src/moneywiz_api/model/transaction.py @@ -15,7 +15,6 @@ def approx_equal(a: Decimal, b: Decimal, abs_tol: Decimal = ABS_TOLERANCE) -> bool: return abs(a - b) <= abs_tol - @dataclass class Transaction(Record, ABC): reconciled: bool diff --git a/src/moneywiz_api/schema/raw_data_handler.py b/src/moneywiz_api/schema/raw_data_handler.py index a8b3527..0996a89 100644 --- a/src/moneywiz_api/schema/raw_data_handler.py +++ b/src/moneywiz_api/schema/raw_data_handler.py @@ -27,13 +27,34 @@ def get_decimal(raw_value: Any) -> Decimal: ) return Decimal(str(raw_value)) + @staticmethod + def get_decimal_alias(row: Dict[str, Any], *keys: str) -> Decimal: + """Read a required decimal field from the first available column alias.""" + for key in keys: + if key in row: + return RawDataHandler.get_decimal(row, key) + raise KeyError(f"none of the schema aliases are present: {', '.join(keys)}") + + @staticmethod + def get_nullable_decimal_alias( + row: Dict[str, Any], *keys: str + ) -> Optional[Decimal]: + """Read an optional decimal field from the first available column alias.""" + for key in keys: + if key in row: + return RawDataHandler.get_nullable_decimal(row, key) + raise KeyError(f"none of the schema aliases are present: {', '.join(keys)}") + @staticmethod def filter_row(row: Dict[str, Any]) -> Dict[str, Any]: copy = {k: v for k, v in row.items()} - del copy["ZMANUALHISTORICALPRICESPERSHARE"] - del copy["ZIMPORTLINKIDARRAY2"] - del copy["ZIMPORTLINKIDARRAY"] - del copy["ZBANKLOGOPRIMARYCOLOR"] + for key in ( + "ZMANUALHISTORICALPRICESPERSHARE", + "ZIMPORTLINKIDARRAY2", + "ZIMPORTLINKIDARRAY", + "ZBANKLOGOPRIMARYCOLOR", + ): + copy.pop(key, None) return { k: v for k, v in copy.items() diff --git a/src/moneywiz_api/schema_profile.py b/src/moneywiz_api/schema_profile.py new file mode 100644 index 0000000..4e48b95 --- /dev/null +++ b/src/moneywiz_api/schema_profile.py @@ -0,0 +1,55 @@ +"""Schema capability profiles for MoneyWiz SQLite stores.""" + +from __future__ import annotations + +import sqlite3 +from dataclasses import dataclass + + +@dataclass(frozen=True) +class SchemaProfile: + """Capabilities inferred from the physical store schema.""" + + profile_id: str + number_of_shares_column: str | None + price_per_share_column: str | None + + @property + def is_known(self) -> bool: + return self.profile_id != "unknown" + + +def detect_schema_profile(connection: sqlite3.Connection) -> SchemaProfile: + """Detect a read profile from columns, not from Core Data metadata alone.""" + columns = set() + for row in connection.execute("PRAGMA table_info(ZSYNCOBJECT)").fetchall(): + columns.add(str(row["name"] if isinstance(row, dict) else row[1])) + has_suffixed_shares = "ZNUMBEROFSHARES1" in columns + has_unsuffixed_shares = "ZNUMBEROFSHARES" in columns + has_suffixed_price = "ZPRICEPERSHARE1" in columns + has_unsuffixed_price = "ZPRICEPERSHARE" in columns + + if has_suffixed_shares: + profile_id = "suffixed-investment-columns" + elif has_unsuffixed_shares: + profile_id = "unsuffixed-investment-columns" + else: + profile_id = "unknown" + + return SchemaProfile( + profile_id=profile_id, + number_of_shares_column=( + "ZNUMBEROFSHARES1" + if has_suffixed_shares + else "ZNUMBEROFSHARES" + if has_unsuffixed_shares + else None + ), + price_per_share_column=( + "ZPRICEPERSHARE1" + if has_suffixed_shares and has_suffixed_price + else "ZPRICEPERSHARE" + if has_unsuffixed_shares and has_unsuffixed_price + else None + ), + ) diff --git a/tests/unit/test_schema_profiles.py b/tests/unit/test_schema_profiles.py new file mode 100644 index 0000000..102eb23 --- /dev/null +++ b/tests/unit/test_schema_profiles.py @@ -0,0 +1,47 @@ +import sqlite3 + +from moneywiz_api.model.raw_data_handler import RawDataHandler +from moneywiz_api.schema_profile import detect_schema_profile + + +def make_connection(*columns: str) -> sqlite3.Connection: + connection = sqlite3.connect(":memory:") + column_sql = ", ".join(f'"{column}" FLOAT' for column in columns) + connection.execute(f"CREATE TABLE ZSYNCOBJECT (Z_PK INTEGER, {column_sql})") + return connection + + +def test_detects_suffixed_fixture_profile() -> None: + connection = make_connection("ZNUMBEROFSHARES1", "ZPRICEPERSHARE1") + + profile = detect_schema_profile(connection) + + assert profile.profile_id == "suffixed-investment-columns" + assert profile.number_of_shares_column == "ZNUMBEROFSHARES1" + assert profile.price_per_share_column == "ZPRICEPERSHARE1" + + +def test_detects_unsuffixed_live_profile() -> None: + connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1") + + profile = detect_schema_profile(connection) + + assert profile.profile_id == "unsuffixed-investment-columns" + assert profile.number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.price_per_share_column == "ZPRICEPERSHARE" + + +def test_decimal_alias_reads_both_profiles() -> None: + assert RawDataHandler.get_decimal_alias( + {"ZNUMBEROFSHARES": 2.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" + ) == 2.5 + assert RawDataHandler.get_decimal_alias( + {"ZNUMBEROFSHARES1": 3.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" + ) == 3.5 + assert RawDataHandler.get_nullable_decimal_alias( + {"ZNUMBEROFSHARES": None}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" + ) is None + + +def test_filter_row_accepts_missing_optional_blob_columns() -> None: + assert RawDataHandler.filter_row({"Z_PK": 1}) == {"Z_PK": 1} From 5e3eadaa52c935babf46a9a00407aac8d5323903 Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Wed, 12 Aug 2026 15:17:52 +0200 Subject: [PATCH 02/12] fix: harden read compatibility validation --- src/moneywiz_api/schema/raw_data_handler.py | 9 ++++- src/moneywiz_api/schema_profile.py | 42 +++++++++++++-------- tests/unit/test_schema_profiles.py | 20 ++++++++++ 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/moneywiz_api/schema/raw_data_handler.py b/src/moneywiz_api/schema/raw_data_handler.py index 0996a89..1398454 100644 --- a/src/moneywiz_api/schema/raw_data_handler.py +++ b/src/moneywiz_api/schema/raw_data_handler.py @@ -31,7 +31,7 @@ def get_decimal(raw_value: Any) -> Decimal: def get_decimal_alias(row: Dict[str, Any], *keys: str) -> Decimal: """Read a required decimal field from the first available column alias.""" for key in keys: - if key in row: + if key in row and row[key] is not None: return RawDataHandler.get_decimal(row, key) raise KeyError(f"none of the schema aliases are present: {', '.join(keys)}") @@ -40,9 +40,14 @@ def get_nullable_decimal_alias( row: Dict[str, Any], *keys: str ) -> Optional[Decimal]: """Read an optional decimal field from the first available column alias.""" + found_alias = False for key in keys: if key in row: - return RawDataHandler.get_nullable_decimal(row, key) + found_alias = True + if row[key] is not None: + return RawDataHandler.get_nullable_decimal(row, key) + if found_alias: + return None raise KeyError(f"none of the schema aliases are present: {', '.join(keys)}") @staticmethod diff --git a/src/moneywiz_api/schema_profile.py b/src/moneywiz_api/schema_profile.py index 4e48b95..ac26d78 100644 --- a/src/moneywiz_api/schema_profile.py +++ b/src/moneywiz_api/schema_profile.py @@ -29,27 +29,37 @@ def detect_schema_profile(connection: sqlite3.Connection) -> SchemaProfile: has_suffixed_price = "ZPRICEPERSHARE1" in columns has_unsuffixed_price = "ZPRICEPERSHARE" in columns - if has_suffixed_shares: + if has_suffixed_shares and has_suffixed_price: profile_id = "suffixed-investment-columns" - elif has_unsuffixed_shares: + number_of_shares_column = "ZNUMBEROFSHARES1" + price_per_share_column = "ZPRICEPERSHARE1" + elif has_unsuffixed_shares and has_unsuffixed_price: profile_id = "unsuffixed-investment-columns" - else: - profile_id = "unknown" - - return SchemaProfile( - profile_id=profile_id, - number_of_shares_column=( + number_of_shares_column = "ZNUMBEROFSHARES" + price_per_share_column = "ZPRICEPERSHARE" + elif ( + has_suffixed_shares or has_unsuffixed_shares + ) and ( + has_suffixed_price or has_unsuffixed_price + ): + profile_id = "mixed-investment-columns" + number_of_shares_column = ( "ZNUMBEROFSHARES1" if has_suffixed_shares else "ZNUMBEROFSHARES" - if has_unsuffixed_shares - else None - ), - price_per_share_column=( + ) + price_per_share_column = ( "ZPRICEPERSHARE1" - if has_suffixed_shares and has_suffixed_price + if has_suffixed_price else "ZPRICEPERSHARE" - if has_unsuffixed_shares and has_unsuffixed_price - else None - ), + ) + else: + profile_id = "unknown" + number_of_shares_column = None + price_per_share_column = None + + return SchemaProfile( + profile_id=profile_id, + number_of_shares_column=number_of_shares_column, + price_per_share_column=price_per_share_column, ) diff --git a/tests/unit/test_schema_profiles.py b/tests/unit/test_schema_profiles.py index 102eb23..71edc6b 100644 --- a/tests/unit/test_schema_profiles.py +++ b/tests/unit/test_schema_profiles.py @@ -31,6 +31,16 @@ def test_detects_unsuffixed_live_profile() -> None: assert profile.price_per_share_column == "ZPRICEPERSHARE" +def test_detects_mixed_investment_columns() -> None: + connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE1") + + profile = detect_schema_profile(connection) + + assert profile.profile_id == "mixed-investment-columns" + assert profile.number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.price_per_share_column == "ZPRICEPERSHARE1" + + def test_decimal_alias_reads_both_profiles() -> None: assert RawDataHandler.get_decimal_alias( {"ZNUMBEROFSHARES": 2.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" @@ -38,9 +48,19 @@ def test_decimal_alias_reads_both_profiles() -> None: assert RawDataHandler.get_decimal_alias( {"ZNUMBEROFSHARES1": 3.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" ) == 3.5 + assert RawDataHandler.get_decimal_alias( + {"ZNUMBEROFSHARES1": None, "ZNUMBEROFSHARES": 4.5}, + "ZNUMBEROFSHARES1", + "ZNUMBEROFSHARES", + ) == 4.5 assert RawDataHandler.get_nullable_decimal_alias( {"ZNUMBEROFSHARES": None}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" ) is None + assert RawDataHandler.get_nullable_decimal_alias( + {"ZNUMBEROFSHARES1": None, "ZNUMBEROFSHARES": 5.5}, + "ZNUMBEROFSHARES1", + "ZNUMBEROFSHARES", + ) == 5.5 def test_filter_row_accepts_missing_optional_blob_columns() -> None: From 7ab09e3030442ae70e2fa87a9f3ed8efa357687d Mon Sep 17 00:00:00 2001 From: Marco MC Date: Wed, 12 Aug 2026 18:01:44 +0200 Subject: [PATCH 03/12] fix: handle zero-rate transfer compatibility Reject zero-rate transfer reconstructions as recoverable parse failures, cover manager recovery, and format schema compatibility tests. --- src/moneywiz_api/model/transaction.py | 12 +++ src/moneywiz_api/schema_profile.py | 12 +-- tests/unit/test_schema_profiles.py | 53 ++++++++----- tests/unit/test_transaction_compatibility.py | 81 ++++++++++++++++++++ 4 files changed, 130 insertions(+), 28 deletions(-) create mode 100644 tests/unit/test_transaction_compatibility.py diff --git a/src/moneywiz_api/model/transaction.py b/src/moneywiz_api/model/transaction.py index 405d9cf..05907ff 100644 --- a/src/moneywiz_api/model/transaction.py +++ b/src/moneywiz_api/model/transaction.py @@ -15,6 +15,7 @@ def approx_equal(a: Decimal, b: Decimal, abs_tol: Decimal = ABS_TOLERANCE) -> bool: return abs(a - b) <= abs_tol + @dataclass class Transaction(Record, ABC): reconciled: bool @@ -343,6 +344,17 @@ def __init__(self, row, field_values): self.recipient_amount = -self.original_amount * self.original_exchange_rate if self.recipient_amount is not None: self.recipient_amount = abs(self.recipient_amount) + if ( + self.original_amount == 0 + and self.recipient_amount not in (None, Decimal(0)) + ): + if self.original_exchange_rate == 0: + raise ValueError( + "cannot reconstruct a transfer amount with a zero exchange rate" + ) + self.original_amount = -abs( + self.recipient_amount / self.original_exchange_rate + ) def validate(self) -> None: super().validate() diff --git a/src/moneywiz_api/schema_profile.py b/src/moneywiz_api/schema_profile.py index ac26d78..10a331c 100644 --- a/src/moneywiz_api/schema_profile.py +++ b/src/moneywiz_api/schema_profile.py @@ -37,21 +37,15 @@ def detect_schema_profile(connection: sqlite3.Connection) -> SchemaProfile: profile_id = "unsuffixed-investment-columns" number_of_shares_column = "ZNUMBEROFSHARES" price_per_share_column = "ZPRICEPERSHARE" - elif ( - has_suffixed_shares or has_unsuffixed_shares - ) and ( + elif (has_suffixed_shares or has_unsuffixed_shares) and ( has_suffixed_price or has_unsuffixed_price ): profile_id = "mixed-investment-columns" number_of_shares_column = ( - "ZNUMBEROFSHARES1" - if has_suffixed_shares - else "ZNUMBEROFSHARES" + "ZNUMBEROFSHARES1" if has_suffixed_shares else "ZNUMBEROFSHARES" ) price_per_share_column = ( - "ZPRICEPERSHARE1" - if has_suffixed_price - else "ZPRICEPERSHARE" + "ZPRICEPERSHARE1" if has_suffixed_price else "ZPRICEPERSHARE" ) else: profile_id = "unknown" diff --git a/tests/unit/test_schema_profiles.py b/tests/unit/test_schema_profiles.py index 71edc6b..2d63557 100644 --- a/tests/unit/test_schema_profiles.py +++ b/tests/unit/test_schema_profiles.py @@ -42,25 +42,40 @@ def test_detects_mixed_investment_columns() -> None: def test_decimal_alias_reads_both_profiles() -> None: - assert RawDataHandler.get_decimal_alias( - {"ZNUMBEROFSHARES": 2.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" - ) == 2.5 - assert RawDataHandler.get_decimal_alias( - {"ZNUMBEROFSHARES1": 3.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" - ) == 3.5 - assert RawDataHandler.get_decimal_alias( - {"ZNUMBEROFSHARES1": None, "ZNUMBEROFSHARES": 4.5}, - "ZNUMBEROFSHARES1", - "ZNUMBEROFSHARES", - ) == 4.5 - assert RawDataHandler.get_nullable_decimal_alias( - {"ZNUMBEROFSHARES": None}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" - ) is None - assert RawDataHandler.get_nullable_decimal_alias( - {"ZNUMBEROFSHARES1": None, "ZNUMBEROFSHARES": 5.5}, - "ZNUMBEROFSHARES1", - "ZNUMBEROFSHARES", - ) == 5.5 + assert ( + RawDataHandler.get_decimal_alias( + {"ZNUMBEROFSHARES": 2.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" + ) + == 2.5 + ) + assert ( + RawDataHandler.get_decimal_alias( + {"ZNUMBEROFSHARES1": 3.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" + ) + == 3.5 + ) + assert ( + RawDataHandler.get_decimal_alias( + {"ZNUMBEROFSHARES1": None, "ZNUMBEROFSHARES": 4.5}, + "ZNUMBEROFSHARES1", + "ZNUMBEROFSHARES", + ) + == 4.5 + ) + assert ( + RawDataHandler.get_nullable_decimal_alias( + {"ZNUMBEROFSHARES": None}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" + ) + is None + ) + assert ( + RawDataHandler.get_nullable_decimal_alias( + {"ZNUMBEROFSHARES1": None, "ZNUMBEROFSHARES": 5.5}, + "ZNUMBEROFSHARES1", + "ZNUMBEROFSHARES", + ) + == 5.5 + ) def test_filter_row_accepts_missing_optional_blob_columns() -> None: diff --git a/tests/unit/test_transaction_compatibility.py b/tests/unit/test_transaction_compatibility.py new file mode 100644 index 0000000..2004e87 --- /dev/null +++ b/tests/unit/test_transaction_compatibility.py @@ -0,0 +1,81 @@ +from decimal import Decimal + +import pytest + +from moneywiz_api.managers.record_manager import RecordManager +from moneywiz_api.model.transaction import TransferWithdrawTransaction + + +def transfer_withdraw_row(**overrides): + row = { + "Z_ENT": 46, + "ZOBJECTCREATIONDATE": 0.0, + "ZGID": "transfer-withdraw", + "Z_PK": 1, + "ZRECONCILED": 0, + "ZAMOUNT1": -10.0, + "ZDESC2": "Transfer", + "ZDATE1": 0.0, + "ZNOTES1": None, + "ZACCOUNT2": 10, + "ZRECIPIENTACCOUNT1": 20, + "ZRECIPIENTTRANSACTION": 2, + "ZORIGINALAMOUNT": 0.0, + "ZORIGINALCURRENCY": "EUR", + "ZORIGINALRECIPIENTAMOUNT": 10.0, + "ZORIGINALRECIPIENTCURRENCY": "USD", + "ZORIGINALFEE": None, + "ZORIGINALFEECURRENCY": None, + "ZORIGINALEXCHANGERATE": 0.0, + } + row.update(overrides) + return row + + +def test_transfer_withdraw_rejects_zero_rate_reconstruction() -> None: + with pytest.raises(ValueError, match="zero exchange rate"): + TransferWithdrawTransaction(transfer_withdraw_row()) + + +def test_transfer_withdraw_reconstructs_with_nonzero_rate() -> None: + transaction = TransferWithdrawTransaction( + transfer_withdraw_row(ZORIGINALEXCHANGERATE=1.0) + ) + + assert transaction.original_amount == Decimal("-10.0") + + +class TransferWithdrawManager(RecordManager): + @property + def ents(self): + return {"TransferWithdrawTransaction": TransferWithdrawTransaction} + + +class TransferWithdrawAccessor: + def __init__(self, rows): + self.rows = rows + + def query_objects(self, _typenames): + return self.rows + + def typename_for(self, _ent_id): + return "TransferWithdrawTransaction" + + +def test_transfer_manager_skips_zero_rate_and_loads_later_valid_row() -> None: + manager = TransferWithdrawManager() + manager.load( + TransferWithdrawAccessor( + [ + transfer_withdraw_row(), + transfer_withdraw_row( + Z_PK=2, + ZGID="valid-transfer-withdraw", + ZORIGINALEXCHANGERATE=1.0, + ), + ] + ) + ) + + assert list(manager.records()) == [2] + assert manager.load_errors == [(1, "TransferWithdrawTransaction", "ValueError")] From 1590032fd1d9e7fc6f3cb87baa42571f8cd42d5e Mon Sep 17 00:00:00 2001 From: Marco MC Date: Wed, 12 Aug 2026 18:44:56 +0200 Subject: [PATCH 04/12] fix: separate investment schema aliases Model holding and transaction share columns independently and preserve canonical stored amounts for rounded transfers. --- src/moneywiz_api/schema_profile.py | 43 ++++++++++++++++---- tests/unit/test_schema_profiles.py | 22 ++++++++-- tests/unit/test_transaction_compatibility.py | 12 ++++++ 3 files changed, 67 insertions(+), 10 deletions(-) diff --git a/src/moneywiz_api/schema_profile.py b/src/moneywiz_api/schema_profile.py index 10a331c..fcfb40c 100644 --- a/src/moneywiz_api/schema_profile.py +++ b/src/moneywiz_api/schema_profile.py @@ -11,7 +11,8 @@ class SchemaProfile: """Capabilities inferred from the physical store schema.""" profile_id: str - number_of_shares_column: str | None + holding_number_of_shares_column: str | None + transaction_number_of_shares_column: str | None price_per_share_column: str | None @property @@ -29,19 +30,45 @@ def detect_schema_profile(connection: sqlite3.Connection) -> SchemaProfile: has_suffixed_price = "ZPRICEPERSHARE1" in columns has_unsuffixed_price = "ZPRICEPERSHARE" in columns - if has_suffixed_shares and has_suffixed_price: + if ( + has_suffixed_shares + and has_unsuffixed_shares + and has_suffixed_price + and has_unsuffixed_price + ): + profile_id = "unknown" + holding_number_of_shares_column = None + transaction_number_of_shares_column = None + price_per_share_column = None + elif ( + has_suffixed_shares + and has_unsuffixed_shares + and (has_suffixed_price or has_unsuffixed_price) + ): + profile_id = "mixed-investment-columns" + holding_number_of_shares_column = "ZNUMBEROFSHARES" + transaction_number_of_shares_column = "ZNUMBEROFSHARES1" + price_per_share_column = ( + "ZPRICEPERSHARE1" if has_suffixed_price else "ZPRICEPERSHARE" + ) + elif has_suffixed_shares and has_suffixed_price: profile_id = "suffixed-investment-columns" - number_of_shares_column = "ZNUMBEROFSHARES1" + holding_number_of_shares_column = "ZNUMBEROFSHARES1" + transaction_number_of_shares_column = "ZNUMBEROFSHARES1" price_per_share_column = "ZPRICEPERSHARE1" elif has_unsuffixed_shares and has_unsuffixed_price: profile_id = "unsuffixed-investment-columns" - number_of_shares_column = "ZNUMBEROFSHARES" + holding_number_of_shares_column = "ZNUMBEROFSHARES" + transaction_number_of_shares_column = "ZNUMBEROFSHARES" price_per_share_column = "ZPRICEPERSHARE" elif (has_suffixed_shares or has_unsuffixed_shares) and ( has_suffixed_price or has_unsuffixed_price ): profile_id = "mixed-investment-columns" - number_of_shares_column = ( + holding_number_of_shares_column = ( + "ZNUMBEROFSHARES" if has_unsuffixed_shares else "ZNUMBEROFSHARES1" + ) + transaction_number_of_shares_column = ( "ZNUMBEROFSHARES1" if has_suffixed_shares else "ZNUMBEROFSHARES" ) price_per_share_column = ( @@ -49,11 +76,13 @@ def detect_schema_profile(connection: sqlite3.Connection) -> SchemaProfile: ) else: profile_id = "unknown" - number_of_shares_column = None + holding_number_of_shares_column = None + transaction_number_of_shares_column = None price_per_share_column = None return SchemaProfile( profile_id=profile_id, - number_of_shares_column=number_of_shares_column, + holding_number_of_shares_column=holding_number_of_shares_column, + transaction_number_of_shares_column=transaction_number_of_shares_column, price_per_share_column=price_per_share_column, ) diff --git a/tests/unit/test_schema_profiles.py b/tests/unit/test_schema_profiles.py index 2d63557..29187ee 100644 --- a/tests/unit/test_schema_profiles.py +++ b/tests/unit/test_schema_profiles.py @@ -17,7 +17,8 @@ def test_detects_suffixed_fixture_profile() -> None: profile = detect_schema_profile(connection) assert profile.profile_id == "suffixed-investment-columns" - assert profile.number_of_shares_column == "ZNUMBEROFSHARES1" + assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES1" + assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES1" assert profile.price_per_share_column == "ZPRICEPERSHARE1" @@ -27,7 +28,8 @@ def test_detects_unsuffixed_live_profile() -> None: profile = detect_schema_profile(connection) assert profile.profile_id == "unsuffixed-investment-columns" - assert profile.number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES" assert profile.price_per_share_column == "ZPRICEPERSHARE" @@ -37,7 +39,21 @@ def test_detects_mixed_investment_columns() -> None: profile = detect_schema_profile(connection) assert profile.profile_id == "mixed-investment-columns" - assert profile.number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.price_per_share_column == "ZPRICEPERSHARE1" + + +def test_detects_consumer_specific_mixed_share_columns() -> None: + connection = make_connection( + "ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1" + ) + + profile = detect_schema_profile(connection) + + assert profile.profile_id == "mixed-investment-columns" + assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES1" assert profile.price_per_share_column == "ZPRICEPERSHARE1" diff --git a/tests/unit/test_transaction_compatibility.py b/tests/unit/test_transaction_compatibility.py index 2004e87..907706b 100644 --- a/tests/unit/test_transaction_compatibility.py +++ b/tests/unit/test_transaction_compatibility.py @@ -45,6 +45,18 @@ def test_transfer_withdraw_reconstructs_with_nonzero_rate() -> None: assert transaction.original_amount == Decimal("-10.0") +def test_transfer_withdraw_preserves_stored_amount_with_rounded_rate() -> None: + transaction = TransferWithdrawTransaction( + transfer_withdraw_row( + ZAMOUNT1=-3.333, + ZORIGINALRECIPIENTAMOUNT=10.0, + ZORIGINALEXCHANGERATE=3.0, + ) + ) + + assert transaction.original_amount == Decimal("-3.333") + + class TransferWithdrawManager(RecordManager): @property def ents(self): From 6b5fe1dcf1207acbaa1c6478f31be3aa2e4014f6 Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Tue, 15 Sep 2026 21:38:11 +0200 Subject: [PATCH 05/12] chore: reconcile read compatibility with merged main --- src/moneywiz_api/database_accessor.py | 5 +++++ src/moneywiz_api/managers/record_manager.py | 4 +++- src/moneywiz_api/model/transaction.py | 6 +++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/moneywiz_api/database_accessor.py b/src/moneywiz_api/database_accessor.py index 0c79b65..01808f9 100644 --- a/src/moneywiz_api/database_accessor.py +++ b/src/moneywiz_api/database_accessor.py @@ -6,6 +6,11 @@ from moneywiz_api.schema.raw_data_handler import RawDataHandler as RDH from moneywiz_api.model.record import Record +from moneywiz_api.model.investment_holding import InvestmentHolding +from moneywiz_api.model.transaction import ( + InvestmentBuyTransaction, + InvestmentSellTransaction, +) from moneywiz_api.schema_profile import SchemaProfile, detect_schema_profile from moneywiz_api.types import ENT_ID, GID, ID from moneywiz_api.schema.schema_profile import SchemaProfile diff --git a/src/moneywiz_api/managers/record_manager.py b/src/moneywiz_api/managers/record_manager.py index 2319614..d587fd1 100644 --- a/src/moneywiz_api/managers/record_manager.py +++ b/src/moneywiz_api/managers/record_manager.py @@ -28,7 +28,9 @@ def load(self, db_accessor: DatabaseAccessor) -> None: typename = db_accessor.typename_for(record["Z_ENT"]) if typename in self.ents: try: - obj = self.construct_record(self.ents[typename], record, db_accessor) + obj = self.construct_record( + self.ents[typename], record, db_accessor + ) obj.validate() except (AssertionError, KeyError, ValueError) as exc: record_id = record.get("Z_PK") diff --git a/src/moneywiz_api/model/transaction.py b/src/moneywiz_api/model/transaction.py index 05907ff..4c1a8b4 100644 --- a/src/moneywiz_api/model/transaction.py +++ b/src/moneywiz_api/model/transaction.py @@ -344,9 +344,9 @@ def __init__(self, row, field_values): self.recipient_amount = -self.original_amount * self.original_exchange_rate if self.recipient_amount is not None: self.recipient_amount = abs(self.recipient_amount) - if ( - self.original_amount == 0 - and self.recipient_amount not in (None, Decimal(0)) + if self.original_amount == 0 and self.recipient_amount not in ( + None, + Decimal(0), ): if self.original_exchange_rate == 0: raise ValueError( From a1acac824edadfca85e6a079ac88e5b5de45d38a Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Fri, 25 Sep 2026 17:49:17 +0200 Subject: [PATCH 06/12] fix: detect investment columns from entity rows --- src/moneywiz_api/schema_profile.py | 157 +++++---- tests/unit/test_profiled_investment_values.py | 298 ++++++++++++++++++ tests/unit/test_schema_profiles.py | 106 ++++++- 3 files changed, 500 insertions(+), 61 deletions(-) create mode 100644 tests/unit/test_profiled_investment_values.py diff --git a/src/moneywiz_api/schema_profile.py b/src/moneywiz_api/schema_profile.py index fcfb40c..71fb411 100644 --- a/src/moneywiz_api/schema_profile.py +++ b/src/moneywiz_api/schema_profile.py @@ -4,11 +4,12 @@ import sqlite3 from dataclasses import dataclass +from typing import Any, Iterable @dataclass(frozen=True) class SchemaProfile: - """Capabilities inferred from the physical store schema.""" + """Capabilities inferred from physical columns and investment rows.""" profile_id: str holding_number_of_shares_column: str | None @@ -20,69 +21,109 @@ def is_known(self) -> bool: return self.profile_id != "unknown" +def _value(row: Any, name: str, index: int) -> Any: + if isinstance(row, dict): + return row[name] + return row[index] + + +def _column_for_entities( + connection: sqlite3.Connection, + columns: set[str], + unsuffixed_column: str, + suffixed_column: str, + entity_names: Iterable[str], +) -> str | None: + has_unsuffixed = unsuffixed_column in columns + has_suffixed = suffixed_column in columns + if has_unsuffixed != has_suffixed: + return unsuffixed_column if has_unsuffixed else suffixed_column + if not has_unsuffixed: + return None + + tables = { + str(_value(row, "name", 0)) + for row in connection.execute( + "SELECT name FROM sqlite_master WHERE type = 'table'" + ).fetchall() + } + if "Z_PRIMARYKEY" not in tables: + return None + + names = tuple(entity_names) + placeholders = ", ".join("?" for _ in names) + entity_rows = connection.execute( + f'SELECT "Z_ENT", "Z_NAME" FROM "Z_PRIMARYKEY" ' + f'WHERE "Z_NAME" IN ({placeholders})', + names, + ).fetchall() + entity_ids = tuple(_value(row, "Z_ENT", 0) for row in entity_rows) + if not entity_ids: + return None + + placeholders = ", ".join("?" for _ in entity_ids) + counts = connection.execute( + f'SELECT COUNT("{unsuffixed_column}") AS "unsuffixed_count", ' + f'COUNT("{suffixed_column}") AS "suffixed_count" ' + f'FROM "ZSYNCOBJECT" WHERE "Z_ENT" IN ({placeholders})', + entity_ids, + ).fetchone() + unsuffixed_count = int(_value(counts, "unsuffixed_count", 0)) + suffixed_count = int(_value(counts, "suffixed_count", 1)) + if unsuffixed_count and not suffixed_count: + return unsuffixed_column + if suffixed_count and not unsuffixed_count: + return suffixed_column + return None + + def detect_schema_profile(connection: sqlite3.Connection) -> SchemaProfile: - """Detect a read profile from columns, not from Core Data metadata alone.""" - columns = set() - for row in connection.execute("PRAGMA table_info(ZSYNCOBJECT)").fetchall(): - columns.add(str(row["name"] if isinstance(row, dict) else row[1])) - has_suffixed_shares = "ZNUMBEROFSHARES1" in columns - has_unsuffixed_shares = "ZNUMBEROFSHARES" in columns - has_suffixed_price = "ZPRICEPERSHARE1" in columns - has_unsuffixed_price = "ZPRICEPERSHARE" in columns - - if ( - has_suffixed_shares - and has_unsuffixed_shares - and has_suffixed_price - and has_unsuffixed_price - ): + """Select investment aliases using columns and entity-specific row evidence.""" + columns = { + str(_value(row, "name", 1)) + for row in connection.execute("PRAGMA table_info(ZSYNCOBJECT)").fetchall() + } + + holding_shares = _column_for_entities( + connection, + columns, + "ZNUMBEROFSHARES", + "ZNUMBEROFSHARES1", + ("InvestmentHolding",), + ) + transaction_entities = ("InvestmentBuyTransaction", "InvestmentSellTransaction") + transaction_shares = _column_for_entities( + connection, + columns, + "ZNUMBEROFSHARES", + "ZNUMBEROFSHARES1", + transaction_entities, + ) + price_per_share = _column_for_entities( + connection, + columns, + "ZPRICEPERSHARE", + "ZPRICEPERSHARE1", + transaction_entities, + ) + + if not holding_shares or not transaction_shares or not price_per_share: profile_id = "unknown" - holding_number_of_shares_column = None - transaction_number_of_shares_column = None - price_per_share_column = None - elif ( - has_suffixed_shares - and has_unsuffixed_shares - and (has_suffixed_price or has_unsuffixed_price) - ): + elif len( + { + column.endswith("1") + for column in (holding_shares, transaction_shares, price_per_share) + } + ) > 1: profile_id = "mixed-investment-columns" - holding_number_of_shares_column = "ZNUMBEROFSHARES" - transaction_number_of_shares_column = "ZNUMBEROFSHARES1" - price_per_share_column = ( - "ZPRICEPERSHARE1" if has_suffixed_price else "ZPRICEPERSHARE" - ) - elif has_suffixed_shares and has_suffixed_price: + elif holding_shares.endswith("1"): profile_id = "suffixed-investment-columns" - holding_number_of_shares_column = "ZNUMBEROFSHARES1" - transaction_number_of_shares_column = "ZNUMBEROFSHARES1" - price_per_share_column = "ZPRICEPERSHARE1" - elif has_unsuffixed_shares and has_unsuffixed_price: - profile_id = "unsuffixed-investment-columns" - holding_number_of_shares_column = "ZNUMBEROFSHARES" - transaction_number_of_shares_column = "ZNUMBEROFSHARES" - price_per_share_column = "ZPRICEPERSHARE" - elif (has_suffixed_shares or has_unsuffixed_shares) and ( - has_suffixed_price or has_unsuffixed_price - ): - profile_id = "mixed-investment-columns" - holding_number_of_shares_column = ( - "ZNUMBEROFSHARES" if has_unsuffixed_shares else "ZNUMBEROFSHARES1" - ) - transaction_number_of_shares_column = ( - "ZNUMBEROFSHARES1" if has_suffixed_shares else "ZNUMBEROFSHARES" - ) - price_per_share_column = ( - "ZPRICEPERSHARE1" if has_suffixed_price else "ZPRICEPERSHARE" - ) else: - profile_id = "unknown" - holding_number_of_shares_column = None - transaction_number_of_shares_column = None - price_per_share_column = None + profile_id = "unsuffixed-investment-columns" return SchemaProfile( profile_id=profile_id, - holding_number_of_shares_column=holding_number_of_shares_column, - transaction_number_of_shares_column=transaction_number_of_shares_column, - price_per_share_column=price_per_share_column, + holding_number_of_shares_column=holding_shares, + transaction_number_of_shares_column=transaction_shares, + price_per_share_column=price_per_share, ) diff --git a/tests/unit/test_profiled_investment_values.py b/tests/unit/test_profiled_investment_values.py new file mode 100644 index 0000000..087de65 --- /dev/null +++ b/tests/unit/test_profiled_investment_values.py @@ -0,0 +1,298 @@ +from decimal import Decimal +from typing import Any, cast + +import pytest + +from moneywiz_api.managers.investment_holding_manager import InvestmentHoldingManager +from moneywiz_api.managers.transaction_manager import TransactionManager +from moneywiz_api.database_accessor import DatabaseAccessor +from moneywiz_api.model.investment_holding import InvestmentHolding +from moneywiz_api.model.schema_mapped_row import mapped_row +from moneywiz_api.model.transaction import ( + InvestmentBuyTransaction, + InvestmentSellTransaction, +) +from moneywiz_api.schema_profile import ( + SchemaProfile, + UnsupportedInvestmentSchemaError, +) + + +UNSUFFIXED_PROFILE = SchemaProfile( + profile_id="unsuffixed-investment-columns", + holding_number_of_shares_column="ZNUMBEROFSHARES", + transaction_number_of_shares_column="ZNUMBEROFSHARES", + price_per_share_column="ZPRICEPERSHARE", +) +MIXED_PROFILE = SchemaProfile( + profile_id="mixed-investment-columns", + holding_number_of_shares_column="ZNUMBEROFSHARES", + transaction_number_of_shares_column="ZNUMBEROFSHARES1", + price_per_share_column="ZPRICEPERSHARE1", +) +OBSERVED_STORE_PROFILE = SchemaProfile( + profile_id="mixed-investment-columns", + holding_number_of_shares_column="ZNUMBEROFSHARES", + transaction_number_of_shares_column="ZNUMBEROFSHARES", + price_per_share_column="ZPRICEPERSHARE1", +) +UNKNOWN_PROFILE = SchemaProfile("unknown", None, None, None) + + +def investment_transaction_row(ent: int, amount: float) -> dict: + return { + "Z_ENT": ent, + "ZOBJECTCREATIONDATE": 0.0, + "ZGID": f"investment-{ent}", + "Z_PK": ent, + "ZRECONCILED": 0, + "ZAMOUNT1": amount, + "ZDESC2": "Investment", + "ZDATE1": 0.0, + "ZNOTES1": None, + "ZACCOUNT2": 1, + "ZFEE2": 0.0, + "ZINVESTMENTHOLDING": 2, + "ZNUMBEROFSHARES": 2.0, + "ZNUMBEROFSHARES1": 9.0, + "ZPRICEPERSHARE": 10.0, + "ZPRICEPERSHARE1": 1.0, + } + + +def investment_holding_row() -> dict: + return { + "Z_ENT": 24, + "ZOBJECTCREATIONDATE": 0.0, + "ZGID": "holding", + "Z_PK": 24, + "ZINVESTMENTACCOUNT": 1, + "ZOPENNINGNUMBEROFSHARES": None, + "ZNUMBEROFSHARES": 2.0, + "ZNUMBEROFSHARES1": 9.0, + "ZSYMBOL": "ACME", + "ZHOLDINGTYPE": None, + "ZDESC": "Acme Corp.", + "ZISPRICEPERSHAREAVAILABLEONLINE": 0, + "ZINVESTMENTOBJECTTYPE": 0, + "ZCOSTBASISOFMISSINGOBSHARES": 0.0, + } + + +@pytest.mark.parametrize( + ("constructor", "row"), + [ + (InvestmentBuyTransaction, investment_transaction_row(40, -20.0)), + (InvestmentSellTransaction, investment_transaction_row(41, 20.0)), + ], +) +def test_transactions_use_selected_profile_alias(constructor, row) -> None: + transaction = constructor( + mapped_row(row, constructor, schema_profile=UNSUFFIXED_PROFILE) + ) + + assert transaction.number_of_shares == Decimal("2.0") + assert transaction.price_per_share == Decimal("10.0") + + +def test_holding_uses_selected_profile_alias() -> None: + holding = InvestmentHolding( + mapped_row( + investment_holding_row(), + InvestmentHolding, + schema_profile=UNSUFFIXED_PROFILE, + ) + ) + + assert holding.number_of_shares == Decimal("2.0") + + +@pytest.mark.parametrize( + ("constructor", "transaction_row"), + [ + (InvestmentBuyTransaction, investment_transaction_row(40, -9.0)), + (InvestmentSellTransaction, investment_transaction_row(41, 9.0)), + ], +) +def test_mixed_profile_uses_consumer_specific_share_aliases( + constructor, transaction_row +) -> None: + holding_row = investment_holding_row() + + transaction = constructor( + mapped_row(transaction_row, constructor, schema_profile=MIXED_PROFILE) + ) + holding = InvestmentHolding( + mapped_row(holding_row, InvestmentHolding, schema_profile=MIXED_PROFILE) + ) + + assert transaction.number_of_shares == Decimal("9.0") + assert transaction.price_per_share == Decimal("1.0") + assert holding.number_of_shares == Decimal("2.0") + + +@pytest.mark.parametrize( + ("constructor", "row"), + [ + (InvestmentBuyTransaction, investment_transaction_row(40, -20.0)), + (InvestmentSellTransaction, investment_transaction_row(41, 20.0)), + ], +) +def test_observed_store_profile_uses_suffixed_transaction_price( + constructor, row +) -> None: + transaction = constructor( + mapped_row(row, constructor, schema_profile=OBSERVED_STORE_PROFILE) + ) + + assert transaction.number_of_shares == Decimal("2.0") + assert transaction.price_per_share == Decimal("1.0") + + +class ProfileAccessor: + schema_profile = UNSUFFIXED_PROFILE + + +def test_managers_pass_profile_to_investment_constructors() -> None: + accessor = ProfileAccessor() + transaction = TransactionManager().construct_record( + InvestmentBuyTransaction, + investment_transaction_row(40, -20.0), + cast(DatabaseAccessor, accessor), + ) + holding = InvestmentHoldingManager().construct_record( + InvestmentHolding, + investment_holding_row(), + cast(DatabaseAccessor, accessor), + ) + + assert transaction.price_per_share == Decimal("10.0") + assert holding.number_of_shares == Decimal("2.0") + + +class ManagerAccessor(ProfileAccessor): + def __init__( + self, + typename: str, + row: dict, + schema_profile: SchemaProfile = UNSUFFIXED_PROFILE, + ): + self.typename = typename + self.row = row + self.schema_profile = schema_profile + + def query_objects(self, _typenames): + return [self.row] + + def typename_for(self, _ent_id): + return self.typename + + def get_category_assignment(self): + return {} + + def get_refund_maps(self): + return {} + + def get_tags_map(self): + return {} + + +def test_managers_load_profiled_investment_records() -> None: + transaction_manager = TransactionManager() + transaction_manager.load( + cast( + DatabaseAccessor, + ManagerAccessor( + "InvestmentBuyTransaction", investment_transaction_row(40, -20.0) + ), + ) + ) + holding_manager = InvestmentHoldingManager() + holding_manager.load( + cast( + DatabaseAccessor, + ManagerAccessor("InvestmentHolding", investment_holding_row()), + ) + ) + + transaction = transaction_manager.get(40) + holding = holding_manager.get(24) + assert isinstance(transaction, InvestmentBuyTransaction) + assert holding is not None + assert transaction.price_per_share == Decimal("10.0") + assert holding.number_of_shares == Decimal("2.0") + + +class StaticCursor: + def __init__(self, row: dict): + self.row = row + + def execute(self, _query, _parameters): + return self + + def fetchone(self): + return self.row + + +class StaticConnection: + def __init__(self, row: dict): + self.row = row + + def cursor(self): + return StaticCursor(self.row) + + +def test_accessor_public_constructors_receive_schema_profile() -> None: + transaction_accessor = DatabaseAccessor.__new__(DatabaseAccessor) + transaction_accessor._con = cast( + Any, StaticConnection(investment_transaction_row(40, -20.0)) + ) + transaction_accessor._schema_profile = UNSUFFIXED_PROFILE + holding_accessor = DatabaseAccessor.__new__(DatabaseAccessor) + holding_accessor._con = cast(Any, StaticConnection(investment_holding_row())) + holding_accessor._schema_profile = UNSUFFIXED_PROFILE + + transaction = transaction_accessor.get_record(40, InvestmentBuyTransaction) + holding = holding_accessor.get_record_by_gid("holding", InvestmentHolding) + + assert transaction.price_per_share == Decimal("10.0") + assert holding.number_of_shares == Decimal("2.0") + + +@pytest.mark.parametrize( + ("constructor", "row"), + [ + (InvestmentBuyTransaction, investment_transaction_row(40, -20.0)), + (InvestmentSellTransaction, investment_transaction_row(41, 20.0)), + (InvestmentHolding, investment_holding_row()), + ], +) +def test_investment_models_reject_ambiguous_profile(constructor, row) -> None: + with pytest.raises( + UnsupportedInvestmentSchemaError, + match="unsupported investment schema profile", + ): + constructor(mapped_row(row, constructor, schema_profile=UNKNOWN_PROFILE)) + + +@pytest.mark.parametrize( + ("manager", "typename", "row"), + [ + ( + TransactionManager(), + "InvestmentBuyTransaction", + investment_transaction_row(40, -20.0), + ), + (InvestmentHoldingManager(), "InvestmentHolding", investment_holding_row()), + ], +) +def test_managers_propagate_unsupported_investment_schema_errors( + manager, typename, row +) -> None: + with pytest.raises( + UnsupportedInvestmentSchemaError, + match="unsupported investment schema profile", + ): + manager.load( + cast(DatabaseAccessor, ManagerAccessor(typename, row, UNKNOWN_PROFILE)) + ) diff --git a/tests/unit/test_schema_profiles.py b/tests/unit/test_schema_profiles.py index 29187ee..85f8c5e 100644 --- a/tests/unit/test_schema_profiles.py +++ b/tests/unit/test_schema_profiles.py @@ -7,10 +7,36 @@ def make_connection(*columns: str) -> sqlite3.Connection: connection = sqlite3.connect(":memory:") column_sql = ", ".join(f'"{column}" FLOAT' for column in columns) - connection.execute(f"CREATE TABLE ZSYNCOBJECT (Z_PK INTEGER, {column_sql})") + connection.execute( + f"CREATE TABLE ZSYNCOBJECT (Z_PK INTEGER, Z_ENT INTEGER, {column_sql})" + ) return connection +def add_investment_entities(connection: sqlite3.Connection) -> None: + connection.execute("CREATE TABLE Z_PRIMARYKEY (Z_ENT INTEGER, Z_NAME TEXT)") + connection.executemany( + "INSERT INTO Z_PRIMARYKEY VALUES (?, ?)", + [ + (24, "InvestmentHolding"), + (40, "InvestmentBuyTransaction"), + (41, "InvestmentSellTransaction"), + ], + ) + + +def insert_investment_row( + connection: sqlite3.Connection, entity: int, **values: float | None +) -> None: + columns = ["Z_ENT", *values] + placeholders = ", ".join("?" for _ in columns) + quoted_columns = ", ".join(f'"{column}"' for column in columns) + connection.execute( + f'INSERT INTO ZSYNCOBJECT ({quoted_columns}) VALUES ({placeholders})', + [entity, *values.values()], + ) + + def test_detects_suffixed_fixture_profile() -> None: connection = make_connection("ZNUMBEROFSHARES1", "ZPRICEPERSHARE1") @@ -22,8 +48,8 @@ def test_detects_suffixed_fixture_profile() -> None: assert profile.price_per_share_column == "ZPRICEPERSHARE1" -def test_detects_unsuffixed_live_profile() -> None: - connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1") +def test_detects_unsuffixed_profile_when_aliases_are_absent() -> None: + connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE") profile = detect_schema_profile(connection) @@ -48,6 +74,16 @@ def test_detects_consumer_specific_mixed_share_columns() -> None: connection = make_connection( "ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1" ) + add_investment_entities(connection) + insert_investment_row( + connection, 24, ZNUMBEROFSHARES=2.0, ZNUMBEROFSHARES1=None + ) + insert_investment_row( + connection, 40, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=9.0 + ) + insert_investment_row( + connection, 41, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=3.0 + ) profile = detect_schema_profile(connection) @@ -57,6 +93,70 @@ def test_detects_consumer_specific_mixed_share_columns() -> None: assert profile.price_per_share_column == "ZPRICEPERSHARE1" +def test_detects_observed_store_profile_from_entity_rows() -> None: + connection = make_connection( + "ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1" + ) + add_investment_entities(connection) + insert_investment_row( + connection, + 24, + ZNUMBEROFSHARES=None, + ZPRICEPERSHARE=15.0, + ZPRICEPERSHARE1=None, + ) + insert_investment_row( + connection, + 40, + ZNUMBEROFSHARES=2.0, + ZPRICEPERSHARE=None, + ZPRICEPERSHARE1=10.0, + ) + insert_investment_row( + connection, + 41, + ZNUMBEROFSHARES=3.0, + ZPRICEPERSHARE=None, + ZPRICEPERSHARE1=11.0, + ) + + profile = detect_schema_profile(connection) + + assert profile.profile_id == "mixed-investment-columns" + assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES" + assert profile.price_per_share_column == "ZPRICEPERSHARE1" + + +def test_rejects_dual_price_columns_without_discriminating_rows() -> None: + connection = make_connection( + "ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1" + ) + add_investment_entities(connection) + insert_investment_row( + connection, 40, ZNUMBEROFSHARES=2.0, ZPRICEPERSHARE=10.0, ZPRICEPERSHARE1=1.0 + ) + + profile = detect_schema_profile(connection) + + assert profile.profile_id == "unknown" + assert not profile.is_known + + +def test_rejects_ambiguous_investment_column_profile() -> None: + connection = make_connection( + "ZNUMBEROFSHARES", + "ZNUMBEROFSHARES1", + "ZPRICEPERSHARE", + "ZPRICEPERSHARE1", + ) + + profile = detect_schema_profile(connection) + + assert profile.profile_id == "unknown" + assert not profile.is_known + + def test_decimal_alias_reads_both_profiles() -> None: assert ( RawDataHandler.get_decimal_alias( From 61763cc7ead1e3e180d51350156e7a0c899812e7 Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Fri, 25 Sep 2026 18:37:12 +0200 Subject: [PATCH 07/12] fix: preserve callable record constructors --- src/moneywiz_api/database_accessor.py | 15 ++++++++++----- tests/unit/test_database_accessor.py | 13 +++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/moneywiz_api/database_accessor.py b/src/moneywiz_api/database_accessor.py index 01808f9..50b90cd 100644 --- a/src/moneywiz_api/database_accessor.py +++ b/src/moneywiz_api/database_accessor.py @@ -2,7 +2,7 @@ from collections import defaultdict from decimal import Decimal from pathlib import Path -from typing import Any, Dict, List, Tuple +from typing import Any, Callable, Dict, List, Tuple from moneywiz_api.schema.raw_data_handler import RawDataHandler as RDH from moneywiz_api.model.record import Record @@ -89,7 +89,12 @@ def query_objects(self, typenames: List[str]) -> List[Any]: ) return res.fetchall() - def get_record(self, pk_id: ID, constructor: type[Record] = Record): + def _construct_record(self, row, constructor: Callable): + if isinstance(constructor, type) and issubclass(constructor, Record): + return self.schema_profile.create_record(row, constructor) + return constructor(row) + + def get_record(self, pk_id: ID, constructor: Callable = Record): cur = self._con.cursor() res = cur.execute( """ @@ -99,9 +104,9 @@ def get_record(self, pk_id: ID, constructor: type[Record] = Record): [pk_id], ) - return self.schema_profile.create_record(res.fetchone(), constructor) + return self._construct_record(res.fetchone(), constructor) - def get_record_by_gid(self, gid: GID, constructor: type[Record] = Record): + def get_record_by_gid(self, gid: GID, constructor: Callable = Record): cur = self._con.cursor() res = cur.execute( """ @@ -111,7 +116,7 @@ def get_record_by_gid(self, gid: GID, constructor: type[Record] = Record): [gid], ) - return self.schema_profile.create_record(res.fetchone(), constructor) + return self._construct_record(res.fetchone(), constructor) def get_category_assignment(self) -> Dict[ID, List[Tuple[ID, Decimal]]]: transaction_map: Dict[ID, List[Tuple[ID, Decimal]]] = defaultdict(list) diff --git a/tests/unit/test_database_accessor.py b/tests/unit/test_database_accessor.py index e1d12e0..be56d83 100644 --- a/tests/unit/test_database_accessor.py +++ b/tests/unit/test_database_accessor.py @@ -49,3 +49,16 @@ def test_get_tags_map_raises_when_tags_table_is_missing(tmp_path): with pytest.raises(sqlite3.OperationalError, match="no such table"): accessor.get_tags_map() + + +def test_get_record_preserves_callable_constructors(tmp_path): + db_path = tmp_path / "moneywiz.sqlite" + accessor = _create_accessor(db_path) + accessor._con.execute("CREATE TABLE ZSYNCOBJECT (Z_PK INTEGER, ZGID TEXT)") + accessor._con.execute("INSERT INTO ZSYNCOBJECT VALUES (1, 'record-gid')") + + def get_gid(row): + return row["ZGID"] + + assert accessor.get_record(1, get_gid) == "record-gid" + assert accessor.get_record_by_gid("record-gid", get_gid) == "record-gid" From db29aea637eaa5701a88b3537cbca864e50a944b Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Fri, 25 Sep 2026 21:08:23 +0200 Subject: [PATCH 08/12] style: format schema profile tests --- tests/unit/test_schema_profiles.py | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/tests/unit/test_schema_profiles.py b/tests/unit/test_schema_profiles.py index 85f8c5e..471238b 100644 --- a/tests/unit/test_schema_profiles.py +++ b/tests/unit/test_schema_profiles.py @@ -32,7 +32,7 @@ def insert_investment_row( placeholders = ", ".join("?" for _ in columns) quoted_columns = ", ".join(f'"{column}"' for column in columns) connection.execute( - f'INSERT INTO ZSYNCOBJECT ({quoted_columns}) VALUES ({placeholders})', + f"INSERT INTO ZSYNCOBJECT ({quoted_columns}) VALUES ({placeholders})", [entity, *values.values()], ) @@ -75,15 +75,9 @@ def test_detects_consumer_specific_mixed_share_columns() -> None: "ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1" ) add_investment_entities(connection) - insert_investment_row( - connection, 24, ZNUMBEROFSHARES=2.0, ZNUMBEROFSHARES1=None - ) - insert_investment_row( - connection, 40, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=9.0 - ) - insert_investment_row( - connection, 41, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=3.0 - ) + insert_investment_row(connection, 24, ZNUMBEROFSHARES=2.0, ZNUMBEROFSHARES1=None) + insert_investment_row(connection, 40, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=9.0) + insert_investment_row(connection, 41, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=3.0) profile = detect_schema_profile(connection) @@ -94,9 +88,7 @@ def test_detects_consumer_specific_mixed_share_columns() -> None: def test_detects_observed_store_profile_from_entity_rows() -> None: - connection = make_connection( - "ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1" - ) + connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1") add_investment_entities(connection) insert_investment_row( connection, @@ -129,9 +121,7 @@ def test_detects_observed_store_profile_from_entity_rows() -> None: def test_rejects_dual_price_columns_without_discriminating_rows() -> None: - connection = make_connection( - "ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1" - ) + connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1") add_investment_entities(connection) insert_investment_row( connection, 40, ZNUMBEROFSHARES=2.0, ZPRICEPERSHARE=10.0, ZPRICEPERSHARE1=1.0 From c9a257db7af9b4b09122a683137bcfba9ee7528c Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Wed, 14 Oct 2026 21:07:54 +0200 Subject: [PATCH 09/12] Refactor schema profile handling and tests - Removed the schema_profile.py file and integrated its functionality into the schema module. - Updated tests to utilize the new schema profile resolution approach. - Enhanced investment profile creation logic to accommodate custom field mappings. - Improved test coverage for ambiguous investment column scenarios and error handling. - Adjusted database setup in tests to ensure proper schema detection and validation. --- README.md | 12 +- src/moneywiz_api/__init__.py | 2 + src/moneywiz_api/database_accessor.py | 16 +- src/moneywiz_api/managers/record_manager.py | 17 +- src/moneywiz_api/model/transaction.py | 4 +- src/moneywiz_api/schema/raw_data_handler.py | 23 - .../schema/schema_profile_resolver.py | 153 +++++- src/moneywiz_api/schema_profile.py | 129 ----- tests/unit/test_profiled_investment_values.py | 478 ++++++++++++------ tests/unit/test_schema_profiles.py | 277 ++++++---- tests/unit/test_transaction_compatibility.py | 39 +- 11 files changed, 688 insertions(+), 462 deletions(-) delete mode 100644 src/moneywiz_api/schema_profile.py diff --git a/README.md b/README.md index 3bf3a34..2eae49d 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,13 @@ profile.validate() ``` `SchemaProfileResolver(db_path, baseline=None).resolve()` enriches a profile -with database-specific tag-join table details. When no baseline is provided, it +with database-specific tag-join table details and investment column choices. +Explicit baseline alias overrides are preserved; automatic detection applies to +investment fields that retain the default aliases. +Holdings and buy/sell transactions are resolved separately using physical +columns and entity-specific row evidence. If populated investment entities have +no safe column choice, resolution raises `UnsupportedInvestmentSchemaError` +before records are loaded. When no baseline is provided, it uses `DEFAULT_SCHEMA_PROFILE`. The resolved profile can be passed to `MoneywizApi` or the lower-level `DatabaseAccessor`: @@ -101,6 +107,10 @@ a raw row, reports field-resolution failures together, and creates the model instance. Record classes receive mapped field values and do not depend on `SchemaProfile`. +Managers skip malformed rows and report them through `manager.load_errors`, +which returns a copy of the skipped records' IDs, model names, and error types. +Unsupported database schemas stop loading instead of being treated as row errors. + ## Tests Run unit tests without a database: diff --git a/src/moneywiz_api/__init__.py b/src/moneywiz_api/__init__.py index dff6c14..cb3f5f6 100644 --- a/src/moneywiz_api/__init__.py +++ b/src/moneywiz_api/__init__.py @@ -3,11 +3,13 @@ from moneywiz_api.schema.schema_profile_resolver import ( DEFAULT_SCHEMA_PROFILE, SchemaProfileResolver, + UnsupportedInvestmentSchemaError, ) __all__ = [ "MoneywizApi", "SchemaProfile", "SchemaProfileResolver", + "UnsupportedInvestmentSchemaError", "DEFAULT_SCHEMA_PROFILE", ] diff --git a/src/moneywiz_api/database_accessor.py b/src/moneywiz_api/database_accessor.py index 50b90cd..811e801 100644 --- a/src/moneywiz_api/database_accessor.py +++ b/src/moneywiz_api/database_accessor.py @@ -2,16 +2,10 @@ from collections import defaultdict from decimal import Decimal from pathlib import Path -from typing import Any, Callable, Dict, List, Tuple +from typing import Any, Callable, Dict, List, Tuple, cast from moneywiz_api.schema.raw_data_handler import RawDataHandler as RDH from moneywiz_api.model.record import Record -from moneywiz_api.model.investment_holding import InvestmentHolding -from moneywiz_api.model.transaction import ( - InvestmentBuyTransaction, - InvestmentSellTransaction, -) -from moneywiz_api.schema_profile import SchemaProfile, detect_schema_profile from moneywiz_api.types import ENT_ID, GID, ID from moneywiz_api.schema.schema_profile import SchemaProfile @@ -32,7 +26,6 @@ def dict_factory(cursor, row): return record self._con.row_factory = dict_factory - self._schema_profile = detect_schema_profile(self._con) self._ent_to_typename: Dict[ENT_ID, str] = self._load_primarykey() self._typename_to_ent: Dict[str, ENT_ID] = { @@ -56,11 +49,6 @@ def __repr__(self): f"{key}: {value}" for key, value in self._ent_to_typename.items() ) - @property - def schema_profile(self) -> SchemaProfile: - """Return the physical-column compatibility profile for this store.""" - return self._schema_profile - def typename_for(self, ent_id: ENT_ID) -> str: typename = self._ent_to_typename.get(ent_id) assert typename is not None, f"Unknown ent_id {ent_id}" @@ -92,7 +80,7 @@ def query_objects(self, typenames: List[str]) -> List[Any]: def _construct_record(self, row, constructor: Callable): if isinstance(constructor, type) and issubclass(constructor, Record): return self.schema_profile.create_record(row, constructor) - return constructor(row) + return cast(Callable, constructor)(row) def get_record(self, pk_id: ID, constructor: Callable = Record): cur = self._con.cursor() diff --git a/src/moneywiz_api/managers/record_manager.py b/src/moneywiz_api/managers/record_manager.py index d587fd1..74b88b9 100644 --- a/src/moneywiz_api/managers/record_manager.py +++ b/src/moneywiz_api/managers/record_manager.py @@ -1,9 +1,10 @@ from abc import ABC, abstractmethod import logging -from typing import Callable, Dict, Generic, TypeVar +from typing import Dict, Generic, TypeVar from moneywiz_api.database_accessor import DatabaseAccessor from moneywiz_api.model.record import Record +from moneywiz_api.schema.schema_profile_resolver import UnsupportedInvestmentSchemaError from moneywiz_api.types import GID, ID T = TypeVar("T", bound=Record) @@ -28,11 +29,13 @@ def load(self, db_accessor: DatabaseAccessor) -> None: typename = db_accessor.typename_for(record["Z_ENT"]) if typename in self.ents: try: - obj = self.construct_record( - self.ents[typename], record, db_accessor + obj = db_accessor.schema_profile.create_record( + record, self.ents[typename] ) obj.validate() - except (AssertionError, KeyError, ValueError) as exc: + except UnsupportedInvestmentSchemaError: + raise + except (AssertionError, KeyError, RuntimeError, ValueError) as exc: record_id = record.get("Z_PK") detail = type(exc).__name__ self._load_errors.append((record_id, typename, detail)) @@ -45,12 +48,6 @@ def load(self, db_accessor: DatabaseAccessor) -> None: continue self.add(obj) - def construct_record( - self, constructor: Callable, record, db_accessor: DatabaseAccessor - ): - """Construct a record; subclasses can supply schema-specific context.""" - return db_accessor.schema_profile.create_record(record, constructor) - def add(self, record: T) -> None: self._records[record.id] = record if record.gid in self._gid_to_id: diff --git a/src/moneywiz_api/model/transaction.py b/src/moneywiz_api/model/transaction.py index 4c1a8b4..2a8f7b7 100644 --- a/src/moneywiz_api/model/transaction.py +++ b/src/moneywiz_api/model/transaction.py @@ -352,9 +352,7 @@ def __init__(self, row, field_values): raise ValueError( "cannot reconstruct a transfer amount with a zero exchange rate" ) - self.original_amount = -abs( - self.recipient_amount / self.original_exchange_rate - ) + self.original_amount = self.amount def validate(self) -> None: super().validate() diff --git a/src/moneywiz_api/schema/raw_data_handler.py b/src/moneywiz_api/schema/raw_data_handler.py index 1398454..49ae790 100644 --- a/src/moneywiz_api/schema/raw_data_handler.py +++ b/src/moneywiz_api/schema/raw_data_handler.py @@ -27,29 +27,6 @@ def get_decimal(raw_value: Any) -> Decimal: ) return Decimal(str(raw_value)) - @staticmethod - def get_decimal_alias(row: Dict[str, Any], *keys: str) -> Decimal: - """Read a required decimal field from the first available column alias.""" - for key in keys: - if key in row and row[key] is not None: - return RawDataHandler.get_decimal(row, key) - raise KeyError(f"none of the schema aliases are present: {', '.join(keys)}") - - @staticmethod - def get_nullable_decimal_alias( - row: Dict[str, Any], *keys: str - ) -> Optional[Decimal]: - """Read an optional decimal field from the first available column alias.""" - found_alias = False - for key in keys: - if key in row: - found_alias = True - if row[key] is not None: - return RawDataHandler.get_nullable_decimal(row, key) - if found_alias: - return None - raise KeyError(f"none of the schema aliases are present: {', '.join(keys)}") - @staticmethod def filter_row(row: Dict[str, Any]) -> Dict[str, Any]: copy = {k: v for k, v in row.items()} diff --git a/src/moneywiz_api/schema/schema_profile_resolver.py b/src/moneywiz_api/schema/schema_profile_resolver.py index 7f5a8c7..9306478 100644 --- a/src/moneywiz_api/schema/schema_profile_resolver.py +++ b/src/moneywiz_api/schema/schema_profile_resolver.py @@ -4,7 +4,14 @@ import sqlite3 from pathlib import Path +from moneywiz_api.model.investment_holding import InvestmentHolding +from moneywiz_api.model.transaction import ( + InvestmentBuyTransaction, + InvestmentSellTransaction, +) + from moneywiz_api.schema.schema_fields import ( + FieldSpec, datetime_field, decimal_field, is_one_field, @@ -15,6 +22,10 @@ from moneywiz_api.utils import get_datetime +class UnsupportedInvestmentSchemaError(ValueError): + """Raised when investment columns cannot be selected safely for a store.""" + + class SchemaProfileResolver: """Build a profile from a database path and an optional baseline profile.""" @@ -23,19 +34,149 @@ def __init__(self, db_path: str | Path, baseline: SchemaProfile | None = None): self._baseline = DEFAULT_SCHEMA_PROFILE if baseline is None else baseline def resolve(self) -> SchemaProfile: - """Return the baseline enriched with tag-table details from the database.""" - if self._baseline.tag_table_info is not None: - return self._baseline - + """Return the baseline enriched with database-specific schema details.""" connection = sqlite3.connect(self._db_path, uri=True) connection.row_factory = sqlite3.Row try: - return self._baseline.with_tag_table_info( - self._get_tags_table_info(connection) + column_map = { + model_name: dict(fields) + for model_name, fields in self._baseline.column_map.items() + } + self._resolve_investment_fields(connection, column_map) + return SchemaProfile(column_map).with_tag_table_info( + self._baseline.tag_table_info or self._get_tags_table_info(connection) ) finally: connection.close() + def _resolve_investment_fields( + self, + connection: sqlite3.Connection, + column_map: dict[str | type, dict[str, FieldSpec]], + ) -> None: + tables = { + row["name"] + for row in connection.execute( + "SELECT name FROM sqlite_master WHERE type = 'table'" + ).fetchall() + } + if "ZSYNCOBJECT" not in tables or "Z_PRIMARYKEY" not in tables: + return + + entities: dict[str, int] = { + row["Z_NAME"]: row["Z_ENT"] + for row in connection.execute( + "SELECT Z_ENT, Z_NAME FROM Z_PRIMARYKEY WHERE Z_NAME IN (?, ?, ?)", + ( + "InvestmentHolding", + "InvestmentBuyTransaction", + "InvestmentSellTransaction", + ), + ).fetchall() + } + if not entities: + return + + active_entity_ids = { + row["Z_ENT"] + for row in connection.execute( + "SELECT DISTINCT Z_ENT FROM ZSYNCOBJECT WHERE Z_ENT IN (" + + ", ".join("?" for _ in entities) + + ")", + tuple(entities.values()), + ).fetchall() + } + active_entities = { + name: entity_id + for name, entity_id in entities.items() + if entity_id in active_entity_ids + } + if not active_entities: + return + + columns = { + row["name"] + for row in connection.execute('PRAGMA table_info("ZSYNCOBJECT")') + } + for models, field_name, aliases in ( + ( + (InvestmentHolding,), + "number_of_shares", + ("ZNUMBEROFSHARES", "ZNUMBEROFSHARES1"), + ), + ( + (InvestmentBuyTransaction, InvestmentSellTransaction), + "number_of_shares", + ("ZNUMBEROFSHARES", "ZNUMBEROFSHARES1"), + ), + ( + (InvestmentBuyTransaction, InvestmentSellTransaction), + "price_per_share", + ("ZPRICEPERSHARE", "ZPRICEPERSHARE1"), + ), + ): + specs = { + model: self._baseline.specs_for(model)[field_name] + for model in models + if model.__name__ in active_entities + } + automatic_specs = { + model: spec + for model, spec in specs.items() + if spec.aliases + == DEFAULT_SCHEMA_PROFILE.specs_for(model)[field_name].aliases + } + if not automatic_specs: + continue + column = self._column_for_entities( + connection, + columns, + aliases[0], + aliases[1], + tuple(active_entities[model.__name__] for model in automatic_specs), + ) + if column is None: + consumer = ( + "holding shares column" + if models[0] is InvestmentHolding + else "transaction columns" + ) + raise UnsupportedInvestmentSchemaError( + f"unsupported investment schema profile: ambiguous {consumer}" + ) + for model, spec in automatic_specs.items(): + column_map.setdefault(model.__name__, {})[field_name] = FieldSpec( + (column,), spec.converter + ) + + @staticmethod + def _column_for_entities( + connection: sqlite3.Connection, + columns: set[str], + unsuffixed: str, + suffixed: str, + entity_ids: tuple[int, ...], + ) -> str | None: + has_unsuffixed = unsuffixed in columns + has_suffixed = suffixed in columns + if has_unsuffixed != has_suffixed: + return unsuffixed if has_unsuffixed else suffixed + if not has_unsuffixed: + return None + + placeholders = ", ".join("?" for _ in entity_ids) + counts = connection.execute( + f'SELECT COUNT("{unsuffixed}"), COUNT("{suffixed}") ' + f'FROM "ZSYNCOBJECT" WHERE "Z_ENT" IN ({placeholders})', + entity_ids, + ).fetchone() + unsuffixed_count, suffixed_count = counts[0], counts[1] + if unsuffixed_count and not suffixed_count: + return unsuffixed + if suffixed_count and not unsuffixed_count: + return suffixed + return None + @staticmethod def _get_tags_table_info(connection: sqlite3.Connection) -> TagTableInfo: cursor = connection.cursor() diff --git a/src/moneywiz_api/schema_profile.py b/src/moneywiz_api/schema_profile.py deleted file mode 100644 index 71fb411..0000000 --- a/src/moneywiz_api/schema_profile.py +++ /dev/null @@ -1,129 +0,0 @@ -"""Schema capability profiles for MoneyWiz SQLite stores.""" - -from __future__ import annotations - -import sqlite3 -from dataclasses import dataclass -from typing import Any, Iterable - - -@dataclass(frozen=True) -class SchemaProfile: - """Capabilities inferred from physical columns and investment rows.""" - - profile_id: str - holding_number_of_shares_column: str | None - transaction_number_of_shares_column: str | None - price_per_share_column: str | None - - @property - def is_known(self) -> bool: - return self.profile_id != "unknown" - - -def _value(row: Any, name: str, index: int) -> Any: - if isinstance(row, dict): - return row[name] - return row[index] - - -def _column_for_entities( - connection: sqlite3.Connection, - columns: set[str], - unsuffixed_column: str, - suffixed_column: str, - entity_names: Iterable[str], -) -> str | None: - has_unsuffixed = unsuffixed_column in columns - has_suffixed = suffixed_column in columns - if has_unsuffixed != has_suffixed: - return unsuffixed_column if has_unsuffixed else suffixed_column - if not has_unsuffixed: - return None - - tables = { - str(_value(row, "name", 0)) - for row in connection.execute( - "SELECT name FROM sqlite_master WHERE type = 'table'" - ).fetchall() - } - if "Z_PRIMARYKEY" not in tables: - return None - - names = tuple(entity_names) - placeholders = ", ".join("?" for _ in names) - entity_rows = connection.execute( - f'SELECT "Z_ENT", "Z_NAME" FROM "Z_PRIMARYKEY" ' - f'WHERE "Z_NAME" IN ({placeholders})', - names, - ).fetchall() - entity_ids = tuple(_value(row, "Z_ENT", 0) for row in entity_rows) - if not entity_ids: - return None - - placeholders = ", ".join("?" for _ in entity_ids) - counts = connection.execute( - f'SELECT COUNT("{unsuffixed_column}") AS "unsuffixed_count", ' - f'COUNT("{suffixed_column}") AS "suffixed_count" ' - f'FROM "ZSYNCOBJECT" WHERE "Z_ENT" IN ({placeholders})', - entity_ids, - ).fetchone() - unsuffixed_count = int(_value(counts, "unsuffixed_count", 0)) - suffixed_count = int(_value(counts, "suffixed_count", 1)) - if unsuffixed_count and not suffixed_count: - return unsuffixed_column - if suffixed_count and not unsuffixed_count: - return suffixed_column - return None - - -def detect_schema_profile(connection: sqlite3.Connection) -> SchemaProfile: - """Select investment aliases using columns and entity-specific row evidence.""" - columns = { - str(_value(row, "name", 1)) - for row in connection.execute("PRAGMA table_info(ZSYNCOBJECT)").fetchall() - } - - holding_shares = _column_for_entities( - connection, - columns, - "ZNUMBEROFSHARES", - "ZNUMBEROFSHARES1", - ("InvestmentHolding",), - ) - transaction_entities = ("InvestmentBuyTransaction", "InvestmentSellTransaction") - transaction_shares = _column_for_entities( - connection, - columns, - "ZNUMBEROFSHARES", - "ZNUMBEROFSHARES1", - transaction_entities, - ) - price_per_share = _column_for_entities( - connection, - columns, - "ZPRICEPERSHARE", - "ZPRICEPERSHARE1", - transaction_entities, - ) - - if not holding_shares or not transaction_shares or not price_per_share: - profile_id = "unknown" - elif len( - { - column.endswith("1") - for column in (holding_shares, transaction_shares, price_per_share) - } - ) > 1: - profile_id = "mixed-investment-columns" - elif holding_shares.endswith("1"): - profile_id = "suffixed-investment-columns" - else: - profile_id = "unsuffixed-investment-columns" - - return SchemaProfile( - profile_id=profile_id, - holding_number_of_shares_column=holding_shares, - transaction_number_of_shares_column=transaction_shares, - price_per_share_column=price_per_share, - ) diff --git a/tests/unit/test_profiled_investment_values.py b/tests/unit/test_profiled_investment_values.py index 087de65..2f1af76 100644 --- a/tests/unit/test_profiled_investment_values.py +++ b/tests/unit/test_profiled_investment_values.py @@ -1,42 +1,51 @@ +import sqlite3 from decimal import Decimal -from typing import Any, cast +from typing import cast +from unittest.mock import Mock, patch import pytest +from moneywiz_api import ( + DEFAULT_SCHEMA_PROFILE, + MoneywizApi, + SchemaProfile, + SchemaProfileResolver, +) +from moneywiz_api.database_accessor import DatabaseAccessor from moneywiz_api.managers.investment_holding_manager import InvestmentHoldingManager from moneywiz_api.managers.transaction_manager import TransactionManager -from moneywiz_api.database_accessor import DatabaseAccessor from moneywiz_api.model.investment_holding import InvestmentHolding -from moneywiz_api.model.schema_mapped_row import mapped_row from moneywiz_api.model.transaction import ( InvestmentBuyTransaction, InvestmentSellTransaction, ) -from moneywiz_api.schema_profile import ( - SchemaProfile, - UnsupportedInvestmentSchemaError, +from moneywiz_api.schema.schema_fields import ( + decimal_field, + nullable_decimal_field, + schema_field, ) +from moneywiz_api.schema.schema_profile_resolver import UnsupportedInvestmentSchemaError -UNSUFFIXED_PROFILE = SchemaProfile( - profile_id="unsuffixed-investment-columns", - holding_number_of_shares_column="ZNUMBEROFSHARES", - transaction_number_of_shares_column="ZNUMBEROFSHARES", - price_per_share_column="ZPRICEPERSHARE", -) -MIXED_PROFILE = SchemaProfile( - profile_id="mixed-investment-columns", - holding_number_of_shares_column="ZNUMBEROFSHARES", - transaction_number_of_shares_column="ZNUMBEROFSHARES1", - price_per_share_column="ZPRICEPERSHARE1", +def investment_profile(holding_shares, transaction_shares, price): + definitions = { + name: dict(fields) for name, fields in DEFAULT_SCHEMA_PROFILE.column_map.items() + } + definitions["InvestmentHolding"]["number_of_shares"] = nullable_decimal_field( + holding_shares + ) + for model in ("InvestmentBuyTransaction", "InvestmentSellTransaction"): + definitions[model]["number_of_shares"] = decimal_field(transaction_shares) + definitions[model]["price_per_share"] = decimal_field(price) + return SchemaProfile(definitions) + + +UNSUFFIXED_PROFILE = investment_profile( + "ZNUMBEROFSHARES", "ZNUMBEROFSHARES", "ZPRICEPERSHARE" ) -OBSERVED_STORE_PROFILE = SchemaProfile( - profile_id="mixed-investment-columns", - holding_number_of_shares_column="ZNUMBEROFSHARES", - transaction_number_of_shares_column="ZNUMBEROFSHARES", - price_per_share_column="ZPRICEPERSHARE1", +MIXED_PROFILE = investment_profile( + "ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1" ) -UNKNOWN_PROFILE = SchemaProfile("unknown", None, None, None) def investment_transaction_row(ent: int, amount: float) -> dict: @@ -52,7 +61,7 @@ def investment_transaction_row(ent: int, amount: float) -> dict: "ZNOTES1": None, "ZACCOUNT2": 1, "ZFEE2": 0.0, - "ZINVESTMENTHOLDING": 2, + "ZINVESTMENTHOLDING": 24, "ZNUMBEROFSHARES": 2.0, "ZNUMBEROFSHARES1": 9.0, "ZPRICEPERSHARE": 10.0, @@ -74,8 +83,6 @@ def investment_holding_row() -> dict: "ZHOLDINGTYPE": None, "ZDESC": "Acme Corp.", "ZISPRICEPERSHAREAVAILABLEONLINE": 0, - "ZINVESTMENTOBJECTTYPE": 0, - "ZCOSTBASISOFMISSINGOBSHARES": 0.0, } @@ -87,44 +94,30 @@ def investment_holding_row() -> dict: ], ) def test_transactions_use_selected_profile_alias(constructor, row) -> None: - transaction = constructor( - mapped_row(row, constructor, schema_profile=UNSUFFIXED_PROFILE) - ) + transaction = UNSUFFIXED_PROFILE.create_record(row, constructor) assert transaction.number_of_shares == Decimal("2.0") assert transaction.price_per_share == Decimal("10.0") def test_holding_uses_selected_profile_alias() -> None: - holding = InvestmentHolding( - mapped_row( - investment_holding_row(), - InvestmentHolding, - schema_profile=UNSUFFIXED_PROFILE, - ) + holding = UNSUFFIXED_PROFILE.create_record( + investment_holding_row(), InvestmentHolding ) assert holding.number_of_shares == Decimal("2.0") @pytest.mark.parametrize( - ("constructor", "transaction_row"), + ("constructor", "row"), [ (InvestmentBuyTransaction, investment_transaction_row(40, -9.0)), (InvestmentSellTransaction, investment_transaction_row(41, 9.0)), ], ) -def test_mixed_profile_uses_consumer_specific_share_aliases( - constructor, transaction_row -) -> None: - holding_row = investment_holding_row() - - transaction = constructor( - mapped_row(transaction_row, constructor, schema_profile=MIXED_PROFILE) - ) - holding = InvestmentHolding( - mapped_row(holding_row, InvestmentHolding, schema_profile=MIXED_PROFILE) - ) +def test_mixed_profile_uses_consumer_specific_share_aliases(constructor, row) -> None: + transaction = MIXED_PROFILE.create_record(row, constructor) + holding = MIXED_PROFILE.create_record(investment_holding_row(), InvestmentHolding) assert transaction.number_of_shares == Decimal("9.0") assert transaction.price_per_share == Decimal("1.0") @@ -141,48 +134,108 @@ def test_mixed_profile_uses_consumer_specific_share_aliases( def test_observed_store_profile_uses_suffixed_transaction_price( constructor, row ) -> None: - transaction = constructor( - mapped_row(row, constructor, schema_profile=OBSERVED_STORE_PROFILE) - ) + transaction = DEFAULT_SCHEMA_PROFILE.create_record(row, constructor) assert transaction.number_of_shares == Decimal("2.0") assert transaction.price_per_share == Decimal("1.0") -class ProfileAccessor: - schema_profile = UNSUFFIXED_PROFILE +def create_investment_database(db_path, rows): + columns = sorted({column for row in rows for column in row}) + column_sql = ", ".join(f'"{column}"' for column in columns) + placeholders = ", ".join("?" for _ in columns) + with sqlite3.connect(db_path) as connection: + connection.execute(f"CREATE TABLE ZSYNCOBJECT ({column_sql})") + connection.executemany( + f"INSERT INTO ZSYNCOBJECT VALUES ({placeholders})", + [[row.get(column) for column in columns] for row in rows], + ) + connection.execute("CREATE TABLE Z_PRIMARYKEY (Z_ENT INTEGER, Z_NAME TEXT)") + connection.executemany( + "INSERT INTO Z_PRIMARYKEY VALUES (?, ?)", + [ + (24, "InvestmentHolding"), + (40, "InvestmentBuyTransaction"), + (41, "InvestmentSellTransaction"), + ] + + [ + (100 + index, name) + for index, name in enumerate(TransactionManager().ents) + if name not in {"InvestmentBuyTransaction", "InvestmentSellTransaction"} + ], + ) + connection.execute( + "CREATE TABLE Z_37TAGS (Z_3TRANSACTIONS INTEGER, Z_4TAGS INTEGER)" + ) + connection.close() -def test_managers_pass_profile_to_investment_constructors() -> None: - accessor = ProfileAccessor() - transaction = TransactionManager().construct_record( - InvestmentBuyTransaction, - investment_transaction_row(40, -20.0), - cast(DatabaseAccessor, accessor), - ) - holding = InvestmentHoldingManager().construct_record( - InvestmentHolding, +@pytest.mark.parametrize( + ("holding_shares", "transaction_shares", "price"), + [ + ("ZNUMBEROFSHARES", "ZNUMBEROFSHARES", "ZPRICEPERSHARE"), + ("ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1"), + ("ZNUMBEROFSHARES", "ZNUMBEROFSHARES", "ZPRICEPERSHARE1"), + ("ZNUMBEROFSHARES1", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1"), + ], +) +def test_managers_and_direct_reads_use_same_resolved_profile( + tmp_path, holding_shares, transaction_shares, price +) -> None: + rows = [ investment_holding_row(), - cast(DatabaseAccessor, accessor), - ) - - assert transaction.price_per_share == Decimal("10.0") - assert holding.number_of_shares == Decimal("2.0") - + investment_transaction_row(40, -20.0), + investment_transaction_row(41, 20.0), + ] + for row in rows: + for column in ( + "ZNUMBEROFSHARES", + "ZNUMBEROFSHARES1", + "ZPRICEPERSHARE", + "ZPRICEPERSHARE1", + ): + row[column] = None + row[holding_shares if row["Z_ENT"] == 24 else transaction_shares] = 2.0 + if row["Z_ENT"] != 24: + row[price] = 10.0 + db_path = tmp_path / "moneywiz.sqlite" + create_investment_database(db_path, rows) + profile = SchemaProfileResolver(db_path).resolve() + accessor = DatabaseAccessor(db_path, profile) + try: + transaction_manager = TransactionManager() + transaction_manager.load(accessor) + holding_manager = InvestmentHoldingManager() + holding_manager.load(accessor) + + for pk, constructor in ( + (40, InvestmentBuyTransaction), + (41, InvestmentSellTransaction), + ): + loaded = transaction_manager.get(pk) + direct = accessor.get_record(pk, constructor) + by_gid = accessor.get_record_by_gid(f"investment-{pk}", constructor) + assert loaded == direct == by_gid + assert direct.number_of_shares == Decimal("2.0") + assert direct.price_per_share == Decimal("10.0") + holding = accessor.get_record(24, InvestmentHolding) + assert holding_manager.get(24) == holding + assert holding.number_of_shares == Decimal("2.0") + assert accessor.schema_profile is profile + assert transaction_manager.load_errors == holding_manager.load_errors == [] + finally: + accessor._con.close() + + +class ManagerAccessor: + schema_profile = UNSUFFIXED_PROFILE -class ManagerAccessor(ProfileAccessor): - def __init__( - self, - typename: str, - row: dict, - schema_profile: SchemaProfile = UNSUFFIXED_PROFILE, - ): + def __init__(self, typename, rows): self.typename = typename - self.row = row - self.schema_profile = schema_profile + self.rows = rows def query_objects(self, _typenames): - return [self.row] + return self.rows def typename_for(self, _ent_id): return self.typename @@ -197,102 +250,219 @@ def get_tags_map(self): return {} -def test_managers_load_profiled_investment_records() -> None: - transaction_manager = TransactionManager() - transaction_manager.load( - cast( - DatabaseAccessor, - ManagerAccessor( - "InvestmentBuyTransaction", investment_transaction_row(40, -20.0) - ), - ) - ) - holding_manager = InvestmentHoldingManager() - holding_manager.load( +@pytest.mark.parametrize("malformation", ["invalid", "null", "missing", "identity"]) +def test_managers_report_bad_rows_and_load_later_valid_rows(malformation) -> None: + bad_row = investment_transaction_row(40, -20.0) + if malformation == "missing": + del bad_row["ZPRICEPERSHARE"] + elif malformation == "identity": + bad_row["ZGID"] = None + else: + bad_row["ZPRICEPERSHARE"] = None if malformation == "null" else "invalid" + good_row = {**investment_transaction_row(40, -20.0), "Z_PK": 42, "ZGID": "valid"} + manager = TransactionManager() + + manager.load( cast( DatabaseAccessor, - ManagerAccessor("InvestmentHolding", investment_holding_row()), + ManagerAccessor("InvestmentBuyTransaction", [bad_row, good_row]), ) ) - transaction = transaction_manager.get(40) - holding = holding_manager.get(24) - assert isinstance(transaction, InvestmentBuyTransaction) - assert holding is not None - assert transaction.price_per_share == Decimal("10.0") - assert holding.number_of_shares == Decimal("2.0") - + assert list(manager.records()) == [42] + error_type = "AssertionError" if malformation == "identity" else "RuntimeError" + assert manager.load_errors == [(40, "InvestmentBuyTransaction", error_type)] + manager.load_errors.clear() + assert len(manager.load_errors) == 1 -class StaticCursor: - def __init__(self, row: dict): - self.row = row - def execute(self, _query, _parameters): - return self +def test_ambiguous_store_schema_stops_before_record_loading(tmp_path) -> None: + db_path = tmp_path / "moneywiz.sqlite" + create_investment_database(db_path, [investment_transaction_row(40, -20.0)]) - def fetchone(self): - return self.row + with patch("moneywiz_api.moneywiz_api.DatabaseAccessor") as accessor: + with pytest.raises(UnsupportedInvestmentSchemaError): + MoneywizApi(db_path) + accessor.assert_not_called() -class StaticConnection: - def __init__(self, row: dict): - self.row = row - - def cursor(self): - return StaticCursor(self.row) - - -def test_accessor_public_constructors_receive_schema_profile() -> None: - transaction_accessor = DatabaseAccessor.__new__(DatabaseAccessor) - transaction_accessor._con = cast( - Any, StaticConnection(investment_transaction_row(40, -20.0)) +def test_manager_does_not_skip_unsupported_schema_errors() -> None: + accessor = Mock() + accessor.query_objects.return_value = [investment_transaction_row(40, -20.0)] + accessor.typename_for.return_value = "InvestmentBuyTransaction" + accessor.schema_profile.create_record.side_effect = ( + UnsupportedInvestmentSchemaError("unsupported schema") ) - transaction_accessor._schema_profile = UNSUFFIXED_PROFILE - holding_accessor = DatabaseAccessor.__new__(DatabaseAccessor) - holding_accessor._con = cast(Any, StaticConnection(investment_holding_row())) - holding_accessor._schema_profile = UNSUFFIXED_PROFILE + manager = TransactionManager() - transaction = transaction_accessor.get_record(40, InvestmentBuyTransaction) - holding = holding_accessor.get_record_by_gid("holding", InvestmentHolding) + with pytest.raises(UnsupportedInvestmentSchemaError): + manager.load(accessor) + assert manager.load_errors == [] - assert transaction.price_per_share == Decimal("10.0") - assert holding.number_of_shares == Decimal("2.0") + +def default_investment_rows(): + rows = [ + investment_holding_row(), + investment_transaction_row(40, -20.0), + investment_transaction_row(41, 20.0), + ] + for row in rows: + row["ZNUMBEROFSHARES1"] = None + row["ZPRICEPERSHARE"] = None + if row["Z_ENT"] != 24: + row["ZPRICEPERSHARE1"] = 10.0 + return rows @pytest.mark.parametrize( - ("constructor", "row"), + ("model", "field_name"), [ - (InvestmentBuyTransaction, investment_transaction_row(40, -20.0)), - (InvestmentSellTransaction, investment_transaction_row(41, 20.0)), - (InvestmentHolding, investment_holding_row()), + (InvestmentHolding, "number_of_shares"), + (InvestmentBuyTransaction, "number_of_shares"), + (InvestmentSellTransaction, "number_of_shares"), + (InvestmentBuyTransaction, "price_per_share"), + (InvestmentSellTransaction, "price_per_share"), ], ) -def test_investment_models_reject_ambiguous_profile(constructor, row) -> None: - with pytest.raises( - UnsupportedInvestmentSchemaError, - match="unsupported investment schema profile", +def test_resolver_preserves_custom_investment_aliases(tmp_path, model, field_name): + definitions = { + name: dict(fields) for name, fields in DEFAULT_SCHEMA_PROFILE.column_map.items() + } + spec = DEFAULT_SCHEMA_PROFILE.specs_for(model)[field_name] + definitions[model.__name__][field_name] = schema_field( + "CUSTOM_FIELD", converter=spec.converter + ) + rows = default_investment_rows() + entity = { + InvestmentHolding: 24, + InvestmentBuyTransaction: 40, + InvestmentSellTransaction: 41, + }[model] + row = next(row for row in rows if row["Z_ENT"] == entity) + row["CUSTOM_FIELD"] = 7.0 if field_name == "number_of_shares" else 12.0 + # Ambiguous stock aliases on the customized consumer cannot override its mapping + # or invalidate inference for the other consumer. + row[ + "ZNUMBEROFSHARES1" if field_name == "number_of_shares" else "ZPRICEPERSHARE" + ] = 9.0 + if entity != 24: + amount = 70.0 if field_name == "number_of_shares" else 24.0 + row["ZAMOUNT1"] = -amount if entity == 40 else amount + db_path = tmp_path / "moneywiz.sqlite" + create_investment_database(db_path, rows) + baseline = SchemaProfile(definitions) + + profile = SchemaProfileResolver(db_path, baseline).resolve() + record = profile.create_record(row, model) + record.validate() + + assert getattr(record, field_name) == Decimal(str(row["CUSTOM_FIELD"])) + assert profile.specs_for(model)[field_name].aliases == ("CUSTOM_FIELD",) + assert profile.specs_for(model)[field_name].converter is spec.converter + assert baseline.column_map == definitions + + +@pytest.mark.parametrize("custom_aliases", [False, True]) +def test_resolver_preserves_inherited_investment_fields(tmp_path, custom_aliases): + definitions = { + name: dict(fields) for name, fields in DEFAULT_SCHEMA_PROFILE.column_map.items() + } + definitions["InvestmentTransaction"] = definitions["InvestmentBuyTransaction"] + definitions["InvestmentBuyTransaction"] = {} + definitions["InvestmentSellTransaction"] = {} + if custom_aliases: + definitions["InvestmentTransaction"]["number_of_shares"] = decimal_field( + "CUSTOM_SHARES" + ) + definitions["InvestmentTransaction"]["price_per_share"] = decimal_field( + "CUSTOM_PRICE" + ) + rows = default_investment_rows() + for row in rows[1:]: + row["CUSTOM_SHARES"] = 2.0 + row["CUSTOM_PRICE"] = 10.0 + row["ZNUMBEROFSHARES"] = None + row["ZNUMBEROFSHARES1"] = 2.0 + row["ZPRICEPERSHARE"] = 10.0 + row["ZPRICEPERSHARE1"] = None + db_path = tmp_path / "moneywiz.sqlite" + create_investment_database(db_path, rows) + baseline = SchemaProfile(definitions) + + profile = SchemaProfileResolver(db_path, baseline).resolve() + + for row, model in zip( + rows[1:], (InvestmentBuyTransaction, InvestmentSellTransaction) ): - constructor(mapped_row(row, constructor, schema_profile=UNKNOWN_PROFILE)) + record = profile.create_record(row, model) + record.validate() + assert record.number_of_shares == Decimal("2.0") + assert record.price_per_share == Decimal("10.0") + assert baseline.column_map == definitions -@pytest.mark.parametrize( - ("manager", "typename", "row"), - [ - ( - TransactionManager(), - "InvestmentBuyTransaction", - investment_transaction_row(40, -20.0), - ), - (InvestmentHoldingManager(), "InvestmentHolding", investment_holding_row()), - ], -) -def test_managers_propagate_unsupported_investment_schema_errors( - manager, typename, row -) -> None: - with pytest.raises( - UnsupportedInvestmentSchemaError, - match="unsupported investment schema profile", - ): - manager.load( - cast(DatabaseAccessor, ManagerAccessor(typename, row, UNKNOWN_PROFILE)) - ) +def test_unrelated_custom_mapping_keeps_investment_detection_and_converter(tmp_path): + definitions = { + name: dict(fields) for name, fields in DEFAULT_SCHEMA_PROFILE.column_map.items() + } + definitions["Tag"]["name"] = schema_field("CUSTOM_TAG_NAME") + + def converter(value): + return Decimal(str(value)) + + definitions["InvestmentBuyTransaction"]["price_per_share"] = schema_field( + "ZPRICEPERSHARE1", converter=converter + ) + rows = default_investment_rows() + for row in rows[1:]: + row["ZPRICEPERSHARE"] = 10.0 + row["ZPRICEPERSHARE1"] = None + db_path = tmp_path / "moneywiz.sqlite" + create_investment_database(db_path, rows) + + profile = SchemaProfileResolver(db_path, SchemaProfile(definitions)).resolve() + record = profile.create_record(rows[1], InvestmentBuyTransaction) + record.validate() + + assert record.price_per_share == Decimal("10.0") + assert ( + profile.specs_for(InvestmentBuyTransaction)["price_per_share"].converter + is converter + ) + assert profile.column_map["Tag"]["name"].aliases == ("CUSTOM_TAG_NAME",) + + +def test_custom_price_does_not_require_stock_price_columns(tmp_path): + definitions = { + name: dict(fields) for name, fields in DEFAULT_SCHEMA_PROFILE.column_map.items() + } + definitions["InvestmentBuyTransaction"]["price_per_share"] = decimal_field( + "CUSTOM_PRICE" + ) + row = investment_transaction_row(40, -24.0) + row["ZNUMBEROFSHARES1"] = None + row["CUSTOM_PRICE"] = 12.0 + del row["ZPRICEPERSHARE"] + del row["ZPRICEPERSHARE1"] + db_path = tmp_path / "moneywiz.sqlite" + create_investment_database(db_path, [row]) + + profile = SchemaProfileResolver(db_path, SchemaProfile(definitions)).resolve() + record = profile.create_record(row, InvestmentBuyTransaction) + record.validate() + assert record.price_per_share == Decimal("12.0") + + +def test_custom_price_does_not_bypass_ambiguous_default_shares(tmp_path): + definitions = { + name: dict(fields) for name, fields in DEFAULT_SCHEMA_PROFILE.column_map.items() + } + definitions["InvestmentBuyTransaction"]["price_per_share"] = decimal_field( + "CUSTOM_PRICE" + ) + row = {**investment_transaction_row(40, -24.0), "CUSTOM_PRICE": 12.0} + db_path = tmp_path / "moneywiz.sqlite" + create_investment_database(db_path, [row]) + + with pytest.raises(UnsupportedInvestmentSchemaError): + SchemaProfileResolver(db_path, SchemaProfile(definitions)).resolve() diff --git a/tests/unit/test_schema_profiles.py b/tests/unit/test_schema_profiles.py index 471238b..291b17a 100644 --- a/tests/unit/test_schema_profiles.py +++ b/tests/unit/test_schema_profiles.py @@ -1,19 +1,18 @@ import sqlite3 -from moneywiz_api.model.raw_data_handler import RawDataHandler -from moneywiz_api.schema_profile import detect_schema_profile +import pytest +from moneywiz_api import DEFAULT_SCHEMA_PROFILE, SchemaProfile, SchemaProfileResolver +from moneywiz_api.schema.raw_data_handler import RawDataHandler +from moneywiz_api.schema.schema_profile_resolver import UnsupportedInvestmentSchemaError -def make_connection(*columns: str) -> sqlite3.Connection: - connection = sqlite3.connect(":memory:") + +def make_connection(db_path, *columns: str) -> sqlite3.Connection: + connection = sqlite3.connect(db_path) column_sql = ", ".join(f'"{column}" FLOAT' for column in columns) connection.execute( f"CREATE TABLE ZSYNCOBJECT (Z_PK INTEGER, Z_ENT INTEGER, {column_sql})" ) - return connection - - -def add_investment_entities(connection: sqlite3.Connection) -> None: connection.execute("CREATE TABLE Z_PRIMARYKEY (Z_ENT INTEGER, Z_NAME TEXT)") connection.executemany( "INSERT INTO Z_PRIMARYKEY VALUES (?, ?)", @@ -23,6 +22,10 @@ def add_investment_entities(connection: sqlite3.Connection) -> None: (41, "InvestmentSellTransaction"), ], ) + connection.execute( + "CREATE TABLE Z_37TAGS (Z_3TRANSACTIONS INTEGER, Z_4TAGS INTEGER)" + ) + return connection def insert_investment_row( @@ -37,151 +40,209 @@ def insert_investment_row( ) -def test_detects_suffixed_fixture_profile() -> None: - connection = make_connection("ZNUMBEROFSHARES1", "ZPRICEPERSHARE1") - - profile = detect_schema_profile(connection) - - assert profile.profile_id == "suffixed-investment-columns" - assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES1" - assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES1" - assert profile.price_per_share_column == "ZPRICEPERSHARE1" +def resolve_profile(connection, db_path): + connection.commit() + connection.close() + return SchemaProfileResolver(db_path).resolve() -def test_detects_unsuffixed_profile_when_aliases_are_absent() -> None: - connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE") +@pytest.mark.parametrize( + ("shares_column", "price_column"), + [ + ("ZNUMBEROFSHARES1", "ZPRICEPERSHARE1"), + ("ZNUMBEROFSHARES", "ZPRICEPERSHARE"), + ("ZNUMBEROFSHARES", "ZPRICEPERSHARE1"), + ], +) +def test_detects_investment_columns_when_aliases_are_absent( + tmp_path, shares_column, price_column +) -> None: + db_path = tmp_path / "moneywiz.sqlite" + connection = make_connection(db_path, shares_column, price_column) + insert_investment_row(connection, 24, **{shares_column: 2.0}) + for entity in (40, 41): + insert_investment_row( + connection, entity, **{shares_column: 2.0, price_column: 10.0} + ) - profile = detect_schema_profile(connection) + profile = resolve_profile(connection, db_path) - assert profile.profile_id == "unsuffixed-investment-columns" - assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES" - assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES" - assert profile.price_per_share_column == "ZPRICEPERSHARE" + assert profile.column_map["InvestmentHolding"]["number_of_shares"].aliases == ( + shares_column, + ) + for model in ("InvestmentBuyTransaction", "InvestmentSellTransaction"): + assert profile.column_map[model]["number_of_shares"].aliases == (shares_column,) + assert profile.column_map[model]["price_per_share"].aliases == (price_column,) -def test_detects_mixed_investment_columns() -> None: - connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE1") +def test_detects_consumer_specific_mixed_share_columns(tmp_path) -> None: + db_path = tmp_path / "moneywiz.sqlite" + connection = make_connection( + db_path, "ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1" + ) + insert_investment_row(connection, 24, ZNUMBEROFSHARES=2.0, ZNUMBEROFSHARES1=None) + insert_investment_row( + connection, 40, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=9.0, ZPRICEPERSHARE1=10.0 + ) + insert_investment_row( + connection, 41, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=3.0, ZPRICEPERSHARE1=11.0 + ) - profile = detect_schema_profile(connection) + profile = resolve_profile(connection, db_path) - assert profile.profile_id == "mixed-investment-columns" - assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES" - assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES" - assert profile.price_per_share_column == "ZPRICEPERSHARE1" + assert profile.column_map["InvestmentHolding"]["number_of_shares"].aliases == ( + "ZNUMBEROFSHARES", + ) + for model in ("InvestmentBuyTransaction", "InvestmentSellTransaction"): + assert profile.column_map[model]["number_of_shares"].aliases == ( + "ZNUMBEROFSHARES1", + ) -def test_detects_consumer_specific_mixed_share_columns() -> None: +def test_detects_observed_store_profile_from_entity_rows(tmp_path) -> None: + db_path = tmp_path / "moneywiz.sqlite" connection = make_connection( - "ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1" + db_path, "ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1" ) - add_investment_entities(connection) - insert_investment_row(connection, 24, ZNUMBEROFSHARES=2.0, ZNUMBEROFSHARES1=None) - insert_investment_row(connection, 40, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=9.0) - insert_investment_row(connection, 41, ZNUMBEROFSHARES=None, ZNUMBEROFSHARES1=3.0) + insert_investment_row( + connection, 24, ZNUMBEROFSHARES=None, ZPRICEPERSHARE=15.0, ZPRICEPERSHARE1=None + ) + for entity in (40, 41): + insert_investment_row( + connection, + entity, + ZNUMBEROFSHARES=2.0, + ZPRICEPERSHARE=None, + ZPRICEPERSHARE1=10.0, + ) - profile = detect_schema_profile(connection) + profile = resolve_profile(connection, db_path) - assert profile.profile_id == "mixed-investment-columns" - assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES" - assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES1" - assert profile.price_per_share_column == "ZPRICEPERSHARE1" + for model in ("InvestmentBuyTransaction", "InvestmentSellTransaction"): + assert profile.column_map[model]["number_of_shares"].aliases == ( + "ZNUMBEROFSHARES", + ) + assert profile.column_map[model]["price_per_share"].aliases == ( + "ZPRICEPERSHARE1", + ) -def test_detects_observed_store_profile_from_entity_rows() -> None: - connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1") - add_investment_entities(connection) - insert_investment_row( - connection, - 24, - ZNUMBEROFSHARES=None, - ZPRICEPERSHARE=15.0, - ZPRICEPERSHARE1=None, +@pytest.mark.parametrize("prices", [(10.0, 1.0), (None, None)]) +def test_rejects_dual_price_columns_without_discriminating_rows( + tmp_path, prices +) -> None: + db_path = tmp_path / "moneywiz.sqlite" + connection = make_connection( + db_path, "ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1" ) insert_investment_row( connection, 40, ZNUMBEROFSHARES=2.0, - ZPRICEPERSHARE=None, - ZPRICEPERSHARE1=10.0, + ZPRICEPERSHARE=prices[0], + ZPRICEPERSHARE1=prices[1], + ) + + with pytest.raises(UnsupportedInvestmentSchemaError, match="transaction columns"): + resolve_profile(connection, db_path) + + +@pytest.mark.parametrize("shares", [(2.0, 9.0), (None, None)]) +def test_rejects_ambiguous_holding_share_columns(tmp_path, shares) -> None: + db_path = tmp_path / "moneywiz.sqlite" + connection = make_connection( + db_path, "ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1" ) insert_investment_row( - connection, - 41, - ZNUMBEROFSHARES=3.0, - ZPRICEPERSHARE=None, - ZPRICEPERSHARE1=11.0, + connection, 24, ZNUMBEROFSHARES=shares[0], ZNUMBEROFSHARES1=shares[1] ) - profile = detect_schema_profile(connection) - - assert profile.profile_id == "mixed-investment-columns" - assert profile.holding_number_of_shares_column == "ZNUMBEROFSHARES" - assert profile.transaction_number_of_shares_column == "ZNUMBEROFSHARES" - assert profile.price_per_share_column == "ZPRICEPERSHARE1" + with pytest.raises(UnsupportedInvestmentSchemaError, match="holding shares column"): + resolve_profile(connection, db_path) -def test_rejects_dual_price_columns_without_discriminating_rows() -> None: - connection = make_connection("ZNUMBEROFSHARES", "ZPRICEPERSHARE", "ZPRICEPERSHARE1") - add_investment_entities(connection) - insert_investment_row( - connection, 40, ZNUMBEROFSHARES=2.0, ZPRICEPERSHARE=10.0, ZPRICEPERSHARE1=1.0 +def test_rejects_conflicting_transaction_column_evidence(tmp_path) -> None: + db_path = tmp_path / "moneywiz.sqlite" + connection = make_connection( + db_path, "ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE1" ) + insert_investment_row(connection, 40, ZNUMBEROFSHARES=2.0, ZPRICEPERSHARE1=10.0) + insert_investment_row(connection, 41, ZNUMBEROFSHARES1=3.0, ZPRICEPERSHARE1=11.0) - profile = detect_schema_profile(connection) + with pytest.raises(UnsupportedInvestmentSchemaError, match="transaction columns"): + resolve_profile(connection, db_path) - assert profile.profile_id == "unknown" - assert not profile.is_known +def test_rejects_missing_investment_columns(tmp_path) -> None: + db_path = tmp_path / "moneywiz.sqlite" + connection = make_connection(db_path, "ZNUMBEROFSHARES") + insert_investment_row(connection, 40, ZNUMBEROFSHARES=2.0) -def test_rejects_ambiguous_investment_column_profile() -> None: + with pytest.raises(UnsupportedInvestmentSchemaError, match="transaction columns"): + resolve_profile(connection, db_path) + + +def test_empty_investment_entities_do_not_require_row_evidence(tmp_path) -> None: + db_path = tmp_path / "moneywiz.sqlite" connection = make_connection( + db_path, "ZNUMBEROFSHARES", "ZNUMBEROFSHARES1", "ZPRICEPERSHARE", "ZPRICEPERSHARE1", ) - profile = detect_schema_profile(connection) + profile = resolve_profile(connection, db_path) - assert profile.profile_id == "unknown" - assert not profile.is_known + assert profile.column_map == DEFAULT_SCHEMA_PROFILE.column_map + assert profile.tag_table_info is not None -def test_decimal_alias_reads_both_profiles() -> None: - assert ( - RawDataHandler.get_decimal_alias( - {"ZNUMBEROFSHARES": 2.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" - ) - == 2.5 - ) - assert ( - RawDataHandler.get_decimal_alias( - {"ZNUMBEROFSHARES1": 3.5}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" - ) - == 3.5 - ) - assert ( - RawDataHandler.get_decimal_alias( - {"ZNUMBEROFSHARES1": None, "ZNUMBEROFSHARES": 4.5}, - "ZNUMBEROFSHARES1", - "ZNUMBEROFSHARES", - ) - == 4.5 - ) - assert ( - RawDataHandler.get_nullable_decimal_alias( - {"ZNUMBEROFSHARES": None}, "ZNUMBEROFSHARES1", "ZNUMBEROFSHARES" - ) - is None +@pytest.mark.parametrize("entity", [24, 40]) +def test_resolves_only_populated_investment_consumers(tmp_path, entity) -> None: + db_path = tmp_path / "moneywiz.sqlite" + connection = make_connection( + db_path, + "ZNUMBEROFSHARES", + "ZNUMBEROFSHARES1", + "ZPRICEPERSHARE", + "ZPRICEPERSHARE1", ) - assert ( - RawDataHandler.get_nullable_decimal_alias( - {"ZNUMBEROFSHARES1": None, "ZNUMBEROFSHARES": 5.5}, - "ZNUMBEROFSHARES1", - "ZNUMBEROFSHARES", + if entity == 24: + insert_investment_row(connection, entity, ZNUMBEROFSHARES=0.0) + else: + insert_investment_row( + connection, entity, ZNUMBEROFSHARES1=2.0, ZPRICEPERSHARE1=10.0 ) - == 5.5 + + profile = resolve_profile(connection, db_path) + + model = "InvestmentHolding" if entity == 24 else "InvestmentBuyTransaction" + column = "ZNUMBEROFSHARES" if entity == 24 else "ZNUMBEROFSHARES1" + assert profile.column_map[model]["number_of_shares"].aliases == (column,) + + +def test_resolves_investment_fields_with_preconfigured_tag_table(tmp_path) -> None: + db_path = tmp_path / "moneywiz.sqlite" + connection = make_connection(db_path, "ZNUMBEROFSHARES1", "ZPRICEPERSHARE") + insert_investment_row(connection, 40, ZNUMBEROFSHARES1=2.0, ZPRICEPERSHARE=10.0) + default = resolve_profile(connection, db_path) + baseline = SchemaProfile(DEFAULT_SCHEMA_PROFILE.column_map, default.tag_table_info) + + profile = SchemaProfileResolver(db_path, baseline).resolve() + + assert profile.tag_table_info == baseline.tag_table_info + assert profile.column_map["InvestmentBuyTransaction"][ + "number_of_shares" + ].aliases == ("ZNUMBEROFSHARES1",) + assert profile.column_map["InvestmentBuyTransaction"][ + "price_per_share" + ].converter is ( + DEFAULT_SCHEMA_PROFILE.column_map["InvestmentBuyTransaction"][ + "price_per_share" + ].converter ) + assert baseline.column_map == DEFAULT_SCHEMA_PROFILE.column_map def test_filter_row_accepts_missing_optional_blob_columns() -> None: diff --git a/tests/unit/test_transaction_compatibility.py b/tests/unit/test_transaction_compatibility.py index 907706b..25c5db5 100644 --- a/tests/unit/test_transaction_compatibility.py +++ b/tests/unit/test_transaction_compatibility.py @@ -1,7 +1,10 @@ from decimal import Decimal +from typing import cast import pytest +from moneywiz_api import DEFAULT_SCHEMA_PROFILE +from moneywiz_api.database_accessor import DatabaseAccessor from moneywiz_api.managers.record_manager import RecordManager from moneywiz_api.model.transaction import TransferWithdrawTransaction @@ -34,24 +37,27 @@ def transfer_withdraw_row(**overrides): def test_transfer_withdraw_rejects_zero_rate_reconstruction() -> None: with pytest.raises(ValueError, match="zero exchange rate"): - TransferWithdrawTransaction(transfer_withdraw_row()) + DEFAULT_SCHEMA_PROFILE.create_record( + transfer_withdraw_row(), TransferWithdrawTransaction + ) def test_transfer_withdraw_reconstructs_with_nonzero_rate() -> None: - transaction = TransferWithdrawTransaction( - transfer_withdraw_row(ZORIGINALEXCHANGERATE=1.0) + transaction = DEFAULT_SCHEMA_PROFILE.create_record( + transfer_withdraw_row(ZORIGINALEXCHANGERATE=1.0), TransferWithdrawTransaction ) assert transaction.original_amount == Decimal("-10.0") def test_transfer_withdraw_preserves_stored_amount_with_rounded_rate() -> None: - transaction = TransferWithdrawTransaction( + transaction = DEFAULT_SCHEMA_PROFILE.create_record( transfer_withdraw_row( ZAMOUNT1=-3.333, ZORIGINALRECIPIENTAMOUNT=10.0, ZORIGINALEXCHANGERATE=3.0, - ) + ), + TransferWithdrawTransaction, ) assert transaction.original_amount == Decimal("-3.333") @@ -64,6 +70,8 @@ def ents(self): class TransferWithdrawAccessor: + schema_profile = DEFAULT_SCHEMA_PROFILE + def __init__(self, rows): self.rows = rows @@ -77,15 +85,18 @@ def typename_for(self, _ent_id): def test_transfer_manager_skips_zero_rate_and_loads_later_valid_row() -> None: manager = TransferWithdrawManager() manager.load( - TransferWithdrawAccessor( - [ - transfer_withdraw_row(), - transfer_withdraw_row( - Z_PK=2, - ZGID="valid-transfer-withdraw", - ZORIGINALEXCHANGERATE=1.0, - ), - ] + cast( + DatabaseAccessor, + TransferWithdrawAccessor( + [ + transfer_withdraw_row(), + transfer_withdraw_row( + Z_PK=2, + ZGID="valid-transfer-withdraw", + ZORIGINALEXCHANGERATE=1.0, + ), + ] + ), ) ) From ce3f530a34bf214b0ffeaca08d63a1a487ad88f7 Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Wed, 14 Oct 2026 21:19:07 +0200 Subject: [PATCH 10/12] Fix duplicate record identity recovery --- src/moneywiz_api/managers/record_manager.py | 6 +- tests/unit/test_profiled_investment_values.py | 64 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/src/moneywiz_api/managers/record_manager.py b/src/moneywiz_api/managers/record_manager.py index 74b88b9..838a936 100644 --- a/src/moneywiz_api/managers/record_manager.py +++ b/src/moneywiz_api/managers/record_manager.py @@ -33,6 +33,7 @@ def load(self, db_accessor: DatabaseAccessor) -> None: record, self.ents[typename] ) obj.validate() + self.add(obj) except UnsupportedInvestmentSchemaError: raise except (AssertionError, KeyError, RuntimeError, ValueError) as exc: @@ -46,15 +47,16 @@ def load(self, db_accessor: DatabaseAccessor) -> None: detail, ) continue - self.add(obj) def add(self, record: T) -> None: - self._records[record.id] = record + if record.id in self._records: + raise RuntimeError(f"Duplicate id {record.id}") if record.gid in self._gid_to_id: raise RuntimeError( f"Duplicate gid for {record}, existing record Id {self._gid_to_id[record.gid]}" ) + self._records[record.id] = record self._gid_to_id[record.gid] = record.id def get(self, record_id: ID) -> T | None: diff --git a/tests/unit/test_profiled_investment_values.py b/tests/unit/test_profiled_investment_values.py index 2f1af76..1583b9e 100644 --- a/tests/unit/test_profiled_investment_values.py +++ b/tests/unit/test_profiled_investment_values.py @@ -276,6 +276,70 @@ def test_managers_report_bad_rows_and_load_later_valid_rows(malformation) -> Non assert len(manager.load_errors) == 1 +@pytest.mark.parametrize("identity", ["gid", "id", "both"]) +def test_manager_duplicate_refusal_preserves_existing_records(identity) -> None: + manager = TransactionManager() + row = investment_transaction_row(40, -20.0) + first = UNSUFFIXED_PROFILE.create_record(row, InvestmentBuyTransaction) + manager.add(first) + duplicate_row = { + **row, + "Z_PK": 41 if identity == "gid" else 40, + "ZGID": "duplicate" if identity == "id" else row["ZGID"], + } + duplicate = UNSUFFIXED_PROFILE.create_record( + duplicate_row, InvestmentBuyTransaction + ) + + with pytest.raises(RuntimeError, match="Duplicate"): + manager.add(duplicate) + + assert list(manager.records()) == [40] + assert manager.get(40) is first + assert manager.get_by_gid(row["ZGID"]) is first + assert manager.get_by_gid("duplicate") is None + assert manager.load_errors == [] + + +@pytest.mark.parametrize("identity", ["gid", "id", "both"]) +def test_manager_reports_duplicates_and_loads_later_rows(tmp_path, identity) -> None: + first_row = investment_transaction_row(40, -20.0) + first_row["ZNUMBEROFSHARES1"] = None + first_row["ZPRICEPERSHARE1"] = None + duplicate_row = { + **first_row, + "Z_PK": 41 if identity == "gid" else 40, + "ZGID": "duplicate" if identity == "id" else first_row["ZGID"], + "ZAMOUNT1": -40.0, + "ZNUMBEROFSHARES": 4.0, + } + later_row = {**first_row, "Z_PK": 42, "ZGID": "later"} + db_path = tmp_path / "moneywiz.sqlite" + create_investment_database(db_path, [first_row, duplicate_row, later_row]) + profile = SchemaProfileResolver(db_path, UNSUFFIXED_PROFILE).resolve() + accessor = DatabaseAccessor(db_path, profile) + manager = TransactionManager() + + try: + manager.load(accessor) + + assert list(manager.records()) == [40, 42] + first = manager.get(40) + assert first is not None + assert manager.get_by_gid(first_row["ZGID"]) is first + assert first.amount == Decimal("-20.0") + assert manager.get_by_gid("later") is manager.get(42) + assert manager.get_by_gid("duplicate") is None + assert manager.load_errors == [ + (duplicate_row["Z_PK"], "InvestmentBuyTransaction", "RuntimeError") + ] + assert ( + manager.category_assignment == manager.refund_maps == manager.tags_map == {} + ) + finally: + accessor._con.close() + + def test_ambiguous_store_schema_stops_before_record_loading(tmp_path) -> None: db_path = tmp_path / "moneywiz.sqlite" create_investment_database(db_path, [investment_transaction_row(40, -20.0)]) From ddfece8a3b85de2db2f934421df76e58868b7377 Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Wed, 14 Oct 2026 21:27:53 +0200 Subject: [PATCH 11/12] Reject zero-rate transfer withdrawals before reconstruction --- src/moneywiz_api/model/transaction.py | 6 ++-- tests/unit/test_transaction_compatibility.py | 32 ++++++++++++++++---- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/moneywiz_api/model/transaction.py b/src/moneywiz_api/model/transaction.py index 2a8f7b7..aa90e99 100644 --- a/src/moneywiz_api/model/transaction.py +++ b/src/moneywiz_api/model/transaction.py @@ -336,6 +336,8 @@ def __init__(self, row, field_values): super().__init__(row, field_values) # Fixes + if self.original_exchange_rate == 0: + raise ValueError("cannot read a transfer with a zero exchange rate") if ( self.recipient_amount in (None, Decimal(0)) and self.original_amount is not None @@ -348,10 +350,6 @@ def __init__(self, row, field_values): None, Decimal(0), ): - if self.original_exchange_rate == 0: - raise ValueError( - "cannot reconstruct a transfer amount with a zero exchange rate" - ) self.original_amount = self.amount def validate(self) -> None: diff --git a/tests/unit/test_transaction_compatibility.py b/tests/unit/test_transaction_compatibility.py index 25c5db5..960f215 100644 --- a/tests/unit/test_transaction_compatibility.py +++ b/tests/unit/test_transaction_compatibility.py @@ -35,17 +35,29 @@ def transfer_withdraw_row(**overrides): return row -def test_transfer_withdraw_rejects_zero_rate_reconstruction() -> None: +@pytest.mark.parametrize( + ("original_amount", "recipient_amount"), [(0.0, 10.0), (-10.0, 10.0), (-10.0, 0.0)] +) +def test_transfer_withdraw_rejects_zero_rate(original_amount, recipient_amount) -> None: with pytest.raises(ValueError, match="zero exchange rate"): DEFAULT_SCHEMA_PROFILE.create_record( - transfer_withdraw_row(), TransferWithdrawTransaction + transfer_withdraw_row( + ZORIGINALAMOUNT=original_amount, + ZORIGINALRECIPIENTAMOUNT=recipient_amount, + ), + TransferWithdrawTransaction, ) -def test_transfer_withdraw_reconstructs_with_nonzero_rate() -> None: +@pytest.mark.parametrize("original_amount", [0.0, -10.0]) +def test_transfer_withdraw_preserves_amount_with_nonzero_rate(original_amount) -> None: transaction = DEFAULT_SCHEMA_PROFILE.create_record( - transfer_withdraw_row(ZORIGINALEXCHANGERATE=1.0), TransferWithdrawTransaction + transfer_withdraw_row( + ZORIGINALAMOUNT=original_amount, ZORIGINALEXCHANGERATE=1.0 + ), + TransferWithdrawTransaction, ) + transaction.validate() assert transaction.original_amount == Decimal("-10.0") @@ -82,14 +94,22 @@ def typename_for(self, _ent_id): return "TransferWithdrawTransaction" -def test_transfer_manager_skips_zero_rate_and_loads_later_valid_row() -> None: +@pytest.mark.parametrize( + ("original_amount", "recipient_amount"), [(0.0, 10.0), (-10.0, 10.0), (-10.0, 0.0)] +) +def test_transfer_manager_skips_zero_rate_and_loads_later_valid_row( + original_amount, recipient_amount +) -> None: manager = TransferWithdrawManager() manager.load( cast( DatabaseAccessor, TransferWithdrawAccessor( [ - transfer_withdraw_row(), + transfer_withdraw_row( + ZORIGINALAMOUNT=original_amount, + ZORIGINALRECIPIENTAMOUNT=recipient_amount, + ), transfer_withdraw_row( Z_PK=2, ZGID="valid-transfer-withdraw", From c7954306b49765f0a92d2833315d3ae4f9b30b10 Mon Sep 17 00:00:00 2001 From: MarcoMC Date: Thu, 15 Oct 2026 12:27:11 +0200 Subject: [PATCH 12/12] docs: enhance schema resolution and record identity admission guidelines --- AGENTS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 9c3ff70..b7fc8fc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,6 +23,10 @@ Use four-space indentation, double-quoted strings, and Ruff's 88-character line Use pytest and favor focused unit tests with row dictionaries or temporary SQLite databases. Integration tests are opt-in: set `MONEYWIZ_TEST_DB_PATH` to an absolute path to a disposable MoneyWiz database, then run `.venv/bin/python -m pytest tests`. They skip when the variable is unset. Do not use a live personal database. CI runs unit tests, Ruff, and mypy; no minimum coverage threshold is configured. +When automatic schema resolution is added or migrated, preserve caller-supplied aliases and converters, resolve inherited effective specifications rather than own-class defaults, and cover explicit aliases, inherited fields, partial consumer customization, and record construction. Restrict automatic row-evidence resolution to consumers that require it. + +Treat record identity admission as part of recoverable row processing: preflight every ID and GID collision before changing either lookup map, then perform insertion inside the row-recovery boundary. Cover duplicate identities, unchanged first-record lookups, and later valid-row recovery. + ## Commit & Pull Request Guidelines Recent commits use short subjects such as `Fix schema profile validation and record construction`; capitalization and prefixes vary. Write a brief imperative subject that names the change. In pull requests, explain behavior changes, list the checks run, and link a relevant issue when one exists. Include a CLI example when changing user-visible shell behavior.