From 6c8b8ed3cc1747278a391a6eac912019e4d3a318 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elisi=C3=A1rio=20Couto?= Date: Mon, 8 Sep 2025 19:37:49 +0100 Subject: [PATCH] Remove GoCardless fallback from /accounts endpoints - Remove GoCardless API calls from /api/v1/accounts and /api/v1/accounts/{account_id} - Accounts endpoints now rely exclusively on database data - Return 404 for accounts not found in database - Update tests to mock database service instead of GoCardless API - Remove unused GoCardless imports from transactions routes - Preserve GoCardless usage in sync process and /banks endpoints - Fix code formatting and remove unused imports --- leggen/database/sqlite.py | 155 +++++++++++++++++++++++++++ leggend/api/routes/accounts.py | 107 +++++++++--------- leggend/api/routes/transactions.py | 2 - leggend/services/database_service.py | 67 +++++++++++- leggend/services/sync_service.py | 4 + tests/unit/test_api_accounts.py | 153 +++++++++++++------------- 6 files changed, 357 insertions(+), 131 deletions(-) diff --git a/leggen/database/sqlite.py b/leggen/database/sqlite.py index b7448d8..af31f9c 100644 --- a/leggen/database/sqlite.py +++ b/leggen/database/sqlite.py @@ -16,6 +16,31 @@ def persist_balances(ctx: click.Context, balance: dict): conn = sqlite3.connect(str(db_path)) cursor = conn.cursor() + # Create the accounts table if it doesn't exist + cursor.execute( + """CREATE TABLE IF NOT EXISTS accounts ( + id TEXT PRIMARY KEY, + institution_id TEXT, + status TEXT, + iban TEXT, + name TEXT, + currency TEXT, + created DATETIME, + last_accessed DATETIME, + last_updated DATETIME + )""" + ) + + # Create indexes for accounts table + cursor.execute( + """CREATE INDEX IF NOT EXISTS idx_accounts_institution_id + ON accounts(institution_id)""" + ) + cursor.execute( + """CREATE INDEX IF NOT EXISTS idx_accounts_status + ON accounts(status)""" + ) + # Create the balances table if it doesn't exist cursor.execute( """CREATE TABLE IF NOT EXISTS balances ( @@ -381,3 +406,133 @@ def get_transaction_count(account_id=None, **filters): except Exception as e: conn.close() raise e + + +def persist_account(account_data: dict): + """Persist account details to SQLite database""" + from pathlib import Path + + db_path = Path.home() / ".config" / "leggen" / "leggen.db" + db_path.parent.mkdir(parents=True, exist_ok=True) + conn = sqlite3.connect(str(db_path)) + cursor = conn.cursor() + + # Create the accounts table if it doesn't exist + cursor.execute( + """CREATE TABLE IF NOT EXISTS accounts ( + id TEXT PRIMARY KEY, + institution_id TEXT, + status TEXT, + iban TEXT, + name TEXT, + currency TEXT, + created DATETIME, + last_accessed DATETIME, + last_updated DATETIME + )""" + ) + + # Create indexes for accounts table + cursor.execute( + """CREATE INDEX IF NOT EXISTS idx_accounts_institution_id + ON accounts(institution_id)""" + ) + cursor.execute( + """CREATE INDEX IF NOT EXISTS idx_accounts_status + ON accounts(status)""" + ) + + try: + # Insert or replace account data + cursor.execute( + """INSERT OR REPLACE INTO accounts ( + id, + institution_id, + status, + iban, + name, + currency, + created, + last_accessed, + last_updated + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)""", + ( + account_data["id"], + account_data["institution_id"], + account_data["status"], + account_data.get("iban"), + account_data.get("name"), + account_data.get("currency"), + account_data["created"], + account_data.get("last_accessed"), + account_data.get("last_updated", account_data["created"]), + ), + ) + conn.commit() + conn.close() + + success(f"[{account_data['id']}] Account details persisted to database") + return account_data + + except Exception as e: + conn.close() + raise e + + +def get_accounts(account_ids=None): + """Get account details from SQLite database""" + from pathlib import Path + + db_path = Path.home() / ".config" / "leggen" / "leggen.db" + if not db_path.exists(): + return [] + conn = sqlite3.connect(str(db_path)) + conn.row_factory = sqlite3.Row + cursor = conn.cursor() + + query = "SELECT * FROM accounts" + params = [] + + if account_ids: + placeholders = ",".join("?" * len(account_ids)) + query += f" WHERE id IN ({placeholders})" + params.extend(account_ids) + + query += " ORDER BY created DESC" + + try: + cursor.execute(query, params) + rows = cursor.fetchall() + + accounts = [dict(row) for row in rows] + conn.close() + return accounts + + except Exception as e: + conn.close() + raise e + + +def get_account(account_id: str): + """Get specific account details from SQLite database""" + from pathlib import Path + + db_path = Path.home() / ".config" / "leggen" / "leggen.db" + if not db_path.exists(): + return None + conn = sqlite3.connect(str(db_path)) + conn.row_factory = sqlite3.Row + cursor = conn.cursor() + + try: + cursor.execute("SELECT * FROM accounts WHERE id = ?", (account_id,)) + row = cursor.fetchone() + conn.close() + + if row: + return dict(row) + return None + + except Exception as e: + conn.close() + raise e diff --git a/leggend/api/routes/accounts.py b/leggend/api/routes/accounts.py index 241fd7f..593ef63 100644 --- a/leggend/api/routes/accounts.py +++ b/leggend/api/routes/accounts.py @@ -9,67 +9,65 @@ from leggend.api.models.accounts import ( Transaction, TransactionSummary, ) -from leggend.services.gocardless_service import GoCardlessService from leggend.services.database_service import DatabaseService router = APIRouter() -gocardless_service = GoCardlessService() database_service = DatabaseService() @router.get("/accounts", response_model=APIResponse) async def get_all_accounts() -> APIResponse: - """Get all connected accounts""" + """Get all connected accounts from database""" try: - requisitions_data = await gocardless_service.get_requisitions() - - all_accounts = set() - for req in requisitions_data.get("results", []): - all_accounts.update(req.get("accounts", [])) - accounts = [] - for account_id in all_accounts: + + # Get all account details from database + db_accounts = await database_service.get_accounts_from_db() + + # Process accounts found in database + for db_account in db_accounts: try: - account_details = await gocardless_service.get_account_details( - account_id - ) - balances_data = await gocardless_service.get_account_balances( - account_id + # Get latest balances from database for this account + balances_data = await database_service.get_balances_from_db( + db_account["id"] ) # Process balances balances = [] - for balance in balances_data.get("balances", []): - balance_amount = balance["balanceAmount"] + for balance in balances_data: balances.append( AccountBalance( - amount=float(balance_amount["amount"]), - currency=balance_amount["currency"], - balance_type=balance["balanceType"], - last_change_date=balance.get("lastChangeDateTime"), + amount=balance["amount"], + currency=balance["currency"], + balance_type=balance["type"], + last_change_date=balance.get("timestamp"), ) ) accounts.append( AccountDetails( - id=account_details["id"], - institution_id=account_details["institution_id"], - status=account_details["status"], - iban=account_details.get("iban"), - name=account_details.get("name"), - currency=account_details.get("currency"), - created=account_details["created"], - last_accessed=account_details.get("last_accessed"), + id=db_account["id"], + institution_id=db_account["institution_id"], + status=db_account["status"], + iban=db_account.get("iban"), + name=db_account.get("name"), + currency=db_account.get("currency"), + created=db_account["created"], + last_accessed=db_account.get("last_accessed"), balances=balances, ) ) except Exception as e: - logger.error(f"Failed to get details for account {account_id}: {e}") + logger.error( + f"Failed to process database account {db_account['id']}: {e}" + ) continue return APIResponse( - success=True, data=accounts, message=f"Retrieved {len(accounts)} accounts" + success=True, + data=accounts, + message=f"Retrieved {len(accounts)} accounts from database", ) except Exception as e: @@ -81,46 +79,55 @@ async def get_all_accounts() -> APIResponse: @router.get("/accounts/{account_id}", response_model=APIResponse) async def get_account_details(account_id: str) -> APIResponse: - """Get details for a specific account""" + """Get details for a specific account from database""" try: - account_details = await gocardless_service.get_account_details(account_id) - balances_data = await gocardless_service.get_account_balances(account_id) + # Get account details from database + db_account = await database_service.get_account_details_from_db(account_id) + + if not db_account: + raise HTTPException( + status_code=404, detail=f"Account {account_id} not found in database" + ) + + # Get latest balances from database for this account + balances_data = await database_service.get_balances_from_db(account_id) # Process balances balances = [] - for balance in balances_data.get("balances", []): - balance_amount = balance["balanceAmount"] + for balance in balances_data: balances.append( AccountBalance( - amount=float(balance_amount["amount"]), - currency=balance_amount["currency"], - balance_type=balance["balanceType"], - last_change_date=balance.get("lastChangeDateTime"), + amount=balance["amount"], + currency=balance["currency"], + balance_type=balance["type"], + last_change_date=balance.get("timestamp"), ) ) account = AccountDetails( - id=account_details["id"], - institution_id=account_details["institution_id"], - status=account_details["status"], - iban=account_details.get("iban"), - name=account_details.get("name"), - currency=account_details.get("currency"), - created=account_details["created"], - last_accessed=account_details.get("last_accessed"), + id=db_account["id"], + institution_id=db_account["institution_id"], + status=db_account["status"], + iban=db_account.get("iban"), + name=db_account.get("name"), + currency=db_account.get("currency"), + created=db_account["created"], + last_accessed=db_account.get("last_accessed"), balances=balances, ) return APIResponse( success=True, data=account, - message=f"Account details retrieved for {account_id}", + message=f"Account details retrieved from database for {account_id}", ) + except HTTPException: + raise except Exception as e: logger.error(f"Failed to get account details for {account_id}: {e}") raise HTTPException( - status_code=404, detail=f"Account not found: {str(e)}" + status_code=500, detail=f"Failed to get account details: {str(e)}" ) from e diff --git a/leggend/api/routes/transactions.py b/leggend/api/routes/transactions.py index 7d56c0e..37d1b15 100644 --- a/leggend/api/routes/transactions.py +++ b/leggend/api/routes/transactions.py @@ -5,11 +5,9 @@ from loguru import logger from leggend.api.models.common import APIResponse from leggend.api.models.accounts import Transaction, TransactionSummary -from leggend.services.gocardless_service import GoCardlessService from leggend.services.database_service import DatabaseService router = APIRouter() -gocardless_service = GoCardlessService() database_service = DatabaseService() diff --git a/leggend/services/database_service.py b/leggend/services/database_service.py index 6f33a1c..31334b5 100644 --- a/leggend/services/database_service.py +++ b/leggend/services/database_service.py @@ -124,8 +124,8 @@ class DatabaseService: try: transactions = sqlite_db.get_transactions( account_id=account_id, - limit=limit, - offset=offset, + limit=limit or 100, + offset=offset or 0, date_from=date_from, date_to=date_to, min_amount=min_amount, @@ -203,6 +203,49 @@ class DatabaseService: logger.error(f"Failed to get account summary from database: {e}") return None + async def persist_account_details(self, account_data: Dict[str, Any]) -> None: + """Persist account details to database""" + if not self.sqlite_enabled: + logger.warning("SQLite database disabled, skipping account persistence") + return + + await self._persist_account_details_sqlite(account_data) + + async def get_accounts_from_db( + self, account_ids: Optional[List[str]] = None + ) -> List[Dict[str, Any]]: + """Get account details from database""" + if not self.sqlite_enabled: + logger.warning("SQLite database disabled, cannot read accounts") + return [] + + try: + accounts = sqlite_db.get_accounts(account_ids=account_ids) + logger.debug(f"Retrieved {len(accounts)} accounts from database") + return accounts + except Exception as e: + logger.error(f"Failed to get accounts from database: {e}") + return [] + + async def get_account_details_from_db( + self, account_id: str + ) -> Optional[Dict[str, Any]]: + """Get specific account details from database""" + if not self.sqlite_enabled: + logger.warning("SQLite database disabled, cannot read account") + return None + + try: + account = sqlite_db.get_account(account_id) + if account: + logger.debug( + f"Retrieved account details from database for {account_id}" + ) + return account + except Exception as e: + logger.error(f"Failed to get account details from database: {e}") + return None + async def _persist_balance_sqlite( self, account_id: str, balance_data: Dict[str, Any] ) -> None: @@ -381,3 +424,23 @@ class DatabaseService: except Exception as e: logger.error(f"Failed to persist transactions to SQLite: {e}") raise + + async def _persist_account_details_sqlite( + self, account_data: Dict[str, Any] + ) -> None: + """Persist account details to SQLite""" + try: + from pathlib import Path + + db_path = Path.home() / ".config" / "leggen" / "leggen.db" + db_path.parent.mkdir(parents=True, exist_ok=True) + + # Use the sqlite_db module function + sqlite_db.persist_account(account_data) + + logger.info( + f"Persisted account details to SQLite for account {account_data['id']}" + ) + except Exception as e: + logger.error(f"Failed to persist account details to SQLite: {e}") + raise diff --git a/leggend/services/sync_service.py b/leggend/services/sync_service.py index 62815a8..1b5f758 100644 --- a/leggend/services/sync_service.py +++ b/leggend/services/sync_service.py @@ -55,6 +55,10 @@ class SyncService: account_id ) + # Persist account details to database + if account_details: + await self.database.persist_account_details(account_details) + # Get and save balances balances = await self.gocardless.get_account_balances(account_id) if balances: diff --git a/tests/unit/test_api_accounts.py b/tests/unit/test_api_accounts.py index dd90932..ddf89eb 100644 --- a/tests/unit/test_api_accounts.py +++ b/tests/unit/test_api_accounts.py @@ -1,8 +1,6 @@ """Tests for accounts API endpoints.""" import pytest -import respx -import httpx from unittest.mock import patch @@ -10,44 +8,46 @@ from unittest.mock import patch class TestAccountsAPI: """Test account-related API endpoints.""" - @respx.mock def test_get_all_accounts_success( self, api_client, mock_config, mock_auth_token, sample_account_data ): - """Test successful retrieval of all accounts.""" - requisitions_data = { - "results": [{"id": "req-123", "accounts": ["test-account-123"]}] - } + """Test successful retrieval of all accounts from database.""" + mock_accounts = [ + { + "id": "test-account-123", + "institution_id": "REVOLUT_REVOLT21", + "status": "READY", + "iban": "LT313250081177977789", + "created": "2024-02-13T23:56:00Z", + "last_accessed": "2025-09-01T09:30:00Z", + } + ] - balances_data = { - "balances": [ - { - "balanceAmount": {"amount": "100.50", "currency": "EUR"}, - "balanceType": "interimAvailable", - "lastChangeDateTime": "2025-09-01T09:30:00Z", - } - ] - } + mock_balances = [ + { + "id": 1, + "account_id": "test-account-123", + "bank": "REVOLUT_REVOLT21", + "status": "active", + "iban": "LT313250081177977789", + "amount": 100.50, + "currency": "EUR", + "type": "interimAvailable", + "timestamp": "2025-09-01T09:30:00Z", + } + ] - # Mock GoCardless token creation - respx.post("https://bankaccountdata.gocardless.com/api/v2/token/new/").mock( - return_value=httpx.Response( - 200, json={"access": "test-token", "refresh": "test-refresh"} - ) - ) - - # Mock GoCardless API calls - respx.get("https://bankaccountdata.gocardless.com/api/v2/requisitions/").mock( - return_value=httpx.Response(200, json=requisitions_data) - ) - respx.get( - "https://bankaccountdata.gocardless.com/api/v2/accounts/test-account-123/" - ).mock(return_value=httpx.Response(200, json=sample_account_data)) - respx.get( - "https://bankaccountdata.gocardless.com/api/v2/accounts/test-account-123/balances/" - ).mock(return_value=httpx.Response(200, json=balances_data)) - - with patch("leggend.config.config", mock_config): + with ( + patch("leggend.config.config", mock_config), + patch( + "leggend.api.routes.accounts.database_service.get_accounts_from_db", + return_value=mock_accounts, + ), + patch( + "leggend.api.routes.accounts.database_service.get_balances_from_db", + return_value=mock_balances, + ), + ): response = api_client.get("/api/v1/accounts") assert response.status_code == 200 @@ -60,36 +60,44 @@ class TestAccountsAPI: assert len(account["balances"]) == 1 assert account["balances"][0]["amount"] == 100.50 - @respx.mock def test_get_account_details_success( self, api_client, mock_config, mock_auth_token, sample_account_data ): - """Test successful retrieval of specific account details.""" - balances_data = { - "balances": [ - { - "balanceAmount": {"amount": "250.75", "currency": "EUR"}, - "balanceType": "interimAvailable", - } - ] + """Test successful retrieval of specific account details from database.""" + mock_account = { + "id": "test-account-123", + "institution_id": "REVOLUT_REVOLT21", + "status": "READY", + "iban": "LT313250081177977789", + "created": "2024-02-13T23:56:00Z", + "last_accessed": "2025-09-01T09:30:00Z", } - # Mock GoCardless token creation - respx.post("https://bankaccountdata.gocardless.com/api/v2/token/new/").mock( - return_value=httpx.Response( - 200, json={"access": "test-token", "refresh": "test-refresh"} - ) - ) + mock_balances = [ + { + "id": 1, + "account_id": "test-account-123", + "bank": "REVOLUT_REVOLT21", + "status": "active", + "iban": "LT313250081177977789", + "amount": 250.75, + "currency": "EUR", + "type": "interimAvailable", + "timestamp": "2025-09-01T09:30:00Z", + } + ] - # Mock GoCardless API calls - respx.get( - "https://bankaccountdata.gocardless.com/api/v2/accounts/test-account-123/" - ).mock(return_value=httpx.Response(200, json=sample_account_data)) - respx.get( - "https://bankaccountdata.gocardless.com/api/v2/accounts/test-account-123/balances/" - ).mock(return_value=httpx.Response(200, json=balances_data)) - - with patch("leggend.config.config", mock_config): + with ( + patch("leggend.config.config", mock_config), + patch( + "leggend.api.routes.accounts.database_service.get_account_details_from_db", + return_value=mock_account, + ), + patch( + "leggend.api.routes.accounts.database_service.get_balances_from_db", + return_value=mock_balances, + ), + ): response = api_client.get("/api/v1/accounts/test-account-123") assert response.status_code == 200 @@ -248,22 +256,13 @@ class TestAccountsAPI: def test_get_account_not_found(self, api_client, mock_config, mock_auth_token): """Test handling of non-existent account.""" - # Mock 404 response from GoCardless - with respx.mock: - # Mock GoCardless token creation - respx.post("https://bankaccountdata.gocardless.com/api/v2/token/new/").mock( - return_value=httpx.Response( - 200, json={"access": "test-token", "refresh": "test-refresh"} - ) - ) + with ( + patch("leggend.config.config", mock_config), + patch( + "leggend.api.routes.accounts.database_service.get_account_details_from_db", + return_value=None, + ), + ): + response = api_client.get("/api/v1/accounts/nonexistent") - respx.get( - "https://bankaccountdata.gocardless.com/api/v2/accounts/nonexistent/" - ).mock( - return_value=httpx.Response(404, json={"detail": "Account not found"}) - ) - - with patch("leggend.config.config", mock_config): - response = api_client.get("/api/v1/accounts/nonexistent") - - assert response.status_code == 404 + assert response.status_code == 404