Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions application/account_facts_readonly.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,14 @@ def collect_firstrade_account_facts(
raise AccountFactsUnavailable("observation_time_invalid") from None
try:
accounts = client.account_numbers()
if (
not isinstance(accounts, list)
or any(not isinstance(account, str) or not account for account in accounts)
or accounts.count(expected) != 1
if not isinstance(accounts, list) or any(
not isinstance(account, str) or not account for account in accounts
):
raise AccountFactsUnavailable("account_identity_mismatch")
if not accounts:
raise AccountFactsUnavailable("account_list_empty")
if accounts.count(expected) != 1:
raise AccountFactsUnavailable("account_identity_mismatch")
selected_account = client.select_account(expected)
if selected_account != expected:
raise AccountFactsUnavailable("account_identity_mismatch")
Expand Down
38 changes: 33 additions & 5 deletions application/firstrade_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,24 @@ def mask_account_id(account_id: str) -> str:
return "*" * len(value)
return f"{'*' * max(0, len(value) - 4)}{value[-4:]}"

def normalize_account_number(value: object) -> str | None:
"""Coerce Firstrade account identifiers to comparable non-empty strings."""
if value is None or isinstance(value, bool):
return None
if isinstance(value, int):
text = str(value)
elif isinstance(value, float):
if not value.is_integer():
return None
text = str(int(value))
elif isinstance(value, str):
text = value.strip()
else:
return None
return text or None




def _coerce_positive_float(value: float | None, field: str) -> float | None:
if value is None:
Expand Down Expand Up @@ -472,16 +490,22 @@ def require_connected(self) -> tuple[Any, Any]:

def account_numbers(self) -> list[str]:
_, account_data = self.require_connected()
return list(getattr(account_data, "account_numbers", []))
normalized: list[str] = []
for item in list(getattr(account_data, "account_numbers", []) or []):
account = normalize_account_number(item)
if account is not None:
normalized.append(account)
return normalized

def select_account(self, requested_account: str | None = None) -> str:
accounts = self.account_numbers()
if requested_account:
if requested_account not in accounts:
wanted = normalize_account_number(requested_account)
if wanted is None or wanted not in accounts:
raise FirstradePlatformError(
f"Requested account {mask_account_id(requested_account)} was not returned by Firstrade."
f"Requested account {mask_account_id(str(requested_account or ''))} was not returned by Firstrade."
)
return requested_account
return wanted
if len(accounts) == 1:
return accounts[0]
if not accounts:
Expand All @@ -504,7 +528,11 @@ def list_account_summaries(self) -> list[dict[str, Any]]:
def get_balances(self, account: str) -> dict[str, Any]:
_, account_data = self.require_connected()
balances = dict(account_data.get_account_balances(account))
account_balances = dict(getattr(account_data, "account_balances", {}) or {})
account_balances = {
key: value
for raw_key, value in dict(getattr(account_data, "account_balances", {}) or {}).items()
if (key := normalize_account_number(raw_key)) is not None
}
account_list_total_value = account_balances.get(account)
if account_list_total_value is not None and "account_list_total_value" not in balances:
balances["account_list_total_value"] = account_list_total_value
Expand Down
6 changes: 5 additions & 1 deletion main.py
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,11 @@ def account_facts_sync():
account_scope=str(runtime_target.account_scope),
)
except AccountFactsUnavailable as exc:
app.logger.warning("account_facts_sync collect failed reason=%s", exc.reason_code)
app.logger.warning(
"account_facts_sync collect failed reason=%s expected=%s",
exc.reason_code,
mask_account_id(config.account_id or ""),
)
return jsonify({"ok": False, "error": exc.reason_code}), 503
except AccountFactsPublishError as exc:
app.logger.warning("account_facts_sync build failed reason=%s", exc.reason_code)
Expand Down
21 changes: 21 additions & 0 deletions tests/test_firstrade_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,3 +536,24 @@ def env(name, default=None):
assert credentials.username == "synthetic-user"
assert credentials.password == credentials.mfa_secret == credentials.mfa_code == ""
assert reads == ["firstrade-username"]


def test_account_numbers_coerces_int_ids_for_selector_match():
class IntAccountData(FakeAccountData):
account_numbers = [12345678]
account_balances = {12345678: "100.00"}

def get_account_balances(self, account):
return {"account": account, "total_value": "100.00"}

credentials = FirstradeCredentials(username="user", password="pass")
client = FirstradeBrokerClient(
credentials,
session_factory=FakeSession,
account_data_factory=IntAccountData,
order_factory=FakeOrder,
).connect()
assert client.account_numbers() == ["12345678"]
assert client.select_account("12345678") == "12345678"
balances = client.get_balances("12345678")
assert balances["account_list_total_value"] == "100.00"
Loading